[llvm] [NewGVN] Prevent cyclic reference when building phiofops (PR #69418)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 21 11:12:42 PDT 2023
https://github.com/XChy updated https://github.com/llvm/llvm-project/pull/69418
>From 7c836285f54b94624544c7aaa9e6cc319cb21a23 Mon Sep 17 00:00:00 2001
From: XChy <xxs_chy at outlook.com>
Date: Wed, 18 Oct 2023 01:20:43 +0800
Subject: [PATCH] [NewGVN] Prevent cyclic reference when building phiofops
---
llvm/lib/Transforms/Scalar/NewGVN.cpp | 68 ++++++++-
.../Transforms/NewGVN/cyclic-phi-handling.ll | 140 +++++++++++++++++-
2 files changed, 195 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 19ac9526b5f88b6..76c4c39f8ab708b 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -191,6 +191,15 @@ struct TarjanSCC {
FindSCC(Start);
}
+ // ExtraEdges is designed to handle the value edges possibly but not yet
+ // inserted.
+ void
+ Start(const Instruction *Start,
+ const DenseMap<const Value *, SmallVector<Value *, 4>> &ExtraEdges) {
+ if (Root.lookup(Start) == 0)
+ FindSCC(Start, ExtraEdges);
+ }
+
const SmallPtrSetImpl<const Value *> &getComponentFor(const Value *V) const {
unsigned ComponentID = ValueToComponent.lookup(V);
@@ -201,17 +210,30 @@ struct TarjanSCC {
private:
void FindSCC(const Instruction *I) {
+ FindSCC(I, DenseMap<const Value *, SmallVector<Value *, 4>>());
+ }
+
+ void
+ FindSCC(const Instruction *I,
+ const DenseMap<const Value *, SmallVector<Value *, 4>> &ExtraEdges) {
Root[I] = ++DFSNum;
// Store the DFS Number we had before it possibly gets incremented.
unsigned int OurDFS = DFSNum;
- for (const auto &Op : I->operands()) {
- if (auto *InstOp = dyn_cast<Instruction>(Op)) {
- if (Root.lookup(Op) == 0)
- FindSCC(InstOp);
- if (!InComponent.count(Op))
- Root[I] = std::min(Root.lookup(I), Root.lookup(Op));
+
+ auto ProcessValueSuccs = [&](Value *Succ) {
+ if (auto *InstOp = dyn_cast<Instruction>(Succ)) {
+ if (Root.lookup(Succ) == 0)
+ FindSCC(InstOp, ExtraEdges);
+ if (!InComponent.count(Succ))
+ Root[I] = std::min(Root.lookup(I), Root.lookup(Succ));
}
- }
+ };
+
+ llvm::for_each(I->operands(), ProcessValueSuccs);
+
+ if (ExtraEdges.count(I))
+ llvm::for_each(ExtraEdges.lookup(I), ProcessValueSuccs);
+
// See if we really were the root of a component, by seeing if we still have
// our DFSNumber. If we do, we are the root of the component, and we have
// completed a component. If we do not, we are not the root of a component,
@@ -3808,13 +3830,45 @@ Value *NewGVN::findPHIOfOpsLeader(const Expression *E,
if (alwaysAvailable(CC->getLeader()))
return CC->getLeader();
+ // TODO: May too expensive to perform SCC finding here
+ TarjanSCC CycleFinder;
+ DenseMap<const Value *, SmallVector<Value *, 4>> Edges;
+ Value *OrigInstLeader =
+ lookupOperandLeader(const_cast<Value *>((const Value *)OrigInst));
+ auto *LeaderInst = dyn_cast<Instruction>(OrigInstLeader);
+
+ SmallVector<Value *, 4> Members(CC->begin(), CC->end());
+ Edges[OrigInst] = Members;
+
+ CycleFinder.Start(OrigInst, Edges);
+ if (LeaderInst) {
+ Edges[LeaderInst] = Members;
+ CycleFinder.Start(LeaderInst, Edges);
+ }
+
+ const SmallPtrSetImpl<const Value *> &Empty = SmallPtrSet<const Value *, 1>();
+ auto &SCC = CycleFinder.getComponentFor(OrigInst);
+ auto &LeaderSCC =
+ LeaderInst ? CycleFinder.getComponentFor(OrigInstLeader) : Empty;
+
for (auto *Member : *CC) {
auto *MemberInst = dyn_cast<Instruction>(Member);
if (MemberInst == OrigInst)
continue;
+
// Anything that isn't an instruction is always available.
if (!MemberInst)
return Member;
+
+ // Prevent cyclic reference, such as:
+ // %a = add i64 %phi, 1
+ // %foundinst = add i64 %a, 1
+ // =>
+ // %phiofops = phi i64 [1, %BB1], [%foundinst, %BB2]
+ // %foundinst = add i64 %phiofops, 1
+ if (SCC.contains(Member) || LeaderSCC.contains(Member))
+ continue;
+
if (DT->dominates(getBlockForValue(MemberInst), BB))
return Member;
}
diff --git a/llvm/test/Transforms/NewGVN/cyclic-phi-handling.ll b/llvm/test/Transforms/NewGVN/cyclic-phi-handling.ll
index 4a2f0b972c9fe1a..0c589acc95a0a4b 100644
--- a/llvm/test/Transforms/NewGVN/cyclic-phi-handling.ll
+++ b/llvm/test/Transforms/NewGVN/cyclic-phi-handling.ll
@@ -5,15 +5,15 @@ target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
define void @foo(i32 %arg, i32 %arg1, ptr %arg2) {
; CHECK-LABEL: @foo(
; CHECK-NEXT: bb:
-; CHECK-NEXT: br label %bb3
+; CHECK-NEXT: br label [[BB3:%.*]]
; CHECK: bb3:
-; CHECK-NEXT: [[TMP:%.*]] = phi i32 [ %arg1, %bb ], [ [[TMP:%.*]]4, %bb7 ]
-; CHECK-NEXT: [[TMP4:%.*]] = phi i32 [ %arg, %bb ], [ [[TMP]], %bb7 ]
-; CHECK-NEXT: [[TMP5:%.*]] = call i32 %arg2(i32 [[TMP4]], i32 [[TMP]])
+; CHECK-NEXT: [[TMP:%.*]] = phi i32 [ [[ARG1:%.*]], [[BB:%.*]] ], [ [[TMP4:%.*]], [[BB7:%.*]] ]
+; CHECK-NEXT: [[TMP4]] = phi i32 [ [[ARG:%.*]], [[BB]] ], [ [[TMP]], [[BB7]] ]
+; CHECK-NEXT: [[TMP5:%.*]] = call i32 [[ARG2:%.*]](i32 [[TMP4]], i32 [[TMP]])
; CHECK-NEXT: [[TMP6:%.*]] = icmp ne i32 [[TMP5]], 0
-; CHECK-NEXT: br i1 [[TMP6]], label %bb7, label %bb8
+; CHECK-NEXT: br i1 [[TMP6]], label [[BB7]], label [[BB8:%.*]]
; CHECK: bb7:
-; CHECK-NEXT: br label %bb3
+; CHECK-NEXT: br label [[BB3]]
; CHECK: bb8:
; CHECK-NEXT: ret void
;
@@ -35,3 +35,131 @@ bb7: ; preds = %bb3
bb8: ; preds = %bb3
ret void
}
+
+; Don't let %b be the candidate when making %a phi of ops
+define i64 @phis_of_ops_cyclic() {
+; CHECK-LABEL: @phis_of_ops_cyclic(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[A:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[A]] = add i64 [[INDVARS_IV]], 1
+; CHECK-NEXT: [[B:%.*]] = add i64 [[A]], 1
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i64 [[A]], 100
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_BODY]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i64 [[B]]
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %a = add i64 %indvars.iv, 1
+ %b = add i64 %a, 1
+
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ %tobool = icmp slt i64 %indvars.iv.next, 100
+ br i1 %tobool, label %for.body, label %end
+
+end:
+ ret i64 %b
+}
+
+define i64 @phis_of_ops_cyclic_multiple() {
+; CHECK-LABEL: @phis_of_ops_cyclic_multiple(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[A:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[A]] = add i64 [[INDVARS_IV]], 1
+; CHECK-NEXT: [[B:%.*]] = add i64 [[A]], 1
+; CHECK-NEXT: [[D:%.*]] = add i64 [[B]], 1
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i64 [[A]], 100
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_BODY]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i64 [[D]]
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %a = add i64 %indvars.iv, 1
+ %b = add i64 %a, 1
+ %c = add i64 %a, 1
+ %d = add i64 %b, 1
+
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ %tobool = icmp slt i64 %indvars.iv.next, 100
+ br i1 %tobool, label %for.body, label %end
+
+end:
+ ret i64 %d
+}
+
+define i64 @phis_of_ops_cyclic_indirect1(i64 %e) {
+; CHECK-LABEL: @phis_of_ops_cyclic_indirect1(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[ORIGINAL:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[ORIGINAL]] = add i64 [[INDVARS_IV]], 1
+; CHECK-NEXT: [[B:%.*]] = add i64 [[ORIGINAL]], 1
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i64 [[ORIGINAL]], 100
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_BODY]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i64 [[B]]
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %original = add i64 %indvars.iv, 1
+
+ %b = add i64 %original, 1
+ %c = sub i64 %b, 1
+
+ %phileader = add i64 %c, 1
+
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ %tobool = icmp slt i64 %indvars.iv.next, 100
+ br i1 %tobool, label %for.body, label %end
+
+end:
+ ret i64 %phileader
+}
+
+define i64 @phis_of_ops_cyclic_indirect2(i64 %e) {
+; CHECK-LABEL: @phis_of_ops_cyclic_indirect2(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[ORIGINAL:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[ORIGINAL]] = add i64 [[INDVARS_IV]], 1
+; CHECK-NEXT: [[B:%.*]] = add i64 [[ORIGINAL]], 1
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i64 [[ORIGINAL]], 100
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_BODY]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i64 [[B]]
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %original = add i64 %indvars.iv, 1
+
+ %b = add i64 %original, 1
+ %c = sub i64 %b, 1
+
+ %phileader = add i64 %c, 1
+
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ %tobool = icmp slt i64 %indvars.iv.next, 100
+ br i1 %tobool, label %for.body, label %end
+
+end:
+ ret i64 %phileader
+}
More information about the llvm-commits
mailing list