Skip to content

True fixpoint algorithm in RS4GC #75826

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.

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

Merged
merged 3 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
94 changes: 48 additions & 46 deletions llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down
34 changes: 31 additions & 3 deletions llvm/test/Transforms/RewriteStatepointsForGC/base-pointers.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down Expand Up @@ -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()