[PATCH] D49966: [X86] Performing DAG pruning before selection of LEA instructions.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:703-707
+// This is last stage before instruction selection, all the
+// legalization and combining has been performed and no more
+// target independent/specific DAG transformation will be done
+// now ,look for pattern which could be transformed to facilitate
+// addressing mode based LEA selection.
----------------
I suspect this comment should be at the call site.
The comment *here* should ideally explain what the *function* does, more specifically.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:712
+  unsigned Opcode = N->getOpcode();
+  if (Opcode == ISD::SHL) {
+    SDNode *Shift = N->getOperand(1).getNode();
----------------
craig.topper wrote:
> Why are we matching the pattern in reverse? Normally we would look for a sub followed by a shift. Why are we starting with a shift and looking backwards?
Early return?


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:720
+    SDNode *User = *(N->use_begin());
+    if (User->getOpcode() == ISD::SUB) {
+      if (User->getOperand(1).getNode() == N) {
----------------
Early return?


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:729
+        CurDAG->ReplaceAllUsesOfValueWith(SDValue(User, 0), NewAdd);
+      } else {
+        SDValue OpsSUB[] = {CurDAG->getConstant(0, DL, VT),
----------------
It would be great of there was a comment before each of these two cases showing which pattern it transforms into what.


Repository:
  rL LLVM

https://reviews.llvm.org/D49966