Re: [PATCH] match: Change (A + CST0) * CST1 to (A + sign_extend(CST0)) * CST1 [PR116845]
Folks,
we'd appreciate it if someone could take the time to review this fix
for PR116845.
Thanks,
Philipp.
On Tue, 31 Dec 2024 at 10:03, Konstantinos Eleftheriou
wrote:
>
> From: kelefth
>
> `(A * B) + (-C) to (B - C/A) * A` fails to match on ILP32 targets due to
> the upper bits of CST0 being zeros in some cases.
>
> This patch adds the following pattern in match.pd:
> (A + CST0) * CST1 -> (A + CST0') * CST1, where CST1 is a power of 2
> constant and CST0' is CST0 with the log2(CST1) MS bits sign-extended.
>
> This pattern sign-extends the log2(CST1) MS bits of CST0, in case that
> the bit that follows them is set. These bits will be pushed out by
> the multiplication with CST1.
>
> Bootstrapped/regtested on x86 and AArch64.
>
> PR tree-optimization/116845
>
> gcc/ChangeLog:
>
> * match.pd: New pattern.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr116845.c: New test.
> ---
> gcc/match.pd | 21 -
> gcc/testsuite/gcc.target/i386/pr116845.c | 23 +++
> 2 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr116845.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 83eca8b2e0a..aa2108726f9 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4400,7 +4400,26 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> /* Calculate @2 / @0 in order to factorize the expression. */
> wide_int div_res = wi::sdiv_trunc (c2, c0);
> tree div_cst = wide_int_to_tree (type, div_res); }
> - (mult (plus @1 { div_cst; }) @0
> + (mult (plus @1 { div_cst; }) @0)))
> + /* (A + CST0) * CST1 -> (A + CST0') * CST1, where CST1 is a power of 2
> +constant and CST0' is CST0 with the log2(CST1) MS bits sign-extended. */
> + (simplify
> + (mult (plus:c @0 INTEGER_CST@1) integer_pow2p@2)
> + (with { wide_int c1 = wi::to_wide (@1); }
> + (if (!wi::neg_p (c1))
> + (with {
> + int bits_to_extend = wi::exact_log2 (wi::to_wide (@2));
> + uint rest_bits = tree_to_uhwi (TYPE_SIZE (type)) - bits_to_extend;
> + /* Get the value of the MSB of the rest_bits. */
> + wide_int bitmask = wi::set_bit_in_zero (rest_bits - 1,
> + TYPE_PRECISION (type));
> + wide_int bit_value = wi::lrshift (c1 & bitmask, rest_bits - 1); }
> + /* If the bit is set, extend c1 with its value. */
> + (if (!wi::cmp (bit_value, 1, TYPE_SIGN (type)))
> +(with {
> + c1 |= wi::mask (rest_bits, true, TYPE_PRECISION (type));
> + tree c1_ext = wide_int_to_tree (type, c1); }
> +(mult (plus @0 { c1_ext; }) @2
>
> #if GIMPLE
> /* Canonicalize X + (X << C) into X * (1 + (1 << C)) and
> diff --git a/gcc/testsuite/gcc.target/i386/pr116845.c
> b/gcc/testsuite/gcc.target/i386/pr116845.c
> new file mode 100644
> index 000..230e4d5f8b5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr116845.c
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/116845 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized -m32" } */
> +
> +int foo(int *a, int j)
> +{
> + int k = j - 1;
> + return a[j - 1] == a[k];
> +}
> +
> +int foo2(int *a, int j)
> +{
> + int k = j - 5;
> + return a[j - 5] == a[k];
> +}
> +
> +int bar(int *a, int j)
> +{
> + int k = j - 1;
> + return (&a[j + 1] - 2) == &a[k];
> +}
> +
> +/* { dg-final { scan-tree-dump-times "return 1;" 3 "optimized" } } */
> --
> 2.47.0
>
Re: [PATCH v9] c++: Fix overeager Woverloaded-virtual with conversion operators [PR109918]
On 1/17/25 9:52 AM, Simon Martin wrote: Hi Jason, On 16 Jan 2025, at 22:49, Jason Merrill wrote: On 10/16/24 11:43 AM, Simon Martin wrote: As you know the patch had to be reverted due to PR117114, that highlighted a bunch of issues with comparing DECL_VINDEXes: it might give false positives in case of multiple inheritance (the case in PR117114), but also if there’s single inheritance by the hierarchy has more than two levels (another issue I found while bootstrapping with rust enabled). Yes, relying on DECL_VINDEX equality was wrong, sorry to mislead you. The attached updated patch introduces an overrides_p function, based on the existing check_final_overrider, and uses it when the signatures match. That seems unnecessary. It seems like removing that only breaks Woverloaded-virt11.C, and making that work again only requires bringing back the check that DECL_VINDEX (fndecl) is set (to any value). Or remembering that fndecl was a template, so it can't really have the same signature as a non-template, whatever same_signature_p says. That’s right, only Woverloaded-virt11.C fails without the check_final_overrider call. Thanks for the suggestion to check whether fndecl is a template. This is what the updated attached patch does, successfully tested on x86_64-pc-linux-gnu. OK for GCC 15? And if so, thoughts on backporting to release branches (technically it’s a regression but it’s “just” an incorrect warning fix, so probably not worth the risk)? Right, I wouldn't backport. + if (warn_overloaded_virtual == 1 + && overrider_fndecls.elements () == num_fns) + /* All the fns override a base virtual. */ + continue; This looks like the only use of the overrider_fndecls hash_set. A hash_set seems a bit overkill for checking whether everything in fns is an overrider; keeping track of how many times the old any_override was set should work just as well? + /* fndecls hides base_fndecls[k]. */ + auto_vec &hiders = + hidden_base_fndecls.get_or_insert (base_fndecls[k]); + if (!hiders.contains (fndecl)) + hiders.safe_push (fndecl); Hmm, do you think users want a full list of the overloads that don't override? I'd think the problem is more the overload that doesn't exist rather than the ones that do. The current code ends up in the OVERLOAD handling of dump_decl that just prints scope::name. Jason
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
On Fri, Jan 17, 2025 at 8:02 AM Qing Zhao wrote:
> Joseph, Kees and Bill,
>
> I need your input on this.
> > On Jan 17, 2025, at 10:12, Martin Uecker wrote:
> >
> > Am Donnerstag, dem 16.01.2025 um 21:18 + schrieb Qing Zhao:
> >>
> > ..
> >>
> >> Although in the previous discussion, I agreed with Martin that we should
> >> use the
> >> designator syntax (i.e, counted_by (.n) instead of counted_by (n)) for the
> >> counted_by attribute for pointer fields, after more consideration and
> >> discussion
> >> with Bill Wendling (who is working on the same work for CLANG), we decided
> >> to
> >> keep the current syntax of FAM for pointer fields. And leave the new
> >> syntax (.n)
> >> and more complicate expressions to a later work.
> >>
> > I think this would be a mistake. Once you have established the confusing
> > syntax,
> > it will not easily go away anymore. So I think you should use the
> > unambiguous
> > and future-prove syntax right away. Support for more complicated
> > expressions
> > could be left for later, of course.
>
> Personally I agree with you. -:)
>
> Actually we might need to use such syntax in the very beginning when adding
> counted_by of FAM.
> As I know, linux kernel community asked for the following new feature for
> counted_by of FAM:
>
> struct fs_bulk {
> ...
> int len;
> ...
> }
>
> struct fc_bulk {
> ...
> struct fs_bulk fs_bulk;
> struct fc fcs[] __counted_by(fs_bulk.len);
> };
>
> i.e, the “counted_by” field is in the inner structure of the current
> structure of the FAM.
> With the current syntax, it’s not easy to extend to support this.
>
> But with the designator syntax, it might be much easier to be extended to
> support this.
>
> So, Kees and Bill, what’s your opinion on this? I think that it’s better to
> have a consistent interface between GCC
> and Clang.
>
> Joseph, what’s your opinion on this new syntax? Shall we support the
> designator syntax for counted_by attribute?
>
I've been thinking more about what we discussed regarding this and I
have to agree with Martin that putting off using the new syntax for
both FAMs and pointers will make things worse in the long run. We'll
already have to deprecate the current syntax in compilers for code
using the current syntax, but it may not be too late. Linux is pretty
much the main user of this feature, and we have pretty good control
over how it's used there.
The one wrinkle is that Apple allows for full expressions, not just a
field designator---and they're currently using full expressions.
AFAIK, that code is internal to Apple, but they want to externalize
it. So we'll eventually have to come up with some sort of plan for how
to deal with it probably sooner rather than later. I'll open up a
discussion with them about it.
-bw
[WIP PATCH] Testing hack for better RAW_DATA_CST coverage
Hi! I'd like to revert the r15-6448-gd09628742bb17719360ff447a8e604f5c6801bdf reversion (of course without doing git revert of that because that breaks ChangeLog generation) soon. In order to do that and feel comfortable about that, I've performed an x86_64-linux and i686-linux bootstrap/regtest with that the reversion of that reversion and the following hack so that it triggers far more often. We have right now only around 60 tests for #embed directly and even for the automatic turning of larger comma separated sequences of 0-255 constants into RAW_DATA_CST our testsuite doesn't test that too often. So, the following hack allows to trigger that even for really small sequences (from 4 up, so one INTEGER_CST first, then 2 element RAW_DATA_CST and another INTEGER_CST last). With these patches I got +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++11 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++11 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++14 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++14 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++17 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++17 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++20 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++20 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++23 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++23 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++26 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++26 (test for excess errors) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++98 at line 10 (test for errors, line 5) +FAIL: c-c++-common/cpp/embed-10.c -std=gnu++98 (test for excess errors) +FAIL: g++.dg/cpp2a/class-deduction-aggr11.C -std=c++20 (test for excess errors) +FAIL: g++.dg/cpp2a/class-deduction-aggr11.C -std=c++23 (test for excess errors) +FAIL: g++.dg/cpp2a/class-deduction-aggr11.C -std=c++26 (test for excess errors) +FAIL: g++.dg/cpp2a/explicit1.C(test for errors, line 48) +FAIL: 25_algorithms/fill_n/constrained.cc -std=gnu++20 (test for excess errors) +UNRESOLVED: 25_algorithms/fill_n/constrained.cc -std=gnu++20 compilation failed to produce executable +FAIL: 25_algorithms/fill_n/constrained.cc -std=gnu++26 (test for excess errors) +UNRESOLVED: 25_algorithms/fill_n/constrained.cc -std=gnu++26 compilation failed to produce executable +FAIL: experimental/net/internet/address/v4/cons.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/address/v4/cons.cc -std=gnu++17 compilation failed to produce executable +FAIL: experimental/net/internet/address/v4/creation.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/address/v4/creation.cc -std=gnu++17 compilation failed to produce executable +FAIL: experimental/net/internet/address/v6/members.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/address/v6/members.cc -std=gnu++17 compilation failed to produce executable +FAIL: experimental/net/internet/network/v4/cons.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/network/v4/cons.cc -std=gnu++17 compilation failed to produce executable +FAIL: experimental/net/internet/network/v4/members.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/network/v4/members.cc -std=gnu++17 compilation failed to produce executable +FAIL: experimental/net/internet/network/v6/cons.cc -std=gnu++17 (test for excess errors) +UNRESOLVED: experimental/net/internet/network/v6/cons.cc -std=gnu++17 compilation failed to produce executable +FAIL: std/ranges/adaptors/all.cc -std=gnu++20 (test for excess errors) +UNRESOLVED: std/ranges/adaptors/all.cc -std=gnu++20 compilation failed to produce executable +FAIL: std/ranges/adaptors/all.cc -std=gnu++26 (test for excess errors) +UNRESOLVED: std/ranges/adaptors/all.cc -std=gnu++26 compilation failed to produce executable +FAIL: std/ranges/zip_transform/1.cc -std=gnu++23 (test for excess errors) +UNRESOLVED: std/ranges/zip_transform/1.cc -std=gnu++23 compilation failed to produce executable +FAIL: std/ranges/zip_transform/1.cc -std=gnu++26 (test for excess errors) +UNRESOLVED: std/ranges/zip_transform/1.cc -std=gnu++26 compilation failed to produce executable regressions and filed from that PR118528, PR118532 and PR118534 and will post fixes for that momentarily. Posting this patch mainly for archival purposes and so that I can refer to it in the patches. --- gcc/c/c-typeck.cc.jj2024-12-19 21:46:13.585116197 +0100 +++ gcc/c/c-typeck.cc 2024-12-31 11:50:32.514734084 +0100 @@ -11690,7 +11690,7 @@ c_maybe_optimize_large_byte_initializer widest_int w = wi::to_wid
[PATCH] c, c++: Return 1 for __has_builtin(__builtin_va_arg) and __has_builtin(__builtin_c23_va_start)
Hi!
The Linux kernel uses its own copy of stdarg.h.
Now, before GCC 15, our stdarg.h had
#if defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L
#define va_start(v, ...)__builtin_va_start(v, 0)
#else
#define va_start(v,l) __builtin_va_start(v,l)
#endif
va_start definition but GCC 15 has:
#if defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L
#define va_start(...) __builtin_c23_va_start(__VA_ARGS__)
#else
#define va_start(v,l) __builtin_va_start(v,l)
#endif
I wanted to suggest to the kernel people during their porting to C23
that they'd better use C23 compatible va_start macro definition,
but to make it portable, I think they really want something like
#if defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L
#define va_start(v, ...)__builtin_va_start(v, 0)
#ifdef __has_builtin
#if __has_builtin(__builtin_c23_va_start)
#undef va_start
#define va_start(...) __builtin_c23_va_start(__VA_ARGS__)
#endif
#else
#define va_start(v,l) __builtin_va_start(v,l)
#endif
or so (or with >= 202311L), as GCC 13-14 and clang don't support
__builtin_c23_va_start (yet?) and one gets better user experience with
that.
Except it seems __has_builtin(__builtin_c23_va_start) doesn't actually work,
it works for most of the stdarg.h __builtin_va_*, doesn't work for
__builtin_va_arg (neither C nor C++) and didn't work for
__builtin_c23_va_start if it was available.
The following patch wires __has_builtin for those.
Bootstrapped on x86_64-linux and i686-linux, regtest ongoing, ok for trunk
if it passes?
2025-01-17 Jakub Jelinek
gcc/c/
* c-decl.cc (names_builtin_p): Return 1 for RID_C23_VA_START and
RID_VA_ARG.
gcc/cp/
* cp-objcp-common.cc (names_builtin_p): Return 1 for RID_VA_ARG.
gcc/testsuite/
* c-c++-common/cpp/has-builtin-4.c: New test.
--- gcc/c/c-decl.cc.jj 2025-01-17 11:29:33.276694451 +0100
+++ gcc/c/c-decl.cc 2025-01-17 15:13:44.497856864 +0100
@@ -11804,6 +11804,8 @@ names_builtin_p (const char *name)
case RID_CHOOSE_EXPR:
case RID_OFFSETOF:
case RID_TYPES_COMPATIBLE_P:
+case RID_C23_VA_START:
+case RID_VA_ARG:
return 1;
default:
break;
--- gcc/cp/cp-objcp-common.cc.jj2025-01-02 11:47:10.703494721 +0100
+++ gcc/cp/cp-objcp-common.cc 2025-01-17 15:16:07.218885238 +0100
@@ -587,6 +587,7 @@ names_builtin_p (const char *name)
case RID_BUILTIN_ASSOC_BARRIER:
case RID_BUILTIN_BIT_CAST:
case RID_OFFSETOF:
+case RID_VA_ARG:
return 1;
case RID_BUILTIN_OPERATOR_NEW:
case RID_BUILTIN_OPERATOR_DELETE:
--- gcc/testsuite/c-c++-common/cpp/has-builtin-4.c.jj 2025-01-17
15:15:44.077204928 +0100
+++ gcc/testsuite/c-c++-common/cpp/has-builtin-4.c 2025-01-17
15:15:02.520779011 +0100
@@ -0,0 +1,16 @@
+/* { dg-do preprocess } */
+
+#if __has_builtin (__builtin_va_start) != 1
+#error "No __builtin_va_start"
+#endif
+#if __has_builtin (__builtin_va_end) != 1
+#error "No __builtin_va_end"
+#endif
+#if __has_builtin (__builtin_va_arg) != 1
+#error "no __builtin_va_arg"
+#endif
+#if __STDC_VERSION__ >= 202311L
+#if __has_builtin (__builtin_c23_va_start) != 1
+#error "no __builtin_c23_va_start"
+#endif
+#endif
Jakub
Re: [PATCH] c++: Handle RAW_DATA_CST in make_tree_vector_from_ctor [PR118528]
On 1/17/25 3:08 PM, Jakub Jelinek wrote:
Hi!
This is the first bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-21.C FAILed
since introduction of optimized #embed support and the other when
optimizing large C++ initializers using RAW_DATA_CST.
The problem is that the C++ FE calls make_tree_vector_from_ctor
and uses that as arguments vector for deduction guide handling.
The call.cc code isn't prepared to handle RAW_DATA_CST just about
everywhere, so I think it is safer to make sure RAW_DATA_CST only
appears in CONSTRUCTOR_ELTS and nowhere else.
Thus, the following patch expands the RAW_DATA_CSTs from initializers
into multiple INTEGER_CSTs in the returned vector.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
2025-01-17 Jakub Jelinek
PR c++/118528
* c-common.cc (make_tree_vector_from_ctor): Expand RAW_DATA_CST
elements from the CONSTRUCTOR to individual INTEGER_CSTs.
* g++.dg/cpp/embed-21.C: New test.
* g++.dg/cpp2a/class-deduction-aggr16.C: New test.
--- gcc/c-family/c-common.cc.jj 2025-01-02 11:47:29.803228077 +0100
+++ gcc/c-family/c-common.cc2025-01-17 13:32:53.512294482 +0100
@@ -9016,9 +9016,26 @@ vec *
make_tree_vector_from_ctor (tree ctor)
{
vec *ret = make_tree_vector ();
+ unsigned nelts = CONSTRUCTOR_NELTS (ctor);
vec_safe_reserve (ret, CONSTRUCTOR_NELTS (ctor));
for (unsigned i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
-ret->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
+if (TREE_CODE (CONSTRUCTOR_ELT (ctor, i)->value) == RAW_DATA_CST)
+ {
+ tree raw_data = CONSTRUCTOR_ELT (ctor, i)->value;
+ nelts += RAW_DATA_LENGTH (raw_data);
+ vec_safe_reserve (ret, nelts);
I notice that the second argument to vec_safe_reserve is the number of
empty spaces to provide, rather than the new desired capacity. So this
will allocate space for an additional copy of the whole constructor.
I guess you want to say reserve (ret, nelts - vec_safe_length (res))?
It's weird that there isn't a more convenient way to express that.
Note that this behavior differs from the C++ standard library, where
reserve indeed specifies the new capacity. This seems likely to cause a
lot of confusion (and over-allocation) going forward. And indeed
looking through exising uses turns up at least two that seem to assume
the standard library semantics (as well as others that assume the actual
semantics).
I wonder about renaming the existing reserve to reserve_space and adding
a reserve_capacity?
Jason
[r15-7008 Regression] FAIL: gcc.target/i386/pr118067-2.c (test for excess errors) on Linux/x86_64
On Linux/x86_64,
9f009e8865cda01310c52f7ec8bdaa3c557a2745 is the first bad commit
commit 9f009e8865cda01310c52f7ec8bdaa3c557a2745
Author: Vladimir N. Makarov
Date: Fri Jan 17 15:56:29 2025 -0500
[PR118067][LRA]: Check secondary memory mode for the reg class
caused
FAIL: gcc.target/i386/pr118067-2.c (test for excess errors)
with GCC configured with
../../gcc/configure
--prefix=/export/users/haochenj/src/gcc-bisect/master/master/r15-7008/usr
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl
--enable-libmpx x86_64-linux --disable-bootstrap
To reproduce:
$ cd {build_dir}/gcc && make check
RUNTESTFLAGS="i386.exp=gcc.target/i386/pr118067-2.c --target_board='unix{-m64}'"
$ cd {build_dir}/gcc && make check
RUNTESTFLAGS="i386.exp=gcc.target/i386/pr118067-2.c --target_board='unix{-m64\
-march=cascadelake}'"
(Please do not reply to this email, for question about this report, contact me
at haochen dot jiang at intel.com.)
(If you met problems with cascadelake related, disabling AVX512F in command
line might save that.)
(However, please make sure that there is no potential problems with AVX512.)
Re: [PATCH] c++: Fix up find_array_ctor_elt RAW_DATA_CST handling [PR118534]
On 1/17/25 3:21 PM, Jakub Jelinek wrote:
Hi!
This is the third bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-24.C FAILed
since introduction of optimized #embed support and the others when
optimizing large C++ initializers using RAW_DATA_CST.
find_array_ctor_elt already has RAW_DATA_CST support, but on the
following testcases it misses one case I've missed.
The CONSTRUCTORs in question went through the braced_list_to_string
optimization which can turn INTEGER_CST RAW_DATA_CST INTEGER_CST
into just larger RAW_DATA_CST covering even those 2 bytes around it
(if they appear there in the underlying RAW_DATA_OWNER).
With this optimization, RAW_DATA_CST can be the last CONSTRUCTOR_ELTS
elt in a CONSTRUCTOR, either the sole one or say preceeded by some
unrelated other elements. Now, if RAW_DATA_CST is the only one or
if there are no RAW_DATA_CSTs earlier in CONSTRUCTOR_ELTS, we can
trigger a bug in find_array_ctor_elt.
It has a smart optimization for the very common case where
CONSTRUCTOR_ELTS have indexes and index of the last elt is equal
to CONSTRUCTOR_NELTS (ary) - 1, then obviously we know there are
no RAW_DATA_CSTs before it and the indexes just go from 0 to nelts-1,
so when we care about any of those earlier indexes, we can just return i;
and not worry about anything.
Except it uses if (i < end) return i; rather than if (i < end - 1) return i;
For the latter cases, i.e. anything before the last elt, we know there
are no surprises and return i; is right. But for the if (i == end - 1)
case, return i; is only correct if the last elt is not RAW_DATA_CST, if it
is RAW_DATA_CST, we still need to split it, which is handled by the code
later in the function. So, for that we need begin = end - 1, so that the
binary search will just care about that last element.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
OK.
2025-01-17 Jakub Jelinek
PR c++/118534
* constexpr.cc (find_array_ctor_elt): Don't return i early if
i == end - 1 and the last elt's value is RAW_DATA_CST.
* g++.dg/cpp/embed-24.C: New test.
* g++.dg/cpp1y/pr118534.C: New test.
--- gcc/cp/constexpr.cc.jj 2025-01-17 11:29:33.507691199 +0100
+++ gcc/cp/constexpr.cc 2025-01-17 19:07:36.422646176 +0100
@@ -4155,12 +4155,17 @@ find_array_ctor_elt (tree ary, tree dind
else if (TREE_CODE (cindex) == INTEGER_CST
&& compare_tree_int (cindex, end - 1) == 0)
{
- if (i < end)
- return i;
tree value = (*elts)[end - 1].value;
- if (TREE_CODE (value) == RAW_DATA_CST
- && wi::to_offset (dindex) < (wi::to_offset (cindex)
- + RAW_DATA_LENGTH (value)))
+ if (i < end)
+ {
+ if (i == end - 1 && TREE_CODE (value) == RAW_DATA_CST)
+ begin = end - 1;
+ else
+ return i;
+ }
+ else if (TREE_CODE (value) == RAW_DATA_CST
+ && wi::to_offset (dindex) < (wi::to_offset (cindex)
+ + RAW_DATA_LENGTH (value)))
begin = end - 1;
else
begin = end;
--- gcc/testsuite/g++.dg/cpp/embed-24.C.jj 2025-01-17 17:55:56.127894446
+0100
+++ gcc/testsuite/g++.dg/cpp/embed-24.C 2025-01-17 19:15:39.268992556 +0100
@@ -0,0 +1,30 @@
+// PR c++/118534
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+template
+constexpr bool
+foo ()
+{
+ T x[160] = {
+#embed __FILE__ limit (160)
+ };
+ const int y[160] = {
+42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
+#embed __FILE__ limit (147) gnu::offset (13)
+ };
+ unsigned long n = 13;
+ for (T *p = x; n; --n, p++)
+*p = 42;
+ for (int i = 0; i < 160; ++i)
+if (x[i] != y[i])
+ return false;
+ return true;
+}
+
+int
+main ()
+{
+ static_assert (foo (), "");
+ static_assert (foo (), "");
+}
--- gcc/testsuite/g++.dg/cpp1y/pr118534.C.jj2025-01-17 18:28:21.492097114
+0100
+++ gcc/testsuite/g++.dg/cpp1y/pr118534.C 2025-01-17 19:16:05.362632984
+0100
@@ -0,0 +1,31 @@
+// PR c++/118534
+// { dg-do compile { target c++14 } }
+
+template
+constexpr bool
+foo ()
+{
+ T x[160] = {
+#define I8 1, 2, 3, 4, 5, 6, 7, 8
+#define I64 I8, I8, I8, I8, I8, I8, I8, I8
+I64, I64, I8, I8, I8, I8
+ };
+ const int y[160] = {
+42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 6, 7, 8,
+I64, I64, I8, I8
+ };
+ unsigned long n = 13;
+ for (T *p = x; n; --n, p++)
+*p = 42;
+ for (int i = 0; i < 160; ++i)
+if (x[i] != y[i])
+ return false;
+ return true;
+}
+
+int
+main ()
+{
+ static_assert (foo (), "");
+ static_assert (foo (), "");
+}
Jakub
[PATCH] [ifcombine] avoid dropping tree_could_trap_p [PR118514]
Unlike other access patterns, BIT_FIELD_REFs aren't regarded as
possibly-trapping out of referencing out-of-bounds bits.
So, if decode_field_reference finds a load that could trap, but whose
inner object can't, bail out if it accesses past the inner object's
size.
This may drop some ifcombine optimizations in VLAs, that could be safe
if we managed to reuse the same pre-existing load for both compares, but
those get optimized later (as in the new testcase). The cases of most
interest are those that merge separate fields, and they necessarily
involve new BIT_FIELD_REFs loads.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
PR tree-optimization/118514
* gimple-fold.cc (decode_field_reference): Refuse to consider
BIT_FIELD_REF of non-trapping object if the original load
could trap for being out-of-bounds.
(make_bit_field_ref): Check that replacement loads could trap
as much as the original loads.
for gcc/testsuite/ChangeLog
PR tree-optimization/118514
* gcc.dg/field-merge-23.c: New.
---
gcc/gimple-fold.cc| 19 +++
gcc/testsuite/gcc.dg/field-merge-23.c | 19 +++
2 files changed, 34 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/field-merge-23.c
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 3c971a29ef045..41b21ecd58a32 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -7686,10 +7686,16 @@ decode_field_reference (tree *pexp, HOST_WIDE_INT
*pbitsize,
|| bs <= shiftrt
|| offset != 0
|| TREE_CODE (inner) == PLACEHOLDER_EXPR
- /* Reject out-of-bound accesses (PR79731). */
- || (! AGGREGATE_TYPE_P (TREE_TYPE (inner))
- && compare_tree_int (TYPE_SIZE (TREE_TYPE (inner)),
- bp + bs) < 0)
+ /* Reject out-of-bound accesses (PR79731). BIT_FIELD_REFs aren't flagged
+as tree_could_trap_p just because of out-of-range bits, so don't even
+try to optimize loads that could trap if they access out-of-range bits
+of an object that could not trap (PR118514). */
+ || ((! AGGREGATE_TYPE_P (TREE_TYPE (inner))
+ || (load && tree_could_trap_p (gimple_assign_rhs1 (load))
+ & !tree_could_trap_p (inner)))
+ && (!tree_fits_uhwi_p (TYPE_SIZE (TREE_TYPE (inner)))
+ || compare_tree_int (TYPE_SIZE (TREE_TYPE (inner)),
+ bp + bs) < 0))
|| (INTEGRAL_TYPE_P (TREE_TYPE (inner))
&& !type_has_mode_precision_p (TREE_TYPE (inner
return NULL_TREE;
@@ -7859,6 +7865,11 @@ make_bit_field_load (location_t loc, tree inner, tree
orig_inner, tree type,
gimple *new_stmt = gsi_stmt (i);
if (gimple_has_mem_ops (new_stmt))
gimple_set_vuse (new_stmt, reaching_vuse);
+ gcc_checking_assert (! (gimple_assign_load_p (point)
+ && gimple_assign_load_p (new_stmt))
+ || (tree_could_trap_p (gimple_assign_rhs1 (point))
+ == tree_could_trap_p (gimple_assign_rhs1
+(new_stmt;
}
gimple_stmt_iterator gsi = gsi_for_stmt (point);
diff --git a/gcc/testsuite/gcc.dg/field-merge-23.c
b/gcc/testsuite/gcc.dg/field-merge-23.c
new file mode 100644
index 0..96604d43c9dec
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/field-merge-23.c
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+/* { dg-options "-O" } */
+
+/* PR tree-optimization/118514 */
+
+/* Check that we don't create BIT_FIELD_REFs that could trap, because they are
+ assumed not to trap and could be pulled out of loops. */
+
+int a, c = 1;
+unsigned char b[1], d;
+int main() {
+ while (a || !c) {
+signed char e = b[10];
+d = e < 0 || b[10] > 1;
+if (d)
+ __builtin_abort ();
+ }
+ return 0;
+}
--
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] libfortran: G formatting for UNSIGNED [PR118536]
Dear all,
the attached obvious patch extends G formatting to UNSIGNED by reusing
the code for I formatting.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 5ba7e37a089257dc40e9f347a835a481121a3f3f Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Fri, 17 Jan 2025 21:20:31 +0100
Subject: [PATCH] libfortran: G formatting for UNSIGNED [PR118536]
PR libfortran/118536
libgfortran/ChangeLog:
* io/transfer.c (formatted_transfer_scalar_write): Handle UNSIGNED
in G formatting.
gcc/testsuite/ChangeLog:
* gfortran.dg/unsigned_write_2.f90: New test.
---
.../gfortran.dg/unsigned_write_2.f90 | 30 +++
libgfortran/io/transfer.c | 3 ++
2 files changed, 33 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/unsigned_write_2.f90
diff --git a/gcc/testsuite/gfortran.dg/unsigned_write_2.f90 b/gcc/testsuite/gfortran.dg/unsigned_write_2.f90
new file mode 100644
index 000..091e9b99f10
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unsigned_write_2.f90
@@ -0,0 +1,30 @@
+! { dg-do run }
+! This is a libgfortran (runtime library) test, need to run only once!
+!
+! { dg-additional-options "-funsigned" }
+!
+! PR libfortran/118536 - G formatting for UNSIGNED
+
+program print_unsigned_g_formatted
+ character(21) :: s1, s2
+ unsigned(1) :: u1 = huge(0U_1)
+ unsigned(2) :: u2 = huge(0U_2)
+ unsigned(4) :: u4 = huge(0U_4)
+ unsigned(8) :: u8 = huge(0U_8)
+
+ write(s1,'(i0)') u1
+ write(s2,'(g0)') u1
+ if (s1 /= s2) stop 1
+
+ write(s1,'(i0)') u2
+ write(s2,'(g0)') u2
+ if (s1 /= s2) stop 2
+
+ write(s1,'(i0)') u4
+ write(s2,'(g0)') u4
+ if (s1 /= s2) stop 3
+
+ write(s1,'(i0)') u8
+ write(s2,'(g0)') u8
+ if (s1 /= s2) stop 4
+end
diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c
index 0177e052062..b3b72f39c5b 100644
--- a/libgfortran/io/transfer.c
+++ b/libgfortran/io/transfer.c
@@ -2365,6 +2365,9 @@ formatted_transfer_scalar_write (st_parameter_dt *dtp, bt type, void *p, int kin
case BT_INTEGER:
write_i (dtp, f, p, kind);
break;
+ case BT_UNSIGNED:
+ write_iu (dtp, f, p, kind);
+ break;
case BT_LOGICAL:
write_l (dtp, f, p, kind);
break;
--
2.43.0
Re: [PATCH] libfortran: G formatting for UNSIGNED [PR118536]
Looks Ok by me. Thanks, Jerry On Fri, Jan 17, 2025, 12:25 PM Harald Anlauf wrote: > Dear all, > > the attached obvious patch extends G formatting to UNSIGNED by reusing > the code for I formatting. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > > Thanks, > Harald > >
Re: [PATCH,LRA] Restrict the reuse of spill slots [PR117868]
On 1/11/25 1:15 PM, Denis Chertykov wrote:
The fix for PR117868.
In brief:
this is an LRA bug derived from reuse spilling slots after frame
pointer spilling.
The slot was created for QImode (1 byte) and it was reused after
spilling of the
frame pointer for TImode register (16 bytes long) and it overlaps
other slots.
Denis, thank you for working on the PR, finding the reason for wrong
code generation, and proposing the patch.
Ok for trunk ?
I'd prefer something like the patch in the attachment.
It is simpler and even removing more LRA code than adding one.
But most important, it generates smaller reserved stack space as QI and
TI pseudos will share the same slot (of TImode) vs 2 slots QI and TI in
your fix.
I'll test my patch and submit it on the next week.
PR rtl-optimization/117868
gcc/
* lra-spills.cc (assign_stack_slot_num_and_sort_pseudos): Reuse slots
only without allocated memory or only with equal or smaller registers
with equal or smaller alignment.
(lra_spill): Print slot size as width.
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index db78dcd28a3..93a0c92db9f 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -386,7 +386,18 @@ assign_stack_slot_num_and_sort_pseudos (int
*pseudo_regnos, int n)
&& ! (lra_intersected_live_ranges_p
(slots[j].live_ranges,
lra_reg_info[regno].live_ranges)))
- break;
+ {
+ /* A slot without allocated memory can be shared. */
+ if (slots[j].mem == NULL_RTX)
+ break;
+
+ /* A slot with allocated memory can be shared only with equal
+ or smaller register with equal or smaller alignment. */
+ if (slots[j].align >= spill_slot_alignment (mode)
+ && compare_sizes_for_sort (slots[j].size,
+ GET_MODE_SIZE (mode)) != -1)
+ break;
+ }
}
if (j >= slots_num)
{
@@ -656,8 +667,7 @@ lra_spill (void)
for (i = 0; i < slots_num; i++)
{
fprintf (lra_dump_file, " Slot %d regnos (width = ", i);
- print_dec (GET_MODE_SIZE (GET_MODE (slots[i].mem)),
- lra_dump_file, SIGNED);
+ print_dec (slots[i].size, lra_dump_file, SIGNED);
fprintf (lra_dump_file, "):");
for (curr_regno = slots[i].regno;;
curr_regno = pseudo_slots[curr_regno].next - pseudo_slots)
diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
index db78dcd28a3..e0d88e86c18 100644
--- a/gcc/lra-spills.cc
+++ b/gcc/lra-spills.cc
@@ -635,17 +635,12 @@ lra_spill (void)
n = assign_spill_hard_regs (pseudo_regnos, n);
slots_num = 0;
assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n);
- for (i = 0; i < n; i++)
+ if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos + n)) > 0)
+/* Assign stack slots to spilled pseudos assigned to fp. */
+assign_stack_slot_num_and_sort_pseudos (pseudo_regnos + n, n2);
+ for (i = 0; i < n + n2; i++)
if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
assign_mem_slot (pseudo_regnos[i]);
- if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos)) > 0)
-{
- /* Assign stack slots to spilled pseudos assigned to fp. */
- assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n2);
- for (i = 0; i < n2; i++)
- if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
- assign_mem_slot (pseudo_regnos[i]);
-}
if (n + n2 > 0 && crtl->stack_alignment_needed)
/* If we have a stack frame, we must align it now. The stack size
may be a part of the offset computation for register
Re: [PATCH] c, c++: Return 1 for __has_builtin(__builtin_va_arg) and __has_builtin(__builtin_c23_va_start)
On Fri, 17 Jan 2025, Jakub Jelinek wrote: > Bootstrapped on x86_64-linux and i686-linux, regtest ongoing, ok for trunk > if it passes? > > 2025-01-17 Jakub Jelinek > > gcc/c/ > * c-decl.cc (names_builtin_p): Return 1 for RID_C23_VA_START and > RID_VA_ARG. > gcc/cp/ > * cp-objcp-common.cc (names_builtin_p): Return 1 for RID_VA_ARG. > gcc/testsuite/ > * c-c++-common/cpp/has-builtin-4.c: New test. OK in the absence of C++ maintainer objections within 72 hours. -- Joseph S. Myers josmy...@redhat.com
Re: [PATCH 3/4] RISC-V: Add .note.gnu.property for ZICFILP and ZICFISS ISA extension
Thanks, I will fix it.
> Mark Wielaard 於 2025年1月17日 晚上10:32 寫道：
>
> Hi Monk,
>
>> On Fri, Nov 15, 2024 at 06:53:09PM +0800, Monk Chiang wrote:
>> gcc/ChangeLog:
>>* gcc/config/riscv/riscv.cc
>>(riscv_file_end_indicate_exec_stack): Add .note.gnu.property.
>>* gcc/config/riscv/linux.h (TARGET_ASM_FILE_END): Define.
>>
>> [...]
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 8201a965ed1..bda982f085c 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -10195,6 +10195,56 @@ riscv_file_start (void)
>> riscv_emit_attribute ();
>> }
>>
>> +void
>> +riscv_file_end_indicate_exec_stack ()
>> +{
>> + file_end_indicate_exec_stack ();
>> + long GNU_PROPERTY_RISCV_FEATURE_1_AND = 0;
>
> This broke the riscv bootstrap because
> GNU_PROPERTY_RISCV_FEATURE_1_AND is never used.
>
> ../../gcc/gcc/config/riscv/riscv.cc:10340:8: error: unused variable
> ‘GNU_PROPERTY_RISCV_FEATURE_1_AND’ [-Werror=unused-variable]
> 10340 | long GNU_PROPERTY_RISCV_FEATURE_1_AND = 0;
> |^~~~
>
> See https://builder.sourceware.org/buildbot/#/builders/310/builds/863
>
> Could you fix that?
>
> Thanks,
>
> Mark
Re: [PATCH,LRA] Restrict the reuse of spill slots [PR117868]
Vladimir Makarov writes:
> On 1/11/25 1:15 PM, Denis Chertykov wrote:
>> The fix for PR117868.
>>
>> In brief:
>> this is an LRA bug derived from reuse spilling slots after frame
>> pointer spilling.
>> The slot was created for QImode (1 byte) and it was reused after
>> spilling of the
>> frame pointer for TImode register (16 bytes long) and it overlaps
>> other slots.
>>
> Denis, thank you for working on the PR, finding the reason for wrong
> code generation, and proposing the patch.
>>
>> Ok for trunk ?
>>
>>
> I'd prefer something like the patch in the attachment.
>
> It is simpler and even removing more LRA code than adding one.
>
> But most important, it generates smaller reserved stack space as QI
> and TI pseudos will share the same slot (of TImode) vs 2 slots QI and
> TI in your fix.
Your patch seems wrong to me.
The original code:
--part of lra-spill.cc ---
slots_num = 0;
1 assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n);
1 for (i = 0; i < n; i++)
1if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
1 assign_mem_slot (pseudo_regnos[i]);
- if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos)) > 0)
{
/* Assign stack slots to spilled pseudos assigned to fp. */
2 assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n2);
2 for (i = 0; i < n2; i++)
2 if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
2 assign_mem_slot (pseudo_regnos[i]);
}
--
The original code consists of two passes of the stack allocation process
separated
by call `lra_update_fp2sp_elimination()'.
It's right because real allocation is made by `assign_mem_slot()' and
the AVR port used frame size as a trigger for frame pointer elimination.
(If we have a frame then we can't eliminate frame pointer)
The first pass allocate stack for spilled registers
after that LRA asks target about possibility of fp2sp elimination
(call `lra_update_fp2sp_elimination()')
and after that LRA allocates a memory slots for registers spilled in the
process of
spilling hard frame pointer.
In your patch call of `lra_update_fp2sp_elimination()' happened before
real stack allocation that will be done by `assign_mem_slot()' and information
about fp2sp elimination will be wrong.
Denis.
>
> I'll test my patch and submit it on the next week.
>
>>
>> PR rtl-optimization/117868
>> gcc/
>> * lra-spills.cc (assign_stack_slot_num_and_sort_pseudos): Reuse slots
>> only without allocated memory or only with equal or smaller registers
>> with equal or smaller alignment.
>> (lra_spill): Print slot size as width.
>>
>>
>> diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
>> index db78dcd28a3..93a0c92db9f 100644
>> --- a/gcc/lra-spills.cc
>> +++ b/gcc/lra-spills.cc
>> @@ -386,7 +386,18 @@ assign_stack_slot_num_and_sort_pseudos (int
>> *pseudo_regnos, int n)
>> && ! (lra_intersected_live_ranges_p
>> (slots[j].live_ranges,
>> lra_reg_info[regno].live_ranges)))
>> - break;
>> + {
>> + /* A slot without allocated memory can be shared. */
>> + if (slots[j].mem == NULL_RTX)
>> + break;
>> +
>> + /* A slot with allocated memory can be shared only with equal
>> + or smaller register with equal or smaller alignment. */
>> + if (slots[j].align >= spill_slot_alignment (mode)
>> + && compare_sizes_for_sort (slots[j].size,
>> + GET_MODE_SIZE (mode)) != -1)
>> + break;
>> + }
>> }
>> if (j >= slots_num)
>> {
>> @@ -656,8 +667,7 @@ lra_spill (void)
>> for (i = 0; i < slots_num; i++)
>> {
>> fprintf (lra_dump_file, " Slot %d regnos (width = ", i);
>> - print_dec (GET_MODE_SIZE (GET_MODE (slots[i].mem)),
>> - lra_dump_file, SIGNED);
>> + print_dec (slots[i].size, lra_dump_file, SIGNED);
>> fprintf (lra_dump_file, "):");
>> for (curr_regno = slots[i].regno;;
>> curr_regno = pseudo_slots[curr_regno].next - pseudo_slots)
>>
>
> diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc
> index db78dcd28a3..e0d88e86c18 100644
> --- a/gcc/lra-spills.cc
> +++ b/gcc/lra-spills.cc
> @@ -635,17 +635,12 @@ lra_spill (void)
>n = assign_spill_hard_regs (pseudo_regnos, n);
>slots_num = 0;
>assign_stack_slot_num_and_sort_pseudos (pseudo_regnos, n);
> - for (i = 0; i < n; i++)
> + if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos + n)) > 0)
> +/* Assign stack slots to spilled pseudos assigned to fp. */
> +assign_stack_slot_num_and_sort_pseudos (pseudo_regnos + n, n2);
> + for (i = 0; i < n + n2; i++)
> if (pseudo_slots[pseudo_regnos[i]].mem == NULL_RTX)
>assign_mem_slot (pseudo_regnos[i]);
> - if ((n2 = lra_update_fp2sp_elimination (pseudo_regnos)) > 0)
> -{
> - /* Assign stack slots to spilled pseudos assigned to fp.
[PATCH] c++: Handle RAW_DATA_CST in make_tree_vector_from_ctor [PR118528]
Hi!
This is the first bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-21.C FAILed
since introduction of optimized #embed support and the other when
optimizing large C++ initializers using RAW_DATA_CST.
The problem is that the C++ FE calls make_tree_vector_from_ctor
and uses that as arguments vector for deduction guide handling.
The call.cc code isn't prepared to handle RAW_DATA_CST just about
everywhere, so I think it is safer to make sure RAW_DATA_CST only
appears in CONSTRUCTOR_ELTS and nowhere else.
Thus, the following patch expands the RAW_DATA_CSTs from initializers
into multiple INTEGER_CSTs in the returned vector.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
2025-01-17 Jakub Jelinek
PR c++/118528
* c-common.cc (make_tree_vector_from_ctor): Expand RAW_DATA_CST
elements from the CONSTRUCTOR to individual INTEGER_CSTs.
* g++.dg/cpp/embed-21.C: New test.
* g++.dg/cpp2a/class-deduction-aggr16.C: New test.
--- gcc/c-family/c-common.cc.jj 2025-01-02 11:47:29.803228077 +0100
+++ gcc/c-family/c-common.cc2025-01-17 13:32:53.512294482 +0100
@@ -9016,9 +9016,26 @@ vec *
make_tree_vector_from_ctor (tree ctor)
{
vec *ret = make_tree_vector ();
+ unsigned nelts = CONSTRUCTOR_NELTS (ctor);
vec_safe_reserve (ret, CONSTRUCTOR_NELTS (ctor));
for (unsigned i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
-ret->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
+if (TREE_CODE (CONSTRUCTOR_ELT (ctor, i)->value) == RAW_DATA_CST)
+ {
+ tree raw_data = CONSTRUCTOR_ELT (ctor, i)->value;
+ nelts += RAW_DATA_LENGTH (raw_data);
+ vec_safe_reserve (ret, nelts);
+ if (TYPE_PRECISION (TREE_TYPE (raw_data)) > CHAR_BIT
+ || TYPE_UNSIGNED (TREE_TYPE (raw_data)))
+ for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
+ ret->quick_push (build_int_cst (TREE_TYPE (raw_data),
+ RAW_DATA_UCHAR_ELT (raw_data, j)));
+ else
+ for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
+ ret->quick_push (build_int_cst (TREE_TYPE (raw_data),
+ RAW_DATA_SCHAR_ELT (raw_data, j)));
+ }
+else
+ ret->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
return ret;
}
--- gcc/testsuite/g++.dg/cpp/embed-21.C.jj 2025-01-17 12:18:00.571912195
+0100
+++ gcc/testsuite/g++.dg/cpp/embed-21.C 2025-01-17 13:37:31.478489868 +0100
@@ -0,0 +1,22 @@
+// PR c++/118528
+// { dg-do compile { target c++20 } }
+// { dg-options "" }
+
+template
+struct E { T t[130][2]; };
+
+E e1 {
+#embed __FILE__ limit (260)
+};
+
+template
+struct F { T t[2][130]; };
+
+F f1 {
+#embed __FILE__ limit (260)
+};
+F f2 { { {
+#embed __FILE__ limit (130)
+}, {
+#embed __FILE__ limit (130)
+} } };
--- gcc/testsuite/g++.dg/cpp2a/class-deduction-aggr16.C.jj 2025-01-17
12:17:23.715424611 +0100
+++ gcc/testsuite/g++.dg/cpp2a/class-deduction-aggr16.C 2025-01-17
12:17:43.717146527 +0100
@@ -0,0 +1,17 @@
+// PR c++/118528
+// { dg-do compile { target c++20 } }
+
+template
+struct E { T t[130][2]; };
+
+#define P 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16
+#define Q { 1, 2 }, { 3, 4 }, { 5, 6 }, { 7, 8 }, { 9, 10 }, { 11, 12 }, \
+ { 13, 14 }, { 15, 16 }
+E e1 { P, P, P, P, P, P, P, P, P, P, P, P, P, P, P, P, 1, 2, 3, 4 };
+E e2 { { Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, Q, { 1, 2 }, { 3, 4 } }
};
+
+template
+struct F { T t[2][130]; };
+
+F f1 { P, P, P, P, P, P, P, P, P, P, P, P, P, P, P, P, 1, 2, 3, 4 };
+F f2 { { { P, P, P, P, P, P, P, P, 1, 2 }, { P, P, P, P, P, P, P, P, 3, 4 } }
};
Jakub
[PATCH] c++: Handle RAW_DATA_CST in add_list_candidates [PR118532]
Hi!
This is the second bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-2[23].C FAILed
since introduction of optimized #embed support and the others when
optimizing large C++ initializers using RAW_DATA_CST.
The add_list_candidates problem is the same as with
make_tree_vector_from_ctor, unfortunately it can't call that
function because it can have those additional artificial arguments
that need to be pushed earlier.
When working on the patch, I've also noticed an error where we didn't
know how to dump RAW_DATA_CST, so I've added support for that too.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
2025-01-17 Jakub Jelinek
PR c++/118532
* call.cc (add_list_candidates): Handle RAW_DATA_CST among init_list
elts.
* error.cc (dump_expr_init_vec): Handle RAW_DATA_CST among v elts.
* g++.dg/cpp/embed-22.C: New test.
* g++.dg/cpp/embed-23.C: New test.
* g++.dg/cpp0x/pr118532.C: New test.
* g++.dg/cpp2a/explicit20.C: New test.
--- gcc/cp/call.cc.jj 2025-01-15 18:24:36.135503866 +0100
+++ gcc/cp/call.cc 2025-01-17 14:42:38.201643385 +0100
@@ -4258,11 +4258,30 @@ add_list_candidates (tree fns, tree firs
/* Expand the CONSTRUCTOR into a new argument vec. */
vec *new_args;
- vec_alloc (new_args, nart + CONSTRUCTOR_NELTS (init_list));
+ unsigned nelts = nart + CONSTRUCTOR_NELTS (init_list);
+ vec_alloc (new_args, nelts);
for (unsigned i = 0; i < nart; ++i)
new_args->quick_push ((*args)[i]);
for (unsigned i = 0; i < CONSTRUCTOR_NELTS (init_list); ++i)
-new_args->quick_push (CONSTRUCTOR_ELT (init_list, i)->value);
+if (TREE_CODE (CONSTRUCTOR_ELT (init_list, i)->value) == RAW_DATA_CST)
+ {
+ tree raw_data = CONSTRUCTOR_ELT (init_list, i)->value;
+ nelts += RAW_DATA_LENGTH (raw_data);
+ vec_safe_reserve (new_args, nelts);
+ if (TYPE_PRECISION (TREE_TYPE (raw_data)) > CHAR_BIT
+ || TYPE_UNSIGNED (TREE_TYPE (raw_data)))
+ for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
+ new_args->quick_push (build_int_cst (TREE_TYPE (raw_data),
+RAW_DATA_UCHAR_ELT (raw_data,
+j)));
+ else
+ for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
+ new_args->quick_push (build_int_cst (TREE_TYPE (raw_data),
+RAW_DATA_SCHAR_ELT (raw_data,
+j)));
+ }
+else
+ new_args->quick_push (CONSTRUCTOR_ELT (init_list, i)->value);
/* We aren't looking for list-ctors anymore. */
flags &= ~LOOKUP_LIST_ONLY;
--- gcc/cp/error.cc.jj 2025-01-10 10:26:37.390685341 +0100
+++ gcc/cp/error.cc 2025-01-17 16:20:44.089508010 +0100
@@ -2289,7 +2289,26 @@ dump_expr_init_vec (cxx_pretty_printer *
FOR_EACH_CONSTRUCTOR_VALUE (v, idx, value)
{
- dump_expr (pp, value, flags | TFF_EXPR_IN_PARENS);
+ if (TREE_CODE (value) == RAW_DATA_CST)
+ for (unsigned i = 0; i < (unsigned) RAW_DATA_LENGTH (value); ++i)
+ {
+ if (TYPE_UNSIGNED (TREE_TYPE (value))
+ || TYPE_PRECISION (TREE_TYPE (value)) > CHAR_BIT)
+ pp_decimal_int (pp, RAW_DATA_UCHAR_ELT (value, i));
+ else
+ pp_decimal_int (pp, RAW_DATA_SCHAR_ELT (value, i));
+ if (i == RAW_DATA_LENGTH (value) - 1U)
+ break;
+ else if (i == 9 && RAW_DATA_LENGTH (value) > 20)
+ {
+ pp_string (pp, ", ..., ");
+ i = RAW_DATA_LENGTH (value) - 11;
+ }
+ else
+ pp_separate_with_comma (pp);
+ }
+ else
+ dump_expr (pp, value, flags | TFF_EXPR_IN_PARENS);
if (idx != v->length () - 1)
pp_separate_with_comma (pp);
}
--- gcc/testsuite/g++.dg/cpp/embed-22.C.jj 2025-01-17 17:06:54.731279618
+0100
+++ gcc/testsuite/g++.dg/cpp/embed-22.C 2025-01-17 16:56:02.499282953 +0100
@@ -0,0 +1,24 @@
+// PR c++/118532
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+struct S {
+ S (int, int, int);
+#define I8 int, int, int, int, int, int, int, int
+#define I64 I8, I8, I8, I8, I8, I8, I8, I8
+ S (I64, I64, I64, I64, I8);
+};
+
+void
+foo (S &)
+{
+}
+
+int
+main ()
+{
+ S s = {
+#embed __FILE__ limit (264)
+ };
+ foo (s);
+}
--- gcc/testsuite/g++.dg/cpp/embed-23.C.jj 2025-01-17 17:02:17.333105186
+0100
+++ gcc/testsuite/g++.dg/cpp/embed-23.C 2025-01-17 17:02:32.650893591 +0100
@@ -0,0 +1,21 @@
+// PR c++/118532
+// { dg-do compile { target c++20 } }
+// { dg-options "" }
+
+constexpr int fn0 () { return 0; }
+constexpr int fn1 () { return 1; }
+
+struct S {
+ explicit(fn0()) S(int, int
[GCC-14][committed] d: Fix ICE in expand_d_format when diagnosing empty enum [PR117115]
Hi,
This patch backports the individual fix for PR117115 from the upstream
merge in r15-6824 into the releases/gcc-14 branch.
Bootstrapped and regression tested on x86_64-linux-gnu/-m32, and
committed to branch.
Regards,
Iain.
---
PR d/117115
gcc/testsuite/ChangeLog:
* gdc.dg/pr117115.d: New test.
(cherry picked from commit 975c4f1a5de4ede89ee9499cd1a73d613a4aeae4)
---
gcc/d/dmd/enumsem.d | 2 +-
gcc/testsuite/gdc.dg/pr117115.d | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gdc.dg/pr117115.d
diff --git a/gcc/d/dmd/enumsem.d b/gcc/d/dmd/enumsem.d
index 3886ca25e97..06683b25490 100644
--- a/gcc/d/dmd/enumsem.d
+++ b/gcc/d/dmd/enumsem.d
@@ -186,7 +186,7 @@ void enumSemantic(Scope* sc, EnumDeclaration ed)
if (ed.members.length == 0)
{
-.error(ed.loc, "%s `%s enum `%s` must have at least one member",
ed.kind, ed.toPrettyChars, ed.toChars());
+.error(ed.loc, "%s `%s` enum `%s` must have at least one member",
ed.kind, ed.toPrettyChars, ed.toChars());
ed.errors = true;
ed.semanticRun = PASS.semanticdone;
return;
diff --git a/gcc/testsuite/gdc.dg/pr117115.d b/gcc/testsuite/gdc.dg/pr117115.d
new file mode 100644
index 000..b012268b509
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr117115.d
@@ -0,0 +1,3 @@
+// { dg-do compile }
+
+enum E117115 {} // { dg-error "must have at least one member" }
--
2.43.0
[committed] d: Add testcase for fixed PR117115
Hi,
This patch adds a testcase for a PR that was fixed in upstream, and
merged in r15-6824.
Regression tested on x86_64-linux-gnu, and committed to mainline.
Regards,
Iain.
---
PR d/117115
gcc/testsuite/ChangeLog:
* gdc.dg/pr117115.d: New test.
---
gcc/testsuite/gdc.dg/pr117115.d | 3 +++
1 file changed, 3 insertions(+)
create mode 100644 gcc/testsuite/gdc.dg/pr117115.d
diff --git a/gcc/testsuite/gdc.dg/pr117115.d b/gcc/testsuite/gdc.dg/pr117115.d
new file mode 100644
index 000..b012268b509
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr117115.d
@@ -0,0 +1,3 @@
+// { dg-do compile }
+
+enum E117115 {} // { dg-error "must have at least one member" }
--
2.43.0
[PATCH v4 1/1] Add warning for non-spec compliant FMV in Aarch64
This patch adds a warning when FMV is used for Aarch64.
The reasoning for this is the ACLE [1] spec for FMV has diverged
significantly from the current implementation and we want to prevent
potential future compatability issues.
There is a patch for an ACLE compliant version of target_version and
target_clone in progress but it won't make gcc-15.
This has been bootstrap and regression tested for Aarch64.
Is this okay for master and packport to gcc-14?
[1]
https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
gcc/ChangeLog:
* config/aarch64/aarch64.cc
(aarch64_process_target_version_attr): Add experimental warning.
* config/aarch64/aarch64.opt: Add command line option to disable
warning.
gcc/testsuite/ChangeLog:
* g++.target/aarch64/mv-1.C: Add CLI flag
* g++.target/aarch64/mv-symbols1.C: Add CLI flag
* g++.target/aarch64/mv-symbols2.C: Add CLI flag
* g++.target/aarch64/mv-symbols3.C: Add CLI flag
* g++.target/aarch64/mv-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-symbols5.C: Add CLI flag
* g++.target/aarch64/mvc-symbols1.C: Add CLI flag
* g++.target/aarch64/mvc-symbols2.C: Add CLI flag
* g++.target/aarch64/mvc-symbols3.C: Add CLI flag
* g++.target/aarch64/mvc-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-pragma.C: Add CLI flag
* g++.target/aarch64/mv-warning1.C: New test.
* g++.target/aarch64/mcv-warning1.C: New test.
---
gcc/config/aarch64/aarch64.cc | 9 +
gcc/config/aarch64/aarch64.opt | 4
gcc/doc/invoke.texi | 11 ++-
gcc/testsuite/g++.target/aarch64/mv-1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 +
gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++
15 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 1dbbc9c3cf9..dba779a8e51 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20257,6 +20257,15 @@ aarch64_parse_fmv_features (const char *str, aarch64_feature_flags *isa_flags,
static bool
aarch64_process_target_version_attr (tree args)
{
+ static bool issued_warning = false;
+ if (!issued_warning)
+{
+ warning (OPT_Wexperimental_fmv_target,
+ "Function Multi Versioning support is experimental, and the "
+ "behavior is likely to change");
+ issued_warning = true;
+}
+
if (TREE_CODE (args) == TREE_LIST)
{
if (TREE_CHAIN (args))
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index da9e0c18d47..7e309d9efe4 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -431,3 +431,7 @@ handling. One means we try to form pairs involving one or more existing
individual writeback accesses where possible. A value of two means we
also try to opportunistically form writeback opportunities by folding in
trailing destructive updates of the base register used by a pair.
+
+Wexperimental-fmv-target
+Target Var(warn_experimental_fmv) Warning Init(1)
+Warn about usage of experimental Function Multi Versioning.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 13afb4a0d0d..4d1612ce7cb 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -827,7 +827,8 @@ Objective-C and Objective-C++ Dialects}.
-moverride=@var{string} -mverbose-cost-dump
-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg}
-mstack-protector-guard-offset=@var{offset} -mtrack-speculation
--moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion}
+-moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion
+-Wexperimental-fmv-target}
@emph{Adapteva Epiphany Options} (@ref{Adapteva Epiphany Options})
@gccoptlist{-mhalf-reg-file -mprefer-short-insn-regs
@@ -21882,6 +21883,14 @@ The default is @samp{-msve-vector-bits=scalable}, which produces
vector-length agnostic code.
@end table
+@opindex Wexperimental-fmv-target
+@opindex Wno-experimental-fmv-target
+@item -Wexperimental-fmv-target
+Warn about use of experimental Function Multi Versioning.
+The Arm C Language Extension specification for Function Multi Versioning
+is beta and subject to change. Any usage
[PATCH v4 0/1] FMV AArch64 warning
From: Alfie Richards Hi, Thank you Richard for catching that! It's good to know about keeping it around. Rebased onto master, fixed up a new test, and addressed your comment. Kind regards, Alfie Alfie Richards (1): Add warning for non-spec compliant FMV in Aarch64 gcc/config/aarch64/aarch64.cc | 9 + gcc/config/aarch64/aarch64.opt | 4 gcc/doc/invoke.texi | 11 ++- gcc/testsuite/g++.target/aarch64/mv-1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 + gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 + gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++ 15 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C -- 2.34.1
[committed] testsuite: Make embed-10.c test more robust
Hi!
With the https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack we get slightly different error wording in one of the errors, given that
the test actually does use #embed, I think both wordings are just fine and
we should accept them.
Tested on x86_64-linux, committed to trunk as obvious.
2025-01-17 Jakub Jelinek
* c-c++-common/cpp/embed-10.c: Allow a different error wording for
C++.
--- gcc/testsuite/c-c++-common/cpp/embed-10.c.jj2024-09-12
11:05:22.0 +0200
+++ gcc/testsuite/c-c++-common/cpp/embed-10.c 2025-01-17 10:00:20.769854161
+0100
@@ -7,4 +7,4 @@ const char *p =
/* { dg-error "makes pointer from integer without a cast" "" { target c } .-2
} */
/* { dg-error "expected identifier" "" { target c } .-3 } */
/* { dg-error "invalid conversion" "" { target c++ } .-4 } */
-/* { dg-error "expected unqualified-id before numeric constant" "" { target
c++ } .-5 } */
+/* { dg-error "expected unqualified-id before (?:numeric constant|'#embed')"
"" { target c++ } .-5 } */
Jakub
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
On Fri, Jan 17, 2025 at 3:14 PM Joseph Myers wrote:
>
> On Fri, 17 Jan 2025, Qing Zhao wrote:
>
> > struct fc_bulk {
> > ...
> > struct fs_bulk fs_bulk;
> > struct fc fcs[] __counted_by(fs_bulk.len);
> > };
> >
> > i.e, the “counted_by” field is in the inner structure of the current
> > structure of the FAM.
> > With the current syntax, it’s not easy to extend to support this.
> >
> > But with the designator syntax, it might be much easier to be extended to
> > support this.
> >
> > So, Kees and Bill, what’s your opinion on this? I think that it’s better to
> > have a consistent interface between GCC
> > and Clang.
> >
> > Joseph, what’s your opinion on this new syntax? Shall we support the
> > designator syntax for counted_by attribute?
>
> Designator syntax seems reasonable.
>
> I think basic C language design principles here include:
>
> * It should be unambiguous in a given context what name space an
> identifier is to be looked up in. (So you can have designator syntax
> where the identifier is always looked up as a member of the relevant
> struct, or use a plain identifier where the semantics are defined that
> way. But what would be a bad idea would be any attempt to automagically
> guess whether something that looks like an expression should be
> interpreted as an expression or with identifiers instead looked up as
> structure members. If you allow fs_bulk.len above, there should be no
> possibility of fs_bulk being an ordinary identifier (global variable etc.)
> - the name lookup rules should mean it's always only looked up as a member
> of the current structure.)
>
> * Don't introduce something "like expression but with different name
> lookup rules". Designators aren't expressions and have limited syntax.
> It would be a bad idea to try to e.g. have something allowing arithmetic
> on designators. For example, don't allow __counted_by(.len1 + .len2)
> where len1 and len2 are both members, as that's inventing a complete new
> expression-like-but-not-expression syntactic construct.
>
I re-opened the discussion on the LLVM discourse page. There wasn't a
lot of pushback when it was first brought up, so I suspect that people
will be generally amenable to it.
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/131?u=gwelymernans
-bw
> --
> Joseph S. Myers
> josmy...@redhat.com
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
On Fri, 17 Jan 2025, Qing Zhao wrote:
> struct fc_bulk {
> ...
> struct fs_bulk fs_bulk;
> struct fc fcs[] __counted_by(fs_bulk.len);
> };
>
> i.e, the “counted_by” field is in the inner structure of the current
> structure of the FAM.
> With the current syntax, it’s not easy to extend to support this.
>
> But with the designator syntax, it might be much easier to be extended to
> support this.
>
> So, Kees and Bill, what’s your opinion on this? I think that it’s better to
> have a consistent interface between GCC
> and Clang.
>
> Joseph, what’s your opinion on this new syntax? Shall we support the
> designator syntax for counted_by attribute?
Designator syntax seems reasonable.
I think basic C language design principles here include:
* It should be unambiguous in a given context what name space an
identifier is to be looked up in. (So you can have designator syntax
where the identifier is always looked up as a member of the relevant
struct, or use a plain identifier where the semantics are defined that
way. But what would be a bad idea would be any attempt to automagically
guess whether something that looks like an expression should be
interpreted as an expression or with identifiers instead looked up as
structure members. If you allow fs_bulk.len above, there should be no
possibility of fs_bulk being an ordinary identifier (global variable etc.)
- the name lookup rules should mean it's always only looked up as a member
of the current structure.)
* Don't introduce something "like expression but with different name
lookup rules". Designators aren't expressions and have limited syntax.
It would be a bad idea to try to e.g. have something allowing arithmetic
on designators. For example, don't allow __counted_by(.len1 + .len2)
where len1 and len2 are both members, as that's inventing a complete new
expression-like-but-not-expression syntactic construct.
--
Joseph S. Myers
josmy...@redhat.com
Re: [PATCH v2] c++: fix wrong-code with constexpr prvalue opt [PR118396]
On 1/17/25 1:31 PM, Marek Polacek wrote:
On Fri, Jan 17, 2025 at 08:10:24AM -0500, Jason Merrill wrote:
On 1/16/25 8:04 PM, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
In that case, I'd think we want to use the value of 'object' (which should
be in ctx.ctor?) instead of the return value of
cxx_eval_constant_expression.
Ah, I'm sorry I didn't choose that approach. Maybe like this, then?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK. Maybe also add an assert that TREE_TYPE (r) is close enough to type?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
For non-simple TARGET_EXPRs, we should return ctx.ctor rather than
the result of cxx_eval_constant_expression.
PR c++/118396
PR c++/118523
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): For non-simple
TARGET_EXPRs, return ctx.ctor rather than the result of
cxx_eval_constant_expression.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-prvalue4.C: New test.
* g++.dg/cpp1y/constexpr-prvalue3.C: New test.
---
gcc/cp/constexpr.cc | 5 +++
.../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++
.../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++
3 files changed, 83 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index c898e3bfa6e..078f7b19f24 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8978,6 +8978,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
r = cxx_eval_constant_expression (&ctx, r, vc_prvalue,
&non_constant_p, &overflow_p);
+ /* If we got a non-simple TARGET_EXPR, the initializer was a sequence
+ of statements, and the result ought to be stored in ctx.ctor. */
+ if (r == void_node && !constexpr_dtor && ctx.ctor)
+r = ctx.ctor;
+
if (!constexpr_dtor)
verify_constant (r, allow_non_constant, &non_constant_p, &overflow_p);
else
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
new file mode 100644
index 000..afcee65f880
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
@@ -0,0 +1,33 @@
+// PR c++/118396
+// { dg-do run { target c++11 } }
+// { dg-options "-O" }
+
+void *operator new(__SIZE_TYPE__, void *__p) { return __p; }
+
+struct Foo {
+ virtual ~Foo() = default;
+};
+struct Data {
+ int status;
+ Foo data{};
+};
+
+Data *P, *Q;
+
+struct vector {
+ vector (const Data &__value) {
+P = static_cast(__builtin_operator_new(0));
+new (P) Data (__value);
+Q = P + 1;
+ }
+ Data *begin() { return P; }
+ Data *end() { return Q; }
+};
+
+int
+main ()
+{
+ vector items_(Data{});
+ for (auto item : items_)
+item.status == 0 ? void() : __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
new file mode 100644
index 000..8ea86c60be5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
@@ -0,0 +1,45 @@
+// PR c++/118523
+// { dg-do compile { target c++14 } }
+// { dg-options "-O2 -Wall" }
+
+struct __new_allocator {
+ constexpr __new_allocator() {}
+ __new_allocator(__new_allocator &) {}
+};
+template using __allocator_base = __new_allocator;
+template struct allocator_traits;
+template struct allocator : __allocator_base {};
+template struct allocator_traits> {
+ using pointer = _Tp *;
+ template using rebind_alloc = allocator<_Up>;
+ static void deallocate(allocator<_Tp>, pointer, long);
+};
+struct __alloc_traits : allocator_traits> {};
+struct _Vector_impl_data {
+ __alloc_traits::pointer _M_start;
+ __alloc_traits::pointer _M_end_of_storage;
+ constexpr _Vector_impl_data() : _M_start(), _M_end_of_storage() {}
+};
+struct _Vector_impl : __alloc_traits::rebind_alloc, _Vector_impl_data {};
+struct _Vector_base {
+ ~_Vector_base() {
+_M_deallocate(_M_i
[PATCH v5 1/1] Add warning for non-spec compliant FMV in Aarch64
This patch adds a warning when FMV is used for Aarch64.
The reasoning for this is the ACLE [1] spec for FMV has diverged
significantly from the current implementation and we want to prevent
potential future compatability issues.
There is a patch for an ACLE compliant version of target_version and
target_clone in progress but it won't make gcc-15.
This has been bootstrap and regression tested for Aarch64.
Is this okay for master and packport to gcc-14?
[1]
https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
gcc/ChangeLog:
* config/aarch64/aarch64.cc
(aarch64_process_target_version_attr): Add experimental warning.
* config/aarch64/aarch64.opt: Add command line option to disable
warning.
gcc/testsuite/ChangeLog:
* g++.target/aarch64/mv-1.C: Add CLI flag
* g++.target/aarch64/mv-symbols1.C: Add CLI flag
* g++.target/aarch64/mv-symbols2.C: Add CLI flag
* g++.target/aarch64/mv-symbols3.C: Add CLI flag
* g++.target/aarch64/mv-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-symbols5.C: Add CLI flag
* g++.target/aarch64/mvc-symbols1.C: Add CLI flag
* g++.target/aarch64/mvc-symbols2.C: Add CLI flag
* g++.target/aarch64/mvc-symbols3.C: Add CLI flag
* g++.target/aarch64/mvc-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-pragma.C: Add CLI flag
* g++.target/aarch64/mv-warning1.C: New test.
* g++.target/aarch64/mcv-warning1.C: New test.
---
gcc/config/aarch64/aarch64.cc | 9 +
gcc/config/aarch64/aarch64.opt | 4
gcc/doc/invoke.texi | 11 ++-
gcc/testsuite/g++.target/aarch64/mv-1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-pragma.C| 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 +
gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++
16 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 1dbbc9c3cf9..dba779a8e51 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20257,6 +20257,15 @@ aarch64_parse_fmv_features (const char *str, aarch64_feature_flags *isa_flags,
static bool
aarch64_process_target_version_attr (tree args)
{
+ static bool issued_warning = false;
+ if (!issued_warning)
+{
+ warning (OPT_Wexperimental_fmv_target,
+ "Function Multi Versioning support is experimental, and the "
+ "behavior is likely to change");
+ issued_warning = true;
+}
+
if (TREE_CODE (args) == TREE_LIST)
{
if (TREE_CHAIN (args))
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index da9e0c18d47..7e309d9efe4 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -431,3 +431,7 @@ handling. One means we try to form pairs involving one or more existing
individual writeback accesses where possible. A value of two means we
also try to opportunistically form writeback opportunities by folding in
trailing destructive updates of the base register used by a pair.
+
+Wexperimental-fmv-target
+Target Var(warn_experimental_fmv) Warning Init(1)
+Warn about usage of experimental Function Multi Versioning.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 13afb4a0d0d..6f2f54ae752 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -827,7 +827,8 @@ Objective-C and Objective-C++ Dialects}.
-moverride=@var{string} -mverbose-cost-dump
-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg}
-mstack-protector-guard-offset=@var{offset} -mtrack-speculation
--moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion}
+-moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion
+-Wexperimental-fmv-target}
@emph{Adapteva Epiphany Options} (@ref{Adapteva Epiphany Options})
@gccoptlist{-mhalf-reg-file -mprefer-short-insn-regs
@@ -21880,6 +21881,14 @@ hardware SVE vector lengths.
The default is @samp{-msve-vector-bits=scalable}, which produces
vector-length agnostic code.
+
+@opindex Wexperimental-fmv-target
+@opindex Wno-experimental-fmv-target
+@item -Wexperimental-fmv-target
+Warn about use of experimental Function Multi Versioning.
+The Arm C Language Extension specificat
[PATCH v5 0/1] FMV AArch64 warning
From: Alfie Richards Appologies, forgot to commit my changes before sending. Alfie Richards (1): Add warning for non-spec compliant FMV in Aarch64 gcc/config/aarch64/aarch64.cc | 9 + gcc/config/aarch64/aarch64.opt | 4 gcc/doc/invoke.texi | 11 ++- gcc/testsuite/g++.target/aarch64/mv-1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-pragma.C| 1 + gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 + gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 + gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++ 16 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C -- 2.34.1
[PATCH] c++: Fix up find_array_ctor_elt RAW_DATA_CST handling [PR118534]
Hi!
This is the third bug discovered today with the
https://gcc.gnu.org/pipermail/gcc-patches/2025-January/673945.html
hack but then turned into proper testcases where embed-24.C FAILed
since introduction of optimized #embed support and the others when
optimizing large C++ initializers using RAW_DATA_CST.
find_array_ctor_elt already has RAW_DATA_CST support, but on the
following testcases it misses one case I've missed.
The CONSTRUCTORs in question went through the braced_list_to_string
optimization which can turn INTEGER_CST RAW_DATA_CST INTEGER_CST
into just larger RAW_DATA_CST covering even those 2 bytes around it
(if they appear there in the underlying RAW_DATA_OWNER).
With this optimization, RAW_DATA_CST can be the last CONSTRUCTOR_ELTS
elt in a CONSTRUCTOR, either the sole one or say preceeded by some
unrelated other elements. Now, if RAW_DATA_CST is the only one or
if there are no RAW_DATA_CSTs earlier in CONSTRUCTOR_ELTS, we can
trigger a bug in find_array_ctor_elt.
It has a smart optimization for the very common case where
CONSTRUCTOR_ELTS have indexes and index of the last elt is equal
to CONSTRUCTOR_NELTS (ary) - 1, then obviously we know there are
no RAW_DATA_CSTs before it and the indexes just go from 0 to nelts-1,
so when we care about any of those earlier indexes, we can just return i;
and not worry about anything.
Except it uses if (i < end) return i; rather than if (i < end - 1) return i;
For the latter cases, i.e. anything before the last elt, we know there
are no surprises and return i; is right. But for the if (i == end - 1)
case, return i; is only correct if the last elt is not RAW_DATA_CST, if it
is RAW_DATA_CST, we still need to split it, which is handled by the code
later in the function. So, for that we need begin = end - 1, so that the
binary search will just care about that last element.
Bootstrapped on x86_64-linux and i686-linux, regtests pending, ok
for trunk if it passes?
2025-01-17 Jakub Jelinek
PR c++/118534
* constexpr.cc (find_array_ctor_elt): Don't return i early if
i == end - 1 and the last elt's value is RAW_DATA_CST.
* g++.dg/cpp/embed-24.C: New test.
* g++.dg/cpp1y/pr118534.C: New test.
--- gcc/cp/constexpr.cc.jj 2025-01-17 11:29:33.507691199 +0100
+++ gcc/cp/constexpr.cc 2025-01-17 19:07:36.422646176 +0100
@@ -4155,12 +4155,17 @@ find_array_ctor_elt (tree ary, tree dind
else if (TREE_CODE (cindex) == INTEGER_CST
&& compare_tree_int (cindex, end - 1) == 0)
{
- if (i < end)
- return i;
tree value = (*elts)[end - 1].value;
- if (TREE_CODE (value) == RAW_DATA_CST
- && wi::to_offset (dindex) < (wi::to_offset (cindex)
- + RAW_DATA_LENGTH (value)))
+ if (i < end)
+ {
+ if (i == end - 1 && TREE_CODE (value) == RAW_DATA_CST)
+ begin = end - 1;
+ else
+ return i;
+ }
+ else if (TREE_CODE (value) == RAW_DATA_CST
+ && wi::to_offset (dindex) < (wi::to_offset (cindex)
+ + RAW_DATA_LENGTH (value)))
begin = end - 1;
else
begin = end;
--- gcc/testsuite/g++.dg/cpp/embed-24.C.jj 2025-01-17 17:55:56.127894446
+0100
+++ gcc/testsuite/g++.dg/cpp/embed-24.C 2025-01-17 19:15:39.268992556 +0100
@@ -0,0 +1,30 @@
+// PR c++/118534
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+template
+constexpr bool
+foo ()
+{
+ T x[160] = {
+#embed __FILE__ limit (160)
+ };
+ const int y[160] = {
+42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
+#embed __FILE__ limit (147) gnu::offset (13)
+ };
+ unsigned long n = 13;
+ for (T *p = x; n; --n, p++)
+*p = 42;
+ for (int i = 0; i < 160; ++i)
+if (x[i] != y[i])
+ return false;
+ return true;
+}
+
+int
+main ()
+{
+ static_assert (foo (), "");
+ static_assert (foo (), "");
+}
--- gcc/testsuite/g++.dg/cpp1y/pr118534.C.jj2025-01-17 18:28:21.492097114
+0100
+++ gcc/testsuite/g++.dg/cpp1y/pr118534.C 2025-01-17 19:16:05.362632984
+0100
@@ -0,0 +1,31 @@
+// PR c++/118534
+// { dg-do compile { target c++14 } }
+
+template
+constexpr bool
+foo ()
+{
+ T x[160] = {
+#define I8 1, 2, 3, 4, 5, 6, 7, 8
+#define I64 I8, I8, I8, I8, I8, I8, I8, I8
+I64, I64, I8, I8, I8, I8
+ };
+ const int y[160] = {
+42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 6, 7, 8,
+I64, I64, I8, I8
+ };
+ unsigned long n = 13;
+ for (T *p = x; n; --n, p++)
+*p = 42;
+ for (int i = 0; i < 160; ++i)
+if (x[i] != y[i])
+ return false;
+ return true;
+}
+
+int
+main ()
+{
+ static_assert (foo (), "");
+ static_assert (foo (), "");
+}
Jakub
[pushed]PR118067][LRA]: Check secondary memory mode for the reg class
This is one more patch to solve
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118067
with different -mcpu used.
The patch was successfully bootstrapped and tested on x86-64, aarch64,
and ppc64le.
commit 9f009e8865cda01310c52f7ec8bdaa3c557a2745
Author: Vladimir N. Makarov
Date: Fri Jan 17 15:56:29 2025 -0500
[PR118067][LRA]: Check secondary memory mode for the reg class
This is the second patch for the PR for the new test. The patch
solves problem in the case when secondary memory mode (SImode in the
PR test) returned by hook secondary_memory_needed_mode can not be used
for reg class (ALL_MASK_REGS) involved in secondary memory moves. The
patch uses reg mode instead of one returned by
secondary_memory_needed_mode in this case.
gcc/ChangeLog:
PR rtl-optimization/118067
* lra-constraints.cc (invalid_mode_reg_p): New function.
(curr_insn_transform): Use it to check mode returned by target
secondary_memory_needed_mode.
gcc/testsuite/ChangeLog:
* gcc.target/i386/pr118067-2.c: New.
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 3d5abcfaeb0..cd19da294db 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -4129,6 +4129,19 @@ swap_operands (int nop)
lra_update_dup (curr_id, nop + 1);
}
+/* Return TRUE if X is a (subreg of) reg and there are no hard regs of X class
+ which can contain value of MODE. */
+static bool invalid_mode_reg_p (enum machine_mode mode, rtx x)
+{
+ if (SUBREG_P (x))
+x = SUBREG_REG (x);
+ if (! REG_P (x))
+return false;
+ enum reg_class rclass = get_reg_class (REGNO (x));
+ return hard_reg_set_subset_p (ira_prohibited_class_mode_regs[rclass][mode],
+reg_class_contents[rclass]);
+}
+
/* Main entry point of the constraint code: search the body of the
current insn to choose the best alternative. It is mimicking insn
alternative cost calculation model of former reload pass. That is
@@ -4389,6 +4402,10 @@ curr_insn_transform (bool check_only_p)
rld = partial_subreg_p (GET_MODE (src), GET_MODE (dest)) ? src : dest;
rld_mode = GET_MODE (rld);
sec_mode = targetm.secondary_memory_needed_mode (rld_mode);
+ if (rld_mode != sec_mode
+ && (invalid_mode_reg_p (sec_mode, dest)
+ || invalid_mode_reg_p (sec_mode, src)))
+ sec_mode = rld_mode;
new_reg = lra_create_new_reg (sec_mode, NULL_RTX, NO_REGS, NULL,
"secondary");
/* If the mode is changed, it should be wider. */
diff --git a/gcc/testsuite/gcc.target/i386/pr118067-2.c b/gcc/testsuite/gcc.target/i386/pr118067-2.c
new file mode 100644
index 000..831871db0b4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr118067-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O -fno-split-wide-types -mavx512f -mcpu=k8" } */
+
+typedef unsigned short U __attribute__((__vector_size__(64)));
+typedef int V __attribute__((__vector_size__(64)));
+typedef __int128 W __attribute__((__vector_size__(64)));
+
+W
+foo(U u, V v)
+{
+ W w;
+ /* __asm__ volatile ("" : "=v"(w)); prevents the -Wuninitialized warning */
+ u[0] >>= 1;
+ v %= (V)w;
+ return (W)u + (W)v;
+}
Re: [PATCH] libfortran: G formatting for UNSIGNED [PR118536]
Hi Jerry, Am 17.01.25 um 22:04 schrieb Jerry Delisle: Looks Ok by me. Committed and pushed r15-7009-gca2681d45a4507 . Thanks, Harald Thanks, Jerry On Fri, Jan 17, 2025, 12:25 PM Harald Anlauf wrote: Dear all, the attached obvious patch extends G formatting to UNSIGNED by reusing the code for I formatting. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald
Re: [PATCH] i386: Fix wrong insn generated by shld/shrd ndd split [PR118510]
Uros Bizjak 于2025年1月17日周五 15:05写道： > Is there a reason to have operand 0 with "nonimmediate_operand" > predicate? If you have to generate a register temporary and then > unconditionally copy it to the output, it is better to use > "register_operand" predicate and leave middle end to do the copy for > you. Please see the patch at comment #3 in the PR. Ah, thanks for pointing that out, yes the memory dest is meaningless here. Updated patch in attachment. Ok for trunk after bootstrap & regtest? 0001-i386-Fix-wrong-insn-generated-by-shld-shrd-ndd-split.patch Description: Binary data
[PATCH v2] MIPS: Add conditions for use of the -mmips16e2 and -mips16 option.
Changes from V1:
* Raise the minimal revision to r2.
MIPS16e2 ASE is a superset of MIPS16e ASE, which is again a superset
of MIPS16 ASE. Later, all of them are forbidden in Release 6.
Make -mmips16e2 imply -mips16 as the ASE requires, so users won't
be surprised even if they expect it to. Meanwhile, check if
mips_isa_rev <= 5 when -mips16 is effective and >= 2 when -mmips16e2
is effective.
gcc/ChangeLog:
* config/mips/mips.cc(mips_option_override):Add conditions
for use of the -mmips16e2 and -mips16 option.
gcc/testsuite/ChangeLog:
* gcc.target/mips/mips16e2-cache.c: Use isa_rev>=2 instead of
-mips32r2 and remove -mips16 option.
* gcc.target/mips/mips16e2-cmov.c: Add isa_rev>=2 and remove
-mips16 option.
* gcc.target/mips/mips16e2-gp.c: Same as above.
* gcc.target/mips/mips16e2.c: Same as above.
Co-developed-by: Rong Zhang
Signed-off-by: Rong Zhang
---
gcc/config/mips/mips.cc | 19 +++
.../gcc.target/mips/mips16e2-cache.c | 2 +-
gcc/testsuite/gcc.target/mips/mips16e2-cmov.c | 2 +-
gcc/testsuite/gcc.target/mips/mips16e2-gp.c | 2 +-
gcc/testsuite/gcc.target/mips/mips16e2.c | 2 +-
5 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 492fa285477..173e3fe72e6 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -20402,6 +20402,16 @@ mips_option_override (void)
if (TARGET_MICROMIPS && TARGET_MIPS16)
error ("unsupported combination: %s", "-mips16 -mmicromips");
+ /* Make -mmips16e2 imply -mips16 and forbid its coexistence with
+ -mmicromips as the ASE requires. */
+ if (TARGET_MIPS16E2)
+ {
+if (TARGET_MICROMIPS)
+ error ("unsupported combination: %s", "-mmips16e2 -mmicromips");
+
+target_flags |= MASK_MIPS16;
+ }
+
/* Prohibit Paired-Single and MSA combination. This is software restriction
rather than architectural. */
if (ISA_HAS_MSA && TARGET_PAIRED_SINGLE_FLOAT)
@@ -20654,6 +20664,15 @@ mips_option_override (void)
"-mcompact-branches=never");
}
+ /* MIPS16* ASE is forbidden in Release 6, so -mips16 is not available
+ for MIPS R6 onwards. */
+ if ((mips_base_compression_flags & MASK_MIPS16) && mips_isa_rev >= 6)
+error ("MIPS16* ASE is forbidden in Release 6");
+
+ /* Make sure that the user use Release[2,5] when using -mmips16e2. */
+ if (TARGET_MIPS16E2 && mips_isa_rev < 2)
+error ("%<-mmips16e2%> requires Release[2,5]");
+
/* Require explicit relocs for MIPS R6 onwards. This enables simplification
of the compact branch and jump support through the backend. */
if (!TARGET_EXPLICIT_RELOCS && mips_isa_rev >= 6)
diff --git a/gcc/testsuite/gcc.target/mips/mips16e2-cache.c
b/gcc/testsuite/gcc.target/mips/mips16e2-cache.c
index dcc39b580f5..8caacb17d7a 100644
--- a/gcc/testsuite/gcc.target/mips/mips16e2-cache.c
+++ b/gcc/testsuite/gcc.target/mips/mips16e2-cache.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 -mips32r2 -mips16
-mmips16e2" } */
+/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 isa_rev>=2 -mmips16e2" } */
/* { dg-skip-if "naming registers makes this a code quality test" { *-*-* } {
"-O0" } { "" } } */
/* Test cache. */
diff --git a/gcc/testsuite/gcc.target/mips/mips16e2-cmov.c
b/gcc/testsuite/gcc.target/mips/mips16e2-cmov.c
index 129ea23b65b..a8a28a4d860 100644
--- a/gcc/testsuite/gcc.target/mips/mips16e2-cmov.c
+++ b/gcc/testsuite/gcc.target/mips/mips16e2-cmov.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 -mips16 -mmips16e2
-mbranch-cost=2" } */
+/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 isa_rev>=2 -mmips16e2
-mbranch-cost=2" } */
/* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
/* Test MOVN. */
diff --git a/gcc/testsuite/gcc.target/mips/mips16e2-gp.c
b/gcc/testsuite/gcc.target/mips/mips16e2-gp.c
index 7955472bde3..70d6230f017 100644
--- a/gcc/testsuite/gcc.target/mips/mips16e2-gp.c
+++ b/gcc/testsuite/gcc.target/mips/mips16e2-gp.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 -mips16 -mmips16e2" } */
+/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 isa_rev>=2 -mmips16e2" } */
/* { dg-skip-if "per-function expected output" { *-*-* } { "-flto" } { "" } }
*/
/* Generate GP-relative ADDIU. */
diff --git a/gcc/testsuite/gcc.target/mips/mips16e2.c
b/gcc/testsuite/gcc.target/mips/mips16e2.c
index 166aa742268..1b4b840bb40 100644
--- a/gcc/testsuite/gcc.target/mips/mips16e2.c
+++ b/gcc/testsuite/gcc.target/mips/mips16e2.c
@@ -1,4 +1,4 @@
-/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 -mips16 -mmips16e2" } */
+/* { dg-options "-mno-abicalls -mgpopt -G8 -mabi=32 isa_rev>=2 -mmips16e2" } */
/* { dg-skip-if "per-function expected output" { *-*-* } { "-flto" } { "" } }
*/
/* ANDI is a two operand instruction. Hence, it won't be generated if src and
--
2.42.0
[PATCH] RISC-V: Enable and adjust the testsuite for XTheadVector.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/xtheadvector/pr114194.c: Adjust correctly.
* gcc.target/riscv/rvv/xtheadvector/prefix.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlb-vsb.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlbu-vsb.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlh-vsh.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlhu-vsh.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlw-vsw.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/vlwu-vsw.c: Likewise.
* gcc.target/riscv/rvv/xtheadvector/xtheadvector.exp: New test.
---
.../riscv/rvv/xtheadvector/pr114194.c | 32 +++
.../riscv/rvv/xtheadvector/prefix.c | 2 +-
.../riscv/rvv/xtheadvector/vlb-vsb.c | 17
.../riscv/rvv/xtheadvector/vlbu-vsb.c | 17
.../riscv/rvv/xtheadvector/vlh-vsh.c | 17
.../riscv/rvv/xtheadvector/vlhu-vsh.c | 17
.../riscv/rvv/xtheadvector/vlw-vsw.c | 17
.../riscv/rvv/xtheadvector/vlwu-vsw.c | 17
.../riscv/rvv/xtheadvector/xtheadvector.exp | 40 +++
9 files changed, 117 insertions(+), 59 deletions(-)
create mode 100644
gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/xtheadvector.exp
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr114194.c
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr114194.c
index a82e2d3fbfe6..5c9777b071b5 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr114194.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr114194.c
@@ -1,11 +1,11 @@
/* { dg-do compile { target { ! riscv_abi_e } } } */
-/* { dg-options "-march=rv32gc_xtheadvector" { target { rv32 } } } */
-/* { dg-options "-march=rv64gc_xtheadvector" { target { rv64 } } } */
+/* { dg-options "-march=rv32gc_xtheadvector -O2" { target { rv32 } } } */
+/* { dg-options "-march=rv64gc_xtheadvector -O2" { target { rv64 } } } */
/* { dg-final { check-function-bodies "**" "" } } */
/*
** foo0_1:
-** sb\tzero,0([a-x0-9]+)
+** sb\tzero,0\([a-x0-9]+\)
** ret
*/
void foo0_1 (void *p)
@@ -15,13 +15,13 @@ void foo0_1 (void *p)
/*
** foo0_7:
-** sb\tzero,0([a-x0-9]+)
-** sb\tzero,1([a-x0-9]+)
-** sb\tzero,2([a-x0-9]+)
-** sb\tzero,3([a-x0-9]+)
-** sb\tzero,4([a-x0-9]+)
-** sb\tzero,5([a-x0-9]+)
-** sb\tzero,6([a-x0-9]+)
+** sb\tzero,0\([a-x0-9]+\)
+** sb\tzero,1\([a-x0-9]+\)
+** sb\tzero,2\([a-x0-9]+\)
+** sb\tzero,3\([a-x0-9]+\)
+** sb\tzero,4\([a-x0-9]+\)
+** sb\tzero,5\([a-x0-9]+\)
+** sb\tzero,6\([a-x0-9]+\)
** ret
*/
void foo0_7 (void *p)
@@ -32,7 +32,7 @@ void foo0_7 (void *p)
/*
** foo1_1:
** li\t[a-x0-9]+,1
-** sb\t[a-x0-9]+,0([a-x0-9]+)
+** sb\t[a-x0-9]+,0\([a-x0-9]+\)
** ret
*/
void foo1_1 (void *p)
@@ -43,11 +43,11 @@ void foo1_1 (void *p)
/*
** foo1_5:
** li\t[a-x0-9]+,1
-** sb\t[a-x0-9]+,0([a-x0-9]+)
-** sb\t[a-x0-9]+,1([a-x0-9]+)
-** sb\t[a-x0-9]+,2([a-x0-9]+)
-** sb\t[a-x0-9]+,3([a-x0-9]+)
-** sb\t[a-x0-9]+,4([a-x0-9]+)
+** sb\t[a-x0-9]+,0\([a-x0-9]+\)
+** sb\t[a-x0-9]+,1\([a-x0-9]+\)
+** sb\t[a-x0-9]+,2\([a-x0-9]+\)
+** sb\t[a-x0-9]+,3\([a-x0-9]+\)
+** sb\t[a-x0-9]+,4\([a-x0-9]+\)
** ret
*/
void foo1_5 (void *p)
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/prefix.c
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/prefix.c
index eee727ef6b42..0a18e697830c 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/prefix.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/prefix.c
@@ -9,4 +9,4 @@ prefix (vint32m1_t vx, vint32m1_t vy, size_t vl)
return __riscv_vadd_vv_i32m1 (vx, vy, vl);
}
-/* { dg-final { scan-assembler {\mth\.v\M} } } */
+/* { dg-final { scan-assembler {\mth\.vadd\.vv\M} } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vlb-vsb.c
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vlb-vsb.c
index 3c12c1245974..16073ccb2366 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vlb-vsb.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vlb-vsb.c
@@ -5,7 +5,8 @@
/*
** f1:
-** th.vsetivli\tzero,4,e32,m1,tu,ma
+** li\t[a-x0-9]+,4
+** th.vsetvli\tzero,[a-x0-9]+,e32,m1
** th.vlb\.v\tv[0-9]+,0\([a-x0-9]+\)
** th.vlb\.v\tv[0-9]+,0\([a-x0-9]+\)
** th.vadd\.vv\tv[0-9]+,\s*v[0-9]+,\s*v[0-9]+
@@ -24,9 +25,10 @@ void f1 (void * in, void *out)
/*
** f2:
-** th.vsetvli\t[a-x0-9]+,zero,e8,mf4,ta,ma
-** th.vlm.v\tv[0-9]+,0\([a-x0-9]+\)
-** th.vsetivli\tzero,4,e32,m1,ta,ma
+** th.vsetvli\tzero,zero,e8,m1
+** th.vle.v\tv[0-9]+,0\([a-x0-9]+\)
+** li\t[a-x0-9]+,4
+** th.vsetvli\tzero,[a-x0-9]+,e32,m1
** th.vlb.v\tv[0-9]+,0\([a-x0-9]+\),v0.t
** th.vadd\.vv\tv[0-9]+,\s*v[0-9]+,\s*v[0-9]+
** th.vadd\.vv\tv[1-9][0-9]?,\s*v[0-9]+,\s*v[0-9]+,\s*v0.t
@@ -46,9 +48,10 @@ void f2 (
[PATCH] c++: Copy over further 2 flags for !TREE_PUBLIC in copy_linkage [PR118513]
Hi!
The following testcase ICEs in import_export_decl.
When cp_finish_decomp handles std::tuple* using structural binding,
it calls copy_linkage to copy various VAR_DECL flags from the structured
binding base to the individual sb variables.
In this case the base variable is in anonymous union, so we call
constrain_visibility (..., VISIBILITY_ANON, ...) on it which e.g.
clears TREE_PUBLIC etc. (flags which copy_linkage copies) but doesn't
copy over DECL_INTERFACE_KNOWN/DECL_NOT_REALLY_EXTERN.
When cp_finish_decl calls determine_visibility on the individual sb
variables, those have !TREE_PUBLIC since copy_linkage and so nothing tries
to determine visibility and nothing sets DECL_INTERFACE_KNOWN and
DECL_NOT_REALLY_EXTERN.
Now, this isn't a big deal without modules, the individual variables are
var_finalized_p and so nothing really cares about missing
DECL_INTERFACE_KNOWN. But in the module case the variables are streamed
out and in and care about those bits.
The following patch is an attempt to copy over also those flags (but I've
limited it to the !TREE_PUBLIC case just in case). Other option would be
to call it unconditionally, or call constrain_visibility with
VISIBILITY_ANON for !TREE_PUBLIC (but are all !TREE_PUBLIC constrained
visibility) or do it only in the cp_finish_decomp case
after the copy_linkage call there.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-01-17 Jakub Jelinek
PR c++/118513
* decl2.cc (copy_linkage): If not TREE_PUBLIC, also copy
DECL_INTERFACE_KNOWN and DECL_NOT_REALLY_EXTERN flags.
* g++.dg/modules/decomp-3_a.H: New test.
* g++.dg/modules/decomp-3_b.C: New test.
--- gcc/cp/decl2.cc.jj 2025-01-16 13:34:29.719360710 +0100
+++ gcc/cp/decl2.cc 2025-01-16 16:25:37.006911025 +0100
@@ -3656,6 +3656,12 @@ copy_linkage (tree guard, tree decl)
comdat_linkage (guard);
DECL_VISIBILITY (guard) = DECL_VISIBILITY (decl);
DECL_VISIBILITY_SPECIFIED (guard) = DECL_VISIBILITY_SPECIFIED (decl);
+ if (!TREE_PUBLIC (decl))
+ {
+ DECL_INTERFACE_KNOWN (guard) = DECL_INTERFACE_KNOWN (decl);
+ if (DECL_LANG_SPECIFIC (decl) && DECL_LANG_SPECIFIC (guard))
+ DECL_NOT_REALLY_EXTERN (guard) = DECL_NOT_REALLY_EXTERN (decl);
+ }
}
}
--- gcc/testsuite/g++.dg/modules/decomp-3_a.H.jj2025-01-16
13:43:36.492879734 +0100
+++ gcc/testsuite/g++.dg/modules/decomp-3_a.H 2025-01-16 13:43:11.314224231
+0100
@@ -0,0 +1,20 @@
+// PR c++/118513
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+namespace std {
+ template struct tuple_size;
+ template struct tuple_element;
+}
+
+struct A {
+ int a, b;
+ template int &get () { if (I == 0) return a; else return b; }
+};
+
+template <> struct std::tuple_size { static const int value = 2; };
+template struct std::tuple_element { using type = int; };
+
+namespace {
+auto [x, y] = A { 42, 43 };
+}
--- gcc/testsuite/g++.dg/modules/decomp-3_b.C.jj2025-01-16
13:43:41.782807354 +0100
+++ gcc/testsuite/g++.dg/modules/decomp-3_b.C 2025-01-16 13:41:49.256346946
+0100
@@ -0,0 +1,12 @@
+// PR c++/118513
+// { dg-do run }
+// { dg-additional-options "-fmodules-ts" }
+
+import "decomp-3_a.H";
+
+int
+main ()
+{
+ if (x != 42 || y != 43)
+__builtin_abort ();
+}
Jakub
Re: [PATCH] i386: Fix wrong insn generated by shld/shrd ndd split [PR118510]
On Fri, Jan 17, 2025 at 9:50 AM Hongyu Wang wrote: > > Uros Bizjak 于2025年1月17日周五 15:05写道： > > > Is there a reason to have operand 0 with "nonimmediate_operand" > > predicate? If you have to generate a register temporary and then > > unconditionally copy it to the output, it is better to use > > "register_operand" predicate and leave middle end to do the copy for > > you. Please see the patch at comment #3 in the PR. > > Ah, thanks for pointing that out, yes the memory dest is meaningless here. > > Updated patch in attachment. > > Ok for trunk after bootstrap & regtest? Yes, LGTM. Thanks, Uros.
Re: [PATCH v3 1/2] RISC-V: Allocate the initial register in the expand phase for the vl of XTheadVector
> Since the parameter vl of XTheadVector does not support immediate numbers, we
> need
> to put it in the register in advance. That generates the initial code
> correctly.
>
> PR 116593
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vector-builtins.cc
> (function_expander::add_input_operand):
> Put immediate for vl to GPR for XTheadVector.
Generally both patches look reasonable to me and the change is less invasive
than going via spec_restriction.
How was this tested? The Rivos CI has already picked it up but please still
always specify. Thanks.
> + /* Since the parameter vl of XTheadVector does not support
> + immediate numbers, we need to put it in the register
> + in advance. */
> + if (TARGET_XTHEADVECTOR
> + && CONST_INT_P (x)
> + && base->apply_vl_p ()
> + && argno == (unsigned) (call_expr_nargs (exp) - 1)
> + && x != CONST0_RTX (GET_MODE (x)))
> +{
> + x = force_reg (word_mode, x);
> + add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> +}
> + else
> +add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> }
To me it feels as if this would better fit one level higher rather than
checking ARGNO and apply_vl_p here. Is that somehow too cumbersome?
> +# GCC testsuite that uses the `dg.exp' driver.
> +
> +# Test the front-end for C++.
> +# We don't need to test back-end code-gen in RV32 system for C++
> +# Because it is already tested in C.
> +# Exit immediately if this isn't a RISC-V target.
> +if ![istarget riscv*-*-*] then {
> + return
> +}
Might want to adjust those comments slightly ;)
--
Regards
Robin
[patch,avr] Add const attribute to built-in functions if possible.
Most of the avr built-in functions can be attributed "const".
Ok for trunk?
Johann
--
AVR: Add "const" attribute to avr built-in functions if possible.
gcc/
* config/avr/avr-c.cc (DEF_BUILTIN): Add ATTRS argument to macro
definition.
* config/avr/avr.cc: Same.
(avr_init_builtins) : New variable that can be used
as ATTRS argument in DEF_BUILTIN.
* config/avr/builtins.def (DEF_BUILTIN): Add ATTRS parameter
to all definitions.diff --git a/gcc/config/avr/avr-c.cc b/gcc/config/avr/avr-c.cc
index 67a97fa445c..53f15f2be7b 100644
--- a/gcc/config/avr/avr-c.cc
+++ b/gcc/config/avr/avr-c.cc
@@ -36,7 +36,7 @@
enum avr_builtin_id
{
-#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME) \
+#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME, ATTRS) \
AVR_BUILTIN_ ## NAME,
#include "builtins.def"
#undef DEF_BUILTIN
@@ -499,7 +499,7 @@ avr_cpu_cpp_builtins (cpp_reader *pfile)
/* Define builtin macros so that the user can easily query whether or
not a specific builtin is available. */
-#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME) \
+#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME, ATTRS) \
cpp_define (pfile, "__BUILTIN_AVR_" #NAME);
#include "builtins.def"
#undef DEF_BUILTIN
diff --git a/gcc/config/avr/avr.cc b/gcc/config/avr/avr.cc
index c44ed742453..3ef7ad4c96b 100644
--- a/gcc/config/avr/avr.cc
+++ b/gcc/config/avr/avr.cc
@@ -15291,7 +15291,7 @@ avr_out_insert_bits (rtx *op, int *plen)
enum avr_builtin_id
{
-#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME) \
+#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME, ATTRS) \
AVR_BUILTIN_ ## NAME,
#include "builtins.def"
#undef DEF_BUILTIN
@@ -15314,7 +15314,7 @@ struct GTY(()) avr_builtin_description
static GTY(()) avr_builtin_description
avr_bdesc[AVR_BUILTIN_COUNT] =
{
-#define DEF_BUILTIN(NAME, N_ARGS, TYPE, ICODE, LIBNAME) \
+#define DEF_BUILTIN(NAME, N_ARGS, TYPE, ICODE, LIBNAME, ATTRS) \
{ (enum insn_code) CODE_FOR_ ## ICODE, N_ARGS, NULL_TREE },
#include "builtins.def"
#undef DEF_BUILTIN
@@ -15514,8 +15514,9 @@ avr_init_builtins (void)
FX_FTYPE_INTX (ul);
FX_FTYPE_INTX (ull);
+ tree attr_const = tree_cons (get_identifier ("const"), NULL, NULL);
-#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME) \
+#define DEF_BUILTIN(NAME, N_ARGS, TYPE, CODE, LIBNAME, ATTRS) \
{ \
int id = AVR_BUILTIN_ ## NAME; \
const char *Name = "__builtin_avr_" #NAME; \
@@ -15524,7 +15525,7 @@ avr_init_builtins (void)
gcc_assert (id < AVR_BUILTIN_COUNT);\
avr_bdesc[id].fndecl\
= add_builtin_function (avr_tolower (name, Name), TYPE, id, \
- BUILT_IN_MD, LIBNAME, NULL_TREE); \
+ BUILT_IN_MD, LIBNAME, ATTRS); \
}
#include "builtins.def"
#undef DEF_BUILTIN
diff --git a/gcc/config/avr/builtins.def b/gcc/config/avr/builtins.def
index eeffb686631..61dbc3a6c1b 100644
--- a/gcc/config/avr/builtins.def
+++ b/gcc/config/avr/builtins.def
@@ -20,7 +20,7 @@
builtins defined in the AVR part of the GNU compiler.
Befor including this file, define a macro
- DEF_BUILTIN(NAME, N_ARGS, TYPE, ICODE, LIBNAME)
+ DEF_BUILTIN(NAME, N_ARGS, TYPE, ICODE, LIBNAME, ATTRS)
NAME:`__builtin_avr_name' will be the user-level name of the builtin.
`AVR_BUILTIN_NAME' will be the internal builtin's id.
@@ -30,31 +30,33 @@
ICODE: Name of attached insn or expander. If special treatment in avr.cc
is needed to expand the built-in, use `nothing'.
LIBNAME: Name of the attached implementation in libgcc which is used if
-the builtin cannot be folded away and there is no insn. */
+the builtin cannot be folded away and there is no insn.
+ ATTRS: Function attributes like "attr_const" for the `const' attribute
+or "NULL_TREE" for no attribute. */
/* Mapped to respective instruction. */
-DEF_BUILTIN (NOP, -1, void_ftype_void, nothing, NULL)
-DEF_BUILTIN (SEI, 0, void_ftype_void, enable_interrupt, NULL)
-DEF_BUILTIN (CLI, 0, void_ftype_void, disable_interrupt, NULL)
-DEF_BUILTIN (WDR, 0, void_ftype_void, wdr, NULL)
-DEF_BUILTIN (SLEEP, 0, void_ftype_void, sleep, NULL)
+DEF_BUILTIN (NOP, -1, void_ftype_void, nothing, NULL, NULL_TREE)
+DEF_BUILTIN (SEI, 0, void_ftype_void, enable_interrupt, NULL, NULL_TREE)
+DEF_BUILTIN (CLI, 0, void_ftype_void, disable_interrupt, NULL, NULL_TREE)
+DEF_BUILTIN (WDR, 0, void_ftype_void, wdr, NULL, NULL_TREE)
+DEF_BUILTIN (SLEEP, 0, void_ftype_void, sleep, NULL, NULL_TREE)
/* Mapped to respective instruction but might also be folded away
or emit as libgcc call if ISA does not provide the instruction. */
-DEF_BUILTIN (SWAP, 1, uintQ
Re: [PATCH v3 1/2] RISC-V: Allocate the initial register in the expand phase for the vl of XTheadVector
>> +# GCC testsuite that uses the `dg.exp' driver.
>> +
>> +# Test the front-end for C++.
>> +# We don't need to test back-end code-gen in RV32 system for C++
>> +# Because it is already tested in C.
>> +# Exit immediately if this isn't a RISC-V target.
>> +if ![istarget riscv*-*-*] then {
>> + return
>> +}
>
> Might want to adjust those comments slightly ;)
Apologies, I mixed up patches. Still it's not really a front-end test IMHO
but just a minor nit anyway.
--
Regards
Robin
Re: [PATCH] RISC-V: Disable fusing vsetvl instructions by VSETVL_VTYPE_CHANGE_ONLY for XTheadVector.
> In RVV 1.0, the instruction "vsetvli zero,zero,*" indicates that the > available vector length (avl) does not change. However, in XTheadVector, > this same instruction signifies that the avl should take the maximum value. > Consequently, when fusing vsetvl instructions, the optimization labeled > "VSETVL_VTYPE_CHANGE_ONLY" is disabled for XTheadVector. LGTM. -- Regards Robin
Re: [PATCH v4] AArch64: Add LUTI ACLE for SVE2
On 1/16/2025 8:44 AM, Richard Sandiford wrote:
Thanks for the update. Mostly LGTM, but some comments below:
writes:
diff --git a/gcc/config/aarch64/aarch64-sve2.md
b/gcc/config/aarch64/aarch64-sve2.md
index f8cfe08f4c0..0a1dc314f94 100644
--- a/gcc/config/aarch64/aarch64-sve2.md
+++ b/gcc/config/aarch64/aarch64-sve2.md
@@ -133,6 +133,7 @@
;; Optional AES extensions
;; Optional SHA-3 extensions
;; Optional SM4 extensions
+;; Table lookup
This puts it under:
;; == Cryptographic extensions
but it's not a crytographic extension. Probably better to put it under:
;; == General
instead.
;; =
;; == Moves
@@ -4211,3 +4212,35 @@
"sm4ekey\t%0.s, %1.s, %2.s"
[(set_attr "type" "crypto_sm4")]
)
+
+;; -
+;; Table lookup
+;; -
+;; Includes:
+;; - LUTI2
+;; - LUTI4
+;; -
+
+(define_insn "@aarch64_sve_luti"
+ [(set (match_operand:SVE_FULL_BH 0 "register_operand" "=w")
+ (unspec:SVE_FULL_BH
+[(match_operand:SVE_FULL_BH 1 "register_operand" "w")
+ (match_operand:VNx16QI 2 "register_operand" "w")
This is correct
+ (match_operand:DI 3 "const_int_operand")
+ (const_int LUTI_BITS)]
+UNSPEC_SVE_LUTI))]
+ "TARGET_LUT && TARGET_SVE2_OR_SME2"
+ "luti\t%0., { %1. }, %2[%3]"
+)
+
+(define_insn "@aarch64_sve_luti"
+ [(set (match_operand: 0 "register_operand" "=w")
+ (unspec:
+[(match_operand:SVE_FULL_Hx2 1 "register_operand" "Uw2")
...but this should use aligned_register_operand instead of
register_operand.
Are you sure we want it to be aligned? That requirement is not there on
the { .H, .H } operand here:
https://developer.arm.com/documentation/ddi0602/2024-12/SVE-Instructions/LUTI4--Lookup-table-read-with-4-bit-indices-?lang=en,
as in, nothing like it has to be "Zn times 2"
Agree with the rest of the review.
+ (match_operand:VNx16QI 2 "register_operand" "w")
+ (match_operand:DI 3 "const_int_operand")
+ (const_int LUTI_BITS)]
+ UNSPEC_SVE_LUTI))]
+ "TARGET_LUT && TARGET_SVE2_OR_SME2"
+ "luti\t%0., %1, %2[%3]"
+)
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index ff0f34dd043..0fbf96f1ab9 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -553,6 +553,18 @@
(define_mode_iterator SVE_FULL_BHS [VNx16QI VNx8HI VNx4SI
VNx8BF VNx8HF VNx4SF])
+;; Fully-packed SVE vector byte modes that have 32-bit or smaller elements.
+(define_mode_iterator SVE_FULL_BS [VNx16QI VNx4SI VNx4SF])
This is no longer needed.
+
+;; Fully-packed SVE vector byte modes that have 16-bit or smaller elements.
+(define_mode_iterator SVE_FULL_BH [VNx16QI VNx8HI VNx8HF VNx8BF])
+
+;; Fully-packed half word SVE vector modes
+(define_mode_iterator SVE_FULL_H [VNx8HI VNx8HF VNx8BF])
Similarly, SVE_FULL_H is no longer needed.
+
+;; Pairs of fully-packed SVE vector modes (half word only)
+(define_mode_iterator SVE_FULL_Hx2 [VNx16HI VNx16HF VNx16BF])
+
;; Fully-packed SVE vector modes that have 32-bit elements.
(define_mode_iterator SVE_FULL_S [VNx4SI VNx4SF])
@@ -1186,6 +1198,7 @@
UNSPEC_UZPQ2
UNSPEC_ZIPQ1
UNSPEC_ZIPQ2
+UNSPEC_SVE_LUTI
;; All used in aarch64-sme.md
UNSPEC_SME_ADD
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/asm/test_sve_acle.h
b/gcc/testsuite/gcc.target/aarch64/sve/acle/asm/test_sve_acle.h
index d3ae707ac49..c0dd89fa924 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/acle/asm/test_sve_acle.h
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/asm/test_sve_acle.h
@@ -780,4 +780,20 @@
"w" (z16), "w" (z22), "w" (z29)); \
}
+#define TEST_1X2_NARROW(NAME, RTYPE, TTYPE, ZTYPE, CODE1, CODE2) \
+ PROTO(NAME, void, ())
\
+ {\
+register RTYPE z0 __asm ("z0"); \
+register ZTYPE z5 __asm ("z5"); \
+register TTYPE z6 __asm ("z6"); \
+register RTYPE z16 __asm ("z16");\
+register ZTYPE z22 __asm ("z22");\
+register TTYPE z29 __asm ("z29");\
+register RTYPE z0_res __asm ("z0"); \
+__asm volatile ("" : "=w" (z0), "=w" (z5), "=w" (z6), \
+ "=w" (z16), "=w" (z22), "=w" (z29)); \
+INVOKE (CODE1, CODE2); \
+__asm volati
[PATCH] testsuite/117958 - ifcombine differences on aarch64 vs rest
ifcombine depends on BRANCH_COST and the testcase relies on ifcombine
to fully optimize the function. But the important parts are optimized
everywhere, so the following delectively XFAILs the less important part.
Tested on aarch64 and x86_64-unknown-linux-gnu, pushed.
PR testsuite/117958
* g++.dg/tree-ssa/pr117123.C: XFAIL parts on aarch64-*-*.
---
gcc/testsuite/g++.dg/tree-ssa/pr117123.C | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr117123.C
b/gcc/testsuite/g++.dg/tree-ssa/pr117123.C
index 2aa2810de95..29b69dfa432 100644
--- a/gcc/testsuite/g++.dg/tree-ssa/pr117123.C
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr117123.C
@@ -49,4 +49,6 @@ int patatino(int a) {
}
// { dg-final { scan-tree-dump-not "dont_be_here" "optimized" } }
-// { dg-final { scan-tree-dump-times "if " 3 "optimized" } }
+// Depending on LOGICAL_OP_NON_SHORT_CIRCUIT (or BRANCH_COST) this might
+// or might not be optimized fully
+// { dg-final { scan-tree-dump-times "if " 3 "optimized" { xfail { aarch64-*-*
} } } }
--
2.43.0
Re: [PATCH] MAINTAINERS: Add myself to write after approval
Ping BR, Jin
Re: [PATCH] MAINTAINERS: Add myself to write after approval
See https://gcc.gnu.org/gitwrite.html#authenticated.
Re: [PATCH] AArch64: Deprecate -mabi=ilp32
Tamar Christina writes:
>> -Original Message-
>> From: Wilco Dijkstra
>> Sent: Tuesday, January 14, 2025 5:30 PM
>> To: Richard Sandiford
>> Cc: Richard Earnshaw ; ktkac...@nvidia.com; GCC
>> Patches ; sch...@linux-m68k.org
>> Subject: Re: [PATCH] AArch64: Deprecate -mabi=ilp32
>>
>> Hi Richard,
>>
>> >> + if (TARGET_ILP32)
>> >> + warning (OPT_Wdeprecated, "%<-mabi=ilp32%> is deprecated.");
>> >
>> > There should be no "." at the end of the message.
>>
>> Right, fixed in v2 below.
>>
>> > Otherwise it looks good to me, although like Kyrill says, it'll also
>> > need a release note.
>>
>> I've added one, see https://gcc.gnu.org/pipermail/gcc-patches/2025-
>> January/673575.html
>>
>> > Please give others 24 hours to comment. Otherwise the patch is OK
>> > with that change.
>>
>> Sure.
>>
>> > (I saw Andreas's comment about starting sentences with a lowercase
>> > letter, but IMO that's ok for program text.)
>>
>> I changed it to "The @samp{ilp32} model is deprecated." (using "option" felt
>> wrong here).
>>
>> Cheers,
>> Wilco
>>
>>
>> v2: Update after review
>>
>> ILP32 was originally intended to make porting to AArch64 easier. Support was
>> never merged in the Linux kernel or GLIBC, so it has been unsupported for
>> many
>> years. There isn't a benefit in keeping unsupported features forever, so
>> deprecate it now (and it could be removed in a future release).
>>
>> Passes regress & bootstrap, OK for commit?
>
> Hi,
>
> This patch seems to break embedded builds as on -elf platforms we build ilp32
> as
> a default multilib variant.
>
> That is in gcc/config.gcc
>
> aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*
>
> all do:
>
> aarch64_multilibs="${with_multilib_list}"
> if test "$aarch64_multilibs" = "default"; then
> aarch64_multilibs="lp64,ilp32"
> fi
>
> Which then fails the build because of -Werror.
>
> So the question is as the discussion above has been about Linux, should the
> deprecation just be on Linux or also ELF, and if ELF should we remove it from
> the default multilib?
FWIW, my vote would be for removing the multilib.
Thanks,
Richard
Re: [RFC] PR81358: Enable automatic linking of libatomic
Prathamesh Kulkarni writes:
>> -Original Message-
>> From: Prathamesh Kulkarni
>> Sent: 10 January 2025 09:48
>> To: Thomas Schwinge
>> Cc: Tobias Burnus ; Joseph Myers
>> ; Xi Ruoyao ; Matthew
>> Malcomson ; gcc-patches@gcc.gnu.org; Tom de
>> Vries
>> Subject: RE: [RFC] PR81358: Enable automatic linking of libatomic
>>
>> External email: Use caution opening links or attachments
>>
>>
>> > -Original Message-
>> > From: Thomas Schwinge
>> > Sent: 07 January 2025 17:45
>> > To: Prathamesh Kulkarni
>> > Cc: Tobias Burnus ; Joseph Myers
>> > ; Xi Ruoyao ; Matthew
>> > Malcomson ; gcc-patches@gcc.gnu.org; Tom de
>> > Vries
>> > Subject: RE: [RFC] PR81358: Enable automatic linking of libatomic
>> >
>> > External email: Use caution opening links or attachments
>> >
>> >
>> > Hi Prathamesh!
>> Hi Thomas, thanks for the review!
>> >
>> > Thanks for working on this!
>> >
>> >
>> > Per my understanding, this patch won't automagically resolve the
>> need
>> > to
>> > (occasionally) having to specify '-foffload-options=nvptx-none=-
>> > latomic'
>> > for nvptx offloading: it doesn't use 'LINK_LIBATOMIC_SPEC',
>> currently
>> > only used via 'GNU_USER_TARGET_LINK_GCC_C_SEQUENCE_SPEC' from
>> > 'gcc/config/gnu-user.h' (general issue, affecting a lot of
>> > configurations, to be addressed as necessary):
>> >
>> > > --- a/gcc/config/gnu-user.h
>> > > +++ b/gcc/config/gnu-user.h
>> >
>> > > #define GNU_USER_TARGET_LINK_GCC_C_SEQUENCE_SPEC \
>> > > - "%{static|static-pie:--start-group} %G %{!nolibc:%L} \
>> > > + "%{static|static-pie:--start-group} %G %{!nolibc:"
>> > > + LINK_LIBATOMIC_SPEC "%L} \
>> > > %{static|static-pie:--end-group}%{!static:%{!static-pie:%G}}"
>> >
>> > > --- a/gcc/gcc.cc
>> > > +++ b/gcc/gcc.cc
>> >
>> > > /* Here is the spec for running the linker, after compiling all
>> > > files. */
>> > >
>> > > +#if defined(TARGET_PROVIDES_LIBATOMIC) &&
>> defined(USE_LD_AS_NEEDED)
>> > > +#define LINK_LIBATOMIC_SPEC "%{!fno-link-libatomic:"
>> > LD_AS_NEEDED_OPTION \
>> > > + " -latomic " LD_NO_AS_NEEDED_OPTION "} "
>> > > +#else
>> > > +#define LINK_LIBATOMIC_SPEC ""
>> > > +#endif
>> > > +
>> > > /* This is overridable by the target in case they need to specify
>> > the
>> > > -lgcc and -lc order specially, yet not require them to
>> override
>> > all
>> > > of LINK_COMMAND_SPEC. */
>> >
>> > ..., and the nvptx linker doesn't support '--as-needed'.
>> >
>> > I'll think about it; indeed it'd be good to get that resolved, too.
>> >
>> >
>> > On 2024-12-20T15:37:42+, Prathamesh Kulkarni
>> > wrote:
>> > > [...] copying libatomic.a over to $(gcc_objdir)$(MULTISUBDIR)/,
>> and
>> > > can confirm that 64-bit libatomic.a is copied to $build/gcc/ and
>> 32-
>> > bit libatomic.a is copied to $build/gcc/32/.
>> >
>> > So this:
>> >
>> > > --- a/libatomic/Makefile.am
>> > > +++ b/libatomic/Makefile.am
>> >
>> > > @@ -162,6 +162,11 @@ libatomic_convenience_la_LIBADD =
>> > > $(libatomic_la_LIBADD) # when it is reloaded during the build of
>> > all-multi.
>> > > all-multi: $(libatomic_la_LIBADD)
>> > >
>> > > +gcc_objdir = $(MULTIBUILDTOP)../../$(host_subdir)/gcc
>> > > +all: all-multi libatomic.la libatomic_convenience.la
>> > > + $(INSTALL_DATA) .libs/libatomic.a
>> $(gcc_objdir)$(MULTISUBDIR)/
>> > > + chmod 644 $(gcc_objdir)$(MULTISUBDIR)/libatomic.a
>> >
>> > ... is conceptually modelled after libgcc, where the libraries get
>> > copied into 'gcc/'? However, here we only copy the static
>> > 'libatomic.a'; libgcc does a 'make install-leaf', see
>> > 'libgcc/Makefile.in':
>> >
>> > all: all-multi
>> > # Now that we have built all the objects, we need to copy
>> > # them back to the GCC directory. Too many things (other
>> > # in-tree libraries, and DejaGNU) know about the layout
>> > # of the build tree, for now.
>> > $(MAKE) install-leaf DESTDIR=$(gcc_objdir) \
>> > slibdir= libsubdir= MULTIOSDIR=$(MULTIDIR)
>> >
>> > ..., which also installs dynamic libraries. Is that difference
>> > intentional and/or possibly important?
>> Well, I wasn't sure what extension to use for shared libraries, and
>> initially avoided copying them.
>> libgcc seems to use $(SHLIB_EXT) to specify extension name for shared
>> libraries, which can be overridden by targets.
>>
>> Matthew pointed out to me that using libtool --mode=install works for
>> copying both static and shared libraries (with the numbered version
>> libatomic.so.1.2.0), so in the attached patch, I changed Makefile.am
>> rule to following:
>> gcc_objdir = `pwd`/$(MULTIBUILDTOP)../../gcc/
>> all: all-multi libatomic.la libatomic_convenience.la
>> $(LIBTOOL) --mode=install $(INSTALL_DATA) libatomic.la
>> $(gcc_objdir)$(MULTISUBDIR)/
>>
>> Which seems to install libatomic.a, libatomic.so and the numbered
>> version in $build/gcc/ and in $build/gcc/32/ (and $build/gcc/mgomp/
>> for nvptx build).
>> Does it look OK ?
>> >
>> >
Re: [PATCH v4] AArch64: Add LUTI ACLE for SVE2
Saurabh Jha writes:
> On 1/16/2025 8:44 AM, Richard Sandiford wrote:
>> Thanks for the update. Mostly LGTM, but some comments below:
>>
>> writes:
>>> diff --git a/gcc/config/aarch64/aarch64-sve2.md
>>> b/gcc/config/aarch64/aarch64-sve2.md
>>> index f8cfe08f4c0..0a1dc314f94 100644
>>> --- a/gcc/config/aarch64/aarch64-sve2.md
>>> +++ b/gcc/config/aarch64/aarch64-sve2.md
>>> @@ -133,6 +133,7 @@
>>> ;; Optional AES extensions
>>> ;; Optional SHA-3 extensions
>>> ;; Optional SM4 extensions
>>> +;; Table lookup
>>
>> This puts it under:
>>
>> ;; == Cryptographic extensions
>>
>> but it's not a crytographic extension. Probably better to put it under:
>>
>> ;; == General
>>
>> instead.
>>
>>> ;;
>>> =
>>> ;; == Moves
>>> @@ -4211,3 +4212,35 @@
>>> "sm4ekey\t%0.s, %1.s, %2.s"
>>> [(set_attr "type" "crypto_sm4")]
>>> )
>>> +
>>> +;;
>>> -
>>> +;; Table lookup
>>> +;;
>>> -
>>> +;; Includes:
>>> +;; - LUTI2
>>> +;; - LUTI4
>>> +;;
>>> -
>>> +
>>> +(define_insn "@aarch64_sve_luti"
>>> + [(set (match_operand:SVE_FULL_BH 0 "register_operand" "=w")
>>> + (unspec:SVE_FULL_BH
>>> +[(match_operand:SVE_FULL_BH 1 "register_operand" "w")
>>> + (match_operand:VNx16QI 2 "register_operand" "w")
>>
>> This is correct
>>
>>> + (match_operand:DI 3 "const_int_operand")
>>> + (const_int LUTI_BITS)]
>>> +UNSPEC_SVE_LUTI))]
>>> + "TARGET_LUT && TARGET_SVE2_OR_SME2"
>>> + "luti\t%0., { %1. }, %2[%3]"
>>> +)
>>> +
>>> +(define_insn "@aarch64_sve_luti"
>>> + [(set (match_operand: 0 "register_operand" "=w")
>>> + (unspec:
>>> +[(match_operand:SVE_FULL_Hx2 1 "register_operand" "Uw2")
>>
>> ...but this should use aligned_register_operand instead of
>> register_operand.
> Are you sure we want it to be aligned? That requirement is not there on
> the { .H, .H } operand here:
> https://developer.arm.com/documentation/ddi0602/2024-12/SVE-Instructions/LUTI4--Lookup-table-read-with-4-bit-indices-?lang=en,
>
> as in, nothing like it has to be "Zn times 2
Ah, right, sorry. I'd even said that last time:
> This operand also isn't required to be aligned: Zn has a 5-bit encoding.
but got it the wrong way round when doing this review. :(
But in that case the constraint should be "w" rather than "Uw2".
"Uw2" is for aligned registers only:
(define_register_constraint "Uw2" "FP_REGS"
"Even floating point and SIMD vector registers."
"regno % 2 == 0")
Richard
Re: [RFC] PR81358: Enable automatic linking of libatomic
On Fri, Jan 17, 2025 at 12:37:49PM +, Sam James wrote: > So far, testing has gone well on my end (multilib issues fixed too), but > I suspect it introduced an issue with RPATH: libtool install into gcc/$(MULTISUBDIR) isn't a good idea. Jakub
Re: [PATCH] s390: Replace some checking assertions with output_operand_lossage [PR118511]
On Fri, Jan 17, 2025 at 10:47:09AM +0100, Jakub Jelinek wrote:
> Hi!
>
> r15-2002 s390: Fully exploit vgm, vgbm, vrepi change added
> some code to print_operand and added gcc_checking_asserts in there.
> But print_operand ideally should have no assertions in it, as most
> of the assumptions can be easily violated by people using it in
> inline asm.
> This issue in particular was seen by failure to compile s390-tools,
> which had in its extended inline asm uses of %r1 and %r2.
> I really don't know if they meant %%r1 and %%r2 or %1 and %2 and
> will leave that decision to the maintainers, but the thing is that
https://github.com/ibm-s390-linux/s390-tools/pull/180
> %r1 and %r2 used to expand like %1 and %2 in GCC 14 and earlier,
> now in checking build it ICEs and in --enable-checking=release build
> fails to assemble (the checking assert is ignored and the compiler just uses
> some uninitialized variables to emit something arbitrary).
>
> With the following patch it is diagnosed as error consistently
> regardless if it is release checking or no checking or checking compiler.
>
> Bootstrapped/regtested on s390x-linux, ok for trunk?
Ok.
Thanks for fixing and pointing this out. Much appreciated.
>
> Note, I see also
> else if (GET_CODE (x) == UNSPEC && XINT (x, 1) == UNSPEC_TLSLDM)
> {
> fprintf (file, "%s", ":tls_ldcall:");
> const char *name = get_some_local_dynamic_name ();
> gcc_assert (name);
> assemble_name (file, name);
> }
> in print_operand, maybe that isn't a big deal because it might be
> impossible to construct inline asm argument which is UNSPEC_TLSLDM.
> And then there is
> case 'e': case 'f':
> case 's': case 't':
> {
> int start, end;
> int len;
> bool ok;
>
> len = (code == 's' || code == 'e' ? 64 : 32);
> ok = s390_contiguous_bitmask_p (ival, true, len, &start, &end);
> gcc_assert (ok);
> if (code == 's' || code == 't')
> ival = start;
> else
> ival = end;
> }
> break;
> which likely should be also output_operand_lossage but I haven't tried
> to reproduce that.
AFAICS those shouldn't be constructable via inline asm arguments.
However, I agree that asserts are not ideal here. For example, in
*insv_z10_noshift_cc the second alternative uses %e for an operand which
isn't guarded by predicate contiguous_bitmask_operand. Though, I
haven't come up with a reproducer either.
Cheers,
Stefan
>
> 2025-01-17 Jakub Jelinek
>
> PR target/118511
> * config/s390/s390.cc (print_operand) : Use
> output_operand_lossage instead of gcc_checking_assert.
> (print_operand) : Likewise.
> (print_operand) : Likewise.
>
> * gcc.target/s390/pr118511.c: New test.
>
> --- gcc/config/s390/s390.cc.jj2025-01-10 16:09:37.280043199 +0100
> +++ gcc/config/s390/s390.cc 2025-01-16 10:43:58.533809724 +0100
> @@ -8688,8 +8688,12 @@ print_operand (FILE *file, rtx x, int co
>{
> machine_mode mode;
> short imm;
> - bool b = s390_constant_via_vrepi_p (x, &mode, &imm);
> - gcc_checking_assert (b);
> + if (!s390_constant_via_vrepi_p (x, &mode, &imm))
> + {
> + output_operand_lossage ("invalid constant for output "
> + "modifier '%c'", code);
> + return;
> + }
> switch (mode)
> {
> case QImode:
> @@ -8714,8 +8718,12 @@ print_operand (FILE *file, rtx x, int co
>{
> machine_mode mode;
> int start, end;
> - bool b = s390_constant_via_vgm_p (x, &mode, &start, &end);
> - gcc_checking_assert (b);
> + if (!s390_constant_via_vgm_p (x, &mode, &start, &end))
> + {
> + output_operand_lossage ("invalid constant for output "
> + "modifier '%c'", code);
> + return;
> + }
> switch (mode)
> {
> case QImode:
> @@ -8739,8 +8747,12 @@ print_operand (FILE *file, rtx x, int co
> case 'r':
>{
> unsigned mask;
> - bool b = s390_constant_via_vgbm_p (x, &mask);
> - gcc_checking_assert (b);
> + if (!s390_constant_via_vgbm_p (x, &mask))
> + {
> + output_operand_lossage ("invalid constant for output "
> + "modifier '%c'", code);
> + return;
> + }
> fprintf (file, "%u", mask);
>}
>return;
> --- gcc/testsuite/gcc.target/s390/pr118511.c.jj 2025-01-16
> 10:51:14.025721964 +0100
> +++ gcc/testsuite/gcc.target/s390/pr118511.c 2025-01-16 10:50:48.009085651
> +0100
> @@ -0,0 +1,11 @@
> +/* PR target/118511 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void
> +foo (void)
> +{
> + asm volatile ("# %p0" : : "d" (1));/* { dg-error "invalid 'asm':
> invalid constant for output modifier 'p'" } */
> + asm volat
Re: [PATCH] c++: Allow pragmas in NSDMIs [PR118147]
On 12/20/24 6:17 AM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk and
maybe release branches?
OK for all.
-- >8 --
This patch removes the (unnecessary) CPP_PRAGMA_EOL case from
cp_parser_cache_defarg, which currently has the result that any pragmas
in the NSDMI cause an error.
PR c++/118147
gcc/cp/ChangeLog:
* parser.cc (cp_parser_cache_defarg): Don't error when
CPP_PRAGMA_EOL.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/nsdmi-defer7.C: New test.
Signed-off-by: Nathaniel Shead
---
gcc/cp/parser.cc | 1 -
gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C | 13 +
2 files changed, 13 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 23c6a2fd30e..527a858ea52 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -36316,7 +36316,6 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
/* If we run out of tokens, issue an error message. */
case CPP_EOF:
- case CPP_PRAGMA_EOL:
error_at (token->location, "file ends in default argument");
return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
new file mode 100644
index 000..3bef636ccbd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
@@ -0,0 +1,13 @@
+// PR c++/118147
+// { dg-do compile { target c++11 } }
+
+struct F {
+ int i = []{
+#pragma message "test" // { dg-message "test" }
+return 1;
+ }();
+};
+
+struct G {
+ int i =
+#pragma GCC diagnostic push // { dg-error "file ends in default
argument|expected" }
Re: [PATCH] c++/modules: Propagate FNDECL_USED_AUTO alongside deduced return types [PR118049]
On 1/17/25 5:50 AM, Nathaniel Shead wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
OK.
-- >8 --
In the linked testcase, we're erroring because the declared return types
of the functions do not appear to match. This is because when merging
the deduced return types for 'foo' in 'auto-5_b.C', we overwrote the
return type for the declaration with the deduced return type from
'auto-5_a.C' but neglected to track that we were originally declared
with 'auto'.
As a drive-by improvement to QOI, also add checks for if the deduced
return types do not match; this is currently useful because we do not
check the equivalence of the bodies of functions yet.
PR c++/118049
gcc/cp/ChangeLog:
* module.cc (trees_in::is_matching_decl): Propagate
FNDECL_USED_AUTO as well.
gcc/testsuite/ChangeLog:
* g++.dg/modules/auto-5_a.C: New test.
* g++.dg/modules/auto-5_b.C: New test.
* g++.dg/modules/auto-5_c.C: New test.
* g++.dg/modules/auto-6_a.H: New test.
* g++.dg/modules/auto-6_b.C: New test.
Signed-off-by: Nathaniel Shead
---
gcc/cp/module.cc| 5 +
gcc/testsuite/g++.dg/modules/auto-5_a.C | 10 ++
gcc/testsuite/g++.dg/modules/auto-5_b.C | 14 ++
gcc/testsuite/g++.dg/modules/auto-5_c.C | 4
gcc/testsuite/g++.dg/modules/auto-6_a.H | 5 +
gcc/testsuite/g++.dg/modules/auto-6_b.C | 6 ++
6 files changed, 44 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 61116fe7669..6fe64bb538c 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11902,8 +11902,13 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
{
dump (dumper::MERGE)
&& dump ("Propagating deduced return type to %N", existing);
+ FNDECL_USED_AUTO (e_inner) = true;
+ DECL_SAVED_AUTO_RETURN_TYPE (existing) = TREE_TYPE (e_type);
TREE_TYPE (existing) = change_return_type (TREE_TYPE (d_type),
e_type);
}
+ else if (type_uses_auto (d_ret)
+ && !same_type_p (TREE_TYPE (d_type), TREE_TYPE (e_type)))
+ goto mismatch;
}
else if (is_typedef)
{
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_a.C
b/gcc/testsuite/g++.dg/modules/auto-5_a.C
new file mode 100644
index 000..fcab6f301e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_a.C
@@ -0,0 +1,10 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -Wno-global-module" }
+// { dg-module-cmi A }
+
+module;
+template struct S {
+ auto foo() {}
+};
+export module A;
+template struct S;
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_b.C
b/gcc/testsuite/g++.dg/modules/auto-5_b.C
new file mode 100644
index 000..f75ed2d0f0c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_b.C
@@ -0,0 +1,14 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -Wno-global-module" }
+// { dg-module-cmi B }
+
+module;
+template struct S {
+ auto foo() {}
+};
+template struct S;
+export module B;
+import A;
+template void x() {
+ S{}.foo();
+}
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_c.C
b/gcc/testsuite/g++.dg/modules/auto-5_c.C
new file mode 100644
index 000..f351c2b1ae4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_c.C
@@ -0,0 +1,4 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -fno-module-lazy
-fdump-lang-module-alias" }
+
+import B;
diff --git a/gcc/testsuite/g++.dg/modules/auto-6_a.H
b/gcc/testsuite/g++.dg/modules/auto-6_a.H
new file mode 100644
index 000..3ad08ab81ce
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-6_a.H
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodule-header" }
+
+inline auto foo() {
+ return 1;
+}
diff --git a/gcc/testsuite/g++.dg/modules/auto-6_b.C
b/gcc/testsuite/g++.dg/modules/auto-6_b.C
new file mode 100644
index 000..aab7be4e530
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-6_b.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+inline auto foo() { // { dg-error "conflicting" }
+ return 1.0;
+}
+import "auto-6_a.H";
[PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
Hi All,
Following the deprecation of ILP32 *-elf builds fail now due to -Werror on the
deprecation warning. This is because on embedded builds ILP32 is part of the
default multilib.
This patch removed it from the default target as the build would fail anyway.
Cross compiled on aarch64-none-elf and build succeeds now.
Ok for master?
Sending this out in case there are no objections to the approach
Thanks,
Tamar
gcc/ChangeLog:
* config.gcc (aarch64-*-elf): Drop ILP32 from default multilibs.
---
diff --git a/gcc/config.gcc b/gcc/config.gcc
index
55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c3a0ad91905286777
100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
esac
aarch64_multilibs="${with_multilib_list}"
if test "$aarch64_multilibs" = "default"; then
- aarch64_multilibs="lp64,ilp32"
+ aarch64_multilibs="lp64"
fi
aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
for aarch64_multilib in ${aarch64_multilibs}; do
--
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c3a0ad91905286777 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*)
esac
aarch64_multilibs="${with_multilib_list}"
if test "$aarch64_multilibs" = "default"; then
- aarch64_multilibs="lp64,ilp32"
+ aarch64_multilibs="lp64"
fi
aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
for aarch64_multilib in ${aarch64_multilibs}; do
RE: [PATCH v3 1/2] aarch64: Use standard names for saturating arithmetic
> -Original Message-
> From: Richard Sandiford
> Sent: Friday, January 10, 2025 4:50 PM
> To: Akram Ahmad
> Cc: ktkac...@nvidia.com; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH v3 1/2] aarch64: Use standard names for saturating
> arithmetic
>
> Akram Ahmad writes:
> > Ah whoops- I didn't see this before sending off V4 just now, my apologies.
> > I'll try my best to get this implemented before the end of the day so that
> > it doesn't miss the deadline.
>
> No rush! The delay here is entirely my fault, so no problem if the
> patch lands early stage 4.
>
Hi, I'm picking up the remainder of the patch for Akram.
I believe I have addressed all the review comments, and I've also rebased to
master which needed some small changes.
Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
Ok for master?
Thanks,
Tamar
gcc/ChangeLog:
* config/aarch64/aarch64-builtins.cc: Expand iterators.
* config/aarch64/aarch64-simd-builtins.def: Use standard names
* config/aarch64/aarch64-simd.md: Use standard names, split insn
definitions on signedness of operator and type of operands.
* config/aarch64/arm_neon.h: Use standard builtin names.
* config/aarch64/iterators.md: Add VSDQ_I_QI_HI iterator to
simplify splitting of insn for unsigned scalar arithmetic.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/scalar_intrinsics.c: Update testcases.
*
gcc.target/aarch64/advsimd-intrinsics/saturating_arithmetic_autovect.inc:
Template file for unsigned vector saturating arithmetic tests.
*
gcc.target/aarch64/advsimd-intrinsics/saturating_arithmetic_autovect_1.c:
8-bit vector type tests.
*
gcc.target/aarch64/advsimd-intrinsics/saturating_arithmetic_autovect_2.c:
16-bit vector type tests.
*
gcc.target/aarch64/advsimd-intrinsics/saturating_arithmetic_autovect_3.c:
32-bit vector type tests.
*
gcc.target/aarch64/advsimd-intrinsics/saturating_arithmetic_autovect_4.c:
64-bit vector type tests.
* gcc.target/aarch64/saturating_arithmetic.inc: Template file
for scalar saturating arithmetic tests.
* gcc.target/aarch64/saturating_arithmetic_1.c: 8-bit tests.
* gcc.target/aarch64/saturating_arithmetic_2.c: 16-bit tests.
* gcc.target/aarch64/saturating_arithmetic_3.c: 32-bit tests.
* gcc.target/aarch64/saturating_arithmetic_4.c: 64-bit tests.
Co-authored-by: Tamar Christina
-- inline copy --
diff --git a/gcc/config/aarch64/aarch64-builtins.cc
b/gcc/config/aarch64/aarch64-builtins.cc
index
86eebc16885976387d941efc61c55975359b6099..6d5479c2e4492078312b05561d682ead9e9c2d13
100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -5039,6 +5039,18 @@ aarch64_general_gimple_fold_builtin (unsigned int fcode,
gcall *stmt,
new_stmt = gimple_build_assign (gimple_call_lhs (stmt),
LSHIFT_EXPR, args[0], args[1]);
break;
+ /* lower saturating add/sub neon builtins to gimple. */
+ BUILTIN_VSDQ_I (BINOP, ssadd, 3, DEFAULT)
+ BUILTIN_VSDQ_I (BINOPU, usadd, 3, DEFAULT)
+ new_stmt = gimple_build_call_internal (IFN_SAT_ADD, 2, args[0],
args[1]);
+ gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt));
+ break;
+ BUILTIN_VSDQ_I (BINOP, sssub, 3, DEFAULT)
+ BUILTIN_VSDQ_I (BINOPU, ussub, 3, DEFAULT)
+ new_stmt = gimple_build_call_internal (IFN_SAT_SUB, 2, args[0],
args[1]);
+ gimple_call_set_lhs (new_stmt, gimple_call_lhs (stmt));
+ break;
+
BUILTIN_VSDQ_I_DI (BINOP, sshl, 0, DEFAULT)
BUILTIN_VSDQ_I_DI (BINOP_UUS, ushl, 0, DEFAULT)
{
diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def
b/gcc/config/aarch64/aarch64-simd-builtins.def
index
286272a331180d7682826dc3ec9e921b1ebbeab6..6cc45b18a723fc9621e2da6220231e61b621185a
100644
--- a/gcc/config/aarch64/aarch64-simd-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-builtins.def
@@ -71,10 +71,10 @@
BUILTIN_VSDQ_I (BINOP, sqrshl, 0, DEFAULT)
BUILTIN_VSDQ_I (BINOP_UUS, uqrshl, 0, DEFAULT)
/* Implemented by aarch64_. */
- BUILTIN_VSDQ_I (BINOP, sqadd, 0, DEFAULT)
- BUILTIN_VSDQ_I (BINOPU, uqadd, 0, DEFAULT)
- BUILTIN_VSDQ_I (BINOP, sqsub, 0, DEFAULT)
- BUILTIN_VSDQ_I (BINOPU, uqsub, 0, DEFAULT)
+ BUILTIN_VSDQ_I (BINOP, ssadd, 3, DEFAULT)
+ BUILTIN_VSDQ_I (BINOPU, usadd, 3, DEFAULT)
+ BUILTIN_VSDQ_I (BINOP, sssub, 3, DEFAULT)
+ BUILTIN_VSDQ_I (BINOPU, ussub, 3, DEFAULT)
/* Implemented by aarch64_qadd. */
BUILTIN_VSDQ_I (BINOP_SSU, suqadd, 0, DEFAULT)
BUILTIN_VSDQ_I (BINOP_UUS, usqadd, 0, DEFAULT)
diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index
eeb626f129a812d11de8c7b90cb20633739baac6..e2afe87e5130cc066b8348659209ab40747327e5
100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -5162,15 +5162,
Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> On 17 Jan 2025, at 13:56, Tamar Christina wrote:
>
> Hi All,
>
> Following the deprecation of ILP32 *-elf builds fail now due to -Werror on the
> deprecation warning. This is because on embedded builds ILP32 is part of the
> default multilib.
>
> This patch removed it from the default target as the build would fail anyway.
>
> Cross compiled on aarch64-none-elf and build succeeds now.
>
> Ok for master?
I was going to say that it’s more common to just announce deprecation in the
documentation/release notes for one release cycle, to give time for potential
users to come forward.
If we remove the multilib build now we’d be helping the support bitrot much
faster, which would make it harder to restore if such a user does come forward.
But it also seems odd to build an entire multilib for a deprecated feature so
I’m not opposed to it.
I don’t know if we document the ILP32 multilib somewhere. If so, it should be
updated.
In any case the gcc-15 release notes should also mention this change.
Thanks,
Kyrill
>
> Sending this out in case there are no objections to the approach
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> * config.gcc (aarch64-*-elf): Drop ILP32 from default multilibs.
>
> ---
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index
> 55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c3a0ad91905286777
> 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* |
> aarch64*-*-rtems*)
> esac
> aarch64_multilibs="${with_multilib_list}"
> if test "$aarch64_multilibs" = "default"; then
> - aarch64_multilibs="lp64,ilp32"
> + aarch64_multilibs="lp64"
> fi
> aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
> for aarch64_multilib in ${aarch64_multilibs}; do
>
>
>
>
> --
>
RE: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> -Original Message-
> From: Kyrylo Tkachov
> Sent: Friday, January 17, 2025 1:04 PM
> To: Tamar Christina
> Cc: GCC Patches ; nd ; Richard
> Earnshaw ; ktkac...@gcc.gnu.org; Richard
> Sandiford
> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after
> deprecation
>
>
>
> > On 17 Jan 2025, at 13:56, Tamar Christina wrote:
> >
> > Hi All,
> >
> > Following the deprecation of ILP32 *-elf builds fail now due to -Werror on
> > the
> > deprecation warning. This is because on embedded builds ILP32 is part of
> > the
> > default multilib.
> >
> > This patch removed it from the default target as the build would fail
> > anyway.
> >
> > Cross compiled on aarch64-none-elf and build succeeds now.
> >
> > Ok for master?
>
> I was going to say that it’s more common to just announce deprecation in the
> documentation/release notes for one release cycle, to give time for potential
> users
> to come forward.
> If we remove the multilib build now we’d be helping the support bitrot much
> faster, which would make it harder to restore if such a user does come
> forward.
>
The alternative approach would be to suppress the warning during build, the
downside
is ofcourse that this warning does not have a unique identifier, so I believe
we'd have to
suppress all deprecation warnings:
cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated]
But could work..
Cheers,
Tamar
> But it also seems odd to build an entire multilib for a deprecated feature so
> I’m not
> opposed to it.
> I don’t know if we document the ILP32 multilib somewhere. If so, it should be
> updated.
> In any case the gcc-15 release notes should also mention this change.
>
> Thanks,
> Kyrill
>
> >
> > Sending this out in case there are no objections to the approach
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > * config.gcc (aarch64-*-elf): Drop ILP32 from default multilibs.
> >
> > ---
> > diff --git a/gcc/config.gcc b/gcc/config.gcc
> > index
> 55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c
> 3a0ad91905286777 100644
> > --- a/gcc/config.gcc
> > +++ b/gcc/config.gcc
> > @@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-
> rtems*)
> > esac
> > aarch64_multilibs="${with_multilib_list}"
> > if test "$aarch64_multilibs" = "default"; then
> > - aarch64_multilibs="lp64,ilp32"
> > + aarch64_multilibs="lp64"
> > fi
> > aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
> > for aarch64_multilib in ${aarch64_multilibs}; do
> >
> >
> >
> >
> > --
> >
Re: [PATCH] c++: fix wrong-code with constexpr prvalue opt [PR118396]
On 1/16/25 8:04 PM, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
In that case, I'd think we want to use the value of 'object' (which
should be in ctx.ctor?) instead of the return value of
cxx_eval_constant_expression.
I think the new prvalue optimization (r15-6052) should only be enabled
for simple TARGET_EXPRs -- ones that don't initialize the object via
a sequence of statement as the one above.
I also think we want an assert so that this doesn't happen again.
PR c++/118396
PR c++/118523
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): Assert
that we don't throw away the initializer by evaluating it away.
* cp-gimplify.cc (cp_fold_r): Only attempt to evaluate the initializer
if the TARGET_EXPR is simple.
gcc/testsuite/ChangeLog:
* g++.dg/tree-ssa/pr78687.C: Revert r15-6052.
* g++.dg/tree-ssa/pr90883.C: Likewise.
* g++.dg/cpp0x/constexpr-prvalue4.C: New test.
* g++.dg/cpp1y/constexpr-prvalue3.C: New test.
---
gcc/cp/constexpr.cc | 14 --
gcc/cp/cp-gimplify.cc | 4 +-
.../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++
.../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++
gcc/testsuite/g++.dg/tree-ssa/pr78687.C | 3 +-
gcc/testsuite/g++.dg/tree-ssa/pr90883.C | 3 +-
6 files changed, 94 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index c898e3bfa6e..61b25277419 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8876,9 +8876,17 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
/* Turn off -frounding-math for manifestly constant evaluation. */
warning_sentinel rm (flag_rounding_math,
ctx.manifestly_const_eval == mce_true);
- tree type = (object
- ? cv_unqualified (TREE_TYPE (object))
- : initialized_type (t));
+ tree type;
+ if (object)
+{
+ type = cv_unqualified (TREE_TYPE (object));
+ /* If there is an object to initialize, make sure we don't throw
+away the initializer. */
+ gcc_assert (!VOID_TYPE_P (initialized_type (t)) || constexpr_dtor);
+}
+ else
+type = initialized_type (t);
+
tree r = t;
bool is_consteval = false;
if (VOID_TYPE_P (type))
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index c7074b00cef..283c0fa3e26 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1475,7 +1475,9 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
cp_walk_tree (&init, cp_fold_r, data, NULL);
cp_walk_tree (&TARGET_EXPR_CLEANUP (stmt), cp_fold_r, data, NULL);
*walk_subtrees = 0;
- if (!flag_no_inline)
+ /* Only attempt to evaluate the initializer if we're inlining and
+the TARGET_EXPR is simple. */
+ if (!flag_no_inline && !VOID_TYPE_P (TREE_TYPE (init)))
{
tree folded = maybe_constant_init (init, TARGET_EXPR_SLOT (stmt));
if (folded != init && TREE_CONSTANT (folded))
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
new file mode 100644
index 000..afcee65f880
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
@@ -0,0 +1,33 @@
+// PR c++/118396
+// { dg-do run { target c++11 } }
+// { dg-options "-O" }
+
+void *operator new(__SIZE_TYPE__, void *__p) { return __p; }
+
+struct Foo {
+ virtual ~Foo() = default;
+};
+struct Data {
+ int status;
+ Foo data{};
+};
+
+Data *P, *Q;
+
+struct vector {
+ vector (const Data &__value) {
+P = static_cast(__builtin_operator_new(0));
+new (P) Data (__value);
+Q = P + 1;
+ }
+ Data *begin() { return P; }
+ Data *end() { return Q; }
+};
+
+int
+main ()
+{
+ vector items_(Data{});
+ for (auto item : items_)
+item.status == 0 ? void() : __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
new file mode 100644
index 000..8ea86c60be5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
@@ -0,0 +1,45 @@
+// PR c++/118523
+// { dg-do compile { target c++14 } }
+// { dg-options "-O2 -Wall" }
+
+struct __new_allocator {
+ constexpr __new
RE: [RFC] PR81358: Enable automatic linking of libatomic
Hi Prathamesh!
On 2025-01-10T04:17:52+, Prathamesh Kulkarni wrote:
>> -Original Message-
>> From: Thomas Schwinge
>> Sent: 07 January 2025 17:45
>> On 2024-12-20T15:37:42+, Prathamesh Kulkarni
>> wrote:
>> > [...] copying libatomic.a over to $(gcc_objdir)$(MULTISUBDIR)/, and
>> > can confirm that 64-bit libatomic.a is copied to $build/gcc/ and 32-
>> bit libatomic.a is copied to $build/gcc/32/.
>>
>> So this:
>>
>> > --- a/libatomic/Makefile.am
>> > +++ b/libatomic/Makefile.am
>>
>> > @@ -162,6 +162,11 @@ libatomic_convenience_la_LIBADD =
>> > $(libatomic_la_LIBADD) # when it is reloaded during the build of
>> > all-multi.
>> > all-multi: $(libatomic_la_LIBADD)
>> >
>> > +gcc_objdir = $(MULTIBUILDTOP)../../$(host_subdir)/gcc
>> > +all: all-multi libatomic.la libatomic_convenience.la
>> > + $(INSTALL_DATA) .libs/libatomic.a $(gcc_objdir)$(MULTISUBDIR)/
>> > + chmod 644 $(gcc_objdir)$(MULTISUBDIR)/libatomic.a
>>
>> ... is conceptually modelled after libgcc, where the libraries get
>> copied into 'gcc/'?
Could you please clarify that this is indeed the case, or if there's a
different reason for doing it this way ("libraries get copied into
'gcc/'"), and overall what the rationale is -- is it "just convenience"
(meaning: not having to set up search paths to the actual libatomic
build), or something else? I suggest you actually put a comment next to
this new 'libatomic/Makefile.am' rule, also cross-referencing the
corresponding libgcc thing, as applicable.
>> However, here we only copy the static
>> 'libatomic.a'; libgcc does a 'make install-leaf', see
>> 'libgcc/Makefile.in':
>>
>> all: all-multi
>> # Now that we have built all the objects, we need to copy
>> # them back to the GCC directory. Too many things (other
>> # in-tree libraries, and DejaGNU) know about the layout
>> # of the build tree, for now.
>> $(MAKE) install-leaf DESTDIR=$(gcc_objdir) \
>> slibdir= libsubdir= MULTIOSDIR=$(MULTIDIR)
>>
>> ..., which also installs dynamic libraries. Is that difference
>> intentional and/or possibly important?
>
> Well, I wasn't sure what extension to use for shared libraries, and initially
> avoided copying them.
> libgcc seems to use $(SHLIB_EXT) to specify extension name for shared
> libraries, which can be overridden
> by targets.
>
> Matthew pointed out to me that using libtool --mode=install works for copying
> both
> static and shared libraries (with the numbered version libatomic.so.1.2.0),
> so in the attached patch,
> I changed Makefile.am rule to following:
> gcc_objdir = `pwd`/$(MULTIBUILDTOP)../../gcc/
> all: all-multi libatomic.la libatomic_convenience.la
> $(LIBTOOL) --mode=install $(INSTALL_DATA) libatomic.la
> $(gcc_objdir)$(MULTISUBDIR)/
>
> Which seems to install libatomic.a, libatomic.so and the numbered version in
> $build/gcc/ and in $build/gcc/32/
> (and $build/gcc/mgomp/ for nvptx build).
> Does it look OK ?
I confirm this appears to work as intended -- but I can't comment on
whether it's conceptually the right thing to do.
>> Does libatomic even need a switch corresponding to '-static-libgcc'?
>
> I am not sure, hoping for Joseph to chime in.
>> Given that libatomic libraries get copied into 'gcc/', will we be able
>> (later, incrementally) to remove some setup code from the test suites'
>> '*.exp' files, to locate build-tree libatomic?
>
> I'd guess so.
>> Also, given the presumed similarity to how libgcc is handled (with, of
>> course, the difference that libatomic isn't built for all
>> configurations), should we maybe in the build system place the new
>> libatomic handling next to the existing libgcc handling?
>> Specifically, instead of:
>>
>> > --- a/Makefile.def
>> > +++ b/Makefile.def
>>
>> > +dependencies = { module=configure-target-libbacktrace;
>> > on=all-target-libatomic; };
>> > +dependencies = { module=configure-target-libgloss;
>> > on=all-target-libatomic; };
>> > +dependencies = { module=configure-target-newlib; on=all-target-libatomic;
>> > };
>> > +dependencies = { module=configure-target-libgomp;
>> > on=all-target-libatomic; };
>> > [Etc.]
>>
>> ... handle libatomic like:
>>
>> // [...] By default target modules depend
>> // on libgcc and newlib/libgloss.
>
> The patch adjusts Makefile.tpl to add no_atomic to lang_env_dependencies, and
> adding dependency on libatomic if the attribute
> is not set for target library, similar to others (no_gcc, no_c). This also
> fixes the newlib failure with offloading.
> Does it look OK ?
Ah, great, that looks much better in my opinion. I'm not an expert of
the top-level build system, but I confirm the combined-tree GCC plus
newlib does build fine for nvptx offloading.
>> ..., and regarding:
>>
>> > --- a/configure.ac
>> > +++ b/configure.ac
>>
>> > +# If we are building libatomic, bootstrap it.
>> > +if echo " ${target_configdirs} " | grep " libatomic " > /dev/null
>> > +2>&1 ; then
>> > + boots
RE: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> -Original Message-
> From: Kyrylo Tkachov
> Sent: Friday, January 17, 2025 1:22 PM
> To: Tamar Christina
> Cc: GCC Patches ; nd ; Richard
> Earnshaw ; ktkac...@gcc.gnu.org; Richard
> Sandiford
> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after
> deprecation
>
>
>
> > On 17 Jan 2025, at 14:06, Tamar Christina wrote:
> >
> >> -Original Message-
> >> From: Kyrylo Tkachov
> >> Sent: Friday, January 17, 2025 1:04 PM
> >> To: Tamar Christina
> >> Cc: GCC Patches ; nd ; Richard
> >> Earnshaw ; ktkac...@gcc.gnu.org; Richard
> >> Sandiford
> >> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after
> >> deprecation
> >>
> >>
> >>
> >>> On 17 Jan 2025, at 13:56, Tamar Christina wrote:
> >>>
> >>> Hi All,
> >>>
> >>> Following the deprecation of ILP32 *-elf builds fail now due to -Werror
> >>> on the
> >>> deprecation warning. This is because on embedded builds ILP32 is part of
> >>> the
> >>> default multilib.
> >>>
> >>> This patch removed it from the default target as the build would fail
> >>> anyway.
> >>>
> >>> Cross compiled on aarch64-none-elf and build succeeds now.
> >>>
> >>> Ok for master?
> >>
> >> I was going to say that it’s more common to just announce deprecation in
> >> the
> >> documentation/release notes for one release cycle, to give time for
> >> potential
> users
> >> to come forward.
> >> If we remove the multilib build now we’d be helping the support bitrot much
> >> faster, which would make it harder to restore if such a user does come
> >> forward.
> >>
> >
> > The alternative approach would be to suppress the warning during build, the
> downside
> > is ofcourse that this warning does not have a unique identifier, so I
> > believe we'd
> have to
> > suppress all deprecation warnings:
> >
> > cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated]
> >
> > But could work..
> >
>
> I think if the warning suppression only happened for the ILP32 multilib build
> that
> would be okay.
> Other deprecation warnings would be caught in the LP64 build
I can make that work by conditionally changing MULTILIB_OPTIONS in
gcc/config/aarch64/t-aarch64 when it's collecting the options for ILP32.
What do others think?
Thanks,
Tamar
>
> Kyrill
>
> > Cheers,
> > Tamar
> >
> >> But it also seems odd to build an entire multilib for a deprecated feature
> >> so I’m
> not
> >> opposed to it.
> >> I don’t know if we document the ILP32 multilib somewhere. If so, it should
> >> be
> >> updated.
> >> In any case the gcc-15 release notes should also mention this change.
> >>
> >> Thanks,
> >> Kyrill
> >>
> >>>
> >>> Sending this out in case there are no objections to the approach
> >>>
> >>> Thanks,
> >>> Tamar
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * config.gcc (aarch64-*-elf): Drop ILP32 from default multilibs.
> >>>
> >>> ---
> >>> diff --git a/gcc/config.gcc b/gcc/config.gcc
> >>> index
> >>
> 55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c
> >> 3a0ad91905286777 100644
> >>> --- a/gcc/config.gcc
> >>> +++ b/gcc/config.gcc
> >>> @@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-
> *-
> >> rtems*)
> >>> esac
> >>> aarch64_multilibs="${with_multilib_list}"
> >>> if test "$aarch64_multilibs" = "default"; then
> >>> - aarch64_multilibs="lp64,ilp32"
> >>> + aarch64_multilibs="lp64"
> >>> fi
> >>> aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
> >>> for aarch64_multilib in ${aarch64_multilibs}; do
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>>
> >
Re: [PATCH] c++: Friend classes don't shadow enclosing template class paramater [PR118255]
On 1/17/25 1:50 AM, Simon Martin wrote:
On 17 Jan 2025, at 0:12, Jason Merrill wrote:
On 1/5/25 11:19 AM, Simon Martin wrote:
We currently reject the following code
=== code here ===
template struct S { friend class non_template; };
class non_template {};
S<0> s;
=== code here ===
While EDG agrees with the current behaviour, clang and MSVC don't
(see
https://godbolt.org/z/69TGaabhd), and I believe that this code is
valid,
since the friend clause does not actually declare a type, so it
cannot
shadow anything. The fact that we didn't error out if the
non_template
class was declared before S backs this up as well.
This patch fixes this by skipping the call to check_template_shadow
for
hidden bindings.
OK.
Thanks Jason. Since it’s a regression from GCC 8, OK as well for
active branches?
OK, I guess it seems safe enough.
Simon
Successfully tested on x86_64-pc-linux-gnu.
PR c++/118255
gcc/cp/ChangeLog:
* name-lookup.cc (pushdecl): Don't call check_template_shadow
for hidden bindings.
gcc/testsuite/ChangeLog:
* g++.dg/lookup/pr99116-1.C: Adjust test expectation.
* g++.dg/template/friend84.C: New test.
---
gcc/cp/name-lookup.cc| 5 -
gcc/testsuite/g++.dg/lookup/pr99116-1.C | 2 +-
gcc/testsuite/g++.dg/template/friend84.C | 26
3 files changed, 31 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/friend84.C
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 0e185d3ef42..d1abb205bc7 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -4040,7 +4040,10 @@ pushdecl (tree decl, bool hiding)
if (old && anticipated_builtin_p (old))
old = OVL_CHAIN (old);
- check_template_shadow (decl);
+ if (hiding)
+ ; /* Hidden bindings don't shadow anything. */
+ else
+ check_template_shadow (decl);
if (DECL_DECLARES_FUNCTION_P (decl))
{
diff --git a/gcc/testsuite/g++.dg/lookup/pr99116-1.C
b/gcc/testsuite/g++.dg/lookup/pr99116-1.C
index 01b483ea915..efee3e4aca3 100644
--- a/gcc/testsuite/g++.dg/lookup/pr99116-1.C
+++ b/gcc/testsuite/g++.dg/lookup/pr99116-1.C
@@ -2,7 +2,7 @@
template struct Z {
- friend struct T; // { dg-error "shadows template parameter" }
+ friend struct T; // { dg-bogus "shadows template parameter" }
};
struct Y {
diff --git a/gcc/testsuite/g++.dg/template/friend84.C
b/gcc/testsuite/g++.dg/template/friend84.C
new file mode 100644
index 000..64ea41a552b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/friend84.C
@@ -0,0 +1,26 @@
+// PR c++/118255
+// { dg-do "compile" }
+
+// The PR's case, that used to error out.
+template
+struct S {
+ friend class non_template; // { dg-bogus "shadows template
parameter" }
+};
+
+class non_template {};
+S<0> s;
+
+// We already accepted cases where the friend is already declared.
+template
+struct T {
+ friend class non_template;
+};
+T<0> t;
+
+// We should reject (re)declarations.
+template
+struct U {
+ class non_template {}; // { dg-error "shadows template parameter"
}
+ void non_template () {} // { dg-error "shadows template parameter"
}
+};
+U<0> u;
Re: [PATCH v3 1/2] aarch64: Use standard names for saturating arithmetic
Tamar Christina writes: >> -Original Message- >> From: Richard Sandiford >> Sent: Friday, January 10, 2025 4:50 PM >> To: Akram Ahmad >> Cc: ktkac...@nvidia.com; gcc-patches@gcc.gnu.org >> Subject: Re: [PATCH v3 1/2] aarch64: Use standard names for saturating >> arithmetic >> >> Akram Ahmad writes: >> > Ah whoops- I didn't see this before sending off V4 just now, my apologies. >> > I'll try my best to get this implemented before the end of the day so that >> > it doesn't miss the deadline. >> >> No rush! The delay here is entirely my fault, so no problem if the >> patch lands early stage 4. >> > > Hi, I'm picking up the remainder of the patch for Akram. > > I believe I have addressed all the review comments, and I've also rebased to > master which needed some small changes. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? LGTM, thanks. (I just diffed the old and new patches rather than reviewing the whole thing from scratch.) So OK from my POV, but please give Kyrill chance to comment too. Richard
Re: [PATCH] Fix uniqueness of symtab_node::get_dump_name.
> On Thu, 16 Jan 2025, Michal Jires wrote: > > > symtab_node::get_dump_name uses node order to identify nodes. > > Order is no longer unique because of Incremental LTO patches. > > This patch moves uid from cgraph_node node to symtab_node, > > so get_dump_name can use uid instead and get back unique dump names. > > > > In inlining passes, uid is replaced with more appropriate (more compact > > for indexing) summary id. > > > > Bootstrapped/regtested on x86_64-linux. > > Ok for trunk? > > This looks reasonable, but I defer to Honza for a final ack. It is OK. Thanks! Honza >
Re: [PATCH 3/4] RISC-V: Add .note.gnu.property for ZICFILP and ZICFISS ISA extension
Hi Monk,
On Fri, Nov 15, 2024 at 06:53:09PM +0800, Monk Chiang wrote:
> gcc/ChangeLog:
> * gcc/config/riscv/riscv.cc
> (riscv_file_end_indicate_exec_stack): Add .note.gnu.property.
> * gcc/config/riscv/linux.h (TARGET_ASM_FILE_END): Define.
>
> [...]
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 8201a965ed1..bda982f085c 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -10195,6 +10195,56 @@ riscv_file_start (void)
> riscv_emit_attribute ();
> }
>
> +void
> +riscv_file_end_indicate_exec_stack ()
> +{
> + file_end_indicate_exec_stack ();
> + long GNU_PROPERTY_RISCV_FEATURE_1_AND = 0;
This broke the riscv bootstrap because
GNU_PROPERTY_RISCV_FEATURE_1_AND is never used.
../../gcc/gcc/config/riscv/riscv.cc:10340:8: error: unused variable
‘GNU_PROPERTY_RISCV_FEATURE_1_AND’ [-Werror=unused-variable]
10340 | long GNU_PROPERTY_RISCV_FEATURE_1_AND = 0;
|^~~~
See https://builder.sourceware.org/buildbot/#/builders/310/builds/863
Could you fix that?
Thanks,
Mark
[PATCH v3 0/1] FMV AArch64 warning
From: Alfie Richards Hi both, I updated this patch as in other work I found a more natural place for this warning to be issued. I also implemented Kyrylo's suggestion to only issue the warning once. I don't yet have commit access so if this patch is okay would either of you be okay to commit it. Kind regards, Alfie Richards Alfie Richards (1): Add warning for non-spec compliant FMV in Aarch64 gcc/config/aarch64/aarch64.cc | 9 + gcc/config/aarch64/aarch64.opt | 4 gcc/doc/invoke.texi | 11 ++- gcc/testsuite/g++.target/aarch64/mv-1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 + gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 + gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 + gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++ 15 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C -- 2.34.1
Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> On 17 Jan 2025, at 14:06, Tamar Christina wrote:
>
>> -Original Message-
>> From: Kyrylo Tkachov
>> Sent: Friday, January 17, 2025 1:04 PM
>> To: Tamar Christina
>> Cc: GCC Patches ; nd ; Richard
>> Earnshaw ; ktkac...@gcc.gnu.org; Richard
>> Sandiford
>> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after
>> deprecation
>>
>>
>>
>>> On 17 Jan 2025, at 13:56, Tamar Christina wrote:
>>>
>>> Hi All,
>>>
>>> Following the deprecation of ILP32 *-elf builds fail now due to -Werror on
>>> the
>>> deprecation warning. This is because on embedded builds ILP32 is part of
>>> the
>>> default multilib.
>>>
>>> This patch removed it from the default target as the build would fail
>>> anyway.
>>>
>>> Cross compiled on aarch64-none-elf and build succeeds now.
>>>
>>> Ok for master?
>>
>> I was going to say that it’s more common to just announce deprecation in the
>> documentation/release notes for one release cycle, to give time for
>> potential users
>> to come forward.
>> If we remove the multilib build now we’d be helping the support bitrot much
>> faster, which would make it harder to restore if such a user does come
>> forward.
>>
>
> The alternative approach would be to suppress the warning during build, the
> downside
> is ofcourse that this warning does not have a unique identifier, so I believe
> we'd have to
> suppress all deprecation warnings:
>
> cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated]
>
> But could work..
>
I think if the warning suppression only happened for the ILP32 multilib build
that would be okay.
Other deprecation warnings would be caught in the LP64 build
Kyrill
> Cheers,
> Tamar
>
>> But it also seems odd to build an entire multilib for a deprecated feature
>> so I’m not
>> opposed to it.
>> I don’t know if we document the ILP32 multilib somewhere. If so, it should be
>> updated.
>> In any case the gcc-15 release notes should also mention this change.
>>
>> Thanks,
>> Kyrill
>>
>>>
>>> Sending this out in case there are no objections to the approach
>>>
>>> Thanks,
>>> Tamar
>>>
>>> gcc/ChangeLog:
>>>
>>> * config.gcc (aarch64-*-elf): Drop ILP32 from default multilibs.
>>>
>>> ---
>>> diff --git a/gcc/config.gcc b/gcc/config.gcc
>>> index
>> 55e37146ee0356b67b8a1a09d263eccdf69cd91a..432798d16fdbce099f69821c
>> 3a0ad91905286777 100644
>>> --- a/gcc/config.gcc
>>> +++ b/gcc/config.gcc
>>> @@ -1210,7 +1210,7 @@ aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-
>> rtems*)
>>> esac
>>> aarch64_multilibs="${with_multilib_list}"
>>> if test "$aarch64_multilibs" = "default"; then
>>> - aarch64_multilibs="lp64,ilp32"
>>> + aarch64_multilibs="lp64"
>>> fi
>>> aarch64_multilibs=`echo $aarch64_multilibs | sed -e 's/,/ /g'`
>>> for aarch64_multilib in ${aarch64_multilibs}; do
>>>
>>>
>>>
>>>
>>> --
>>>
>
Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
Tamar Christina writes: >> -Original Message- >> From: Kyrylo Tkachov >> Sent: Friday, January 17, 2025 1:22 PM >> To: Tamar Christina >> Cc: GCC Patches ; nd ; Richard >> Earnshaw ; ktkac...@gcc.gnu.org; Richard >> Sandiford >> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after >> deprecation >> >> >> >> > On 17 Jan 2025, at 14:06, Tamar Christina wrote: >> > >> >> -Original Message- >> >> From: Kyrylo Tkachov >> >> Sent: Friday, January 17, 2025 1:04 PM >> >> To: Tamar Christina >> >> Cc: GCC Patches ; nd ; Richard >> >> Earnshaw ; ktkac...@gcc.gnu.org; Richard >> >> Sandiford >> >> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after >> >> deprecation >> >> >> >> >> >> >> >>> On 17 Jan 2025, at 13:56, Tamar Christina >> >>> wrote: >> >>> >> >>> Hi All, >> >>> >> >>> Following the deprecation of ILP32 *-elf builds fail now due to -Werror >> >>> on the >> >>> deprecation warning. This is because on embedded builds ILP32 is part >> >>> of the >> >>> default multilib. >> >>> >> >>> This patch removed it from the default target as the build would fail >> >>> anyway. >> >>> >> >>> Cross compiled on aarch64-none-elf and build succeeds now. >> >>> >> >>> Ok for master? >> >> >> >> I was going to say that it’s more common to just announce deprecation in >> >> the >> >> documentation/release notes for one release cycle, to give time for >> >> potential >> users >> >> to come forward. >> >> If we remove the multilib build now we’d be helping the support bitrot >> >> much >> >> faster, which would make it harder to restore if such a user does come >> >> forward. >> >> >> > >> > The alternative approach would be to suppress the warning during build, the >> downside >> > is ofcourse that this warning does not have a unique identifier, so I >> > believe we'd >> have to >> > suppress all deprecation warnings: >> > >> > cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated] >> > >> > But could work.. >> > >> >> I think if the warning suppression only happened for the ILP32 multilib >> build that >> would be okay. >> Other deprecation warnings would be caught in the LP64 build > > I can make that work by conditionally changing MULTILIB_OPTIONS in > gcc/config/aarch64/t-aarch64 when it's collecting the options for ILP32. > > What do others think? To be honest, it seems like a waste of effort to me. If anyone complains about the removal of the multilib in GCC 15, we can always revert it for later releases in the GCC 15 series. A reversion like that would presumably also mean reverting the deprecation, since it wouldn't be worth reinstating the multilib for one release series only. Thanks, Richard
[PATCH v3 1/1] Add warning for non-spec compliant FMV in Aarch64
This patch adds a warning when FMV is used for Aarch64.
The reasoning for this is the ACLE [1] spec for FMV has diverged
significantly from the current implementation and we want to prevent
potential future compatability issues.
There is a patch for an ACLE compliant version of target_version and
target_clone in progress but it won't make gcc-15.
This has been bootstrap and regression tested for Aarch64.
Is this okay for master and packport to gcc-14?
[1]
https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
gcc/ChangeLog:
* config/aarch64/aarch64.cc
(aarch64_process_target_version_attr): Add experimental warning.
* config/aarch64/aarch64.opt: Add command line option to disable
warning.
gcc/testsuite/ChangeLog:
* g++.target/aarch64/mv-1.C: Add CLI flag
* g++.target/aarch64/mv-symbols1.C: Add CLI flag
* g++.target/aarch64/mv-symbols2.C: Add CLI flag
* g++.target/aarch64/mv-symbols3.C: Add CLI flag
* g++.target/aarch64/mv-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-symbols5.C: Add CLI flag
* g++.target/aarch64/mvc-symbols1.C: Add CLI flag
* g++.target/aarch64/mvc-symbols2.C: Add CLI flag
* g++.target/aarch64/mvc-symbols3.C: Add CLI flag
* g++.target/aarch64/mvc-symbols4.C: Add CLI flag
* g++.target/aarch64/mv-warning1.C: New test.
* g++.target/aarch64/mvc-warning1.C: New test.
---
gcc/config/aarch64/aarch64.cc | 9 +
gcc/config/aarch64/aarch64.opt | 4
gcc/doc/invoke.texi | 11 ++-
gcc/testsuite/g++.target/aarch64/mv-1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 +
gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 +
gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 +
gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++
15 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 91de13159cb..eae184c7d65 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -20125,6 +20125,15 @@ aarch64_parse_fmv_features (const char *str, aarch64_feature_flags *isa_flags,
static bool
aarch64_process_target_version_attr (tree args)
{
+ static bool issued_warning = false;
+ if (!issued_warning)
+{
+ warning (OPT_Wexperimental_fmv_target,
+ "Function Multi Versioning support is experimental, and the "
+ "behavior is likely to change");
+ issued_warning = true;
+}
+
if (TREE_CODE (args) == TREE_LIST)
{
if (TREE_CHAIN (args))
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 36bc719b822..2a8dd8ea66c 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -431,3 +431,7 @@ handling. One means we try to form pairs involving one or more existing
individual writeback accesses where possible. A value of two means we
also try to opportunistically form writeback opportunities by folding in
trailing destructive updates of the base register used by a pair.
+
+Wexperimental-fmv-target
+Target Var(warn_experimental_fmv) Warning Init(1)
+Warn about usage of experimental Function Multi Versioning.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 51dc871e6bc..bdf9ee1bc0c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -822,7 +822,8 @@ Objective-C and Objective-C++ Dialects}.
-moverride=@var{string} -mverbose-cost-dump
-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg}
-mstack-protector-guard-offset=@var{offset} -mtrack-speculation
--moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion}
+-moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion
+-Wexperimental-fmv-target}
@emph{Adapteva Epiphany Options}
@gccoptlist{-mhalf-reg-file -mprefer-short-insn-regs
@@ -22087,6 +22088,14 @@ which specify use of that register as a fixed register,
and @samp{none}, which means that no register is used for this
purpose. The default is @option{-m1reg-none}.
+@opindex Wexperimental-fmv-target
+@opindex Wno-experimental-fmv-target
+@item -Wexperimental-fmv-target
+Warn about use of experimental Function Multi Versioning.
+The Arm C Language Extension specification for Function Multi Versioning
+is beta and subject to change. Any usage of FMV is caveated that fut
[PATCH v9] c++: Fix overeager Woverloaded-virtual with conversion operators [PR109918]
Hi Jason,
On 16 Jan 2025, at 22:49, Jason Merrill wrote:
> On 10/16/24 11:43 AM, Simon Martin wrote:
>> As you know the patch had to be reverted due to PR117114, that
>> highlighted a bunch of issues with comparing DECL_VINDEXes: it might
>> give false positives in case of multiple inheritance (the case in
>> PR117114), but also if there’s single inheritance by the hierarchy
>> has
>> more than two levels (another issue I found while bootstrapping with
>> rust enabled).
>
> Yes, relying on DECL_VINDEX equality was wrong, sorry to mislead you.
>
>> The attached updated patch introduces an overrides_p function, based
>> on
>> the existing check_final_overrider, and uses it when the signatures
>> match.
>
> That seems unnecessary. It seems like removing that only breaks
> Woverloaded-virt11.C, and making that work again only requires
> bringing back the check that DECL_VINDEX (fndecl) is set (to any
> value). Or remembering that fndecl was a template, so it can't really
> have the same signature as a non-template, whatever same_signature_p
> says.
That’s right, only Woverloaded-virt11.C fails without the
check_final_overrider call.
Thanks for the suggestion to check whether fndecl is a template. This is
what the updated attached patch does, successfully tested on
x86_64-pc-linux-gnu.
OK for GCC 15? And if so, thoughts on backporting to release branches
(technically it’s a regression but it’s “just” an incorrect
warning fix, so probably not worth the risk)?
Thanks, Simon
From 1d94d90e979720049f27025233a4ea6e036ae712 Mon Sep 17 00:00:00 2001
From: Simon Martin
Date: Fri, 17 Jan 2025 10:37:33 +0100
Subject: [PATCH] c++: Fix overeager Woverloaded-virtual with conversion
operators [PR109918]
We currently emit an incorrect -Woverloaded-virtual warning upon the
following test case
=== cut here ===
struct A {
virtual operator int() { return 42; }
virtual operator char() = 0;
};
struct B : public A {
operator char() { return 'A'; }
};
=== cut here ===
The problem is that when iterating over ovl_range (fns), warn_hidden
gets confused by the conversion operator marker, concludes that
seen_non_override is true and therefore emits a warning for all
conversion operators in A that do not convert to char, even if
-Woverloaded-virtual is 1 (e.g. with -Wall, the case reported).
A second set of problems is highlighted when -Woverloaded-virtual is 2.
First, with the same test case, since base_fndecls contains all
conversion operators in A (except the one to char, that's been removed
when iterating over ovl_range (fns)), we emit a spurious warning for
the conversion operator to int, even though it's unrelated.
Second, in case there are several conversion operators with different
cv-qualifiers to the same type in A, we rightfully emit a warning,
however the note uses the location of the conversion operator marker
instead of the right one; location_of should go over conv_op_marker.
This patch fixes all these by explicitly keeping track of (1) base
methods that are overriden, as well as (2) base methods that are hidden
but not overriden (and by what), and warning about methods that are in
(2) but not (1). It also ignores non virtual base methods, per
"definition" of -Woverloaded-virtual.
Successfully tested on x86_64-pc-linux-gnu.
Co-authored-by: Jason Merrill
PR c++/117114
PR c++/109918
gcc/cp/ChangeLog:
* class.cc (warn_hidden): Keep track of overloaded and of hidden
base methods. Mention the actual hiding function in the warning,
not the first overload.
* error.cc (location_of): Skip over conv_op_marker.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Woverloaded-virt1.C: Check that no warning is
emitted for non virtual base methods.
* g++.dg/warn/Woverloaded-virt10.C: New test.
* g++.dg/warn/Woverloaded-virt11.C: New test.
* g++.dg/warn/Woverloaded-virt12.C: New test.
* g++.dg/warn/Woverloaded-virt13.C: New test.
* g++.dg/warn/Woverloaded-virt5.C: New test.
* g++.dg/warn/Woverloaded-virt6.C: New test.
* g++.dg/warn/Woverloaded-virt7.C: New test.
* g++.dg/warn/Woverloaded-virt8.C: New test.
* g++.dg/warn/Woverloaded-virt9.C: New test.
---
gcc/cp/class.cc | 90 ---
gcc/cp/error.cc | 3 +-
gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C | 2 +
.../g++.dg/warn/Woverloaded-virt10.C | 11 +++
.../g++.dg/warn/Woverloaded-virt11.C | 25 ++
.../g++.dg/warn/Woverloaded-virt12.C | 23 +
.../g++.dg/warn/Woverloaded-virt13.C | 28 ++
gcc/testsuite/g++.dg/warn/Woverloaded-virt5.C | 12 +++
gcc/testsuite/g++.dg/warn/Woverloaded-virt6.C | 12 +++
gcc/testsuite/g++.dg/warn/Woverloaded-virt7.C | 37
gcc/testsuite/g++.dg/warn/Woverloaded-virt8.C | 15
gcc/testsuite/g++.dg/warn/Woverloaded-virt9.C | 14 +++
12 files changed, 237 insertions(+),
Re: [PATCH v3 1/2] RISC-V: Allocate the initial register in the expand phase for the vl of XTheadVector
> > Since the parameter vl of XTheadVector does not support immediate numbers,
> > we need
> > to put it in the register in advance. That generates the initial code
> > correctly.
> >
> > PR 116593
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-vector-builtins.cc
> > (function_expander::add_input_operand):
> > Put immediate for vl to GPR for XTheadVector.
>
> Generally both patches look reasonable to me and the change is less invasive
> than going via spec_restriction.
>
> How was this tested? The Rivos CI has already picked it up but please still
> always specify. Thanks.
I'm not sure what you mean, but I'll do my best to understand.
If you're referring to how to test the logic of this code to ensure it works
as intended, it can be quite challenging to explain. Testing this particular
code is not straightforward. Even without any specific processing, the
predicate
"vector_length_operand" still constrains the XTheadVector, and the immediate
value for VL will be stored in the register. In this sense, the code itself
may seem redundant.
However, I submitted the patch to generate the correct pattern in generate_insn
rather than relying solely on "vector_length_operand" to ensure correctness.
This approach aims to make the code more robust and reliable.
As for how to test, I don't have a good idea yet. At present, it is just a
simple test to store the immediate number of Vl in the register. Obviously,
this coincides with the function of "vector_length_operand.
In fact, I was also hesitating whether we needed this patch, and finally
submitted it in order to see your opinion.
> > + /* Since the parameter vl of XTheadVector does not support
> > + immediate numbers, we need to put it in the register
> > + in advance. */
> > + if (TARGET_XTHEADVECTOR
> > + && CONST_INT_P (x)
> > + && base->apply_vl_p ()
> > + && argno == (unsigned) (call_expr_nargs (exp) - 1)
> > + && x != CONST0_RTX (GET_MODE (x)))
> > +{
> > + x = force_reg (word_mode, x);
> > + add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> > +}
> > + else
> > +add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> > }
>
> To me it feels as if this would better fit one level higher rather than
> checking ARGNO and apply_vl_p here. Is that somehow too cumbersome?
While it is generally preferable to handle operations at the upper layer,
involving
multiple upper layers could complicate maintenance. Additionally, the
processing for
VL has already been largely completed by ARGNO and apply_vl_p. Therefore, I
prefer
not to introduce further changes at the upper level, even though this means
that every
call to add_input_operand will be checked, potentially leading to a slight
decrease in
compilation efficiency.
> > +# GCC testsuite that uses the `dg.exp' driver.
> > +
> > +# Test the front-end for C++.
> > +# We don't need to test back-end code-gen in RV32 system for C++
> > +# Because it is already tested in C.
> > +# Exit immediately if this isn't a RISC-V target.
> > +if ![istarget riscv*-*-*] then {
> > + return
> > +}
>
> Might want to adjust those comments slightly ;)
>
> Apologies, I mixed up patches. Still it's not really a front-end test IMHO
> but just a minor nit anyway.
Here I simply copied it form rvv.exp and didn't notice the comments, my fault.
I will modify, thanks. :)
Regards,
Jin
>
> --
> Regards
> Robin
[PATCH] tree-optimization/118529 - ICE with condition vectorization
On sparc we end up choosing vector(8) for the
condition but vector(2) int for the value of a COND_EXPR but we
fail to verify their shapes match and thus things go downhill.
This is a missed-optimization on the pattern recognition side
as well as unhandled vector decomposition in vectorizable_condition.
The following plugs just the observed ICE for now.
Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.
PR tree-optimization/118529
* tree-vect-stmts.cc (vectorizable_condition): Check the
shape of the vector and condition vector type are compatible.
* gcc.target/sparc/pr118529.c: New testcase.
---
gcc/testsuite/gcc.target/sparc/pr118529.c | 17 +
gcc/tree-vect-stmts.cc| 5 +++--
2 files changed, 20 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/sparc/pr118529.c
diff --git a/gcc/testsuite/gcc.target/sparc/pr118529.c
b/gcc/testsuite/gcc.target/sparc/pr118529.c
new file mode 100644
index 000..1393763e9db
--- /dev/null
+++ b/gcc/testsuite/gcc.target/sparc/pr118529.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -mvis3" } */
+
+long c;
+int d[10];
+int e;
+void g() {
+ int b = 1 & e;
+ int *f = d;
+ b = -b;
+ c = 0;
+ for (; c < 10; c++) {
+int h = f[c] ^ c;
+h &= b;
+f[c] ^= h;
+ }
+}
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index b5dd1a2e40f..833029fcb00 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -12676,8 +12676,9 @@ vectorizable_condition (vec_info *vinfo,
masked = !COMPARISON_CLASS_P (cond_expr);
vec_cmp_type = truth_type_for (comp_vectype);
-
- if (vec_cmp_type == NULL_TREE)
+ if (vec_cmp_type == NULL_TREE
+ || maybe_ne (TYPE_VECTOR_SUBPARTS (vectype),
+ TYPE_VECTOR_SUBPARTS (vec_cmp_type)))
return false;
cond_code = TREE_CODE (cond_expr);
--
2.43.0
Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> On 17 Jan 2025, at 14:47, Richard Sandiford wrote: > > Tamar Christina writes: >>> -Original Message- >>> From: Kyrylo Tkachov >>> Sent: Friday, January 17, 2025 1:22 PM >>> To: Tamar Christina >>> Cc: GCC Patches ; nd ; Richard >>> Earnshaw ; ktkac...@gcc.gnu.org; Richard >>> Sandiford >>> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after >>> deprecation >>> >>> >>> On 17 Jan 2025, at 14:06, Tamar Christina wrote: > -Original Message- > From: Kyrylo Tkachov > Sent: Friday, January 17, 2025 1:04 PM > To: Tamar Christina > Cc: GCC Patches ; nd ; Richard > Earnshaw ; ktkac...@gcc.gnu.org; Richard > Sandiford > Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after > deprecation > > > >> On 17 Jan 2025, at 13:56, Tamar Christina >> wrote: >> >> Hi All, >> >> Following the deprecation of ILP32 *-elf builds fail now due to -Werror >> on the >> deprecation warning. This is because on embedded builds ILP32 is part >> of the >> default multilib. >> >> This patch removed it from the default target as the build would fail >> anyway. >> >> Cross compiled on aarch64-none-elf and build succeeds now. >> >> Ok for master? > > I was going to say that it’s more common to just announce deprecation in > the > documentation/release notes for one release cycle, to give time for > potential >>> users > to come forward. > If we remove the multilib build now we’d be helping the support bitrot > much > faster, which would make it harder to restore if such a user does come > forward. > The alternative approach would be to suppress the warning during build, the >>> downside is ofcourse that this warning does not have a unique identifier, so I believe we'd >>> have to suppress all deprecation warnings: cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated] But could work.. >>> >>> I think if the warning suppression only happened for the ILP32 multilib >>> build that >>> would be okay. >>> Other deprecation warnings would be caught in the LP64 build >> >> I can make that work by conditionally changing MULTILIB_OPTIONS in >> gcc/config/aarch64/t-aarch64 when it's collecting the options for ILP32. >> >> What do others think? > > To be honest, it seems like a waste of effort to me. If anyone complains > about the removal of the multilib in GCC 15, we can always revert it for > later releases in the GCC 15 series. A reversion like that would > presumably also mean reverting the deprecation, since it wouldn't be > worth reinstating the multilib for one release series only. Ok, let’s get rid of the multilib in this release then. My comment about documenting that in the release notes still stands Thanks for handling this. Kyrill > > Thanks, > Richard
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
Am Donnerstag, dem 16.01.2025 um 21:18 + schrieb Qing Zhao: > .. > > Although in the previous discussion, I agreed with Martin that we should use > the > designator syntax (i.e, counted_by (.n) instead of counted_by (n)) for the > counted_by attribute for pointer fields, after more consideration and > discussion > with Bill Wendling (who is working on the same work for CLANG), we decided to > keep the current syntax of FAM for pointer fields. And leave the new syntax > (.n) > and more complicate expressions to a later work. > I think this would be a mistake. Once you have established the confusing syntax, it will not easily go away anymore. So I think you should use the unambiguous and future-prove syntax right away. Support for more complicated expressions could be left for later, of course. Martin > This patch set includes 3 parts: > > 1.Extend "counted_by" attribute to pointer fields of structures. > 2.Convert a pointer reference with counted_by attribute to .ACCESS_WITH_SIZE > and use it in builtinin-object-size. > 3.Use the counted_by attribute of pointers in array bound checker. > > In which, the patch 1 and 2 are simple and straightforward, however, the > patch 3 > is a little complicate due to the following reason: > > Current array bound checker only instruments ARRAY_REF, and the INDEX > information is the 2nd operand of the ARRAY_REF. > > When extending the array bound checker to pointer references with > counted_by attributes, the hardest part is to get the INDEX of the > corresponding array ref from the offset computation expression of > the pointer ref. > > The whole patch set has been bootstrapped and regression tested on both > aarch64 > and x86. > > Let me know any comments and suggestions. > > Thanks. > > Qing > > Qing Zhao (3): > Extend "counted_by" attribute to pointer fields of structures. > Convert a pointer reference with counted_by attribute to > .ACCESS_WITH_SIZE and use it in builtinin-object-size. > Use the counted_by attribute of pointers in array bound checker. > > gcc/c-family/c-attribs.cc | 15 +- > gcc/c-family/c-gimplify.cc| 7 + > gcc/c-family/c-ubsan.cc | 264 -- > gcc/c/c-decl.cc | 91 +++--- > gcc/c/c-typeck.cc | 41 +-- > gcc/doc/extend.texi | 37 ++- > gcc/testsuite/gcc.dg/flex-array-counted-by.c | 2 +- > gcc/testsuite/gcc.dg/pointer-counted-by-2.c | 8 + > gcc/testsuite/gcc.dg/pointer-counted-by-3.c | 127 + > gcc/testsuite/gcc.dg/pointer-counted-by-4.c | 63 + > gcc/testsuite/gcc.dg/pointer-counted-by-5.c | 48 > gcc/testsuite/gcc.dg/pointer-counted-by-6.c | 47 > gcc/testsuite/gcc.dg/pointer-counted-by-7.c | 30 ++ > gcc/testsuite/gcc.dg/pointer-counted-by-8.c | 30 ++ > gcc/testsuite/gcc.dg/pointer-counted-by.c | 70 + > .../ubsan/pointer-counted-by-bounds-2.c | 47 > .../ubsan/pointer-counted-by-bounds-3.c | 35 +++ > .../ubsan/pointer-counted-by-bounds-4.c | 35 +++ > .../ubsan/pointer-counted-by-bounds-5.c | 46 +++ > .../ubsan/pointer-counted-by-bounds-6.c | 33 +++ > .../gcc.dg/ubsan/pointer-counted-by-bounds.c | 46 +++ > gcc/tree-object-size.cc | 11 +- > 22 files changed, 1045 insertions(+), 88 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-2.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-3.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-4.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-5.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-6.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-7.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-8.c > create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-2.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-3.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-4.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-5.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-6.c > create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds.c >
[PATCH] aarch64: Add missing simd requirements for INS [PR118531]
In g:b096a6ebe9d9f9fed4c105f6555f724eb32af95c I'd forgotten
to gate some uses of INS on TARGET_SIMD.
Tested on aarch64-linux-gnu. I'll push around this time on Monday
if there are no comments before then.
Richard
gcc/
PR target/118531
* config/aarch64/aarch64.md (*insv_reg_)
(*aarch64_bfi_)
(*aarch64_bfidi_subreg_): Add missing
simd requirements.
gcc/testsuite/
* gcc.target/aarch64/ins_bitfield_1a.c: New test.
* gcc.target/aarch64/ins_bitfield_3a.c: Likewise.
* gcc.target/aarch64/ins_bitfield_5a.c: Likewise.
---
gcc/config/aarch64/aarch64.md | 9 ++---
gcc/testsuite/gcc.target/aarch64/ins_bitfield_1a.c | 8
gcc/testsuite/gcc.target/aarch64/ins_bitfield_3a.c | 8
gcc/testsuite/gcc.target/aarch64/ins_bitfield_5a.c | 8
4 files changed, 30 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_1a.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_3a.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/ins_bitfield_5a.c
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 44f5b7a54d2..1b67ccc31dd 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -6361,7 +6361,8 @@ (define_insn "*insv_reg_"
return "ins\t%0.[%1], %2.[0]";
return "ins\t%0.[%1], %w2";
}
- [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")]
+ [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")
+ (set_attr "arch" "*,simd,simd")]
)
(define_insn "*insv_reg"
@@ -6394,7 +6395,8 @@ (define_insn_and_split
"*aarch64_bfi_"
operands[2] = lowpart_subreg (mode, operands[2],
mode);
}
- [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")]
+ [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")
+ (set_attr "arch" "*,simd,simd")]
)
(define_insn "*aarch64_bfi4"
@@ -6426,7 +6428,8 @@ (define_insn_and_split
"*aarch64_bfidi_subreg_"
{
operands[2] = lowpart_subreg (DImode, operands[3], mode);
}
- [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")]
+ [(set_attr "type" "bfm,neon_ins_q,neon_ins_q")
+ (set_attr "arch" "*,simd,simd")]
)
;; Match a bfi instruction where the shift of OP3 means that we are
diff --git a/gcc/testsuite/gcc.target/aarch64/ins_bitfield_1a.c
b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_1a.c
new file mode 100644
index 000..028d4aa1e89
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_1a.c
@@ -0,0 +1,8 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2 --save-temps" } */
+
+#pragma GCC target "+nosimd"
+
+#include "ins_bitfield_1.c"
+
+/* { dg-final { scan-assembler-not {\tins\t} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ins_bitfield_3a.c
b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_3a.c
new file mode 100644
index 000..1c153667a8d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_3a.c
@@ -0,0 +1,8 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2 --save-temps" } */
+
+#pragma GCC target "+nosimd"
+
+#include "ins_bitfield_3.c"
+
+/* { dg-final { scan-assembler-not {\tins\t} } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ins_bitfield_5a.c
b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_5a.c
new file mode 100644
index 000..f6bdde97f98
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ins_bitfield_5a.c
@@ -0,0 +1,8 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2 --save-temps" } */
+
+#pragma GCC target "+nosimd"
+
+#include "ins_bitfield_5.c"
+
+/* { dg-final { scan-assembler-not {\tins\t} } } */
--
2.25.1
Re: [PATCH v3 1/2] aarch64: Use standard names for saturating arithmetic
> On 17 Jan 2025, at 15:01, Richard Sandiford wrote: > > Tamar Christina writes: >>> -Original Message- >>> From: Richard Sandiford >>> Sent: Friday, January 10, 2025 4:50 PM >>> To: Akram Ahmad >>> Cc: ktkac...@nvidia.com; gcc-patches@gcc.gnu.org >>> Subject: Re: [PATCH v3 1/2] aarch64: Use standard names for saturating >>> arithmetic >>> >>> Akram Ahmad writes: Ah whoops- I didn't see this before sending off V4 just now, my apologies. I'll try my best to get this implemented before the end of the day so that it doesn't miss the deadline. >>> >>> No rush! The delay here is entirely my fault, so no problem if the >>> patch lands early stage 4. >>> >> >> Hi, I'm picking up the remainder of the patch for Akram. >> >> I believe I have addressed all the review comments, and I've also rebased to >> master which needed some small changes. >> >> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >> >> Ok for master? > > LGTM, thanks. (I just diffed the old and new patches rather than > reviewing the whole thing from scratch.) > > So OK from my POV, but please give Kyrill chance to comment too. It looks fine to me too. Thanks for the work, Kyrill > > Richard
Re: [PATCH] rs6000, fix test builtins-1-p10-runnable.c
Kewen: Thanks for the approval. Sorry for the delay in getting this committed. I made the fix requested and retested the patch on the most recent mainline. I will get this committed. Thanks for all the help. Carl On 11/24/24 5:42 AM, Kewen Lin wrote: rs6000, fix test builtins-1-p10-runnable.c The test has two issues: 1) The test should generate execute abort() if an error is found. However, the test contains a #define 0 which actually enables the error prints not exectuting void() because the debug code is protected by an #ifdef not #if. The #define DEBUG needs to be removed to so the test will abort on an error. 2) The vec_i_expected output was tweeked to test that it would fail. The test value was not removed. By removing the #define DEBUG, the test fails and reports 1 failure. Removing the intentionally wrong expected value results in the test passing with no errors as expected. gcc/testsuite/ChangeLog: * gcc.target/powerpc/builtins-1-p10-runnable.c: Remove #define DEBUG. Replace vec_i_expected value with correct value. Nit: Three more spaces before "Replace", so s/ //. OK for trunk with this tweaked, thanks! BR,
Re: [PATCH ver2 1/4] rs6000, add testcases to the overloaded vec_perm built-in
Kewen: I fixed the space issue and moved the sll, ull and bll together. Sorry for the delay in getting the patch committed. Thanks for all the help. Carl On 11/24/24 5:43 AM, Kewen Lin wrote: extern __vector int si[][4]; extern __vector short ss[][4]; extern __vector signed char sc[][4]; @@ -55,7 +57,6 @@ extern __vector double d[][4]; extern __vector long sl[][4]; extern __vector long long sll[][4]; extern __vector unsigned long ul[][4]; -extern __vector unsigned long long ull[][4]; Nit: I prefer to put sll, ull and bll together, so could you just move the above bll here? OK for trunk with these nits tweaked. Thanks. BR, Kewen
Re: [PATCH ver2 2/4] rs6000, remove built-ins __builtin_vsx_vperm_8hi and, __builtin_vsx_vperm_8hi_uns
Kewen: OK, thanks for the approval. Sorry for the delay in getting it committed. Carl On 11/24/24 5:43 AM, Kewen Lin wrote: --- rs6000, remove built-ins __builtin_vsx_vperm_8hi and __builtin_vsx_vperm_8hi_uns The two built-ins __builtin_vsx_vperm_8hi and __builtin_vsx_vperm_8hi_uns are redundant. The are covered by the overloaded vec_perm built-in. The built-ins are not documented and do not have test cases. The removal of these built-ins was missed in commit gcc r15-1923 on 7/9/2024. This patch removes the redundant built-ins. OK for trunk, thanks. BR, Kewen
Re: [PATCH ver2 3/4] rs6000, Remove redundant built-in __builtin_vsx_xvcvuxwdp
Kewen: Sorry for the delay in getting the patch committed. Thanks for all the help. Carl On 11/24/24 5:43 AM, Kewen Lin wrote: rs6000, Remove redundant built-in __builtin_vsx_xvcvuxwdp The built-in __builtin_vsx_xvcvuxwdp can be covered with PVIPR function vec_doubleo on LE and vec_doublee on BE. There are no test cases or documentation for __builtin_vsx_xvcvuxwdp. This patch removes the redundant built-in. OK for trunk, thanks! BR, Kewen
[PATCH] testcase: Add testcase for shrink wrapping of vector::push_back [PR118502]
LLVM folks noticed that GCC was shrink wrapping the call to
vector::push_back.
So I thought it was a good idea to commit a testcase to make sure GCC does not
regress
in this area unknowning.
Note the shrink wrapping started with r15-1619-g3b9b8d6cfdf593.
Note this enables the testcase for x86_64 (!ia32), powerpc, aarch64 and riscv
which I tested
via godbolt to see the shrink wrapping occurs. Also tested the testcase for both
x86_64-linux-gnu and aarch64-linux-gnu to make sure I got the target selects
correct.
PR rtl-optimization/118502
gcc/testsuite/ChangeLog:
* g++.dg/opt/shrink-wrapping-vector-1.C: New test.
Signed-off-by: Andrew Pinski
---
.../g++.dg/opt/shrink-wrapping-vector-1.C | 17 +
1 file changed, 17 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/opt/shrink-wrapping-vector-1.C
diff --git a/gcc/testsuite/g++.dg/opt/shrink-wrapping-vector-1.C
b/gcc/testsuite/g++.dg/opt/shrink-wrapping-vector-1.C
new file mode 100644
index 000..5ebbc0dab64
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/shrink-wrapping-vector-1.C
@@ -0,0 +1,17 @@
+// { dg-do compile { target { { { i?86-*-* x86_64-*-* } && { ! ia32 } } || {
powerpc*-*-* aarch64*-*-* riscv*-*-* } } } }
+// { dg-options "-O2 -fdump-rtl-pro_and_epilogue" }
+// { dg-skip-if "requires hosted libstdc++ for vector" { ! hostedlib } }
+
+// PR rtl-optimization/118502
+
+// The shrink-wrapping should happen around the slow path of
vector::push_back,
+// The fasth path is just checking if there is enough space and doing a few
stores.
+// We want to verify that shrink wrapping happen always.
+
+#include
+
+void push_back(std::vector& xs, unsigned char x) {
+xs.push_back(x);
+}
+
+/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" "pro_and_epilogue"
} } */
--
2.43.0
Re: [patch, avr] Add const attribute to built-in functions if possible.
Georg-Johann Lay writes: > Most of the avr built-in functions can be attributed "const". > > Ok for trunk? > Ok. Denis. > Johann > > -- > > AVR: Add "const" attribute to avr built-in functions if possible. > > gcc/ > * config/avr/avr-c.cc (DEF_BUILTIN): Add ATTRS argument to macro > definition. > * config/avr/avr.cc: Same. > (avr_init_builtins) : New variable that can be used > as ATTRS argument in DEF_BUILTIN. > * config/avr/builtins.def (DEF_BUILTIN): Add ATTRS parameter > to all definitions.
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
Hi, Bill,
Thanks a lot for your testing case.
I studied this testing case and realized that we need to decide
what’s the expected behavior for the following situation:
struct bar;
struct a {
int dummy;
struct bar *array __attribute__((counted_by(count)));
char count;
};
when the size information of the element of the pointer array is not available
in the current compilation, i.e., there is no definition of the structure
“bar” in the
current file, the size of “structure bar” is not known, as a result,
compilation is not
able to compute the object size of the pointer array “array” even though the
length
of the array is known.
So, my question is:
1. When should the compiler issue warning for such situation?
A. During C frontend when checking the counted_by attributes.
B. During middle-end when __builtin_dynamic_object_size is computing the
object size.
I prefer B. The reason is: even though the counted_by attribute under such
situation is not enough for object_size,
It should be enough for the bound sanitizer?
Let me know your opinion on this.
thanks.
Qing
> On Jan 16, 2025, at 17:50, Bill Wendling wrote:
>
> I'm also getting this ICE:
>
> kiff ~/gcc $ cat g.c
> #include
> #include
>
> #define __noinline __attribute__((noinline))
>
> struct bar;
>
> struct a {
>int dummy;
>struct bar *array __attribute__((counted_by(count)));
>char count;
> };
>
> struct a * __noinline alloc(int count) {
> struct a *ptr = malloc(sizeof(struct a) + sizeof(struct bar *) * count);
>
> ptr->count = count;
> return ptr;
> }
>
> int main() {
> struct a *ptr = alloc(10);
>
> printf("__bdos(ptr->array) == %lu\n",
> __builtin_dynamic_object_size(ptr->array, 0));
>
> return 0;
> }
>
>
> kiff ~/gcc $ ./gcc.install/bin/gcc -O2 g.c
> during GIMPLE pass: objsz
> g.c: In function ‘main’:
> g.c:21:5: internal compiler error: Segmentation fault
> 21 | int main() {
> | ^~~~
> 0x2645f5f internal_error(char const*, ...)
> ../../gcc.src/gcc/diagnostic-global-context.cc:517
> 0x1133e76 crash_signal
> ../../gcc.src/gcc/toplev.cc:322
> 0x7f82c195558f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0xd1d8b0 contains_struct_check(tree_node*, tree_node_structure_enum,
> char const*, int, char const*)
> ../../gcc.src/gcc/tree.h:3832
> 0xd1d8b0 fold_convert_loc(unsigned long, tree_node*, tree_node*)
> ../../gcc.src/gcc/fold-const.cc:2618
> 0x1201efe access_with_size_object_size
> ../../gcc.src/gcc/tree-object-size.cc:812
> 0x1201efe call_object_size
> ../../gcc.src/gcc/tree-object-size.cc:1429
> 0x1201efe collect_object_sizes_for
> ../../gcc.src/gcc/tree-object-size.cc:1883
> 0x12040ba merge_object_sizes
> ../../gcc.src/gcc/tree-object-size.cc:1470
> 0x12012f6 collect_object_sizes_for
> ../../gcc.src/gcc/tree-object-size.cc:1861
> 0x1202b7b compute_builtin_object_size(tree_node*, int, tree_node**)
> ../../gcc.src/gcc/tree-object-size.cc:1283
> 0xb6a5be fold_builtin_object_size
> ../../gcc.src/gcc/builtins.cc:11819
> 0xb6a5be fold_builtin_2
> ../../gcc.src/gcc/builtins.cc:10838
> 0xb6a5be fold_builtin_n
> ../../gcc.src/gcc/builtins.cc:10950
> 0x1203910 dynamic_object_sizes_execute_one
> ../../gcc.src/gcc/tree-object-size.cc:2187
> 0x1203910 object_sizes_execute
> ../../gcc.src/gcc/tree-object-size.cc:2250
> Please submit a full bug report, with preprocessed source (by using
> -freport-bug).
> Please include the complete backtrace with any bug report.
> See
RE: [PATCH]AArch64: Drop ILP32 from default elf multilibs after deprecation
> -Original Message- > From: Kyrylo Tkachov > Sent: Friday, January 17, 2025 3:10 PM > To: Richard Sandiford > Cc: Tamar Christina ; GCC Patches patc...@gcc.gnu.org>; nd ; Richard Earnshaw > ; ktkac...@gcc.gnu.org > Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after > deprecation > > > > > On 17 Jan 2025, at 14:47, Richard Sandiford > wrote: > > > > Tamar Christina writes: > >>> -Original Message- > >>> From: Kyrylo Tkachov > >>> Sent: Friday, January 17, 2025 1:22 PM > >>> To: Tamar Christina > >>> Cc: GCC Patches ; nd ; Richard > >>> Earnshaw ; ktkac...@gcc.gnu.org; Richard > >>> Sandiford > >>> Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after > >>> deprecation > >>> > >>> > >>> > On 17 Jan 2025, at 14:06, Tamar Christina > wrote: > > > -Original Message- > > From: Kyrylo Tkachov > > Sent: Friday, January 17, 2025 1:04 PM > > To: Tamar Christina > > Cc: GCC Patches ; nd ; Richard > > Earnshaw ; ktkac...@gcc.gnu.org; Richard > > Sandiford > > Subject: Re: [PATCH]AArch64: Drop ILP32 from default elf multilibs after > > deprecation > > > > > > > >> On 17 Jan 2025, at 13:56, Tamar Christina > wrote: > >> > >> Hi All, > >> > >> Following the deprecation of ILP32 *-elf builds fail now due to > >> -Werror on > the > >> deprecation warning. This is because on embedded builds ILP32 is part > >> of > the > >> default multilib. > >> > >> This patch removed it from the default target as the build would fail > anyway. > >> > >> Cross compiled on aarch64-none-elf and build succeeds now. > >> > >> Ok for master? > > > > I was going to say that it’s more common to just announce deprecation in > the > > documentation/release notes for one release cycle, to give time for > > potential > >>> users > > to come forward. > > If we remove the multilib build now we’d be helping the support bitrot > > much > > faster, which would make it harder to restore if such a user does come > forward. > > > > The alternative approach would be to suppress the warning during build, > the > >>> downside > is ofcourse that this warning does not have a unique identifier, so I > believe > we'd > >>> have to > suppress all deprecation warnings: > > cc1: error: '-mabi=ilp32' is deprecated [-Werror=deprecated] > > But could work.. > > >>> > >>> I think if the warning suppression only happened for the ILP32 multilib > >>> build > that > >>> would be okay. > >>> Other deprecation warnings would be caught in the LP64 build > >> > >> I can make that work by conditionally changing MULTILIB_OPTIONS in > >> gcc/config/aarch64/t-aarch64 when it's collecting the options for ILP32. > >> > >> What do others think? > > > > To be honest, it seems like a waste of effort to me. If anyone complains > > about the removal of the multilib in GCC 15, we can always revert it for > > later releases in the GCC 15 series. A reversion like that would > > presumably also mean reverting the deprecation, since it wouldn't be > > worth reinstating the multilib for one release series only. > > Ok, let’s get rid of the multilib in this release then. > My comment about documenting that in the release notes still stands Not a problem, Thanks for the discussion, > I don’t know if we document the ILP32 multilib somewhere. If so, it should be > updated. From what I can tell, we've never documented what the default multilib is for AArch64. There are documentation for other targets but can't find any for AArch64. > In any case the gcc-15 release notes should also mention this change. Agreed, we haven't written out release notes yet this year, and I'm waiting till features stabilize a bit. Is it ok if I do this when I publish the remainder? As it stands we haven't documented any Arm or AArch64 changes yet to https://gcc.gnu.org/gcc-15/changes.html But I've added it to the internal list. Thanks, Tamar > Thanks for handling this. > Kyrill > > > > > Thanks, > > Richard
Re: [PATCH] c++/modules: Propagate FNDECL_USED_AUTO alongside deduced return types [PR118049]
On Fri, 17 Jan 2025, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>
> -- >8 --
>
> In the linked testcase, we're erroring because the declared return types
> of the functions do not appear to match. This is because when merging
> the deduced return types for 'foo' in 'auto-5_b.C', we overwrote the
> return type for the declaration with the deduced return type from
> 'auto-5_a.C' but neglected to track that we were originally declared
> with 'auto'.
Is this a regression caused by the PR114795 fix? Maybe we should
backport this.
>
> As a drive-by improvement to QOI, also add checks for if the deduced
> return types do not match; this is currently useful because we do not
> check the equivalence of the bodies of functions yet.
>
> PR c++/118049
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_in::is_matching_decl): Propagate
> FNDECL_USED_AUTO as well.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/auto-5_a.C: New test.
> * g++.dg/modules/auto-5_b.C: New test.
> * g++.dg/modules/auto-5_c.C: New test.
> * g++.dg/modules/auto-6_a.H: New test.
> * g++.dg/modules/auto-6_b.C: New test.
>
> Signed-off-by: Nathaniel Shead
> ---
> gcc/cp/module.cc| 5 +
> gcc/testsuite/g++.dg/modules/auto-5_a.C | 10 ++
> gcc/testsuite/g++.dg/modules/auto-5_b.C | 14 ++
> gcc/testsuite/g++.dg/modules/auto-5_c.C | 4
> gcc/testsuite/g++.dg/modules/auto-6_a.H | 5 +
> gcc/testsuite/g++.dg/modules/auto-6_b.C | 6 ++
> 6 files changed, 44 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_b.C
> create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_c.C
> create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 61116fe7669..6fe64bb538c 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -11902,8 +11902,13 @@ trees_in::is_matching_decl (tree existing, tree
> decl, bool is_typedef)
> {
> dump (dumper::MERGE)
> && dump ("Propagating deduced return type to %N", existing);
> + FNDECL_USED_AUTO (e_inner) = true;
> + DECL_SAVED_AUTO_RETURN_TYPE (existing) = TREE_TYPE (e_type);
> TREE_TYPE (existing) = change_return_type (TREE_TYPE (d_type),
> e_type);
> }
> + else if (type_uses_auto (d_ret)
> +&& !same_type_p (TREE_TYPE (d_type), TREE_TYPE (e_type)))
> + goto mismatch;
> }
>else if (is_typedef)
> {
> diff --git a/gcc/testsuite/g++.dg/modules/auto-5_a.C
> b/gcc/testsuite/g++.dg/modules/auto-5_a.C
> new file mode 100644
> index 000..fcab6f301e1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/auto-5_a.C
> @@ -0,0 +1,10 @@
> +// PR c++/118049
> +// { dg-additional-options "-fmodules -Wno-global-module" }
> +// { dg-module-cmi A }
> +
> +module;
> +template struct S {
> + auto foo() {}
> +};
> +export module A;
> +template struct S;
> diff --git a/gcc/testsuite/g++.dg/modules/auto-5_b.C
> b/gcc/testsuite/g++.dg/modules/auto-5_b.C
> new file mode 100644
> index 000..f75ed2d0f0c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/auto-5_b.C
> @@ -0,0 +1,14 @@
> +// PR c++/118049
> +// { dg-additional-options "-fmodules -Wno-global-module" }
> +// { dg-module-cmi B }
> +
> +module;
> +template struct S {
> + auto foo() {}
> +};
> +template struct S;
> +export module B;
> +import A;
> +template void x() {
> + S{}.foo();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/auto-5_c.C
> b/gcc/testsuite/g++.dg/modules/auto-5_c.C
> new file mode 100644
> index 000..f351c2b1ae4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/auto-5_c.C
> @@ -0,0 +1,4 @@
> +// PR c++/118049
> +// { dg-additional-options "-fmodules -fno-module-lazy
> -fdump-lang-module-alias" }
> +
> +import B;
> diff --git a/gcc/testsuite/g++.dg/modules/auto-6_a.H
> b/gcc/testsuite/g++.dg/modules/auto-6_a.H
> new file mode 100644
> index 000..3ad08ab81ce
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/auto-6_a.H
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodule-header" }
> +
> +inline auto foo() {
> + return 1;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/auto-6_b.C
> b/gcc/testsuite/g++.dg/modules/auto-6_b.C
> new file mode 100644
> index 000..aab7be4e530
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/auto-6_b.C
> @@ -0,0 +1,6 @@
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +inline auto foo() { // { dg-error "conflicting" }
> + return 1.0;
> +}
> +import "auto-6_a.H";
> --
> 2.47.0
>
>
Re: [GCC16 stage 1][RFC][PATCH 0/3]extend "counted_by" attribute to pointer fields of structures
Joseph, Kees and Bill,
I need your input on this.
> On Jan 17, 2025, at 10:12, Martin Uecker wrote:
>
> Am Donnerstag, dem 16.01.2025 um 21:18 + schrieb Qing Zhao:
>>
> ..
>>
>> Although in the previous discussion, I agreed with Martin that we should use
>> the
>> designator syntax (i.e, counted_by (.n) instead of counted_by (n)) for the
>> counted_by attribute for pointer fields, after more consideration and
>> discussion
>> with Bill Wendling (who is working on the same work for CLANG), we decided to
>> keep the current syntax of FAM for pointer fields. And leave the new syntax
>> (.n)
>> and more complicate expressions to a later work.
>>
> I think this would be a mistake. Once you have established the confusing
> syntax,
> it will not easily go away anymore. So I think you should use the unambiguous
> and future-prove syntax right away. Support for more complicated expressions
> could be left for later, of course.
Personally I agree with you. -:)
Actually we might need to use such syntax in the very beginning when adding
counted_by of FAM.
As I know, linux kernel community asked for the following new feature for
counted_by of FAM:
struct fs_bulk {
...
int len;
...
}
struct fc_bulk {
...
struct fs_bulk fs_bulk;
struct fc fcs[] __counted_by(fs_bulk.len);
};
i.e, the “counted_by” field is in the inner structure of the current structure
of the FAM.
With the current syntax, it’s not easy to extend to support this.
But with the designator syntax, it might be much easier to be extended to
support this.
So, Kees and Bill, what’s your opinion on this? I think that it’s better to
have a consistent interface between GCC
and Clang.
Joseph, what’s your opinion on this new syntax? Shall we support the
designator syntax for counted_by attribute?
thanks.
Qing
>
> Martin
>
>
>> This patch set includes 3 parts:
>>
>> 1.Extend "counted_by" attribute to pointer fields of structures.
>> 2.Convert a pointer reference with counted_by attribute to .ACCESS_WITH_SIZE
>>and use it in builtinin-object-size.
>> 3.Use the counted_by attribute of pointers in array bound checker.
>>
>> In which, the patch 1 and 2 are simple and straightforward, however, the
>> patch 3
>> is a little complicate due to the following reason:
>>
>>Current array bound checker only instruments ARRAY_REF, and the INDEX
>>information is the 2nd operand of the ARRAY_REF.
>>
>>When extending the array bound checker to pointer references with
>>counted_by attributes, the hardest part is to get the INDEX of the
>>corresponding array ref from the offset computation expression of
>>the pointer ref.
>>
>> The whole patch set has been bootstrapped and regression tested on both
>> aarch64
>> and x86.
>>
>> Let me know any comments and suggestions.
>>
>> Thanks.
>>
>> Qing
>>
>> Qing Zhao (3):
>> Extend "counted_by" attribute to pointer fields of structures.
>> Convert a pointer reference with counted_by attribute to
>>.ACCESS_WITH_SIZE and use it in builtinin-object-size.
>> Use the counted_by attribute of pointers in array bound checker.
>>
>> gcc/c-family/c-attribs.cc | 15 +-
>> gcc/c-family/c-gimplify.cc| 7 +
>> gcc/c-family/c-ubsan.cc | 264 --
>> gcc/c/c-decl.cc | 91 +++---
>> gcc/c/c-typeck.cc | 41 +--
>> gcc/doc/extend.texi | 37 ++-
>> gcc/testsuite/gcc.dg/flex-array-counted-by.c | 2 +-
>> gcc/testsuite/gcc.dg/pointer-counted-by-2.c | 8 +
>> gcc/testsuite/gcc.dg/pointer-counted-by-3.c | 127 +
>> gcc/testsuite/gcc.dg/pointer-counted-by-4.c | 63 +
>> gcc/testsuite/gcc.dg/pointer-counted-by-5.c | 48
>> gcc/testsuite/gcc.dg/pointer-counted-by-6.c | 47
>> gcc/testsuite/gcc.dg/pointer-counted-by-7.c | 30 ++
>> gcc/testsuite/gcc.dg/pointer-counted-by-8.c | 30 ++
>> gcc/testsuite/gcc.dg/pointer-counted-by.c | 70 +
>> .../ubsan/pointer-counted-by-bounds-2.c | 47
>> .../ubsan/pointer-counted-by-bounds-3.c | 35 +++
>> .../ubsan/pointer-counted-by-bounds-4.c | 35 +++
>> .../ubsan/pointer-counted-by-bounds-5.c | 46 +++
>> .../ubsan/pointer-counted-by-bounds-6.c | 33 +++
>> .../gcc.dg/ubsan/pointer-counted-by-bounds.c | 46 +++
>> gcc/tree-object-size.cc | 11 +-
>> 22 files changed, 1045 insertions(+), 88 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-3.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-4.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-5.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-6.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-7.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-8.c
>> create mode
Re: [PATCH v3 1/1] Add warning for non-spec compliant FMV in Aarch64
writes:
> This patch adds a warning when FMV is used for Aarch64.
>
> The reasoning for this is the ACLE [1] spec for FMV has diverged
> significantly from the current implementation and we want to prevent
> potential future compatability issues.
>
> There is a patch for an ACLE compliant version of target_version and
> target_clone in progress but it won't make gcc-15.
>
> This has been bootstrap and regression tested for Aarch64.
> Is this okay for master and packport to gcc-14?
>
> [1]
> https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.cc
> (aarch64_process_target_version_attr): Add experimental warning.
> * config/aarch64/aarch64.opt: Add command line option to disable
> warning.
>
> gcc/testsuite/ChangeLog:
>
> * g++.target/aarch64/mv-1.C: Add CLI flag
> * g++.target/aarch64/mv-symbols1.C: Add CLI flag
> * g++.target/aarch64/mv-symbols2.C: Add CLI flag
> * g++.target/aarch64/mv-symbols3.C: Add CLI flag
> * g++.target/aarch64/mv-symbols4.C: Add CLI flag
> * g++.target/aarch64/mv-symbols5.C: Add CLI flag
> * g++.target/aarch64/mvc-symbols1.C: Add CLI flag
> * g++.target/aarch64/mvc-symbols2.C: Add CLI flag
> * g++.target/aarch64/mvc-symbols3.C: Add CLI flag
> * g++.target/aarch64/mvc-symbols4.C: Add CLI flag
> * g++.target/aarch64/mv-warning1.C: New test.
> * g++.target/aarch64/mvc-warning1.C: New test.
>
> ---
> gcc/config/aarch64/aarch64.cc | 9 +
> gcc/config/aarch64/aarch64.opt | 4
> gcc/doc/invoke.texi | 11 ++-
> gcc/testsuite/g++.target/aarch64/mv-1.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-symbols1.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-symbols2.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-symbols3.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-symbols4.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-symbols5.C | 1 +
> gcc/testsuite/g++.target/aarch64/mv-warning1.C | 9 +
> gcc/testsuite/g++.target/aarch64/mvc-symbols1.C | 1 +
> gcc/testsuite/g++.target/aarch64/mvc-symbols2.C | 1 +
> gcc/testsuite/g++.target/aarch64/mvc-symbols3.C | 1 +
> gcc/testsuite/g++.target/aarch64/mvc-symbols4.C | 1 +
> gcc/testsuite/g++.target/aarch64/mvc-warning1.C | 6 ++
> 15 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.target/aarch64/mv-warning1.C
> create mode 100644 gcc/testsuite/g++.target/aarch64/mvc-warning1.C
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 91de13159cb..eae184c7d65 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -20125,6 +20125,15 @@ aarch64_parse_fmv_features (const char *str,
> aarch64_feature_flags *isa_flags,
> static bool
> aarch64_process_target_version_attr (tree args)
> {
> + static bool issued_warning = false;
> + if (!issued_warning)
> +{
> + warning (OPT_Wexperimental_fmv_target,
> +"Function Multi Versioning support is experimental, and the "
> +"behavior is likely to change");
> + issued_warning = true;
> +}
> +
>if (TREE_CODE (args) == TREE_LIST)
> {
>if (TREE_CHAIN (args))
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index 36bc719b822..2a8dd8ea66c 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -431,3 +431,7 @@ handling. One means we try to form pairs involving one
> or more existing
> individual writeback accesses where possible. A value of two means we
> also try to opportunistically form writeback opportunities by folding in
> trailing destructive updates of the base register used by a pair.
> +
> +Wexperimental-fmv-target
> +Target Var(warn_experimental_fmv) Warning Init(1)
> +Warn about usage of experimental Function Multi Versioning.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 51dc871e6bc..bdf9ee1bc0c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -822,7 +822,8 @@ Objective-C and Objective-C++ Dialects}.
> -moverride=@var{string} -mverbose-cost-dump
> -mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{sysreg}
> -mstack-protector-guard-offset=@var{offset} -mtrack-speculation
> --moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion}
> +-moutline-atomics -mearly-ldp-fusion -mlate-ldp-fusion
> +-Wexperimental-fmv-target}
>
> @emph{Adapteva Epiphany Options}
> @gccoptlist{-mhalf-reg-file -mprefer-short-insn-regs
> @@ -22087,6 +22088,14 @@ which specify use of that register as a fixed
> register,
> and @samp{none}, which means that no register is used for this
> purpose. The default is @option{-m1reg-none}.
>
> +@opindex Wexperimental-fmv-target
> +@opindex Wno-experimental-fmv-target
> +@item -Wexperimental-fmv-target
Re: [PATCH] c++: Copy over further 2 flags for !TREE_PUBLIC in copy_linkage [PR118513]
On 1/17/25 4:26 AM, Jakub Jelinek wrote:
Hi!
The following testcase ICEs in import_export_decl.
When cp_finish_decomp handles std::tuple* using structural binding,
it calls copy_linkage to copy various VAR_DECL flags from the structured
binding base to the individual sb variables.
In this case the base variable is in anonymous union, so we call
constrain_visibility (..., VISIBILITY_ANON, ...) on it which e.g.
clears TREE_PUBLIC etc. (flags which copy_linkage copies) but doesn't
copy over DECL_INTERFACE_KNOWN/DECL_NOT_REALLY_EXTERN.
When cp_finish_decl calls determine_visibility on the individual sb
variables, those have !TREE_PUBLIC since copy_linkage and so nothing tries
to determine visibility and nothing sets DECL_INTERFACE_KNOWN and
DECL_NOT_REALLY_EXTERN.
Now, this isn't a big deal without modules, the individual variables are
var_finalized_p and so nothing really cares about missing
DECL_INTERFACE_KNOWN. But in the module case the variables are streamed
out and in and care about those bits.
The following patch is an attempt to copy over also those flags (but I've
limited it to the !TREE_PUBLIC case just in case). Other option would be
to call it unconditionally, or call constrain_visibility with
VISIBILITY_ANON for !TREE_PUBLIC (but are all !TREE_PUBLIC constrained
visibility) or do it only in the cp_finish_decomp case
after the copy_linkage call there.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-01-17 Jakub Jelinek
PR c++/118513
* decl2.cc (copy_linkage): If not TREE_PUBLIC, also copy
DECL_INTERFACE_KNOWN and DECL_NOT_REALLY_EXTERN flags.
* g++.dg/modules/decomp-3_a.H: New test.
* g++.dg/modules/decomp-3_b.C: New test.
--- gcc/cp/decl2.cc.jj 2025-01-16 13:34:29.719360710 +0100
+++ gcc/cp/decl2.cc 2025-01-16 16:25:37.006911025 +0100
@@ -3656,6 +3656,12 @@ copy_linkage (tree guard, tree decl)
comdat_linkage (guard);
DECL_VISIBILITY (guard) = DECL_VISIBILITY (decl);
DECL_VISIBILITY_SPECIFIED (guard) = DECL_VISIBILITY_SPECIFIED (decl);
+ if (!TREE_PUBLIC (decl))
+ {
+ DECL_INTERFACE_KNOWN (guard) = DECL_INTERFACE_KNOWN (decl);
+ if (DECL_LANG_SPECIFIC (decl) && DECL_LANG_SPECIFIC (guard))
+ DECL_NOT_REALLY_EXTERN (guard) = DECL_NOT_REALLY_EXTERN (decl);
+ }
Hmm, those should always be set on !TREE_PUBLIC. Maybe also add a
checking_assert that DECL_INTERFACE_KNOWN is set on decl? OK with that
change.
DECL_NOT_REALLY_EXTERN might not be as consistent.
}
}
--- gcc/testsuite/g++.dg/modules/decomp-3_a.H.jj 2025-01-16 13:43:36.492879734 +0100
+++ gcc/testsuite/g++.dg/modules/decomp-3_a.H 2025-01-16 13:43:11.314224231
+0100
@@ -0,0 +1,20 @@
+// PR c++/118513
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+namespace std {
+ template struct tuple_size;
+ template struct tuple_element;
+}
+
+struct A {
+ int a, b;
+ template int &get () { if (I == 0) return a; else return b; }
+};
+
+template <> struct std::tuple_size { static const int value = 2; };
+template struct std::tuple_element { using type = int; };
+
+namespace {
+auto [x, y] = A { 42, 43 };
+}
--- gcc/testsuite/g++.dg/modules/decomp-3_b.C.jj2025-01-16
13:43:41.782807354 +0100
+++ gcc/testsuite/g++.dg/modules/decomp-3_b.C 2025-01-16 13:41:49.256346946
+0100
@@ -0,0 +1,12 @@
+// PR c++/118513
+// { dg-do run }
+// { dg-additional-options "-fmodules-ts" }
+
+import "decomp-3_a.H";
+
+int
+main ()
+{
+ if (x != 42 || y != 43)
+__builtin_abort ();
+}
Jakub
[PATCH v2] c++: fix wrong-code with constexpr prvalue opt [PR118396]
On Fri, Jan 17, 2025 at 08:10:24AM -0500, Jason Merrill wrote:
> On 1/16/25 8:04 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> > The recent r15-6369 unfortunately caused a bad wrong-code issue.
> > Here we have
> >
> >TARGET_EXPR > .data={._vptr.Foo=&_ZTV3Foo + 16}})>
> >
> > and call cp_fold_r -> maybe_constant_init with object=D.2996. In
> > cxx_eval_outermost_constant_expr we now take the type of the object
> > if present. An object can't have type 'void' and so we continue to
> > evaluate the initializer. That evaluates into a VOID_CST, meaning
> > we disregard the whole initializer, and terrible things ensue.
>
> In that case, I'd think we want to use the value of 'object' (which should
> be in ctx.ctor?) instead of the return value of
> cxx_eval_constant_expression.
Ah, I'm sorry I didn't choose that approach. Maybe like this, then?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
The recent r15-6369 unfortunately caused a bad wrong-code issue.
Here we have
TARGET_EXPR
and call cp_fold_r -> maybe_constant_init with object=D.2996. In
cxx_eval_outermost_constant_expr we now take the type of the object
if present. An object can't have type 'void' and so we continue to
evaluate the initializer. That evaluates into a VOID_CST, meaning
we disregard the whole initializer, and terrible things ensue.
For non-simple TARGET_EXPRs, we should return ctx.ctor rather than
the result of cxx_eval_constant_expression.
PR c++/118396
PR c++/118523
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_outermost_constant_expr): For non-simple
TARGET_EXPRs, return ctx.ctor rather than the result of
cxx_eval_constant_expression.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-prvalue4.C: New test.
* g++.dg/cpp1y/constexpr-prvalue3.C: New test.
---
gcc/cp/constexpr.cc | 5 +++
.../g++.dg/cpp0x/constexpr-prvalue4.C | 33 ++
.../g++.dg/cpp1y/constexpr-prvalue3.C | 45 +++
3 files changed, 83 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index c898e3bfa6e..078f7b19f24 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8978,6 +8978,11 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
r = cxx_eval_constant_expression (&ctx, r, vc_prvalue,
&non_constant_p, &overflow_p);
+ /* If we got a non-simple TARGET_EXPR, the initializer was a sequence
+ of statements, and the result ought to be stored in ctx.ctor. */
+ if (r == void_node && !constexpr_dtor && ctx.ctor)
+r = ctx.ctor;
+
if (!constexpr_dtor)
verify_constant (r, allow_non_constant, &non_constant_p, &overflow_p);
else
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
new file mode 100644
index 000..afcee65f880
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-prvalue4.C
@@ -0,0 +1,33 @@
+// PR c++/118396
+// { dg-do run { target c++11 } }
+// { dg-options "-O" }
+
+void *operator new(__SIZE_TYPE__, void *__p) { return __p; }
+
+struct Foo {
+ virtual ~Foo() = default;
+};
+struct Data {
+ int status;
+ Foo data{};
+};
+
+Data *P, *Q;
+
+struct vector {
+ vector (const Data &__value) {
+P = static_cast(__builtin_operator_new(0));
+new (P) Data (__value);
+Q = P + 1;
+ }
+ Data *begin() { return P; }
+ Data *end() { return Q; }
+};
+
+int
+main ()
+{
+ vector items_(Data{});
+ for (auto item : items_)
+item.status == 0 ? void() : __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
new file mode 100644
index 000..8ea86c60be5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-prvalue3.C
@@ -0,0 +1,45 @@
+// PR c++/118523
+// { dg-do compile { target c++14 } }
+// { dg-options "-O2 -Wall" }
+
+struct __new_allocator {
+ constexpr __new_allocator() {}
+ __new_allocator(__new_allocator &) {}
+};
+template using __allocator_base = __new_allocator;
+template struct allocator_traits;
+template struct allocator : __allocator_base {};
+template struct allocator_traits> {
+ using pointer = _Tp *;
+ template using rebind_alloc = allocator<_Up>;
+ static void deallocate(allocator<_Tp>, pointer, long);
+};
+struct __alloc_traits : allocator_traits> {};
+struct _Vector_impl_data {
+ __alloc_traits::pointer _M_start;
+ __alloc_traits::pointer _M_end_of_storage;
+ constexpr _Vector_impl_data() : _M_start(), _M_end_of_storage() {}
+};
+struct _Vector_impl : __alloc_traits::rebind_alloc, _Vector_impl_data {};
+struct _Vector_base {
+ ~_Vector_base() {
+_M_deallocate(_M_impl._M_start,
+ _M_i
[PATCH] match.pd: Fix (FTYPE) N CMP (FTYPE) M optimization for GENERIC [PR118522]
Hi!
The last case of this optimization assumes that if 2 integral types
have same precision and TYPE_UNSIGNED, then they are uselessly convertible.
While that is very likely the case for GIMPLE, it is not the case for
GENERIC, so the following patch adds there a convert so that the
optimization produces also valid GENERIC. Without it we got
(int) p == b where b had _BitInt(32) type, so incompatible types.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-01-17 Jakub Jelinek
PR tree-optimization/118522
* match.pd ((FTYPE) N CMP (FTYPE) M): Add convert, as in GENERIC
integral types with the same precision and sign might actually not
be compatible types.
* gcc.dg/bitint-120.c: New test.
--- gcc/match.pd.jj 2025-01-16 09:09:06.0 +0100
+++ gcc/match.pd2025-01-16 19:24:16.838305254 +0100
@@ -7253,7 +7253,7 @@ (define_operator_list SYNC_FETCH_AND_AND
(icmp (convert:type2 @1) @2)
(if (TYPE_PRECISION (type1) == TYPE_PRECISION (type2)
&& type1_signed_p == type2_signed_p)
- (icmp @1 @2))
+ (icmp @1 (convert @2)))
/* Optimize various special cases of (FTYPE) N CMP CST. */
(for cmp (lt le eq ne ge gt)
--- gcc/testsuite/gcc.dg/bitint-120.c.jj2025-01-16 19:30:14.816518100
+0100
+++ gcc/testsuite/gcc.dg/bitint-120.c 2025-01-16 19:29:56.429763982 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/118522 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O2" } */
+
+_BitInt(32) b;
+
+int
+foo (unsigned short p)
+{
+ return p == (double) b;
+}
Jakub
[Patch] OpenMP/C++: Fix declare_variant's 'adjust_args' if there is a 'this' pointer [PR118321]
OpenMP's adjust_args clause to 'declare variant' can request to
convert a pointer to a device pointer in the call ('need_device_ptr').
The arguments are stored by their position in the argument list,
but if hidden arguments get added - like C++'s 'this' pointer,
the list if off by one. This patch fixes it for the 'this' pointer.
Tested on x86_64-gnu-linux.
Any comment, suggestion, remark before I push it?
Tobias
PS: I wonder whether any of the other ways to get hidden arguments
(at the front) matter. For C++, I see the VTT parm and for Fortran
the ENTRY master argument, but I don't yet understand whether that
matters for 'declare variant' or not.
OpenMP/C++: Fix declare_variant's 'adjust_args' if there is a 'this' pointer [PR118321]
The adjust_args clause is internally store as the i-th argument to the function,
which fails if hidden arguments come before. This commit handles the C++ 'this'
pointer by shifting the internal arg index by one.
PR fortran/118321
gcc/cp/ChangeLog:
* decl.cc (omp_declare_variant_finalize_one): Shift adjust_args index
by one for non-static class function's 'this' pointer.
gcc/testsuite/ChangeLog:
* g++.dg/gomp/adjust-args-4.C: New test.
gcc/cp/decl.cc| 19 --
gcc/testsuite/g++.dg/gomp/adjust-args-4.C | 60 +++
2 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index ef887915ff1..af1b89fb8c9 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8629,9 +8629,22 @@ omp_declare_variant_finalize_one (tree decl, tree attr)
// Prepend adjust_args list to variant attributes
tree adjust_args_list = TREE_CHAIN (TREE_CHAIN (chain));
if (adjust_args_list != NULL_TREE)
- DECL_ATTRIBUTES (variant) = tree_cons (
- get_identifier ("omp declare variant variant args"),
- TREE_VALUE (adjust_args_list), DECL_ATTRIBUTES (variant));
+ {
+ if (DECL_NONSTATIC_MEMBER_P (variant)
+ && TREE_VALUE (adjust_args_list))
+ {
+ /* Shift arg position for the added 'this' pointer. */
+ /* Handle need_device_ptr */
+ for (tree t = TREE_PURPOSE (TREE_VALUE (adjust_args_list));
+ t; t = TREE_CHAIN (t))
+ TREE_VALUE (t)
+ = build_int_cst (TREE_TYPE (t),
+ tree_to_uhwi (TREE_VALUE (t)) + 1);
+ }
+ DECL_ATTRIBUTES (variant) = tree_cons (
+ get_identifier ("omp declare variant variant args"),
+ TREE_VALUE (adjust_args_list), DECL_ATTRIBUTES (variant));
+ }
}
}
else if (!processing_template_decl)
diff --git a/gcc/testsuite/g++.dg/gomp/adjust-args-4.C b/gcc/testsuite/g++.dg/gomp/adjust-args-4.C
new file mode 100644
index 000..83100457e3e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/adjust-args-4.C
@@ -0,0 +1,60 @@
+/* { dg-additional-options "-fdump-tree-gimple" } */
+
+/* PR fortran/118321. */
+
+/* Check that adjust_args applies to the right argument,
+ if C++ inserts a 'this' pointer. */
+
+struct t1 {
+ void f1(int *x, int *y, int *z);
+ #pragma omp declare variant(f1) match(construct={dispatch}) \
+ adjust_args(need_device_ptr : y)
+ void g1(int *x, int *y, int *z);
+};
+
+struct t2 {
+ void f2(int *x, int *y, int *z);
+ #pragma omp declare variant(f2) match(construct={dispatch}) \
+ adjust_args(need_device_ptr : x, y, z)
+ void g2(int *x, int *y, int *z);
+};
+
+struct t3 {
+ void f3(int *x, int *y, int *z);
+ #pragma omp declare variant(f3) match(construct={dispatch}) \
+ adjust_args(nothing : x, y, z)
+ void g3(int *x, int *y, int *z);
+};
+
+
+void test(int *a1, int *b1, int *c1,
+ int *a2, int *b2, int *c2,
+ int *a3, int *b3, int *c3)
+{
+ struct t1 s1;
+ struct t2 s2;
+ struct t3 s3;
+
+ #pragma omp dispatch
+ s1.g1 (a1, b1, c1);
+ #pragma omp dispatch
+ s2.g2 (a2, b2, c2);
+ #pragma omp dispatch
+ s3.g3 (a3, b3, c3);
+}
+
+
+/* { dg-final { scan-tree-dump-times "D\.\[0-9\]+ = __builtin_omp_get_default_device \\(\\);" 2 "gimple" } } */
+
+/* { dg-final { scan-tree-dump-times "__builtin_omp_get_mapped_ptr" 4 "gimple" } } */
+
+/* { dg-final { scan-tree-dump "D\.\[0-9\]+ = __builtin_omp_get_mapped_ptr \\(b1, D\.\[0-9\]+\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "t1::f1 \\(&s1, a1, D\.\[0-9\]+, c1\\);" "gimple" } } */
+
+
+/* { dg-final { scan-tree-dump "D\.\[0-9\]+ = __builtin_omp_get_mapped_ptr \\(c2, D\.\[0-9\]+\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "D\.\[0-9\]+ = __builtin_omp_get_mapped_ptr \\(b2, D\.\[0-9\]+\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "D\.\[0-9\]+ = __builtin_omp_get_mapped_ptr \\(a2, D\.\[0-9\]+\\);" "gimple" } } */
+/* { dg-final { scan-tree-dump "t2::f2 \\(&s2, D\.\[0-9\]+, D\.\[0-9\]+, D\.\[0-9\]+\\);" "gimple" } } */
+
+/* { dg-final { scan-tree-dump "t3::f3 \\(&s3, a3, b3, c3\\);" "gimple" } } */
Re: [patch,avr] Use INT_N to built-in define __int24.
чт, 16 янв. 2025 г. в 21:19, Georg-Johann Lay : > > This patch uses the INT_N interface to define __int24. > > Ok for trunk? Ok. Please apply. Denis.
[PATCH] s390: Replace some checking assertions with output_operand_lossage [PR118511]
Hi!
r15-2002 s390: Fully exploit vgm, vgbm, vrepi change added
some code to print_operand and added gcc_checking_asserts in there.
But print_operand ideally should have no assertions in it, as most
of the assumptions can be easily violated by people using it in
inline asm.
This issue in particular was seen by failure to compile s390-tools,
which had in its extended inline asm uses of %r1 and %r2.
I really don't know if they meant %%r1 and %%r2 or %1 and %2 and
will leave that decision to the maintainers, but the thing is that
%r1 and %r2 used to expand like %1 and %2 in GCC 14 and earlier,
now in checking build it ICEs and in --enable-checking=release build
fails to assemble (the checking assert is ignored and the compiler just uses
some uninitialized variables to emit something arbitrary).
With the following patch it is diagnosed as error consistently
regardless if it is release checking or no checking or checking compiler.
Bootstrapped/regtested on s390x-linux, ok for trunk?
Note, I see also
else if (GET_CODE (x) == UNSPEC && XINT (x, 1) == UNSPEC_TLSLDM)
{
fprintf (file, "%s", ":tls_ldcall:");
const char *name = get_some_local_dynamic_name ();
gcc_assert (name);
assemble_name (file, name);
}
in print_operand, maybe that isn't a big deal because it might be
impossible to construct inline asm argument which is UNSPEC_TLSLDM.
And then there is
case 'e': case 'f':
case 's': case 't':
{
int start, end;
int len;
bool ok;
len = (code == 's' || code == 'e' ? 64 : 32);
ok = s390_contiguous_bitmask_p (ival, true, len, &start, &end);
gcc_assert (ok);
if (code == 's' || code == 't')
ival = start;
else
ival = end;
}
break;
which likely should be also output_operand_lossage but I haven't tried
to reproduce that.
2025-01-17 Jakub Jelinek
PR target/118511
* config/s390/s390.cc (print_operand) : Use
output_operand_lossage instead of gcc_checking_assert.
(print_operand) : Likewise.
(print_operand) : Likewise.
* gcc.target/s390/pr118511.c: New test.
--- gcc/config/s390/s390.cc.jj 2025-01-10 16:09:37.280043199 +0100
+++ gcc/config/s390/s390.cc 2025-01-16 10:43:58.533809724 +0100
@@ -8688,8 +8688,12 @@ print_operand (FILE *file, rtx x, int co
{
machine_mode mode;
short imm;
- bool b = s390_constant_via_vrepi_p (x, &mode, &imm);
- gcc_checking_assert (b);
+ if (!s390_constant_via_vrepi_p (x, &mode, &imm))
+ {
+ output_operand_lossage ("invalid constant for output "
+ "modifier '%c'", code);
+ return;
+ }
switch (mode)
{
case QImode:
@@ -8714,8 +8718,12 @@ print_operand (FILE *file, rtx x, int co
{
machine_mode mode;
int start, end;
- bool b = s390_constant_via_vgm_p (x, &mode, &start, &end);
- gcc_checking_assert (b);
+ if (!s390_constant_via_vgm_p (x, &mode, &start, &end))
+ {
+ output_operand_lossage ("invalid constant for output "
+ "modifier '%c'", code);
+ return;
+ }
switch (mode)
{
case QImode:
@@ -8739,8 +8747,12 @@ print_operand (FILE *file, rtx x, int co
case 'r':
{
unsigned mask;
- bool b = s390_constant_via_vgbm_p (x, &mask);
- gcc_checking_assert (b);
+ if (!s390_constant_via_vgbm_p (x, &mask))
+ {
+ output_operand_lossage ("invalid constant for output "
+ "modifier '%c'", code);
+ return;
+ }
fprintf (file, "%u", mask);
}
return;
--- gcc/testsuite/gcc.target/s390/pr118511.c.jj 2025-01-16 10:51:14.025721964
+0100
+++ gcc/testsuite/gcc.target/s390/pr118511.c2025-01-16 10:50:48.009085651
+0100
@@ -0,0 +1,11 @@
+/* PR target/118511 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void
+foo (void)
+{
+ asm volatile ("# %p0" : : "d" (1)); /* { dg-error "invalid 'asm': invalid
constant for output modifier 'p'" } */
+ asm volatile ("# %q0" : : "d" (1)); /* { dg-error "invalid 'asm': invalid
constant for output modifier 'q'" } */
+ asm volatile ("# %r0" : : "d" (1)); /* { dg-error "invalid 'asm': invalid
constant for output modifier 'r'" } */
+}
Jakub
Re: [PATCH] match.pd: Fix (FTYPE) N CMP (FTYPE) M optimization for GENERIC [PR118522]
On Fri, 17 Jan 2025, Jakub Jelinek wrote:
> Hi!
>
> The last case of this optimization assumes that if 2 integral types
> have same precision and TYPE_UNSIGNED, then they are uselessly convertible.
> While that is very likely the case for GIMPLE, it is not the case for
> GENERIC, so the following patch adds there a convert so that the
> optimization produces also valid GENERIC. Without it we got
> (int) p == b where b had _BitInt(32) type, so incompatible types.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2025-01-17 Jakub Jelinek
>
> PR tree-optimization/118522
> * match.pd ((FTYPE) N CMP (FTYPE) M): Add convert, as in GENERIC
> integral types with the same precision and sign might actually not
> be compatible types.
>
> * gcc.dg/bitint-120.c: New test.
>
> --- gcc/match.pd.jj 2025-01-16 09:09:06.0 +0100
> +++ gcc/match.pd 2025-01-16 19:24:16.838305254 +0100
> @@ -7253,7 +7253,7 @@ (define_operator_list SYNC_FETCH_AND_AND
> (icmp (convert:type2 @1) @2)
> (if (TYPE_PRECISION (type1) == TYPE_PRECISION (type2)
>&& type1_signed_p == type2_signed_p)
> - (icmp @1 @2))
> + (icmp @1 (convert @2)))
>
> /* Optimize various special cases of (FTYPE) N CMP CST. */
> (for cmp (lt le eq ne ge gt)
> --- gcc/testsuite/gcc.dg/bitint-120.c.jj 2025-01-16 19:30:14.816518100
> +0100
> +++ gcc/testsuite/gcc.dg/bitint-120.c 2025-01-16 19:29:56.429763982 +0100
> @@ -0,0 +1,11 @@
> +/* PR tree-optimization/118522 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-O2" } */
> +
> +_BitInt(32) b;
> +
> +int
> +foo (unsigned short p)
> +{
> + return p == (double) b;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
[PATCH] RISC-V: Disable fusing vsetvl instructions by VSETVL_VTYPE_CHANGE_ONLY for XTheadVector.
In RVV 1.0, the instruction "vsetvlizero,zero,*" indicates that the
available vector length (avl) does not change. However, in XTheadVector,
this same instruction signifies that the avl should take the maximum value.
Consequently, when fusing vsetvl instructions, the optimization labeled
"VSETVL_VTYPE_CHANGE_ONLY" is disabled for XTheadVector.
PR 118357
gcc/ChangeLog:
* config/riscv/riscv-vsetvl.cc: Function change_vtype_only_p always
returns false for XTheadVector.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/xtheadvector/pr118357.c: New test.
Reported-by: nihui
---
gcc/config/riscv/riscv-vsetvl.cc| 3 ++-
.../gcc.target/riscv/rvv/xtheadvector/pr118357.c| 13 +
2 files changed, 15 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr118357.c
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index a4016beebc0c..72c4c59514e5 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -903,7 +903,8 @@ public:
bool valid_p () const { return m_state == state_type::VALID; }
bool unknown_p () const { return m_state == state_type::UNKNOWN; }
bool empty_p () const { return m_state == state_type::EMPTY; }
- bool change_vtype_only_p () const { return m_change_vtype_only; }
+ bool change_vtype_only_p () const { return m_change_vtype_only
+&& !TARGET_XTHEADVECTOR; }
void set_valid () { m_state = state_type::VALID; }
void set_unknown () { m_state = state_type::UNKNOWN; }
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr118357.c
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr118357.c
new file mode 100644
index ..aebb0e3088ab
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/pr118357.c
@@ -0,0 +1,13 @@
+/* { dg-do compile { target { rv64 } } } */
+/* { dg-options "-march=rv64gc_xtheadvector -mabi=lp64d -O2" } */
+
+#include
+
+vfloat16m4_t foo (float *ptr, size_t vl)
+{
+ vfloat32m8_t _p = __riscv_vle32_v_f32m8 (ptr, vl);
+ vfloat16m4_t _half = __riscv_vfncvt_f_f_w_f16m4 (_p, vl);
+ return _half;
+}
+
+/* { dg-final { scan-assembler-not {th.vsetvli\tzero,zero} } }*/
--
2.25.1
[PATCH] c++/modules: Propagate FNDECL_USED_AUTO alongside deduced return types [PR118049]
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
In the linked testcase, we're erroring because the declared return types
of the functions do not appear to match. This is because when merging
the deduced return types for 'foo' in 'auto-5_b.C', we overwrote the
return type for the declaration with the deduced return type from
'auto-5_a.C' but neglected to track that we were originally declared
with 'auto'.
As a drive-by improvement to QOI, also add checks for if the deduced
return types do not match; this is currently useful because we do not
check the equivalence of the bodies of functions yet.
PR c++/118049
gcc/cp/ChangeLog:
* module.cc (trees_in::is_matching_decl): Propagate
FNDECL_USED_AUTO as well.
gcc/testsuite/ChangeLog:
* g++.dg/modules/auto-5_a.C: New test.
* g++.dg/modules/auto-5_b.C: New test.
* g++.dg/modules/auto-5_c.C: New test.
* g++.dg/modules/auto-6_a.H: New test.
* g++.dg/modules/auto-6_b.C: New test.
Signed-off-by: Nathaniel Shead
---
gcc/cp/module.cc| 5 +
gcc/testsuite/g++.dg/modules/auto-5_a.C | 10 ++
gcc/testsuite/g++.dg/modules/auto-5_b.C | 14 ++
gcc/testsuite/g++.dg/modules/auto-5_c.C | 4
gcc/testsuite/g++.dg/modules/auto-6_a.H | 5 +
gcc/testsuite/g++.dg/modules/auto-6_b.C | 6 ++
6 files changed, 44 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-5_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/auto-6_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 61116fe7669..6fe64bb538c 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11902,8 +11902,13 @@ trees_in::is_matching_decl (tree existing, tree decl,
bool is_typedef)
{
dump (dumper::MERGE)
&& dump ("Propagating deduced return type to %N", existing);
+ FNDECL_USED_AUTO (e_inner) = true;
+ DECL_SAVED_AUTO_RETURN_TYPE (existing) = TREE_TYPE (e_type);
TREE_TYPE (existing) = change_return_type (TREE_TYPE (d_type),
e_type);
}
+ else if (type_uses_auto (d_ret)
+ && !same_type_p (TREE_TYPE (d_type), TREE_TYPE (e_type)))
+ goto mismatch;
}
else if (is_typedef)
{
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_a.C
b/gcc/testsuite/g++.dg/modules/auto-5_a.C
new file mode 100644
index 000..fcab6f301e1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_a.C
@@ -0,0 +1,10 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -Wno-global-module" }
+// { dg-module-cmi A }
+
+module;
+template struct S {
+ auto foo() {}
+};
+export module A;
+template struct S;
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_b.C
b/gcc/testsuite/g++.dg/modules/auto-5_b.C
new file mode 100644
index 000..f75ed2d0f0c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_b.C
@@ -0,0 +1,14 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -Wno-global-module" }
+// { dg-module-cmi B }
+
+module;
+template struct S {
+ auto foo() {}
+};
+template struct S;
+export module B;
+import A;
+template void x() {
+ S{}.foo();
+}
diff --git a/gcc/testsuite/g++.dg/modules/auto-5_c.C
b/gcc/testsuite/g++.dg/modules/auto-5_c.C
new file mode 100644
index 000..f351c2b1ae4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-5_c.C
@@ -0,0 +1,4 @@
+// PR c++/118049
+// { dg-additional-options "-fmodules -fno-module-lazy
-fdump-lang-module-alias" }
+
+import B;
diff --git a/gcc/testsuite/g++.dg/modules/auto-6_a.H
b/gcc/testsuite/g++.dg/modules/auto-6_a.H
new file mode 100644
index 000..3ad08ab81ce
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-6_a.H
@@ -0,0 +1,5 @@
+// { dg-additional-options "-fmodule-header" }
+
+inline auto foo() {
+ return 1;
+}
diff --git a/gcc/testsuite/g++.dg/modules/auto-6_b.C
b/gcc/testsuite/g++.dg/modules/auto-6_b.C
new file mode 100644
index 000..aab7be4e530
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-6_b.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+inline auto foo() { // { dg-error "conflicting" }
+ return 1.0;
+}
+import "auto-6_a.H";
--
2.47.0
Re: [PATCH] c++: Allow pragmas in NSDMIs [PR118147]
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2024-December/672098.html.
On Fri, Dec 20, 2024 at 10:17:43PM +1100, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk and
> maybe release branches?
>
> -- >8 --
>
> This patch removes the (unnecessary) CPP_PRAGMA_EOL case from
> cp_parser_cache_defarg, which currently has the result that any pragmas
> in the NSDMI cause an error.
>
> PR c++/118147
>
> gcc/cp/ChangeLog:
>
> * parser.cc (cp_parser_cache_defarg): Don't error when
> CPP_PRAGMA_EOL.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/nsdmi-defer7.C: New test.
>
> Signed-off-by: Nathaniel Shead
> ---
> gcc/cp/parser.cc | 1 -
> gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C | 13 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
>
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 23c6a2fd30e..527a858ea52 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -36316,7 +36316,6 @@ cp_parser_cache_defarg (cp_parser *parser, bool nsdmi)
>
> /* If we run out of tokens, issue an error message. */
> case CPP_EOF:
> - case CPP_PRAGMA_EOL:
> error_at (token->location, "file ends in default argument");
> return error_mark_node;
>
> diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
> b/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
> new file mode 100644
> index 000..3bef636ccbd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-defer7.C
> @@ -0,0 +1,13 @@
> +// PR c++/118147
> +// { dg-do compile { target c++11 } }
> +
> +struct F {
> + int i = []{
> +#pragma message "test" // { dg-message "test" }
> +return 1;
> + }();
> +};
> +
> +struct G {
> + int i =
> +#pragma GCC diagnostic push // { dg-error "file ends in default
> argument|expected" }
> --
> 2.47.0
>
RE: [PATCH] AArch64: Deprecate -mabi=ilp32
> -Original Message-
> From: Wilco Dijkstra
> Sent: Tuesday, January 14, 2025 5:30 PM
> To: Richard Sandiford
> Cc: Richard Earnshaw ; ktkac...@nvidia.com; GCC
> Patches ; sch...@linux-m68k.org
> Subject: Re: [PATCH] AArch64: Deprecate -mabi=ilp32
>
> Hi Richard,
>
> >> + if (TARGET_ILP32)
> >> + warning (OPT_Wdeprecated, "%<-mabi=ilp32%> is deprecated.");
> >
> > There should be no "." at the end of the message.
>
> Right, fixed in v2 below.
>
> > Otherwise it looks good to me, although like Kyrill says, it'll also
> > need a release note.
>
> I've added one, see https://gcc.gnu.org/pipermail/gcc-patches/2025-
> January/673575.html
>
> > Please give others 24 hours to comment. Otherwise the patch is OK
> > with that change.
>
> Sure.
>
> > (I saw Andreas's comment about starting sentences with a lowercase
> > letter, but IMO that's ok for program text.)
>
> I changed it to "The @samp{ilp32} model is deprecated." (using "option" felt
> wrong here).
>
> Cheers,
> Wilco
>
>
> v2: Update after review
>
> ILP32 was originally intended to make porting to AArch64 easier. Support was
> never merged in the Linux kernel or GLIBC, so it has been unsupported for many
> years. There isn't a benefit in keeping unsupported features forever, so
> deprecate it now (and it could be removed in a future release).
>
> Passes regress & bootstrap, OK for commit?
Hi,
This patch seems to break embedded builds as on -elf platforms we build ilp32 as
a default multilib variant.
That is in gcc/config.gcc
aarch64*-*-elf | aarch64*-*-fuchsia* | aarch64*-*-rtems*
all do:
aarch64_multilibs="${with_multilib_list}"
if test "$aarch64_multilibs" = "default"; then
aarch64_multilibs="lp64,ilp32"
fi
Which then fails the build because of -Werror.
So the question is as the discussion above has been about Linux, should the
deprecation just be on Linux or also ELF, and if ELF should we remove it from
the default multilib?
Thanks,
Tamar
>
> gcc:
> * config/aarch64/aarch64.cc (aarch64_override_options): Add warning.
> * doc/invoke.texi: Document -mabi=ilp32 as deprecated.
>
> gcc/testsuite:
> * gcc.target/aarch64/inline-mem-set-pr112804.c: Add -Wno-deprecated.
> * gcc.target/aarch64/pr100518.c: Likewise.
> * gcc.target/aarch64/pr113114.c: Likewise.
> * gcc.target/aarch64/pr80295.c: Likewise.
> * gcc.target/aarch64/pr94201.c: Likewise.
> * gcc.target/aarch64/pr94577.c: Likewise.
> * gcc.target/aarch64/sve/pr108603.c: Likewise.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index
> ad31e9d255c05dda00c7c2b4755ccec33ae2c83d..1dbbc9c3cf9b2afba80fa05d6b
> 37e11d5780169e 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -19356,6 +19356,8 @@ aarch64_override_options (void)
>if (TARGET_ILP32)
> error ("assembler does not support %<-mabi=ilp32%>");
> #endif
> + if (TARGET_ILP32)
> +warning (OPT_Wdeprecated, "%<-mabi=ilp32%> is deprecated");
>
>/* Convert -msve-vector-bits to a VG count. */
>aarch64_sve_vg = aarch64_convert_sve_vector_bits (aarch64_sve_vector_bits);
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index
> dd0d2b41a1a9ada3a10280b4188d5bf3a0a873e6..13afb4a0d0d87dacc63bcc46
> 1612f7ffa2afd3ad 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -21472,6 +21472,8 @@ The default depends on the specific target
> configuration. Note that
> the LP64 and ILP32 ABIs are not link-compatible; you must compile your
> entire program with the same ABI, and link with a compatible set of
> libraries.
>
> +The @samp{ilp32} model is deprecated.
> +
> @opindex mbig-endian
> @item -mbig-endian
> Generate big-endian code. This is the default when GCC is configured for an
> diff --git a/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c
> b/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c
> index
> fe8414559864db4a8584fd3f5a7145b5e3d1f322..276c10cd0e86ff2c74a5c09ce
> 70f7d76614978ec 100644
> --- a/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c
> +++ b/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-finline-stringops -mabi=ilp32
> -ftrivial-auto-var-init=zero" } */
> +/* { dg-options "-finline-stringops -mabi=ilp32 -Wno-deprecated
> -ftrivial-auto-
> var-init=zero" } */
>
> short m(unsigned k) {
>const unsigned short *n[65];
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr100518.c
> b/gcc/testsuite/gcc.target/aarch64/pr100518.c
> index
> 5ca599f5d2e0e1603456b2eaf2e98866871faad1..177991cfb2289530e4ee3e36
> 33fddde5972e9e28 100644
> --- a/gcc/testsuite/gcc.target/aarch64/pr100518.c
> +++ b/gcc/testsuite/gcc.target/aarch64/pr100518.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-mabi=ilp32 -mstrict-align -O2" } */
> +/* { dg-options "-mabi=ilp32
