[llvm] r205565 - [ARM64] Teach the ARM64DeadRegisterDefinition pass to respect implicit-defs.

Lang Hames lhames at gmail.com
Wed Apr 9 12:00:07 PDT 2014


Nice. Thanks Dave!

- Lang.


On Sat, Apr 5, 2014 at 3:48 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Apr 3, 2014 at 5:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > On Thu, Apr 3, 2014 at 5:10 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> >>
> >> On Apr 3, 2014, at 5:06 PM, Lang Hames <lhames at gmail.com> wrote:
> >>
> >> Hi Evan,
> >>
> >> The loop in processMachineBasicBlock is only visiting explicit def
> operands,
> >> and the loop in implicitlyDefinesSubReg is only visiting implicit
> operands.
> >> In the common case where there's only a single explicit def I think
> this is
> >> as good as things get. If having more than one explicit def is common
> then
> >> we could optimize to speed up subsequent passes over the implicit
> operands
> >> (by putting only the implicit-defs in a set, for instance, and
> discarding
> >> the implicit uses), but I suspect the overhead required to do that in
> the
> >> common case would outweigh the benefit.
> >>
> >> Caveat: I have a nasty head-cold and could have totally mis-analysed
> this,
> >> so please let me know if I've missed something. :)
> >>
> >>
> >> Ok, I missed that implicitlyDefinesSubReg() is starting at the end of
> >> explicit operands. Ignore me then. :-)
> >
> > Perhaps we could provide some range accessors for the implicit and
> > explicit operands so it's a bit easier to read/tell which things the
> > loop is iterating over.
>
> r205680
>
>
> >
> >>
> >> Evan
> >>
> >>
> >>
> >> Cheers,
> >> Lang.
> >>
> >>
> >>
> >> On Thu, Apr 3, 2014 at 4:48 PM, Evan Cheng <evan.cheng at apple.com>
> wrote:
> >>>
> >>> To clarify. The loop in processMachineBasicBlock() is already visiting
> >>> each operand. For each def operand, it then calls
> implicitlyDefinesSubReg()
> >>> which then iterate over all the operands. That doesn't seem super
> efficient.
> >>>
> >>> Evan
> >>>
> >>> On Apr 3, 2014, at 4:31 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> >>>
> >>> > Why not just look at the implicit def operands? I forgot how
> expensive
> >>> > isSubRegister() is, but the loop still seems slower than necessary.
> >>> >
> >>> > Evan
> >>> >
> >>> > On Apr 3, 2014, at 1:51 PM, Lang Hames <lhames at gmail.com> wrote:
> >>> >
> >>> >> Author: lhames
> >>> >> Date: Thu Apr  3 15:51:08 2014
> >>> >> New Revision: 205565
> >>> >>
> >>> >> URL: http://llvm.org/viewvc/llvm-project?rev=205565&view=rev
> >>> >> Log:
> >>> >> [ARM64] Teach the ARM64DeadRegisterDefinition pass to respect
> >>> >> implicit-defs.
> >>> >>
> >>> >> When rematerializing through truncates, the coalescer may produce
> >>> >> instructions
> >>> >> with dead defs, but live implicit-defs of subregs:
> >>> >> E.g.
> >>> >> %X1<def,dead> = MOVi64imm 2, %W1<imp-def>; %X1:GPR64, %W1:GPR32
> >>> >>
> >>> >> These instructions are live, and their definitions should not be
> >>> >> rewritten.
> >>> >>
> >>> >> Fixes <rdar://problem/16492408>
> >>> >>
> >>> >>
> >>> >> Added:
> >>> >>   llvm/trunk/test/CodeGen/ARM64/dead-register-def-bug.ll
> >>> >> Modified:
> >>> >>   llvm/trunk/lib/Target/ARM64/ARM64DeadRegisterDefinitionsPass.cpp
> >>> >>
> >>> >> Modified:
> >>> >> llvm/trunk/lib/Target/ARM64/ARM64DeadRegisterDefinitionsPass.cpp
> >>> >> URL:
> >>> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM64/ARM64DeadRegisterDefinitionsPass.cpp?rev=205565&r1=205564&r2=205565&view=diff
> >>> >>
> >>> >>
> ==============================================================================
> >>> >> --- llvm/trunk/lib/Target/ARM64/ARM64DeadRegisterDefinitionsPass.cpp
> >>> >> (original)
> >>> >> +++ llvm/trunk/lib/Target/ARM64/ARM64DeadRegisterDefinitionsPass.cpp
> >>> >> Thu Apr  3 15:51:08 2014
> >>> >> @@ -27,6 +27,8 @@ STATISTIC(NumDeadDefsReplaced, "Number o
> >>> >> namespace {
> >>> >> class ARM64DeadRegisterDefinitions : public MachineFunctionPass {
> >>> >> private:
> >>> >> +  const TargetRegisterInfo *TRI;
> >>> >> +  bool implicitlyDefinesSubReg(unsigned Reg, const MachineInstr
> *MI);
> >>> >>  bool processMachineBasicBlock(MachineBasicBlock *MBB);
> >>> >>
> >>> >> public:
> >>> >> @@ -45,6 +47,19 @@ public:
> >>> >> char ARM64DeadRegisterDefinitions::ID = 0;
> >>> >> } // end anonymous namespace
> >>> >>
> >>> >> +bool ARM64DeadRegisterDefinitions::implicitlyDefinesSubReg(
> >>> >> +                                                       unsigned
> Reg,
> >>> >> +                                                       const
> >>> >> MachineInstr *MI) {
> >>> >> +  for (unsigned i = MI->getNumExplicitOperands(), e =
> >>> >> MI->getNumOperands();
> >>> >> +       i != e; ++i) {
> >>> >> +    const MachineOperand &MO = MI->getOperand(i);
> >>> >> +    if (MO.isReg() && MO.isDef())
> >>> >> +      if (TRI->isSubRegister(Reg, MO.getReg()))
> >>> >> +        return true;
> >>> >> +  }
> >>> >> +  return false;
> >>> >> +}
> >>> >> +
> >>> >> bool
> >>> >>
> >>> >>
> ARM64DeadRegisterDefinitions::processMachineBasicBlock(MachineBasicBlock
> >>> >> *MBB) {
> >>> >>  bool Changed = false;
> >>> >> @@ -62,6 +77,11 @@ ARM64DeadRegisterDefinitions::processMac
> >>> >>          DEBUG(dbgs() << "    Ignoring, def is tied operand.\n");
> >>> >>          continue;
> >>> >>        }
> >>> >> +        // Don't change the register if there's an implicit def of
> a
> >>> >> subreg.
> >>> >> +        if (implicitlyDefinesSubReg(MO.getReg(), MI)) {
> >>> >> +          DEBUG(dbgs() << "    Ignoring, implicitly defines
> >>> >> subregister.\n");
> >>> >> +          continue;
> >>> >> +        }
> >>> >>        // Make sure the instruction take a register class that
> contains
> >>> >>        // the zero register and replace it if so.
> >>> >>        unsigned NewReg;
> >>> >> @@ -90,6 +110,7 @@ ARM64DeadRegisterDefinitions::processMac
> >>> >> // register. Replace that register with the zero register when
> >>> >> possible.
> >>> >> bool
> ARM64DeadRegisterDefinitions::runOnMachineFunction(MachineFunction
> >>> >> &mf) {
> >>> >>  MachineFunction *MF = &mf;
> >>> >> +  TRI = MF->getTarget().getRegisterInfo();
> >>> >>  bool Changed = false;
> >>> >>  DEBUG(dbgs() << "***** ARM64DeadRegisterDefinitions *****\n");
> >>> >>
> >>> >>
> >>> >> Added: llvm/trunk/test/CodeGen/ARM64/dead-register-def-bug.ll
> >>> >> URL:
> >>> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM64/dead-register-def-bug.ll?rev=205565&view=auto
> >>> >>
> >>> >>
> ==============================================================================
> >>> >> --- llvm/trunk/test/CodeGen/ARM64/dead-register-def-bug.ll (added)
> >>> >> +++ llvm/trunk/test/CodeGen/ARM64/dead-register-def-bug.ll Thu Apr
>  3
> >>> >> 15:51:08 2014
> >>> >> @@ -0,0 +1,32 @@
> >>> >> +; RUN: llc -mtriple="arm64-apple-ios" < %s | FileCheck %s
> >>> >> +;
> >>> >> +; Check that the dead register definition pass is considering
> implicit
> >>> >> defs.
> >>> >> +; When rematerializing through truncates, the coalescer may produce
> >>> >> instructions
> >>> >> +; with dead defs, but live implicit-defs of subregs:
> >>> >> +; E.g. %X1<def, dead> = MOVi64imm 2, %W1<imp-def>; %X1:GPR64,
> >>> >> %W1:GPR32
> >>> >> +; These instructions are live, and their definitions should not be
> >>> >> rewritten.
> >>> >> +;
> >>> >> +; <rdar://problem/16492408>
> >>> >> +
> >>> >> +define void @testcase() {
> >>> >> +; CHECK: testcase:
> >>> >> +; CHECK-NOT: orr xzr, xzr, #0x2
> >>> >> +
> >>> >> +bb1:
> >>> >> +  %tmp1 = tail call float @ceilf(float 2.000000e+00)
> >>> >> +  %tmp2 = fptoui float %tmp1 to i64
> >>> >> +  br i1 undef, label %bb2, label %bb3
> >>> >> +
> >>> >> +bb2:
> >>> >> +  tail call void @foo()
> >>> >> +  br label %bb3
> >>> >> +
> >>> >> +bb3:
> >>> >> +  %tmp3 = trunc i64 %tmp2 to i32
> >>> >> +  tail call void @bar(i32 %tmp3)
> >>> >> +  ret void
> >>> >> +}
> >>> >> +
> >>> >> +declare void @foo()
> >>> >> +declare void @bar(i32)
> >>> >> +declare float @ceilf(float) nounwind readnone
> >>> >>
> >>> >>
> >>> >> _______________________________________________
> >>> >> llvm-commits mailing list
> >>> >> llvm-commits at cs.uiuc.edu
> >>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>> >
> >>> > _______________________________________________
> >>> > llvm-commits mailing list
> >>> > llvm-commits at cs.uiuc.edu
> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>>
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140409/4e39b80e/attachment.html>


More information about the llvm-commits mailing list