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

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp:76
+        continue;
+      }
+    }
----------------
RKSimon wrote:
> This LHS test shouldn't be needed - constants in ADD/OR should have been canonicalized to the RHS.
You still have both of these here, but removing the Base->getOperand(0) doesn't seem to have any effect on the tests. Do you have cases where Base->getOperand(0) is constant?


================
Comment at: test/CodeGen/MSP430/Inst16mm.ll:68
+; CHECK-DAG:	mov.w	2(r1), 6(r1)
+; CHECK-DAG:	mov.w	0(r1), 4(r1)
 }
----------------
pftbest wrote:
> RKSimon wrote:
> > If these are DAG checks would it better to replace the hard coded registers with regex?
> I think this test checks that we do direct memory to memory transfer without loading into intermediate register. r1 is a stack pointer, so it's fixed and cannot change.
> 
> What can change is the order of this two movs. Is there some CHECK keyword that ignores the order of instructions?
Yes CHECK-DAG should do that. If we're guaranteed to get r1 then we don't need a regex - thanks for the clarification.


https://reviews.llvm.org/D34472