Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote:
> On Tue, 5 Apr 2022, Jakub Jelinek wrote:
> 
> > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote:
> > > > In GIMPLE, we call:
> > > >   && gimple_builtin_call_types_compatible_p (stmt, fndecl)
> > > > but that is insufficient, that verifies whether the arguments passed to
> > > > GIMPLE_CALL match the fndecl argument types.  But that fndecl may very 
> > > > well
> > > > be the user declaration, so doesn't have to match exactly the builtin
> > > > as predeclared by builtins.def etc. (though, there is the cotcha that 
> > > > say
> > > > for FILE * we just use void * etc.).  So e.g. in tree-ssa-strlen.cc
> > > > we use additional:
> > > >   tree callee = gimple_call_fndecl (stmt);
> > > >   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
> > > >   if (decl
> > > >   && decl != callee
> > > >   && !gimple_builtin_call_types_compatible_p (stmt, decl))
> > > > return false;
> > > 
> > > Yeah, I think we should use that (and only that) function decl
> > > in get_call_combined_fn and gimple_call_combined_fn until the
> > > frontend stops slapping wrong BUILT_IN_* on random decls.
> > 
> > So, as a preparation step, this patch adjusts gimple_call_builtin_p
> > and gimple_call_combined_fn so that they check argument types against
> > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the
> > actual used fndecl.
> > 
> > Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> 
> You forgot to attach the patch ...

Oops, here it is.  Also bootstrapped/regtested on {x86_64,i686}-linux.

2022-04-06  Jakub Jelinek  

PR tree-optimization/105150
* gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn):
For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p
preferrably on builtin_decl_explicit decl rather than fndecl.
* tree-ssa-strlen.cc (valid_builtin_call): Don't call
gimple_builtin_call_types_compatible_p here.

--- gcc/gimple.cc.jj2022-02-09 15:15:59.436837973 +0100
+++ gcc/gimple.cc   2022-04-05 13:04:58.405586995 +0200
@@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
-return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+{
+  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+   if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+ fndecl = decl;
+  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+}
   return false;
 }
 
@@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && DECL_BUILT_IN_CLASS (fndecl) == klass)
-return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+{
+  if (klass == BUILT_IN_NORMAL)
+   if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+ fndecl = decl;
+  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+}
   return false;
 }
 
@@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && fndecl_built_in_p (fndecl, code))
-return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+{
+  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+   fndecl = decl;
+  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
+}
   return false;
 }
 
@@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s
return as_combined_fn (gimple_call_internal_fn (call));
 
   tree fndecl = gimple_call_fndecl (stmt);
-  if (fndecl
- && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
- && gimple_builtin_call_types_compatible_p (stmt, fndecl))
-   return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
+  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
+   {
+ tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
+ if (!decl)
+   decl = fndecl;
+ if (gimple_builtin_call_types_compatible_p (stmt, decl))
+   return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
+   }
 }
   return CFN_LAST;
 }
--- gcc/tree-ssa-strlen.cc.jj   2022-03-30 09:11:53.310103911 +0200
+++ gcc/tree-ssa-strlen.cc  2022-04-05 12:22:56.023057416 +0200
@@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt)
 return false;
 
   tree callee = gimple_call_fndecl (stmt);
-  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
-  if (decl
-  && decl != callee
-  && !gimple_builtin_call_types_compatible_p (stmt, decl))
-return false;
-
   switch (DECL_FUNCTION_CODE (callee))
 {
 case BUILT_IN_MEMCMP:


Jakub

Re: [PATCH] gcc-changelog: ignore one more revision

[Martin Liška]

On 4/4/22 22:07, Joseph Myers wrote:

On Mon, 4 Apr 2022, Martin Liška wrote:


Ignore:

Checking 86d8e0c0652ef5236a460b75c25e4f7093cc0651: FAILED
ERR: line should start with a tab: "This reverts commits r12-7804 and
r12-7929."
ERR: could not deduce ChangeLog file

It seems Jason pushed the revision to origin/trunk where the checking script
is not run.


I thought I'd fixed that (last August) by making the hooks-bin scripts
handle master and trunk the same.


Well, I can't prove it, it's just a guess. Maybe Jason can answer the question?
Note the server hook should have rejected the revision.

Cheers,
Martin


 

[PATCH] tree.cc: Add tree_builtin_call_types_compatible_p [PR105150]

[Jakub Jelinek via Gcc-patches]

Hi!

And here is the follow-up patch that does the argument checking
on GENERIC.  It ensures TYPE_MAIN_VARIANT == TYPE_MAIN_VARIANT
compatibility on the arguments, except for pointer arguments like
FILE *, const struct tm * etc. that would never match that way
and similarly to the gimple one also char/short promotions.
Tested that it works correctly with fprintf.

Not sure if we shouldn't allow any useless_type_conversion_p
differences for all pointers though, say if somebody uses
a K&R declared builtin and passes in char msg[] = "...";
then it will be char * instead of const char *.

Bootstrapped/regtested on {powerpc64le,x86_64,i686}-linux, ok for trunk?

2022-04-06  Jakub Jelinek  

PR tree-optimization/105150
* tree.cc (tree_builtin_call_types_compatible_p): New function.
(get_call_combined_fn): Use it.

* gcc.dg/pr105150.c: New test.

--- gcc/tree.cc.jj  2022-03-26 08:11:28.447530077 +0100
+++ gcc/tree.cc 2022-04-05 17:09:46.892114064 +0200
@@ -8406,6 +8406,61 @@ get_callee_fndecl (const_tree call)
   return NULL_TREE;
 }
 
+/* Return true when STMTs arguments and return value match those of FNDECL,
+   a decl of a builtin function.  */
+
+static bool
+tree_builtin_call_types_compatible_p (const_tree call, tree fndecl)
+{
+  gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
+
+  if (TYPE_MAIN_VARIANT (TREE_TYPE (call))
+  != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (fndecl
+return false;
+
+  tree targs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
+  unsigned nargs = call_expr_nargs (call);
+  for (unsigned i = 0; i < nargs; ++i, targs = TREE_CHAIN (targs))
+{
+  /* Variadic args follow.  */
+  if (!targs)
+   return true;
+  tree arg = CALL_EXPR_ARG (call, i);
+  tree type = TREE_VALUE (targs);
+  if (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
+   {
+ /* If argument in the builtin fndecl is the artificial pointer
+for FILE * etc., allow any pointer type arg for it.  */
+ if (POINTER_TYPE_P (type)
+ && POINTER_TYPE_P (TREE_TYPE (arg))
+ && useless_type_conversion_p (type, TREE_TYPE (arg)))
+   {
+ unsigned int j;
+ for (j = 0; j < ARRAY_SIZE (builtin_structptr_types); ++j)
+   if (type == builtin_structptr_types[j].node
+   && (builtin_structptr_types[j].node
+   != builtin_structptr_types[j].base))
+ break;
+ if (j < ARRAY_SIZE (builtin_structptr_types))
+   continue;
+   }
+ /* char/short integral arguments are promoted to int
+by several frontends if targetm.calls.promote_prototypes
+is true.  Allow such promotion too.  */
+ if (INTEGRAL_TYPE_P (type)
+ && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node)
+ && targetm.calls.promote_prototypes (TREE_TYPE (fndecl))
+ && useless_type_conversion_p (integer_type_node,
+   TREE_TYPE (arg)))
+   continue;
+ return false;
+   }
+}
+  if (targs && !VOID_TYPE_P (TREE_VALUE (targs)))
+return false;
+  return true;
+}
+
 /* If CALL_EXPR CALL calls a normal built-in function or an internal function,
return the associated function code, otherwise return CFN_LAST.  */
 
@@ -8420,7 +8475,13 @@ get_call_combined_fn (const_tree call)
 
   tree fndecl = get_callee_fndecl (call);
   if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
-return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
+{
+  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
+  if (!decl)
+   decl = fndecl;
+  if (tree_builtin_call_types_compatible_p (call, decl))
+   return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
+}
 
   return CFN_LAST;
 }
--- gcc/testsuite/gcc.dg/pr105150.c.jj  2022-04-05 16:56:22.894319239 +0200
+++ gcc/testsuite/gcc.dg/pr105150.c 2022-04-05 16:56:16.646406314 +0200
@@ -0,0 +1,8 @@
+/* PR tree-optimization/105150 */
+/* { dg-options "-w -Ofast" } */
+
+#define A(name) __typeof (__builtin_##name (0)) name (); \
+  float name##1 () { return !name (1); } \
+  double name##2 () { return name (1.0L); }
+#define B(name) A(name) A(name##l)
+B (sqrt)

Jakub

[PATCH] docs: Document new param x86-stlf-window-ninsns.

[Martin Liška]

Hi.

The patch documents the newly added parameter. One question I have is
if it's fine listing it under 'i386 and x86_64 targets'?

Cheers,
Martin

gcc/ChangeLog:

* doc/invoke.texi: Document it.
---
 gcc/doc/invoke.texi | 8 
 1 file changed, 8 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3936aef69d0..1a51759e6e4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15247,6 +15247,14 @@ loop.  The default value is four.
 
 @end table
 
+The following choices of @var{name} are available on i386 and x86_64 targets:

+
+@table @gcctabopt
+@item x86-stlf-window-ninsns
+Instructions number above which STFL stall penalty can be compensated.
+
+@end table
+
 @end table
 
 @node Instrumentation Options

--
2.35.1

Re: [PATCH] Add condition coverage profiling

[Sebastian Huber]

On 05/04/2022 22:07, Jørgen Kvalsvik wrote:

In action it looks pretty similar to the branch coverage. The -g short
opt carries no significance, but was chosen because it was an available
option with the upper-case free too.

gcov --conditions:

  3:   17:void fn (int a, int b, int c, int d) {
  3:   18:    if ((a && (b || c)) && d)
conditions covered 5/8
condition  1 not covered (false)
condition  2 not covered (true)
condition  2 not covered (false)
  1:   19:    x = 1;
  -:   20:    else
  2:   21:    x = 2;
  3:   22:}

I have some trouble to understand the output. Would 8/8 mean that we have 100%
MC/DC coverage? What does "not covered (false)" or "not covered (true)" mean?


Yes, 8/8 would mean that the expression is 100% covered (all conditions take on
both values and have independent effect on the outcome). 


This is really great.


"not covered" is a
report of missing coverage, that is "condition  1 not covered (false)" means
that bit N (N = 1, b in this case) has not taken on false yet, and to achieve
100% coverage you need a test case where b = false.

The wording is arbitrary, and I tried to strike a balance between density,
clarity, grepability and noise. I am open to other suggestions that would
improve this.


Ok, for the default output this is good. The output can be explained in 
the documentation. I will try to help here.


In theory, would it be possible to print the state of the truth table 
with the information available in the gcda and gcno files? For example:


Truth table for: a && (b || c)) && d

0 | 1 | 2 | 3 || covered
--+---+---+---++
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
0 | X | X | X || Y
1 | 0 | 0 | X || N
1 | 0 | 0 | X || N
1 | 0 | 1 | 0 || N
1 | 0 | 1 | 1 || N
1 | 1 | X | 0 || Y
1 | 1 | X | 0 || Y
1 | 1 | X | 1 || Y
1 | 1 | X | 1 || Y

Would it be possible to map the condition index to a source code 
snippet? For example condition 1 to "b"?




Unrelated to this, in typing up some notes on this I found a few minor and one
quite significant (really, the masking algorithm is broken) error in the
algorithm, which I am working on correcting. I will propose the new patch with
these fixes too once I have finished writing and testing it.


Thanks a lot for this work.

--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote:
> > On Tue, 5 Apr 2022, Jakub Jelinek wrote:
> > 
> > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote:
> > > > > In GIMPLE, we call:
> > > > >   && gimple_builtin_call_types_compatible_p (stmt, fndecl)
> > > > > but that is insufficient, that verifies whether the arguments passed 
> > > > > to
> > > > > GIMPLE_CALL match the fndecl argument types.  But that fndecl may 
> > > > > very well
> > > > > be the user declaration, so doesn't have to match exactly the builtin
> > > > > as predeclared by builtins.def etc. (though, there is the cotcha that 
> > > > > say
> > > > > for FILE * we just use void * etc.).  So e.g. in tree-ssa-strlen.cc
> > > > > we use additional:
> > > > >   tree callee = gimple_call_fndecl (stmt);
> > > > >   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
> > > > >   if (decl
> > > > >   && decl != callee
> > > > >   && !gimple_builtin_call_types_compatible_p (stmt, decl))
> > > > > return false;
> > > > 
> > > > Yeah, I think we should use that (and only that) function decl
> > > > in get_call_combined_fn and gimple_call_combined_fn until the
> > > > frontend stops slapping wrong BUILT_IN_* on random decls.
> > > 
> > > So, as a preparation step, this patch adjusts gimple_call_builtin_p
> > > and gimple_call_combined_fn so that they check argument types against
> > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the
> > > actual used fndecl.
> > > 
> > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> > 
> > You forgot to attach the patch ...
> 
> Oops, here it is.  Also bootstrapped/regtested on {x86_64,i686}-linux.

OK.

Thanks,
Richard.

> 2022-04-06  Jakub Jelinek  
> 
>   PR tree-optimization/105150
>   * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn):
>   For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p
>   preferrably on builtin_decl_explicit decl rather than fndecl.
>   * tree-ssa-strlen.cc (valid_builtin_call): Don't call
>   gimple_builtin_call_types_compatible_p here.
> 
> --- gcc/gimple.cc.jj  2022-02-09 15:15:59.436837973 +0100
> +++ gcc/gimple.cc 2022-04-05 13:04:58.405586995 +0200
> @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +{
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +   fndecl = decl;
> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +}
>return false;
>  }
>  
> @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) == klass)
> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +{
> +  if (klass == BUILT_IN_NORMAL)
> + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +   fndecl = decl;
> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +}
>return false;
>  }
>  
> @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& fndecl_built_in_p (fndecl, code))
> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +{
> +  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> + fndecl = decl;
> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> +}
>return false;
>  }
>  
> @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s
>   return as_combined_fn (gimple_call_internal_fn (call));
>  
>tree fndecl = gimple_call_fndecl (stmt);
> -  if (fndecl
> -   && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
> -   && gimple_builtin_call_types_compatible_p (stmt, fndecl))
> - return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> +  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> + {
> +   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
> +   if (!decl)
> + decl = fndecl;
> +   if (gimple_builtin_call_types_compatible_p (stmt, decl))
> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> + }
>  }
>return CFN_LAST;
>  }
> --- gcc/tree-ssa-strlen.cc.jj 2022-03-30 09:11:53.310103911 +0200
> +++ gcc/tree-ssa-strlen.cc2022-04-05 12:22:56.023057416 +0200
> @@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt)
>  return false;
>  
>tree callee = gimple_call_fndecl (stmt);
> -  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));

RE: [PATCH] docs: Document new param x86-stlf-window-ninsns.

[Liu, Hongtao via Gcc-patches]

> -Original Message-
> From: Martin Liška 
> Sent: Wednesday, April 6, 2022 3:35 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Liu, Hongtao 
> Subject: [PATCH] docs: Document new param x86-stlf-window-ninsns.
> 
> Hi.
> 
> The patch documents the newly added parameter. One question I have is if it's
> fine listing it under 'i386 and x86_64 targets'?
Yes, thanks.
> 
> Cheers,
> Martin
> 
> gcc/ChangeLog:
> 
>   * doc/invoke.texi: Document it.
> ---
>   gcc/doc/invoke.texi | 8 
>   1 file changed, 8 insertions(+)
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index
> 3936aef69d0..1a51759e6e4 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -15247,6 +15247,14 @@ loop.  The default value is four.
> 
>   @end table
> 
> +The following choices of @var{name} are available on i386 and x86_64 targets:
> +
> +@table @gcctabopt
> +@item x86-stlf-window-ninsns
> +Instructions number above which STFL stall penalty can be compensated.
> +
> +@end table
> +
>   @end table
> 
>   @node Instrumentation Options
> --
> 2.35.1

Re: [PATCH] tree.cc: Add tree_builtin_call_types_compatible_p [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> Hi!
> 
> And here is the follow-up patch that does the argument checking
> on GENERIC.  It ensures TYPE_MAIN_VARIANT == TYPE_MAIN_VARIANT
> compatibility on the arguments, except for pointer arguments like
> FILE *, const struct tm * etc. that would never match that way
> and similarly to the gimple one also char/short promotions.
> Tested that it works correctly with fprintf.
> 
> Not sure if we shouldn't allow any useless_type_conversion_p
> differences for all pointers though, say if somebody uses
> a K&R declared builtin and passes in char msg[] = "...";
> then it will be char * instead of const char *.
> 
> Bootstrapped/regtested on {powerpc64le,x86_64,i686}-linux, ok for trunk?
> 
> 2022-04-06  Jakub Jelinek  
> 
>   PR tree-optimization/105150
>   * tree.cc (tree_builtin_call_types_compatible_p): New function.
>   (get_call_combined_fn): Use it.
> 
>   * gcc.dg/pr105150.c: New test.
> 
> --- gcc/tree.cc.jj2022-03-26 08:11:28.447530077 +0100
> +++ gcc/tree.cc   2022-04-05 17:09:46.892114064 +0200
> @@ -8406,6 +8406,61 @@ get_callee_fndecl (const_tree call)
>return NULL_TREE;
>  }
>  
> +/* Return true when STMTs arguments and return value match those of FNDECL,
> +   a decl of a builtin function.  */
> +
> +static bool
> +tree_builtin_call_types_compatible_p (const_tree call, tree fndecl)
> +{
> +  gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
> +
> +  if (TYPE_MAIN_VARIANT (TREE_TYPE (call))
> +  != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (fndecl
> +return false;
> +
> +  tree targs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
> +  unsigned nargs = call_expr_nargs (call);
> +  for (unsigned i = 0; i < nargs; ++i, targs = TREE_CHAIN (targs))
> +{
> +  /* Variadic args follow.  */
> +  if (!targs)
> + return true;
> +  tree arg = CALL_EXPR_ARG (call, i);
> +  tree type = TREE_VALUE (targs);
> +  if (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
> + {
> +   /* If argument in the builtin fndecl is the artificial pointer
> +  for FILE * etc., allow any pointer type arg for it.  */
> +   if (POINTER_TYPE_P (type)
> +   && POINTER_TYPE_P (TREE_TYPE (arg))
> +   && useless_type_conversion_p (type, TREE_TYPE (arg)))
> + {
> +   unsigned int j;
> +   for (j = 0; j < ARRAY_SIZE (builtin_structptr_types); ++j)
> + if (type == builtin_structptr_types[j].node
> + && (builtin_structptr_types[j].node
> + != builtin_structptr_types[j].base))
> +   break;
> +   if (j < ARRAY_SIZE (builtin_structptr_types))
> + continue;
> + }
> +   /* char/short integral arguments are promoted to int
> +  by several frontends if targetm.calls.promote_prototypes
> +  is true.  Allow such promotion too.  */
> +   if (INTEGRAL_TYPE_P (type)
> +   && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node)
> +   && targetm.calls.promote_prototypes (TREE_TYPE (fndecl))
> +   && useless_type_conversion_p (integer_type_node,
> + TREE_TYPE (arg)))

On trees we'd use tree_[sign_]nop_conversion () instead of
useless_type_conversion_p, I think it's OK to allow all such
pointer conversions.  In the end this probably means being
more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
would also make the code more similar to 
gimple_builtin_call_types_compatible_p besides
s/useless_type_conversion_p/tree_sign_nop_conversion/

What do you think?  If we don't go for strict TYPE_MAIN_VARIANT
compatibility we should apply lax rules for all, the return
type and all argument types.  It's not that the GENERIC we end
up building from GIMPLE in various places adheres to the strict
GENERIC type compatibility rules ...

Richard.

> + continue;
> +   return false;
> + }
> +}
> +  if (targs && !VOID_TYPE_P (TREE_VALUE (targs)))
> +return false;
> +  return true;
> +}
> +
>  /* If CALL_EXPR CALL calls a normal built-in function or an internal 
> function,
> return the associated function code, otherwise return CFN_LAST.  */
>  
> @@ -8420,7 +8475,13 @@ get_call_combined_fn (const_tree call)
>  
>tree fndecl = get_callee_fndecl (call);
>if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> -return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> +{
> +  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
> +  if (!decl)
> + decl = fndecl;
> +  if (tree_builtin_call_types_compatible_p (call, decl))
> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> +}
>  
>return CFN_LAST;
>  }
> --- gcc/testsuite/gcc.dg/pr105150.c.jj2022-04-05 16:56:22.894319239 
> +0200
> +++ gcc/testsuite/gcc.dg/pr105150.c   2022-04-05 16:56:16.646406314 +0200
> @@ -0,0 +1,8 @@
> +/* PR tree-optimiza

Re: [PATCH] tree.cc: Add tree_builtin_call_types_compatible_p [PR105150]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 09:49:25AM +0200, Richard Biener wrote:
> On trees we'd use tree_[sign_]nop_conversion () instead of
> useless_type_conversion_p, I think it's OK to allow all such
> pointer conversions.  In the end this probably means being
> more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
> would also make the code more similar to 
> gimple_builtin_call_types_compatible_p besides
> s/useless_type_conversion_p/tree_sign_nop_conversion/
> 
> What do you think?  If we don't go for strict TYPE_MAIN_VARIANT
> compatibility we should apply lax rules for all, the return
> type and all argument types.  It's not that the GENERIC we end
> up building from GIMPLE in various places adheres to the strict
> GENERIC type compatibility rules ...

Dunno, we don't really have a verifier for GENERIC.

Don't we even require that the types are actually same rather than
similar for say binary ops other than shifts, or for comparisons,
or result and first argument for unary and binary etc.?
I think at least for the most common cases the C/C++ FEs ensure that
through choosing some common type for the operation and folding the
arguments to that type.  Though I bet we violate it pretty often
with e.g. sizetype vs. size_type_node differences etc.

Jakub

Re: [PATCH] tree.cc: Add tree_builtin_call_types_compatible_p [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> On Wed, Apr 06, 2022 at 09:49:25AM +0200, Richard Biener wrote:
> > On trees we'd use tree_[sign_]nop_conversion () instead of
> > useless_type_conversion_p, I think it's OK to allow all such
> > pointer conversions.  In the end this probably means being
> > more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
> > would also make the code more similar to 
> > gimple_builtin_call_types_compatible_p besides
> > s/useless_type_conversion_p/tree_sign_nop_conversion/
> > 
> > What do you think?  If we don't go for strict TYPE_MAIN_VARIANT
> > compatibility we should apply lax rules for all, the return
> > type and all argument types.  It's not that the GENERIC we end
> > up building from GIMPLE in various places adheres to the strict
> > GENERIC type compatibility rules ...
> 
> Dunno, we don't really have a verifier for GENERIC.
> 
> Don't we even require that the types are actually same rather than
> similar for say binary ops other than shifts, or for comparisons,
> or result and first argument for unary and binary etc.?

I think it's fine to use typedef variants or CV qualified variants there.
But indeed fold-const.cc in most places fold_convert()s everything,
but mostly because it strips nop conversions before applying patterns.

> I think at least for the most common cases the C/C++ FEs ensure that
> through choosing some common type for the operation and folding the
> arguments to that type.  Though I bet we violate it pretty often
> with e.g. sizetype vs. size_type_node differences etc.

Yeah.  I think if we can trust the GENERIC input then we'll be fine.
Of course as we see here, the GENERIC input from frontends can be
garbage (which IMHO as said is a bug in the frontends).  We
shouldn't worry too much to be precise when trying to fend off
garbage input, we just have to make sure to make the output not
even worse.

Richard.

[PATCH] tree-optimization/105163 - abnormal SSA coalescing and reassoc

[Richard Biener via Gcc-patches]

The negate propagation optimizations in reassoc did not look out for
abnormal SSA coalescing issues.  The following fixes that.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2022-04-06  Richard Biener  

PR tree-optimization/105163
* tree-ssa-reassoc.cc (repropagate_negates): Avoid propagating
negated abnormals.

* gcc.dg/torture/pr105163.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr105163.c | 17 +
 gcc/tree-ssa-reassoc.cc | 25 +
 2 files changed, 30 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr105163.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr105163.c 
b/gcc/testsuite/gcc.dg/torture/pr105163.c
new file mode 100644
index 000..23e04107f68
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr105163.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target nonlocal_goto } */
+
+#include 
+
+extern int bar (unsigned int *);
+extern jmp_buf *baz (void);
+struct C { int c1; };
+void foo (struct C *x, int *z, int e)
+{
+  unsigned int d = 0;
+  long f;
+  setjmp (*baz());
+  f = 1 + ~d;
+  d = 8;
+  if ((!0) && !e && bar(z)) *z = 1 + f;
+}
diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
index 7ee50946556..0d55fc7e2d8 100644
--- a/gcc/tree-ssa-reassoc.cc
+++ b/gcc/tree-ssa-reassoc.cc
@@ -5970,10 +5970,14 @@ repropagate_negates (void)
   FOR_EACH_VEC_ELT (plus_negates, i, negate)
 {
   gimple *user = get_single_immediate_use (negate);
-
   if (!user || !is_gimple_assign (user))
continue;
 
+  tree negateop = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (negate));
+  if (TREE_CODE (negateop) == SSA_NAME
+ && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (negateop))
+   continue;
+
   /* The negate operand can be either operand of a PLUS_EXPR
 (it can be the LHS if the RHS is a constant for example).
 
@@ -5996,9 +6000,9 @@ repropagate_negates (void)
  if (gimple_assign_rhs2 (user) == negate)
{
  tree rhs1 = gimple_assign_rhs1 (user);
- tree rhs2 = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (negate));
  gimple_stmt_iterator gsi = gsi_for_stmt (user);
- gimple_assign_set_rhs_with_ops (&gsi, MINUS_EXPR, rhs1, rhs2);
+ gimple_assign_set_rhs_with_ops (&gsi, MINUS_EXPR, rhs1,
+ negateop);
  update_stmt (user);
}
}
@@ -6007,21 +6011,20 @@ repropagate_negates (void)
  if (gimple_assign_rhs1 (user) == negate)
{
  /* We have
-  x = -a
+  x = -negateop
   y = x - b
 which we transform into
-  x = a + b
+  x = negateop + b
   y = -x .
 This pushes down the negate which we possibly can merge
 into some other operation, hence insert it into the
 plus_negates vector.  */
  gimple *feed = SSA_NAME_DEF_STMT (negate);
- tree a = gimple_assign_rhs1 (feed);
  tree b = gimple_assign_rhs2 (user);
  gimple_stmt_iterator gsi = gsi_for_stmt (feed);
  gimple_stmt_iterator gsi2 = gsi_for_stmt (user);
  tree x = make_ssa_name (TREE_TYPE (gimple_assign_lhs (feed)));
- gimple *g = gimple_build_assign (x, PLUS_EXPR, a, b);
+ gimple *g = gimple_build_assign (x, PLUS_EXPR, negateop, b);
  gsi_insert_before (&gsi2, g, GSI_SAME_STMT);
  gimple_assign_set_rhs_with_ops (&gsi2, NEGATE_EXPR, x);
  user = gsi_stmt (gsi2);
@@ -6032,13 +6035,11 @@ repropagate_negates (void)
}
  else
{
- /* Transform "x = -a; y = b - x" into "y = b + a", getting
-rid of one operation.  */
- gimple *feed = SSA_NAME_DEF_STMT (negate);
- tree a = gimple_assign_rhs1 (feed);
+ /* Transform "x = -negateop; y = b - x" into "y = b + negateop",
+getting rid of one operation.  */
  tree rhs1 = gimple_assign_rhs1 (user);
  gimple_stmt_iterator gsi = gsi_for_stmt (user);
- gimple_assign_set_rhs_with_ops (&gsi, PLUS_EXPR, rhs1, a);
+ gimple_assign_set_rhs_with_ops (&gsi, PLUS_EXPR, rhs1, negateop);
  update_stmt (gsi_stmt (gsi));
}
}
-- 
2.34.1

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Sandiford via Gcc-patches]

Richard Biener via Gcc-patches  writes:
> On Wed, 6 Apr 2022, Jakub Jelinek wrote:
>
>> On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote:
>> > On Tue, 5 Apr 2022, Jakub Jelinek wrote:
>> > 
>> > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote:
>> > > > > In GIMPLE, we call:
>> > > > >   && gimple_builtin_call_types_compatible_p (stmt, fndecl)
>> > > > > but that is insufficient, that verifies whether the arguments passed 
>> > > > > to
>> > > > > GIMPLE_CALL match the fndecl argument types.  But that fndecl may 
>> > > > > very well
>> > > > > be the user declaration, so doesn't have to match exactly the builtin
>> > > > > as predeclared by builtins.def etc. (though, there is the cotcha 
>> > > > > that say
>> > > > > for FILE * we just use void * etc.).  So e.g. in tree-ssa-strlen.cc
>> > > > > we use additional:
>> > > > >   tree callee = gimple_call_fndecl (stmt);
>> > > > >   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee));
>> > > > >   if (decl
>> > > > >   && decl != callee
>> > > > >   && !gimple_builtin_call_types_compatible_p (stmt, decl))
>> > > > > return false;
>> > > > 
>> > > > Yeah, I think we should use that (and only that) function decl
>> > > > in get_call_combined_fn and gimple_call_combined_fn until the
>> > > > frontend stops slapping wrong BUILT_IN_* on random decls.
>> > > 
>> > > So, as a preparation step, this patch adjusts gimple_call_builtin_p
>> > > and gimple_call_combined_fn so that they check argument types against
>> > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the
>> > > actual used fndecl.
>> > > 
>> > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
>> > 
>> > You forgot to attach the patch ...
>> 
>> Oops, here it is.  Also bootstrapped/regtested on {x86_64,i686}-linux.
>
> OK.

But it seems like the magic incantation to detect “real” built-in
function calls is getting longer and longer.  Can we not abstract this
in a single place rather than have to repeat the same long sequence in
multiple places?

Thanks,
Richard

>
> Thanks,
> Richard.
>
>> 2022-04-06  Jakub Jelinek  
>> 
>>  PR tree-optimization/105150
>>  * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn):
>>  For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p
>>  preferrably on builtin_decl_explicit decl rather than fndecl.
>>  * tree-ssa-strlen.cc (valid_builtin_call): Don't call
>>  gimple_builtin_call_types_compatible_p here.
>> 
>> --- gcc/gimple.cc.jj 2022-02-09 15:15:59.436837973 +0100
>> +++ gcc/gimple.cc2022-04-05 13:04:58.405586995 +0200
>> @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm
>>if (is_gimple_call (stmt)
>>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>&& DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
>> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +{
>> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> +if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +  fndecl = decl;
>> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +}
>>return false;
>>  }
>>  
>> @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm
>>if (is_gimple_call (stmt)
>>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>&& DECL_BUILT_IN_CLASS (fndecl) == klass)
>> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +{
>> +  if (klass == BUILT_IN_NORMAL)
>> +if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +  fndecl = decl;
>> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +}
>>return false;
>>  }
>>  
>> @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm
>>if (is_gimple_call (stmt)
>>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>>&& fndecl_built_in_p (fndecl, code))
>> -return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +{
>> +  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> +fndecl = decl;
>> +  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>> +}
>>return false;
>>  }
>>  
>> @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s
>>  return as_combined_fn (gimple_call_internal_fn (call));
>>  
>>tree fndecl = gimple_call_fndecl (stmt);
>> -  if (fndecl
>> -  && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
>> -  && gimple_builtin_call_types_compatible_p (stmt, fndecl))
>> -return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>> +  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
>> +{
>> +  tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
>> +  if (!decl)
>> +decl = fndecl;
>> +  if (gimple_builtin_call_types_compatible_p (stmt, decl))
>> +return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>> +  

[PATCH] combine, v2: Don't record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]

[Jakub Jelinek via Gcc-patches]

On Tue, Apr 05, 2022 at 04:56:55PM -0500, Segher Boessenkool wrote:
> > Or there are variant patches in the PR, either a minimal version of
> > this patch, or one that records regno and adjusts all SUBST_MODE uses.
> 
> I like this one best I think :-)
> 
> > Also, not sure I like very much a function name in all caps, but I wanted
> > to preserve the users and all other SUBST and SUBST_* are also all caps.
> > Yet another possibility would be keep do_SUBST_MODE with the new
> > arguments and
> > #define SUBST_MODE(INTO, NEWVAL)  do_SUBST_MODE ((INTO), (NEWVAL))
> 
> Like the other things in combine.c do, so please change to that?  Which
> means changing less than you do now :-)
> 
> Changing this all to not have macros is better of course, and can be
> tastefully done even with C++ (it would use C++ features even :-) ), but
> let's do that all at once, and in stage 1.
> 
> > -  union { rtx *r; int *i; struct insn_link **l; } where;
> > +  union { rtx *r; int *i; rtx m; struct insn_link **l; } where;
> 
> NAK.  It is not clear at all what "rtx m" means, esp. since there is an
> "rtx *r" already.  In the PR you said "machine_mode m", that is clear of
> course, can you do that instead?

So in that case something like this (i.e. the regno variant, renamed
to subst_mode from SUBST_MODE, and naming the union member regno rather
than m)?

And for GCC 13 change do_SUBST_INT to subst_int with int &into arg,
do_SUBST_LINK to subst_link with struct insn_link *&into arg,
and do_SUBST into subst with rtx &into arg?

2022-04-06  Jakub Jelinek  

PR rtl-optimization/104985
* combine.cc (struct undo): Add where.regno member.
(do_SUBST_MODE): Rename to ...
(subst_mode): ... this.  Change first argument from rtx * into int,
operate on regno_reg_rtx[regno] and save regno into where.regno.
(SUBST_MODE): Remove.
(try_combine): Use subst_mode instead of SUBST_MODE, change first
argument from regno_reg_rtx[whatever] to whatever.  For UNDO_MODE, use
regno_reg_rtx[undo->where.regno] instead of *undo->where.r.
(undo_to_marker): For UNDO_MODE, use regno_reg_rtx[undo->where.regno]
instead of *undo->where.r.
(simplify_set): Use subst_mode instead of SUBST_MODE, change first
argument from regno_reg_rtx[whatever] to whatever.

--- gcc/combine.cc.jj   2022-03-30 09:11:42.331261823 +0200
+++ gcc/combine.cc  2022-04-06 10:29:55.333106447 +0200
@@ -382,7 +382,7 @@ struct undo
   struct undo *next;
   enum undo_kind kind;
   union { rtx r; int i; machine_mode m; struct insn_link *l; } old_contents;
-  union { rtx *r; int *i; struct insn_link **l; } where;
+  union { rtx *r; int *i; int regno; struct insn_link **l; } where;
 };
 
 /* Record a bunch of changes to be undone, up to MAX_UNDO of them.
@@ -761,10 +761,11 @@ do_SUBST_INT (int *into, int newval)
well.  */
 
 static void
-do_SUBST_MODE (rtx *into, machine_mode newval)
+subst_mode (int regno, machine_mode newval)
 {
   struct undo *buf;
-  machine_mode oldval = GET_MODE (*into);
+  rtx reg = regno_reg_rtx[regno];
+  machine_mode oldval = GET_MODE (reg);
 
   if (oldval == newval)
 return;
@@ -775,15 +776,13 @@ do_SUBST_MODE (rtx *into, machine_mode n
 buf = XNEW (struct undo);
 
   buf->kind = UNDO_MODE;
-  buf->where.r = into;
+  buf->where.regno = regno;
   buf->old_contents.m = oldval;
-  adjust_reg_mode (*into, newval);
+  adjust_reg_mode (reg, newval);
 
   buf->next = undobuf.undos, undobuf.undos = buf;
 }
 
-#define SUBST_MODE(INTO, NEWVAL)  do_SUBST_MODE (&(INTO), (NEWVAL))
-
 /* Similar to SUBST, but NEWVAL is a LOG_LINKS expression.  */
 
 static void
@@ -3186,7 +3185,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
newpat_dest = gen_rtx_REG (compare_mode, regno);
  else
{
- SUBST_MODE (regno_reg_rtx[regno], compare_mode);
+ subst_mode (regno, compare_mode);
  newpat_dest = regno_reg_rtx[regno];
}
}
@@ -3576,7 +3575,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
ni2dest = gen_rtx_REG (new_mode, REGNO (i2dest));
  else
{
- SUBST_MODE (regno_reg_rtx[REGNO (i2dest)], new_mode);
+ subst_mode (REGNO (i2dest), new_mode);
  ni2dest = regno_reg_rtx[REGNO (i2dest)];
}
 
@@ -3712,7 +3711,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
newdest = gen_rtx_REG (split_mode, REGNO (i2dest));
  else
{
- SUBST_MODE (regno_reg_rtx[REGNO (i2dest)], split_mode);
+ subst_mode (REGNO (i2dest), split_mode);
  newdest = regno_reg_rtx[REGNO (i2dest)];
}
}
@@ -4082,7 +4081,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
   for (undo = undobuf.undos; undo; undo = undo->next)
if (undo->kind == UNDO_MODE)

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
> But it seems like the magic incantation to detect “real” built-in
> function calls is getting longer and longer.  Can we not abstract this
> in a single place rather than have to repeat the same long sequence in
> multiple places?

I've already committed it, so it can be only dealt with an incremental
patch.
One possibility is to do it inside of
gimple_builtin_call_types_compatible_p, after the assert do that:
  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
  fndecl = decl;
but we then lose the theoretical possibility of comparing against the
actual user declaration.  Though I guess in the
gimple-fold.cc
gimple-low.cc
gimple-match-head.cc
calls to that function we also want this rather than what they do currently.

Jakub

libgomp testsuite: Don't amend 'LD_LIBRARY_PATH' for system-provided HSA Runtime library (was: [PATCH 1/4] Remove build dependence on HSA run-time)

[Thomas Schwinge]

Hi!

On 2021-01-14T15:50:23+0100, I wrote:
> I'm raising here an issue with HSA libgomp plugin code changes from a
> while ago.  While HSA is now no longer relevant for GCC master branch,
> the same code has also been copied into the GCN libgomp plugin.

Here is another small clean-up patch (to enable further clean-up):

> This is commit b8d89b03db5f212919e4571671ebb4f5f8b1e19d (r242749) "Remove
> build dependence on HSA run-time":
>
> On 2016-11-22T14:27:44+0100, Martin Jambor  wrote:
>> --- a/libgomp/plugin/configfrag.ac
>> +++ b/libgomp/plugin/configfrag.ac
>
>> @@ -195,8 +183,8 @@ if test x"$enable_offload_targets" != x; then
>>  tgt_name=hsa
>>  PLUGIN_HSA=$tgt
>>  PLUGIN_HSA_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
>> -PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS $HSA_KMT_LDFLAGS"
>> -PLUGIN_HSA_LIBS="-lhsa-runtime64 -lhsakmt"
>> +PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS"
>> +PLUGIN_HSA_LIBS="-ldl"
>
> So this switched from directly linking against 'libhsa-runtime64.so' to a
> 'libdl'-based runtime linking variant.

(Not intending to change anything regarding that.)

> For avoidance of doubt, [an earlier] change doesn't affect (build-tree) 
> testsuite
> usage, where we have:
>
> libgomp/testsuite/libgomp-test-support.exp.in:set hsa_runtime_lib 
> "@HSA_RUNTIME_LIB@"
>
> libgomp/testsuite/lib/libgomp.exp:  append always_ld_library_path 
> ":$hsa_runtime_lib"

But, as I argue in the attached "libgomp testsuite: Don't amend
'LD_LIBRARY_PATH' for system-provided HSA Runtime library", we should
actually clean this up as well.  OK to push that?


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 364d01339883f5276ef09d68a5d9a2e0010ab641 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 6 Apr 2022 10:39:56 +0200
Subject: [PATCH] libgomp testsuite: Don't amend 'LD_LIBRARY_PATH' for
 system-provided HSA Runtime library

This is only active if GCC is 'configure'd with '--with-hsa-runtime=[...]' or
'--with-hsa-runtime-lib=[...]' -- which nobody really is doing, as far as I can
tell.

'libgomp/testsuite/lib/libgomp.exp:libgomp_init' states:

# For build-tree testing, also consider the library paths used for builing.
# For installed testing, we assume all that to be provided in the sysroot.
if { $blddir != "" } {
[...]
global hsa_runtime_lib
if { $hsa_runtime_lib != "" } {
append always_ld_library_path ":$hsa_runtime_lib"
}
}

However, the libgomp GCN plugin is unconditionally built against the
GCC-shipped 'include/hsa*.h' header files, and at run time does
'dlopen("libhsa-runtime64.so.1")', so there is no system-provided HSA Runtime
library "used for builing".  It thus doesn't make sense to amend
'LD_LIBRARY_PATH' for system-provided HSA Runtime library.

	libgomp/
	* testsuite/lib/libgomp.exp (libgomp_init): Don't
	'append always_ld_library_path ":$hsa_runtime_lib"'.
	* testsuite/libgomp-test-support.exp.in (hsa_runtime_lib): Don't set.
---
 libgomp/testsuite/lib/libgomp.exp | 4 
 libgomp/testsuite/libgomp-test-support.exp.in | 1 -
 2 files changed, 5 deletions(-)

diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
index 8c5ecfff0ac..0aaa58f19c5 100644
--- a/libgomp/testsuite/lib/libgomp.exp
+++ b/libgomp/testsuite/lib/libgomp.exp
@@ -202,10 +202,6 @@ proc libgomp_init { args } {
 	lappend ALWAYS_CFLAGS "additional_flags=-L$cuda_driver_lib"
 	append always_ld_library_path ":$cuda_driver_lib"
 	}
-	global hsa_runtime_lib
-	if { $hsa_runtime_lib != "" } {
-	append always_ld_library_path ":$hsa_runtime_lib"
-	}
 }
 
 # We use atomic operations in the testcases to validate results.
diff --git a/libgomp/testsuite/libgomp-test-support.exp.in b/libgomp/testsuite/libgomp-test-support.exp.in
index 98fb442b537..3c88d1d5a62 100644
--- a/libgomp/testsuite/libgomp-test-support.exp.in
+++ b/libgomp/testsuite/libgomp-test-support.exp.in
@@ -1,6 +1,5 @@
 set cuda_driver_include "@CUDA_DRIVER_INCLUDE@"
 set cuda_driver_lib "@CUDA_DRIVER_LIB@"
-set hsa_runtime_lib "@HSA_RUNTIME_LIB@"
 
 set offload_plugins "@offload_plugins@"
 set offload_targets "@offload_targets@"
-- 
2.35.1

[PATCH] tree-optimization/105175 - avoid -Wvector-operation-performance

[Richard Biener via Gcc-patches]

This avoids -Wvector-operation-performance diagnostics for vectorizer
produced code.  It's unfortunate the warning_at code in
tree-vect-generic.cc needs adjustments but the diagnostic suppression
code doesn't magically suppress those otherwise.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Martin/David - did I miss something obvious when doing the
tree-vect-generic.cc adjustment?

Thanks,
Richard.

2022-04-06  Richard Biener  

PR tree-optimization/105175
* tree-vect-stmts.cc (vectorizable_operation): Suppress
-Wvector-operation-performance if using emulated vectors.
* tree-vect-generic.cc (expand_vector_piecewise): Do not diagnose
-Wvector-operation-performance when suppressed.
(expand_vector_parallel): Likewise.
(expand_vector_comparison): Likewise.
(expand_vector_condition): Likewise.
(lower_vec_perm): Likewise.
(expand_vector_conversion): Likewise.

* gcc.dg/pr105175.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr105175.c | 16 +
 gcc/tree-vect-generic.cc| 41 ++---
 gcc/tree-vect-stmts.cc  |  2 ++
 3 files changed, 45 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr105175.c

diff --git a/gcc/testsuite/gcc.dg/pr105175.c b/gcc/testsuite/gcc.dg/pr105175.c
new file mode 100644
index 000..d8d7edb942a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105175.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wvector-operation-performance" } */
+/* { dg-additional-options "-mno-sse" { target x86_64-*-* i?86-*-* } } */
+
+enum { QEMU_MIGRATION_COOKIE_PERSISTENT = 1 };
+struct {
+  unsigned flags;
+  unsigned flagsMandatory;
+} qemuMigrationCookieGetPersistent_mig;
+void qemuMigrationCookieGetPersistent()
+{
+  qemuMigrationCookieGetPersistent_mig.flags &=  /* { dg-bogus "will be 
expanded" } */
+  QEMU_MIGRATION_COOKIE_PERSISTENT;
+  qemuMigrationCookieGetPersistent_mig.flagsMandatory &=
+  QEMU_MIGRATION_COOKIE_PERSISTENT;
+}
diff --git a/gcc/tree-vect-generic.cc b/gcc/tree-vect-generic.cc
index 12a553ec8be..8b7227e8b58 100644
--- a/gcc/tree-vect-generic.cc
+++ b/gcc/tree-vect-generic.cc
@@ -317,8 +317,11 @@ expand_vector_piecewise (gimple_stmt_iterator *gsi, 
elem_op_func f,
   int i;
   location_t loc = gimple_location (gsi_stmt (*gsi));
 
-  if (nunits == 1)
-/* Do not diagnose decomposing single element vectors.  */
+  if (nunits == 1
+  || warning_suppressed_p (gsi_stmt (*gsi),
+  OPT_Wvector_operation_performance))
+/* Do not diagnose decomposing single element vectors or when
+   decomposing vectorizer produced operations.  */
 ;
   else if (ret_type || !parallel_p)
 warning_at (loc, OPT_Wvector_operation_performance,
@@ -379,14 +382,16 @@ expand_vector_parallel (gimple_stmt_iterator *gsi, 
elem_op_func f, tree type,
   else
 {
   /* Use a single scalar operation with a mode no wider than word_mode.  */
+  if (!warning_suppressed_p (gsi_stmt (*gsi),
+OPT_Wvector_operation_performance))
+   warning_at (loc, OPT_Wvector_operation_performance,
+   "vector operation will be expanded with a "
+   "single scalar operation");
   scalar_int_mode mode
= int_mode_for_size (tree_to_uhwi (TYPE_SIZE (type)), 0).require ();
   compute_type = lang_hooks.types.type_for_mode (mode, 1);
   result = f (gsi, compute_type, a, b, bitsize_zero_node,
  TYPE_SIZE (compute_type), code, type);
-  warning_at (loc, OPT_Wvector_operation_performance,
- "vector operation will be expanded with a "
- "single scalar operation");
 }
 
   return result;
@@ -487,8 +492,10 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, tree 
type, tree op0,
 
  if (TYPE_PRECISION (ret_inner_type) != 1)
ret_inner_type = build_nonstandard_integer_type (1, 1);
- warning_at (loc, OPT_Wvector_operation_performance,
- "vector operation will be expanded piecewise");
+ if (!warning_suppressed_p (gsi_stmt (*gsi),
+OPT_Wvector_operation_performance))
+   warning_at (loc, OPT_Wvector_operation_performance,
+   "vector operation will be expanded piecewise");
  for (i = 0; i < nunits;
   i++, index = int_const_binop (PLUS_EXPR, index, part_width))
{
@@ -1098,8 +1105,9 @@ expand_vector_condition (gimple_stmt_iterator *gsi, 
bitmap dce_ssa_names)
 
   /* TODO: try and find a smaller vector type.  */
 
-  warning_at (loc, OPT_Wvector_operation_performance,
- "vector condition will be expanded piecewise");
+  if (!warning_suppressed_p (stmt, OPT_Wvector_operation_performance))
+warning_at (loc, OPT_Wvector_operation_performance,
+   "vector condition will be expanded piecewise");
 
 

Re: libgomp testsuite: Don't amend 'LD_LIBRARY_PATH' for system-provided HSA Runtime library (was: [PATCH 1/4] Remove build dependence on HSA run-time)

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 11:20:47AM +0200, Thomas Schwinge wrote:
> However, the libgomp GCN plugin is unconditionally built against the
> GCC-shipped 'include/hsa*.h' header files, and at run time does
> 'dlopen("libhsa-runtime64.so.1")', so there is no system-provided HSA Runtime
> library "used for builing".  It thus doesn't make sense to amend
> 'LD_LIBRARY_PATH' for system-provided HSA Runtime library.

But perhaps having some other hsa_runtime_lib path in LD_LIBRARY_PATH
allows that dlopen to succeed if libhsa-runtime64.so.1 isn't installed
in the standard searched directories?

Jakub

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
> > But it seems like the magic incantation to detect “real” built-in
> > function calls is getting longer and longer.  Can we not abstract this
> > in a single place rather than have to repeat the same long sequence in
> > multiple places?
> 
> I've already committed it, so it can be only dealt with an incremental
> patch.
> One possibility is to do it inside of
> gimple_builtin_call_types_compatible_p, after the assert do that:
>   if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>   fndecl = decl;
> but we then lose the theoretical possibility of comparing against the
> actual user declaration.  Though I guess in the
> gimple-fold.cc
> gimple-low.cc
> gimple-match-head.cc
> calls to that function we also want this rather than what they do currently.

Yes, I think it would be clearer to pass a BUILT_IN_* code to
gimple_builtin_call_types_compatible_p and no decl and simply return
false if we cannot get out hands at the "proper" decl from
builtin_decl_explicit ...

Richard.

Re: libgomp testsuite: Don't amend 'LD_LIBRARY_PATH' for system-provided HSA Runtime library

[Thomas Schwinge]

Hi Jakub!

On 2022-04-06T11:24:17+0200, Jakub Jelinek  wrote:
> On Wed, Apr 06, 2022 at 11:20:47AM +0200, Thomas Schwinge wrote:
>> However, the libgomp GCN plugin is unconditionally built against the
>> GCC-shipped 'include/hsa*.h' header files, and at run time does
>> 'dlopen("libhsa-runtime64.so.1")', so there is no system-provided HSA Runtime
>> library "used for builing".  It thus doesn't make sense to amend
>> 'LD_LIBRARY_PATH' for system-provided HSA Runtime library.
>
> But perhaps having some other hsa_runtime_lib path in LD_LIBRARY_PATH
> allows that dlopen to succeed if libhsa-runtime64.so.1 isn't installed
> in the standard searched directories?

Yes, but that's then standard test harness set up (for example, set
'LD_LIBRARY_PATH' environment variable accordingly) for 'make check'.
In particular, that won't be different for build-tree vs. installed
testing, and shouldn't be done conditional to 'if { $blddir != "" }' in
'libgomp/testsuite/lib/libgomp.exp:libgomp_init'.


Grüße
 Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955

RE: [PATCH][GCC 9] arm: Fix ICEs with compare-and-swap and -march=armv8-m.base [PR99977]

[Kyrylo Tkachov via Gcc-patches]

> -Original Message-
> From: Alex Coplan 
> Sent: Thursday, March 31, 2022 5:20 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw ; Kyrylo Tkachov
> 
> Subject: [PATCH][GCC 9] arm: Fix ICEs with compare-and-swap and -
> march=armv8-m.base [PR99977]
> 
> Hi,
> 
> This is a backport of the fix for PR99977 to the GCC 9 branch. The only
> case where the GCC 10 patch did not apply cleanly was on sync.md, where
> some of the context has changed, but the substance of the patch has not
> changed, it simply required applying by hand.
> 
> Tested as follows:
>  - Bootstrap/regtest on arm-linux-gnueabihf.
>  - Regression tested a cross compiler configured with
>--with-arch=armv8-m.base.
> 
> OK for the GCC 9 branch?
> 

Ok.
Thanks,
Kyrill

> Thanks,
> Alex
> 
> ---
> 
> The PR shows two ICEs with __sync_bool_compare_and_swap and
> -mcpu=cortex-m23 (equivalently, -march=armv8-m.base): one in LRA and
> one
> later on, after the CAS insn is split.
> 
> The LRA ICE occurs because the
> @atomic_compare_and_swap_1 pattern attempts
> to tie
> two output operands together (operands 0 and 1 in the third
> alternative). LRA can't handle this, since it doesn't make sense for an
> insn to assign to the same operand twice.
> 
> The later (post-splitting) ICE occurs because the expansion of the
> cbranchsi4_scratch insn doesn't quite go according to plan. As it
> stands, arm_split_compare_and_swap calls gen_cbranchsi4_scratch,
> attempting to pass a register (neg_bval) to use as a scratch register.
> However, since the RTL template has a match_scratch here,
> gen_cbranchsi4_scratch ignores this argument and produces a scratch rtx.
> Since this is all happening after RA, this is doomed to fail (and we get
> an ICE about the insn not matching its constraints).
> 
> It seems that the motivation for the choice of constraints in the
> atomic_compare_and_swap pattern comes from an attempt to satisfy the
> constraints of the cbranchsi4_scratch insn. This insn requires the
> scratch register to be the same as the input register in the case that
> we use a larger negative immediate (one that satisfies J, but not L).
> 
> Of course, as noted above, LRA refuses to assign two output operands to
> the same register, so this was never going to work.
> 
> The solution I'm proposing here is to collapse the alternatives to the
> CAS insn (allowing the two output register operands to be matched to
> different registers) and to ensure that the constraints for
> cbranchsi4_scratch are met in arm_split_compare_and_swap. We do this by
> inserting a move to ensure the source and destination registers match if
> necessary (i.e. in the case of large negative immediates).
> 
> Another notable change here is that we only do:
> 
>   emit_move_insn (neg_bval, const1_rtx);
> 
> for non-negative immediates. This is because the ADDS instruction used in
> the negative case suffices to leave a suitable value in neg_bval: if the
> operands compare equal, we don't take the branch (so neg_bval will be
> set by the load exclusive). Otherwise, the ADDS will leave a nonzero
> value in neg_bval, which will correctly signal that the CAS has failed
> when it is later negated.
> 
> gcc/ChangeLog:
> 
> PR target/99977
> * config/arm/arm.c (arm_split_compare_and_swap): Fix up codegen
> with negative immediates: ensure we expand cbranchsi4_scratch
> correctly and ensure we satisfy its constraints.
> * config/arm/sync.md
> (@atomic_compare_and_swap_1): Don't
> attempt to tie two output operands together with constraints;
> collapse two alternatives.
> (@atomic_compare_and_swap_1): Likewise.
> * config/arm/thumb1.md (cbranchsi4_neg_late): New.
> 
> gcc/testsuite/ChangeLog:
> 
> PR target/99977
> * gcc.target/arm/pr99977.c: New test.

Re: Proposal to remove '--with-cuda-driver' (was: [wwwdocs][patch] gcc-12: Nvptx updates)

[Tom de Vries via Gcc-patches]

On 4/5/22 17:14, Thomas Schwinge wrote:

Hi!

Still catching up with GCC/nvptx back end changes...  %-)


In the following I'm not discussing the patch to document
"gcc-12: Nvptx updates", but rather one aspect of the
"gcc-12: Nvptx updates" themselves.  ;-)

On 2022-03-30T14:27:41+0200, Tom de Vries  wrote:

+  The -march flag has been added.  The -misa
+flag is now considered an alias of the -march flag.
+  Support for PTX ISA target architectures sm_53,
+sm_70, sm_75 and sm_80 has been
+added.  These can be specified using the -march flag.
+  The default PTX ISA target architecture has been set back
+to sm_30, to fix support for sm_30 boards.
+  The -march-map flag has been added.  The
+-march-map value will be mapped to an valid
+-march flag value.  For instance,
+-march-map=sm_50 maps to -march=sm_35.
+This can be used to specify that generated code is to be executed on a
+board with at least some specific compute capability, without having to
+know the valid values for the -march flag.


Regarding the following:


The -mptx flag has been added to specify the PTX ISA 
version
for the generated code; permitted values are 3.1
-  (default, matches previous GCC versions) and 6.3.
+  (matches previous GCC versions), 6.0, 6.3,
+  and 7.0. If not specified, the used version is the minimal
+  version required for -march but at least 6.0.



For "the PTX ISA version [used is] at least '6.0'", per
,
this means we now require "CUDA 9.0, driver r384" (or more recent).


Well, that would be the case if there was no -mptx=3.1.


Per :
"CUDA Toolkit 9.0 (Sept 2017)", so ~4.5 years old.
Per , I'm guessing a


I just see a list with version numbers there, I'm not sure what 
information you're referring to.



similar timeframe for the imprecise "r384" Driver version stated in that
table.  That should all be fine (re not mandating use of all-too-recent
versions).



I don't know what an imprecise driver is.


Now, consider doing a GCC/nvptx offloading build with
'--with-cuda-driver' pointing to CUDA 9.0 (or more recent).  This means
that the libgomp nvptx plugin may now use CUDA Driver features of the
CUDA 9.0 distribution ("driver r384", etc.) -- because that's what it is
being 'configure'd and linked against.  (I say "may now use", because
we're currently not making a lot of effort to use "modern" CUDA Driver
features -- but we could, and probably should.  That's a separate
discussion, of course.)  It then follows that the libgomp nvptx plugin
has a hard dependency on CUDA Driver features of the CUDA 9.0
distribution ("driver r384", etc.).  That's dependency as in ABI: via
'*.so' symbol versions as well as internal CUDA interface configuration;
see  doing different '#define's for different
'__CUDA_API_VERSION' etc.)

Now assume one such dependency on "modern" CUDA Driver were not
implemented by:



Thanks for reminding me, I forgot about this configure option.


+  An mptx-3.1 multilib was added.  This allows using older
+  drivers which do not support PTX ISA version 6.0.


... this "old" CUDA Driver.  Then you do have the '-mptx-3.1' multilib to
use with "old" CUDA Driver -- but you cannot actually use the libgomp
nvptx plugin, because that's been built against "modern" CUDA Driver.



I remember the following problem: using -with-cuda-driver to specify 
what cuda driver interface (version) you want to link the libgomp plugin 
against, and then using an older driver in combination with that libgomp 
plugin.   We may run into trouble, typically at libgomp plugin load 
time, with an error mentioning an unresolved symbol or some abi symbol 
version being not sufficient.


So, do I understand it correctly that your point is that using -mptx=3.1 
doesn't fix that problem?



Same problem, generally, for 'nvptx-run' of the nvptx-tools, which has
similar CUDA Driver dependencies.

Now, that may currently be a latent problem only, because we're not
actually making use of "modern" CUDA Driver features.  But, I'd like to
resolve this "impedance mismatch", before we actually run into such
problems.



It would be helpful for me if you would come up with an example of a 
modification to the libgomp plugin that would cause trouble in 
combination with mptx=3.1.



Already long ago Jakub put in changes to use '--without-cuda-driver' to
"Allow building GCC with PTX offloading even without CUDA being installed
(gcc and nvptx-tools patches)": "Especially for distributions it is
undesirable to need to have proprietary CUDA libraries and headers
installed when building GCC.", and I understand GNU/Linux distributions
all use that.  That configuration uses the GCC-provided
'libgomp/plugin/cuda/cuda.h', 'libgomp/plugin/cuda-lib.def' to manually
define the CUDA Driver ABI to use, and then 'dl

libgomp GCN plugin: Clean up unused references to system-provided HSA Runtime library (was: [PATCH 1/4] Remove build dependence on HSA run-time)

[Thomas Schwinge]

Hi!

On 2021-01-14T15:50:23+0100, I wrote:
> I'm raising here an issue with HSA libgomp plugin code changes from a
> while ago.  While HSA is now no longer relevant for GCC master branch,
> the same code has also been copied into the GCN libgomp plugin.

Here is another small clean-up patch (to enable further clean-up):

> This is commit b8d89b03db5f212919e4571671ebb4f5f8b1e19d (r242749) "Remove
> build dependence on HSA run-time":
>
> On 2016-11-22T14:27:44+0100, Martin Jambor  wrote:
>> --- a/libgomp/plugin/configfrag.ac
>> +++ b/libgomp/plugin/configfrag.ac
>
>> @@ -195,8 +183,8 @@ if test x"$enable_offload_targets" != x; then
>>  tgt_name=hsa
>>  PLUGIN_HSA=$tgt
>>  PLUGIN_HSA_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
>> -PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS $HSA_KMT_LDFLAGS"
>> -PLUGIN_HSA_LIBS="-lhsa-runtime64 -lhsakmt"
>> +PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS"
>> +PLUGIN_HSA_LIBS="-ldl"
>
> So this switched from directly linking against 'libhsa-runtime64.so' to a
> 'libdl'-based runtime linking variant.

(Not intending to change anything regarding that.)

Given the 'PLUGIN_HSA_LIBS' change cited above, OK to push the attached
"libgomp GCN plugin: Clean up unused references to system-provided HSA
Runtime library"?


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From 5e28a267a34282e4d6001e5f89a3b7bd7a0f20c7 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 6 Apr 2022 11:31:45 +0200
Subject: [PATCH] libgomp GCN plugin: Clean up unused references to
 system-provided HSA Runtime library

This is only active if GCC is 'configure'd with '--with-hsa-runtime=[...]' or
'--with-hsa-runtime-include=[...]', '--with-hsa-runtime-lib=[...]' -- which
nobody really is doing, as far as I can tell.

Originally changed for the libgomp HSA plugin in
commit b8d89b03db5f212919e4571671ebb4f5f8b1e19d (r242749)
"Remove build dependence on HSA run-time", and later propagated into the GCN
plugin, these are no longer built against system-provided HSA Runtime library.
Instead, unconditionally built against the GCC-shipped 'include/hsa*.h' header
files, and at run time does 'dlopen("libhsa-runtime64.so.1")'.  It thus doesn't
make sense to consider references to system-provided HSA Runtime library during
libgomp GCN plugin build.

	libgomp/
	* plugin/configfrag.ac (HSA_RUNTIME_CPPFLAGS)
	(HSA_RUNTIME_LDFLAGS): Remove.
	* configure: Regenerate.
---
 libgomp/configure| 10 --
 libgomp/plugin/configfrag.ac | 10 --
 2 files changed, 20 deletions(-)

diff --git a/libgomp/configure b/libgomp/configure
index a73a6d44003..8bb67c650a6 100755
--- a/libgomp/configure
+++ b/libgomp/configure
@@ -15244,8 +15244,6 @@ HSA_RUNTIME_INCLUDE=
 HSA_RUNTIME_LIB=
 
 
-HSA_RUNTIME_CPPFLAGS=
-HSA_RUNTIME_LDFLAGS=
 
 
 # Check whether --with-hsa-runtime was given.
@@ -15275,12 +15273,6 @@ fi
 if test "x$with_hsa_runtime_lib" != x; then
   HSA_RUNTIME_LIB=$with_hsa_runtime_lib
 fi
-if test "x$HSA_RUNTIME_INCLUDE" != x; then
-  HSA_RUNTIME_CPPFLAGS=-I$HSA_RUNTIME_INCLUDE
-fi
-if test "x$HSA_RUNTIME_LIB" != x; then
-  HSA_RUNTIME_LDFLAGS=-L$HSA_RUNTIME_LIB
-fi
 
 PLUGIN_GCN=0
 PLUGIN_GCN_CPPFLAGS=
@@ -15390,8 +15382,6 @@ rm -f core conftest.err conftest.$ac_objext \
 	  *)
 		tgt_plugin=gcn
 		PLUGIN_GCN=$tgt
-		PLUGIN_GCN_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
-		PLUGIN_GCN_LDFLAGS="$HSA_RUNTIME_LDFLAGS"
 		PLUGIN_GCN_LIBS="-ldl"
 		PLUGIN_GCN=1
 		;;
diff --git a/libgomp/plugin/configfrag.ac b/libgomp/plugin/configfrag.ac
index da573bd8387..56461b89117 100644
--- a/libgomp/plugin/configfrag.ac
+++ b/libgomp/plugin/configfrag.ac
@@ -97,8 +97,6 @@ HSA_RUNTIME_INCLUDE=
 HSA_RUNTIME_LIB=
 AC_SUBST(HSA_RUNTIME_INCLUDE)
 AC_SUBST(HSA_RUNTIME_LIB)
-HSA_RUNTIME_CPPFLAGS=
-HSA_RUNTIME_LDFLAGS=
 
 AC_ARG_WITH(hsa-runtime,
 	[AS_HELP_STRING([--with-hsa-runtime=PATH],
@@ -121,12 +119,6 @@ fi
 if test "x$with_hsa_runtime_lib" != x; then
   HSA_RUNTIME_LIB=$with_hsa_runtime_lib
 fi
-if test "x$HSA_RUNTIME_INCLUDE" != x; then
-  HSA_RUNTIME_CPPFLAGS=-I$HSA_RUNTIME_INCLUDE
-fi
-if test "x$HSA_RUNTIME_LIB" != x; then
-  HSA_RUNTIME_LDFLAGS=-L$HSA_RUNTIME_LIB
-fi
 
 PLUGIN_GCN=0
 PLUGIN_GCN_CPPFLAGS=
@@ -225,8 +217,6 @@ if test x"$enable_offload_targets" != x; then
 	  *)
 		tgt_plugin=gcn
 		PLUGIN_GCN=$tgt
-		PLUGIN_GCN_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
-		PLUGIN_GCN_LDFLAGS="$HSA_RUNTIME_LDFLAGS"
 		PLUGIN_GCN_LIBS="-ldl"
 		PLUGIN_GCN=1
 		;;
-- 
2.25.1

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote:
> On Wed, 6 Apr 2022, Jakub Jelinek wrote:
> 
> > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
> > > But it seems like the magic incantation to detect “real” built-in
> > > function calls is getting longer and longer.  Can we not abstract this
> > > in a single place rather than have to repeat the same long sequence in
> > > multiple places?
> > 
> > I've already committed it, so it can be only dealt with an incremental
> > patch.
> > One possibility is to do it inside of
> > gimple_builtin_call_types_compatible_p, after the assert do that:
> >   if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> >   fndecl = decl;
> > but we then lose the theoretical possibility of comparing against the
> > actual user declaration.  Though I guess in the
> > gimple-fold.cc
> > gimple-low.cc
> > gimple-match-head.cc
> > calls to that function we also want this rather than what they do currently.
> 
> Yes, I think it would be clearer to pass a BUILT_IN_* code to
> gimple_builtin_call_types_compatible_p and no decl and simply return
> false if we cannot get out hands at the "proper" decl from
> builtin_decl_explicit ...

That would mean we wouldn't verify the md or FE builtins anymore
and we would need to check for BUILT_IN_NORMAL in every caller (right now
we do that only in some of them).

Here is what I had in mind (untested so far):

2022-04-06  Jakub Jelinek  

PR tree-optimization/105150
* gimple.cc (gimple_builtin_call_types_compatible_p): Use
builtin_decl_explicit here...
(gimple_call_builtin_p, gimple_call_combined_fn): ... rather than
here.

--- gcc/gimple.cc.jj2022-04-06 10:07:23.043064595 +0200
+++ gcc/gimple.cc   2022-04-06 11:31:31.704255242 +0200
@@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p (
 {
   gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
 
+  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+  fndecl = decl;
+
   tree ret = gimple_call_lhs (stmt);
   if (ret
   && !useless_type_conversion_p (TREE_TYPE (ret),
@@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
-{
-  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
-   if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
- fndecl = decl;
-  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
-}
+return gimple_builtin_call_types_compatible_p (stmt, fndecl);
   return false;
 }
 
@@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && DECL_BUILT_IN_CLASS (fndecl) == klass)
-{
-  if (klass == BUILT_IN_NORMAL)
-   if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
- fndecl = decl;
-  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
-}
+return gimple_builtin_call_types_compatible_p (stmt, fndecl);
   return false;
 }
 
@@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm
   if (is_gimple_call (stmt)
   && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
   && fndecl_built_in_p (fndecl, code))
-{
-  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
-   fndecl = decl;
-  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
-}
+return gimple_builtin_call_types_compatible_p (stmt, fndecl);
   return false;
 }
 
@@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s
return as_combined_fn (gimple_call_internal_fn (call));
 
   tree fndecl = gimple_call_fndecl (stmt);
-  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
-   {
- tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
- if (!decl)
-   decl = fndecl;
- if (gimple_builtin_call_types_compatible_p (stmt, decl))
-   return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
-   }
+  if (fndecl
+ && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
+ && gimple_builtin_call_types_compatible_p (stmt, fndecl))
+   return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
 }
   return CFN_LAST;
 }


Jakub

[PATCH] ipa/105166 - avoid modref queries with mismatching types

[Richard Biener via Gcc-patches]

We should avoid mismatched argument values (integers for pointers)
when doing modref queries.  This is the third place to guard.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2022-04-06  Richard Biener  

PR ipa/105166
* ipa-modref-tree.cc (modref_access_node::get_ao_ref ): Bail
out for non-pointer arguments.

* gcc.dg/torture/pr105166.c: New testcase.
---
 gcc/ipa-modref-tree.cc  | 4 +++-
 gcc/testsuite/gcc.dg/torture/pr105166.c | 9 +
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr105166.c

diff --git a/gcc/ipa-modref-tree.cc b/gcc/ipa-modref-tree.cc
index d0ec2fbf004..f19af8c2b55 100644
--- a/gcc/ipa-modref-tree.cc
+++ b/gcc/ipa-modref-tree.cc
@@ -678,7 +678,9 @@ modref_access_node::get_ao_ref (const gcall *stmt, ao_ref 
*ref) const
 {
   tree arg;
 
-  if (!parm_offset_known || !(arg = get_call_arg (stmt)))
+  if (!parm_offset_known
+  || !(arg = get_call_arg (stmt))
+  || !POINTER_TYPE_P (TREE_TYPE (arg)))
 return false;
   poly_offset_int off = (poly_offset_int)offset
+ ((poly_offset_int)parm_offset << LOG2_BITS_PER_UNIT);
diff --git a/gcc/testsuite/gcc.dg/torture/pr105166.c 
b/gcc/testsuite/gcc.dg/torture/pr105166.c
new file mode 100644
index 000..60e8b73a466
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr105166.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+
+int bar (foo, a)
+  int (**foo) ();
+  int a;
+{
+  (foo)[1] = bar;
+  foo[1] (1);
+}
-- 
2.34.1

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote:
> > On Wed, 6 Apr 2022, Jakub Jelinek wrote:
> > 
> > > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
> > > > But it seems like the magic incantation to detect “real” built-in
> > > > function calls is getting longer and longer.  Can we not abstract this
> > > > in a single place rather than have to repeat the same long sequence in
> > > > multiple places?
> > > 
> > > I've already committed it, so it can be only dealt with an incremental
> > > patch.
> > > One possibility is to do it inside of
> > > gimple_builtin_call_types_compatible_p, after the assert do that:
> > >   if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> > > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> > >   fndecl = decl;
> > > but we then lose the theoretical possibility of comparing against the
> > > actual user declaration.  Though I guess in the
> > > gimple-fold.cc
> > > gimple-low.cc
> > > gimple-match-head.cc
> > > calls to that function we also want this rather than what they do 
> > > currently.
> > 
> > Yes, I think it would be clearer to pass a BUILT_IN_* code to
> > gimple_builtin_call_types_compatible_p and no decl and simply return
> > false if we cannot get out hands at the "proper" decl from
> > builtin_decl_explicit ...
> 
> That would mean we wouldn't verify the md or FE builtins anymore
> and we would need to check for BUILT_IN_NORMAL in every caller (right now
> we do that only in some of them).
> 
> Here is what I had in mind (untested so far):

Yes, that works as well for me.

Richard.

> 2022-04-06  Jakub Jelinek  
> 
>   PR tree-optimization/105150
>   * gimple.cc (gimple_builtin_call_types_compatible_p): Use
>   builtin_decl_explicit here...
>   (gimple_call_builtin_p, gimple_call_combined_fn): ... rather than
>   here.
> 
> --- gcc/gimple.cc.jj  2022-04-06 10:07:23.043064595 +0200
> +++ gcc/gimple.cc 2022-04-06 11:31:31.704255242 +0200
> @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p (
>  {
>gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
>  
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +  fndecl = decl;
> +
>tree ret = gimple_call_lhs (stmt);
>if (ret
>&& !useless_type_conversion_p (TREE_TYPE (ret),
> @@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
> -{
> -  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> -   fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) == klass)
> -{
> -  if (klass == BUILT_IN_NORMAL)
> - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> -   fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& fndecl_built_in_p (fndecl, code))
> -{
> -  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> - fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s
>   return as_combined_fn (gimple_call_internal_fn (call));
>  
>tree fndecl = gimple_call_fndecl (stmt);
> -  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> - {
> -   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
> -   if (!decl)
> - decl = fndecl;
> -   if (gimple_builtin_call_types_compatible_p (stmt, decl))
> - return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> - }
> +  if (fndecl
> +   && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
> +   && gimple_builtin_call_types_compatible_p (stmt, fndecl))
> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>  }
>return CFN_LAST;
>  }
> 
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG N

Re: [PATCH] gimple.cc: Adjust gimple_call_builtin_p and gimple_call_combined_fn [PR105150]

[Richard Sandiford via Gcc-patches]

Jakub Jelinek  writes:
> On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote:
>> On Wed, 6 Apr 2022, Jakub Jelinek wrote:
>> 
>> > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote:
>> > > But it seems like the magic incantation to detect “real” built-in
>> > > function calls is getting longer and longer.  Can we not abstract this
>> > > in a single place rather than have to repeat the same long sequence in
>> > > multiple places?
>> > 
>> > I've already committed it, so it can be only dealt with an incremental
>> > patch.
>> > One possibility is to do it inside of
>> > gimple_builtin_call_types_compatible_p, after the assert do that:
>> >   if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
>> > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
>> >   fndecl = decl;
>> > but we then lose the theoretical possibility of comparing against the
>> > actual user declaration.  Though I guess in the
>> > gimple-fold.cc
>> > gimple-low.cc
>> > gimple-match-head.cc
>> > calls to that function we also want this rather than what they do 
>> > currently.
>> 
>> Yes, I think it would be clearer to pass a BUILT_IN_* code to
>> gimple_builtin_call_types_compatible_p and no decl and simply return
>> false if we cannot get out hands at the "proper" decl from
>> builtin_decl_explicit ...
>
> That would mean we wouldn't verify the md or FE builtins anymore
> and we would need to check for BUILT_IN_NORMAL in every caller (right now
> we do that only in some of them).
>
> Here is what I had in mind (untested so far):
>
> 2022-04-06  Jakub Jelinek  
>
>   PR tree-optimization/105150
>   * gimple.cc (gimple_builtin_call_types_compatible_p): Use
>   builtin_decl_explicit here...
>   (gimple_call_builtin_p, gimple_call_combined_fn): ... rather than
>   here.

Nice!  Thanks for doing this.

Richard

> --- gcc/gimple.cc.jj  2022-04-06 10:07:23.043064595 +0200
> +++ gcc/gimple.cc 2022-04-06 11:31:31.704255242 +0200
> @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p (
>  {
>gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
>  
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +  fndecl = decl;
> +
>tree ret = gimple_call_lhs (stmt);
>if (ret
>&& !useless_type_conversion_p (TREE_TYPE (ret),
> @@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
> -{
> -  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> -   fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& DECL_BUILT_IN_CLASS (fndecl) == klass)
> -{
> -  if (klass == BUILT_IN_NORMAL)
> - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> -   fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm
>if (is_gimple_call (stmt)
>&& (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE
>&& fndecl_built_in_p (fndecl, code))
> -{
> -  if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> - fndecl = decl;
> -  return gimple_builtin_call_types_compatible_p (stmt, fndecl);
> -}
> +return gimple_builtin_call_types_compatible_p (stmt, fndecl);
>return false;
>  }
>  
> @@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s
>   return as_combined_fn (gimple_call_internal_fn (call));
>  
>tree fndecl = gimple_call_fndecl (stmt);
> -  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> - {
> -   tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl));
> -   if (!decl)
> - decl = fndecl;
> -   if (gimple_builtin_call_types_compatible_p (stmt, decl))
> - return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
> - }
> +  if (fndecl
> +   && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
> +   && gimple_builtin_call_types_compatible_p (stmt, fndecl))
> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>  }
>return CFN_LAST;
>  }
>
>
>   Jakub

[PATCH] tree-optimization/105173 - fix insertion logic in reassoc

[Richard Biener via Gcc-patches]

The find_insert_point logic around deciding whether to insert
before or after the found insertion point does not handle
the case of _12 = ..;, _12, 1.0 well.  The following puts the
logic into find_insert_point itself instead.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2022-04-06  Richard Biener  

PR tree-optimization/105173
* tree-ssa-reassoc.cc (find_insert_point): Get extra
insert_before output argument and compute it.
(insert_stmt_before_use): Adjust.
(rewrite_expr_tree): Likewise.

* gcc.dg/pr105173.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr105173.c | 12 ++
 gcc/tree-ssa-reassoc.cc | 41 +++--
 2 files changed, 41 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr105173.c

diff --git a/gcc/testsuite/gcc.dg/pr105173.c b/gcc/testsuite/gcc.dg/pr105173.c
new file mode 100644
index 000..3effb2996b0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105173.c
@@ -0,0 +1,12 @@
+/* { dg-do compile { target dfp } } */
+/* { dg-options "-Ofast" } */
+
+int i;
+
+int
+foo(char c, _Decimal32 d)
+{
+  d *= i;
+  d *= -(_Decimal64)c;
+  return d;
+}
diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
index 0d55fc7e2d8..4ab3c3361f9 100644
--- a/gcc/tree-ssa-reassoc.cc
+++ b/gcc/tree-ssa-reassoc.cc
@@ -5185,17 +5185,26 @@ swap_ops_for_binary_stmt (const vec 
&ops,
 }
 
 /* If definition of RHS1 or RHS2 dominates STMT, return the later of those
-   two definitions, otherwise return STMT.  */
+   two definitions, otherwise return STMT.  Sets INSERT_BEFORE to indicate
+   whether RHS1 op RHS2 can be inserted before or needs to be inserted
+   after the returned stmt.  */
 
 static inline gimple *
-find_insert_point (gimple *stmt, tree rhs1, tree rhs2)
+find_insert_point (gimple *stmt, tree rhs1, tree rhs2, bool &insert_before)
 {
+  insert_before = true;
   if (TREE_CODE (rhs1) == SSA_NAME
   && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs1)))
-stmt = SSA_NAME_DEF_STMT (rhs1);
+{
+  stmt = SSA_NAME_DEF_STMT (rhs1);
+  insert_before = false;
+}
   if (TREE_CODE (rhs2) == SSA_NAME
   && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs2)))
-stmt = SSA_NAME_DEF_STMT (rhs2);
+{
+  stmt = SSA_NAME_DEF_STMT (rhs2);
+  insert_before = false;
+}
   return stmt;
 }
 
@@ -5207,7 +5216,8 @@ insert_stmt_before_use (gimple *stmt, gimple 
*stmt_to_insert)
   gcc_assert (is_gimple_assign (stmt_to_insert));
   tree rhs1 = gimple_assign_rhs1 (stmt_to_insert);
   tree rhs2 = gimple_assign_rhs2 (stmt_to_insert);
-  gimple *insert_point = find_insert_point (stmt, rhs1, rhs2);
+  bool insert_before;
+  gimple *insert_point = find_insert_point (stmt, rhs1, rhs2, insert_before);
   gimple_stmt_iterator gsi = gsi_for_stmt (insert_point);
   gimple_set_uid (stmt_to_insert, gimple_uid (insert_point));
 
@@ -5215,7 +5225,7 @@ insert_stmt_before_use (gimple *stmt, gimple 
*stmt_to_insert)
  the point where operand rhs1 or rhs2 is defined. In this case,
  stmt_to_insert has to be inserted afterwards. This would
  only happen when the stmt insertion point is flexible. */
-  if (stmt == insert_point)
+  if (insert_before)
 gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
   else
 insert_stmt_after (stmt_to_insert, insert_point);
@@ -5275,22 +5285,25 @@ rewrite_expr_tree (gimple *stmt, enum tree_code 
rhs_code, unsigned int opindex,
 return lhs), force creation of a new SSA_NAME.  */
  if (changed || ((rhs1 != oe2->op || rhs2 != oe1->op) && opindex))
{
+ bool insert_before;
  gimple *insert_point
-   = find_insert_point (stmt, oe1->op, oe2->op);
+   = find_insert_point (stmt, oe1->op, oe2->op, insert_before);
  lhs = make_ssa_name (TREE_TYPE (lhs));
  stmt
= gimple_build_assign (lhs, rhs_code,
   oe1->op, oe2->op);
  gimple_set_uid (stmt, uid);
  gimple_set_visited (stmt, true);
- if (insert_point == gsi_stmt (gsi))
+ if (insert_before)
gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
  else
insert_stmt_after (stmt, insert_point);
}
  else
{
- gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op)
+ bool insert_before;
+ gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op,
+ insert_before)
   == stmt);
  gimple_assign_set_rhs1 (stmt, oe1->op);
  gimple_assign_set_rhs2 (stmt, oe2->op);
@@ -5346,21 +5359,25 @@ rewrite_expr_tree (gimple *stmt, enum tree_code 
rhs_code, unsigned int opindex,
{
  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
  unsigned 

[PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Xi Ruoyao via Gcc-patches]

Another MIPS function return ABI fix.  Ok for trunk?

--

This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure.  Clang++
ignores C++17 empty bases in return values as well.

gcc/
* config/mips/mips.cc (mips_fpr_return_fields): Ignore
cxx17_empty_base_field_p fields.
---
 gcc/config/mips/mips.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 0f2492219f3..5010f99f761 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
   i = 0;
   for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN (field))
 {
-  if (TREE_CODE (field) != FIELD_DECL)
+  if (TREE_CODE (field) != FIELD_DECL
+ || cxx17_empty_base_field_p (field))
continue;
 
   if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
-- 
2.35.1

mips: Use c.ngl instead of c.ueq for LTGT [PR 91323]

[Xi Ruoyao via Gcc-patches]

Fixes gcc.dg/torture/pr91323.c fail for MIPS.  Ok for trunk?

LTGT should trap for unordered operands (see discussion in bugzilla),
but c.ueq does not trap for qNaN.  Use c.ngl as it handles non-NaN
operands like c.ueq, but traps for qNaN as we want for LTGT.

gcc/
PR target/91323
* config/mips/mips.md (sngl__using_cc): New insn pattern
for c.ngl.{f,d}.
* config/mips/mips.cc (mips_emit_compare): Use c.ngl.{f,d} for
LTGT.
---
 gcc/config/mips/mips.cc | 38 +-
 gcc/config/mips/mips.md | 10 ++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 5010f99f761..77675a57a01 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -5625,6 +5625,7 @@ mips_emit_compare (enum rtx_code *code, rtx *op0, rtx 
*op1, bool need_eq_ne_p)
 }
   else
 {
+  bool need_ngl = false;
   enum rtx_code cmp_code;
 
   /* Floating-point tests use a separate C.cond.fmt or CMP.cond.fmt
@@ -5643,10 +5644,20 @@ mips_emit_compare (enum rtx_code *code, rtx *op0, rtx 
*op1, bool need_eq_ne_p)
}
   else
{
- /* Three FP conditions cannot be implemented by reversing the
-operands for C.cond.fmt, instead a reversed condition code is
-required and a test for false.  */
- *code = mips_reversed_fp_cond (&cmp_code) ? EQ : NE;
+ /* We'll need a special case.  LTGT is not an unordered compare
+(PR 91323) so we can't simply use !UNEQ for it.  MIPS has
+"ngl" (Not Greater than or Less than) condition as a perfect
+opposite of LTGT, but we don't have an rtx_code for it.  */
+ if (cmp_code == LTGT)
+   {
+ need_ngl = true;
+ *code = EQ;
+   }
+ else
+   /* Three FP conditions cannot be implemented by reversing the
+  operands for C.cond.fmt, instead a reversed condition code
+  is required and a test for false.  */
+   *code = mips_reversed_fp_cond (&cmp_code) ? EQ : NE;
  if (ISA_HAS_8CC)
*op0 = mips_allocate_fcc (CCmode);
  else
@@ -5654,7 +5665,24 @@ mips_emit_compare (enum rtx_code *code, rtx *op0, rtx 
*op1, bool need_eq_ne_p)
}
 
   *op1 = const0_rtx;
-  mips_emit_binary (cmp_code, *op0, cmp_op0, cmp_op1);
+  if (need_ngl)
+   {
+ rtx insn;
+ switch (GET_MODE (cmp_op0))
+   {
+ case SFmode:
+   insn = gen_sngl_sf_using_cc (*op0, cmp_op0, cmp_op1);
+   break;
+ case DFmode:
+   insn = gen_sngl_df_using_cc (*op0, cmp_op0, cmp_op1);
+   break;
+ default:
+   gcc_unreachable ();
+   }
+ emit_insn(insn);
+   }
+  else
+   mips_emit_binary (cmp_code, *op0, cmp_op0, cmp_op1);
 }
 }
 
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index e0f0a582732..47405f03d7a 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -6293,6 +6293,16 @@ (define_insn "s__using_"
   "..\t%Z0%2,%1"
   [(set_attr "type" "fcmp")
(set_attr "mode" "FPSW")])
+
+(define_insn "sngl__using_cc"
+  [(set (match_operand:CC 0 "register_operand" "=z")
+   (eq:CC (const_int 0)
+  (ltgt:CC (match_operand:SCALARF 1 "register_operand" "f")
+   (match_operand:SCALARF 2 "register_operand" "f"]
+  "!ISA_HAS_CCF"
+  "c.ngl.\t%Z0%1,%2"
+  [(set_attr "type" "fcmp")
+   (set_attr "mode" "FPSW")])
 
 ;;
 ;;  
-- 
2.35.1

Move 'libgomp/plugin/cuda/cuda.h' to 'include/cuda/cuda.h' (was: [PATCH] Allow building GCC with PTX offloading even without CUDA being installed (gcc and nvptx-tools patches))

[Thomas Schwinge]

Hi!

On 2017-01-13T19:11:23+0100, Jakub Jelinek  wrote:
> Especially for distributions it is undesirable to need to have proprietary
> CUDA libraries and headers installed when building GCC.

> I've talked to our lawyers and they said that the cuda.h header included
> in this patch doesn't infringe anyone's copyright or is otherwise a fair
> use, it has been created by gathering all the cu*/CU* symbols from the
> current and older nvptx plugin and some oacc tests, then stubbing the
> pointer-ish typedefs, grabing most enum values and function prototypes from
> https://raw.githubusercontent.com/shinpei0208/gdev/master/cuda/driver/cuda.h
> and verifying assembly with that header against assembly when compiled
> against NVidia's cuda.h.

..., and later accordingly was slightly extended, as necessary to use
further CUDA features in libgomp's nvptx plugin.

> --- libgomp/plugin/cuda/cuda.h.jj 2017-01-13 15:58:00.966544147 +0100
> +++ libgomp/plugin/cuda/cuda.h2017-01-13 17:02:47.355817896 +0100
> @@ -0,0 +1,174 @@
> +/* CUDA API description.
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 3, or (at your option)
> +any later version.
> +
> +GCC is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +.
> +
> +This header provides the minimum amount of typedefs, enums and function
> +declarations to be able to compile plugin-nvptx.c if cuda.h and
> +libcuda.so.1 are not available.  */
> +
> +#ifndef GCC_CUDA_H
> +#define GCC_CUDA_H
> +[...]
> +#endif /* GCC_CUDA_H */

OK to push the attached
"Move 'libgomp/plugin/cuda/cuda.h' to 'include/cuda/cuda.h'", so that I'm
also able to use that file in the nvptx-tools, which inherit GCC's
'include' directory?


Grüße
 Thomas


-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
>From a6f9d53277ff8408cdbd7b89f3e7595e40333d48 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 6 Apr 2022 14:12:29 +0200
Subject: [PATCH] Move 'libgomp/plugin/cuda/cuda.h' to 'include/cuda/cuda.h'

... so that it may be used by other projects that inherit GCC's 'include'
directory.

	include/
	* cuda/cuda.h: New file.
	libgomp/
	* plugin/cuda/cuda.h: Remove file.
	* plugin/plugin-nvptx.c [PLUGIN_NVPTX_DYNAMIC]: Include
	"cuda/cuda.h" instead of .
	* plugin/configfrag.ac : Don't set
	'PLUGIN_NVPTX_CPPFLAGS'.
	* configure: Regenerate.
---
 {libgomp/plugin => include}/cuda/cuda.h | 7 +++
 libgomp/configure   | 1 -
 libgomp/plugin/configfrag.ac| 1 -
 libgomp/plugin/plugin-nvptx.c   | 6 +-
 4 files changed, 8 insertions(+), 7 deletions(-)
 rename {libgomp/plugin => include}/cuda/cuda.h (97%)

diff --git a/libgomp/plugin/cuda/cuda.h b/include/cuda/cuda.h
similarity index 97%
rename from libgomp/plugin/cuda/cuda.h
rename to include/cuda/cuda.h
index 5c679c1767a..5c813ad2cf8 100644
--- a/libgomp/plugin/cuda/cuda.h
+++ b/include/cuda/cuda.h
@@ -1,4 +1,4 @@
-/* CUDA API description.
+/* CUDA Driver API description.
Copyright (C) 2017-2022 Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -22,9 +22,8 @@ a copy of the GCC Runtime Library Exception along with this program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .
 
-This header provides the minimum amount of typedefs, enums and function
-declarations to be able to compile plugin-nvptx.c if cuda.h and
-libcuda.so.1 are not available.  */
+This header provides parts of the CUDA Driver API, without having to rely on
+the proprietary CUDA toolkit.  */
 
 #ifndef GCC_CUDA_H
 #define GCC_CUDA_H
diff --git a/libgomp/configure b/libgomp/configure
index b1b620cabc3..f863aa2ead4 100755
--- a/libgomp/configure
+++ b/libgomp/configure
@@ -15297,7 +15297,6 @@ rm -f core conftest.err conftest.$ac_objext \
 		   && (test "x$CUDA_DRIVER_LIB" = x \
 			   || test "x$CUDA_DRIVER_LIB" = xno); then
 		  PLUGIN_NVPTX=1
-		  PLUGIN_NVPTX_CPPFLAGS='-I$(srcdir)/plugin/cuda'
 		  PLUGIN_NVPTX_LIBS='-ldl'
 		  PLUGIN_NVPTX_DYNAMIC=1
 		else
diff --git a/libgomp/plugin/configfrag.ac b/libgomp/plugin/configfrag.ac
index fc298391d4c..54d4b675c4e 100644
--- a/libgomp/plugin/configfrag.ac
+++ b/libgomp/plugin/configfrag.ac
@@ -156,7 +156,6 @@ if test x"$enable_of

Re: Move 'libgomp/plugin/cuda/cuda.h' to 'include/cuda/cuda.h' (was: [PATCH] Allow building GCC with PTX offloading even without CUDA being installed (gcc and nvptx-tools patches))

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 02:39:18PM +0200, Thomas Schwinge wrote:
> ... so that it may be used by other projects that inherit GCC's 'include'
> directory.
> 
>   include/
>   * cuda/cuda.h: New file.
>   libgomp/
>   * plugin/cuda/cuda.h: Remove file.
>   * plugin/plugin-nvptx.c [PLUGIN_NVPTX_DYNAMIC]: Include
>   "cuda/cuda.h" instead of .
>   * plugin/configfrag.ac : Don't set
>   'PLUGIN_NVPTX_CPPFLAGS'.
>   * configure: Regenerate.

Ok.

Jakub

[PATCH] middle-end/105165 - sorry instead of ICE for _Complex asm goto

[Richard Biener via Gcc-patches]

Complex lowering cannot currently deal with asm gotos with _Complex
output operands.  Emit a sorry instead of ICEing, those should not
appear in practice.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK for trunk and branch?

Thanks,
Richard.

2022-04-06  Richard Biener  

PR middle-end/105165
* tree-complex.cc (expand_complex_asm): Sorry for asm goto
_Complex outputs.

* gcc.dg/pr105165.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr105165.c | 13 +
 gcc/tree-complex.cc | 16 
 2 files changed, 29 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr105165.c

diff --git a/gcc/testsuite/gcc.dg/pr105165.c b/gcc/testsuite/gcc.dg/pr105165.c
new file mode 100644
index 000..055a10528b5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105165.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+unsigned int _Complex a0;
+unsigned int _Complex
+foo (unsigned int _Complex a1, unsigned int _Complex a2)
+{
+  unsigned int _Complex x;
+  asm goto ("" : "=r" (x) : : : lab); /* { dg-message "sorry, unimplemented" } 
*/
+  a0 = x;
+ lab:
+  return x + a1 + a2 + 1;
+}
diff --git a/gcc/tree-complex.cc b/gcc/tree-complex.cc
index 8f03b236094..42db96a132b 100644
--- a/gcc/tree-complex.cc
+++ b/gcc/tree-complex.cc
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "cfganal.h"
 #include "gimple-fold.h"
+#include "diagnostic-core.h"
 
 
 /* For each complex ssa name, a lattice value.  We're interested in finding
@@ -1614,6 +1615,7 @@ expand_complex_asm (gimple_stmt_iterator *gsi)
 {
   gasm *stmt = as_a  (gsi_stmt (*gsi));
   unsigned int i;
+  bool diagnosed_p = false;
 
   for (i = 0; i < gimple_asm_noutputs (stmt); ++i)
 {
@@ -1622,6 +1624,20 @@ expand_complex_asm (gimple_stmt_iterator *gsi)
   if (TREE_CODE (op) == SSA_NAME
  && TREE_CODE (TREE_TYPE (op)) == COMPLEX_TYPE)
{
+ if (gimple_asm_nlabels (stmt) > 0)
+   {
+ if (!diagnosed_p)
+   {
+ sorry_at (gimple_location (stmt),
+   "% with complex typed outputs");
+ diagnosed_p = true;
+   }
+ /* Make sure to not ICE later, see PR105165.  */
+ tree zero = build_zero_cst (TREE_TYPE (TREE_TYPE (op)));
+ set_component_ssa_name (op, false, zero);
+ set_component_ssa_name (op, true, zero);
+ continue;
+   }
  tree type = TREE_TYPE (op);
  tree inner_type = TREE_TYPE (type);
  tree r = build1 (REALPART_EXPR, inner_type, op);
-- 
2.34.1

Re: [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 08:33:40PM +0800, Xi Ruoyao via Gcc-patches wrote:
> Another MIPS function return ABI fix.  Ok for trunk?
> 
> --
> 
> This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure.  Clang++
> ignores C++17 empty bases in return values as well.
> 
> gcc/
>   * config/mips/mips.cc (mips_fpr_return_fields): Ignore
>   cxx17_empty_base_field_p fields.
> ---
>  gcc/config/mips/mips.cc | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> index 0f2492219f3..5010f99f761 100644
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree 
> *fields,
>i = 0;
>for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN (field))
>  {
> -  if (TREE_CODE (field) != FIELD_DECL)
> +  if (TREE_CODE (field) != FIELD_DECL
> +   || cxx17_empty_base_field_p (field))
>   continue;
>  
>if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))

Well, this won't diagnose the ABI change.
So, if cxx17_empty_base_field_p, it should set some flag before continuing
and if it is considered a fpr return and that flag is set, it should emit a
-Wpsabi warning too.

Jakub

Re: [PATCH] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Xi Ruoyao via Gcc-patches]

On Wed, 2022-04-06 at 14:44 +0200, Jakub Jelinek wrote:
> On Wed, Apr 06, 2022 at 08:33:40PM +0800, Xi Ruoyao via Gcc-patches wrote:
> > Another MIPS function return ABI fix.  Ok for trunk?
> > 
> > --
> > 
> > This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure.  Clang++
> > ignores C++17 empty bases in return values as well.
> > 
> > gcc/
> > * config/mips/mips.cc (mips_fpr_return_fields): Ignore
> > cxx17_empty_base_field_p fields.
> > ---
> >  gcc/config/mips/mips.cc | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> > index 0f2492219f3..5010f99f761 100644
> > --- a/gcc/config/mips/mips.cc
> > +++ b/gcc/config/mips/mips.cc
> > @@ -6337,7 +6337,8 @@ mips_fpr_return_fields (const_tree valtype, tree 
> > *fields,
> >    i = 0;
> >    for (field = TYPE_FIELDS (valtype); field != 0; field = DECL_CHAIN 
> > (field))
> >  {
> > -  if (TREE_CODE (field) != FIELD_DECL)
> > +  if (TREE_CODE (field) != FIELD_DECL
> > + || cxx17_empty_base_field_p (field))
> > continue;
> >  
> >    if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
> 
> Well, this won't diagnose the ABI change.
> So, if cxx17_empty_base_field_p, it should set some flag before continuing
> and if it is considered a fpr return and that flag is set, it should emit a
> -Wpsabi warning too.

Ok, will add it.

When I learnt from PR94704 fix I failed to notice the second commit
adding -Wpsabi warning :(.


-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Xi Ruoyao via Gcc-patches]

v2: Add psABI warning and test.

--

This fixes tmpdir-g++.dg-struct-layout-1/{t032,t059} failure.  Clang++
also ignores C++17 empty bases in return values.

gcc/
* config/mips/mips.cc (mips_fpr_return_fields): Ignore
cxx17_empty_base_field_p fields and set an indicator.
(mips_return_in_msb): Adjust for mips_fpr_return_fields change.
(mips_function_value_1): Inform psABI change about C++ 17 empty
bases.

gcc/testsuite/
* g++.target/mips/cxx17_empty_base.C: New test.
---
 gcc/config/mips/mips.cc   | 58 +--
 .../g++.target/mips/cxx17_empty_base.C| 20 +++
 2 files changed, 73 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/mips/cxx17_empty_base.C

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 079bb03968a..57ce0884951 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -6346,12 +6346,21 @@ mips_callee_copies (cumulative_args_t, const 
function_arg_info &arg)
The C++ FE used to remove zero-width bit-fields in GCC 11 and earlier.
To make a proper diagnostic, this function will set
HAS_CXX_ZERO_WIDTH_BF to true once a C++ zero-width bit-field shows up,
-   and then ignore it. Then the caller can determine if this zero-width
-   bit-field will make a difference and emit a -Wpsabi inform.  */
+   and then ignore it.
+
+   We had failed to ignore C++ 17 empty bases in GCC 7, 8, 9, 10, and 11.
+   This caused an ABI incompatibility between C++ 14 and C++ 17.  This is
+   fixed now and to make a proper diagnostic, this function will set
+   HAS_CXX17_EMPTY_BASE to true once a C++ 17 empty base shows up, and
+   then ignore it.
+
+   The caller should use the value of HAS_CXX17_EMPTY_BASE and/or
+   HAS_CXX_ZERO_WIDTH_BF to emit a proper -Wpsabi inform.  */
 
 static int
 mips_fpr_return_fields (const_tree valtype, tree *fields,
-   bool *has_cxx_zero_width_bf)
+   bool *has_cxx_zero_width_bf,
+   bool *has_cxx17_empty_base)
 {
   tree field;
   int i;
@@ -6368,6 +6377,12 @@ mips_fpr_return_fields (const_tree valtype, tree *fields,
   if (TREE_CODE (field) != FIELD_DECL)
continue;
 
+  if (cxx17_empty_base_field_p (field))
+   {
+ *has_cxx17_empty_base = true;
+ continue;
+   }
+
   if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
{
  *has_cxx_zero_width_bf = true;
@@ -6403,8 +6418,13 @@ mips_return_in_msb (const_tree valtype)
 
   tree fields[2];
   bool has_cxx_zero_width_bf = false;
+
+  /* Its value is not used.  */
+  bool has_cxx17_empty_base = false;
+
   return (mips_fpr_return_fields (valtype, fields,
- &has_cxx_zero_width_bf) == 0
+ &has_cxx_zero_width_bf,
+ &has_cxx17_empty_base) == 0
  || has_cxx_zero_width_bf);
 }
 
@@ -6501,11 +6521,18 @@ mips_function_value_1 (const_tree valtype, const_tree 
fn_decl_or_type,
   mode = promote_function_mode (valtype, mode, &unsigned_p, func, 1);
 
   bool has_cxx_zero_width_bf = false;
+  bool has_cxx17_empty_base = false;
   int use_fpr = mips_fpr_return_fields (valtype, fields,
-   &has_cxx_zero_width_bf);
+   &has_cxx_zero_width_bf,
+   &has_cxx17_empty_base);
+
+  /* If has_cxx_zero_width_bf and has_cxx17_empty_base are both
+true, it *happens* that there is no ABI change.  So we won't
+inform in this case.  */
   if (TARGET_HARD_FLOAT
  && warn_psabi
  && has_cxx_zero_width_bf
+ && !has_cxx17_empty_base
  && use_fpr != 0)
{
  static unsigned last_reported_type_uid;
@@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype, const_tree 
fn_decl_or_type,
   if (has_cxx_zero_width_bf)
use_fpr = 0;
 
+  if (TARGET_HARD_FLOAT
+ && warn_psabi
+ && use_fpr != 0
+ && has_cxx17_empty_base)
+   {
+ static unsigned last_reported_type_uid;
+ unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
+ if (uid != last_reported_type_uid)
+   {
+ static const char *url
+   = CHANGES_ROOT_URL
+ "gcc-12/changes.html#mips_cxx17_empty_bases";
+ inform (input_location,
+ "the ABI for returning a value with C++ 17 empty "
+ "bases but otherwise an aggregate with only one or "
+ "two floating-point fields was changed in GCC "
+ "%{12.1%}", url);
+ last_reported_type_uid = uid;
+   }
+   }
+
   /* Handle structures whose fields are returned in $f0/$f2.  */
   switch (use_fpr)
{
diff --git a/gcc/testsuite/g++.target/mips/cxx17_empty_base.C 
b/gcc/te

[PATCH RFA(pointer-query)] c++: -Wplacement-new and anon union member [PR100370]

[Jason Merrill via Gcc-patches]

This bug was an object/value confusion; we are interested in the size
of *b.ip, but instead the code was calculating the size of b.ip itself.

This seems to be because compute_objsize will compute the size of whatever
object it can find in the argument: if you pass it a VAR_DECL, it gives you
the size of that variable.  If you pass it an ADDR_EXPR of a VAR_DECL, it
again gives you the size of the variable.  The way you can tell the
difference is by looking at the deref member of access_ref: if it's -1, the
argument is a pointer to the object.  Since that's what we're interested in,
we should check for that, like check_dangling_stores does.

This regressed some tests because compute_objsize_r was wrongly zeroing
deref in the POINTER_PLUS_EXPR handling; adding an offset to a pointer
doesn't change whether the pointer is itself a variable or a pointer to
one.  In fact, handling POINTER_PLUS_EXPR only really makes sense for deref
== -1, where we're adjusting a pointer to the variable.

Tested x86_64-pc-linux-gnu, OK for trunk?

PR c++/100370

gcc/cp/ChangeLog:

* init.cc (warn_placement_new_too_small): Check deref.

gcc/ChangeLog:

* pointer-query.cc (compute_objsize_r) [POINTER_PLUS_EXPR]: Require
deref == -1.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wplacement-new-size-11.C: New test.
---
 gcc/cp/init.cc|  5 +
 gcc/pointer-query.cc  |  7 ---
 .../g++.dg/warn/Wplacement-new-size-11.C  | 15 +++
 3 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wplacement-new-size-11.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 01e762320f3..43097121244 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -2811,6 +2811,11 @@ warn_placement_new_too_small (tree type, tree nelts, 
tree size, tree oper)
   if (!objsize)
 return;
 
+  /* We can only draw conclusions if ref.deref == -1,
+ i.e. oper is the address of the object.  */
+  if (ref.deref != -1)
+return;
+
   offset_int bytes_avail = wi::to_offset (objsize);
   offset_int bytes_need;
 
diff --git a/gcc/pointer-query.cc b/gcc/pointer-query.cc
index 4390535ef56..d93657f3206 100644
--- a/gcc/pointer-query.cc
+++ b/gcc/pointer-query.cc
@@ -2299,9 +2299,10 @@ compute_objsize_r (tree ptr, gimple *stmt, bool addr, 
int ostype,
   if (!compute_objsize_r (ref, stmt, addr, ostype, pref, snlim, qry))
return false;
 
-  /* Clear DEREF since the offset is being applied to the target
-of the dereference.  */
-  pref->deref = 0;
+  /* The below only makes sense if the offset is being applied to the
+address of the object.  */
+  if (pref->deref != -1)
+   return false;
 
   offset_int orng[2];
   tree off = pref->eval (TREE_OPERAND (ptr, 1));
diff --git a/gcc/testsuite/g++.dg/warn/Wplacement-new-size-11.C 
b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-11.C
new file mode 100644
index 000..a6fe82e90ae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-11.C
@@ -0,0 +1,15 @@
+// PR c++/100370
+// { dg-do compile { target c++11 } }
+
+using size_t = decltype(sizeof(1));
+inline void *operator new (size_t s, void *p) { return p; }
+
+int main()
+{
+  struct s1 { int iv[4]; };
+  struct s2 { union { char* cp; int* ip; }; };
+
+  s2 b;
+  b.ip=new int[8];
+  new (b.ip+4) s1; // { dg-bogus "-Wplacement-new" }
+}

base-commit: 44fe49401725055a740ce47e80561b6932b8cd01
-- 
2.27.0

[pushed] c++: -Wshadow=compatible-local type vs var [PR100608]

[Jason Merrill via Gcc-patches]

The patch for PR92024 changed -Wshadow=compatible-local to warn if either
new or old decl was a type, but the rationale only talked about the case
where both are types.  If only one is, they aren't compatible.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/100608

gcc/cp/ChangeLog:

* name-lookup.cc (check_local_shadow): Use -Wshadow=local
if exactly one of 'old' and 'decl' is a type.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wshadow-compatible-local-3.C: New test.
---
 gcc/cp/name-lookup.cc  |  4 
 gcc/testsuite/g++.dg/warn/Wshadow-compatible-local-3.C | 10 ++
 2 files changed, 14 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-compatible-local-3.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index c833b84ca8a..d16c577c029 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -3249,6 +3249,10 @@ check_local_shadow (tree decl)
   enum opt_code warning_code;
   if (warn_shadow)
warning_code = OPT_Wshadow;
+  else if ((TREE_CODE (decl) == TYPE_DECL)
+  ^ (TREE_CODE (old) == TYPE_DECL))
+   /* If exactly one is a type, they aren't compatible.  */
+   warning_code = OPT_Wshadow_local;
   else if ((TREE_TYPE (old)
&& TREE_TYPE (decl)
&& same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-compatible-local-3.C 
b/gcc/testsuite/g++.dg/warn/Wshadow-compatible-local-3.C
new file mode 100644
index 000..0e5ece74b37
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-compatible-local-3.C
@@ -0,0 +1,10 @@
+// PR c++/100608
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wshadow=compatible-local" }
+
+template  class X {};
+
+void foo()
+{
+  auto a = X{};   // no warning, not compatible
+}

base-commit: e1a5e7562d53a8d2256f754714b06595bea72196
-- 
2.27.0

Re: [PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 09:44:33PM +0800, Xi Ruoyao wrote:
> @@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype, const_tree 
> fn_decl_or_type,
>if (has_cxx_zero_width_bf)
>   use_fpr = 0;
>  
> +  if (TARGET_HARD_FLOAT
> +   && warn_psabi
> +   && use_fpr != 0
> +   && has_cxx17_empty_base)
> + {
> +   static unsigned last_reported_type_uid;
> +   unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> +   if (uid != last_reported_type_uid)
> + {
> +   static const char *url
> + = CHANGES_ROOT_URL
> +   "gcc-12/changes.html#mips_cxx17_empty_bases";
> +   inform (input_location,
> +   "the ABI for returning a value with C++ 17 empty "

Better write C++17 without space, that is what is used elsewhere too.
Ok with that change.

Jakub

[PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586]

[Jakub Jelinek via Gcc-patches]

On Fri, Feb 11, 2022 at 07:55:50PM +0100, Jakub Jelinek via Gcc-patches wrote:
> Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
> will still be needed with adjusted testcase from
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
> __builtin_clear_padding is called directly on var addresses rather than
> in separate functions.

Here is an updated version of the
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15
patch which uses FIELD_DECL in the langhook instead of its TREE_TYPE,
and the testcases have been adjusted for the builtin accepting
pointers to non-trivially-copyable types only if it is address of a
declaration.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2022-04-06  Jakub Jelinek  

PR tree-optimization/102586
gcc/
* langhooks.h (struct lang_hooks_for_types): Add classtype_as_base
langhook.
* langhooks-def.h (LANG_HOOKS_CLASSTYPE_AS_BASE): Define.
(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it.
* gimple-fold.cc (clear_padding_type): Use ftype instead of
TREE_TYPE (field) some more.  For artificial FIELD_DECLs without
name try the lang_hooks.types.classtype_as_base langhook and
if it returns non-NULL, use that instead of ftype for recursive call.
gcc/cp/
* cp-objcp-common.h (cp_classtype_as_base): Declare.
(LANG_HOOKS_CLASSTYPE_AS_BASE): Redefine.
* cp-objcp-common.cc (cp_classtype_as_base): New function.
gcc/testsuite/
* g++.dg/torture/builtin-clear-padding-5.C: New test.
* g++.dg/cpp2a/builtin-clear-padding1.C (bar): Uncomment one
call that is now accepted.

--- gcc/langhooks.h.jj  2022-02-11 00:18:54.909437559 +0100
+++ gcc/langhooks.h 2022-04-06 12:34:43.312087323 +0200
@@ -188,6 +188,11 @@ struct lang_hooks_for_types
   /* Returns a tree for the unit size of T excluding tail padding that
  might be used by objects inheriting from T.  */
   tree (*unit_size_without_reusable_padding) (tree);
+
+  /* Returns type corresponding to FIELD's type when FIELD is a C++ base class
+ i.e., type without virtual base classes or tail padding.  Returns
+ NULL_TREE otherwise.  */
+  tree (*classtype_as_base) (const_tree);
 };
 
 /* Language hooks related to decls and the symbol table.  */
--- gcc/langhooks-def.h.jj  2022-02-11 00:18:54.887437859 +0100
+++ gcc/langhooks-def.h 2022-04-06 12:31:39.149670170 +0200
@@ -216,6 +216,7 @@ extern tree lhd_unit_size_without_reusab
 #define LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO NULL
 #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTElhd_type_dwarf_attribute
 #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING 
lhd_unit_size_without_reusable_padding
+#define LANG_HOOKS_CLASSTYPE_AS_BASE   hook_tree_const_tree_null
 
 #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
   LANG_HOOKS_MAKE_TYPE, \
@@ -243,7 +244,8 @@ extern tree lhd_unit_size_without_reusab
   LANG_HOOKS_GET_DEBUG_TYPE, \
   LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO, \
   LANG_HOOKS_TYPE_DWARF_ATTRIBUTE, \
-  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING \
+  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING, \
+  LANG_HOOKS_CLASSTYPE_AS_BASE \
 }
 
 /* Declaration hooks.  */
--- gcc/gimple-fold.cc.jj   2022-04-06 09:59:32.744654454 +0200
+++ gcc/gimple-fold.cc  2022-04-06 12:35:29.413440758 +0200
@@ -4747,7 +4747,7 @@ clear_padding_type (clear_padding_struct
  "have well defined padding bits for %qs",
field, "__builtin_clear_padding");
  }
-   else if (is_empty_type (TREE_TYPE (field)))
+   else if (is_empty_type (ftype))
  continue;
else
  {
@@ -4758,8 +4758,9 @@ clear_padding_type (clear_padding_struct
gcc_assert (pos >= 0 && fldsz >= 0 && pos >= cur_pos);
clear_padding_add_padding (buf, pos - cur_pos);
cur_pos = pos;
-   clear_padding_type (buf, TREE_TYPE (field),
-   fldsz, for_auto_init);
+   if (tree asbase = lang_hooks.types.classtype_as_base (field))
+ ftype = asbase;
+   clear_padding_type (buf, ftype, fldsz, for_auto_init);
cur_pos += fldsz;
  }
  }
--- gcc/cp/cp-objcp-common.h.jj 2022-02-11 00:18:54.730439994 +0100
+++ gcc/cp/cp-objcp-common.h2022-04-06 12:31:39.151670141 +0200
@@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
 extern int cp_type_dwarf_attribute (const_tree, int);
 extern void cp_common_init_ts (void);
 extern tree cp_unit_size_without_reusable_padding (tree);
+extern tree cp_classtype_as_base (const_tree);
 extern tree cp_get_global_decls ();
 extern tree cp_pushdecl (tree);
 extern void cp_register_dumps (gcc::dump_manager *);
@@ -167,6 +168,8 @@ extern tree cxx_simulate_record_decl (lo
 #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_type_dwarf_attribute
 #undef LANG_HOOKS_UNIT_SIZE_WITHOUT_RE

Re: [PATCH] libstdc++-v3 expected: Don't test ABI-variant properties in requirements.cc

[Hans-Peter Nilsson via Gcc-patches]

> From: Jonathan Wakely 
> Date: Tue, 5 Apr 2022 20:47:58 +0200

> On Tue, 5 Apr 2022, 17:44 Hans-Peter Nilsson via
> Libstdc++,
> mailto:libstdc%2b...@gcc.gnu.org>>
> wrote:
> Ok to commit?
> -- 8< --
> 
> Without this, for a target where alignment and structure-sizes are by
> default byte-aligned, such as cris-elf, you'll see, in libstdc++.log:
> 
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error: 
> static assertion failed
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: note: the 
> comparison reduces to '(5 == 2)'
> compiler exited with status 1
> FAIL: 20_util/expected/requirements.cc (test for excess errors)
> Excess errors:
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error: 
> static assertion failed
> 
> It seems the intent is a smoke-test and that conditionals for ABI
> properties are out of scope, so best to just delete this particular
> line.
> 
> The idea is to ensure the object is no larger than necessary.
> 
> I think we could use == sizeof(void*)+alignof(void*) which
> would be correct everywhere. Does that work for cris-elf?

Oh right, yes it does.  Ok then, I'll commit this:

-- 8< --

[PATCH v2] libstdc++-v3 expected: Correct minimal-size test in requirements.cc

Without this, for a target where alignment and structure-sizes are by
default byte-aligned, such as cris-elf, you'll see, in libstdc++.log:

/X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error: 
static assertion failed
/X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: note: the 
comparison reduces to '(5 == 2)'
compiler exited with status 1
FAIL: 20_util/expected/requirements.cc (test for excess errors)
Excess errors:
/X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error: 
static assertion failed

The intent of that line is to check that the object is not larger than
necessary.

libstdc++-v3/:
* testsuite/20_util/expected/requirements.cc: Correct minimal-size
test.

---
 libstdc++-v3/testsuite/20_util/expected/requirements.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/20_util/expected/requirements.cc 
b/libstdc++-v3/testsuite/20_util/expected/requirements.cc
index 485aa338679c..a51a007a4fc3 100644
--- a/libstdc++-v3/testsuite/20_util/expected/requirements.cc
+++ b/libstdc++-v3/testsuite/20_util/expected/requirements.cc
@@ -124,6 +124,6 @@ static_assert( move_assignable< void, G > );
 // QoI properties
 static_assert( sizeof(std::expected) == 2 );
 static_assert( sizeof(std::expected) == 2 );
-static_assert( sizeof(std::expected) == 2 * __alignof(void*) );
+static_assert( sizeof(std::expected) == sizeof(void*) + 
__alignof(void*) );
 static_assert( alignof(std::expected) == 1 );
 static_assert( alignof(std::expected) == alignof(void*) );
-- 
2.30.2

Ping Re: [PATCH] c++: Fix up ICE when cplus_decl_attributes is called with error_mark_node attributes [PR104668]

[Jakub Jelinek via Gcc-patches]

On Fri, Mar 25, 2022 at 07:55:56PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Mar 25, 2022 at 07:48:33PM +0100, Jakub Jelinek wrote:
> > We then wouldn't propagate TREE_DEPRECATED/TREE_UNAVAILABLE from templates
> > to their instantiations and wouldn't diagnose static data members in OpenMP
> > declare target.
> > But perhaps that is fine, with error_mark_attribute it is error recovery
> > anyway.
> > Can the splice_template_attributes change stay too, or just
> > change cplus_decl_attributes (add it to the early out, remove from other
> > spots in the function)?
> 
> So like (or without the first hunk):
> 
> 2022-03-25  Jakub Jelinek  
> 
>   PR c++/104668
>   * decl2.cc (splice_template_attributes): Return NULL if *p is
>   error_mark_node.
>   (cplus_decl_attributes): Return early if attributes is
>   error_mark_node.  Don't check that later.
> 
>   * g++.dg/cpp0x/pr104668.C: New test.

I'd like to ping this patch.  Bootstrapped/regtested successfully on
{powerpc64le,x86_64,i686}-linux.

> --- gcc/cp/decl2.cc.jj2022-03-09 09:09:55.415843331 +0100
> +++ gcc/cp/decl2.cc   2022-03-25 19:54:12.454749089 +0100
> @@ -1336,7 +1336,7 @@ splice_template_attributes (tree *attr_p
>tree late_attrs = NULL_TREE;
>tree *q = &late_attrs;
>  
> -  if (!p)
> +  if (!p || *p == error_mark_node)
>  return NULL_TREE;
>  
>for (; *p; )
> @@ -1631,7 +1631,7 @@ void
>  cplus_decl_attributes (tree *decl, tree attributes, int flags)
>  {
>if (*decl == NULL_TREE || *decl == void_type_node
> -  || *decl == error_mark_node)
> +  || *decl == error_mark_node || attributes == error_mark_node)
>  return;
>  
>/* Add implicit "omp declare target" attribute if requested.  */
> @@ -1668,7 +1668,7 @@ cplus_decl_attributes (tree *decl, tree
>  
>cp_check_const_attributes (attributes);
>  
> -  if ((flag_openmp || flag_openmp_simd) && attributes != error_mark_node)
> +  if (flag_openmp || flag_openmp_simd)
>  {
>bool diagnosed = false;
>for (tree *pa = &attributes; *pa; )
> --- gcc/testsuite/g++.dg/cpp0x/pr104668.C.jj  2022-03-25 17:25:42.280068058 
> +0100
> +++ gcc/testsuite/g++.dg/cpp0x/pr104668.C 2022-03-25 17:24:44.862881444 
> +0100
> @@ -0,0 +1,13 @@
> +// PR c++/104668
> +// { dg-do compile { target c++11 } }
> +// { dg-excess-errors "" }
> +
> +template 
> +void sink(Ts...);
> +template 
> +void f(Ts...) {
> +  sink([] { struct alignas:Ts) S {}; }...); }
> +}
> +int main() {
> +  f(0);
> +}

Jakub

[PATCH] tree.cc, v2: Add tree_builtin_call_types_compatible_p [PR105150]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 09:49:25AM +0200, Richard Biener wrote:
> On trees we'd use tree_[sign_]nop_conversion () instead of
> useless_type_conversion_p, I think it's OK to allow all such
> pointer conversions.  In the end this probably means being
> more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
> would also make the code more similar to 
> gimple_builtin_call_types_compatible_p besides
> s/useless_type_conversion_p/tree_sign_nop_conversion/

Here is an updated patch to do that, allow differences in pointer types
and tweak the promotion handling.
There is no tree_sign_nop_conversion_p, so I've used
tree_nop_conversion_p.  What tree_sign_nop_conversion does on top of
that is just that it verifies both types are pointer types or
neither is and in the latter case TYPE_UNSIGNED is the same.
So the patch verifies both are POINTER_TYPE_P for the one case
and for the promotion where it already checks the unpromoted type
is integral checks if promoted one is integral too and signed.

I've also changed the patch to match the now committed gimple.cc
change where the builtin_decl_explicit is inside of the
*_call_types_compatible_p function instead of the caller.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2022-04-06  Jakub Jelinek  

PR tree-optimization/105150
* tree.cc (tree_builtin_call_types_compatible_p): New function.
(get_call_combined_fn): Use it.

* gcc.dg/pr105150.c: New test.

--- gcc/tree.cc.jj  2022-04-06 09:59:03.312066863 +0200
+++ gcc/tree.cc 2022-04-06 10:52:55.176755024 +0200
@@ -8406,6 +8406,59 @@ get_callee_fndecl (const_tree call)
   return NULL_TREE;
 }
 
+/* Return true when STMTs arguments and return value match those of FNDECL,
+   a decl of a builtin function.  */
+
+static bool
+tree_builtin_call_types_compatible_p (const_tree call, tree fndecl)
+{
+  gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
+
+  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
+  fndecl = decl;
+
+  if (TYPE_MAIN_VARIANT (TREE_TYPE (call))
+  != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (fndecl
+return false;
+
+  tree targs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
+  unsigned nargs = call_expr_nargs (call);
+  for (unsigned i = 0; i < nargs; ++i, targs = TREE_CHAIN (targs))
+{
+  /* Variadic args follow.  */
+  if (!targs)
+   return true;
+  tree arg = CALL_EXPR_ARG (call, i);
+  tree type = TREE_VALUE (targs);
+  if (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
+   {
+ /* For pointer arguments be more forgiving, e.g. due to
+FILE * vs. fileptr_type_node, or say char * vs. const char *
+differences etc.  */
+ if (POINTER_TYPE_P (type)
+ && POINTER_TYPE_P (TREE_TYPE (arg))
+ && tree_nop_conversion_p (type, TREE_TYPE (arg)))
+   continue;
+ /* char/short integral arguments are promoted to int
+by several frontends if targetm.calls.promote_prototypes
+is true.  Allow such promotion too.  */
+ if (INTEGRAL_TYPE_P (type)
+ && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node)
+ && INTEGRAL_TYPE_P (TREE_TYPE (arg))
+ && !TYPE_UNSIGNED (TREE_TYPE (arg))
+ && targetm.calls.promote_prototypes (TREE_TYPE (fndecl))
+ && tree_nop_conversion_p (integer_type_node,
+   TREE_TYPE (arg)))
+   continue;
+ return false;
+   }
+}
+  if (targs && !VOID_TYPE_P (TREE_VALUE (targs)))
+return false;
+  return true;
+}
+
 /* If CALL_EXPR CALL calls a normal built-in function or an internal function,
return the associated function code, otherwise return CFN_LAST.  */
 
@@ -8419,7 +8472,9 @@ get_call_combined_fn (const_tree call)
 return as_combined_fn (CALL_EXPR_IFN (call));
 
   tree fndecl = get_callee_fndecl (call);
-  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
+  if (fndecl
+  && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
+  && tree_builtin_call_types_compatible_p (call, fndecl))
 return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
 
   return CFN_LAST;
--- gcc/testsuite/gcc.dg/pr105150.c.jj  2022-04-06 10:51:12.801191206 +0200
+++ gcc/testsuite/gcc.dg/pr105150.c 2022-04-06 10:51:12.801191206 +0200
@@ -0,0 +1,8 @@
+/* PR tree-optimization/105150 */
+/* { dg-options "-w -Ofast" } */
+
+#define A(name) __typeof (__builtin_##name (0)) name (); \
+  float name##1 () { return !name (1); } \
+  double name##2 () { return name (1.0L); }
+#define B(name) A(name) A(name##l)
+B (sqrt)


Jakub

Re: [PATCH v2] mips: Fix C++14 vs. C++17 ABI incompatibility on mips64

[Xi Ruoyao via Gcc-patches]

On Wed, 2022-04-06 at 16:34 +0200, Jakub Jelinek wrote:
> On Wed, Apr 06, 2022 at 09:44:33PM +0800, Xi Ruoyao wrote:
> > @@ -6527,6 +6554,27 @@ mips_function_value_1 (const_tree valtype,
> > const_tree fn_decl_or_type,
> >    if (has_cxx_zero_width_bf)
> > use_fpr = 0;
> >  
> > +  if (TARGET_HARD_FLOAT
> > + && warn_psabi
> > + && use_fpr != 0
> > + && has_cxx17_empty_base)
> > +   {
> > + static unsigned last_reported_type_uid;
> > + unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (valtype));
> > + if (uid != last_reported_type_uid)
> > +   {
> > + static const char *url
> > +   = CHANGES_ROOT_URL
> > + "gcc-12/changes.html#mips_cxx17_empty_bases";
> > + inform (input_location,
> > + "the ABI for returning a value with C++ 17
> > empty "
> 
> Better write C++17 without space, that is what is used elsewhere too.
> Ok with that change.

Pushed r12-8023 with the whitespace change.

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586]

[Jason Merrill via Gcc-patches]

On 4/6/22 10:41, Jakub Jelinek wrote:

On Fri, Feb 11, 2022 at 07:55:50PM +0100, Jakub Jelinek via Gcc-patches wrote:

Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
will still be needed with adjusted testcase from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
__builtin_clear_padding is called directly on var addresses rather than
in separate functions.


Here is an updated version of the
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15
patch which uses FIELD_DECL in the langhook instead of its TREE_TYPE,
and the testcases have been adjusted for the builtin accepting
pointers to non-trivially-copyable types only if it is address of a
declaration.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2022-04-06  Jakub Jelinek  

PR tree-optimization/102586
gcc/
* langhooks.h (struct lang_hooks_for_types): Add classtype_as_base
langhook.
* langhooks-def.h (LANG_HOOKS_CLASSTYPE_AS_BASE): Define.
(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it.
* gimple-fold.cc (clear_padding_type): Use ftype instead of
TREE_TYPE (field) some more.  For artificial FIELD_DECLs without
name try the lang_hooks.types.classtype_as_base langhook and
if it returns non-NULL, use that instead of ftype for recursive call.
gcc/cp/
* cp-objcp-common.h (cp_classtype_as_base): Declare.
(LANG_HOOKS_CLASSTYPE_AS_BASE): Redefine.
* cp-objcp-common.cc (cp_classtype_as_base): New function.
gcc/testsuite/
* g++.dg/torture/builtin-clear-padding-5.C: New test.
* g++.dg/cpp2a/builtin-clear-padding1.C (bar): Uncomment one
call that is now accepted.

--- gcc/langhooks.h.jj  2022-02-11 00:18:54.909437559 +0100
+++ gcc/langhooks.h 2022-04-06 12:34:43.312087323 +0200
@@ -188,6 +188,11 @@ struct lang_hooks_for_types
/* Returns a tree for the unit size of T excluding tail padding that
   might be used by objects inheriting from T.  */
tree (*unit_size_without_reusable_padding) (tree);
+
+  /* Returns type corresponding to FIELD's type when FIELD is a C++ base class
+ i.e., type without virtual base classes or tail padding.  Returns
+ NULL_TREE otherwise.  */
+  tree (*classtype_as_base) (const_tree);
  };
  
  /* Language hooks related to decls and the symbol table.  */

--- gcc/langhooks-def.h.jj  2022-02-11 00:18:54.887437859 +0100
+++ gcc/langhooks-def.h 2022-04-06 12:31:39.149670170 +0200
@@ -216,6 +216,7 @@ extern tree lhd_unit_size_without_reusab
  #define LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO NULL
  #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE   lhd_type_dwarf_attribute
  #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING 
lhd_unit_size_without_reusable_padding
+#define LANG_HOOKS_CLASSTYPE_AS_BASE   hook_tree_const_tree_null
  
  #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \

LANG_HOOKS_MAKE_TYPE, \
@@ -243,7 +244,8 @@ extern tree lhd_unit_size_without_reusab
LANG_HOOKS_GET_DEBUG_TYPE, \
LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO, \
LANG_HOOKS_TYPE_DWARF_ATTRIBUTE, \
-  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING \
+  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING, \
+  LANG_HOOKS_CLASSTYPE_AS_BASE \
  }
  
  /* Declaration hooks.  */

--- gcc/gimple-fold.cc.jj   2022-04-06 09:59:32.744654454 +0200
+++ gcc/gimple-fold.cc  2022-04-06 12:35:29.413440758 +0200
@@ -4747,7 +4747,7 @@ clear_padding_type (clear_padding_struct
  "have well defined padding bits for %qs",
field, "__builtin_clear_padding");
  }
-   else if (is_empty_type (TREE_TYPE (field)))
+   else if (is_empty_type (ftype))
  continue;
else
  {
@@ -4758,8 +4758,9 @@ clear_padding_type (clear_padding_struct
gcc_assert (pos >= 0 && fldsz >= 0 && pos >= cur_pos);
clear_padding_add_padding (buf, pos - cur_pos);
cur_pos = pos;
-   clear_padding_type (buf, TREE_TYPE (field),
-   fldsz, for_auto_init);
+   if (tree asbase = lang_hooks.types.classtype_as_base (field))
+ ftype = asbase;
+   clear_padding_type (buf, ftype, fldsz, for_auto_init);
cur_pos += fldsz;
  }
  }
--- gcc/cp/cp-objcp-common.h.jj 2022-02-11 00:18:54.730439994 +0100
+++ gcc/cp/cp-objcp-common.h2022-04-06 12:31:39.151670141 +0200
@@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
  extern int cp_type_dwarf_attribute (const_tree, int);
  extern void cp_common_init_ts (void);
  extern tree cp_unit_size_without_reusable_padding (tree);
+extern tree cp_classtype_as_base (const_tree);
  extern tree cp_get_global_decls ();
  extern tree cp_pushdecl (tree);
  extern void cp_register_dumps (gcc::dump_manager *);
@@ -167,6 +168,8 @@ extern tree cxx_simulate_record_decl (lo
  #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_t

[PATCH] c++: respect complain for -Wctad-maybe-unsupported [PR105143]

[Patrick Palka via Gcc-patches]

We were attempting to issue a -Wctad-maybe-unsupported warning even when
complain=tf_none, which led to a crash in the first testcase below and a
bogus error during SFINAE in the second testcase.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/11?

PR c++/105143

gcc/cp/ChangeLog:

* pt.cc (do_class_deduction): Check complain before issuing a
-Wctad-maybe-unsupported warning.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/nodiscard1.C: New test.
* g++.dg/warn/Wctad-maybe-unsupported4.C: New test.
---
 gcc/cp/pt.cc|  2 +-
 gcc/testsuite/g++.dg/cpp2a/nodiscard1.C | 13 +
 .../g++.dg/warn/Wctad-maybe-unsupported4.C  | 12 
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 1f0231f70e6..1805fa68440 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -30075,7 +30075,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
 
   /* If CTAD succeeded but the type doesn't have any explicit deduction
  guides, this deduction might not be what the user intended.  */
-  if (fndecl != error_mark_node && !any_dguides_p)
+  if ((complain & tf_warning) && fndecl != error_mark_node && !any_dguides_p)
 {
   if ((!DECL_IN_SYSTEM_HEADER (fndecl)
   || global_dc->dc_warn_system_headers)
diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C 
b/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
new file mode 100644
index 000..c3c5094b619
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
@@ -0,0 +1,13 @@
+// PR c++/105143
+// { dg-do compile { target c++20 } }
+// We used to crash here with "Error reporting routines re-entered".
+
+template struct A { };
+
+template using type = int;
+
+template [[nodiscard]] type get();
+
+int main() {
+  get<{}>(); // { dg-warning "nodiscard" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C 
b/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C
new file mode 100644
index 000..0d0e2fb45eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C
@@ -0,0 +1,12 @@
+// PR c++/105143
+// { dg-do compile { target c++17 } }
+// { dg-additional-options "-Werror=ctad-maybe-unsupported" }
+
+template struct A { };
+
+template class TT> auto f(...) -> decltype(TT()); // #1
+template class TT> void f(int); // #2
+
+int main() {
+  f(0); // Calls #2 without issuing a -Wctad-maybe-unsupported diagnostic.
+}
-- 
2.36.0.rc0

Re: [PATCH] c++: Fix up ICE when cplus_decl_attributes is called with error_mark_node attributes [PR104668]

[Jason Merrill via Gcc-patches]

On 3/25/22 14:08, Jason Merrill wrote:

On 3/25/22 12:34, Jakub Jelinek wrote:

Hi!

cplus_decl_attributes can be called with attributes equal to
error_mark_node, there are some spots in the function that test
it or decl_attributes it calls starts with:
   if (TREE_TYPE (*node) == error_mark_node || attributes == 
error_mark_node)

 return NULL_TREE;
But the recent PR104245 change broke this when processing_template_decl
is true.

This fixes it and also fixes an OpenMP problem with such attributes.

Ok for trunk if it passes bootstrap/regtest?

2022-03-25  Jakub Jelinek  

PR c++/104668
* decl2.cc (splice_template_attributes): Return NULL if *p is
error_mark_node.
(cplus_decl_attributes): Don't chain on OpenMP attributes if
attributes is error_mark_node.

* g++.dg/cpp0x/pr104668.C: New test.

--- gcc/cp/decl2.cc.jj    2022-03-09 09:09:55.415843331 +0100
+++ gcc/cp/decl2.cc    2022-03-25 17:17:27.769036749 +0100
@@ -1336,7 +1336,7 @@ splice_template_attributes (tree *attr_p
    tree late_attrs = NULL_TREE;
    tree *q = &late_attrs;
-  if (!p)
+  if (!p || *p == error_mark_node)
  return NULL_TREE;
    for (; *p; )
@@ -1644,6 +1644,8 @@ cplus_decl_attributes (tree *decl, tree
    && DECL_CLASS_SCOPE_P (*decl))
  error ("%q+D static data member inside of declare target 
directive",

 *decl);
+  else if (attributes == error_mark_node)
+    ;


Why not check at the beginning of the function?


You just pinged this patch, but I haven't seen a response to this question.


    else if (VAR_P (*decl)
 && (processing_template_decl
 || !cp_omp_mappable_type (TREE_TYPE (*decl
--- gcc/testsuite/g++.dg/cpp0x/pr104668.C.jj    2022-03-25 
17:25:42.280068058 +0100
+++ gcc/testsuite/g++.dg/cpp0x/pr104668.C    2022-03-25 
17:24:44.862881444 +0100

@@ -0,0 +1,13 @@
+// PR c++/104668
+// { dg-do compile { target c++11 } }
+// { dg-excess-errors "" }
+
+template 
+void sink(Ts...);
+template 
+void f(Ts...) {
+  sink([] { struct alignas:Ts) S {}; }...); }
+}
+int main() {
+  f(0);
+}

Jakub

Re: [PATCH] c++: respect complain for -Wctad-maybe-unsupported [PR105143]

[Jason Merrill via Gcc-patches]

On 4/6/22 11:11, Patrick Palka wrote:

We were attempting to issue a -Wctad-maybe-unsupported warning even when
complain=tf_none, which led to a crash in the first testcase below and a
bogus error during SFINAE in the second testcase.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/11?


OK.


PR c++/105143

gcc/cp/ChangeLog:

* pt.cc (do_class_deduction): Check complain before issuing a
-Wctad-maybe-unsupported warning.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/nodiscard1.C: New test.
* g++.dg/warn/Wctad-maybe-unsupported4.C: New test.
---
  gcc/cp/pt.cc|  2 +-
  gcc/testsuite/g++.dg/cpp2a/nodiscard1.C | 13 +
  .../g++.dg/warn/Wctad-maybe-unsupported4.C  | 12 
  3 files changed, 26 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
  create mode 100644 gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 1f0231f70e6..1805fa68440 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -30075,7 +30075,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
  
/* If CTAD succeeded but the type doesn't have any explicit deduction

   guides, this deduction might not be what the user intended.  */
-  if (fndecl != error_mark_node && !any_dguides_p)
+  if ((complain & tf_warning) && fndecl != error_mark_node && !any_dguides_p)
  {
if ((!DECL_IN_SYSTEM_HEADER (fndecl)
   || global_dc->dc_warn_system_headers)
diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C 
b/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
new file mode 100644
index 000..c3c5094b619
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard1.C
@@ -0,0 +1,13 @@
+// PR c++/105143
+// { dg-do compile { target c++20 } }
+// We used to crash here with "Error reporting routines re-entered".
+
+template struct A { };
+
+template using type = int;
+
+template [[nodiscard]] type get();
+
+int main() {
+  get<{}>(); // { dg-warning "nodiscard" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C 
b/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C
new file mode 100644
index 000..0d0e2fb45eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wctad-maybe-unsupported4.C
@@ -0,0 +1,12 @@
+// PR c++/105143
+// { dg-do compile { target c++17 } }
+// { dg-additional-options "-Werror=ctad-maybe-unsupported" }
+
+template struct A { };
+
+template class TT> auto f(...) -> decltype(TT()); // #1
+template class TT> void f(int); // #2
+
+int main() {
+  f(0); // Calls #2 without issuing a -Wctad-maybe-unsupported diagnostic.
+}

Re: [PATCH] middle-end/105165 - sorry instead of ICE for _Complex asm goto

[Jeff Law via Gcc-patches]

On 4/6/2022 6:44 AM, Richard Biener via Gcc-patches wrote:

Complex lowering cannot currently deal with asm gotos with _Complex
output operands.  Emit a sorry instead of ICEing, those should not
appear in practice.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK for trunk and branch?

Thanks,
Richard.

2022-04-06  Richard Biener  

PR middle-end/105165
* tree-complex.cc (expand_complex_asm): Sorry for asm goto
_Complex outputs.

* gcc.dg/pr105165.c: New testcase.

LGTM.
jeff

Re: [PATCH] testsuite: Add further zero size elt passing tests [PR102024]

[Jeff Law via Gcc-patches]

On 3/31/2022 9:54 AM, Jakub Jelinek via Gcc-patches wrote:

Hi!

As discussed in PR102024, zero width bitfields might not be the only ones
causing ABI issues at least on mips, zero size arrays or (in C only) zero
sized (empty) structures can be problematic too.

The following patch adds some coverage for it too.

Tested on x86_64-linux with
make check-gcc check-g++ RUNTESTFLAGS='ALT_CC_UNDER_TEST=gcc 
ALT_CXX_UNDER_TEST=g++ --target_board=unix\{-m32,-m64\} compat.exp=pr102024*'
make check-gcc check-g++ RUNTESTFLAGS='ALT_CC_UNDER_TEST=clang 
ALT_CXX_UNDER_TEST=clang++ --target_board=unix\{-m32,-m64\} 
compat.exp=pr102024*'
with gcc/g++ 10.3 and clang 11.  Everything but (expectedly)
FAIL: gcc.dg/compat/pr102024 c_compat_x_tst.o-c_compat_y_alt.o execute
FAIL: gcc.dg/compat/pr102024 c_compat_x_alt.o-c_compat_y_tst.o execute
for -m64 ALT_CC_UNDER_TEST=gcc passes.

Ok for trunk?

2022-03-31  Jakub Jelinek  

PR target/102024
* gcc.dg/compat/pr102024_test.h: Add further tests with zero sized
structures and arrays.
* g++.dg/compat/pr102024_test.h: Add further tests with zero sized
arrays.
I'd generally lean towards a new test rather than extending an existing 
one, but that's a nit that primarily helps automated testing find 
regressions, so I wouldn't consider it a requirement to break out the 
new cases.


OK as-is or with the new cases in their own testfile.

jeff

Re: [PATCH] sh: Fix up __attribute__((optimize ("Os"))) handling on SH [PR105069]

[Jeff Law via Gcc-patches]

On 3/31/2022 2:30 AM, Jakub Jelinek wrote:

Hi!

As mentioned in the PR, various tests on sh-elf ICE like:
make check-gcc RUNTESTFLAGS="compile.exp='pr104327.c pr58332.c pr81360.c 
pr84425.c'"
FAIL: gcc.c-torture/compile/pr104327.c   -O0  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr104327.c   -O0  (test for excess errors)
FAIL: gcc.c-torture/compile/pr104327.c   -O1  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr104327.c   -O1  (test for excess errors)
FAIL: gcc.c-torture/compile/pr104327.c   -O2  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr104327.c   -O2  (test for excess errors)
FAIL: gcc.c-torture/compile/pr104327.c   -O3 -g  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr104327.c   -O3 -g  (test for excess errors)
FAIL: gcc.c-torture/compile/pr104327.c   -Os  (test for excess errors)
FAIL: gcc.c-torture/compile/pr58332.c   -O0  (test for excess errors)
FAIL: gcc.c-torture/compile/pr58332.c   -O1  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr58332.c   -O1  (test for excess errors)
FAIL: gcc.c-torture/compile/pr58332.c   -O2  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr58332.c   -O2  (test for excess errors)
FAIL: gcc.c-torture/compile/pr58332.c   -O3 -g  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr58332.c   -O3 -g  (test for excess errors)
FAIL: gcc.c-torture/compile/pr58332.c   -Os  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr58332.c   -Os  (test for excess errors)
FAIL: gcc.c-torture/compile/pr81360.c   -O0  (test for excess errors)
FAIL: gcc.c-torture/compile/pr81360.c   -O1  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr81360.c   -O1  (test for excess errors)
FAIL: gcc.c-torture/compile/pr81360.c   -O2  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr81360.c   -O2  (test for excess errors)
FAIL: gcc.c-torture/compile/pr81360.c   -O3 -g  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr81360.c   -O3 -g  (test for excess errors)
FAIL: gcc.c-torture/compile/pr81360.c   -Os  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr81360.c   -Os  (test for excess errors)
FAIL: gcc.c-torture/compile/pr84425.c   -O0  (test for excess errors)
FAIL: gcc.c-torture/compile/pr84425.c   -O1  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr84425.c   -O1  (test for excess errors)
FAIL: gcc.c-torture/compile/pr84425.c   -O2  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr84425.c   -O2  (test for excess errors)
FAIL: gcc.c-torture/compile/pr84425.c   -O3 -g  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr84425.c   -O3 -g  (test for excess errors)
FAIL: gcc.c-torture/compile/pr84425.c   -Os  (internal compiler error: 
'global_options' are modified in local context)
FAIL: gcc.c-torture/compile/pr84425.c   -Os  (test for excess errors)
With the following patch, none of those tests ICE anymore, though
pr104327.c still FAILs with:
Excess errors:
/usr/src/gcc/gcc/testsuite/gcc.c-torture/compile/pr104327.c:6:1: error: 
inlining failed in call to 'always_inline' 'bar': target specific option 
mismatch
I think that would be fixable by overriding TARGET_CAN_INLINE_P
hook and allowing at least for always_inline changes in sh_div_str.

Is the following patch ok for trunk as at least a small step forward?

2022-03-31  Jakub Jelinek  

PR target/105069
* config/sh/sh.opt (mdiv=): Add Save.

OK.   IIRC sh isn't the only target that's broken for this stuff.

Jeff

Re: [PATCH] c++: Fix up ICE when cplus_decl_attributes is called with error_mark_node attributes [PR104668]

[Jakub Jelinek via Gcc-patches]

On Wed, Apr 06, 2022 at 11:18:32AM -0400, Jason Merrill wrote:
> > Why not check at the beginning of the function?
> 
> You just pinged this patch, but I haven't seen a response to this question.

I thought the
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592368.html
is the response to that.
In my =sent I have also a mail which just replied to your
> Why not check at the beginning of the function?
but didn't contain any patch, apparently that wasn't sent due to
some outgoing mail sending issues.
But the above mail is a reply to that mail citing that
reply and including a patch.

Jakub

[wwwdocs PATCH v2] document zero-width field ABI changes on MIPS

[Xi Ruoyao via Gcc-patches]

Document ABI changes in r12-7961, 7962, and 8023.  Ok for wwwdocs?

---
 htdocs/gcc-12/changes.html | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html
index 4f2ee77f..c924bca3 100644
--- a/htdocs/gcc-12/changes.html
+++ b/htdocs/gcc-12/changes.html
@@ -50,6 +50,10 @@ a work-in-progress.
 (so there is a C++ ABI incompatibility, GCC 4.4 and earlier compatible
 with GCC 12 or later, incompatible with GCC 4.5 through GCC 11).
 RISC-V has changed the handling of these already starting with GCC 10.
+As the ABI requires, MIPS takes them into account handling function
+return values so there is a C++ ABI incompatibility with GCC 4.5
+through 11.  For function arguments on MIPS, refer to
+the MIPS specific entry.
 GCC 12 on the above targets will report such incompatibilities as
 warnings or other diagnostics unless -Wno-psabi is used.
   
@@ -549,7 +553,26 @@ a work-in-progress.
   
 
 
-
+MIPS
+
+  The ABI passing arguments
+  containing zero-width fields (for example, C/C++ zero-width
+  bit-fields, GNU C/C++ zero-length arrays, and GNU C empty structs)
+  has changed.  Now a zero-width field will not prevent an aligned
+  64-bit floating-point field next to it from being passed through
+  FPR.  This is compatible with LLVM, but incompatible with previous
+  GCC releases. GCC 12 on MIPS will report such incompatibilities as
+  an inform unless -Wno-psabi is used.
+  
+  The ABI returning values
+  containing C++17 empty bases has changed.  Now an empty base will
+  not prevent an aggregate containing only one or two floating-point
+  fields from being returned through FPR.  This is compatible with
+  GCC 6 and earlier, but incompatible with GCC 7 through 11. GCC 12 on
+  MIPS will report such incompatibilities as an inform unless
+  -Wno-psabi is used.
+  
+
 
 
 
-- 
2.35.1

Re: [PATCH] --target-help: align with --help=target

[Jeff Law via Gcc-patches]

On 3/31/2022 12:56 AM, Martin Liška wrote:

Hi.

Before the patch we have:

$ gcc-11 --help | grep target-help
  --target-help    Display target specific command line options.
$ gcc-11 --help=common | grep target-help
  --target-help   Alias for --help=target.

and --target-help prints undocumented options (that was reported in 
the PR).


After my change we do:

$ gcc --help | grep target-help
  --target-help    Display target specific command line 
options (including assembler and linker options).

$ gcc --help=common | grep target-help
  --target-help   Display target specific command line 
options (including assembler and linker options).


and the undocumented options are not printed.

Ready to be installed after tests?
Thanks,
Martin

PR driver/105096

gcc/ChangeLog:

* common.opt: Document properly based on what it does.
* gcc.cc (display_help): Unify with what we have in common.opt.
* opts.cc (common_handle_option): Do not print undocumented
options.

OK
jeff

Re: [PATCH] middle-end/105049 - fix uniform_vector_p and vector CTOR gimplification

[Jeff Law via Gcc-patches]

On 3/25/2022 3:51 AM, Richard Biener via Gcc-patches wrote:

We have

   return VIEW_CONVERT_EXPR( VEC_PERM_EXPR < {<<< Unknown tree: 
compound_literal_expr
 V D.1984 = { 0 }; >>>, { 0 }} , {<<< Unknown tree: 
compound_literal_expr
 V D.1985 = { 0 }; >>>, { 0 }} , { 0, 0 } >  & {(short int) SAVE_EXPR , 
(short int) SAVE_EXPR });

where we gimplify the init CTORs to

   _1 = {{ 0 }, { 0 }};
   _2 = {{ 0 }, { 0 }};

instead of to vector constants.  That later runs into a bug in
uniform_vector_p which doesn't handle CTORs of vector elements
correctly.

The following adjusts uniform_vector_p to handle CTORs of vector
elements and also makes sure to simplify the CTORs to VECTOR_CSTs
during gimplification by re-ordering the simplification to after
CTOR flag recomputation.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  At this
point I'm leaning towards delaying the gimplification change
to stage1 - do you agree?

Thanks,
Richard.

2022-03-25  Richard Biener  

PR middle-end/105049
* gimplify.cc (gimplify_init_constructor): First gimplify,
then simplify the result to a VECTOR_CST.
* tree.cc (uniform_vector_p): Recurse for VECTOR_CST or
CONSTRUCTOR first elements.

* gcc/testsuite/gcc.dg/pr105049.c: New testcase.
Your call on the gimplify change, though I'd tend to lean towards 
waiting unless we find that it's necessary to fix a regression.  I'm 
assuming the uniform_vector_p change fixes the reported bug and that the 
gimplify change is more of a cleanup.


jeff

[PATCH] rs6000/testsuite: Skip pr105140.c

[Segher Boessenkool]

This test fails with error "AltiVec argument passed to unprototyped
function", but the code (in rs6000.c:invalid_arg_for_unprototyped_fn,
from 2005) actually tests for any vector type argument.  It also does
not fail on Darwin, not reflected here though.

Andreas, s390 has this same hook code, you may need to do the same?


Segher


2022-04-06  Segher Boessenkool  

PR target/105147
* testsuite/gcc.dg/pr105140.c: Skip for powerpc*-*-*.
---
 gcc/testsuite/gcc.dg/pr105140.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/testsuite/gcc.dg/pr105140.c b/gcc/testsuite/gcc.dg/pr105140.c
index 14bff2f7f9c5..da34e7ad6566 100644
--- a/gcc/testsuite/gcc.dg/pr105140.c
+++ b/gcc/testsuite/gcc.dg/pr105140.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-Os -w -Wno-psabi" } */
+/* { dg-skip-if "PR105147" { powerpc*-*-* } } */
 
 typedef char __attribute__((__vector_size__ (16 * sizeof (char U;
 typedef int __attribute__((__vector_size__ (16 * sizeof (int V;
-- 
1.8.3.1

[pushed] c++: -Wunused-value and array init [PR104702]

[Jason Merrill via Gcc-patches]

Here, because of problems with the new warning-control code and expressions
that change location, the suppress_warning on the INDIRECT_REF didn't work.
Those problems still need to be worked out, but it's simple to avoid needing
to use suppress_warning in the first place by using a reference instead.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/104702

gcc/cp/ChangeLog:

* init.cc (build_vec_init): Use a reference for the result.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Wunused-19.C: New test.
---
 gcc/cp/init.cc |  5 ++---
 gcc/testsuite/g++.dg/warn/Wunused-19.C | 16 
 2 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-19.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 01e762320f3..c20ed211f1d 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -4908,10 +4908,9 @@ build_vec_init (tree base, tree maxindex, tree init,
   /* Now make the result have the correct type.  */
   if (TREE_CODE (atype) == ARRAY_TYPE)
 {
-  atype = build_pointer_type (atype);
+  atype = build_reference_type (atype);
   stmt_expr = build1 (NOP_EXPR, atype, stmt_expr);
-  stmt_expr = cp_build_fold_indirect_ref (stmt_expr);
-  suppress_warning (stmt_expr /* What warning? */);
+  stmt_expr = convert_from_reference (stmt_expr);
 }
 
   return stmt_expr;
diff --git a/gcc/testsuite/g++.dg/warn/Wunused-19.C 
b/gcc/testsuite/g++.dg/warn/Wunused-19.C
new file mode 100644
index 000..2fc70b04c18
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wunused-19.C
@@ -0,0 +1,16 @@
+// PR c++/104702
+// { dg-additional-options "-fno-exceptions -Wunused-value" }
+
+struct FlyString {
+  FlyString(char const*);
+  ~FlyString();
+};
+
+struct Array { FlyString __data[1]; };
+
+void frobnicate(Array&);
+
+int main() {
+Array s_reserved_words = { "" }; // { dg-bogus "value computed is not 
used" }
+frobnicate(s_reserved_words);
+}

base-commit: eac5c12c1838d51bf05bd6f30f3f5d7bb760e398
-- 
2.27.0

Re: [PATCH] c++: Fix up ICE when cplus_decl_attributes is called with error_mark_node attributes [PR104668]

[Jason Merrill via Gcc-patches]

On 4/6/22 11:26, Jakub Jelinek wrote:

On Wed, Apr 06, 2022 at 11:18:32AM -0400, Jason Merrill wrote:

Why not check at the beginning of the function?


You just pinged this patch, but I haven't seen a response to this question.


I thought the
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592368.html
is the response to that.


Weird, AFAICT I never got that message.  But yes, that patch is OK.


In my =sent I have also a mail which just replied to your

Why not check at the beginning of the function?

but didn't contain any patch, apparently that wasn't sent due to
some outgoing mail sending issues.
But the above mail is a reply to that mail citing that
reply and including a patch.

Re: [PATCH] tree-optimization/105175 - avoid -Wvector-operation-performance

[Martin Sebor via Gcc-patches]

On 4/6/22 03:23, Richard Biener wrote:

This avoids -Wvector-operation-performance diagnostics for vectorizer
produced code.  It's unfortunate the warning_at code in
tree-vect-generic.cc needs adjustments but the diagnostic suppression
code doesn't magically suppress those otherwise.


It seems like it should, as long as the statement location hasn't
changed after the suppress_diagnostic call in tree-vect-stmts.cc.



Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Martin/David - did I miss something obvious when doing the
tree-vect-generic.cc adjustment?


The only thing I can think of is that because it's not handled in
diagnostic-spec.cc, -Wvector-operation-performance is lumped in with
all other generic warnings that also aren't handled.  It means that
they are all treated as a group.  Whether or not that's what we want
for this specific warning might be something to consider.

Martin



Thanks,
Richard.

2022-04-06  Richard Biener  

PR tree-optimization/105175
* tree-vect-stmts.cc (vectorizable_operation): Suppress
-Wvector-operation-performance if using emulated vectors.
* tree-vect-generic.cc (expand_vector_piecewise): Do not diagnose
-Wvector-operation-performance when suppressed.
(expand_vector_parallel): Likewise.
(expand_vector_comparison): Likewise.
(expand_vector_condition): Likewise.
(lower_vec_perm): Likewise.
(expand_vector_conversion): Likewise.

* gcc.dg/pr105175.c: New testcase.
---
  gcc/testsuite/gcc.dg/pr105175.c | 16 +
  gcc/tree-vect-generic.cc| 41 ++---
  gcc/tree-vect-stmts.cc  |  2 ++
  3 files changed, 45 insertions(+), 14 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr105175.c

diff --git a/gcc/testsuite/gcc.dg/pr105175.c b/gcc/testsuite/gcc.dg/pr105175.c
new file mode 100644
index 000..d8d7edb942a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr105175.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wvector-operation-performance" } */
+/* { dg-additional-options "-mno-sse" { target x86_64-*-* i?86-*-* } } */
+
+enum { QEMU_MIGRATION_COOKIE_PERSISTENT = 1 };
+struct {
+  unsigned flags;
+  unsigned flagsMandatory;
+} qemuMigrationCookieGetPersistent_mig;
+void qemuMigrationCookieGetPersistent()
+{
+  qemuMigrationCookieGetPersistent_mig.flags &=  /* { dg-bogus "will be 
expanded" } */
+  QEMU_MIGRATION_COOKIE_PERSISTENT;
+  qemuMigrationCookieGetPersistent_mig.flagsMandatory &=
+  QEMU_MIGRATION_COOKIE_PERSISTENT;
+}
diff --git a/gcc/tree-vect-generic.cc b/gcc/tree-vect-generic.cc
index 12a553ec8be..8b7227e8b58 100644
--- a/gcc/tree-vect-generic.cc
+++ b/gcc/tree-vect-generic.cc
@@ -317,8 +317,11 @@ expand_vector_piecewise (gimple_stmt_iterator *gsi, 
elem_op_func f,
int i;
location_t loc = gimple_location (gsi_stmt (*gsi));
  
-  if (nunits == 1)

-/* Do not diagnose decomposing single element vectors.  */
+  if (nunits == 1
+  || warning_suppressed_p (gsi_stmt (*gsi),
+  OPT_Wvector_operation_performance))
+/* Do not diagnose decomposing single element vectors or when
+   decomposing vectorizer produced operations.  */
  ;
else if (ret_type || !parallel_p)
  warning_at (loc, OPT_Wvector_operation_performance,
@@ -379,14 +382,16 @@ expand_vector_parallel (gimple_stmt_iterator *gsi, 
elem_op_func f, tree type,
else
  {
/* Use a single scalar operation with a mode no wider than word_mode.  
*/
+  if (!warning_suppressed_p (gsi_stmt (*gsi),
+OPT_Wvector_operation_performance))
+   warning_at (loc, OPT_Wvector_operation_performance,
+   "vector operation will be expanded with a "
+   "single scalar operation");
scalar_int_mode mode
= int_mode_for_size (tree_to_uhwi (TYPE_SIZE (type)), 0).require ();
compute_type = lang_hooks.types.type_for_mode (mode, 1);
result = f (gsi, compute_type, a, b, bitsize_zero_node,
  TYPE_SIZE (compute_type), code, type);
-  warning_at (loc, OPT_Wvector_operation_performance,
- "vector operation will be expanded with a "
- "single scalar operation");
  }
  
return result;

@@ -487,8 +492,10 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, tree 
type, tree op0,
  
  	  if (TYPE_PRECISION (ret_inner_type) != 1)

ret_inner_type = build_nonstandard_integer_type (1, 1);
- warning_at (loc, OPT_Wvector_operation_performance,
- "vector operation will be expanded piecewise");
+ if (!warning_suppressed_p (gsi_stmt (*gsi),
+OPT_Wvector_operation_performance))
+   warning_at (loc, OPT_Wvector_operation_performance,
+   "vector operation will be expanded piecewise");
  for (i = 0; i < nunits;
   i+

Re: [PATCH] libstdc++-v3 expected: Don't test ABI-variant properties in requirements.cc

[Jonathan Wakely via Gcc-patches]

Thanks!

On Wed, 6 Apr 2022, 15:42 Hans-Peter Nilsson,  wrote:

> > From: Jonathan Wakely 
> > Date: Tue, 5 Apr 2022 20:47:58 +0200
>
> > On Tue, 5 Apr 2022, 17:44 Hans-Peter Nilsson via
> > Libstdc++,
> > mailto:libstdc%2b...@gcc.gnu.org>>
> > wrote:
> > Ok to commit?
> > -- 8< --
> >
> > Without this, for a target where alignment and structure-sizes are by
> > default byte-aligned, such as cris-elf, you'll see, in libstdc++.log:
> >
> > /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127:
> error: static assertion failed
> > /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127:
> note: the comparison reduces to '(5 == 2)'
> > compiler exited with status 1
> > FAIL: 20_util/expected/requirements.cc (test for excess errors)
> > Excess errors:
> > /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127:
> error: static assertion failed
> >
> > It seems the intent is a smoke-test and that conditionals for ABI
> > properties are out of scope, so best to just delete this particular
> > line.
> >
> > The idea is to ensure the object is no larger than necessary.
> >
> > I think we could use == sizeof(void*)+alignof(void*) which
> > would be correct everywhere. Does that work for cris-elf?
>
> Oh right, yes it does.  Ok then, I'll commit this:
>
> -- 8< --
>
> [PATCH v2] libstdc++-v3 expected: Correct minimal-size test in
> requirements.cc
>
> Without this, for a target where alignment and structure-sizes are by
> default byte-aligned, such as cris-elf, you'll see, in libstdc++.log:
>
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error:
> static assertion failed
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: note:
> the comparison reduces to '(5 == 2)'
> compiler exited with status 1
> FAIL: 20_util/expected/requirements.cc (test for excess errors)
> Excess errors:
> /X/gcc/libstdc++-v3/testsuite/20_util/expected/requirements.cc:127: error:
> static assertion failed
>
> The intent of that line is to check that the object is not larger than
> necessary.
>
> libstdc++-v3/:
> * testsuite/20_util/expected/requirements.cc: Correct minimal-size
> test.
>
> ---
>  libstdc++-v3/testsuite/20_util/expected/requirements.cc | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/libstdc++-v3/testsuite/20_util/expected/requirements.cc
> b/libstdc++-v3/testsuite/20_util/expected/requirements.cc
> index 485aa338679c..a51a007a4fc3 100644
> --- a/libstdc++-v3/testsuite/20_util/expected/requirements.cc
> +++ b/libstdc++-v3/testsuite/20_util/expected/requirements.cc
> @@ -124,6 +124,6 @@ static_assert( move_assignable< void, G > );
>  // QoI properties
>  static_assert( sizeof(std::expected) == 2 );
>  static_assert( sizeof(std::expected) == 2 );
> -static_assert( sizeof(std::expected) == 2 * __alignof(void*)
> );
> +static_assert( sizeof(std::expected) == sizeof(void*) +
> __alignof(void*) );
>  static_assert( alignof(std::expected) == 1 );
>  static_assert( alignof(std::expected) == alignof(void*) );
> --
> 2.30.2
>
>

Re: [PATCH] combine, v2: Don't record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]

[Segher Boessenkool]

Hi!

So, the core of this problem is once again that regno_reg_rtx is
reallocated.  It will be another decade until we got rid of all fallout
of breaking that guarantee :-(

On Wed, Apr 06, 2022 at 10:50:40AM +0200, Jakub Jelinek wrote:
> On Tue, Apr 05, 2022 at 04:56:55PM -0500, Segher Boessenkool wrote:
> > > -  union { rtx *r; int *i; struct insn_link **l; } where;
> > > +  union { rtx *r; int *i; rtx m; struct insn_link **l; } where;
> > 
> > NAK.  It is not clear at all what "rtx m" means, esp. since there is an
> > "rtx *r" already.  In the PR you said "machine_mode m", that is clear of
> > course, can you do that instead?
> 
> So in that case something like this (i.e. the regno variant, renamed
> to subst_mode from SUBST_MODE, and naming the union member regno rather
> than m)?

I can't say I like that either, the undo for a mode change should just
store the old mode directly, anything else is too fragile.

But, whatever, we'll fix that later.  The patch is okay for trunk.
Thanks!


Segher

[PATCH] Add zero_extendditi2. Improve lxvr*x code generation.

[Michael Meissner via Gcc-patches]

>From bf51c49f1481001c7b3223474d261dcbf9365eda Mon Sep 17 00:00:00 2001
From: Michael Meissner 
Date: Fri, 1 Apr 2022 22:27:13 -0400
Subject: [PATCH] Add zero_extendditi2.  Improve lxvr*x code generation.

This pattern adds zero_extendditi2 so that if we are extending DImode to
TImode, and we want the result in a vector register, the compiler can
generate MTVSRDDD.

In addition the patterns for generating lxvr{b,h,w,d}x were tuned to allow
loading to gpr registers.  This prevents needlessly doing direct moves to
get the value into the vector registers if the gpr register was already
selected.

In updating the insn counts for two tests due to these changes, I noticed
the tests were done at -O0.  I changed this so that the tests are now done
at the normal -O2 optimization level.

I have tested this patch with bootstrap builds and running the regression
testsuite using this patch on:

Little endian power10, --with-cpu=power10
Little endian power9, --with-cpu=power9
Big endian power8, --with-cpu=power8 (both 64/32-bit tests done).

There were no regressions.  Can I check this into the master branch?

2022-04-06   Michael Meissner  

gcc/
* config/rs6000/vsx.md (vsx_lxvrx): Add support for loading to
GPR registers.
(vsx_stxvrx): Add support for storing from GPR registers.
(zero_extendditi2): New insn.

gcc/testsuite/
* gcc.target/powerpc/vsx-load-element-extend-int.c: Use -O2
instead of -O0 and update insn counts.
* gcc.target/powerpc/vsx-load-element-extend-short.c: Likewise.
* gcc.target/powerpc/zero-extend-di-ti.c: New test.

---
 gcc/config/rs6000/vsx.md  | 82 +--
 .../powerpc/vsx-load-element-extend-int.c | 36 
 .../powerpc/vsx-load-element-extend-short.c   | 35 
 .../gcc.target/powerpc/zero-extend-di-ti.c| 62 ++
 4 files changed, 164 insertions(+), 51 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/zero-extend-di-ti.c

diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index c091e5e2f47..ad971e3a1de 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -1315,14 +1315,32 @@ (define_expand "vsx_store_"
 }
 })
 
-;; Load rightmost element from load_data
-;; using lxvrbx, lxvrhx, lxvrwx, lxvrdx.
-(define_insn "vsx_lxvrx"
-  [(set (match_operand:TI 0 "vsx_register_operand" "=wa")
-   (zero_extend:TI (match_operand:INT_ISA3  1 "memory_operand" "Z")))]
-  "TARGET_POWER10"
-  "lxvrx %x0,%y1"
-  [(set_attr "type" "vecload")])
+;; Load rightmost element from load_data using lxvrbx, lxvrhx, lxvrwx, lxvrdx.
+;; Support TImode being in a GPR register to prevent generating lvxr{d,w,b}x
+;; and then two direct moves if we ultimately need the value in a GPR register.
+(define_insn_and_split "vsx_lxvrx"
+  [(set (match_operand:TI 0 "register_operand" "=r,wa")
+   (zero_extend:TI (match_operand:INT_ISA3  1 "memory_operand" "m,Z")))]
+  "TARGET_POWERPC64 && TARGET_POWER10"
+  "@
+   #
+   lxvrx %x0,%y1"
+  "&& reload_completed && int_reg_operand (operands[0], TImode)"
+  [(set (match_dup 2) (match_dup 3))
+   (set (match_dup 4) (const_int 0))]
+{
+  rtx op0 = operands[0];
+  rtx op1 = operands[1];
+
+  operands[2] = gen_lowpart (DImode, op0);
+  operands[3] = (mode == DImode
+? op1
+: gen_rtx_ZERO_EXTEND (DImode, op1));
+
+  operands[4] = gen_highpart (DImode, op0);
+}
+  [(set_attr "type" "load,vecload")
+   (set_attr "num_insns" "2,*")])
 
 ;; Store rightmost element into store_data
 ;; using stxvrbx, stxvrhx, strvxwx, strvxdx.
@@ -5019,6 +5037,54 @@ (define_expand "vsignextend_si_v2di"
   DONE;
 })
 
+;; Zero extend DI to TI.  If we don't have the MTVSRDD instruction (and LXVRDX
+;; in the case of power10), we use the machine independent code.  If we are
+;; loading up GPRs, we fall back to the old code.
+(define_insn_and_split "zero_extendditi2"
+  [(set (match_operand:TI 0 "register_operand" "=r,r, wa,&wa")
+   (zero_extend:TI (match_operand:DI 1 "register_operand"  "r,wa,r,  
wa")))]
+  "TARGET_POWERPC64 && TARGET_P9_VECTOR"
+  "@
+   #
+   #
+   mtvsrdd %x0,0,%1
+   #"
+  "&& reload_completed
+   && (int_reg_operand (operands[0], TImode)
+   || vsx_register_operand (operands[1], DImode))"
+  [(pc)]
+{
+  rtx dest = operands[0];
+  rtx src = operands[1];
+  int dest_regno = reg_or_subregno (dest);
+
+  /* Handle conversion to GPR registers.  Load up the low part and then do
+ zero out the upper part.  */
+  if (INT_REGNO_P (dest_regno))
+{
+  rtx dest_hi = gen_highpart (DImode, dest);
+  rtx dest_lo = gen_lowpart (DImode, dest);
+
+  emit_move_insn (dest_lo, src);
+  emit_move_insn (dest_hi, const0_rtx);
+  DONE;
+}
+
+  /* For settomg a VSX register from another VSX register, clear the result
+ register, and use XXPERMDI to shift the value into the lower 64-bits.  */
+  rtx dest_v2di = gen_rtx_REG (V2DImode, 

Re: [PATCH] PR middle-end/104885: Fix ICE with large stack frame on powerpc64.

[Segher Boessenkool]

On Sat, Mar 12, 2022 at 07:40:08PM -, Roger Sayle wrote:
> My recent testcase for PR c++/84964.C stress tests the middle-end by
> attempting to pass a UINT_MAX sized structure on the stack.  Although
> my fix to PR84964 avoids the ICE after sorry() on x86_64 and similar
> targets, a related issue still exists on powerpc64 (and similar
> ACCUMULATE_OUTGOING_ARGS/ARGS_GROW_DOWNWARD targets) which don't
> issue a "sorry, unimplemented" message, but instead ICE elsewhere.
> 
> After attempting several alternate fixes, the simplest solution is
> to just defensively check in mark_stack_region_used that the upper
> bound of the region lies within the allocated stack_usage_map
> array, which is of size highest_outgoing_arg_in_use.  When this isn't
> the case, the code now follows the same path as for variable sized
> regions, and uses stack_usage_watermark rather than a map.

Thanks for the fix!


Segher

Re: [PATCH] rs6000: Handle pcrel sibcalls to longcall functions [PR104894]

[Peter Bergner via Gcc-patches]

On 4/5/22 10:33 PM, Peter Bergner via Gcc-patches wrote:
> On 4/5/22 5:32 PM, Segher Boessenkool wrote:
>> On Tue, Apr 05, 2022 at 05:06:50PM -0500, Peter Bergner wrote:
>>>  
>>> +  /* Handle longcall attributes.  */
>>> +  if ((INTVAL (cookie) & CALL_LONG) != 0
>>> +  && GET_CODE (func_desc) == SYMBOL_REF)
>>
>> Don't say "!= 0" here please.
>>
>>   if (INTVAL (cookie) & CALL_LONG && SYMBOL_REF_P (func_desc))
> 
> Yes, cut & paste.  I'll change it.
[snip]
>> Don't say "Only", it is the "New" syndrome: it is out of date before you
>> know it :-(  You can just leave it away here.
> 
> Ok, I can drop "Only" and keep the rest the same.

So the updated change looks like below with the ChangeLog entry and tests being 
the same:

Is this better and ok for trunk?

Peter


@@ -25659,11 +25659,20 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx 
tlsarg, rtx cookie)
   rtx r12 = NULL_RTX;
   rtx func_addr = func_desc;
 
-  gcc_assert (INTVAL (cookie) == 0);
-
   if (global_tlsarg)
 tlsarg = global_tlsarg;
 
+  /* Handle longcall attributes.  */
+  if (INTVAL (cookie) & CALL_LONG && SYMBOL_REF_P (func_desc))
+{
+  /* 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 ());
+  func_desc = rs6000_longcall_ref (func_desc, tlsarg);
+}
+  else
+gcc_assert (INTVAL (cookie) == 0);
+
   /* For ELFv2, r12 and CTR need to hold the function address
  for an indirect call.  */
   if (GET_CODE (func_desc) != SYMBOL_REF && DEFAULT_ABI == ABI_ELFv2)

tolerate cdtors returning this in constexpr

[Alexandre Oliva via Gcc-patches]

On targets that return this from cdtors, cxx_eval_call_expression may
flag flowing off the end of a dtor.  That's preempted for ctors, and
avoided entirely when dtors return void, but when they return this,
the return value should be conceptually disregarded, without making
room for such internal ABI details to make a program ill-formed, as in
g++.dg/cpp2a/constexpr-dtor12.C on arm-eabi.

Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
arm-eabi.  Ok to install?


for  gcc/cp/ChangeLog

* constexpr.cc (cxx_eval_call_expression): Disregard dtor
result.
---
 gcc/cp/constexpr.cc |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9c40b0515747d..d8bc864ae6bcc 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2889,7 +2889,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
  else
{
  result = *ctx->global->values.get (res);
- if (result == NULL_TREE && !*non_constant_p)
+ if (result == NULL_TREE && !*non_constant_p
+ && !DECL_DESTRUCTOR_P (fun))
{
  if (!ctx->quiet)
error ("% call flows off the end "

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

set loc on call even if result is discarded

[Alexandre Oliva via Gcc-patches]

This patch fixes a divergence in line numbers in diagnostics and,
presumably, debug information, between targets whose cdtors return
this and those that don't.

The problem was visible in g++.dg/cpp2a/constexpr-dtor3.C: while the
dtor call in the cleanup for f4 was expected at the closing brace, on
returning-this targets it came up at the assignment.

The reason is convoluted: statements in cleanups have their location
information removed, to avoid bumpy debugger behavior, and then set to
the location of the end of the scope.

The cleanup dtor call has its locus cleared in both kinds of targets,
but the end-of-scope locus doesn't make it on returning-this targets.
The calls are wrapped with a cast-to-void to discard the unused return
value, and the existing logic only attached the locus to the
conversion NOP_EXPR.

The call thus remains locus-less.  When constexpr logic copies and
evals the body, it sets unset locations; while copying cleanups, the
locus is taken from the cleanup expression, rather than matching the
end-of-scope locus set by the parser.  So we end up with different
locations.

This patch sets the locus of the call even when it's wrapped by a
convert-to-void NOP_EXPR, so it won't diverge any more.

Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
arm-eabi.  Ok to install?


for  gcc/ChangeLog

* tree.cc (protected_set_expr_location): Propagate locus to
call wrapped in cast-to-void.
---
 gcc/tree.cc |   13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gcc/tree.cc b/gcc/tree.cc
index ec200e9a7eb43..228d279ab0aa1 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -5372,7 +5372,18 @@ void
 protected_set_expr_location (tree t, location_t loc)
 {
   if (CAN_HAVE_LOCATION_P (t))
-SET_EXPR_LOCATION (t, loc);
+{
+  SET_EXPR_LOCATION (t, loc);
+  /* Avoid locus differences for C++ cdtor calls depending on whether
+cdtor_returns_this: a conversion to void is added to discard the return
+value, and this conversion ends up carrying the location, and when it
+gets discarded, the location is lost.  So hold it in the call as
+well.  */
+  if (TREE_CODE (t) == NOP_EXPR
+ && TREE_TYPE (t) == void_type_node
+ && TREE_CODE (TREE_OPERAND (t, 0)) == CALL_EXPR)
+   SET_EXPR_LOCATION (TREE_OPERAND (t, 0), loc);
+}
   else if (t && TREE_CODE (t) == STATEMENT_LIST)
 {
   t = expr_single (t);


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

Re: tolerate cdtors returning this in constexpr

[Marek Polacek via Gcc-patches]

On Wed, Apr 06, 2022 at 04:36:49PM -0300, Alexandre Oliva via Gcc-patches wrote:
> 
> On targets that return this from cdtors, cxx_eval_call_expression may
> flag flowing off the end of a dtor.  That's preempted for ctors, and
> avoided entirely when dtors return void, but when they return this,
> the return value should be conceptually disregarded, without making
> room for such internal ABI details to make a program ill-formed, as in
> g++.dg/cpp2a/constexpr-dtor12.C on arm-eabi.
> 
> Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
> arm-eabi.  Ok to install?
> 
> 
> for  gcc/cp/ChangeLog
> 
>   * constexpr.cc (cxx_eval_call_expression): Disregard dtor
>   result.
> ---
>  gcc/cp/constexpr.cc |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 9c40b0515747d..d8bc864ae6bcc 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -2889,7 +2889,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, 
> tree t,
> else
>   {
> result = *ctx->global->values.get (res);
> -   if (result == NULL_TREE && !*non_constant_p)
> +   if (result == NULL_TREE && !*non_constant_p
> +   && !DECL_DESTRUCTOR_P (fun))

Would it make sense to use 

  && !(DECL_DESTRUCTOR_P (fun) && targetm.cxx.cdtor_returns_this ())

instead?

>   {
> if (!ctx->quiet)
>   error ("% call flows off the end "

Marek

Re: [PATCH] Add zero_extendditi2. Improve lxvr*x code generation.

[will schmidt via Gcc-patches]

On Wed, 2022-04-06 at 14:21 -0400, Michael Meissner wrote:
> From bf51c49f1481001c7b3223474d261dcbf9365eda Mon Sep 17 00:00:00 2001
> From: Michael Meissner 
> Date: Fri, 1 Apr 2022 22:27:13 -0400
> Subject: [PATCH] Add zero_extendditi2.  Improve lxvr*x code generation.
> 

Hi,

> This pattern adds zero_extendditi2 so that if we are extending DImode to
> TImode, and we want the result in a vector register, the compiler can
> generate MTVSRDDD.
> 
> In addition the patterns for generating lxvr{b,h,w,d}x were tuned to allow
> loading to gpr registers.  This prevents needlessly doing direct moves to
> get the value into the vector registers if the gpr register was already
> selected.

ok

> 
> In updating the insn counts for two tests due to these changes, I noticed
> the tests were done at -O0.  I changed this so that the tests are now done
> at the normal -O2 optimization level.

Per the comments (which you fixed up later in patch), I note they were
deliberately done at -O0 since under higher optimizations gcc would
generate other load instructions during those tests.  Presumably with
these changes that is no longer the case.  :-)
> 
> I have tested this patch with bootstrap builds and running the regression
> testsuite using this patch on:
> 
>   Little endian power10, --with-cpu=power10
>   Little endian power9, --with-cpu=power9
>   Big endian power8, --with-cpu=power8 (both 64/32-bit tests done).
> 
> There were no regressions.  Can I check this into the master branch?
> 
> 2022-04-06   Michael Meissner  
> 
> gcc/
>   * config/rs6000/vsx.md (vsx_lxvrx): Add support for loading to
>   GPR registers.
>   (vsx_stxvrx): Add support for storing from GPR registers.
>   (zero_extendditi2): New insn.
> 
> gcc/testsuite/
>   * gcc.target/powerpc/vsx-load-element-extend-int.c: Use -O2
>   instead of -O0 and update insn counts.
>   * gcc.target/powerpc/vsx-load-element-extend-short.c: Likewise.
>   * gcc.target/powerpc/zero-extend-di-ti.c: New test.
> 
> ---
>  gcc/config/rs6000/vsx.md  | 82 +--
>  .../powerpc/vsx-load-element-extend-int.c | 36 
>  .../powerpc/vsx-load-element-extend-short.c   | 35 
>  .../gcc.target/powerpc/zero-extend-di-ti.c| 62 ++
>  4 files changed, 164 insertions(+), 51 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/zero-extend-di-ti.c
> 
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index c091e5e2f47..ad971e3a1de 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -1315,14 +1315,32 @@ (define_expand "vsx_store_"
>  }
>  })
> 
> -;; Load rightmost element from load_data
> -;; using lxvrbx, lxvrhx, lxvrwx, lxvrdx.
> -(define_insn "vsx_lxvrx"
> -  [(set (match_operand:TI 0 "vsx_register_operand" "=wa")
> - (zero_extend:TI (match_operand:INT_ISA3  1 "memory_operand" "Z")))]
> -  "TARGET_POWER10"
> -  "lxvrx %x0,%y1"
> -  [(set_attr "type" "vecload")])
> +;; Load rightmost element from load_data using lxvrbx, lxvrhx, lxvrwx, 
> lxvrdx.
> +;; Support TImode being in a GPR register to prevent generating lvxr{d,w,b}x
> +;; and then two direct moves if we ultimately need the value in a GPR 
> register.
> +(define_insn_and_split "vsx_lxvrx"
> +  [(set (match_operand:TI 0 "register_operand" "=r,wa")
> + (zero_extend:TI (match_operand:INT_ISA3  1 "memory_operand" "m,Z")))]
> +  "TARGET_POWERPC64 && TARGET_POWER10"
> +  "@
> +   #
> +   lxvrx %x0,%y1"
> +  "&& reload_completed && int_reg_operand (operands[0], TImode)"
> +  [(set (match_dup 2) (match_dup 3))
> +   (set (match_dup 4) (const_int 0))]
> +{
> +  rtx op0 = operands[0];
> +  rtx op1 = operands[1];
> +
> +  operands[2] = gen_lowpart (DImode, op0);
> +  operands[3] = (mode == DImode
> +  ? op1
> +  : gen_rtx_ZERO_EXTEND (DImode, op1));
> +
> +  operands[4] = gen_highpart (DImode, op0);
> +}
> +  [(set_attr "type" "load,vecload")
> +   (set_attr "num_insns" "2,*")])
> 
>  ;; Store rightmost element into store_data
>  ;; using stxvrbx, stxvrhx, strvxwx, strvxdx.
> @@ -5019,6 +5037,54 @@ (define_expand "vsignextend_si_v2di"
>DONE;
>  })
> 
> +;; Zero extend DI to TI.  If we don't have the MTVSRDD instruction (and 
> LXVRDX
> +;; in the case of power10), we use the machine independent code.  If we are
> +;; loading up GPRs, we fall back to the old code.

In this context it's not clear what is the "old code" ?
The mtvsrdd
instruction is referenced in this code path.  I see no direct reference
to lxvrdx here, though I suppose it's assumed somewhere behind the
emit_ calls.


> +(define_insn_and_split "zero_extendditi2"
> +  [(set (match_operand:TI 0 "register_operand" "=r,r, 
> wa,&wa")
> + (zero_extend:TI (match_operand:DI 1 "register_operand"  "r,wa,r,  
> wa")))]
> +  "TARGET_POWERPC64 && TARGET_P9_VECTOR"
> +  "@
> +   #
> +   #
> +   mtvsrdd %x0,0,%1
> +   #"
> +  "&& reload_completed
> +   && (int_reg_op

Re: tolerate cdtors returning this in constexpr

[Alexandre Oliva via Gcc-patches]

On Apr  6, 2022, Marek Polacek  wrote:

> On Wed, Apr 06, 2022 at 04:36:49PM -0300, Alexandre Oliva via Gcc-patches 
> wrote:
>> else
>> {
>> result = *ctx->global->values.get (res);
>> -  if (result == NULL_TREE && !*non_constant_p)
>> +  if (result == NULL_TREE && !*non_constant_p
>> +  && !DECL_DESTRUCTOR_P (fun))

> Would it make sense to use 

>   && !(DECL_DESTRUCTOR_P (fun) && targetm.cxx.cdtor_returns_this ())

> instead?

Just before the 'else' above, there's a test for a void result type,
which covers the '!targetm.cxx.cdtor_returns_this ()' case, so IMHO that
would be excessive.

And then, avoiding that error for dtors is not something that should be
done only on some targets: no dtor should ever trigger that error.

-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 

[committed] jit: fix location of .png files for "make jit.pdf" [PR102824]

[David Malcolm via Gcc-patches]

On Fri, 2022-04-01 at 12:26 -0400, Eric Gallager wrote:
> On Fri, Apr 1, 2022 at 9:28 AM David Malcolm via Gcc-patches
>  wrote:
> > 
> > Further jit doc fixes, which fix links to
> > gcc_jit_function_type_get_param_type and gcc_jit_struct_get_field.
> > 
> > I also regenerated libgccjit.texi (not included in the diff below).
> > 
> > Tested with "make html" and with a bootstrap.
> 
>  Could you test with `make pdf` and `make dvi` too, to see if this
> fixes 102824?
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102824

The following seems to fix "make pdf"

"make jit.dvi" is still failing after this; it seems to be looking for
.eps files for the images.


>From c9a22b67954f4cd46a739bfe1ddedd544dbfc133 Mon Sep 17 00:00:00 2001
From: David Malcolm 
Date: Thu, 31 Mar 2022 12:04:54 -0400
Subject: [committed] jit: fix location of .png files for "make jit.pdf" 
[PR102824]

"make jit.pdf" seems to be looking in
  gcc/jit/docs/_build/texinfo/libgccjit-figures
for the .png files, but they were in the source tree in:
  gcc/jit/docs/_build/texinfo

Fix "make jit.pdf" via:
  git mv \
gcc/jit/docs/_build/texinfo/*.png \
gcc/jit/docs/_build/texinfo/libgccjit-figures

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Manually verified that it fixes "make jit.pdf"; the generated
libgccjit.pdf seems to correctly contain the images.

Pushed to trunk as r12-8031-g790e9814454662.

gcc/jit/ChangeLog:
PR jit/102824
* docs/_build/texinfo/factorial.png: Move to...
* docs/_build/texinfo/libgccjit-figures/factorial.png: ...here.
* docs/_build/texinfo/factorial1.png: Move to...
* docs/_build/texinfo/libgccjit-figures/factorial1.png: ...here.
* docs/_build/texinfo/sum-of-squares.png: Move to...
* docs/_build/texinfo/libgccjit-figures/sum-of-squares.png: ...here.
* docs/_build/texinfo/sum-of-squares1.png: Move to...
* docs/_build/texinfo/libgccjit-figures/sum-of-squares1.png: ...here.

Signed-off-by: David Malcolm 
---
 .../texinfo/{ => libgccjit-figures}/factorial.png   | Bin
 .../texinfo/{ => libgccjit-figures}/factorial1.png  | Bin
 .../{ => libgccjit-figures}/sum-of-squares.png  | Bin
 .../{ => libgccjit-figures}/sum-of-squares1.png | Bin
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename gcc/jit/docs/_build/texinfo/{ => libgccjit-figures}/factorial.png (100%)
 rename gcc/jit/docs/_build/texinfo/{ => libgccjit-figures}/factorial1.png 
(100%)
 rename gcc/jit/docs/_build/texinfo/{ => libgccjit-figures}/sum-of-squares.png 
(100%)
 rename gcc/jit/docs/_build/texinfo/{ => libgccjit-figures}/sum-of-squares1.png 
(100%)

diff --git a/gcc/jit/docs/_build/texinfo/factorial.png 
b/gcc/jit/docs/_build/texinfo/libgccjit-figures/factorial.png
similarity index 100%
rename from gcc/jit/docs/_build/texinfo/factorial.png
rename to gcc/jit/docs/_build/texinfo/libgccjit-figures/factorial.png
diff --git a/gcc/jit/docs/_build/texinfo/factorial1.png 
b/gcc/jit/docs/_build/texinfo/libgccjit-figures/factorial1.png
similarity index 100%
rename from gcc/jit/docs/_build/texinfo/factorial1.png
rename to gcc/jit/docs/_build/texinfo/libgccjit-figures/factorial1.png
diff --git a/gcc/jit/docs/_build/texinfo/sum-of-squares.png 
b/gcc/jit/docs/_build/texinfo/libgccjit-figures/sum-of-squares.png
similarity index 100%
rename from gcc/jit/docs/_build/texinfo/sum-of-squares.png
rename to gcc/jit/docs/_build/texinfo/libgccjit-figures/sum-of-squares.png
diff --git a/gcc/jit/docs/_build/texinfo/sum-of-squares1.png 
b/gcc/jit/docs/_build/texinfo/libgccjit-figures/sum-of-squares1.png
similarity index 100%
rename from gcc/jit/docs/_build/texinfo/sum-of-squares1.png
rename to gcc/jit/docs/_build/texinfo/libgccjit-figures/sum-of-squares1.png
-- 
2.26.3

[PATCH] PR fortran/105184 - ICE in gfc_array_init_size, at fortran/trans-array.cc:5841

[Harald Anlauf via Gcc-patches]

Dear all,

the logic for checking the allocate-coshape-spec in an ALLOCATE
statement was sort of sideways, and allowed to pass invalid
specifications to the code generation.

The fix seems obvious (to me).

Regtested on x86_64-pc-linux-gnu.  OK for mainline?
(12 or wait for 13?).

Thanks,
Harald

From 2adcdbd40e3a64d1f2d42eb0e0fdcc7e593da137 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Wed, 6 Apr 2022 22:24:21 +0200
Subject: [PATCH] Fortran: fix logic for checking coshape specification in
 ALLOCATE statement

gcc/fortran/ChangeLog:

	PR fortran/105184
	* resolve.cc (resolve_allocate_expr): Fix logic for checking
	allocate-coshape-spec in ALLOCATE statement.

gcc/testsuite/ChangeLog:

	PR fortran/105184
	* gfortran.dg/coarray_44.f90: Adjust expected output.
	* gfortran.dg/coarray_50.f90: New test.
---
 gcc/fortran/resolve.cc   | 10 +-
 gcc/testsuite/gfortran.dg/coarray_44.f90 |  2 ++
 gcc/testsuite/gfortran.dg/coarray_50.f90 | 17 +
 3 files changed, 24 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/coarray_50.f90

diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 21c8797c938..45a04dab703 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -8108,12 +8108,12 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code, bool *array_alloc_wo_spec)
 	goto failure;

 	  case  DIMEN_RANGE:
-	if (ar->start[i] == 0 || ar->end[i] == 0)
+	// F2018:R937:
+	// allocate-coshape-spec is [ lower-bound-expr : ] upper-bound-expr
+	if (ar->start[i] == 0 || ar->end[i] == 0 || ar->stride[i] != NULL)
 	  {
-		/* If ar->stride[i] is NULL, we issued a previous error.  */
-		if (ar->stride[i] == NULL)
-		  gfc_error ("Bad array specification in ALLOCATE statement "
-			 "at %L", &e->where);
+		gfc_error ("Bad array specification in ALLOCATE statement "
+			   "at %L", &e->where);
 		goto failure;
 	  }
 	else if (gfc_dep_compare_expr (ar->start[i], ar->end[i]) == 1)
diff --git a/gcc/testsuite/gfortran.dg/coarray_44.f90 b/gcc/testsuite/gfortran.dg/coarray_44.f90
index 15fb8c76ce4..f83e3e9b19d 100644
--- a/gcc/testsuite/gfortran.dg/coarray_44.f90
+++ b/gcc/testsuite/gfortran.dg/coarray_44.f90
@@ -10,3 +10,5 @@ program pr70071
   allocate (z(2)[1::2,*])  ! { dg-error "Bad array dimension" }
   allocate (z(1::2)[2,*])  ! { dg-error "Bad array specification in ALLOCATE" }
 end program pr70071
+
+! { dg-prune-output "Bad array specification in ALLOCATE statement" }
diff --git a/gcc/testsuite/gfortran.dg/coarray_50.f90 b/gcc/testsuite/gfortran.dg/coarray_50.f90
new file mode 100644
index 000..9e8bd5d53de
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_50.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! { dg-options "-fcoarray=single" }
+!
+! PR fortran/105184
+! Based on testcases by Gerhard Steinmetz
+
+program p
+  real, allocatable :: x[:,:]
+  integer :: n = 2
+  allocate (x[  n, *])
+  allocate (x[1:n, *])
+  allocate (x[n:n, *])
+  allocate (x[ :n,   *]) ! { dg-error "Bad array specification" }
+  allocate (x[::n,   *]) ! { dg-error "Bad array specification" }
+  allocate (x[ :1:1, *]) ! { dg-error "Bad array specification" }
+  allocate (x[1:n:n, *]) ! { dg-error "Bad array specification" }
+end
--
2.34.1

Re: [PATCH] c, c++: attribute format on a ctor with a vbase [PR101833, PR47634]

[Jason Merrill via Gcc-patches]

On 4/1/22 15:14, Marek Polacek wrote:

Attribute format takes three arguments: archetype, string-index, and
first-to-check.  The last two specify the position in the function
parameter list.  r63030 clarified that "Since non-static C++ methods have
an implicit this argument, the arguments of such methods should be counted
from two, not one, when giving values for string-index and first-to-check."
Therefore one has to write

   struct D {
 D(const char *, ...) __attribute__((format(printf, 2, 3)));
   };

However -- and this is the problem in this PR -- ctors with virtual
bases also get two additional parameters: the in-charge parameter and
the VTT parameter (added in maybe_retrofit_in_chrg).  In fact we'll end up
with two clones of the ctor: an in-charge and a not-in-charge version (see
build_cdtor_clones).  That means that the argument position the user
specified in the attribute argument will refer to different arguments,
depending on which constructor we're currently dealing with.  This can
cause a range of problems: wrong errors, confusing warnings, or crashes.

This patch corrects that; for C we don't have to do anything, and in C++
we can use num_artificial_parms_for.  It would be wrong to rewrite the
attributes the user supplied, so I've added an extra parameter called
adjust_pos.

Attribute format_arg is not affected, because it requires that the
function returns "const char *" which will never be the case for cdtors.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

PR c++/101833
PR c++/47634

gcc/c-family/ChangeLog:

* c-attribs.cc (positional_argument): Add new argument adjust_pos,
use it.
* c-common.cc (check_function_arguments): Pass fndecl to
check_function_format.
* c-common.h (check_function_format): Adjust declaration.
(maybe_adjust_arg_pos_for_attribute): Add.
(positional_argument): Adjust declaration.
* c-format.cc (decode_format_attr): Add fndecl argument.  Pass it to
maybe_adjust_arg_pos_for_attribute.  Adjust calls to get_constant.


I wonder about, instead of adding another parameter, allowing the 
current fntype parameter to be the fndecl when we have one.


And then that gets passed down into positional_argument, so we can call 
maybe_adjust_arg_pos_for_attribute there, and adjust the return value 
appropriately so we don't need the extra adjustment in get_constant?



(handle_format_arg_attribute): Pass 0 to get_constant.
(get_constant): Add new argument adjust_pos, use it.
(check_function_format): Add fndecl argument.  Pass it to
decode_format_attr.
(handle_format_attribute): Get the fndecl from node[2].  Pass it to
decode_format_attr.

gcc/c/ChangeLog:

* c-objc-common.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/cp/ChangeLog:

* tree.cc (maybe_adjust_arg_pos_for_attribute): New.

gcc/testsuite/ChangeLog:

* g++.dg/ext/attr-format-arg1.C: New test.
* g++.dg/ext/attr-format1.C: New test.
* g++.dg/ext/attr-format2.C: New test.
* g++.dg/ext/attr-format3.C: New test.
---
  gcc/c-family/c-attribs.cc   | 14 ---
  gcc/c-family/c-common.cc|  4 +-
  gcc/c-family/c-common.h |  5 ++-
  gcc/c-family/c-format.cc| 46 +
  gcc/c/c-objc-common.cc  |  9 
  gcc/cp/tree.cc  | 19 +
  gcc/testsuite/g++.dg/ext/attr-format-arg1.C | 26 
  gcc/testsuite/g++.dg/ext/attr-format1.C | 32 ++
  gcc/testsuite/g++.dg/ext/attr-format2.C | 38 +
  gcc/testsuite/g++.dg/ext/attr-format3.C | 15 +++
  10 files changed, 182 insertions(+), 26 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format-arg1.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format1.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format2.C
  create mode 100644 gcc/testsuite/g++.dg/ext/attr-format3.C

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 111a33f405a..6e17847ec9e 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -599,12 +599,15 @@ attribute_takes_identifier_p (const_tree attr_id)
 matching all C integral types except bool.  If successful, return
 POS after default conversions, if any.  Otherwise, issue appropriate
 warnings and return null.  A non-zero 1-based ARGNO should be passed
-   in by callers only for attributes with more than one argument.  */
+   in by callers only for attributes with more than one argument.
+   ADJUST_POS is used and non-zero in C++ when the function type has
+   invisible parameters generated by the compiler, such as the in-charge
+   or VTT parameters.  */
  
  tree

  positional_argument (const_tree fntype, const_tree atname, tree pos,
 tree_code code, int argno /* = 0 */,
- 

Re: -Wformat-overflow handling for %b and %B directives in C2X standard

[Frolov Daniil via Gcc-patches]

Hello! Thanks for your feedback. I've tried to take into account your
comments. New patch applied to the letter.

The only thing I have not removed is the check_std_c2x () function. From my
point of view -Wformat-overflow shouldn't be thrown if the standard < C2X.
So it's protection for false triggering.

сб, 2 апр. 2022 г. в 01:15, Marek Polacek :

> On Sat, Apr 02, 2022 at 12:19:47AM +0500, Frolov Daniil via Gcc-patches
> wrote:
> > Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
> > directives in the sprintf function. I've added a relevant issue in
> bugzilla
> > (bug #105129).
> > I attach a patch with a possible solution to the letter.
>
> Thanks for the patch.  Support for C2X %b, %B formats is relatively new
> (Oct 2021) so it looks like gimple-ssa-sprintf.cc hasn't caught up.
>
> This is not a regression, so should probably wait till GCC 13.  Anyway...
>
> > From 2051344e9500651f6e94c44cbc7820715382b957 Mon Sep 17 00:00:00 2001
> > From: Frolov Daniil 
> > Date: Fri, 1 Apr 2022 00:47:03 +0500
> > Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> >
> > testsuite: add tests to check -Wformat-overflow on %b.
> > Wformat-overflow1.c is compiled using -std=c2x so warning has to
> > be throwed
> >
> > Wformat-overflow2.c doesn't throw warnings cause c2x std isn't
> > used
> >
> > gcc/ChangeLog:
> >
> >   * gimple-ssa-sprintf.cc
> > (check_std_c2x): New function
> >   (fmtresult::type_max_digits): add base == 2 handling
> >   (tree_digits): add handle for base == 2
> >   (format_integer): now handle %b and %B using base = 2
> >   (parse_directive): add cases to handle %b and %B directives
> >   (compute_format_length): add handling for base = 2
>
> The descriptions should start with a capital letter and end with a period,
> like "Handle base == 2."
>
> > gcc/testsuite/ChangeLog:
> >
> >   * gcc.dg/Wformat-overflow1.c: New test. (using -std=c2x)
> >   * gcc.dg/Wformat-overflow2.c: New test. (-std=c11 no warning)
>
> You can just say "New test."
>
> > ---
> >  gcc/gimple-ssa-sprintf.cc| 42 
> >  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 
> >  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 +
> >  3 files changed, 79 insertions(+), 7 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> >
> > diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> > index c93f12f90b5..7f68c2b6e51 100644
> > --- a/gcc/gimple-ssa-sprintf.cc
> > +++ b/gcc/gimple-ssa-sprintf.cc
> > @@ -107,6 +107,15 @@ namespace {
> >
> >  static int warn_level;
> >
> > +/* b_overflow_flag depends on the current standart when using gcc */
>
> "standard"
>
> /* Comments should be formatted like this.  */
>
> > +static bool b_overflow_flag;
> > +
> > +/* check is current standart version equals C2X*/
> > +static bool check_std_c2x ()
> > +{
> > +  return !strcmp (lang_hooks.name, "GNU C2X");
> > +}
>
> Is this really needed?  ISTM that this new checking shouldn't depend on
> -std=c2x.  If not using C2X, you only get a warning if -Wpedantic.  So
> I think you should remove b_overflow_flag.
>
> >  /* The minimum, maximum, likely, and unlikely maximum number of bytes
> > of output either a formatting function or an individual directive
> > can result in.  */
> > @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
> >unsigned prec = TYPE_PRECISION (type);
> >switch (base)
> >  {
> > +case 2:
> > +  return prec;
> >  case 8:
> >return (prec + 2) / 3;
> >  case 10:
> > @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec,
> bool plus, bool prefix)
> >
> >/* Adjust a non-zero value for the base prefix, either hexadecimal,
> >   or, unless precision has resulted in a leading zero, also octal.
> */
> > -  if (prefix && absval && (base == 16 || prec <= ndigs))
> > +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
> >  {
> >if (base == 8)
> >   res += 1;
> > -  else if (base == 16)
> > +  else if (base == 16 || base == 2) /*0x...(0X...) and
> 0b...(0B...)*/
> >   res += 2;
> >  }
> >
> > @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >  case 'u':
> >base = 10;
> >break;
> > +case 'b':
> > +case 'B':
> > +  base = 2;
> > +  break;
> >  case 'o':
> >base = 8;
> >break;
> > @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >
> >/* Bump up the counters if WIDTH is greater than LEN.  */
> >res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > -  (sign | maybebase) + (base ==
> 16));
> > +  (sign | maybebase) + (base == 2
> ||

Re: tolerate cdtors returning this in constexpr

[Jason Merrill via Gcc-patches]

On 4/6/22 15:36, Alexandre Oliva wrote:

Please adjust your patch subject lines for the new guidelines adopted as 
part of the git transition:


https://gcc.gnu.org/contribute.html#patches

e.g. [PATCH] c++: tolerate cdtors returning this in constexpr [PRn]


On targets that return this from cdtors, cxx_eval_call_expression may
flag flowing off the end of a dtor.  That's preempted for ctors, and
avoided entirely when dtors return void, but when they return this,
the return value should be conceptually disregarded, without making
room for such internal ABI details to make a program ill-formed, as in
g++.dg/cpp2a/constexpr-dtor12.C on arm-eabi.


Is there a PR for this issue?

The patch looks fine, but why doesn't the implicit return 'this' on 
arm-eabi already make the result non-null?


Thanks,
Jason


Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
arm-eabi.  Ok to install?


for  gcc/cp/ChangeLog

* constexpr.cc (cxx_eval_call_expression): Disregard dtor
result.
---
  gcc/cp/constexpr.cc |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9c40b0515747d..d8bc864ae6bcc 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2889,7 +2889,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
  else
{
  result = *ctx->global->values.get (res);
- if (result == NULL_TREE && !*non_constant_p)
+ if (result == NULL_TREE && !*non_constant_p
+ && !DECL_DESTRUCTOR_P (fun))
{
  if (!ctx->quiet)
error ("% call flows off the end "

Re: set loc on call even if result is discarded

[Jason Merrill via Gcc-patches]

On 4/6/22 15:37, Alexandre Oliva wrote:

Need to adjust this subject line, as well.


This patch fixes a divergence in line numbers in diagnostics and,
presumably, debug information, between targets whose cdtors return
this and those that don't.

The problem was visible in g++.dg/cpp2a/constexpr-dtor3.C: while the
dtor call in the cleanup for f4 was expected at the closing brace, on
returning-this targets it came up at the assignment.

The reason is convoluted: statements in cleanups have their location
information removed, to avoid bumpy debugger behavior, and then set to
the location of the end of the scope.

The cleanup dtor call has its locus cleared in both kinds of targets,
but the end-of-scope locus doesn't make it on returning-this targets.
The calls are wrapped with a cast-to-void to discard the unused return
value, and the existing logic only attached the locus to the
conversion NOP_EXPR.

The call thus remains locus-less.  When constexpr logic copies and
evals the body, it sets unset locations; while copying cleanups, the
locus is taken from the cleanup expression, rather than matching the
end-of-scope locus set by the parser.  So we end up with different
locations.

This patch sets the locus of the call even when it's wrapped by a
convert-to-void NOP_EXPR, so it won't diverge any more.

Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
arm-eabi.  Ok to install?


for  gcc/ChangeLog

* tree.cc (protected_set_expr_location): Propagate locus to
call wrapped in cast-to-void.


I'm reluctant to put this C++-specific change in a simple function 
shared by all languages; how about handling it in set_cleanup_locs instead?



---
  gcc/tree.cc |   13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gcc/tree.cc b/gcc/tree.cc
index ec200e9a7eb43..228d279ab0aa1 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -5372,7 +5372,18 @@ void
  protected_set_expr_location (tree t, location_t loc)
  {
if (CAN_HAVE_LOCATION_P (t))
-SET_EXPR_LOCATION (t, loc);
+{
+  SET_EXPR_LOCATION (t, loc);
+  /* Avoid locus differences for C++ cdtor calls depending on whether
+cdtor_returns_this: a conversion to void is added to discard the return
+value, and this conversion ends up carrying the location, and when it
+gets discarded, the location is lost.  So hold it in the call as
+well.  */
+  if (TREE_CODE (t) == NOP_EXPR
+ && TREE_TYPE (t) == void_type_node
+ && TREE_CODE (TREE_OPERAND (t, 0)) == CALL_EXPR)
+   SET_EXPR_LOCATION (TREE_OPERAND (t, 0), loc);
+}
else if (t && TREE_CODE (t) == STATEMENT_LIST)
  {
t = expr_single (t);

Re: tolerate cdtors returning this in constexpr

[Jason Merrill via Gcc-patches]

On 4/6/22 15:45, Marek Polacek via Gcc-patches wrote:

On Wed, Apr 06, 2022 at 04:36:49PM -0300, Alexandre Oliva via Gcc-patches wrote:


On targets that return this from cdtors, cxx_eval_call_expression may
flag flowing off the end of a dtor.  That's preempted for ctors, and
avoided entirely when dtors return void, but when they return this,
the return value should be conceptually disregarded, without making
room for such internal ABI details to make a program ill-formed, as in
g++.dg/cpp2a/constexpr-dtor12.C on arm-eabi.

Regstrapped on x86_64-linux-gnu, also verified the testcase fix on
arm-eabi.  Ok to install?


for  gcc/cp/ChangeLog

* constexpr.cc (cxx_eval_call_expression): Disregard dtor
result.
---
  gcc/cp/constexpr.cc |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9c40b0515747d..d8bc864ae6bcc 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2889,7 +2889,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree 
t,
  else
{
  result = *ctx->global->values.get (res);
- if (result == NULL_TREE && !*non_constant_p)
+ if (result == NULL_TREE && !*non_constant_p
+ && !DECL_DESTRUCTOR_P (fun))


Would it make sense to use

   && !(DECL_DESTRUCTOR_P (fun) && targetm.cxx.cdtor_returns_this ())

instead?


I don't think that's necessary; it's the destructorness that makes it 
OK.  And if it doesn't return this we would have taken the VOID_TYPE_P 
branch before.


Jason

[PATCH] mips: testsuite: enforce -ffat-lto-objects for pr102024-4.c

[Xi Ruoyao via Gcc-patches]

Another brown paper bag fix for MIPS :(.

This failure was not detected running mips.exp=pr102024-* with a cross
compiler, so I just spotted it now running the test natively.

---

The body of func is optimized away with -flto -fno-fat-lto-objects, so
the psABI inform is not emitted, causing a test failure.

gcc/testsuite/

* gcc.target/mips/pr102024-4.c (dg-options): Add
-ffat-lto-objects.
---
 gcc/testsuite/gcc.target/mips/pr102024-4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.target/mips/pr102024-4.c 
b/gcc/testsuite/gcc.target/mips/pr102024-4.c
index 2147cc769d0..ea49e890ee5 100644
--- a/gcc/testsuite/gcc.target/mips/pr102024-4.c
+++ b/gcc/testsuite/gcc.target/mips/pr102024-4.c
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-options "-mabi=64 -mhard-float" }
+// { dg-options "-mabi=64 -mhard-float -ffat-lto-objects" }
 
 struct __attribute__((aligned(16))) test {
   int x[0];
-- 
2.35.1

[PATCH] toplev: use HOST_WIDE_INT for local_tick to prevent overflow

[Jason A. Donenfeld via Gcc-patches]

In gcc/toplev.c, there's the comment:

  /* A local time stamp derived from the time of compilation. It will be
 zero if the system cannot provide a time.  It will be -1u, if the
 user has specified a particular random seed.  */
  unsigned local_tick;

This is affirmed by init_local_tick()'s comment:

  /* Initialize local_tick with the time of day, or -1 if
 flag_random_seed is set.  */
  static void init_local_tick (void)

And indeed we see it assigned -1 when flag_random_seed != NULL:

  else
local_tick = -1;

So far so good. However, in the case where flag_random_seed == NULL,
local_tick is assigned like this:

  struct timeval tv;
  gettimeofday (&tv, NULL);
  local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;

local_tick is currently of type "unsigned int". Somewhat often, that
expression calculates to be 0x, which means local_tick winds up
being -1 even when flag_random_seed == NULL.

Interestingly enough, Jakub already fixed one such overflow with that
assignment with 3db31fd1cc7 ("toplev.c (init_local_tick): Avoid signed
integer multiplication overflow."), but this patch missed the integer
size issue.

This is a problem for plugins that follow the documentation comments in
order to determine whether -frandom-seed is being used. To work around
this bug, these plugins must either call get_random_seed(noinit=true) in
their plugin init functions and check there whether the return value is
zero, or more laboriously reparse common_deferred_options or
save_decoded_options. If they use a local_tick==-1 check, once in a blue
moon, it'll be wrong.

Actually, one such user of this isn't a plugin and is actually in tree:
coverage.cc, which unlink()s a file that it shouldn't from time to time:

  if (!flag_branch_probabilities && flag_test_coverage
  && (!local_tick || local_tick == (unsigned)-1))
/* Only remove the da file, if we're emitting coverage code and
   cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
unlink (da_file_name);

This patch fixes the issue by just making local_tick 64 bits, which
should also allow that timestamp to be a bit more unique as well. This
way there's no possibility of overflowing to -1. It then changes the
comparisons to use the properly typed HOST_WIDE_INT_M1U macro.

Cc: PaX Team 
Cc: Brad Spengler 
Cc: Andrew Pinski 
Cc: Jakub Jelinek 
Closes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105171
Fixes: c07e5477521 ("tree.h (default_flag_random_seed): Remove.")
Signed-off-by: Jason A. Donenfeld 
---
 gcc/coverage.cc |  4 ++--
 gcc/toplev.cc   | 10 +-
 gcc/toplev.h|  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/gcc/coverage.cc b/gcc/coverage.cc
index 8ece5db680e..aa482152b3b 100644
--- a/gcc/coverage.cc
+++ b/gcc/coverage.cc
@@ -1310,7 +1310,7 @@ coverage_init (const char *filename)
   memcpy (da_file_name + prefix_len, filename, len);
   strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
 
-  bbg_file_stamp = local_tick;
+  bbg_file_stamp = (unsigned) local_tick;
   if (flag_auto_profile)
 read_autofdo_file ();
   else if (flag_branch_probabilities)
@@ -1360,7 +1360,7 @@ coverage_finish (void)
 unlink (bbg_file_name);
 
   if (!flag_branch_probabilities && flag_test_coverage
-  && (!local_tick || local_tick == (unsigned)-1))
+  && (!local_tick || local_tick == HOST_WIDE_INT_M1U))
 /* Only remove the da file, if we're emitting coverage code and
cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
 unlink (da_file_name);
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 2d432fb2d84..7c6badeb052 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -135,9 +135,9 @@ const char * current_function_func_begin_label;
 static const char *flag_random_seed;
 
 /* A local time stamp derived from the time of compilation. It will be
-   zero if the system cannot provide a time.  It will be -1u, if the
+   zero if the system cannot provide a time.  It will be -1, if the
user has specified a particular random seed.  */
-unsigned local_tick;
+unsigned HOST_WIDE_INT local_tick;
 
 /* Random number for this compilation */
 HOST_WIDE_INT random_seed;
@@ -248,19 +248,19 @@ init_local_tick (void)
struct timeval tv;
 
gettimeofday (&tv, NULL);
-   local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
+   local_tick = (unsigned HOST_WIDE_INT) tv.tv_sec * 1000 + tv.tv_usec / 
1000;
   }
 #else
   {
time_t now = time (NULL);
 
if (now != (time_t)-1)
- local_tick = (unsigned) now;
+ local_tick = (unsigned HOST_WIDE_INT) now;
   }
 #endif
 }
   else
-local_tick = -1;
+local_tick = HOST_WIDE_INT_M1U;
 }
 
 /* Obtain the random_seed.  Unless NOINIT, initialize it if
diff --git a/gcc/toplev.h b/gcc/toplev.h
index a82ef8b8fd3..4468396b725 100644
--- a/gcc/toplev.h
+++ b/gcc/toplev.h
@@ -74,7 +74,7 @@ extern void dump_profile_report (void);
 extern void target_

Re: [PATCH] toplev: use HOST_WIDE_INT for local_tick to prevent overflow

[Andrew Pinski via Gcc-patches]

On Wed, Apr 6, 2022 at 3:44 PM Jason A. Donenfeld  wrote:
>
> In gcc/toplev.c, there's the comment:
>
>   /* A local time stamp derived from the time of compilation. It will be
>  zero if the system cannot provide a time.  It will be -1u, if the
>  user has specified a particular random seed.  */
>   unsigned local_tick;
>
> This is affirmed by init_local_tick()'s comment:
>
>   /* Initialize local_tick with the time of day, or -1 if
>  flag_random_seed is set.  */
>   static void init_local_tick (void)
>
> And indeed we see it assigned -1 when flag_random_seed != NULL:
>
>   else
> local_tick = -1;
>
> So far so good. However, in the case where flag_random_seed == NULL,
> local_tick is assigned like this:
>
>   struct timeval tv;
>   gettimeofday (&tv, NULL);
>   local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
>
> local_tick is currently of type "unsigned int". Somewhat often, that
> expression calculates to be 0x, which means local_tick winds up
> being -1 even when flag_random_seed == NULL.
>
> Interestingly enough, Jakub already fixed one such overflow with that
> assignment with 3db31fd1cc7 ("toplev.c (init_local_tick): Avoid signed
> integer multiplication overflow."), but this patch missed the integer
> size issue.
>
> This is a problem for plugins that follow the documentation comments in
> order to determine whether -frandom-seed is being used. To work around
> this bug, these plugins must either call get_random_seed(noinit=true) in
> their plugin init functions and check there whether the return value is
> zero, or more laboriously reparse common_deferred_options or
> save_decoded_options. If they use a local_tick==-1 check, once in a blue
> moon, it'll be wrong.
>
> Actually, one such user of this isn't a plugin and is actually in tree:
> coverage.cc, which unlink()s a file that it shouldn't from time to time:
>
>   if (!flag_branch_probabilities && flag_test_coverage
>   && (!local_tick || local_tick == (unsigned)-1))
> /* Only remove the da file, if we're emitting coverage code and
>cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
> unlink (da_file_name);


You are totally reading this comment incorrectly.
When there is a timestamp, the compiler does not need to delete the da
file as it will be ignored. So if there was no valid timestamp/tick
for it will be removed so the merge in libgcov code does get mix
matches.
So removing it for 0 or -1u is fine and does nothing special really
because it would have been done anyways.

Thanks.
Andrew Pinski

>
> This patch fixes the issue by just making local_tick 64 bits, which
> should also allow that timestamp to be a bit more unique as well. This
> way there's no possibility of overflowing to -1. It then changes the
> comparisons to use the properly typed HOST_WIDE_INT_M1U macro.
>
> Cc: PaX Team 
> Cc: Brad Spengler 
> Cc: Andrew Pinski 
> Cc: Jakub Jelinek 
> Closes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105171
> Fixes: c07e5477521 ("tree.h (default_flag_random_seed): Remove.")
> Signed-off-by: Jason A. Donenfeld 
> ---
>  gcc/coverage.cc |  4 ++--
>  gcc/toplev.cc   | 10 +-
>  gcc/toplev.h|  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/coverage.cc b/gcc/coverage.cc
> index 8ece5db680e..aa482152b3b 100644
> --- a/gcc/coverage.cc
> +++ b/gcc/coverage.cc
> @@ -1310,7 +1310,7 @@ coverage_init (const char *filename)
>memcpy (da_file_name + prefix_len, filename, len);
>strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
>
> -  bbg_file_stamp = local_tick;
> +  bbg_file_stamp = (unsigned) local_tick;
>if (flag_auto_profile)
>  read_autofdo_file ();
>else if (flag_branch_probabilities)
> @@ -1360,7 +1360,7 @@ coverage_finish (void)
>  unlink (bbg_file_name);
>
>if (!flag_branch_probabilities && flag_test_coverage
> -  && (!local_tick || local_tick == (unsigned)-1))
> +  && (!local_tick || local_tick == HOST_WIDE_INT_M1U))
>  /* Only remove the da file, if we're emitting coverage code and
> cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
>  unlink (da_file_name);
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 2d432fb2d84..7c6badeb052 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -135,9 +135,9 @@ const char * current_function_func_begin_label;
>  static const char *flag_random_seed;
>
>  /* A local time stamp derived from the time of compilation. It will be
> -   zero if the system cannot provide a time.  It will be -1u, if the
> +   zero if the system cannot provide a time.  It will be -1, if the
> user has specified a particular random seed.  */
> -unsigned local_tick;
> +unsigned HOST_WIDE_INT local_tick;
>
>  /* Random number for this compilation */
>  HOST_WIDE_INT random_seed;
> @@ -248,19 +248,19 @@ init_local_tick (void)
> struct timeval tv;
>
> gettimeofday (&tv, NULL);
> -   local_tick = (unsigned) tv

[pushed] c++: vector compound literal [PR105187]

[Jason Merrill via Gcc-patches]

My cleanup in r12-296 cleared TREE_HAS_CONSTRUCTOR on digested class
initializers, but we leave it set for vectors, since we can't wrap them in
TARGET_EXPR.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/105187

gcc/cp/ChangeLog:

* typeck2.cc (store_init_value): Allow TREE_HAS_CONSTRUCTOR for
vectors.

gcc/testsuite/ChangeLog:

* gcc.c-torture/compile/20050113-1.c: Moved to...
* c-c++-common/torture/20050113-1.c: ...here.
---
 gcc/cp/typeck2.cc| 1 +
 .../{gcc.c-torture/compile => c-c++-common/torture}/20050113-1.c | 0
 2 files changed, 1 insertion(+)
 rename gcc/testsuite/{gcc.c-torture/compile => 
c-c++-common/torture}/20050113-1.c (100%)

diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
index cebe6acf487..23ed81ec063 100644
--- a/gcc/cp/typeck2.cc
+++ b/gcc/cp/typeck2.cc
@@ -922,6 +922,7 @@ store_init_value (tree decl, tree init, vec** 
cleanups, int flags)
  here it should have been digested into an actual value for the type.  */
   gcc_checking_assert (TREE_CODE (value) != CONSTRUCTOR
   || processing_template_decl
+  || TREE_CODE (type) == VECTOR_TYPE
   || !TREE_HAS_CONSTRUCTOR (value));
 
   /* If the initializer is not a constant, fill in DECL_INITIAL with
diff --git a/gcc/testsuite/gcc.c-torture/compile/20050113-1.c 
b/gcc/testsuite/c-c++-common/torture/20050113-1.c
similarity index 100%
rename from gcc/testsuite/gcc.c-torture/compile/20050113-1.c
rename to gcc/testsuite/c-c++-common/torture/20050113-1.c

base-commit: cc76c502a761ddaee215bcbd8fe4720e46d3b9dd
-- 
2.27.0

[pushed] c++: nested generic lambda in DMI [PR101717]

[Jason Merrill via Gcc-patches]

We were already checking COMPLETE_TYPE_P to recognize instantiation of a
generic lambda, but didn't consider that we might be nested in a non-generic
lambda.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/101717

gcc/cp/ChangeLog:

* lambda.cc (lambda_expr_this_capture): Check all enclosing
lambdas for completeness.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1y/lambda-generic-this4.C: New test.
---
 gcc/cp/lambda.cc  | 8 +++-
 gcc/testsuite/g++.dg/cpp1y/lambda-generic-this4.C | 7 +++
 2 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-this4.C

diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index 4cc3a47f9c2..f22798d51e8 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -741,6 +741,7 @@ lambda_expr_this_capture (tree lambda, int add_capture_p)
 {
   tree lambda_stack = NULL_TREE;
   tree init = NULL_TREE;
+  bool saw_complete = false;
 
   /* If we are in a lambda function, we can move out until we hit:
1. a non-lambda function or NSDMI,
@@ -759,6 +760,11 @@ lambda_expr_this_capture (tree lambda, int add_capture_p)
  lambda_stack);
 
  tree closure = LAMBDA_EXPR_CLOSURE (tlambda);
+ if (COMPLETE_TYPE_P (closure))
+   /* We're instantiating a generic lambda op(), the containing
+  scope may be gone.  */
+   saw_complete = true;
+
  tree containing_function
= decl_function_context (TYPE_NAME (closure));
 
@@ -768,7 +774,7 @@ lambda_expr_this_capture (tree lambda, int add_capture_p)
  /* Lambda in an NSDMI.  We don't have a function to look up
 'this' in, but we can find (or rebuild) the fake one from
 inject_this_parameter.  */
- if (!containing_function && !COMPLETE_TYPE_P (closure))
+ if (!containing_function && !saw_complete)
/* If we're parsing a lambda in a non-local class,
   we can find the fake 'this' in scope_chain.  */
init = scope_chain->x_current_class_ptr;
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-this4.C 
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-this4.C
new file mode 100644
index 000..38d582bec5e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-this4.C
@@ -0,0 +1,7 @@
+// PR c++/101717
+// { dg-do compile { target c++14 } }
+
+struct x {
+  static void f() { }
+  void (*_)() = [] { [=](auto) { f(); }(0); };
+};

base-commit: d9421784980276b42ecdce85b6dde28e965c88c6
-- 
2.27.0

[pushed] c++: conversion with trailing return type [PR101051]

[Jason Merrill via Gcc-patches]

We've had a diagnostic for this, but since r10-6571 added an assert to
splice_late_return_type, we need to diagnose before we call it.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/101051

gcc/cp/ChangeLog:

* decl.cc (grokdeclarator): Reject conversion with trailing return
sooner.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/trailing15.C: New test.
---
 gcc/cp/decl.cc  |  7 +--
 gcc/testsuite/g++.dg/cpp0x/trailing15.C | 14 ++
 2 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/trailing15.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 0ff13e99595..c136dbbba1a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -12866,6 +12866,11 @@ grokdeclarator (const cp_declarator *declarator,
"type specifier", name);
return error_mark_node;
  }
+   if (late_return_type && sfk == sfk_conversion)
+ {
+   error ("a conversion function cannot have a trailing return 
type");
+   return error_mark_node;
+ }
type = splice_late_return_type (type, late_return_type);
if (type == error_mark_node)
  return error_mark_node;
@@ -13030,8 +13035,6 @@ grokdeclarator (const cp_declarator *declarator,
maybe_warn_cpp0x (CPP0X_EXPLICIT_CONVERSION);
explicitp = 2;
  }
-   if (late_return_type_p)
- error ("a conversion function cannot have a trailing return 
type");
  }
else if (sfk == sfk_deduction_guide)
  {
diff --git a/gcc/testsuite/g++.dg/cpp0x/trailing15.C 
b/gcc/testsuite/g++.dg/cpp0x/trailing15.C
new file mode 100644
index 000..3fa74d08e39
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/trailing15.C
@@ -0,0 +1,14 @@
+// PR c++/101051
+// { dg-do compile { target c++11 } }
+
+template 
+class Foo
+{
+constexpr operator T() -> T {} // { dg-error "trailing return" }
+};
+
+struct S {
+  operator int() const -> double; // { dg-error "trailing return" }
+};
+
+class A { operator auto*() -> int; }; // { dg-error "auto|trailing return" }

base-commit: 8e4339f5023286d25c7dfa40b4c88e63b780cfd7
-- 
2.27.0

[PATCH] Disable float128 tests on VxWorks, PR target/104253.

[Michael Meissner via Gcc-patches]

Disable float128 tests on VxWorks, PR target/104253.

In PR target/104253, it was pointed out the that test case added as part
of fixing the PR does not work on VxWorks because float128 is not
supported on that system.  I have modified the three tests for float128 so
that they are manually excluded on VxWorks systems.  In looking at the
code, I also added checks in check_effective_target_ppc_ieee128_ok to
disable the systems that will never support VSX instructions which are
required for float128 support (eabi, eabispe, darwin).

I have run the tests on my usual Linux systems (little endian power10, little
endian power9, big endian power8), but I don't have access to a VxWorks
system.  Eric does this fix the failure for you?

If it does fix the failure, can I apply the patch to the master branch and
backport it to GCC 11 and GCC 10?  Sorry about the breakage.

2022-04-07   Michael Meissner  

gcc/testsuite/
PR target/104253
* lib/target-supports.exp (check_ppc_float128_sw_available): Do
not run float128 tests on VxWorks.
(check_ppc_float128_hw_available): Likewise.
(check_effective_target_ppc_ieee128_ok): Do not run float128 tests
on VxWorks.  Also disable systems that do not support VSX
instructions.
---
 gcc/testsuite/lib/target-supports.exp | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/lib/target-supports.exp 
b/gcc/testsuite/lib/target-supports.exp
index ff8edbd3e17..a4142eaee27 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -2318,10 +2318,12 @@ proc check_ppc_mma_hw_available { } {
 proc check_ppc_float128_sw_available { } {
 return [check_cached_effective_target ppc_float128_sw_available {
# Some simulators are known to not support VSX/power8/power9
-   # instructions. For now, disable on Darwin.
+   # instructions. For now, disable on Darwin.  Disable on VxWorks as
+   # well.
if { [istarget powerpc-*-eabi]
 || [istarget powerpc*-*-eabispe]
-|| [istarget *-*-darwin*]} {
+|| [istarget *-*-darwin*]
+|| [istarget *-*-vxworks]} {
expr 0
} else {
set options "-mfloat128 -mvsx"
@@ -2344,10 +2346,11 @@ proc check_ppc_float128_sw_available { } {
 proc check_ppc_float128_hw_available { } {
 return [check_cached_effective_target ppc_float128_hw_available {
# Some simulators are known to not support VSX/power8/power9
-   # instructions. For now, disable on Darwin.
+   # instructions. For now, disable on Darwin and VxWorks.
if { [istarget powerpc-*-eabi]
 || [istarget powerpc*-*-eabispe]
-|| [istarget *-*-darwin*]} {
+|| [istarget *-*-darwin*]
+|| [istarget *-*-vxworks]} {
expr 0
} else {
set options "-mfloat128 -mvsx -mfloat128-hardware -mpower9-vector"
@@ -2370,8 +2373,12 @@ proc check_ppc_float128_hw_available { } {
 # See if the __ieee128 keyword is understood.
 proc check_effective_target_ppc_ieee128_ok { } {
 return [check_cached_effective_target ppc_ieee128_ok {
-   # disable on AIX.
-   if { [istarget *-*-aix*] } {
+   # disable on AIX, Darwin, VxWorks and targets that don't support VSX.
+   if { [istarget *-*-aix*]
+|| [istarget powerpc-*-eabi]
+|| [istarget powerpc*-*-eabispe]
+|| [istarget *-*-darwin*]
+|| [istarget *-*-vxworks]} {
expr 0
} else {
set options "-mfloat128"
-- 
2.35.1


-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meiss...@linux.ibm.com

Re: [PATCH] rs6000/testsuite: Skip pr105140.c

[Andreas Krebbel via Gcc-patches]

On 4/6/22 17:32, Segher Boessenkool wrote:
> This test fails with error "AltiVec argument passed to unprototyped
> function", but the code (in rs6000.c:invalid_arg_for_unprototyped_fn,
> from 2005) actually tests for any vector type argument.  It also does
> not fail on Darwin, not reflected here though.
> 
> Andreas, s390 has this same hook code, you may need to do the same?

Yes, thanks for the pointer. I've just committed the following:

IBM zSystems/testsuite: PR105147: Skip pr105140.c

pr105140.c fails on IBM zSystems with "vector argument passed to
unprototyped function".  s390_invalid_arg_for_unprototyped_fn in
s390.cc is triggered by that.

gcc/testsuite/ChangeLog:

PR target/105147
* gcc.dg/pr105140.c: Skip for s390*-*-*.
---
 gcc/testsuite/gcc.dg/pr105140.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/pr105140.c b/gcc/testsuite/gcc.dg/pr105140.c
index da34e7ad656..7d30985e850 100644
--- a/gcc/testsuite/gcc.dg/pr105140.c
+++ b/gcc/testsuite/gcc.dg/pr105140.c
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-Os -w -Wno-psabi" } */
-/* { dg-skip-if "PR105147" { powerpc*-*-* } } */
+/* { dg-skip-if "PR105147" { powerpc*-*-* s390*-*-* } } */

 typedef char __attribute__((__vector_size__ (16 * sizeof (char U;
 typedef int __attribute__((__vector_size__ (16 * sizeof (int V;

Re: [PATCH] tree.cc, v2: Add tree_builtin_call_types_compatible_p [PR105150]

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Jakub Jelinek wrote:

> On Wed, Apr 06, 2022 at 09:49:25AM +0200, Richard Biener wrote:
> > On trees we'd use tree_[sign_]nop_conversion () instead of
> > useless_type_conversion_p, I think it's OK to allow all such
> > pointer conversions.  In the end this probably means being
> > more forgiving than TYPE_MAIN_VARIANT equivalence throughout, that
> > would also make the code more similar to 
> > gimple_builtin_call_types_compatible_p besides
> > s/useless_type_conversion_p/tree_sign_nop_conversion/
> 
> Here is an updated patch to do that, allow differences in pointer types
> and tweak the promotion handling.
> There is no tree_sign_nop_conversion_p, so I've used
> tree_nop_conversion_p.  What tree_sign_nop_conversion does on top of
> that is just that it verifies both types are pointer types or
> neither is and in the latter case TYPE_UNSIGNED is the same.
> So the patch verifies both are POINTER_TYPE_P for the one case
> and for the promotion where it already checks the unpromoted type
> is integral checks if promoted one is integral too and signed.
> 
> I've also changed the patch to match the now committed gimple.cc
> change where the builtin_decl_explicit is inside of the
> *_call_types_compatible_p function instead of the caller.
> 
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2022-04-06  Jakub Jelinek  
> 
>   PR tree-optimization/105150
>   * tree.cc (tree_builtin_call_types_compatible_p): New function.
>   (get_call_combined_fn): Use it.
> 
>   * gcc.dg/pr105150.c: New test.
> 
> --- gcc/tree.cc.jj2022-04-06 09:59:03.312066863 +0200
> +++ gcc/tree.cc   2022-04-06 10:52:55.176755024 +0200
> @@ -8406,6 +8406,59 @@ get_callee_fndecl (const_tree call)
>return NULL_TREE;
>  }
>  
> +/* Return true when STMTs arguments and return value match those of FNDECL,
> +   a decl of a builtin function.  */
> +
> +static bool
> +tree_builtin_call_types_compatible_p (const_tree call, tree fndecl)
> +{
> +  gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN);
> +
> +  if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
> +if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)))
> +  fndecl = decl;
> +
> +  if (TYPE_MAIN_VARIANT (TREE_TYPE (call))
> +  != TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (fndecl
> +return false;
> +
> +  tree targs = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
> +  unsigned nargs = call_expr_nargs (call);
> +  for (unsigned i = 0; i < nargs; ++i, targs = TREE_CHAIN (targs))
> +{
> +  /* Variadic args follow.  */
> +  if (!targs)
> + return true;
> +  tree arg = CALL_EXPR_ARG (call, i);
> +  tree type = TREE_VALUE (targs);
> +  if (TYPE_MAIN_VARIANT (type) != TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
> + {
> +   /* For pointer arguments be more forgiving, e.g. due to
> +  FILE * vs. fileptr_type_node, or say char * vs. const char *
> +  differences etc.  */
> +   if (POINTER_TYPE_P (type)
> +   && POINTER_TYPE_P (TREE_TYPE (arg))
> +   && tree_nop_conversion_p (type, TREE_TYPE (arg)))
> + continue;
> +   /* char/short integral arguments are promoted to int
> +  by several frontends if targetm.calls.promote_prototypes
> +  is true.  Allow such promotion too.  */
> +   if (INTEGRAL_TYPE_P (type)
> +   && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node)
> +   && INTEGRAL_TYPE_P (TREE_TYPE (arg))
> +   && !TYPE_UNSIGNED (TREE_TYPE (arg))
> +   && targetm.calls.promote_prototypes (TREE_TYPE (fndecl))
> +   && tree_nop_conversion_p (integer_type_node,
> + TREE_TYPE (arg)))
> + continue;
> +   return false;
> + }
> +}
> +  if (targs && !VOID_TYPE_P (TREE_VALUE (targs)))
> +return false;
> +  return true;
> +}
> +
>  /* If CALL_EXPR CALL calls a normal built-in function or an internal 
> function,
> return the associated function code, otherwise return CFN_LAST.  */
>  
> @@ -8419,7 +8472,9 @@ get_call_combined_fn (const_tree call)
>  return as_combined_fn (CALL_EXPR_IFN (call));
>  
>tree fndecl = get_callee_fndecl (call);
> -  if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
> +  if (fndecl
> +  && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
> +  && tree_builtin_call_types_compatible_p (call, fndecl))
>  return as_combined_fn (DECL_FUNCTION_CODE (fndecl));
>  
>return CFN_LAST;
> --- gcc/testsuite/gcc.dg/pr105150.c.jj2022-04-06 10:51:12.801191206 
> +0200
> +++ gcc/testsuite/gcc.dg/pr105150.c   2022-04-06 10:51:12.801191206 +0200
> @@ -0,0 +1,8 @@
> +/* PR tree-optimization/105150 */
> +/* { dg-options "-w -Ofast" } */
> +
> +#define A(name) __typeof (__builtin_##name (0)) name (); \
> +  float name##1 () { return !name (1); } \
> +  double name##2 () { return name (1.0L); }
> +#define B(name) A(name) A(name##l)
> +B (s

Re: [PATCH] toplev: use HOST_WIDE_INT for local_tick to prevent overflow

[Richard Biener via Gcc-patches]

On Thu, Apr 7, 2022 at 12:44 AM Jason A. Donenfeld via Gcc-patches
 wrote:
>
> In gcc/toplev.c, there's the comment:
>
>   /* A local time stamp derived from the time of compilation. It will be
>  zero if the system cannot provide a time.  It will be -1u, if the
>  user has specified a particular random seed.  */
>   unsigned local_tick;
>
> This is affirmed by init_local_tick()'s comment:
>
>   /* Initialize local_tick with the time of day, or -1 if
>  flag_random_seed is set.  */
>   static void init_local_tick (void)
>
> And indeed we see it assigned -1 when flag_random_seed != NULL:
>
>   else
> local_tick = -1;
>
> So far so good. However, in the case where flag_random_seed == NULL,
> local_tick is assigned like this:
>
>   struct timeval tv;
>   gettimeofday (&tv, NULL);
>   local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
>
> local_tick is currently of type "unsigned int". Somewhat often, that
> expression calculates to be 0x, which means local_tick winds up
> being -1 even when flag_random_seed == NULL.
>
> Interestingly enough, Jakub already fixed one such overflow with that
> assignment with 3db31fd1cc7 ("toplev.c (init_local_tick): Avoid signed
> integer multiplication overflow."), but this patch missed the integer
> size issue.
>
> This is a problem for plugins that follow the documentation comments in
> order to determine whether -frandom-seed is being used. To work around
> this bug, these plugins must either call get_random_seed(noinit=true) in
> their plugin init functions and check there whether the return value is
> zero, or more laboriously reparse common_deferred_options or
> save_decoded_options. If they use a local_tick==-1 check, once in a blue
> moon, it'll be wrong.

While I support using a 64bit type for local_tick please use uint64_t and
not unsigned HOST_WIDE_INT.  For the issue of overloading -1 I'd
instead suggest to do

>   struct timeval tv;
>   gettimeofday (&tv, NULL);
>   local_tick = (unsigned) tv.tv_sec * 1000 + tv.tv_usec / 1000;
 /* Avoid aliasing with the special-meaning -1.  */
 if (local_tick == -1)
   local_tick = 1;

because even with uint64_t the result could be -1, no?

> Actually, one such user of this isn't a plugin and is actually in tree:
> coverage.cc, which unlink()s a file that it shouldn't from time to time:
>
>   if (!flag_branch_probabilities && flag_test_coverage
>   && (!local_tick || local_tick == (unsigned)-1))
> /* Only remove the da file, if we're emitting coverage code and
>cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
> unlink (da_file_name);
>
> This patch fixes the issue by just making local_tick 64 bits, which
> should also allow that timestamp to be a bit more unique as well. This
> way there's no possibility of overflowing to -1. It then changes the
> comparisons to use the properly typed HOST_WIDE_INT_M1U macro.
>
> Cc: PaX Team 
> Cc: Brad Spengler 
> Cc: Andrew Pinski 
> Cc: Jakub Jelinek 
> Closes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105171
> Fixes: c07e5477521 ("tree.h (default_flag_random_seed): Remove.")
> Signed-off-by: Jason A. Donenfeld 
> ---
>  gcc/coverage.cc |  4 ++--
>  gcc/toplev.cc   | 10 +-
>  gcc/toplev.h|  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/coverage.cc b/gcc/coverage.cc
> index 8ece5db680e..aa482152b3b 100644
> --- a/gcc/coverage.cc
> +++ b/gcc/coverage.cc
> @@ -1310,7 +1310,7 @@ coverage_init (const char *filename)
>memcpy (da_file_name + prefix_len, filename, len);
>strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
>
> -  bbg_file_stamp = local_tick;
> +  bbg_file_stamp = (unsigned) local_tick;
>if (flag_auto_profile)
>  read_autofdo_file ();
>else if (flag_branch_probabilities)
> @@ -1360,7 +1360,7 @@ coverage_finish (void)
>  unlink (bbg_file_name);
>
>if (!flag_branch_probabilities && flag_test_coverage
> -  && (!local_tick || local_tick == (unsigned)-1))
> +  && (!local_tick || local_tick == HOST_WIDE_INT_M1U))
>  /* Only remove the da file, if we're emitting coverage code and
> cannot uniquely stamp it.  If we can stamp it, libgcov will DTRT.  */
>  unlink (da_file_name);
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 2d432fb2d84..7c6badeb052 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -135,9 +135,9 @@ const char * current_function_func_begin_label;
>  static const char *flag_random_seed;
>
>  /* A local time stamp derived from the time of compilation. It will be
> -   zero if the system cannot provide a time.  It will be -1u, if the
> +   zero if the system cannot provide a time.  It will be -1, if the
> user has specified a particular random seed.  */
> -unsigned local_tick;
> +unsigned HOST_WIDE_INT local_tick;
>
>  /* Random number for this compilation */
>  HOST_WIDE_INT random_seed;
> @@ -248,19 +248,19 @@ init_local_tick (void)
> struct timeval tv;
>
> 

Re: [PATCH] tree-optimization/105175 - avoid -Wvector-operation-performance

[Richard Biener via Gcc-patches]

On Wed, 6 Apr 2022, Martin Sebor wrote:

> On 4/6/22 03:23, Richard Biener wrote:
> > This avoids -Wvector-operation-performance diagnostics for vectorizer
> > produced code.  It's unfortunate the warning_at code in
> > tree-vect-generic.cc needs adjustments but the diagnostic suppression
> > code doesn't magically suppress those otherwise.
> 
> It seems like it should, as long as the statement location hasn't
> changed after the suppress_diagnostic call in tree-vect-stmts.cc.

The location doesn't change.

> > 
> > Bootstrap / regtest running on x86_64-unknown-linux-gnu.
> > 
> > Martin/David - did I miss something obvious when doing the
> > tree-vect-generic.cc adjustment?
> 
> The only thing I can think of is that because it's not handled in
> diagnostic-spec.cc, -Wvector-operation-performance is lumped in with
> all other generic warnings that also aren't handled.  It means that
> they are all treated as a group.  Whether or not that's what we want
> for this specific warning might be something to consider.

So when I call suppress_warning (gimple *, ..) the suppress_warning_at
call constructs a nowarn_spec_t with NW_OTHER, it queries the
nowarn_map where it doesn't find anything yet, and goes on
with

  nowarn_map->put (loc, optspec);
  return true;

suppress_warning then (since supp is true anyway) goes on with

  set_no_warning_bit (stmt, supp);

which is likely what my changes to tree-vect-generic.cc in the end
key on.  When I simply invoke

   warning_at (loc, OPT_Wvector_operation_performance,
   "...")

I see nowhere that nowarn_spec_t::nowarn_spec_t is invoked, nor
is warning_suppressed_at.  Maybe I'm missing that being done
but I think that's by design?  It at least was surprising to me.
Of course since we lack a warning_at (gimple *, ..) overload
or alternatively extending rich-location to cover diagnostic
suppression contexts, doing this would only work for stmts with
a location that doesn't fall back to that of the current
declaration (for UNKNOWN_LOCATION loc).

So my main question was if the diagnostic suppression is supposed
to be transparently handled by warning_at (...) or whether indeed
explicit guards need to be added to each diagnostic emission.

As I'm now doing

   if (!warning_suppressed_p (gsi_stmt (*gsi),
 OPT_Wvector_operation_performance))


I get to get_nowarn_spec for the stmt which will return NULL
because the no-warning bit is set (but it's always set in the
warning suppression call when done on a stmt!)

When I'm doing

  if (!warning_suppressed_at (loc,
 OPT_Wvector_operation_performance))

then it also suppresses the diagnostic but I think I'm not supposed
to call that function since it will ICE on UNKNOWN_LOCATION and it
would lack the fallback to the nowarning bit for stmts without
location.

That is - the stmt-based query looks correct to me but it will
always use the non-specific flag, at least when I suppress the
diagnostic based on the stmt?!  I think suppress_warning (gimple *,...)
should not set the no-warning bit when it succeeded in amending
the nowarn_map?

So, again, was the requirement to explicitely guard warning_at ()
calls with warning_suppressed_p () calls by design?  If it wasn't
intentional then I think we need to somehow allow to specify
a gimple * or tree as location argument to warning_at, since
we have rich-location it might be tempting to use that as vehicle
to carry the nowarn query result or alternatively the stmt/tree
itself?

Thanks,
Richard.


> Martin
> 
> > 
> > Thanks,
> > Richard.
> > 
> > 2022-04-06  Richard Biener  
> > 
> >  PR tree-optimization/105175
> >  * tree-vect-stmts.cc (vectorizable_operation): Suppress
> >  -Wvector-operation-performance if using emulated vectors.
> >  * tree-vect-generic.cc (expand_vector_piecewise): Do not diagnose
> >  -Wvector-operation-performance when suppressed.
> >  (expand_vector_parallel): Likewise.
> >  (expand_vector_comparison): Likewise.
> >  (expand_vector_condition): Likewise.
> >  (lower_vec_perm): Likewise.
> >  (expand_vector_conversion): Likewise.
> > 
> > * gcc.dg/pr105175.c: New testcase.
> > ---
> >   gcc/testsuite/gcc.dg/pr105175.c | 16 +
> >   gcc/tree-vect-generic.cc| 41 ++---
> >   gcc/tree-vect-stmts.cc  |  2 ++
> >   3 files changed, 45 insertions(+), 14 deletions(-)
> >   create mode 100644 gcc/testsuite/gcc.dg/pr105175.c
> > 
> > diff --git a/gcc/testsuite/gcc.dg/pr105175.c
> > b/gcc/testsuite/gcc.dg/pr105175.c
> > new file mode 100644
> > index 000..d8d7edb942a
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr105175.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -Wvector-operation-performance" } */
> > +/* { dg-additional-options "-mno-sse" { target x86_64-*-* i?86-*-* } } */
> > +
> > +enum { QEMU_MIGRATION_COOKIE_PERSISTENT = 1 };
> > +struct {
> > +  unsigned flags;
> > +  unsigned flags