From f5a0b034a4133263736c07934f3eb60567889396 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 6 Aug 2018 15:06:16 +0100 Subject: [PATCH] [GCLowering] Expand support for vectors of pointers (#28455) 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 --- src/llvm-late-gc-lowering.cpp | 47 +++++++++++++++++++++++++++-------- test/core.jl | 20 +++++++++++++++ test/llvmpasses/gcroots.ll | 13 ++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index a48fd1a96eb8f..d97bf02a05f22 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -486,7 +486,9 @@ static std::pair FindBaseValue(const State &S, Value *V, bool UseCac isa(CurrentV) || isa(CurrentV) || isa(CurrentV) || isa(CurrentV) || isa(CurrentV) || isa(CurrentV) || - isa(CurrentV)); + isa(CurrentV) || + isa(CurrentV) || + isa(CurrentV)); return std::make_pair(CurrentV, fld_idx); } @@ -602,22 +604,37 @@ std::vector LateLowerGCFrame::NumberVectorBase(State &S, Value *CurrentV) { ((isa(CurrentV) || isa(CurrentV) || isa(CurrentV)) && getValueAddrSpace(CurrentV) != AddressSpace::Tracked)) { + Numbers.resize(cast(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(CurrentV)) { - assert(false && "TODO Shuffle"); - } else if (isa(CurrentV)) { - assert(false && "TODO Insert"); - } else if (isa(CurrentV)) { + else if (auto *SVI = dyn_cast(CurrentV)) { + std::vector Numbers1 = NumberVectorBase(S, SVI->getOperand(0)); + std::vector 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(CurrentV)) { + unsigned idx = cast(IEI->getOperand(2))->getZExtValue(); + Numbers = NumberVectorBase(S, IEI->getOperand(0)); + int ElNumber = Number(S, IEI->getOperand(1)); + Numbers[idx] = ElNumber; + } else if (isa(CurrentV) || isa(CurrentV) || isa(CurrentV)) { // This is simple, we can just number them sequentially for (unsigned i = 0; i < cast(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; @@ -629,9 +646,17 @@ std::vector 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 Numbers{}; + Numbers.resize(cast(V->getType())->getNumElements(), + NumberBase(S, V, CurrentV.first)); + return Numbers; + } } static void MaybeResize(BBState &BBS, unsigned Idx) { @@ -714,6 +739,8 @@ void LateLowerGCFrame::NoteUse(State &S, BBState &BBS, Value *V, BitVector &Uses std::vector Nums = NumberVector(S, V); for (int Num : Nums) { MaybeResize(BBS, Num); + if (Num < 0) + continue; Uses[Num] = 1; } } @@ -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); } diff --git a/test/core.jl b/test/core.jl index f9a08f1a990bf..7dd27fefe03fc 100644 --- a/test/core.jl +++ b/test/core.jl @@ -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() diff --git a/test/llvmpasses/gcroots.ll b/test/llvmpasses/gcroots.ll index 25d75a8a7a0d1..958a735b8efca 100644 --- a/test/llvmpasses/gcroots.ll +++ b/test/llvmpasses/gcroots.ll @@ -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}