[AArch64] Reject (high (const (plus anchor offset)))

Message ID 87608ha3s1.fsf@linaro.org
State New
Headers show
Series
  • [AArch64] Reject (high (const (plus anchor offset)))
Related show

Commit Message

Richard Sandiford Jan. 4, 2018, 6:15 p.m.
The aarch64_legitimate_constant_p tests for HIGH and CONST seem
to be the wrong way round: (high (const ...)) is valid rtl that
could be passed in, but (const (high ...)) isn't.  As it stands,
we disallow anchor+offset but allow (high anchor+offset).

TBH I can't remember whether this caused a test failure or if it
was just something I noticed by inspection.  Either way, the SVE port
adds more invalid (const ...)s and it doesn't make sense to test for
them before peeling the HIGH.

Tested on aarch64-linux-gnu.  OK to install?

Richard


2018-01-04  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* config/aarch64/aarch64.c (aarch64_legitimate_constant_p): Fix
	order of HIGH and CONST checks.

Comments

James Greenhalgh Jan. 9, 2018, 12:18 p.m. | #1
On Thu, Jan 04, 2018 at 06:15:58PM +0000, Richard Sandiford wrote:
> The aarch64_legitimate_constant_p tests for HIGH and CONST seem

> to be the wrong way round: (high (const ...)) is valid rtl that

> could be passed in, but (const (high ...)) isn't.  As it stands,

> we disallow anchor+offset but allow (high anchor+offset).

> 

> TBH I can't remember whether this caused a test failure or if it

> was just something I noticed by inspection.  Either way, the SVE port

> adds more invalid (const ...)s and it doesn't make sense to test for

> them before peeling the HIGH.

> 

> Tested on aarch64-linux-gnu.  OK to install?


OK.

Thanks,
James

> 2018-01-04  Richard Sandiford  <richard.sandiford@linaro.org>

> 

> gcc/

> 	* config/aarch64/aarch64.c (aarch64_legitimate_constant_p): Fix

> 	order of HIGH and CONST checks.

> 

> Index: gcc/config/aarch64/aarch64.c

> ===================================================================

> --- gcc/config/aarch64/aarch64.c	2018-01-03 21:43:30.290452983 +0000

> +++ gcc/config/aarch64/aarch64.c	2018-01-04 18:11:30.299730142 +0000

> @@ -10449,6 +10449,9 @@ aarch64_legitimate_constant_p (machine_m

>    if (CONST_WIDE_INT_P (x))

>      return false;

>  

> +  if (GET_CODE (x) == HIGH)

> +    x = XEXP (x, 0);

> +

>    /* Do not allow const (plus (anchor_symbol, const_int)).  */

>    if (GET_CODE (x) == CONST)

>      {

> @@ -10460,9 +10463,6 @@ aarch64_legitimate_constant_p (machine_m

>  	return false;

>      }

>  

> -  if (GET_CODE (x) == HIGH)

> -    x = XEXP (x, 0);

> -

>    /* Treat symbols as constants.  Avoid TLS symbols as they are complex,

>       so spilling them is better than rematerialization.  */

>    if (SYMBOL_REF_P (x) && !SYMBOL_REF_TLS_MODEL (x))

Patch

Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c	2018-01-03 21:43:30.290452983 +0000
+++ gcc/config/aarch64/aarch64.c	2018-01-04 18:11:30.299730142 +0000
@@ -10449,6 +10449,9 @@  aarch64_legitimate_constant_p (machine_m
   if (CONST_WIDE_INT_P (x))
     return false;
 
+  if (GET_CODE (x) == HIGH)
+    x = XEXP (x, 0);
+
   /* Do not allow const (plus (anchor_symbol, const_int)).  */
   if (GET_CODE (x) == CONST)
     {
@@ -10460,9 +10463,6 @@  aarch64_legitimate_constant_p (machine_m
 	return false;
     }
 
-  if (GET_CODE (x) == HIGH)
-    x = XEXP (x, 0);
-
   /* Treat symbols as constants.  Avoid TLS symbols as they are complex,
      so spilling them is better than rematerialization.  */
   if (SYMBOL_REF_P (x) && !SYMBOL_REF_TLS_MODEL (x))