[PATCH] D61047: [X86] Delay creating index register negations during address matching until after we know for sure the match will succeed

[Craig Topper via Phabricator via llvm-commits]

craig.topper updated this revision to Diff 199301.
craig.topper added a comment.

-Move into getAddressOperands.
-Emit the NEG machine directly as I'm not sure I trust accessing N at this point in the code to use insertDAGNode. N can be changed by CSEing and other DAG modifications that can occur during address matching.
-Add -pre-RA-sched=linearize to one test that would fail previously due to the dangling sub node getting left behind when LEA costing failed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61047/new/

https://reviews.llvm.org/D61047

Files:
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/test/CodeGen/X86/imul.ll


Index: llvm/test/CodeGen/X86/imul.ll
===================================================================
--- llvm/test/CodeGen/X86/imul.ll
+++ llvm/test/CodeGen/X86/imul.ll
@@ -2,6 +2,8 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux-gnu | FileCheck %s --check-prefix=X64
 ; RUN: llc < %s -mtriple=x86_64-pc-linux-gnux32 | FileCheck %s --check-prefix=X64
 ; RUN: llc < %s -mtriple=i686-pc-linux | FileCheck %s --check-prefix=X86
+; At least one of the test cases in here crashed when linearizing the DAG.
+; RUN: llc < %s -mtriple=x86_64-pc-linux-gnu -pre-RA-sched=linearize
 
 define i32 @mul4_32(i32 %A) {
 ; X64-LABEL: mul4_32:
Index: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -73,6 +73,7 @@
     int JT;
     unsigned Align;    // CP alignment.
     unsigned char SymbolFlags;  // X86II::MO_*
+    bool NegateIndex = false;
 
     X86ISelAddressMode()
         : BaseType(RegBase), Base_FrameIndex(0), Scale(1), IndexReg(), Disp(0),
@@ -115,6 +116,8 @@
         dbgs() << " Base.FrameIndex " << Base_FrameIndex << '\n';
       dbgs() << " Scale " << Scale << '\n'
              << "IndexReg ";
+      if (NegateIndex)
+        dbgs() << "negate ";
       if (IndexReg.getNode())
         IndexReg.getNode()->dump(DAG);
       else
@@ -271,6 +274,14 @@
 
       Scale = getI8Imm(AM.Scale, DL);
 
+      // Negate the index if needed.
+      if (AM.NegateIndex) {
+        unsigned NegOpc = VT == MVT::i64 ? X86::NEG64r : X86::NEG32r;
+        SDValue Neg = SDValue(CurDAG->getMachineNode(NegOpc, DL, VT, MVT::i32,
+                                                     AM.IndexReg), 0);
+        AM.IndexReg = Neg;
+      }
+
       if (AM.IndexReg.getNode())
         Index = AM.IndexReg;
       else
@@ -1863,14 +1874,11 @@
     }
 
     // Ok, the transformation is legal and appears profitable. Go for it.
-    SDValue Zero = CurDAG->getConstant(0, dl, N.getValueType());
-    SDValue Neg = CurDAG->getNode(ISD::SUB, dl, N.getValueType(), Zero, RHS);
-    AM.IndexReg = Neg;
+    // Negation will be emitted later to avoid creating dangling nodes if this
+    // was an unprofitable LEA.
+    AM.IndexReg = RHS;
+    AM.NegateIndex = true;
     AM.Scale = 1;
-
-    // Insert the new nodes into the topological ordering.
-    insertDAGNode(*CurDAG, N, Zero);
-    insertDAGNode(*CurDAG, N, Neg);
     return false;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61047.199301.patch
Type: text/x-patch
Size: 2482 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190513/447ad54c/attachment.bin>