Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jeff Law
On 11/15/10 16:07, Richard Guenther wrote: If the address of the auto isn't taken, then why is the object in memory to begin with (with the obvious exception for aggregates). Exactly sort of my point. If people pass the address of&x to an asm and modify&x + 8 expecting the "adjacent" stack loc

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Richard Guenther
On Mon, Nov 15, 2010 at 11:58 PM, Jeff Law wrote: > On 11/15/10 15:07, Richard Guenther wrote: >> >> On Mon, Nov 15, 2010 at 7:45 PM, Jeff Law  wrote: >>> >>> On 11/08/10 03:49, Richard Guenther wrote: On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen  wrote: > > Andreas Schwab  

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jeff Law
On 11/15/10 15:07, Richard Guenther wrote: On Mon, Nov 15, 2010 at 7:45 PM, Jeff Law wrote: On 11/08/10 03:49, Richard Guenther wrote: On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleenwrote: Andreas Schwabwrites: The asm fails to mention that it modifies *regs. It has a memory clobber, th

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 11:43:22PM +0100, Andi Kleen wrote: > > testcase shows that in 4.1/4.2/4.3/4.4 this is miscompiled only when using > > -fno-ipa-reference, in 4.5 it is miscompiled always when optimizing > > unless -fno-ipa-pure-const (as 4.5 added local-pure-const pass which is run > > befo

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Andi Kleen
> testcase shows that in 4.1/4.2/4.3/4.4 this is miscompiled only when using > -fno-ipa-reference, in 4.5 it is miscompiled always when optimizing > unless -fno-ipa-pure-const (as 4.5 added local-pure-const pass which is run > before ipa-reference) and in 4.6 this has been fixed by Honza when > doi

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Richard Guenther
On Mon, Nov 15, 2010 at 7:45 PM, Jeff Law wrote: > On 11/08/10 03:49, Richard Guenther wrote: >> >> On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen  wrote: >>> >>> Andreas Schwab  writes: The asm fails to mention that it modifies *regs. >>> >>> It has a memory clobber, that should be enough,

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 08:51 PM, Jakub Jelinek wrote: > On Mon, Nov 15, 2010 at 11:21:30AM -0800, Linus Torvalds wrote: >> On Mon, Nov 15, 2010 at 11:12 AM, Jakub Jelinek wrote: >>> >>> Ah, the problem is that memory_identifier_string is only initialized in >>> ipa-reference.c's initialization, so it can b

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 11:53:05AM -0800, Richard Henderson wrote: > On 11/15/2010 11:12 AM, Jakub Jelinek wrote: > > - if (simple_cst_equal(TREE_VALUE (op), memory_identifier_string) == 1) > > + if (strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory") == 0) > > I prefer this solutio

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Richard Henderson
On 11/15/2010 11:12 AM, Jakub Jelinek wrote: > - if (simple_cst_equal(TREE_VALUE (op), memory_identifier_string) == 1) > + if (strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory") == 0) I prefer this solution. I think memory_identifier_string is over-engineering. Patch to remove

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 11:21:30AM -0800, Linus Torvalds wrote: > On Mon, Nov 15, 2010 at 11:12 AM, Jakub Jelinek wrote: > > > > Ah, the problem is that memory_identifier_string is only initialized in > > ipa-reference.c's initialization, so it can be (and is in this case) NULL in > > ipa-pure-con

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Linus Torvalds
On Mon, Nov 15, 2010 at 11:12 AM, Jakub Jelinek wrote: > > Ah, the problem is that memory_identifier_string is only initialized in > ipa-reference.c's initialization, so it can be (and is in this case) NULL in > ipa-pure-const.c. Ok. And I guess you can verify that all versions of gcc do this cor

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 07:58:48PM +0100, Jakub Jelinek wrote: > Now, not sure why this happens, as there is > case GIMPLE_ASM: > for (i = 0; i < gimple_asm_nclobbers (stmt); i++) > { > tree op = gimple_asm_clobber_op (stmt, i); > if (simple_cst_equal(TREE_VALU

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 07:26 PM, Jakub Jelinek wrote: > On Mon, Nov 15, 2010 at 07:17:31PM +0100, Jim Bos wrote: >> # gcc -v >> Reading specs from /usr/lib/gcc/i486-slackware-linux/4.5.1/specs >> COLLECT_GCC=gcc >> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i486-slackware-linux/4.5.1/lto-wrapper >> Target: i486-

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Linus Torvalds
On Mon, Nov 15, 2010 at 10:45 AM, Jeff Law wrote: > > A memory clobber should clobber anything in memory, including autos in > memory; if it doesn't, then that seems like a major problem.  I'd like to > see the rationale behind not clobbering autos in memory. Yes. It turns out that the "asm optim

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 07:30:35PM +0100, Jim Bos wrote: > On 11/15/2010 07:08 PM, Linus Torvalds wrote: > > On Mon, Nov 15, 2010 at 9:40 AM, Jim Bos wrote: > >> > >> Hmm, that doesn't work. > >> > >> [ Not sure if you read to whole thread but initial workaround was to > >> change the asm(..) to a

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Linus Torvalds
On Mon, Nov 15, 2010 at 10:30 AM, Jim Bos wrote: > > Attached version with plain 2.6.36 source and version with the committed > patch, i.e with the '"+m" (*regs)' Looks 100% identical in i8k_smm() itself, and I'm not seeing anything bad. The asm has certainly not been optimized away as implied in

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jeff Law
On 11/08/10 03:49, Richard Guenther wrote: On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: Andreas Schwab writes: The asm fails to mention that it modifies *regs. It has a memory clobber, that should be enough, no? No. A memory clobber does not cover automatic storage. A memory clobber

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jeff Law
On 11/07/10 15:41, Andreas Schwab wrote: Andi Kleen writes: Jim writes: After upgrading my Dell laptop, both OS+kernel the i8k interface was giving nonsensical output. As it turned out it's not the kernel but compiler upgrade which broke this. Guys at Archlinux have found the underlying ca

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 07:30 PM, Jim Bos wrote: > On 11/15/2010 07:08 PM, Linus Torvalds wrote: >> On Mon, Nov 15, 2010 at 9:40 AM, Jim Bos wrote: >>> >>> Hmm, that doesn't work. >>> >>> [ Not sure if you read to whole thread but initial workaround was to >>> change the asm(..) to asm volatile(..) which di

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 07:08 PM, Linus Torvalds wrote: > On Mon, Nov 15, 2010 at 9:40 AM, Jim Bos wrote: >> >> Hmm, that doesn't work. >> >> [ Not sure if you read to whole thread but initial workaround was to >> change the asm(..) to asm volatile(..) which did work. ] > > Since I have a different gcc tha

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 07:17:31PM +0100, Jim Bos wrote: > # gcc -v > Reading specs from /usr/lib/gcc/i486-slackware-linux/4.5.1/specs > COLLECT_GCC=gcc > COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i486-slackware-linux/4.5.1/lto-wrapper > Target: i486-slackware-linux > Configured with: ../gcc-4.5.1/conf

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 06:44 PM, Jakub Jelinek wrote: > On Mon, Nov 15, 2010 at 06:36:06PM +0100, Jim Bos wrote: >> On 11/15/2010 12:37 PM, Andi Kleen wrote: >> See attached, note this is the vanilla 2.6.36 i8k.c (without any patch). >> And to be 100% sure, if I build this (make drivers/char/i8k.ko) it won'

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Linus Torvalds
On Mon, Nov 15, 2010 at 9:40 AM, Jim Bos wrote: > > Hmm, that doesn't work. > > [ Not sure if you read to whole thread but initial workaround was to > change the asm(..) to asm volatile(..) which did work. ] Since I have a different gcc than yours (and I'm not going to compile my own), have you p

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 06:36:06PM +0100, Jim Bos wrote: > On 11/15/2010 12:37 PM, Andi Kleen wrote: > See attached, note this is the vanilla 2.6.36 i8k.c (without any patch). > And to be 100% sure, if I build this (make drivers/char/i8k.ko) it won't > work. > > [ The i8k.i is rather big, even gzi

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jim Bos
On 11/15/2010 05:04 PM, Linus Torvalds wrote: > On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek wrote: >> >> I don't see any problems on the assembly level. i8k_smm is >> not inlined in this case and checks all 3 conditions. > > If it really is related to gcc not understanding that "*regs" has >

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Linus Torvalds
On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek wrote: > > I don't see any problems on the assembly level.  i8k_smm is > not inlined in this case and checks all 3 conditions. If it really is related to gcc not understanding that "*regs" has changed due to the memory being an automatic variable, an

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Andi Kleen
> Guess we need somebody who actually reported the problem, state what > gcc was actually used and post preprocessed source, gcc options > from his case. Jim Bos, Can you please supply that? Please use rm drivers/char/i8k.o make V=1 drivers/char/i8k.o make drivers/char/i8k.i and supply the

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 11:54:46AM +0100, Andi Kleen wrote: > > And for this the starting point should be what has been requested, > > i.e. preprocessed source + gcc options + gcc version and some hints what > > actually misbehaves (with the , "+m" (*regs) change reverted) > > in gcc bugzilla. Onl

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Andi Kleen
> And for this the starting point should be what has been requested, > i.e. preprocessed source + gcc options + gcc version and some hints what > actually misbehaves (with the , "+m" (*regs) change reverted) > in gcc bugzilla. Only with that we can actually look at what has been > happening, see w

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Richard Guenther
On Mon, Nov 15, 2010 at 9:56 AM, Jakub Jelinek wrote: > On Sun, Nov 14, 2010 at 07:21:50PM -0800, Linus Torvalds wrote: >> So when Richard Gunther says "a memory clobber doesn't cover automatic >> storage", to me that very clearly spells "gcc is buggy as hell". >> Because automatic storage with it

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 09:56:05AM +0100, Jakub Jelinek wrote: > On Sun, Nov 14, 2010 at 07:21:50PM -0800, Linus Torvalds wrote: > > So when Richard Gunther says "a memory clobber doesn't cover automatic > > storage", to me that very clearly spells "gcc is buggy as hell". > > Because automatic stor

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Mon, Nov 15, 2010 at 09:56:05AM +0100, Jakub Jelinek wrote: > Yes, reload should figure out it has address of regs already tied to %eax, > unfortunately starting with IRA it doesn't (I'll file a GCC bug about that; http://gcc.gnu.org/PR46479 Jakub

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Andi Kleen
> That said, changing the inline asm to just clobber one less register > would be completely sufficient to make it work well with all gccs out there, > just push/pop one of the register around the whole body. I doubt calling > out SMM BIOS is actually so performance critical that one push and one

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-15 Thread Jakub Jelinek
On Sun, Nov 14, 2010 at 07:21:50PM -0800, Linus Torvalds wrote: > So when Richard Gunther says "a memory clobber doesn't cover automatic > storage", to me that very clearly spells "gcc is buggy as hell". > Because automatic storage with its address taken _very_ much gets > clobbered by things like

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-14 Thread Linus Torvalds
On Sun, Nov 14, 2010 at 4:52 PM, James Cloos wrote: > Gcc 4.5.1 running on an amd64 box "cross"-compiling for a P3 i8k fails > to compile the module since commit 6b4e81db2552bad04100e7d5ddeed7e848f53b48 > with: > >  CC      drivers/char/i8k.o > drivers/char/i8k.c: In function ‘i8k_smm’: > drivers/

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-14 Thread James Cloos
Gcc 4.5.1 running on an amd64 box "cross"-compiling for a P3 i8k fails to compile the module since commit 6b4e81db2552bad04100e7d5ddeed7e848f53b48 with: CC drivers/char/i8k.o drivers/char/i8k.c: In function ‘i8k_smm’: drivers/char/i8k.c:149:2: error: can't find a register in class ‘GENERAL_

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-09 Thread Jim
On 11/09/2010 02:57 PM, Andreas Schwab wrote: > Andi Kleen writes: > >> @@ -142,7 +142,7 @@ static int i8k_smm(struct smm_regs *regs) >> "lahf\n\t" >> "shrl $8,%%eax\n\t" >> "andl $1,%%eax\n" >> -:"=a"(rc) >> +:"=a"(rc), "=m" (*regs)

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-09 Thread Andreas Schwab
Andi Kleen writes: > @@ -142,7 +142,7 @@ static int i8k_smm(struct smm_regs *regs) > "lahf\n\t" > "shrl $8,%%eax\n\t" > "andl $1,%%eax\n" > - :"=a"(rc) > + :"=a"(rc), "=m" (*regs) I think this should be "+m". Andreas. -- Andrea

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-09 Thread Michael Matz
Hi, On Mon, 8 Nov 2010, Dave Korn wrote: > void foo (void) > { > int x, y, z; > x = 23; > asm ("do something" : "=r" (y) : "r" (x) ); > z = y + 1; > } The case in i8k.c really is different. It does use the value by influencing the return value and the callers use the returned value in

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-09 Thread Andi Kleen
> My speculation is, that the asm is not removed but rather that regs.eax > isn't reloaded after the asm because the memory clobber doesn't clobber > automatic variables. Yes that makes sense. I wasn't able to verify it so far though. Maybe the original poster could try the obvious patch inste

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Dave Korn
On 08/11/2010 11:20, Andi Kleen wrote: > An asm with live inputs and outputs should never be optimized > way. If 4.5.1 started doing that it's seriously broken. I don't see that. Consider: void foo (void) { int x, y, z; x = 23; y = x + 1; z = y + 1; } So far, you'd agree the compil

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Michael Matz
Hi, On Mon, 8 Nov 2010, Andi Kleen wrote: > Richard Guenther writes: > > > On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: > >> Andreas Schwab writes: > >>> > >>> The asm fails to mention that it modifies *regs. > >> > >> It has a memory clobber, that should be enough, no? > > > > No. A m

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Jakub Jelinek
On Mon, Nov 08, 2010 at 06:47:59AM -0500, Paul Koning wrote: > I don't know about 4.5, but I noticed that with 4.6 (trunk), testcasese > like gcc.c-torture/compile/2804-1.c optimize away the asm and all the > operand generation except for -O0. That's fine, the asm isn't volatile and the output

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Paul Koning
On Nov 8, 2010, at 6:20 AM, Richard Guenther wrote: > On Mon, Nov 8, 2010 at 12:20 PM, Andi Kleen wrote: >> Richard Guenther writes: >> >>> On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: Andreas Schwab writes: > > The asm fails to mention that it modifies *regs.

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Richard Guenther
On Mon, Nov 8, 2010 at 12:20 PM, Andi Kleen wrote: > Richard Guenther writes: > >> On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: >>> Andreas Schwab writes: The asm fails to mention that it modifies *regs. >>> >>> It has a memory clobber, that should be enough, no? >> >> No.  A me

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Andi Kleen
Richard Guenther writes: > On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: >> Andreas Schwab writes: >>> >>> The asm fails to mention that it modifies *regs. >> >> It has a memory clobber, that should be enough, no? > > No. A memory clobber does not cover automatic storage. That's a separa

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-08 Thread Richard Guenther
On Mon, Nov 8, 2010 at 12:03 AM, Andi Kleen wrote: > Andreas Schwab writes: >> >> The asm fails to mention that it modifies *regs. > > It has a memory clobber, that should be enough, no? No. A memory clobber does not cover automatic storage. Btw, I can't see a testcase anywhere so I just assum

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-07 Thread Andi Kleen
Andreas Schwab writes: > > The asm fails to mention that it modifies *regs. It has a memory clobber, that should be enough, no? Besides in any case it cannot be eliminated because it has valid non dead inputs and outputs. -Andi -- a...@linux.intel.com -- Speaking for myself only.

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-07 Thread Andreas Schwab
Andi Kleen writes: > Jim writes: > >> After upgrading my Dell laptop, both OS+kernel the i8k interface was giving >> nonsensical output. As it turned out it's not the kernel but compiler >> upgrade which broke this. >> >> Guys at Archlinux have found the underlying cause (but don't seem to have

Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

2010-11-07 Thread Andi Kleen
Jim writes: > After upgrading my Dell laptop, both OS+kernel the i8k interface was giving > nonsensical output. As it turned out it's not the kernel but compiler > upgrade which broke this. > > Guys at Archlinux have found the underlying cause (but don't seem to have > submitted a patch yet): >