Skip to content

Revert "[X86] Reuse already-materialized values when forming LEAs (#2… - #211958

Merged
mustartt merged 1 commit into
llvm:mainfrom
mustartt:revert-580604f
Jul 24, 2026
Merged

Revert "[X86] Reuse already-materialized values when forming LEAs (#2…#211958
mustartt merged 1 commit into
llvm:mainfrom
mustartt:revert-580604f

Conversation

@mustartt

Copy link
Copy Markdown
Member

…10739)"

This reverts commit 580604f.

@mustartt
mustartt enabled auto-merge (squash) July 24, 2026 23:02
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-backend-x86

Author: Henry Jiang (mustartt)

Changes

…10739)"

This reverts commit 580604f.


Patch is 23.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/211958.diff

6 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelDAGToDAG.cpp (+5-101)
  • (modified) llvm/test/CodeGen/X86/lea-opt-cse1.ll (+10-3)
  • (modified) llvm/test/CodeGen/X86/lea-opt-cse2.ll (+18-11)
  • (modified) llvm/test/CodeGen/X86/lea-opt-cse4.ll (+31-22)
  • (modified) llvm/test/CodeGen/X86/lea-recursion.ll (+18-12)
  • (removed) llvm/test/CodeGen/X86/pr51707.ll (-251)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 94cc1e49e79fe..9a0045367a8bf 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -79,11 +79,6 @@ namespace {
     Align Alignment;            // CP alignment.
     unsigned char SymbolFlags = X86II::MO_NO_FLAG;  // X86II::MO_*
     bool NegateIndex = false;
-    // True when this address is being matched to be emitted as a LEA rather
-    // than folded into a memory operand. Unlike a memory operand, a LEA turns
-    // the folded arithmetic into real instructions, so it is not profitable to
-    // split an already-materialized (multi-use) value here. (Issue #51707)
-    bool IsForLEA = false;
 
     X86ISelAddressMode() = default;
 
@@ -211,7 +206,6 @@ namespace {
     bool matchAddress(SDValue N, X86ISelAddressMode &AM);
     bool matchVectorAddress(SDValue N, X86ISelAddressMode &AM);
     bool matchAdd(SDValue &N, X86ISelAddressMode &AM, unsigned Depth);
-    bool hasMaterializingUse(SDValue V) const;
     SDValue matchIndexRecursively(SDValue N, X86ISelAddressMode &AM,
                                   unsigned Depth);
     bool matchAddressRecursively(SDValue N, X86ISelAddressMode &AM,
@@ -2067,111 +2061,22 @@ bool X86DAGToDAGISel::matchAddress(SDValue N, X86ISelAddressMode &AM) {
   return false;
 }
 
-// Returns true if V has a use that materializes it in a register as a value -
-// a stored value operand or a CopyToReg (a return value, call argument, or a
-// value that is live out of the block). Such a use means V will be in a
-// register regardless, so reusing it when forming an LEA is free. Uses where V
-// is only an address (a load/store pointer, or folded into another address
-// computation) do not materialize it. This is a more precise replacement for
-// the !hasOneUse() proxy: an address-only multi-use value is not materialized.
-bool X86DAGToDAGISel::hasMaterializingUse(SDValue V) const {
-  const TargetInstrInfo *TII = Subtarget->getInstrInfo();
-  for (SDUse &U : V->uses()) {
-    if (U.getResNo() != V.getResNo())
-      continue;
-    SDNode *User = U.getUser();
-    // A return value, call argument, or a value live out of the block.
-    if (User->getOpcode() == ISD::CopyToReg)
-      return true;
-    // A stored value materializes V (V as a store *address* does not).
-    if (auto *St = dyn_cast<StoreSDNode>(User)) {
-      if (St->getValue() == V)
-        return true;
-      continue;
-    }
-    // Selection may already have turned the ISD::STORE into a machine store by
-    // the time we get here. V materializes it if it is a stored value, i.e. an
-    // operand that is neither part of the memory reference (the address
-    // operands) nor the chain/glue. The memory reference is not always the
-    // first operand, so locate it via the instruction's memory-operand info
-    // rather than assuming a fixed layout. (No getOperandBias() is needed:
-    // unlike a MachineInstr, an SDNode's operand list has no leading defs.)
-    if (!User->isMachineOpcode())
-      continue;
-    const MCInstrDesc &Desc = TII->get(User->getMachineOpcode());
-    if (!Desc.mayStore())
-      continue;
-    int MemRefBegin = X86II::getMemoryOperandNo(Desc.TSFlags);
-    if (MemRefBegin < 0)
-      continue;
-    unsigned MemRefEnd = MemRefBegin + X86::AddrNumOperands;
-    for (unsigned I = 0, E = User->getNumOperands(); I != E; ++I) {
-      if (I >= static_cast<unsigned>(MemRefBegin) && I < MemRefEnd)
-        continue; // an address operand
-      SDValue Opnd = User->getOperand(I);
-      if (Opnd.getValueType() == MVT::Other || Opnd.getValueType() == MVT::Glue)
-        continue; // chain / glue
-      if (Opnd == V)
-        return true; // a stored value operand
-    }
-  }
-  return false;
-}
-
 bool X86DAGToDAGISel::matchAdd(SDValue &N, X86ISelAddressMode &AM,
                                unsigned Depth) {
   // Add an artificial use to this node so that we can keep track of
   // it if it gets CSE'd with a different node.
   HandleSDNode Handle(N);
 
-  auto IsAddOrAddLike = [&](SDValue V) {
-    return V.getOpcode() == ISD::ADD || CurDAG->isADDLike(V);
-  };
-
-  // When forming a LEA, avoid splitting an already-materialized value: use the
-  // operand directly as a base/index register instead. hasMaterializingUse()
-  // decides whether the operand is genuinely materialized - it has a use that
-  // puts it in a register as a value. A value used only as an address is not
-  // materialized, and splitting it there would only add a redundant
-  // materialization (see the two_ptrs test).
-  auto SplitsMaterializedValue = [&](SDValue Op) {
-    if (!AM.IsForLEA || !hasMaterializingUse(Op))
-      return false;
-
-    // add-like: decomposes to base + index (+ disp)
-    if (IsAddOrAddLike(Op))
-      return IsAddOrAddLike(Op.getOperand(0)) ||
-             IsAddOrAddLike(Op.getOperand(1));
-
-    // shl by 1/2/3 folds to a scaled index
-    if (Op.getOpcode() == ISD::SHL)
-      if (auto *C = dyn_cast<ConstantSDNode>(Op.getOperand(1)))
-        return C->getZExtValue() >= 1 && C->getZExtValue() <= 3 &&
-               IsAddOrAddLike(Op.getOperand(0));
-
-    return false;
-  };
-
-  // The check is applied here, per add operand, rather than inside
-  // matchAddressRecursively, so that it only fires when an add directly
-  // consumes the value. matchAddressRecursively is also entered for the LEA
-  // root itself and from the SUB case's operand fold.
-  // Firing there produces worse code.
-  auto MatchOperand = [&](SDValue Op) {
-    if (SplitsMaterializedValue(Op))
-      return matchAddressBase(Op, AM);
-    return matchAddressRecursively(Op, AM, Depth + 1);
-  };
-
   X86ISelAddressMode Backup = AM;
-  if (!MatchOperand(N.getOperand(0)) &&
-      !MatchOperand(Handle.getValue().getOperand(1)))
+  if (!matchAddressRecursively(N.getOperand(0), AM, Depth+1) &&
+      !matchAddressRecursively(Handle.getValue().getOperand(1), AM, Depth+1))
     return false;
   AM = Backup;
 
   // Try again after commutating the operands.
-  if (!MatchOperand(Handle.getValue().getOperand(1)) &&
-      !MatchOperand(Handle.getValue().getOperand(0)))
+  if (!matchAddressRecursively(Handle.getValue().getOperand(1), AM,
+                               Depth + 1) &&
+      !matchAddressRecursively(Handle.getValue().getOperand(0), AM, Depth + 1))
     return false;
   AM = Backup;
 
@@ -3248,7 +3153,6 @@ bool X86DAGToDAGISel::selectLEAAddr(SDValue N,
                                     SDValue &Index, SDValue &Disp,
                                     SDValue &Segment) {
   X86ISelAddressMode AM;
-  AM.IsForLEA = true;
 
   // Save the DL and VT before calling matchAddress, it can invalidate N.
   SDLoc DL(N);
diff --git a/llvm/test/CodeGen/X86/lea-opt-cse1.ll b/llvm/test/CodeGen/X86/lea-opt-cse1.ll
index 88fcd09612048..5ceca9fbd9b5f 100644
--- a/llvm/test/CodeGen/X86/lea-opt-cse1.ll
+++ b/llvm/test/CodeGen/X86/lea-opt-cse1.ll
@@ -9,21 +9,28 @@ define void @test_func(ptr nocapture %ctx, i32 %n) local_unnamed_addr {
 ; X64:       # %bb.0: # %entry
 ; X64-NEXT:    movl (%rdi), %eax
 ; X64-NEXT:    movl 16(%rdi), %ecx
+; X64-NEXT:    leal (%rax,%rcx), %edx
 ; X64-NEXT:    leal 1(%rax,%rcx), %eax
 ; X64-NEXT:    movl %eax, 12(%rdi)
-; X64-NEXT:    addl %ecx, %eax
+; X64-NEXT:    leal 1(%rcx,%rdx), %eax
 ; X64-NEXT:    movl %eax, 16(%rdi)
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: test_func:
 ; X86:       # %bb.0: # %entry
+; X86-NEXT:    pushl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 8
+; X86-NEXT:    .cfi_offset %esi, -8
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl (%eax), %ecx
 ; X86-NEXT:    movl 16(%eax), %edx
-; X86-NEXT:    leal 1(%ecx,%edx), %ecx
-; X86-NEXT:    movl %ecx, 12(%eax)
+; X86-NEXT:    leal 1(%ecx,%edx), %esi
 ; X86-NEXT:    addl %edx, %ecx
+; X86-NEXT:    movl %esi, 12(%eax)
+; X86-NEXT:    leal 1(%edx,%ecx), %ecx
 ; X86-NEXT:    movl %ecx, 16(%eax)
+; X86-NEXT:    popl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 4
 ; X86-NEXT:    retl
  entry:
    %0 = load i32, ptr %ctx, align 8
diff --git a/llvm/test/CodeGen/X86/lea-opt-cse2.ll b/llvm/test/CodeGen/X86/lea-opt-cse2.ll
index fbee135df0b0c..e39d01f1447f8 100644
--- a/llvm/test/CodeGen/X86/lea-opt-cse2.ll
+++ b/llvm/test/CodeGen/X86/lea-opt-cse2.ll
@@ -10,22 +10,26 @@ define void @foo(ptr nocapture %ctx, i32 %n) local_unnamed_addr #0 {
 ; X64-NEXT:    .p2align 4
 ; X64-NEXT:  .LBB0_1: # %loop
 ; X64-NEXT:    # =>This Inner Loop Header: Depth=1
-; X64-NEXT:    movl (%rdi), %ecx
-; X64-NEXT:    movl 16(%rdi), %eax
-; X64-NEXT:    leal 1(%rcx,%rax), %ecx
-; X64-NEXT:    movl %ecx, 12(%rdi)
+; X64-NEXT:    movl (%rdi), %eax
+; X64-NEXT:    movl 16(%rdi), %ecx
+; X64-NEXT:    leal 1(%rax,%rcx), %edx
+; X64-NEXT:    movl %edx, 12(%rdi)
 ; X64-NEXT:    decl %esi
 ; X64-NEXT:    jne .LBB0_1
 ; X64-NEXT:  # %bb.2: # %exit
-; X64-NEXT:    addl %eax, %ecx
-; X64-NEXT:    movl %ecx, 16(%rdi)
+; X64-NEXT:    addl %ecx, %eax
+; X64-NEXT:    leal 1(%rcx,%rax), %eax
+; X64-NEXT:    movl %eax, 16(%rdi)
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: foo:
 ; X86:       # %bb.0: # %entry
-; X86-NEXT:    pushl %esi
+; X86-NEXT:    pushl %edi
 ; X86-NEXT:    .cfi_def_cfa_offset 8
-; X86-NEXT:    .cfi_offset %esi, -8
+; X86-NEXT:    pushl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 12
+; X86-NEXT:    .cfi_offset %esi, -12
+; X86-NEXT:    .cfi_offset %edi, -8
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    .p2align 4
@@ -33,14 +37,17 @@ define void @foo(ptr nocapture %ctx, i32 %n) local_unnamed_addr #0 {
 ; X86-NEXT:    # =>This Inner Loop Header: Depth=1
 ; X86-NEXT:    movl (%eax), %edx
 ; X86-NEXT:    movl 16(%eax), %esi
-; X86-NEXT:    leal 1(%edx,%esi), %edx
-; X86-NEXT:    movl %edx, 12(%eax)
+; X86-NEXT:    leal 1(%edx,%esi), %edi
+; X86-NEXT:    movl %edi, 12(%eax)
 ; X86-NEXT:    decl %ecx
 ; X86-NEXT:    jne .LBB0_1
 ; X86-NEXT:  # %bb.2: # %exit
 ; X86-NEXT:    addl %esi, %edx
-; X86-NEXT:    movl %edx, 16(%eax)
+; X86-NEXT:    leal 1(%esi,%edx), %ecx
+; X86-NEXT:    movl %ecx, 16(%eax)
 ; X86-NEXT:    popl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 8
+; X86-NEXT:    popl %edi
 ; X86-NEXT:    .cfi_def_cfa_offset 4
 ; X86-NEXT:    retl
  entry:
diff --git a/llvm/test/CodeGen/X86/lea-opt-cse4.ll b/llvm/test/CodeGen/X86/lea-opt-cse4.ll
index 40868d11eb7d5..4fa9acd99bb2f 100644
--- a/llvm/test/CodeGen/X86/lea-opt-cse4.ll
+++ b/llvm/test/CodeGen/X86/lea-opt-cse4.ll
@@ -12,10 +12,11 @@ define void @foo(ptr nocapture %ctx, i32 %n) local_unnamed_addr #0 {
 ; X64-NEXT:    addl %eax, %ecx
 ; X64-NEXT:    addl %eax, %ecx
 ; X64-NEXT:    addl %eax, %ecx
+; X64-NEXT:    leal (%rcx,%rax), %edx
 ; X64-NEXT:    leal 1(%rax,%rcx), %ecx
 ; X64-NEXT:    movl %ecx, 12(%rdi)
-; X64-NEXT:    addl %eax, %ecx
-; X64-NEXT:    movl %ecx, 16(%rdi)
+; X64-NEXT:    leal 1(%rax,%rdx), %eax
+; X64-NEXT:    movl %eax, 16(%rdi)
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: foo:
@@ -29,10 +30,11 @@ define void @foo(ptr nocapture %ctx, i32 %n) local_unnamed_addr #0 {
 ; X86-NEXT:    addl %ecx, %edx
 ; X86-NEXT:    addl %ecx, %edx
 ; X86-NEXT:    addl %ecx, %edx
-; X86-NEXT:    leal 1(%ecx,%edx), %edx
-; X86-NEXT:    movl %edx, 12(%eax)
+; X86-NEXT:    leal 1(%ecx,%edx), %esi
 ; X86-NEXT:    addl %ecx, %edx
-; X86-NEXT:    movl %edx, 16(%eax)
+; X86-NEXT:    movl %esi, 12(%eax)
+; X86-NEXT:    leal 1(%ecx,%edx), %ecx
+; X86-NEXT:    movl %ecx, 16(%eax)
 ; X86-NEXT:    popl %esi
 ; X86-NEXT:    .cfi_def_cfa_offset 4
 ; X86-NEXT:    retl
@@ -62,43 +64,50 @@ define void @foo_loop(ptr nocapture %ctx, i32 %n) local_unnamed_addr #0 {
 ; X64-NEXT:    # =>This Inner Loop Header: Depth=1
 ; X64-NEXT:    movl (%rdi), %ecx
 ; X64-NEXT:    movl 16(%rdi), %eax
-; X64-NEXT:    leal 1(%rcx,%rax), %ecx
-; X64-NEXT:    movl %ecx, 12(%rdi)
+; X64-NEXT:    leal 1(%rcx,%rax), %edx
+; X64-NEXT:    movl %edx, 12(%rdi)
 ; X64-NEXT:    decl %esi
 ; X64-NEXT:    jne .LBB1_1
 ; X64-NEXT:  # %bb.2: # %exit
+; X64-NEXT:    addl %eax, %ecx
+; X64-NEXT:    leal 1(%rax,%rcx), %ecx
 ; X64-NEXT:    leal (%rax,%rax), %edx
+; X64-NEXT:    addl %eax, %edx
 ; X64-NEXT:    addl %edx, %ecx
-; X64-NEXT:    addl %edx, %eax
-; X64-NEXT:    addl %ecx, %eax
-; X64-NEXT:    addl %edx, %eax
-; X64-NEXT:    movl %eax, 16(%rdi)
+; X64-NEXT:    addl %edx, %ecx
+; X64-NEXT:    movl %ecx, 16(%rdi)
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: foo_loop:
 ; X86:       # %bb.0: # %entry
-; X86-NEXT:    pushl %esi
+; X86-NEXT:    pushl %edi
 ; X86-NEXT:    .cfi_def_cfa_offset 8
-; X86-NEXT:    .cfi_offset %esi, -8
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %esi
+; X86-NEXT:    pushl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 12
+; X86-NEXT:    .cfi_offset %esi, -12
+; X86-NEXT:    .cfi_offset %edi, -8
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %edx
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    .p2align 4
 ; X86-NEXT:  .LBB1_1: # %loop
 ; X86-NEXT:    # =>This Inner Loop Header: Depth=1
-; X86-NEXT:    movl (%eax), %edx
+; X86-NEXT:    movl (%eax), %esi
 ; X86-NEXT:    movl 16(%eax), %ecx
-; X86-NEXT:    leal 1(%edx,%ecx), %edx
-; X86-NEXT:    movl %edx, 12(%eax)
-; X86-NEXT:    decl %esi
+; X86-NEXT:    leal 1(%esi,%ecx), %edi
+; X86-NEXT:    movl %edi, 12(%eax)
+; X86-NEXT:    decl %edx
 ; X86-NEXT:    jne .LBB1_1
 ; X86-NEXT:  # %bb.2: # %exit
+; X86-NEXT:    addl %ecx, %esi
+; X86-NEXT:    leal 1(%ecx,%esi), %edx
 ; X86-NEXT:    leal (%ecx,%ecx), %esi
+; X86-NEXT:    addl %ecx, %esi
 ; X86-NEXT:    addl %esi, %edx
-; X86-NEXT:    addl %esi, %ecx
-; X86-NEXT:    addl %edx, %ecx
-; X86-NEXT:    addl %esi, %ecx
-; X86-NEXT:    movl %ecx, 16(%eax)
+; X86-NEXT:    addl %esi, %edx
+; X86-NEXT:    movl %edx, 16(%eax)
 ; X86-NEXT:    popl %esi
+; X86-NEXT:    .cfi_def_cfa_offset 8
+; X86-NEXT:    popl %edi
 ; X86-NEXT:    .cfi_def_cfa_offset 4
 ; X86-NEXT:    retl
  entry:
diff --git a/llvm/test/CodeGen/X86/lea-recursion.ll b/llvm/test/CodeGen/X86/lea-recursion.ll
index a8bfe8b3b675c..07a550fa394d6 100644
--- a/llvm/test/CodeGen/X86/lea-recursion.ll
+++ b/llvm/test/CodeGen/X86/lea-recursion.ll
@@ -17,25 +17,31 @@ define dso_local void @foo() {
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    movl g0(%rip), %eax
 ; CHECK-NEXT:    movl g1(%rip), %ecx
+; CHECK-NEXT:    leal (%rax,%rcx), %edx
 ; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
 ; CHECK-NEXT:    movl %eax, g0+4(%rip)
-; CHECK-NEXT:    movl g1+4(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+4(%rip), %eax
+; CHECK-NEXT:    leal 1(%rax,%rdx), %ecx
+; CHECK-NEXT:    leal 2(%rax,%rdx), %eax
 ; CHECK-NEXT:    movl %eax, g0+8(%rip)
-; CHECK-NEXT:    movl g1+8(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+8(%rip), %eax
+; CHECK-NEXT:    leal 1(%rax,%rcx), %edx
+; CHECK-NEXT:    leal 2(%rax,%rcx), %eax
 ; CHECK-NEXT:    movl %eax, g0+12(%rip)
-; CHECK-NEXT:    movl g1+12(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+12(%rip), %eax
+; CHECK-NEXT:    leal 1(%rax,%rdx), %ecx
+; CHECK-NEXT:    leal 2(%rax,%rdx), %eax
 ; CHECK-NEXT:    movl %eax, g0+16(%rip)
-; CHECK-NEXT:    movl g1+16(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+16(%rip), %eax
+; CHECK-NEXT:    leal 1(%rax,%rcx), %edx
+; CHECK-NEXT:    leal 2(%rax,%rcx), %eax
 ; CHECK-NEXT:    movl %eax, g0+20(%rip)
-; CHECK-NEXT:    movl g1+20(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+20(%rip), %eax
+; CHECK-NEXT:    leal 1(%rax,%rdx), %ecx
+; CHECK-NEXT:    leal 2(%rax,%rdx), %eax
 ; CHECK-NEXT:    movl %eax, g0+24(%rip)
-; CHECK-NEXT:    movl g1+24(%rip), %ecx
-; CHECK-NEXT:    leal 1(%rax,%rcx), %eax
+; CHECK-NEXT:    movl g1+24(%rip), %eax
+; CHECK-NEXT:    leal 2(%rax,%rcx), %eax
 ; CHECK-NEXT:    movl %eax, g0+28(%rip)
 ; CHECK-NEXT:    retq
 entry:
diff --git a/llvm/test/CodeGen/X86/pr51707.ll b/llvm/test/CodeGen/X86/pr51707.ll
deleted file mode 100644
index ffe4b7b34aaaa..0000000000000
--- a/llvm/test/CodeGen/X86/pr51707.ll
+++ /dev/null
@@ -1,251 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
-; RUN: llc -mtriple=x86_64-- < %s | FileCheck %s
-
-; Issue #51707: add2 = add1 + b should reuse the already-materialized value of
-; add1 (a + b + 17) instead of rematerializing a + b.
-
-define i32 @reduced(i32 %a, i32 %b, ptr %p) {
-; CHECK-LABEL: reduced:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $esi killed $esi def $rsi
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    leal 17(%rdi,%rsi), %eax
-; CHECK-NEXT:    movl %eax, (%rdx)
-; CHECK-NEXT:    addl %esi, %eax
-; CHECK-NEXT:    retq
-  %add = add i32 %a, 17
-  %add1 = add i32 %add, %b
-  store i32 %add1, ptr %p, align 4
-  %add2 = add nsw i32 %add1, %b
-  ret i32 %add2
-}
-
-; int32_t f(int32_t & __restrict a, const int32_t & __restrict b) {
-;   a += b + 17;
-;   return a + b;
-; }
-define i32 @f(ptr noalias %a, ptr noalias readonly %b) {
-; CHECK-LABEL: f:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl (%rsi), %ecx
-; CHECK-NEXT:    movl (%rdi), %eax
-; CHECK-NEXT:    leal 17(%rcx,%rax), %eax
-; CHECK-NEXT:    movl %eax, (%rdi)
-; CHECK-NEXT:    addl %ecx, %eax
-; CHECK-NEXT:    retq
-  %lb = load i32, ptr %b, align 4
-  %la = load i32, ptr %a, align 4
-  %t = add i32 %lb, 17
-  %sum = add i32 %t, %la
-  store i32 %sum, ptr %a, align 4
-  %ret = add i32 %sum, %lb
-  ret i32 %ret
-}
-
-; Commuted: the reused value is the second operand of the final add.
-define i32 @commuted(i32 %a, i32 %b, i32 %c, ptr %p) {
-; CHECK-LABEL: commuted:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $esi killed $esi def $rsi
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    leal 17(%rdi,%rsi), %eax
-; CHECK-NEXT:    movl %eax, (%rcx)
-; CHECK-NEXT:    addl %edx, %eax
-; CHECK-NEXT:    retq
-  %s = add i32 %a, %b
-  %add1 = add i32 %s, 17
-  store i32 %add1, ptr %p
-  %r = add i32 %c, %add1
-  ret i32 %r
-}
-
-; Deeper nesting: v = ((a + b) + c) + 17 is materialized, then reused for v + d.
-define i32 @deeper(i32 %a, i32 %b, i32 %c, i32 %d, ptr %p) {
-; CHECK-LABEL: deeper:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $edx killed $edx def $rdx
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    addl %esi, %edi
-; CHECK-NEXT:    leal 17(%rdx,%rdi), %eax
-; CHECK-NEXT:    movl %eax, (%r8)
-; CHECK-NEXT:    addl %ecx, %eax
-; CHECK-NEXT:    retq
-  %s1 = add i32 %a, %b
-  %s2 = add i32 %s1, %c
-  %v = add i32 %s2, 17
-  store i32 %v, ptr %p
-  %r = add i32 %v, %d
-  ret i32 %r
-}
-
-; A multi-use shl-by-constant of an add folds to a scaled index; the materialized
-; value m = (a + b) << 2 should be reused for m + c, not recomputed.
-define i32 @shl_reuse(i32 %a, i32 %b, i32 %c, ptr %p) {
-; CHECK-LABEL: shl_reuse:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $esi killed $esi def $rsi
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    leal (%rdi,%rsi), %eax
-; CHECK-NEXT:    shll $2, %eax
-; CHECK-NEXT:    movl %eax, (%rcx)
-; CHECK-NEXT:    addl %edx, %eax
-; CHECK-NEXT:    retq
-  %s = add i32 %a, %b
-  %m = shl i32 %s, 2
-  store i32 %m, ptr %p
-  %r = add i32 %m, %c
-  ret i32 %r
-}
-
-; Boundary/other-operand coverage: the following already produce optimal code
-; (with or without the reuse fix); they guard against a future matcher change
-; re-introducing the de-CSE.
-
-; A shift by 4 cannot fold into an LEA scale (max is <<3 == scale 8), so m is
-; kept whole regardless - nothing to split.
-define i32 @shl_by_4(i32 %a, i32 %b, i32 %c, ptr %p) {
-; CHECK-LABEL: shl_by_4:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $esi killed $esi def $rsi
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    leal (%rdi,%rsi), %eax
-; CHECK-NEXT:    shll $4, %eax
-; CHECK-NEXT:    movl %eax, (%rcx)
-; CHECK-NEXT:    addl %edx, %eax
-; CHECK-NEXT:    retq
-  %s = add i32 %a, %b
-  %m = shl i32 %s, 4
-  store i32 %m, ptr %p
-  %r = add i32 %m, %c
-  ret i32 %r
-}
-
-; mul by 3/5/9 folds to lea (X, X, {2,4,8}), consuming both base and index, so
-; the sibling add operand cannot be folded and m is reused whole.
-define i32 @mul_3(i32 %a, i32 %b, i32 %c, ptr %p) {
-; CHECK-LABEL: mul_3:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    addl %esi, %edi
-; CHECK-NEXT:    leal (...
[truncated]

@github-actions

Copy link
Copy Markdown

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Target/X86/X86ISelDAGToDAG.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 9a0045367..7847dbbc9 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -2068,8 +2068,8 @@ bool X86DAGToDAGISel::matchAdd(SDValue &N, X86ISelAddressMode &AM,
   HandleSDNode Handle(N);
 
   X86ISelAddressMode Backup = AM;
-  if (!matchAddressRecursively(N.getOperand(0), AM, Depth+1) &&
-      !matchAddressRecursively(Handle.getValue().getOperand(1), AM, Depth+1))
+  if (!matchAddressRecursively(N.getOperand(0), AM, Depth + 1) &&
+      !matchAddressRecursively(Handle.getValue().getOperand(1), AM, Depth + 1))
     return false;
   AM = Backup;
 

@mustartt
mustartt merged commit 9a606af into llvm:main Jul 24, 2026
11 of 14 checks passed
@mustartt
mustartt deleted the revert-580604f branch July 24, 2026 23:46
@mustartt

Copy link
Copy Markdown
Member Author

#210739

nickitat added a commit to nickitat/llvm-project that referenced this pull request Jul 26, 2026
The LEA already-materialized-value reuse path in matchAdd places an operand
directly as a base/index register via matchAddressBase, bypassing the check in
matchAddressRecursively that only immediates may merge into an address that is
already %rip-relative. When one addend is a RIP-relative global and the other is
a materialized multi-use value, the value was folded in as an index on top of
the RIP base, forming an address with both a RIP base and an index register.
That is invalid: [%rip + disp32] takes no register beyond RIP itself (its
implicit base) - no additional base and no index - so it asserted "Invalid
rip-relative address" in X86MCCodeEmitter::emitMemModRMByte.

Only take the reuse shortcut when AM is not yet %rip-relative; otherwise fall
back to matchAddressRecursively, which correctly refuses to fold a register into
a %rip-relative address, so matchAdd keeps the operands separate and emits a
legal LEA. The materialized value is still reused (kept whole) as before.

This fixes the crash that caused llvm#210739 to be reverted in llvm#211958.
midhuncodes7 pushed a commit to midhuncodes7/llvm-project that referenced this pull request Jul 28, 2026
phoebewang pushed a commit that referenced this pull request Jul 28, 2026
This reapplies #210739, which was reverted in #211958 because it could
crash the backend with "Invalid rip-relative address" when an LEA added
a RIP-relative global and an already-materialized value.

Fixes #51707
asudarsa-qti pushed a commit to asudarsa-qti/llvm-project that referenced this pull request Jul 29, 2026
asudarsa-qti pushed a commit to asudarsa-qti/llvm-project that referenced this pull request Jul 29, 2026
This reapplies llvm#210739, which was reverted in llvm#211958 because it could
crash the backend with "Invalid rip-relative address" when an LEA added
a RIP-relative global and an already-materialized value.

Fixes llvm#51707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant