[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/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)) {
----------------
I couldn't find it because I was searching for 'EliminateFI' from the comment instead of the 'eliminateFI' found in our backend. It's probably best to refer to eliminateFrameIndex since that's the interface with the target independent code but 'eliminateFI' is ok too as long as we correct the upper case 'E'

================
Comment at: lib/Target/Mips/MipsSEISelDAGToDAG.cpp:488-492
@@ +487,7 @@
+        unsigned OffsetDiff =  OffsetToAlignment(CnstOff, Align);
+        // If offset is not aligned, call selectAddrDefault
+        if(OffsetDiff)
+          return selectAddrDefault(Addr, Base, Offset);
+      }
+    }
+
----------------
Thanks.

> 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?

The current code checks for an offset that is a simm10 and is a multiple of the alignment but it should be checking for a simm10/simm11/simm12/simm13 (depending on the alignment) that is a multiple of the alignment. To put this another way, we need to check that the offset is a multiple of the alignment and that `Offset / Alignment` is a simm10.

For example, the offsets have to follow these constraints:
| Instruction | >= | <= | Multiple of |
| lw.b | -1024 | 1023 | 1 |
| lw.h | -2048 | 2046 | 2 |
| lw.w | -4096 | 4092 | 4 |
| lw.d | -8192 | 8184 | 8 |
but the current implementation implements these rules:
| Instruction | >= | <= | Multiple of |
| lw.b | -1024 | 1023 | 1 |
| lw.h | -1024 | 1022 | 2 |
| lw.w | -1024 | 1020 | 4 |
| lw.d | -1024 | 1016 | 8 |


http://reviews.llvm.org/D19475