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

Lang Hames lhames at gmail.com
Thu Apr 3 17:06:10 PDT 2014


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. :)

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140403/85ff5621/attachment.html>


More information about the llvm-commits mailing list