[llvm] 29c98e5 - Revert "[Inline][Cloning] Defer simplification after phi-nodes resolution" #87963
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 24 14:52:04 PDT 2024
Author: Vitaly Buka
Date: 2024-04-24T14:51:54-07:00
New Revision: 29c98e59cd84ed5c8dde27876779d6b8830ccac5
URL: https://github.com/llvm/llvm-project/commit/29c98e59cd84ed5c8dde27876779d6b8830ccac5
DIFF: https://github.com/llvm/llvm-project/commit/29c98e59cd84ed5c8dde27876779d6b8830ccac5.diff
LOG: Revert "[Inline][Cloning] Defer simplification after phi-nodes resolution" #87963
Reopens #87534.
Breaks multiple bots:
https://lab.llvm.org/buildbot/#/builders/168/builds/20028
https://lab.llvm.org/buildbot/#/builders/74/builds/27773
And reproducer in a61f9fe31750cee65c726fb51f1b14e31e177258.
This reverts commit a61f9fe31750cee65c726fb51f1b14e31e177258.
Added:
Modified:
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/test/Transforms/Inline/inline-deferred-instsimplify.ll
llvm/test/Transforms/Inline/prof-update-sample-alwaysinline.ll
llvm/test/Transforms/Inline/prof-update-sample.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 42e6484844166b..3eac726994ae13 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -14,7 +14,6 @@
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/Analysis/ConstantFolding.h"
#include "llvm/Analysis/DomTreeUpdater.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/LoopInfo.h"
@@ -541,13 +540,18 @@ void PruningFunctionCloner::CloneBlock(
RemapInstruction(NewInst, VMap,
ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges);
- // Eagerly constant fold the newly cloned instruction. If successful, add
- // a mapping to the new value. Non-constant operands may be incomplete at
- // this stage, thus instruction simplification is performed after
- // processing phi-nodes.
- if (Value *V = ConstantFoldInstruction(
- NewInst, BB->getModule()->getDataLayout())) {
- if (isInstructionTriviallyDead(NewInst)) {
+ // If we can simplify this instruction to some other value, simply add
+ // a mapping to that value rather than inserting a new instruction into
+ // the basic block.
+ if (Value *V =
+ simplifyInstruction(NewInst, BB->getModule()->getDataLayout())) {
+ // On the off-chance that this simplifies to an instruction in the old
+ // function, map it back into the new function.
+ if (NewFunc != OldFunc)
+ if (Value *MappedV = VMap.lookup(V))
+ V = MappedV;
+
+ if (!NewInst->mayHaveSideEffects()) {
VMap[&*II] = V;
NewInst->eraseFromParent();
continue;
@@ -819,34 +823,52 @@ void llvm::CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
}
}
- // As phi-nodes have been now remapped, allow incremental simplification of
- // newly-cloned instructions.
+ // Make a second pass over the PHINodes now that all of them have been
+ // remapped into the new function, simplifying the PHINode and performing any
+ // recursive simplifications exposed. This will transparently update the
+ // WeakTrackingVH in the VMap. Notably, we rely on that so that if we coalesce
+ // two PHINodes, the iteration over the old PHIs remains valid, and the
+ // mapping will just map us to the new node (which may not even be a PHI
+ // node).
const DataLayout &DL = NewFunc->getParent()->getDataLayout();
- for (const auto &BB : *OldFunc) {
- for (const auto &I : BB) {
- auto *NewI = dyn_cast_or_null<Instruction>(VMap.lookup(&I));
- if (!NewI)
- continue;
-
- // Skip over non-intrinsic callsites, we don't want to remove any nodes
- // from the CGSCC.
- CallBase *CB = dyn_cast<CallBase>(NewI);
- if (CB && CB->getCalledFunction() &&
- !CB->getCalledFunction()->isIntrinsic())
- continue;
-
- if (Value *V = simplifyInstruction(NewI, DL)) {
- NewI->replaceAllUsesWith(V);
-
- if (isInstructionTriviallyDead(NewI)) {
- NewI->eraseFromParent();
- } else {
- // Did not erase it? Restore the new instruction into VMap previously
- // dropped by `ValueIsRAUWd`.
- VMap[&I] = NewI;
- }
- }
- }
+ SmallSetVector<const Value *, 8> Worklist;
+ for (unsigned Idx = 0, Size = PHIToResolve.size(); Idx != Size; ++Idx)
+ if (isa<PHINode>(VMap[PHIToResolve[Idx]]))
+ Worklist.insert(PHIToResolve[Idx]);
+
+ // Note that we must test the size on each iteration, the worklist can grow.
+ for (unsigned Idx = 0; Idx != Worklist.size(); ++Idx) {
+ const Value *OrigV = Worklist[Idx];
+ auto *I = dyn_cast_or_null<Instruction>(VMap.lookup(OrigV));
+ if (!I)
+ continue;
+
+ // Skip over non-intrinsic callsites, we don't want to remove any nodes from
+ // the CGSCC.
+ CallBase *CB = dyn_cast<CallBase>(I);
+ if (CB && CB->getCalledFunction() &&
+ !CB->getCalledFunction()->isIntrinsic())
+ continue;
+
+ // See if this instruction simplifies.
+ Value *SimpleV = simplifyInstruction(I, DL);
+ if (!SimpleV)
+ continue;
+
+ // Stash away all the uses of the old instruction so we can check them for
+ // recursive simplifications after a RAUW. This is cheaper than checking all
+ // uses of To on the recursive step in most cases.
+ for (const User *U : OrigV->users())
+ Worklist.insert(cast<Instruction>(U));
+
+ // Replace the instruction with its simplified value.
+ I->replaceAllUsesWith(SimpleV);
+
+ // If the original instruction had no side effects, remove it.
+ if (isInstructionTriviallyDead(I))
+ I->eraseFromParent();
+ else
+ VMap[OrigV] = I;
}
// Remap debug intrinsic operands now that all values have been mapped.
diff --git a/llvm/test/Transforms/Inline/inline-deferred-instsimplify.ll b/llvm/test/Transforms/Inline/inline-deferred-instsimplify.ll
index f02d03688f0397..4a9c576f02719e 100644
--- a/llvm/test/Transforms/Inline/inline-deferred-instsimplify.ll
+++ b/llvm/test/Transforms/Inline/inline-deferred-instsimplify.ll
@@ -38,6 +38,8 @@ store_ptr_in_gvar: ; preds = %entry
check_pointers_are_equal: ; preds = %store_ptr_in_gvar, %entry
%phi = phi ptr [ %ptr, %store_ptr_in_gvar ], [ @other_g_var, %entry ]
+; FIXME: While inlining, the following is miscompiled to i1 false,
+; as %ptr in the phi-node is not taken into account.
%.not1 = icmp eq ptr %phi, %ptr
br i1 %.not1, label %return, label %abort
@@ -62,13 +64,9 @@ define i32 @main() {
; CHECK-NEXT: br label [[CHECK_POINTERS_ARE_EQUAL_I]]
; CHECK: check_pointers_are_equal.i:
; CHECK-NEXT: [[PHI_I:%.*]] = phi ptr [ [[G_VAR]], [[STORE_PTR_IN_GVAR_I]] ], [ @other_g_var, [[TMP0:%.*]] ]
-; CHECK-NEXT: [[DOTNOT1_I:%.*]] = icmp eq ptr [[PHI_I]], [[G_VAR]]
-; CHECK-NEXT: br i1 [[DOTNOT1_I]], label [[CALLEE_EXIT:%.*]], label [[ABORT_I:%.*]]
-; CHECK: abort.i:
; CHECK-NEXT: call void @abort()
; CHECK-NEXT: unreachable
; CHECK: callee.exit:
-; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 20, ptr [[G_VAR]])
; CHECK-NEXT: ret i32 0
;
call void @callee(ptr noundef byval(%struct.a) align 8 @g_var)
diff --git a/llvm/test/Transforms/Inline/prof-update-sample-alwaysinline.ll b/llvm/test/Transforms/Inline/prof-update-sample-alwaysinline.ll
index 8af4d89663a4da..d6b771e2629d2e 100644
--- a/llvm/test/Transforms/Inline/prof-update-sample-alwaysinline.ll
+++ b/llvm/test/Transforms/Inline/prof-update-sample-alwaysinline.ll
@@ -53,6 +53,7 @@ define void @caller() {
!18 = !{!"VP", i32 0, i64 140, i64 111, i64 80, i64 222, i64 40, i64 333, i64 20}
attributes #0 = { alwaysinline }
; CHECK: ![[ENTRY_COUNT]] = !{!"function_entry_count", i64 600}
+; CHECK: ![[COUNT_CALLEE1]] = !{!"branch_weights", i32 2000}
; CHECK: ![[COUNT_CALLEE]] = !{!"branch_weights", i32 1200}
; CHECK: ![[COUNT_IND_CALLEE]] = !{!"VP", i32 0, i64 84, i64 111, i64 48, i64 222, i64 24, i64 333, i64 12}
; CHECK: ![[COUNT_CALLER]] = !{!"branch_weights", i32 800}
diff --git a/llvm/test/Transforms/Inline/prof-update-sample.ll b/llvm/test/Transforms/Inline/prof-update-sample.ll
index e09b859b698120..6cdd70e84e0c6d 100644
--- a/llvm/test/Transforms/Inline/prof-update-sample.ll
+++ b/llvm/test/Transforms/Inline/prof-update-sample.ll
@@ -52,6 +52,7 @@ define void @caller() {
!17 = !{!"branch_weights", i32 400}
!18 = !{!"VP", i32 0, i64 140, i64 111, i64 80, i64 222, i64 40, i64 333, i64 20}
; CHECK: ![[ENTRY_COUNT]] = !{!"function_entry_count", i64 600}
+; CHECK: ![[COUNT_CALLEE1]] = !{!"branch_weights", i32 2000}
; CHECK: ![[COUNT_CALLEE]] = !{!"branch_weights", i32 1200}
; CHECK: ![[COUNT_IND_CALLEE]] = !{!"VP", i32 0, i64 84, i64 111, i64 48, i64 222, i64 24, i64 333, i64 12}
; CHECK: ![[COUNT_CALLER]] = !{!"branch_weights", i32 800}
More information about the llvm-commits
mailing list