[llvm-commits] [llvm] r152907 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp test/Transforms/InstCombine/2012-03-16-StoreAlignment.ll

[Chris Lattner]

On Mar 16, 2012, at 1:55 AM, Duncan Sands wrote:

> Hi Bill,
> 
>> The alignment of the pointer part of the store instruction may have an
>> alignment. If that's the case, then we want to make sure that we don't increase
>> the alignment of the store instruction. Because if we increase it to be "more
>> aligned" than the pointer, code-gen may use instructions which require a greater
>> alignment than the pointer guarantees.
>> <rdar://problem/11043589>
> 
> this seems highly dubious to me.  As far as I can see the problem is that the
> front-end generated an 8 byte aligned store to a 4 byte aligned global.  That's
> a front-end bug, not something you can work around here.  [I say that the store
> is 8 byte aligned because that's the definition of the alignment of a store with
> no explicit alignment on it: the ABI alignment of the storee].  Also, it has to
> be wrong from another viewpoint: you only fix things up when you can see that
> the store is to a global, but the store could still be to that same global if
> eg the pointer is the return value of a function call that returns the address
> of the global.  But you don't fix things then while presumably you still need
> to.

I agree, this isn't the right fix.  The input IR has a store without an explicit alignment, so it follows target data rules.  Per your target data string, i64's are 64-bit aligned.

Please revert this patch, it is a frontend bug.

-Chris