Skip to content

Commit

Permalink
[CodeGen] Remove redundent instructions generated by combineAddrModes.
Browse files Browse the repository at this point in the history
CodeGenPare may optimize memory access modes.
During such optimization, it might create a new instruction representing combined value.
Later, If the optimization failed, the generated value is not removed and remains a dead instruction.

Normally this won't be a problem as dead code will be eliminated later.
However, in this case (Issue 58538), the generated instruction may trigger an infinite loop.
The infinite loop involves `sinkCmpExpression`, where it tries to optimize the placeholder generated by us.
(See the test case detailed in the issue)

To fix this, we remove the unnecessary placeholder immediately when we abort the optimization.
`AddressingModeCombiner` will keep track of the placeholder, and remove it if it is an inserted placeholder and has no uses.
This patch fixes #58538, a test is also included.

Reviewed By: skatkov

Differential Revision: https://reviews.llvm.org/D147041
  • Loading branch information
DataCorrupted committed Mar 30, 2023
1 parent 8c124e3 commit 670c92a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
15 changes: 14 additions & 1 deletion llvm/lib/CodeGen/CodeGenPrepare.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3559,10 +3559,15 @@ class AddressingModeCombiner {
/// Original Address.
Value *Original;

/// Common value among addresses
Value *CommonValue = nullptr;

public:
AddressingModeCombiner(const SimplifyQuery &_SQ, Value *OriginalValue)
: SQ(_SQ), Original(OriginalValue) {}

~AddressingModeCombiner() { eraseCommonValueIfDead(); }

/// Get the combined AddrMode
const ExtAddrMode &getAddrMode() const { return AddrModes[0]; }

Expand Down Expand Up @@ -3647,13 +3652,21 @@ class AddressingModeCombiner {
if (!initializeMap(Map))
return false;

Value *CommonValue = findCommon(Map);
CommonValue = findCommon(Map);
if (CommonValue)
AddrModes[0].SetCombinedField(DifferentField, CommonValue, AddrModes);
return CommonValue != nullptr;
}

private:
/// `CommonValue` may be a placeholder inserted by us.
/// If the placeholder is not used, we should remove this dead instruction.
void eraseCommonValueIfDead() {
if (CommonValue && CommonValue->getNumUses() == 0)
if (Instruction *CommonInst = dyn_cast<Instruction>(CommonValue))
CommonInst->eraseFromParent();
}

/// Initialize Map with anchor values. For address seen
/// we set the value of different field saw in this address.
/// At the same time we find a common type for different field we will
Expand Down
29 changes: 29 additions & 0 deletions llvm/test/CodeGen/X86/pr58538.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
; RUN: opt -codegenprepare -mtriple=x86_64 %s -S -o - | FileCheck %s
; RUN: opt -codegenprepare -mtriple=i386 %s -S -o - | FileCheck %s

define i32 @f(i32 %0) {
; CHECK-LABEL: @f
; CHECK: BB:
; CHECK: %P0 = alloca i32, i32 8, align 4
; CHECK: %P1 = getelementptr i32, ptr %P0, i32 1
; CHECK: %1 = icmp eq i32 %0, 0
; CHECK: %P2 = getelementptr i1, ptr %P1, i1 %1
; CHECK: %2 = icmp eq i32 %0, 0
; CHECK: %P3 = select i1 %2, ptr %P1, ptr %P2
; CHECK: %L1 = load i32, ptr %P3, align 4
; CHECK: ret i32 %L1
BB:
%P0 = alloca i32, i32 8
%P1 = getelementptr i32, ptr %P0, i32 1
%B0 = icmp eq i32 %0, 0
br label %BB1

BB1: ; preds = %BB1, %BB
%P2 = getelementptr i1, ptr %P1, i1 %B0
br i1 false, label %BB1, label %BB2

BB2: ; preds = %BB1
%P3 = select i1 %B0, ptr %P1, ptr %P2
%L1 = load i32, ptr %P3
ret i32 %L1
}

0 comments on commit 670c92a

Please sign in to comment.