[llvm] 3c246ef - True fixpoint algorithm in RS4GC (#75826)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 22 06:10:08 PST 2024
Author: Petr Maj
Date: 2024-01-22T09:10:04-05:00
New Revision: 3c246efd04210af56ab6ce960b98283ec5bc7c30
URL: https://github.com/llvm/llvm-project/commit/3c246efd04210af56ab6ce960b98283ec5bc7c30
DIFF: https://github.com/llvm/llvm-project/commit/3c246efd04210af56ab6ce960b98283ec5bc7c30.diff
LOG: True fixpoint algorithm in RS4GC (#75826)
Fixes a problem where the explicit marking of various instructions as
conflicts did not propagate to their users. An example of this:
```
%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
```
Here the vector shuffles will get single base (gep) during the fixpoint
and therefore the select will get a known base (gep). We later mark the
shuffles as conflicts, but this does not change the base of select. This
gets caught by an assert where the select's type will differ from its
(wrong) base later on.
The solution in the MR is to move the explicit conflict marking into the
fixpoint phase.
---------
Co-authored-by: Petr Maj <pmaj at azul.com>
Added:
Modified:
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index b98f823ab00b1de..c09983702d6e2fc 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
diff ers 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
diff ers 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
diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll b/llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
index d85e23659177099..3718b4ead2f81b8 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