[llvm] r322313 - PeepholeOptimizer: Do not form PHI with subreg arguments
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 17 08:30:34 PST 2018
Merged to 6.0 in r322684.
On Thu, Jan 11, 2018 at 10:57 PM, Matthias Braun via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: matze
> Date: Thu Jan 11 13:57:03 2018
> New Revision: 322313
>
> URL: http://llvm.org/viewvc/llvm-project?rev=322313&view=rev
> Log:
> 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/trunk/test/CodeGen/ARM/peephole-phi.mir
> Modified:
> llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp
>
> Modified: llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp?rev=322313&r1=322312&r2=322313&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp (original)
> +++ llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp Thu Jan 11 13:57:03 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(),
>
> Added: llvm/trunk/test/CodeGen/ARM/peephole-phi.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/peephole-phi.mir?rev=322313&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/peephole-phi.mir (added)
> +++ llvm/trunk/test/CodeGen/ARM/peephole-phi.mir Thu Jan 11 13:57:03 2018
> @@ -0,0 +1,67 @@
> +# 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
> +...
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list