Skip to content
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.

Sign up for GitHub

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

Refine liveness of phi and select #24035

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Refine liveness of phi and select #24035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 52 additions & 25 deletions src/llvm-late-gc-lowering.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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}