[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTi=jwrt2VE5tnL9NR2WhGDnQtv+k6rapnHfsy_aL@mail.gmail.com>
Date: Mon, 15 Nov 2010 08:04:49 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Jakub Jelinek <jakub@...hat.com>
Cc: Andi Kleen <andi@...stfloor.org>, James Cloos <cloos@...loos.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andreas Schwab <schwab@...hat.com>,
Michael Matz <matz@...e.de>,
Dave Korn <dave.korn.cygwin@...il.com>,
Richard Guenther <richard.guenther@...il.com>, gcc@....gnu.org,
Jim Bos <jim876@...all.nl>
Subject: Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?
On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek <jakub@...hat.com> 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, and passing in
"regs" itself as a pointer to that automatic variable together with
the "memory" clobber not being sufficient, than I think it's the lack
of inlining that will automatically hide the bug.
(Side note: and I think this does show how much of a gcc bug it is not
to consider "memory" together with passing in a pointer to an asm to
always be a clobber).
Because if it isn't inlined, then "regs" will be seen a a real pointer
to some external memory (the caller) rather than being optimized to
just be the auto structure on the stack. Because *mem is auto only
within the context of the caller.
Which actually points to a possible simpler:
- remove the "+m" since it adds too much register pressure
- mark the i8k_smm() as "noinline" instead.
Quite frankly, I'd hate to add even more crud to that inline asm (to
save/restore the registers manually). It's already not the prettiest
thing around.
So does the attached patch work for everybody?
Linus
View attachment "patch.diff" of type "text/x-patch" (1019 bytes)
Powered by blists - more mailing lists