Skip to content

Commit

Permalink
[X86] Delay creating index register negations during address matching…
Browse files Browse the repository at this point in the history
… until after we know for sure the match will succeed

If we're trying to match an LEA, its possible the LEA match will be deemed unprofitable. In which case the negation we created in matchAddress would be left dangling in the SelectionDAG. This could artificially increase use counts for other nodes in the DAG. Though I don't have an example of that. But it just seems like bad form to have dangling nodes in isel.

Differential Revision: https://reviews.llvm.org/D61047

llvm-svn: 360823
  • Loading branch information
topperc committed May 15, 2019
1 parent 8b92bb3 commit e43bdf1
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
22 changes: 15 additions & 7 deletions llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
Expand Up @@ -73,6 +73,7 @@ namespace {
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),
Expand Down Expand Up @@ -115,6 +116,8 @@ namespace {
dbgs() << " Base.FrameIndex " << Base_FrameIndex << '\n';
dbgs() << " Scale " << Scale << '\n'
<< "IndexReg ";
if (NegateIndex)
dbgs() << "negate ";
if (IndexReg.getNode())
IndexReg.getNode()->dump(DAG);
else
Expand Down Expand Up @@ -271,6 +274,14 @@ namespace {

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
Expand Down Expand Up @@ -1863,14 +1874,11 @@ bool X86DAGToDAGISel::matchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
}

// 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;
}

Expand Down
2 changes: 2 additions & 0 deletions llvm/test/CodeGen/X86/imul.ll
Expand Up @@ -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 | FileCheck %s --check-prefix=X64

define i32 @mul4_32(i32 %A) {
; X64-LABEL: mul4_32:
Expand Down

0 comments on commit e43bdf1

Please sign in to comment.