[llvm-dev] BUGS in code generated for target i386-win32

[Tim Northover via llvm-dev]

Hi Stefan,

On Mon, 26 Nov 2018 at 12:37, Stefan Kanthak via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> LLVM/clang generates wrong code for the following program
> (see <https://godbolt.org/z/UZrrkG>):

It looks like all of these issues come down to mismatched expectations
on what goes into and out of an __asm block. If you look at the MSVC
documentation (which I think Clang is trying to be compatible with),
it warns against assuming known values are in particular registers at
any point, and even using __fastcall at all for a function with an
__asm block for that reason:
https://docs.microsoft.com/en-gb/cpp/assembler/inline/using-and-preserving-registers-in-inline-assembly?view=vs-2017

I'll try to explain a little below how that one mismatch causes the
issues you're seeing.

> BUG #1: the compiler fails to allocate (EAX for) the variable "lfsr"!
> BUG #2: the variable "lfsr" is NOT initialized!

Since the __asm isn't linked (as far as Clang is concerned) to either
input for lfsr32, they're both unused. The compiler has actually
inlined that function as you said, then noticed this lack of use and
decided to completely eliminate the lfsr variable.

You can see this in LLVM IR if you add "-emit-llvm -g0" to the godbolt
command-line. The loop *only* contains a counter and the lonely asm
block

> BUG #3: the compiler allocates EDX for the variable "period"!
> ~~~~~~~
> EDX is a volatile register, it is not preserved through function calls;

There is no function call after inlining, of course, so it doesn't
need to be preserved in this case. It looks like there might be an
issue with an __asm block that actually *makes* a call though:

    __asm {
      call Wibble
    }

doesn't seem to mark registers used by the call as clobbered from my
testing. It's slightly unclear what MSVC's behaviour is in this
situation from the documentation, but some vague poking suggests it
does know that a call instruction might clobber edx.

> additionally it holds the second argument with the __fastcall calling
> convention, so the constant 0x04C11DB7 MUST be loaded into EDX before
> label LBB1_1:

This is the same unused-as-far-as-the-compiler-knows thing as the first two.

Cheers.

Tim.