, Patch #1 replacement (fix issues with future TLS patches)

After I submitted the patches, Aaron Sawdey tested the branch that has
the patches on it, along with Alan's TLS patches.  Alan's patch causes
the functions that determine if the insn is prefixed or not to be run
earlier that before.  The compiler was dying because the virtual arg
pointer and frame pointer registers weren't eliminated at that point,
and because I was only checking if the regno was between 0 and 31 for
GPRs.

I rewrote the test in reg_to_insn_form to use INT_REGNO_P macro (which
includes tests for arg pointer and frame pointer virtual registers).  I
used the other two macros (FP_REGNO_P and ALTIVEC_REGNO_P) for
consistency.

In addition, I removed the gcc_unreachable call if the register class
is not a GPR, FPR, or VMX register, and used the GPR defaults.  This is
in case the function gets called in the middle of reload where the
final moves are not done.

This patch replaces patch #1.  I have bootstrapped the compiler with
these changes and verified it fixed the problem Aaron was seeing.  Can
I check this into the FSF trunk?

2019-08-15   Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/predicates.md (pcrel_address): Rewrite to use
	pcrel_addr_p.
	(pcrel_external_address): Rewrite to use pcrel_addr_p.
	(prefixed_mem_operand): Rewrite to use prefixed_local_addr_p.
	(pcrel_external_mem_operand): Rewrite to use pcrel_addr_p.
	* config/rs6000/rs6000-protos.h (reg_to_insn_form): New
	declaration.
	(pcrel_info_type): New declaration.
	(PCREL_NULL): New macro.
	(pcrel_addr_p): New declaration.
	(rs6000_prefixed_address_mode_p): Delete.
	* config/rs6000/rs6000.c (struct rs6000_reg_addr): Add fields for
	instruction format and prefixed memory support.
	(rs6000_debug_insn_form): New debug function.
	(rs6000_debug_print_mode): Print instruction formats.
	(setup_insn_form): New function.
	(rs6000_init_hard_regno_mode_ok): Call setup_insn_form.
	(print_operand_address): Call pcrel_addr_p instead of
	pcrel_address.  Add support for external pc-relative labels.
	(mode_supports_prefixed_address_p): Delete.
	(rs6000_prefixed_address_mode_p): Delete, replace with
	prefixed_local_addr_p.
	(prefixed_local_addr_p): Replace rs6000_prefixed_address_mode_p.
	Add argument to specify the instruction format.
	(pcrel_addr_p): New function.
	(reg_to_insn_form): New function.
	* config/rs6000/rs6000.md (enum insn_form): New enumeration.

Hi Mike,

On Thu, Aug 15, 2019 at 05:19:16PM -0400, Michael Meissner wrote:
> -;; Return true if the operand is a pc-relative address.
> +;; Return true if the operand is a pc-relative address to a local symbol.

The pcrel_addr_p comment says it is *not* just for local symbols.  So
which is it?

Having both something called "pcrel_address" and something called
"pcrel_addr_p" is confusing.  And "pcrel_addr_p" isn't a predicate at
all, so it should not be called that.  Or you can remove that "info"
argument, which is probably a good idea.

>  (define_predicate "pcrel_address"
>    (match_code "label_ref,symbol_ref,const")
>  {
> +  return pcrel_addr_p (op, true, false, PCREL_NULL);
>  })

Please avoid boolean arguments altogether; it isn't clear at all what
they mean here.

Ah, they say only locals are allowed here.  So this RTL predicate
shouldn't be called "pcrel_address"; it should have "local" in the name
somewhere.

>  ;; Return 1 if op is a prefixed memory operand.
>  (define_predicate "prefixed_mem_operand"
>    (match_code "mem")
>  {
> -  return rs6000_prefixed_address_mode_p (XEXP (op, 0), GET_MODE (op));
> +  return prefixed_local_addr_p (XEXP (op, 0), mode, INSN_FORM_UNKNOWN);
>  })

Similar issues with "local" here.

>  (define_predicate "pcrel_external_mem_operand"
>    (match_code "mem")
>  {
> -  return pcrel_external_address (XEXP (op, 0), Pmode);
> +  return pcrel_addr_p (XEXP (op, 0), false, true, PCREL_NULL);
>  })

Why this change?

> +/* Pc-relative address broken into component parts by pcrel_addr_p.  */
> +typedef struct {
> +  rtx base_addr;		/* SYMBOL_REF or LABEL_REF.  */
> +  HOST_WIDE_INT offset;		/* Offset from the base address.  */
> +  bool external_p;		/* Is the symbol external?  */
> +} pcrel_info_type;

Don't use typedefs please.

Don't call booleans xxx_p; xxx_p is a name used for a predicate, that
is, a pure (or "const" in GCC terms) function returning a boolean.

Don't name types "*_type".

> +#define PCREL_NULL ((pcrel_info_type *)0)

Please just use NULL where you use this.  (Or 0 as far as I care, but
that's not the GCC coding style :-) ).

> --- gcc/config/rs6000/rs6000.c	(revision 274172)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -369,8 +369,11 @@ struct rs6000_reg_addr {
>    enum insn_code reload_fpr_gpr;	/* INSN to move from FPR to GPR.  */
>    enum insn_code reload_gpr_vsx;	/* INSN to move from GPR to VSX.  */
>    enum insn_code reload_vsx_gpr;	/* INSN to move from VSX to GPR.  */
> +  enum insn_form default_insn_form;	/* Default format for offsets.  */
> +  enum insn_form insn_form[(int)N_RELOAD_REG]; /* Register insn format.  */
>    addr_mask_type addr_mask[(int)N_RELOAD_REG]; /* Valid address masks.  */

Why the casts here?  Not all places use this cast, so why is it needed
here and not in all cases?

> +/* Return a character that can be printed out to describe an instruction
> +   format.  */
> +
> +DEBUG_FUNCTION char
> +rs6000_debug_insn_form (enum insn_form iform)
> +{
> +  char ret;
> +
> +  switch (iform)
> +    {
> +    case INSN_FORM_UNKNOWN:  ret = '-'; break;
> +    case INSN_FORM_D:        ret = 'd'; break;
> +    case INSN_FORM_DS:       ret = 's'; break;
> +    case INSN_FORM_DQ:       ret = 'q'; break;
> +    case INSN_FORM_X:        ret = 'x'; break;
> +    case INSN_FORM_PREFIXED: ret = 'p'; break;
> +    default:                 ret = '?'; break;
> +    }
> +
> +  return ret;
> +}

This doesn't follow the coding style.

> +  fprintf (stderr, "  Format: %c:%c%c%c",
> +          rs6000_debug_insn_form (reg_addr[m].default_insn_form),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_GPR]),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_FPR]),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_VMX]));

Is this useful?  For others I mean, not just for you.

> +/* Set up the instruction format for each mode and register type from the
> +   addr_mask.  */
> +
> +static void
> +setup_insn_form (void)
> +{
> +  for (ssize_t m = 0; m < NUM_MACHINE_MODES; ++m)

Why ssize_t?  Most places just use int.

> +    {
> +      machine_mode scalar_mode = (machine_mode) m;
> +
> +      /* Convert complex and IBM double double/_Decimal128 into their scalar
> +	 parts that the registers will be split into for doing load or
> +	 store.  */
> +      if (COMPLEX_MODE_P (scalar_mode))
> +	scalar_mode = GET_MODE_INNER (scalar_mode);

Do you also need to handle some vector modes here?

> +      if (FLOAT128_2REG_P (scalar_mode))
> +	scalar_mode = DFmode;
> +
> +      for (ssize_t rc = FIRST_RELOAD_REG_CLASS; rc <= LAST_RELOAD_REG_CLASS; rc++)

(overwide line)

> +	{
> +	  machine_mode single_reg_mode = scalar_mode;
> +	  size_t msize = GET_MODE_SIZE (scalar_mode);
> +	  addr_mask_type addr_mask = reg_addr[scalar_mode].addr_mask[rc];
> +	  enum insn_form iform = INSN_FORM_UNKNOWN;
> +
> +	  /* Is the mode permitted in the GPR/FPR/Altivec registers?  */
> +	  if ((addr_mask & RELOAD_REG_VALID) != 0)
> +	    {
> +	      /* The addr_mask does not have the offsettable or indexed bits
> +		 set for modes that are split into multiple registers (like
> +		 IFmode).  It doesn't need this set, since typically by time it
> +		 is used in secondary reload, the modes are split into
> +		 component parts.
> +
> +		 The instruction format however can be used earlier in the
> +		 compilation, so we need to setup what kind of instruction can
> +		 be generated for the modes that are split.  */

If it's only "typically" split, is it true that the bits do not have to
be set?

Snip the rest of this function...  It needs much better factoring, and
better commenting, I cannot make heads or tails of it, sorry.

> +  /* Update the instruction formats.  */
> +  setup_insn_form ();

"Update"?  And s/format/form/ throughout.

>  void
>  print_operand_address (FILE *file, rtx x)
>  {
> +  pcrel_info_type pcrel_info;
> +
>    if (REG_P (x))
>      fprintf (file, "0(%s)", reg_names[ REGNO (x) ]);
>  
>    /* Is it a pc-relative address?  */
> -  else if (pcrel_address (x, Pmode))
> +  else if (pcrel_addr_p (x, true, true, &pcrel_info))

This is the only place that uses non-null for the info argument.  So
delete that from the normal function please, and handle that here, maybe
with some helper functions.

Don't do two (or more) different things in one function, in general.

> +/* Return true if the address ADDR is a prefixed address either with a large
> +   offset, an offset that does not fit in the normal instruction form, or a
> +   pc-relative address to a local symbol.

"Return true if ADDR is a local address that needs a prefix insn to
encode."?

You don't need to describe all exact reasons here...  That probably is
out-of-date (and/or not so exact) anyway.

>  
> +   MODE is the mode of the memory.
>  
> +   IFORM is used to determine if the traditional address is either DS format or
> +   DQ format and the bottom bits of the offset are non-zero.  */

s/traditional/non-prefixed/?  "Traditional" is a word like "old"; it is
stale almost before you write it.

>  bool
> +prefixed_local_addr_p (rtx addr, machine_mode mode, enum insn_form iform)

> +      if (!SIGNED_34BIT_OFFSET_P (offset))
> +	return false;

      /* Can this be described with a non-prefixed insn?  */
      if (!SIGNED_16BIT_OFFSET_P (offset))
	return true;

(and then the DS/DQ forms that need a prefix).

> +	  /* Use default if we don't know the precise instruction format.  */
> +	  if (iform == INSN_FORM_UNKNOWN)
> +	    iform = reg_addr[mode].default_insn_form;

So is this default as strict as possible?  As lenient as possible?  Or
what?

> +/* Given a register and a mode, return the instruction format for that
> +   register.  If the register is a pseudo register, use the default format.

What *is* the default form?  How is that decided?

Nowhere do you say if what something returns is as optimistic or as
pessimistic as possible.  Either option has something to say for it,
but it is important we know what it is :-)

> +enum insn_form
> +reg_to_insn_form (rtx reg, machine_mode mode)

What does this describe?  The insn form used for loading/storing that
register?

> +  /* If it isn't a register, use the defaults.  */
> +  if (!REG_P (reg) && !SUBREG_P (reg))
> +    iform = reg_addr[mode].default_insn_form;

If so...  How can that happen?

> +	  /* For anything else (SPR, CA, etc.) assume the GPR registers will be
> +	     used to load or store the value.  */

That's a big assumption.  gcc_unreachable instead?


Segher

Hi Mike, just a couple points from me...

On 8/15/19 4:19 PM, Michael Meissner wrote:

<snip>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 274172)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -369,8 +369,11 @@ struct rs6000_reg_addr {
>    enum insn_code reload_fpr_gpr;	/* INSN to move from FPR to GPR.  */
>    enum insn_code reload_gpr_vsx;	/* INSN to move from GPR to VSX.  */
>    enum insn_code reload_vsx_gpr;	/* INSN to move from VSX to GPR.  */
> +  enum insn_form default_insn_form;	/* Default format for offsets.  */
> +  enum insn_form insn_form[(int)N_RELOAD_REG]; /* Register insn format.  */
>    addr_mask_type addr_mask[(int)N_RELOAD_REG]; /* Valid address masks.  */
>    bool scalar_in_vmx_p;			/* Scalar value can go in VMX.  */
> +  bool prefixed_memory_p;		/* We can use prefixed memory.  */
>  };
>
>  static struct rs6000_reg_addr reg_addr[NUM_MACHINE_MODES];
> @@ -2053,6 +2056,28 @@ rs6000_debug_vector_unit (enum rs6000_ve
>    return ret;
>  }
>
> +/* Return a character that can be printed out to describe an instruction
> +   format.  */
> +
> +DEBUG_FUNCTION char
> +rs6000_debug_insn_form (enum insn_form iform)
> +{
> +  char ret;
> +
> +  switch (iform)
> +    {
> +    case INSN_FORM_UNKNOWN:  ret = '-'; break;
> +    case INSN_FORM_D:        ret = 'd'; break;
> +    case INSN_FORM_DS:       ret = 's'; break;
> +    case INSN_FORM_DQ:       ret = 'q'; break;
> +    case INSN_FORM_X:        ret = 'x'; break;
> +    case INSN_FORM_PREFIXED: ret = 'p'; break;
> +    default:                 ret = '?'; break;
> +    }
> +
> +  return ret;
> +}
> +
>  /* Inner function printing just the address mask for a particular reload
>     register class.  */
>  DEBUG_FUNCTION char *
> @@ -2115,6 +2140,12 @@ rs6000_debug_print_mode (ssize_t m)
>      fprintf (stderr, " %s: %s", reload_reg_map[rc].name,
>  	     rs6000_debug_addr_mask (reg_addr[m].addr_mask[rc], true));
>
> +  fprintf (stderr, "  Format: %c:%c%c%c",
> +          rs6000_debug_insn_form (reg_addr[m].default_insn_form),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_GPR]),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_FPR]),
> +          rs6000_debug_insn_form (reg_addr[m].insn_form[RELOAD_REG_VMX]));
> +
>    if ((reg_addr[m].reload_store != CODE_FOR_nothing)
>        || (reg_addr[m].reload_load != CODE_FOR_nothing))
>      {
> @@ -2668,6 +2699,153 @@ rs6000_setup_reg_addr_masks (void)
>      }
>  }
>
> +/* Set up the instruction format for each mode and register type from the
> +   addr_mask.  */
> +
> +static void
> +setup_insn_form (void)
> +{
> +  for (ssize_t m = 0; m < NUM_MACHINE_MODES; ++m)
> +    {
> +      machine_mode scalar_mode = (machine_mode) m;
> +
> +      /* Convert complex and IBM double double/_Decimal128 into their scalar
> +	 parts that the registers will be split into for doing load or
> +	 store.  */
> +      if (COMPLEX_MODE_P (scalar_mode))
> +	scalar_mode = GET_MODE_INNER (scalar_mode);
> +
> +      if (FLOAT128_2REG_P (scalar_mode))
> +	scalar_mode = DFmode;
> +
> +      for (ssize_t rc = FIRST_RELOAD_REG_CLASS; rc <= LAST_RELOAD_REG_CLASS; rc++)
> +	{
> +	  machine_mode single_reg_mode = scalar_mode;
> +	  size_t msize = GET_MODE_SIZE (scalar_mode);
> +	  addr_mask_type addr_mask = reg_addr[scalar_mode].addr_mask[rc];
> +	  enum insn_form iform = INSN_FORM_UNKNOWN;
> +
> +	  /* Is the mode permitted in the GPR/FPR/Altivec registers?  */
> +	  if ((addr_mask & RELOAD_REG_VALID) != 0)

To help with readability and maintainability, may I suggest factoring
the following into a separate function...
> +	    {
> +	      /* The addr_mask does not have the offsettable or indexed bits
> +		 set for modes that are split into multiple registers (like
> +		 IFmode).  It doesn't need this set, since typically by time it
> +		 is used in secondary reload, the modes are split into
> +		 component parts.
> +
> +		 The instruction format however can be used earlier in the
> +		 compilation, so we need to setup what kind of instruction can
> +		 be generated for the modes that are split.  */
> +	      if ((addr_mask & (RELOAD_REG_MULTIPLE
> +				| RELOAD_REG_OFFSET
> +				| RELOAD_REG_INDEXED)) == RELOAD_REG_MULTIPLE)
> +		{
> +		  /* Multiple register types in GPRs depend on whether we can
> +		     use DImode in a single register or SImode.  */
> +		  if (rc == RELOAD_REG_GPR)
> +		    {
> +		      if (TARGET_POWERPC64)
> +			{
> +			  gcc_assert ((msize % 8) == 0);
> +			  single_reg_mode = DImode;
> +			}
> +
> +		      else
> +			{
> +			  gcc_assert ((msize % 4) == 0);
> +			  single_reg_mode = SImode;
> +			}
> +		    }
> +
> +		  /* Multiple VSX vector sized data items will use a single
> +		     vector type as an instruction format.  */
> +		  else if (TARGET_VSX)
> +		    {
> +		      gcc_assert ((rc == RELOAD_REG_FPR)
> +				  || (rc == RELOAD_REG_VMX));
> +
> +		      if ((msize % 16) == 0)
> +			single_reg_mode = V2DImode;
> +		    }
> +
> +		  /* Multiple Altivec vector sized data items will use a single
> +		     vector type as an instruction format.  */
> +		  else if (TARGET_ALTIVEC && rc == RELOAD_REG_VMX
> +			   && (msize % 16) == 0
> +			   && VECTOR_MODE_P (single_reg_mode))
> +		    single_reg_mode = V4SImode;
> +
> +		  /* If we only have the traditional floating point unit, use
> +		     DFmode as the base type.  */
> +		  else if (!TARGET_VSX && TARGET_HARD_FLOAT
> +			   && rc == RELOAD_REG_FPR && (msize % 8) == 0)
> +		    single_reg_mode = DFmode;
> +
> +		  /* Get the information for the register mode used after
> +		     splitting.  */
> +		  addr_mask = reg_addr[single_reg_mode].addr_mask[rc];
> +		  msize = GET_MODE_SIZE (single_reg_mode);
> +		}
> +
> +	      /* Figure out the instruction format of each mode.
> +
> +		 For offsettable addresses that aren't specifically quad mode,
> +		 see if the default form is D or DS.  GPR 64-bit offsettable
> +		 addresses are DS format.  Likewise, all Altivec offsettable
> +		 adddresses are DS format.  */
> +	      if ((addr_mask & RELOAD_REG_OFFSET) != 0)
> +		{
> +		  if ((addr_mask & RELOAD_REG_QUAD_OFFSET) != 0)
> +		    iform = INSN_FORM_DQ;
> +
> +		  else if (rc == RELOAD_REG_VMX
> +			   || (rc == RELOAD_REG_GPR && TARGET_POWERPC64
> +			       && (msize >= 8)))
> +		    iform = INSN_FORM_DS;
> +
> +		  else
> +		    iform = INSN_FORM_D;
> +		}
> +
> +	      else if ((addr_mask & RELOAD_REG_INDEXED) != 0)
> +		iform = INSN_FORM_X;
> +	    }
> +
> +	  reg_addr[m].insn_form[rc] = iform;
> +	}

... until here.  Having all this in a doubly nested loop makes it
difficult to read.
> +
> +      /* Figure out the default insn format that is used for offsettable memory
> +	 instructions.  For scalar floating point use the FPR addressing, for
> +	 vectors and IEEE 128-bit use a suitable vector register type, and
> +	 otherwise use GPRs.  */
> +      ssize_t def_rc;
> +      if (TARGET_VSX
> +	  && (VECTOR_MODE_P (scalar_mode) || FLOAT128_IEEE_P (scalar_mode)))
> +	{
> +	  if ((reg_addr[m].addr_mask[RELOAD_REG_FPR] & RELOAD_REG_VALID) != 0)
> +	    def_rc = RELOAD_REG_FPR;
> +	  else
> +	    def_rc = RELOAD_REG_VMX;
> +	}
> +
> +      else if (TARGET_ALTIVEC && !TARGET_VSX && VECTOR_MODE_P (scalar_mode))
> +	def_rc = RELOAD_REG_VMX;
> +
> +      else if (TARGET_HARD_FLOAT && SCALAR_FLOAT_MODE_P (scalar_mode))
> +	def_rc = RELOAD_REG_FPR;
> +
> +      else
> +	def_rc = RELOAD_REG_GPR;
> +
> +      reg_addr[m].default_insn_form = reg_addr[m].insn_form[def_rc];
> +
> +      /* Don't enable prefixed memory support until all of the infrastructure
> +	 changes are in.  */
> +      reg_addr[m].prefixed_memory_p = false;
> +    }
> +}
> +
>  
>  /* Initialize the various global tables that are based on register size.  */
>  static void
> @@ -3181,6 +3359,9 @@ rs6000_init_hard_regno_mode_ok (bool glo
>       use.  */
>    rs6000_setup_reg_addr_masks ();
>
> +  /* Update the instruction formats.  */
> +  setup_insn_form ();
> +
>    if (global_init_p || TARGET_DEBUG_TARGET)
>      {
>        if (TARGET_DEBUG_REG)
> @@ -13070,29 +13251,21 @@ print_operand (FILE *file, rtx x, int co
>  void
>  print_operand_address (FILE *file, rtx x)
>  {
> +  pcrel_info_type pcrel_info;
> +
>    if (REG_P (x))
>      fprintf (file, "0(%s)", reg_names[ REGNO (x) ]);
>
>    /* Is it a pc-relative address?  */
> -  else if (pcrel_address (x, Pmode))
> +  else if (pcrel_addr_p (x, true, true, &pcrel_info))
>      {
> -      HOST_WIDE_INT offset;
> +      output_addr_const (file, pcrel_info.base_addr);
>
> -      if (GET_CODE (x) == CONST)
> -	x = XEXP (x, 0);
> +      if (pcrel_info.offset)
> +	fprintf (file, "%+" PRId64, pcrel_info.offset);
>
> -      if (GET_CODE (x) == PLUS)
> -	{
> -	  offset = INTVAL (XEXP (x, 1));
> -	  x = XEXP (x, 0);
> -	}
> -      else
> -	offset = 0;
> -
> -      output_addr_const (file, x);
> -
> -      if (offset)
> -	fprintf (file, "%+" PRId64, offset);
> +      if (pcrel_info.external_p)
> +	fputs ("@got", file);
>
>        fputs ("@pcrel", file);
>      }
> @@ -13579,70 +13752,206 @@ rs6000_pltseq_template (rtx *operands, i
>    return str;
>  }
>  #endif
> +
> +/* Return true if the address ADDR is a prefixed address either with a large
> +   offset, an offset that does not fit in the normal instruction form, or a
> +   pc-relative address to a local symbol.

I was confused as to the difference between the first two clauses in the
above comment.  I think that in the second perhaps you mean it doesn't
"fit" because the low-order 2 or 4 bits are nonzero for DS/DQ; is that
right?  If so, this comment could be clarified.  Not fitting sounds like
it requires more than 16 bits (possibly shifted) to describe.

Thanks,
Bill
>
> -/* Helper function to return whether a MODE can do prefixed loads/stores.
> -   VOIDmode is used when we are loading the pc-relative address into a base
> -   register, but we are not using it as part of a memory operation.  As modes
> -   add support for prefixed memory, they will be added here.  */
> -
> -static bool
> -mode_supports_prefixed_address_p (machine_mode mode)
> -{
> -  return mode == VOIDmode;
> -}
> +   MODE is the mode of the memory.
>
> -/* Function to return true if ADDR is a valid prefixed memory address that uses
> -   mode MODE.  */
> +   IFORM is used to determine if the traditional address is either DS format or
> +   DQ format and the bottom bits of the offset are non-zero.  */
>
>  bool
> -rs6000_prefixed_address_mode_p (rtx addr, machine_mode mode)
> +prefixed_local_addr_p (rtx addr, machine_mode mode, enum insn_form iform)
>  {
> -  if (!TARGET_PREFIXED_ADDR || !mode_supports_prefixed_address_p (mode))
> +  if (!reg_addr[mode].prefixed_memory_p)
>      return false;
>
> -  /* Check for PC-relative addresses.  */
> -  if (pcrel_address (addr, Pmode))
> -    return true;
> +  if (GET_CODE (addr) == CONST)
> +    addr = XEXP (addr, 0);
> +
> +  /* Single register, not prefixed.  */
> +  if (REG_P (addr) || SUBREG_P (addr))
> +    return false;
> +
> +  /* Register + offset.  */
> +  else if (GET_CODE (addr) == PLUS
> +	   && (REG_P (XEXP (addr, 0)) || SUBREG_P (XEXP (addr, 0)))
> +	   && CONST_INT_P (XEXP (addr, 1)))
> +    {
> +      HOST_WIDE_INT offset = INTVAL (XEXP (addr, 1));
> +
> +      /* Prefixed instructions can only access 34-bits.  Fail if the value
> +	 is larger than that.  */
> +      if (!SIGNED_34BIT_OFFSET_P (offset))
> +	return false;
> +
> +      /* For small offsets see whether it might be a DS or DQ instruction where
> +	 the bottom bits non-zero.  This would require using a prefixed
> +	 address.  If the offset is larger than 16 bits, then the instruction
> +	 must be prefixed.  */
> +      if (SIGNED_16BIT_OFFSET_P (offset))
> +	{
> +	  /* Use default if we don't know the precise instruction format.  */
> +	  if (iform == INSN_FORM_UNKNOWN)
> +	    iform = reg_addr[mode].default_insn_form;
> +
> +	  if (iform == INSN_FORM_DS)
> +	    return (offset & 3) != 0;
> +
> +	  else if (iform == INSN_FORM_DQ)
> +	    return (offset & 15) != 0;
> +
> +	  else if (iform != INSN_FORM_PREFIXED)
> +	    return false;
> +	}
> +
> +      return true;
> +    }
> +
> +  else if (!TARGET_PCREL)
> +    return false;
>
> -  /* Check for prefixed memory addresses that have a large numeric offset,
> -     or an offset that can't be used for a DS/DQ-form memory operation.  */
>    if (GET_CODE (addr) == PLUS)
>      {
> -      rtx op0 = XEXP (addr, 0);
>        rtx op1 = XEXP (addr, 1);
>
> -      if (!base_reg_operand (op0, Pmode) || !CONST_INT_P (op1))
> +      if (!CONST_INT_P (op1) || !SIGNED_34BIT_OFFSET_P (INTVAL (op1)))
>  	return false;
>
> -      HOST_WIDE_INT value = INTVAL (op1);
> -      if (!SIGNED_34BIT_OFFSET_P (value))
> +      addr = XEXP (addr, 0);
> +    }
> +
> +  /* Local pc-relative symbols/labels.  */
> +  return (LABEL_REF_P (addr)
> +	  || (SYMBOL_REF_P (addr) && SYMBOL_REF_LOCAL_P (addr)));
> +}
> +
> +/* Return true if the address ADDR is a prefixed address that is a pc-relative
> +   reference either to a local symbol or to an external symbol.  We break apart
> +   the address and return the parts.  LOCAL_SYMBOL_P and EXTERNAL_SYMBOL_P says
> +   whether local and external pc-relative symbols are allowed.  P_INFO points
> +   to a structure that returns the broken out component parts if desired.  */
> +
> +bool
> +pcrel_addr_p (rtx addr,
> +	      bool local_symbol_p,
> +	      bool external_symbol_p,
> +	      pcrel_info_type *p_info)
> +{
> +  rtx base_addr = NULL_RTX;
> +  HOST_WIDE_INT offset = 0;
> +  bool was_external_p = false;
> +
> +  if (p_info)
> +    {
> +      p_info->base_addr = NULL_RTX;
> +      p_info->offset = 0;
> +      p_info->external_p = false;
> +    }
> +
> +  if (!TARGET_PCREL)
> +    return false;
> +
> +  if (GET_CODE (addr) == CONST)
> +    addr = XEXP (addr, 0);
> +
> +  /* Pc-relative symbols/labels without offsets.  */
> +  if (SYMBOL_REF_P (addr))
> +    {
> +      base_addr = addr;
> +      was_external_p = !SYMBOL_REF_LOCAL_P (addr);
> +    }
> +
> +  else if (LABEL_REF_P (addr))
> +    base_addr = addr;
> +
> +  /* Pc-relative symbols with offsets.  */
> +  else if (GET_CODE (addr) == PLUS
> +	   && SYMBOL_REF_P (XEXP (addr, 0))
> +	   && CONST_INT_P (XEXP (addr, 1)))
> +    {
> +      base_addr = XEXP (addr, 0);
> +      offset = INTVAL (XEXP (addr, 1));
> +      was_external_p = !SYMBOL_REF_LOCAL_P (base_addr);
> +
> +      if (!SIGNED_34BIT_OFFSET_P (offset))
>  	return false;
> +    }
>
> -      /* Offset larger than 16-bits?  */
> -      if (!SIGNED_16BIT_OFFSET_P (value))
> -	return true;
> +  else
> +    return false;
> +
> +  if (was_external_p && !external_symbol_p)
> +    return false;
> +
> +  if (!was_external_p && !local_symbol_p)
> +    return false;
>
> -      /* DQ instruction (bottom 4 bits must be 0) for vectors.  */
> -      HOST_WIDE_INT mask;
> -      if (GET_MODE_SIZE (mode) >= 16)
> -	mask = 15;
> -
> -      /* DS instruction (bottom 2 bits must be 0).  For 32-bit integers, we
> -	 need to use DS instructions if we are sign-extending the value with
> -	 LWA.  For 32-bit floating point, we need DS instructions to load and
> -	 store values to the traditional Altivec registers.  */
> -      else if (GET_MODE_SIZE (mode) >= 4)
> -	mask = 3;
> +  if (p_info)
> +    {
> +      p_info->base_addr = base_addr;
> +      p_info->offset = offset;
> +      p_info->external_p = was_external_p;
> +    }
> +
> +  return true;
> +}
> +
> +/* Given a register and a mode, return the instruction format for that
> +   register.  If the register is a pseudo register, use the default format.
> +   Otherwise if it is hard register, look to see exactly what type of
> +   addressing is used.  */
> +
> +enum insn_form
> +reg_to_insn_form (rtx reg, machine_mode mode)
> +{
> +  enum insn_form iform;
>
> -      /* QImode/HImode has no restrictions.  */
> +  /* Handle UNSPECs, such as the special UNSPEC_SF_FROM_SI and
> +     UNSPEC_SI_FROM_SF UNSPECs, which are used to hide SF/SI interactions.
> +     Look at the first argument, and if it is a register, use that.  */
> +  if (GET_CODE (reg) == UNSPEC || GET_CODE (reg) == UNSPEC_VOLATILE)
> +    {
> +      rtx op0 = XVECEXP (reg, 0, 0);
> +      if (REG_P (op0) || SUBREG_P (op0))
> +	reg = op0;
> +    }
> +
> +  /* If it isn't a register, use the defaults.  */
> +  if (!REG_P (reg) && !SUBREG_P (reg))
> +    iform = reg_addr[mode].default_insn_form;
> +
> +  else
> +    {
> +      unsigned int r = reg_or_subregno (reg);
> +
> +      /* If we have a pseudo, use the default instruction format.  */
> +      if (r >= FIRST_PSEUDO_REGISTER)
> +	iform = reg_addr[mode].default_insn_form;
> +
> +      /* If we have a hard register, use the address format of that hard
> +	 register.  */
>        else
> -	return true;
> +	{
> +	  if (INT_REGNO_P (r))
> +	    iform = reg_addr[mode].insn_form[RELOAD_REG_GPR];
> +
> +	  else if (FP_REGNO_P (r))
> +	    iform = reg_addr[mode].insn_form[RELOAD_REG_FPR];
>
> -      /* Return true if we must use a prefixed instruction.  */
> -      return (value & mask) != 0;
> +	  else if (ALTIVEC_REGNO_P (r))
> +	    iform = reg_addr[mode].insn_form[RELOAD_REG_VMX];
> +
> +	  /* For anything else (SPR, CA, etc.) assume the GPR registers will be
> +	     used to load or store the value.  */
> +	  else
> +	    iform = reg_addr[mode].insn_form[RELOAD_REG_GPR];
> +	}
>      }
>
> -  return false;
> +  return iform;
>  }
>  
>  #if defined (HAVE_GAS_HIDDEN) && !TARGET_MACHO
> Index: gcc/config/rs6000/rs6000.md
> ===================================================================
> --- gcc/config/rs6000/rs6000.md	(revision 274172)
> +++ gcc/config/rs6000/rs6000.md	(working copy)
> @@ -252,6 +252,23 @@ (define_attr "var_shift" "no,yes"
>  ;; Is copying of this instruction disallowed?
>  (define_attr "cannot_copy" "no,yes" (const_string "no"))
>
> +;; Enumeration of the PowerPC instruction formats.  We only list the
> +;; instruction formats that are used by the code, and not every possible
> +;; instruction format that the machine supports.
> +
> +;; The main use for this enumeration is to determine if a particular
> +;; offsettable instruction has a valid offset field for a traditional
> +;; instruction, or whether a prefixed instruction might be needed to hold the
> +;; offset.  For DS/DQ format instructions, if we have an offset that has the
> +;; bottom bits non-zero, we can use a prefixed instruction instead of pushing
> +;; the offset to an index register.
> +(define_enum "insn_form" [unknown	; Unknown format
> +			  d		; Offset addressing uses 16 bits
> +			  ds		; Offset addressing uses 14 bits
> +			  dq		; Offset addressing uses 12 bits
> +			  x		; Indexed addressing
> +			  prefixed])	; Prefixed instruction
> +
>  ;; Length of the instruction (in bytes).
>  (define_attr "length" "" (const_int 4))
>
>