[llvm-dev] {ARM} IfConversion does not detect BX instruction as a branch
Kyle Butt via llvm-dev
llvm-dev at lists.llvm.org
Tue Oct 10 18:07:29 PDT 2017
On Tue, Oct 10, 2017 at 4:48 PM, Friedman, Eli via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On 10/9/2017 3:10 AM, Gaël Jobin via llvm-dev wrote:
>
> Hi all,
>
> I got a silly bug when compiling our project with the latest Clang. Here's
> the outputted assembly:
>
> tst r3, #255
> strbeq r6, [r7]
> ldreq r6, [r4, r6, lsl #2]
> strne r6, [r7, #4]
> ldr r6, [r4, r6, lsl #2]
> bx r6
>
> For the code to execute correctly, either the *ldr* should be a *ldrne*
> instruction or the *ldreq* instruction should be removed. The error seems
> to come from the IfConvertion MachinePass. Here's is what it looks like
> before and after.
>
> #BEFORE IfConversion MachinePass
>
> BB#7:
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#5 BB#6
> STRBi12 %R5, %R6<kill>, 0, pred:14, pred:%noreg;
> mem:ST1[%cond.i23.i.i.i]
> %R6<def> = LDRBi12 %R7, 0, pred:14, pred:%noreg; mem:LD1[%15](align=4)
> %R3<def> = EORri %R6, 254, pred:14, pred:%noreg, opt:%noreg
> %R3<def> = ANDrr %R3<kill>, %R6<kill>, pred:14, pred:%noreg, opt:%noreg
> %R6<def> = MOVi 0, pred:14, pred:%noreg, opt:%noreg
> TSTri %R3<kill>, 255, pred:14, pred:%noreg, %CPSR<imp-def>;
> Bcc <BB#9>, pred:0, pred:%CPSR<kill>;
>
>
> BB#8:
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRi12 %R6, %R7<kill>, 4, pred:14, pred:%noreg;
> mem:ST4[%__size_.i3.i.i.i.i]
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg;
> mem:LD4[%0]
> BX %R6<kill>
>
> BB#9:
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg;
> mem:LD4[%0]
> BX %R6<kill>
>
> #AFTER IfConversion MachinePass
>
> BB#7:
> ...
> TSTri %R3<kill>, 255, pred:14, pred:%noreg, %CPSR<imp-def>;
> STRBi12 %R6, %R7, 0, pred:0, pred:%CPSR; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:0, pred:%CPSR,
> %R6<imp-use,kill>; mem:LD4[%0]
> STRi12 %R6, %R7<kill>, 4, pred:1, pred:%CPSR<kill>;
> mem:ST4[%__size_.i3.i.i.i.i]
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg;
> mem:LD4[%0]
> BX %R6<kill>
>
> Inside the *IfConvertDiamondCommon(...)* function of IfConversion.cpp,
> the function is called with *NumDups2=1*, which makes sense because BB#8
> and BB#9 share the same *LDRrs* instruction with the same operands. The
> problem is the call to *TTI->removeBranch(...)* function that does not
> remove the *BX* instruction. Thus, when removing the common instructions,
> the *BX* is removed instead of the *LDRs* instruction.
>
> # Before removeBranch call on MBB1
> BB#9: derived from LLVM BB %if.else.i.i.i.i
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg;
> mem:LD4[%0]
> BX %R6<kill>
>
> # After removeBranch call on MBB1, returned value:0
> BB#9: derived from LLVM BB %if.else.i.i.i.i
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg;
> mem:LD4[%0]
> BX %R6<kill>
>
> #After removing common instructions (NumDups2=1) on MBB1
> BB#9: derived from LLVM BB %if.else.i.i.i.i
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRBi12 %R6, %R7<kill>, 0, pred:14, pred:%noreg; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:14, pred:%noreg; mem:LD4[%0]
>
> #After predicated on MBB1
> BB#9: derived from LLVM BB %if.else.i.i.i.i
> Live Ins: %LR %R0 %R1 %R2 %R4 %R5 %R6 %R7 %R8 %R9 %R10 %R12
> Predecessors according to CFG: BB#7
> STRBi12 %R6, %R7<kill>, 0, pred:0, pred:%CPSR; mem:ST1[%21](align=4)
> %R6<def> = LDRrs %R4, %R6<kill>, 16386, pred:0, pred:%CPSR; mem:LD4[%0]
>
> We can clearly see that the *LDRrs* is still there. As a result, after
> BB#9 and BB#8 have been merged into BB#7, the *LDRs* instruction is done
> twice when the "positive" path is taken.
>
> My current fix is the following:
>
> @@ -408,7 +408,8 @@ unsigned ARMBaseInstrInfo::removeBranch(MachineBasicBlock
> &MBB,
> return 0;
>
> if (!isUncondBranchOpcode(I->getOpcode()) &&
> - !isCondBranchOpcode(I->getOpcode()))
> + !isCondBranchOpcode(I->getOpcode()) &&
> + !isIndirectBranchOpcode(I->getOpcode()))
> return 0;
>
> Does that makes sense?
>
>
> Target-independent code is only supposed to call removeBranch in cases
> where analyzeBranch returns false; as far as I can tell, ARMBaseInstrInfo::analyzeBranch
> will never return false for an indirect branch. So I would guess there's a
> bug in IfConversion, rather than the ARM backend.
>
> -Eli
>
Eli's right.
Gaël, do you think you could patch ifconversion to remove the instructions
rather than relying on removeBranch?
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171010/fac0f6b2/attachment.html>
More information about the llvm-dev
mailing list