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