[llvm] r224806 - LiveInterval: Introduce createMainRangeFromSubranges().
Quentin Colombet
qcolombet at apple.com
Tue Jan 6 17:30:50 PST 2015
Hi Matthias,
Thanks for adding this new capability.
I made a few comments to ease readability of the whole code base, nothing correctness related. Feel free to fix them (or argue against them :P) whenever you have time.
Thanks,
-Quentin
On Dec 23, 2014, at 6:11 PM, Matthias Braun <matze at braunis.de> wrote:
> Author: matze
> Date: Tue Dec 23 20:11:51 2014
> New Revision: 224806
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224806&view=rev
> Log:
> LiveInterval: Introduce createMainRangeFromSubranges().
>
> This function constructs the main liverange by merging all subranges if
> subregister liveness tracking is available. This should be slightly
> faster to compute instead of performing the liveness calculation again
> for the main range. More importantly it avoids cases where the main
> liverange would cover positions where no subrange was live. These cases
> happened for partial definitions where the actual defined part was dead
> and only the undefined parts used later.
>
> The register coalescing requires that every part covered by the main
> live range has at least one subrange live.
>
> I also expect this function to become usefull later for places where the
> subranges are modified in a way that it is hard to correctly fix the
> main liverange in the machine scheduler, we can simply reconstruct it
> from subranges then.
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/LiveInterval.h
> llvm/trunk/lib/CodeGen/LiveInterval.cpp
> llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/LiveInterval.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LiveInterval.h?rev=224806&r1=224805&r2=224806&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/LiveInterval.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/LiveInterval.h Tue Dec 23 20:11:51 2014
> @@ -552,6 +552,10 @@ namespace llvm {
> void verify() const;
> #endif
>
> + protected:
> + /// Append a segment to the list of segments.
> + void append(const LiveRange::Segment S);
> +
> private:
>
> iterator addSegmentFrom(Segment S, iterator From);
> @@ -685,6 +689,10 @@ namespace llvm {
> /// are not considered valid and should only exist temporarily).
> void removeEmptySubRanges();
>
> + /// Construct main live range by merging the SubRanges of @p LI.
> + void constructMainRangeFromSubranges(const SlotIndexes &Indexes,
> + VNInfo::Allocator &VNIAllocator);
> +
> /// getSize - Returns the sum of sizes of all the LiveRange's.
> ///
> unsigned getSize() const;
>
> Modified: llvm/trunk/lib/CodeGen/LiveInterval.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveInterval.cpp?rev=224806&r1=224805&r2=224806&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LiveInterval.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveInterval.cpp Tue Dec 23 20:11:51 2014
> @@ -300,6 +300,12 @@ LiveRange::extendSegmentStartTo(iterator
> return MergeTo;
> }
>
> +void LiveRange::append(const Segment S) {
> + // Check that the segment belongs to the back of the list.
> + assert(segments.empty() || segments.back().end <= S.start);
> + segments.push_back(S);
> +}
> +
> LiveRange::iterator LiveRange::addSegmentFrom(Segment S, iterator From) {
> SlotIndex Start = S.start, End = S.end;
> iterator it = std::upper_bound(From, end(), Start);
> @@ -609,6 +615,214 @@ void LiveInterval::removeEmptySubRanges(
> }
> }
>
> +/// Helper function for constructMainRangeFromSubranges(): Search the CFG
> +/// backwards until we find a place covered by a LiveRange segment that actually
> +/// has a valno set.
> +static VNInfo *searchForVNI(const SlotIndexes &Indexes, LiveRange &LR,
> + const MachineBasicBlock *MBB,
> + SmallPtrSetImpl<const MachineBasicBlock*> &Visited) {
> + // We start the search at the end of MBB.
> + SlotIndex EndIdx = Indexes.getMBBEndIdx(MBB);
> + // In our use case we can't live the area covered by the live segments without
> + // finding an actual VNI def.
> + LiveRange::iterator I = LR.find(EndIdx.getPrevSlot());
> + assert(I != LR.end());
> + LiveRange::Segment &S = *I;
> + if (S.valno != nullptr)
> + return S.valno;
> +
> + VNInfo *VNI = nullptr;
> + // Continue at predecessors (we could even go to idom with domtree available).
> + for (const MachineBasicBlock *Pred : MBB->predecessors()) {
> + // Avoid going in circles.
> + if (Visited.count(Pred))
> + continue;
> + Visited.insert(Pred);
Could be replaced by:
if (!Visited.insert(Pred).second)
continue;
> +
> + VNI = searchForVNI(Indexes, LR, Pred, Visited);
> + if (VNI != nullptr) {
> + S.valno = VNI;
> + break;
> + }
> + }
> +
> + return VNI;
> +}
> +
> +void LiveInterval::constructMainRangeFromSubranges(
> + const SlotIndexes &Indexes, VNInfo::Allocator &VNIAllocator) {
> + // The basic observations on which this algorithm is based:
> + // - Each Def/ValNo in a subrange must have a corresponding def on the main
> + // range, but not further defs/valnos are necessary.
> + // - If any of the subranges is live at a point the main liverange has to be
> + // live too, conversily if no subrange is live the main range mustn't be
> + // live either.
> + // We do this by scannig through all the subranges simultaneously creating new
> + // segments in the main range as segments start/ends come up in the subranges.
> + assert(hasSubRanges());
Add a message in the assert (even obvious one are useful :)).
> + assert(segments.empty() && valnos.empty() && "expected empty main range");
> +
> + // Collect subrange, iterator pairs for the walk and determine first and last
> + // SlotIndex involved.
> + SmallVector<std::pair<const SubRange*, const_iterator>, 4> SRs;
> + SlotIndex First;
> + SlotIndex Last;
> + for (const SubRange &SR : subranges()) {
> + if (SR.empty())
> + continue;
> + SRs.push_back(std::make_pair(&SR, SR.begin()));
> + if (!First.isValid() || SR.segments.front().start < First)
> + First = SR.segments.front().start;
> + if (!Last.isValid() || SR.segments.back().end > Last)
> + Last = SR.segments.back().end;
> + }
> +
> + errs() << "Compute: " << *this << "\n";
> +
> + // Walk over all subranges simultaneously.
> + Segment CurrentSegment;
> + bool ConstructingSegment = false;
> + bool NeedVNIFixup = false;
> + unsigned ActiveMask = 0;
> + SlotIndex Pos = First;
> + while (true) {
> + SlotIndex NextPos = Last;
> + enum {
> + NOTHING,
> + BEGIN_SEGMENT,
> + END_SEGMENT,
> + } Event = NOTHING;
Maybe add a comment saying that the EventMask covers the lanes that we will add in the main range for this iteration.
I think that having a comment on the purpose of the variable will help reading the code later on.
> + unsigned EventMask = 0;
> + bool IsDef = false;
> + // Find the next begin or end of a subrange segment. Combine masks if we
> + // have multiple begins/ends at the same position. Ends take precedence over
> + // Begins.
I would do a function for this loop so that the code is more compact. I think that would help the high level reading of the algorithm.
> + for (auto &SRP : SRs) {
> + const SubRange &SR = *SRP.first;
> + const_iterator &I = SRP.second;
Maybe add a comment before this loop saying something along the line: Drop any segment that we already recorded in the main live-range. The invariant is anything ending before Pos is already in main live-range (unless I am mistaken of course :)).
> + while (I != SR.end() &&
> + (I->end < Pos ||
> + (I->end == Pos && (ActiveMask & SR.LaneMask) == 0)))
> + ++I;
> + if (I == SR.end())
> + continue;
> + if ((ActiveMask & SR.LaneMask) == 0 &&
> + Pos <= I->start && I->start <= NextPos) {
> + // Merge multiple begins at the same position
Period at the end of the comment.
> + if (I->start == NextPos && Event == BEGIN_SEGMENT) {
> + EventMask |= SR.LaneMask;
> + IsDef |= I->valno->def == I->start;
> + } else if (I->start < NextPos || Event != END_SEGMENT) {
> + Event = BEGIN_SEGMENT;
> + NextPos = I->start;
> + EventMask = SR.LaneMask;
> + IsDef = I->valno->def == I->start;
> + }
> + }
> + if ((ActiveMask & SR.LaneMask) != 0 &&
> + Pos <= I->end && I->end <= NextPos) {
> + // Merge multiple ends at the same position.
> + if (I->end == NextPos && Event == END_SEGMENT)
> + EventMask |= SR.LaneMask;
> + else {
> + Event = END_SEGMENT;
> + NextPos = I->end;
> + EventMask = SR.LaneMask;
> + }
> + }
> + }
> +
> +#if 1
> + errs() << '\t' << (Event == NOTHING ? "nothing "
> + : Event == BEGIN_SEGMENT ? "begin "
> + : "end ")
> + << NextPos << " mask " << ActiveMask << " evmask " << EventMask << " def " << IsDef << "\n";
> +#endif
> +
Create a sub function for this?
Something like applySegment perhaps?
I found that less critical for the reading though.
> + // Advance scan position.
> + Pos = NextPos;
> + if (Event == BEGIN_SEGMENT) {
> + if (ConstructingSegment && IsDef) {
> + // Finish previous segment because we have to start a new one.
> + CurrentSegment.end = Pos;
> + append(CurrentSegment);
> + ConstructingSegment = false;
> + }
> +
> + // Start a new segment if necessary.
> + if (!ConstructingSegment) {
> + // Determine value number for the segment.
> + VNInfo *VNI;
> + if (IsDef) {
> + VNI = getNextValue(Pos, VNIAllocator);
> + } else {
> + // We have to reuse an existing value number, if we are lucky
> + // then we already passed one of the predecessor blocks and determined
> + // its value number (with blocks in reverse postorder this would be
> + // always true but we have no such guarantee).
> + assert(Pos.isBlock());
> + const MachineBasicBlock *MBB = Indexes.getMBBFromIndex(Pos);
> + // See if any of the predecessor blocks has a lower number and a VNI
> + for (const MachineBasicBlock *Pred : MBB->predecessors()) {
> + SlotIndex PredEnd = Indexes.getMBBEndIdx(Pred);
> + VNI = getVNInfoBefore(PredEnd);
> + if (VNI != nullptr)
> + break;
> + }
> + // Def will come later: We have to do an extra fixup pass.
> + if (VNI == nullptr)
> + NeedVNIFixup = true;
> + }
> +
> + CurrentSegment.start = Pos;
> + CurrentSegment.valno = VNI;
> + ConstructingSegment = true;
> + }
> + ActiveMask |= EventMask;
> + } else if (Event == END_SEGMENT) {
> + assert(ConstructingSegment);
> + // Finish segment if no lane is active anymore.
> + ActiveMask &= ~EventMask;
> + if (ActiveMask == 0) {
> + CurrentSegment.end = Pos;
> + append(CurrentSegment);
> + ConstructingSegment = false;
> + }
> + } else {
> + // We reached the end of the last subranges and can stop.
> + assert(Event == NOTHING);
> + break;
> + }
> + }
> +
> + // We might not be able to assign new valnos for all segments if the basic
> + // block containing the definition comes after a segment using the valno.
> + // Do a fixup pass for this uncommon case.
> + if (NeedVNIFixup) {
Sub function?
> + SmallPtrSet<const MachineBasicBlock*, 5> Visited;
> + for (Segment &S : segments) {
> + if (S.valno != nullptr)
> + continue;
> + // This can only happen at the begin of a basic block.
> + assert(S.start.isBlock());
Message in assert.
> +
> + Visited.clear();
> + const MachineBasicBlock *MBB = Indexes.getMBBFromIndex(S.start);
> + for (const MachineBasicBlock *Pred : MBB->predecessors()) {
> + VNInfo *VNI = searchForVNI(Indexes, *this, Pred, Visited);
> + if (VNI != nullptr) {
> + S.valno = VNI;
> + break;
> + }
> + }
> + assert(S.valno != nullptr);
Message in assert.
> + }
> + }
> + assert(ActiveMask == 0 && !ConstructingSegment);
> + errs() << "Result: " << *this << "\n";
> + verify();
> +}
> +
> unsigned LiveInterval::getSize() const {
> unsigned Sum = 0;
> for (const Segment &S : segments)
>
> Modified: llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp?rev=224806&r1=224805&r2=224806&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp (original)
> +++ llvm/trunk/lib/CodeGen/LiveRangeCalc.cpp Tue Dec 23 20:11:51 2014
> @@ -105,8 +105,9 @@ void LiveRangeCalc::calculate(LiveInterv
> }
> }
>
> - // Create the def in the main liverange.
> - if (MO.isDef())
> + // Create the def in the main liverange. We do not have to do this if
> + // subranges are tracked as we recreate the main range later in this case.
> + if (MO.isDef() && !LI.hasSubRanges())
> createDeadDef(*Indexes, *Alloc, LI, MO);
> }
>
> @@ -116,13 +117,17 @@ void LiveRangeCalc::calculate(LiveInterv
>
> // Step 2: Extend live segments to all uses, constructing SSA form as
> // necessary.
> - for (LiveInterval::SubRange &S : LI.subranges()) {
> + if (LI.hasSubRanges()) {
> + for (LiveInterval::SubRange &S : LI.subranges()) {
> + resetLiveOutMap();
> + extendToUses(S, Reg, S.LaneMask);
> + }
> + LI.clear();
> + LI.constructMainRangeFromSubranges(*Indexes, *Alloc);
> + } else {
> resetLiveOutMap();
> - extendToUses(S, Reg, S.LaneMask);
> + extendToUses(LI, Reg, ~0u);
> }
> -
> - resetLiveOutMap();
> - extendToUses(LI, Reg, ~0u);
> }
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list