[llvm] True fixpoint algorithm in RS4GC (PR #75826)
Petr Maj via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 20 05:52:20 PST 2023
https://github.com/zduka updated https://github.com/llvm/llvm-project/pull/75826
>From 6a4c55d75ac2c2b35b41549c22b98ab6a3ef4e91 Mon Sep 17 00:00:00 2001
From: Petr Maj <pmaj at azul.com>
Date: Mon, 18 Dec 2023 17:47:45 +0100
Subject: [PATCH 1/3] True fixpoint for RS4GC
---
.../Scalar/RewriteStatepointsForGC.cpp | 94 ++++++++++---------
1 file changed, 48 insertions(+), 46 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 40b4ea92e1ff90..af49074d3832da 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -967,6 +967,44 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
return BDVState(BaseValue, BDVState::Base, BaseValue);
};
+ // Even though we have identified a concrete base (or a conflict) for all live
+ // pointers at this point, there are cases where the base is of an
+ // incompatible type compared to the original instruction. We conservatively
+ // mark those as conflicts to ensure that corresponding BDVs will be generated
+ // in the next steps.
+
+ // this is a rather explicit check for all cases where we should mark the
+ // state as a conflict to force the latter stages of the algorithm to emit
+ // the BDVs.
+ // TODO: in many cases the instructions emited for the conflicting states
+ // will be identical to the I itself (if the I's operate on their BDVs
+ // themselves). We should exploit this, but can't do it here since it would
+ // break the invariant about the BDVs not being known to be a base.
+ // TODO: the code also does not handle constants at all - the algorithm relies
+ // on all constants having the same BDV and therefore constant-only insns
+ // will never be in conflict, but this check is ignored here. If the
+ // constant conflicts will be to BDVs themselves, they will be identical
+ // instructions and will get optimized away (as in the above TODO)
+ auto MarkConflict = [&](Instruction *I, Value *BaseValue) {
+ // II and EE mixes vector & scalar so is always a conflict
+ if (isa<InsertElementInst>(I) || isa<ExtractElementInst>(I))
+ return true;
+ // Shuffle vector is always a conflict as it creates new vector from
+ // existing ones.
+ if (isa<ShuffleVectorInst>(I))
+ return true;
+ // Any instructions where the computed base type differs from the
+ // instruction type. An example is where an extract instruction is used by a
+ // select. Here the select's BDV is a vector (because of extract's BDV),
+ // while the select itself is a scalar type. Note that the IE and EE
+ // instruction check is not fully subsumed by the vector<->scalar check at
+ // the end, this is due to the BDV algorithm being ignorant of BDV types at
+ // this junction.
+ if (!areBothVectorOrScalar(BaseValue, I))
+ return true;
+ return false;
+ };
+
bool Progress = true;
while (Progress) {
#ifndef NDEBUG
@@ -993,6 +1031,14 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
NewState.meet(OpState);
});
+ // if the instruction has known base, but should in fact be marked as
+ // conflict because of incompatible in/out types, we mark it as such
+ // ensuring that it will propagate through the fixpoint iteration
+ auto I = cast<Instruction>(BDV);
+ auto BV = NewState.getBaseValue();
+ if (BV && MarkConflict(I, BV))
+ NewState = BDVState(I, BDVState::Conflict);
+
BDVState OldState = Pair.second;
if (OldState != NewState) {
Progress = true;
@@ -1012,44 +1058,8 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
}
#endif
- // Even though we have identified a concrete base (or a conflict) for all live
- // pointers at this point, there are cases where the base is of an
- // incompatible type compared to the original instruction. We conservatively
- // mark those as conflicts to ensure that corresponding BDVs will be generated
- // in the next steps.
-
- // this is a rather explicit check for all cases where we should mark the
- // state as a conflict to force the latter stages of the algorithm to emit
- // the BDVs.
- // TODO: in many cases the instructions emited for the conflicting states
- // will be identical to the I itself (if the I's operate on their BDVs
- // themselves). We should expoit this, but can't do it here since it would
- // break the invariant about the BDVs not being known to be a base.
- // TODO: the code also does not handle constants at all - the algorithm relies
- // on all constants having the same BDV and therefore constant-only insns
- // will never be in conflict, but this check is ignored here. If the
- // constant conflicts will be to BDVs themselves, they will be identical
- // instructions and will get optimized away (as in the above TODO)
- auto MarkConflict = [&](Instruction *I, Value *BaseValue) {
- // II and EE mixes vector & scalar so is always a conflict
- if (isa<InsertElementInst>(I) || isa<ExtractElementInst>(I))
- return true;
- // Shuffle vector is always a conflict as it creates new vector from
- // existing ones.
- if (isa<ShuffleVectorInst>(I))
- return true;
- // Any instructions where the computed base type differs from the
- // instruction type. An example is where an extract instruction is used by a
- // select. Here the select's BDV is a vector (because of extract's BDV),
- // while the select itself is a scalar type. Note that the IE and EE
- // instruction check is not fully subsumed by the vector<->scalar check at
- // the end, this is due to the BDV algorithm being ignorant of BDV types at
- // this junction.
- if (!areBothVectorOrScalar(BaseValue, I))
- return true;
- return false;
- };
-
+ // since we do the conflict marking as part of the fixpoint iteration this loop
+ // only asserts that invariants are met
for (auto Pair : States) {
Instruction *I = cast<Instruction>(Pair.first);
BDVState State = Pair.second;
@@ -1061,14 +1071,6 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
(!isKnownBase(I, KnownBases) || !areBothVectorOrScalar(I, BaseValue)) &&
"why did it get added?");
assert(!State.isUnknown() && "Optimistic algorithm didn't complete!");
-
- // since we only mark vec-scalar insns as conflicts in the pass, our work is
- // done if the instruction already conflicts
- if (State.isConflict())
- continue;
-
- if (MarkConflict(I, BaseValue))
- States[I] = BDVState(I, BDVState::Conflict);
}
#ifndef NDEBUG
>From eebcff41703e601e08b0615f3cc7ec7d876b4992 Mon Sep 17 00:00:00 2001
From: Petr Maj <pmaj at azul.com>
Date: Mon, 18 Dec 2023 17:49:33 +0100
Subject: [PATCH 2/3] format
---
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index af49074d3832da..76660bf88fe277 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1031,13 +1031,13 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
NewState.meet(OpState);
});
- // if the instruction has known base, but should in fact be marked as
- // conflict because of incompatible in/out types, we mark it as such
+ // if the instruction has known base, but should in fact be marked as
+ // conflict because of incompatible in/out types, we mark it as such
// ensuring that it will propagate through the fixpoint iteration
auto I = cast<Instruction>(BDV);
auto BV = NewState.getBaseValue();
if (BV && MarkConflict(I, BV))
- NewState = BDVState(I, BDVState::Conflict);
+ NewState = BDVState(I, BDVState::Conflict);
BDVState OldState = Pair.second;
if (OldState != NewState) {
@@ -1058,8 +1058,8 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
}
#endif
- // since we do the conflict marking as part of the fixpoint iteration this loop
- // only asserts that invariants are met
+ // since we do the conflict marking as part of the fixpoint iteration this
+ // loop only asserts that invariants are met
for (auto Pair : States) {
Instruction *I = cast<Instruction>(Pair.first);
BDVState State = Pair.second;
>From 61736cc56404d007de91cad13d3c2033f79a5845 Mon Sep 17 00:00:00 2001
From: Petr Maj <pmaj at azul.com>
Date: Wed, 20 Dec 2023 14:09:35 +0100
Subject: [PATCH 3/3] Test added
---
.../RewriteStatepointsForGC/base-pointers.ll | 34 +++++++++++++++++--
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll b/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
index d85e2365917709..3718b4ead2f81b 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
@@ -39,9 +39,9 @@ define ptr addrspace(1) @test1(i32 %caller, ptr addrspace(1) %a, ptr addrspace(1
; CHECK-NEXT: br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
; CHECK: left:
; CHECK-NEXT: switch i32 [[UNKNOWN:%.*]], label [[RIGHT]] [
-; CHECK-NEXT: i32 0, label [[MERGE:%.*]]
-; CHECK-NEXT: i32 1, label [[MERGE]]
-; CHECK-NEXT: i32 5, label [[MERGE]]
+; CHECK-NEXT: i32 0, label [[MERGE:%.*]]
+; CHECK-NEXT: i32 1, label [[MERGE]]
+; CHECK-NEXT: i32 5, label [[MERGE]]
; CHECK-NEXT: ]
; CHECK: right:
; CHECK-NEXT: br label [[MERGE]]
@@ -221,4 +221,32 @@ next:
ret ptr addrspace(1) %bdv
}
+declare i32 @snork()
+
+define void @test7() gc "statepoint-example" {
+; CHECK-LABEL: @test7(
+; CHECK-NEXT: bb:
+; CHECK-NEXT: [[GETELEMENTPTR:%.*]] = getelementptr i8, <2 x ptr addrspace(1)> zeroinitializer, <2 x i64> <i64 888, i64 908>
+; CHECK-NEXT: [[SHUFFLEVECTOR_BASE:%.*]] = shufflevector <2 x ptr addrspace(1)> zeroinitializer, <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>, !is_base_value !0
+; CHECK-NEXT: [[SHUFFLEVECTOR:%.*]] = shufflevector <2 x ptr addrspace(1)> [[GETELEMENTPTR]], <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+; CHECK-NEXT: [[SHUFFLEVECTOR1_BASE:%.*]] = shufflevector <2 x ptr addrspace(1)> zeroinitializer, <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>, !is_base_value !0
+; CHECK-NEXT: [[SHUFFLEVECTOR1:%.*]] = shufflevector <2 x ptr addrspace(1)> [[GETELEMENTPTR]], <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+; CHECK-NEXT: [[SELECT_BASE:%.*]] = select i1 false, <4 x ptr addrspace(1)> [[SHUFFLEVECTOR1_BASE]], <4 x ptr addrspace(1)> [[SHUFFLEVECTOR_BASE]], !is_base_value !0
+; CHECK-NEXT: [[SELECT:%.*]] = select i1 false, <4 x ptr addrspace(1)> [[SHUFFLEVECTOR1]], <4 x ptr addrspace(1)> [[SHUFFLEVECTOR]]
+; CHECK-NEXT: [[STATEPOINT_TOKEN:%.*]] = call token (i64, i32, ptr, i32, i32, ...) @llvm.experimental.gc.statepoint.p0(i64 2882400000, i32 0, ptr elementtype(i32 ()) @snork, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(<4 x ptr addrspace(1)> [[SELECT]], <4 x ptr addrspace(1)> [[SELECT_BASE]]) ]
+; CHECK-NEXT: [[SELECT_RELOCATED:%.*]] = call coldcc <4 x ptr addrspace(1)> @llvm.experimental.gc.relocate.v4p1(token [[STATEPOINT_TOKEN]], i32 1, i32 0)
+; CHECK-NEXT: [[SELECT_BASE_RELOCATED:%.*]] = call coldcc <4 x ptr addrspace(1)> @llvm.experimental.gc.relocate.v4p1(token [[STATEPOINT_TOKEN]], i32 1, i32 1)
+; CHECK-NEXT: [[EXTRACTELEMENT:%.*]] = extractelement <4 x ptr addrspace(1)> [[SELECT_RELOCATED]], i32 0
+; CHECK-NEXT: ret void
+;
+bb:
+ %getelementptr = getelementptr i8, <2 x ptr addrspace(1)> zeroinitializer, <2 x i64> <i64 888, i64 908>
+ %shufflevector = shufflevector <2 x ptr addrspace(1)> %getelementptr, <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+ %shufflevector1 = shufflevector <2 x ptr addrspace(1)> %getelementptr, <2 x ptr addrspace(1)> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
+ %select = select i1 false, <4 x ptr addrspace(1)> %shufflevector1, <4 x ptr addrspace(1)> %shufflevector
+ %call = call i32 @snork()
+ %extractelement = extractelement <4 x ptr addrspace(1)> %select, i32 0
+ ret void
+}
+
declare void @foo()
More information about the llvm-commits
mailing list