diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp index 0cba8739441bc..6591b0a433b2b 100644 --- a/llvm/lib/Transforms/Scalar/NewGVN.cpp +++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp @@ -3564,11 +3564,9 @@ struct NewGVN::ValueDFS { int DFSOut = 0; int LocalNum = 0; - // Only one of Def and U will be set. // The bool in the Def tells us whether the Def is the stored value of a // store. PointerIntPair Def; - Use *U = nullptr; bool operator<(const ValueDFS &Other) const { // It's not enough that any given field be less than - we have sets @@ -3585,33 +3583,8 @@ struct NewGVN::ValueDFS { // Each LLVM instruction only produces one value, and thus the lowest-level // differentiator that really matters for the stack (and what we use as a // replacement) is the local dfs number. - // Everything else in the structure is instruction level, and only affects - // the order in which we will replace operands of a given instruction. - // - // For a given instruction (IE things with equal dfsin, dfsout, localnum), - // the order of replacement of uses does not matter. - // IE given, - // a = 5 - // b = a + a - // When you hit b, you will have two valuedfs with the same dfsin, out, and - // localnum. - // The .val will be the same as well. - // The .u's will be different. - // You will replace both, and it does not matter what order you replace them - // in (IE whether you replace operand 2, then operand 1, or operand 1, then - // operand 2). - // Similarly for the case of same dfsin, dfsout, localnum, but different - // .val's - // a = 5 - // b = 6 - // c = a + b - // in c, we will a valuedfs for a, and one for b,with everything the same - // but .val and .u. - // It does not matter what order we replace these operands in. - // You will always end up with the same IR, and this is guaranteed. - return std::tie(DFSIn, DFSOut, LocalNum, Def, U) < - std::tie(Other.DFSIn, Other.DFSOut, Other.LocalNum, Other.Def, - Other.U); + return std::tie(DFSIn, DFSOut, LocalNum, Def) < + std::tie(Other.DFSIn, Other.DFSOut, Other.LocalNum, Other.Def); } }; @@ -3672,30 +3645,14 @@ void NewGVN::convertClassToDFSOrdered( // Don't try to replace into dead uses if (InstructionsToErase.count(I)) continue; - ValueDFS VDUse; // Put the phi node uses in the incoming block. - BasicBlock *IBlock; - if (auto *P = dyn_cast(I)) { - IBlock = P->getIncomingBlock(U); - // Make phi node users appear last in the incoming block - // they are from. - VDUse.LocalNum = InstrDFS.size() + 1; - } else { - IBlock = getBlockForValue(I); - VDUse.LocalNum = InstrToDFSNum(I); - } - + auto *PN = dyn_cast(I); + BasicBlock *IBlock = PN ? PN->getIncomingBlock(U) : getBlockForValue(I); // Skip uses in unreachable blocks, as we're going // to delete them. if (!ReachableBlocks.contains(IBlock)) continue; - - DomTreeNode *DomNode = DT->getNode(IBlock); - VDUse.DFSIn = DomNode->getDFSNumIn(); - VDUse.DFSOut = DomNode->getDFSNumOut(); - VDUse.U = &U; ++UseCount; - DFSOrderedSet.emplace_back(VDUse); } } @@ -4004,7 +3961,6 @@ bool NewGVN::eliminateInstructions(Function &F) { int MemberDFSOut = VD.DFSOut; Value *Def = VD.Def.getPointer(); bool FromStore = VD.Def.getInt(); - Use *U = VD.U; // We ignore void things because we can't get a value from them. if (Def && Def->getType()->isVoidTy()) continue; @@ -4061,101 +4017,48 @@ bool NewGVN::eliminateInstructions(Function &F) { } } - // Skip the Def's, we only want to eliminate on their uses. But mark - // dominated defs as dead. - if (Def) { - // For anything in this case, what and how we value number - // guarantees that any side-effects that would have occurred (ie - // throwing, etc) can be proven to either still occur (because it's - // dominated by something that has the same side-effects), or never - // occur. Otherwise, we would not have been able to prove it value - // equivalent to something else. For these things, we can just mark - // it all dead. Note that this is different from the "ProbablyDead" - // set, which may not be dominated by anything, and thus, are only - // easy to prove dead if they are also side-effect free. Note that - // because stores are put in terms of the stored value, we skip - // stored values here. If the stored value is really dead, it will - // still be marked for deletion when we process it in its own class. - auto *DefI = dyn_cast(Def); - if (!EliminationStack.empty() && DefI && !FromStore) { - Value *DominatingLeader = EliminationStack.back(); - if (DominatingLeader != Def) { + // For anything in this case, what and how we value number + // guarantees that any side-effects that would have occurred (ie + // throwing, etc) can be proven to either still occur (because it's + // dominated by something that has the same side-effects), or never + // occur. Otherwise, we would not have been able to prove it value + // equivalent to something else. For these things, we can just mark + // it all dead. Note that this is different from the "ProbablyDead" + // set, which may not be dominated by anything, and thus, are only + // easy to prove dead if they are also side-effect free. Note that + // because stores are put in terms of the stored value, we skip + // stored values here. If the stored value is really dead, it will + // still be marked for deletion when we process it in its own class. + auto *DefI = dyn_cast(Def); + if (!EliminationStack.empty() && DefI && !FromStore) { + Value *DominatingLeader = EliminationStack.back(); + auto *II = dyn_cast(DominatingLeader); + bool isSSACopy = II && II->getIntrinsicID() == Intrinsic::ssa_copy; + if (isSSACopy) + DominatingLeader = II->getOperand(0); + if (DominatingLeader != Def) { + // All uses of Def are now uses of the dominating leader, which + // means if the dominating leader was dead, it's now live! + auto &LeaderUseCount = UseCounts[DominatingLeader]; + if (LeaderUseCount == 0 && isa(DominatingLeader)) + ProbablyDead.erase(cast(DominatingLeader)); + LeaderUseCount += Def->getNumUses(); + + // Don't patch metadata if ssa_copy is being replaced by their + // original. + auto *PI = PredInfo->getPredicateInfoFor(DefI); + if (PI && DominatingLeader == PI->OriginalOp) { + Def->replaceAllUsesWith(DominatingLeader); + } else { // Even if the instruction is removed, we still need to update // flags/metadata due to downstreams users of the leader. - if (!match(DefI, m_Intrinsic())) - patchReplacementInstruction(DefI, DominatingLeader); - - markInstructionForDeletion(DefI); + patchAndReplaceAllUsesWith(cast(Def), + DominatingLeader); } - } - continue; - } - // At this point, we know it is a Use we are trying to possibly - // replace. - - assert(isa(U->get()) && - "Current def should have been an instruction"); - assert(isa(U->getUser()) && - "Current user should have been an instruction"); - - // If the thing we are replacing into is already marked to be dead, - // this use is dead. Note that this is true regardless of whether - // we have anything dominating the use or not. We do this here - // because we are already walking all the uses anyway. - Instruction *InstUse = cast(U->getUser()); - if (InstructionsToErase.count(InstUse)) { - auto &UseCount = UseCounts[U->get()]; - if (--UseCount == 0) { - ProbablyDead.insert(cast(U->get())); - } - } - - // If we get to this point, and the stack is empty we must have a use - // with nothing we can use to eliminate this use, so just skip it. - if (EliminationStack.empty()) - continue; - - Value *DominatingLeader = EliminationStack.back(); - - auto *II = dyn_cast(DominatingLeader); - bool isSSACopy = II && II->getIntrinsicID() == Intrinsic::ssa_copy; - if (isSSACopy) - DominatingLeader = II->getOperand(0); - - // Don't replace our existing users with ourselves. - if (U->get() == DominatingLeader) - continue; - - // If we replaced something in an instruction, handle the patching of - // metadata. Skip this if we are replacing predicateinfo with its - // original operand, as we already know we can just drop it. - auto *ReplacedInst = cast(U->get()); - auto *PI = PredInfo->getPredicateInfoFor(ReplacedInst); - if (!PI || DominatingLeader != PI->OriginalOp) - patchReplacementInstruction(ReplacedInst, DominatingLeader); - - LLVM_DEBUG(dbgs() - << "Found replacement " << *DominatingLeader << " for " - << *U->get() << " in " << *(U->getUser()) << "\n"); - U->set(DominatingLeader); - // This is now a use of the dominating leader, which means if the - // dominating leader was dead, it's now live! - auto &LeaderUseCount = UseCounts[DominatingLeader]; - // It's about to be alive again. - if (LeaderUseCount == 0 && isa(DominatingLeader)) - ProbablyDead.erase(cast(DominatingLeader)); - // For copy instructions, we use their operand as a leader, - // which means we remove a user of the copy and it may become dead. - if (isSSACopy) { - auto It = UseCounts.find(II); - if (It != UseCounts.end()) { - unsigned &IIUseCount = It->second; - if (--IIUseCount == 0) - ProbablyDead.insert(II); + AnythingReplaced = true; + markInstructionForDeletion(DefI); } } - ++LeaderUseCount; - AnythingReplaced = true; } } } diff --git a/llvm/test/Transforms/NewGVN/commute.ll b/llvm/test/Transforms/NewGVN/commute.ll index e1622fb923716..3ea910c685591 100644 --- a/llvm/test/Transforms/NewGVN/commute.ll +++ b/llvm/test/Transforms/NewGVN/commute.ll @@ -48,6 +48,7 @@ declare i16 @llvm.umul.fix.i16(i16, i16, i32) define i16 @intrinsic_3_args(i16 %x, i16 %y) { ; CHECK-LABEL: @intrinsic_3_args( +; CHECK-NEXT: [[M1:%.*]] = call i16 @llvm.smul.fix.i16(i16 [[X:%.*]], i16 [[Y:%.*]], i32 1) ; CHECK-NEXT: ret i16 0 ; %m1 = call i16 @llvm.smul.fix.i16(i16 %x, i16 %y, i32 1) diff --git a/llvm/test/Transforms/NewGVN/invariant.group.ll b/llvm/test/Transforms/NewGVN/invariant.group.ll index 7c14059c88c67..5c2b621c296de 100644 --- a/llvm/test/Transforms/NewGVN/invariant.group.ll +++ b/llvm/test/Transforms/NewGVN/invariant.group.ll @@ -74,6 +74,7 @@ entry: define i1 @proveEqualityForStrip(ptr %a) { ; CHECK-LABEL: define i1 @proveEqualityForStrip( ; CHECK-SAME: ptr [[A:%.*]]) { +; CHECK-NEXT: [[B1:%.*]] = call ptr @llvm.strip.invariant.group.p0(ptr [[A]]) ; CHECK-NEXT: ret i1 true ; %b1 = call ptr @llvm.strip.invariant.group.p0(ptr %a) diff --git a/llvm/test/Transforms/NewGVN/load-constant-mem.ll b/llvm/test/Transforms/NewGVN/load-constant-mem.ll index ae91147237fad..3362cadb1e22b 100644 --- a/llvm/test/Transforms/NewGVN/load-constant-mem.ll +++ b/llvm/test/Transforms/NewGVN/load-constant-mem.ll @@ -7,6 +7,7 @@ define i32 @test(ptr %p, i32 %i) nounwind { ; CHECK-LABEL: @test( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[P:%.*]] = getelementptr [4 x i32], ptr @G, i32 0, i32 [[I:%.*]] +; CHECK-NEXT: [[A:%.*]] = load i32, ptr [[P]], align 4 ; CHECK-NEXT: store i8 4, ptr [[P1:%.*]], align 1 ; CHECK-NEXT: ret i32 0 ; diff --git a/llvm/test/Transforms/NewGVN/pr31682.ll b/llvm/test/Transforms/NewGVN/pr31682.ll index 2f0d35a1611d2..e7ffefad65589 100644 --- a/llvm/test/Transforms/NewGVN/pr31682.ll +++ b/llvm/test/Transforms/NewGVN/pr31682.ll @@ -12,6 +12,7 @@ define void @bar(i1 %arg) { ; CHECK-NEXT: [[TMP:%.*]] = load ptr, ptr @global, align 8 ; CHECK-NEXT: br label [[BB2:%.*]] ; CHECK: bb2: +; CHECK-NEXT: [[TMP4:%.*]] = getelementptr [[STRUCT_FOO:%.*]], ptr [[TMP]], i64 0, i32 1 ; CHECK-NEXT: br i1 %arg, label [[BB2]], label [[BB7:%.*]] ; CHECK: bb7: ; CHECK-NEXT: br label [[BB10:%.*]] diff --git a/llvm/test/Transforms/NewGVN/tbaa.ll b/llvm/test/Transforms/NewGVN/tbaa.ll index 20c09aa68726a..65827cf39d9a3 100644 --- a/llvm/test/Transforms/NewGVN/tbaa.ll +++ b/llvm/test/Transforms/NewGVN/tbaa.ll @@ -95,6 +95,7 @@ define i32 @test7(ptr %p, ptr %q) { define i32 @test8(ptr %p, ptr %q) { ; CHECK-LABEL: define i32 @test8( ; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]]) { +; CHECK-NEXT: [[A:%.*]] = load i32, ptr [[Q]], align 4, !tbaa [[TBAA7:![0-9]+]] ; CHECK-NEXT: store i32 15, ptr [[P]], align 4 ; CHECK-NEXT: ret i32 0 ; @@ -111,6 +112,7 @@ define i32 @test8(ptr %p, ptr %q) { define i32 @test9(ptr %p, ptr %q) { ; CHECK-LABEL: define i32 @test9( ; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]]) { +; CHECK-NEXT: [[A:%.*]] = load i32, ptr [[Q]], align 4, !tbaa [[TBAA7]] ; CHECK-NEXT: call void @clobber() ; CHECK-NEXT: ret i32 0 ; @@ -172,9 +174,9 @@ declare i32 @foo(ptr) readonly ; CHECK: [[TBAA4]] = !{[[META5:![0-9]+]], [[META5]], i64 0} ; CHECK: [[META5]] = !{!"B", [[META2]]} ; CHECK: [[TBAA6]] = !{[[META2]], [[META2]], i64 0} -; CHECK: [[TBAA7]] = !{[[META8:![0-9]+]], [[META8]], i64 0} -; CHECK: [[META8]] = !{!"scalar type", [[META9:![0-9]+]]} -; CHECK: [[META9]] = !{!"another root"} +; CHECK: [[TBAA7]] = !{[[META8:![0-9]+]], [[META8]], i64 0, i64 1} +; CHECK: [[META8]] = !{!"node", [[META9:![0-9]+]]} +; CHECK: [[META9]] = !{!"yet another root"} ; CHECK: [[TBAA10]] = !{[[META11:![0-9]+]], [[META12:![0-9]+]], i64 0} ; CHECK: [[META11]] = !{!"struct X", [[META12]], i64 0} ; CHECK: [[META12]] = !{!"int", [[META13:![0-9]+]], i64 0}