<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 10, 2017 at 4:48 PM, Friedman, Eli via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="cremed">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><div><div class="h5">
<div class="m_-1428546983460218606moz-cite-prefix">On 10/9/2017 3:10 AM, Gaël Jobin via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<p>Hi all,</p>
<p>I got a silly bug when compiling our project with the latest
Clang. Here's the outputted assembly:</p>
<blockquote type="cite" style="padding:0 0.4em;border-left:#1010ff 2px solid;margin:0">
<p>tst r3, #255<br>
strbeq r6, [r7]<br>
ldreq r6, [r4, r6, lsl #2]<br>
strne r6, [r7, #4]<br>
ldr r6, [r4, r6, lsl #2]<br>
bx r6</p>
</blockquote>
<p>For the code to execute correctly, either the <em>ldr</em>
should be a <em>ldrne</em> instruction or the <em>ldreq</em>
instruction should be removed. The error seems to come from the
IfConvertion MachinePass. Here's is what it looks like before
and after.</p>
<blockquote type="cite" style="padding:0 0.4em;border-left:#1010ff 2px solid;margin:0">
<p>#BEFORE <span>IfConversion MachinePass</span></p>
<p>BB#7:<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12<br>
Predecessors according to CFG: BB#5 BB#6<br>
STRBi12 %R5, %R6<kill>, 0, pred:14, pred:%noreg;
mem:ST1[%cond.i23.i.i.i] <br>
%R6<def> = LDRBi12 %R7, 0, pred:14, pred:%noreg;
mem:LD1[%15](align=4) <br>
%R3<def> = EORri %R6, 254, pred:14, pred:%noreg,
opt:%noreg<br>
%R3<def> = ANDrr %R3<kill>, %R6<kill>,
pred:14, pred:%noreg, opt:%noreg<br>
%R6<def> = MOVi 0, pred:14, pred:%noreg, opt:%noreg<br>
TSTri %R3<kill>, 255, pred:14, pred:%noreg,
%CPSR<imp-def>; <br>
Bcc <BB#9>, pred:0, pred:%CPSR<kill>;</p>
<p><br>
BB#8:<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12<br>
Predecessors according to CFG: BB#7<br>
STRi12 %R6, %R7<kill>, 4, pred:14, pred:%noreg;
mem:ST4[%__size_.i3.i.i.i.i] <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
BX %R6<kill></p>
<p>BB#9:<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12<br>
Predecessors according to CFG: BB#7<br>
STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
BX %R6<kill></p>
<p>#AFTER IfConversion MachinePass</p>
<p><span>BB#7:<br>
</span><span> ...<br>
</span> TSTri %R3<kill>, 255, pred:14, pred:%noreg,
%CPSR<imp-def>; <br>
STRBi12 %R6, %R7, 0, pred:0, pred:%CPSR;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:0,
pred:%CPSR, %R6<imp-use,kill>; mem:LD4[%0]<br>
STRi12 %R6, %R7<kill>, 4, pred:1,
pred:%CPSR<kill>; mem:ST4[%__size_.i3.i.i.i.i] <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
BX %R6<kill></p>
</blockquote>
<p>Inside the <em>IfConvertDiamondCommon(...)</em> function of
IfConversion.cpp, the function is called with <em>NumDups2=1</em>,
which makes sense because <span>BB#8 and BB#9 share the same <em>LDRrs</em>
instruction with the same operands. The problem is the call to
<em>TTI->removeBranch(...)</em> function that does not
remove the <em>BX</em> instruction. Thus, when removing the
common instructions, the <em>BX</em> is removed instead of
the <em>LDRs</em> instruction.</span></p>
<blockquote type="cite" style="padding:0 0.4em;border-left:#1010ff 2px solid;margin:0">
<p><span># Before removeBranch call on MBB1<br>
BB#9: derived from LLVM BB %if.else.i.i.i.i<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10
%R12<br>
Predecessors according to CFG: BB#7<br>
STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
BX %R6<kill><br>
</span><span><br>
# After removeBranch call on MBB1, returned value:0<br>
BB#9: derived from LLVM BB %if.else.i.i.i.i<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10
%R12<br>
Predecessors according to CFG: BB#7<br>
STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
BX %R6<kill><br>
</span></p>
<p><span>#After removing common instructions (NumDups2=1) on
MBB1<br>
BB#9: derived from LLVM BB %if.else.i.i.i.i<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10
%R12<br>
Predecessors according to CFG: BB#7<br>
STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:14, pred:%noreg; mem:LD4[%0]<br>
</span></p>
<p><span>#After predicated on MBB1<br>
BB#9: derived from LLVM BB %if.else.i.i.i.i<br>
Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10
%R12<br>
Predecessors according to CFG: BB#7<br>
STRBi12 %R6, %R7<kill>, 0, pred:0, pred:%CPSR</span><span>;
mem:ST1[%21](align=4) <br>
%R6<def> = LDRrs %R4, %R6<kill>, 16386,
pred:0, pred:%CPSR; mem:LD4[%0]</span></p>
</blockquote>
<p><span>We can clearly see that the <em>LDRrs</em> is still
there. As a result, after BB#9 and BB#8 have been merged
into BB#7, the <em>LDRs</em> instruction is done twice when
the "positive" path is taken.</span></p>
<p><span>My current fix is the following:</span></p>
<blockquote type="cite" style="padding:0 0.4em;border-left:#1010ff 2px solid;margin:0">
<p><span>@@ -408,7 +408,8 @@ unsigned
ARMBaseInstrInfo::<wbr>removeBranch(MachineBasicBlock &MBB,<br>
return 0;<br>
<br>
if (!isUncondBranchOpcode(I-><wbr>getOpcode()) &&<br>
- !isCondBranchOpcode(I-><wbr>getOpcode()))<br>
+ !isCondBranchOpcode(I-><wbr>getOpcode()) &&<br>
+ !isIndirectBranchOpcode(I-><wbr>getOpcode()))<br>
return 0;<br>
</span></p>
</blockquote>
<p><span>Does that makes sense?</span></p>
</blockquote>
<br></div></div>
Target-independent code is only supposed to call removeBranch in
cases where analyzeBranch returns false; as far as I can tell,
ARMBaseInstrInfo::<wbr>analyzeBranch will never return false for an
indirect branch. So I would guess there's a bug in IfConversion,
rather than the ARM backend.<br>
<br>
-Eli</div></blockquote><div><br></div><div>Eli's right. </div><div> </div><div>Gaël, do you think you could patch ifconversion to remove the instructions rather than relying on removeBranch?</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><span class="HOEnZb"><font color="#888888"><br>
<pre class="m_-1428546983460218606moz-signature" cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
</font></span></div>
<br>______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" class="cremed">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="cremed">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div></div>