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

[Evan Cheng]

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.

> 
> 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.

Test case?

Evan

> +
>   // Then peek at the next instruction to see if it's predicated on CC or OCC.
>   // If not, then there is nothing to be gained by moving the copy.
>   MachineBasicBlock::iterator I = MI; ++I;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits