[PATCH] D31113: [AArch64] Add new subtarget feature to fold LSL into address mode.

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:337
+  // computation, since the computation will be kept.
+  const SDNode *Node = V.getNode();
+  for (SDNode *UI : Node->uses())
----------------
Don't we need to add assert( V.getOpcode() == ISD::SHL )  ?


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:342
+        if (!isa<MemSDNode>(*UII))
+          return false;
+  // Check if a subtarget supports folding logical shift of up to 3 places
----------------
It will be good to add a test for this case.


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:347
+    unsigned Val = C->getZExtValue();
+    return (Subtarget->hasLSLFast() && Val < 4);
+  }
----------------
I think "Subtarget->hasLSLFast()" need to be check earlier. Maybe in isWorthFolding() ?


================
Comment at: lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:369
   // for code size.
   return ForCodeSize || V.hasOneUse();
 }
----------------
Why not doing this first.


https://reviews.llvm.org/D31113