Skip to content

Commit

Permalink
[GCLowering] Expand support for vectors of pointers (#28455)
Browse files Browse the repository at this point in the history
Most of the support was already there, but it was mostly unexercised.
The recent activation of the SLP vectorizer made these patterns appear
in the IR, so fixup the support.

Fixes #28445
  • Loading branch information
Keno authored Aug 6, 2018
1 parent 00d6f72 commit f5a0b03
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 10 deletions.
47 changes: 37 additions & 10 deletions src/llvm-late-gc-lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ static std::pair<Value*,int> FindBaseValue(const State &S, Value *V, bool UseCac
isa<Argument>(CurrentV) || isa<SelectInst>(CurrentV) ||
isa<PHINode>(CurrentV) || isa<AddrSpaceCastInst>(CurrentV) ||
isa<Constant>(CurrentV) || isa<AllocaInst>(CurrentV) ||
isa<ExtractValueInst>(CurrentV));
isa<ExtractValueInst>(CurrentV) ||
isa<InsertElementInst>(CurrentV) ||
isa<ShuffleVectorInst>(CurrentV));
return std::make_pair(CurrentV, fld_idx);
}

Expand Down Expand Up @@ -602,22 +604,37 @@ std::vector<int> LateLowerGCFrame::NumberVectorBase(State &S, Value *CurrentV) {
((isa<Argument>(CurrentV) || isa<AllocaInst>(CurrentV) ||
isa<AddrSpaceCastInst>(CurrentV)) &&
getValueAddrSpace(CurrentV) != AddressSpace::Tracked)) {
Numbers.resize(cast<VectorType>(CurrentV->getType())->getNumElements(), -1);
}
/* We (the frontend) don't insert either of these, but it would be legal -
though a bit strange, considering they're pointers - for the optimizer to
do so. All that's needed here is to NumberVector the previous vector/value
and lift the operation */
else if (isa<ShuffleVectorInst>(CurrentV)) {
assert(false && "TODO Shuffle");
} else if (isa<InsertElementInst>(CurrentV)) {
assert(false && "TODO Insert");
} else if (isa<LoadInst>(CurrentV)) {
else if (auto *SVI = dyn_cast<ShuffleVectorInst>(CurrentV)) {
std::vector<int> Numbers1 = NumberVectorBase(S, SVI->getOperand(0));
std::vector<int> Numbers2 = NumberVectorBase(S, SVI->getOperand(1));
auto Mask = SVI->getShuffleMask();
for (unsigned idx : Mask) {
if (idx < Numbers1.size()) {
Numbers.push_back(Numbers1[idx]);
} else {
Numbers.push_back(Numbers2[idx - Numbers1.size()]);
}
}
} else if (auto *IEI = dyn_cast<InsertElementInst>(CurrentV)) {
unsigned idx = cast<ConstantInt>(IEI->getOperand(2))->getZExtValue();
Numbers = NumberVectorBase(S, IEI->getOperand(0));
int ElNumber = Number(S, IEI->getOperand(1));
Numbers[idx] = ElNumber;
} else if (isa<LoadInst>(CurrentV) || isa<CallInst>(CurrentV) || isa<PHINode>(CurrentV)) {
// This is simple, we can just number them sequentially
for (unsigned i = 0; i < cast<VectorType>(CurrentV->getType())->getNumElements(); ++i) {
int Num = ++S.MaxPtrNumber;
Numbers.push_back(Num);
S.ReversePtrNumbering[Num] = CurrentV;
}
} else {
assert(false && "Unexpected vector generating operating");
}
S.AllVectorNumbering[CurrentV] = Numbers;
return Numbers;
Expand All @@ -629,9 +646,17 @@ std::vector<int> LateLowerGCFrame::NumberVector(State &S, Value *V) {
return it->second;
auto CurrentV = FindBaseValue(S, V);
assert(CurrentV.second == -1);
auto Numbers = NumberVectorBase(S, CurrentV.first);
S.AllVectorNumbering[V] = Numbers;
return Numbers;
// E.g. if this is a gep, it's possible for the base to be a single ptr
if (isSpecialPtrVec(CurrentV.first->getType())) {
auto Numbers = NumberVectorBase(S, CurrentV.first);
S.AllVectorNumbering[V] = Numbers;
return Numbers;
} else {
std::vector<int> Numbers{};
Numbers.resize(cast<VectorType>(V->getType())->getNumElements(),
NumberBase(S, V, CurrentV.first));
return Numbers;
}
}

static void MaybeResize(BBState &BBS, unsigned Idx) {
Expand Down Expand Up @@ -714,6 +739,8 @@ void LateLowerGCFrame::NoteUse(State &S, BBState &BBS, Value *V, BitVector &Uses
std::vector<int> Nums = NumberVector(S, V);
for (int Num : Nums) {
MaybeResize(BBS, Num);
if (Num < 0)
continue;
Uses[Num] = 1;
}
}
Expand All @@ -729,7 +756,7 @@ void LateLowerGCFrame::NoteUse(State &S, BBState &BBS, Value *V, BitVector &Uses
void LateLowerGCFrame::NoteOperandUses(State &S, BBState &BBS, User &UI, BitVector &Uses) {
for (Use &U : UI.operands()) {
Value *V = U;
if (!isSpecialPtr(V->getType()))
if (!isSpecialPtr(V->getType()) && !isSpecialPtrVec(V->getType()))
continue;
NoteUse(S, BBS, V, Uses);
}
Expand Down
20 changes: 20 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6675,3 +6675,23 @@ c28399 = 42
@test g28399(0)() == 42
@test g28399(1)() == 42
@test_throws UndefVarError(:__undef_28399__) f28399()

# issue #28445
mutable struct foo28445
x::Int
end

@noinline make_foo28445() = (foo28445(1), foo28445(rand(1:10)), foo28445(rand(1:10)))
@noinline function use_tuple28445(c)
@test isa(c[2], foo28445)
@test isa(c[3], foo28445)
end

function repackage28445()
(_, a, b) = make_foo28445()
GC.gc()
c = (foo28445(1), foo28445(2), a, b)
use_tuple28445(c)
true
end
@test repackage28445()
13 changes: 13 additions & 0 deletions test/llvmpasses/gcroots.ll
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,19 @@ top:
ret i8 %val
}

define %jl_value_t addrspace(10)* @vecstoreload(<2 x %jl_value_t addrspace(10)*> *%arg) {
; CHECK-LABEL: @vecstoreload
; CHECK: %gcframe = alloca %jl_value_t addrspace(10)*, i32 4
top:
%ptls = call %jl_value_t*** @julia.ptls_states()
%loaded = load <2 x %jl_value_t addrspace(10)*>, <2 x %jl_value_t addrspace(10)*> *%arg
call void @jl_safepoint()
%obj = call %jl_value_t addrspace(10) *@alloc()
%casted = bitcast %jl_value_t addrspace(10)* %obj to <2 x %jl_value_t addrspace(10)*> addrspace(10)*
store <2 x %jl_value_t addrspace(10)*> %loaded, <2 x %jl_value_t addrspace(10)*> addrspace(10)* %casted
ret %jl_value_t addrspace(10)* %obj
}

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

0 comments on commit f5a0b03

Please sign in to comment.