Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Michael Meissner via Gcc-patches]

On Thu, Dec 15, 2022 at 11:59:49AM -0600, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Dec 14, 2022 at 10:36:03AM +0100, Jakub Jelinek wrote:
> > On Wed, Dec 14, 2022 at 04:46:07PM +0800, Kewen.Lin via Gcc-patches wrote:
> > > Since function useless_type_conversion_p considers two float types are 
> > > compatible
> > > if they have the same mode, so it doesn't require the explicit 
> > > conversions between
> > > these two types.  I think it's exactly what we want.  And to me, it looks 
> > > unexpected
> > > to have two types with the same mode but different precision.
> > > 
> > > So could we consider disabling the above workaround to make _Float128 
> > > have the same
> > > precision as __float128 (long double) (the underlying TFmode)?  I tried 
> > > the below
> > > change:
> > 
> > The hacks with different precisions of powerpc 128-bit floating types are
> > very unfortunate, it is I assume because the middle-end asserted that scalar
> > floating point types with different modes have different precision.
> 
> IEEE QP and double-double cannot be ordered, neither represents a subset
> of the other.  But the current middle end does require a total ordering
> for all floating point types (that can be converted to each other).
> 
> Mike's precision hack was supposed to give us some time until an actual
> fix was made.  But no one has worked on that, and there have been
> failures found with the precision hack as well, it worked remarkably
> well but it isn't perfect.
> 
> We cannot move forward in a meaningful way until these problems are
> fixed.  We can move around like headless chickens some more of course.

In general I tend to think most of these automatic widenings are
problematical.  But there are cases where it makes sense.

Lets see.  On the PowerPC, there is no support for 32-bit decimal arithmetic.
There you definately the compiler to automatically promoto SDmode to DDmode to
do the arithmetic and then possibly convert it back.  Similarly for the limited
16-bit floating point modes, where you have operations to pack and unpack the
object, but you have no arithmetic.

But I would argue that you NEVER want to automatically promoto DFmode to either
KFmode, TFmode, or IFmode, since those modes are (almost) always going to be
slower than doing the emulation.  This is particularly true if we only support
a subset of operations, where some things can be done inline, but a lot of
operations would need to be done via emulation (such as on power8 where we
don't have IEEE 128-bit support in hardware).

If the machine independent part of the compiler decides oh we can do this
operation because some operations are present (such as move, negate, absolute
value, and compare), then you likely will wind up promoting the 64-bit type(s)
to 128-bit, doing a call to a slower 128-bit function, and then truncating the
value back to 64-bit is faster than calling a 64-bit emulation function.  And
even if the operation is does have a named insn to do the operation, it doesn't
mean that you want to use that operation in general.

I recall in the past that for some x86 boxes, the 80-bit XFmode insns floating
point stack operations on the x86 were really slow compared to the current
SFmode and DFmode SSE operations.  But for some of the older machines, it may
have been faster.  And chosing a different -march= would change whether or
not you want to do the optimization.  Having these tables built statically can
be a recipie for disaster.  For floating point at least, I would prefer if a
target had an option to dispense with the statically built get_wider tables,
and did everything via target hooks.

While for the PowerPC, we want to control what is the logical wider type for
floating point types, I can imagine we don't want all backends to have to
implment these hooks if they just have the standard 2-3 floating point modes.

I purposelly haven't been looking into 16-bit floating point support, but I
have to imagine there you have the problem that there are at least 2-3
different 16-bit formats roaming around.  This is essentially the same issue in
the PowerPC where you have 2 128-bit floating point types, neither of which is
a pure subset of the other.

To my way of thinking, it is a many branching tree.  On the PowerPC, you want
SDmode to promoto to DDmode, and possibly to TDmode.  And SFmode mode would
promote to DFmode, but DFmode would not generally promote automtically to
IFmode, TFmode, or KFmode.  We don't have any machines that support it, but I
lets say some machine wanted to support both decimal types (DFP and BID).  You
would logically not want any DFP type to promoto to a BID type or vice versa.

Sure, explicit conversions would be allowed, but not the invisibile conversions
done to promote the type.

In terms of these machine dependent types, there are some issues that show up
when a port creates these special types.

   1)   It would be nice if _Complex worked with MD types.  It is tiresome to
have 

Re: [PATCH] coroutines: Build pointer initializers with nullptr_node [PR107768]

[Jason Merrill via Gcc-patches]

On 12/10/22 10:54, Iain Sandoe wrote:

From: Andrew Pinski 

This is Andrew Pinski's patch, I just did testing, adjusted the test case and
provided the Changelog.
tested on x86-64-Darwin21,
OK for trunk?
Iain


OK.


--- >8 ---

The PR reports that using integer_zero_node triggers a warning for
-Wzero-as-null-pointer-constant which comes from compiler-generated code so
makes no sense to the end user.

Co-Authored-By: Iain Sandoe 

PR c++/107768

gcc/cp/ChangeLog:

* coroutines.cc (coro_rewrite_function_body): Initialize pointers
from nullptr_node.  (morph_fn_to_coro): Likewise.

gcc/testsuite/ChangeLog:

* g++.dg/coroutines/pr107768.C: New test.
---
  gcc/cp/coroutines.cc   |  6 ++---
  gcc/testsuite/g++.dg/coroutines/pr107768.C | 26 ++
  2 files changed, 29 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/coroutines/pr107768.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 3f23317a315..88d6c30d8b1 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4132,7 +4132,7 @@ coro_rewrite_function_body (location_t fn_start, tree 
fnbody, tree orig,
/* We will need to be able to set the resume function pointer to nullptr
   to signal that the coroutine is 'done'.  */
tree zero_resume
-= build1 (CONVERT_EXPR, resume_fn_ptr_type, integer_zero_node);
+= build1 (CONVERT_EXPR, resume_fn_ptr_type, nullptr_node);
  
/* The pointer to the destroy function.  */

tree var = coro_build_artificial_var (fn_start, coro_destroy_fn_id,
@@ -4519,7 +4519,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
tree ramp_body = push_stmt_list ();
  
tree zeroinit = build1_loc (fn_start, CONVERT_EXPR,

- coro_frame_ptr, integer_zero_node);
+ coro_frame_ptr, nullptr_node);
tree coro_fp = coro_build_artificial_var (fn_start, "_Coro_frameptr",
coro_frame_ptr, orig, zeroinit);
tree varlist = coro_fp;
@@ -4754,7 +4754,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree 
*destroyer)
  
gcc_checking_assert (same_type_p (fn_return_type, TREE_TYPE (grooaf)));

tree if_stmt = begin_if_stmt ();
-  tree cond = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node);
+  tree cond = build1 (CONVERT_EXPR, coro_frame_ptr, nullptr_node);
cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond);
finish_if_stmt_cond (cond, if_stmt);
if (VOID_TYPE_P (fn_return_type))
diff --git a/gcc/testsuite/g++.dg/coroutines/pr107768.C 
b/gcc/testsuite/g++.dg/coroutines/pr107768.C
new file mode 100644
index 000..22d7074f261
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr107768.C
@@ -0,0 +1,26 @@
+//  { dg-additional-options "-Wzero-as-null-pointer-constant -fsyntax-only" }
+
+#include 
+
+struct task
+{
+struct promise_type
+{
+task get_return_object() { return {}; }
+std::suspend_never initial_suspend() { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+void return_void() {}
+void unhandled_exception() {}
+};
+};
+
+task resuming_on_new_thread(void)
+{
+struct awaitable
+{
+bool await_ready() { return false; }
+void await_suspend(std::coroutine_handle<> h) { }
+void await_resume() {}
+};
+co_await awaitable{};
+}

Re: [PATCH] c++: variadic using-decl with parm pack in terminal name [PR102104]

[Jason Merrill via Gcc-patches]

On 12/15/22 11:15, Patrick Palka wrote:

There's a curious corner case with variadic member using-decls: the
terminal name can also contain a parameter pack, and only through naming
a conversion function, e.g.

   using A::operator Ts...;

We currently only handle parameter packs appearing in the qualifying
scope of a variadic using-decl; this patch adds support for the above
case as well, representing such a using-decl with two pack expansions,
one for the qualifying scope and one for the terminal name (despite
logically there being just one).  Then at instantiation time we manually
merge them.

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


OK.


PR c++/102104
PR c++/108090

gcc/cp/ChangeLog:

* error.cc (dump_decl) : Look through a
pack expansion in the name as well.
* parser.c (cp_parser_using_declaration): Handle a parameter
pack appearing in the terminal name of a variadic using-decl.
* pt.c (tsubst_decl) : Likewise.  Combine the
handling of variadic and non-variadic using-decls.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/using-variadic1.C: New test.
* g++.dg/cpp1z/using-variadic1a.C: New test.
* g++.dg/cpp1z/using-variadic1b.C: New test.
* g++.dg/cpp1z/using-variadic1c.C: New test.
* g++.dg/cpp1z/using-variadic2.C: New test.
* g++.dg/cpp1z/using-variadic3.C: New test.
---
  gcc/cp/error.cc   |  9 ++
  gcc/cp/parser.cc  | 33 ++-
  gcc/cp/pt.cc  | 91 ++-
  gcc/testsuite/g++.dg/cpp1z/using-variadic1.C  | 29 ++
  gcc/testsuite/g++.dg/cpp1z/using-variadic1a.C | 34 +++
  gcc/testsuite/g++.dg/cpp1z/using-variadic1b.C | 37 
  gcc/testsuite/g++.dg/cpp1z/using-variadic1c.C | 33 +++
  gcc/testsuite/g++.dg/cpp1z/using-variadic2.C  | 24 +
  gcc/testsuite/g++.dg/cpp1z/using-variadic3.C  |  8 ++
  9 files changed, 272 insertions(+), 26 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1a.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1b.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1c.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic2.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic3.C

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 12b28e8ee5b..e7f60335cc0 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -1477,11 +1477,20 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
if (!(flags & TFF_UNQUALIFIED_NAME))
  {
tree scope = USING_DECL_SCOPE (t);
+   tree name = DECL_NAME (t);
if (PACK_EXPANSION_P (scope))
  {
scope = PACK_EXPANSION_PATTERN (scope);
variadic = true;
  }
+   if (identifier_p (name)
+   && IDENTIFIER_CONV_OP_P (name)
+   && PACK_EXPANSION_P (TREE_TYPE (name)))
+ {
+   name = make_conv_op_name (PACK_EXPANSION_PATTERN
+ (TREE_TYPE (name)));
+   variadic = true;
+ }
dump_type (pp, scope, flags);
pp_cxx_colon_colon (pp);
  }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 03550308365..2e2d81c1316 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -21705,7 +21705,38 @@ cp_parser_using_declaration (cp_parser* parser,
pedwarn (ell->location, OPT_Wc__17_extensions,
 "pack expansion in using-declaration only available "
 "with %<-std=c++17%> or %<-std=gnu++17%>");
-  qscope = make_pack_expansion (qscope);
+
+  /* A parameter pack can appear in the qualifying scope, and/or in the
+terminal name (if naming a conversion function).  Logically they're
+part of a single pack expansion of the overall USING_DECL, but we
+express them as separate pack expansions within the USING_DECL since
+we can't create a pack expansion over a USING_DECL.  */
+  bool saw_parm_pack = false;
+  if (uses_parameter_packs (qscope))
+   {
+ qscope = make_pack_expansion (qscope);
+ saw_parm_pack = true;
+   }
+  /* It can also appear in the terminal name (if naming a conversion
+function).  */
+  if (identifier_p (identifier)
+ && IDENTIFIER_CONV_OP_P (identifier)
+ && uses_parameter_packs (TREE_TYPE (identifier)))
+   {
+ identifier = make_conv_op_name (make_pack_expansion
+ (TREE_TYPE (identifier)));
+ saw_parm_pack = true;
+   }
+  if (!saw_parm_pack)
+   {
+ /* Issue an error in terms using a SCOPE_REF that includes both
+components.  */
+ tree name
+   = build_qualified_name 

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Jason Merrill via Gcc-patches]

On 12/15/22 14:31, Patrick Palka wrote:

On Thu, 15 Dec 2022, Patrick Palka wrote:


On Thu, 15 Dec 2022, Jason Merrill wrote:


On 12/12/22 12:20, Patrick Palka wrote:

When instantiating a constrained hidden template friend, we need to
substitute into its constraints for sake of declaration matching.


Hmm, we shouldn't need to do declaration matching when there's only a single
declaration.


Oops, indeed.  It looks like in this case we're not calling
maybe_substitute_reqs_for during declaration matching, but rather from
tsubst_friend_function and specifically for the non-trailing requirements:

   if (TREE_CODE (new_friend) == TEMPLATE_DECL)
 {
   DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
   DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
   DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
 = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));

   /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
  match in decls_match.  */
   tree parms = DECL_TEMPLATE_PARMS (new_friend);
   tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
   treqs = maybe_substitute_reqs_for (treqs, new_friend);
   if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
 {
   TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
   /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
   tsubst_each_template_parm_constraints (parms, args,
  tf_warning_or_error);
 }
 }

I'll adjust the commit message appropriately.




For
this substitution we use a full argument vector whose outer levels
correspond to the class's arguments and innermost level corresponds to
the template's level-lowered generic arguments.

But for A::f here, for which the relevant argument vector is
{{int}, {Us...}}, this substitution triggers the assert in
use_pack_expansion_extra_args_p since one argument is a pack expansion
and the other isn't.

And for A::f, for which the relevant argument vector is
{{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
also trigger but we first get a bogus "mismatched argument pack lengths"
error from tsubst_pack_expansion.

These might ultimately be bugs in tsubst_pack_expansion, but it seems
we can work around them by tweaking the constraint substitution in
maybe_substitute_reqs_for to only use the friend's outer arguments in
the case of a template friend.


Yes, this is how we normally substitute a member template during class
instantiation: with the class' template args, not its own.  The assert seems
to be enforcing that.


Ah, makes snese.




This should be otherwise equivalent to
substituting using the full arguments, since the template's innermost
arguments are just its generic arguments with level=1.

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

PR c++/108066
PR c++/108067

gcc/cp/ChangeLog:

* constraint.cc (maybe_substitute_reqs_for): For a template
friend, substitute using only its outer arguments.  Remove
dead uses_template_parms test.


I don't see any removal?


Oops, I reverted that change before posting the patch but forgot to adjust the
ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
it's not necessary to fix the bug.




gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-friend12.C: New test.
---
   gcc/cp/constraint.cc  |  8 +++
   .../g++.dg/cpp2a/concepts-friend12.C  | 22 +++
   2 files changed, 30 insertions(+)
   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index d4cd92ec3b4..f9d1009c9fe 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
decl)
 tree tmpl = DECL_TI_TEMPLATE (decl);
 tree gargs = generic_targs_for (tmpl);
 processing_template_decl_sentinel s;
+  if (PRIMARY_TEMPLATE_P (tmpl))
+   {
+ if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
+   return reqs;
+ ++processing_template_decl;
+ gargs = copy_node (gargs);
+ --TREE_VEC_LENGTH (gargs);


Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
for DECL_FRIEND_CONTEXT instead of decl itself?


IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template friend
is declared inside a partial specialization:

   template
   concept C = __is_same(T, U);

   template
   struct A;

   template
   struct A {
 template
   requires (__is_same(T, U))
 friend void f() { };
   };

   template struct A;

The DECL_FRIEND_CONTEXT of f is A, whose TYPE_TI_ARGS are {int*},
relative to the primary template instead of the partial specialization.  So
we'd wrongly end up substituting T=int* instead of T=int into f's
TEMPLATE_PARMS_CONSTRAINTS.

Re: [PATCH] contracts: Stop relying on mangling for naming .pre/.post clones

[Jason Merrill via Gcc-patches]

On 12/15/22 13:00, Arsen Arsenović wrote:

Hi Jason,

Jason Merrill  writes:


On 12/10/22 08:13, Arsen Arsenović wrote:

If the mangler is relied on, functions with extern "C" on them emit multiple
definitions of the same name.


But doing it here interferes with lazy mangling.  How about appending the
suffix into write_mangled_name instead of write_encoding?  The demangler
already expects "clone" suffixes at the end of the mangled name.


Ah, sorry.  I'm not well versed in the mangler code, so I didn't realize
(frankly, I was initially surprised when I saw that DECL_ASSEMBLER_NAME
was set that early, but went with it).  That makes sense.

How about this?  Tested on x86_64-pc-linux-gnu via check-g++.



I did run c++filt (afaik, it uses the libiberty demangler) on this
revision, and I got:

   .typef(int) [clone .pre], @function
   f(int) [clone .pre]:

out of ``void f(int x) [[ pre: x > 10 ]] {}'', which seems to match your
description.

If I understand this right, write_xxx corresponds to xxx in the Itanium
ABI mangling BNF, in which case, I believe I have the correct spot here.
In that case, a similar change should happen for coroutines; I think
Iain was working on that.



* mangle.cc (write_encoding): Move contract pre/post function mangling


It's best to wrap the ChangeLog entry at column 75 so that the extra 
indentation added by git log doesn't push past column 80.


Applied with that change.

Jason

Re: [PATCH] contracts: Stop relying on mangling for naming .pre/.post clones

[Iain Sandoe]

Hi Arsen,

> On 15 Dec 2022, at 18:00, Arsen Arsenović via Gcc-patches 
>  wrote:

> Jason Merrill  writes:
> 
>> On 12/10/22 08:13, Arsen Arsenović wrote:
>>> If the mangler is relied on, functions with extern "C" on them emit multiple
>>> definitions of the same name.
>> 
>> But doing it here interferes with lazy mangling.  How about appending the
>> suffix into write_mangled_name instead of write_encoding?  The demangler
>> already expects "clone" suffixes at the end of the mangled name.
> 
> Ah, sorry.  I'm not well versed in the mangler code, so I didn't realize
> (frankly, I was initially surprised when I saw that DECL_ASSEMBLER_NAME
> was set that early, but went with it).  That makes sense.
> 
> How about this?  Tested on x86_64-pc-linux-gnu via check-g++.
> 
> From 2a2d98e94bdd7a8d7f862b2accda849927e4509e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= 
> Date: Thu, 15 Dec 2022 18:56:59 +0100
> Subject: [PATCH v2] c++: Mangle contracts in write_mangled_name
> unconditionally
> 
> This fixes contract-checked extern "C" functions.
> 
> gcc/cp/ChangeLog:
> 
>   * mangle.cc (write_encoding): Move contract pre/post function mangling
>   from here...
>   (write_mangled_name): ... to here, and make it happen always.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/contracts/contracts-externC.C: New test.
> ---
> gcc/cp/mangle.cc  | 14 +++---
> .../g++.dg/contracts/contracts-externC.C  | 19 +++
> 2 files changed, 26 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/contracts/contracts-externC.C
> 
> diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> index e363ef35b9f..074cf27ec7a 100644
> --- a/gcc/cp/mangle.cc
> +++ b/gcc/cp/mangle.cc
> @@ -798,6 +798,13 @@ write_mangled_name (const tree decl, bool top_level)
>   write_string ("_Z");
>   write_encoding (decl);
> }
> +
> +  /* If this is the pre/post function for a guarded function, append
> + .pre/post, like something from create_virtual_clone.  */
> +  if (DECL_IS_PRE_FN_P (decl))
> +write_string (".pre");
> +  else if (DECL_IS_POST_FN_P (decl))
> +write_string (".post");
> }

I think you want to use 'write_string (JOIN_STR “pre”);’ etc. since that 
handles targets
that cannot use a period in symbol names.

> 
> /* Returns true if the return type of DECL is part of its signature, and
> @@ -856,13 +863,6 @@ write_encoding (const tree decl)
>   mangle_return_type_p (decl),
>   d);
> 
> -  /* If this is the pre/post function for a guarded function, append
> -  .pre/post, like something from create_virtual_clone.  */
> -  if (DECL_IS_PRE_FN_P (decl))
> - write_string (".pre");
> -  else if (DECL_IS_POST_FN_P (decl))
> - write_string (".post");
> -
>   /* If this is a coroutine helper, then append an appropriate string to
>identify which.  */
>   if (tree ramp = DECL_RAMP_FN (decl))
> diff --git a/gcc/testsuite/g++.dg/contracts/contracts-externC.C 
> b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
> new file mode 100644
> index 000..873056b742b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
> @@ -0,0 +1,19 @@
> +// simple check to ensure we don't emit a function with the same name twice,
> +// when wrapping functions in pre- and postconditions.
> +// { dg-do link }
> +// { dg-options "-std=c++2a -fcontracts -fcontract-continuation-mode=on" }
> +
> +volatile int x = 10;
> +
> +extern "C" void
> +f ()
> +  [[ pre: x < 10 ]]
> +{
> +}
> +
> +int
> +main ()
> +  [[ post: x > 10 ]]
> +{
> +  f();
> +}
> -- 
> 2.39.0
> 
> 
> I did run c++filt (afaik, it uses the libiberty demangler) on this
> revision, and I got:
> 
>  .typef(int) [clone .pre], @function
>  f(int) [clone .pre]:
> 
> out of ``void f(int x) [[ pre: x > 10 ]] {}'', which seems to match your
> description.
> 
> If I understand this right, write_xxx corresponds to xxx in the Itanium
> ABI mangling BNF, in which case, I believe I have the correct spot here.
> In that case, a similar change should happen for coroutines; I think
> Iain was working on that.

If this is the right place, then I can update my patch for coroutines - both
Gor and Lewis replied that ‘extern “C”’ coroutines seemed reasonable to
them.

Iain

Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Segher Boessenkool]

On Thu, Dec 15, 2022 at 07:56:14PM +0100, Jakub Jelinek wrote:
> On Thu, Dec 15, 2022 at 12:49:27PM -0600, Segher Boessenkool wrote:
> > Certainly.  But different types with the same mode having different
> > precision is not so very reasonable, and will likely cause other
> > problems as well.
> > 
> > We cannot use precision to order modes or types, that is the core
> > problem.  A conversion from IEEE QP to double-double (or vice versa) is
> > neither widening nor narrowing.
> 
> Sure.  For optabs, I bet we don't necessarily need to care that much, if
> precision is the same, we can ask for widening and narrowing conversion
> and expect only one to be implemented or both doing the same thing between
> such modes.  But when using libcalls, which library function we use is quite
> important because not all of them might be actually implemented in the
> library (better keep doing what we've done before).

I don't think we should name non-widenings widening, or non-narrowings
narrowing.  We should call conversions conversions.

Even if it doesn't cause technical problems the way it is now (of which
I am not convinced at all), faulty names like this are mightily
confusing.  This is a very real *practical* problem :-(


Segher

Re: [Patch] libgomp: Handle OpenMP's reverse offloads

[Tobias Burnus]

Hi,

On 15.12.22 20:42, Tobias Burnus wrote:

If the libgomp plugin doesn't request special
'host_to_dev_cpy'/'dev_to_host_cpy' for 'gomp_target_rev', then standard
'gomp_copy_host2dev'/'gomp_copy_dev2host' are used, which use
'gomp_device_copy', which expects the device to be locked.  (As can be
told by the unconditional 'gomp_mutex_unlock (>lock);' before
'gomp_fatal'.)  However, in a number of the
'gomp_copy_host2dev'/'gomp_copy_dev2host' calls from 'gomp_target_rev',
the device definitely is not locked; see


Actually, reading it + the source code again, I think it makes sense to
return a boolean – similar to devicep->host2dev_func and
devicep->dev2host_func — and possibly wrap it into some convenience
function, similar to gomp_device_copy – at least a bare exit() without
further diagnostic does not seem to userfriendly.

BTW: In line with the other code, you could use CUDA_CALL instead of
CUDA_CALL_ERET; the fomer already calls the latter with 'false' as first
argument + is used elsewhere.

Regarding the lock: It seems the problem is the copying of
devaddrs/sizes/kinds; this does not need any lock as the stack variables
are on the device and only used for this reverse offload. Thus, there is
no need for a lock as there are no races.

However, as the existing gomp_copy_dev2host removes the lock, we could
simply keep this lock – and probably should move it down to just before
the user-function call – removing all (non-error) locks and unlocks on
the way. — I mean something like the attached patch.

Finally, I think we need to find a solution for the issue Andrew tried
to address. — The current code invokes CUDA_CALL_ASSERT – which calls
GOMP_PLUGIN_fatal.

Tobias
-
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
diff --git a/libgomp/target.c b/libgomp/target.c
index e38cc3b6f1c..4b7233307cd 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -3319,5 +3319,6 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, uint64_t devaddrs_ptr,
   gomp_mutex_lock (>lock);
   n = gomp_map_lookup_rev (>mem_map_rev, );
-  gomp_mutex_unlock (>lock);
+  if (devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
+gomp_mutex_unlock (>lock);
 
   if (n == NULL)
@@ -3409,5 +3410,4 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, uint64_t devaddrs_ptr,
   cdata = gomp_alloca (sizeof (*cdata) * mapnum);
   memset (cdata, '\0', sizeof (*cdata) * mapnum);
-  gomp_mutex_lock (>lock);
   for (uint64_t i = 0; i < mapnum; i++)
 	{
@@ -3643,4 +3643,5 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, uint64_t devaddrs_ptr,
   uint64_t struct_cpy = 0;
   bool clean_struct = false;
+  gomp_mutex_lock (>lock);
   for (uint64_t i = 0; i < mapnum; i++)
 	{
@@ -3695,5 +3696,5 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, uint64_t devaddrs_ptr,
 	  gomp_aligned_free ((void *) (uintptr_t) devaddrs[i]);
 	}
-
+  gomp_mutex_unlock (>lock);
   free (devaddrs);
   free (sizes);

Re: [Patch] libgomp: Handle OpenMP's reverse offloads

[Tobias Burnus]

Hi,

I have not fully tried to understand it, yet.

(A) Regarding the issue of stalling, see als Andrew's patch and the
discussion about it in

"[PATCH] libgomp: fix hang on fatal error",
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603616.html

and in particular Jakub's two replies.

(b) I think you want to remove this:

On 15.12.22 18:34, Thomas Schwinge wrote:

--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -1,3 +1,5 @@
+#pragma GCC optimize "O0"
+
  /* Plugin for NVPTX execution.


(c)


If the libgomp plugin doesn't request special
'host_to_dev_cpy'/'dev_to_host_cpy' for 'gomp_target_rev', then standard
'gomp_copy_host2dev'/'gomp_copy_dev2host' are used, which use
'gomp_device_copy', which expects the device to be locked.  (As can be
told by the unconditional 'gomp_mutex_unlock (>lock);' before
'gomp_fatal'.)  However, in a number of the
'gomp_copy_host2dev'/'gomp_copy_dev2host' calls from 'gomp_target_rev',
the device definitely is not locked; see the calls adjacent to the TODO


The question is what unlocks the device – it is surely locked in 
gomp_target_rev by:

  if (!(devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM))
...
  gomp_mutex_lock (>lock);
  for (uint64_t i = 0; i < mapnum; i++)
...
}
  gomp_mutex_unlock (>lock);
}

Except for code like:
gomp_mutex_unlock (>lock);
gomp_fatal ("gomp_target_rev unhandled kind 0x%.4x", kinds[i]);

The only functions that know about the pointer and get called are those behind
the dev_to_host_cpy and host_to_dev_cpy - thus, they seemingly mess about with 
the
unlocking?!?

 * * *

Regarding your patch, I do not understand why you call twice unlock and
have trice TODO unlock; that does not seem to make any sense.

I think it is worthwhile to understand why plugin-nvptx.c unlocks the lock in
the non-error case - as you observe that it is not locked in the error case.

Additionally, it seems to make more sense to look into a revised patch of
Andrew's patch, your patch looks like a rather bad band aid.

Tobias

-
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] c++: template friend with variadic constraints [PR108066]

[Patrick Palka via Gcc-patches]

On Thu, 15 Dec 2022, Patrick Palka wrote:

> On Thu, 15 Dec 2022, Jason Merrill wrote:
> 
> > On 12/12/22 12:20, Patrick Palka wrote:
> > > When instantiating a constrained hidden template friend, we need to
> > > substitute into its constraints for sake of declaration matching.
> > 
> > Hmm, we shouldn't need to do declaration matching when there's only a single
> > declaration.
> 
> Oops, indeed.  It looks like in this case we're not calling
> maybe_substitute_reqs_for during declaration matching, but rather from
> tsubst_friend_function and specifically for the non-trailing requirements:
> 
>   if (TREE_CODE (new_friend) == TEMPLATE_DECL)
> {
>   DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
>   DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
>   DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
> = DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));
> 
>   /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
>  match in decls_match.  */
>   tree parms = DECL_TEMPLATE_PARMS (new_friend);
>   tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
>   treqs = maybe_substitute_reqs_for (treqs, new_friend);
>   if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
> {
>   TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
>   /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
>   tsubst_each_template_parm_constraints (parms, args,
>  tf_warning_or_error);
> }
> }
> 
> I'll adjust the commit message appropriately.
> 
> > 
> > > For
> > > this substitution we use a full argument vector whose outer levels
> > > correspond to the class's arguments and innermost level corresponds to
> > > the template's level-lowered generic arguments.
> > > 
> > > But for A::f here, for which the relevant argument vector is
> > > {{int}, {Us...}}, this substitution triggers the assert in
> > > use_pack_expansion_extra_args_p since one argument is a pack expansion
> > > and the other isn't.
> > > 
> > > And for A::f, for which the relevant argument vector is
> > > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
> > > also trigger but we first get a bogus "mismatched argument pack lengths"
> > > error from tsubst_pack_expansion.
> > > 
> > > These might ultimately be bugs in tsubst_pack_expansion, but it seems
> > > we can work around them by tweaking the constraint substitution in
> > > maybe_substitute_reqs_for to only use the friend's outer arguments in
> > > the case of a template friend.
> > 
> > Yes, this is how we normally substitute a member template during class
> > instantiation: with the class' template args, not its own.  The assert seems
> > to be enforcing that.
> 
> Ah, makes snese.
> 
> > 
> > > This should be otherwise equivalent to
> > > substituting using the full arguments, since the template's innermost
> > > arguments are just its generic arguments with level=1.
> > > 
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > trunk/12?
> > > 
> > >   PR c++/108066
> > >   PR c++/108067
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > >   * constraint.cc (maybe_substitute_reqs_for): For a template
> > >   friend, substitute using only its outer arguments.  Remove
> > >   dead uses_template_parms test.
> > 
> > I don't see any removal?
> 
> Oops, I reverted that change before posting the patch but forgot to adjust the
> ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
> it's not necessary to fix the bug.
> 
> > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > >   * g++.dg/cpp2a/concepts-friend12.C: New test.
> > > ---
> > >   gcc/cp/constraint.cc  |  8 +++
> > >   .../g++.dg/cpp2a/concepts-friend12.C  | 22 +++
> > >   2 files changed, 30 insertions(+)
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > > 
> > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > index d4cd92ec3b4..f9d1009c9fe 100644
> > > --- a/gcc/cp/constraint.cc
> > > +++ b/gcc/cp/constraint.cc
> > > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
> > > decl)
> > > tree tmpl = DECL_TI_TEMPLATE (decl);
> > > tree gargs = generic_targs_for (tmpl);
> > > processing_template_decl_sentinel s;
> > > +  if (PRIMARY_TEMPLATE_P (tmpl))
> > > + {
> > > +   if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> > > + return reqs;
> > > +   ++processing_template_decl;
> > > +   gargs = copy_node (gargs);
> > > +   --TREE_VEC_LENGTH (gargs);
> > 
> > Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
> > for DECL_FRIEND_CONTEXT instead of decl itself?
> 
> IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template 
> friend
> is declared inside a partial specialization:
> 
>   template
>   concept C = __is_same(T, U);
> 
>   template
>   struct A;
> 
>   

[committed] [PR90706] IRA: Check that reg classes contain a hard reg of given mode in reg move cost calculation

[Vladimir Makarov via Gcc-patches]

The following patch solves a spill problem for

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90706

There are still redundant moves which should be removed to solve PR. 
I'll continue my work on this in Jan.



commit 12abd5a7d13209f79664ea603b3f3517f71b8c4f
Author: Vladimir N. Makarov 
Date:   Thu Dec 15 14:11:05 2022 -0500

IRA: Check that reg classes contain a hard reg of given mode in reg move cost calculation

IRA calculates wrong AVR costs for moving general hard regs of SFmode.  To
calculate the costs we did not exclude sub-classes which do not contain
hard regs of given mode.  This was the reason for spilling a pseudo in the
PR. The patch fixes this.

PR rtl-optimization/90706

gcc/ChangeLog:

* ira-costs.cc: Include print-rtl.h.
(record_reg_classes, scan_one_insn): Add code to print debug info.
* ira.cc (ira_init_register_move_cost): Check that at least one hard
reg of the mode are in the class contents to calculate the
register move costs.

gcc/testsuite/ChangeLog:

* gcc.target/avr/pr90706.c: New.

diff --git a/gcc/ira-costs.cc b/gcc/ira-costs.cc
index 964c94a06ef..732a0edd4c1 100644
--- a/gcc/ira-costs.cc
+++ b/gcc/ira-costs.cc
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ira-int.h"
 #include "addresses.h"
 #include "reload.h"
+#include "print-rtl.h"
 
 /* The flags is set up every time when we calculate pseudo register
classes through function ira_set_pseudo_classes.  */
@@ -503,6 +504,18 @@ record_reg_classes (int n_alts, int n_ops, rtx *ops,
   int insn_allows_mem[MAX_RECOG_OPERANDS];
   move_table *move_in_cost, *move_out_cost;
   short (*mem_cost)[2];
+  const char *p;
+
+  if (ira_dump_file != NULL && internal_flag_ira_verbose > 5)
+{
+  fprintf (ira_dump_file, "Processing insn %u", INSN_UID (insn));
+  if (INSN_CODE (insn) >= 0
+	  && (p = get_insn_name (INSN_CODE (insn))) != NULL)
+	fprintf (ira_dump_file, " {%s}", p);
+  fprintf (ira_dump_file, " (freq=%d)\n",
+	   REG_FREQ_FROM_BB (BLOCK_FOR_INSN (insn)));
+  dump_insn_slim (ira_dump_file, insn);
+  }
 
   for (i = 0; i < n_ops; i++)
 insn_allows_mem[i] = 0;
@@ -526,6 +539,21 @@ record_reg_classes (int n_alts, int n_ops, rtx *ops,
 	  continue;
 	}
 
+  if (ira_dump_file != NULL && internal_flag_ira_verbose > 5)
+	{
+	  fprintf (ira_dump_file, "  Alt %d:", alt);
+	  for (i = 0; i < n_ops; i++)
+	{
+	  p = constraints[i];
+	  if (*p == '\0')
+		continue;
+	  fprintf (ira_dump_file, "  (%d) ", i);
+	  for (; *p != '\0' && *p != ',' && *p != '#'; p++)
+		fputc (*p, ira_dump_file);
+	}
+	  fprintf (ira_dump_file, "\n");
+	}
+  
   for (i = 0; i < n_ops; i++)
 	{
 	  unsigned char c;
@@ -593,12 +621,16 @@ record_reg_classes (int n_alts, int n_ops, rtx *ops,
 		 register, this alternative can't be used.  */
 
 		  if (classes[j] == NO_REGS)
-		alt_fail = 1;
-		  /* Otherwise, add to the cost of this alternative
-		 the cost to copy the other operand to the hard
-		 register used for this operand.  */
+		{
+		  alt_fail = 1;
+		}
 		  else
-		alt_cost += copy_cost (ops[j], mode, classes[j], 1, NULL);
+		/* Otherwise, add to the cost of this alternative the cost
+		   to copy the other operand to the hard register used for
+		   this operand.  */
+		{
+		  alt_cost += copy_cost (ops[j], mode, classes[j], 1, NULL);
+		}
 		}
 	  else
 		{
@@ -1021,18 +1053,45 @@ record_reg_classes (int n_alts, int n_ops, rtx *ops,
   for (i = 0; i < n_ops; i++)
 	if (REG_P (ops[i]) && REGNO (ops[i]) >= FIRST_PSEUDO_REGISTER)
 	  {
+	int old_cost;
+	bool cost_change_p = false;
 	struct costs *pp = op_costs[i], *qq = this_op_costs[i];
 	int *pp_costs = pp->cost, *qq_costs = qq->cost;
 	int scale = 1 + (recog_data.operand_type[i] == OP_INOUT);
 	cost_classes_t cost_classes_ptr
 	  = regno_cost_classes[REGNO (ops[i])];
 
-	pp->mem_cost = MIN (pp->mem_cost,
+	old_cost = pp->mem_cost;
+	pp->mem_cost = MIN (old_cost,
 (qq->mem_cost + op_cost_add) * scale);
 
+	if (ira_dump_file != NULL && internal_flag_ira_verbose > 5
+		&& pp->mem_cost < old_cost)
+	  {
+		cost_change_p = true;
+		fprintf (ira_dump_file, "op %d(r=%u) new costs MEM:%d",
+			 i, REGNO(ops[i]), pp->mem_cost);
+	  }
 	for (k = cost_classes_ptr->num - 1; k >= 0; k--)
-	  pp_costs[k]
-		= MIN (pp_costs[k], (qq_costs[k] + op_cost_add) * scale);
+	  {
+		old_cost = pp_costs[k];
+		pp_costs[k]
+		  = MIN (old_cost, (qq_costs[k] + op_cost_add) * scale);
+		if (ira_dump_file != NULL && internal_flag_ira_verbose > 5
+		&& pp_costs[k] < old_cost)
+		  {
+		if (!cost_change_p)
+		  fprintf (ira_dump_file, "op %d(r=%u) new costs",
+			   i, REGNO(ops[i]));
+		cost_change_p = true;
+		fprintf 

[PATCH] tree-optimization/105043: Object Size Checking docs cleanup

[Siddhesh Poyarekar]

Break the _FORTIFY_SOURCE-specific builtins out into a separate
subsection from Object Size Checking built-ins and mention
_FORTIFY_SOURCE in there so that the link between the object size
checking builtins, the helper builtins (e.g. __builtin___memcpy_chk) and
_FORTIFY_SOURCE is clearer.

gcc/ChangeLog:

PR tree-optimization/105043
* doc/extend.texi (Object Size Checking): Split out into two
subsections and mention _FORTIFY_SOURCE.

Signed-off-by: Siddhesh Poyarekar 
---
 gcc/doc/extend.texi | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 608ff54f845..8cff7dd65dd 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -12796,7 +12796,9 @@ __atomic_store_n(, 0, 
__ATOMIC_RELEASE|__ATOMIC_HLE_RELEASE);
 @end smallexample
 
 @node Object Size Checking
-@section Object Size Checking Built-in Functions
+@section Object Size Checking
+
+@subsection Object Size Checking Built-in Functions
 @findex __builtin_object_size
 @findex __builtin_dynamic_object_size
 @findex __builtin___memcpy_chk
@@ -12878,11 +12880,17 @@ which objects @var{ptr} points to at compile time are 
the same as in the case
 of @code{__builtin_object_size}.
 @end deftypefn
 
-There are built-in functions added for many common string operation
-functions, e.g., for @code{memcpy} @code{__builtin___memcpy_chk}
-built-in is provided.  This built-in has an additional last argument,
-which is the number of bytes remaining in the object the @var{dest}
-argument points to or @code{(size_t) -1} if the size is not known.
+@subsection Object Size Checking and Source Fortification
+
+Hardening of function calls using the @code{_FORTIFY_SOURCE} macro is
+one of the key uses of the object size checking built-in functions.  To
+make implementation of these features more convenient and improve
+optimization and diagnostics, there are built-in functions added for
+many common string operation functions, e.g., for @code{memcpy}
+@code{__builtin___memcpy_chk} built-in is provided.  This built-in has
+an additional last argument, which is the number of bytes remaining in
+the object the @var{dest} argument points to or @code{(size_t) -1} if
+the size is not known.
 
 The built-in functions are optimized into the normal string functions
 like @code{memcpy} if the last argument is @code{(size_t) -1} or if
-- 
2.38.1

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Patrick Palka via Gcc-patches]

On Thu, 15 Dec 2022, Jason Merrill wrote:

> On 12/12/22 12:20, Patrick Palka wrote:
> > When instantiating a constrained hidden template friend, we need to
> > substitute into its constraints for sake of declaration matching.
> 
> Hmm, we shouldn't need to do declaration matching when there's only a single
> declaration.

Oops, indeed.  It looks like in this case we're not calling
maybe_substitute_reqs_for during declaration matching, but rather from
tsubst_friend_function and specifically for the non-trailing requirements:

  if (TREE_CODE (new_friend) == TEMPLATE_DECL)
{
  DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (new_friend) = false;
  DECL_USE_TEMPLATE (DECL_TEMPLATE_RESULT (new_friend)) = 0;
  DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (new_friend))
= DECL_SAVED_TREE (DECL_TEMPLATE_RESULT (decl));

  /* Substitute TEMPLATE_PARMS_CONSTRAINTS so that parameter levels will
 match in decls_match.  */
  tree parms = DECL_TEMPLATE_PARMS (new_friend);
  tree treqs = TEMPLATE_PARMS_CONSTRAINTS (parms);
  treqs = maybe_substitute_reqs_for (treqs, new_friend);
  if (treqs != TEMPLATE_PARMS_CONSTRAINTS (parms))
{
  TEMPLATE_PARMS_CONSTRAINTS (parms) = treqs;
  /* As well as each TEMPLATE_PARM_CONSTRAINTS.  */
  tsubst_each_template_parm_constraints (parms, args,
 tf_warning_or_error);
}
}

I'll adjust the commit message appropriately.

> 
> > For
> > this substitution we use a full argument vector whose outer levels
> > correspond to the class's arguments and innermost level corresponds to
> > the template's level-lowered generic arguments.
> > 
> > But for A::f here, for which the relevant argument vector is
> > {{int}, {Us...}}, this substitution triggers the assert in
> > use_pack_expansion_extra_args_p since one argument is a pack expansion
> > and the other isn't.
> > 
> > And for A::f, for which the relevant argument vector is
> > {{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
> > also trigger but we first get a bogus "mismatched argument pack lengths"
> > error from tsubst_pack_expansion.
> > 
> > These might ultimately be bugs in tsubst_pack_expansion, but it seems
> > we can work around them by tweaking the constraint substitution in
> > maybe_substitute_reqs_for to only use the friend's outer arguments in
> > the case of a template friend.
> 
> Yes, this is how we normally substitute a member template during class
> instantiation: with the class' template args, not its own.  The assert seems
> to be enforcing that.

Ah, makes snese.

> 
> > This should be otherwise equivalent to
> > substituting using the full arguments, since the template's innermost
> > arguments are just its generic arguments with level=1.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/12?
> > 
> > PR c++/108066
> > PR c++/108067
> > 
> > gcc/cp/ChangeLog:
> > 
> > * constraint.cc (maybe_substitute_reqs_for): For a template
> > friend, substitute using only its outer arguments.  Remove
> > dead uses_template_parms test.
> 
> I don't see any removal?

Oops, I reverted that change before posting the patch but forgot to adjust the
ChangeLog as well.  Removing the uses_template_parms test seems to be safe but
it's not necessary to fix the bug.

> 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/cpp2a/concepts-friend12.C: New test.
> > ---
> >   gcc/cp/constraint.cc  |  8 +++
> >   .../g++.dg/cpp2a/concepts-friend12.C  | 22 +++
> >   2 files changed, 30 insertions(+)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index d4cd92ec3b4..f9d1009c9fe 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree
> > decl)
> > tree tmpl = DECL_TI_TEMPLATE (decl);
> > tree gargs = generic_targs_for (tmpl);
> > processing_template_decl_sentinel s;
> > +  if (PRIMARY_TEMPLATE_P (tmpl))
> > +   {
> > + if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
> > +   return reqs;
> > + ++processing_template_decl;
> > + gargs = copy_node (gargs);
> > + --TREE_VEC_LENGTH (gargs);
> 
> Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the targs
> for DECL_FRIEND_CONTEXT instead of decl itself?

IIUC DECL_FRIEND_CONTEXT wouldn't give us the right args if the template friend
is declared inside a partial specialization:

  template
  concept C = __is_same(T, U);

  template
  struct A;

  template
  struct A {
template
  requires (__is_same(T, U))
friend void f() { };
  };

  template struct A;

The DECL_FRIEND_CONTEXT of f is A, whose TYPE_TI_ARGS are {int*},
relative to the primary template instead of the partial specialization.  So
we'd wrongly 

Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 12:49:27PM -0600, Segher Boessenkool wrote:
> On Thu, Dec 15, 2022 at 06:28:19PM +, Joseph Myers wrote:
> > On Thu, 15 Dec 2022, Kewen.Lin via Gcc-patches wrote:
> > > By investigating the exposed NaN failures, I found it's due to that it 
> > > wants
> > > to convert _Float128 type constant to long double type constant, it goes
> > > through function real_convert which clears the signalling bit in the 
> > > context
> > > of !HONOR_SNANS (arg).
> > > 
> > >   if (r->cl == rvc_nan)
> > > r->signalling = 0;
> > > 
> > > The test cases don't have the explicit option -fsignaling-nans, I'm 
> > > inclined
> > > to believe it's intentional since there is only a sNaN generation.  If so,
> > > we don't want this kind of conversion which is useless and can clear 
> > > signalling
> > > bit unexpectedly, one shortcut is to just copy the corresponding 
> > > REAL_VALUE_TYPE
> > > and rebuild with the given type if the modes are the same.
> > 
> > I think this approach - treating floating-point conversions to a type with 
> > the same mode consistently as a copy rather than a convertFormat operation 
> > - is reasonable.
> 
> Certainly.  But different types with the same mode having different
> precision is not so very reasonable, and will likely cause other
> problems as well.
> 
> We cannot use precision to order modes or types, that is the core
> problem.  A conversion from IEEE QP to double-double (or vice versa) is
> neither widening nor narrowing.

Sure.  For optabs, I bet we don't necessarily need to care that much, if
precision is the same, we can ask for widening and narrowing conversion
and expect only one to be implemented or both doing the same thing between
such modes.  But when using libcalls, which library function we use is quite
important because not all of them might be actually implemented in the
library (better keep doing what we've done before).

Jakub

Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Segher Boessenkool]

On Thu, Dec 15, 2022 at 06:28:19PM +, Joseph Myers wrote:
> On Thu, 15 Dec 2022, Kewen.Lin via Gcc-patches wrote:
> > By investigating the exposed NaN failures, I found it's due to that it wants
> > to convert _Float128 type constant to long double type constant, it goes
> > through function real_convert which clears the signalling bit in the context
> > of !HONOR_SNANS (arg).
> > 
> >   if (r->cl == rvc_nan)
> > r->signalling = 0;
> > 
> > The test cases don't have the explicit option -fsignaling-nans, I'm inclined
> > to believe it's intentional since there is only a sNaN generation.  If so,
> > we don't want this kind of conversion which is useless and can clear 
> > signalling
> > bit unexpectedly, one shortcut is to just copy the corresponding 
> > REAL_VALUE_TYPE
> > and rebuild with the given type if the modes are the same.
> 
> I think this approach - treating floating-point conversions to a type with 
> the same mode consistently as a copy rather than a convertFormat operation 
> - is reasonable.

Certainly.  But different types with the same mode having different
precision is not so very reasonable, and will likely cause other
problems as well.

We cannot use precision to order modes or types, that is the core
problem.  A conversion from IEEE QP to double-double (or vice versa) is
neither widening nor narrowing.


Segher

Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Joseph Myers]

On Thu, 15 Dec 2022, Kewen.Lin via Gcc-patches wrote:

> By investigating the exposed NaN failures, I found it's due to that it wants
> to convert _Float128 type constant to long double type constant, it goes
> through function real_convert which clears the signalling bit in the context
> of !HONOR_SNANS (arg).
> 
>   if (r->cl == rvc_nan)
> r->signalling = 0;
> 
> The test cases don't have the explicit option -fsignaling-nans, I'm inclined
> to believe it's intentional since there is only a sNaN generation.  If so,
> we don't want this kind of conversion which is useless and can clear 
> signalling
> bit unexpectedly, one shortcut is to just copy the corresponding 
> REAL_VALUE_TYPE
> and rebuild with the given type if the modes are the same.

I think this approach - treating floating-point conversions to a type with 
the same mode consistently as a copy rather than a convertFormat operation 
- is reasonable.

-- 
Joseph S. Myers
jos...@codesourcery.com

nvptx: Make 'nvptx_uniform_warp_check' fit for non-full-warp execution (was: [committed][nvptx] Add uniform_warp_check insn)

[Thomas Schwinge]

Hi Tom!

First "a bit" of context; skip to "the proposed patch" if you'd like to
see just that.


On 2022-02-01T19:31:27+0100, Tom de Vries via Gcc-patches 
 wrote:
> On a GT 1030, with driver version 470.94 and -mptx=3.1 I run into:
> ...
> FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/parallel-dims.c \
>   -DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 -foffload=nvptx-none \
>   -O2 execution test
> ...
> which minimizes to the same test-case as listed in commit "[nvptx]
> Update default ptx isa to 6.3".
>
> The problem is again that the first diverging branch is not handled as such in
> SASS, which causes problems with a subsequent shfl insn, but given that we
> have -mptx=3.1 we can't use the bar.warp.sync insn.
>
> Given that the default is now -mptx=6.3, and consequently -mptx=3.1 is of a
> lesser importance, implement the next best thing: abort when detecting
> non-convergence using this insn:
> ...
>   { .reg.b32 act;
> vote.ballot.b32 act,1;
> .reg.pred uni;
> setp.eq.b32 uni,act,0x;
> @ !uni trap;
> @ !uni exit;
>   }
> ...
>
> Interestingly, the effect of this is that rather than aborting, the test-case
> now passes.

(I suppose this "nudges" the PTX -> SASS compiler into the right
direction?)


For avoidance of doubt, my following discussion is not about the specific
(first) use of 'nvptx_uniform_warp_check' introduced here in this
commit r12-6971-gf32f74c2e8cef5fe37af6d4e8d7e8f6b4c8ae9a8
"[nvptx] Add uniform_warp_check insn":

> --- a/gcc/config/nvptx/nvptx.cc
> +++ b/gcc/config/nvptx/nvptx.cc
> @@ -4631,15 +4631,29 @@ nvptx_single (unsigned mask, basic_block from, 
> basic_block to)
>   if (tail_branch)
> {
>   label_insn = emit_label_before (label, before);
> - if (TARGET_PTX_6_0 && mode == GOMP_DIM_VECTOR)
> -   warp_sync = emit_insn_after (gen_nvptx_warpsync (), label_insn);
> + if (mode == GOMP_DIM_VECTOR)
> +   {
> + if (TARGET_PTX_6_0)
> +   warp_sync = emit_insn_after (gen_nvptx_warpsync (),
> +label_insn);
> + else
> +   warp_sync = emit_insn_after (gen_nvptx_uniform_warp_check (),
> +label_insn);
> +   }
>   before = label_insn;
> }
>   else
> {
>   label_insn = emit_label_after (label, tail);
> - if (TARGET_PTX_6_0 && mode == GOMP_DIM_VECTOR)
> -   warp_sync = emit_insn_after (gen_nvptx_warpsync (), label_insn);
> + if (mode == GOMP_DIM_VECTOR)
> +   {
> + if (TARGET_PTX_6_0)
> +   warp_sync = emit_insn_after (gen_nvptx_warpsync (),
> +label_insn);
> + else
> +   warp_sync = emit_insn_after (gen_nvptx_uniform_warp_check (),
> +label_insn);
> +   }
>   if ((mode == GOMP_DIM_VECTOR || mode == GOMP_DIM_WORKER)
>   && CALL_P (tail) && find_reg_note (tail, REG_NORETURN, NULL))
> emit_insn_after (gen_exit (), label_insn);

Later, other uses have been added, for example in OpenMP '-muniform-simt'
code generation.

My following discussion is about the implementation of
'nvptx_uniform_warp_check', originally introduced as follows:

> --- a/gcc/config/nvptx/nvptx.md
> +++ b/gcc/config/nvptx/nvptx.md
> @@ -57,6 +57,7 @@ (define_c_enum "unspecv" [
> UNSPECV_XCHG
> UNSPECV_BARSYNC
> UNSPECV_WARPSYNC
> +   UNSPECV_UNIFORM_WARP_CHECK
> UNSPECV_MEMBAR
> UNSPECV_MEMBAR_CTA
> UNSPECV_MEMBAR_GL
> @@ -1985,6 +1986,23 @@ (define_insn "nvptx_warpsync"
>"\\tbar.warp.sync\\t0x;"
>[(set_attr "predicable" "false")])
>
> +(define_insn "nvptx_uniform_warp_check"
> +  [(unspec_volatile [(const_int 0)] UNSPECV_UNIFORM_WARP_CHECK)]
> +  ""
> +  {
> +output_asm_insn ("{", NULL);
> +output_asm_insn ("\\t"".reg.b32""\\t" "act;", NULL);
> +output_asm_insn ("\\t""vote.ballot.b32" "\\t" "act,1;", NULL);
> +output_asm_insn ("\\t"".reg.pred"   "\\t" "uni;", NULL);
> +output_asm_insn ("\\t""setp.eq.b32" "\\t" "uni,act,0x;",
> +  NULL);
> +output_asm_insn ("@ !uni\\t" "trap;", NULL);
> +output_asm_insn ("@ !uni\\t" "exit;", NULL);
> +output_asm_insn ("}", NULL);
> +return "";
> +  }
> +  [(set_attr "predicable" "false")])

Later adjusted, but the fundamental idea is still the same.


Via temporarily disabling 'nvptx_uniform_warp_check':

 (define_insn "nvptx_uniform_warp_check"
   [(unspec_volatile [(const_int 0)] UNSPECV_UNIFORM_WARP_CHECK)]
   ""
   {
+#if 0
 const char *insns[] = {
   "{",
   "\\t"  ".reg.b32""\\t" "%%r_act;",
   "%.\\t""vote.ballot.b32" "\\t" "%%r_act,1;",
   "\\t"  ".reg.pred"   "\\t" 

Re: [PATCH] contracts: Stop relying on mangling for naming .pre/.post clones

[Arsen Arsenović via Gcc-patches]

Hi Jason,

Jason Merrill  writes:

> On 12/10/22 08:13, Arsen Arsenović wrote:
>> If the mangler is relied on, functions with extern "C" on them emit multiple
>> definitions of the same name.
>
> But doing it here interferes with lazy mangling.  How about appending the
> suffix into write_mangled_name instead of write_encoding?  The demangler
> already expects "clone" suffixes at the end of the mangled name.

Ah, sorry.  I'm not well versed in the mangler code, so I didn't realize
(frankly, I was initially surprised when I saw that DECL_ASSEMBLER_NAME
was set that early, but went with it).  That makes sense.

How about this?  Tested on x86_64-pc-linux-gnu via check-g++.

From 2a2d98e94bdd7a8d7f862b2accda849927e4509e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= 
Date: Thu, 15 Dec 2022 18:56:59 +0100
Subject: [PATCH v2] c++: Mangle contracts in write_mangled_name
 unconditionally

This fixes contract-checked extern "C" functions.

gcc/cp/ChangeLog:

* mangle.cc (write_encoding): Move contract pre/post function mangling
from here...
(write_mangled_name): ... to here, and make it happen always.

gcc/testsuite/ChangeLog:

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

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index e363ef35b9f..074cf27ec7a 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -798,6 +798,13 @@ write_mangled_name (const tree decl, bool top_level)
   write_string ("_Z");
   write_encoding (decl);
 }
+
+  /* If this is the pre/post function for a guarded function, append
+ .pre/post, like something from create_virtual_clone.  */
+  if (DECL_IS_PRE_FN_P (decl))
+write_string (".pre");
+  else if (DECL_IS_POST_FN_P (decl))
+write_string (".post");
 }
 
 /* Returns true if the return type of DECL is part of its signature, and
@@ -856,13 +863,6 @@ write_encoding (const tree decl)
mangle_return_type_p (decl),
d);
 
-  /* If this is the pre/post function for a guarded function, append
-.pre/post, like something from create_virtual_clone.  */
-  if (DECL_IS_PRE_FN_P (decl))
-   write_string (".pre");
-  else if (DECL_IS_POST_FN_P (decl))
-   write_string (".post");
-
   /* If this is a coroutine helper, then append an appropriate string to
 identify which.  */
   if (tree ramp = DECL_RAMP_FN (decl))
diff --git a/gcc/testsuite/g++.dg/contracts/contracts-externC.C 
b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
new file mode 100644
index 000..873056b742b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
@@ -0,0 +1,19 @@
+// simple check to ensure we don't emit a function with the same name twice,
+// when wrapping functions in pre- and postconditions.
+// { dg-do link }
+// { dg-options "-std=c++2a -fcontracts -fcontract-continuation-mode=on" }
+
+volatile int x = 10;
+
+extern "C" void
+f ()
+  [[ pre: x < 10 ]]
+{
+}
+
+int
+main ()
+  [[ post: x > 10 ]]
+{
+  f();
+}
-- 
2.39.0


I did run c++filt (afaik, it uses the libiberty demangler) on this
revision, and I got:

  .type f(int) [clone .pre], @function
  f(int) [clone .pre]:

out of ``void f(int x) [[ pre: x > 10 ]] {}'', which seems to match your
description.

If I understand this right, write_xxx corresponds to xxx in the Itanium
ABI mangling BNF, in which case, I believe I have the correct spot here.
In that case, a similar change should happen for coroutines; I think
Iain was working on that.

Thanks, have a great day.
-- 
Arsen Arsenović


signature.asc
Description: PGP signature

Re: [PATCH 2/3] Make __float128 use the _Float128 type, PR target/107299

[Segher Boessenkool]

Hi!

On Wed, Dec 14, 2022 at 10:36:03AM +0100, Jakub Jelinek wrote:
> On Wed, Dec 14, 2022 at 04:46:07PM +0800, Kewen.Lin via Gcc-patches wrote:
> > Since function useless_type_conversion_p considers two float types are 
> > compatible
> > if they have the same mode, so it doesn't require the explicit conversions 
> > between
> > these two types.  I think it's exactly what we want.  And to me, it looks 
> > unexpected
> > to have two types with the same mode but different precision.
> > 
> > So could we consider disabling the above workaround to make _Float128 have 
> > the same
> > precision as __float128 (long double) (the underlying TFmode)?  I tried the 
> > below
> > change:
> 
> The hacks with different precisions of powerpc 128-bit floating types are
> very unfortunate, it is I assume because the middle-end asserted that scalar
> floating point types with different modes have different precision.

IEEE QP and double-double cannot be ordered, neither represents a subset
of the other.  But the current middle end does require a total ordering
for all floating point types (that can be converted to each other).

Mike's precision hack was supposed to give us some time until an actual
fix was made.  But no one has worked on that, and there have been
failures found with the precision hack as well, it worked remarkably
well but it isn't perfect.

We cannot move forward in a meaningful way until these problems are
fixed.  We can move around like headless chickens some more of course.


Segher

Re: [PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Jan Hubicka via Gcc-patches]

> On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
>  wrote:
> >
> > > Hi,
> > >
> > > Honza requested this after reviewing the patch that taught IPA-SRA
> > > that REFERENCE_TYPEs are always non-NULL that the pass also handles
> > > the first parameters of methods, this pointers, in the same way.  So
> > > this patch does that.
> > >
> > > The patch is undergoing bootstrap and testing on an x86_64-linux right
> > > now.  OK if it passes?
> > >
> > > Thanks,
> > >
> > > Martin
> > >
> > >
> > > gcc/ChangeLog:
> > >
> > > 2022-12-14  Martin Jambor  
> > >
> > >   * ipa-sra.cc (create_parameter_descriptors): Consider the first
> > >   parameter of a method safe to dereference.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 2022-12-14  Martin Jambor  
> > >
> > >   * g++.dg/ipa/ipa-sra-6.C: New test.
> >
> > OK,
> 
> Are you sure that's safe?  The docs for METHOD_TYPE doesn't say 'this'
> is the first parameter nor does it say methods cannot be invoked for a
> nullptr object.
> 
> Do frontends other than C++ create METHOD_TYPE functions?  Grep
> shows uses in ada and objective C at least.

I think for Objective-C++ calling method is also considerd a
dereference.  We make similar assumptions in devirtualization code for
some time.  We could also check that the type is METHOD_TYPE satisfies
odr_type_p if we want to look specifically for C++ methods.

Honza
> 
> > thanks a lot!
> > Honza

Re: [Patch] libgomp: Handle OpenMP's reverse offloads

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 06:34:30PM +0100, Thomas Schwinge wrote:
> --- a/libgomp/libgomp-plugin.c
> +++ b/libgomp/libgomp-plugin.c
> @@ -82,9 +82,9 @@ GOMP_PLUGIN_fatal (const char *msg, ...)
>  void
>  GOMP_PLUGIN_target_rev (uint64_t fn_ptr, uint64_t mapnum, uint64_t 
> devaddrs_ptr,
>   uint64_t sizes_ptr, uint64_t kinds_ptr, int dev_num,
> - void (*dev_to_host_cpy) (void *, const void *, size_t,
> + bool (*dev_to_host_cpy) (void *, const void *, size_t,
>void *),
> - void (*host_to_dev_cpy) (void *, const void *, size_t,
> + bool (*host_to_dev_cpy) (void *, const void *, size_t,
>void *), void *token)
>  {
>gomp_target_rev (fn_ptr, mapnum, devaddrs_ptr, sizes_ptr, kinds_ptr, 
> dev_num,
> diff --git a/libgomp/libgomp-plugin.h b/libgomp/libgomp-plugin.h
> index ac3878289506..fb533164bf9b 100644
> --- a/libgomp/libgomp-plugin.h
> +++ b/libgomp/libgomp-plugin.h
> @@ -122,9 +122,9 @@ extern void GOMP_PLUGIN_fatal (const char *, ...)
>  
>  extern void GOMP_PLUGIN_target_rev (uint64_t, uint64_t, uint64_t, uint64_t,
>   uint64_t, int,
> - void (*) (void *, const void *, size_t,
> + bool (*) (void *, const void *, size_t,
> void *),
> - void (*) (void *, const void *, size_t,
> + bool (*) (void *, const void *, size_t,
> void *), void *);
>  
>  /* Prototypes for functions implemented by libgomp plugins.  */
> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -1130,8 +1130,8 @@ extern int gomp_get_num_devices (void);
>  extern bool gomp_target_task_fn (void *);
>  extern void gomp_target_rev (uint64_t, uint64_t, uint64_t, uint64_t, 
> uint64_t,
>int,
> -  void (*) (void *, const void *, size_t, void *),
> -  void (*) (void *, const void *, size_t, void *),
> +  bool (*) (void *, const void *, size_t, void *),
> +  bool (*) (void *, const void *, size_t, void *),
>void *);
>  
>  /* Splay tree definitions.  */

I think returning bool from those is fine.

> --- a/libgomp/plugin/plugin-nvptx.c
> +++ b/libgomp/plugin/plugin-nvptx.c
> @@ -1,3 +1,5 @@
> +#pragma GCC optimize "O0"

But the pragmas are not.

> --- a/libgomp/target.c
> +++ b/libgomp/target.c
> @@ -1,3 +1,5 @@
> +#pragma GCC optimize "O0"

Neither here.

> @@ -3340,12 +3342,21 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, 
> uint64_t devaddrs_ptr,
>kinds = (unsigned short *) gomp_malloc (mapnum * sizeof (unsigned 
> short));
>if (dev_to_host_cpy)
> {
> - dev_to_host_cpy (devaddrs, (const void *) (uintptr_t) devaddrs_ptr,
> -  mapnum * sizeof (uint64_t), token);
> - dev_to_host_cpy (sizes, (const void *) (uintptr_t) sizes_ptr,
> -  mapnum * sizeof (uint64_t), token);
> - dev_to_host_cpy (kinds, (const void *) (uintptr_t) kinds_ptr,
> -  mapnum * sizeof (unsigned short), token);
> + bool ok = true;
> + ok = ok && dev_to_host_cpy (devaddrs,
> + (const void *) (uintptr_t) devaddrs_ptr,
> + mapnum * sizeof (uint64_t), token);
> + ok = ok && dev_to_host_cpy (sizes,
> + (const void *) (uintptr_t) sizes_ptr,
> + mapnum * sizeof (uint64_t), token);
> + ok = ok && dev_to_host_cpy (kinds,
> + (const void *) (uintptr_t) kinds_ptr,
> + mapnum * sizeof (unsigned short), 
> token);

Why not just
if (!dev_to_host_cpy (...)
|| !dev_to_host_cpy (...)
|| !dev_to_host_cpy (...))
?

> + if (!ok)
> +   {
> + /*TODO gomp_mutex_unlock (>lock); */

Why the comment?  That makes no sense, devicep->lock isn't locked here.

>   else if (dev_to_host_cpy)
> -   dev_to_host_cpy (tgt + tgt_size, (void *) (uintptr_t) devaddrs[i],
> -(size_t) sizes[i], token);
> +   {
> + if (!dev_to_host_cpy (tgt + tgt_size,
> +   (void *) (uintptr_t) devaddrs[i],
> +   (size_t) sizes[i], token))
> +   {
> + /*TODO gomp_mutex_unlock (>lock); */

Neither here.
> @@ -3662,9 +3692,15 @@ gomp_target_rev (uint64_t fn_ptr, uint64_t mapnum, 
> uint64_t devaddrs_ptr,
> case GOMP_MAP_FROM:
> case 

Re: [PATCH 2/4] libstdc++: Improve output of default contract violation handler [PR107792]

[Arsen Arsenović via Gcc-patches]

Hi,

Jason Merrill  writes:

>
> I'd actually suggest "off" and "on" since it's really a boolean since
> "always_continue" was dropped.
>
>> +  std::cerr << "contract violation in function " << 
>> violation.function_name()
>> +<< " at " << violation.file_name() << ':' << violation.line_number()
>> +<< ": " << violation.comment()
>> +<< "\n[level:" << violation.assertion_level()
>
> Maybe omit level/role if "default"?

ACK on both, sounds good.

Have a great day.
-- 
Arsen Arsenović


signature.asc
Description: PGP signature

Re: [PATCH 1/4] contracts: Lowercase {MAYBE,NEVER}_CONTINUE

[Arsen Arsenović via Gcc-patches]

Hi,

Jason Merrill  writes:

>> The lowercase constants are more consistent with the standard, and it is
>> unlikely that the uppercase versions would've been accepted.
>
> OK.

Thanks.  Could you push this for me?  I don't have write access.

Have a great day.
-- 
Arsen Arsenović


signature.asc
Description: PGP signature

Re: [PATCH 4/4] contrib: Add dg-out-generator.pl

[Arsen Arsenović via Gcc-patches]

Hi Jason,

Jason Merrill  writes:

> I wonder if you want to wrap the pattern in {} instead of "" so you don't need
> the "special stuff in TCL itself" quoting?

{}s lack generality, for instance, try: puts {unbalanced \}}.  I could
try to write a revision that complies with the minimal escaping style
when I take the opportunity to address your other comment.

(also, it just occurred to me that I forgot to escape dollar signs)

Thanks, have a great day.
-- 
Arsen Arsenović


signature.asc
Description: PGP signature

Re: [Patch] libgomp: Handle OpenMP's reverse offloads

[Thomas Schwinge]

Hi!

On 2022-12-06T08:45:07+0100, Tobias Burnus  wrote:
> This patch finally handles reverse offload.

Yay!  \o/


The 'libgomp.fortran/reverse-offload-5.f90' test case for nvptx
offloading runs into an error condition (thus, XFAILed) -- but then
blocks until timeout, insted of terminating promptly:

libgomp: cuMemcpyDtoHAsync error: invalid argument
WARNING: program timed out.
XFAIL: libgomp.fortran/reverse-offload-5.f90   -O  execution test

Attempt to fix that by not calling 'CUDA_CALL_ASSERT'/'GOMP_PLUGIN_fatal'
with device lock held; see
"libgomp: Handle OpenMP's reverse offloads, unlocking on error paths, pt. 1"
attached.  OK to push?  That unfortunately doesn't resolve the issue (get
another hang elsewhere, later on), but does seem like an improvement
anyway?


We then hang when tearing down the device:

#7  0x772f7137 in nvptx_close_device (ptx_dev=0x6cf580) at 
[...]/libgomp/plugin/plugin-nvptx.c:576
#8  0x772f88e6 in GOMP_OFFLOAD_fini_device (n=0) at 
[...]/libgomp/plugin/plugin-nvptx.c:1238
#9  0x778d767e in gomp_fini_device (devicep=0x6cf400) at 
[...]/libgomp/target.c:2648
#10 0x778de69f in gomp_target_fini () at [...]/libgomp/target.c:5026
#11 0x776b88a7 in __run_exit_handlers (status=1, 
listp=0x7785e718 <__exit_funcs>, 
run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at 
exit.c:108
#12 0x776b8a60 in __GI_exit (status=) at exit.c:139
#13 0x778d9df5 in gomp_target_rev (fn_ptr=2870272, mapnum=21, 
devaddrs_ptr=140736641236016, sizes_ptr=140736641235848, 
kinds_ptr=14073274304, dev_num=0, dev_to_host_cpy=0x772fae00 
, host_to_dev_cpy=0x772faec0 
, token=0xb0f910) at [...]/libgomp/target.c:3508
#14 0x772fb3c1 in GOMP_OFFLOAD_run (ord=0, tgt_fn=0xad0fd8, 
tgt_vars=0x7fffcd80, args=0x7fffd140) at 
[...]/libgomp/plugin/plugin-nvptx.c:2155
#15 0x778d8b91 in GOMP_target_ext (device=-1, fn=0x4012aa 
, mapnum=11, hostaddrs=0x7fffd560, sizes=0x7fffd500, 
kinds=0x6adb20 , flags=0, depend=0x0, args=0x7fffd670) 
at [...]/libgomp/target.c:3146
#16 0x00400f44 in MAIN__ ()

That is:

(gdb) frame 7
#7  0x772f7137 in nvptx_close_device (ptx_dev=0x6cf580) at 
[...]/libgomp/plugin/plugin-nvptx.c:576
576 CUDA_CALL (cuMemFree, ptx_dev->omp_stacks.ptr);

At the point where we called 'gomp_target_rev' via
'GOMP_PLUGIN_target_rev' in 'GOMP_OFFLOAD_run', 'omp_stacks' was still in
active use, and so the 'cuMemFree' here waits for that CUDA stream to
complete?  (I haven't looked up the exact details, because...)

..., then I found that I generally don't understand the locking scheme
applied here.

If the libgomp plugin doesn't request special
'host_to_dev_cpy'/'dev_to_host_cpy' for 'gomp_target_rev', then standard
'gomp_copy_host2dev'/'gomp_copy_dev2host' are used, which use
'gomp_device_copy', which expects the device to be locked.  (As can be
told by the unconditional 'gomp_mutex_unlock (>lock);' before
'gomp_fatal'.)  However, in a number of the
'gomp_copy_host2dev'/'gomp_copy_dev2host' calls from 'gomp_target_rev',
the device definitely is not locked; see the calls adjacent to the TODO
markers in my patch.  That means, we potentially
'gomp_mutex_unlock (>lock);' when not actually holding that
lock?

How, generally, is libgomp device locking supposed to interact with
OpenMP reverse offloading?


Grüße
 Thomas


> Due to the prep work,
> it essentially only adds content to libgomp/target.c's gomp_target_rev(),
> except that it additionally saves the reverse-offload-function table
> in gomp_load_image_to_device.
>
> In the comment to "[Patch] libgomp: Add reverse-offload splay tree",
> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601368.html ,
> it was suggested not to keep track of all the variable mappings and
> to reconstruct the mapping from the normal splay tree, which this
> patch does.
> (Albeit in the very slow walk-everything way. Given that reverse-offload
> target regions likely have only few map items and program should only use
> few reverse-offload regions and expect them not being fast, that might
> be okay.)
>
> Specification references:
> - For pointer attachment, I assume that the pointer is already fine on
>the host (if existed on the host before) and it does not need to get
>updated. I think the spec lacks a wording for this; cf. OpenMP Spec Issue 
> #3424.
> - There are plans to permit 'nowait'. I think it wouldn't change anything
>except for not spin waiting for the result - and (only for shared memory),
>the argument lists (addr, kinds, sizes) need to be copied to have a 
> sufficent
>life time. (To be implemented in future; cf. OpenMP Spec Pull Req. 3423
>for Issue 2038.)
>
>   * * *
>
> 32bit vs. 64bit: libgomp itself is compiled with both -m32 and -m64; however,
> nvptx and gcn requires -m64 on the device side and assume that the device
> 

Re: [PATCH] doc: Fix documentation for __builtin_dynamic_object_size

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 12:21:21PM -0500, Siddhesh Poyarekar wrote:
> On 2022-12-15 12:09, Jakub Jelinek wrote:
> > > -This is a propagation pass similar to CCP that tries to remove calls
> > > -to @code{__builtin_object_size} when the size of the object can be
> > > -computed at compile-time.  This pass is located in
> > > -@file{tree-object-size.cc} and is described by
> > > +This is a propagation pass similar to CCP that tries to remove calls to
> > > +@code{__builtin_object_size} or @code{__builtin_dynamic_object_size}
> > > +when the size of the object can be computed at compile-time.  This pass
> > 
> > Can be computed at compile-time is only true for __bos, for
> > __bdos is if it can be computed.
> 
> How about:
> 
> 
> This is a propagation pass similar to CCP that tries to remove calls to
> @code{__builtin_object_size} when the size of the object can be computed at
> compile-time.  It also tries to replace calls to
> 
> @code{__builtin_dynamic_object_size} with an expression that evaluates
> the size of the object.  This pass is located in
> @file{tree-object-size.cc} and is described by @code{pass_object_sizes}.

Without the empty space in between and perhaps with replacing size with
upper or lower bound for the size of the object in both cases ok.

Jakub

Re: [PATCH] doc: Fix documentation for __builtin_dynamic_object_size

[Siddhesh Poyarekar]

On 2022-12-15 12:09, Jakub Jelinek wrote:

-This is a propagation pass similar to CCP that tries to remove calls
-to @code{__builtin_object_size} when the size of the object can be
-computed at compile-time.  This pass is located in
-@file{tree-object-size.cc} and is described by
+This is a propagation pass similar to CCP that tries to remove calls to
+@code{__builtin_object_size} or @code{__builtin_dynamic_object_size}
+when the size of the object can be computed at compile-time.  This pass


Can be computed at compile-time is only true for __bos, for
__bdos is if it can be computed.


How about:


This is a propagation pass similar to CCP that tries to remove calls to
@code{__builtin_object_size} when the size of the object can be computed 
at compile-time.  It also tries to replace calls to 


@code{__builtin_dynamic_object_size} with an expression that evaluates
the size of the object.  This pass is located in
@file{tree-object-size.cc} and is described by @code{pass_object_sizes}.

Re: [PATCH] doc: Fix documentation for __builtin_dynamic_object_size

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 11:58:14AM -0500, Siddhesh Poyarekar wrote:
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -14291,8 +14291,14 @@ and GCC does not issue a warning.
>  @end deftypefn
>  
>  @deftypefn {Built-in Function}{size_t} __builtin_object_size (const void * 
> @var{ptr}, int @var{type})
> -Returns the size of an object pointed to by @var{ptr}.  @xref{Object Size
> -Checking}, for a detailed description of the function.
> +Returns a constant size estimate of an object pointed to by @var{ptr}.
> +@xref{Object Size Checking}, for a detailed description of the function.
> +@end deftypefn
> +
> +@deftypefn {Built-in Function}{size_t} __builtin_dynamic_object_size (const 
> void * @var{ptr}, int @var{type})
> +Similar to @code{__builtin_object_size} except that the return value
> +need not be a constant.  @xref{Object Size Checking}, for a detailed
> +description of the function.
>  @end deftypefn
>  
>  @deftypefn {Built-in Function} double __builtin_huge_val (void)

The above is ok.

> diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
> index 9e8b4f50ad6..d649db72bbe 100644
> --- a/gcc/doc/passes.texi
> +++ b/gcc/doc/passes.texi
> @@ -843,12 +843,13 @@ foo()}, this pass tries to change the call so that the 
> address of
>  pass is located in @code{tree-nrv.cc} and is described by
>  @code{pass_return_slot}.
>  
> -@item Optimize calls to @code{__builtin_object_size}
> +@item Optimize calls to @code{__builtin_object_size} or
> +@code{__builtin_dynamic_object_size}
>  
> -This is a propagation pass similar to CCP that tries to remove calls
> -to @code{__builtin_object_size} when the size of the object can be
> -computed at compile-time.  This pass is located in
> -@file{tree-object-size.cc} and is described by
> +This is a propagation pass similar to CCP that tries to remove calls to
> +@code{__builtin_object_size} or @code{__builtin_dynamic_object_size}
> +when the size of the object can be computed at compile-time.  This pass

Can be computed at compile-time is only true for __bos, for
__bdos is if it can be computed.

> +is located in @file{tree-object-size.cc} and is described by
>  @code{pass_object_sizes}.
>  
>  @item Loop invariant motion

Otherwise LGTM.

Jakub

Re: [PATCH] c++: class-scope qualified constrained auto [PR107188]

[Jason Merrill via Gcc-patches]

On 12/8/22 11:42, Patrick Palka wrote:

Here when parsing the class-scope auto constrained by a qualified
concept-id, we first tentatively parse the overall member-declaration as
a deprecated access-declaration, during which we parse C as a
standalone TEMPLATE_ID_EXPR (not part of the auto) and end up emitting
the bogus error

concepts-placeholder11.C:9:6: error: wrong number of template arguments (1, 
should be 2)
 9 |   N::C auto f() { return 0; }
   |  ^~
concepts-placeholder11.C:5:34: note: provided for ‘template 
concept N::C’
 5 |   template concept C = true;
   |  ^

from build_concept_id called from cp_parser_template_id_expr.

We could fix this by adding a complain parameter to build_concept_id and
passing tf_none when parsing tentatively.  However, it seems we can fix
this in a more general way that might benefit non-concepts code: when
tentatively parsing an access-declaration, abort the parse early if the
qualifying scope isn't possibly a class type, so that we avoid parsing
C as a TEMPLATE_ID_EXPR in the first place.  This patch takes this
latter approach.

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


OK.


PR c++/107188

gcc/cp/ChangeLog:

* parser.cc (cp_parser_using_declaration): Abort the tentative
parse early if the scope of an access-declaration isn't possibly
a class type.

gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-placeholder11.C: New test.
---
  gcc/cp/parser.cc|  5 +
  gcc/testsuite/g++.dg/cpp2a/concepts-placeholder11.C | 10 ++
  2 files changed, 15 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder11.C

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index e8a50904243..ccacf6d7dd0 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -21670,6 +21670,11 @@ cp_parser_using_declaration (cp_parser* parser,
  
cp_warn_deprecated_use_scopes (qscope);
  
+  if (access_declaration_p && !MAYBE_CLASS_TYPE_P (qscope))

+/* If the qualifying scope of an access-declaration isn't possibly
+   a class type then it must be invalid.  */
+cp_parser_simulate_error (parser);
+
if (access_declaration_p && cp_parser_error_occurred (parser))
  /* Something has already gone wrong; there's no need to parse
 further.  Since an error has occurred, the return value of
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder11.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder11.C
new file mode 100644
index 000..61eef743bae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder11.C
@@ -0,0 +1,10 @@
+// PR c++/107188
+// { dg-do compile { target c++20 } }
+
+namespace N {
+  template concept C = true;
+}
+
+struct X {
+  N::C auto f() { return 0; }
+};

[PATCH] doc: Fix documentation for __builtin_dynamic_object_size

[Siddhesh Poyarekar]

__builtin_dynamic_object_size is missing from the full list of builtins,
so add it.  Also mention it alongside __builtin_object_size in the
passes description.

gcc/ChangeLog:

* doc/extend.texi (__builtin_dynamic_object_size): Document
builtin.
* doc/passes.texi
(Optimize calls to @code{__builtin_object_size}): Also mention
__builtin_dynamic_object_size.

Signed-off-by: Siddhesh Poyarekar 
---
 gcc/doc/extend.texi | 10 --
 gcc/doc/passes.texi | 11 ++-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d3812fa55b0..608ff54f845 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -14291,8 +14291,14 @@ and GCC does not issue a warning.
 @end deftypefn
 
 @deftypefn {Built-in Function}{size_t} __builtin_object_size (const void * 
@var{ptr}, int @var{type})
-Returns the size of an object pointed to by @var{ptr}.  @xref{Object Size
-Checking}, for a detailed description of the function.
+Returns a constant size estimate of an object pointed to by @var{ptr}.
+@xref{Object Size Checking}, for a detailed description of the function.
+@end deftypefn
+
+@deftypefn {Built-in Function}{size_t} __builtin_dynamic_object_size (const 
void * @var{ptr}, int @var{type})
+Similar to @code{__builtin_object_size} except that the return value
+need not be a constant.  @xref{Object Size Checking}, for a detailed
+description of the function.
 @end deftypefn
 
 @deftypefn {Built-in Function} double __builtin_huge_val (void)
diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
index 9e8b4f50ad6..d649db72bbe 100644
--- a/gcc/doc/passes.texi
+++ b/gcc/doc/passes.texi
@@ -843,12 +843,13 @@ foo()}, this pass tries to change the call so that the 
address of
 pass is located in @code{tree-nrv.cc} and is described by
 @code{pass_return_slot}.
 
-@item Optimize calls to @code{__builtin_object_size}
+@item Optimize calls to @code{__builtin_object_size} or
+@code{__builtin_dynamic_object_size}
 
-This is a propagation pass similar to CCP that tries to remove calls
-to @code{__builtin_object_size} when the size of the object can be
-computed at compile-time.  This pass is located in
-@file{tree-object-size.cc} and is described by
+This is a propagation pass similar to CCP that tries to remove calls to
+@code{__builtin_object_size} or @code{__builtin_dynamic_object_size}
+when the size of the object can be computed at compile-time.  This pass
+is located in @file{tree-object-size.cc} and is described by
 @code{pass_object_sizes}.
 
 @item Loop invariant motion
-- 
2.38.1

Re: [PATCH] c++: extract_local_specs and unevaluated contexts [PR100295]

[Jason Merrill via Gcc-patches]

On 12/9/22 16:57, Patrick Palka wrote:

Here during partial instantiation of the constexpr if, extra_local_specs
walks the statement looking for local specializations within to save and
possibly capture.  However, we're thwarted by the fact that 'ts' first
appears inside an unevaluated context, and so the calls to
process_outer_var_ref for its local specializations are a no-op.  And
since we walk each tree exactly once, we end up not capturing them
despite it later occuring in an evaluated context.

This patch fixes this by making extract_local_specs walk evaluated
contexts first before walking unevaluated contexts.  We could probably
get away with not walking unevaluated contexts at all, but this approach
seems safer.

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


OK.


PR c++/100295
PR c++/107579

gcc/cp/ChangeLog:

* pt.cc (el_data::skip_unevaluated_operands): New data member.
(extract_locals_r): If skip_unevaluated_operands is true,
don't walk into unevaluated contexts.
(extract_local_specs): Walk the pattern twice, first with
skip_unevaluated_operands true followed by it set to false.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/constexpr-if-lambda5.C: New test.
---
  gcc/cp/pt.cc  | 19 ++-
  .../g++.dg/cpp1z/constexpr-if-lambda5.C   | 15 +++
  2 files changed, 33 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index d05a49b1c11..2b22bf14c53 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -13015,17 +13015,26 @@ public:
/* List of local_specializations used within the pattern.  */
tree extra;
tsubst_flags_t complain;
+  /* True iff we don't want to walk into unevaluated contexts.  */
+  bool skip_unevaluated_operands = false;
  
el_data (tsubst_flags_t c)

  : extra (NULL_TREE), complain (c) {}
  };
  static tree
-extract_locals_r (tree *tp, int */*walk_subtrees*/, void *data_)
+extract_locals_r (tree *tp, int *walk_subtrees, void *data_)
  {
el_data  = *reinterpret_cast(data_);
tree *extra = 
tsubst_flags_t complain = data.complain;
  
+  if (data.skip_unevaluated_operands

+  && unevaluated_p (TREE_CODE (*tp)))
+{
+  *walk_subtrees = 0;
+  return NULL_TREE;
+}
+
if (TYPE_P (*tp) && typedef_variant_p (*tp))
  /* Remember local typedefs (85214).  */
  tp = _NAME (*tp);
@@ -13117,6 +13126,14 @@ static tree
  extract_local_specs (tree pattern, tsubst_flags_t complain)
  {
el_data data (complain);
+  /* Walk the pattern twice, ignoring unevaluated operands the first time
+ around, so that if a local specialization appears in both an
+ evaluated and unevaluated context we prefer to process it in the
+ former context (since e.g. process_outer_var_ref is a no-op inside
+ an unevaluated context).  */
+  data.skip_unevaluated_operands = true;
+  cp_walk_tree (, extract_locals_r, , );
+  data.skip_unevaluated_operands = false;
cp_walk_tree (, extract_locals_r, , );
return data.extra;
  }
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C 
b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
new file mode 100644
index 000..d2bf0221743
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if-lambda5.C
@@ -0,0 +1,15 @@
+// PR c++/100295
+// { dg-do compile { target c++17 } }
+
+template
+void f(Ts... ts) {
+  auto lambda = [=](auto x) {
+if constexpr (sizeof((ts+x) + ...) != 0)
+  (..., ts);
+  };
+  lambda(0);
+}
+
+int main() {
+  f(0, 'a');
+}

Re: [PATCH] c++: Ensure !!var is not an lvalue [PR107065]

[Jason Merrill via Gcc-patches]

On 12/10/22 04:33, Jakub Jelinek wrote:

Hi!

The TRUTH_NOT_EXPR case in cp_build_unary_op is one of the spots where
we somewhat fold immediately using invert_truthvalue_loc.
I've tried using
   return build1_loc (location, TRUTH_NOT_EXPR, boolean_type_node, arg);
in there instead, but unfortunately that regressed
Wlogical-not-parentheses-*.c pr49706.c pr62199.c pr65120.c sequence-pt-1.C
tests, so at least for backporting that doesn't seem to be a way to go.

So, this patch instead wraps it into NON_LVALUE_EXPR if needed (which also
need a tweak for some tests in the pr47906.c test, but nothing major),
with the intent to make it backportable, and later I'll try to do further
steps to avoid folding here prematurely.  Most of the problems with
build1 TRUTH_NOT_EXPR are that it doesn't even invert comparisons as most
common case and lots of warning code isn't able to deal with ! around
comparisons; so perhaps one way to do this would be fold by hand only
invertable comparisons and for the rest create TRUTH_NOT_EXPR.

Anyway, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
(for now) and for release branches (after a while on the trunk)?


OK.


2022-12-10  Jakub Jelinek  

PR c++/107065
gcc/cp/
* typeck.cc (cp_build_unary_op) : If
invert_truthvalue_loc returns obvalue_p, wrap it into NON_LVALUE_EXPR.
* parser.cc (cp_parser_binary_expression): Don't call
warn_logical_not_parentheses if current.lhs is a NON_LVALUE_EXPR
of a decl with boolean type.
gcc/testsuite/
* g++.dg/cpp0x/pr107065.C: New test.

--- gcc/cp/typeck.cc.jj 2022-11-30 10:29:42.024701797 +0100
+++ gcc/cp/typeck.cc2022-12-09 17:47:54.132856233 +0100
@@ -7396,9 +7396,13 @@ cp_build_unary_op (enum tree_code code,
   build_zero_cst (TREE_TYPE (arg)), complain);
arg = perform_implicit_conversion (boolean_type_node, arg,
 complain);
-  val = invert_truthvalue_loc (location, arg);
if (arg != error_mark_node)
-   return val;
+   {
+ val = invert_truthvalue_loc (location, arg);
+ if (obvalue_p (val))
+   val = non_lvalue_loc (location, val);
+ return val;
+   }
errstring = _("in argument to unary !");
break;
  
--- gcc/cp/parser.cc.jj	2022-12-09 11:02:35.871444993 +0100

+++ gcc/cp/parser.cc2022-12-09 19:14:26.698847734 +0100
@@ -10224,7 +10224,10 @@ cp_parser_binary_expression (cp_parser*
  || (TREE_CODE (TREE_TYPE (TREE_OPERAND (current.lhs, 0)))
  != BOOLEAN_TYPE
  /* Avoid warning for !!b == y where b is boolean.  */
- && (!DECL_P (tree_strip_any_location_wrapper (current.lhs))
+ && (!(DECL_P (tree_strip_any_location_wrapper (current.lhs))
+   || (TREE_CODE (current.lhs) == NON_LVALUE_EXPR
+   && DECL_P (tree_strip_any_location_wrapper
+   (TREE_OPERAND (current.lhs, 0)
  || TREE_TYPE (current.lhs) == NULL_TREE
  || TREE_CODE (TREE_TYPE (current.lhs)) != BOOLEAN_TYPE))
warn_logical_not_parentheses (current.loc, current.tree_type,
--- gcc/testsuite/g++.dg/cpp0x/pr107065.C.jj2022-12-09 16:22:59.686548071 
+0100
+++ gcc/testsuite/g++.dg/cpp0x/pr107065.C   2022-12-09 16:22:59.686548071 
+0100
@@ -0,0 +1,14 @@
+// PR c++/107065
+// { dg-do compile { target c++11 } }
+
+template struct is_same { static constexpr bool value = false; };
+template struct is_same { static constexpr bool value = true; };
+
+int
+main ()
+{
+  bool b = true;
+  static_assert (is_same::value, "");
+  auto bb = (!(!b));
+  static_assert (is_same::value, "");
+}

Jakub

Re: [PATCH v5 3/4] OpenMP: Pointers and member mappings

[Julian Brown]

On Thu, 15 Dec 2022 14:54:58 +
Julian Brown  wrote:

> On Wed, 7 Dec 2022 17:31:20 +0100
> Tobias Burnus  wrote:
> 
> > Hi Julian,
> > 
> > I think this patch is OK; however, at least for gimplify.cc Jakub
> > needs to have a second look.  
> 
> Thanks for the review!  Here's a new version that hopefully addresses
> your comments.  (The gimplify bits change a bit more in this version!)

FYI, this is the current dependency list for this patch:

(1) "OpenMP/OpenACC: Reindent TO/FROM/_CACHE_ stanza in
{c_}finish_omp_clause"
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603791.html
Approved.

(2) "OpenMP/OpenACC: Rework clause expansion and nested struct handling"
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603792.html
Approved, but waiting for *this* patch to avoid regressing Fortran
pointer-mapping behaviour, and which Tobias noticed an issue with prior
to committing, addressed by (4).

(3) "OpenMP/OpenACC: Refine condition for when map clause expansion
happens"
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607543.html
Not reviewed (partly OpenACC).

(4) "OpenMP: implicitly map base pointer for array-section pointer
components"
https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608318.html
Not reviewed.

The following patches also depend on this one and the above:

(5) "OpenMP: lvalue parsing for map clauses (C++)"
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605367.html
Mostly approved.

(6) "OpenMP: C++ "declare mapper" support"
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607544.html
Revised version unreviewed.

...and the to-be-revised "lvalue parsing for C", and C/Fortran "declare
mapper" patches.

HTH,

Julian

Re: [PATCH] middle-end/70090: Document that -fsanitize=object-size uses dynamic size

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 11:33:40AM -0500, Siddhesh Poyarekar wrote:
> Fix the documentation to say that object sizes are deduced using
> __builtin_dynamic_object_size.
> 
> gcc/ChangeLog:
> 
>   PR middle-end/70090
>   * gcc/doc/invoke.texi (-fsanitize=object-size): Use
>   __builtin_dynamic_object_size instead of
>   __builtin_object_size.
> 
> Signed-off-by: Siddhesh Poyarekar 

Ok.

Jakub

[PATCH] middle-end/70090: Document that -fsanitize=object-size uses dynamic size

[Siddhesh Poyarekar]

Fix the documentation to say that object sizes are deduced using
__builtin_dynamic_object_size.

gcc/ChangeLog:

PR middle-end/70090
* gcc/doc/invoke.texi (-fsanitize=object-size): Use
__builtin_dynamic_object_size instead of
__builtin_object_size.

Signed-off-by: Siddhesh Poyarekar 
---
 gcc/doc/invoke.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f48df64cc2a..a50417a4ab7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16744,8 +16744,8 @@ or when a method or constructor is invoked on 
insufficiently aligned object.
 @item -fsanitize=object-size
 @opindex fsanitize=object-size
 This option enables instrumentation of memory references using the
-@code{__builtin_object_size} function.  Various out of bounds pointer
-accesses are detected.
+@code{__builtin_dynamic_object_size} function.  Various out of bounds
+pointer accesses are detected.
 
 @item -fsanitize=float-divide-by-zero
 @opindex fsanitize=float-divide-by-zero
-- 
2.38.1

Re: [PATCH 4/4] contrib: Add dg-out-generator.pl

[Jason Merrill via Gcc-patches]

On 12/10/22 04:43, Arsen Arsenović wrote:

This script is a helper used to generate dg-output lines from an existing
program output conveniently.  It takes care of escaping Tcl and ARE stuff.
contrib/ChangeLog:

* dg-out-generator.pl: New file.
---
  contrib/dg-out-generator.pl | 67 +
  1 file changed, 67 insertions(+)
  create mode 100755 contrib/dg-out-generator.pl

diff --git a/contrib/dg-out-generator.pl b/contrib/dg-out-generator.pl
new file mode 100755
index 000..38aed2aa38d
--- /dev/null
+++ b/contrib/dg-out-generator.pl
@@ -0,0 +1,67 @@
+#!/usr/bin/env perl
+#
+# Copyright (C) 2022 GCC Contributors.
+# Contributed by Arsen Arsenović.
+#
+# This script 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.
+
+# This script reads program output on STDIN, and out of it produces a block of
+# dg-output lines that can be yanked at the end of a file.  It will escape
+# special ARE and Tcl constructs automatically.
+#
+# Each argument passed on the standard input is treated as a string to be
+# replaced by ``.*'' in the final result.  This is intended to mask out build
+# paths, filenames, etc.
+#
+# Usage example:
+
+# $ g++-13 -fcontracts -o test \
+#  'g++.dg/contracts/contracts-access1.C' && \
+#   ./test |& dg-out-generator.pl 'g++.dg/contracts/contracts-access1.C'
+# // { dg-output "contract violation in function Base::b at .*:11: pub > 
0(\n|\r\n|\r)*" }
+# // { dg-output "\\\[level:default, role:default, continuation 
mode:never\\\](\n|\r\n|\r)*" }
+# // { dg-output "terminate called without an active exception(\n|\r\n|\r)*" }
+# You can now freely dump the above into your testcase.
+
+use strict;
+use warnings;
+use POSIX 'floor';
+
+my $escapees = '(' . join ('|', map { quotemeta } @ARGV) . ')';
+
+sub gboundary($)
+{
+  my $str = shift;
+  my $sz = 10.0;
+  for (;;)
+{
+  my $bnd = join '', (map chr 64 + rand 27, 1 .. floor $sz);
+  return $bnd unless index ($str, $bnd) >= 0;
+  $sz += 0.1;
+}
+}
+
+while ()
+  {
+# Escape our escapees.
+my $boundary = gboundary $_;
+s/$escapees/$boundary/;
+
+# Quote stuff special in Tcl ARE.
+s/([[\]*+?{}()\\])/\\$1/g;
+
+# Then, special stuff in TCL itself.
+s/([\][\\])/\\$1/g;
+
+# Newlines should be more tolerant.
+s/\n$/(\\n|\\r\\n|\\r)*/;
+
+# Then split out the boundary, replacing it with .*.
+s/$boundary/.*/;
+
+# Then, let's print it in a dg-output block.
+print "// { dg-output \"$_\" }\n";


I wonder if you want to wrap the pattern in {} instead of "" so you 
don't need the "special stuff in TCL itself" quoting?


Jason

Re: [PATCH 2/4] libstdc++: Improve output of default contract violation handler [PR107792]

[Jason Merrill via Gcc-patches]

On 12/10/22 04:43, Arsen Arsenović wrote:

From: Jonathan Wakely 

Make the output more readable. Don't output anything unless verbose
termination is enabled at configure-time.

libstdc++-v3/ChangeLog:

PR libstdc++/107792
PR libstdc++/107778
* src/experimental/contract.cc (handle_contract_violation): Make
output more readable.
---
  libstdc++-v3/src/experimental/contract.cc | 23 +--
  1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/libstdc++-v3/src/experimental/contract.cc 
b/libstdc++-v3/src/experimental/contract.cc
index c8d2697eddc..6a7f064d35e 100644
--- a/libstdc++-v3/src/experimental/contract.cc
+++ b/libstdc++-v3/src/experimental/contract.cc
@@ -1,4 +1,5 @@
  // -*- C++ -*- std::experimental::contract_violation and friends
+
  // Copyright (C) 2019-2022 Free Software Foundation, Inc.
  //
  // This file is part of GCC.
@@ -23,19 +24,21 @@
  // .
  
  #include 

-#include 
+#if _GLIBCXX_HOSTED && _GLIBCXX_VERBOSE
+# include 
+#endif
  
  __attribute__ ((weak)) void

  handle_contract_violation (const std::experimental::contract_violation 
)
  {
-  std::cerr << "default std::handle_contract_violation called: \n"
-<< " " << violation.file_name()
-<< " " << violation.line_number()
-<< " " << violation.function_name()
-<< " " << violation.comment()
-<< " " << violation.assertion_level()
-<< " " << violation.assertion_role()
-<< " " << (int)violation.continuation_mode()
+#if _GLIBCXX_HOSTED && _GLIBCXX_VERBOSE
+  const char* modes[]{ "never", "maybe" }; // Must match enumerators in header.


I'd actually suggest "off" and "on" since it's really a boolean since 
"always_continue" was dropped.



+  std::cerr << "contract violation in function " << violation.function_name()
+<< " at " << violation.file_name() << ':' << violation.line_number()
+<< ": " << violation.comment()
+<< "\n[level:" << violation.assertion_level()


Maybe omit level/role if "default"?


+<< ", role:" << violation.assertion_role() << ", continuation mode:"
+<< modes[(int)violation.continuation_mode()] << ']'
  << std::endl;
+#endif
  }

Re: [PATCH 1/4] contracts: Lowercase {MAYBE,NEVER}_CONTINUE

[Jason Merrill via Gcc-patches]

On 12/10/22 04:43, Arsen Arsenović wrote:

The lowercase constants are more consistent with the standard, and it is
unlikely that the uppercase versions would've been accepted.


OK.


gcc/cp/ChangeLog:

* contracts.cc: Rename references to
contract_violation_continuation_mode constants to be lowercase.

libstdc++-v3/ChangeLog:

* include/experimental/contract: Lowercase the constants in
contract_violation_continuation_mode.
---
  gcc/cp/contracts.cc| 4 ++--
  libstdc++-v3/include/experimental/contract | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
index 45f52b20392..26316372389 100644
--- a/gcc/cp/contracts.cc
+++ b/gcc/cp/contracts.cc
@@ -41,9 +41,9 @@ along with GCC; see the file COPYING3.  If not see
 "v > 0", // comment,
 "default", // assertion_level,
 "default", // assertion_role,
-MAYBE_CONTINUE, // continuation_mode
+maybe_continue, // continuation_mode
 });
-   terminate (); // if NEVER_CONTINUE
+   terminate (); // if never_continue
   }
  
 We use an internal type with the same layout as contract_violation rather

diff --git a/libstdc++-v3/include/experimental/contract 
b/libstdc++-v3/include/experimental/contract
index cf655023da7..a2babed6301 100644
--- a/libstdc++-v3/include/experimental/contract
+++ b/libstdc++-v3/include/experimental/contract
@@ -45,7 +45,7 @@ namespace experimental
  {
// From P1332
enum class contract_violation_continuation_mode {
-NEVER_CONTINUE, MAYBE_CONTINUE
+never_continue, maybe_continue
};
  
class contract_violation {

Re: [PATCH] c++: template friend with variadic constraints [PR108066]

[Jason Merrill via Gcc-patches]

On 12/12/22 12:20, Patrick Palka wrote:

When instantiating a constrained hidden template friend, we need to
substitute into its constraints for sake of declaration matching.


Hmm, we shouldn't need to do declaration matching when there's only a 
single declaration.



For
this substitution we use a full argument vector whose outer levels
correspond to the class's arguments and innermost level corresponds to
the template's level-lowered generic arguments.

But for A::f here, for which the relevant argument vector is
{{int}, {Us...}}, this substitution triggers the assert in
use_pack_expansion_extra_args_p since one argument is a pack expansion
and the other isn't.

And for A::f, for which the relevant argument vector is
{{int, int}, {Us...}}, the use_pack_expansion_extra_args_p assert would
also trigger but we first get a bogus "mismatched argument pack lengths"
error from tsubst_pack_expansion.

These might ultimately be bugs in tsubst_pack_expansion, but it seems
we can work around them by tweaking the constraint substitution in
maybe_substitute_reqs_for to only use the friend's outer arguments in
the case of a template friend.


Yes, this is how we normally substitute a member template during class 
instantiation: with the class' template args, not its own.  The assert 
seems to be enforcing that.



This should be otherwise equivalent to
substituting using the full arguments, since the template's innermost
arguments are just its generic arguments with level=1.

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

PR c++/108066
PR c++/108067

gcc/cp/ChangeLog:

* constraint.cc (maybe_substitute_reqs_for): For a template
friend, substitute using only its outer arguments.  Remove
dead uses_template_parms test.


I don't see any removal?


gcc/testsuite/ChangeLog:

* g++.dg/cpp2a/concepts-friend12.C: New test.
---
  gcc/cp/constraint.cc  |  8 +++
  .../g++.dg/cpp2a/concepts-friend12.C  | 22 +++
  2 files changed, 30 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index d4cd92ec3b4..f9d1009c9fe 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1352,6 +1352,14 @@ maybe_substitute_reqs_for (tree reqs, const_tree decl)
tree tmpl = DECL_TI_TEMPLATE (decl);
tree gargs = generic_targs_for (tmpl);
processing_template_decl_sentinel s;
+  if (PRIMARY_TEMPLATE_P (tmpl))
+   {
+ if (TEMPLATE_ARGS_DEPTH (gargs) == 1)
+   return reqs;
+ ++processing_template_decl;
+ gargs = copy_node (gargs);
+ --TREE_VEC_LENGTH (gargs);


Maybe instead of messing with TEMPLATE_ARGS_DEPTH we want to grab the 
targs for DECL_FRIEND_CONTEXT instead of decl itself?



+   }
if (uses_template_parms (gargs))
++processing_template_decl;
reqs = tsubst_constraint (reqs, gargs,
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C 
b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
new file mode 100644
index 000..95973842afb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend12.C
@@ -0,0 +1,22 @@
+// PR c++/108066
+// PR c++/108067
+// { dg-do compile { target c++20 } }
+
+template
+concept C = __is_same(T, U);
+
+template
+struct A {
+  template
+requires (... && C)
+  friend void f(A, A) { }
+};
+
+int main() {
+  A x;
+  f(x, x);
+  A y;
+  f(y, y);
+  A z;
+  f(x, z); // { dg-error "no match" }
+}

[PATCH] c++: variadic using-decl with parm pack in terminal name [PR102104]

[Patrick Palka via Gcc-patches]

There's a curious corner case with variadic member using-decls: the
terminal name can also contain a parameter pack, and only through naming
a conversion function, e.g.

  using A::operator Ts...;

We currently only handle parameter packs appearing in the qualifying
scope of a variadic using-decl; this patch adds support for the above
case as well, representing such a using-decl with two pack expansions,
one for the qualifying scope and one for the terminal name (despite
logically there being just one).  Then at instantiation time we manually
merge them.

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

PR c++/102104
PR c++/108090

gcc/cp/ChangeLog:

* error.cc (dump_decl) : Look through a
pack expansion in the name as well.
* parser.c (cp_parser_using_declaration): Handle a parameter
pack appearing in the terminal name of a variadic using-decl.
* pt.c (tsubst_decl) : Likewise.  Combine the
handling of variadic and non-variadic using-decls.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/using-variadic1.C: New test.
* g++.dg/cpp1z/using-variadic1a.C: New test.
* g++.dg/cpp1z/using-variadic1b.C: New test.
* g++.dg/cpp1z/using-variadic1c.C: New test.
* g++.dg/cpp1z/using-variadic2.C: New test.
* g++.dg/cpp1z/using-variadic3.C: New test.
---
 gcc/cp/error.cc   |  9 ++
 gcc/cp/parser.cc  | 33 ++-
 gcc/cp/pt.cc  | 91 ++-
 gcc/testsuite/g++.dg/cpp1z/using-variadic1.C  | 29 ++
 gcc/testsuite/g++.dg/cpp1z/using-variadic1a.C | 34 +++
 gcc/testsuite/g++.dg/cpp1z/using-variadic1b.C | 37 
 gcc/testsuite/g++.dg/cpp1z/using-variadic1c.C | 33 +++
 gcc/testsuite/g++.dg/cpp1z/using-variadic2.C  | 24 +
 gcc/testsuite/g++.dg/cpp1z/using-variadic3.C  |  8 ++
 9 files changed, 272 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1b.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic1c.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/using-variadic3.C

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 12b28e8ee5b..e7f60335cc0 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -1477,11 +1477,20 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
if (!(flags & TFF_UNQUALIFIED_NAME))
  {
tree scope = USING_DECL_SCOPE (t);
+   tree name = DECL_NAME (t);
if (PACK_EXPANSION_P (scope))
  {
scope = PACK_EXPANSION_PATTERN (scope);
variadic = true;
  }
+   if (identifier_p (name)
+   && IDENTIFIER_CONV_OP_P (name)
+   && PACK_EXPANSION_P (TREE_TYPE (name)))
+ {
+   name = make_conv_op_name (PACK_EXPANSION_PATTERN
+ (TREE_TYPE (name)));
+   variadic = true;
+ }
dump_type (pp, scope, flags);
pp_cxx_colon_colon (pp);
  }
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 03550308365..2e2d81c1316 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -21705,7 +21705,38 @@ cp_parser_using_declaration (cp_parser* parser,
pedwarn (ell->location, OPT_Wc__17_extensions,
 "pack expansion in using-declaration only available "
 "with %<-std=c++17%> or %<-std=gnu++17%>");
-  qscope = make_pack_expansion (qscope);
+
+  /* A parameter pack can appear in the qualifying scope, and/or in the
+terminal name (if naming a conversion function).  Logically they're
+part of a single pack expansion of the overall USING_DECL, but we
+express them as separate pack expansions within the USING_DECL since
+we can't create a pack expansion over a USING_DECL.  */
+  bool saw_parm_pack = false;
+  if (uses_parameter_packs (qscope))
+   {
+ qscope = make_pack_expansion (qscope);
+ saw_parm_pack = true;
+   }
+  /* It can also appear in the terminal name (if naming a conversion
+function).  */
+  if (identifier_p (identifier)
+ && IDENTIFIER_CONV_OP_P (identifier)
+ && uses_parameter_packs (TREE_TYPE (identifier)))
+   {
+ identifier = make_conv_op_name (make_pack_expansion
+ (TREE_TYPE (identifier)));
+ saw_parm_pack = true;
+   }
+  if (!saw_parm_pack)
+   {
+ /* Issue an error in terms using a SCOPE_REF that includes both
+components.  */
+ tree name
+   = build_qualified_name (NULL_TREE, qscope, identifier, false);
+ 

RE: [PATCH] initialize fde objects lazily

[Tamar Christina via Gcc-patches]

Thanks Thomas!

We've tested it and this brings the startup time back into a reasonable amount!

We'd quite like to see this get into GCC 13.

Regards,
Tamar

> -Original Message-
> From: Thomas Neumann 
> Sent: Friday, December 9, 2022 5:34 PM
> To: gcc-patches@gcc.gnu.org
> Cc: H.J. Lu ; Jakub Jelinek ;
> Tamar Christina ; Jason Merrill
> ; Jonathan Wakely ; Florian
> Weimer 
> Subject: [PATCH] initialize fde objects lazily
> 
> When registering an unwind frame with __register_frame_info_bases we
> currently initialize that fde object eagerly. This has the advantage that it 
> is
> immutable afterwards and we can safely access it from multiple threads, but
> it has the disadvantage that we pay the initialization cost even if the
> application never throws an exception.
> 
> This commit changes the logic to initialize the objects lazily.
> The objects themselves are inserted into the b-tree when registering the
> frame, but the sorted fde_vector is not constructed yet. Only on the first
> time that an exception tries to pass through the registered code the object is
> initialized. We notice that with a double checking, first doing a relaxed 
> load of
> the sorted bit and then re-checking under a mutex when the object was not
> initialized yet.
> 
> Note that the check must implicitly be safe concering a concurrent frame
> deregistration, as trying the deregister a frame that is on the unwinding path
> of a concurrent exception is inherently racy.
> 
> libgcc/ChangeLog:
>  * unwind-dw2-fde.c: Initialize fde object lazily when
>  the first exception tries to pass through.
> ---
>   libgcc/unwind-dw2-fde.c | 52 -
> 
>   1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c index
> 3c0cc654ec0..6f69c20ff4b 100644
> --- a/libgcc/unwind-dw2-fde.c
> +++ b/libgcc/unwind-dw2-fde.c
> @@ -63,8 +63,6 @@ release_registered_frames (void)
> 
>   static void
>   get_pc_range (const struct object *ob, uintptr_type *range); -static void -
> init_object (struct object *ob);
> 
>   #else
>   /* Without fast path frame deregistration must always succeed.  */ @@ -
> 76,6 +74,7 @@ static const int in_shutdown = 0;
>  by decreasing value of pc_begin.  */
>   static struct object *unseen_objects;
>   static struct object *seen_objects;
> +#endif
> 
>   #ifdef __GTHREAD_MUTEX_INIT
>   static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT; @@
> -103,7 +102,6 @@ init_object_mutex_once (void)
>   static __gthread_mutex_t object_mutex;
>   #endif
>   #endif
> -#endif
> 
>   /* Called from crtbegin.o to register the unwind info for an object.  */
> 
> @@ -126,10 +124,7 @@ __register_frame_info_bases (const void *begin,
> struct object *ob,
>   #endif
> 
>   #ifdef ATOMIC_FDE_FAST_PATH
> -  // Initialize eagerly to avoid locking later
> -  init_object (ob);
> -
> -  // And register the frame
> +  // Register the frame in the b-tree
> uintptr_type range[2];
> get_pc_range (ob, range);
> btree_insert (_frames, range[0], range[1] - range[0], ob); @@
> -180,10 +175,7 @@ __register_frame_info_table_bases (void *begin, struct
> object *ob,
> ob->s.b.encoding = DW_EH_PE_omit;
> 
>   #ifdef ATOMIC_FDE_FAST_PATH
> -  // Initialize eagerly to avoid locking later
> -  init_object (ob);
> -
> -  // And register the frame
> +  // Register the frame in the b-tree
> uintptr_type range[2];
> get_pc_range (ob, range);
> btree_insert (_frames, range[0], range[1] - range[0], ob); @@
> -892,7 +884,15 @@ init_object (struct object* ob)
> accu.linear->orig_data = ob->u.single;
> ob->u.sort = accu.linear;
> 
> +#ifdef ATOMIC_FDE_FAST_PATH
> +  // We must update the sorted bit with an atomic operation
> +  struct object tmp;
> +  tmp.s.b = ob->s.b;
> +  tmp.s.b.sorted = 1;
> +  __atomic_store (&(ob->s.b), &(tmp.s.b), __ATOMIC_SEQ_CST); #else
> ob->s.b.sorted = 1;
> +#endif
>   }
> 
>   #ifdef ATOMIC_FDE_FAST_PATH
> @@ -1130,6 +1130,21 @@ search_object (struct object* ob, void *pc)
>   }
>   }
> 
> +#ifdef ATOMIC_FDE_FAST_PATH
> +
> +// Check if the object was already initialized static inline bool
> +is_object_initialized (struct object *ob) {
> +  // We have to use relaxed atomics for the read, which
> +  // is a bit involved as we read from a bitfield
> +  struct object tmp;
> +  __atomic_load (&(ob->s.b), &(tmp.s.b), __ATOMIC_RELAXED);
> +  return tmp.s.b.sorted;
> +}
> +
> +#endif
> +
>   const fde *
>   _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases)
>   {
> @@ -1141,6 +1156,21 @@ _Unwind_Find_FDE (void *pc, struct
> dwarf_eh_bases *bases)
> if (!ob)
>   return NULL;
> 
> +  // Initialize the object lazily
> +  if (!is_object_initialized (ob))
> +{
> +  // Check again under mutex
> +  init_object_mutex_once ();
> +  __gthread_mutex_lock (_mutex);
> +
> +  if (!ob->s.b.sorted)
> + {
> +   init_object (ob);
> + }
> +
> +  

Re: [PATCH] contracts: Stop relying on mangling for naming .pre/.post clones

[Jason Merrill via Gcc-patches]

On 12/10/22 08:13, Arsen Arsenović wrote:

If the mangler is relied on, functions with extern "C" on them emit multiple
definitions of the same name.


But doing it here interferes with lazy mangling.  How about appending 
the suffix into write_mangled_name instead of write_encoding?  The 
demangler already expects "clone" suffixes at the end of the mangled name.



gcc/cp/ChangeLog:

* contracts.cc (build_contract_condition_function): Add pre/post
suffixes to pre- and postcondition clones.
* mangle.cc (write_encoding): Don't mangle pre- and postconditions.

gcc/testsuite/ChangeLog:

* g++.dg/contracts/contracts-externC.C: New test.
---
Afternoon,

This change prevents contracts from emitting wrapper functions that have the
same name as the original function, by moving the logic that disambiguates them
from the mangler into the build_contract_condition_function helper.

Thanks, have nice day.

  gcc/cp/contracts.cc   |  3 +++
  gcc/cp/mangle.cc  |  7 ---
  .../g++.dg/contracts/contracts-externC.C  | 19 +++
  3 files changed, 22 insertions(+), 7 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/contracts/contracts-externC.C

diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc
index 26316372389..f09eb87e283 100644
--- a/gcc/cp/contracts.cc
+++ b/gcc/cp/contracts.cc
@@ -161,6 +161,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "tree-iterator.h"
  #include "print-tree.h"
  #include "stor-layout.h"
+#include "cgraph.h"
  
  const int max_custom_roles = 32;

  static contract_role contract_build_roles[max_custom_roles] = {
@@ -1451,6 +1452,8 @@ build_contract_condition_function (tree fndecl, bool pre)
  TREE_TYPE (fn) = build_method_type (class_type, TREE_TYPE (fn));
  
DECL_NAME (fn) = copy_node (DECL_NAME (fn));

+  auto suffix = pre ? "pre" : "post";
+  SET_DECL_ASSEMBLER_NAME (fn, clone_function_name (fn, suffix));
DECL_INITIAL (fn) = error_mark_node;
DECL_ABSTRACT_ORIGIN (fn) = fndecl;
  
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc

index e363ef35b9f..e97428e8f30 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -856,13 +856,6 @@ write_encoding (const tree decl)
mangle_return_type_p (decl),
d);
  
-  /* If this is the pre/post function for a guarded function, append

-.pre/post, like something from create_virtual_clone.  */
-  if (DECL_IS_PRE_FN_P (decl))
-   write_string (".pre");
-  else if (DECL_IS_POST_FN_P (decl))
-   write_string (".post");
-
/* If this is a coroutine helper, then append an appropriate string to
 identify which.  */
if (tree ramp = DECL_RAMP_FN (decl))
diff --git a/gcc/testsuite/g++.dg/contracts/contracts-externC.C 
b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
new file mode 100644
index 000..873056b742b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/contracts/contracts-externC.C
@@ -0,0 +1,19 @@
+// simple check to ensure we don't emit a function with the same name twice,
+// when wrapping functions in pre- and postconditions.
+// { dg-do link }
+// { dg-options "-std=c++2a -fcontracts -fcontract-continuation-mode=on" }
+
+volatile int x = 10;
+
+extern "C" void
+f ()
+  [[ pre: x < 10 ]]
+{
+}
+
+int
+main ()
+  [[ post: x > 10 ]]
+{
+  f();
+}

Re: Ping---[V3][PATCH 2/2] Add a new warning option -Wstrict-flex-arrays.

[Qing Zhao via Gcc-patches]

> On Dec 15, 2022, at 2:47 AM, Richard Biener  wrote:
> 
> On Wed, 14 Dec 2022, Qing Zhao wrote:
> 
>> Hi, Richard,
>> 
>> I guess that we now agreed on the following:
>> 
>> “ the information that we ran into a trailing array but didn't consider 
>> it a flex array because of -fstrict-flex-arrays is always a useful 
>> information”
>> 
>> The only thing we didn’t decide is:
>> 
>> A. Amend such new information to -Warray-bounds when -fstrict-flex-arrays=N 
>> (N>0) specified.
>> 
>> OR
>> 
>> B. Issue such new information with a new warning option -Wstrict-flex-arrays 
>> when -fstrict-flex-arrays=N (N>0) specified.
>> 
>> My current patch implemented B. 
> 
> Plus it implements it to specify a different flex-array variant for
> the extra diagnostic.
Could you clarify a little bit on this? (Don’t quite understand…)
> 
>> If you think A is better, I will change the patch as A. 
> 
> I would tend to A since, as I said, it's useful information that
> shouldn't be hidden and not adding an option removes odd combination
> possibilities such as -Wno-array-bounds -Wstrict-flex-arrays.

With current implementation, the above combination will ONLY report the misuse 
of trailing array as flex-array.  No out-of-bounds warnings issued.

> In particular I find, say, -fstrict-flex-arrays=2 -Wstrict-flex-arrays=1
> hardly useful.

The above combination will NOT happen, because there is NO level argument for 
-Wstrict-flex-arrays.

The only combination will be:-fstrict-flex-arrays=N -Wstrict-flex-arrays

When N > 0, -Wstrict-flex-arrays will report any misuse of trailing arrays as 
flexible array per the value of N.
> 
> But I'm interested in other opinions.

Adding a separate -Wstrict-flex-arrays will provide users a choice to ONLY look 
at the mis-use of trailing arrays as flex-arrays.  Without this new option, 
such information will be buried into tons of out-of-bounds messges. 

I think this is the major benefit to have one separate new warning 
-Wstrict-flex-arrays. 

Do we need to provide the users this choice?

Thanks.

Qing
> 
> Thanks,
> Richard.
> 
>> Let me know your opinion.
>> 
>> thanks.
>> 
>> Qing
>> 
>> 
>>> On Dec 14, 2022, at 4:03 AM, Richard Biener  wrote:
>>> 
>>> On Tue, 13 Dec 2022, Qing Zhao wrote:
>>> 
 Richard, 
 
 Do you have any decision on this one? 
 Do we need this warning option For GCC? 
>>> 
>>> Looking at the testcases it seems that the diagnostic amends
>>> -Warray-bounds diagnostics for trailing but not flexible arrays?
>>> Wouldn't it be better to generally diagnose this, so have
>>> -Warray-bounds, with -fstrict-flex-arrays, for
>>> 
>>> struct X { int a[1]; };
>>> int foo (struct X *p)
>>> {
>>> return p->a[1];
>>> }
>>> 
>>> emit
>>> 
>>> warning: array subscript 1 is above array bounds ...
>>> note: the trailing array is only a flexible array member with 
>>> -fno-strict-flex-arrays
>>> 
>>> ?  Having -Wstrict-flex-arrays=N and N not agree with the
>>> -fstrict-flex-arrays level sounds hardly useful to me but the
>>> information that we ran into a trailing array but didn't consider
>>> it a flex array because of -fstrict-flex-arrays is always a
>>> useful information?
>>> 
>>> But maybe I misunderstood this new diagnostic?
>>> 
>>> Thanks,
>>> Richard.
>>> 
>>> 
 thanks.
 
 Qing
 
> On Dec 6, 2022, at 11:18 AM, Qing Zhao  wrote:
> 
> '-Wstrict-flex-arrays'
>   Warn about inproper usages of flexible array members according to
>   the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to
>   the trailing array field of a structure if it's available,
>   otherwise according to the LEVEL of the option
>   '-fstrict-flex-arrays=LEVEL'.
> 
>   This option is effective only when LEVEL is bigger than 0.
>   Otherwise, it will be ignored with a warning.
> 
>   when LEVEL=1, warnings will be issued for a trailing array
>   reference of a structure that have 2 or more elements if the
>   trailing array is referenced as a flexible array member.
> 
>   when LEVEL=2, in addition to LEVEL=1, additional warnings will be
>   issued for a trailing one-element array reference of a structure if
>   the array is referenced as a flexible array member.
> 
>   when LEVEL=3, in addition to LEVEL=2, additional warnings will be
>   issued for a trailing zero-length array reference of a structure if
>   the array is referenced as a flexible array member.
> 
> gcc/ChangeLog:
> 
>   * doc/invoke.texi: Document -Wstrict-flex-arrays option.
>   * gimple-array-bounds.cc (check_out_of_bounds_and_warn): Add two more
>   arguments.
>   (array_bounds_checker::check_array_ref): Issue warnings for
>   -Wstrict-flex-arrays.
>   * opts.cc (finish_options): Issue warning for unsupported combination
>   of -Wstrict_flex_arrays and -fstrict-flex-array.
>   * tree-vrp.cc (execute_ranger_vrp): Enable the pass when
>   warn_strict_flex_array 

Re: [PATCH 1/2] ivopts: Revert computation of address cost complexity.

[Dimitrije Milosevic]

Hi Richard,

Sorry for the delayed response, I couldn't find the time to fully focus on this 
topic.

> I'm not sure this is accurate but at least the cost of using an unsupported
> addressing mode should be at least that of the compensating code to
> mangle it to a supported form.

I'm pretty sure IVOPTS does not filter out candidates which aren't supported by
the target architecture. It does, however, adjust the cost for a subset of 
those.
The adjustment code modifies only the cost part of the address cost (which
consists of a cost and a complexity).
Having said this, I'd propose two approaches:
1. Cover all cases of unsupported addressing modes (if needed, I'm not 
entirely
sure they aren't already covered), leaving complexity for unsupported
addressing modes zero.
2. Revert the complexity calculation (which my initial patch does), leaving
everything else as it is.
3. A combination of both - if the control path gets into the adjustment 
code, we
use the reverted complexity calculation.
I'd love to get feedback regarding this, so I could focus on a concrete 
approach.

Kind regards,
Dimitrije

From: Richard Biener 
Sent: Monday, November 7, 2022 2:35 PM
To: Dimitrije Milosevic 
Cc: Jeff Law ; gcc-patches@gcc.gnu.org 
; Djordje Todorovic 
Subject: Re: [PATCH 1/2] ivopts: Revert computation of address cost complexity. 
 
On Wed, Nov 2, 2022 at 9:40 AM Dimitrije Milosevic
 wrote:
>
> Hi Jeff,
>
> > This is exactly what I was trying to get to.   If the addressing mode
> > isn't supported, then we shouldn't be picking it as a candidate.  If it
> > is, then we've probably got a problem somewhere else in this code and
> > this patch is likely papering over it.

I'm not sure this is accurate but at least the cost of using an unsupported
addressing mode should be at least that of the compensating code to
mangle it to a supported form.

> I'll take a deeper look into the candidate selection algorithm then. Will
> get back to you.

Thanks - as said the unfortunate situation is that both the original author and
the one who did the last bigger reworks of the code are gone.

Richard.

> Regards,
> Dimitrije
>
> 
> From: Jeff Law 
> Sent: Tuesday, November 1, 2022 7:46 PM
> To: Richard Biener; Dimitrije Milosevic
> Cc: gcc-patches@gcc.gnu.org; Djordje Todorovic
> Subject: Re: [PATCH 1/2] ivopts: Revert computation of address cost 
> complexity.
>
>
> On 10/28/22 01:00, Richard Biener wrote:
> > On Fri, Oct 28, 2022 at 8:43 AM Dimitrije Milosevic
> >  wrote:
> >> Hi Jeff,
> >>
> >>> THe part I don't understand is, if you only have BASE+OFF, why does
> >>> preventing the calculation of more complex addressing modes matter?  ie,
> >>> what's the point of computing the cost of something like base + off +
> >>> scaled index when the target can't utilize it?
> >> Well, the complexities of all addressing modes other than BASE + OFFSET are
> >> equal to 0. For targets like Mips, which only has BASE + OFFSET, it would 
> >> still
> >> be more complex to use a candidate with BASE + INDEX << SCALE + OFFSET
> >> than a candidate with BASE + INDEX, for example, as it has to compensate
> >> the lack of other addressing modes somehow. If complexities for both of
> >> those are equal to 0, in cases where complexities decide which candidate is
> >> to be chosen, a more complex candidate may be picked.
> > But something is wrong then - it shouldn't ever pick a candidate with
> > an addressing
> > mode that isn't supported?  So you say that the cost of expressing
> > 'BASE + INDEX << SCALE + OFFSET' as 'BASE + OFFSET' is not computed
> > accurately?
>
> This is exactly what I was trying to get to.   If the addressing mode
> isn't supported, then we shouldn't be picking it as a candidate.  If it
> is, then we've probably got a problem somewhere else in this code and
> this patch is likely papering over it.
>
>
> Jeff
>

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 04:01:33PM +0100, Thomas Schwinge wrote:
> Or, options are applicable to just one front end, and can just be a no-op
> for others, for shared-language compilation.  For example, '-nostdinc++',
> or '-frust-incomplete-and-experimental-compiler-do-not-use' need not
> necessarily emit a diagnostic, but can just just be ignored by 'cc1',
> 'f951', 'lto1'.

One simple change could be to add a new warning option and use it for
complain_wrong_lang warnings:
  else if (ok_langs[0] != '\0')
/* Eventually this should become a hard error IMO.  */
warning (0, "command-line option %qs is valid for %s but not for %s",
 text, ok_langs, bad_lang);
  else
/* Happens for -Werror=warning_name.  */
warning (0, "%<-Werror=%> argument %qs is not valid for %s",
 text, bad_lang);
We could keep the existing behavior, but give users (and our testsuite)
a way to silence that warning if they are ok with it applying only to a
subset of languages.
Then one could use
-frust-incomplete-and-experimental-compiler-do-not-use -Wno-whatever
or add that -Wno-whatever in check_compile if the snippet is different
language from main language of the testsuite (or always) etc.

Jakub

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Thomas Schwinge]

Hi!

On 2022-12-15T12:50:44+0100, Jakub Jelinek via Gcc-patches 
 wrote:
> On Thu, Dec 15, 2022 at 12:39:38PM +0100, Iain Buclaw wrote:
>> For the gdc testsuite, those warnings arise because both language files
>> are compiled in the same invocation (dg-additional-sources "cpp11.cpp"),
>> so it ends up looking something like:
>>
>> gdc -fextern-std=c++11 testcpp11.d cpp11.cpp -o testcpp11.exe

..., and this is a compilation mode that GCC generally intends to
support, right?  For example, also in use with Fortran, to bring in C
code.

>From that it follows that it's either the duty of all drivers ('gcc',
'g++', 'gdc', 'gfortran', 'grust', etc.) to appropriately handle such
options (like,
"Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' 
option"
that I proposed), or indeed:

> Maybe it would be nice to be able to pass options only to a certain
> gcc backend binary and not to others, similarly to how
> -Wa,option passes options to assembler, -Wp,option to preprocessor and
> -Wl,option to linker.
> -Wc,language,option ?
> Then one could
>   gdc -Wc,d,-fextern-std=c++11 testcpp11.d cpp11.cpp -o testcpp11.exe
> or
>   gccrs -Wc,rust,-frust-incomplete-and-experimental-compiler-do-not-use 
> whatever.rs something.c -o whatever
> etc.
> If we do, one would need to use it with care, because then e.g. for
> gcc -Wc,c,-fsanitize=address
> the driver wouldn't know it should link in -lasan etc.

Hmm.  :-) On the one hand, I like it (or some similar syntax), on the
other hand, isn't this a bit over-engineered?  (..., and then also
"under-engineered": which front end(s) does "-Wp,option to preprocessor",
then apply to, for example; all of them, and we ought to add a mechanism
to have separate such options for different front ends...)

One step back:

Typically, per my understanding, GCC options are intended to convey
similar semantics, when they apply to more than one front end.  For
example, '-Wunused' means similar things in C, C++, Fortran, Rust
compilation.

Or, options are applicable to just one front end, and can just be a no-op
for others, for shared-language compilation.  For example, '-nostdinc++',
or '-frust-incomplete-and-experimental-compiler-do-not-use' need not
necessarily emit a diagnostic, but can just just be ignored by 'cc1',
'f951', 'lto1'.

For others, a diagnostic may be considered appropriate.  For example, as
we already have:

cc1: warning: command-line option '-std=c++11' is valid for C++/ObjC++ but 
not for C

But, you could also argue that the 'c++' in '-std=c++11' imples that it's
targeted at the 'b.cc' C++ compilation part of 'g++ -std=c++11 a.c b.cc',
and 'a.c' is compiled with default C '-std=[...]'; and you might in fact
have: 'g++ -std=c89 -std=c++11 a.c b.cc', huh...

If we go for extending the syntax, should we do something similar to
'-foffload-options', where might have syntax similar to:

-ffront-end-options=-ffoo\ -fbar -ffront-end-options=c,c++=-fsigned-char\ 
-fshort-enums -ffront-end-options=d,rust=[...]

That is, in contrast to your proposed '-Wc,[...]', this allows for
specifying the same options for multiple front ends in one go.  Is that
useful?


Anyway: how to get us un-stuck here -- maybe pragmatically?  :-)


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] c++, libstdc++: Add typeinfo for _Float{16,32,64,128,32x,64x} and __bf16 types [PR108075]

[Jason Merrill via Gcc-patches]

On 12/13/22 04:40, Jakub Jelinek wrote:

Hi!

The following patch adds typeinfos for the extended floating point
types and _Float{32,64}x.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?


OK.


2022-12-13  Jakub Jelinek  

PR libstdc++/108075
gcc/cp/
* rtti.cc (emit_support_tinfos): Add pointers to
{bfloat16,float{16,32,64,128,32x,64x,128x}}_type_node to fundamentals
array.
gcc/testsuite/
* g++.dg/cpp23/ext-floating13.C: New test.
libstdc++-v3/
* config/abi/pre/gnu.ver (CXXABI_1.3.14): Export
_ZTIDF[0-9]*[_bx], _ZTIPDF[0-9]*[_bx] and _ZTIPKDF[0-9]*[_bx].
* testsuite/util/testsuite_abi.cc (check_version): Handle
CXXABI_1.3.14.

--- gcc/cp/rtti.cc.jj   2022-10-17 12:29:33.519016406 +0200
+++ gcc/cp/rtti.cc  2022-12-12 15:23:48.244190755 +0100
@@ -1603,7 +1603,9 @@ emit_support_tinfos (void)
  _long_integer_type_node, _long_unsigned_type_node,
  _type_node, _type_node, _double_type_node,
  _type_node, _type_node, _type_node,
-_type_node,
+_type_node, _type_node, _type_node,
+_type_node, _type_node, _type_node,
+_type_node, _type_node, _type_node,
  0
};
int ix;
--- gcc/testsuite/g++.dg/cpp23/ext-floating13.C.jj  2022-12-12 
15:38:51.357009408 +0100
+++ gcc/testsuite/g++.dg/cpp23/ext-floating13.C 2022-12-12 15:39:04.568816597 
+0100
@@ -0,0 +1,35 @@
+// P1467R9 - Extended floating-point types and standard names.
+// { dg-do link { target c++23 } }
+// { dg-options "" }
+
+#include 
+
+#ifdef __STDCPP_FLOAT16_T__
+const std::type_info  = typeid(decltype(0.0f16));
+#endif
+#ifdef __STDCPP_BFLOAT16_T__
+const std::type_info  = typeid(decltype(0.0bf16));
+#endif
+#ifdef __STDCPP_FLOAT32_T__
+const std::type_info  = typeid(decltype(0.0f32));
+#endif
+#ifdef __STDCPP_FLOAT64_T__
+const std::type_info  = typeid(decltype(0.0f64));
+#endif
+#ifdef __STDCPP_FLOAT128_T__
+const std::type_info  = typeid(decltype(0.0f128));
+#endif
+#ifdef __FLT32X_MAX__
+const std::type_info  = typeid(decltype(0.0f32x));
+#endif
+#ifdef __FLT64X_MAX__
+const std::type_info  = typeid(decltype(0.0f64x));
+#endif
+#ifdef __FLT128X_MAX__
+const std::type_info  = typeid(decltype(0.0f128x));
+#endif
+
+int
+main ()
+{
+}
--- libstdc++-v3/config/abi/pre/gnu.ver.jj  2022-11-11 08:15:45.646183974 
+0100
+++ libstdc++-v3/config/abi/pre/gnu.ver 2022-12-12 15:34:08.178142084 +0100
@@ -2794,6 +2794,16 @@ CXXABI_1.3.13 {
  
  } CXXABI_1.3.12;
  
+CXXABI_1.3.14 {

+
+# typeinfo for _Float{16,32,64,128,32x,64x,128x} and
+# __bf16
+_ZTIDF[0-9]*[_bx];
+_ZTIPDF[0-9]*[_bx];
+_ZTIPKDF[0-9]*[_bx];
+
+} CXXABI_1.3.13;
+
  # Symbols in the support library (libsupc++) supporting transactional memory.
  CXXABI_TM_1 {
  
--- libstdc++-v3/testsuite/util/testsuite_abi.cc.jj	2022-09-12 11:30:14.224870022 +0200

+++ libstdc++-v3/testsuite/util/testsuite_abi.cc2022-12-12 
15:46:41.036156477 +0100
@@ -230,6 +230,7 @@ check_version(symbol& test, bool added)
known_versions.push_back("CXXABI_1.3.11");
known_versions.push_back("CXXABI_1.3.12");
known_versions.push_back("CXXABI_1.3.13");
+  known_versions.push_back("CXXABI_1.3.14");
known_versions.push_back("CXXABI_IEEE128_1.3.13");
known_versions.push_back("CXXABI_TM_1");
known_versions.push_back("CXXABI_FLOAT128");
@@ -251,7 +252,7 @@ check_version(symbol& test, bool added)
bool latestp = (test.version_name == "GLIBCXX_3.4.31"
  // XXX remove next line when baselines have been regenerated.
 || test.version_name == "GLIBCXX_IEEE128_3.4.30"
-|| test.version_name == "CXXABI_1.3.13"
+|| test.version_name == "CXXABI_1.3.14"
 || test.version_name == "CXXABI_FLOAT128"
 || test.version_name == "CXXABI_TM_1");
if (added && !latestp)

Jakub

Re: [PATCH v5 3/4] OpenMP: Pointers and member mappings

[Julian Brown]

On Wed, 7 Dec 2022 17:31:20 +0100
Tobias Burnus  wrote:

> Hi Julian,
> 
> I think this patch is OK; however, at least for gimplify.cc Jakub
> needs to have a second look.

Thanks for the review!  Here's a new version that hopefully addresses
your comments.  (The gimplify bits change a bit more in this version!)

> As remarked for the 2/4 patch, I believe mapping 'map(tofrom:
> var%f(2:3))' should work without explicitly mapping 'map(tofrom:
> var%f)' (→ [TR11 157:21-26] (approx. [5.2 154:22-27], [5.1
> 352:17-22], [5.0 320:22-27]). →
> https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608100.html
> (+ previously in the thread).
> 
> Testing the patch, that seems to work fine (i.e. contrary to C/C++,
> cf. 2/4), which matches the dump and, if I understood correctly, also
> your (Julian's) expectation. Thus, no need to modify the code part.

That change is undone.

> Regarding the testcases:
> * I would prefer if you don't modify the existing
> libgomp.fortran/struct-elem-map-1.f90 testcase; However, you could
> add your version as another variant ('subroutine nine()',
> 'four_var()' or what's the next free name, possibly with a comment
> telling that it is 'four()' but with an added explicit basepointer
> mapping.).

I've added new functions "nine" through "twelve" with the added base
pointer.

> * As the new version should map *less*, I wonder whether some
> -fdump-tree-{original,gimple,omplower} scan-dump-tree checks would be
> useful besides testing whether it works at run time. (Your decision
> regarding which tree, which testcases and whether at all.)

A couple added...

> * Likewise, maybe a 'target enter/exit data' check? However, you
> might very well run into my 'omp target data exit' issue, cf.
> https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604887.html
> (needs to be revised based on Jakub's comments; I think those were on
> IRC only – the problem is that not only 'alloc' is affected but also
> 'from' etc.)

Hmm -- this wasn't quite as straightforward as it probably should have
been!  This version of the patch fixes a couple of bugs with "enter
data" and "exit data" directives I found when adding a new test
(map-subarray-8.f90).

(I'm not sure if this patch fixes all the problems you saw previously
with "alloc", etc. for enter/exit data -- there might still be work to
do there.)

Re-tested with offloading to NVPTX.  Does this look OK now?

Thanks,

Julian
commit 377dba3ca7dc6fdf86bdb82936e80100ef2dbf4f
Author: Julian Brown 
Date:   Mon Oct 17 16:44:31 2022 +

OpenMP: Pointers and member mappings

This patch changes the mapping node arrangement used for array components
of derived types, e.g.:

  type T
  integer, pointer, dimension(:) :: arrptr
  end type T

  type(T) :: tvar
  [...]
  !$omp target map(tofrom: tvar%arrptr)

This will currently be mapped using three mapping nodes:

  GOMP_MAP_TO tvar%arrptr   (the descriptor)
  GOMP_MAP_TOFROM *tvar%arrptr%data (the actual array data)
  GOMP_MAP_ALWAYS_POINTER tvar%arrptr%data  (a pointer to the array data)

This follows OMP 5.0, 2.19.7.1 (or OpenMP 5.2, 5.8.3) "map Clause":

  "If a list item in a map clause is an associated pointer and the
   pointer is not the base pointer of another list item in a map clause
   on the same construct, then it is treated as if its pointer target
   is implicitly mapped in the same clause. For the purposes of the map
   clause, the mapped pointer target is treated as if its base pointer
   is the associated pointer."

However, we can also write this:

  map(to: tvar%arrptr) map(tofrom: tvar%arrptr(3:8))

and then instead we should follow (OpenMP 5.2, 5.8.3 "map Clause"):

  "For map clauses on map-entering constructs, if any list item has a base
   pointer for which a corresponding pointer exists in the data environment
   upon entry to the region and either a new list item or the corresponding
   pointer is created in the device data environment on entry to the region,
   then:
   1. [Fortran] The corresponding pointer variable is associated with
  a pointer target that has the same rank and bounds as the pointer
  target of the original pointer, such that the corresponding list item
  can be accessed through the pointer in a target region.
   2. The corresponding pointer variable becomes an attached pointer
  for the corresponding list item."

With this patch you can write the above mappings, and the mapping nodes
used to map pointers to array sections (with descriptors) now look
like this:

  1) map(to: tvar%arrptr)   -->
  GOMP_MAP_TO [implicit]  *tvar%arrptr%data  (the array data)
  GOMP_MAP_TO_PSETtvar%arrptr(the descriptor)
  GOMP_MAP_ATTACH_DETACH  tvar%arrptr%data

  2) map(tofrom: tvar%arrptr(3:8)   -->

Re: [PATCH] c++: local alias in typename in lambda [PR105518]

[Jason Merrill via Gcc-patches]

On 12/14/22 12:48, Patrick Palka wrote:

We substitute the qualifying scope of a TYPENAME_TYPE directly using
tsubst_aggr_type (so that we can pass entering_scope=true) instead of
going through tsubst, which means we don't properly reuse typedefs
during this substitution.  This ends up causing us to reject the below
testcase because we substitute the TYPENAME_TYPE impl::type as if it
were written without the typedef impl for A, and thus we expect the
non-capturing lambda to capture t.

This patch fixes this by making tsubst_aggr_type delegate typedefs
to tsubst so that get properly reused, and then adjusting the result
appropriately if entering_scope is true.  In passing, this refactors
tsubst_aggr_type into two functions, one that's intended to be called
directly and a more minimal one that's intended to be called only from
the RECORD/UNION/ENUMERAL_TYPE cases of tsubst (and contains only the
necessary bits for that call site).

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Patch generated with -w to suppress noisy whitespace changes.


OK.


PR c++/105518

gcc/cp/ChangeLog:

* pt.cc (tsubst_aggr_type): Handle typedefs by delegating to
tsubst and adjusting the result if entering_scope.  Split out
the main part of the function into ...
(tsubst_aggr_type_1) ... here.
(tsubst): Use tsubst_aggr_type_1 instead of tsubst_aggr_type.
Handle TYPE_PTRMEMFUNC_P RECORD_TYPEs here instead of in
tsubst_aggr_type_1.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/lambda/lambda-alias1.C: New test.
---
  gcc/cp/pt.cc  | 58 ++-
  .../g++.dg/cpp0x/lambda/lambda-alias1.C   | 23 
  2 files changed, 65 insertions(+), 16 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-alias1.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 81b7787fd3d..86862e56410 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -185,6 +185,7 @@ static tree tsubst_template_parms (tree, tree, 
tsubst_flags_t);
  static void tsubst_each_template_parm_constraints (tree, tree, 
tsubst_flags_t);
  tree most_specialized_partial_spec (tree, tsubst_flags_t);
  static tree tsubst_aggr_type (tree, tree, tsubst_flags_t, tree, int);
+static tree tsubst_aggr_type_1 (tree, tree, tsubst_flags_t, tree, int);
  static tree tsubst_arg_types (tree, tree, tree, tsubst_flags_t, tree);
  static tree tsubst_function_type (tree, tree, tsubst_flags_t, tree);
  static bool check_specialization_scope (void);
@@ -13845,23 +13846,49 @@ tsubst_aggr_type (tree t,
if (t == NULL_TREE)
  return NULL_TREE;
  
-  /* If T is an alias template specialization, we want to substitute that

- rather than strip it, especially if it's dependent_alias_template_spec_p.
- It should be OK not to handle entering_scope in this case, since
- DECL_CONTEXT will never be an alias template specialization.  We only get
- here with an alias when tsubst calls us for TYPENAME_TYPE.  */
-  if (alias_template_specialization_p (t, nt_transparent))
-return tsubst (t, args, complain, in_decl);
+  /* Handle typedefs via tsubst so that they get reused.  */
+  if (typedef_variant_p (t))
+{
+  t = tsubst (t, args, complain, in_decl);
+  if (t == error_mark_node)
+   return error_mark_node;
+
+  /* The effect of entering_scope is that when substitution yields a
+dependent specialization A, lookup_template_class prefers to
+return A's primary template type instead of the implicit instantiation.
+So when entering_scope, we mirror this behavior by inspecting
+TYPE_CANONICAL appropriately, taking advantage of the fact that
+lookup_template_class links the two types by setting TYPE_CANONICAL of
+the latter to the former.  */
+  if (entering_scope
+ && CLASS_TYPE_P (t)
+ && dependent_type_p (t)
+ && TYPE_CANONICAL (t) == TREE_TYPE (TYPE_TI_TEMPLATE (t)))
+   t = TYPE_CANONICAL (t);
+  return t;
+}
  
switch (TREE_CODE (t))

  {
case RECORD_TYPE:
-  if (TYPE_PTRMEMFUNC_P (t))
-   return tsubst (TYPE_PTRMEMFUNC_FN_TYPE (t), args, complain, in_decl);
-
-  /* Fall through.  */
case ENUMERAL_TYPE:
case UNION_TYPE:
+   return tsubst_aggr_type_1 (t, args, complain, in_decl, entering_scope);
+
+  default:
+   return tsubst (t, args, complain, in_decl);
+}
+}
+
+/* The part of tsubst_aggr_type that's shared with tsubst.  */
+
+static tree
+tsubst_aggr_type_1 (tree t,
+   tree args,
+   tsubst_flags_t complain,
+   tree in_decl,
+   int entering_scope)
+{
if (TYPE_TEMPLATE_INFO (t) && uses_template_parms (t))
  {
tree argvec;
@@ -13892,10 +13919,6 @@ tsubst_aggr_type (tree t,
else
  /* This is not a template type, so there's nothing to do.  */
  return t;
-
-default:
-

Re: [PATCH] c++: partial ordering with memfn pointer cst [PR108104]

[Jason Merrill via Gcc-patches]

On 12/14/22 19:01, Patrick Palka wrote:

Here we're triggering an overzealous assert in unify during partial
ordering since the member function pointer constants are represented as
ordinary CONSTRUCTORs (with TYPE_PTRMEMFUNC_P TREE_TYPE) but the assert
expects only COMPOUND_LITERAL_P constructors.

Bootstrapped and regtested on x86_64-pc-linux, does this look OK for
trunk and perhaps 12?


OK for both.


PR c++/108104

gcc/cp/ChangeLog:

* pt.cc (unify) : Relax assert to accept any
CONSTRUCTOR not just COMPOUND_LITERAL_P ones.

gcc/testsuite/ChangeLog:

* g++.dg/template/ptrmem33.C: New test.
---
  gcc/cp/pt.cc |  2 +-
  gcc/testsuite/g++.dg/template/ptrmem33.C | 30 
  2 files changed, 31 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/template/ptrmem33.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2f0f7a39698..44058d30799 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -24921,7 +24921,7 @@ unify (tree tparms, tree targs, tree parm, tree arg, 
int strict,
if (is_overloaded_fn (parm) || type_unknown_p (parm))
return unify_success (explain_p);
gcc_assert (EXPR_P (parm)
- || COMPOUND_LITERAL_P (parm)
+ || TREE_CODE (parm) == CONSTRUCTOR
  || TREE_CODE (parm) == TRAIT_EXPR);
  expr:
/* We must be looking at an expression.  This can happen with
diff --git a/gcc/testsuite/g++.dg/template/ptrmem33.C 
b/gcc/testsuite/g++.dg/template/ptrmem33.C
new file mode 100644
index 000..dca741ae5e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ptrmem33.C
@@ -0,0 +1,30 @@
+// PR c++/108104
+// { dg-do compile { target c++11 } }
+
+struct A {
+  void x();
+  void y();
+};
+
+enum State { On };
+
+template
+struct B {
+  static void f();
+};
+
+template
+struct B {
+  static void g();
+};
+
+template
+struct B {
+  static void h();
+};
+
+int main() {
+  B::f();
+  B::g();
+  B::h();
+}

[committed, pushed] PR-107607 m2: Remove bdepend on realpath, cut and echo

[Gaius Mulley via Gcc-patches]

It can be replaced by a subshell'd cd just fine.
(cd gcc/m2; autoconf-2.69)

gcc/m2/ChangeLog:

* configure.ac: Stop probing for realpath.
* tools-src/calcpath: Break dependency on realpath, cut
and echo.
* configure: Rebuilt
---
 gcc/m2/configure.ac   |  5 -
 gcc/m2/tools-src/calcpath | 34 ++
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/gcc/m2/configure.ac b/gcc/m2/configure.ac
index 756e01c4321..5583af7f64c 100644
--- a/gcc/m2/configure.ac
+++ b/gcc/m2/configure.ac
@@ -24,11 +24,6 @@ AC_CANONICAL_BUILD
 AC_CANONICAL_HOST
 AC_CANONICAL_TARGET
 
-AC_CHECK_PROGS(regex_realpath, realpath)
-if test x$regex_realpath = "x" ; then
-AC_MSG_ERROR([realpath is required to build GNU Modula-2 (hint install 
coreutils).])
-fi
-
 AC_CHECK_FUNCS([stpcpy])
 
 AC_CHECK_HEADERS(sys/types.h)
diff --git a/gcc/m2/tools-src/calcpath b/gcc/m2/tools-src/calcpath
index e0817704f64..05324513aa1 100755
--- a/gcc/m2/tools-src/calcpath
+++ b/gcc/m2/tools-src/calcpath
@@ -23,27 +23,29 @@
 
 
 Usage () {
-   echo "Usage: calcpath pathcomponent1 pathcomponent2 subdir"
-   echo -n "  if pathcomponent1 is relative then 
pathcomponent1/pathcomponet2/subdir is"
-   echo " returned"
-   echo "  otherwise pathcomponet2/subdir is returned"
-   echo "  the path is checked for legality in subdir."
+   cat<&2
+   exit 1
+}
 
 if [ $# -eq 3 ]; then
-   if [ "$(echo $2 | cut -b 1)" = "." ] ; then
-   # relative path
-   the_path=$1/$2/$3
+   case "$2" in
+  /*) the_path="$2/$3" ;;
+  *) the_path="$1/$2/$3" ;;
+   esac
+   cd "$3" || die "could not access $3"
+   if ( cd "$the_path" ); then
+  printf '%s\n' "${the_path}"
else
-   the_path=$2/$3
-   fi
-   cd $3
-   if realpath ${the_path} > /dev/null ; then
-   echo ${the_path}
-   else
-   echo "calcpath: error ${the_path} is not a valid path in subdirectory 
$3" 1>&2
-   exit 1
+  die "${the_path} is not a valid path in subdirectory $3"
fi
 else
Usage
-- 
2.39.0

Re: [PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 02:03:36PM +0100, Andreas Schwab wrote:
> On Dez 15 2022, Jakub Jelinek via Gcc-rust wrote:
> 
> > @@ -58,13 +60,15 @@ proc check_compile {basename type conten
> > set options ""
> >  }
> >  switch -glob -- $contents {
> > -   "*/* Assembly*" { set src ${basename}[pid].S }
> > +   "*/\* Assembly*" { set src ${basename}[pid].S }
> > "*! Fortran*" { set src ${basename}[pid].f90 }
> > "*// C++*" { set src ${basename}[pid].cc }
> > "*// D*" { set src ${basename}[pid].d }
> > "*// ObjC++*" { set src ${basename}[pid].mm }
> > "*/* ObjC*" { set src ${basename}[pid].m }
> 
> You probably want to quote the * here too.

You're right on both, I've committed this follow-up
after verifying that Assembly test still works (it works even with \\\*
but doesn't with *) and verifying that changing 
check_effective_target_property_1_needed
to have // Assembly instead of /* Assembly incorrectly works with
"*/* Assembly*", "*/\* Assembly*" but correctly doesn't work with
"*/\\* Assembly*" or "*/\\\* Assembly*".

Committed to trunk.  Sorry.

2022-12-15  Jakub Jelinek  

* lib/target-supports.exp (check_compile): Further quoting
fixes for /* Assembly, /* ObjC and (* Modula-2 *) checks.

--- gcc/testsuite/lib/target-supports.exp.jj2022-12-15 13:57:40.0 
+0100
+++ gcc/testsuite/lib/target-supports.exp   2022-12-15 14:14:02.987854385 
+0100
@@ -60,15 +60,15 @@ proc check_compile {basename type conten
set options ""
 }
 switch -glob -- $contents {
-   "*/\* Assembly*" { set src ${basename}[pid].S }
+   "*/\\* Assembly*" { set src ${basename}[pid].S }
"*! Fortran*" { set src ${basename}[pid].f90 }
"*// C++*" { set src ${basename}[pid].cc }
"*// D*" { set src ${basename}[pid].d }
"*// ObjC++*" { set src ${basename}[pid].mm }
-   "*/* ObjC*" { set src ${basename}[pid].m }
+   "*/\\* ObjC*" { set src ${basename}[pid].m }
"*// Go*" { set src ${basename}[pid].go }
"*// Rust*" { set src ${basename}[pid].rs }
-   "*(\* Modula-2*" { set src ${basename}[pid].mod }
+   "*(\\* Modula-2*" { set src ${basename}[pid].mod }
default {
switch -- $tool {
"objc" { set src ${basename}[pid].m }


Jakub

Re: [PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Andreas Schwab via Gcc-patches]

On Dez 15 2022, Jakub Jelinek via Gcc-rust wrote:

> @@ -58,13 +60,15 @@ proc check_compile {basename type conten
>   set options ""
>  }
>  switch -glob -- $contents {
> - "*/* Assembly*" { set src ${basename}[pid].S }
> + "*/\* Assembly*" { set src ${basename}[pid].S }
>   "*! Fortran*" { set src ${basename}[pid].f90 }
>   "*// C++*" { set src ${basename}[pid].cc }
>   "*// D*" { set src ${basename}[pid].d }
>   "*// ObjC++*" { set src ${basename}[pid].mm }
>   "*/* ObjC*" { set src ${basename}[pid].m }

You probably want to quote the * here too.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Andreas Schwab via Gcc-patches]

On Dez 15 2022, Jakub Jelinek via Gcc-rust wrote:

> @@ -58,13 +60,15 @@ proc check_compile {basename type conten
>   set options ""
>  }
>  switch -glob -- $contents {
> - "*/* Assembly*" { set src ${basename}[pid].S }
> + "*/\* Assembly*" { set src ${basename}[pid].S }

That's a no-op.  Either double the backslash or quote with {} instead of
"".

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] middle-end/108086 - avoid quadraticness in copy_edges_for_bb

[Richard Biener via Gcc-patches]

For the testcase in PR108086 it's visible that we split blocks
multiple times when inlining and that causes us to adjust the
block tail stmt BBs multiple times, once for each split.  The
fix is to walk backwards and split from the tail instead.

For a reduced testcase this improves compile-time at -O by 4%.

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

PR middle-end/108086
* tree-inline.cc (copy_edges_for_bb): Walk stmts backwards for
splitting the block to avoid quadratic behavior with setting
stmts BB on multliple splits.
---
 gcc/tree-inline.cc | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 7b507dd2a38..79897f41e86 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -2569,13 +2569,17 @@ copy_edges_for_bb (basic_block bb, profile_count num, 
profile_count den,
  && !old_edge->src->aux)
new_bb->count -= old_edge->count ().apply_scale (num, den);
 
-  for (si = gsi_start_bb (new_bb); !gsi_end_p (si);)
+  /* Walk stmts from end to start so that splitting will adjust the BB
+ pointer for each stmt at most once, even when we split the block
+ multiple times.  */
+  bool seen_nondebug = false;
+  for (si = gsi_last_bb (new_bb); !gsi_end_p (si);)
 {
   bool can_throw, nonlocal_goto;
   gimple *copy_stmt = gsi_stmt (si);
 
   /* Do this before the possible split_block.  */
-  gsi_next ();
+  gsi_prev ();
 
   /* If this tree could throw an exception, there are two
  cases where we need to add abnormal edge(s): the
@@ -2595,25 +2599,23 @@ copy_edges_for_bb (basic_block bb, profile_count num, 
profile_count den,
 
   if (can_throw || nonlocal_goto)
{
- if (!gsi_end_p (si))
-   {
- while (!gsi_end_p (si) && is_gimple_debug (gsi_stmt (si)))
-   gsi_next ();
- if (gsi_end_p (si))
-   need_debug_cleanup = true;
-   }
- if (!gsi_end_p (si))
-   /* Note that bb's predecessor edges aren't necessarily
-  right at this point; split_block doesn't care.  */
+ /* If there's only debug insns after copy_stmt don't split
+the block but instead mark the block for cleanup.  */
+ if (!seen_nondebug)
+   need_debug_cleanup = true;
+ else
{
+ /* Note that bb's predecessor edges aren't necessarily
+right at this point; split_block doesn't care.  */
  edge e = split_block (new_bb, copy_stmt);
-
- new_bb = e->dest;
- new_bb->aux = e->src->aux;
- si = gsi_start_bb (new_bb);
+ e->dest->aux = new_bb->aux;
+ seen_nondebug = false;
}
}
 
+  if (!is_gimple_debug (copy_stmt))
+   seen_nondebug = true;
+
   bool update_probs = false;
 
   if (gimple_code (copy_stmt) == GIMPLE_EH_DISPATCH)
-- 
2.35.3

Re: [PATCH v5 1/19] modula2 front end: Fixes, improvements detecting python3 and documentation generation (shorter).

[Gaius Mulley via Gcc-patches]

Jakub Jelinek  writes:

> On Wed, Dec 14, 2022 at 08:35:07AM +, Gaius Mulley via Gcc-patches wrote:
>> thanks - this is the last patch tick.  So I'll actually do the merge now :-)
>
> I've committed following patch to fix up formatting of ChangeLog entries.

Many thanks and apologies for messing up the house style.

Re: [PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Arthur Cohen]

Hi Jakub,

On 12/15/22 13:23, Jakub Jelinek wrote:

Hi!

This patch allows magic comments also for Rust and Modula-2
for effective target tests etc. and fixes up the Assembly entry
- it is a glob, so /* Assembly can match /whatever Assembly and
not just /* Assembly.

Tested on x86_64-linux with
make check-g++ RUNTESTFLAGS=i386.exp=pr35513*
and verifying it still uses *.S extension for the property_1_needed
effective target test.

Ok for trunk?

2022-12-15  Jakub Jelinek  

* lib/target-supports.exp (check_compile): Add support for
Rust and Modula-2.  Use \* rather than * for /* comment for
Assembly.

--- gcc/testsuite/lib/target-supports.exp.jj2022-11-30 10:29:42.217698938 
+0100
+++ gcc/testsuite/lib/target-supports.exp   2022-12-15 13:08:47.941221943 
+0100
@@ -36,7 +36,9 @@
  # "! Fortran" for Fortran code,
  # "/* ObjC", for ObjC
  # "// ObjC++" for ObjC++
-# and "// Go" for Go
+# "// Go" for Go
+# "// Rust" for Rust
+# and "(* Modula-2" for Modula-2
  # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
  # allow for ObjC/ObjC++ specific flags.
  
@@ -58,13 +60,15 @@ proc check_compile {basename type conten

set options ""
  }
  switch -glob -- $contents {
-   "*/* Assembly*" { set src ${basename}[pid].S }
+   "*/\* Assembly*" { set src ${basename}[pid].S }
"*! Fortran*" { set src ${basename}[pid].f90 }
"*// C++*" { set src ${basename}[pid].cc }
"*// D*" { set src ${basename}[pid].d }
"*// ObjC++*" { set src ${basename}[pid].mm }
"*/* ObjC*" { set src ${basename}[pid].m }
"*// Go*" { set src ${basename}[pid].go }
+   "*// Rust*" { set src ${basename}[pid].rs }
+   "*(\* Modula-2*" { set src ${basename}[pid].mod }
default {
switch -- $tool {
"objc" { set src ${basename}[pid].m }

Jakub



LGTM :)

Thank you,

Arthur


OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Richard Biener via Gcc-patches]

On Thu, 15 Dec 2022, Jakub Jelinek wrote:

> Hi!
> 
> This patch allows magic comments also for Rust and Modula-2
> for effective target tests etc. and fixes up the Assembly entry
> - it is a glob, so /* Assembly can match /whatever Assembly and
> not just /* Assembly.
> 
> Tested on x86_64-linux with
> make check-g++ RUNTESTFLAGS=i386.exp=pr35513*
> and verifying it still uses *.S extension for the property_1_needed
> effective target test.
> 
> Ok for trunk?

OK.

Thanks,
Richard.

> 2022-12-15  Jakub Jelinek  
> 
>   * lib/target-supports.exp (check_compile): Add support for
>   Rust and Modula-2.  Use \* rather than * for /* comment for
>   Assembly.
> 
> --- gcc/testsuite/lib/target-supports.exp.jj  2022-11-30 10:29:42.217698938 
> +0100
> +++ gcc/testsuite/lib/target-supports.exp 2022-12-15 13:08:47.941221943 
> +0100
> @@ -36,7 +36,9 @@
>  # "! Fortran" for Fortran code,
>  # "/* ObjC", for ObjC
>  # "// ObjC++" for ObjC++
> -# and "// Go" for Go
> +# "// Go" for Go
> +# "// Rust" for Rust
> +# and "(* Modula-2" for Modula-2
>  # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to 
>  # allow for ObjC/ObjC++ specific flags.
>  
> @@ -58,13 +60,15 @@ proc check_compile {basename type conten
>   set options ""
>  }
>  switch -glob -- $contents {
> - "*/* Assembly*" { set src ${basename}[pid].S }
> + "*/\* Assembly*" { set src ${basename}[pid].S }
>   "*! Fortran*" { set src ${basename}[pid].f90 }
>   "*// C++*" { set src ${basename}[pid].cc }
>   "*// D*" { set src ${basename}[pid].d }
>   "*// ObjC++*" { set src ${basename}[pid].mm }
>   "*/* ObjC*" { set src ${basename}[pid].m }
>   "*// Go*" { set src ${basename}[pid].go }
> + "*// Rust*" { set src ${basename}[pid].rs }
> + "*(\* Modula-2*" { set src ${basename}[pid].mod }
>   default {
>   switch -- $tool {
>   "objc" { set src ${basename}[pid].m }
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

[PATCH] testsuite: Add support for Rust and Modula-2 effective target tests

[Jakub Jelinek via Gcc-patches]

Hi!

This patch allows magic comments also for Rust and Modula-2
for effective target tests etc. and fixes up the Assembly entry
- it is a glob, so /* Assembly can match /whatever Assembly and
not just /* Assembly.

Tested on x86_64-linux with
make check-g++ RUNTESTFLAGS=i386.exp=pr35513*
and verifying it still uses *.S extension for the property_1_needed
effective target test.

Ok for trunk?

2022-12-15  Jakub Jelinek  

* lib/target-supports.exp (check_compile): Add support for
Rust and Modula-2.  Use \* rather than * for /* comment for
Assembly.

--- gcc/testsuite/lib/target-supports.exp.jj2022-11-30 10:29:42.217698938 
+0100
+++ gcc/testsuite/lib/target-supports.exp   2022-12-15 13:08:47.941221943 
+0100
@@ -36,7 +36,9 @@
 # "! Fortran" for Fortran code,
 # "/* ObjC", for ObjC
 # "// ObjC++" for ObjC++
-# and "// Go" for Go
+# "// Go" for Go
+# "// Rust" for Rust
+# and "(* Modula-2" for Modula-2
 # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to 
 # allow for ObjC/ObjC++ specific flags.
 
@@ -58,13 +60,15 @@ proc check_compile {basename type conten
set options ""
 }
 switch -glob -- $contents {
-   "*/* Assembly*" { set src ${basename}[pid].S }
+   "*/\* Assembly*" { set src ${basename}[pid].S }
"*! Fortran*" { set src ${basename}[pid].f90 }
"*// C++*" { set src ${basename}[pid].cc }
"*// D*" { set src ${basename}[pid].d }
"*// ObjC++*" { set src ${basename}[pid].mm }
"*/* ObjC*" { set src ${basename}[pid].m }
"*// Go*" { set src ${basename}[pid].go }
+   "*// Rust*" { set src ${basename}[pid].rs }
+   "*(\* Modula-2*" { set src ${basename}[pid].mod }
default {
switch -- $tool {
"objc" { set src ${basename}[pid].m }

Jakub

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 12:39:38PM +0100, Iain Buclaw wrote:
> For the gdc testsuite, those warnings arise because both language files
> are compiled in the same invocation (dg-additional-sources "cpp11.cpp"),
> so it ends up looking something like:
> 
> gdc -fextern-std=c++11 testcpp11.d cpp11.cpp -o testcpp11.exe
> 
> So ruling out some sort of filtering done by the gdc driver when
> delegating calls to the C/C++/D language compilers, is there a way to
> get dejagnu to compile dg-additional-sources one-at-a-time?

Through extra code in *.exp file certainly, invoke
  gdc -c cpp11.cpp -o cpp11.o
and
  gdc -fextern-std=c++11 testcpp11.d cpp11.o -o testcpp11.exe
afterwards, but for dg-additional-sources I don't think so.

Maybe it would be nice to be able to pass options only to a certain
gcc backend binary and not to others, similarly to how
-Wa,option passes options to assembler, -Wp,option to preprocessor and
-Wl,option to linker.
-Wc,language,option ?
Then one could
  gdc -Wc,d,-fextern-std=c++11 testcpp11.d cpp11.cpp -o testcpp11.exe
or
  gccrs -Wc,rust,-frust-incomplete-and-experimental-compiler-do-not-use 
whatever.rs something.c -o whatever
etc.
If we do, one would need to use it with care, because then e.g. for
gcc -Wc,c,-fsanitize=address
the driver wouldn't know it should link in -lasan etc.

Jakub

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Iain Buclaw via Gcc-patches]

Excerpts from Jakub Jelinek via Gcc-patches's message of Dezember 15, 2022 
12:16 pm:
> We seem to have a problem in other testsuites too:
> grep ' valid for .*but not for' */*.log | sort -u
> gcc/gcc.log:/home/jakub/src/gcc/gcc/testsuite/gcc.dg/pragma-diag-6.c:2:30: 
> warning: option '-Wnoexcept' is valid for C++/ObjC++ but not for C [-Wpragmas]
> gdc/gdc.log:cc1plus: warning: command-line option '-fextern-std=c++11' is 
> valid for D but not for C++
> gdc/gdc.log:cc1plus: warning: command-line option '-fpreview=in' is valid for 
> D but not for C++
> gfortran/gfortran.log:cc1: warning: command-line option '-fcheck=all' is 
> valid for Fortran but not for C
> g++/g++.log:cc1: warning: command-line option '-nostdinc++' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++11' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++14' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++17' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++20' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++23' is valid for 
> C++/ObjC++ but not for C
> g++/g++.log:cc1: warning: command-line option '-std=gnu++98' is valid for 
> C++/ObjC++ but not for C
> rust/rust.log:cc1plus: warning: command-line option 
> '-frust-incomplete-and-experimental-compiler-do-not-use' is valid for Rust 
> but not for C++
> rust/rust.log:cc1: warning: command-line option 
> '-frust-incomplete-and-experimental-compiler-do-not-use' is valid for Rust 
> but not for C
> (of course, some of them could be from tests that this valid for but not for
> messages work right, that is clearly the case of pragma-diag-6.c).
> 
> In gcc/testsuite/lib/target-supports.exp (check_compile) we already
> determine extension for the check_compile snippet based on magic comments
> with default to .c (Rust nor Modula 2 don't have any, should that be
> changed?), shouldn't we at that point based on the language filter out
> known options that will not work?
> 
> So, given the above, at least when in gdc testsuite and language is
> not D filter out -fextern-std=* and -fpreview=in, for gfortran testsuite
> and language not Fortran filter out -fcheck=all, when in g++ testsuite and
> language is not C++ filter out -nostdinc++, -std=gnu++* and when
> in rust testsuite and language is not Rust filter out
> -frust-incomplete-and-experimental-compiler-do-not-use ?
> 

For the gdc testsuite, those warnings arise because both language files
are compiled in the same invocation (dg-additional-sources "cpp11.cpp"),
so it ends up looking something like:

gdc -fextern-std=c++11 testcpp11.d cpp11.cpp -o testcpp11.exe

So ruling out some sort of filtering done by the gdc driver when
delegating calls to the C/C++/D language compilers, is there a way to
get dejagnu to compile dg-additional-sources one-at-a-time?

Iain.

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Jakub Jelinek via Gcc-patches]

On Thu, Dec 15, 2022 at 11:14:10AM +0100, Thomas Schwinge wrote:
> Hi!
> 
> On 2022-12-15T08:53:13+0100, Richard Biener  
> wrote:
> > On Wed, Dec 14, 2022 at 11:58 PM Thomas Schwinge
> >  wrote:
> >> On 2022-12-13T14:40:36+0100, Arthur Cohen  
> >> wrote:
> >> > We've also added one more commit, which only affects files inside the
> >> > Rust front-end folder. This commit adds an experimental flag, which
> >> > blocks the compilation of Rust code when not used.
> >>
> >> (That's commit r13-4675-gb07ef39ffbf4e77a586605019c64e2e070915ac3
> >> "gccrs: Add fatal_error when experimental flag is not present".)
> >>
> >> I noticed that GCC/Rust recently lost all LTO variants in torture
> >> testing -- due to this commit.  :-O
> >>
> >> OK to push the attached
> >> "Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' 
> >> option",
> >> or should this be done differently?
> >
> > Just add 'LTO' to the option in lang.opt, like
> >
> > frust-incomplete-and-experimental-compiler-do-not-use
> > Rust LTO Var(flag_rust_experimental)
> > Enable experimental compilation of Rust files at your own risk
> 
> That doesn't work; it's 'cc1' that is complaining here.

In that case it is a testsuite issue.
I really think making such Rust option a common option is very much
undesirable.

We seem to have a problem in other testsuites too:
grep ' valid for .*but not for' */*.log | sort -u
gcc/gcc.log:/home/jakub/src/gcc/gcc/testsuite/gcc.dg/pragma-diag-6.c:2:30: 
warning: option '-Wnoexcept' is valid for C++/ObjC++ but not for C [-Wpragmas]
gdc/gdc.log:cc1plus: warning: command-line option '-fextern-std=c++11' is valid 
for D but not for C++
gdc/gdc.log:cc1plus: warning: command-line option '-fpreview=in' is valid for D 
but not for C++
gfortran/gfortran.log:cc1: warning: command-line option '-fcheck=all' is valid 
for Fortran but not for C
g++/g++.log:cc1: warning: command-line option '-nostdinc++' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++11' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++14' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++17' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++20' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++23' is valid for 
C++/ObjC++ but not for C
g++/g++.log:cc1: warning: command-line option '-std=gnu++98' is valid for 
C++/ObjC++ but not for C
rust/rust.log:cc1plus: warning: command-line option 
'-frust-incomplete-and-experimental-compiler-do-not-use' is valid for Rust but 
not for C++
rust/rust.log:cc1: warning: command-line option 
'-frust-incomplete-and-experimental-compiler-do-not-use' is valid for Rust but 
not for C
(of course, some of them could be from tests that this valid for but not for
messages work right, that is clearly the case of pragma-diag-6.c).

In gcc/testsuite/lib/target-supports.exp (check_compile) we already
determine extension for the check_compile snippet based on magic comments
with default to .c (Rust nor Modula 2 don't have any, should that be
changed?), shouldn't we at that point based on the language filter out
known options that will not work?

So, given the above, at least when in gdc testsuite and language is
not D filter out -fextern-std=* and -fpreview=in, for gfortran testsuite
and language not Fortran filter out -fcheck=all, when in g++ testsuite and
language is not C++ filter out -nostdinc++, -std=gnu++* and when
in rust testsuite and language is not Rust filter out
-frust-incomplete-and-experimental-compiler-do-not-use ?

Jakub

Re: [PATCH] gcov: annotate uncovered branches [PR107537]

[Martin Liška]

On 12/14/22 21:03, Michael Förderer via Gcc-patches wrote:
> Dear all,
> 
> this is a patch to print the gcov annotations (fallthrough or throw) als
> to uncovered branches.

Hey.

It's fine and I've just pushed the revision on your behalf:
https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=c263c3eba8953c341cd8ac2d0a5f2b8f38623016

Thanks for you contribution.
Martin

> 
> 
> Best regards,
> 
> Michael

[PATCH] middle-end/108086 - reduce operand scanner use from inliner

[Richard Biener via Gcc-patches]

The following avoids a redundant second operand scan on all stmts
during inlining which shows with PR108086.

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

PR middle-end/108086
* tree-inline.cc (copy_edges_for_bb): Do not update all
stmts again.
---
 gcc/tree-inline.cc | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 15a1a389493..addfe7fcbcc 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -2571,12 +2571,8 @@ copy_edges_for_bb (basic_block bb, profile_count num, 
profile_count den,
 
   for (si = gsi_start_bb (new_bb); !gsi_end_p (si);)
 {
-  gimple *copy_stmt;
   bool can_throw, nonlocal_goto;
-
-  copy_stmt = gsi_stmt (si);
-  if (!is_gimple_debug (copy_stmt))
-   update_stmt (copy_stmt);
+  gimple *copy_stmt = gsi_stmt (si);
 
   /* Do this before the possible split_block.  */
   gsi_next ();
-- 
2.35.3

Re: [PATCH 2/2] Corrected pr25521.c target matching.

[Cupertino Miranda via Gcc-patches]

gentle ping

Cupertino Miranda writes:

>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote:
>>> This commit is a follow up of bugzilla #107181.
>>> The commit /a0aafbc/ changed the default implementation of the
>>> SELECT_SECTION hook in order to match clang/llvm behaviour w.r.t the
>>> placement of `const volatile' objects.
>>> However, the following targets use target-specific selection functions
>>> and they choke on the testcase pr25521.c:
>>>   *rx - target sets its const variables as '.section C,"a",@progbits'.
>> That's presumably a constant section.  We should instead twiddle the test to
>> recognize that section.
>
> Although @progbits is indeed a constant section, I believe it is
> more interesting to detect if the `rx' starts selecting more
> standard sections instead of the current @progbits.
> That was the reason why I opted to XFAIL instead of PASSing it.
> Can I keep it as such ?
>
>>
>>>   *powerpc - its 32bit version is eager to allocate globals in .sdata
>>>  sections.
>>> Normally, one can expect for the variable to be allocated in .srodata,
>>> however, in case of powerpc-*-* or powerpc64-*-* (with -m32)
>>> 'targetm.have_srodata_section == false' and the code in
>>> categorize_decl_for_section(varasm.cc), forces it to allocate in .sdata.
>>>/* If the target uses small data sections, select it.  */
>>>else if (targetm.in_small_data_p (decl))
>>>  {
>>>if (ret == SECCAT_BSS)
>>> ret = SECCAT_SBSS;
>>>else if targetm.have_srodata_section && ret == SECCAT_RODATA)
>>> ret = SECCAT_SRODATA;
>>>else
>>> ret = SECCAT_SDATA;
>>>  }
>> I'd just skip the test for 32bit ppc.  There should be suitable 
>> effective-target
>> tests you can use.
>>
>> jeff

Re: Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' option (was: Rust front-end patches v4)

[Thomas Schwinge]

Hi!

On 2022-12-15T08:53:13+0100, Richard Biener  wrote:
> On Wed, Dec 14, 2022 at 11:58 PM Thomas Schwinge
>  wrote:
>> On 2022-12-13T14:40:36+0100, Arthur Cohen  wrote:
>> > We've also added one more commit, which only affects files inside the
>> > Rust front-end folder. This commit adds an experimental flag, which
>> > blocks the compilation of Rust code when not used.
>>
>> (That's commit r13-4675-gb07ef39ffbf4e77a586605019c64e2e070915ac3
>> "gccrs: Add fatal_error when experimental flag is not present".)
>>
>> I noticed that GCC/Rust recently lost all LTO variants in torture
>> testing -- due to this commit.  :-O
>>
>> OK to push the attached
>> "Make '-frust-incomplete-and-experimental-compiler-do-not-use' a 'Common' 
>> option",
>> or should this be done differently?
>
> Just add 'LTO' to the option in lang.opt, like
>
> frust-incomplete-and-experimental-compiler-do-not-use
> Rust LTO Var(flag_rust_experimental)
> Enable experimental compilation of Rust files at your own risk

That doesn't work; it's 'cc1' that is complaining here.


Grüße
 Thomas


>> With that, we get back:
>>
>>  PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O0  (test 
>> for excess errors)
>>  PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O1  (test 
>> for excess errors)
>>  PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2  (test 
>> for excess errors)
>> +PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2 -flto 
>> -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
>> +PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2 -flto 
>> -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
>>  PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O3 -g  
>> (test for excess errors)
>>  PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -Os  (test 
>> for excess errors)
>>
>> Etc., and in total:
>>
>> === rust Summary for unix ===
>>
>> # of expected passes[-4990-]{+6718+}
>> # of expected failures  [-39-]{+51+}
>>
>>
>> Grüße
>>  Thomas
>>
>>
>> > We hope this helps
>> > indicate to users that the compiler is not yet ready, but can still be
>> > experimented with :)
>> >
>> > We plan on removing that flag as soon as possible, but in the meantime,
>> > we think it will help not creating divide within the Rust ecosystem, as
>> > well as not waste Rust crate maintainers' time.


-
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 3b2a8a4df1637a0cad738165a2afa9b34e286fcf Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 14 Dec 2022 17:16:42 +0100
Subject: [PATCH] Make '-frust-incomplete-and-experimental-compiler-do-not-use'
 a 'Common' option

I noticed that GCC/Rust recently lost all LTO variants in torture testing:

 PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O0  (test for excess errors)
 PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O1  (test for excess errors)
 PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2  (test for excess errors)
-PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
-PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
 PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -O3 -g  (test for excess errors)
 PASS: rust/compile/torture/all_doc_comment_line_blocks.rs   -Os  (test for excess errors)

Etc.

The reason is that when probing for availability of LTO, we run into:

spawn [...]/build-gcc/gcc/testsuite/rust/../../gccrs -B[...]/build-gcc/gcc/testsuite/rust/../../ -fdiagnostics-plain-output -frust-incomplete-and-experimental-compiler-do-not-use -flto -c -o lto8274.o lto8274.c
cc1: warning: command-line option '-frust-incomplete-and-experimental-compiler-do-not-use' is valid for Rust but not for C

For GCC/Rust testing, this flag is defaulted in
'gcc/testsuite/lib/rust.exp:rust_init':

lappend ALWAYS_RUSTFLAGS "additional_flags=-frust-incomplete-and-experimental-compiler-do-not-use"

Make it generally accepted without "is valid for Rust but not for [...]"
diagnostic.

	gcc/rust/
	* lang.opt
	(-frust-incomplete-and-experimental-compiler-do-not-use): Remove.
	gcc/
	* common.opt
	(-frust-incomplete-and-experimental-compiler-do-not-use): New.
---
 gcc/common.opt| 6 ++
 gcc/rust/lang.opt | 4 
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index 562d73d7f552..eba28e650f94 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2552,6 +2552,12 @@ frounding-math
 Common 

Re: [PATCH 1/2] select .rodata for const volatile variables.

[Cupertino Miranda via Gcc-patches]

gentle ping

Cupertino Miranda writes:

> Hi Jeff,
>
> First of all thanks for your quick review.
> Apologies for the delay replying, the message got lost in my inbox.
>
>> On 12/2/22 10:52, Cupertino Miranda via Gcc-patches wrote:
>>> Changed target code to select .rodata section for 'const volatile'
>>> defined variables.
>>> This change is in the context of the bugzilla #170181.
>>> gcc/ChangeLog:
>>> v850.c(v850_select_section): Changed function.
>> I'm not sure this is safe/correct.  ISTM that you need to look at the 
>> underlying
>> TREE_TYPE to check for const-volatile rather than TREE_SIDE_EFFECTS.
>
> I believe this was asked by Jose when he first sent the generic patches.
> Please notice my change is influenced by his original patch that does
> the same and was approved.
>
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599348.html
> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/602374.html
>
>>
>> Of secondary importance is the ChangeLog.  Just saying "Changed function"
>> provides no real information.  Something like this would be better:
>>
>>  * config/v850/v850.c (v850_select_section): Put const volatile
>>  objects into read-only sections.
>>
>>
>> Jeff
>>
>>
>>
>>
>>> ---
>>>   gcc/config/v850/v850.cc | 1 -
>>>   1 file changed, 1 deletion(-)
>>> diff --git a/gcc/config/v850/v850.cc b/gcc/config/v850/v850.cc
>>> index c7d432990ab..e66893fede4 100644
>>> --- a/gcc/config/v850/v850.cc
>>> +++ b/gcc/config/v850/v850.cc
>>> @@ -2865,7 +2865,6 @@ v850_select_section (tree exp,
>>>   {
>>> int is_const;
>>> if (!TREE_READONLY (exp)
>>> - || TREE_SIDE_EFFECTS (exp)
>>>   || !DECL_INITIAL (exp)
>>>   || (DECL_INITIAL (exp) != error_mark_node
>>>   && !TREE_CONSTANT (DECL_INITIAL (exp

Re: [PATCH] gcov: Fix -fprofile-update=atomic

[Sebastian Huber]

On 13/12/2022 15:30, Richard Biener wrote:

On Fri, Dec 9, 2022 at 2:56 PM Sebastian Huber
  wrote:

The code coverage support uses counters to determine which edges in the control
flow graph were executed.  If a counter overflows, then the code coverage
information is invalid.  Therefore the counter type should be a 64-bit integer.
In multithreaded applications, it is important that the counter increments are
atomic.  This is not the case by default.  The user can enable atomic counter
increments through the -fprofile-update=atomic and
-fprofile-update=prefer-atomic options.

If the hardware supports 64-bit atomic operations, then everything is fine.  If
not and -fprofile-update=prefer-atomic was chosen by the user, then non-atomic
counter increments will be used.  However, if the hardware does not support the
required atomic operations and -fprofile-atomic=update was chosen by the user,
then a warning was issued and as a forced fall-back to non-atomic operations
was done.  This is probably not what a user wants.  There is still hardware on
the market which does not have atomic operations and is used for multithreaded
applications.  A user which selects -fprofile-update=atomic wants consistent
code coverage data and not random data.

This patch removes the fall-back to non-atomic operations for
-fprofile-update=atomic.  If atomic operations in hardware are not available,
then a library call to libatomic is emitted.  To mitigate potential performance
issues an optimization for systems which only support 32-bit atomic operations
is provided.  Here, the edge counter increments are done like this:

   low = __atomic_add_fetch_4 (, 1, MEMMODEL_RELAXED);
   high_inc = low == 0 ? 1 : 0;
   __atomic_add_fetch_4 (, high_inc, MEMMODEL_RELAXED);

You check for compare_and_swapsi and the old code checks
TYPE_PRECISION (gcov_type_node) > 32 to determine whether 8 byte or 4 byte
gcov_type is used.  But you do not seem to handle the case where
neither SImode nor DImode atomic operations are available?  Can we instead
do

   if (gcov_type_size == 4)
 can_support_atomic4
   = HAVE_sync_compare_and_swapsi || HAVE_atomic_compare_and_swapsi;
   else if (gcov_type_size == 8)
 can_support_atomic8
   = HAVE_sync_compare_and_swapdi || HAVE_atomic_compare_and_swapdi;

   if (flag_profile_update == PROFILE_UPDATE_ATOMIC
   && !can_support_atomic4 && !can_support_atomic8)
 {
   warning (0, "target does not support atomic profile update, "
"single mode is selected");
   flag_profile_update = PROFILE_UPDATE_SINGLE;
 }

thus retain the diagnostic & fallback for this case?  


No, if you select -fprofile-update=atomic, then the updates shall be 
atomic from my point of view. If a fallback is acceptable, then you can 
use -fprofile-update=prefer-atomic. Using the fallback in 
-fprofile-update=atomic is a bug which prevents the use of gcov for 
multi-threaded applications on the lower end targets which do not have 
atomic operations in hardware.



The existing
code also suggests
there might be targets with HImode or TImode counters?


Sorry, I don't know.



In another mail you mentioned that for gen_time_profiler this doesn't
work but its
code relies on flag_profile_update as well.  So do we need to split the flag
somehow, or continue using the PROFILE_UPDATE_SINGLE fallback when
we are doing more than just edge profiling?


If atomic operations are not available in hardware, then with this patch 
calls to libatomic are emitted. In gen_time_profiler() this is not an 
issue at all, since the atomic increment is only done if counters[0] == 
0 (if I read the code correctly).


For example for

void f(void) {}

we get on riscv:

gcc --coverage -O2 -S -o - test.c -fprofile-update=atomic

lui a4,%hi(__gcov0.f)
li  a3,1
addia4,a4,%lo(__gcov0.f)
amoadd.w a5,a3,0(a4)
lui a4,%hi(__gcov0.f+4)
addia5,a5,1
seqza5,a5
addia4,a4,%lo(__gcov0.f+4)
amoadd.w zero,a5,0(a4)
ret

gcc --coverage -O2 -S -o - test.c -fprofile-update=atomic -mbig-endian

lui a4,%hi(__gcov0.f+4)
li  a3,1
addia4,a4,%lo(__gcov0.f+4)
amoadd.w a5,a3,0(a4)
lui a4,%hi(__gcov0.f)
addia5,a5,1
seqza5,a5
addia4,a4,%lo(__gcov0.f)
amoadd.w zero,a5,0(a4)
ret

gcc --coverage -O2 -S -o - test.c -fprofile-update=atomic -march=rv64g 
-mabi=lp64


lui a5,%hi(__gcov0.f)
li  a4,1
addia5,a5,%lo(__gcov0.f)
amoadd.d zero,a4,0(a5)
ret

gcc --coverage -O2 -S -o - test.c -fprofile-update=atomic -march=rv32id

lui a0,%hi(__gcov0.f)
li  a3,0
li  a1,1
li  a2,0
addia0,a0,%lo(__gcov0.f)
tail__atomic_fetch_add_8

gcc --coverage -O2 -S -o - test.c -fprofile-update=prefer-atomic 
-march=rv32id


lui 

Re: PING^1 [PATCH v2] predict: Adjust optimize_function_for_size_p [PR105818]

[Kewen.Lin via Gcc-patches]

Hi Honza,

Thanks for the comments.

on 2022/12/14 21:22, Jan Hubicka wrote:
>>> PR middle-end/105818
>>>
>>> gcc/ChangeLog:
>>>
>>> * predict.cc (optimize_function_for_size_p): Further check
>>> optimize_size of fun->decl when it is valid but no cgraph node.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/powerpc/pr105818.c: New test.
>>> * gcc.dg/guality/pr54693-2.c: Adjust for aarch64.
>>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr105818.c 
>>> b/gcc/testsuite/gcc.target/powerpc/pr105818.c
>>> new file mode 100644
>>> index 000..679647e189d
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/powerpc/pr105818.c
>>> @@ -0,0 +1,11 @@
>>> +/* { dg-options "-Os -fno-tree-vectorize" } */
>>> +
>>> +/* Verify there is no ICE.  */
>>> +
>>> +#pragma GCC optimize "-fno-tree-vectorize"
>>> +
>>> +void
>>> +foo (void)
>>> +{
>>> +  void bar (void);
>>> +}
> So the testcase starts with optimize_size set but then it switches to
> optimize_size==0 due to the GCC optimize pragma.  I think this is
> behaviour Martin wants to change, so perhaps the testcase should be
> written with explicit -O2.

No, both the global context and the function context are to optimize
for size, Martin also clarified that.  So the issue is the inconsistent
information on optimizing for size when parsing bar, at that time
cfun->decl is available but no cgraph node, it says OPTIMIZE_SIZE_NO. 

> 
> I also wonder what happen when you add the attribute later?
> /* { dg-options "-Os -fno-tree-vectorize" } */
> 
> /* Verify there is no ICE.  */
> 
> #pragma GCC optimize "-fno-tree-vectorize"
>
// marker A

> void
> foo (void)
> {
>   void bar (void);
> }
> 
> __attribute__ ((optimize("-fno-tree-vectorize"))) void foo (void);

There is still one ICE with this additional decl.  Same ICE if I moved
it to "marker A" above.

> 
> I think we should generally avoid doing decisions about size/speed
> optimizations so early since the setting may change due to attribtes or
> profile feedback...
> 

Good point, I'll make a separated patch to move optimize_function_for_speed_p
uses out of function rs6000_option_override_internal, but I think it's a
separated issue which just results in imperfect "optimize for size" decision.
Fixing it can cover this exposed ICE, but IMHO this exposed inconsistent
information on "optimize for size" exposed here is still an issue, like: all
the context is to optimize for size, but it still returns OPTIMIZE_SIZE_NO.

Do you agree?

BR,
Kewen

Re: [PATCH] ipa-sra: Consider the first parameter of methods safe to dereference

[Richard Biener via Gcc-patches]

On Wed, Dec 14, 2022 at 4:20 PM Jan Hubicka via Gcc-patches
 wrote:
>
> > Hi,
> >
> > Honza requested this after reviewing the patch that taught IPA-SRA
> > that REFERENCE_TYPEs are always non-NULL that the pass also handles
> > the first parameters of methods, this pointers, in the same way.  So
> > this patch does that.
> >
> > The patch is undergoing bootstrap and testing on an x86_64-linux right
> > now.  OK if it passes?
> >
> > Thanks,
> >
> > Martin
> >
> >
> > gcc/ChangeLog:
> >
> > 2022-12-14  Martin Jambor  
> >
> >   * ipa-sra.cc (create_parameter_descriptors): Consider the first
> >   parameter of a method safe to dereference.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2022-12-14  Martin Jambor  
> >
> >   * g++.dg/ipa/ipa-sra-6.C: New test.
>
> OK,

Are you sure that's safe?  The docs for METHOD_TYPE doesn't say 'this'
is the first parameter nor does it say methods cannot be invoked for a
nullptr object.

Do frontends other than C++ create METHOD_TYPE functions?  Grep
shows uses in ada and objective C at least.

> thanks a lot!
> Honza

Re: [PATCH] into-ssa: Fix emitting debug stmts after asm goto [PR108095]

[Richard Biener via Gcc-patches]

On Thu, 15 Dec 2022, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because ccp1 replaced
>   s.0_1 = 
>   __asm__ goto("" : "=r" MEM[(T *)s.0_1] :  :  : "lab" lab);
> with
>   __asm__ goto("" : "=r" s :  :  : "lab" lab);
> and because s is no longer addressable, we are rewriting it into
> ssa and want
>   __asm__ goto("" : "=r" s_7 :  :  : "lab" lab);
> plus debug stmt
>   # DEBUG s => s_7
> The code assumes that there is at most one non-EH edge in that
> case, but with the addition of outputs to asm goto that is no longer the
> case, we can have many outgoing edges.
> 
> The patch keeps the checking assertion that there is at most one such
> edge for everything but asm goto, but moves the addition of the debug
> stmt into the loop, so that it can be added on all edges where it is
> possible, not just one of them.
> 
> Furthermore, looking at gsi_insert_on_edge_immediate
> -> gimple_find_edge_insert_loc, the conditions to insert stmt there
> to the destination block are
>   if (single_pred_p (dest)
>   && gimple_seq_empty_p (phi_nodes (dest))
>   && dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> (plus there is code to insert it in the previous block but that is
> never true when the pred is known to be stmt_ends_bb_p), while
> mayube_register_def was just checking
>  if (ef && single_pred_p (ef->dest)
>  && ef->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> so if for whatever reason ef->dest had any PHIs, we'd split the
> edge for -g and not for -g0, something we must avoid for -fcompare-debug
> stability.  So, I've added the no phi_nodes check too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2022-12-15  Jakub Jelinek  
> 
>   PR tree-optimization/108095
>   * tree-into-ssa.cc (maybe_register_def): Insert debug stmt
>   on all non-EH edges from asm goto if they have a single
>   predecessor rather than asserting there is at most one such edge.
>   Test whether there are no PHI nodes next to the single predecessor
>   test.
> 
>   * gcc.dg/pr108095.c: New test.
> 
> --- gcc/tree-into-ssa.cc.jj   2022-12-05 23:20:24.0 +0100
> +++ gcc/tree-into-ssa.cc  2022-12-14 14:20:50.301283097 +0100
> @@ -1934,9 +1934,8 @@ maybe_register_def (def_operand_p def_p,
> tree tracked_var = target_for_debug_bind (sym);
> if (tracked_var)
>   {
> -   gimple *note = gimple_build_debug_bind (tracked_var, def, stmt);
> -   /* If stmt ends the bb, insert the debug stmt on the single
> -  non-EH edge from the stmt.  */
> +   /* If stmt ends the bb, insert the debug stmt on the non-EH
> +  edge(s) from the stmt.  */
> if (gsi_one_before_end_p (gsi) && stmt_ends_bb_p (stmt))
>   {
> basic_block bb = gsi_bb (gsi);
> @@ -1945,33 +1944,46 @@ maybe_register_def (def_operand_p def_p,
> FOR_EACH_EDGE (e, ei, bb->succs)
>   if (!(e->flags & EDGE_EH))
> {
> - gcc_checking_assert (!ef);
> + /* asm goto can have multiple non-EH edges from the
> +stmt.  Insert on all of them where it is
> +possible.  */
> + gcc_checking_assert (!ef || (gimple_code (stmt)
> +  == GIMPLE_ASM));
>   ef = e;
> -   }
> -   /* If there are other predecessors to ef->dest, then
> -  there must be PHI nodes for the modified
> -  variable, and therefore there will be debug bind
> -  stmts after the PHI nodes.  The debug bind notes
> -  we'd insert would force the creation of a new
> -  block (diverging codegen) and be redundant with
> -  the post-PHI bind stmts, so don't add them.
> + /* If there are other predecessors to ef->dest, then
> +there must be PHI nodes for the modified
> +variable, and therefore there will be debug bind
> +stmts after the PHI nodes.  The debug bind notes
> +we'd insert would force the creation of a new
> +block (diverging codegen) and be redundant with
> +the post-PHI bind stmts, so don't add them.
>  
> -  As for the exit edge, there wouldn't be redundant
> -  bind stmts, but there wouldn't be a PC to bind
> -  them to either, so avoid diverging the CFG.  */
> -   if (ef && single_pred_p (ef->dest)
> -   && ef->dest != EXIT_BLOCK_PTR_FOR_FN (cfun))
> - {
> -   /* If there were PHI nodes in the node, we'd
> -  have to make sure the value we're binding
> -