[PATCH] D34472: [DAG] Rewrite areNonVolatileConsecutiveLoads to use BaseIndexOffset

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Some minor comments.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp:31
+    if (auto *A = dyn_cast<GlobalAddressSDNode>(Base))
+      if (auto *B = dyn_cast<GlobalAddressSDNode>(Other.Base))
         if (A->getGlobal() == B->getGlobal()) {
----------------
This can be done as an NFC commit to trunk now - it's not dependent on the patch.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp:76
+        continue;
+      }
+    }
----------------
This LHS test shouldn't be needed - constants in ADD/OR should have been canonicalized to the RHS.


================
Comment at: test/CodeGen/MSP430/Inst16mm.ll:68
+; CHECK-DAG:	mov.w	2(r1), 6(r1)
+; CHECK-DAG:	mov.w	0(r1), 4(r1)
 }
----------------
If these are DAG checks would it better to replace the hard coded registers with regex?


================
Comment at: test/CodeGen/PowerPC/complex-return.ll:12
   %imag = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* %x, i32 0, i32 1
-  store ppc_fp128 0xM400C0000000000000000000000000000, ppc_fp128* %real
+  store ppc_fp128 0xM400C0000000000300000000010000000, ppc_fp128* %real
   store ppc_fp128 0xMC00547AE147AE1483CA47AE147AE147A, ppc_fp128* %imag
----------------
Should this be done as a pre-commit?


https://reviews.llvm.org/D34472