rs6000: Add basic support for prefixed and PC-relative addresses

Message ID bed0c89d-3d0c-5500-d6e2-1db4de6685ec@linux.ibm.com
State New
Headers show
Series
  • rs6000: Add basic support for prefixed and PC-relative addresses
Related show

Commit Message

Bill Schmidt May 30, 2019, 3:49 a.m.
Hi,

This patch adds basic infrastructure for prefixed and PC-relative addresses,
including new predicates and functions to detect when they apply.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
Is this okay for trunk?

Thanks,
Bill


2019-05-29  Bill Schmidt  <wschmidt@linux.ibm.com>
	    Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/predicates.md (pcrel_address): New
	define_predicate.
	(prefixed_mem_operand): Likewise.
	(non_prefixed_mem_operand): Likewise.
	* config/rs6000/rs6000-protos.h (rs6000_prefixed_address): New
	prototype.
	* config/rs6000/rs6000.c (print_operand_address): Handle
	PC-relative addresses.
	(mode_supports_prefixed_address_p): New function.
	(rs6000_prefixed_address): New function.
	* config/rs6000/rs6000.h (SYMBOL_FLAG_PCREL): New #define.
	(SYMBOL_REF_PCREL_P): Likewise.

Comments

Segher Boessenkool May 30, 2019, 7:20 p.m. | #1
On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
> 	* config/rs6000/predicates.md (pcrel_address): New

> 	define_predicate.


Please put that on one line?

> +      if (LABEL_REF_P (x))

> +	output_asm_label (x);

> +      else

> +	output_addr_const (file, x);


Why does LABEL_REF need separate handling here?

> +      if (offset)

> +	fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",

> +		 offset);


Maybe

      if (offset)
	fprintf (file, "%+" PRId64, offset);

(but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for
that then.  Oh well).

> +rs6000_prefixed_address (rtx addr, machine_mode mode)


> +  if (GET_CODE (addr) == PLUS)

> +    {

> +      HOST_WIDE_INT value, mask;


Please declare these where they are defined?

> +      /* 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;


I don't much like penalising scalar single precision float like this.
But, this also hurts unaligned lwz...  Do we have statistics on that?
Offline, I guess :-)

> +      /* Return true if we must use a prefixed instruction.  */

> +      return ((value & ~mask) != value);


(value & mask) != 0


Okay with those things frobbed a little, or looked at.  Thanks!


Segher


Segher
Bill Schmidt May 30, 2019, 7:43 p.m. | #2
On 5/30/19 2:20 PM, Segher Boessenkool wrote:
> On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:

>> 	* config/rs6000/predicates.md (pcrel_address): New

>> 	define_predicate.

> Please put that on one line?


OK.  Emacs in ChangeLog and Fill modes seems to set a line length
somewhat less than 79.  I generally follow what it tells me, but I can
fix this.
>

>> +      if (LABEL_REF_P (x))

>> +	output_asm_label (x);

>> +      else

>> +	output_addr_const (file, x);

> Why does LABEL_REF need separate handling here?


Mike, please respond?
>

>> +      if (offset)

>> +	fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",

>> +		 offset);

> Maybe

>

>       if (offset)

> 	fprintf (file, "%+" PRId64, offset);

>

> (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for

> that then.  Oh well).


Will have a look.
>

>> +rs6000_prefixed_address (rtx addr, machine_mode mode)

>> +  if (GET_CODE (addr) == PLUS)

>> +    {

>> +      HOST_WIDE_INT value, mask;

> Please declare these where they are defined?


ok.
>

>> +      /* 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;

> I don't much like penalising scalar single precision float like this.

> But, this also hurts unaligned lwz...  Do we have statistics on that?

> Offline, I guess :-)


Again, I'll defer to Mike on those details (online or offline). ;-)
>

>> +      /* Return true if we must use a prefixed instruction.  */

>> +      return ((value & ~mask) != value);

> (value & mask) != 0


OK.
>

>

> Okay with those things frobbed a little, or looked at.  Thanks!


Thanks for the review on your day off!
Bill
>

>

> Segher

>

>

> Segher
Segher Boessenkool May 30, 2019, 8:35 p.m. | #3
On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote:
> On 5/30/19 2:20 PM, Segher Boessenkool wrote:

> > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:

> >> 	* config/rs6000/predicates.md (pcrel_address): New

> >> 	define_predicate.

> > Please put that on one line?

> 

> OK.  Emacs in ChangeLog and Fill modes seems to set a line length

> somewhat less than 79.  I generally follow what it tells me, but I can

> fix this.


This would be just 76 ;-)

> >> +      if (offset)

> >> +	fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",

> >> +		 offset);

> > Maybe

> >

> >       if (offset)

> > 	fprintf (file, "%+" PRId64, offset);

> >

> > (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for

> > that then.  Oh well).

> 

> Will have a look.


Very many things in GCC do this, but it's so inelegant :-)  It's fine to
commit it like this, as I said.  But it annoys me every time I see it :-)


Segher
Michael Meissner May 30, 2019, 9:54 p.m. | #4
On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote:
> On 5/30/19 2:20 PM, Segher Boessenkool wrote:

> > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:

> >> 	* config/rs6000/predicates.md (pcrel_address): New

> >> 	define_predicate.

> > Please put that on one line?

> 

> OK.  Emacs in ChangeLog and Fill modes seems to set a line length

> somewhat less than 79.  I generally follow what it tells me, but I can

> fix this.

> >

> >> +      if (LABEL_REF_P (x))

> >> +	output_asm_label (x);

> >> +      else

> >> +	output_addr_const (file, x);

> > Why does LABEL_REF need separate handling here?

> 

> Mike, please respond?


The current code doesn't need special handling.  I don't recall if earlier code
did, or I just assumed output_addr_const did not handle LABEL_REF's.

> > I don't much like penalising scalar single precision float like this.

> > But, this also hurts unaligned lwz...  Do we have statistics on that?

> > Offline, I guess :-)

> 

> Again, I'll defer to Mike on those details (online or offline). ;-)


I have rewritten this section.  In particular rather than basing everything off
of the mode, I have a now have a more targeted approach that knows which
register, mode, and with sign extension requires D/DS/DQ addresses.  Of course
this only works after register allocation.  Before register allocation, we
still have to make a best guess based only on the mode.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Patch

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index a578e0f27f7..8ca98299950 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1622,6 +1622,45 @@ 
   return GET_CODE (op) == UNSPEC && XINT (op, 1) == UNSPEC_TOCREL;
 })
 
+;; Return true if the operand is a pc-relative address.
+(define_predicate "pcrel_address"
+  (match_code "label_ref,symbol_ref,const")
+{
+  if (!TARGET_PCREL)
+    return false;
+
+  /* Discard any CONST's.  */
+  if (GET_CODE (op) == CONST)
+    op = XEXP (op, 0);
+
+  /* Validate offset.  */
+  if (GET_CODE (op) == PLUS)
+    {
+      rtx op0 = XEXP (op, 0);
+      rtx op1 = XEXP (op, 1);
+
+      if (!CONST_INT_P (op1) || !SIGNED_34BIT_OFFSET_P (INTVAL (op1), 0))
+	return false;
+
+      op = op0;
+    }
+
+  return LABEL_REF_P (op) || SYMBOL_REF_PCREL_P (op);
+})
+
+;; Return 1 if op is a prefixed memory operand
+(define_predicate "prefixed_mem_operand"
+  (match_code "mem")
+{
+  return rs6000_prefixed_address (XEXP (op, 0), GET_MODE (op));
+})
+
+;; Return 1 if op is a memory operand that is not a prefixed memory
+;; operand.
+(define_predicate "non_prefixed_mem_operand"
+  (and (match_operand 0 "memory_operand")
+       (not (match_operand 0 "prefixed_mem_operand"))))
+
 ;; Match the first insn (addis) in fusing the combination of addis and loads to
 ;; GPR registers on power8.
 (define_predicate "fusion_gpr_addis"
diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
index 18ece005a96..feb1250fb8b 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -154,6 +154,7 @@  extern align_flags rs6000_loop_align (rtx);
 extern void rs6000_split_logical (rtx [], enum rtx_code, bool, bool, bool);
 extern bool rs6000_pcrel_p (struct function *);
 extern bool rs6000_fndecl_pcrel_p (const_tree);
+extern bool rs6000_prefixed_address (rtx, machine_mode);
 #endif /* RTX_CODE */
 
 #ifdef TREE_CODE
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index f9ef8e38314..31b86633118 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21085,6 +21085,34 @@  print_operand_address (FILE *file, rtx x)
 {
   if (REG_P (x))
     fprintf (file, "0(%s)", reg_names[ REGNO (x) ]);
+
+  /* Is it a pc-relative address?  */
+  else if (pcrel_address (x, Pmode))
+    {
+      HOST_WIDE_INT offset;
+
+      if (GET_CODE (x) == CONST)
+	x = XEXP (x, 0);
+
+      if (GET_CODE (x) == PLUS)
+	{
+	  offset = INTVAL (XEXP (x, 1));
+	  x = XEXP (x, 0);
+	}
+      else
+	offset = 0;
+
+      if (LABEL_REF_P (x))
+	output_asm_label (x);
+      else
+	output_addr_const (file, x);
+
+      if (offset)
+	fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",
+		 offset);
+
+      fputs ("@pcrel", file);
+    }
   else if (SYMBOL_REF_P (x) || GET_CODE (x) == CONST
 	   || GET_CODE (x) == LABEL_REF)
     {
@@ -21569,6 +21597,71 @@  rs6000_pltseq_template (rtx *operands, int which)
 }
 #endif
 
+/* 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;
+}
+
+/* Function to return true if ADDR is a valid prefixed memory address that uses
+   mode MODE.  */
+
+bool
+rs6000_prefixed_address (rtx addr, machine_mode mode)
+{
+  if (!TARGET_PREFIXED_ADDR || !mode_supports_prefixed_address_p (mode))
+    return false;
+
+  /* Check for PC-relative addresses.  */
+  if (pcrel_address (addr, Pmode))
+    return true;
+
+  /* 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)
+    {
+      HOST_WIDE_INT value, mask;
+      rtx op0 = XEXP (addr, 0);
+      rtx op1 = XEXP (addr, 1);
+
+      if (!base_reg_operand (op0, Pmode) || !CONST_INT_P (op1))
+	return false;
+
+      value = INTVAL (op1);
+      if (!SIGNED_34BIT_OFFSET_P (value, 0))
+	return false;
+
+      /* Offset larger than 16-bits?  */
+      if (!SIGNED_16BIT_OFFSET_P (value, 0))
+	return true;
+
+      /* DQ instruction (bottom 4 bits must be 0) for vectors.  */
+      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;
+
+      /* QImode/HImode has no restrictions.  */
+      else
+	return true;
+
+      /* Return true if we must use a prefixed instruction.  */
+      return ((value & ~mask) != value);
+    }
+
+  return false;
+}
+
 #if defined (HAVE_GAS_HIDDEN) && !TARGET_MACHO
 /* Emit an assembler directive to set symbol visibility for DECL to
    VISIBILITY_TYPE.  */
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
index 8119c6621d5..34fa36b6ed9 100644
--- a/gcc/config/rs6000/rs6000.h
+++ b/gcc/config/rs6000/rs6000.h
@@ -2516,3 +2516,10 @@  extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT];
   IN_RANGE (VALUE,							\
 	    -(HOST_WIDE_INT_1 << 33),					\
 	    (HOST_WIDE_INT_1 << 33) - 1 - (EXTRA))
+
+/* Flag to mark SYMBOL_REF objects to say they are local addresses and are used
+   in pc-relative addresses.  */
+#define SYMBOL_FLAG_PCREL	SYMBOL_FLAG_MACH_DEP
+
+#define SYMBOL_REF_PCREL_P(X)						\
+  (SYMBOL_REF_P (X) && SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_PCREL)