[llvm-commits] [llvm] r106952 - in /llvm/trunk: include/llvm/CodeGen/ISDOpcodes.h include/llvm/CodeGen/SelectionDAG.h lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp lib/CodeGen/SelectionDAG/SelectionDAG

[Duncan Sands]

Hi Rafael

>>> +  const unsigned Align = std::max(OldAlign, TypeAlign);
>>
>> I don't see the point of taking the maximum here: the alignment of the memory
>> address did not change...
>
> This is an issue that Dan noticed: What happens for example with a
> i128, we would split the va_arg twice and lose the alignment
> information of the original one.

I don't see the connection between your remark, and mine about why you
take the maximum.

>>> -  Lo = DAG.getVAArg(NVT, dl, Chain, Ptr, N->getOperand(2));
>>> +  Lo = DAG.getVAArg(NVT, dl, Chain, Ptr, N->getOperand(2), Align);
>>>      Hi = DAG.getVAArg(NVT, dl, Lo.getValue(1), Ptr, N->getOperand(2));
>>
>> Shouldn't you also set the alignment of the Hi part?  For this you can take
>> use MinAlign(Align, amount offset).  It is true that in general to know how
>> much it was offset you need to know details of how VAArg works on the target,
>> which I guess you don't really know here.
>
> This works on the assumption that what we are splitting is contiguous
> in memory, so the alignment will be equal to the ABI alignment of that
> type, no?

Only if the original alignment was not less than the ABI alignment.  How do
you know that?

Ciao,

Duncan.