Skip to content

[NewGVN] Prevent cyclic reference when building phiofops #69418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[NewGVN] Prevent cyclic reference when building phiofops #69418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 63 additions & 7 deletions llvm/lib/Transforms/Scalar/NewGVN.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -3808,13 +3830,47 @@ 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) ||
(Member != OrigInstLeader && LeaderSCC.contains(Member)))
continue;

if (DT->dominates(getBlockForValue(MemberInst), BB))
return Member;
}
Expand Down
140 changes: 134 additions & 6 deletions llvm/test/Transforms/NewGVN/cyclic-phi-handling.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
;
Expand All @@ -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
}