[PATCH] D19475: [mips] Clang generates unaligned offset for MSA instruction st.d

[Matija Amidžić via llvm-commits]

mamidzic marked 2 inline comments as done.

================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:483
@@ +482,3 @@
+  if (selectAddrRegImm10(Addr, Base, Offset)){
+    // If base is a FI, additional offset calculation is done in EliminateFI.
+    if (!isa<FrameIndexSDNode>(Base)) {
----------------
dsanders wrote:
> EliminateFI -> eliminateFrameIndex?
Well both exist. There is MipsRegisterInfo::eliminateFrameIndex and MipsSERegisterInfo::eliminateFI. eliminateFrameIndex only calls eliminateFI and then the offset is actually being calculated.

================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:488-492
@@ +487,7 @@
+        unsigned OffsetDiff =  OffsetToAlignment(CnstOff, Align);
+        // If offset is not aligned, change it to the first greater offset
+        // that is aligned.
+        if(OffsetDiff)
+          Offset = CurDAG->getTargetConstant(CnstOff + OffsetDiff, SDLoc(Addr),
+                                             Addr.getValueType());
+      }
----------------
dsanders wrote:
> We shouldn't modify offsets in this code, we should only accept valid ones. Modifying the offset breaks the address calculation. One example of this is the O32 calling convention which can legitimately produce offsets that are unsuitable for the offset field of ld.[bhwd] and st.[bhwd] instructions (the ABI's stack alignment is insufficient to be able to guarantee that a vector argument/result is fully aligned). These instructions can still be used so long as the offset is added to the base address in a separate instruction since the spec permits unaligned addresses.
> 
> This code needs to fall back on selectAddrDefault() whenever the offset is inappropriate for the instructions instead of modifying the offset. We also need to do the alignment check in selectAddrFrameIndexOffset() so that we can account for the increased range that the scaled offsets allow.
I have done the fall back on selectAddrDefault() when the offset is wrong, but I'm not sure what did you mean with the last sentence. Could you elaborate a bit?


http://reviews.llvm.org/D19475