[llvm] [NewGVN] Prevent cyclic dependencies by ensuring Leader has min RPO number (PR #82110)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 1 02:36:31 PST 2024
https://github.com/ManuelJBrito updated https://github.com/llvm/llvm-project/pull/82110
>From 29801c553fdd96d86492622897605fb9026bbd80 Mon Sep 17 00:00:00 2001
From: ManuelJBrito <manuel.brito at tecnico.ulisboa.pt>
Date: Sat, 17 Feb 2024 12:24:41 +0000
Subject: [PATCH] RPO_Leader
---
llvm/lib/Transforms/Scalar/NewGVN.cpp | 49 ++++++++++------
...6335-phi-undef.ll => pr36335-phi-undef.ll} | 5 +-
llvm/test/Transforms/NewGVN/pr69211.ll | 58 +++++++++++++++++++
llvm/test/Transforms/NewGVN/simp-to-self.ll | 21 +++++--
4 files changed, 108 insertions(+), 25 deletions(-)
rename llvm/test/Transforms/NewGVN/{todo-pr36335-phi-undef.ll => pr36335-phi-undef.ll} (86%)
create mode 100644 llvm/test/Transforms/NewGVN/pr69211.ll
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 19ac9526b5f88b..6770ffd1244098 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -297,7 +297,8 @@ class CongruenceClass {
using MemoryMemberSet = SmallPtrSet<const MemoryMemberType *, 2>;
explicit CongruenceClass(unsigned ID) : ID(ID) {}
- CongruenceClass(unsigned ID, Value *Leader, const Expression *E)
+ CongruenceClass(unsigned ID, std::pair<Value *, unsigned int> Leader,
+ const Expression *E)
: ID(ID), RepLeader(Leader), DefiningExpr(E) {}
unsigned getID() const { return ID; }
@@ -311,15 +312,21 @@ class CongruenceClass {
}
// Leader functions
- Value *getLeader() const { return RepLeader; }
- void setLeader(Value *Leader) { RepLeader = Leader; }
+ Value *getLeader() const { return RepLeader.first; }
+ void setLeader(std::pair<Value *, unsigned int> Leader) {
+ RepLeader = Leader;
+ }
const std::pair<Value *, unsigned int> &getNextLeader() const {
return NextLeader;
}
void resetNextLeader() { NextLeader = {nullptr, ~0}; }
- void addPossibleNextLeader(std::pair<Value *, unsigned int> LeaderPair) {
- if (LeaderPair.second < NextLeader.second)
+ void addPossibleLeader(std::pair<Value *, unsigned int> LeaderPair) {
+ if (LeaderPair.second < RepLeader.second) {
+ NextLeader = RepLeader;
+ RepLeader = LeaderPair;
+ } else if (LeaderPair.second < NextLeader.second) {
NextLeader = LeaderPair;
+ }
}
Value *getStoredValue() const { return RepStoredValue; }
@@ -374,9 +381,10 @@ class CongruenceClass {
if (this == Other)
return true;
- if (std::tie(StoreCount, RepLeader, RepStoredValue, RepMemoryAccess) !=
- std::tie(Other->StoreCount, Other->RepLeader, Other->RepStoredValue,
- Other->RepMemoryAccess))
+ if (std::tie(StoreCount, RepLeader.first, RepStoredValue,
+ RepMemoryAccess) !=
+ std::tie(Other->StoreCount, Other->RepLeader.first,
+ Other->RepStoredValue, Other->RepMemoryAccess))
return false;
if (DefiningExpr != Other->DefiningExpr)
if (!DefiningExpr || !Other->DefiningExpr ||
@@ -393,7 +401,7 @@ class CongruenceClass {
unsigned ID;
// Representative leader.
- Value *RepLeader = nullptr;
+ std::pair<Value *, unsigned int> RepLeader = {nullptr, ~0U};
// The most dominating leader after our current leader, because the member set
// is not sorted and is expensive to keep sorted all the time.
@@ -731,7 +739,13 @@ class NewGVN {
// Congruence class handling.
CongruenceClass *createCongruenceClass(Value *Leader, const Expression *E) {
- auto *result = new CongruenceClass(NextCongruenceNum++, Leader, E);
+ unsigned LeaderDFS = 0;
+ if (!Leader)
+ LeaderDFS = ~0;
+ else if (auto *I = dyn_cast<Instruction>(Leader))
+ LeaderDFS = InstrToDFSNum(I);
+ auto *result =
+ new CongruenceClass(NextCongruenceNum++, {Leader, LeaderDFS}, E);
CongruenceClasses.emplace_back(result);
return result;
}
@@ -2245,7 +2259,8 @@ void NewGVN::moveValueToNewCongruenceClass(Instruction *I, const Expression *E,
NewClass->insert(I);
if (NewClass->getLeader() != I)
- NewClass->addPossibleNextLeader({I, InstrToDFSNum(I)});
+ NewClass->addPossibleLeader({I, InstrToDFSNum(I)});
+
// Handle our special casing of stores.
if (auto *SI = dyn_cast<StoreInst>(I)) {
OldClass->decStoreCount();
@@ -2269,7 +2284,7 @@ void NewGVN::moveValueToNewCongruenceClass(Instruction *I, const Expression *E,
<< " because store joined class\n");
// If we changed the leader, we have to mark it changed because we don't
// know what it will do to symbolic evaluation.
- NewClass->setLeader(SI);
+ NewClass->setLeader({SI, InstrToDFSNum(SI)});
}
// We rely on the code below handling the MemoryAccess change.
}
@@ -2315,7 +2330,8 @@ void NewGVN::moveValueToNewCongruenceClass(Instruction *I, const Expression *E,
if (OldClass->getStoredValue())
OldClass->setStoredValue(nullptr);
}
- OldClass->setLeader(getNextValueLeader(OldClass));
+ OldClass->setLeader({getNextValueLeader(OldClass),
+ InstrToDFSNum(getNextValueLeader(OldClass))});
OldClass->resetNextLeader();
markValueLeaderChangeTouched(OldClass);
}
@@ -2354,15 +2370,15 @@ void NewGVN::performCongruenceFinding(Instruction *I, const Expression *E) {
// Constants and variables should always be made the leader.
if (const auto *CE = dyn_cast<ConstantExpression>(E)) {
- NewClass->setLeader(CE->getConstantValue());
+ NewClass->setLeader({CE->getConstantValue(), 0});
} else if (const auto *SE = dyn_cast<StoreExpression>(E)) {
StoreInst *SI = SE->getStoreInst();
- NewClass->setLeader(SI);
+ NewClass->setLeader({SI, InstrToDFSNum(SI)});
NewClass->setStoredValue(SE->getStoredValue());
// The RepMemoryAccess field will be filled in properly by the
// moveValueToNewCongruenceClass call.
} else {
- NewClass->setLeader(I);
+ NewClass->setLeader({I, InstrToDFSNum(I)});
}
assert(!isa<VariableExpression>(E) &&
"VariableExpression should have been handled already");
@@ -3253,7 +3269,6 @@ void NewGVN::verifyMemoryCongruency() const {
return ReachableEdges.count(
{FirstMP->getIncomingBlock(U), FirstMP->getBlock()}) &&
isa<MemoryDef>(U);
-
};
// All arguments should in the same class, ignoring unreachable arguments
auto FilteredPhiArgs =
diff --git a/llvm/test/Transforms/NewGVN/todo-pr36335-phi-undef.ll b/llvm/test/Transforms/NewGVN/pr36335-phi-undef.ll
similarity index 86%
rename from llvm/test/Transforms/NewGVN/todo-pr36335-phi-undef.ll
rename to llvm/test/Transforms/NewGVN/pr36335-phi-undef.ll
index f3f3d2bccb68e2..839e7175b813c7 100644
--- a/llvm/test/Transforms/NewGVN/todo-pr36335-phi-undef.ll
+++ b/llvm/test/Transforms/NewGVN/pr36335-phi-undef.ll
@@ -1,8 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes=newgvn -S %s | FileCheck %s
-; TODO: NewGVN currently miscomiles the function below. PR36335.
-
declare void @foo(i32)
define void @main(i1 %c1, i1 %c2, i32 %x) {
@@ -11,7 +9,8 @@ define void @main(i1 %c1, i1 %c2, i32 %x) {
; CHECK-NEXT: br i1 [[C1:%.*]], label [[L:%.*]], label [[END:%.*]]
; CHECK: L:
; CHECK-NEXT: [[XOR:%.*]] = xor i32 [[X:%.*]], -1
-; CHECK-NEXT: call void @foo(i32 [[XOR]])
+; CHECK-NEXT: [[NEG:%.*]] = xor i32 [[XOR]], -1
+; CHECK-NEXT: call void @foo(i32 [[NEG]])
; CHECK-NEXT: br label [[L]]
; CHECK: end:
; CHECK-NEXT: ret void
diff --git a/llvm/test/Transforms/NewGVN/pr69211.ll b/llvm/test/Transforms/NewGVN/pr69211.ll
new file mode 100644
index 00000000000000..dd6a0ec438657f
--- /dev/null
+++ b/llvm/test/Transforms/NewGVN/pr69211.ll
@@ -0,0 +1,58 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=newgvn -S | FileCheck %s
+
+define i32 @main() {
+; CHECK-LABEL: @main(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY9:%.*]]
+; CHECK: for.body9:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[TMP0:%.*]], [[FOR_BODY9]] ]
+; CHECK-NEXT: [[TMP0]] = add i64 1, [[INDVARS_IV]]
+; CHECK-NEXT: [[TMP1:%.*]] = add i64 [[TMP0]], 1
+; CHECK-NEXT: br label [[FOR_BODY9]]
+;
+entry:
+ br label %for.body9
+
+for.body9: ; preds = %for.body9, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body9 ]
+ %0 = add i64 1, %indvars.iv
+ %1 = add i64 %0, 1
+ %2 = trunc i64 %1 to i32
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ br label %for.body9
+}
+
+declare void @dummy(i64)
+
+define i32 @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_BODY:%.*]]
+; CHECK: for.body:
+; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[TMP0:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT: [[TMP0]] = add i64 [[INDVARS_IV]], 1
+; CHECK-NEXT: [[TMP1:%.*]] = add i64 [[TMP0]], 1
+; CHECK-NEXT: call void @dummy(i64 [[TMP1]])
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i64 [[TMP0]], 1000
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_BODY]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+ %0 = add i64 %indvars.iv, 1
+ %1 = add i64 %0, 1
+ call void @dummy(i64 %1)
+
+ %indvars.iv.next = add i64 %indvars.iv, 1
+ %tobool = icmp slt i64 %indvars.iv.next, 1000
+ br i1 %tobool, label %for.body, label %end
+
+end:
+ ret i32 0
+}
+
diff --git a/llvm/test/Transforms/NewGVN/simp-to-self.ll b/llvm/test/Transforms/NewGVN/simp-to-self.ll
index fb8a01962959bc..97d70b52103b36 100644
--- a/llvm/test/Transforms/NewGVN/simp-to-self.ll
+++ b/llvm/test/Transforms/NewGVN/simp-to-self.ll
@@ -1,13 +1,24 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
; RUN: opt -S < %s -passes=newgvn | FileCheck %s
-; CHECK-LABEL: for.cond:
-; CHECK-NEXT: %lv = load i32, ptr @a
-; CHECK-NEXT: %bf.clear = and i32 %lv, -131072
-; CHECK-NEXT: %bf.set = or i32 1, %bf.clear
-; CHECK-NEXT: br i1 %bc, label %for.cond, label %exit
@a = external global i64
define void @fn1(i1 %bc) {
+; CHECK-LABEL: define void @fn1(
+; CHECK-SAME: i1 [[BC:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[FOR_COND:%.*]]
+; CHECK: for.cond:
+; CHECK-NEXT: [[LV:%.*]] = load i32, ptr @a, align 4
+; CHECK-NEXT: [[BF_CLEAR:%.*]] = and i32 [[LV]], -131072
+; CHECK-NEXT: [[BF_SET:%.*]] = or i32 1, [[BF_CLEAR]]
+; CHECK-NEXT: [[BF_CLEAR_1:%.*]] = and i32 [[BF_SET]], -131072
+; CHECK-NEXT: [[BF_SET_1:%.*]] = or i32 1, [[BF_CLEAR_1]]
+; CHECK-NEXT: br i1 [[BC]], label [[FOR_COND]], label [[EXIT:%.*]]
+; CHECK: exit:
+; CHECK-NEXT: store i32 [[BF_SET_1]], ptr @a, align 4
+; CHECK-NEXT: ret void
+;
entry:
br label %for.cond
More information about the llvm-commits
mailing list