diff mbox series

rs6000: Mirror fix for PR102347 into the new builtins support

Message ID a1f71af4-9c58-b9b0-0d85-3f92195a3d14@linux.ibm.com
State Committed
Commit d683a1b3e89007211a7c800bb61647d8ac42cb6b
Headers
Series rs6000: Mirror fix for PR102347 into the new builtins support |

Commit Message

Li, Pan2 via Gcc-patches Dec. 1, 2021, 3:29 p.m. UTC 
  Hi!

Recently Kewen fixed a problem in the old builtins support where
rs6000_builtin_decl prematurely indicated that a target builtin is
unavailable.  This also needs to be done for the new builtins support, but in
this case we have to ensure the error message is still produced from the
overload support in rs6000-c.c.  Unfortunately, this is less straightforward
than it could be, because header file includes need to be adjusted to make this
happen.  Someday we'll consolidate all the builtin code in one file and this
won't have to be so ugly.

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

Thanks!
Bill


2021-12-01  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	PR target/102347
	* config/rs6000/rs6000-c.c (rs6000-builtins.h): Stop including.
	(rs6000-internal.h): Include.
	(altivec_resolve_new_overloaded_builtin): Move call to
	rs6000_invalid_new_builtin here from rs6000_new_builtin_decl.
	* config/rs6000/rs6000-call.c (rs6000-builtins.h): Stop including.
	(rs6000_invalid_new_builtin): Remove static qualifier.
	(rs6000_new_builtin_decl): Remove test for supported builtin.
	* config/rs6000/rs6000-internal.h (rs6000-builtins.h): Include.
	(rs6000_invalid_new_builtin): Declare.
	* config/rs6000/rs6000.c (rs6000-builtins.h): Don't include.
---
 gcc/config/rs6000/rs6000-c.c        | 11 +++++++----
 gcc/config/rs6000/rs6000-call.c     |  9 +--------
 gcc/config/rs6000/rs6000-internal.h |  3 +++
 gcc/config/rs6000/rs6000.c          |  1 -
 4 files changed, 11 insertions(+), 13 deletions(-)

Comments

Segher Boessenkool Dec. 1, 2021, 5:21 p.m. UTC | #1 
Hi!

On Wed, Dec 01, 2021 at 09:29:42AM -0600, Bill Schmidt wrote:
> Recently Kewen fixed a problem in the old builtins support where
> rs6000_builtin_decl prematurely indicated that a target builtin is
> unavailable.  This also needs to be done for the new builtins support, but in
> this case we have to ensure the error message is still produced from the
> overload support in rs6000-c.c.  Unfortunately, this is less straightforward
> than it could be, because header file includes need to be adjusted to make this
> happen.  Someday we'll consolidate all the builtin code in one file and this
> won't have to be so ugly.
> 
> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this
> okay for trunk?

This is okay for trunk.  Thanks!

Is there some place we can store what original builtin was used when
some overload is resolved?  Just in the new builtin code, don't spend
time on the old stuff :-)


Segher
Li, Pan2 via Gcc-patches Dec. 1, 2021, 6:06 p.m. UTC | #2 
On 12/1/21 11:21 AM, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Dec 01, 2021 at 09:29:42AM -0600, Bill Schmidt wrote:
>> Recently Kewen fixed a problem in the old builtins support where
>> rs6000_builtin_decl prematurely indicated that a target builtin is
>> unavailable.  This also needs to be done for the new builtins support, but in
>> this case we have to ensure the error message is still produced from the
>> overload support in rs6000-c.c.  Unfortunately, this is less straightforward
>> than it could be, because header file includes need to be adjusted to make this
>> happen.  Someday we'll consolidate all the builtin code in one file and this
>> won't have to be so ugly.
>>
>> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is this
>> okay for trunk?
> This is okay for trunk.  Thanks!
>
> Is there some place we can store what original builtin was used when
> some overload is resolved?  Just in the new builtin code, don't spend
> time on the old stuff :-)

I think we can do much better, with a little work.  The macros are a little
problematic, but I have ideas about how to make this better in a future
patch.  (There's a fair amount of test suite fallout, so it should wait.)

Thanks for the review!

Bill
>
>
> Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index 5eeac9d4c06..8e83d97e72f 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -35,7 +35,7 @@ 
 #include "langhooks.h"
 #include "c/c-tree.h"
 
-#include "rs6000-builtins.h"
+#include "rs6000-internal.h"
 
 static tree altivec_resolve_new_overloaded_builtin (location_t, tree, void *);
 
@@ -2987,11 +2987,14 @@  altivec_resolve_new_overloaded_builtin (location_t loc, tree fndecl,
 	const char *name = rs6000_overload_info[adj_fcode].ovld_name;
 	if (!supported)
 	  {
+	    /* Indicate that the instantiation of the overloaded builtin
+	       name is not available with the target flags in effect.  */
+	    rs6000_gen_builtins fcode = (rs6000_gen_builtins) instance->bifid;
+	    rs6000_invalid_new_builtin (fcode);
+	    /* Provide clarity of the relationship between the overload
+	       and the instantiation.  */
 	    const char *internal_name
 	      = rs6000_builtin_info_x[instance->bifid].bifname;
-	    /* An error message making reference to the name of the
-	       non-overloaded function has already been issued.  Add
-	       clarification of the previous message.  */
 	    rich_location richloc (line_table, input_location);
 	    inform (&richloc,
 		    "overloaded builtin %qs is implemented by builtin %qs",
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index cd477fa4876..01688c4169d 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -69,7 +69,6 @@ 
 #include "opts.h"
 
 #include "rs6000-internal.h"
-#include "rs6000-builtins.h"
 
 #if TARGET_MACHO
 #include "gstab.h"  /* for N_SLINE */
@@ -11905,7 +11904,7 @@  rs6000_invalid_builtin (enum rs6000_builtins fncode)
 /* Raise an error message for a builtin function that is called without the
    appropriate target options being set.  */
 
-static void
+void
 rs6000_invalid_new_builtin (enum rs6000_gen_builtins fncode)
 {
   size_t j = (size_t) fncode;
@@ -16624,12 +16623,6 @@  rs6000_new_builtin_decl (unsigned code, bool /* initialize_p */)
   if (fcode >= RS6000_OVLD_MAX)
     return error_mark_node;
 
-  if (!rs6000_new_builtin_is_supported (fcode))
-    {
-      rs6000_invalid_new_builtin (fcode);
-      return error_mark_node;
-    }
-
   return rs6000_builtin_decls_x[code];
 }
 
diff --git a/gcc/config/rs6000/rs6000-internal.h b/gcc/config/rs6000/rs6000-internal.h
index 88cf9bd5692..a880fd37618 100644
--- a/gcc/config/rs6000/rs6000-internal.h
+++ b/gcc/config/rs6000/rs6000-internal.h
@@ -22,6 +22,8 @@ 
 #ifndef GCC_RS6000_INTERNAL_H
 #define GCC_RS6000_INTERNAL_H
 
+#include "rs6000-builtins.h"
+
 /* Structure used to define the rs6000 stack */
 typedef struct rs6000_stack {
   int reload_completed;		/* stack info won't change from here on */
@@ -140,6 +142,7 @@  extern void rs6000_output_mi_thunk (FILE *file,
 extern bool rs6000_output_addr_const_extra (FILE *file, rtx x);
 extern bool rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi);
 extern tree rs6000_build_builtin_va_list (void);
+extern void rs6000_invalid_new_builtin (rs6000_gen_builtins fncode);
 extern void rs6000_va_start (tree valist, rtx nextarg);
 extern tree rs6000_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p,
 				    gimple_seq *post_p);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 289c1b3df24..945157b1c1a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -78,7 +78,6 @@ 
 #include "case-cfn-macros.h"
 #include "ppc-auxv.h"
 #include "rs6000-internal.h"
-#include "rs6000-builtins.h"
 #include "opts.h"
 
 /* This file should be included last.  */