InstructionCombining forgets alignment of globals

Project Infrastructure LLVM Dev List Archives
#1

Hi all,

The InstructionCombining pass causes alignment of globals to be ignored.

I’ve attached a replacement of Fibonacci.cpp which reproduces this (I used 2.3 release). Here’s the x86 code it produces:

03C20019 movaps xmm0,xmmword ptr ds:[164E799h]

03C20020 mulps xmm0,xmmword ptr ds:[164E79Ah]

03C20027 movaps xmmword ptr ds:[164E799h],xmm0

03C2002E mov esp,ebp

03C20030 pop ebp

03C20031 ret

All three SSE instructions will generate a fault for accessing unaligned memory. Disabling InstructionCombining gives me the following correct code:

03B10010 push ebp

03B10011 mov ebp,esp

03B10013 and esp,0FFFFFFF0h

03B10019 movups xmm0,xmmword ptr ds:[164E79Ah]

03B10020 movups xmm1,xmmword ptr ds:[164E799h]

03B10027 mulps xmm1,xmm0

03B1002A movups xmmword ptr ds:[164E799h],xmm1

03B10031 mov esp,ebp

03B10033 pop ebp

03B10034 ret

Unless I’m missing something this is quite clearly a bug. I’ll give it a try to locate the faulty code but all help is very welcome.

Cheers,

Nicolas Capens

fibonacci.cpp (2.13 KB)

#2

I think I found it. In InstCombiner::ComputeMaskedBits we have the following lines:

if (GlobalValue *GV = dyn_cast(V)) {

unsigned Align = GV->getAlignment();

if (Align == 0 && TD && GV->getType()->getElementType()->isSized())

Align = TD->getPrefTypeAlignment(GV->getType()->getElementType());

It assumes that global values are always optimally aligned. I think this is incorrect and the bottom two lines should be removed.

However, I do think it’s useful to specify the alignment at the time of GlobalValue creation, so I propose to add a constructor with an Alignment argument.

#3

Hi Nicolas,

  if (GlobalValue *GV = dyn_cast<GlobalValue>(V)) {

    unsigned Align = GV->getAlignment();

    if (Align == 0 && TD && GV->getType()->getElementType()->isSized())

      Align = TD->getPrefTypeAlignment(GV->getType()->getElementType());

It assumes that global values are always optimally aligned. I think this is
incorrect and the bottom two lines should be removed.

I don't understand - if Align is zero it means that GV was marked
as having preferred alignment (that's what Align == 0 means), so
it is not wrong to change it explicitly to the preferred alignment
for the target.

Ciao,

Duncan.

#4

Hi Duncan,

I see. I didn't know GlobalValue had a method for changing the alignment
(and that 0 meant natural alignment). Explicitly calling setAlignment works
like a charm.

So it's not an LLVM bug.

Thanks,

Nicolas

#5

I'm looking at a very similar bug in a test here. I've tracked it down to
some problem in ComputeMaskedBits but I haven't been able to get
further than that. I'm trying to reduce the testcase (hence the thread
about Function cloning).

                                                     -Dave

#6

Nicolas,

Where are you explicitly calling setAlignment? From llvm-gcc code?

                                           -Dave

#7

Hi Dave,

See the fibonacci.cpp file in my first message in this thread. Now I call
setAlignment right after creating the GlobalVariable's and everything is
fine.

Cheers,

Nicolas