diff mbox

[AArch64] Mark symbols as constant

Message ID VI1PR0802MB2621C37760491F332EF9897283C40@VI1PR0802MB2621.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Wilco Dijkstra June 19, 2017, 6:59 p.m. UTC 
Aarch64_legitimate_constant_p currently returns false for symbols,
eventhough they are always valid constants.  This means LOSYM isn't
CSEd correctly.  If we return true CSE works better, resulting in
smaller/faster code (0.3% smaller code on SPEC2006).

int x0 = 1, x1 = 2, x2 = 3;

int 
f (int x, int y)
{
  x += x1;
  if (x > 100)
    y += x2;
  x += x0;
  return x + y;
}

Before:
	adrp	x3, .LANCHOR0
	add	x4, x3, :lo12:.LANCHOR0
	ldr	w2, [x3, #:lo12:.LANCHOR0]
	add	w0, w0, w2
	cmp	w0, 100
	ble	.L5
	ldr	w2, [x4, 8]
	add	w1, w1, w2
.L5:
	add	x3, x3, :lo12:.LANCHOR0
	ldr	w2, [x3, 4]
	add	w0, w0, w2
	add	w0, w0, w1
	ret

After:
	adrp	x2, .LANCHOR0
	add	x3, x2, :lo12:.LANCHOR0
	ldr	w2, [x2, #:lo12:.LANCHOR0]
	add	w0, w0, w2
	cmp	w0, 100
	ble	.L5
	ldr	w2, [x3, 8]
	add	w1, w1, w2
.L5:
	ldr	w2, [x3, 4]
	add	w0, w0, w2
	add	w0, w0, w1
	ret

Passes regress and bootstrap, OK for commit?

ChangeLog:
2017-06-19  Wilco Dijkstra  <wdijkstr@arm.com>

	* config/aarch64/aarch64.c (aarch64_legitimate_constant_p):
	Return true for symbols.
--

Comments

Richard Earnshaw June 19, 2017, 10:13 p.m. UTC | #1 
On 19/06/17 19:59, Wilco Dijkstra wrote:
> Aarch64_legitimate_constant_p currently returns false for symbols,
> eventhough they are always valid constants.  This means LOSYM isn't
> CSEd correctly.  If we return true CSE works better, resulting in
> smaller/faster code (0.3% smaller code on SPEC2006).
> 
> int x0 = 1, x1 = 2, x2 = 3;
> 
> int 
> f (int x, int y)
> {
>   x += x1;
>   if (x > 100)
>     y += x2;
>   x += x0;
>   return x + y;
> }
> 
> Before:
> 	adrp	x3, .LANCHOR0
> 	add	x4, x3, :lo12:.LANCHOR0
> 	ldr	w2, [x3, #:lo12:.LANCHOR0]
> 	add	w0, w0, w2
> 	cmp	w0, 100
> 	ble	.L5
> 	ldr	w2, [x4, 8]
> 	add	w1, w1, w2
> .L5:
> 	add	x3, x3, :lo12:.LANCHOR0
> 	ldr	w2, [x3, 4]
> 	add	w0, w0, w2
> 	add	w0, w0, w1
> 	ret
> 
> After:
> 	adrp	x2, .LANCHOR0
> 	add	x3, x2, :lo12:.LANCHOR0
> 	ldr	w2, [x2, #:lo12:.LANCHOR0]
> 	add	w0, w0, w2
> 	cmp	w0, 100
> 	ble	.L5
> 	ldr	w2, [x3, 8]
> 	add	w1, w1, w2
> .L5:
> 	ldr	w2, [x3, 4]
> 	add	w0, w0, w2
> 	add	w0, w0, w1
> 	ret
> 
> Passes regress and bootstrap, OK for commit?
> 
> ChangeLog:
> 2017-06-19  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	* config/aarch64/aarch64.c (aarch64_legitimate_constant_p):
> 	Return true for symbols.
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 5ec6bbfcf484baa4005b8a88cb98d0d04f710877..4b7d961102e41ce927d89d458fc89ddfc2adcd6f 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -10111,6 +10111,9 @@ aarch64_legitimate_constant_p (machine_mode mode, rtx x)
>        && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0))))
>      return true;
>  
> +  if (SYMBOL_REF_P (x))
> +    return true;
> +
>    return aarch64_constant_address_p (x);
>  }
>  
> 

What testing has this had with -fpic?  I'm not convinced that this
assertion is true in that case?

R.
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5ec6bbfcf484baa4005b8a88cb98d0d04f710877..4b7d961102e41ce927d89d458fc89ddfc2adcd6f 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10111,6 +10111,9 @@  aarch64_legitimate_constant_p (machine_mode mode, rtx x)
       && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0))))
     return true;
 
+  if (SYMBOL_REF_P (x))
+    return true;
+
   return aarch64_constant_address_p (x);
 }