On 05/20/2011 02:37 PM, Andrew Stubbs wrote:
On 20/05/11 10:45, Dmitry Plotnikov wrote:
This patch adds support for 12-bit immediate values for Thumb-2 in ADD and
SUB instructions.  We added two new alternatives for *arm_addsi3 which
make use of two new constraints for 12-bit values.  Also we modified
costs of PLUS rtx expression.

A already have a patch submitted for review that does all this.

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01657.html
Sorry, we missed this message.

This patch reduces size of libevas by 1788 bytes (from 464916 to
463128), and sqlite by 54 bytes (from 266156 to 266052).
Regtested with Cortex-A8 QEMU.

I also have another patch that improves support for thumb2 replicated constants. I'd be interested whether the patch makes a difference to your benchmark?

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01757.html (approved, but blocked on the addw/subw review).
I tested the patches on libevas. Code size is reduced by 64 bytes with your addw/subw patch (this effect is actually similar to our patch; there was an error with numbers reported in the previous message). Adding your replicated constants patch reduces code size additionally by 808 bytes. These patches didn't affect the performance on libevas.

+/* Return TRUE if int I is a valid immediate THUMB-2 constant in add/sub
+ *  instructions in 12 bit encoding variants.  */
+
+int
+const_ok_for_thumb2_12bit (HOST_WIDE_INT i)
+{
+ /* According to Thumb-2 instruction set manual such constants should be
+   *  in range 0-4095.  */
+  if ((i&  ~(unsigned HOST_WIDE_INT) 0xfff) == 0)
+    return TRUE;
+
+  return FALSE;
+}

As Richard and Chung-Lin said, we have const_ok_for_op for this sort of thing. I already patched it for movw 16-bit constants, and my patch above covers addw and subw.

@@ -7164,7 +7179,10 @@ arm_rtx_costs_1 (rtx x, enum rtx_code outer, int* total, bool speed)

      case CONST_INT:
        if (const_ok_for_arm (INTVAL (x))
-      || const_ok_for_arm (~INTVAL (x)))
+      || const_ok_for_arm (~INTVAL (x))
+      || (TARGET_THUMB2&&  (outer == PLUS)
+ &&  (const_ok_for_thumb2_12bit (INTVAL (x))
+              || const_ok_for_thumb2_12bit (~INTVAL (x)))))
      *total = COSTS_N_INSNS (1);
        else
      *total = COSTS_N_INSNS (arm_gen_constant (SET, mode, NULL_RTX,

I think my patch may be missing this bit - good catch. It really should be recast in terms of const_ok_for_op, though.

Would you include this in your patch? Or should we submit it as a separate patch?

--
Best regards,
  Dmitry



Reply via email to