Skip to content

Commit 3d29c41

Browse files
committed
[InstCombine] Insert instructions before adding them to worklist
Summary: This patch adds instructions to the InstCombine worklist after they are properly inserted. This way we don't get `<badref>`s printed when logging added instructions. It also adds a check in `Worklist::Add` that ensures that all added instructions have parents. Simple test case that illustrates the difference when run with `--debug-only=instcombine`: ``` define i32 @test35(i32 %a, i32 %b) { %1 = or i32 %a, 1135 %2 = or i32 %1, %b ret i32 %2 } ``` Before this patch: ``` INSTCOMBINE ITERATION #1 on test35 IC: ADDING: 3 instrs to worklist IC: Visiting: %1 = or i32 %a, 1135 IC: Visiting: %2 = or i32 %1, %b IC: ADD: %2 = or i32 %a, %b IC: Old = %3 = or i32 %1, %b New = <badref> = or i32 %2, 1135 IC: ADD: <badref> = or i32 %2, 1135 ... ``` With this patch: ``` INSTCOMBINE ITERATION #1 on test35 IC: ADDING: 3 instrs to worklist IC: Visiting: %1 = or i32 %a, 1135 IC: Visiting: %2 = or i32 %1, %b IC: ADD: %2 = or i32 %a, %b IC: Old = %3 = or i32 %1, %b New = <badref> = or i32 %2, 1135 IC: ADD: %3 = or i32 %2, 1135 ... ``` Reviewers: fhahn, davide, spatel, foad, grosser, nikic Reviewed By: nikic Subscribers: nikic, lebedev.ri, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D71093
1 parent 11d5fa6 commit 3d29c41

File tree

4 files changed

+14
-9
lines changed

4 files changed

+14
-9
lines changed

llvm/include/llvm/Transforms/InstCombine/InstCombineWorklist.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ namespace llvm {
2424
/// InstCombineWorklist - This is the worklist management logic for
2525
/// InstCombine.
2626
class InstCombineWorklist {
27-
SmallVector<Instruction*, 256> Worklist;
28-
DenseMap<Instruction*, unsigned> WorklistMap;
27+
SmallVector<Instruction *, 256> Worklist;
28+
DenseMap<Instruction *, unsigned> WorklistMap;
2929

3030
public:
3131
InstCombineWorklist() = default;
@@ -38,6 +38,9 @@ class InstCombineWorklist {
3838
/// Add - Add the specified instruction to the worklist if it isn't already
3939
/// in it.
4040
void Add(Instruction *I) {
41+
assert(I);
42+
assert(I->getParent() && "Instruction not inserted yet?");
43+
4144
if (WorklistMap.insert(std::make_pair(I, Worklist.size())).second) {
4245
LLVM_DEBUG(dbgs() << "IC: ADD: " << *I << '\n');
4346
Worklist.push_back(I);

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -1189,15 +1189,17 @@ Instruction *InstCombiner::visitZExt(ZExtInst &CI) {
11891189
// zext (or icmp, icmp) -> or (zext icmp), (zext icmp)
11901190
Value *LCast = Builder.CreateZExt(LHS, CI.getType(), LHS->getName());
11911191
Value *RCast = Builder.CreateZExt(RHS, CI.getType(), RHS->getName());
1192-
BinaryOperator *Or = BinaryOperator::Create(Instruction::Or, LCast, RCast);
1192+
Value *Or = Builder.CreateOr(LCast, RCast, CI.getName());
1193+
if (auto *OrInst = dyn_cast<Instruction>(Or))
1194+
Builder.SetInsertPoint(OrInst);
11931195

11941196
// Perform the elimination.
11951197
if (auto *LZExt = dyn_cast<ZExtInst>(LCast))
11961198
transformZExtICmp(LHS, *LZExt);
11971199
if (auto *RZExt = dyn_cast<ZExtInst>(RCast))
11981200
transformZExtICmp(RHS, *RZExt);
11991201

1200-
return Or;
1202+
return replaceInstUsesWith(CI, Or);
12011203
}
12021204
}
12031205

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -3361,10 +3361,6 @@ bool InstCombiner::run() {
33613361
// Move the name to the new instruction first.
33623362
Result->takeName(I);
33633363

3364-
// Push the new instruction and any users onto the worklist.
3365-
Worklist.AddUsersToWorkList(*Result);
3366-
Worklist.Add(Result);
3367-
33683364
// Insert the new instruction into the basic block...
33693365
BasicBlock *InstParent = I->getParent();
33703366
BasicBlock::iterator InsertPos = I->getIterator();
@@ -3376,6 +3372,10 @@ bool InstCombiner::run() {
33763372

33773373
InstParent->getInstList().insert(InsertPos, Result);
33783374

3375+
// Push the new instruction and any users onto the worklist.
3376+
Worklist.AddUsersToWorkList(*Result);
3377+
Worklist.Add(Result);
3378+
33793379
eraseInstFromFunction(*I);
33803380
} else {
33813381
LLVM_DEBUG(dbgs() << "IC: Mod = " << OrigI << '\n'

llvm/test/Transforms/InstCombine/zext-or-icmp.ll

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ define i8 @zext_or_icmp_icmp(i8 %a, i8 %b) {
1515
; CHECK-NEXT: %toBool2 = icmp eq i8 %b, 0
1616
; CHECK-NEXT: %toBool22 = zext i1 %toBool2 to i8
1717
; CHECK-NEXT: %1 = xor i8 %mask, 1
18-
; CHECK-NEXT: %zext = or i8 %1, %toBool22
18+
; CHECK-NEXT: %zext3 = or i8 %1, %toBool22
1919
; CHECK-NEXT: ret i8 %zext
2020
}
2121

0 commit comments

Comments
 (0)