Skip to content

Commit 1536e29

Browse files
committed
InstSimplify: Require instruction be parented
Unlike every other analysis and transform, simplifyInstruction permitted operating on instructions which are not inserted into a function. This created an edge case no other code needs to really worry about, and limited transforms in cases that can make use of the context function. Only the inliner and a handful of other utilities were making use of this, so just fix up these edge cases. Results in some IR ordering differences since cloned blocks are inserted eagerly now. Plus some additional simplifications trigger (e.g. some add 0s now folded out that previously didn't).
1 parent 12d967c commit 1536e29

File tree

16 files changed

+45
-64
lines changed

16 files changed

+45
-64
lines changed

llvm/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ Changes to LLVM infrastructure
7575
legacy inliner pass. Backend stack coloring should handle cases alloca
7676
merging initially set out to handle.
7777

78+
* InstructionSimplify APIs now require instructions be inserted into a
79+
parent function.
80+
7881
Changes to building LLVM
7982
------------------------
8083

llvm/include/llvm/Analysis/InstructionSimplify.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,8 @@
1919
// values. This will prevent other code from seeing the same undef uses and
2020
// resolving them to different values.
2121
//
22-
// These routines are designed to tolerate moderately incomplete IR, such as
23-
// instructions that are not connected to basic blocks yet. However, they do
24-
// require that all the IR that they encounter be valid. In particular, they
25-
// require that all non-constant values be defined in the same function, and the
26-
// same call context of that function (and not split between caller and callee
27-
// contexts of a directly recursive call, for example).
22+
// They require that all the IR that they encounter be valid and inserted into a
23+
// parent function.
2824
//
2925
// Additionally, these routines can't simplify to the instructions that are not
3026
// def-reachable, meaning we can't just scan the basic block for instructions

llvm/include/llvm/IR/BasicBlock.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,10 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
251251

252252
/// Unlink this basic block from its current function and insert it into
253253
/// the function that \p MovePos lives in, right before \p MovePos.
254-
void moveBefore(BasicBlock *MovePos);
254+
inline void moveBefore(BasicBlock *MovePos) {
255+
moveBefore(MovePos->getIterator());
256+
}
257+
void moveBefore(SymbolTableList<BasicBlock>::iterator MovePos);
255258

256259
/// Unlink this basic block from its current function and insert it
257260
/// right after \p MovePos in the function \p MovePos lives in.

llvm/lib/Analysis/InstructionSimplify.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6748,6 +6748,7 @@ static Value *simplifyInstructionWithOperands(Instruction *I,
67486748
ArrayRef<Value *> NewOps,
67496749
const SimplifyQuery &SQ,
67506750
unsigned MaxRecurse) {
6751+
assert(I->getFunction() && "instruction should be inserted in a function");
67516752
const SimplifyQuery Q = SQ.CxtI ? SQ : SQ.getWithInstruction(I);
67526753

67536754
switch (I->getOpcode()) {

llvm/lib/IR/BasicBlock.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,8 @@ iplist<BasicBlock>::iterator BasicBlock::eraseFromParent() {
133133
return getParent()->getBasicBlockList().erase(getIterator());
134134
}
135135

136-
void BasicBlock::moveBefore(BasicBlock *MovePos) {
137-
MovePos->getParent()->splice(MovePos->getIterator(), getParent(),
138-
getIterator());
136+
void BasicBlock::moveBefore(SymbolTableList<BasicBlock>::iterator MovePos) {
137+
getParent()->splice(MovePos, getParent(), getIterator());
139138
}
140139

141140
void BasicBlock::moveAfter(BasicBlock *MovePos) {

llvm/lib/Transforms/Scalar/JumpThreading.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2643,6 +2643,7 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
26432643
// mapping and using it to remap operands in the cloned instructions.
26442644
for (; BI != BB->end(); ++BI) {
26452645
Instruction *New = BI->clone();
2646+
New->insertInto(PredBB, OldPredBranch->getIterator());
26462647

26472648
// Remap operands to patch up intra-block references.
26482649
for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i)
@@ -2660,7 +2661,7 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
26602661
{BB->getModule()->getDataLayout(), TLI, nullptr, nullptr, New})) {
26612662
ValueMapping[&*BI] = IV;
26622663
if (!New->mayHaveSideEffects()) {
2663-
New->deleteValue();
2664+
New->eraseFromParent();
26642665
New = nullptr;
26652666
}
26662667
} else {
@@ -2669,7 +2670,6 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred(
26692670
if (New) {
26702671
// Otherwise, insert the new instruction into the block.
26712672
New->setName(BI->getName());
2672-
New->insertInto(PredBB, OldPredBranch->getIterator());
26732673
// Update Dominance from simplified New instruction operands.
26742674
for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i)
26752675
if (BasicBlock *SuccBB = dyn_cast<BasicBlock>(New->getOperand(i)))

llvm/lib/Transforms/Utils/CloneFunction.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -470,9 +470,8 @@ void PruningFunctionCloner::CloneBlock(
470470

471471
// Nope, clone it now.
472472
BasicBlock *NewBB;
473-
BBEntry = NewBB = BasicBlock::Create(BB->getContext());
474-
if (BB->hasName())
475-
NewBB->setName(BB->getName() + NameSuffix);
473+
Twine NewName(BB->hasName() ? Twine(BB->getName()) + NameSuffix : "");
474+
BBEntry = NewBB = BasicBlock::Create(BB->getContext(), NewName, NewFunc);
476475

477476
// It is only legal to clone a function if a block address within that
478477
// function is never referenced outside of the function. Given that, we
@@ -498,6 +497,7 @@ void PruningFunctionCloner::CloneBlock(
498497
++II) {
499498

500499
Instruction *NewInst = cloneInstruction(II);
500+
NewInst->insertInto(NewBB, NewBB->end());
501501

502502
if (HostFuncIsStrictFP) {
503503
// All function calls in the inlined function must get 'strictfp'
@@ -516,8 +516,6 @@ void PruningFunctionCloner::CloneBlock(
516516
// If we can simplify this instruction to some other value, simply add
517517
// a mapping to that value rather than inserting a new instruction into
518518
// the basic block.
519-
//
520-
// FIXME: simplifyInstruction should know the context of the new function.
521519
if (Value *V =
522520
simplifyInstruction(NewInst, BB->getModule()->getDataLayout())) {
523521
// On the off-chance that this simplifies to an instruction in the old
@@ -528,7 +526,7 @@ void PruningFunctionCloner::CloneBlock(
528526

529527
if (!NewInst->mayHaveSideEffects()) {
530528
VMap[&*II] = V;
531-
NewInst->deleteValue();
529+
NewInst->eraseFromParent();
532530
continue;
533531
}
534532
}
@@ -537,7 +535,6 @@ void PruningFunctionCloner::CloneBlock(
537535
if (II->hasName())
538536
NewInst->setName(II->getName() + NameSuffix);
539537
VMap[&*II] = NewInst; // Add instruction map to value.
540-
NewInst->insertInto(NewBB, NewBB->end());
541538
if (isa<CallInst>(II) && !II->isDebugOrPseudoInst()) {
542539
hasCalls = true;
543540
hasMemProfMetadata |= II->hasMetadata(LLVMContext::MD_memprof);
@@ -685,8 +682,8 @@ void llvm::CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
685682
if (!NewBB)
686683
continue; // Dead block.
687684

688-
// Add the new block to the new function.
689-
NewFunc->insert(NewFunc->end(), NewBB);
685+
// Move the new block to preserve the order in the original function.
686+
NewBB->moveBefore(NewFunc->end());
690687

691688
// Handle PHI nodes specially, as we have to remove references to dead
692689
// blocks.

llvm/lib/Transforms/Utils/LoopRotationUtils.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,8 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
435435

436436
// Otherwise, create a duplicate of the instruction.
437437
Instruction *C = Inst->clone();
438+
C->insertBefore(LoopEntryBranch);
439+
438440
++NumInstrsDuplicated;
439441

440442
// Eagerly remap the operands of the instruction.
@@ -444,7 +446,7 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
444446
// Avoid inserting the same intrinsic twice.
445447
if (auto *DII = dyn_cast<DbgVariableIntrinsic>(C))
446448
if (DbgIntrinsics.count(makeHash(DII))) {
447-
C->deleteValue();
449+
C->eraseFromParent();
448450
continue;
449451
}
450452

@@ -457,7 +459,7 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
457459
// in the map.
458460
InsertNewValueIntoMap(ValueMap, Inst, V);
459461
if (!C->mayHaveSideEffects()) {
460-
C->deleteValue();
462+
C->eraseFromParent();
461463
C = nullptr;
462464
}
463465
} else {
@@ -466,7 +468,6 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
466468
if (C) {
467469
// Otherwise, stick the new instruction into the new block!
468470
C->setName(Inst->getName());
469-
C->insertBefore(LoopEntryBranch);
470471

471472
if (auto *II = dyn_cast<AssumeInst>(C))
472473
AC->registerAssumption(II);

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3211,6 +3211,9 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
32113211
}
32123212
// Clone the instruction.
32133213
Instruction *N = BBI->clone();
3214+
// Insert the new instruction into its new home.
3215+
N->insertInto(EdgeBB, InsertPt);
3216+
32143217
if (BBI->hasName())
32153218
N->setName(BBI->getName() + ".c");
32163219

@@ -3226,17 +3229,15 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
32263229
if (!BBI->use_empty())
32273230
TranslateMap[&*BBI] = V;
32283231
if (!N->mayHaveSideEffects()) {
3229-
N->deleteValue(); // Instruction folded away, don't need actual inst
3232+
N->eraseFromParent(); // Instruction folded away, don't need actual
3233+
// inst
32303234
N = nullptr;
32313235
}
32323236
} else {
32333237
if (!BBI->use_empty())
32343238
TranslateMap[&*BBI] = N;
32353239
}
32363240
if (N) {
3237-
// Insert the new instruction into its new home.
3238-
N->insertInto(EdgeBB, InsertPt);
3239-
32403241
// Register the new instruction with the assumption cache if necessary.
32413242
if (auto *Assume = dyn_cast<AssumeInst>(N))
32423243
if (AC)

llvm/test/Transforms/Inline/inline_inv_group.ll

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ define ptr @callee() alwaysinline {
1414
ret ptr %1
1515
}
1616

17-
define ptr @caller() {
18-
; CHECK-LABEL: define ptr @caller() {
17+
define ptr @caller() null_pointer_is_valid {
18+
; CHECK-LABEL: define ptr @caller
19+
; CHECK-SAME: () #[[ATTR1:[0-9]+]] {
1920
; CHECK-NEXT: [[TMP1:%.*]] = call ptr @llvm.strip.invariant.group.p0(ptr null)
2021
; CHECK-NEXT: ret ptr [[TMP1]]
2122
;

llvm/test/Transforms/Inline/simplify-instruction-computeKnownFPClass-context.ll

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ define i1 @simplify_fcmp_ord_fdiv_caller(double nofpclass(zero nan inf) %i0, dou
3030
; CHECK-LABEL: define i1 @simplify_fcmp_ord_fdiv_caller
3131
; CHECK-SAME: (double nofpclass(nan inf zero) [[I0:%.*]], double nofpclass(nan inf zero) [[I1:%.*]]) {
3232
; CHECK-NEXT: [[SUB_DOUBLE_SUB_I:%.*]] = fdiv double [[I0]], [[I1]]
33-
; CHECK-NEXT: [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
34-
; CHECK-NEXT: ret i1 [[CMP_I]]
33+
; CHECK-NEXT: ret i1 true
3534
;
3635
%call = call i1 @simplify_fcmp_ord_fdiv_callee(double %i0, double %i1)
3736
ret i1 %call
@@ -48,8 +47,7 @@ define i1 @simplify_fcmp_ord_frem_caller(double nofpclass(zero nan inf) %i0, dou
4847
; CHECK-LABEL: define i1 @simplify_fcmp_ord_frem_caller
4948
; CHECK-SAME: (double nofpclass(nan inf zero) [[I0:%.*]], double nofpclass(nan inf zero) [[I1:%.*]]) {
5049
; CHECK-NEXT: [[SUB_DOUBLE_SUB_I:%.*]] = frem double [[I0]], [[I1]]
51-
; CHECK-NEXT: [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
52-
; CHECK-NEXT: ret i1 [[CMP_I]]
50+
; CHECK-NEXT: ret i1 true
5351
;
5452
%call = call i1 @simplify_fcmp_ord_frem_callee(double %i0, double %i1)
5553
ret i1 %call
@@ -66,8 +64,7 @@ define i1 @simplify_fcmp_ord_fmul_caller(double nofpclass(zero nan) %i0, double
6664
; CHECK-LABEL: define i1 @simplify_fcmp_ord_fmul_caller
6765
; CHECK-SAME: (double nofpclass(nan zero) [[I0:%.*]], double nofpclass(nan zero) [[I1:%.*]]) {
6866
; CHECK-NEXT: [[SUB_DOUBLE_SUB_I:%.*]] = fmul double [[I0]], [[I1]]
69-
; CHECK-NEXT: [[CMP_I:%.*]] = fcmp ord double [[SUB_DOUBLE_SUB_I]], 0.000000e+00
70-
; CHECK-NEXT: ret i1 [[CMP_I]]
67+
; CHECK-NEXT: ret i1 true
7168
;
7269
%call = call i1 @simplify_fcmp_ord_fmul_callee(double %i0, double %i1)
7370
ret i1 %call

llvm/test/Transforms/LoopRotate/pr56260.ll

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,17 @@ define void @main() {
1414
; CHECK: L0.preheader:
1515
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 0, 0
1616
; CHECK-NEXT: [[INC:%.*]] = zext i1 [[CMP]] to i32
17-
; CHECK-NEXT: [[SPEC_SELECT1:%.*]] = add nsw i32 0, [[INC]]
18-
; CHECK-NEXT: [[TOBOOL3_NOT2:%.*]] = icmp eq i32 [[SPEC_SELECT1]], 0
19-
; CHECK-NEXT: br i1 [[TOBOOL3_NOT2]], label [[L0_PREHEADER_LOOPEXIT]], label [[L1_PREHEADER_LR_PH:%.*]]
17+
; CHECK-NEXT: [[TOBOOL3_NOT1:%.*]] = icmp eq i32 [[INC]], 0
18+
; CHECK-NEXT: br i1 [[TOBOOL3_NOT1]], label [[L0_PREHEADER_LOOPEXIT]], label [[L1_PREHEADER_LR_PH:%.*]]
2019
; CHECK: L1.preheader.lr.ph:
2120
; CHECK-NEXT: br label [[L1_PREHEADER:%.*]]
2221
; CHECK: L1.preheader:
23-
; CHECK-NEXT: [[SPEC_SELECT4:%.*]] = phi i32 [ [[SPEC_SELECT1]], [[L1_PREHEADER_LR_PH]] ], [ [[SPEC_SELECT:%.*]], [[L0_LATCH:%.*]] ]
24-
; CHECK-NEXT: [[K_03:%.*]] = phi i32 [ 0, [[L1_PREHEADER_LR_PH]] ], [ [[SPEC_SELECT4]], [[L0_LATCH]] ]
25-
; CHECK-NEXT: [[TOBOOL8_NOT:%.*]] = icmp eq i32 [[K_03]], 0
22+
; CHECK-NEXT: [[SPEC_SELECT3:%.*]] = phi i32 [ [[INC]], [[L1_PREHEADER_LR_PH]] ], [ [[SPEC_SELECT:%.*]], [[L0_LATCH:%.*]] ]
23+
; CHECK-NEXT: [[K_02:%.*]] = phi i32 [ 0, [[L1_PREHEADER_LR_PH]] ], [ [[SPEC_SELECT3]], [[L0_LATCH]] ]
24+
; CHECK-NEXT: [[TOBOOL8_NOT:%.*]] = icmp eq i32 [[K_02]], 0
2625
; CHECK-NEXT: br label [[L0_LATCH]]
2726
; CHECK: L0.latch:
28-
; CHECK-NEXT: [[SPEC_SELECT]] = add nsw i32 [[SPEC_SELECT4]], [[INC]]
27+
; CHECK-NEXT: [[SPEC_SELECT]] = add nsw i32 [[SPEC_SELECT3]], [[INC]]
2928
; CHECK-NEXT: [[TOBOOL3_NOT:%.*]] = icmp eq i32 [[SPEC_SELECT]], 0
3029
; CHECK-NEXT: br i1 [[TOBOOL3_NOT]], label [[L0_L0_PREHEADER_LOOPEXIT_CRIT_EDGE:%.*]], label [[L1_PREHEADER]]
3130
;

llvm/test/Transforms/SampleProfile/profile-context-tracker-debug.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
; INLINE-ALL-NEXT: Getting callee context for instr: %call.i = tail call i32 @_Z8funcLeafi
2828
; INLINE-ALL-NEXT: Callee context found: main:3 @ _Z5funcAi:1 @ _Z8funcLeafi
2929
; INLINE-ALL-NEXT: Marking context profile as inlined: main:3 @ _Z5funcAi:1 @ _Z8funcLeafi
30-
; INLINE-ALL-NEXT: Getting callee context for instr: %call.i1 = tail call i32 @_Z3fibi
30+
; INLINE-ALL-NEXT: Getting callee context for instr: %call.i2 = tail call i32 @_Z3fibi
3131
; INLINE-ALL-NEXT: Getting callee context for instr: %call5.i = tail call i32 @_Z3fibi
3232
; INLINE-ALL-DAG: Getting base profile for function: _Z5funcAi
3333
; INLINE-ALL-DAG-NEXT: Merging context profile into base profile: _Z5funcAi

llvm/test/Transforms/SampleProfile/pseudo-probe-stale-profile-matching.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@
8080
; CHECK: 5: call void @llvm.pseudoprobe(i64 -2624081020897602054, i64 5, i32 0, i64 -1), !dbg ![[#]] - weight: 0 - factor: 1.00)
8181
; CHECK: 1: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 1, i32 0, i64 -1), !dbg ![[#]] - weight: 112 - factor: 1.00)
8282
; CHECK: 2: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 2, i32 0, i64 -1), !dbg ![[#]] - weight: 101 - factor: 1.00)
83-
; CHECK: 5: %call.i3 = call i32 @bar(i32 noundef %1), !dbg ![[#]] - weight: 101 - factor: 1.00)
83+
; CHECK: 5: %call.i8 = call i32 @bar(i32 noundef %1), !dbg ![[#]] - weight: 101 - factor: 1.00)
8484
; CHECK: 3: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 3, i32 0, i64 -1), !dbg ![[#]] - weight: 13 - factor: 1.00)
85-
; CHECK: 6: %call1.i6 = call i32 @bar(i32 noundef %add.i5), !dbg ![[#]] - weight: 13 - factor: 1.00)
85+
; CHECK: 6: %call1.i5 = call i32 @bar(i32 noundef %add.i4), !dbg ![[#]] - weight: 13 - factor: 1.00)
8686
; CHECK: 4: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 4, i32 0, i64 -1), !dbg ![[#]] - weight: 112 - factor: 1.00)
8787
; CHECK: 14: %call2 = call i32 @bar(i32 noundef %3), !dbg ![[#]] - weight: 124 - factor: 1.00)
8888
; CHECK: 8: call void @llvm.pseudoprobe(i64 -2624081020897602054, i64 8, i32 0, i64 -1), !dbg ![[#]] - weight: 0 - factor: 1.00)

llvm/test/Transforms/SimplifyCFG/pr46638.ll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ define void @pr46638(i1 %c, i32 %x) {
1515
; CHECK: common.ret:
1616
; CHECK-NEXT: ret void
1717
; CHECK: true2.critedge:
18-
; CHECK-NEXT: [[CMP2_C:%.*]] = icmp sgt i32 [[X]], 0
19-
; CHECK-NEXT: [[EXT_C:%.*]] = zext i1 [[CMP2_C]] to i32
20-
; CHECK-NEXT: call void @dummy(i32 [[EXT_C]])
18+
; CHECK-NEXT: call void @dummy(i32 0)
2119
; CHECK-NEXT: call void @dummy(i32 2)
2220
; CHECK-NEXT: br label [[COMMON_RET]]
2321
;

llvm/unittests/Transforms/Utils/LocalTest.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -588,21 +588,6 @@ TEST_F(SalvageDebugInfoTest, RecursiveBlockSimplification) {
588588
verifyDebugValuesAreSalvaged();
589589
}
590590

591-
TEST(Local, SimplifyVScaleWithRange) {
592-
LLVMContext C;
593-
Module M("Module", C);
594-
595-
IntegerType *Ty = Type::getInt32Ty(C);
596-
Function *VScale = Intrinsic::getDeclaration(&M, Intrinsic::vscale, {Ty});
597-
auto *CI = CallInst::Create(VScale, {}, "vscale");
598-
599-
// Test that simplifyCall won't try to query it's parent function for
600-
// vscale_range attributes in order to simplify llvm.vscale -> constant.
601-
EXPECT_EQ(simplifyCall(CI, VScale, {}, SimplifyQuery(M.getDataLayout())),
602-
nullptr);
603-
delete CI;
604-
}
605-
606591
TEST(Local, wouldInstructionBeTriviallyDead) {
607592
LLVMContext Ctx;
608593
std::unique_ptr<Module> M = parseIR(Ctx,

0 commit comments

Comments
 (0)