[llvm] r357032 - [LiveRange] Reset the VNIs when splitting subranges
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 26 15:53:27 PDT 2019
> On Mar 26, 2019, at 3:13 PM, Justin Bogner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org <mailto: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.
I don’t know if the code was manually formatted, but moving the argument before the std::function did not reverted the to the block-style formatting :(.
I am using clang-format version 8.0.0 (tags/RELEASE_800/final)
Attaching the patch in case you have an idea how to convince clang to produce the block-style thing.
>
>>
>> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190326/c47d9009/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch
Type: application/octet-stream
Size: 5164 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190326/c47d9009/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190326/c47d9009/attachment-0001.html>
More information about the llvm-commits
mailing list