Patch [7 of 7]: Enable using vector pair load/store for -mcpu=future
Late in the development of power10, we discovered that there were some issues in using load vector pair and store vector pair instructions to do memory copies, so the defaults were modified to not use these instructions. This patch re-enables using load and store vector pair instructions. Previously the -mblock-ops-vector-pair switch was not set in POWERPC_MASKS. This means the option was not reset if the cpu was changed via target attributes or targt pragmas. I added this mask to POWERPC_MASKS since the option is set via -mcpu=future. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Turn on -mblock-ops-vector-pair for -mcpu=future. (POWERPC_MASKS): Add -mblock-ops-vector-pair. --- gcc/config/rs6000/rs6000-cpus.def | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def index a47075f8249..ed934e0213e 100644 --- a/gcc/config/rs6000/rs6000-cpus.def +++ b/gcc/config/rs6000/rs6000-cpus.def @@ -90,6 +90,7 @@ /* Flags for a potential future processor that may or may not be delivered. */ #define ISA_FUTURE_MASKS_SERVER(ISA_3_1_MASKS_SERVER \ +| OPTION_MASK_BLOCK_OPS_VECTOR_PAIR\ | OPTION_MASK_FUTURE) /* Flags that need to be turned off if -mno-power9-vector. */ @@ -127,6 +128,7 @@ /* Mask of all options to set the default isa flags based on -mcpu=. */ #define POWERPC_MASKS (OPTION_MASK_ALTIVEC\ +| OPTION_MASK_BLOCK_OPS_VECTOR_PAIR\ | OPTION_MASK_CMPB \ | OPTION_MASK_CRYPTO \ | OPTION_MASK_DFP \ -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [6 of 7]: Set future machine type in assembler if -mcpu=future
This patch uses the .machine directive to tell the assembler to use any possible future instructions. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000.cc (rs6000_machine_from_flags): Output .machine future if -mcpu=future. --- gcc/config/rs6000/rs6000.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 5e5e677e153..6bd537836d1 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -5968,6 +5968,8 @@ rs6000_machine_from_flags (void) /* Disable the flags that should never influence the .machine selection. */ flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT | OPTION_MASK_ISEL); + if ((flags & (ISA_FUTURE_MASKS_SERVER & ~ISA_3_1_MASKS_SERVER)) != 0) +return "future"; if ((flags & (ISA_3_1_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0) return "power10"; if ((flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0) -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [5 of 7]: Make -mtune=future be the same as -mtune=power10.
This patch makes -mcpu=future act like -mcpu=power10 in terms of tuning. If future patches changes the tuning, then this patch woucl be changed to use the new tuning information. Until there is different tuning, this patch does not allow the user to explicitly use -mtune=future. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000.cc (rs6000_option_override_internal): Make -mtune=future become -mtune=power10. --- gcc/config/rs6000/rs6000.cc | 28 ++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 2064709aa97..5e5e677e153 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -3756,16 +3756,40 @@ rs6000_option_override_internal (bool global_init_p) rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; #endif + /* At the moment, we don't have explicit -mtune=future support. If the user + explicitly uses -mtune=future, give a warning. If not, use the power10 + tuning until future tuning is added. */ if (rs6000_tune_index >= 0) -tune_index = rs6000_tune_index; +{ + enum processor_type cur_proc + = processor_target_table[rs6000_tune_index].processor; + + if (cur_proc == PROCESSOR_FUTURE) + { + warning (0, "%qs is not currently supported", "-mtune=future"); + rs6000_tune_index = rs6000_cpu_name_lookup ("power10"); + } + tune_index = rs6000_tune_index; +} else if (cpu_index >= 0) -rs6000_tune_index = tune_index = cpu_index; +{ + enum processor_type cur_cpu + = processor_target_table[cpu_index].processor; + + rs6000_tune_index = tune_index + = (cur_cpu == PROCESSOR_FUTURE + ? rs6000_cpu_name_lookup ("power10") + : cpu_index); +} else { size_t i; enum processor_type tune_proc = (TARGET_POWERPC64 ? PROCESSOR_DEFAULT64 : PROCESSOR_DEFAULT); + if (tune_proc == PROCESSOR_FUTURE) + tune_proc = PROCESSOR_POWER10; + tune_index = -1; for (i = 0; i < ARRAY_SIZE (processor_target_table); i++) if (processor_target_table[i].processor == tune_proc) -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [4 of 7]: Pass -mfuture to assembler if -mcpu=future.
This patch passes -mfuture to the assembler if the user used -mcpu=future. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000.h (ASM_CPU_SPEC): If -mcpu=future, pass -mfuture to the assembler. --- gcc/config/rs6000/rs6000.h | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 2291fe8d3a3..43209f9a6e7 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -163,6 +163,7 @@ mcpu=e5500: -me5500; \ mcpu=e6500: -me6500; \ mcpu=titan: -mtitan; \ + mcpu=future: -mfuture; \ !mcpu*: %{mpower9-vector: -mpower9; \ mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \ mvsx: -mpower7; \ -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [3 of 7]: Define _ARCH_PWR_FUTURE if -mcpu=future.
This patch defines _ARCH_PWR_FUTURE if -mcpu=future was used. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Define _ARCH_PWR_FUTURE if -mcpu=future. --- gcc/config/rs6000/rs6000-c.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc index ce0b14a8d37..f2fb5bef678 100644 --- a/gcc/config/rs6000/rs6000-c.cc +++ b/gcc/config/rs6000/rs6000-c.cc @@ -447,6 +447,8 @@ rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags) rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR9"); if ((flags & OPTION_MASK_POWER10) != 0) rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR10"); + if ((flags & OPTION_MASK_FUTURE) != 0) +rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR_FUTURE"); if ((flags & OPTION_MASK_SOFT_FLOAT) != 0) rs6000_define_or_undefine_macro (define_p, "_SOFT_FLOAT"); if ((flags & OPTION_MASK_RECIP_PRECISION) != 0) -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [2 of 7]: Add debugging for -mcpu=future
This patch prints that -mcpu=future was selected if you use the debugging switch -mdebug=reg. 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000.cc (rs6000_opt_masks): Add entry to print out -mfuture in the isa flags. --- gcc/config/rs6000/rs6000.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 68a14c6f88a..2064709aa97 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -24505,6 +24505,7 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] = { "float128-hardware", OPTION_MASK_FLOAT128_HW,false, true }, { "fprnd", OPTION_MASK_FPRND, false, true }, { "power10", OPTION_MASK_POWER10,false, true }, + { "future", OPTION_MASK_FUTURE, false, true }, { "hard-dfp",OPTION_MASK_DFP,false, true }, { "htm", OPTION_MASK_HTM,false, true }, { "isel",OPTION_MASK_ISEL, false, true }, -- 2.43.0 -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
Patch [1 of 7]: Add initial -mcpu=future support.
This patch adds the basic support for -mcpu=future, which is a framework to add support for possible future PowerPCs. This patch is only sets the future bit in the ISA options. Can I check this into GCC 15 when it opens up, and immediately back port it to GCC 14 after GCC 14.0 is released? 2024-02-14 Michael Meissner gcc/ * config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): New option bits for -mcpu=future. (POWERPC_MASKS): Add -mfuture mask. (future cpu): Add -mcpu=future. * config/rs6000/rs6000-opts.h (PROCESSOR_FUTURE): New processor type. * config/rs6000/rs6000-tables.opt (rs6000_cpu_opt_value): Likewise. * config/rs6000/rs6000.md (cpu attribute): Likewise. * config/rs6000/rs6000.opt (-mfuture): New insert mask for -mcpu=future. * doc/invoke.texi (PowerPC options): Add -mcpu=future. --- gcc/config/rs6000/rs6000-cpus.def | 6 ++ gcc/config/rs6000/rs6000-opts.h | 1 + gcc/config/rs6000/rs6000-tables.opt | 3 +++ gcc/config/rs6000/rs6000.md | 2 +- gcc/config/rs6000/rs6000.opt| 4 gcc/doc/invoke.texi | 2 +- 6 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def index d28cc87eb2a..a47075f8249 100644 --- a/gcc/config/rs6000/rs6000-cpus.def +++ b/gcc/config/rs6000/rs6000-cpus.def @@ -88,6 +88,10 @@ | OPTION_MASK_POWER10 \ | OTHER_POWER10_MASKS) +/* Flags for a potential future processor that may or may not be delivered. */ +#define ISA_FUTURE_MASKS_SERVER(ISA_3_1_MASKS_SERVER \ +| OPTION_MASK_FUTURE) + /* Flags that need to be turned off if -mno-power9-vector. */ #define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW\ | OPTION_MASK_P9_MINMAX) @@ -135,6 +139,7 @@ | OPTION_MASK_LOAD_VECTOR_PAIR \ | OPTION_MASK_POWER10 \ | OPTION_MASK_P10_FUSION \ +| OPTION_MASK_FUTURE \ | OPTION_MASK_HTM \ | OPTION_MASK_ISEL \ | OPTION_MASK_MFCRF\ @@ -267,3 +272,4 @@ RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, OPTION_MASK_PPC_GFXOPT RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER | OPTION_MASK_HTM) RS6000_CPU ("rs64", PROCESSOR_RS64A, OPTION_MASK_PPC_GFXOPT | MASK_POWERPC64) +RS6000_CPU ("future", PROCESSOR_FUTURE, MASK_POWERPC64 | ISA_FUTURE_MASKS_SERVER) diff --git a/gcc/config/rs6000/rs6000-opts.h b/gcc/config/rs6000/rs6000-opts.h index 33fd0efc936..b2ae180c0f0 100644 --- a/gcc/config/rs6000/rs6000-opts.h +++ b/gcc/config/rs6000/rs6000-opts.h @@ -62,6 +62,7 @@ enum processor_type PROCESSOR_POWER8, PROCESSOR_POWER9, PROCESSOR_POWER10, + PROCESSOR_FUTURE, PROCESSOR_RS64A, PROCESSOR_MPCCORE, diff --git a/gcc/config/rs6000/rs6000-tables.opt b/gcc/config/rs6000/rs6000-tables.opt index 65f46709716..97fa98a2e65 100644 --- a/gcc/config/rs6000/rs6000-tables.opt +++ b/gcc/config/rs6000/rs6000-tables.opt @@ -197,3 +197,6 @@ Enum(rs6000_cpu_opt_value) String(powerpc64le) Value(55) EnumValue Enum(rs6000_cpu_opt_value) String(rs64) Value(56) +EnumValue +Enum(rs6000_cpu_opt_value) String(future) Value(57) + diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 4acb4031ae0..bd0ecdd9007 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -350,7 +350,7 @@ (define_attr "cpu" ppc750,ppc7400,ppc7450, ppc403,ppc405,ppc440,ppc476, ppc8540,ppc8548,ppce300c2,ppce300c3,ppce500mc,ppce500mc64,ppce5500,ppce6500, - power4,power5,power6,power7,power8,power9,power10, + power4,power5,power6,power7,power8,power9,power10,future, rs64a,mpccore,cell,ppca2,titan" (const (symbol_ref "(enum attr_cpu) rs6000_tune"))) diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt index 60b923f5e4b..4ad1d8e302c 100644 --- a/gcc/config/rs6000/rs6000.opt +++ b/gcc/config/rs6000/rs6000.opt @@ -612,6 +612,10 @@ mrop-protect Target Var(rs6000_rop_protect) Init(0) Enable instructions that guard against return-oriented programming attacks. +mfuture +Target Undocumented Mask(FUTURE) Var(rs6000_isa_flags) Warn(Do not use %<-mfuture>) +Generate (do not generate) store vector pair instructions. + mprivileged Target Var(rs6000_privileged) Init(0) Generate code that will run in privileged state. diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 71339b8b30f..dcc9f82171c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -31091,7 +31091,7 @@
Patch [0 of 7]: PowerPC: Add -mcpu=future
This series of patches takes the first 2 patches from the dense math patches to add the support for -mcpu=future. If these patches are approved, I will re-base the rest of the dense math patches off of these patches. While patch #1 of the series was fairly self contained, in that it added the basic support for -mcpu=future, I have split that patch into 6 patches. I have included the 2nd patch (enable using load vector pair and store vector pair for memcpy) as patch #7 in this series. Note, you need patches 1-5 at least to be installed to enable using -mcpu=future. Patch #1 adds the -mcpu=future option. I added a new ISA bit to make things like target attribute or target pragmas correctly enable or disable -mcpu=future support. Because you have to have a switch to create the ISA bits, I added -mfuture, but I added a warning if the user used this option directly. Patch #2 adds support to print out that future is used with -mdebug=reg. Patch #3 defines _ARCH_PWR_FUTURE if -mcpu=future is used. Patch #4 passes -mfuture to the assembler if -mcpu=future is used. Patch #5 turns the tuning for -mcpu=future into -mtune=power10. It is likely that we will need to flesh out the future tuning support if/when the future machine becomes a real PowerPC. Patch #6 emits the .machine future option to the assembler if -mcpu=future is used (or target attribute/pragma are used). Patch #7 enables generating load vector pair or store vector pair instruction for memcpy and similar functions if -mcpu=future was used. Late in the power10 development, we decided to not generate load vector pair and store vector pair instructures due to some interactions that those instructions caused in some cases. I have tested these patches on both little endian and big endian systems, and there were no regressions. Even though these patches have been posted for 1.5 years now, I assume they have to wait for GCC 15. But I will immediately want to back port these to GCC 14.1 after they go into GCC 15. -- Michael Meissner, IBM PO Box 98, Ayer, Massachusetts, USA, 01432 email: meiss...@linux.ibm.com
[PATCH] RISC-V: Add tests for constraints "i" and "s"
The constraints "i" and "s" can be used with a symbol that binds externally, e.g. ``` namespace ns { extern int var, a[4]; } void foo() { asm(".pushsection .xxx,\"aw\"; .dc.a %0; .popsection" :: "s"(::var)); asm(".reloc ., BFD_RELOC_NONE, %0" :: "s"(::a[3])); } ``` gcc/testsuite/ChangeLog: * gcc.target/riscv/asm-raw-symbol.c: New test. --- gcc/doc/md.texi | 2 +- gcc/testsuite/gcc.target/riscv/asm-raw-symbol.c | 14 ++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/riscv/asm-raw-symbol.c diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi index b0c61925120..c75e5bf259d 100644 --- a/gcc/doc/md.texi +++ b/gcc/doc/md.texi @@ -1947,7 +1947,7 @@ Integer constant that is valid as an immediate operand in a 64-bit @code{MOV} pseudo instruction @item S -An absolute symbolic address or a label reference +A symbolic reference or label reference. @item Y Floating point constant zero diff --git a/gcc/testsuite/gcc.target/riscv/asm-raw-symbol.c b/gcc/testsuite/gcc.target/riscv/asm-raw-symbol.c new file mode 100644 index 000..28305a8b1f0 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/asm-raw-symbol.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-fpic" } */ + +extern int var, arr[2][2]; + +void +test (void) +{ + __asm__ ("@ %0" : : "i"()); + __asm__ ("@ %0 %1 %2" : : "s"(), "s"([1][1]), "s"(test)); +} + +/* { dg-final { scan-assembler "@ var arr\\+12 test" } } */ +/* { dg-final { scan-assembler "@ var" } } */ -- 2.43.0.687.g38aa6559b0-goog
Re: [PATCH] RISC-V: Fix macro fusion for auipc+add, when identifying UNSPEC_AUIPC. [PR113742]
Hi Jeff, I don't have permission to commit, can you push it for me? If you look good to you. > Jeff Law 於 2024年2月14日 凌晨12:03 寫道: > > > >> On 2/4/24 20:20, Monk Chiang wrote: >> gcc/ChangeLog: >> PR target/113742 >> * config/riscv/riscv.cc (riscv_macro_fusion_pair_p): Fix >> recognizes UNSPEC_AUIPC for RISCV_FUSE_LUI_ADDI. >> gcc/testsuite/ChangeLog: >> * gcc.target/riscv/pr113742.c: New test. > I was going through the patchwork queue after the call today and it looks > like this didn't get pushed. So I took care of it. > > Thanks again, > Jeff
Re: [PATCH] RISC-V: Fix macro fusion for auipc+add, when identifying UNSPEC_AUIPC. [PR113742]
Hi Jeff, I don't have permission to commit, can you push it for me? If you look good to you. > Jeff Law 於 2024年2月14日 凌晨12:03 寫道: > > > >> On 2/4/24 20:20, Monk Chiang wrote: >> gcc/ChangeLog: >>PR target/113742 >>* config/riscv/riscv.cc (riscv_macro_fusion_pair_p): Fix >>recognizes UNSPEC_AUIPC for RISCV_FUSE_LUI_ADDI. >> gcc/testsuite/ChangeLog: >>* gcc.target/riscv/pr113742.c: New test. > I was going through the patchwork queue after the call today and it looks > like this didn't get pushed. So I took care of it. > > Thanks again, > Jeff
Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
On 2/13/24 20:34, Nathaniel Shead wrote: On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote: On 2/11/24 08:26, Nathaniel Shead wrote: Currently inline vars imported from modules aren't correctly finalised, which means that import_export_decl gets called at the end of TU processing despite not being meant to for these kinds of declarations. I disagree that it's not meant to; inline variables are vague linkage just like template instantiations, so the bug seems to be that import_export_decl doesn't accept them. And on the other side, that make_rtl_for_nonlocal_decl doesn't defer them like instantations. Jason True, that's a good point. I think I confused myself here. Here's a fixed patch that looks a lot cleaner. Bootstrapped and regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? OK. -- >8 -- Inline variables are vague-linkage, and may or may not need to be emitted in any TU that they are part of, similarly to e.g. template instantiations. Currently 'import_export_decl' assumes that inline variables have already been emitted when it comes to end-of-TU processing, and so crashes when importing non-trivially-initialised variables from a module, as they have not yet been finalised. This patch fixes this by ensuring that inline variables are always deferred till end-of-TU processing, unifying the behaviour for module and non-module code. PR c++/113708 gcc/cp/ChangeLog: * decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables. * decl2.cc (import_export_decl): Support inline variables. gcc/testsuite/ChangeLog: * g++.dg/modules/init-7_a.H: New test. * g++.dg/modules/init-7_b.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 4 gcc/cp/decl2.cc | 7 +-- gcc/testsuite/g++.dg/modules/init-7_a.H | 6 ++ gcc/testsuite/g++.dg/modules/init-7_b.C | 6 ++ 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..969513c069a 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec) && DECL_IMPLICIT_INSTANTIATION (decl)) defer_p = 1; + /* Defer vague-linkage variables. */ + if (DECL_INLINE_VAR_P (decl)) +defer_p = 1; + /* If we're not deferring, go ahead and assemble the variable. */ if (!defer_p) rest_of_decl_compilation (decl, toplev, at_eof); diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index f569d4045ec..1dddbaab38b 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -3360,7 +3360,9 @@ import_export_decl (tree decl) * implicit instantiations of function templates - * inline function + * inline functions + + * inline variables * implicit instantiations of static data members of class templates @@ -3383,6 +3385,7 @@ import_export_decl (tree decl) || DECL_DECLARED_INLINE_P (decl)); else gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl) + || DECL_INLINE_VAR_P (decl) || DECL_VTABLE_OR_VTT_P (decl) || DECL_TINFO_P (decl)); /* Check that a definition of DECL is available in this translation @@ -3511,7 +3514,7 @@ import_export_decl (tree decl) this entity as undefined in this translation unit. */ import_p = true; } - else if (DECL_FUNCTION_MEMBER_P (decl)) + else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl)) { if (!DECL_DECLARED_INLINE_P (decl)) { diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H new file mode 100644 index 000..7a0bb721c30 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/init-7_a.H @@ -0,0 +1,6 @@ +// PR c++/113708 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +inline int f() { return 42; } +inline int a = f(); diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C new file mode 100644 index 000..58bb0620ca5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/init-7_b.C @@ -0,0 +1,6 @@ +// PR c++/113708 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts" } + +import "init-7_a.H"; +int main() { a; }
Re: [PATCH] c++: Fix error recovery when redeclaring enum in different module [PR99573]
On 2/13/24 18:20, Nathaniel Shead wrote: On Tue, Feb 13, 2024 at 06:12:51PM -0500, Jason Merrill wrote: On 2/11/24 21:26, Nathaniel Shead wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This ensures that with modules enabled, redeclaring an enum in the wrong module or with the wrong underlying type no longer ICEs. The patch also rearranges the order of the checks a little because I think it's probably more important to note that you can't redeclare the enum all before complaining about mismatched underlying types etc. As a drive by this patch also adds some missing diagnostic groups, and rewords the module redeclaration error message to more closely match the wording used in other places this check is done. PR c++/99573 gcc/cp/ChangeLog: * decl.cc (start_enum): Reorder check for redeclaring in module. Add missing auto_diagnostic_groups. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-12.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 31 +++--- gcc/testsuite/g++.dg/modules/enum-12.C | 10 + 2 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-12.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..f982b5f88de 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -16951,12 +16951,28 @@ start_enum (tree name, tree enumtype, tree underlying_type, /*tag_scope=*/TAG_how::CURRENT_ONLY, /*template_header_p=*/false); + /* Attempt to set the declaring module. */ + if (modules_p () && enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) +{ + tree decl = TYPE_NAME (enumtype); + if (!module_may_redeclare (decl)) + { + auto_diagnostic_group d; + error ("cannot declare %qD in different module", decl); + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); + enumtype = error_mark_node; + } + else + set_instantiating_module (decl); +} + /* In case of a template_decl, the only check that should be deferred to instantiation time is the comparison of underlying types. */ if (enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) { How about moving the module checks here, instead of a few lines higher, where you need to duplicate the ENUMERAL_TYPE condition? So like this? OK. -- >8 -- This ensures that with modules enabled, redeclaring an enum in the wrong module or with the wrong underlying type no longer ICEs. The patch also rearranges the order of the checks a little because I think it's probably more important to note that you can't redeclare the enum all before complaining about mismatched underlying types etc. As a drive by this patch also adds some missing diagnostic groups, and rewords the module redeclaration error message to more closely match the wording used in other places this check is done. PR c++/99573 gcc/cp/ChangeLog: * decl.cc (start_enum): Reorder check for redeclaring in module. Add missing auto_diagnostic_groups. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-12.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 35 +++--- gcc/testsuite/g++.dg/modules/enum-12.C | 10 2 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-12.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..d19d09adde4 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -16955,8 +16955,26 @@ start_enum (tree name, tree enumtype, tree underlying_type, to instantiation time is the comparison of underlying types. */ if (enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) { - if (scoped_enum_p != SCOPED_ENUM_P (enumtype)) + /* Attempt to set the declaring module. */ + if (modules_p ()) { + tree decl = TYPE_NAME (enumtype); + if (!module_may_redeclare (decl)) + { + auto_diagnostic_group d; + error ("cannot declare %qD in different module", decl); + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); + enumtype = error_mark_node; + } + else + set_instantiating_module (decl); + } + + if (enumtype == error_mark_node) + ; + else if (scoped_enum_p != SCOPED_ENUM_P (enumtype)) + { + auto_diagnostic_group d; error_at (input_location, "scoped/unscoped mismatch " "in enum %q#T", enumtype); inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)), @@ -16965,6 +16983,7 @@ start_enum (tree name, tree enumtype, tree underlying_type, } else if (ENUM_FIXED_UNDERLYING_TYPE_P (enumtype) != !! underlying_type) { +
Re: [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules
On 2/10/24 22:54, Nathaniel Shead wrote: Bootstrapped and regtested (so far just modules.exp and dg.exp) on x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? (Also I noticed I forgot to add the PR to the changelog in my last patch, I've fixed that locally.) -- >8 -- After fixing PR111710, I noticed that we currently ICE when declaring a type that derives from 'decltype([]{})'. As far as I can tell this should be legal code, since by [basic.link] p15.2 a lambda defined in a class-specifier should not be TU-local. This patch also adds a bunch of tests for unevaluated lambdas in other contexts, which generally seem to work now. One interesting case is 'E::f' in the attached testcase: it appears to get a merge kind of 'MK_field', rather than 'MK_keyed' as most other lambdas do. You mean the lambda in the type of E::f? That happens because when we see a class in the body of another class, we treat it as a member even if it's a closure. This ought to work fine. It also works with the suggested direction for https://github.com/itanium-cxx-abi/cxx-abi/issues/165 So, the patch is OK with the missing ChangeLog bits filled in. Do we also need to key lambdas to concepts? template concept L = requires { []{ T(); }; }; I'm not entirely sure if this will cause issues in the future, but I haven't been able to construct a testcase that causes problems with this, and conversely wrapping the class body in 'start_lambda_scope' causes issues with symbol duplication in COMDAT groups, so I've left it as-is for now. gcc/cp/ChangeLog: * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. (maybe_key_decl): Likewise. * parser.cc (cp_parser_class_head): Start a lambda scope when parsing base classes. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7_a.C: * g++.dg/modules/lambda-7_b.C: Signed-off-by: Nathaniel Shead --- gcc/cp/module.cc | 8 +--- gcc/cp/parser.cc | 10 -- gcc/testsuite/g++.dg/modules/lambda-7_a.C | 19 +++ gcc/testsuite/g++.dg/modules/lambda-7_b.C | 23 +++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 9742bca922c..cceec79b26b 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -10784,7 +10784,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, (TREE_TYPE (inner))); gcc_checking_assert (TREE_CODE (scope) == VAR_DECL || TREE_CODE (scope) == FIELD_DECL -|| TREE_CODE (scope) == PARM_DECL); +|| TREE_CODE (scope) == PARM_DECL +|| TREE_CODE (scope) == TYPE_DECL); auto *root = keyed_table->get (scope); unsigned ix = root->length (); /* If we don't find it, we'll write a really big number @@ -18980,10 +18981,11 @@ maybe_key_decl (tree ctx, tree decl) return; /* We only need to deal with lambdas attached to var, field, - or parm decls. */ + parm, or type decls. */ if (TREE_CODE (ctx) != VAR_DECL && TREE_CODE (ctx) != FIELD_DECL - && TREE_CODE (ctx) != PARM_DECL) + && TREE_CODE (ctx) != PARM_DECL + && TREE_CODE (ctx) != TYPE_DECL) return; if (!keyed_table) diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 09ecfa23b5d..151e724ed66 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27663,10 +27663,16 @@ cp_parser_class_head (cp_parser* parser, if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) { if (type) - pushclass (type); + { + pushclass (type); + start_lambda_scope (TYPE_NAME (type)); + } bases = cp_parser_base_clause (parser); if (type) - popclass (); + { + finish_lambda_scope (); + popclass (); + } } else bases = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C index 289285cd926..9a23827a280 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-7_a.C +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C @@ -18,3 +18,22 @@ export struct S { export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { return f(x); } + +// unevaluated lambdas +#if __cplusplus >= 202002L +export struct E : decltype([](int x) { return x * 6; }) { + decltype([](int x) { return x * 7; }) f; +}; + +export template +struct G : decltype([](int x) { return x * 8; }) { + decltype([](int x) { return x * 9; }) h; +}; + +template <> +struct G : decltype([](int x) { return x * 10; }) { + decltype([](int x) { return x * 11; }) i; +}; + +export decltype([](int x) { return x * 12; }) j; +#endif diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C
[PATCH v2] c++: Defer emitting inline variables [PR113708]
On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote: > On 2/11/24 08:26, Nathaniel Shead wrote: > > > > Currently inline vars imported from modules aren't correctly finalised, > > which means that import_export_decl gets called at the end of TU > > processing despite not being meant to for these kinds of declarations. > > I disagree that it's not meant to; inline variables are vague linkage just > like template instantiations, so the bug seems to be that import_export_decl > doesn't accept them. And on the other side, that make_rtl_for_nonlocal_decl > doesn't defer them like instantations. > > Jason > True, that's a good point. I think I confused myself here. Here's a fixed patch that looks a lot cleaner. Bootstrapped and regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? -- >8 -- Inline variables are vague-linkage, and may or may not need to be emitted in any TU that they are part of, similarly to e.g. template instantiations. Currently 'import_export_decl' assumes that inline variables have already been emitted when it comes to end-of-TU processing, and so crashes when importing non-trivially-initialised variables from a module, as they have not yet been finalised. This patch fixes this by ensuring that inline variables are always deferred till end-of-TU processing, unifying the behaviour for module and non-module code. PR c++/113708 gcc/cp/ChangeLog: * decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables. * decl2.cc (import_export_decl): Support inline variables. gcc/testsuite/ChangeLog: * g++.dg/modules/init-7_a.H: New test. * g++.dg/modules/init-7_b.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 4 gcc/cp/decl2.cc | 7 +-- gcc/testsuite/g++.dg/modules/init-7_a.H | 6 ++ gcc/testsuite/g++.dg/modules/init-7_b.C | 6 ++ 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..969513c069a 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec) && DECL_IMPLICIT_INSTANTIATION (decl)) defer_p = 1; + /* Defer vague-linkage variables. */ + if (DECL_INLINE_VAR_P (decl)) +defer_p = 1; + /* If we're not deferring, go ahead and assemble the variable. */ if (!defer_p) rest_of_decl_compilation (decl, toplev, at_eof); diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc index f569d4045ec..1dddbaab38b 100644 --- a/gcc/cp/decl2.cc +++ b/gcc/cp/decl2.cc @@ -3360,7 +3360,9 @@ import_export_decl (tree decl) * implicit instantiations of function templates - * inline function + * inline functions + + * inline variables * implicit instantiations of static data members of class templates @@ -3383,6 +3385,7 @@ import_export_decl (tree decl) || DECL_DECLARED_INLINE_P (decl)); else gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl) + || DECL_INLINE_VAR_P (decl) || DECL_VTABLE_OR_VTT_P (decl) || DECL_TINFO_P (decl)); /* Check that a definition of DECL is available in this translation @@ -3511,7 +3514,7 @@ import_export_decl (tree decl) this entity as undefined in this translation unit. */ import_p = true; } - else if (DECL_FUNCTION_MEMBER_P (decl)) + else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl)) { if (!DECL_DECLARED_INLINE_P (decl)) { diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H new file mode 100644 index 000..7a0bb721c30 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/init-7_a.H @@ -0,0 +1,6 @@ +// PR c++/113708 +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } + +inline int f() { return 42; } +inline int a = f(); diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C new file mode 100644 index 000..58bb0620ca5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/init-7_b.C @@ -0,0 +1,6 @@ +// PR c++/113708 +// { dg-module-do link } +// { dg-additional-options "-fmodules-ts" } + +import "init-7_a.H"; +int main() { a; } -- 2.43.0
[pushed] libstdc++: C++ item p2442 is version 1 only
Following a mail exchange with Jonathan back in December; finally implementing what we discussed/he confirmed. Gerald libstdc++-v3: * doc/xml/manual/status_cxx2023.xml: Fix C++ item p2442 to be version 1. * doc/html/manual/status.html: Regenerate. --- libstdc++-v3/doc/html/manual/status.html | 4 ++-- libstdc++-v3/doc/xml/manual/status_cxx2023.xml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libstdc++-v3/doc/html/manual/status.html b/libstdc++-v3/doc/html/manual/status.html index 97f803063d0..dafd48035a6 100644 --- a/libstdc++-v3/doc/html/manual/status.html +++ b/libstdc++-v3/doc/html/manual/status.html @@ -1803,8 +1803,8 @@ or any notes about the implementation. 13.1 __cpp_lib_ranges_join_with = 202202L Windowing range adaptors: views::chunk and views::slide -https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2442r2.html; target="_top"> -P2442R2 +https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2442r1.html; target="_top"> +P2442R1 13.1 __cpp_lib_ranges_slide = 202202L views::chunk_by https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2443r1.html; target="_top"> diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2023.xml b/libstdc++-v3/doc/xml/manual/status_cxx2023.xml index 9d121af82df..4bf22f00bce 100644 --- a/libstdc++-v3/doc/xml/manual/status_cxx2023.xml +++ b/libstdc++-v3/doc/xml/manual/status_cxx2023.xml @@ -235,8 +235,8 @@ or any notes about the implementation. Windowing range adaptors: views::chunk and views::slide -http://www.w3.org/1999/xlink; xlink:href="https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2442r2.html;> -P2442R2 +http://www.w3.org/1999/xlink; xlink:href="https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2442r1.html;> +P2442R1 13.1 -- 2.43.0
[pushed] wwwdocs: index: Update link to FOSDEM announcement
Pushed. Gerald --- htdocs/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/index.html b/htdocs/index.html index ed505637..032cc4b6 100644 --- a/htdocs/index.html +++ b/htdocs/index.html @@ -55,7 +55,7 @@ mission statement. News -https://inbox.sourceware.org/36fadb0549c3dca716eb3b923d66a11be2c67a61.ca...@redhat.com;>GCC developer room at FOSDEM 2024: Call for Participation open +https://inbox.sourceware.org/gcc/36fadb0549c3dca716eb3b923d66a11be2c67a61.ca...@redhat.com;>GCC developer room at FOSDEM 2024: Call for Participation open [2023-11-20] wwwdocs: FOSDEM 2024: Brussels, Belgium, February 3-4 2024 -- 2.43.0
[pushed] install: Update gettext link
This follows a web server redirect. Gerald gcc: * doc/install.texi (Prerequisites): Update gettext link. --- gcc/doc/install.texi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index c7794439107..173233096d1 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -477,8 +477,8 @@ is shown below: Necessary to build GCC with internationalization support via @option{--enable-nls}. It can be downloaded from -@uref{https://gnu.org/s/gettext/}. If a GNU gettext distribution is -found in a subdirectory of your GCC sources named @file{gettext}, it +@uref{https://www.gnu.org/software/gettext/}. If a GNU gettext distribution +is found in a subdirectory of your GCC sources named @file{gettext}, it will be built together with GCC, unless present in the system (either in libc or as a stand-alone library). -- 2.43.0
Re: [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710]
On 2/10/24 17:57, Nathaniel Shead wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? Or should I re-introduce the tree checking and just add checks for the new kinds of declarations that could be have keyed decls? This way is fine. -- >8 -- The fix for PR107398 weakened the restrictions that lambdas must belong to namespace scope. However this was not sufficient: we also need to allow lambdas keyed to FIELD_DECLs or PARM_DECLs. I wonder about keying such lambdas to the class and function, respectively, rather than specifically to the field or parameter, but I suppose it doesn't matter. OK with the comment adjustment below. Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a fairly large number of different kinds of DECLs, and that in general it's safe to just get 'false' as a result of a check on an unexpected DECL type, this patch also removes the tree checking from the accessor. gcc/cp/ChangeLog: * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. (struct lang_decl_base): Update comments and fix whitespace. * module.cc (trees_out::lang_decl_bools): Always write module_keyed_decls_p flag... (trees_in::lang_decl_bools): ...and always read it. (trees_out::decl_value): Also handle keyed decls on decls other than VAR_DECL or FUNCTION_DECL. (trees_in::decl_value): Likewise. (trees_out::get_merge_kind): Likewise. (maybe_key_decl): Also handle lambdas attached to FIELD_DECLs and PARM_DECLS. (trees_out::key_mergeable): Likewise. (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL container. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7_a.C: New test. * g++.dg/modules/lambda-7_b.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/cp-tree.h | 23 gcc/cp/module.cc | 70 +++ gcc/testsuite/g++.dg/modules/lambda-7_a.C | 20 +++ gcc/testsuite/g++.dg/modules/lambda-7_b.C | 16 ++ 4 files changed, 81 insertions(+), 48 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.C create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 334c11396c2..6ab82dc2d0f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) /* DECL that has attached decls for ODR-relatedness. */ -#define DECL_MODULE_KEYED_DECLS_P(NODE)\ - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ - ->u.base.module_keyed_decls_p) +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) /* Whether this is an exported DECL. Held on any decl that can appear at namespace scope (function, var, type, template, const or @@ -2887,21 +2886,19 @@ struct GTY(()) lang_decl_base { unsigned friend_or_tls : 1;/* var, fn, type or template */ unsigned unknown_bound_p : 1; /* var */ unsigned odr_used : 1; /* var or fn */ - unsigned concept_p : 1; /* applies to vars and functions */ + unsigned concept_p : 1; /* applies to vars and functions */ unsigned var_declared_inline_p : 1;/* var */ unsigned dependent_init_p : 1; /* var */ /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE decls. */ With your reformatting this comment now seems to apply to module_keyed_decls_p, which I don't think you mean. Maybe just adjust this comment to say "the following 4"? - unsigned module_purview_p : 1; // in named-module purview - unsigned module_attach_p : 1; // attached to named module - unsigned module_import_p : 1; /* from an import */ - unsigned module_entity_p : 1; /* is in the entitity ary & - hash. */ - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ - unsigned module_keyed_decls_p : 1; - - /* 12 spare bits. */ + unsigned module_purview_p : 1; /* in named-module purview */ + unsigned module_attach_p : 1; /* attached to named module */ + unsigned module_import_p : 1; /* from an import */ + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ + + /* 11 spare bits. */ }; /* True for DECL codes which have template info and access. */ diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 560d8f3b614..9742bca922c 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t)
[pushed] wwwdocs: gcc-14: Fix typo in AVR section
Note that is not part of current HTML standards; can we simply remove it? Gerald --- htdocs/gcc-14/changes.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 6ac7c8b1..92bd0a7b 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -370,7 +370,7 @@ a work-in-progress. precedence over __flmap. For example, linking with -Wl,--defsym,__RODATA_FLASH_START__=32k - choses the second 32KiB block. + chooses the second 32KiB block. The default uses the last 32KiB block, which is also the hardware default for bit-field NVMCTRL_CTRLB.FLMAP. When a non-default block is used, -- 2.43.0
[pushed] wwwdocs: gcc-14: Fix markup in avr section.
In addition, I believe it might be good to rephrase that sentence. Do you mean "the linker will not pull in that code from ... any more"? Gerald --- htdocs/gcc-14/changes.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index cd5f5157..6ac7c8b1 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -390,7 +390,7 @@ a work-in-progress. can be used: __asm (".global __flmap_lock" "\n\t" "__flmap_lock = 1"); - When you do not want the code from#931, then define global + When you do not want the code from#931, then define global symbol __do_flmap_init and the linker will no more drag that code from libmcu.a. In order to return to the old placement of read-only data in RAM, -- 2.43.0
[PATCH] aarch64: Reword error message for mismatch guard size and probing interval [PR90155]
The error message is not clear what options are being taked about when it says the values need to match; plus there is a wrong quotation dealing with the diagnostic. So this changes the error message to be exactly talking about the param options that are being taked about and now with the options, it needs the quoting. OK? Built and tested for aarch64-linux-gnu. gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_override_options_internal): Fix error message for mismatch guard size and probing interval. Signed-off-by: Andrew Pinski --- gcc/config/aarch64/aarch64.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 32eae49d4e9..2da743469ae 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -18334,7 +18334,7 @@ aarch64_override_options_internal (struct gcc_options *opts) "size. Given value %d (%llu KB) is out of range", guard_size, (1ULL << guard_size) / 1024ULL); - /* Enforce that interval is the same size as size so the mid-end does the + /* Enforce that interval is the same size as size so the middle-end does the right thing. */ SET_OPTION_IF_UNSET (opts, _options_set, param_stack_clash_protection_probe_interval, @@ -18346,8 +18346,8 @@ aarch64_override_options_internal (struct gcc_options *opts) int probe_interval = param_stack_clash_protection_probe_interval; if (guard_size != probe_interval) -error ("stack clash guard size %<%d%> must be equal to probing interval " - "%<%d%>", guard_size, probe_interval); +error ("%<--param stack-clash-protection-probe-interval=%d%> needs to match " + "%<--param stack-clash-protection-guard-size=%d%>", probe_interval, guard_size); /* Enable sw prefetching at specified optimization level for CPUS that have prefetch. Lower optimization level threshold by 1 -- 2.43.0
Re: [PATCH] c++: Fix error recovery when redeclaring enum in different module [PR99573]
On Tue, Feb 13, 2024 at 06:12:51PM -0500, Jason Merrill wrote: > On 2/11/24 21:26, Nathaniel Shead wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > This ensures that with modules enabled, redeclaring an enum in the wrong > > module or with the wrong underlying type no longer ICEs. > > > > The patch also rearranges the order of the checks a little because I > > think it's probably more important to note that you can't redeclare the > > enum all before complaining about mismatched underlying types etc. > > > > As a drive by this patch also adds some missing diagnostic groups, and > > rewords the module redeclaration error message to more closely match the > > wording used in other places this check is done. > > > > PR c++/99573 > > > > gcc/cp/ChangeLog: > > > > * decl.cc (start_enum): Reorder check for redeclaring in module. > > Add missing auto_diagnostic_groups. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/enum-12.C: New test. > > > > Signed-off-by: Nathaniel Shead > > --- > > gcc/cp/decl.cc | 31 +++--- > > gcc/testsuite/g++.dg/modules/enum-12.C | 10 + > > 2 files changed, 28 insertions(+), 13 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/enum-12.C > > > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > > index 3e41fd4fa31..f982b5f88de 100644 > > --- a/gcc/cp/decl.cc > > +++ b/gcc/cp/decl.cc > > @@ -16951,12 +16951,28 @@ start_enum (tree name, tree enumtype, tree > > underlying_type, > > /*tag_scope=*/TAG_how::CURRENT_ONLY, > > /*template_header_p=*/false); > > + /* Attempt to set the declaring module. */ > > + if (modules_p () && enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) > > +{ > > + tree decl = TYPE_NAME (enumtype); > > + if (!module_may_redeclare (decl)) > > + { > > + auto_diagnostic_group d; > > + error ("cannot declare %qD in different module", decl); > > + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); > > + enumtype = error_mark_node; > > + } > > + else > > + set_instantiating_module (decl); > > +} > > + > > /* In case of a template_decl, the only check that should be deferred > >to instantiation time is the comparison of underlying types. */ > > if (enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) > > { > > How about moving the module checks here, instead of a few lines higher, > where you need to duplicate the ENUMERAL_TYPE condition? So like this? -- >8 -- This ensures that with modules enabled, redeclaring an enum in the wrong module or with the wrong underlying type no longer ICEs. The patch also rearranges the order of the checks a little because I think it's probably more important to note that you can't redeclare the enum all before complaining about mismatched underlying types etc. As a drive by this patch also adds some missing diagnostic groups, and rewords the module redeclaration error message to more closely match the wording used in other places this check is done. PR c++/99573 gcc/cp/ChangeLog: * decl.cc (start_enum): Reorder check for redeclaring in module. Add missing auto_diagnostic_groups. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-12.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 35 +++--- gcc/testsuite/g++.dg/modules/enum-12.C | 10 2 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-12.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..d19d09adde4 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -16955,8 +16955,26 @@ start_enum (tree name, tree enumtype, tree underlying_type, to instantiation time is the comparison of underlying types. */ if (enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) { - if (scoped_enum_p != SCOPED_ENUM_P (enumtype)) + /* Attempt to set the declaring module. */ + if (modules_p ()) { + tree decl = TYPE_NAME (enumtype); + if (!module_may_redeclare (decl)) + { + auto_diagnostic_group d; + error ("cannot declare %qD in different module", decl); + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); + enumtype = error_mark_node; + } + else + set_instantiating_module (decl); + } + + if (enumtype == error_mark_node) + ; + else if (scoped_enum_p != SCOPED_ENUM_P (enumtype)) + { + auto_diagnostic_group d; error_at (input_location, "scoped/unscoped mismatch " "in enum %q#T", enumtype); inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)), @@ -16965,6 +16983,7 @@ start_enum (tree name, tree enumtype, tree
Re: [PATCH] c++: Fix error recovery when redeclaring enum in different module [PR99573]
On 2/11/24 21:26, Nathaniel Shead wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This ensures that with modules enabled, redeclaring an enum in the wrong module or with the wrong underlying type no longer ICEs. The patch also rearranges the order of the checks a little because I think it's probably more important to note that you can't redeclare the enum all before complaining about mismatched underlying types etc. As a drive by this patch also adds some missing diagnostic groups, and rewords the module redeclaration error message to more closely match the wording used in other places this check is done. PR c++/99573 gcc/cp/ChangeLog: * decl.cc (start_enum): Reorder check for redeclaring in module. Add missing auto_diagnostic_groups. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-12.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/decl.cc | 31 +++--- gcc/testsuite/g++.dg/modules/enum-12.C | 10 + 2 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-12.C diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3e41fd4fa31..f982b5f88de 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -16951,12 +16951,28 @@ start_enum (tree name, tree enumtype, tree underlying_type, /*tag_scope=*/TAG_how::CURRENT_ONLY, /*template_header_p=*/false); + /* Attempt to set the declaring module. */ + if (modules_p () && enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) +{ + tree decl = TYPE_NAME (enumtype); + if (!module_may_redeclare (decl)) + { + auto_diagnostic_group d; + error ("cannot declare %qD in different module", decl); + inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); + enumtype = error_mark_node; + } + else + set_instantiating_module (decl); +} + /* In case of a template_decl, the only check that should be deferred to instantiation time is the comparison of underlying types. */ if (enumtype && TREE_CODE (enumtype) == ENUMERAL_TYPE) { How about moving the module checks here, instead of a few lines higher, where you need to duplicate the ENUMERAL_TYPE condition? if (scoped_enum_p != SCOPED_ENUM_P (enumtype)) { + auto_diagnostic_group d; error_at (input_location, "scoped/unscoped mismatch " "in enum %q#T", enumtype); inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)), @@ -16965,6 +16981,7 @@ start_enum (tree name, tree enumtype, tree underlying_type, } else if (ENUM_FIXED_UNDERLYING_TYPE_P (enumtype) != !! underlying_type) { + auto_diagnostic_group d; error_at (input_location, "underlying type mismatch " "in enum %q#T", enumtype); inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)), @@ -16975,25 +16992,13 @@ start_enum (tree name, tree enumtype, tree underlying_type, && !same_type_p (underlying_type, ENUM_UNDERLYING_TYPE (enumtype))) { + auto_diagnostic_group d; error_at (input_location, "different underlying type " "in enum %q#T", enumtype); inform (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)), "previous definition here"); underlying_type = NULL_TREE; } - - if (modules_p ()) - { - if (!module_may_redeclare (TYPE_NAME (enumtype))) - { - error ("cannot define %qD in different module", -TYPE_NAME (enumtype)); - inform (DECL_SOURCE_LOCATION (TYPE_NAME (enumtype)), - "declared here"); - enumtype = error_mark_node; - } - set_instantiating_module (TYPE_NAME (enumtype)); - } } if (!enumtype || TREE_CODE (enumtype) != ENUMERAL_TYPE diff --git a/gcc/testsuite/g++.dg/modules/enum-12.C b/gcc/testsuite/g++.dg/modules/enum-12.C new file mode 100644 index 000..57eeb85d92a --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-12.C @@ -0,0 +1,10 @@ +// PR c++/99573 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi !foo } + +export module foo; +namespace std { + enum class align_val_t : decltype(sizeof(int)) {}; // { dg-error "different module" } +} + +// { dg-prune-output "not writing module" }
Re: [PATCH] c++/modules: Finalise non-local imported vars [PR113708]
On 2/11/24 08:26, Nathaniel Shead wrote: Currently inline vars imported from modules aren't correctly finalised, which means that import_export_decl gets called at the end of TU processing despite not being meant to for these kinds of declarations. I disagree that it's not meant to; inline variables are vague linkage just like template instantiations, so the bug seems to be that import_export_decl doesn't accept them. And on the other side, that make_rtl_for_nonlocal_decl doesn't defer them like instantations. Jason
Re: [PATCH] c++: adjust the extra ; warning [PR113760]
On 2/13/24 17:59, Marek Polacek wrote: Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. -- >8 -- A minimal fix to quash an extra ; warning. I have a more complete patch for GCC 15. DR 1693 PR c++/113760 gcc/cp/ChangeLog: * parser.cc (cp_parser_member_declaration): Only pedwarn about an extra semicolon in C++98. gcc/testsuite/ChangeLog: * g++.dg/semicolon-fixits.C: Run in C++98 only. * g++.dg/warn/pedantic2.C: Adjust dg-warning. * g++.old-deja/g++.jason/parse11.C: Adjust dg-error. * g++.dg/DRs/dr1693-1.C: New test. * g++.dg/DRs/dr1693-2.C: New test. --- gcc/cp/parser.cc | 2 +- gcc/testsuite/g++.dg/DRs/dr1693-1.C| 9 + gcc/testsuite/g++.dg/DRs/dr1693-2.C| 9 + gcc/testsuite/g++.dg/semicolon-fixits.C| 1 + gcc/testsuite/g++.dg/warn/pedantic2.C | 4 ++-- gcc/testsuite/g++.old-deja/g++.jason/parse11.C | 4 ++-- 6 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/DRs/dr1693-1.C create mode 100644 gcc/testsuite/g++.dg/DRs/dr1693-2.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 68ab74d70b9..9d0914435fb 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27999,7 +27999,7 @@ cp_parser_member_declaration (cp_parser* parser) if (!decl_specifiers.any_specifiers_p) { cp_token *token = cp_lexer_peek_token (parser->lexer); - if (!in_system_header_at (token->location)) + if (cxx_dialect < cxx11 && !in_system_header_at (token->location)) { gcc_rich_location richloc (token->location); richloc.add_fixit_remove (); diff --git a/gcc/testsuite/g++.dg/DRs/dr1693-1.C b/gcc/testsuite/g++.dg/DRs/dr1693-1.C new file mode 100644 index 000..ed27026109c --- /dev/null +++ b/gcc/testsuite/g++.dg/DRs/dr1693-1.C @@ -0,0 +1,9 @@ +// DR 1693, Superfluous semicolons in class definitions +// PR c++/113760 +// { dg-do compile } +// { dg-options "" } + +struct S { + int a; + ; +}; diff --git a/gcc/testsuite/g++.dg/DRs/dr1693-2.C b/gcc/testsuite/g++.dg/DRs/dr1693-2.C new file mode 100644 index 000..c52259d4602 --- /dev/null +++ b/gcc/testsuite/g++.dg/DRs/dr1693-2.C @@ -0,0 +1,9 @@ +// DR 1693, Superfluous semicolons in class definitions +// PR c++/113760 +// { dg-do compile } +// { dg-options "-pedantic-errors" } + +struct S { + int a; + ;// { dg-error "extra" "" { target c++98_only } } +}; diff --git a/gcc/testsuite/g++.dg/semicolon-fixits.C b/gcc/testsuite/g++.dg/semicolon-fixits.C index a9cc783b172..198e306c950 100644 --- a/gcc/testsuite/g++.dg/semicolon-fixits.C +++ b/gcc/testsuite/g++.dg/semicolon-fixits.C @@ -1,3 +1,4 @@ +// { dg-do compile { target c++98_only } } /* { dg-options "-fdiagnostics-show-caret -Wpedantic" } */ /* Struct with extra semicolon. */ diff --git a/gcc/testsuite/g++.dg/warn/pedantic2.C b/gcc/testsuite/g++.dg/warn/pedantic2.C index 6c834162c1b..37d77daaef3 100644 --- a/gcc/testsuite/g++.dg/warn/pedantic2.C +++ b/gcc/testsuite/g++.dg/warn/pedantic2.C @@ -5,6 +5,6 @@ class foo foo() {}; void bar() {}; - foo(int) {};; // { dg-warning "extra" } - void bar(int) {};; // { dg-warning "extra" } + foo(int) {};; // { dg-warning "extra" "" { target c++98_only } } + void bar(int) {};; // { dg-warning "extra" "" { target c++98_only } } }; diff --git a/gcc/testsuite/g++.old-deja/g++.jason/parse11.C b/gcc/testsuite/g++.old-deja/g++.jason/parse11.C index 40864c108ba..157a9c46287 100644 --- a/gcc/testsuite/g++.old-deja/g++.jason/parse11.C +++ b/gcc/testsuite/g++.old-deja/g++.jason/parse11.C @@ -3,7 +3,7 @@ class aClass { - ; // { dg-error "" } missing declaration + ; // { dg-error "" "" { target c++98_only } } missing declaration private: - ; // { dg-error "" } missing declaration + ; // { dg-error "" "" { target c++98_only } } missing declaration }; base-commit: ab71fd7ac7a2307723117c796e7ae4d7e9711058
[PATCH] c++: adjust the extra ; warning [PR113760]
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- A minimal fix to quash an extra ; warning. I have a more complete patch for GCC 15. DR 1693 PR c++/113760 gcc/cp/ChangeLog: * parser.cc (cp_parser_member_declaration): Only pedwarn about an extra semicolon in C++98. gcc/testsuite/ChangeLog: * g++.dg/semicolon-fixits.C: Run in C++98 only. * g++.dg/warn/pedantic2.C: Adjust dg-warning. * g++.old-deja/g++.jason/parse11.C: Adjust dg-error. * g++.dg/DRs/dr1693-1.C: New test. * g++.dg/DRs/dr1693-2.C: New test. --- gcc/cp/parser.cc | 2 +- gcc/testsuite/g++.dg/DRs/dr1693-1.C| 9 + gcc/testsuite/g++.dg/DRs/dr1693-2.C| 9 + gcc/testsuite/g++.dg/semicolon-fixits.C| 1 + gcc/testsuite/g++.dg/warn/pedantic2.C | 4 ++-- gcc/testsuite/g++.old-deja/g++.jason/parse11.C | 4 ++-- 6 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/DRs/dr1693-1.C create mode 100644 gcc/testsuite/g++.dg/DRs/dr1693-2.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 68ab74d70b9..9d0914435fb 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27999,7 +27999,7 @@ cp_parser_member_declaration (cp_parser* parser) if (!decl_specifiers.any_specifiers_p) { cp_token *token = cp_lexer_peek_token (parser->lexer); - if (!in_system_header_at (token->location)) + if (cxx_dialect < cxx11 && !in_system_header_at (token->location)) { gcc_rich_location richloc (token->location); richloc.add_fixit_remove (); diff --git a/gcc/testsuite/g++.dg/DRs/dr1693-1.C b/gcc/testsuite/g++.dg/DRs/dr1693-1.C new file mode 100644 index 000..ed27026109c --- /dev/null +++ b/gcc/testsuite/g++.dg/DRs/dr1693-1.C @@ -0,0 +1,9 @@ +// DR 1693, Superfluous semicolons in class definitions +// PR c++/113760 +// { dg-do compile } +// { dg-options "" } + +struct S { + int a; + ; +}; diff --git a/gcc/testsuite/g++.dg/DRs/dr1693-2.C b/gcc/testsuite/g++.dg/DRs/dr1693-2.C new file mode 100644 index 000..c52259d4602 --- /dev/null +++ b/gcc/testsuite/g++.dg/DRs/dr1693-2.C @@ -0,0 +1,9 @@ +// DR 1693, Superfluous semicolons in class definitions +// PR c++/113760 +// { dg-do compile } +// { dg-options "-pedantic-errors" } + +struct S { + int a; + ;// { dg-error "extra" "" { target c++98_only } } +}; diff --git a/gcc/testsuite/g++.dg/semicolon-fixits.C b/gcc/testsuite/g++.dg/semicolon-fixits.C index a9cc783b172..198e306c950 100644 --- a/gcc/testsuite/g++.dg/semicolon-fixits.C +++ b/gcc/testsuite/g++.dg/semicolon-fixits.C @@ -1,3 +1,4 @@ +// { dg-do compile { target c++98_only } } /* { dg-options "-fdiagnostics-show-caret -Wpedantic" } */ /* Struct with extra semicolon. */ diff --git a/gcc/testsuite/g++.dg/warn/pedantic2.C b/gcc/testsuite/g++.dg/warn/pedantic2.C index 6c834162c1b..37d77daaef3 100644 --- a/gcc/testsuite/g++.dg/warn/pedantic2.C +++ b/gcc/testsuite/g++.dg/warn/pedantic2.C @@ -5,6 +5,6 @@ class foo foo() {}; void bar() {}; - foo(int) {};; // { dg-warning "extra" } - void bar(int) {};; // { dg-warning "extra" } + foo(int) {};; // { dg-warning "extra" "" { target c++98_only } } + void bar(int) {};; // { dg-warning "extra" "" { target c++98_only } } }; diff --git a/gcc/testsuite/g++.old-deja/g++.jason/parse11.C b/gcc/testsuite/g++.old-deja/g++.jason/parse11.C index 40864c108ba..157a9c46287 100644 --- a/gcc/testsuite/g++.old-deja/g++.jason/parse11.C +++ b/gcc/testsuite/g++.old-deja/g++.jason/parse11.C @@ -3,7 +3,7 @@ class aClass { - ; // { dg-error "" } missing declaration + ; // { dg-error "" "" { target c++98_only } } missing declaration private: - ; // { dg-error "" } missing declaration + ; // { dg-error "" "" { target c++98_only } } missing declaration }; base-commit: ab71fd7ac7a2307723117c796e7ae4d7e9711058 -- 2.43.0
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On 2/13/24 15:52, Marek Polacek wrote: On Tue, Feb 13, 2024 at 03:41:53PM -0500, Jason Merrill wrote: On 2/13/24 14:43, Marek Polacek wrote: On Tue, Feb 13, 2024 at 08:38:18PM +0100, Jakub Jelinek wrote: On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: Sadly, I must admit this is looking like GCC 15 material. If deferred for GCC 15, can't we at least do some minimal change and just guard the member pedwarn with cxx_dialect < something? I could do something like that, but... Given that -Wextra-semi isn't on by default nor included in -Wall -W, I think even accepting this for GCC 14 wouldn't be that risky. ...I also don't think it's that risky but technically, it's not a regression I think. Jason's decision. + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in +effect in C++11 and below. DR 1693 added "empty-declaration" to the +syntax for "member-declaration". */ + else if (pedantic && cxx_dialect < cxx14) If it was a DR, did it apply just to C++14 or changed C++11 as well? It's got Status: C++14 so I thought that C++11/C++98 had not been adjusted. That's just the timeframe it was accepted in; below that it says "moved to DR" which usually means it applies to earlier standards, whereas "accepted" or "applied to WP" do not. For 14 let's go with the minimal change Jakub suggests. Okay. Do we want to pedwarn about the extra ; here struct S { int a;; }; only in C++98 or C++11 too? Only in 98, I think. Jason
Re: [PATCH] c++: implicitly_declare_fn and access checks [PR113908]
On 2/13/24 11:49, Patrick Palka wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, are one of both of these fixes OK for trunk? -- >8 -- Here during ahead of time checking of the non-dependent new-expr we synthesize B's copy constructor, which should be defined as deleted due to A's inaccessible copy constructor. But enforce_access incorrectly decides to defer the (silent) access check for A::A(const A&) during synthesization since current_template_parms is still set (before r14-557 it checked processing_template_decl which got cleared from implicitly_declare_fn), which leads to the access check leaking out to the template context that needed the synthesization. This patch narrowly fixes this regression in two sufficient ways: 1. Clear current_template_parms alongside processing_template_decl in implicitly_declare_fn so that it's more independent of context. Hmm, perhaps it or synthesized_method_walk should use maybe_push_to_top_level? 2. Don't defer a silent access check when in a template context, since such deferred checks will be replayed noisily at instantiation time which may not be what the caller intended. True, but returning a possibly incorrect 'false' is probably also not what the caller intended. It would be better to see that we never call enforce_access with tf_none in a template. If that's not feasible, I think we should still conservatively return true. Jason
[patch, libgfortran] PR99210 X editing for reading file with encoding='utf-8'
The attached patch fixes the X editing. Fairly self explanatory. I created the patch a few years back. Regression tested on x86_64 and new test case. OK for trunk? Regards, Jerrydiff --git a/gcc/testsuite/gfortran.dg/pr99210.f90 b/gcc/testsuite/gfortran.dg/pr99210.f90 new file mode 100644 index 000..9fd2fb468df --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr99210.f90 @@ -0,0 +1,29 @@ +! { dg-do run } +! PR99210 X editing for reading file with encoding='utf-8' +program test_bug_format_x + use iso_fortran_env + integer, parameter :: u = selected_char_kind('ISO_10646') + + character(kind=u, len=1) a, b, a1, b1, b2 + + open(unit=10, file='test_bug_format_x.tmp', encoding='UTF-8') + + a = char(int(z'03B1'), u) + b = char(int(z'03B2'), u) + write(10, '(a1, a1)') a, b + + rewind(10) + read(10, '(a1, a1)') a1, b1 + + rewind(10) + read(10, '(1x, a1)') b2 + + close (10, status="delete") + if(a /= a1 .or. b /= b1) then +error stop 1 + end if + + if(b /= b2) then +error stop 2 + end if +end program test_bug_format_x diff --git a/libgfortran/io/read.c b/libgfortran/io/read.c index 0ffcf76fd38..e2d2f8be806 100644 --- a/libgfortran/io/read.c +++ b/libgfortran/io/read.c @@ -1307,6 +1307,23 @@ read_x (st_parameter_dt *dtp, size_t n) if (n == 0) return; + + if (dtp->u.p.current_unit->flags.encoding == ENCODING_UTF8) +{ + gfc_char4_t c; + size_t nbytes, j; + + /* Proceed with decoding one character at a time. */ + for (j = 0; j < n; j++) + { + c = read_utf8 (dtp, ); + + /* Check for a short read and if so, break out. */ + if (nbytes == 0 || c == (gfc_char4_t)0) + break; + } + return; +} length = n;
[PATCH] vect/testsuite: Fix vect-simd-clone-1[02].c when dg-do default is compile [PR113899]
The vect testsuite will chose the dg-do default based on if it knows the running target does not support running with the vector extensions enabled (for easy of testing). The problem is when it is decided the default is compile instead of run, dg-additional-sources does not work. So the fix is to set dg-do on these two testcases to run explicitly. OK? Tested on x86_64 with a hack to check_vect_support_and_set_flags to set the dg-default to compile. gcc/testsuite/ChangeLog: PR testsuite/113899 * gcc.dg/vect/vect-simd-clone-10.c: Add `dg-do run` * gcc.dg/vect/vect-simd-clone-12.c: Likewise. Signed-off-by: Andrew Pinski --- gcc/testsuite/gcc.dg/vect/vect-simd-clone-10.c | 2 ++ gcc/testsuite/gcc.dg/vect/vect-simd-clone-12.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-10.c b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-10.c index ed63ff59cc0..009c849b7e7 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-10.c +++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-10.c @@ -1,3 +1,5 @@ +/* Since this uses dg-additional-sources, need to specify `dg-do run` instead of the default. */ +/* { dg-do run } */ /* { dg-require-effective-target vect_simd_clones } */ /* { dg-additional-options "-fopenmp-simd" } */ /* { dg-additional-options "-mavx" { target avx_runtime } } */ diff --git a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-12.c b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-12.c index c44471e35bc..4699a3f3c80 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-simd-clone-12.c +++ b/gcc/testsuite/gcc.dg/vect/vect-simd-clone-12.c @@ -1,3 +1,5 @@ +/* Since this uses dg-additional-sources, need to specify `dg-do run` instead of the default. */ +/* { dg-do run } */ /* { dg-require-effective-target vect_simd_clones } */ /* { dg-additional-options "-fopenmp-simd" } */ /* { dg-additional-options "-mavx" { target avx_runtime } } */ -- 2.43.0
[PATCH] x86-64: Generate push2/pop2 only if the incoming stack is 16-byte aligned
Since push2/pop2 requires 16-byte stack alignment, don't generate them if the incoming stack isn't 16-byte aligned. gcc/ PR target/113912 * config/i386/i386.cc (ix86_can_use_push2pop2): New. (ix86_pro_and_epilogue_can_use_push2pop2): Use it. (ix86_emit_save_regs): Don't generate push2 if ix86_can_use_push2pop2 return false. (ix86_expand_epilogue): Don't generate pop2 if ix86_can_use_push2pop2 return false. gcc/testsuite/ PR target/113912 * gcc.target/i386/apx-push2pop2-2.c: New test. --- gcc/config/i386/i386.cc | 24 ++- .../gcc.target/i386/apx-push2pop2-2.c | 24 +++ 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index a4e12602f70..46f238651a6 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -6802,16 +6802,24 @@ get_probe_interval (void) #define SPLIT_STACK_AVAILABLE 256 -/* Helper function to determine whether push2/pop2 can be used in prologue or - epilogue for register save/restore. */ +/* Return true if push2/pop2 can be generated. */ + static bool -ix86_pro_and_epilogue_can_use_push2pop2 (int nregs) +ix86_can_use_push2pop2 (void) { /* Use push2/pop2 only if the incoming stack is 16-byte aligned. */ unsigned int incoming_stack_boundary = (crtl->parm_stack_boundary > ix86_incoming_stack_boundary ? crtl->parm_stack_boundary : ix86_incoming_stack_boundary); - if (incoming_stack_boundary % 128 != 0) + return incoming_stack_boundary % 128 == 0; +} + +/* Helper function to determine whether push2/pop2 can be used in prologue or + epilogue for register save/restore. */ +static bool +ix86_pro_and_epilogue_can_use_push2pop2 (int nregs) +{ + if (!ix86_can_use_push2pop2 ()) return false; int aligned = cfun->machine->fs.sp_offset % 16 == 0; return TARGET_APX_PUSH2POP2 @@ -7401,7 +7409,9 @@ ix86_emit_save_regs (void) int regno; rtx_insn *insn; - if (!TARGET_APX_PUSH2POP2 || cfun->machine->func_type != TYPE_NORMAL) + if (!TARGET_APX_PUSH2POP2 + || !ix86_can_use_push2pop2 () + || cfun->machine->func_type != TYPE_NORMAL) { for (regno = FIRST_PSEUDO_REGISTER - 1; regno >= 0; regno--) if (GENERAL_REGNO_P (regno) && ix86_save_reg (regno, true, true)) @@ -10039,7 +10049,9 @@ ix86_expand_epilogue (int style) m->fs.cfa_reg == stack_pointer_rtx); } - if (TARGET_APX_PUSH2POP2 && m->func_type == TYPE_NORMAL) + if (TARGET_APX_PUSH2POP2 + && ix86_can_use_push2pop2 () + && m->func_type == TYPE_NORMAL) ix86_emit_restore_regs_using_pop2 (); else ix86_emit_restore_regs_using_pop (TARGET_APX_PPX); diff --git a/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c b/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c new file mode 100644 index 000..975a6212b30 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/apx-push2pop2-2.c @@ -0,0 +1,24 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-options "-O2 -mpreferred-stack-boundary=3 -mapx-features=push2pop2 -fomit-frame-pointer" } */ + +extern int bar (int); + +void foo () +{ + int a,b,c,d,e,f,i; + a = bar (5); + b = bar (a); + c = bar (b); + d = bar (c); + e = bar (d); + f = bar (e); + for (i = 1; i < 10; i++) + { +a += bar (a + i) + bar (b + i) + + bar (c + i) + bar (d + i) + + bar (e + i) + bar (f + i); + } +} + +/* { dg-final { scan-assembler-not "push2(|p)\[\\t \]*%r" } } */ +/* { dg-final { scan-assembler-not "pop2(|p)\[\\t \]*%r" } } */ -- 2.43.0
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On Tue, Feb 13, 2024 at 03:41:53PM -0500, Jason Merrill wrote: > On 2/13/24 14:43, Marek Polacek wrote: > > On Tue, Feb 13, 2024 at 08:38:18PM +0100, Jakub Jelinek wrote: > > > On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: > > > > Sadly, I must admit this is looking like GCC 15 material. > > > > > > If deferred for GCC 15, can't we at least do some minimal > > > change and just guard the member pedwarn with cxx_dialect < something? > > > > I could do something like that, but... > > > > > Given that -Wextra-semi isn't on by default nor included in > > > -Wall -W, I think even accepting this for GCC 14 wouldn't be that > > > risky. > > > > ...I also don't think it's that risky but technically, it's not > > a regression I think. > > > Jason's decision. > > > > > > + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in > > > +effect in C++11 and below. DR 1693 added "empty-declaration" to > > > the > > > +syntax for "member-declaration". */ > > > + else if (pedantic && cxx_dialect < cxx14) > > > > > > If it was a DR, did it apply just to C++14 or changed C++11 as well? > > > > It's got Status: C++14 so I thought that C++11/C++98 had not been > > adjusted. > > That's just the timeframe it was accepted in; below that it says "moved to > DR" which usually means it applies to earlier standards, whereas "accepted" > or "applied to WP" do not. > > For 14 let's go with the minimal change Jakub suggests. Okay. Do we want to pedwarn about the extra ; here struct S { int a;; }; only in C++98 or C++11 too? Marek
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On 2/13/24 14:43, Marek Polacek wrote: On Tue, Feb 13, 2024 at 08:38:18PM +0100, Jakub Jelinek wrote: On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: Sadly, I must admit this is looking like GCC 15 material. If deferred for GCC 15, can't we at least do some minimal change and just guard the member pedwarn with cxx_dialect < something? I could do something like that, but... Given that -Wextra-semi isn't on by default nor included in -Wall -W, I think even accepting this for GCC 14 wouldn't be that risky. ...I also don't think it's that risky but technically, it's not a regression I think. Jason's decision. + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in +effect in C++11 and below. DR 1693 added "empty-declaration" to the +syntax for "member-declaration". */ + else if (pedantic && cxx_dialect < cxx14) If it was a DR, did it apply just to C++14 or changed C++11 as well? It's got Status: C++14 so I thought that C++11/C++98 had not been adjusted. That's just the timeframe it was accepted in; below that it says "moved to DR" which usually means it applies to earlier standards, whereas "accepted" or "applied to WP" do not. For 14 let's go with the minimal change Jakub suggests. Jason
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On Tue, Feb 13, 2024 at 02:43:39PM -0500, Marek Polacek wrote: > On Tue, Feb 13, 2024 at 08:38:18PM +0100, Jakub Jelinek wrote: > > On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: > > > Sadly, I must admit this is looking like GCC 15 material. > > > > If deferred for GCC 15, can't we at least do some minimal > > change and just guard the member pedwarn with cxx_dialect < something? > > I could do something like that, but... > > > Given that -Wextra-semi isn't on by default nor included in > > -Wall -W, I think even accepting this for GCC 14 wouldn't be that > > risky. > > ...I also don't think it's that risky but technically, it's not > a regression I think. > > > Jason's decision. > > > > + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in > > > > > > +effect in C++11 and below. DR 1693 added "empty-declaration" to > > the > > > > +syntax for "member-declaration". */ > > > > > > + else if (pedantic && cxx_dialect < cxx14) > > > > > > > > If it was a DR, did it apply just to C++14 or changed C++11 as well? > > It's got Status: C++14 so I thought that C++11/C++98 had not been > adjusted. ...but then so was DR 1601 which I implemented in https://gcc.gnu.org/git/?p=gcc.git=commit;h=e295e3d981355c61b72eca2ee58864958655cc31 and made it apply to C++11 as well. So I'm starting to lean towards adjusting this patch to make DR 1693 apply to C++11 as well, but not C++98. Probably not a very important distinction in practice though. Marek
Re: [PATCH] testsuite: Fix c-c++-common/pr103798-2.c on Solaris [PR113706]
On 2/13/24 05:27, Rainer Orth wrote: Hi Jason, On 2/2/24 10:23, Rainer Orth wrote: c-c++-common/pr103798-2.c FAILs on Solaris when compiled as C++: FAIL: c-c++-common/pr103798-2.c -std=gnu++14 scan-assembler-not memchr FAIL: c-c++-common/pr103798-2.c -std=gnu++17 scan-assembler-not memchr FAIL: c-c++-common/pr103798-2.c -std=gnu++20 scan-assembler-not memchr FAIL: c-c++-common/pr103798-2.c -std=gnu++98 scan-assembler-not memchr As H.J. analyzed in the PR, Solaris declares std::memchr, not memchr, which isn't treated as __builtin_memchr. The problem seems to be not the std::, but that the Solaris string.h declares const void *memchr(const void *, int, size_t); as specified by the C++ standard, while gcc expects the return type to be void* like in C. This looks like a GCC bug, not Solaris; I'd prefer to xfail the testcase rather than work around the compiler bug. thanks for the analysis. What I found with my current patch, just the memchr prototype changed to always return const void *, the test still PASSes as C, but FAILs as C++. In the C++ case I get a warning: /vol/gcc/src/hg/master/local/gcc/testsuite/c-c++-common/pr103798-2.c:10:20: warning: declaration of ‘const void* memchr(const void*, int, size_t)’ conflicts with built-in declaration ‘void* memchr(const void*, int, unsigned int)’ [-Wbuiltin-declaration-mismatch] Here's the patch to xfail the test instead. Tested on sparc-sun-solaris2.11 and x86_64-pc-linux-gnu. Ok for trunk? OK, thanks. Jason
Re: [Patch] OpenMP: Reject non-const 'condition' trait in Fortran
Hi Jakub, Jakub Jelinek wrote: Of course it makes me wonder to what extent we actually do support the OpenMP 5.1 target_device device_num trait with constant or non-constant device num: Answer: If one removes some early errors such that the compiler continues a bit further, one gets: 36 | !$omp declare variant(variant4) match(target_device={device_num(0)}) ! OK | 1 sorry, unimplemented: 'target_device' selector set is not supported yet Hence: Not yet supported, but the Sandra added the basic parsing support for it to GCC 14 when she improved the handling. However, the review-pending metadirective patch set https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642005.html has [PATCH 3/8] libgomp: runtime support for target_device selector Thus, once we GCC 15 development work has started, we can look into this feature and some other patches ... Other pending patches:https://gcc.gnu.org/wiki/openmpPendingPatches Tobias
Re: [PATCH] x86-64: Use push2/pop2 only if the incoming stack is 16-byte aligned
On Tue, Feb 13, 2024 at 11:58:00AM -0800, H.J. Lu wrote: > Since push2/pop2 requires 16-byte stack alignment, don't use them if the > incoming stack isn't 16-byte aligned. > > gcc/ > > PR target/113876 > * config/i386/i386.cc (ix86_pro_and_epilogue_can_use_push2pop2): > Return false if the incoming stack isn't 16-byte aligned. > > gcc/testsuite/ > > PR target/113876 > * gcc.target/i386/pr113876.c: New test. LGTM. Jakub
[PATCH] x86-64: Use push2/pop2 only if the incoming stack is 16-byte aligned
Since push2/pop2 requires 16-byte stack alignment, don't use them if the incoming stack isn't 16-byte aligned. gcc/ PR target/113876 * config/i386/i386.cc (ix86_pro_and_epilogue_can_use_push2pop2): Return false if the incoming stack isn't 16-byte aligned. gcc/testsuite/ PR target/113876 * gcc.target/i386/pr113876.c: New test. --- gcc/config/i386/i386.cc | 6 ++ gcc/testsuite/gcc.target/i386/pr113876.c | 10 ++ 2 files changed, 16 insertions(+) create mode 100644 gcc/testsuite/gcc.target/i386/pr113876.c diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index dbb26e8f76a..a4e12602f70 100644 --- a/gcc/config/i386/i386.cc +++ b/gcc/config/i386/i386.cc @@ -6807,6 +6807,12 @@ get_probe_interval (void) static bool ix86_pro_and_epilogue_can_use_push2pop2 (int nregs) { + /* Use push2/pop2 only if the incoming stack is 16-byte aligned. */ + unsigned int incoming_stack_boundary += (crtl->parm_stack_boundary > ix86_incoming_stack_boundary + ? crtl->parm_stack_boundary : ix86_incoming_stack_boundary); + if (incoming_stack_boundary % 128 != 0) +return false; int aligned = cfun->machine->fs.sp_offset % 16 == 0; return TARGET_APX_PUSH2POP2 && !cfun->machine->frame.save_regs_using_mov diff --git a/gcc/testsuite/gcc.target/i386/pr113876.c b/gcc/testsuite/gcc.target/i386/pr113876.c new file mode 100644 index 000..fbf26f6ab8f --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr113876.c @@ -0,0 +1,10 @@ +/* { dg-do compile { target { lp64 } } } */ +/* { dg-options "-O -mapxf -mpreferred-stack-boundary=3 -finstrument-functions -mcmodel=large" } */ + +void +bar (unsigned long *p) +{ + p[0] = 0; + p[1] = 0; + p[2] = 0; +} -- 2.43.0
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On Tue, Feb 13, 2024 at 08:38:18PM +0100, Jakub Jelinek wrote: > On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: > > Sadly, I must admit this is looking like GCC 15 material. > > If deferred for GCC 15, can't we at least do some minimal > change and just guard the member pedwarn with cxx_dialect < something? I could do something like that, but... > Given that -Wextra-semi isn't on by default nor included in > -Wall -W, I think even accepting this for GCC 14 wouldn't be that > risky. ...I also don't think it's that risky but technically, it's not a regression I think. > Jason's decision. > > + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in > > > +effect in C++11 and below. DR 1693 added "empty-declaration" to the > > > +syntax for "member-declaration". */ > > > + else if (pedantic && cxx_dialect < cxx14) > > > > If it was a DR, did it apply just to C++14 or changed C++11 as well? It's got Status: C++14 so I thought that C++11/C++98 had not been adjusted. Marek
Re: [PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
On Tue, Feb 13, 2024 at 02:24:11PM -0500, Marek Polacek wrote: > Sadly, I must admit this is looking like GCC 15 material. If deferred for GCC 15, can't we at least do some minimal change and just guard the member pedwarn with cxx_dialect < something? Given that -Wextra-semi isn't on by default nor included in -Wall -W, I think even accepting this for GCC 14 wouldn't be that risky. Jason's decision. + /* If -Wextra-semi wasn't specified, warn only when -pedantic is in +effect in C++11 and below. DR 1693 added "empty-declaration" to the +syntax for "member-declaration". */ + else if (pedantic && cxx_dialect < cxx14) If it was a DR, did it apply just to C++14 or changed C++11 as well? Jakub
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Am 13.02.24 um 19:13 schrieb Harald Anlauf: indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 This is now tracked as:: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113911 Will have to wait until the cause is found and fixed... As it is sufficient to disable the deferred-length test, I've done that and pushed the amended patch as https://gcc.gnu.org/g:f4935df217ad89f884f908f39086b322e80123d0 Thanks, Harald
[PATCH] c++: DR 569, DR 1693: fun with semicolons [PR113760]
Sadly, I must admit this is looking like GCC 15 material. Bootstrapped/regtested on x86_64-pc-linux-gnu. -- >8 -- Prompted by c++/113760, I started looking into a bogus "extra ;" warning in C++14. It quickly turned out that if I want to fix this for good, the fix will not be so small. This patch touches on DR 569, an extra ; at namespace scope should be allowed since C++11: struct S { }; ; // pedwarn in C++98 It also implements DR 1693, which allows superfluous semicolons in class definitions since C++14: struct S { int a; ; // pedwarn in C++98 + C++11 }; Note that a single semicolon is valid after a member function definition: struct S { void foo () {}; // only warns with -Wextra-semi }; There's a new function maybe_warn_extra_semi to handle all of the above in a single place. So now they all get a fix-it hint. -Wextra-semi turns on all "extra ;" diagnostics. Currently, options like -Wc++11-compat or -Wc++11-extensions are not considered. DR 1693 PR c++/113760 DR 569 gcc/c-family/ChangeLog: * c.opt (Wextra-semi): Initialize to -1. gcc/cp/ChangeLog: * parser.cc (extra_semi_kind): New. (maybe_warn_extra_semi): New. (cp_parser_declaration): Call maybe_warn_extra_semi. (cp_parser_member_declaration): Likewise. gcc/ChangeLog: * doc/invoke.texi: Update -Wextra-semi documentation. gcc/testsuite/ChangeLog: * g++.dg/semicolon-fixits.C: Use -Wextra-semi instead of -Wpedantic. * g++.dg/warn/pedantic2.C: Adjust dg-warning. * g++.old-deja/g++.jason/parse11.C: Adjust dg-error. * g++.dg/diagnostic/semicolon1.C: New test. * g++.dg/diagnostic/semicolon10.C: New test. * g++.dg/diagnostic/semicolon11.C: New test. * g++.dg/diagnostic/semicolon12.C: New test. * g++.dg/diagnostic/semicolon13.C: New test. * g++.dg/diagnostic/semicolon14.C: New test. * g++.dg/diagnostic/semicolon15.C: New test. * g++.dg/diagnostic/semicolon16.C: New test. * g++.dg/diagnostic/semicolon17.C: New test. * g++.dg/diagnostic/semicolon2.C: New test. * g++.dg/diagnostic/semicolon3.C: New test. * g++.dg/diagnostic/semicolon4.C: New test. * g++.dg/diagnostic/semicolon5.C: New test. * g++.dg/diagnostic/semicolon6.C: New test. * g++.dg/diagnostic/semicolon7.C: New test. * g++.dg/diagnostic/semicolon8.C: New test. * g++.dg/diagnostic/semicolon9.C: New test. --- gcc/c-family/c.opt| 2 +- gcc/cp/parser.cc | 92 +++ gcc/doc/invoke.texi | 29 +- gcc/testsuite/g++.dg/diagnostic/semicolon1.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon10.C | 11 +++ gcc/testsuite/g++.dg/diagnostic/semicolon11.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon12.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon13.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon14.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon15.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon16.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon17.C | 29 ++ gcc/testsuite/g++.dg/diagnostic/semicolon2.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon3.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon4.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon5.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon6.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon7.C | 18 gcc/testsuite/g++.dg/diagnostic/semicolon8.C | 11 +++ gcc/testsuite/g++.dg/diagnostic/semicolon9.C | 11 +++ gcc/testsuite/g++.dg/semicolon-fixits.C | 2 +- gcc/testsuite/g++.dg/warn/pedantic2.C | 4 +- .../g++.old-deja/g++.jason/parse11.C | 4 +- 23 files changed, 472 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon1.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon10.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon11.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon12.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon13.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon14.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon15.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon16.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon17.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon2.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon3.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon4.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon5.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon6.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon7.C create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon8.C create mode 100644
Re: [PATCH] libgccjit: Add ability to get CPU features
David: Ping. On Tue, 2024-02-06 at 07:54 -0500, Antoni Boucher wrote: > David: Ping. > > On Tue, 2024-01-30 at 10:50 -0500, Antoni Boucher wrote: > > David: I'm unsure what to do here. It seems we cannot find a > > reviewer. > > Would it help if I show you the code in gccrs that is similar? > > Would it help if I ask someone from gccrs to review this code? > > > > On Sat, 2024-01-20 at 09:50 -0500, Antoni Boucher wrote: > > > CC-ing Iain in case they can do the review since it is based on > > > how > > > they did it in the D frontend. > > > Could you please do the review? > > > Thanks! > > > > > > On Thu, 2023-11-09 at 18:04 -0500, David Malcolm wrote: > > > > On Thu, 2023-11-09 at 17:27 -0500, Antoni Boucher wrote: > > > > > Hi. > > > > > This patch adds support for getting the CPU features in > > > > > libgccjit > > > > > (bug > > > > > 112466) > > > > > > > > > > There's a TODO in the test: > > > > > I'm not sure how to test that gcc_jit_target_info_arch > > > > > returns > > > > > the > > > > > correct value since it is dependant on the CPU. > > > > > Any idea on how to improve this? > > > > > > > > > > Also, I created a CStringHash to be able to have a > > > > > std::unordered_set. Is there any built-in way > > > > > of > > > > > doing > > > > > this? > > > > > > > > Thanks for the patch. > > > > > > > > Some high-level questions: > > > > > > > > Is this specifically about detecting capabilities of the host > > > > that > > > > libgccjit is currently running on? or how the target was > > > > configured > > > > when libgccjit was built? > > > > > > > > One of the benefits of libgccjit is that, in theory, we support > > > > all > > > > of > > > > the targets that GCC already supports. Does this patch change > > > > that, > > > > or > > > > is this more about giving client code the ability to determine > > > > capabilities of the specific host being compiled for? > > > > > > > > I'm nervous about having per-target jit code. Presumably > > > > there's > > > > a > > > > reason that we can't reuse existing target logic here - can you > > > > please > > > > describe what the problem is. I see that the ChangeLog has: > > > > > > > > > * config/i386/i386-jit.cc: New file. > > > > > > > > where i386-jit.cc has almost 200 lines of nontrivial code. > > > > Where > > > > did > > > > this come from? Did you base it on existing code in our source > > > > tree, > > > > making modifications to fit the new internal API, or did you > > > > write > > > > it > > > > from scratch? In either case, how onerous would this be for > > > > other > > > > targets? > > > > > > > > I'm not at expert at target hooks (or at the i386 backend), so > > > > if > > > > we > > > > do > > > > go with this approach I'd want someone else to review those > > > > parts > > > > of > > > > the patch. > > > > > > > > Have you verified that GCC builds with this patch with jit > > > > *not* > > > > enabled in the enabled languages? > > > > > > > > [...snip...] > > > > > > > > A nitpick: > > > > > > > > > +.. function:: const char * \ > > > > > + gcc_jit_target_info_arch (gcc_jit_target_info > > > > > *info) > > > > > + > > > > > + Get the architecture of the currently running CPU. > > > > > > > > What does this string look like? > > > > How long does the pointer remain valid? > > > > > > > > Thanks again; hope the above makes sense > > > > Dave > > > > > > > > > >
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Hi Steve, Am 13.02.24 um 18:21 schrieb Steve Kargl: On Mon, Feb 12, 2024 at 09:57:08PM +0100, Harald Anlauf wrote: Dear all, the attached patch fixes a mis-handling of optional dummy arguments passed to optional dummy arguments of procedures with the bind(c) attribute. When those procedures are expecting CFI descriptors, there is no special treatment like a presence check necessary that by default passes a NULL pointer as default. The testcase tries to exercise various combinations of passing assumed-length character between bind(c) and non-bind(c), which apparently was insufficiently covered in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes. Thanks for filling out the more detailed testcase. indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 Will have to wait until the cause is found and fixed...
Re: [PATCH] ipa: call destructors on lattices before freeing them (PR 113476)
On Mon, Feb 12 2024, Jan Hubicka wrote: >> Believe it or not, even though I have re-worked the internals of the >> lattices completely, the array itself is older than my involvement with >> GCC (or at least with ipa-cp.c ;-). >> >> So it being an array and not a vector is historical coincidence, as far >> as I am concerned :-). But that may be the reason, or because vector >> macros at that time looked scary, or perhaps the initialization by >> XCNEWVEC zeroing everything out was considered attractive (I kind of >> like that but constructors would probably be cleaner), I don't know. > > If your class is no longer a POD, then the clearing before construcion > is dead and GCC may optimize it out. So fixing this may solve some > surprised in foreseable future when we will try to compile older GCC's > with newer ones. > That's a good point. I'll prepare a patch converting the whole thing to use constructors and vectors. Thanks, Martin
Re: [Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant')
On Tue, Feb 13, 2024 at 06:31:02PM +0100, Tobias Burnus wrote: > PR middle-end/113904 > > gcc/c/ChangeLog: > > * c-parser.cc (c_parser_omp_context_selector): Handle splitting of > OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. > > gcc/cp/ChangeLog: > > * parser.cc (cp_parser_omp_context_selector): Handle splitting of > OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. > > gcc/fortran/ChangeLog: > > * openmp.cc (gfc_match_omp_context_selector): > * trans-openmp.cc (gfc_trans_omp_declare_variant): Handle splitting of > OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. > > > gcc/ChangeLog: > > * omp-general.cc (struct omp_ts_info): Update for splitting of > OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTYDEV_NUM,BOOL}_EXPR. _{ missing above before DEV_NUM > * omp-selectors.h (enum omp_tp_type): Replace > OMP_TRAIT_PROPERTY_EXPR by OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/gomp/declare-variant-1.f90: Change 'condition' trait's > argument from integer to a logical expression. > * gfortran.dg/gomp/declare-variant-11.f90: Likewise. > * gfortran.dg/gomp/declare-variant-12.f90: Likewise. > * gfortran.dg/gomp/declare-variant-13.f90: Likewise. > * gfortran.dg/gomp/declare-variant-2.f90: Likewise. > * gfortran.dg/gomp/declare-variant-2a.f90: Likewise. > * gfortran.dg/gomp/declare-variant-3.f90: Likewise. > * gfortran.dg/gomp/declare-variant-4.f90: Likewise. > * gfortran.dg/gomp/declare-variant-6.f90: Likewise. > * gfortran.dg/gomp/declare-variant-8.f90: Likewise. > * gfortran.dg/gomp/declare-variant-20.f90: New test. Otherwise LGTM. Of course it makes me wonder to what extent we actually do support the OpenMP 5.1 target_device device_num trait with constant or non-constant device num, because similarly to the user condition trait if it something that can't be decided at compile time but needs to be checked at runtime, we need to be able to compute scores of all the variants that could be effective depending on the device_num comparison and/or condition evaluation and turn that into set of runtime comparisons with choices of the variants depending on the scores. If one is lucky, it can be device == device_num ? fn1 : fn2, but it can be more complex than that. Jakub
[Patch] OpenMP: Reject non-const 'condition' trait in Fortran (was: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant')
Jakub Jelinek wrote: Isn't all this caused just by the missing check that condition trait has a constant expression? IMHO that is the way to handle it in GCC 14. Concur – how about the following patch? Tobias PS: See PR113904 for follow up tasks. / Instead of '.AND.' etc. I could have also used some more '==', '<' etc. expressions in the modified examples (as should have Sandra in the initial version), but, fortunately, there is at least one '=='. OpenMP: Reject non-const 'condition' trait in Fortran OpenMP 5.0 only permits constant expressions for the 'condition' trait in context selectors; this is relaxed in 5.2 but not implemented. In order to avoid wrong code, it is now rejected. Additionally, in Fortran, 'condition' should not accept an integer expression, which is now ensured. Additionally, as 'device_num' should be a conforming device number, there is now a check on the value. PR middle-end/113904 gcc/c/ChangeLog: * c-parser.cc (c_parser_omp_context_selector): Handle splitting of OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. gcc/cp/ChangeLog: * parser.cc (cp_parser_omp_context_selector): Handle splitting of OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. gcc/fortran/ChangeLog: * openmp.cc (gfc_match_omp_context_selector): * trans-openmp.cc (gfc_trans_omp_declare_variant): Handle splitting of OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. gcc/ChangeLog: * omp-general.cc (struct omp_ts_info): Update for splitting of OMP_TRAIT_PROPERTY_EXPR into OMP_TRAIT_PROPERTYDEV_NUM,BOOL}_EXPR. * omp-selectors.h (enum omp_tp_type): Replace OMP_TRAIT_PROPERTY_EXPR by OMP_TRAIT_PROPERTY_{DEV_NUM,BOOL}_EXPR. gcc/testsuite/ChangeLog: * gfortran.dg/gomp/declare-variant-1.f90: Change 'condition' trait's argument from integer to a logical expression. * gfortran.dg/gomp/declare-variant-11.f90: Likewise. * gfortran.dg/gomp/declare-variant-12.f90: Likewise. * gfortran.dg/gomp/declare-variant-13.f90: Likewise. * gfortran.dg/gomp/declare-variant-2.f90: Likewise. * gfortran.dg/gomp/declare-variant-2a.f90: Likewise. * gfortran.dg/gomp/declare-variant-3.f90: Likewise. * gfortran.dg/gomp/declare-variant-4.f90: Likewise. * gfortran.dg/gomp/declare-variant-6.f90: Likewise. * gfortran.dg/gomp/declare-variant-8.f90: Likewise. * gfortran.dg/gomp/declare-variant-20.f90: New test. gcc/c/c-parser.cc | 3 +- gcc/cp/parser.cc | 3 +- gcc/fortran/openmp.cc | 30 ++--- gcc/fortran/trans-openmp.cc| 3 +- gcc/omp-general.cc | 4 +- gcc/omp-selectors.h| 3 +- .../gfortran.dg/gomp/declare-variant-1.f90 | 4 +- .../gfortran.dg/gomp/declare-variant-11.f90| 4 +- .../gfortran.dg/gomp/declare-variant-12.f90| 12 ++--- .../gfortran.dg/gomp/declare-variant-13.f90| 2 +- .../gfortran.dg/gomp/declare-variant-2.f90 | 8 ++-- .../gfortran.dg/gomp/declare-variant-20.f90| 51 ++ .../gfortran.dg/gomp/declare-variant-2a.f90| 4 +- .../gfortran.dg/gomp/declare-variant-3.f90 | 8 ++-- .../gfortran.dg/gomp/declare-variant-4.f90 | 8 ++-- .../gfortran.dg/gomp/declare-variant-6.f90 | 14 +++--- .../gfortran.dg/gomp/declare-variant-8.f90 | 2 +- 17 files changed, 119 insertions(+), 44 deletions(-) diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index c31349dae2f..3be91d666a5 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -24656,7 +24656,8 @@ c_parser_omp_context_selector (c_parser *parser, enum omp_tss_code set, } while (1); break; - case OMP_TRAIT_PROPERTY_EXPR: + case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR: + case OMP_TRAIT_PROPERTY_BOOL_EXPR: t = c_parser_expr_no_commas (parser, NULL).value; if (t != error_mark_node) { diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index f0c8f9c4005..68ab74d70b9 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -47984,7 +47984,8 @@ cp_parser_omp_context_selector (cp_parser *parser, enum omp_tss_code set, } while (1); break; - case OMP_TRAIT_PROPERTY_EXPR: + case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR: + case OMP_TRAIT_PROPERTY_BOOL_EXPR: /* FIXME: this is bogus, the expression need not be constant. */ t = cp_parser_constant_expression (parser); diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 0af80d54fad..d8cce6922b0 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -5790,19 +5790,39 @@ gfc_match_omp_context_selector (gfc_omp_set_selector *oss) } while (1); break; - case OMP_TRAIT_PROPERTY_EXPR: + case OMP_TRAIT_PROPERTY_DEV_NUM_EXPR: + case OMP_TRAIT_PROPERTY_BOOL_EXPR: if (gfc_match_expr (>expr) != MATCH_YES) {
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
On Mon, Feb 12, 2024 at 09:57:08PM +0100, Harald Anlauf wrote: > Dear all, > > the attached patch fixes a mis-handling of optional dummy arguments > passed to optional dummy arguments of procedures with the bind(c) > attribute. When those procedures are expecting CFI descriptors, > there is no special treatment like a presence check necessary > that by default passes a NULL pointer as default. > > The testcase tries to exercise various combinations of passing > assumed-length character between bind(c) and non-bind(c), which > apparently was insufficiently covered in the testsuite. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > Yes. Thanks for filling out the more detailed testcase. -- Steve
[PATCH] c++: implicitly_declare_fn and access checks [PR113908]
Bootstrapped and regtested on x86_64-pc-linux-gnu, are one of both of these fixes OK for trunk? -- >8 -- Here during ahead of time checking of the non-dependent new-expr we synthesize B's copy constructor, which should be defined as deleted due to A's inaccessible copy constructor. But enforce_access incorrectly decides to defer the (silent) access check for A::A(const A&) during synthesization since current_template_parms is still set (before r14-557 it checked processing_template_decl which got cleared from implicitly_declare_fn), which leads to the access check leaking out to the template context that needed the synthesization. This patch narrowly fixes this regression in two sufficient ways: 1. Clear current_template_parms alongside processing_template_decl in implicitly_declare_fn so that it's more independent of context. 2. Don't defer a silent access check when in a template context, since such deferred checks will be replayed noisily at instantiation time which may not be what the caller intended. We need to adjust cp_parser_make_typename_type since otherwise the diagnostic for typedef22.C regresses because cp_parser_invalid_type_name otherwise will emit "invalid combination of multiple type-specifiers" for what is an inaccessible qualified-id. PR c++/113908 gcc/cp/ChangeLog: * method.cc (implicitly_declare_fn): Clear current_template_parms alongside processing_template_decl. * parser.cc (cp_parser_make_typename_type): Temporarily defer access checks around the tf_none call to make_typename_type. * semantics.cc (enforce_access): Don't defer silent access checks from a template context. gcc/testsuite/ChangeLog: * g++.dg/template/non-dependent31.C: New test. --- gcc/cp/method.cc | 8 +++- gcc/cp/parser.cc | 2 ++ gcc/cp/semantics.cc| 1 + .../g++.dg/template/non-dependent31.C | 18 ++ 4 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/template/non-dependent31.C diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc index 957496d3e18..17d42143299 100644 --- a/gcc/cp/method.cc +++ b/gcc/cp/method.cc @@ -3088,6 +3088,7 @@ implicitly_declare_fn (special_function_kind kind, tree type, tree this_parm; tree name; HOST_WIDE_INT saved_processing_template_decl; + tree saved_current_template_parms; bool deleted_p = false; bool constexpr_p = false; tree inherited_ctor = (kind == sfk_inheriting_constructor @@ -3132,6 +3133,10 @@ implicitly_declare_fn (special_function_kind kind, tree type, when not in a template. */ saved_processing_template_decl = processing_template_decl; processing_template_decl = 0; + /* Also clear CURRENT_TEMPLATE_PARMS so enforce_access immediately checks + access even in a template context. */ + saved_current_template_parms = current_template_parms; + current_template_parms = NULL_TREE; type = TYPE_MAIN_VARIANT (type); @@ -3341,8 +3346,9 @@ implicitly_declare_fn (special_function_kind kind, tree type, set_constraints (fn, new_ci); } - /* Restore PROCESSING_TEMPLATE_DECL. */ + /* Restore PROCESSING_TEMPLATE_DECL and CURRENT_TEMPLATE_PARMS. */ processing_template_decl = saved_processing_template_decl; + current_template_parms = saved_current_template_parms; if (inherited_ctor && TREE_CODE (inherited_ctor) == TEMPLATE_DECL) fn = add_inherited_template_parms (fn, inherited_ctor); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index c4292c49ce2..29120995449 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -4350,8 +4350,10 @@ cp_parser_make_typename_type (cp_parser *parser, tree id, tree result; if (identifier_p (id)) { + push_deferring_access_checks (dk_deferred); result = make_typename_type (parser->scope, id, typename_type, /*complain=*/tf_none); + pop_to_parent_deferring_access_checks (); if (result == error_mark_node) cp_parser_diagnose_invalid_type_name (parser, id, id_location); return result; diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 3299e270446..0a18eaf9c2e 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -346,6 +346,7 @@ enforce_access (tree basetype_path, tree decl, tree diag_decl, tree cs = current_scope (); if (in_template_context + && (complain & tf_error) && (CLASS_TYPE_P (cs) || TREE_CODE (cs) == FUNCTION_DECL)) if (tree template_info = get_template_info (cs)) { diff --git a/gcc/testsuite/g++.dg/template/non-dependent31.C b/gcc/testsuite/g++.dg/template/non-dependent31.C new file mode 100644 index 000..3fa68f40fe1 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/non-dependent31.C @@ -0,0 +1,18 @@ +// PR c++/113908 +// { dg-do compile { target c++11 } } + +struct A { + A();
Re: [PATCH v2] x86: Support x32 and IBT in heap trampoline
On Tue, Feb 13, 2024 at 08:40:52AM -0800, H.J. Lu wrote: > Add x32 and IBT support to x86 heap trampoline implementation with a > testcase. > > 2024-02-13 Jakub Jelinek > H.J. Lu > > libgcc/ > > PR target/113855 > * config/i386/heap-trampoline.c (trampoline_insns): Add IBT > support and pad to the multiple of 4 bytes. Use movabsq > instead of movabs in comments. Add -mx32 variant. > > gcc/testsuite/ > > PR target/113855 > * gcc.dg/heap-trampoline-1.c: New test. > * lib/target-supports.exp (check_effective_target_heap_trampoline): > New. LGTM, but please give Iain a day or two to chime in. Jakub
[PATCH v2] x86: Support x32 and IBT in heap trampoline
Add x32 and IBT support to x86 heap trampoline implementation with a testcase. 2024-02-13 Jakub Jelinek H.J. Lu libgcc/ PR target/113855 * config/i386/heap-trampoline.c (trampoline_insns): Add IBT support and pad to the multiple of 4 bytes. Use movabsq instead of movabs in comments. Add -mx32 variant. gcc/testsuite/ PR target/113855 * gcc.dg/heap-trampoline-1.c: New test. * lib/target-supports.exp (check_effective_target_heap_trampoline): New. --- gcc/testsuite/gcc.dg/heap-trampoline-1.c | 23 + gcc/testsuite/lib/target-supports.exp| 12 +++ libgcc/config/i386/heap-trampoline.c | 42 ++-- 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/heap-trampoline-1.c diff --git a/gcc/testsuite/gcc.dg/heap-trampoline-1.c b/gcc/testsuite/gcc.dg/heap-trampoline-1.c new file mode 100644 index 000..1aebe00d731 --- /dev/null +++ b/gcc/testsuite/gcc.dg/heap-trampoline-1.c @@ -0,0 +1,23 @@ +/* { dg-do run { target heap_trampoline } } */ +/* { dg-options "-ftrampoline-impl=heap" } */ + +__attribute__((noipa)) int +bar (int (*fn) (int)) +{ + return fn (42) + 1; +} + +int +main () +{ + int a = 0; + int foo (int x) { if (x != 42) __builtin_abort (); return ++a; } + if (bar (foo) != 2 || a != 1) +__builtin_abort (); + if (bar (foo) != 3 || a != 2) +__builtin_abort (); + a = 42; + if (bar (foo) != 44 || a != 43) +__builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 6ce8557c9a9..81715999f87 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -13477,3 +13477,15 @@ proc dg-require-python-h { args } { eval lappend extra-tool-flags $python_flags verbose "After appending, extra-tool-flags: ${extra-tool-flags}" 3 } + +# Return 1 if the target supports heap-trampoline, 0 otherwise. +proc check_effective_target_heap_trampoline {} { +if { [istarget aarch64*-*-linux*] +|| [istarget i?86-*-darwin*] +|| [istarget x86_64-*-darwin*] +|| [istarget i?86-*-linux*] +|| [istarget x86_64-*-linux*] } { + return 1 +} +return 0 +} diff --git a/libgcc/config/i386/heap-trampoline.c b/libgcc/config/i386/heap-trampoline.c index 1df0aa06108..a8637dc92d3 100644 --- a/libgcc/config/i386/heap-trampoline.c +++ b/libgcc/config/i386/heap-trampoline.c @@ -30,28 +30,64 @@ void __gcc_nested_func_ptr_created (void *chain, void *func, void *dst); void __gcc_nested_func_ptr_deleted (void); #if __x86_64__ + +#ifdef __LP64__ static const uint8_t trampoline_insns[] = { - /* movabs $,%r11 */ +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movabsq $,%r11 */ 0x49, 0xbb, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* movabs $,%r10 */ + /* movabsq $,%r10 */ 0x49, 0xba, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* rex.WB jmpq *%r11 */ - 0x41, 0xff, 0xe3 + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 }; +#else +static const uint8_t trampoline_insns[] = { +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movl $,%r11d */ + 0x41, 0xbb, + 0x00, 0x00, 0x00, 0x00, + + /* movl $,%r10d */ + 0x41, 0xba, + 0x00, 0x00, 0x00, 0x00, + + /* rex.WB jmpq *%r11 */ + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 +}; +#endif union ix86_trampoline { uint8_t insns[sizeof(trampoline_insns)]; struct __attribute__((packed)) fields { +#if defined __CET__ && (__CET__ & 1) != 0 +uint8_t endbr64[4]; +#endif uint8_t insn_0[2]; void *func_ptr; uint8_t insn_1[2]; void *chain_ptr; uint8_t insn_2[3]; +uint8_t pad; } fields; }; -- 2.43.0
Re: [PATCH] RISC-V: Add support for B standard extension
On 2/6/24 10:38, Edwin Lu wrote: This patch adds support for recognizing the B standard extension to be the collection of Zba, Zbb, Zbs extensions for consistency and conciseness across toolchains * https://github.com/riscv/riscv-b/tags gcc/ChangeLog: * common/config/riscv/riscv-common.cc: Add imply rules for B extension * config/riscv/arch-canonicalize: ditto So similar to Patrick's change, no major concerns here. We just need to wait to see how the proposal moves through RVI before committing. jeff
[pushed] c++: variable partial spec redeclaration [PR113612]
Tested x86_64-pc-linux-gnu, applying to trunk. -- 8< -- If register_specialization finds a previous declaration and throws the new one away, we shouldn't still add the new one to DECL_TEMPLATE_SPECIALIZATIONS. PR c++/113612 gcc/cp/ChangeLog: * pt.cc (process_partial_specialization): Return early on redeclaration. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/var-templ85.C: New test. --- gcc/cp/pt.cc | 11 --- gcc/testsuite/g++.dg/cpp1y/var-templ85.C | 6 ++ 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ85.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 9c225c095c8..8bef2f8f6a2 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -5417,9 +5417,14 @@ process_partial_specialization (tree decl) } if (VAR_P (decl)) -/* We didn't register this in check_explicit_specialization so we could - wait until the constraints were set. */ -decl = register_specialization (decl, maintmpl, specargs, false, 0); +{ + /* We didn't register this in check_explicit_specialization so we could +wait until the constraints were set. */ + tree reg = register_specialization (decl, maintmpl, specargs, false, 0); + if (reg != decl) + /* Redeclaration. */ + return reg; +} else associate_classtype_constraints (type); diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ85.C b/gcc/testsuite/g++.dg/cpp1y/var-templ85.C new file mode 100644 index 000..33c24e0c284 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ85.C @@ -0,0 +1,6 @@ +// PR c++/113612 +// { dg-do compile { target c++14 } } + +template T t; +template extern T *t; +template T *t = t; base-commit: f29f7f86935e29786bf9f976ec99d7639b381b14 -- 2.43.0
Re: [RFC 1/3] RISC-V: Add basic Zaamo and Zalrsc support
On 2/7/24 17:30, Patrick O'Neill wrote: There is a proposal to split the A extension into two parts: Zaamo and Zalrsc. This patch adds basic support by making the A extension imply Zaamo and Zalrsc. Proposal: https://github.com/riscv/riscv-zaamo-zalrsc/tags gcc/ChangeLog: * common/config/riscv/riscv-common.cc: Add Zaamo and Zalrsc. * config/riscv/arch-canonicalize: Make A imply Zaamo and Zalrsc. * config/riscv/riscv.opt: Add Zaamo and Zalrsc * config/riscv/sync.md: Convert TARGET_ATOMIC to TARGET_ZAAMO and TARGET_ZALRSC. gcc/testsuite/ChangeLog: * gcc.target/riscv/attribute-15.c: Adjust expected arch string. * gcc.target/riscv/attribute-16.c: Ditto. * gcc.target/riscv/attribute-17.c: Ditto. * gcc.target/riscv/attribute-18.c: Ditto. * gcc.target/riscv/pr110696.c: Ditto. So no technical concerns here. Per the discussion in the sync call this morning, let's defer committing though. We'd like to see the proposal get accepted before committing the series. jeff
Re: [PATCH] pretty-print: Fix up ptrdiff handling for %tu, %to, %tx
On 2/13/24 01:21, Jakub Jelinek wrote: Hi! This is IMHO more of a theoretical case, I believe my current code doesn't handle %tu or %to or %tx correctly if ptrdiff_t is not one of int, long int or long long int. For size_t and %zd or %zi I'm using va_arg (whatever, ssize_t) and hope that ssize_t is the signed type corresponding to size_t which C99 talks about. For ptrdiff_t there is no type for unsigned type corresponding to ptrdiff_t and I'm not aware of a portable way on the host to get such a type (the fmt tests use hacks like #define signed /* Type might or might not have explicit 'signed'. */ typedef unsigned __PTRDIFF_TYPE__ unsigned_ptrdiff_t; #undef signed but that only works with compilers which predefine __PTRDIFF_TYPE__), std::make_unsigned I believe only works reliably if ptrdiff_t is one of char, signed char, short, int, long or long long, but won't work e.g. for __int20__ or whatever other weird ptrdiff_t the host might have. The following patch assumes host is two's complement (I think we rely on it pretty much everywhere anyway) and prints unsigned type corresponding to ptrdiff_t as unsigned long long printing of ptrdiff_t value masked with 2ULL * PTRDIFF_MAX + 1. So the only actual limitation is that it doesn't support ptrdiff_t wider than long long int. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-13 Jakub Jelinek * pretty-print.cc (PTRDIFF_MAX): Define if not yet defined. (pp_integer_with_precision): For unsigned ptrdiff_t printing with u, o or x print ptrdiff_t argument converted to unsigned long long and masked with 2ULL * PTRDIFF_MAX + 1. * error.cc (error_print): For u printing of ptrdiff_t, print ptrdiff_t argument converted to unsigned long long and masked with 2ULL * PTRDIFF_MAX + 1. OK jeff
Re: [PATCH] Add %[zt][diox] tests to gcc.dg/format/
On 2/13/24 01:26, Jakub Jelinek wrote: On Mon, Feb 12, 2024 at 04:10:33PM +, Joseph Myers wrote: On Sat, 10 Feb 2024, Jakub Jelinek wrote: * c-format.cc (gcc_diag_length_specs): Add t and z modifiers. (PP_FORMAT_CHAR_TABLE, gcc_gfc_char_table): Add entries for t and z modifiers. Please also add some tests of format checking for these modifiers in gcc.dg/format/gcc_*.c. The following patch does that. Haven't added tests for bad type (but I think we don't have them in c99-printf* either) for these because it is hard to figure out what type from {,unsigned }{int,long,long long} size_t/ptrdiff_t certainly is not, guess one could do that with preprocessor conditionals, e.g. comparing __PTRDIFF_MAX__ with __INT_MAX__, __LONG_MAX__ and __LONG_LONG_MAX__ and pick up the one which is different; but we'd need to find out corresponding effective targets for the expected diagnostics. Tested on x86_64-linux and i686-linux, ok for trunk? 2024-02-13 Jakub Jelinek * gcc.dg/format/gcc_diag-1.c (foo): Add tests for z and t modifiers. * gcc.dg/format/gcc_gfc-1.c (foo): Add tests for ll, z and t modifiers. OK. jeff
Re: [PATCH] RISC-V: Adjust vec unit-stride load/store costs.
On 2/13/24 06:42, Robin Dapp wrote: Hi, scalar loads provide offset addressing while unit-stride vector instructions cannot. The offset must be loaded into a general-purpose register before it can be used. In order to account for this, this patch adds an address arithmetic heuristic that keeps track of data reference operands. If we haven't seen the operand before we add the cost of a scalar statement. This helps to get rid of an lbm regression when vectorizing (roughly 0.5% fewer dynamic instructions). gcc5 improves by 0.2% and deepsjeng by 0.25%. wrf and nab degrade by 0.1%. This is because before we now adjust the cost of SLP as well as loop-vectorized instructions whereas we would only adjust loop-vectorized instructions before. Considering higher scalar_to_vec costs (3 vs 1) for all vectorization types causes some snippets not to get vectorized anymore. Given these costs the decisions look correct but appear worse when just counting dynamic instructions. In total SPECint 2017 has 4 bn dynamic instructions less and SPECfp 0.7 bn less so not a whole lot. Regtested on riscv64. Regards Robin gcc/ChangeLog: * config/riscv/riscv-vector-costs.cc (adjust_stmt_cost): Move... (costs::adjust_stmt_cost): ... to here and add vec_load/vec_store offset handling. (costs::add_stmt_cost): Also adjust cost for statements without stmt_info. * config/riscv/riscv-vector-costs.h: Define zero constant. gcc/testsuite/ChangeLog: * gcc.dg/vect/costmodel/riscv/rvv/vse-slp-1.c: New test. * gcc.dg/vect/costmodel/riscv/rvv/vse-slp-2.c: New test. OK. I do have this nagging suspicion that the trivial "vectorization" we sometimes get out of SLP isn't actually profitable in practice. So I'm not going to lose any sleep if we're less aggressive on those snippets of code. Jeff
Re: [PATCH] RISC-V: Fix macro fusion for auipc+add, when identifying UNSPEC_AUIPC. [PR113742]
On 2/4/24 20:20, Monk Chiang wrote: gcc/ChangeLog: PR target/113742 * config/riscv/riscv.cc (riscv_macro_fusion_pair_p): Fix recognizes UNSPEC_AUIPC for RISCV_FUSE_LUI_ADDI. gcc/testsuite/ChangeLog: * gcc.target/riscv/pr113742.c: New test. I was going through the patchwork queue after the call today and it looks like this didn't get pushed. So I took care of it. Thanks again, Jeff
Re: [PATCH] RISC-V: Point our Python scripts at python3
On 2/9/24 09:53, Palmer Dabbelt wrote: This builds for me, and I frequently have python-is-python3 type packages installed so I think I've been implicitly testing it for a while. Looks like Kito's tested similar configurations, and the bugzilla indicates we should be moving over. gcc/ChangeLog: PR 109668 * config/riscv/arch-canonicalize: Move to python3 * config/riscv/multilib-generator: Likewise Just to summarize from the coordination call this morning. We've agreed this should go forward. While there is minor risk (this code is rarely run), it's something we're prepared to handle if there is fallout. Jeff
Re: [PATCH] testsuite: Disable test for incompatible Arm targets
On 13/02/2024 10:44, Torbjörn SVENSSON wrote: Ok for trunk and releases/gcc-13? The alternative approach (that is changing the result a bit) is to drop the special treatment for arm*-*-*. I'm not sure if this is prefered or just disable the test for incompatible flags for arm*-*-*. -- The test assumes it's okay to supply -march=armv7-a+simd, but it depends on what target you are running the tests for. For example, running the GCC testsuite for Cortex-M0 produces the follwing entry in the logs: Running the testsuite with -mcpu= in runtest/site.exp flags will uncover a whole host of problems with tests that try to specify an architecture. It's essentially broken/unsupported at present. I have some ideas for how to fix this properly, but they will have to wait for gcc-15 now. In the mean time, I'd rather we didn't try to paper over the problem by putting random changes into the tests right now. R. Testing gcc.dg/pr41574.c doing compile Executing on host: arm-none-eabi-gcc .../pr41574.c -mthumb -march=armv6s-m -mcpu=cortex-m0 -mfloat-abi=soft -fdiagnostics-plain-output -O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine -ffat-lto-objects -S -o pr41574.s(timeout = 800) spawn -ignore SIGHUP arm-none-eabi-gcc .../pr41574.c -mthumb -march=armv6s-m -mcpu=cortex-m0 -mfloat-abi=soft -fdiagnostics-plain-output -O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine -ffat-lto-objects -S -o pr41574.s pid is 9799 -9799 cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' pid is -1 output is cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' status 0 FAIL: gcc.dg/pr41574.c (test for excess errors) Excess errors: cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' PASS: gcc.dg/pr41574.c scan-rtl-dump-not combine "\\(plus:DF \\(mult:DF" Patch has been verified on Linux. gcc/testsuite/ChangeLog: * gcc.dg/pr41574.c: Disable test for Arm targets incompatible with -march=armv7-a+simd. Signed-off-by: Torbjörn SVENSSON --- gcc/testsuite/gcc.dg/pr41574.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr41574.c b/gcc/testsuite/gcc.dg/pr41574.c index 062c0044532..f6af0c34273 100644 --- a/gcc/testsuite/gcc.dg/pr41574.c +++ b/gcc/testsuite/gcc.dg/pr41574.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine" { target { arm*-*-* } } } */ -/* { dg-options "-O2 -fno-unsafe-math-optimizations -fdump-rtl-combine" { target { ! arm*-*-* } } } */ +/* { dg-options "-O2 -fno-unsafe-math-optimizations -fdump-rtl-combine" } */ +/* { dg-require-effective-target arm_arch_v7a_neon_multilib { target { arm*-*-* } } } */ +/* { dg-additional-options "-march=armv7-a -mfloat-abi=softfp -mfpu=neon" { target { arm*-*-* } } } */ static const double one=1.0;
[PATCH] x86: Support x32 and IBT in heap trampoline
On Tue, Feb 13, 2024 at 10:42:52AM +0100, Jakub Jelinek wrote: > On Sat, Feb 10, 2024 at 10:05:34AM -0800, H.J. Lu wrote: > > > I bet it probably doesn't work properly for -mx32 (which defines > > > __x86_64__), CCing H.J. on that, but that is a preexisting issue > > > (and I don't have any experience with it; I guess one would either > > > need to add 4 bytes of padding after the func_ptr so that those > > > bits remain zeros as sizeof (void *) is 4, but presumably it would be > > > better to just use movl (but into %r10) and maybe the jmpl instead > > > of movabsq. > > > > Are there any testcases to exercise this code on Linux? > > Here is an untested attempt to implement it for -mx32 (well, I've compiled > it with -mx32 in libgcc by hand after stubbing > /usr/include/gnu/stubs-x32.h). > > Testcase could be something like: > > /* { dg-do run } */ > /* { dg-options "-ftrampoline-impl=heap" } */ > > __attribute__((noipa)) int > bar (int (*fn) (int)) > { > return fn (42) + 1; > } > > int > main () > { > int a = 0; > int foo (int x) { if (x != 42) __builtin_abort (); return ++a; } > if (bar (foo) != 2 || a != 1) > __builtin_abort (); > if (bar (foo) != 3 || a != 2) > __builtin_abort (); > a = 42; > if (bar (foo) != 44 || a != 43) > __builtin_abort (); > return 0; > } > but I must say I'm also surprised we have no tests for this in the > testsuite. Sure, we'd also need to add some effective target whether > -ftrampoline-impl=heap can be used for a link/runtime test or not. > > 2024-02-13 Jakub Jelinek > > PR target/113855 > * config/i386/heap-trampoline.c (trampoline_insns): Use movabsq > instead of movabs in comments. Add -mx32 variant. > It works on x32. I modified your patch to add IBT support and pad the trampoline to the multiple of 4 bytes. Thanks. H.J. --- 2024-02-13 Jakub Jelinek H.J. Lu PR target/113855 * config/i386/heap-trampoline.c (trampoline_insns): Add IBT support and pad to the multiple of 4 bytes. Use movabsq instead of movabs in comments. Add -mx32 variant. --- libgcc/config/i386/heap-trampoline.c | 42 ++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/libgcc/config/i386/heap-trampoline.c b/libgcc/config/i386/heap-trampoline.c index 1df0aa06108..a8637dc92d3 100644 --- a/libgcc/config/i386/heap-trampoline.c +++ b/libgcc/config/i386/heap-trampoline.c @@ -30,28 +30,64 @@ void __gcc_nested_func_ptr_created (void *chain, void *func, void *dst); void __gcc_nested_func_ptr_deleted (void); #if __x86_64__ + +#ifdef __LP64__ static const uint8_t trampoline_insns[] = { - /* movabs $,%r11 */ +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movabsq $,%r11 */ 0x49, 0xbb, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* movabs $,%r10 */ + /* movabsq $,%r10 */ 0x49, 0xba, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* rex.WB jmpq *%r11 */ - 0x41, 0xff, 0xe3 + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 }; +#else +static const uint8_t trampoline_insns[] = { +#if defined __CET__ && (__CET__ & 1) != 0 + /* endbr64. */ + 0xf3, 0x0f, 0x1e, 0xfa, +#endif + + /* movl $,%r11d */ + 0x41, 0xbb, + 0x00, 0x00, 0x00, 0x00, + + /* movl $,%r10d */ + 0x41, 0xba, + 0x00, 0x00, 0x00, 0x00, + + /* rex.WB jmpq *%r11 */ + 0x41, 0xff, 0xe3, + + /* Pad to the multiple of 4 bytes. */ + 0x90 +}; +#endif union ix86_trampoline { uint8_t insns[sizeof(trampoline_insns)]; struct __attribute__((packed)) fields { +#if defined __CET__ && (__CET__ & 1) != 0 +uint8_t endbr64[4]; +#endif uint8_t insn_0[2]; void *func_ptr; uint8_t insn_1[2]; void *chain_ptr; uint8_t insn_2[3]; +uint8_t pad; } fields; }; -- 2.43.0
Re: [Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'
On Tue, Feb 13, 2024 at 03:54:28PM +0100, Tobias Burnus wrote: > Inomp_resolve_declare_variant, a code path generates a new decl for the base > function – in doing so, it ignores the assembler name. As the included > Fortran example shows, this will lead to a linker error. Solution: Also copy > over the assembler name. Comments, suggestions, remarks before I commit it? > Tobias PS: As a fallout of some testing, motivated by the original testcase, > I have filled a couple of declare-variant and context-selector PRs: 113904 > (dyn. user={condition(...)}), 113905 (multiple users of variant funcs), > 113906 (construct={...} lacks constructs). > OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant' > > gcc/ChangeLog: > > * omp-general.cc (omp_resolve_declare_variant): When building the decl > for the base variant, honor also the assembler name. That artificial function I believe should never be emitted, neither it nor calls to it, so it shouldn't matter what DECL_ASSEMBLER_NAME it has. Isn't all this caused just by the missing check that condition trait has a constant expression? IMHO that is the way to handle it in GCC 14. Once middle-end has support for the dynamic conditions, it again should handle it properly, never resolve to the artificial one, but figure out what variants are possible given the unknown state of conditions, what scores they have and depending on that figure out what checks to perform at runtime to find out which function should be called in each case. It can get quite a bit complicated with lots of possible functions and lots of different score values. Jakub
[Patch] OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant'
Inomp_resolve_declare_variant, a code path generates a new decl for the base function – in doing so, it ignores the assembler name. As the included Fortran example shows, this will lead to a linker error. Solution: Also copy over the assembler name. Comments, suggestions, remarks before I commit it? Tobias PS: As a fallout of some testing, motivated by the original testcase, I have filled a couple of declare-variant and context-selector PRs: 113904 (dyn. user={condition(...)}), 113905 (multiple users of variant funcs), 113906 (construct={...} lacks constructs). OpenMP: Handle DECL_ASSEMBLER_NAME with 'declare variant' gcc/ChangeLog: * omp-general.cc (omp_resolve_declare_variant): When building the decl for the base variant, honor also the assembler name. gcc/testsuite/ChangeLog: * gfortran.dg/gomp/declare-variant-20.f90: New test. gcc/omp-general.cc | 2 + .../gfortran.dg/gomp/declare-variant-20.f90| 62 ++ 2 files changed, 64 insertions(+) diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc index 2e31a3f9290..bc92a170e96 100644 --- a/gcc/omp-general.cc +++ b/gcc/omp-general.cc @@ -2630,6 +2630,8 @@ omp_resolve_declare_variant (tree base) (*slot)->variants = entry.variants; tree alt = build_decl (DECL_SOURCE_LOCATION (base), FUNCTION_DECL, DECL_NAME (base), TREE_TYPE (base)); + if (DECL_ASSEMBLER_NAME_SET_P (base)) + SET_DECL_ASSEMBLER_NAME (alt, DECL_ASSEMBLER_NAME (base)); DECL_ARTIFICIAL (alt) = 1; DECL_IGNORED_P (alt) = 1; TREE_STATIC (alt) = 1; diff --git a/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90 b/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90 new file mode 100644 index 000..c7050a22365 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/gomp/declare-variant-20.f90 @@ -0,0 +1,62 @@ +! { dg-additional-options "-fdump-tree-gimple-asmname" } + +! This tests that mangled names, i.e. DECL_NAME != DECL_ASSEMBLER_NAME +! are properly handled + +! This test case failed before with: +! undefined reference to `foo' +! as the actual symbol is __m_MOD_foo + +! NOTE 1: This test relies on late resolution of condition, +! which is here enforced via the always_false_flag variable. +! +! NOTE 2: Using a variable is an OpenMP 5.1 feature that is/was not supported +! when this test case was created, cf. PR middle-end/113904 + +module m + implicit none (type, external) + logical :: always_false_flag = .false. +contains + integer function variant1() result(res) +res = 1 + end function + + integer function variant2() result(res) +res = 2 + end function + + integer function variant3() result(res) +res = 3 + end function + + integer function foo() result(res) +!$omp declare variant(variant1) match(construct={teams}) +!$omp declare variant(variant2) match(construct={parallel}) +!$omp declare variant(variant3) match(user={condition(always_false_flag)},construct={target}) +res = 99 + end +end module m + +program main + use m + implicit none (type, external) + integer :: r1, r2, r3 + + r1 = foo() + if (r1 /= 99) stop 1 + + !$omp parallel if (.false.) +r2 = foo() +if (r2 /= 2) stop 2 + !$omp end parallel + + !$omp teams num_teams(1) +r3 = foo() +if (r3 /= 1) stop 3 + !$omp end teams + +end program + +! { dg-final { scan-tree-dump-times "r1 = __m_MOD_foo \\(\\);" 1 "gimple" } } +! { dg-final { scan-tree-dump-times "r2 = __m_MOD_variant2 \\(\\);" 1 "gimple" } } +! { dg-final { scan-tree-dump-times "r3 = __m_MOD_variant1 \\(\\);" 1 "gimple" } }
[PATCH] RISC-V: Adjust vec unit-stride load/store costs.
Hi, scalar loads provide offset addressing while unit-stride vector instructions cannot. The offset must be loaded into a general-purpose register before it can be used. In order to account for this, this patch adds an address arithmetic heuristic that keeps track of data reference operands. If we haven't seen the operand before we add the cost of a scalar statement. This helps to get rid of an lbm regression when vectorizing (roughly 0.5% fewer dynamic instructions). gcc5 improves by 0.2% and deepsjeng by 0.25%. wrf and nab degrade by 0.1%. This is because before we now adjust the cost of SLP as well as loop-vectorized instructions whereas we would only adjust loop-vectorized instructions before. Considering higher scalar_to_vec costs (3 vs 1) for all vectorization types causes some snippets not to get vectorized anymore. Given these costs the decisions look correct but appear worse when just counting dynamic instructions. In total SPECint 2017 has 4 bn dynamic instructions less and SPECfp 0.7 bn less so not a whole lot. Regtested on riscv64. Regards Robin gcc/ChangeLog: * config/riscv/riscv-vector-costs.cc (adjust_stmt_cost): Move... (costs::adjust_stmt_cost): ... to here and add vec_load/vec_store offset handling. (costs::add_stmt_cost): Also adjust cost for statements without stmt_info. * config/riscv/riscv-vector-costs.h: Define zero constant. gcc/testsuite/ChangeLog: * gcc.dg/vect/costmodel/riscv/rvv/vse-slp-1.c: New test. * gcc.dg/vect/costmodel/riscv/rvv/vse-slp-2.c: New test. --- gcc/config/riscv/riscv-vector-costs.cc| 86 --- gcc/config/riscv/riscv-vector-costs.h | 10 +++ .../vect/costmodel/riscv/rvv/vse-slp-1.c | 51 +++ .../vect/costmodel/riscv/rvv/vse-slp-2.c | 53 4 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/vect/costmodel/riscv/rvv/vse-slp-1.c create mode 100644 gcc/testsuite/gcc.dg/vect/costmodel/riscv/rvv/vse-slp-2.c diff --git a/gcc/config/riscv/riscv-vector-costs.cc b/gcc/config/riscv/riscv-vector-costs.cc index 7c9840df4e9..adf9c197df5 100644 --- a/gcc/config/riscv/riscv-vector-costs.cc +++ b/gcc/config/riscv/riscv-vector-costs.cc @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see #include "backend.h" #include "tree-data-ref.h" #include "tree-ssa-loop-niter.h" +#include "tree-hash-traits.h" /* This file should be included last. */ #include "riscv-vector-costs.h" @@ -1047,18 +1048,81 @@ costs::better_main_loop_than_p (const vector_costs *uncast_other) const top of riscv_builtin_vectorization_cost handling which doesn't have any information on statement operation codes etc. */ -static unsigned -adjust_stmt_cost (enum vect_cost_for_stmt kind, tree vectype, int stmt_cost) +unsigned +costs::adjust_stmt_cost (enum vect_cost_for_stmt kind, loop_vec_info loop, +stmt_vec_info stmt_info, +slp_tree, tree vectype, int stmt_cost) { const cpu_vector_cost *costs = get_vector_costs (); switch (kind) { case scalar_to_vec: - return stmt_cost += (FLOAT_TYPE_P (vectype) ? costs->regmove->FR2VR - : costs->regmove->GR2VR); + stmt_cost += (FLOAT_TYPE_P (vectype) ? costs->regmove->FR2VR + : costs->regmove->GR2VR); + break; case vec_to_scalar: - return stmt_cost += (FLOAT_TYPE_P (vectype) ? costs->regmove->VR2FR - : costs->regmove->VR2GR); + stmt_cost += (FLOAT_TYPE_P (vectype) ? costs->regmove->VR2FR + : costs->regmove->VR2GR); + break; +case vector_load: +case vector_store: + { + /* Unit-stride vector loads and stores do not have offset addressing +as opposed to scalar loads and stores. +If the address depends on a variable we need an additional +add/sub for each load/store in the worst case. */ + if (stmt_info && stmt_info->stmt) + { + data_reference *dr = STMT_VINFO_DATA_REF (stmt_info); + class loop *father = stmt_info->stmt->bb->loop_father; + if (!loop && father && !father->inner && father->superloops) + { + tree ref; + if (TREE_CODE (dr->ref) != MEM_REF + || !(ref = TREE_OPERAND (dr->ref, 0)) + || TREE_CODE (ref) != SSA_NAME) + break; + + if (SSA_NAME_IS_DEFAULT_DEF (ref)) + break; + + if (memrefs.contains ({ref, cst0})) + break; + + memrefs.add ({ref, cst0}); + + /* In case we have not seen REF before and the base address +is a pointer operation try a bit harder. */ + tree base = DR_BASE_ADDRESS
[PATCH] tree-optimization/113895 - copy_reference_ops_from_ref vs. bitfields
The recent enhancement to discover constant array indices by range info used by get_ref_base_and_extent doesn't work when the outermost component reference is to a bitfield because we track the running offset in the reference ops as bytes. The following does as ao_ref_init_from_vn_reference and recovers that manually, tracking the offset for the purpose of discovering the constant array index in bits instead. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/113895 * tree-ssa-sccvn.cc (copy_reference_ops_from_ref): Track offset to discover constant array indices in bits, handle COMPONENT_REF to bitfields. * gcc.dg/torture/pr113895-1.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr113895-1.c | 16 ++ gcc/tree-ssa-sccvn.cc | 26 --- 2 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr113895-1.c diff --git a/gcc/testsuite/gcc.dg/torture/pr113895-1.c b/gcc/testsuite/gcc.dg/torture/pr113895-1.c new file mode 100644 index 000..e96cb2f33e1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr113895-1.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ + +int main_i; +void transparent_crc(int); +#pragma pack(1) +struct { + signed : 17; + signed : 6; + unsigned : 13; + unsigned f6 : 12; +} g_20[1]; +int main() +{ + transparent_crc(g_20[main_i].f6); + return 0; +} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 95670ae2ed6..d6b8c734e7b 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -1119,14 +1119,14 @@ copy_reference_ops_from_ref (tree ref, vec *result) unsigned HOST_WIDE_INT elsz = tree_to_uhwi (op.op2) * vn_ref_op_align_unit (); unsigned HOST_WIDE_INT idx - = (coffset / BITS_PER_UNIT - off.to_constant ()) / elsz; + = (coffset - off.to_constant ()) / BITS_PER_UNIT / elsz; if (idx == 0) op.op0 = op.op1; else op.op0 = wide_int_to_tree (TREE_TYPE (op.op0), wi::to_poly_wide (op.op1) + idx); op.off = idx * elsz; - off += op.off; + off += op.off * BITS_PER_UNIT; } else { @@ -1140,10 +1140,30 @@ copy_reference_ops_from_ref (tree ref, vec *result) || TREE_CODE_CLASS (op.opcode) == tcc_constant) /* end-of ref. */ gcc_assert (i == result->length ()); + else if (op.opcode == COMPONENT_REF) + { + /* op.off is tracked in bytes, re-do it manually +because of bitfields. */ + tree field = op.op0; + /* We do not have a complete COMPONENT_REF tree here so we +cannot use component_ref_field_offset. Do the interesting +parts manually. */ + tree this_offset = DECL_FIELD_OFFSET (field); + if (op.op1 || !poly_int_tree_p (this_offset)) + gcc_unreachable (); + else + { + poly_offset_int woffset + = (wi::to_poly_offset (this_offset) + << LOG2_BITS_PER_UNIT); + woffset += wi::to_offset (DECL_FIELD_BIT_OFFSET (field)); + off += woffset.force_shwi (); + } + } else { gcc_assert (known_ne (op.off, -1)); - off += op.off; + off += op.off * BITS_PER_UNIT; } } } -- 2.35.3
[PATCH] tree-optimization/113896 - testcase for fixed PR
The SLP permute optimization rewrite fixed this. Tested on x86_64-unknown-linux-gnu, pushed to trunk and 13 branch. PR tree-optimization/113896 * g++.dg/torture/pr113896.C: New testcase. --- gcc/testsuite/g++.dg/torture/pr113896.C | 35 + 1 file changed, 35 insertions(+) create mode 100644 gcc/testsuite/g++.dg/torture/pr113896.C diff --git a/gcc/testsuite/g++.dg/torture/pr113896.C b/gcc/testsuite/g++.dg/torture/pr113896.C new file mode 100644 index 000..534c1c2e1cc --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr113896.C @@ -0,0 +1,35 @@ +// { dg-do run } +// { dg-additional-options "-ffast-math" } + +double a1 = 1.0; +double a2 = 1.0; + +void __attribute__((noipa)) +f(double K[2], bool b) +{ +double A[] = { +b ? a1 : a2, +0, +0, +0 +}; + +double sum{}; +for(double a : A) sum += a; +for(double& a : A) a /= sum; + +if (b) { +K[0] = A[0]; // 1.0 +K[1] = A[1]; // 0.0 +} else { +K[0] = A[0] + A[1]; +} +} + +int main() +{ + double K[2]{}; + f(K, true); + if (K[0] != 1. || K[1] != 0.) +__builtin_abort (); +} -- 2.35.3
Re: [RFC] GCC Security policy
On 2024-02-12 10:00, Richard Biener wrote: GCC driver support is then to the extent identifying the inputs and the outputs. We already have -MM to generate a list of non-system dependencies, so gcc should be able to pass on inputs to the tool, which could then map those files (and the system headers directory) into the sandbox before invocation. The output file could perhaps be enforced as having to be a new one, i.e. fail if the target is already found. I'm not sure a generic utility can achieve this unless the outputs need to be retrieved from somewhere else (not "usual" place when invoking un-sandboxed). Even the driver doesn't necessarily know all files read/written. So I suppose better defining of the actual goal is in order. gcc -sandboxed -O2 -c t.ii -fdump-tree-all what should this do? IMO invoked tools (gas, cc1plus) should be restricted to access the input files. Ideally the dumps would appear where they appear when not sandboxed but clearly overwriting existing files would be problematic, writing new files not so much, but only to the standard (or specified) auxiliary output file paths. Couldn't we get away with not having to handle dump files? They don't seem to be sensitive targets. Thanks, Sid
Re: [PATCH] libgm2: Fix libm2iso/wraptime.cc compilation on Solaris
Rainer Orth writes: > As it turned out, my patch to complete the libgm2 autoconf macros works > on both Linux/sparc64 and Linux/x86_64, but breaks Solaris bootstrap: > > /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc: In function 'int > m2iso_wraptime_gettimeofday(void*, timezone*)': > /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:148:24: error: > invalid conversion from 'void*' to 'timeval*' [-fpermissive] > 148 | return gettimeofday (tv, tz); > |^~ > || > |void* > In file included from /usr/include/sys/select.h:27, > from /usr/include/sys/types.h:665, > from > /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:35: > /usr/include/sys/time.h:444:18: note: initializing argument 1 of 'int > gettimeofday(timeval*, void*)' > 444 | int gettimeofday(struct timeval *_RESTRICT_KYWD, void > *_RESTRICT_KYWD); > | ^ > /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc: In function 'int > m2iso_wraptime_settimeofday(void*, timezone*)': > /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:165:24: error: > invalid conversion from 'void*' to 'timeval*' [-fpermissive] > 165 | return settimeofday (tv, tz); > |^~ > || > |void* > /usr/include/sys/time.h:431:18: note: initializing argument 1 of 'int > settimeofday(timeval*, void*)' > 431 | int settimeofday(struct timeval *, void *); > | ^~~~ > > This happens because on Linux only HAVE_[GS]ETTIMEOFDAY is defined, > while Solaris has both that and HAVE_STRUCT_TIMEZONE, selecting > different implementations. > > Fixed by casting tv to struct timeval *. > > I thought about changing the signatures instead to take a struct timeval > * instead, but that seemed risky given that there's a > HAVE_STRUCT_TIMEVAL, so would probably break other targets. > > Bootstrapped without regressions on i386-pc-solaris2.11, > sparc-sun-solaris2.11, and x86_64-pc-linux-gnu. > > Ok for trunk? yes sure, lgtm - many thanks, regards, Gaius
[PATCH] Fix comment typo in ao_ref_init_from_vn_reference
Pushed. PR tree-optimization/113831 * tree-ssa-sccvn.cc (ao_ref_init_from_vn_reference): Fix typo in comment. --- gcc/tree-ssa-sccvn.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 74ead082558..d6b8c734e7b 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -1308,7 +1308,7 @@ ao_ref_init_from_vn_reference (ao_ref *ref, case ARRAY_RANGE_REF: case ARRAY_REF: - /* Use the recored constant offset. */ + /* Use the recorded constant offset. */ if (maybe_eq (op->off, -1)) max_size = -1; else -- 2.35.3
[PATCH] tree-optimization/113902 - fix VUSE update in move_early_exit_stmts
The following adjusts move_early_exit_stmts to track the last seen VUSE instead of getting it from the last store which could be a PHI where gimple_vuse doesn't work. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/113902 * tree-vect-loop.cc (move_early_exit_stmts): Track last_seen_vuse for VUSE updating. * gcc.dg/vect/pr113902.c: New testcase. --- gcc/testsuite/gcc.dg/vect/pr113902.c | 15 +++ gcc/tree-vect-loop.cc| 12 +++- 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/vect/pr113902.c diff --git a/gcc/testsuite/gcc.dg/vect/pr113902.c b/gcc/testsuite/gcc.dg/vect/pr113902.c new file mode 100644 index 000..a2fe3df26e2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/pr113902.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-add-options vect_early_break } */ + +int g_66, g_80_2; +void func_1func_41(int p_43) +{ +lbl_1434: + g_80_2 = 0; + for (; g_80_2 <= 7; g_80_2 += 1) { +g_66 = 0; +for (; g_66 <= 7; g_66 += 1) + if (p_43) +goto lbl_1434; + } +} diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc index 04f4b5b6b2f..4e498bb45ad 100644 --- a/gcc/tree-vect-loop.cc +++ b/gcc/tree-vect-loop.cc @@ -11787,6 +11787,7 @@ move_early_exit_stmts (loop_vec_info loop_vinfo) basic_block dest_bb = LOOP_VINFO_EARLY_BRK_DEST_BB (loop_vinfo); gimple_stmt_iterator dest_gsi = gsi_after_labels (dest_bb); + tree last_seen_vuse = NULL_TREE; for (gimple *stmt : LOOP_VINFO_EARLY_BRK_STORES (loop_vinfo)) { /* We have to update crossed degenerate virtual PHIs. Simply @@ -11805,6 +11806,7 @@ move_early_exit_stmts (loop_vec_info loop_vinfo) } auto gsi = gsi_for_stmt (stmt); remove_phi_node (, true); + last_seen_vuse = vuse; continue; } @@ -11819,17 +11821,17 @@ move_early_exit_stmts (loop_vec_info loop_vinfo) gimple_stmt_iterator stmt_gsi = gsi_for_stmt (stmt); gsi_move_before (_gsi, _gsi, GSI_NEW_STMT); + last_seen_vuse = gimple_vuse (stmt); } /* Update all the stmts with their new reaching VUSES. */ - tree vuse -= gimple_vuse (LOOP_VINFO_EARLY_BRK_STORES (loop_vinfo).last ()); for (auto p : LOOP_VINFO_EARLY_BRK_VUSES (loop_vinfo)) { if (dump_enabled_p ()) dump_printf_loc (MSG_NOTE, vect_location, - "updating vuse to %T for load %G", vuse, p); - gimple_set_vuse (p, vuse); + "updating vuse to %T for load %G", + last_seen_vuse, p); + gimple_set_vuse (p, last_seen_vuse); update_stmt (p); } @@ -11837,7 +11839,7 @@ move_early_exit_stmts (loop_vec_info loop_vinfo) for (edge e : get_loop_exit_edges (LOOP_VINFO_LOOP (loop_vinfo))) if (!dominated_by_p (CDI_DOMINATORS, e->src, dest_bb)) if (gphi *phi = get_virtual_phi (e->dest)) - SET_PHI_ARG_DEF_ON_EDGE (phi, e, vuse); + SET_PHI_ARG_DEF_ON_EDGE (phi, e, last_seen_vuse); } /* Function vect_transform_loop. -- 2.35.3
[PATCH] testsuite: Disable test for incompatible Arm targets
Ok for trunk and releases/gcc-13? The alternative approach (that is changing the result a bit) is to drop the special treatment for arm*-*-*. I'm not sure if this is prefered or just disable the test for incompatible flags for arm*-*-*. -- The test assumes it's okay to supply -march=armv7-a+simd, but it depends on what target you are running the tests for. For example, running the GCC testsuite for Cortex-M0 produces the follwing entry in the logs: Testing gcc.dg/pr41574.c doing compile Executing on host: arm-none-eabi-gcc .../pr41574.c -mthumb -march=armv6s-m -mcpu=cortex-m0 -mfloat-abi=soft -fdiagnostics-plain-output -O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine -ffat-lto-objects -S -o pr41574.s(timeout = 800) spawn -ignore SIGHUP arm-none-eabi-gcc .../pr41574.c -mthumb -march=armv6s-m -mcpu=cortex-m0 -mfloat-abi=soft -fdiagnostics-plain-output -O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine -ffat-lto-objects -S -o pr41574.s pid is 9799 -9799 cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' pid is -1 output is cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' status 0 FAIL: gcc.dg/pr41574.c (test for excess errors) Excess errors: cc1: warning: switch '-mcpu=cortex-m0' conflicts with switch '-march=armv7-a+simd' PASS: gcc.dg/pr41574.c scan-rtl-dump-not combine "\\(plus:DF \\(mult:DF" Patch has been verified on Linux. gcc/testsuite/ChangeLog: * gcc.dg/pr41574.c: Disable test for Arm targets incompatible with -march=armv7-a+simd. Signed-off-by: Torbjörn SVENSSON --- gcc/testsuite/gcc.dg/pr41574.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr41574.c b/gcc/testsuite/gcc.dg/pr41574.c index 062c0044532..f6af0c34273 100644 --- a/gcc/testsuite/gcc.dg/pr41574.c +++ b/gcc/testsuite/gcc.dg/pr41574.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -march=armv7-a -mfloat-abi=softfp -mfpu=neon -fno-unsafe-math-optimizations -fdump-rtl-combine" { target { arm*-*-* } } } */ -/* { dg-options "-O2 -fno-unsafe-math-optimizations -fdump-rtl-combine" { target { ! arm*-*-* } } } */ +/* { dg-options "-O2 -fno-unsafe-math-optimizations -fdump-rtl-combine" } */ +/* { dg-require-effective-target arm_arch_v7a_neon_multilib { target { arm*-*-* } } } */ +/* { dg-additional-options "-march=armv7-a -mfloat-abi=softfp -mfpu=neon" { target { arm*-*-* } } } */ static const double one=1.0; -- 2.25.1
Re: [PATCH]middle-end: update vector loop upper bounds when early break vect [PR113734]
On Tue, 13 Feb 2024, Tamar Christina wrote: > Hi All, > > When doing early break vectorization we should treat the final iteration as > possibly being partial. This so that when we calculate the vector loop upper > bounds we take into account that final iteration could have done some work. > > The attached testcase shows that if we don't then cunroll may unroll the loop > an > if the upper bound is wrong we lose a vector iteration. > > This is similar to how we adjust the scalar loop bounds for the PEELED case. > > Bootstrapped Regtested on aarch64-none-linux-gnu and > x86_64-pc-linux-gnu no issues. > > Ok for master? OK. Thanks, Richard. > Thanks, > Tamar > > gcc/ChangeLog: > > PR tree-optimization/113734 > * tree-vect-loop.cc (vect_transform_loop): Treat the final iteration of > an early break loop as partial. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/113734 > * gcc.dg/vect/vect-early-break_117-pr113734.c: New test. > > --- inline copy of patch -- > diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c > b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c > new file mode 100644 > index > ..36ae09483dfd426f977a3d92cf24a78d76de6961 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c > @@ -0,0 +1,37 @@ > +/* { dg-add-options vect_early_break } */ > +/* { dg-require-effective-target vect_early_break_hw } */ > +/* { dg-require-effective-target vect_int } */ > +/* { dg-additional-options "-O3" } */ > + > +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */ > + > +#include "tree-vect.h" > + > +#define N 306 > +#define NEEDLE 136 > + > +int table[N]; > + > +__attribute__ ((noipa)) > +int foo (int i, unsigned short parse_tables_n) > +{ > + parse_tables_n >>= 9; > + parse_tables_n += 11; > + while (i < N && parse_tables_n--) > +table[i++] = 0; > + > + return table[NEEDLE]; > +} > + > +int main () > +{ > + check_vect (); > + > + for (int j = 0; j < N; j++) > +table[j] = -1; > + > + if (foo (0, 0x) != 0) > +__builtin_abort (); > + > + return 0; > +} > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc > index > 854e9d78bc71721e6559a6bc5dff78c813603a78..0b1656fef2fed83f30295846c382ad9fb318454a > 100644 > --- a/gcc/tree-vect-loop.cc > +++ b/gcc/tree-vect-loop.cc > @@ -12171,7 +12171,8 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple > *loop_vectorized_call) >/* True if the final iteration might not handle a full vector's > worth of scalar iterations. */ >bool final_iter_may_be_partial > -= LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo); > += LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) > + || LOOP_VINFO_EARLY_BREAKS (loop_vinfo); >/* The minimum number of iterations performed by the epilogue. This > is 1 when peeling for gaps because we always need a final scalar > iteration. */ > > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
[PATCH] tree-optimization/113898 - ICE with sanity checking for VN ref adjustment
The following fixes a missing add to the accumulated offset when adjusting an ARRAY_REF op for value-ranges applied to by get_ref_base_and_extent. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. PR tree-optimization/113898 * tree-ssa-sccvn.cc (copy_reference_ops_from_ref): Add missing accumulated off adjustment. * gcc.dg/torture/pr113898.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr113898.c | 16 gcc/tree-ssa-sccvn.cc | 1 + 2 files changed, 17 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/torture/pr113898.c diff --git a/gcc/testsuite/gcc.dg/torture/pr113898.c b/gcc/testsuite/gcc.dg/torture/pr113898.c new file mode 100644 index 000..6832a345271 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr113898.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ + +int a, d; +unsigned **b; +long c, f; +long e[2][1]; +void g() { + int h = 0; + for (; h < 2; h++) { +e[h][d + **b + a] = c; +if (f) + for (;;) +; + } +} +void main() {} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 2823573b656..5a49390a79c 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -1126,6 +1126,7 @@ copy_reference_ops_from_ref (tree ref, vec *result) op.op0 = wide_int_to_tree (TREE_TYPE (op.op0), wi::to_poly_wide (op.op1) + idx); op.off = idx * elsz; + off += op.off; } else { -- 2.35.3
[PATCH]middle-end: update vector loop upper bounds when early break vect [PR113734]
Hi All, When doing early break vectorization we should treat the final iteration as possibly being partial. This so that when we calculate the vector loop upper bounds we take into account that final iteration could have done some work. The attached testcase shows that if we don't then cunroll may unroll the loop an if the upper bound is wrong we lose a vector iteration. This is similar to how we adjust the scalar loop bounds for the PEELED case. Bootstrapped Regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu no issues. Ok for master? Thanks, Tamar gcc/ChangeLog: PR tree-optimization/113734 * tree-vect-loop.cc (vect_transform_loop): Treat the final iteration of an early break loop as partial. gcc/testsuite/ChangeLog: PR tree-optimization/113734 * gcc.dg/vect/vect-early-break_117-pr113734.c: New test. --- inline copy of patch -- diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c new file mode 100644 index ..36ae09483dfd426f977a3d92cf24a78d76de6961 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c @@ -0,0 +1,37 @@ +/* { dg-add-options vect_early_break } */ +/* { dg-require-effective-target vect_early_break_hw } */ +/* { dg-require-effective-target vect_int } */ +/* { dg-additional-options "-O3" } */ + +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */ + +#include "tree-vect.h" + +#define N 306 +#define NEEDLE 136 + +int table[N]; + +__attribute__ ((noipa)) +int foo (int i, unsigned short parse_tables_n) +{ + parse_tables_n >>= 9; + parse_tables_n += 11; + while (i < N && parse_tables_n--) +table[i++] = 0; + + return table[NEEDLE]; +} + +int main () +{ + check_vect (); + + for (int j = 0; j < N; j++) +table[j] = -1; + + if (foo (0, 0x) != 0) +__builtin_abort (); + + return 0; +} diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc index 854e9d78bc71721e6559a6bc5dff78c813603a78..0b1656fef2fed83f30295846c382ad9fb318454a 100644 --- a/gcc/tree-vect-loop.cc +++ b/gcc/tree-vect-loop.cc @@ -12171,7 +12171,8 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call) /* True if the final iteration might not handle a full vector's worth of scalar iterations. */ bool final_iter_may_be_partial -= LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo); += LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) + || LOOP_VINFO_EARLY_BREAKS (loop_vinfo); /* The minimum number of iterations performed by the epilogue. This is 1 when peeling for gaps because we always need a final scalar iteration. */ -- diff --git a/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c new file mode 100644 index ..36ae09483dfd426f977a3d92cf24a78d76de6961 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/vect-early-break_117-pr113734.c @@ -0,0 +1,37 @@ +/* { dg-add-options vect_early_break } */ +/* { dg-require-effective-target vect_early_break_hw } */ +/* { dg-require-effective-target vect_int } */ +/* { dg-additional-options "-O3" } */ + +/* { dg-final { scan-tree-dump "LOOP VECTORIZED" "vect" } } */ + +#include "tree-vect.h" + +#define N 306 +#define NEEDLE 136 + +int table[N]; + +__attribute__ ((noipa)) +int foo (int i, unsigned short parse_tables_n) +{ + parse_tables_n >>= 9; + parse_tables_n += 11; + while (i < N && parse_tables_n--) +table[i++] = 0; + + return table[NEEDLE]; +} + +int main () +{ + check_vect (); + + for (int j = 0; j < N; j++) +table[j] = -1; + + if (foo (0, 0x) != 0) +__builtin_abort (); + + return 0; +} diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc index 854e9d78bc71721e6559a6bc5dff78c813603a78..0b1656fef2fed83f30295846c382ad9fb318454a 100644 --- a/gcc/tree-vect-loop.cc +++ b/gcc/tree-vect-loop.cc @@ -12171,7 +12171,8 @@ vect_transform_loop (loop_vec_info loop_vinfo, gimple *loop_vectorized_call) /* True if the final iteration might not handle a full vector's worth of scalar iterations. */ bool final_iter_may_be_partial -= LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo); += LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) + || LOOP_VINFO_EARLY_BREAKS (loop_vinfo); /* The minimum number of iterations performed by the epilogue. This is 1 when peeling for gaps because we always need a final scalar iteration. */
[PATCH] libgm2: Fix libm2iso/wraptime.cc compilation on Solaris
As it turned out, my patch to complete the libgm2 autoconf macros works on both Linux/sparc64 and Linux/x86_64, but breaks Solaris bootstrap: /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc: In function 'int m2iso_wraptime_gettimeofday(void*, timezone*)': /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:148:24: error: invalid conversion from 'void*' to 'timeval*' [-fpermissive] 148 | return gettimeofday (tv, tz); |^~ || |void* In file included from /usr/include/sys/select.h:27, from /usr/include/sys/types.h:665, from /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:35: /usr/include/sys/time.h:444:18: note: initializing argument 1 of 'int gettimeofday(timeval*, void*)' 444 | int gettimeofday(struct timeval *_RESTRICT_KYWD, void *_RESTRICT_KYWD); | ^ /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc: In function 'int m2iso_wraptime_settimeofday(void*, timezone*)': /vol/gcc/src/hg/master/local/libgm2/libm2iso/wraptime.cc:165:24: error: invalid conversion from 'void*' to 'timeval*' [-fpermissive] 165 | return settimeofday (tv, tz); |^~ || |void* /usr/include/sys/time.h:431:18: note: initializing argument 1 of 'int settimeofday(timeval*, void*)' 431 | int settimeofday(struct timeval *, void *); | ^~~~ This happens because on Linux only HAVE_[GS]ETTIMEOFDAY is defined, while Solaris has both that and HAVE_STRUCT_TIMEZONE, selecting different implementations. Fixed by casting tv to struct timeval *. I thought about changing the signatures instead to take a struct timeval * instead, but that seemed risky given that there's a HAVE_STRUCT_TIMEVAL, so would probably break other targets. Bootstrapped without regressions on i386-pc-solaris2.11, sparc-sun-solaris2.11, and x86_64-pc-linux-gnu. Ok for trunk? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2024-02-13 Rainer Orth libgm2: * libm2iso/wraptime.cc [HAVE_STRUCT_TIMEZONE && HAVE_GETTIMEOFDAY] (EXPORT(gettimeofday)): Cast tv to struct timeval *. [HAVE_STRUCT_TIMEZONE && HAVE_SETTIMEOFDAY] (EXPORT(settimeofday)): Likewise. # HG changeset patch # Parent 54196105ece9b22dbacfebb0bebd5c857cd5c19a libgm2: Fix libm2iso/wraptime.cc compilation on Solaris diff --git a/libgm2/libm2iso/wraptime.cc b/libgm2/libm2iso/wraptime.cc --- a/libgm2/libm2iso/wraptime.cc +++ b/libgm2/libm2iso/wraptime.cc @@ -145,7 +145,7 @@ EXPORT(KillTM) (struct tm *tv) extern "C" int EXPORT(gettimeofday) (void *tv, struct timezone *tz) { - return gettimeofday (tv, tz); + return gettimeofday ((struct timeval *) tv, tz); } #else extern "C" int @@ -162,7 +162,7 @@ EXPORT(gettimeofday) (void *tv, void *tz extern "C" int EXPORT(settimeofday) (void *tv, struct timezone *tz) { - return settimeofday (tv, tz); + return settimeofday ((struct timeval *) tv, tz); } #else extern "C" int
Re: [PATCH] testsuite: Fix c-c++-common/pr103798-2.c on Solaris [PR113706]
Hi Jason, > On 2/2/24 10:23, Rainer Orth wrote: >> c-c++-common/pr103798-2.c FAILs on Solaris when compiled as C++: >> FAIL: c-c++-common/pr103798-2.c -std=gnu++14 scan-assembler-not memchr >> FAIL: c-c++-common/pr103798-2.c -std=gnu++17 scan-assembler-not memchr >> FAIL: c-c++-common/pr103798-2.c -std=gnu++20 scan-assembler-not memchr >> FAIL: c-c++-common/pr103798-2.c -std=gnu++98 scan-assembler-not memchr >> As H.J. analyzed in the PR, Solaris declares std::memchr, not >> memchr, which isn't treated as __builtin_memchr. > > The problem seems to be not the std::, but that the Solaris string.h > declares > > const void *memchr(const void *, int, size_t); > > as specified by the C++ standard, while gcc expects the return type to be > void* like in C. > > This looks like a GCC bug, not Solaris; I'd prefer to xfail the testcase > rather than work around the compiler bug. thanks for the analysis. What I found with my current patch, just the memchr prototype changed to always return const void *, the test still PASSes as C, but FAILs as C++. In the C++ case I get a warning: /vol/gcc/src/hg/master/local/gcc/testsuite/c-c++-common/pr103798-2.c:10:20: warning: declaration of ‘const void* memchr(const void*, int, size_t)’ conflicts with built-in declaration ‘void* memchr(const void*, int, unsigned int)’ [-Wbuiltin-declaration-mismatch] Here's the patch to xfail the test instead. Tested on sparc-sun-solaris2.11 and x86_64-pc-linux-gnu. Ok for trunk? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2024-02-12 Rainer Orth testsuite: PR tree-optimization/113706 * c-c++-common/pr103798-2.c (scan-assembler-not): xfail for C++ on Solaris. # HG changeset patch # Parent 1409f56e818a7240dd65da9566400f308a996beb testsuite: Fix c-c++-common/pr103798-2.c on Solaris [PR113706] diff --git a/gcc/testsuite/c-c++-common/pr103798-2.c b/gcc/testsuite/c-c++-common/pr103798-2.c --- a/gcc/testsuite/c-c++-common/pr103798-2.c +++ b/gcc/testsuite/c-c++-common/pr103798-2.c @@ -27,4 +27,5 @@ main () return 0; } -/* { dg-final { scan-assembler-not "memchr" } } */ +/* See PR tree-optimization/113706 for the xfail. */ +/* { dg-final { scan-assembler-not "memchr" { xfail { c++ && *-*-solaris2* } } } } */
[PATCH] libgcc: Implement -mx32 [PR113855]
On Sat, Feb 10, 2024 at 10:05:34AM -0800, H.J. Lu wrote: > > I bet it probably doesn't work properly for -mx32 (which defines > > __x86_64__), CCing H.J. on that, but that is a preexisting issue > > (and I don't have any experience with it; I guess one would either > > need to add 4 bytes of padding after the func_ptr so that those > > bits remain zeros as sizeof (void *) is 4, but presumably it would be > > better to just use movl (but into %r10) and maybe the jmpl instead > > of movabsq. > > Are there any testcases to exercise this code on Linux? Here is an untested attempt to implement it for -mx32 (well, I've compiled it with -mx32 in libgcc by hand after stubbing /usr/include/gnu/stubs-x32.h). Testcase could be something like: /* { dg-do run } */ /* { dg-options "-ftrampoline-impl=heap" } */ __attribute__((noipa)) int bar (int (*fn) (int)) { return fn (42) + 1; } int main () { int a = 0; int foo (int x) { if (x != 42) __builtin_abort (); return ++a; } if (bar (foo) != 2 || a != 1) __builtin_abort (); if (bar (foo) != 3 || a != 2) __builtin_abort (); a = 42; if (bar (foo) != 44 || a != 43) __builtin_abort (); return 0; } but I must say I'm also surprised we have no tests for this in the testsuite. Sure, we'd also need to add some effective target whether -ftrampoline-impl=heap can be used for a link/runtime test or not. 2024-02-13 Jakub Jelinek PR target/113855 * config/i386/heap-trampoline.c (trampoline_insns): Use movabsq instead of movabs in comments. Add -mx32 variant. --- libgcc/config/i386/heap-trampoline.c.jj 2024-02-12 18:48:08.548886036 +0100 +++ libgcc/config/i386/heap-trampoline.c2024-02-13 10:24:06.348020670 +0100 @@ -30,18 +30,34 @@ void __gcc_nested_func_ptr_created (void void __gcc_nested_func_ptr_deleted (void); #if __x86_64__ + +#ifdef __LP64__ static const uint8_t trampoline_insns[] = { - /* movabs $,%r11 */ + /* movabsq $,%r11 */ 0x49, 0xbb, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - /* movabs $,%r10 */ + /* movabsq $,%r10 */ 0x49, 0xba, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* rex.WB jmpq *%r11 */ 0x41, 0xff, 0xe3 }; +#else +static const uint8_t trampoline_insns[] = { + /* movl $,%r11d */ + 0x41, 0xbb, + 0x00, 0x00, 0x00, 0x00, + + /* movl $,%r10d */ + 0x41, 0xba, + 0x00, 0x00, 0x00, 0x00, + + /* rex.WB jmpq *%r11 */ + 0x41, 0xff, 0xe3 +}; +#endif union ix86_trampoline { uint8_t insns[sizeof(trampoline_insns)]; Jakub
Re: [PATCH] libgcc: Fix UB in FP_FROM_BITINT
On Tue, 13 Feb 2024, Jakub Jelinek wrote: > Hi! > > As I wrote earlier, I was seeing > FAIL: gcc.dg/torture/bitint-24.c -O0 execution test > FAIL: gcc.dg/torture/bitint-24.c -O2 execution test > with the ia32 _BitInt enablement patch on i686-linux. I thought > floatbitintxf.c was miscompiled with -O2 -march=i686 -mtune=generic, but it > turned out to be UB in it. > > If a signed _BitInt to be converted to binary floating point has > (after sign extension from possible partial limb to full limb) one or > more most significant limbs equal to all ones and then in the limb below > (the most significant non-~(UBILtype)0 limb) has the most significant limb > cleared, like for 32-bit limbs > 0x81582c05U, 0x0a8b01e4U, 0xc1b8b18fU, 0x2aac2a08U, -1U, -1U > then bitint_reduce_prec can't reduce it to that 0x2aac2a08U limb, so > msb is all ones and precision is negative (so it reduced precision from > 161 to 192 bits down to 160 bits, in theory could go as low as 129 bits > but that wouldn't change anything on the following behavior). > But still iprec is negative, -160 here. > For that case (i.e. where we are dealing with an negative input), the > code was using 65 - __builtin_clzll (~msb) to compute how many relevant > bits we have from the msb. Unfortunately that invokes UB for msb all ones. > The right number of relevant bits in that case is 1 though (like for > -2 it is 2 and -4 or -3 3 as already computed) - all we care about from that > is that the most significant bit is set (i.e. the number is negative) and > the bits below that should be supplied from the limbs below. > > So, the following patch fixes it by special casing it not to invoke UB. > > For msb 0 we already have a special case from before (but that is also > different because msb 0 implies the whole number is 0 given the way > bitint_reduce_prec works - even if we have limbs like ..., 0x8000U, 0U > the reduction can skip the most significant limb and msb then would be > the one below it), so if iprec > 0, we already don't call __builtin_clzll > on 0. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Richard. > 2024-02-13 Jakub Jelinek > > * soft-fp/bitint.h (FP_FROM_BITINT): If iprec < 0 and msb is all ones, > just set n to 1 instead of using __builtin_clzll (~msb). > > --- libgcc/soft-fp/bitint.h.jj2024-01-12 22:06:06.248661862 +0100 > +++ libgcc/soft-fp/bitint.h 2024-02-12 18:56:42.947974875 +0100 > @@ -276,7 +276,11 @@ bitint_negate (UBILtype *d, const UBILty > } \ >if (iprec < 0) \ > { \ > - n = sizeof (0ULL) * __CHAR_BIT__ + 1 - __builtin_clzll (~msb);\ > + if (msb == (UBILtype) -1) \ > + n = 1; \ > + else \ > + n = (sizeof (0ULL) * __CHAR_BIT__ + 1 \ > + - __builtin_clzll (~msb)); \ > if (BIL_TYPE_SIZE > DI##_BITS && n > DI##_BITS) \ > { \ > iv = msb >> (n - DI##_BITS); \ > > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Re: [PATCH] hwint: Fix up preprocessor conditions for GCC_PRISZ/fmt_size_t
On Tue, 13 Feb 2024, Jakub Jelinek wrote: > Hi! > > Using unsigned long long int for fmt_size_t and "ll" for GCC_PRISZ > as broke the gengtype on i686-linux before the libiberty fix is certainly > unexpected. size_t is there unsigned int, so expected fmt_size_t is > unsigned int (or some other 32-bit type). > > The problem was that I was comparing SIZE_MAX against signed maxima, > but SIZE_MAX is unsigned maximum. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok > for trunk? OK. > 2024-02-13 Jakub Jelinek > > * hwint.h (GCC_PRISZ, fmt_size_t): Fix preprocessor conditions, > instead of comparing SIZE_MAX against INT_MAX and LONG_MAX compare > it against UINT_MAX and ULONG_MAX. > > --- gcc/hwint.h.jj2024-02-09 11:59:17.444974906 +0100 > +++ gcc/hwint.h 2024-02-12 18:53:51.287281199 +0100 > @@ -120,10 +120,10 @@ typedef HOST_WIDE_INT __gcc_host_wide_in > So, instead of doing fprintf ("%zu\n", sizeof (x) * y); use > fprintf (HOST_SIZE_T_PRINT_UNSIGNED "\n", > (fmt_size_t) (sizeof (x) * y)); */ > -#if SIZE_MAX <= INT_MAX > +#if SIZE_MAX <= UINT_MAX > # define GCC_PRISZ "" > # define fmt_size_t unsigned int > -#elif SIZE_MAX <= LONG_MAX > +#elif SIZE_MAX <= ULONG_MAX > # define GCC_PRISZ HOST_LONG_FORMAT > # define fmt_size_t unsigned long int > #else > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Re: GCN RDNA2+ vs. GCC vectorizer "Reduce using vector shifts" (was: [committed] amdgcn: add -march=gfx1030 EXPERIMENTAL)
On Mon, 12 Feb 2024, Thomas Schwinge wrote: > Hi! > > On 2023-10-20T12:51:03+0100, Andrew Stubbs wrote: > > I've committed this patch > > ... as commit c7ec7bd1c6590cf4eed267feab490288e0b8d691 > "amdgcn: add -march=gfx1030 EXPERIMENTAL". > > The RDNA2 ISA variant doesn't support certain instructions previous > implemented in GCC/GCN, so a number of patterns etc. had to be disabled: > > > [...] Vector > > reductions will need to be reworked for RDNA2. [...] > > > * config/gcn/gcn-valu.md (@dpp_move): Disable for RDNA2. > > (addc3): Add RDNA2 syntax variant. > > (subc3): Likewise. > > (2_exec): Add RDNA2 alternatives. > > (vec_cmpdi): Likewise. > > (vec_cmpdi): Likewise. > > (vec_cmpdi_exec): Likewise. > > (vec_cmpdi_exec): Likewise. > > (vec_cmpdi_dup): Likewise. > > (vec_cmpdi_dup_exec): Likewise. > > (reduc__scal_): Disable for RDNA2. > > (*_dpp_shr_): Likewise. > > (*plus_carry_dpp_shr_): Likewise. > > (*plus_carry_in_dpp_shr_): Likewise. > > Etc. The expectation being that GCC middle end copes with this, and > synthesizes some less ideal yet still functional vector code, I presume. > > The later RDNA3/gfx1100 support builds on top of this, and that's what > I'm currently working on getting proper GCC/GCN target (not offloading) > results for. > > I'm seeing a good number of execution test FAILs (regressions compared to > my earlier non-gfx1100 testing), and I've now tracked down where one > large class of those comes into existance -- not yet how to resolve, > unfortunately. But maybe, with you guys' combined vectorizer and back > end experience, the latter will be done quickly? > > Richard, I don't know if you've ever run actual GCC/GCN target (not > offloading) testing; let me know if you have any questions about that. I've only done offload testing - in the x86_64 build tree run check-target-libgomp. If you can tell me how to do GCN target testing (maybe document it on the wiki even!) I can try do that as well. > Given that (at least largely?) the same patterns etc. are disabled as in > my gfx1100 configuration, I suppose your gfx1030 one would exhibit the > same issues. You can build GCC/GCN target like you build the offloading > one, just remove '--enable-as-accelerator-for=[...]'. Likely, you can > even use a offloading GCC/GCN build to reproduce the issue below. > > One example is the attached 'builtin-bitops-1.c', reduced from > 'gcc.c-torture/execute/builtin-bitops-1.c', where 'my_popcount' is > miscompiled as soon as '-ftree-vectorize' is effective: > > $ build-gcc/gcc/xgcc -Bbuild-gcc/gcc/ builtin-bitops-1.c > -Bbuild-gcc/amdgcn-amdhsa/gfx1100/newlib/ > -Lbuild-gcc/amdgcn-amdhsa/gfx1100/newlib -fdump-tree-all-all > -fdump-ipa-all-all -fdump-rtl-all-all -save-temps -march=gfx1100 -O1 > -ftree-vectorize > > In the 'diff' of 'a-builtin-bitops-1.c.179t.vect', for example, for > '-march=gfx90a' vs. '-march=gfx1100', we see: > > +builtin-bitops-1.c:7:17: missed: reduc op not supported by target. > > ..., and therefore: > > -builtin-bitops-1.c:7:17: note: Reduce using direct vector reduction. > +builtin-bitops-1.c:7:17: note: Reduce using vector shifts > +builtin-bitops-1.c:7:17: note: extract scalar result > > That is, instead of one '.REDUC_PLUS' for gfx90a, for gfx1100 we build a > chain of summation of 'VEC_PERM_EXPR's. However, there's wrong code > generated: > > $ flock /tmp/gcn.lock build-gcc/gcc/gcn-run a.out > i=1, ints[i]=0x1 a=1, b=2 > i=2, ints[i]=0x8000 a=1, b=2 > i=3, ints[i]=0x2 a=1, b=2 > i=4, ints[i]=0x4000 a=1, b=2 > i=5, ints[i]=0x1 a=1, b=2 > i=6, ints[i]=0x8000 a=1, b=2 > i=7, ints[i]=0xa5a5a5a5 a=16, b=32 > i=8, ints[i]=0x5a5a5a5a a=16, b=32 > i=9, ints[i]=0xcafe a=11, b=22 > i=10, ints[i]=0xcafe00 a=11, b=22 > i=11, ints[i]=0xcafe a=11, b=22 > i=12, ints[i]=0x a=32, b=64 > > (I can't tell if the 'b = 2 * a' pattern is purely coincidental?) > > I don't speak enough "vectorization" to fully understand the generic > vectorized algorithm and its implementation. It appears that the > "Reduce using vector shifts" code has been around for a very long time, > but also has gone through a number of changes. I can't tell which GCC > targets/configurations it's actually used for (in the same way as for > GCN gfx1100), and thus whether there's an issue in that vectorizer code, > or rather in the GCN back end, or GCN back end parameterizing the generic > code? The "shift" reduction is basically doing reduction by repeatedly adding the upper to the lower half of the vector (each time halving the vector size). > Manually working through the 'a-builtin-bitops-1.c.265t.optimized' code: > > int my_popcount (unsigned int x) > { > int stmp__12.12; > vector(64) int vect__12.11; > vector(64) unsigned int vect__1.8; > vector(64) unsigned int _13; > vector(64) unsigned
[PATCH] Add %[zt][diox] tests to gcc.dg/format/
On Mon, Feb 12, 2024 at 04:10:33PM +, Joseph Myers wrote: > On Sat, 10 Feb 2024, Jakub Jelinek wrote: > > > * c-format.cc (gcc_diag_length_specs): Add t and z modifiers. > > (PP_FORMAT_CHAR_TABLE, gcc_gfc_char_table): Add entries for t and > > z modifiers. > > Please also add some tests of format checking for these modifiers in > gcc.dg/format/gcc_*.c. The following patch does that. Haven't added tests for bad type (but I think we don't have them in c99-printf* either) for these because it is hard to figure out what type from {,unsigned }{int,long,long long} size_t/ptrdiff_t certainly is not, guess one could do that with preprocessor conditionals, e.g. comparing __PTRDIFF_MAX__ with __INT_MAX__, __LONG_MAX__ and __LONG_LONG_MAX__ and pick up the one which is different; but we'd need to find out corresponding effective targets for the expected diagnostics. Tested on x86_64-linux and i686-linux, ok for trunk? 2024-02-13 Jakub Jelinek * gcc.dg/format/gcc_diag-1.c (foo): Add tests for z and t modifiers. * gcc.dg/format/gcc_gfc-1.c (foo): Add tests for ll, z and t modifiers. --- gcc/testsuite/gcc.dg/format/gcc_diag-1.c.jj 2020-01-12 11:54:37.423398171 +0100 +++ gcc/testsuite/gcc.dg/format/gcc_diag-1.c2024-02-12 19:41:55.512559836 +0100 @@ -33,7 +33,8 @@ foo (int i, int i1, int i2, unsigned int ullong ull, unsigned int *un, const int *cn, signed char *ss, unsigned char *us, const signed char *css, unsigned int u1, unsigned int u2, location_t *loc, tree t1, union tree_node *t2, - tree *t3, tree t4[], int *v, unsigned v_len) + tree *t3, tree t4[], int *v, unsigned v_len, size_t sz, ptrdiff_t pd, + ssize_t ssz, unsigned_ptrdiff_t upd) { /* Acceptable C90 specifiers, flags and modifiers. */ diag ("%%"); @@ -66,6 +67,16 @@ foo (int i, int i1, int i2, unsigned int cdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull); cxxdiag ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull); dump ("%wd%wi%wo%wu%wx", ll, ll, ull, ull, ull); + diag ("%zd%zi%zo%zu%zx", ssz, ssz, sz, sz, sz); + tdiag ("%zd%zi%zo%zu%zx", ssz, ssz, sz, sz, sz); + cdiag ("%zd%zi%zo%zu%zx", ssz, ssz, sz, sz, sz); + cxxdiag ("%zd%zi%zo%zu%zx", ssz, ssz, sz, sz, sz); + dump ("%zd%zi%zo%zu%zx", ssz, ssz, sz, sz, sz); + diag ("%td%ti%to%tu%tx", pd, pd, upd, upd, upd); + tdiag ("%td%ti%to%tu%tx", pd, pd, upd, upd, upd); + cdiag ("%td%ti%to%tu%tx", pd, pd, upd, upd, upd); + cxxdiag ("%td%ti%to%tu%tx", pd, pd, upd, upd, upd); + dump ("%td%ti%to%tu%tx", pd, pd, upd, upd, upd); diag ("%.*s", i, s); tdiag ("%.*s", i, s); cdiag ("%.*s", i, s); --- gcc/testsuite/gcc.dg/format/gcc_gfc-1.c.jj 2020-01-11 16:31:55.136291982 +0100 +++ gcc/testsuite/gcc.dg/format/gcc_gfc-1.c 2024-02-12 19:46:26.139775018 +0100 @@ -12,13 +12,19 @@ extern int gfc_warn (const char *, ...) void foo (unsigned int u, int i, char *s, unsigned long int ul, long int l, - llong ll, locus *loc) + llong ll, unsigned long long int ull, locus *loc, size_t sz, + ptrdiff_t pd, ssize_t ssz, unsigned_ptrdiff_t upd) { /* Acceptable C90 specifiers, flags and modifiers. */ gfc_warn ("%%"); gfc_warn ("%u%d%i%c%s%%", u, i, i, i, s); gfc_warn ("%lu%ld%li%%", ul, l, l); + /* Acceptable C99 specifiers, flags and modifiers. */ + gfc_warn ("%llu%lld%lli%%", ull, ll, ll); + gfc_warn ("%zu%zd%zi%%", sz, ssz, ssz); + gfc_warn ("%tu%td%ti%%", upd, pd, pd); + /* Extensions provided in gfc_warn. */ gfc_warn ("%C"); gfc_warn ("%L", loc); Jakub
[PATCH] pretty-print: Fix up ptrdiff handling for %tu, %to, %tx
Hi! This is IMHO more of a theoretical case, I believe my current code doesn't handle %tu or %to or %tx correctly if ptrdiff_t is not one of int, long int or long long int. For size_t and %zd or %zi I'm using va_arg (whatever, ssize_t) and hope that ssize_t is the signed type corresponding to size_t which C99 talks about. For ptrdiff_t there is no type for unsigned type corresponding to ptrdiff_t and I'm not aware of a portable way on the host to get such a type (the fmt tests use hacks like #define signed /* Type might or might not have explicit 'signed'. */ typedef unsigned __PTRDIFF_TYPE__ unsigned_ptrdiff_t; #undef signed but that only works with compilers which predefine __PTRDIFF_TYPE__), std::make_unsigned I believe only works reliably if ptrdiff_t is one of char, signed char, short, int, long or long long, but won't work e.g. for __int20__ or whatever other weird ptrdiff_t the host might have. The following patch assumes host is two's complement (I think we rely on it pretty much everywhere anyway) and prints unsigned type corresponding to ptrdiff_t as unsigned long long printing of ptrdiff_t value masked with 2ULL * PTRDIFF_MAX + 1. So the only actual limitation is that it doesn't support ptrdiff_t wider than long long int. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-13 Jakub Jelinek * pretty-print.cc (PTRDIFF_MAX): Define if not yet defined. (pp_integer_with_precision): For unsigned ptrdiff_t printing with u, o or x print ptrdiff_t argument converted to unsigned long long and masked with 2ULL * PTRDIFF_MAX + 1. * error.cc (error_print): For u printing of ptrdiff_t, print ptrdiff_t argument converted to unsigned long long and masked with 2ULL * PTRDIFF_MAX + 1. --- gcc/pretty-print.cc.jj 2024-02-12 12:44:09.344335964 +0100 +++ gcc/pretty-print.cc 2024-02-12 19:09:27.594483176 +0100 @@ -752,6 +752,9 @@ output_buffer::~output_buffer () obstack_free (_obstack, NULL); } +#ifndef PTRDIFF_MAX +#define PTRDIFF_MAX INTTYPE_MAXIMUM (ptrdiff_t) +#endif /* Format an integer given by va_arg (ARG, type-specifier T) where type-specifier is a precision modifier as indicated by PREC. F is @@ -783,7 +786,15 @@ output_buffer::~output_buffer () break; \ \ case 4:\ -if (sizeof (ptrdiff_t) <= sizeof (int)) \ +if (T (-1) >= T (0)) \ + { \ +unsigned long long a = va_arg (ARG, ptrdiff_t); \ +unsigned long long m = PTRDIFF_MAX; \ +m = 2 * m + 1; \ +pp_scalar (PP, "%" HOST_LONG_LONG_FORMAT F, \ + a & m); \ + } \ +else if (sizeof (ptrdiff_t) <= sizeof (int)) \ pp_scalar (PP, "%" F, \ (int) va_arg (ARG, ptrdiff_t)); \ else if (sizeof (ptrdiff_t) <= sizeof (long))\ --- gcc/fortran/error.cc.jj 2024-02-12 12:44:09.343335978 +0100 +++ gcc/fortran/error.cc2024-02-12 19:11:23.564886530 +0100 @@ -886,13 +886,14 @@ error_print (const char *type, const cha format++; if (*format == 'u') { - ptrdiff_t ptrdiffval = spec[n++].u.ptrdiffval; - if (sizeof (ptrdiff_t) == sizeof (int)) - error_uinteger ((unsigned) ptrdiffval); - else if (sizeof (ptrdiff_t) == sizeof (long)) - error_uinteger ((unsigned long) ptrdiffval); - else - error_uinteger (ptrdiffval); + unsigned long long a = spec[n++].u.ptrdiffval, m; +#ifdef PTRDIFF_MAX + m = PTRDIFF_MAX; +#else + m = INTTYPE_MAXIMUM (ptrdiff_t); +#endif + m = 2 * m + 1; + error_uinteger (a & m); } else error_integer (spec[n++].u.ptrdiffval); Jakub
[PATCH] libgcc: Fix UB in FP_FROM_BITINT
Hi! As I wrote earlier, I was seeing FAIL: gcc.dg/torture/bitint-24.c -O0 execution test FAIL: gcc.dg/torture/bitint-24.c -O2 execution test with the ia32 _BitInt enablement patch on i686-linux. I thought floatbitintxf.c was miscompiled with -O2 -march=i686 -mtune=generic, but it turned out to be UB in it. If a signed _BitInt to be converted to binary floating point has (after sign extension from possible partial limb to full limb) one or more most significant limbs equal to all ones and then in the limb below (the most significant non-~(UBILtype)0 limb) has the most significant limb cleared, like for 32-bit limbs 0x81582c05U, 0x0a8b01e4U, 0xc1b8b18fU, 0x2aac2a08U, -1U, -1U then bitint_reduce_prec can't reduce it to that 0x2aac2a08U limb, so msb is all ones and precision is negative (so it reduced precision from 161 to 192 bits down to 160 bits, in theory could go as low as 129 bits but that wouldn't change anything on the following behavior). But still iprec is negative, -160 here. For that case (i.e. where we are dealing with an negative input), the code was using 65 - __builtin_clzll (~msb) to compute how many relevant bits we have from the msb. Unfortunately that invokes UB for msb all ones. The right number of relevant bits in that case is 1 though (like for -2 it is 2 and -4 or -3 3 as already computed) - all we care about from that is that the most significant bit is set (i.e. the number is negative) and the bits below that should be supplied from the limbs below. So, the following patch fixes it by special casing it not to invoke UB. For msb 0 we already have a special case from before (but that is also different because msb 0 implies the whole number is 0 given the way bitint_reduce_prec works - even if we have limbs like ..., 0x8000U, 0U the reduction can skip the most significant limb and msb then would be the one below it), so if iprec > 0, we already don't call __builtin_clzll on 0. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-13 Jakub Jelinek * soft-fp/bitint.h (FP_FROM_BITINT): If iprec < 0 and msb is all ones, just set n to 1 instead of using __builtin_clzll (~msb). --- libgcc/soft-fp/bitint.h.jj 2024-01-12 22:06:06.248661862 +0100 +++ libgcc/soft-fp/bitint.h 2024-02-12 18:56:42.947974875 +0100 @@ -276,7 +276,11 @@ bitint_negate (UBILtype *d, const UBILty } \ if (iprec < 0) \ { \ - n = sizeof (0ULL) * __CHAR_BIT__ + 1 - __builtin_clzll (~msb);\ + if (msb == (UBILtype) -1) \ + n = 1; \ + else \ + n = (sizeof (0ULL) * __CHAR_BIT__ + 1 \ +- __builtin_clzll (~msb)); \ if (BIL_TYPE_SIZE > DI##_BITS && n > DI##_BITS) \ { \ iv = msb >> (n - DI##_BITS); \ Jakub