[llvm-dev] {ARM} IfConversion does not detect BX instruction as a branch

[Gaël Jobin via llvm-dev]

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? 

Regards, 

Gael
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171009/76f60f27/attachment.html>