[PATCH] Update InstCombine to transform aggregate loads into scalar loads.

[Philip Reames]

On 04/20/2015 08:40 PM, Mehdi Amini wrote:
>> On Apr 20, 2015, at 6:51 PM, Philip Reames <listmail at philipreames.com> wrote:
>>
>> ================
>> Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:517
>> @@ +516,3 @@
>> +                                               ".unpack");
>> +      Instruction *V = InsertValueInst::Create(
>> +        UndefValue::get(T), NewLoad, 0, LI.getName());
>> ----------------
>> deadalnix wrote:
>>> reames wrote:
>>>> You're missing the setPointerOperand step.  Technically, the first element doesn't have to start at the beginning of the object.  I think this is fine in practice, but there's a theoretical issue here.
>>> I'm not sure how this is possible at all.
>> Unless the struct is packed, the data layout alignment could require that the internal field be aligned to a higher alignment than the struct.  This would be a weird arrangement, but I believe it's possible.
> I thought the structure alignment was computed as the largest individual member alignment.
> I think the relevant function is StructLayout::StructLayout(StructType *ST, const DataLayout &DL), and it seems to me that it always places the first element with a 0 offset.
>
> Is there another way to force the structure alignment?
Not that I know of.  There's a possible theoretical issue here, but it 
doesn't appear to exist in the current code.  Given that anyone who 
changed that assumption would have to go through and update various 
parts already, I don't have an issue with adding the code as is.  Thus, 
LGTM.
>
> —
> Mehdi
>