[lkml]   [2010]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: gcc 4.5.1 / as miscompiling drivers/char/i8k.c ?
    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 optimized away" was entirely wrong (we
    never saw that, it was just a report on another mailing list).

    Looking at the asm posted, it seems to me that gcc actually compiles
    the asm itself 100% correctly, and the "memory" clobber is working
    fine inside that function. So the code generated for i8k_smm() itself
    is all good.

    But _while_ generating the good code, gcc doesn't seem to realize that
    it writes to anything, so it decides to mark the function
    "__attribute__((const))", which is obviously wrong (a memory clobber
    definitely implies that it's not const). And as a result, the callers
    will be mis-optimized, because they do things like

    static int i8k_get_bios_version(void)
    struct smm_regs regs = { .eax = I8K_SMM_BIOS_VERSION, };

    return i8k_smm(&regs) ? : regs.eax;

    and since gcc has (incorrectly) decided that "i8k_smm()" is a const
    function, it thinks that "regs.eax" hasn't changed, so it doesn't
    bother to reload it: it "knows" that it is still I8K_SMM_BIOS_VERSION
    that it initialized it with. So it will basically have rewritten that
    final return statement as

    return i8k_smm(&regs) ? : I8K_SMM_BIOS_VERSION;

    which obviously doesn't really work.

    This also explains why adding "volatile" worked. The "asm volatile"
    triggered "this is not a const function".

    Similarly, the "+m" works, because it also makes clear that the asm is
    writing to memory, and isn't a const function.

    Now, the "memory" clobber should clearly also have done that, but I'd
    be willing to bet that some version of gcc (possibly extra slackware
    patches) had forgotten the trivial logic to say "a memory clobber also
    makes the user function non-const".

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2010-11-15 20:17    [W:0.023 / U:1.292 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site