Skip to content

Commit

Permalink
Merge pull request #24035 from JuliaLang/yyc/codegen/select_phi
Browse files Browse the repository at this point in the history 
Refine liveness of phi and select
  • Loading branch information
@yuyichao
yuyichao authored Oct 10, 2017
2 parents 1216e5f + 5769cb0 commit 8ea00cc
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}

5 comments on commit 8ea00cc

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@ararslan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily=true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something went wrong when running your job:

NanosoldierError: encountered error when retrieving old daily build data: didn't find previous daily build data in the past 31 days

Unfortunately, the logs could not be uploaded.
cc @ararslan

@ararslan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright then.

Please sign in to comment.