Skip to content

Commit

Permalink
Refine liveness of phi and select
Browse files Browse the repository at this point in the history 
If both inputs are rooted then we don't need to root the result.
Rename `LoadRefinements` to `Refinements` since it's not load specific anymore.

Fix #22421
  • Loading branch information
@yuyichao
yuyichao committed Oct 9, 2017
1 parent 1da3faf commit 5769cb0
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 25 deletions.
77 changes: 52 additions & 25 deletions src/llvm-late-gc-lowering.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,9 @@ struct State {
// The result of the local analysis
std::map<BasicBlock *, BBState> BBStates;

// Load refinement map. All uses of the keys can be combined with uses
// of the value (but not the other way around).
std::map<int, int> LoadRefinements;
// Refinement map. If all of the values are rooted (-1 means a globally rooted value),
// the key is already rooted (but not the other way around).
std::map<int, SmallVector<int, 1>> Refinements;

// GC preserves map. All safepoints dominated by the map key, but not any
// of its uses need to preserve the values listed in the map value.
Expand Down Expand Up @@ -334,7 +334,7 @@ struct LateLowerGCFrame: public FunctionPass {
Function *big_alloc_func;
CallInst *ptlsStates;

void MaybeNoteDef(State &S, BBState &BBS, Value *Def, const std::vector<int> &SafepointsSoFar, int RefinedPtr = -2);
void MaybeNoteDef(State &S, BBState &BBS, Value *Def, const std::vector<int> &SafepointsSoFar, SmallVector<int, 1> &&RefinedPtr = SmallVector<int, 1>());
void NoteUse(State &S, BBState &BBS, Value *V, BitVector &Uses);
void NoteUse(State &S, BBState &BBS, Value *V) {
NoteUse(S, BBS, V, BBS.UpExposedUses);
Expand Down Expand Up @@ -575,7 +575,7 @@ static void NoteDef(State &S, BBState &BBS, int Num, const std::vector<int> &Saf
}
}

void LateLowerGCFrame::MaybeNoteDef(State &S, BBState &BBS, Value *Def, const std::vector<int> &SafepointsSoFar, int RefinedPtr) {
void LateLowerGCFrame::MaybeNoteDef(State &S, BBState &BBS, Value *Def, const std::vector<int> &SafepointsSoFar, SmallVector<int, 1> &&RefinedPtr) {
int Num = -1;
Type *RT = Def->getType();
if (isSpecialPtr(RT)) {
Expand All @@ -591,17 +591,17 @@ void LateLowerGCFrame::MaybeNoteDef(State &S, BBState &BBS, Value *Def, const st
std::vector<int> Nums = NumberVector(S, Def);
for (int Num : Nums) {
NoteDef(S, BBS, Num, SafepointsSoFar);
if (RefinedPtr != -2)
S.LoadRefinements[Num] = RefinedPtr;
if (!RefinedPtr.empty())
S.Refinements[Num] = RefinedPtr;
}
return;
}
else {
return;
}
NoteDef(S, BBS, Num, SafepointsSoFar);
if (RefinedPtr != -2)
S.LoadRefinements[Num] = RefinedPtr;
if (!RefinedPtr.empty())
S.Refinements[Num] = std::move(RefinedPtr);
}

static int NoteSafepoint(State &S, BBState &BBS, CallInst *CI) {
Expand Down Expand Up @@ -814,21 +814,21 @@ State LateLowerGCFrame::LocalScan(Function &F) {
// object we're loading from is, so we can refine uses
// of this object to uses of the object we're loading
// from.
int RefinedPtr = -2;
SmallVector<int, 1> RefinedPtr{};
if (isLoadFromImmut(LI) && isSpecialPtr(LI->getPointerOperand()->getType())) {
RefinedPtr = Number(S, LI->getPointerOperand());
RefinedPtr.push_back(Number(S, LI->getPointerOperand()));
} else if (LI->getType()->isPointerTy() &&
isSpecialPtr(LI->getType()) &&
LooksLikeFrameRef(LI->getPointerOperand())) {
// Loads from a jlcall argument array
RefinedPtr = -1;
RefinedPtr.push_back(-1);
}
else if (isLoadFromConstGV(LI)) {
// If this is a const load from a global,
// we know that the object is a constant as well and doesn't need rooting.
RefinedPtr = -1;
RefinedPtr.push_back(-1);
}
MaybeNoteDef(S, BBS, LI, BBS.Safepoints, RefinedPtr);
MaybeNoteDef(S, BBS, LI, BBS.Safepoints, std::move(RefinedPtr));
NoteOperandUses(S, BBS, I, BBS.UpExposedUsesUnrooted);
} else if (SelectInst *SI = dyn_cast<SelectInst>(&I)) {
// We need to insert an extra select for the GC root
Expand All @@ -837,23 +837,40 @@ State LateLowerGCFrame::LocalScan(Function &F) {
if (getValueAddrSpace(SI) != AddressSpace::Tracked) {
if (S.AllPtrNumbering.find(SI) != S.AllPtrNumbering.end())
continue;
LiftSelect(S, SI);
auto Num = LiftSelect(S, SI);
if (Num == -1)
continue;
auto SelectBase = cast<SelectInst>(S.ReversePtrNumbering[Num]);
SmallVector<int, 1> RefinedPtr{Number(S, SelectBase->getTrueValue()),
Number(S, SelectBase->getFalseValue())};
S.Refinements[Num] = std::move(RefinedPtr);
} else {
MaybeNoteDef(S, BBS, SI, BBS.Safepoints);
SmallVector<int, 1> RefinedPtr{Number(S, SI->getTrueValue()),
Number(S, SI->getFalseValue())};
MaybeNoteDef(S, BBS, SI, BBS.Safepoints, std::move(RefinedPtr));
NoteOperandUses(S, BBS, I, BBS.UpExposedUsesUnrooted);
}
} else if (PHINode *Phi = dyn_cast<PHINode>(&I)) {
if (!isSpecialPtr(Phi->getType())) {
continue;
}
auto nIncoming = Phi->getNumIncomingValues();
// We need to insert an extra phi for the GC root
if (getValueAddrSpace(Phi) != AddressSpace::Tracked) {
if (S.AllPtrNumbering.find(Phi) != S.AllPtrNumbering.end())
continue;
LiftPhi(S, Phi);
auto Num = LiftPhi(S, Phi);
auto lift = cast<PHINode>(S.ReversePtrNumbering[Num]);
SmallVector<int, 1> RefinedPtr(nIncoming);
for (unsigned i = 0; i < nIncoming; ++i)
RefinedPtr[i] = Number(S, lift->getIncomingValue(i));
S.Refinements[Num] = std::move(RefinedPtr);
} else {
MaybeNoteDef(S, BBS, Phi, BBS.Safepoints);
for (unsigned i = 0; i < Phi->getNumIncomingValues(); ++i) {
SmallVector<int, 1> RefinedPtr(nIncoming);
for (unsigned i = 0; i < nIncoming; ++i)
RefinedPtr[i] = Number(S, Phi->getIncomingValue(i));
MaybeNoteDef(S, BBS, Phi, BBS.Safepoints, std::move(RefinedPtr));
for (unsigned i = 0; i < nIncoming; ++i) {
BBState &IncomingBBS = S.BBStates[Phi->getIncomingBlock(i)];
NoteUse(S, IncomingBBS, Phi->getIncomingValue(i), IncomingBBS.PhiOuts);
}
Expand All @@ -862,14 +879,14 @@ State LateLowerGCFrame::LocalScan(Function &F) {
NoteOperandUses(S, BBS, I, BBS.UpExposedUsesUnrooted);
} else if (auto *ASCI = dyn_cast<AddrSpaceCastInst>(&I)) {
if (getValueAddrSpace(ASCI) == AddressSpace::Tracked) {
int RefinedPtr = -2;
SmallVector<int, 1> RefinedPtr{};
auto origin = ASCI->getPointerOperand()->stripPointerCasts();
if (auto LI = dyn_cast<LoadInst>(origin)) {
if (isLoadFromConstGV(LI)) {
RefinedPtr = -1;
RefinedPtr.push_back(-1);
}
}
MaybeNoteDef(S, BBS, ASCI, BBS.Safepoints, RefinedPtr);
MaybeNoteDef(S, BBS, ASCI, BBS.Safepoints, std::move(RefinedPtr));
}
} else if (auto *AI = dyn_cast<AllocaInst>(&I)) {
if (isSpecialPtr(AI->getAllocatedType()) && !AI->isArrayAllocation() &&
Expand Down Expand Up @@ -982,11 +999,21 @@ void LateLowerGCFrame::ComputeLiveSets(Function &F, State &S) {
}
// Apply refinements
for (int Idx = LS.find_first(); Idx >= 0; Idx = LS.find_next(Idx)) {
if (!S.LoadRefinements.count(Idx))
if (!S.Refinements.count(Idx))
continue;
auto &RefinedPtr = S.Refinements[Idx];
if (RefinedPtr.empty())
continue;
int RefinedPtr = S.LoadRefinements[Idx];
if (RefinedPtr == -1 || HasBitSet(LS, RefinedPtr))
bool rooted = true;
for (auto RefPtr: RefinedPtr) {
if (RefPtr == -1 || HasBitSet(LS, RefPtr))
continue;
rooted = false;
break;
}
if (rooted) {
LS[Idx] = 0;
}
}
// If the function has GC preserves, figure out whether we need to
// add in any extra live values.
Expand Down
32 changes: 32 additions & 0 deletions test/codegen.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,35 @@ end
@generated f23595(g, args...) = Expr(:call, :g, Expr(:(...), :args))
x23595 = rand(1)
@test f23595(Core.arrayref, true, x23595, 1) == x23595[]

# Issue #22421
@noinline f22421_1(x) = x[] + 1
@noinline f22421_2(x) = x[] + 2
@noinline f22421_3(x, y, z, v) = x[] + y[] + z[] + v
function g22421_1(x, y, b)
# Most likely generates a branch with phi node
if b
z = x
v = f22421_1(y)
else
z = y
v = f22421_2(x)
end
return f22421_3(x, y, z, v)
end
function g22421_2(x, y, b)
# Most likely generates a select
return f22421_3(x, y, b ? x : y, 1)
end

@test g22421_1(Ref(1), Ref(2), true) === 7
@test g22421_1(Ref(3), Ref(4), false) === 16
@test g22421_2(Ref(5), Ref(6), true) === 17
@test g22421_2(Ref(7), Ref(8), false) === 24

if opt_level > 0
@test !contains(get_llvm(g22421_1, Tuple{Base.RefValue{Int},Base.RefValue{Int},Bool}),
"%gcframe")
@test !contains(get_llvm(g22421_2, Tuple{Base.RefValue{Int},Base.RefValue{Int},Bool}),
"%gcframe")
end
21 changes: 21 additions & 0 deletions test/llvmpasses/gcroots.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,27 @@ top:
ret %jl_value_t addrspace(10)* %v1
}

define void @refine_select_phi(%jl_value_t addrspace(10)* %x, %jl_value_t addrspace(10)* %y, i1 %b) {
; CHECK-LABEL: @refine_select_phi
; CHECK-NOT: %gcframe
top:
%ptls = call %jl_value_t*** @jl_get_ptls_states()
%s = select i1 %b, %jl_value_t addrspace(10)* %x, %jl_value_t addrspace(10)* %y
br i1 %b, label %L1, label %L2

L1:
br label %L3

L2:
br label %L3

L3:
%p = phi %jl_value_t addrspace(10)* [ %x, %L1 ], [ %y, %L2 ]
call void @one_arg_boxed(%jl_value_t addrspace(10)* %s)
call void @one_arg_boxed(%jl_value_t addrspace(10)* %p)
ret void
}

!0 = !{!"jtbaa"}
!1 = !{!"jtbaa_const", !0, i64 0}
!2 = !{!1, !1, i64 0, i64 1}

0 comments on commit 5769cb0

Please sign in to comment.