[llvm-branch-commits] [llvm-branch] r322684 - Merging r322313:

[Hans Wennborg via llvm-branch-commits]

Author: hans
Date: Wed Jan 17 08:29:10 2018
New Revision: 322684

URL: http://llvm.org/viewvc/llvm-project?rev=322684&view=rev
Log:
Merging r322313:
------------------------------------------------------------------------
r322313 | matze | 2018-01-11 13:57:03 -0800 (Thu, 11 Jan 2018) | 18 lines

PeepholeOptimizer: Do not form PHI with subreg arguments

When replacing a PHI the PeepholeOptimizer currently takes the register
class of the register at the first operand. This however is not correct
if this argument has a subregister index.

As there is currently no API to query the register class resulting from
applying a subregister index to all registers in a class, we can only
abort in these cases and not perform the transformation.

This changes findNextSource() to require the end of all copy chains to
not use a subregister if there is any PHI in the chain. I had to rewrite
the overly complicated inner loop there to have a good place to insert
the new check.

This fixes https://llvm.org/PR33071 (aka rdar://32262041)

Differential Revision: https://reviews.llvm.org/D40758
------------------------------------------------------------------------

Added:
    llvm/branches/release_60/test/CodeGen/ARM/peephole-phi.mir
      - copied unchanged from r322313, llvm/trunk/test/CodeGen/ARM/peephole-phi.mir
Modified:
    llvm/branches/release_60/   (props changed)
    llvm/branches/release_60/lib/CodeGen/PeepholeOptimizer.cpp

Propchange: llvm/branches/release_60/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jan 17 08:29:10 2018
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,321789,321791,321862,321870,321872,321980,321991,321993-321994,322056,322103,322106,322223,322272,322473,322623
+/llvm/trunk:155241,321789,321791,321862,321870,321872,321980,321991,321993-321994,322056,322103,322106,322223,322272,322313,322473,322623

Modified: llvm/branches/release_60/lib/CodeGen/PeepholeOptimizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_60/lib/CodeGen/PeepholeOptimizer.cpp?rev=322684&r1=322683&r2=322684&view=diff
==============================================================================
--- llvm/branches/release_60/lib/CodeGen/PeepholeOptimizer.cpp (original)
+++ llvm/branches/release_60/lib/CodeGen/PeepholeOptimizer.cpp Wed Jan 17 08:29:10 2018
@@ -719,15 +719,14 @@ bool PeepholeOptimizer::findNextSource(u
     CurSrcPair = Pair;
     ValueTracker ValTracker(CurSrcPair.Reg, CurSrcPair.SubReg, *MRI,
                             !DisableAdvCopyOpt, TII);
-    ValueTrackerResult Res;
-    bool ShouldRewrite = false;
 
-    do {
-      // Follow the chain of copies until we reach the top of the use-def chain
-      // or find a more suitable source.
-      Res = ValTracker.getNextSource();
+    // Follow the chain of copies until we find a more suitable source, a phi
+    // or have to abort.
+    while (true) {
+      ValueTrackerResult Res = ValTracker.getNextSource();
+      // Abort at the end of a chain (without finding a suitable source).
       if (!Res.isValid())
-        break;
+        return false;
 
       // Insert the Def -> Use entry for the recently found source.
       ValueTrackerResult CurSrcRes = RewriteMap.lookup(CurSrcPair);
@@ -763,24 +762,19 @@ bool PeepholeOptimizer::findNextSource(u
       if (TargetRegisterInfo::isPhysicalRegister(CurSrcPair.Reg))
         return false;
 
+      // Keep following the chain if the value isn't any better yet.
       const TargetRegisterClass *SrcRC = MRI->getRegClass(CurSrcPair.Reg);
-      ShouldRewrite = TRI->shouldRewriteCopySrc(DefRC, SubReg, SrcRC,
-                                                CurSrcPair.SubReg);
-    } while (!ShouldRewrite);
-
-    // Continue looking for new sources...
-    if (Res.isValid())
-      continue;
-
-    // Do not continue searching for a new source if the there's at least
-    // one use-def which cannot be rewritten.
-    if (!ShouldRewrite)
-      return false;
-  }
+      if (!TRI->shouldRewriteCopySrc(DefRC, SubReg, SrcRC, CurSrcPair.SubReg))
+        continue;
 
-  if (PHICount >= RewritePHILimit) {
-    DEBUG(dbgs() << "findNextSource: PHI limit reached\n");
-    return false;
+      // We currently cannot deal with subreg operands on PHI instructions
+      // (see insertPHI()).
+      if (PHICount > 0 && CurSrcPair.SubReg != 0)
+        continue;
+
+      // We found a suitable source, and are done with this chain.
+      break;
+    }
   }
 
   // If we did not find a more suitable source, there is nothing to optimize.
@@ -799,6 +793,9 @@ insertPHI(MachineRegisterInfo *MRI, cons
   assert(!SrcRegs.empty() && "No sources to create a PHI instruction?");
 
   const TargetRegisterClass *NewRC = MRI->getRegClass(SrcRegs[0].Reg);
+  // NewRC is only correct if no subregisters are involved. findNextSource()
+  // should have rejected those cases already.
+  assert(SrcRegs[0].SubReg == 0 && "should not have subreg operand");
   unsigned NewVR = MRI->createVirtualRegister(NewRC);
   MachineBasicBlock *MBB = OrigPHI->getParent();
   MachineInstrBuilder MIB = BuildMI(*MBB, OrigPHI, OrigPHI->getDebugLoc(),