[llvm] r357032 - [LiveRange] Reset the VNIs when splitting subranges
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 26 15:13:25 PDT 2019
Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: qcolombet
> Date: Tue Mar 26 14:27:15 2019
> New Revision: 357032
>
> URL: http://llvm.org/viewvc/llvm-project?rev=357032&view=rev
> Log:
> [LiveRange] Reset the VNIs when splitting subranges
>
> When splitting a subrange we end up with two different subranges covering
> two different, non overlapping, lanes.
> As part of this splitting the VNIs of the original live-range need
> to be dispatched to the subranges according to which lanes they are
> actually defining.
>
> Prior to this patch we were assuming that all values were defining
> all lanes. This was wrong as demonstrated by llvm.org/PR40835.
>
> Differential Revision: https://reviews.llvm.org/D59731
>
> Added:
> llvm/trunk/test/CodeGen/SystemZ/regcoal-subranges-update.mir
> Modified:
> llvm/trunk/include/llvm/CodeGen/LiveInterval.h
> llvm/trunk/lib/CodeGen/LiveInterval.cpp
> llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp
> llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
> llvm/trunk/lib/CodeGen/SplitKit.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/LiveInterval.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LiveInterval.h?rev=357032&r1=357031&r2=357032&view=diff
> ==============================================================================
>
> --- llvm/trunk/include/llvm/CodeGen/LiveInterval.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/LiveInterval.h Tue Mar 26 14:27:15 2019
> @@ -789,8 +789,15 @@ namespace llvm {
> /// L000F, refining for mask L0018. Will split the L00F0 lane into
> /// L00E0 and L0010 and the L000F lane into L0007 and L0008. The Mod
> /// function will be applied to the L0010 and L0008 subranges.
> + ///
> + /// \p Indexes and \p TRI are required to clean up the VNIs that
> + /// don't defne the related lane masks after they get shrunk. E.g.,
> + /// when L000F gets split into L0007 and L0008 maybe only a subset
> + /// of the VNIs that defined L000F defines L0007.
> void refineSubRanges(BumpPtrAllocator &Allocator, LaneBitmask LaneMask,
> - std::function<void(LiveInterval::SubRange&)> Apply);
> + std::function<void(LiveInterval::SubRange &)> Apply,
> + const SlotIndexes &Indexes,
> + const TargetRegisterInfo &TRI);
It'd be better to put the new arguments before the std::function so that
callers can continue to use the block-style formatting for the lambdas
they pass.
>
> bool operator<(const LiveInterval& other) const {
> const SlotIndex &thisIndex = beginIndex();
>
> Modified: llvm/trunk/lib/CodeGen/LiveInterval.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveInterval.cpp?rev=357032&r1=357031&r2=357032&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LiveInterval.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveInterval.cpp Tue Mar 26 14:27:15 2019
> @@ -879,8 +879,53 @@ void LiveInterval::clearSubRanges() {
> SubRanges = nullptr;
> }
>
> -void LiveInterval::refineSubRanges(BumpPtrAllocator &Allocator,
> - LaneBitmask LaneMask, std::function<void(LiveInterval::SubRange&)> Apply) {
> +/// For each VNI in \p SR, check whether or not that value defines part
> +/// of the mask describe by \p LaneMask and if not, remove that value
> +/// from \p SR.
> +static void stripValuesNotDefiningMask(unsigned Reg, LiveInterval::SubRange &SR,
> + LaneBitmask LaneMask,
> + const SlotIndexes &Indexes,
> + const TargetRegisterInfo &TRI) {
> + // Phys reg should not be tracked at subreg level.
> + // Same for noreg (Reg == 0).
> + if (!TargetRegisterInfo::isVirtualRegister(Reg) || !Reg)
> + return;
> + // Remove the values that don't define those lanes.
> + SmallVector<VNInfo *, 8> ToBeRemoved;
> + for (VNInfo *VNI : SR.valnos) {
> + if (VNI->isUnused())
> + continue;
> + // PHI definitions don't have MI attached, so there is nothing
> + // we can use to strip the VNI.
> + if (VNI->isPHIDef())
> + continue;
> + const MachineInstr *MI = Indexes.getInstructionFromIndex(VNI->def);
> + assert(MI && "Cannot find the definition of a value");
> + bool hasDef = false;
> + for (ConstMIBundleOperands MOI(*MI); MOI.isValid(); ++MOI) {
> + if (!MOI->isReg() || !MOI->isDef())
> + continue;
> + if (MOI->getReg() != Reg)
> + continue;
> + if ((TRI.getSubRegIndexLaneMask(MOI->getSubReg()) & LaneMask).none())
> + continue;
> + hasDef = true;
> + break;
> + }
> +
> + if (!hasDef)
> + ToBeRemoved.push_back(VNI);
> + }
> + for (VNInfo *VNI : ToBeRemoved)
> + SR.removeValNo(VNI);
> +
> + assert(!SR.empty() && "At least one value should be defined by this mask");
> +}
> +
> +void LiveInterval::refineSubRanges(
> + BumpPtrAllocator &Allocator, LaneBitmask LaneMask,
> + std::function<void(LiveInterval::SubRange &)> Apply,
> + const SlotIndexes &Indexes, const TargetRegisterInfo &TRI) {
> LaneBitmask ToApply = LaneMask;
> for (SubRange &SR : subranges()) {
> LaneBitmask SRMask = SR.LaneMask;
> @@ -898,6 +943,10 @@ void LiveInterval::refineSubRanges(BumpP
> SR.LaneMask = SRMask & ~Matching;
> // Create a new subrange for the matching part
> MatchingRange = createSubRangeFrom(Allocator, Matching, SR);
> + // Now that the subrange is split in half, make sure we
> + // only keep in the subranges the VNIs that touch the related half.
> + stripValuesNotDefiningMask(reg, *MatchingRange, Matching, Indexes, TRI);
> + stripValuesNotDefiningMask(reg, SR, SR.LaneMask, Indexes, TRI);
> }
> Apply(*MatchingRange);
> ToApply &= ~Matching;
>
> Modified: llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp?rev=357032&r1=357031&r2=357032&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp Tue Mar 26 14:27:15 2019
> @@ -95,10 +95,11 @@ void LiveRangeCalc::calculate(LiveInterv
> }
>
> LI.refineSubRanges(*Alloc, SubMask,
> - [&MO, this](LiveInterval::SubRange &SR) {
> - if (MO.isDef())
> - createDeadDef(*Indexes, *Alloc, SR, MO);
> - });
> + [&MO, this](LiveInterval::SubRange &SR) {
> + if (MO.isDef())
> + createDeadDef(*Indexes, *Alloc, SR, MO);
> + },
> + *Indexes, TRI);
> }
>
> // Create the def in the main liverange. We do not have to do this if
>
> Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=357032&r1=357031&r2=357032&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Tue Mar 26 14:27:15 2019
> @@ -924,23 +924,25 @@ RegisterCoalescer::removeCopyByCommuting
> }
> SlotIndex AIdx = CopyIdx.getRegSlot(true);
> LaneBitmask MaskA;
> + const SlotIndexes &Indexes = *LIS->getSlotIndexes();
> for (LiveInterval::SubRange &SA : IntA.subranges()) {
> VNInfo *ASubValNo = SA.getVNInfoAt(AIdx);
> assert(ASubValNo != nullptr);
> MaskA |= SA.LaneMask;
>
> - IntB.refineSubRanges(Allocator, SA.LaneMask,
> - [&Allocator,&SA,CopyIdx,ASubValNo,&ShrinkB]
> - (LiveInterval::SubRange &SR) {
> - VNInfo *BSubValNo = SR.empty()
> - ? SR.getNextValue(CopyIdx, Allocator)
> - : SR.getVNInfoAt(CopyIdx);
> - assert(BSubValNo != nullptr);
> - auto P = addSegmentsWithValNo(SR, BSubValNo, SA, ASubValNo);
> - ShrinkB |= P.second;
> - if (P.first)
> - BSubValNo->def = ASubValNo->def;
> - });
> + IntB.refineSubRanges(
> + Allocator, SA.LaneMask,
> + [&Allocator, &SA, CopyIdx, ASubValNo,
> + &ShrinkB](LiveInterval::SubRange &SR) {
> + VNInfo *BSubValNo = SR.empty() ? SR.getNextValue(CopyIdx, Allocator)
> + : SR.getVNInfoAt(CopyIdx);
> + assert(BSubValNo != nullptr);
> + auto P = addSegmentsWithValNo(SR, BSubValNo, SA, ASubValNo);
> + ShrinkB |= P.second;
> + if (P.first)
> + BSubValNo->def = ASubValNo->def;
> + },
> + Indexes, *TRI);
> }
> // Go over all subranges of IntB that have not been covered by IntA,
> // and delete the segments starting at CopyIdx. This can happen if
> @@ -3262,16 +3264,18 @@ void RegisterCoalescer::mergeSubRangeInt
> LaneBitmask LaneMask,
> CoalescerPair &CP) {
> BumpPtrAllocator &Allocator = LIS->getVNInfoAllocator();
> - LI.refineSubRanges(Allocator, LaneMask,
> - [this,&Allocator,&ToMerge,&CP](LiveInterval::SubRange &SR) {
> - if (SR.empty()) {
> - SR.assign(ToMerge, Allocator);
> - } else {
> - // joinSubRegRange() destroys the merged range, so we need a copy.
> - LiveRange RangeCopy(ToMerge, Allocator);
> - joinSubRegRanges(SR, RangeCopy, SR.LaneMask, CP);
> - }
> - });
> + LI.refineSubRanges(
> + Allocator, LaneMask,
> + [this, &Allocator, &ToMerge, &CP](LiveInterval::SubRange &SR) {
> + if (SR.empty()) {
> + SR.assign(ToMerge, Allocator);
> + } else {
> + // joinSubRegRange() destroys the merged range, so we need a copy.
> + LiveRange RangeCopy(ToMerge, Allocator);
> + joinSubRegRanges(SR, RangeCopy, SR.LaneMask, CP);
> + }
> + },
> + *LIS->getSlotIndexes(), *TRI);
> }
>
> bool RegisterCoalescer::isHighCostLiveInterval(LiveInterval &LI) {
>
> Modified: llvm/trunk/lib/CodeGen/SplitKit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SplitKit.cpp?rev=357032&r1=357031&r2=357032&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SplitKit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SplitKit.cpp Tue Mar 26 14:27:15 2019
> @@ -520,17 +520,18 @@ SlotIndex SplitEditor::buildSingleSubReg
> .addReg(FromReg, 0, SubIdx);
>
> BumpPtrAllocator &Allocator = LIS.getVNInfoAllocator();
> + SlotIndexes &Indexes = *LIS.getSlotIndexes();
> if (FirstCopy) {
> - SlotIndexes &Indexes = *LIS.getSlotIndexes();
> Def = Indexes.insertMachineInstrInMaps(*CopyMI, Late).getRegSlot();
> } else {
> CopyMI->bundleWithPred();
> }
> LaneBitmask LaneMask = TRI.getSubRegIndexLaneMask(SubIdx);
> DestLI.refineSubRanges(Allocator, LaneMask,
> - [Def, &Allocator](LiveInterval::SubRange& SR) {
> - SR.createDeadDef(Def, Allocator);
> - });
> + [Def, &Allocator](LiveInterval::SubRange &SR) {
> + SR.createDeadDef(Def, Allocator);
> + },
> + Indexes, TRI);
> return Def;
> }
>
>
> Added: llvm/trunk/test/CodeGen/SystemZ/regcoal-subranges-update.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/regcoal-subranges-update.mir?rev=357032&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/SystemZ/regcoal-subranges-update.mir (added)
> +++ llvm/trunk/test/CodeGen/SystemZ/regcoal-subranges-update.mir Tue Mar 26 14:27:15 2019
> @@ -0,0 +1,94 @@
> +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
> +# RUN: llc -mtriple s390x-ibm-linux -mcpu=z13 -systemz-subreg-liveness -verify-machineinstrs -start-before simple-register-coalescing -stop-after greedy -o - %s | FileCheck %s
> +
> +# Check that when we split the live-range with several active lanes
> +# as part of the live-range update, we correctly eliminate the VNI from
> +# the relevant part.
> +#
> +# In this specific test, the register coalescer will:
> +# 1. Merge %0 with %1, creating a live-range for the full value subreg_l32 + subreg_h32
> +# (actually %0 gets merge with %1 via rematerialization, and technically %0 and %1
> +# remain two different live-ranges.)
> +# 2. Merge %2 with %1 triggering a split into the subreg_l32 + subreg_h32 ranges, since
> +# %2 only touches subreg_l32. As part of the split the subrange covering subreg_h32
> +# must contain only the VNI for the high part (i.e., the one tied with the remaaat of %0).
> +# This used to be broken and trigger a machine verifier error, because we were not
> +# clearing the dead value w.r.t. lanes when doing the splitting. I.e., we were ending
> +# with a subrange referring a value that did not define that lane.
> +#
> +# PR40835
> +---
> +name: main
> +tracksRegLiveness: true
> +body: |
> + bb.0:
> +
> + ; CHECK-LABEL: name: main
> + ; CHECK: [[LGHI:%[0-9]+]]:gr64bit = LGHI 43
> + ; CHECK: [[LGHI1:%[0-9]+]]:gr64bit = LGHI 43
> + ; CHECK: [[LGHI1]].subreg_l32:gr64bit = MSR [[LGHI1]].subreg_l32, [[LGHI1]].subreg_l32
> + ; CHECK: [[LGHI1]].subreg_l32:gr64bit = AHIMux [[LGHI1]].subreg_l32, 9, implicit-def dead $cc
> + ; CHECK: undef %3.subreg_l64:gr128bit = LGFI -245143785, implicit [[LGHI1]].subreg_l32
> + ; CHECK: [[DLGR:%[0-9]+]]:gr128bit = DLGR [[DLGR]], [[LGHI]]
> + ; CHECK: Return implicit [[DLGR]]
> + %0:gr64bit = LGHI 43
> + %1:gr32bit = COPY %0.subreg_l32
> + %1:gr32bit = MSR %1, %1
> + %2:gr32bit = COPY killed %1
> + %2:gr32bit = AHIMux killed %2, 9, implicit-def dead $cc
> + undef %3.subreg_l64:gr128bit = LGFI -245143785, implicit killed %2
> + %3:gr128bit = DLGR %3:gr128bit, killed %0
> + Return implicit killed %3
> +
> +...
> +
> +# Make sure the compiler does not choke on VNIs that don't
> +# an explicit MI as definition.
> +# In that specific example, this is the PHI not explicitly
> +# represented for the value carried by %7.
> +---
> +name: segfault
> +tracksRegLiveness: true
> +liveins: []
> +body: |
> + ; CHECK-LABEL: name: segfault
> + ; CHECK: bb.0:
> + ; CHECK: successors: %bb.1(0x80000000)
> + ; CHECK: [[LGHI:%[0-9]+]]:addr64bit = LGHI 0
> + ; CHECK: bb.1:
> + ; CHECK: successors: %bb.1(0x80000000)
> + ; CHECK: ADJCALLSTACKDOWN 0, 0
> + ; CHECK: [[LGFR:%[0-9]+]]:gr64bit = LGFR [[LGHI]].subreg_l32
> + ; CHECK: $r2d = LGHI 123
> + ; CHECK: $r3d = LGHI 0
> + ; CHECK: $r4d = LGHI 0
> + ; CHECK: $r5d = COPY [[LGFR]]
> + ; CHECK: KILL killed $r2d, killed $r3d, killed $r4d, $r5d, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc
> + ; CHECK: ADJCALLSTACKUP 0, 0
> + ; CHECK: [[LGHI]]:addr64bit = nuw nsw LA [[LGHI]], 1, $noreg
> + ; CHECK: J %bb.1
> + bb.0:
> + successors: %bb.1(0x80000000)
> +
> + %2:gr64bit = LGHI 0
> + %5:gr64bit = LGHI 123
> + %7:addr64bit = COPY %2
> +
> + bb.1:
> + successors: %bb.1(0x80000000)
> +
> + %0:addr64bit = COPY killed %7
> + ADJCALLSTACKDOWN 0, 0
> + %3:gr32bit = COPY %0.subreg_l32
> + %4:gr64bit = LGFR killed %3
> + $r2d = COPY %5
> + $r3d = COPY %2
> + $r4d = COPY %2
> + $r5d = COPY killed %4
> + KILL killed $r2d, killed $r3d, killed $r4d, killed $r5d, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc
> + ADJCALLSTACKUP 0, 0
> + %1:gr64bit = nuw nsw LA killed %0, 1, $noreg
> + %7:addr64bit = COPY killed %1
> + J %bb.1
> +
> +...
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list