Re: [PATCH][ARM] PR60663: Improve RTX costs for asm statements

[Kyrill Tkachov]

On 15/04/14 14:55, Richard Earnshaw wrote:

On 15/04/14 11:56, Kyrill Tkachov wrote:

Hi all,

This patch relates to PR60663 where cse got confused due to asm statements 
being given a cost of zero in the arm backend. Jakub already put in a fix to 
cse for 4.9.0 (http://gcc.gnu.org/ml/gcc-patches/2014-04/msg00512.html) but we 
should still fix the costs in arm.

This patch does that by estimating the number of instructions in the asm 
statement, adding the cost of the input operands and making sure that it's at 
least COSTS_N_INSNS (1).

Tested and bootstrapped on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2014-04-15  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

        PR rtl-optimization/60663
        * config/arm/arm.c (arm_new_rtx_costs): Improve ASM_OPERANDS case,
        avoid 0 cost.

I'd be inclined to use 1 + number of operands, rather than MAX.

OK with that change.

Thanks Richard,

I've committed this slightly modified version as version as r209419.

Kyrill

R.

pr60663.patch


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 91e4cd8..ce7ee82 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -10758,10 +10758,16 @@ arm_new_rtx_costs (rtx x, enum rtx_code code, enum 
rtx_code outer_code,
        return true;

case ASM_OPERANDS:

-      /* Just a guess.  Cost one insn per input.  */
-      *cost = COSTS_N_INSNS (ASM_OPERANDS_INPUT_LENGTH (x));
-      return true;
+      {
+      /* Just a guess.  Guess number of instructions in the asm
+         plus one insn per input.  Always a minimum of COSTS_N_INSNS (1)
+         though (see PR60663).  */
+        int asm_length = asm_str_count (ASM_OPERANDS_TEMPLATE (x));
+        int num_operands = ASM_OPERANDS_INPUT_LENGTH (x);

+ *cost = COSTS_N_INSNS (MAX (1, asm_length + num_operands));

+        return true;
+      }
      default:
        if (mode != VOIDmode)
        *cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index e5cf503..773c353 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -10670,10 +10670,16 @@ arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
       return true;
 
     case ASM_OPERANDS:
-      /* Just a guess.  Cost one insn per input.  */
-      *cost = COSTS_N_INSNS (ASM_OPERANDS_INPUT_LENGTH (x));
-      return true;
+      {
+      /* Just a guess.  Guess number of instructions in the asm
+         plus one insn per input.  Always a minimum of COSTS_N_INSNS (1)
+         though (see PR60663).  */
+        int asm_length = MAX (1, asm_str_count (ASM_OPERANDS_TEMPLATE (x)));
+        int num_operands = ASM_OPERANDS_INPUT_LENGTH (x);
 
+        *cost = COSTS_N_INSNS (asm_length + num_operands);
+        return true;
+      }
     default:
       if (mode != VOIDmode)
 	*cost = COSTS_N_INSNS (ARM_NUM_REGS (mode));