[arm] Avoid STRD with odd register for TARGET_ARM in output_move_double

Message ID 5B3634D5.6000808@foss.arm.com
State New
Headers show
Series
  • [arm] Avoid STRD with odd register for TARGET_ARM in output_move_double
Related show

Commit Message

Kyrill Tkachov June 29, 2018, 1:32 p.m.
Hi all,

In this testcase the user forces an odd register as the starting reg for a DFmode value.
The output_move_double function tries to store that using an STRD instruction.
But for TARGET_ARM the starting register of an STRD must be an even one.
This is always the case with compiler-allocated registers for DFmode values, but the
inline assembly forced our hand here.

This patch  restricts the STRD-emitting logic in output_move_double to not avoid
odd-numbered source registers in STRD.
I'm not a fan of the whole function, we should be exposing a lot of the logic in there
to RTL rather than at the final output stage, but that would need to be fixed separately.

This patch is much safer for backporting purposes.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Committing to trunk.
Thanks,
Kyrill

2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (output_move_double): Don't allow STRD instructions
     if starting source register is not even.

2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/arm/arm-soft-strd-even.c: New test.

Comments

Christophe Lyon July 2, 2018, 12:17 p.m. | #1
On Fri, 29 Jun 2018 at 15:32, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>

> Hi all,

>

> In this testcase the user forces an odd register as the starting reg for a DFmode value.

> The output_move_double function tries to store that using an STRD instruction.

> But for TARGET_ARM the starting register of an STRD must be an even one.

> This is always the case with compiler-allocated registers for DFmode values, but the

> inline assembly forced our hand here.

>

> This patch  restricts the STRD-emitting logic in output_move_double to not avoid

> odd-numbered source registers in STRD.

> I'm not a fan of the whole function, we should be exposing a lot of the logic in there

> to RTL rather than at the final output stage, but that would need to be fixed separately.

>

> This patch is much safer for backporting purposes.

>

> Bootstrapped and tested on arm-none-linux-gnueabihf.

>


Hi Kyrill,

I think you want to skip this test if one overrides -mfloat-abi, like
the small attached patch does.

OK?

Thanks,

Christophe

> Committing to trunk.

> Thanks,

> Kyrill

>

> 2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>      * config/arm/arm.c (output_move_double): Don't allow STRD instructions

>      if starting source register is not even.

>

> 2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>

>      * gcc.target/arm/arm-soft-strd-even.c: New test.
gcc/testsuite/ChangeLog:

2018-07-02  Christophe Lyon  <christophe.lyon@linaro.org>

	* gcc.target/arm/arm-soft-strd-even.c: Skip if -mfloat-abi is
	overriden.
diff --git a/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c b/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c
index fb7317c..4ef3dd8 100644
--- a/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c
+++ b/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c
@@ -1,5 +1,6 @@
 /* { dg-do assemble } */
 /* { dg-require-effective-target arm_arm_ok } */
+/* { dg-skip-if "do not override -mfloat-abi" { *-*-* } { "-mfloat-abi=*" } {"-mfloat-abi=soft" } } */
 /* { dg-options "-O2 -marm -mfloat-abi=soft" } */
 
 /* Check that we don't try to emit STRD in ARM state with
Kyrill Tkachov July 2, 2018, 1:58 p.m. | #2
Hi Christophe,

On 02/07/18 13:17, Christophe Lyon wrote:
> On Fri, 29 Jun 2018 at 15:32, Kyrill Tkachov

> <kyrylo.tkachov@foss.arm.com> wrote:

>> Hi all,

>>

>> In this testcase the user forces an odd register as the starting reg for a DFmode value.

>> The output_move_double function tries to store that using an STRD instruction.

>> But for TARGET_ARM the starting register of an STRD must be an even one.

>> This is always the case with compiler-allocated registers for DFmode values, but the

>> inline assembly forced our hand here.

>>

>> This patch  restricts the STRD-emitting logic in output_move_double to not avoid

>> odd-numbered source registers in STRD.

>> I'm not a fan of the whole function, we should be exposing a lot of the logic in there

>> to RTL rather than at the final output stage, but that would need to be fixed separately.

>>

>> This patch is much safer for backporting purposes.

>>

>> Bootstrapped and tested on arm-none-linux-gnueabihf.

>>

> Hi Kyrill,

>

> I think you want to skip this test if one overrides -mfloat-abi, like

> the small attached patch does.

> OK?


Ok.
Thanks,
Kyrill

>

> Thanks,

>

> Christophe

>

>> Committing to trunk.

>> Thanks,

>> Kyrill

>>

>> 2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>>

>>       * config/arm/arm.c (output_move_double): Don't allow STRD instructions

>>       if starting source register is not even.

>>

>> 2018-06-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

>>

>>       * gcc.target/arm/arm-soft-strd-even.c: New test.

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 0c6738cf4d669b17d063e9c5c7ab3b4f455fe8bb..8f1df45fca589c6af2f136bcba30cc1a04b0ceec 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -18455,12 +18455,18 @@  output_move_double (rtx *operands, bool emit, int *count)
       gcc_assert ((REGNO (operands[1]) != IP_REGNUM)
                   || (TARGET_ARM && TARGET_LDRD));
 
+      /* For TARGET_ARM the first source register of an STRD
+	 must be even.  This is usually the case for double-word
+	 values but user assembly constraints can force an odd
+	 starting register.  */
+      bool allow_strd = TARGET_LDRD
+			 && !(TARGET_ARM && (REGNO (operands[1]) & 1) == 1);
       switch (GET_CODE (XEXP (operands[0], 0)))
         {
 	case REG:
 	  if (emit)
 	    {
-	      if (TARGET_LDRD)
+	      if (allow_strd)
 		output_asm_insn ("strd%?\t%1, [%m0]", operands);
 	      else
 		output_asm_insn ("stm%?\t%m0, %M1", operands);
@@ -18468,7 +18474,7 @@  output_move_double (rtx *operands, bool emit, int *count)
 	  break;
 
         case PRE_INC:
-	  gcc_assert (TARGET_LDRD);
+	  gcc_assert (allow_strd);
 	  if (emit)
 	    output_asm_insn ("strd%?\t%1, [%m0, #8]!", operands);
 	  break;
@@ -18476,7 +18482,7 @@  output_move_double (rtx *operands, bool emit, int *count)
         case PRE_DEC:
 	  if (emit)
 	    {
-	      if (TARGET_LDRD)
+	      if (allow_strd)
 		output_asm_insn ("strd%?\t%1, [%m0, #-8]!", operands);
 	      else
 		output_asm_insn ("stmdb%?\t%m0!, %M1", operands);
@@ -18486,7 +18492,7 @@  output_move_double (rtx *operands, bool emit, int *count)
         case POST_INC:
 	  if (emit)
 	    {
-	      if (TARGET_LDRD)
+	      if (allow_strd)
 		output_asm_insn ("strd%?\t%1, [%m0], #8", operands);
 	      else
 		output_asm_insn ("stm%?\t%m0!, %M1", operands);
@@ -18494,7 +18500,7 @@  output_move_double (rtx *operands, bool emit, int *count)
 	  break;
 
         case POST_DEC:
-	  gcc_assert (TARGET_LDRD);
+	  gcc_assert (allow_strd);
 	  if (emit)
 	    output_asm_insn ("strd%?\t%1, [%m0], #-8", operands);
 	  break;
@@ -18505,8 +18511,8 @@  output_move_double (rtx *operands, bool emit, int *count)
 	  otherops[1] = XEXP (XEXP (XEXP (operands[0], 0), 1), 0);
 	  otherops[2] = XEXP (XEXP (XEXP (operands[0], 0), 1), 1);
 
-	  /* IWMMXT allows offsets larger than ldrd can handle,
-	     fix these up with a pair of ldr.  */
+	  /* IWMMXT allows offsets larger than strd can handle,
+	     fix these up with a pair of str.  */
 	  if (!TARGET_THUMB2
 	      && CONST_INT_P (otherops[2])
 	      && (INTVAL(otherops[2]) <= -256
@@ -18571,7 +18577,7 @@  output_move_double (rtx *operands, bool emit, int *count)
 		  return "";
 		}
 	    }
-	  if (TARGET_LDRD
+	  if (allow_strd
 	      && (REG_P (otherops[2])
 		  || TARGET_THUMB2
 		  || (CONST_INT_P (otherops[2])
diff --git a/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c b/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c
new file mode 100644
index 0000000000000000000000000000000000000000..fb7317c8718229f5a6a01a6ce2a734edb466c151
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/arm-soft-strd-even.c
@@ -0,0 +1,18 @@ 
+/* { dg-do assemble } */
+/* { dg-require-effective-target arm_arm_ok } */
+/* { dg-options "-O2 -marm -mfloat-abi=soft" } */
+
+/* Check that we don't try to emit STRD in ARM state with
+   odd starting register.  */
+
+struct S {
+  double M0;
+} __attribute((aligned)) __attribute((packed));
+
+void bar(void *);
+
+void foo(int x, struct S y) {
+  asm("" : : : "r1", "r8", "r7", "r4");
+  y.M0 ?: bar(0);
+  bar(__builtin_alloca(8));
+}