llvm dev - Sep 2008 - [LLVMdev] Alignment of constant loads

On Fri, September 12, 2008 11:33 am, Stefanus Du Toit
wrote:
> Hi,
>
> We've noticed that constant loads, e.g. of v4f32 types, are not
> aligned to the preferred alignment but rather to the ABI alignment, at
> least on x86.
On x86 targets it's usually 16 for both preferred and ABI.
I guess you're using a target with a lower ABI alignment for
vectors?
> This seems to stem from SelectionDAG::getLoad() being
> called with an alignment of 0, which then does:
>
>    if (Alignment == 0)  // Ensure that codegen never sees alignment 0
>      Alignment = getMVTAlignment(VT);
>
> Inside getMVTAlignment, the ABI alignment is retrieved for the given VT.
>
> It appears that constants are already aligned to the preferred
> alignment, given this code in ScheduleDAG::AddOperand():
>
>    } else if (ConstantPoolSDNode *CP >
dyn_cast<ConstantPoolSDNode>(Op)) {
>     [...]
>        Align = TM.getTargetData()->getPreferredTypeAlignmentShift(Type);
>
> (note there is a curious FIXME there about alignment of vector types
> -- I think this may be a relic since getPreferredTypeAlignmentShift
> should not return zero for vector types -- correct me if I'm wrong!)
I'm not sure about that FIXME offhand, but overall it looks
like the work this code is doing would be better placed in
SelectionDAG::getConstantPool.
> Do you have a suggestion as to how we can make it so that loads for
> constants are aligned to the preferred alignment, rather than the ABI
> alignment? We're not sure where this should be "fixed".
It looks like the best way to do this is to visit the handful of
places in legalize that create loads from constant pools and
add alignment parameters to the getLoad/getExtLoad calls.

If you move the handling of Alignment==0 out of ScheduleDAGEmit.cpp
and into SelectionDAG::getConstantPool, you can then have legalize
read the alignment from the node, instead of making its own
decision:
   cast<ConstantPoolSDNode>(CPIdx)->getAlignment()

Dan

Hi Dan,
> It looks like the best way to do this is to visit the handful of
> places in legalize that create loads from constant pools and
> add alignment parameters to the getLoad/getExtLoad calls.
>
> If you move the handling of Alignment==0 out of ScheduleDAGEmit.cpp
> and into SelectionDAG::getConstantPool, you can then have legalize
> read the alignment from the node, instead of making its own
> decision:
>    cast<ConstantPoolSDNode>(CPIdx)->getAlignment()
>
>   
I followed your suggestion but I've come across a bit of a snag. If you 
get the alignment in LegalizeDAG using:

SDValue CPIdx = DAG.getConstantPool(LLVMC, TLI.getPointerTy());
unsigned Alignment = cast<ConstantPoolSDNode>(CPIdx)->getAlignment();

The ConstantPoolSDNode is created using MVT i32 (TLI.getPointerTy()) and 
the alignment is 4 bytes. It seems there is not enough information to 
know that the final MVT will be v4f32 so passing the alignment to 
getLoad/getExtLoad in LegalizeDAG will still generate unaligned loads 
for constant vectors.

Any ideas?

thanks,
Paul

On Sep 15, 2008, at 2:45 PM, Paul Redmond wrote:
> Hi Dan,
Hi Paul,
>
>> It looks like the best way to do this is to visit the handful of
>> places in legalize that create loads from constant pools and
>> add alignment parameters to the getLoad/getExtLoad calls.
>>
>> If you move the handling of Alignment==0 out of ScheduleDAGEmit.cpp
>> and into SelectionDAG::getConstantPool, you can then have legalize
>> read the alignment from the node, instead of making its own
>> decision:
>>   cast<ConstantPoolSDNode>(CPIdx)->getAlignment()
>>
>>
> I followed your suggestion but I've come across a bit of a snag. If  
> you get the alignment in LegalizeDAG using:
>
> SDValue CPIdx = DAG.getConstantPool(LLVMC, TLI.getPointerTy());
> unsigned Alignment =
cast<ConstantPoolSDNode>(CPIdx)->getAlignment();
>
> The ConstantPoolSDNode is created using MVT i32 (TLI.getPointerTy())  
> and the alignment is 4 bytes. It seems there is not enough  
> information to know that the final MVT will be v4f32 so passing the  
> alignment to getLoad/getExtLoad in LegalizeDAG will still generate  
> unaligned loads for constant vectors.
>
> Any ideas?

In SelectionDAG::getConstantPool, while VT is the type of the
constant pool entry address, C is the actual constant, so the code
should look at C->getType() when it needs to compute the alignment.
That should allow it to obtain the desired alignment and set up the
ConstantPoolSDNode correctly. Then the LegalizeDAG code you show
above should work.

Dan