[7/7,ARC] Consolidate PIC implementation.

This patch refactors a number of functions and compiler hooks into using a
single function which checks if a rtx is suited for pic or not. Removed
functions are arc_legitimate_pc_offset_p and arc_legitimate_pic_operand_p
beeing replaced by calls to arc_legitimate_pic_addr_p. Thus we have an
unitary way of checking a rtx beeing pic.

gcc/
2017-02-24  Claudiu Zissulescu  <claziss@synopsys.com>

	* config/arc/arc-protos.h (arc_legitimate_pc_offset_p): Remove
	proto.
	(arc_legitimate_pic_operand_p): Likewise.
	* config/arc/arc.c (arc_legitimate_pic_operand_p): Remove
	function.
	(arc_needs_pcl_p): Likewise.
	(arc_legitimate_pc_offset_p): Likewise.
	(arc_legitimate_pic_addr_p): Remove LABEL_REF case, as this
	function is also used in constrains.md.
	(arc_legitimate_constant_p): Use arc_legitimate_pic_addr_p to
	validate pic constants. Handle CONST_INT, CONST_DOUBLE, MINUS and
	PLUS.  Only return true/false in known cases, otherwise assert.
	(arc_legitimate_address_p): Remove arc_legitimate_pic_addr_p as it
	is already called in arc_legitimate_constant_p.
	* config/arc/arc.h (CONSTANT_ADDRESS_P): Consider also LABEL for
	pic addresses.
	(LEGITIMATE_PIC_OPERAND_P): Use
	arc_raw_symbolic_reference_mentioned_p function.
	* config/arc/constraints.md (Cpc): Use arc_legitimate_pic_addr_p
	function.
	(Cal): Likewise.
	(C32): Likewise.

gcc/testsuite
2017-02-24  Claudiu Zissulescu  <claziss@synopsys.com>

	* gcc.target/arc/pr9000674901.c: New file.
	* gcc.target/arc/pic-1.c: Likewise.
	* gcc.target/arc/pr9001191897.c: Likewise.
---
 gcc/config/arc/arc-protos.h                 |   2 -
 gcc/config/arc/arc.c                        | 150 +++++++++-------------------
 gcc/config/arc/arc.h                        |  11 +-
 gcc/config/arc/constraints.md               |   6 +-
 gcc/testsuite/gcc.target/arc/pic-1.c        |  11 ++
 gcc/testsuite/gcc.target/arc/pr9000674901.c |  58 +++++++++++
 gcc/testsuite/gcc.target/arc/pr9001191897.c |  10 ++
 7 files changed, 136 insertions(+), 112 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arc/pic-1.c
 create mode 100644 gcc/testsuite/gcc.target/arc/pr9000674901.c
 create mode 100644 gcc/testsuite/gcc.target/arc/pr9001191897.c

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2017-06-01 15:34:57 +0200]:

> This patch refactors a number of functions and compiler hooks into using a
> single function which checks if a rtx is suited for pic or not. Removed
> functions are arc_legitimate_pc_offset_p and arc_legitimate_pic_operand_p
> beeing replaced by calls to arc_legitimate_pic_addr_p. Thus we have an
> unitary way of checking a rtx beeing pic.
> 
> gcc/
> 2017-02-24  Claudiu Zissulescu  <claziss@synopsys.com>
> 
> 	* config/arc/arc-protos.h (arc_legitimate_pc_offset_p): Remove
> 	proto.
> 	(arc_legitimate_pic_operand_p): Likewise.
> 	* config/arc/arc.c (arc_legitimate_pic_operand_p): Remove
> 	function.
> 	(arc_needs_pcl_p): Likewise.
> 	(arc_legitimate_pc_offset_p): Likewise.
> 	(arc_legitimate_pic_addr_p): Remove LABEL_REF case, as this
> 	function is also used in constrains.md.
> 	(arc_legitimate_constant_p): Use arc_legitimate_pic_addr_p to
> 	validate pic constants. Handle CONST_INT, CONST_DOUBLE, MINUS and
> 	PLUS.  Only return true/false in known cases, otherwise assert.
> 	(arc_legitimate_address_p): Remove arc_legitimate_pic_addr_p as it
> 	is already called in arc_legitimate_constant_p.
> 	* config/arc/arc.h (CONSTANT_ADDRESS_P): Consider also LABEL for
> 	pic addresses.
> 	(LEGITIMATE_PIC_OPERAND_P): Use
> 	arc_raw_symbolic_reference_mentioned_p function.
> 	* config/arc/constraints.md (Cpc): Use arc_legitimate_pic_addr_p
> 	function.
> 	(Cal): Likewise.
> 	(C32): Likewise.
> 
> gcc/testsuite
> 2017-02-24  Claudiu Zissulescu  <claziss@synopsys.com>
> 
> 	* gcc.target/arc/pr9000674901.c: New file.
> 	* gcc.target/arc/pic-1.c: Likewise.
> 	* gcc.target/arc/pr9001191897.c: Likewise.


Looks like a good clean up.

Thanks,
Andrew


> ---
>  gcc/config/arc/arc-protos.h                 |   2 -
>  gcc/config/arc/arc.c                        | 150 +++++++++-------------------
>  gcc/config/arc/arc.h                        |  11 +-
>  gcc/config/arc/constraints.md               |   6 +-
>  gcc/testsuite/gcc.target/arc/pic-1.c        |  11 ++
>  gcc/testsuite/gcc.target/arc/pr9000674901.c |  58 +++++++++++
>  gcc/testsuite/gcc.target/arc/pr9001191897.c |  10 ++
>  7 files changed, 136 insertions(+), 112 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arc/pic-1.c
>  create mode 100644 gcc/testsuite/gcc.target/arc/pr9000674901.c
>  create mode 100644 gcc/testsuite/gcc.target/arc/pr9001191897.c
> 
> diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
> index b436dbe..850795a 100644
> --- a/gcc/config/arc/arc-protos.h
> +++ b/gcc/config/arc/arc-protos.h
> @@ -60,10 +60,8 @@ extern rtx arc_return_addr_rtx (int , rtx);
>  extern bool check_if_valid_regno_const (rtx *, int);
>  extern bool check_if_valid_sleep_operand (rtx *, int);
>  extern bool arc_legitimate_constant_p (machine_mode, rtx);
> -extern bool arc_legitimate_pc_offset_p (rtx);
>  extern bool arc_legitimate_pic_addr_p (rtx);
>  extern bool arc_raw_symbolic_reference_mentioned_p (rtx, bool);
> -extern bool arc_legitimate_pic_operand_p (rtx);
>  extern bool arc_is_longcall_p (rtx);
>  extern bool arc_is_shortcall_p (rtx);
>  extern bool valid_brcc_with_delay_p (rtx *);
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 7dfc68e..89de6cd 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -249,7 +249,6 @@ static rtx arc_expand_builtin (tree, rtx, rtx, machine_mode, int);
>  static int branch_dest (rtx);
>  
>  static void  arc_output_pic_addr_const (FILE *,  rtx, int);
> -bool arc_legitimate_pic_operand_p (rtx);
>  static bool arc_function_ok_for_sibcall (tree, tree);
>  static rtx arc_function_value (const_tree, const_tree, bool);
>  const char * output_shift (rtx *);
> @@ -5152,57 +5151,6 @@ arc_rtx_costs (rtx x, machine_mode mode, int outer_code,
>      }
>  }
>  
> -/* Helper used by arc_legitimate_pc_offset_p.  */
> -
> -static bool
> -arc_needs_pcl_p (rtx x)
> -{
> -  register const char *fmt;
> -  register int i, j;
> -
> -  if ((GET_CODE (x) == UNSPEC)
> -      && (XVECLEN (x, 0) == 1)
> -      && (GET_CODE (XVECEXP (x, 0, 0)) == SYMBOL_REF))
> -    switch (XINT (x, 1))
> -      {
> -      case ARC_UNSPEC_GOT:
> -      case ARC_UNSPEC_GOTOFFPC:
> -      case UNSPEC_TLS_GD:
> -      case UNSPEC_TLS_IE:
> -	return true;
> -      default:
> -	break;
> -      }
> -
> -  fmt = GET_RTX_FORMAT (GET_CODE (x));
> -  for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; i--)
> -    {
> -      if (fmt[i] == 'e')
> -	{
> -	  if (arc_needs_pcl_p (XEXP (x, i)))
> -	    return true;
> -	}
> -      else if (fmt[i] == 'E')
> -	for (j = XVECLEN (x, i) - 1; j >= 0; j--)
> -	  if (arc_needs_pcl_p (XVECEXP (x, i, j)))
> -	    return true;
> -    }
> -
> -  return false;
> -}
> -
> -/* Return true if ADDR is an address that needs to be expressed as an
> -   explicit sum of pcl + offset.  */
> -
> -bool
> -arc_legitimate_pc_offset_p (rtx addr)
> -{
> -  if (GET_CODE (addr) != CONST)
> -    return false;
> -
> -  return arc_needs_pcl_p (addr);
> -}
> -
>  /* Return true if ADDR is a valid pic address.
>     A valid pic address on arc should look like
>     const (unspec (SYMBOL_REF/LABEL) (ARC_UNSPEC_GOTOFF/ARC_UNSPEC_GOT))  */
> @@ -5210,8 +5158,6 @@ arc_legitimate_pc_offset_p (rtx addr)
>  bool
>  arc_legitimate_pic_addr_p (rtx addr)
>  {
> -  if (GET_CODE (addr) == LABEL_REF)
> -    return true;
>    if (GET_CODE (addr) != CONST)
>      return false;
>  
> @@ -5915,16 +5861,6 @@ arc_return_addr_rtx (int count, ATTRIBUTE_UNUSED rtx frame)
>    return get_hard_reg_initial_val (Pmode , RETURN_ADDR_REGNUM);
>  }
>  
> -/* Nonzero if the constant value X is a legitimate general operand
> -   when generating PIC code.  It is given that flag_pic is on and
> -   that X satisfies CONSTANT_P or is a CONST_DOUBLE.  */
> -
> -bool
> -arc_legitimate_pic_operand_p (rtx x)
> -{
> -  return !arc_raw_symbolic_reference_mentioned_p (x, true);
> -}
> -
>  /* Determine if a given RTX is a valid constant.  We already know this
>     satisfies CONSTANT_P.  */
>  
> @@ -5940,40 +5876,12 @@ arc_legitimate_constant_p (machine_mode mode, rtx x)
>    switch (GET_CODE (x))
>      {
>      case CONST:
> -      x = XEXP (x, 0);
> -
> -      if (GET_CODE (x) == PLUS)
> +      if (flag_pic)
>  	{
> -	  if (flag_pic
> -	      ? GET_CODE (XEXP (x, 1)) != CONST_INT
> -	      : !arc_legitimate_constant_p (mode, XEXP (x, 1)))
> -	    return false;
> -	  x = XEXP (x, 0);
> -	}
> -
> -      /* Only some unspecs are valid as "constants".  */
> -      if (GET_CODE (x) == UNSPEC)
> -	switch (XINT (x, 1))
> -	  {
> -	  case ARC_UNSPEC_PLT:
> -	  case ARC_UNSPEC_GOTOFF:
> -	  case ARC_UNSPEC_GOTOFFPC:
> -	  case ARC_UNSPEC_GOT:
> -	  case UNSPEC_TLS_GD:
> -	  case UNSPEC_TLS_IE:
> -	  case UNSPEC_TLS_OFF:
> +	  if (arc_legitimate_pic_addr_p (x))
>  	    return true;
> -
> -	  default:
> -	    gcc_unreachable ();
> -	  }
> -
> -      /* We must have drilled down to a symbol.  */
> -      if (arc_raw_symbolic_reference_mentioned_p (x, false))
> -	return false;
> -
> -      /* Return true.  */
> -      break;
> +	}
> +      return arc_legitimate_constant_p (mode, XEXP (x, 0));
>  
>      case SYMBOL_REF:
>        if (SYMBOL_REF_TLS_MODEL (x))
> @@ -5983,13 +5891,53 @@ arc_legitimate_constant_p (machine_mode mode, rtx x)
>        if (flag_pic)
>  	return false;
>        /* Fall through.  */
> +    case CONST_INT:
> +    case CONST_DOUBLE:
> +      return true;
> +
> +    case NEG:
> +      return arc_legitimate_constant_p (mode, XEXP (x, 0));
> +
> +    case PLUS:
> +    case MINUS:
> +      {
> +	bool t1 = arc_legitimate_constant_p (mode, XEXP (x, 0));
> +	bool t2 = arc_legitimate_constant_p (mode, XEXP (x, 1));
> +
> +	return (t1 && t2);
> +      }
> +
> +    case CONST_VECTOR:
> +      switch (mode)
> +	{
> +	case V2HImode:
> +	  return TARGET_PLUS_DMPY;
> +	case V2SImode:
> +	case V4HImode:
> +	  return TARGET_PLUS_QMACW;
> +	default:
> +	  return false;
> +	}
> +
> +    case UNSPEC:
> +      switch (XINT (x, 1))
> +	{
> +	case UNSPEC_TLS_GD:
> +	case UNSPEC_TLS_OFF:
> +	case UNSPEC_TLS_IE:
> +	  return true;
> +	default:
> +	  /* Any other unspec ending here are pic related, hence the above
> +	     constant pic address checking returned false.  */
> +	  return false;
> +	}
> +      /* Fall through.  */
>  
>      default:
> -      break;
> +      fatal_insn ("unrecognized supposed constant", x);
>      }
>  
> -  /* Otherwise we handle everything else in the move patterns.  */
> -  return true;
> +  gcc_unreachable ();
>  }
>  
>  static bool
> @@ -6026,9 +5974,7 @@ arc_legitimate_address_p (machine_mode mode, rtx x, bool strict)
>  
>    if ((GET_MODE_SIZE (mode) != 16) && CONSTANT_P (x))
>      {
> -      if (flag_pic ? arc_legitimate_pic_addr_p (x)
> -	  : arc_legitimate_constant_p (Pmode, x))
> -	return true;
> +      return arc_legitimate_constant_p (mode, x);
>      }
>    if ((GET_CODE (x) == PRE_DEC || GET_CODE (x) == PRE_INC
>         || GET_CODE (x) == POST_DEC || GET_CODE (x) == POST_INC)
> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> index 585e98c..c7cd798 100644
> --- a/gcc/config/arc/arc.h
> +++ b/gcc/config/arc/arc.h
> @@ -901,10 +901,10 @@ extern int arc_initial_elimination_offset(int from, int to);
>     a special predicate for the memory operand of stores, like for the SH.  */
>  
>  /* Recognize any constant value that is a valid address.  */
> -#define CONSTANT_ADDRESS_P(X) \
> -(flag_pic?arc_legitimate_pic_addr_p (X): \
> -(GET_CODE (X) == LABEL_REF || GET_CODE (X) == SYMBOL_REF	\
> - || GET_CODE (X) == CONST_INT || GET_CODE (X) == CONST))
> +#define CONSTANT_ADDRESS_P(X)					\
> +  (flag_pic ? (arc_legitimate_pic_addr_p (X) || LABEL_P (X)):	\
> +   (GET_CODE (X) == LABEL_REF || GET_CODE (X) == SYMBOL_REF	\
> +    || GET_CODE (X) == CONST_INT || GET_CODE (X) == CONST))
>  
>  /* Is the argument a const_int rtx, containing an exact power of 2 */
>  #define  IS_POWEROF2_P(X) (! ( (X) & ((X) - 1)) && (X))
> @@ -1083,7 +1083,8 @@ arc_select_cc_mode (OP, X, Y)
>     check it either.  You need not define this macro if all constants
>     (including SYMBOL_REF) can be immediate operands when generating
>     position independent code.  */
> -#define LEGITIMATE_PIC_OPERAND_P(X)  (arc_legitimate_pic_operand_p(X))
> +#define LEGITIMATE_PIC_OPERAND_P(X)  \
> +  (!arc_raw_symbolic_reference_mentioned_p ((X), true))
>  
>  /* PIC and small data don't mix on ARC because they use the same register.  */
>  #define SDATA_BASE_REGNUM 26
> diff --git a/gcc/config/arc/constraints.md b/gcc/config/arc/constraints.md
> index edab41c..6620daf 100644
> --- a/gcc/config/arc/constraints.md
> +++ b/gcc/config/arc/constraints.md
> @@ -403,7 +403,7 @@
>  
>  (define_constraint "Cpc"
>    "pc-relative constant"
> -  (match_test "arc_legitimate_pc_offset_p (op)"))
> +  (match_test "arc_legitimate_pic_addr_p (op)"))
>  
>  (define_constraint "Clb"
>    "label"
> @@ -412,12 +412,12 @@
>  
>  (define_constraint "Cal"
>    "constant for arithmetic/logical operations"
> -  (match_test "immediate_operand (op, VOIDmode) && !arc_legitimate_pc_offset_p (op)"))
> +  (match_test "immediate_operand (op, VOIDmode) && !arc_legitimate_pic_addr_p (op)"))
>  
>  (define_constraint "C32"
>    "32 bit constant for arithmetic/logical operations"
>    (match_test "immediate_operand (op, VOIDmode)
> -	       && !arc_legitimate_pc_offset_p (op)
> +	       && !arc_legitimate_pic_addr_p (op)
>  	       && !satisfies_constraint_I (op)"))
>  
>  ; Note that the 'cryptic' register constraints will not make reload use the
> diff --git a/gcc/testsuite/gcc.target/arc/pic-1.c b/gcc/testsuite/gcc.target/arc/pic-1.c
> new file mode 100644
> index 0000000..ab24763
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/pic-1.c
> @@ -0,0 +1,11 @@
> +/* Tests how complex pic constant expressions are handled.  */
> +/* { dg-do assemble } */
> +/* { dg-skip-if "PIC not available for ARC6xx" { arc6xx } } */
> +/* { dg-options "-mno-sdata -w -Os -fpic" } */
> +
> +a() {
> +  char *b = "";
> +  char c;
> +  int d = &c - " \n\t\v\b\r\f\a/\0";
> +  e(b[d]);
> +}
> diff --git a/gcc/testsuite/gcc.target/arc/pr9000674901.c b/gcc/testsuite/gcc.target/arc/pr9000674901.c
> new file mode 100644
> index 0000000..2a15c1c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/pr9000674901.c
> @@ -0,0 +1,58 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { ! { clmcpu } } } */
> +/* { dg-options "-mcpu=arc700 -O2 -fpic" } */
> +
> +/* Test if the compiler generates a constant address having that uses
> +   a neg keyword on the pic unspec.  */
> +
> +typedef unsigned int uint32_t;
> +typedef unsigned char uint8_t;
> +typedef unsigned short int uint16_t;
> +typedef unsigned long long int uint64_t;
> +
> +enum type {
> + t_undef = 0x01,
> + t_group = 0x02,
> + t_partition = 0x04,
> + t_spare = 0x08,
> + t_linear = 0x10,
> + t_raid0 = 0x20,
> + t_raid1 = 0x40,
> + t_raid4 = 0x80,
> + t_raid5_ls = 0x100,
> + t_raid5_rs = 0x200,
> + t_raid5_la = 0x400,
> + t_raid5_ra = 0x800,
> + t_raid6 = 0x1000,
> +};
> +
> +struct raid_set {
> +  enum type type;
> +};
> +
> +void
> +_find_factors (struct raid_set *rs, uint8_t * div, uint8_t * sub)
> +{
> +  struct factors {
> +    const uint8_t level;
> +    const uint8_t div, sub;
> +  };
> +  static struct factors factors[] = {
> +    {0, 1, 0},
> +    {1, 2, 0},
> +    {2, 2, 0},
> +    {5, 1, 1},
> +  };
> +  struct factors *f = (factors + (sizeof (factors) / sizeof (*factors)));
> +
> +  while (f-- > factors) {
> +    if (rs->type == f->level) {
> +      *div = f->div;
> +      *sub = f->sub;
> +      return;
> +    }
> +  }
> +
> +  *div = 1;
> +  *sub = 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arc/pr9001191897.c b/gcc/testsuite/gcc.target/arc/pr9001191897.c
> new file mode 100644
> index 0000000..fc36426
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/pr9001191897.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { ! { clmcpu } } } */
> +/* { dg-options "-mcpu=archs -Os -fpic -mno-sdata -mno-indexed-loads -w" } */
> +a;
> +c() {
> +  static char b[25];
> +  for (; a >= 0; a--)
> +    if (b[a])
> +      b[a] = '\0';
> +}
> -- 
> 1.9.1
>

> Looks like a good clean up.
> 
> Thanks,
> Andrew

Committed, thank you for your review,
Claudiu