Commit b539787a authored by Hans Wennborg's avatar Hans Wennborg
Browse files

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
------------------------------------------------------------------------

llvm-svn: 322684
parent 1219711c

llvm/lib/CodeGen/PeepholeOptimizer.cpp

+19 −22
Original line number Diff line number Diff line
@@ -719,15 +719,14 @@ bool PeepholeOptimizer::findNextSource(unsigned Reg, unsigned SubReg, 
    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(unsigned Reg, unsigned SubReg, 
      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);

      if (!TRI->shouldRewriteCopySrc(DefRC, SubReg, SrcRC, CurSrcPair.SubReg))

        continue;



    // Continue looking for new sources...

    if (Res.isValid())

      // We currently cannot deal with subreg operands on PHI instructions

      // (see insertPHI()).

      if (PHICount > 0 && CurSrcPair.SubReg != 0)

        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;

      // We found a suitable source, and are done with this chain.

      break;

    }



  if (PHICount >= RewritePHILimit) {

    DEBUG(dbgs() << "findNextSource: PHI limit reached\n");

    return false;

  }



  // If we did not find a more suitable source, there is nothing to optimize.
@@ -799,6 +793,9 @@ insertPHI(MachineRegisterInfo *MRI, const TargetInstrInfo *TII, 
  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(),

llvm/test/CodeGen/ARM/peephole-phi.mir

0 → 100644
+67 −0
Original line number Diff line number Diff line 
# RUN: llc -o - %s -mtriple=armv7-- -verify-machineinstrs -run-pass=peephole-opt | FileCheck %s

#

# Make sure we do not crash on this input.

# Note that this input could in principle be optimized, but right now we don't

# have this case implemented so the output should simply be unchanged.

#

# CHECK-LABEL: name: func

# CHECK: body: |

# CHECK:   bb.0:

# CHECK:     Bcc %bb.2, 1, undef %cpsr

#

# CHECK:   bb.1:

# CHECK:     %0:dpr = IMPLICIT_DEF

# CHECK:     %1:gpr, %2:gpr = VMOVRRD %0, 14, %noreg

# CHECK:     B %bb.3

#

# CHECK:   bb.2:

# CHECK:     %3:spr = IMPLICIT_DEF

# CHECK:     %4:gpr = VMOVRS %3, 14, %noreg

#

# CHECK:   bb.3:

# CHECK:     %5:gpr = PHI %1, %bb.1, %4, %bb.2

# CHECK:     %6:spr = VMOVSR %5, 14, %noreg

---

name: func0

tracksRegLiveness: true

body: |

  bb.0:

    Bcc %bb.2, 1, undef %cpsr



  bb.1:

    %0:dpr = IMPLICIT_DEF

    %1:gpr, %2:gpr = VMOVRRD %0:dpr, 14, %noreg

    B %bb.3



  bb.2:

    %3:spr = IMPLICIT_DEF

    %4:gpr = VMOVRS %3:spr, 14, %noreg



  bb.3:

    %5:gpr = PHI %1, %bb.1, %4, %bb.2

    %6:spr = VMOVSR %5, 14, %noreg

...



# CHECK-LABEL: name: func1

# CHECK:    %6:spr = PHI %0, %bb.1, %2, %bb.2

# CHEKC:    %7:spr = COPY %6

---

name: func1

tracksRegLiveness: true

body: |

  bb.0:

    Bcc %bb.2, 1, undef %cpsr



  bb.1:

    %1:spr = IMPLICIT_DEF

    %0:gpr = VMOVRS %1, 14, %noreg

    B %bb.3



  bb.2:

    %3:spr = IMPLICIT_DEF

    %2:gpr = VMOVRS %3:spr, 14, %noreg



  bb.3:

    %4:gpr = PHI %0, %bb.1, %2, %bb.2

    %5:spr = VMOVSR %4, 14, %noreg

...