diff mbox

[ARM] Fix ldrd offsets

Message ID AM5PR0802MB2610C0616A19887E6FA5928E83A30@AM5PR0802MB2610.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Wilco Dijkstra Nov. 3, 2016, 12:20 p.m. UTC
Fix ldrd offsets of Thumb-2 - for TARGET_LDRD the range is +-1020,
without -255..4091.  This reduces the number of addressing instructions
when using DI mode operations (such as in PR77308).

Bootstrap & regress OK.

ChangeLog:
2015-11-03  Wilco Dijkstra  <wdijkstr@arm.com>

    gcc/
	* config/arm/arm.c (arm_legitimate_index_p): Add comment.
	(thumb2_legitimate_index_p): Use correct range for DI/DF mode.
--

Comments

Ramana Radhakrishnan Nov. 8, 2016, 2:08 p.m. UTC | #1
On Thu, Nov 3, 2016 at 12:20 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Fix ldrd offsets of Thumb-2 - for TARGET_LDRD the range is +-1020,
> without -255..4091.  This reduces the number of addressing instructions
> when using DI mode operations (such as in PR77308).
>
> Bootstrap & regress OK.
>
> ChangeLog:
> 2015-11-03  Wilco Dijkstra  <wdijkstr@arm.com>
>
>     gcc/
>         * config/arm/arm.c (arm_legitimate_index_p): Add comment.
>         (thumb2_legitimate_index_p): Use correct range for DI/DF mode.
> --
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 3c4c7042d9c2101619722b5822b3d1ca37d637b9..5d12cf9c46c27d60a278d90584bde36ec86bb3fe 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -7486,6 +7486,8 @@ arm_legitimate_index_p (machine_mode mode, rtx index, RTX_CODE outer,
>         {
>           HOST_WIDE_INT val = INTVAL (index);
>
> +         /* Assume we emit ldrd or 2x ldr if !TARGET_LDRD.
> +            If vldr is selected it uses arm_coproc_mem_operand.  */
>           if (TARGET_LDRD)
>             return val > -256 && val < 256;
>           else
> @@ -7613,11 +7615,13 @@ thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
>        if (code == CONST_INT)
>         {
>           HOST_WIDE_INT val = INTVAL (index);
> -         /* ??? Can we assume ldrd for thumb2?  */
> -         /* Thumb-2 ldrd only has reg+const addressing modes.  */
> -         /* ldrd supports offsets of +-1020.
> -            However the ldr fallback does not.  */
> -         return val > -256 && val < 256 && (val & 3) == 0;
> +         /* Thumb-2 ldrd only has reg+const addressing modes.
> +            Assume we emit ldrd or 2x ldr if !TARGET_LDRD.
> +            If vldr is selected it uses arm_coproc_mem_operand.  */
> +         if (TARGET_LDRD)

I suspect this should be : if (TARGET_LDRD && !fix_cm3_ldrd)  - I am a
bit worried about this change because of the non-uniformity with ldr
and the fallout with other places where things may break with this.  I
would like a test with -mcpu=cortex-m3/-mthumb as well for an
arm-none-eabi target to see what the fallout of this change is on that
...


Ramana
Wilco Dijkstra Nov. 11, 2016, 10:59 a.m. UTC | #2
Ramana Radhakrishnan wrote:
> On Thu, Nov 3, 2016 at 12:20 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:

>           HOST_WIDE_INT val = INTVAL (index);
> -         /* ??? Can we assume ldrd for thumb2?  */
> -         /* Thumb-2 ldrd only has reg+const addressing modes.  */
> -         /* ldrd supports offsets of +-1020.
> -            However the ldr fallback does not.  */
> -         return val > -256 && val < 256 && (val & 3) == 0;
> +         /* Thumb-2 ldrd only has reg+const addressing modes.
> +            Assume we emit ldrd or 2x ldr if !TARGET_LDRD.
> +            If vldr is selected it uses arm_coproc_mem_operand.  */
> +         if (TARGET_LDRD)

> I suspect this should be : if (TARGET_LDRD && !fix_cm3_ldrd)  - I am a
> bit worried about this change because of the non-uniformity with ldr
> and the fallout with other places where things may break with this.  I
> would like a test with -mcpu=cortex-m3/-mthumb as well for an
> arm-none-eabi target to see what the fallout of this change is on that

Well it works fine given that Thumb-2 supports add/sub up to 4KB, so
the existing expansion into add+ldrd for the fix_cm3_ldrd case works fine.
I ran a bootstrap with fix_cm3_ldrd forced to true, and that completed without
any issues.

Wilco
diff mbox

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 3c4c7042d9c2101619722b5822b3d1ca37d637b9..5d12cf9c46c27d60a278d90584bde36ec86bb3fe 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -7486,6 +7486,8 @@  arm_legitimate_index_p (machine_mode mode, rtx index, RTX_CODE outer,
 	{
 	  HOST_WIDE_INT val = INTVAL (index);
 
+	  /* Assume we emit ldrd or 2x ldr if !TARGET_LDRD.
+	     If vldr is selected it uses arm_coproc_mem_operand.  */
 	  if (TARGET_LDRD)
 	    return val > -256 && val < 256;
 	  else
@@ -7613,11 +7615,13 @@  thumb2_legitimate_index_p (machine_mode mode, rtx index, int strict_p)
       if (code == CONST_INT)
 	{
 	  HOST_WIDE_INT val = INTVAL (index);
-	  /* ??? Can we assume ldrd for thumb2?  */
-	  /* Thumb-2 ldrd only has reg+const addressing modes.  */
-	  /* ldrd supports offsets of +-1020.
-	     However the ldr fallback does not.  */
-	  return val > -256 && val < 256 && (val & 3) == 0;
+	  /* Thumb-2 ldrd only has reg+const addressing modes.
+	     Assume we emit ldrd or 2x ldr if !TARGET_LDRD.
+	     If vldr is selected it uses arm_coproc_mem_operand.  */
+	  if (TARGET_LDRD)
+	    return IN_RANGE (val, -1020, 1020) && (val & 3) == 0;
+	  else
+	    return IN_RANGE (val, -255, 4095 - 4);
 	}
       else
 	return 0;