[llvm-commits] [llvm] r141589 - /llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp

[Bill Wendling]

On Oct 10, 2011, at 4:59 PM, Evan Cheng wrote:

> Hi Bill,
> 
> Comments below.
> 
> On Oct 10, 2011, at 3:52 PM, Bill Wendling wrote:
> 
>> Author: void
>> Date: Mon Oct 10 17:52:53 2011
>> New Revision: 141589
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=141589&view=rev
>> Log:
>> If the CPSR is defined by a copy, then we don't want to merge it into an IT
>> block. E.g., if we have:
>> 
>> movs  r1, r1
>> rsb   r1, 0
>> movs  r2, r2
>> rsb   r2, 0
>> 
>> we don't want this to be converted to:
>> 
>> movs  r1, r1
>> movs  r2, r2
>> itt   mi
>> rsb   r1, 0
>> rsb   r2, 0
> 
> The comment is confusing. What exactly does the input instructions look like? Which instructions are predicated on 'mi'? Please clarify.
> 
BB#1: derived from LLVM BB %if.end
    Live Ins: %R0 %R1 %R2 %R3 %R9 %R10
    Predecessors according to CFG: BB#0
	t2CMPri %R2, 1, pred:14, pred:%noreg, %CPSR<imp-def>
	%R6<def> = t2MOVi 0, pred:14, pred:%noreg, opt:%noreg
	%R6<def> = t2MOVi 1, pred:11, pred:%CPSR, opt:%noreg
	t2STRi12 %R6<kill>, %SP, 0, pred:14, pred:%noreg; mem:ST4[FixedStack0]
	t2CMPri %R1, 1, pred:14, pred:%noreg, %CPSR<imp-def>
	%R0<def> = t2MOVi 1, pred:11, pred:%CPSR, opt:%noreg
(1)	%R2<def> = t2MOVr %R2<kill>, pred:14, pred:%noreg, opt:%CPSR<def>
(2)	%R2<def> = t2RSBri %R2<kill>, 0, pred:4, pred:%CPSR, opt:%noreg
(3)	%R6<def> = t2MOVr %R1<kill>, pred:14, pred:%noreg, opt:%CPSR<def>
(4)	%R6<def> = t2RSBri %R6<kill>, 0, pred:4, pred:%CPSR, opt:%noreg
	t2CMPri %R9, 16, pred:14, pred:%noreg, %CPSR<imp-def>
	t2Bcc <BB#10>, pred:11, pred:%CPSR
    Successors according to CFG: BB#2 BB#10

Instructions (1), (2), (3), and (4) are the ones affected by this patch. The t2RSBri instructions are the ones which are predicated on the 'mi'. What happens is that the IT pass looks at (1), sees that it can use an IT block for the following t2RSBri instruction. It then looks at the the next instruction to see if it can include it into the IT block. The next instruction is a copy. It didn't check if the CPSR was defined in that copy, so it moved it before the IT block. It then included (4) into the IT block.


>> 
>> PR11107 & <rdar://problem/10259534>
>> 
>> Modified:
>>   llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp
>> 
>> Modified: llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp?rev=141589&r1=141588&r2=141589&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/Thumb2ITBlockPass.cpp Mon Oct 10 17:52:53 2011
>> @@ -124,6 +124,28 @@
>>  if (Uses.count(DstReg) || Defs.count(SrcReg))
>>    return false;
>> 
>> +  // If the CPSR is defined by this copy, then we don't want to move it. E.g.,
>> +  // if we have:
>> +  //
>> +  //   movs  r1, r1
>> +  //   rsb   r1, 0
>> +  //   movs  r2, r2
>> +  //   rsb   r2, 0
>> +  //
>> +  // we don't want this to be converted to:
>> +  //
>> +  //   movs  r1, r1
>> +  //   movs  r2, r2
>> +  //   itt   mi
>> +  //   rsb   r1, 0
>> +  //   rsb   r2, 0
>> +  //
>> +  // 
>> +  for (unsigned I = 0, E = MI->getNumOperands(); I != E; ++I)
>> +    if (MI->getOperand(I).isReg() && MI->getOperand(I).getReg() == ARM::CPSR &&
>> +        MI->getOperand(I).isDef())
>> +      return false;
> 
> Since the code already knows it's a 'copy' instruction. Can't it simply look at the optional def and see if it's set to CPSR? At least the look should not start on index 0.
> 
r141602.

> Test case?
> 
r141607.

-bw