[llvm] r205565 - [ARM64] Teach the ARM64DeadRegisterDefinition pass to respect implicit-defs.
David Blaikie
dblaikie at gmail.com
Sat Apr 5 15:48:54 PDT 2014
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
>>
More information about the llvm-commits
mailing list