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

[Daniel Sanders via llvm-commits]

dsanders requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1076-1081
@@ -1075,2 +1075,8 @@
 def addrimm10 : ComplexPattern<iPTR, 2, "selectIntAddrMSA", [frameindex]>;
+def addrimm10al2 : ComplexPattern<iPTR, 2, "selectIntAddrMSAAlign2",
+                                  [frameindex]>;
+def addrimm10al4 : ComplexPattern<iPTR, 2, "selectIntAddrMSAAlign4",
+                                  [frameindex]>;
+def addrimm10al8 : ComplexPattern<iPTR, 2, "selectIntAddrMSAAlign8",
+                                  [frameindex]>;
 
----------------
For consistency with an existing microMIPS ComplexPattern (addrimm4lsl2) and the mem_simm_lsl* operands could we spell this 'addrimm10lsl2'?

================
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)) {
----------------
EliminateFI -> eliminateFrameIndex?

================
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());
+      }
----------------
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.

================
Comment at: test/CodeGen/Mips/msa/i5_ld_st.ll:68
@@ +67,3 @@
+; CHECK: llvm_mips_ld_h_unaligned_test:
+; CHECK: ld.h [[R1:\$w[0-9]+]], 10(
+; CHECK: st.h
----------------
As noted above, it's incorrect to use an offset other than 9. The expected code should be something like:
  addiu $3, $2, 9
  ld.h $4, 0($3)

Similarly for the ld.[hwd] and st.[hwd] cases.


http://reviews.llvm.org/D19475