Re: [PATCH v6] Fix for powerpc64 long double complex divide failure
On 10/1/2021 3:36 PM, Patrick McGehearty via Gcc-patches wrote: - - - - New in version 6: Due to an oversight (i.e. coding error), version 5 changed the use of __LIBGCC_TF_EPSILON__ to __LIBGCC_DF_EPSILON__ but not the other LIBGCC_TF values. For correct execution of the long double test case it is necessary to also switch to using __LIBGCC_DF_MIN__. For consistency we also switch to using __LIBGCC_DF_MAX__. LDBL_MIN is 2**53 times as larger than DBL_MIN. The larger value causes the code to switch the order of computation when it is not optimal, resulting in failure for one of the values in the cdivchk_ld.c test. Using DBL_MIN does not cause that failure.. There may be opportunity for further refinement of IBM128 format Long Double complex divide, but that's beyond the scope of this patch. - - - - This revision adds a test in libgcc/libgcc2.c for when "__LIBGCC_TF_MANT_DIG__ == 106" to use __LIBGCC_DF_EPSILON__ instead of __LIBGCC_TF_EPSILON__. That is specific to IBM 128-bit format long doubles where EPSILON is very, very small and 1/EPSILON oveflows to infinity. This change avoids the overflow without affecting any other platform. Discussion in the patch is adjusted to reflect this limitation. It does not make any changes to .../rs6000/_divkc3.c, leaving it to use __LIBGCC_KF__*. That means the upstream gcc will not build in older IBM environments that do not recognize the KF floating point mode properly. Environments that do not need IBM longdouble support do build cleanly. - - - - This patch addresses the failure of powerpc64 long double complex divide in native ibm long double format after the patch "Practical improvement to libgcc complex divide". The new code uses the following macros which are intended to be mapped to appropriate values according to the underlying hardware representation. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101104 RBIG a value near the maximum representation RMIN a value near the minimum representation (but not in the subnormal range) RMIN2a value moderately less than 1 RMINSCAL the inverse of RMIN2 RMAX2RBIG * RMIN2 - a value to limit scaling to not overflow When "long double" values were not using the IEEE 128-bit format but the traditional IBM 128-bit, the previous code used the LDBL values which caused overflow for RMINSCAL. The new code uses the DBL values. RBIG LDBL_MAX = 0x1.f800p+1022 DBL_MAX = 0x1.f000p+1022 RMIN LDBL_MIN = 0x1.p-969 RMIN DBL_MIN = 0x1.p-1022 RMIN2 LDBL_EPSILON = 0x0.1000p-1022 = 0x1.0p-1074 RMIN2 DBL_EPSILON = 0x1.p-52 RMINSCAL 1/LDBL_EPSILON = inf (1.0p+1074 does not fit in IBM 128-bit). 1/DBL_EPSILON = 0x1.p+52 RMAX2 = RBIG * RMIN2 = 0x1.f800p-52 RBIG * RMIN2 = 0x1.f000p+970 The MAX and MIN values have only modest changes since the maximum and minimum values are about the same as for double precision. The EPSILON field is considerably different. Due to how very small values can be represented in the lower 64 bits of the IBM 128-bit floating point, EPSILON is extremely small, so far beyond the desired value that inversion of the value overflows and even without the overflow, the RMAX2 is so small as to eliminate most usage of the test. The change has been tested on gcc135.fsffrance.org and gains the expected improvements in accuracy for long double complex divide. libgcc/ PR target/101104 * libgcc2.c (RMIN2, RMINSCAL, RMAX2): Use more correct values for native IBM 128-bit. Thanks. Installed. jeff
Re: [PATCH v2] c-family: Implement -Warray-compare [PR97573]
On 10/1/2021 5:08 PM, Marek Polacek via Gcc-patches wrote:
On Fri, Oct 01, 2021 at 11:15:45PM +0200, Jakub Jelinek wrote:
On Fri, Oct 01, 2021 at 04:11:08PM -0400, Marek Polacek via Gcc-patches wrote:
+ auto_diagnostic_group d;
+ if (warning_at (location, OPT_Warray_compare,
+ "comparison between two arrays%s",
+ (c_dialect_cxx () && cxx_dialect >= cxx20)
+ ? " is deprecated in C++20" : ""))
Not a review, just a comment.
The above is impossible to translate, translators would translate the
first half and the second one would be in English; but even translating
the second part too would mean one can't reword it in some other language
in different word order.
You can e.g. use
if (warning_at (location, OPT_Warray_compare,
(c_dialect_cxx () && cxx_dialect >= cxx20)
? G_("comparison between two arrays is deprecated in C++20")
: G_("comparison between two arrays")))
instead.
Thanks, fixed:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This patch addresses one of my leftovers from GCC 11. C++20 introduced
[depr.array.comp]: "Equality and relational comparisons between two operands
of array type are deprecated." so this patch adds -Warray-compare. Since the
code in question is dubious (the comparison doesn't actually compare the array
elements), I've added this warning for C too, and enabled it in all C++ modes.
PR c++/97573
gcc/c-family/ChangeLog:
* c-common.h (do_warn_array_compare): Declare.
* c-warn.c (do_warn_array_compare): New.
* c.opt (Warray-compare): New option.
gcc/c/ChangeLog:
* c-typeck.c (parser_build_binary_op): Call do_warn_array_compare.
gcc/cp/ChangeLog:
* typeck.c (cp_build_binary_op): Call do_warn_array_compare.
gcc/ChangeLog:
* doc/invoke.texi: Document -Warray-compare.
gcc/testsuite/ChangeLog:
* c-c++-common/Warray-compare-1.c: New test.
* c-c++-common/Warray-compare-2.c: New test.
OK. It'll be interesting to see how many dusty packages trip over this ;-)
jeff
Re: [PATCH] libiberty: d-demangle: use switch instead of if-else
On 9/29/2021 7:08 PM, Luís Ferreira wrote: This patch allows the compiler to efficiently generate jump tables instead of using if-else-if. Signed-off-by: Luís Ferreira I'm not sure this is terribly useful. Compilers have the ability to analyze the underlying code and make sensible decisions for how to implement either form. So the right metric here is does this make the code cleaner/easier to understand. With just 3 clauses it's hard (for me) to make the case that it is considerably cleaner. Jeff
Re: [PATCH] c++: fix cases of core1001/1322 by not dropping cv-qualifier of function parameter of type of typename or decltype[PR101402, PR102033, PR102034, PR102039, PR102
Here I attach [PATCH-v2].
On Sun, Oct 3, 2021 at 7:14 AM Nick Huang wrote:
>
> Hi Jason,
>
> I made a little improvement of my fix by including template
> type parameter to not dropping cv-const because they are similar to dependent
> type which you cannot determine whether they are top-level cv-qualifier or not
> until instantiation.
>
> + if (processing_template_decl
> + && (TREE_CODE (type) == TYPENAME_TYPE
> + || TREE_CODE (type) == DECLTYPE_TYPE
> ++ || TREE_CODE (type) == TEMPLATE_TYPE_PARM // this is new
> + )
> +)
>
> 1. It fix your test case of
> template
> struct A{
>void f(T);
> };
> template
> void A::f(const T){ }
> template<>
> void A::f(const int*){}
>
> current GCC mistakenly accepts without considering the gap of missing "const"
> between declaration and out of line definition. clang correctly rejects it.
> (https://www.godbolt.org/z/qb9Tf99eK) and explains the cause nicely.
> My fix also rejects it.
>
> 2. It also fix a more obvious core1001 issue of free function as my previous
> fix
> only deals with typename/decltype cases.
> template
> void f(const T){}
> template<>
> void f(const int*){}
>
> Your comment is appreciated.
>
> thank you
From 0d096262fc708edea825af713526b1ce1e9e689d Mon Sep 17 00:00:00 2001
From: qingzhe huang
Date: Sun, 3 Oct 2021 16:00:29 -0400
Subject: [PATCH] Fix core1001/1322 by preserving const
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
These bugs are considered duplicate cases of PR51851 which has been
suspended since 2012, an issue known as "core1001/1322". Considering
this background, it deserves a long comment to explain.
Many people believed the root cause of this family of bugs is related
with the nature of how and when the array type is converted to pointer
type during function signature is calculated. This is true, but we may
need to go into details to understand the exact reason.
There is a pattern for these bugs(PR101402,PR102033,PR102034,PR102039).
In the template function declaration, the function parameter is consisted
of a "const" followed by a typename-type which is actually an array type.
According to standard, function signature is calculated by dropping so-
called "top-level-cv-qualifier". As a result, the templater specialization
complains no matching to declaration can be found because specialization
has const and template function declaration doesn't have const which is
dropped as mentioned. Obviously the template function declaration should
NOT drop the const. But why? Let's review the procedure of standard first.
(https://timsong-cpp.github.io/cppwp/dcl.fct#5.sentence-3)
"After determining the type of each parameter, any parameter of type
“array of T” or of function type T is adjusted to be “pointer to T”. After
producing the list of parameter types, any top-level cv-qualifiers
modifying a parameter type are deleted when forming the function type."
Please note the action of deleting top-level cv-qualifiers happens at last
stage after array type is converted to pointer type. More importantly,
there are two conditions:
a) Each type must be able to be determined.
b) The cv-qualifier must be top-level.
Let's analysis if these two conditions can be met one by one.
1) Keyword "typename" indicates inside template it involves dependent name
(https://timsong-cpp.github.io/cppwp/n4659/temp.res#2) for which the name
lookup can be postponed until template instantiation. Clearly the type of
dependent name cannot be determined without name lookup. Then we can NOT
proceed to next step until concrete template argument type is determined
during specialization.
2) After “array of T” is converted to “pointer to T”, the cv-qualifiers
are no longer top-level! Unfortunately in standard there is no definition
of "top-level". Mr. Dan Saks's articals
(https://www.dansaks.com/articles.shtml)
are tremendous help! Especially this wonderful paper
(https://www.dansaks.com/articles/
2000-02%20Top-Level%20cv-Qualifiers%20in%20Function%20Parameters.pdf)
discusses this topic in details. In one short sentence, the "const" before
array type is NOT top-level-cv-qualifier and should NOT be dropped.
So, understanding the root cause makes the fix very clear: Let's NOT drop
cv-qualifier for typename-type inside template. Leave this task for
template substitution later when template specialization locks template
argument types.
Similarly inside template, "decltype" may also include dependent name and
the best strategy for parser is to preserve all original declaration and
postpone the task till template substitution.
For template type parameter, it is obvious to follow similar strategy to
not drop cv-const and leave it till instantiation.
Here is an interesting observation to share. Originally my fix is trying
to use function "resolve_typename_type" to see if the "typename-type" is
indeed an array type so as to decide whether the
[pushed] coroutines: Fail with a sorry when presented with a VLA [PR 101765].
We do not support this yet.
fail with a sorry instead of ICEing.
tested on x86_64,powerpc64le-linux, x86_64-darwin, cppcoro, folly.
pushed to master as obvious, thanks
Iain
Signed-off-by: Iain Sandoe
PR c++/101765
gcc/cp/ChangeLog:
* coroutines.cc (register_local_var_uses): Emit a sorry if
we encounter a VLA in the coroutine local variables.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr101765.C: New test.
---
gcc/cp/coroutines.cc | 10 +
gcc/testsuite/g++.dg/coroutines/pr101765.C | 45 ++
2 files changed, 55 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101765.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 377b90c777f..9017902e6fb 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3927,6 +3927,16 @@ register_local_var_uses (tree *stmt, int *do_subtree,
void *d)
if (local_var.is_static)
continue;
+ poly_uint64 size;
+ if (TREE_CODE (lvtype) == ARRAY_TYPE
+ && !poly_int_tree_p (DECL_SIZE_UNIT (lvar), ))
+ {
+ sorry_at (local_var.def_loc, "variable length arrays are not"
+ " yet supported in coroutines");
+ /* Ignore it, this is broken anyway. */
+ continue;
+ }
+
lvd->local_var_seen = true;
/* If this var is a lambda capture proxy, we want to leave it alone,
and later rewrite the DECL_VALUE_EXPR to indirect through the
diff --git a/gcc/testsuite/g++.dg/coroutines/pr101765.C
b/gcc/testsuite/g++.dg/coroutines/pr101765.C
new file mode 100644
index 000..49a49d11299
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr101765.C
@@ -0,0 +1,45 @@
+// We cannot compile this yet, much run it - but one day it might be
+// feasible, so do the minimum for now.
+// { dg-additional-options " -fsyntax-only -Wno-vla" }
+
+#include "coro.h"
+
+// boiler-plate for tests of codegen
+#include "coro1-ret-int-yield-int.h"
+
+struct coro1
+foo (int arg) noexcept
+{
+ PRINTF ("foo arg = %d\n", arg);
+ char arr[arg]; /* { dg-message "sorry, unimplemented: variable length arrays
are not yet supported in coroutines" "" { target *-*-* } } */
+ if (arg < 4)
+co_return -6174;
+ else
+for (int i = 0; i < arg; ++i) arr[i] = (char) i;
+ co_yield (int) arr[2];
+ co_return (int) arr[3];
+}
+
+int main ()
+{
+ PRINT ("main: create coro1");
+ struct coro1 x = foo (10);
+ PRINT ("main: got coro1 - resuming");
+ if (x.handle.done())
+abort();
+ x.handle.resume();
+ PRINT ("main: after resume");
+ int y = x.handle.promise().get_value();
+ if ( y == -6174 )
+{
+ PRINT ("main: saw -6174");
+ return 1;
+}
+ else if ( y != 2 )
+abort;
+ x.handle.resume();
+ y = x.handle.promise().get_value();
+ if ( y != 3 )
+abort ();
+ return 0;
+}
--
2.24.3 (Apple Git-128)
[pushed] coroutines: Await expressions are not allowed in handlers [PR 99710].
Hi,
C++20 [expr.await] / 2
An await-expression shall appear only in a potentially-evaluated expression
within the compound-statement of a function-body outside of a handler.
tested on x86_64, powerpc64le-linux, x86_64-darwin, cppcoro and folly,
pushed to master as obvious, thanks
Iain
Signed-off-by: Iain Sandoe
PR c++/99710
gcc/cp/ChangeLog:
* coroutines.cc (await_statement_walker): Report an error if
an await expression is found in a handler body.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr99710.C: New test.
---
gcc/cp/coroutines.cc | 17 ++-
gcc/testsuite/g++.dg/coroutines/pr99710.C | 25 +++
2 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99710.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 1dd9abd1e1f..377b90c777f 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -3688,7 +3688,22 @@ await_statement_walker (tree *stmt, int *do_subtree,
void *d)
*do_subtree = 0;
return res;
}
- break;
+ break;
+ case HANDLER:
+ {
+ /* [expr.await] An await-expression shall appear only in a
+ potentially-evaluated expression within the compound-statement
+ of a function-body outside of a handler. */
+ tree *await_ptr;
+ hash_set visited;
+ if (!(cp_walk_tree (_BODY (expr), find_any_await,
+ _ptr, )))
+ return NULL_TREE; /* All OK. */
+ location_t loc = EXPR_LOCATION (*await_ptr);
+ error_at (loc, "await expressions are not permitted in handlers");
+ return NULL_TREE; /* This is going to fail later anyway. */
+ }
+ break;
}
else if (EXPR_P (expr))
{
diff --git a/gcc/testsuite/g++.dg/coroutines/pr99710.C
b/gcc/testsuite/g++.dg/coroutines/pr99710.C
new file mode 100644
index 000..e4f7116b8d7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr99710.C
@@ -0,0 +1,25 @@
+#include
+
+struct task {
+struct promise_type {
+std::suspend_always initial_suspend();
+std::suspend_always final_suspend() noexcept;
+task get_return_object();
+void return_void();
+void unhandled_exception();
+};
+};
+
+task
+my_coro ()
+{
+ try
+{ }
+ catch (...)
+{
+ // [expr.await] An await-expression shall appear only in a potentially-
+ // evaluated expression within the compound-statement of a function-body
+ // outside of a handler
+ co_await std::suspend_always{}; // { dg-error "await expressions are not
permitted in handlers" }
+}
+}
--
2.24.3 (Apple Git-128)
[pushed] coroutines: Fix ICE with an invalid await_suspend type [PR100673].
From: John Eivind Helset
Hi,
The reported ICE occurs when an invalid (non-template) type is found
as the return for an await_suspend.
Fixed by amending build_co_await to ensure that await_suspend return-
type is a template-instantiation before checking to see if it is a
valid coroutine handle type.
Patch by John Eivind Helset, tested on x86_64, powerpc64le linux and
x86_64 darwin, pushed to master as trivial/obvious, thanks
Iain
Signed-off-by: John Eivind Helset
Signed-off-by: Iain Sandoe
PR c++/100673
gcc/cp/ChangeLog:
* coroutines.cc (build_co_await): Guard against NULL
await_suspend types.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr100673.C: New test.
---
gcc/cp/coroutines.cc | 3 ++-
gcc/testsuite/g++.dg/coroutines/pr100673.C | 18 ++
2 files changed, 20 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100673.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index bff5b6343e5..1dd9abd1e1f 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1053,7 +1053,8 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind)
else if (same_type_p (susp_return_type, boolean_type_node))
ok = true;
else if (TREE_CODE (susp_return_type) == RECORD_TYPE
- && CLASS_TYPE_P (susp_return_type))
+ && CLASS_TYPE_P (susp_return_type)
+ && CLASSTYPE_TEMPLATE_INFO (susp_return_type))
{
tree tt = CLASSTYPE_TI_TEMPLATE (susp_return_type);
if (tt == coro_handle_templ)
diff --git a/gcc/testsuite/g++.dg/coroutines/pr100673.C
b/gcc/testsuite/g++.dg/coroutines/pr100673.C
new file mode 100644
index 000..750ce66ae15
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr100673.C
@@ -0,0 +1,18 @@
+// { dg-additional-options "-fsyntax-only -w" }
+
+// Diagnose bad coroutine awatiable type.
+
+#include
+
+struct coro{
+ struct not_a_template{};
+ using promise_type = coro;
+ static constexpr std::suspend_always initial_suspend()noexcept{ return {}; }
+ constexpr bool await_ready()noexcept{ return false; }
+ constexpr not_a_template await_suspend(std::coroutine_handle<>)noexcept{
return{}; }
+ constexpr void await_resume()noexcept{}
+ static coro body()
+ {
+co_await coro{}; // { dg-error {'await_suspend' must return 'void', 'bool'
or a coroutine handle} }
+ }
+};
--
2.24.3 (Apple Git-128)
[PATCH] coroutines: Ensure co_await_exprs have TREE_SIDE_EFFECTS set [PR 101133].
Hi,
Although it is not immediately evident from the symptoms, the PR is
caused by a variable having a DECL_INITIAL() containing a co_await.
This is not correct, since such expressions have side-effects.
We were marking the overall co_await expression correctly, but if a
consumer of that expression stripped out the underlying co_await_expr
then the latter would not be properly marked.
Fixed by marking both the underlying and any containing await expr
with TREE_SIDE_EFFECTS. Also mark type-dependent co_await expressions.
tested on x86_64, powerpc64le linux and x86_64 darwin,
pushed to master as obvious, thanks,
Iain
Signed-off-by: Iain Sandoe
PR c++/101133
gcc/cp/ChangeLog:
* coroutines.cc (build_co_await): Mark co_await_expr trees
with TREE_SIDE_EFFECTS, also mark any containing expression.
(finish_co_await_expr): Mark type-dependent co_await_expr
trees with TREE_SIDE_EFFECTS.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr101133.C: New test.
---
gcc/cp/coroutines.cc | 24 -
gcc/testsuite/g++.dg/coroutines/pr101133.C | 31 ++
2 files changed, 43 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr101133.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 876a14606f8..bff5b6343e5 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1117,13 +1117,15 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind)
a, e_proxy, o, awaiter_calls,
build_int_cst (integer_type_node,
(int) suspend_kind));
+ TREE_SIDE_EFFECTS (await_expr) = true;
if (te)
{
TREE_OPERAND (te, 1) = await_expr;
+ TREE_SIDE_EFFECTS (te) = true;
await_expr = te;
}
- tree t = convert_from_reference (await_expr);
- return t;
+ SET_EXPR_LOCATION (await_expr, loc);
+ return convert_from_reference (await_expr);
}
tree
@@ -1149,8 +1151,13 @@ finish_co_await_expr (location_t kw, tree expr)
co_await with the expression unchanged. */
tree functype = TREE_TYPE (current_function_decl);
if (dependent_type_p (functype) || type_dependent_expression_p (expr))
-return build5_loc (kw, CO_AWAIT_EXPR, unknown_type_node, expr,
- NULL_TREE, NULL_TREE, NULL_TREE, integer_zero_node);
+{
+ tree aw_expr = build5_loc (kw, CO_AWAIT_EXPR, unknown_type_node, expr,
+NULL_TREE, NULL_TREE, NULL_TREE,
+integer_zero_node);
+ TREE_SIDE_EFFECTS (aw_expr) = true;
+ return aw_expr;
+}
/* We must be able to look up the "await_transform" method in the scope of
the promise type, and obtain its return type. */
@@ -1187,14 +1194,7 @@ finish_co_await_expr (location_t kw, tree expr)
}
/* Now we want to build co_await a. */
- tree op = build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT);
- if (op != error_mark_node)
-{
- TREE_SIDE_EFFECTS (op) = 1;
- SET_EXPR_LOCATION (op, kw);
-}
-
- return op;
+ return build_co_await (kw, a, CO_AWAIT_SUSPEND_POINT);
}
/* Take the EXPR given and attempt to build:
diff --git a/gcc/testsuite/g++.dg/coroutines/pr101133.C
b/gcc/testsuite/g++.dg/coroutines/pr101133.C
new file mode 100644
index 000..6c6bc163251
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr101133.C
@@ -0,0 +1,31 @@
+#include
+#include
+
+template
+struct Awaiter
+{
+bool await_ready() const { return false; }
+void await_suspend(std::coroutine_handle<>) const {}
+Tawait_resume() const { return T{}; }
+};
+
+struct ReturnObject
+{
+struct promise_type
+{
+ReturnObject get_return_object(){ return {}; }
+std::suspend_never initial_suspend() noexcept { return {}; }
+std::suspend_never final_suspend() noexcept { return {}; }
+void return_void() {}
+void unhandled_exception() {}
+};
+};
+
+ReturnObject f()
+{
+auto a1 = Awaiter{};
+auto a2 = Awaiter{};
+
+[[maybe_unused]] auto v1 = co_await a1; // ok
+[[maybe_unused]] std::string v2 = co_await a2; // error
+}
--
2.24.3 (Apple Git-128)
[pushed] coroutines: Look through NOPs for awaiter variables [PR 99575].
Hi,
There was a missing STRIP_NOPS which meant that, in some cases,
an awaiter variable could be hidden by a view-convert-expr.
tested on x86_64, powerpc64le linux and x86_86 darwin.
pushed to master as trivial/obvious.
thanks,
Iain
Signed-off-by: Iain Sandoe
PR c++/99575
gcc/cp/ChangeLog:
* coroutines.cc (build_co_await): Strip NOPs from
candidate awaiter expressions before testing to see
if they need a temporary.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr99575.C: New test.
---
gcc/cp/coroutines.cc | 1 +
gcc/testsuite/g++.dg/coroutines/pr99575.C | 35 +++
2 files changed, 36 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/coroutines/pr99575.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 2f45575bd92..876a14606f8 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1008,6 +1008,7 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind)
}
/* Only build a temporary if we need it. */
+ STRIP_NOPS (e_proxy);
if (TREE_CODE (e_proxy) == PARM_DECL
|| (VAR_P (e_proxy) && !is_local_temp (e_proxy)))
{
diff --git a/gcc/testsuite/g++.dg/coroutines/pr99575.C
b/gcc/testsuite/g++.dg/coroutines/pr99575.C
new file mode 100644
index 000..d5f86c1812e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr99575.C
@@ -0,0 +1,35 @@
+
+#include
+
+class Task {
+ public:
+ struct promise_type {
+Task get_return_object() { return Task{}; }
+std::suspend_always initial_suspend() { return {}; }
+std::suspend_always final_suspend() noexcept { return {}; }
+void unhandled_exception() {}
+void return_void() {}
+ };
+
+ bool await_ready() const { return false; }
+ void await_suspend(std::coroutine_handle continuation) {}
+ void await_resume() {}
+};
+
+class NonMoveableTask {
+ public:
+ NonMoveableTask() = default;
+ NonMoveableTask(const NonMoveableTask&) = delete;
+ NonMoveableTask(NonMoveableTask&&) = delete;
+
+ NonMoveableTask& operator=(const NonMoveableTask&) = delete;
+ NonMoveableTask& operator=(NonMoveableTask&& other) = delete;
+
+ bool await_ready() const { return false; }
+ void await_suspend(std::coroutine_handle) {}
+ void await_resume() {}
+};
+
+Task Foo(NonMoveableTask* task) { co_await* task; }
+
+int main() {}
--
2.24.3 (Apple Git-128)
Coroutines backports.
Hi, I’ve backported the following fixes to GCC 11 14ed21f8749ae359690d9c4a69ca38cc45d0d1b0, 8406ed9af2655479a9c8469d7acca2cf5784f5d6, 21b4d0ef543d68187d258415b51d0d6676af89fd, 88974974d8188cf12e87e4ad3d23a8cbdd557f0e, a45a7ecdf34311587daa2e90cc732adcefac447b, addf167a23f61c0ec97f6e71577a0623f3fc13e7, c5a735fa9df7eca4666c8da5e51ed9c5ab7cc81a, 70ee703c479081ac2ea67eb67041551216e66783, fae627162d5f8cfb273b10349883eeb74baaa43f which constitutes as far as we’ve got at present with debugging support (PR 99215) To GCC-10 3bcf19215d88e6ec33d283352c52005f02dbc784, 0d5db79a61af150cba48612c9fbc3267262adb93, d5b1bb0d197f9141a0f0e510f8d1b598c3df9552, 237ab3ee49e2f3110accfcc03b6c0df8b4889f15, which fixes PR95520 and some minor issues. thanks Iain
[PATCH] PR fortran/99348, 102521 - ICEs when initializing DT parameter arrays from scalar
Dear Fortranners,
when initializing parameter arrays from scalars, we did handle only
the case init->expr_type == EXPR_CONSTANT, which misses the case of
derived types. As a consequence the constructor for the r.h.s. was
not set up, which later led to different ICEs.
To solve this I looked at gfc_simplify_spread. I was contemplating
whether to also copy the logic to make this initialization dependent
on -fmax-array-constructor. I chose not to, because there is no
sensible and simple fallback available to handle that case while
allowing the access to array elements. We could instead make that
a warning.
Comments / opinions?
Regtested on x86_64-pc-linux-gnu. OK for mainline?
As this is an ICE on valid, potentially useful code,
I'd like to backport this to 11-branch.
Thanks,
Harald
Fortran: handle initialization of derived type parameter arrays from scalar
gcc/fortran/ChangeLog:
PR fortran/99348
PR fortran/102521
* decl.c (add_init_expr_to_sym): Extend initialization of
parameter arrays from scalars to handle derived types.
gcc/testsuite/ChangeLog:
PR fortran/99348
PR fortran/102521
* gfortran.dg/parameter_array_init_8.f90: New test.
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index b3c65b7175b..d6a22d13451 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -2228,12 +2228,16 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus)
gfc_expr *array;
int n;
if (sym->attr.flavor == FL_PARAMETER
- && init->expr_type == EXPR_CONSTANT
- && spec_size (sym->as, )
- && mpz_cmp_si (size, 0) > 0)
+ && gfc_is_constant_expr (init)
+ && (init->expr_type == EXPR_CONSTANT
+ || init->expr_type == EXPR_STRUCTURE)
+ && spec_size (sym->as, )
+ && mpz_cmp_si (size, 0) > 0)
{
array = gfc_get_array_expr (init->ts.type, init->ts.kind,
>where);
+ if (init->ts.type == BT_DERIVED)
+ array->ts.u.derived = init->ts.u.derived;
for (n = 0; n < (int)mpz_get_si (size); n++)
gfc_constructor_append_expr (>value.constructor,
n == 0
diff --git a/gcc/testsuite/gfortran.dg/parameter_array_init_8.f90 b/gcc/testsuite/gfortran.dg/parameter_array_init_8.f90
new file mode 100644
index 000..05b2e424a3f
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/parameter_array_init_8.f90
@@ -0,0 +1,25 @@
+! { dg-do run }
+! PR fortran/99348
+! PR fortran/102521
+! Check simplifications for initialization of DT paramameter arrays
+
+program p
+ type t
+ integer :: n
+ end type
+ type(t), parameter :: a(4) = t(1)
+ type(t), parameter :: d(*) = a
+ type(t), parameter :: b(2,2) = reshape(d, [2,2])
+ integer, parameter :: nn = b(2,2)% n
+ type u
+ character(3) :: c
+ end type
+ type(u), parameter :: x(2,3) = u('ab')
+ type(u), parameter :: y(*,*) = transpose (x)
+ character(*), parameter :: c = y(3,2)% c
+ integer, parameter :: lc = c% len
+ integer, parameter :: lyc= len (y(3,2)% c)
+! integer, parameter :: lxc= x(1,1)% c% len! fails (pr101735?)
+ if (nn /= 1) stop 1
+ if (lc /= 3 .or. lyc /= 3 .or. c /= "ab ") stop 2
+end
Re: [PATCH] Try placing RTL folded constants in constant pool
On Sun, Oct 3, 2021 at 7:27 AM Roger Sayle wrote:
>
>
> My recent attempts to come up with a testcase for my patch to evaluate
> ss_plus in simplify-rtx.c, identified a missed optimization opportunity
> (that's potentially a long-time regression): The RTL optimizers no longer
> place constants in the constant pool.
>
> The motivating x86_64 example is the simple program:
>
> typedef char v8qi __attribute__ ((vector_size (8)));
>
> v8qi foo()
> {
> v8qi tx = { 1, 0, 0, 0, 0, 0, 0, 0 };
> v8qi ty = { 2, 0, 0, 0, 0, 0, 0, 0 };
> v8qi t = __builtin_ia32_paddsb(tx, ty);
> return t;
> }
>
> which (with my previous patch) currently results in:
> foo:movq.LC0(%rip), %xmm0
> movq.LC1(%rip), %xmm1
> paddsb %xmm1, %xmm0
> ret
>
> even though the RTL contains the result in a REG_EQUAL note:
>
> (insn 7 6 12 2 (set (reg:V8QI 83)
> (ss_plus:V8QI (reg:V8QI 84)
> (reg:V8QI 85))) "ssaddqi3.c":7:12 1419 {*mmx_ssaddv8qi3}
> (expr_list:REG_DEAD (reg:V8QI 85)
> (expr_list:REG_DEAD (reg:V8QI 84)
> (expr_list:REG_EQUAL (const_vector:V8QI [
> (const_int 3 [0x3])
> (const_int 0 [0]) repeated x7
> ])
> (nil)
>
> Together with the patch below, GCC will now generate the much
> more sensible:
> foo:movq.LC2(%rip), %xmm0
> ret
>
> My first approach was to look in cse.c (where the REG_EQUAL note gets
> added) and notice that the constant pool handling functionality has been
> unreachable for a while. A quick search for constant_pool_entries_cost
> shows that it's initialized to zero, but never set to a non-zero value,
> meaning that force_const_mem is never called. This functionality used
> to work way back in 2003, but has been lost over time:
> https://gcc.gnu.org/pipermail/gcc-patches/2003-October/116435.html
>
> The changes to cse.c below restore this functionality (placing suitable
> constants in the constant pool) with two significant refinements;
> (i) it only attempts to do this if the function already uses a constant
> pool (thanks to the availability of crtl->uses_constant_pool since 2003).
> (ii) it allows different constants (i.e. modes) to have different costs,
> so that floating point "doubles" and 64-bit, 128-bit, 256-bit and 512-bit
> vectors don't all have the share the same cost. Back in 2003, the
> assumption was that everything in a constant pool had the same
> cost, hence the global variable constant_pool_entries_cost.
>
> Although this is a useful CSE fix, it turns out that it doesn't cure my
> motivating problem above. CSE only considers a single instruction,
> so determines that it's cheaper to perform the ss_plus (COSTS_N_INSNS(1))
> than read the result from the constant pool (COSTS_N_INSNS(2)). It's
> only when the other reads from the constant pool are also eliminated,
> that this transformation is a win. Hence a better place to perform
> this transformation is in combine, where after failing to "recog" the
> load of a suitable constant, it can retry after calling force_const_mem.
> This achieves the desired transformation and allows the backend insn_cost
> call-back to control whether or not using the constant pool is preferrable.
>
> Alas, it's rare to change code generation without affecting something in
> GCC's testsuite. On x86_64-pc-linux-gnu there were two families of new
> failures (and I'd predict similar benign fallout on other platforms).
> One failure was gcc.target/i386/387-12.c (aka PR target/26915), where
> the test is missing an explicit -m32 flag. On i686, it's very reasonable
> to materialize -1.0 using "fld1; fchs", but on x86_64-pc-linux-gnu we
> currently generate the awkward:
> testm1: fld1
> fchs
> fstpl -8(%rsp)
> movsd -8(%rsp), %xmm0
> ret
>
> which combine now very reasonably simplifies to just:
> testm1: movsd .LC3(%rip), %xmm0
> ret
>
> The other class of x86_64-pc-linux-gnu failure was from materialization
> of vector constants using vpbroadcast (e.g. gcc.target/i386/pr90773-17.c)
> where the decision is finely balanced; the load of an integer register
> with an immediate constant, followed by a vpbroadcast is deemed to be
> COSTS_N_INSNS(2), whereas a load from the constant pool is also reported
> as COSTS_N_INSNS(2). My solution is to tweak the i386.c's rtx_costs
> so that all other things being equal, an instruction (sequence) that
> accesses memory is fractionally more expensive than one that doesn't.
>
>
> Hopefully, this all makes sense. If someone could benchmark this for
> me that would me much appreciated. This patch has been tested on
> x86_64-pc-linux-gnu with "make bootstrap" and "make -k check" with no
> new failures. Ok for mainline?
>
>
> 2021-10-03 Roger Sayle
>
> gcc/ChangeLog
> * combine.c (recog_for_combine): For an unrecognized move/set of
> a constant, try force_const_mem to place it in the
[PATCH] Try placing RTL folded constants in constant pool
My recent attempts to come up with a testcase for my patch to evaluate
ss_plus in simplify-rtx.c, identified a missed optimization opportunity
(that's potentially a long-time regression): The RTL optimizers no longer
place constants in the constant pool.
The motivating x86_64 example is the simple program:
typedef char v8qi __attribute__ ((vector_size (8)));
v8qi foo()
{
v8qi tx = { 1, 0, 0, 0, 0, 0, 0, 0 };
v8qi ty = { 2, 0, 0, 0, 0, 0, 0, 0 };
v8qi t = __builtin_ia32_paddsb(tx, ty);
return t;
}
which (with my previous patch) currently results in:
foo:movq.LC0(%rip), %xmm0
movq.LC1(%rip), %xmm1
paddsb %xmm1, %xmm0
ret
even though the RTL contains the result in a REG_EQUAL note:
(insn 7 6 12 2 (set (reg:V8QI 83)
(ss_plus:V8QI (reg:V8QI 84)
(reg:V8QI 85))) "ssaddqi3.c":7:12 1419 {*mmx_ssaddv8qi3}
(expr_list:REG_DEAD (reg:V8QI 85)
(expr_list:REG_DEAD (reg:V8QI 84)
(expr_list:REG_EQUAL (const_vector:V8QI [
(const_int 3 [0x3])
(const_int 0 [0]) repeated x7
])
(nil)
Together with the patch below, GCC will now generate the much
more sensible:
foo:movq.LC2(%rip), %xmm0
ret
My first approach was to look in cse.c (where the REG_EQUAL note gets
added) and notice that the constant pool handling functionality has been
unreachable for a while. A quick search for constant_pool_entries_cost
shows that it's initialized to zero, but never set to a non-zero value,
meaning that force_const_mem is never called. This functionality used
to work way back in 2003, but has been lost over time:
https://gcc.gnu.org/pipermail/gcc-patches/2003-October/116435.html
The changes to cse.c below restore this functionality (placing suitable
constants in the constant pool) with two significant refinements;
(i) it only attempts to do this if the function already uses a constant
pool (thanks to the availability of crtl->uses_constant_pool since 2003).
(ii) it allows different constants (i.e. modes) to have different costs,
so that floating point "doubles" and 64-bit, 128-bit, 256-bit and 512-bit
vectors don't all have the share the same cost. Back in 2003, the
assumption was that everything in a constant pool had the same
cost, hence the global variable constant_pool_entries_cost.
Although this is a useful CSE fix, it turns out that it doesn't cure my
motivating problem above. CSE only considers a single instruction,
so determines that it's cheaper to perform the ss_plus (COSTS_N_INSNS(1))
than read the result from the constant pool (COSTS_N_INSNS(2)). It's
only when the other reads from the constant pool are also eliminated,
that this transformation is a win. Hence a better place to perform
this transformation is in combine, where after failing to "recog" the
load of a suitable constant, it can retry after calling force_const_mem.
This achieves the desired transformation and allows the backend insn_cost
call-back to control whether or not using the constant pool is preferrable.
Alas, it's rare to change code generation without affecting something in
GCC's testsuite. On x86_64-pc-linux-gnu there were two families of new
failures (and I'd predict similar benign fallout on other platforms).
One failure was gcc.target/i386/387-12.c (aka PR target/26915), where
the test is missing an explicit -m32 flag. On i686, it's very reasonable
to materialize -1.0 using "fld1; fchs", but on x86_64-pc-linux-gnu we
currently generate the awkward:
testm1: fld1
fchs
fstpl -8(%rsp)
movsd -8(%rsp), %xmm0
ret
which combine now very reasonably simplifies to just:
testm1: movsd .LC3(%rip), %xmm0
ret
The other class of x86_64-pc-linux-gnu failure was from materialization
of vector constants using vpbroadcast (e.g. gcc.target/i386/pr90773-17.c)
where the decision is finely balanced; the load of an integer register
with an immediate constant, followed by a vpbroadcast is deemed to be
COSTS_N_INSNS(2), whereas a load from the constant pool is also reported
as COSTS_N_INSNS(2). My solution is to tweak the i386.c's rtx_costs
so that all other things being equal, an instruction (sequence) that
accesses memory is fractionally more expensive than one that doesn't.
Hopefully, this all makes sense. If someone could benchmark this for
me that would me much appreciated. This patch has been tested on
x86_64-pc-linux-gnu with "make bootstrap" and "make -k check" with no
new failures. Ok for mainline?
2021-10-03 Roger Sayle
gcc/ChangeLog
* combine.c (recog_for_combine): For an unrecognized move/set of
a constant, try force_const_mem to place it in the constant pool.
* cse.c (constant_pool_entries_cost, constant_pool_entries_regcost):
Delete global variables (that are no longer assigned a cost value).
(cse_insn): Simplify logic for deciding whether to place a folded
[PATCH][OBVIOUS] options: check for CL_OPTIMIZATION only for cl_options.
One more obvious fix that unblocks ASAN bootstrap. Martin gcc/ChangeLog: * toplev.c (toplev::main): Check opt_index if it is a part of cl_options. --- gcc/toplev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/toplev.c b/gcc/toplev.c index d952319ad95..ec9f998a49b 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -2339,7 +2339,8 @@ toplev::main (int argc, char **argv) /* Save Optimization decoded options. */ for (unsigned i = 1; i < save_decoded_options_count; ++i) -if (cl_options[save_decoded_options[i].opt_index].flags & CL_OPTIMIZATION) +if (save_decoded_options[i].opt_index < cl_options_count + && cl_options[save_decoded_options[i].opt_index].flags & CL_OPTIMIZATION) save_opt_decoded_options.safe_push (save_decoded_options[i]); /* Perform language-specific options initialization. */ -- 2.33.0
[PATCH] c++: fix cases of core1001/1322 by not dropping cv-qualifier of function parameter of type of typename or decltype[PR101402, PR102033, PR102034, PR102039, PR102
Hi Jason,
I made a little improvement of my fix by including template
type parameter to not dropping cv-const because they are similar to dependent
type which you cannot determine whether they are top-level cv-qualifier or not
until instantiation.
+ if (processing_template_decl
+ && (TREE_CODE (type) == TYPENAME_TYPE
+ || TREE_CODE (type) == DECLTYPE_TYPE
++ || TREE_CODE (type) == TEMPLATE_TYPE_PARM // this is new
+ )
+)
1. It fix your test case of
template
struct A{
void f(T);
};
template
void A::f(const T){ }
template<>
void A::f(const int*){}
current GCC mistakenly accepts without considering the gap of missing "const"
between declaration and out of line definition. clang correctly rejects it.
(https://www.godbolt.org/z/qb9Tf99eK) and explains the cause nicely.
My fix also rejects it.
2. It also fix a more obvious core1001 issue of free function as my previous fix
only deals with typename/decltype cases.
template
void f(const T){}
template<>
void f(const int*){}
Your comment is appreciated.
thank you
Re: [committed] libstdc++: Allow visiting inherited variants [PR 90943]
On Sat, 2 Oct 2021, 13:50 Daniel Krügler via Libstdc++, <
libstd...@gcc.gnu.org> wrote:
> Am Fr., 1. Okt. 2021 um 21:57 Uhr schrieb Jonathan Wakely via
> Libstdc++ :
> >
> > Implement the changes from P2162R2 (as a DR for C++17).
> >
> > Signed-off-by: Jonathan Wakely
> >
> > libstdc++-v3/ChangeLog:
> >
> > PR libstdc++/90943
> > * include/std/variant (__cpp_lib_variant): Update value.
> > (__detail::__variant::__as): New helpers implementing the
> > as-variant exposition-only function templates.
> > (visit, visit): Use __as to upcast the variant parameters.
> > * include/std/version (__cpp_lib_variant): Update value.
> > * testsuite/20_util/variant/visit_inherited.cc: New test.
> >
> > Tested powerpc64le-linux. Committed to trunk.
> >
>
> I'm wondering why the first __as overload is not noexcept as well (or
> asking it the other way around: Why different exception-specifications
> are used for the different overloads):
>
> + // The __as function templates implement the exposition-only
> "as-variant"
> +
> + template
> +constexpr std::variant<_Types...>&
> +__as(std::variant<_Types...>& __v)
> +{ return __v; }
>
Probably just an oversight, I'll check again and fix it. Thanks!
Re: [PATCH] [www] Add note about computed gotos to changes and porting guide
On Wed, 29 Sep 2021, apinski--- via Gcc-patches wrote: > Even though there is not many computed gotos in the wild and even less > that would use an integer type, it would still be a good idea to add > this new error message to both changes and the porting to guide. Lovely, thank you! > +Computed goto now require a pointer type Is there an "s" missing somewhere here, e.g. "require*s" or "goto*s*"? Gerald
