[PATCH] D38084: [ARM] add, or, and and xor with shl combining

John Brawn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 24 09:38:19 PDT 2017


john.brawn added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:10164
+  // Check that all the users could perform the shl themselves.
+  SDValue BinOp = SDValue(N, 0);
+  for (auto U : N->uses()) {
----------------
You're setting BinOp here then immediately setting it to something else a few lines later, you should be setting it just once.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:10165-10178
+  for (auto U : N->uses()) {
+    switch(U->getOpcode()) {
+    default:
+      return SDValue();
+    case ISD::ADD:
+    case ISD::SUB:
+    case ISD::AND:
----------------
It would make more sense to do this check nearer to the start of the function. Also you could check that all uses don't already have a shifted operand, as in that case doing this transform doesn't have any benefit. That should mean you don't have to adjust the load-combine tests.


https://reviews.llvm.org/D38084





More information about the llvm-commits mailing list