[llvm-commits] [llvm] r86404 - in /llvm/trunk/lib/Target/ARM: ARMAddressingModes.h ARMISelDAGToDAG.cpp ARMInstrInfo.td AsmPrinter/ARMAsmPrinter.cpp NEONPreAllocPass.cpp

[Dan Gohman]

On Nov 11, 2009, at 6:50 PM, Evan Cheng wrote:

> 
> On Nov 11, 2009, at 8:40 AM, Jim Grosbach wrote:
> 
>> 
>> On Nov 10, 2009, at 1:27 AM, Evan Cheng wrote:
>> 
>>> 
>>> On Nov 7, 2009, at 1:25 PM, Jim Grosbach wrote:
>>> 
>>>> 
>>>> 
>>>> bool ARMDAGToDAGISel::SelectAddrMode6(SDValue Op, SDValue N,
>>>>                                    SDValue &Addr, SDValue &Update,
>>>> -                                      SDValue &Opc) {
>>>> +                                      SDValue &Opc, SDValue &Align) {
>>>> Addr = N;
>>>> // Default to no writeback.
>>>> Update = CurDAG->getRegister(0, MVT::i32);
>>>> Opc = CurDAG->getTargetConstant(ARM_AM::getAM6Opc(false), MVT::i32);
>>>> +  // Default to no alignment.
>>>> +  Align = CurDAG->getTargetConstant(0, MVT::i32);
>>>> return true;
>>>> }
>>> 
>>> Shouldn't we be able to transfer the alignment on the LoadSDNode / StoreSDNode to Align (capped at 64 / 128 for 64-bit / 128-bit memory operations)?
>> 
>> Potentially. I'm concerned about stack objects in frames we don't dynamically align, however. That could result in aligned load/stores of objects w/o guaranteed alignment, and I wanted to avoid that.
> 
> That shouldn't happen. The alignment field on LoadSDNode and StoreSDNode should not be considered optional. They are required. Dan, is that right?

Yes. The current (somewhat naive) rule is that a non-zero alignment value on
a load or store says that if the dynamic address doesn't have at least that
alignment, the behavior is undefined.

Dan