[llvm-commits] [llvm] r140902 - in /llvm/trunk: lib/CodeGen/SelectionDAG/SelectionDAG.cpp test/CodeGen/X86/alignment-2.ll

[John McCall]

On Oct 2, 2011, at 3:26 AM, Duncan Sands wrote:
>>>>>> When inferring the pointer alignment, if the global doesn't have an initializer
>>>>>> and the alignment is 0 (i.e., it's defined globally in one file and declared in
>>>>>> another file) it could get an alignment which is larger than the ABI allows for
>>>>>> that type, resulting in aligned moves being used for unaligned loads.
>>>>> 
>>>>> even if it has an initializer that doesn't mean anything if the global has a
>>>>> weak linkage type since in the final program a different initializer might be
>>>>> used.  In short, the code a line or so above should use hasDefinitiveInitializer
>>>>> not hasInitializer.
>>>>> 
>>>> Does that matter in this case? It's just trying to see if there's a preferred alignment, which from what I can tell relies upon the type and not the initializer. If I read this correctly, the presence of an initializer is what's important here.
>>> 
>>> the type could be different in a different translation unit, so yes it
>>> does matter.
>>> 
>> Really? That sounds like a bug. If it changes the type from, say, a scalar to a vector, that's not good...
> 
> don't forget that the other compilation unit may have been compiled using a
> different compiler.  Even if the type is the same, the other compiler is free
> to use the ABI alignment rather than something better like LLVM's preferred
> alignment.  I think this applies to C++ types too in spite of the ODR: since
> the ABI only mandates ABI alignment, if it is the definition compiled by the
> other compiler that ends up being used in the final executable then only the
> ABI alignment can be assumed.  That means that I was wrong to suggest using
> hasDefinitiveInitializer.  Instead I think the check should be that the linkage
> satisfies !isWeakForLinker, i.e. that the memory allocated for the global by
> LLVM is what will actually be used in the final program.

There are two possible interpretations for alignment on a weak global:
  (a) the minimum guaranteed alignment, which all emissions must satisfy, or
  (b) the alignment to use for this emission, which can be higher.
The value of expressing (b) is to enable runtime optimizations:  we can never assume statically that the global is (say) 16-byte aligned, but if it is (more likely the more individual emissions we over-align), things get faster.  But otherwise it's pretty useless;  nothing in the compiler cares except object emission.  For everyone's sake, the usual alignment computation clearly needs to compute (a), not (b).

I don't think it's a good idea to change the usual alignment computation for this case, either.  As it is, it's very nice and regular:  use explicit alignment if present, otherwise use the ABI alignment of the IR type.

So I would argue that this all means that frontends have the responsibility to ensure that the usual alignment computation doesn't yield something "optimistic".  I think we really don't want the optimizers and the backend to be in the business of second-guessing that.  Recall that the major source languages have ways to raise and (frequently) lower the minimum guaranteed alignment for specific declarations.

John.