public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Peter Bergner <bergner@linux.ibm.com>
To: "Kewen.Lin" <linkw@linux.ibm.com>
Cc: segher@kernel.crashing.org, gcc-patches@gcc.gnu.org,
	Michael Meissner <meissner@linux.ibm.com>,
	P Jeevitha <jeevitha@linux.ibm.com>
Subject: Re: [PATCH] rs6000: Disable PCREL for unsupported targets [PR111045]
Date: Fri, 10 Nov 2023 17:51:30 -0600	[thread overview]
Message-ID: <a250f7f9-9487-4ac6-ba8f-c221320479b4@linux.ibm.com> (raw)
In-Reply-To: <5bd33bfc-ca84-6879-75b9-628313bdd7e7@linux.ibm.com>

On 8/27/23 9:06 PM, Kewen.Lin wrote:
> Assuming we only have ELFv2_ABI_CHECK in PCREL_SUPPORTED_BY_OS, we
> can have either TARGET_PCREL or !TARGET_PCREL after the checking.
> For the latter, it's fine and don't need any checks. For the former,
> if it's implicit, for !TARGET_PREFIXED we will clean it silently;
> while if it's explicit, for !TARGET_PREFIXED we will emit an error.
> TARGET_PREFIXED checking has considered Power10, so it's also
> concerned accordingly.
[snip]
> Yeah, looking forward to their opinions.  IMHO, with the current proposed
> change, pcrel doesn't look like a pure Power10 hardware feature, it also
> quite relies on ABIs, that's why I thought it seems good not to turn it
> on by default for Power10.

Ok, how about the patch below?  This removes OPTION_MASK_PCREL from the
power10 flags, so instead of our options override code needing to disable
PCREL on the systems that don't support it, we now enable it only on those
systems that do support it.

Jeevitha, can you test this patch to see whether it fixes the testsuite
issue caused by your earlier patch that was approved, but not yet pushed?
That was the use GPR2 for register allocation, correct?  Note, you'll need
to update the patch to replace the rs6000_pcrel_p() usage with just
TARGET_PCREL, since this patch removes rs6000_pcrel_p().

If testing is clean and everyone is OK with the patch, I'll officially
submit it for review with git log entry, etc.

Peter


gcc/
	* config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Only test the ABI.
	* config/rs6000/rs6000-cpus.def (RS6000_CPU): Remove OPTION_MASK_PCREL
	from power10.
	* config/rs6000/predicates.md: Use TARGET_PCREL.
	* config/rs6000/rs6000-logue.cc (rs6000_decl_ok_for_sibcall): Likewise.
	(rs6000_global_entry_point_prologue_needed_p): Likewise.
	(rs6000_output_function_prologue): Likewise.
	* config/rs6000/rs6000.md: Likewise.
	* config/rs6000/rs6000.cc (rs6000_option_override_internal): Rework
	the logic for enabling PCREL by default.
	(rs6000_legitimize_tls_address): Use TARGET_PCREL.
	(rs6000_call_template_1): Likewise.
	(rs6000_indirect_call_template_1): Likewise.
	(rs6000_longcall_ref): Likewise.
	(rs6000_call_aix): Likewise.
	(rs6000_sibcall_aix): Likewise.
	(rs6000_pcrel_p): Remove.
	* config/rs6000/rs6000-protos.h (rs6000_pcrel_p): Likewise.

diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
index 98b7255c95f..5b77bd7fd51 100644
--- a/gcc/config/rs6000/linux64.h
+++ b/gcc/config/rs6000/linux64.h
@@ -563,8 +563,5 @@ extern int dot_symbols;
 #define TARGET_FLOAT128_ENABLE_TYPE 1
 
 /* Enable using prefixed PC-relative addressing on POWER10 if the ABI
-   supports it.  The ELF v2 ABI only supports PC-relative relocations for
-   the medium code model.  */
-#define PCREL_SUPPORTED_BY_OS	(TARGET_POWER10 && TARGET_PREFIXED	\
-				 && ELFv2_ABI_CHECK			\
-				 && TARGET_CMODEL == CMODEL_MEDIUM)
+   supports it.  */
+#define PCREL_SUPPORTED_BY_OS	(ELFv2_ABI_CHECK)
diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
index 4f350da378c..fe01a2312ae 100644
--- a/gcc/config/rs6000/rs6000-cpus.def
+++ b/gcc/config/rs6000/rs6000-cpus.def
@@ -256,7 +256,8 @@ RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
 	    | OPTION_MASK_HTM)
 RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
 	    | OPTION_MASK_HTM)
-RS6000_CPU ("power10", PROCESSOR_POWER10, MASK_POWERPC64 | ISA_3_1_MASKS_SERVER)
+RS6000_CPU ("power10", PROCESSOR_POWER10, MASK_POWERPC64
+	    | (ISA_3_1_MASKS_SERVER & ~OPTION_MASK_PCREL))
 RS6000_CPU ("powerpc", PROCESSOR_POWERPC, 0)
 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, OPTION_MASK_PPC_GFXOPT
 	    | MASK_POWERPC64)
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index ef7d3f214c4..0b76541fc0a 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1216,7 +1216,7 @@
 			 && SYMBOL_REF_DECL (op) != NULL
 			 && TREE_CODE (SYMBOL_REF_DECL (op)) == FUNCTION_DECL
 			 && (rs6000_fndecl_pcrel_p (SYMBOL_REF_DECL (op))
-			     != rs6000_pcrel_p ()))")))
+			     != TARGET_PCREL))")))
 
 ;; Return 1 if this operand is a valid input for a move insn.
 (define_predicate "input_operand"
diff --git a/gcc/config/rs6000/rs6000-logue.cc b/gcc/config/rs6000/rs6000-logue.cc
index 98846f781ec..9e08d9bb4d2 100644
--- a/gcc/config/rs6000/rs6000-logue.cc
+++ b/gcc/config/rs6000/rs6000-logue.cc
@@ -1106,7 +1106,7 @@ rs6000_decl_ok_for_sibcall (tree decl)
 	 r2 for its caller's TOC.  Such a function may make sibcalls to any
 	 function, whether local or external, without restriction based on
 	 TOC-save/restore rules.  */
-      if (rs6000_pcrel_p ())
+      if (TARGET_PCREL)
 	return true;
 
       /* Otherwise, under the AIX or ELFv2 ABIs we can't allow sibcalls
@@ -2583,7 +2583,7 @@ rs6000_global_entry_point_prologue_needed_p (void)
     return false;
 
   /* PC-relative functions never generate a global entry point prologue.  */
-  if (rs6000_pcrel_p ())
+  if (TARGET_PCREL)
     return false;
 
   /* Ensure we have a global entry point for thunks.   ??? We could
@@ -4045,7 +4045,7 @@ rs6000_output_function_prologue (FILE *file)
 					       patch_area_entry == 0);
     }
 
-  else if (rs6000_pcrel_p ())
+  else if (TARGET_PCREL)
     {
       const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
       /* All functions compiled to use PC-relative addressing will
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 2a1b5ecfaee..97de6940e07 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -11333,7 +11333,7 @@
 			    (match_operand:P 3 "" "")]
 			   UNSPECV_PLT_PCREL))]
   "HAVE_AS_PLTSEQ && TARGET_ELF
-   && rs6000_pcrel_p ()"
+   && TARGET_PCREL"
 {
   return rs6000_pltseq_template (operands, RS6000_PLTSEQ_PLT_PCREL34);
 }
@@ -11418,7 +11418,7 @@
   else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS)
     output_asm_insn ("creqv 6,6,6", operands);
 
-  if (rs6000_pcrel_p ())
+  if (TARGET_PCREL)
     return "bl %z0@notoc";
   return (DEFAULT_ABI == ABI_V4 && flag_pic) ? "bl %z0@local" : "bl %z0";
 }
@@ -11439,7 +11439,7 @@
   else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
     output_asm_insn ("creqv 6,6,6", operands);
 
-  if (rs6000_pcrel_p ())
+  if (TARGET_PCREL)
     return "bl %z1@notoc";
   return (DEFAULT_ABI == ABI_V4 && flag_pic) ? "bl %z1@local" : "bl %z1";
 }
@@ -11614,7 +11614,7 @@
 }
   [(set_attr "type" "branch")
    (set (attr "length")
-	(if_then_else (match_test "rs6000_pcrel_p ()")
+	(if_then_else (match_test "TARGET_PCREL")
 	  (const_int 4)
 	  (const_int 8)))])
 
@@ -11631,7 +11631,7 @@
 }
   [(set_attr "type" "branch")
    (set (attr "length")
-	(if_then_else (match_test "rs6000_pcrel_p ()")
+	(if_then_else (match_test "TARGET_PCREL")
 	    (const_int 4)
 	    (const_int 8)))])
 
@@ -11705,7 +11705,7 @@
 	 (match_operand 1))
    (use (match_operand:SI 2 "immediate_operand" "n,n,n"))
    (clobber (reg:P LR_REGNO))]
-  "rs6000_pcrel_p ()"
+  "TARGET_PCREL"
 {
   return rs6000_indirect_call_template (operands, 0);
 }
@@ -11742,7 +11742,7 @@
 	      (match_operand:P 2 "unspec_tls" "")))
    (use (match_operand:SI 3 "immediate_operand" "n,n,n"))
    (clobber (reg:P LR_REGNO))]
-  "rs6000_pcrel_p ()"
+  "TARGET_PCREL"
 {
   return rs6000_indirect_call_template (operands, 1);
 }
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 5f56c3ed85b..c4077c786ec 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -4378,17 +4378,23 @@ rs6000_option_override_internal (bool global_init_p)
   /* If the ABI has support for PC-relative relocations, enable it by default.
      This test depends on the sub-target tests above setting the code model to
      medium for ELF v2 systems.  */
-  if (PCREL_SUPPORTED_BY_OS
-      && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
-    rs6000_isa_flags |= OPTION_MASK_PCREL;
-
-  /* -mpcrel requires -mcmodel=medium, but we can't check TARGET_CMODEL until
-      after the subtarget override options are done.  */
-  else if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
+  if (PCREL_SUPPORTED_BY_OS)
     {
-      if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
+      /* PCREL on ELFv2 currently requires -mcmodel=medium, but we can't check
+	 TARGET_CMODEL until after the subtarget override options are done.  */
+      if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
 	error ("%qs requires %qs", "-mpcrel", "-mcmodel=medium");
 
+      if (TARGET_POWER10
+	  && TARGET_PREFIXED
+	  && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
+	rs6000_isa_flags |= OPTION_MASK_PCREL;
+    }
+  else
+    {
+      if (TARGET_PCREL
+	  && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
+	error ("use of %qs is invalid for this target", "-mpcrel");
       rs6000_isa_flags &= ~OPTION_MASK_PCREL;
     }
 
@@ -9645,7 +9651,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model)
 
   dest = gen_reg_rtx (Pmode);
   if (model == TLS_MODEL_LOCAL_EXEC
-      && (rs6000_tls_size == 16 || rs6000_pcrel_p ()))
+      && (rs6000_tls_size == 16 || TARGET_PCREL))
     {
       rtx tlsreg;
 
@@ -9692,7 +9698,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model)
 	 them in the .got section.  So use a pointer to the .got section,
 	 not one to secondary TOC sections used by 64-bit -mminimal-toc,
 	 or to secondary GOT sections used by 32-bit -fPIC.  */
-      if (rs6000_pcrel_p ())
+      if (TARGET_PCREL)
 	got = const0_rtx;
       else if (TARGET_64BIT)
 	got = gen_rtx_REG (Pmode, 2);
@@ -9757,7 +9763,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model)
 	  rtx uns = gen_rtx_UNSPEC (Pmode, vec, UNSPEC_TLS_GET_ADDR);
 	  set_unique_reg_note (get_last_insn (), REG_EQUAL, uns);
 
-	  if (rs6000_tls_size == 16 || rs6000_pcrel_p ())
+	  if (rs6000_tls_size == 16 || TARGET_PCREL)
 	    {
 	      if (TARGET_64BIT)
 		insn = gen_tls_dtprel_64 (dest, tmp1, addr);
@@ -9798,7 +9804,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model model)
 	  else
 	    insn = gen_tls_got_tprel_32 (tmp2, got, addr);
 	  emit_insn (insn);
-	  if (rs6000_pcrel_p ())
+	  if (TARGET_PCREL)
 	    {
 	      if (TARGET_64BIT)
 		insn = gen_tls_tls_pcrel_64 (dest, tmp2, addr);
@@ -14866,7 +14872,7 @@ rs6000_call_template_1 (rtx *operands, unsigned int funop, bool sibcall)
 	    ? "+32768" : ""));
 
   static char str[32];  /* 1 spare */
-  if (rs6000_pcrel_p ())
+  if (TARGET_PCREL)
     sprintf (str, "b%s %s@notoc%s", sibcall ? "" : "l", z, arg);
   else if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
     sprintf (str, "b%s %s%s%s", sibcall ? "" : "l", z, arg,
@@ -15006,7 +15012,7 @@ rs6000_indirect_call_template_1 (rtx *operands, unsigned int funop,
 		     rel64);
 	}
 
-      const char *notoc = rs6000_pcrel_p () ? "_NOTOC" : "";
+      const char *notoc = TARGET_PCREL ? "_NOTOC" : "";
       const char *addend = (DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT
 			    && flag_pic == 2 ? "+32768" : "");
       if (!speculate)
@@ -15023,7 +15029,7 @@ rs6000_indirect_call_template_1 (rtx *operands, unsigned int funop,
   else if (!speculate)
     s += sprintf (s, "crset 2\n\t");
 
-  if (rs6000_pcrel_p ())
+  if (TARGET_PCREL)
     {
       if (speculate)
 	sprintf (s, "b%%T%ul", funop);
@@ -20685,7 +20691,7 @@ rs6000_longcall_ref (rtx call_ref, rtx arg)
     {
       rtx base = const0_rtx;
       int regno = 12;
-      if (rs6000_pcrel_p ())
+      if (TARGET_PCREL)
 	{
 	  rtx reg = gen_rtx_REG (Pmode, regno);
 	  rtx u = gen_rtx_UNSPEC_VOLATILE (Pmode,
@@ -25934,7 +25940,7 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
   if (!SYMBOL_REF_P (func)
       || (DEFAULT_ABI == ABI_AIX && !SYMBOL_REF_FUNCTION_P (func)))
     {
-      if (!rs6000_pcrel_p ())
+      if (!TARGET_PCREL)
 	{
 	  /* Save the TOC into its reserved slot before the call,
 	     and prepare to restore it after the call.  */
@@ -26040,7 +26046,7 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
   else
     {
       /* No TOC register needed for calls from PC-relative callers.  */
-      if (!rs6000_pcrel_p ())
+      if (!TARGET_PCREL)
 	/* Direct calls use the TOC: for local calls, the callee will
 	   assume the TOC register is set; for non-local calls, the
 	   PLT stub needs the TOC register.  */
@@ -26089,7 +26095,7 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
     {
       /* PCREL can do a sibling call to a longcall function
 	 because we don't need to restore the TOC register.  */
-      gcc_assert (rs6000_pcrel_p ());
+      gcc_assert (TARGET_PCREL);
       func_desc = rs6000_longcall_ref (func_desc, tlsarg);
     }
   else
@@ -26116,7 +26122,7 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx tlsarg, rtx cookie)
   insn = emit_call_insn (insn);
 
   /* Note use of the TOC register.  */
-  if (!rs6000_pcrel_p ())
+  if (!TARGET_PCREL)
     use_reg (&CALL_INSN_FUNCTION_USAGE (insn),
 	     gen_rtx_REG (Pmode, TOC_REGNUM));
 
@@ -26416,16 +26422,6 @@ rs6000_function_pcrel_p (struct function *fn)
   return rs6000_fndecl_pcrel_p (fn->decl);
 }
 
-/* Return whether we should generate PC-relative code for the current
-   function.  */
-bool
-rs6000_pcrel_p ()
-{
-  return (DEFAULT_ABI == ABI_ELFv2
-	  && (rs6000_isa_flags & OPTION_MASK_PCREL) != 0
-	  && TARGET_CMODEL == CMODEL_MEDIUM);
-}
-
 \f
 /* Given an address (ADDR), a mode (MODE), and what the format of the
    non-prefixed address (NON_PREFIXED_FORMAT) is, return the instruction format
diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
index f70118ea40f..8ffec295d09 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -158,7 +158,6 @@ extern rtx rs6000_allocate_stack_temp (machine_mode, bool, bool);
 extern align_flags rs6000_loop_align (rtx);
 extern void rs6000_split_logical (rtx [], enum rtx_code, bool, bool, bool);
 extern bool rs6000_function_pcrel_p (struct function *);
-extern bool rs6000_pcrel_p (void);
 extern bool rs6000_fndecl_pcrel_p (const_tree);
 extern void rs6000_output_addr_vec_elt (FILE *, int);
 


  reply	other threads:[~2023-11-10 23:53 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-21 10:32 jeevitha
2023-08-22  1:51 ` Kewen.Lin
2023-08-24  2:07   ` Peter Bergner
2023-08-24  5:56     ` Kewen.Lin
2023-08-25  3:20       ` Peter Bergner
2023-08-25 11:20         ` Kewen.Lin
2023-08-25 22:04           ` Peter Bergner
2023-08-28  2:06             ` Kewen.Lin
2023-11-10 23:51               ` Peter Bergner [this message]
2023-11-13 18:24                 ` jeevitha
2023-11-14  2:33                 ` Kewen.Lin
2023-11-14 21:40                   ` Peter Bergner
2023-11-11  0:03           ` Peter Bergner
2023-11-27 23:11             ` Michael Meissner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a250f7f9-9487-4ac6-ba8f-c221320479b4@linux.ibm.com \
    --to=bergner@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeevitha@linux.ibm.com \
    --cc=linkw@linux.ibm.com \
    --cc=meissner@linux.ibm.com \
    --cc=segher@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).