Canonicalize compares in combine [3/3] ARM backend part

Hi Richard, this part's for you.

The ARM backend changes needed are very little after the prior patches,
basically just a case in arm_canonicalize_comparison() to detect
(zero_extend:SI (subreg:QI (reg:SI ...) 0)), and swap it into (and:SI
(reg:SI) #255).

Had we not tried the combine modifications, this testcase probably could
have also be solved by implementing another version of the corresponding
*andsi3_compare0/_scratch patterns, with ZERO_EXTEND in the body, and
"ands" in the output assembly. Maybe that's an acceptable solution too...

About the (ab)use of CANONICALIZE_COMPARISON, if it really should be
another macro/hook, then this ARM patch will need updating, but the code
should be similar.

Thanks,
Chung-Lin

Ping.

On 04/22/2011 11:21 PM, Chung-Lin Tang wrote:
> Hi Richard, this part's for you.
> 
> The ARM backend changes needed are very little after the prior patches,
> basically just a case in arm_canonicalize_comparison() to detect
> (zero_extend:SI (subreg:QI (reg:SI ...) 0)), and swap it into (and:SI
> (reg:SI) #255).
> 
> Had we not tried the combine modifications, this testcase probably could
> have also be solved by implementing another version of the corresponding
> *andsi3_compare0/_scratch patterns, with ZERO_EXTEND in the body, and
> "ands" in the output assembly. Maybe that's an acceptable solution too...
> 
> About the (ab)use of CANONICALIZE_COMPARISON, if it really should be
> another macro/hook, then this ARM patch will need updating, but the code
> should be similar.
> 
> Thanks,
> Chung-Lin

Ping.
On 2011/5/9 11:02 PM, Chung-Lin Tang wrote:
> Ping.
> 
> On 04/22/2011 11:21 PM, Chung-Lin Tang wrote:
>> Hi Richard, this part's for you.
>>
>> The ARM backend changes needed are very little after the prior patches,
>> basically just a case in arm_canonicalize_comparison() to detect
>> (zero_extend:SI (subreg:QI (reg:SI ...) 0)), and swap it into (and:SI
>> (reg:SI) #255).
>>
>> Had we not tried the combine modifications, this testcase probably could
>> have also be solved by implementing another version of the corresponding
>> *andsi3_compare0/_scratch patterns, with ZERO_EXTEND in the body, and
>> "ands" in the output assembly. Maybe that's an acceptable solution too...
>>
>> About the (ab)use of CANONICALIZE_COMPARISON, if it really should be
>> another macro/hook, then this ARM patch will need updating, but the code
>> should be similar.
>>
>> Thanks,
>> Chung-Lin
>

On 22/04/11 16:21, Chung-Lin Tang wrote:
> Hi Richard, this part's for you.
> 
> The ARM backend changes needed are very little after the prior patches,
> basically just a case in arm_canonicalize_comparison() to detect
> (zero_extend:SI (subreg:QI (reg:SI ...) 0)), and swap it into (and:SI
> (reg:SI) #255).
> 
> Had we not tried the combine modifications, this testcase probably could
> have also be solved by implementing another version of the corresponding
> *andsi3_compare0/_scratch patterns, with ZERO_EXTEND in the body, and
> "ands" in the output assembly. Maybe that's an acceptable solution too...
> 
> About the (ab)use of CANONICALIZE_COMPARISON, if it really should be
> another macro/hook, then this ARM patch will need updating, but the code
> should be similar.
> 
> Thanks,
> Chung-Lin
> 
> 
> 3-arm-parts.diff
> 
> 
> Index: config/arm/arm.c
> ===================================================================
> --- config/arm/arm.c	(revision 172860)
> +++ config/arm/arm.c	(working copy)
> @@ -3276,6 +3276,19 @@
>        return code;
>      }
>  
> +  /* If *op0 is (zero_extend:SI (subreg:QI (reg:SI) 0)) and comparing
> +     with const0_rtx, change it to (and:SI (reg:SI) (const_int 255)),
> +     to facilitate possible combining with a cmp into 'ands'.  */
> +  if (mode == SImode
> +      && GET_CODE (*op0) == ZERO_EXTEND
> +      && GET_CODE (XEXP (*op0, 0)) == SUBREG
> +      && GET_MODE (XEXP (*op0, 0)) == QImode
> +      && GET_MODE (SUBREG_REG (XEXP (*op0, 0))) == SImode
> +      && SUBREG_BYTE (XEXP (*op0, 0)) == 0
> +      && *op1 == const0_rtx)
> +    *op0 = gen_rtx_AND (SImode, SUBREG_REG (XEXP (*op0, 0)),
> +			GEN_INT (255));
> +

This is wrong for big-endian code.  You should use subreg_lowpart_p to
check the subreg expression (after you've checked that you do have a
subreg, of course).

R.