[PATCH] Fix PR40741 (ARM pessimization)

[Richard Earnshaw]

On Sat, 2009-08-01 at 23:24 +0200, Paolo Bonzini wrote:
> This patch fixes PR40741, an ARM pessimization introduced by the
> second round of cond-optab changes (the post-merge changes).
> 
> The main reason here is that the new optimization possibilities
> in emit_store_flag were not conditionalized on their costs.
> This is what this patch does.
> 
> In addition, I took the occasion to tweak the patterns and costs
> for Thumb AND/IOR/XOR.  This saved one instruction even without
> the target-independent patch.  The change to the pattern is to
> exchange the arguments of
> 
>    (set (reg:SI A) (and/ior/xor:SI (reg:SI B) (const_int N)))
> 
> thus generating
> 
>    (set (reg:SI C) (const_int N))         ;; any non-reg actually
>    (set (reg:SI A) (reg:SI C) (reg:SI B))
> 
> and simplifying the task of tying regs A and C to the same register.
> 

Hm, sounds like the register allocator isn't dealing with this case
properly...  :-(  The patterns are correctly marked as commutative.

> On ARM, in addition, in the presence of a complex constant the
> XOR can be done piecewise.
> 
> Tested i686-pc-linux-gnu, plus on arm-elf I looked by hand at some
> testcases to test the paths I modified, but building a combined
> tree failed with an internal assembler error.
> 
> Ok?
> 
> Paolo
> 
> 2009-08-01  Paolo Bonzini  <bonzini@gnu.org>
> 
> 	PR rtl-optimization/40741
> 	* config/arm/arm.c (thumb1_rtx_costs): IOR or XOR with
> 	a small constant is cheap.
> 	* config/arm/arm.c (andsi3, iorsi3): Try to place the
                          ^^^
ITYM arm.md

> 	result of force_reg on the LHS.
> 	(xorsi3): Likewise, and split the XOR if the constant
> 	is complex and not in Thumb mode.
> 	* expmed.c (emit_store_flag): Check costs before
> 	transforming to the opposite representation.
> 
> 2009-08-01  Paolo Bonzini  <bonzini@gnu.org>
> 
> 	* gcc.target/arm/thumb-branch1.c: New.
> 
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	(branch cond-optab2)
> +++ gcc/config/arm/arm.c	(working copy)
> @@ -5124,6 +5124,9 @@ thumb1_rtx_costs (rtx x, enum rtx_code c
>        else if ((outer == PLUS || outer == COMPARE)
>  	       && INTVAL (x) < 256 && INTVAL (x) > -256)
>  	return 0;
> +      else if ((outer == IOR || outer == XOR)
> +	       && INTVAL (x) < 256 && INTVAL (x) >= 0)
> +	return COSTS_N_INSNS (1);

This should be merged into the following case (all of these operations
can use -256...255 with one insn cost by using either mov or mvn
immediate).

Otherwise OK.

R.

btw adding "-F ^(define" to your diff options makes patches to .md files
much easier to read.