[PATCH] D22031: TailDuplicator: Remove live-in updating logic

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 20:58:30 PDT 2016


MatzeB created this revision.
MatzeB added reviewers: qcolombet, atrick.
MatzeB added a subscriber: llvm-commits.
MatzeB set the repository for this revision to rL LLVM.
Herald added a subscriber: mcrosier.

This logic was introduced in r157663 and does not make any sense to me.
The motivating example in rdar://11538365 looks like this:

This is the tail:
BB#16: derived from LLVM BB %if.end68
    Live Ins: %R0 %R4 %R5
    Predecessors according to CFG: BB#15 BB#5
        tBLXi pred:14, pred:%noreg, <ga:@CFRelease>, %R0<kill>, <regmask>, %LR<imp-def,dead>, %SP<imp-use>, %SP<imp-def>
        t2B <BB#20>, pred:14, pred:%noreg
    Successors according to CFG: BB#20

This is the predBB:
BB#5:
    Live Ins: %R5
    Predecessors according to CFG: BB#4
        %R4<def> = t2MOVi 0, pred:14, pred:%noreg, opt:%noreg
        t2B <BB#16>, pred:14, pred:%noreg
    Successors according to CFG: BB#16

However this is invalid machine code to begin with, if %R0 is live-in to
BB#16 then it must be live-in to BB#5 as well if BB#5 does not define
it.  We should not need logic to retroactively fix broken machine code
and in fact the example from r157663 passes cleanly today with the updating 
code removed and I do not see any (newly) failing tests with the machine
verifier enabled.

Repository:
  rL LLVM

http://reviews.llvm.org/D22031

Files:
  include/llvm/CodeGen/TailDuplicator.h
  lib/CodeGen/TailDuplicator.cpp

Index: lib/CodeGen/TailDuplicator.cpp
===================================================================
--- lib/CodeGen/TailDuplicator.cpp
+++ lib/CodeGen/TailDuplicator.cpp
@@ -67,10 +67,6 @@
   assert(MBPI != nullptr && "Machine Branch Probability Info required");
 
   PreRegAlloc = MRI->isSSA();
-  RS.reset();
-
-  if (MRI->tracksLiveness() && TRI->trackLivenessAfterRegAlloc(MF))
-    RS.reset(new RegScavenger());
 }
 
 static void VerifyPHIs(MachineFunction &MF, bool CheckExtra) {
@@ -770,20 +766,6 @@
     // Remove PredBB's unconditional branch.
     TII->RemoveBranch(*PredBB);
 
-    if (RS && !TailBB->livein_empty()) {
-      // Update PredBB livein.
-      RS->enterBasicBlock(*PredBB);
-      if (!PredBB->empty())
-        RS->forward(std::prev(PredBB->end()));
-      for (const auto &LI : TailBB->liveins()) {
-        if (!RS->isRegUsed(LI.PhysReg, false))
-          // If a register is previously livein to the tail but it's not live
-          // at the end of predecessor BB, then it should be added to its
-          // livein list.
-          PredBB->addLiveIn(LI);
-      }
-    }
-
     // Clone the contents of TailBB into PredBB.
     DenseMap<unsigned, RegSubRegPair> LocalVRMap;
     SmallVector<std::pair<unsigned, RegSubRegPair>, 4> CopyInfos;
Index: include/llvm/CodeGen/TailDuplicator.h
===================================================================
--- include/llvm/CodeGen/TailDuplicator.h
+++ include/llvm/CodeGen/TailDuplicator.h
@@ -33,7 +33,6 @@
   const MachineBranchProbabilityInfo *MBPI;
   const MachineModuleInfo *MMI;
   MachineRegisterInfo *MRI;
-  std::unique_ptr<RegScavenger> RS;
   bool PreRegAlloc;
 
   // A list of virtual registers for which to update SSA form.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22031.62819.patch
Type: text/x-patch
Size: 1727 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160706/3aae6947/attachment.bin>


More information about the llvm-commits mailing list