Re: [PATCH 4/6] [ARC] Rework delegitimate_address hook

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2017-11-02 13:30:33 +0100]:

> From: claziss <claziss@synopsys.com>
> 
> Delegitimize address is used to undo the obfuscating effect of PIC
> addresses, returning the address in a way which is understood by the
> compiler.
> 
> gcc/
> 2017-04-25  Claudiu Zissulescu  <claziss@synopsys.com>
> 
> 	* config/arc/arc.c (arc_delegitimize_address_0): Refactored to
> 	recognize new pic like addresses.
> 	(arc_delegitimize_address): Clean up.
> 
> testsuite/
> 2017-08-31  Claudiu Zissulescu  <claziss@synopsys.com>
> 
> 	* testsuite/gcc.target/arc/tdelegitimize_addr.c: New test.

Assuming this has passed all of the tests, then this change is fine
with me.

The commit message you propose above describes what delegitimize does
in general, but it doesn't really explain why _this_ change is
needed.  You remove a lot of code, it would be nice if the commit
message explained why we're able to drop all of this complexity.

Thanks,
Andrew



> ---
>  gcc/config/arc/arc.c                              | 91 ++++++++++-------------
>  gcc/testsuite/gcc.target/arc/tdelegitimize_addr.c | 23 ++++++
>  2 files changed, 62 insertions(+), 52 deletions(-)
>  create mode 100755 gcc/testsuite/gcc.target/arc/tdelegitimize_addr.c
> 
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index e7194a2..07dd072 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -9506,68 +9506,55 @@ arc_legitimize_address (rtx orig_x, rtx oldx, machine_mode mode)
>  }
>  
>  static rtx
> -arc_delegitimize_address_0 (rtx x)
> +arc_delegitimize_address_0 (rtx op)
>  {
> -  rtx u, gp, p;
> -
> -  if (GET_CODE (x) == CONST && GET_CODE (u = XEXP (x, 0)) == UNSPEC)
> +  switch (GET_CODE (op))
>      {
> -      if (XINT (u, 1) == ARC_UNSPEC_GOT
> -	  || XINT (u, 1) == ARC_UNSPEC_GOTOFFPC)
> -	return XVECEXP (u, 0, 0);
> +    case CONST:
> +      return arc_delegitimize_address_0 (XEXP (op, 0));
> +
> +    case UNSPEC:
> +      switch (XINT (op, 1))
> +	{
> +	case ARC_UNSPEC_GOT:
> +	case ARC_UNSPEC_GOTOFFPC:
> +	  return XVECEXP (op, 0, 0);
> +	default:
> +	  break;
> +	}
> +      break;
> +
> +    case PLUS:
> +      {
> +	rtx t1 = arc_delegitimize_address_0 (XEXP (op, 0));
> +	rtx t2 = XEXP (op, 1);
> +
> +	if (t1 && t2)
> +	  return gen_rtx_PLUS (GET_MODE (op), t1, t2);
> +	break;
> +      }
> +
> +    default:
> +      break;
>      }
> -  else if (GET_CODE (x) == CONST && GET_CODE (p = XEXP (x, 0)) == PLUS
> -	   && GET_CODE (u = XEXP (p, 0)) == UNSPEC
> -	   && (XINT (u, 1) == ARC_UNSPEC_GOT
> -	       || XINT (u, 1) == ARC_UNSPEC_GOTOFFPC))
> -    return gen_rtx_CONST
> -	    (GET_MODE (x),
> -	     gen_rtx_PLUS (GET_MODE (p), XVECEXP (u, 0, 0), XEXP (p, 1)));
> -  else if (GET_CODE (x) == PLUS
> -	   && ((REG_P (gp = XEXP (x, 0))
> -		&& REGNO (gp) == PIC_OFFSET_TABLE_REGNUM)
> -	       || (GET_CODE (gp) == CONST
> -		   && GET_CODE (u = XEXP (gp, 0)) == UNSPEC
> -		   && XINT (u, 1) == ARC_UNSPEC_GOT
> -		   && GET_CODE (XVECEXP (u, 0, 0)) == SYMBOL_REF
> -		   && !strcmp (XSTR (XVECEXP (u, 0, 0), 0), "_DYNAMIC")))
> -	   && GET_CODE (XEXP (x, 1)) == CONST
> -	   && GET_CODE (u = XEXP (XEXP (x, 1), 0)) == UNSPEC
> -	   && XINT (u, 1) == ARC_UNSPEC_GOTOFF)
> -    return XVECEXP (u, 0, 0);
> -  else if (GET_CODE (x) == PLUS && GET_CODE (XEXP (x, 0)) == PLUS
> -	   && ((REG_P (gp = XEXP (XEXP (x, 0), 1))
> -		&& REGNO (gp) == PIC_OFFSET_TABLE_REGNUM)
> -	       || (GET_CODE (gp) == CONST
> -		   && GET_CODE (u = XEXP (gp, 0)) == UNSPEC
> -		   && XINT (u, 1) == ARC_UNSPEC_GOT
> -		   && GET_CODE (XVECEXP (u, 0, 0)) == SYMBOL_REF
> -		   && !strcmp (XSTR (XVECEXP (u, 0, 0), 0), "_DYNAMIC")))
> -	   && GET_CODE (XEXP (x, 1)) == CONST
> -	   && GET_CODE (u = XEXP (XEXP (x, 1), 0)) == UNSPEC
> -	   && XINT (u, 1) == ARC_UNSPEC_GOTOFF)
> -    return gen_rtx_PLUS (GET_MODE (x), XEXP (XEXP (x, 0), 0),
> -			 XVECEXP (u, 0, 0));
> -  else if (GET_CODE (x) == PLUS
> -	   && (u = arc_delegitimize_address_0 (XEXP (x, 1))))
> -    return gen_rtx_PLUS (GET_MODE (x), XEXP (x, 0), u);
>    return NULL_RTX;
>  }
>  
>  static rtx
> -arc_delegitimize_address (rtx x)
> +arc_delegitimize_address (rtx orig_x)
>  {
> -  rtx orig_x = x = delegitimize_mem_from_attrs (x);
> -  if (GET_CODE (x) == MEM)
> +  rtx x = orig_x;
> +
> +  if (MEM_P (x))
>      x = XEXP (x, 0);
> +
>    x = arc_delegitimize_address_0 (x);
> -  if (x)
> -    {
> -      if (MEM_P (orig_x))
> -	x = replace_equiv_address_nv (orig_x, x);
> -      return x;
> -    }
> -  return orig_x;
> +  if (!x)
> +    return orig_x;
> +
> +  if (MEM_P (orig_x))
> +    x = replace_equiv_address_nv (orig_x, x);
> +  return x;
>  }
>  
>  /* Return a REG rtx for acc1.  N.B. the gcc-internal representation may
> diff --git a/gcc/testsuite/gcc.target/arc/tdelegitimize_addr.c b/gcc/testsuite/gcc.target/arc/tdelegitimize_addr.c
> new file mode 100755
> index 0000000..0d010ff
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/tdelegitimize_addr.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { ! { clmcpu } } } */
> +/* { dg-options "-mcpu=archs -g -O1 -fpic -mlra" } */
> +
> +/* Check if delegitimize address returns correctly the un-obfuscated
> +   address.  */
> +
> +typedef struct {
> +  long long tv_usec;
> +} t_a;
> +
> +static t_a a;
> +
> +int b;
> +extern void fn2 (t_a);
> +
> +void fn1 (void)
> +{
> + again:
> +  fn2(a);
> +  if (b)
> +    goto again;
> +}
> -- 
> 1.9.1
>