Date: Tue, 21 Oct 2014 18:11:57 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Adam Nowacki <nowakpl@platinum.linux.pl> Cc: freebsd-hackers@freebsd.org Subject: Re: panic in ivy_rng_store() when compiled with -O0 Message-ID: <20141021151157.GC1877@kib.kiev.ua> In-Reply-To: <5443B2D4.3020407@platinum.linux.pl> References: <54384ABD.5080806@FreeBSD.org> <2533199.DHZybpy49d@ralph.baldwin.cx> <54415E13.4000203@delphij.net> <7A5C3D84-1B1F-4BA3-818D-37231BF424FE@FreeBSD.org> <5443B2D4.3020407@platinum.linux.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Oct 19, 2014 at 02:47:16PM +0200, Adam Nowacki wrote: > On 2014-10-17 20:33, Dimitry Andric wrote: > > On 17 Oct 2014, at 20:21, Xin Li <delphij@delphij.net> wrote: > >> On 10/17/14 08:53, John Baldwin wrote: > >>> On Friday, October 10, 2014 02:08:13 PM Navdeep Parhar wrote: > > ... > >> movq %rdi,(%rdi) is obviously wrong (%rdi holds the result from > >> rdrand), which I believed to be a compiler bug in register allocation. > >> > >> Navdeep have committed a change to mark 'tmp' input+output, which does > >> fix the output but I'm not 100% sure if that's right, as 'tmp' is not > >> considered an input of the inline assembler block, and this may break > >> compile on other compilers, but for now it's better than previous > >> situation. > >> > >> Speaking for the compiler issue, Dimitry have reported this upstream at: > >> > >> http://llvm.org/bugs/show_bug.cgi?id=21273 > >> > >> There is a suggestion in the reply, that change 'tmp' to early clobber > >> would workaround the issue, like: > >> > >> Index: ivy.c > >> =================================================================== > >> - --- ivy.c (revision 273195) > >> +++ ivy.c (working copy) > >> @@ -79,7 +79,7 @@ > >> "2:\n\t" > >> "mov %2,%1\n\t" /* *buf = tmp */ > >> "3:" > >> - - : "+q" (retry), "=m" (*buf), "+q" (tmp) : : "cc"); > >> + : "+q" (retry), "=m" (*buf), "=&q" (tmp) : : "cc"); > > > > Yes, this generates the correct code for all cases I tried, e.g. with > > gcc 4.2 from base, gcc 4.7 through 5.0 from ports, clang 3.4, clang 3.5 > > and clang trunk, all at -O0 through -O3, and -Os. > > > > So at the moment this fix is the best option, I think. > > GCC documentation explains what happens: > "The same problem can occur if one output parameter (a) allows a > register constraint and another output parameter (b) allows a memory > constraint. The code generated by GCC to access the memory address in b > can contain registers which might be shared by a, and GCC considers > those registers to be inputs to the asm. As above, GCC assumes that such > input registers are consumed before any outputs are written. This > assumption may result in incorrect behavior if the asm writes to a > before using b. Combining the `&' constraint with the register > constraint ensures that modifying a will not affect what address is > referenced by b. Omitting the `&' constraint means that the location of > b will be undefined if a is modified before using b. " > > Both tmp and retry need the '&' constraint. > > : "+&q" (retry), "=m" (*buf), "=&q" (tmp) : : "cc"); The inline asm documentation was extensively rewritten for gcc 5.0, it seems. Thank you for the pointer, indeed, the situation in ivy_rng_store() exactly matches the paragraph you cite. Should the '&' modifier be applied for register output operands only ? In particular, I wonder if *buf operand should be fixed the same way. The gcc doc does not say explicitely which operand, (a) or (b), should be marked as earlyclobber at all. diff --git a/sys/dev/random/ivy.c b/sys/dev/random/ivy.c index 23fd542..9fbfbf0 100644 --- a/sys/dev/random/ivy.c +++ b/sys/dev/random/ivy.c @@ -79,7 +80,7 @@ ivy_rng_store(long *buf) "2:\n\t" "mov %2,%1\n\t" /* *buf = tmp */ "3:" - : "+q" (retry), "=m" (*buf), "+q" (tmp) : : "cc"); + : "+&q" (retry), "=&m" (*buf), "=&q" (tmp) : : "cc"); return (retry); #else /* __GNUCLIKE_ASM */ return (0);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141021151157.GC1877>