[PATCH v2] [testsuite] [powerpc] adjust -m32 counts for fold-vec-extract*
Ping?-ish https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619678.html It's that time of the year again. The good news is that this is the last patch in my ppc*-vxworks7* set ;-) On May 25, 2023, Segher Boessenkool wrote: > On Thu, May 25, 2023 at 10:55:37AM -0300, Alexandre Oliva wrote: >> I've actually identified the corresponding change to the >> lp64 tests, compared the effects of the codegen changes, and concluded >> the tests needed this changing for ilp32 to keep on testing for the same >> thing after code changes brought about by changes that AFAICT had been >> well understood when making the lp64 adjustments. >> /* -m32 target has an 'add' in place of one of the 'addi'. */ >> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } >> } */ >> -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } >> } } */ >> +/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 } } */ > Just {\madd} or more conservative {\maddi?\M} then? I've made these changes in the v2 below. Codegen changes caused add instruction count mismatches on ppc-*-linux-gnu and other 32-bit ppc targets. At some point the expected counts were adjusted for lp64, but ilp32 differences remained, and published test results confirm it. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog PR testsuite/101169 * gcc.target/powerpc/fold-vec-extract-double.p7.c: Adjust addi counts for ilp32. * gcc.target/powerpc/fold-vec-extract-float.p7.c: Likewise. * gcc.target/powerpc/fold-vec-extract-float.p8.c: Likewise. * gcc.target/powerpc/fold-vec-extract-int.p7.c: Likewise. * gcc.target/powerpc/fold-vec-extract-int.p8.c: Likewise. * gcc.target/powerpc/fold-vec-extract-short.p7.c: Likewise. * gcc.target/powerpc/fold-vec-extract-short.p8.c: Likewise. --- .../powerpc/fold-vec-extract-double.p7.c |5 ++--- .../gcc.target/powerpc/fold-vec-extract-float.p7.c |5 ++--- .../gcc.target/powerpc/fold-vec-extract-float.p8.c |2 +- .../gcc.target/powerpc/fold-vec-extract-int.p7.c |3 +-- .../gcc.target/powerpc/fold-vec-extract-int.p8.c |2 +- .../gcc.target/powerpc/fold-vec-extract-short.p7.c |3 +-- .../gcc.target/powerpc/fold-vec-extract-short.p8.c |2 +- 7 files changed, 9 insertions(+), 13 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c index 3cae644b90b71..e69d9253e2d28 100644 --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-double.p7.c @@ -13,12 +13,11 @@ /* { dg-final { scan-assembler-times {\mxxpermdi\M} 1 } } */ /* { dg-final { scan-assembler-times {\mli\M} 1 } } */ /* -m32 target has an 'add' in place of one of the 'addi'. */ -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */ -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */ +/* { dg-final { scan-assembler-times {\maddi?\M} 2 } } */ /* -m32 target has a rlwinm in place of a rldic . */ /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */ /* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */ -/* { dg-final { scan-assembler-times {\mlfdx\M|\mlfd\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mlfdx?\M} 1 } } */ #include diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c index 59a4979457dcb..9ff197a704906 100644 --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p7.c @@ -12,13 +12,12 @@ /* { dg-final { scan-assembler-times {\mxscvspdp\M} 1 } } */ /* { dg-final { scan-assembler-times {\mli\M} 1 } } */ /* -m32 as an add in place of an addi. */ -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 2 { target lp64 } } } */ -/* { dg-final { scan-assembler-times {\maddi\M|\madd\M} 3 { target ilp32 } } } */ +/* { dg-final { scan-assembler-times {\maddi?\M} 2 } } */ /* { dg-final { scan-assembler-times {\mstxvd2x\M|\mstvx\M|\mstxv\M} 1 } } */ /* -m32 uses rlwinm in place of rldic */ /* { dg-final { scan-assembler-times {\mrldic\M|\mrlwinm\M} 1 } } */ /* -m32 has lfs in place of lfsx */ -/* { dg-final { scan-assembler-times {\mlfsx\M|\mlfs\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mlfsx?\M} 1 } } */ #include diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c index ce4e43c1fb462..cd80c5e1b19c6 100644 --- a/gcc/testsuite/gcc.target/powerpc/fold-vec-extract-float.p8.c +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-ex
[PATCH] decay vect tests from run to link for pr95401
Ping?-ish for the full version of the RFC posted at https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566588.html On Mar 11, 2021, Richard Biener wrote: > On Thu, Mar 11, 2021 at 9:03 AM Alexandre Oliva wrote: >> So I'm leaning towards this proposed change, just extended to other >> platforms that also decay from run to compile rather than link, and thus >> run into this problem in g++.dg/vect/pr95401.cc. Would this be >> acceptable? > I think that's OK. It's probably difficult to make the test UNSUPPORTED > when dg-additional-sources is discovered with a dg-do compile test? Thanks, here's a completed version. When vect.exp finds our configuration disables altivec by default, it disables the execution of vectorization tests, assuming the test hardware doesn't support it. Tests become just compile tests, but compile tests won't work correctly when additional sources are named, e.g. pr95401.cc, because GCC refuses to compile multiple files into the same asm output. With this patch, the default for when execution is not possible becomes link. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * lib/target-supports.exp (check_vect_support_and_set_flags): Decay to link rather than compile. --- gcc/testsuite/lib/target-supports.exp | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 3a5713d98691f..54a55585371b0 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -11625,7 +11625,7 @@ proc check_vect_support_and_set_flags { } { if [check_750cl_hw_available] { set dg-do-what-default run } else { -set dg-do-what-default compile +set dg-do-what-default link } } elseif [istarget powerpc*-*-*] { # Skip targets not supporting -maltivec. @@ -11655,14 +11655,14 @@ proc check_vect_support_and_set_flags { } { # some other cpu type specified above. set DEFAULT_VECTCFLAGS [linsert $DEFAULT_VECTCFLAGS 0 "-mcpu=970"] } -set dg-do-what-default compile +set dg-do-what-default link } } elseif { [istarget i?86-*-*] || [istarget x86_64-*-*] } { lappend DEFAULT_VECTCFLAGS "-msse2" if { [check_effective_target_sse2_runtime] } { set dg-do-what-default run } else { -set dg-do-what-default compile +set dg-do-what-default link } } elseif { [istarget mips*-*-*] && [check_effective_target_nomips16] } { @@ -11681,7 +11681,7 @@ proc check_vect_support_and_set_flags { } { if [check_effective_target_ultrasparc_hw] { set dg-do-what-default run } else { -set dg-do-what-default compile +set dg-do-what-default link } } elseif [istarget alpha*-*-*] { # Alpha's vectorization capabilities are extremely limited. @@ -11694,7 +11694,7 @@ proc check_vect_support_and_set_flags { } { if [check_alpha_max_hw_available] { set dg-do-what-default run } else { -set dg-do-what-default compile +set dg-do-what-default link } } elseif [istarget ia64-*-*] { set dg-do-what-default run @@ -11707,7 +11707,7 @@ proc check_vect_support_and_set_flags { } { if [is-effective-target arm_neon_hw] { set dg-do-what-default run } else { -set dg-do-what-default compile +set dg-do-what-default link } } elseif [istarget aarch64*-*-*] { set dg-do-what-default run @@ -11731,7 +11731,7 @@ proc check_vect_support_and_set_flags { } { set dg-do-what-default run } else { lappend DEFAULT_VECTCFLAGS "-march=z14" "-mzarch" -set dg-do-what-default compile +set dg-do-what-default link } } elseif [istarget amdgcn-*-*] { set dg-do-what-default run @@ -11742,7 +11742,7 @@ proc check_vect_support_and_set_flags { } { foreach item [add_options_for_riscv_v ""] { lappend DEFAULT_VECTCFLAGS $item } - set dg-do-what-default compile + set dg-do-what-default link } } elseif [istarget loongarch*-*-*] { # Set the default vectorization option to "-mlsx" due to the problem @@ -11751,7 +11751,7 @@ proc check_vect_support_and_set_flags { } { if [check_effective_target_loongarch_sx_hw] { set dg-do-what-default run } else { - set dg-do-what-default compile + set dg-do-what-default link } } else { return 0
[PATCH v2] xfail fetestexcept test - ppc always uses fcmpu
On Mar 10, 2021, Joseph Myers wrote: > On Wed, 10 Mar 2021, Alexandre Oliva wrote: >> operand exception for quiet NaN. I couldn't find any evidence that >> the rs6000 backend ever outputs fcmpo. Therefore, I'm adding the same >> execution xfail marker to this test. > In my view, such an XFAIL (for a GCC bug as opposed to an environmental > issue) should have a comment pointing to a corresponding open bug in GCC > Bugzilla. In this case, that's bug 58684. Thanks for the suggestion, yeah, that makes sense. Fixed in v2 below. https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566523.html Ping?-ish gcc.dg/torture/pr91323.c tests that a compare with NaNf doesn't set an exception using builtin compare intrinsics, and that it does when using regular compare operators. That doesn't seem to be expected to work on powerpc targets. It fails on GNU/Linux, it's marked to be skipped on AIX, and a similar test, gcc.dg/torture/pr93133.c, has the execution test xfailed for all of powerpc*-*-*. In this test, the functions that use intrinsics for the compare end up with the same code as the one that uses compare operators, using fcmpu, a floating compare that, unlike fcmpo, does not set the invalid operand exception for quiet NaN. I couldn't find any evidence that the rs6000 backend ever outputs fcmpo. Therefore, I'm adding the same execution xfail marker to this test. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog PR target/58684 * gcc.dg/torture/pr91323.c: Expect execution fail on powerpc*-*-*. --- gcc/testsuite/gcc.dg/torture/pr91323.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/torture/pr91323.c b/gcc/testsuite/gcc.dg/torture/pr91323.c index 1411fcaa3966c..f188faa3ccf47 100644 --- a/gcc/testsuite/gcc.dg/torture/pr91323.c +++ b/gcc/testsuite/gcc.dg/torture/pr91323.c @@ -1,4 +1,5 @@ -/* { dg-do run } */ +/* { dg-do run { xfail powerpc*-*-* } } */ +/* The ppc xfail is because of PR target/58684. */ /* { dg-add-options ieee } */ /* { dg-require-effective-target fenv_exceptions } */ /* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH v2] [testsuite] require sqrt_insn effective target where needed
This patch takes feedback received for 3 earlier patches, and adopts a simpler approach to skip the still-failing tests, that I believe to be in line with ppc maintainers' expressed preferences. https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565939.html https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566617.html https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566521.html Ping?-ish :-) Some tests fail on ppc and ppc64 when testing a compiler [with options for] for a CPU [emulator] that doesn't support the sqrt insn. The gcc.dg/cdce3.c is one in which the expected shrink-wrap optimization only takes place when the target CPU supports a sqrt insn. The gcc.target/powerpc/pr46728-1[0-4].c tests use -mpowerpc-gpopt and call sqrt(), which involves the sqrt insn that the target CPU under test may not support. Require a sqrt_insn effective target for all the affected tests. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also testing with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/cdce3.c: Require sqrt_insn effective target. * gcc.target/powerpc/pr46728-10.c: Likewise. * gcc.target/powerpc/pr46728-11.c: Likewise. * gcc.target/powerpc/pr46728-13.c: Likewise. * gcc.target/powerpc/pr46728-14.c: Likewise. --- gcc/testsuite/gcc.dg/cdce3.c |3 ++- gcc/testsuite/gcc.target/powerpc/pr46728-10.c |1 + gcc/testsuite/gcc.target/powerpc/pr46728-11.c |1 + gcc/testsuite/gcc.target/powerpc/pr46728-13.c |1 + gcc/testsuite/gcc.target/powerpc/pr46728-14.c |1 + 5 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c index 601ddf055fd71..f759a95972e8b 100644 --- a/gcc/testsuite/gcc.dg/cdce3.c +++ b/gcc/testsuite/gcc.dg/cdce3.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ /* { dg-require-effective-target hard_float } */ +/* { dg-require-effective-target sqrt_insn } */ /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */ -/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */ +/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */ /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */ /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */ diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c index 3be4728d333a4..7e9bb638106c2 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr46728-10.c +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-10.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */ /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */ +/* { dg-require-effective-target sqrt_insn } */ #include diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c index 43b6728a4b812..5bfa25925675a 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr46728-11.c +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-11.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */ /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */ +/* { dg-require-effective-target sqrt_insn } */ #include diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c index b9fd63973b728..b66d0209a5e54 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr46728-13.c +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-13.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */ /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */ +/* { dg-require-effective-target sqrt_insn } */ #include diff --git a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c index 5a13bdb6c..71a1a70c4e7a2 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr46728-14.c +++ b/gcc/testsuite/gcc.target/powerpc/pr46728-14.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-skip-if "-mpowerpc-gpopt not supported" { powerpc*-*-darwin* } } */ /* { dg-options "-O2 -ffast-math -fno-inline -fno-unroll-loops -lm -mpowerpc-gpopt" } */ +/* { dg-require-effective-target sqrt_insn } */ #include -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: enable sqrt insns for cdce3.c
[Revamped version of this patch, combined with others, to follow] On Mar 10, 2021, Hans-Peter Nilsson wrote: > On Wed, 10 Mar 2021, Alexandre Oliva wrote: >> >> The test expects shrink-wrapping of the fsqrt call, but that will only >> occur when there is a usable sqrt insn. >> >> Arrange for dejagnu to add the options that enable the sqrt insn, if >> one is available, and to skip the test otherwise. >> >> >> H-P, this *should* obviate the mmix-specific dg-skip-if. > Unfortunately it doesn't. >> Would it be >> easy for you to confirm that this is the case and, if so, drop it? > About as easy as for anyone (this is a compile-test), but no > problem. Unfortunately I get, with your patch applied and the > dg-skip-if removed: > FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]* > function call is shrink-wrapped into error conditions." Is mmix a sqrt_insn effective target? proc check_effective_target_sqrt_insn in gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm surprised it would still try to run the test despite the added /* { dg-require-effective-target sqrt_insn } */ directive. > The dump files and assembly file show no obvious clues to me as > to what is supposed to happen; attached. cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like (x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root instruction. Since we don't know why it still runs for you, I'm keeping the mmix explicit skip in the new version of the patch. Thanks, -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH v2] add explicit ABI and align options to pr88233.c
Ping? https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566530.html (modified version follows) We've observed failures of this test on powerpc configurations that default to different calling conventions and alignment requirements. Both settings are needed for the original expectations to be met. The test was later modified to have different expectations for big and little endian code generation. This patch restores the original codegen expectations, that, with the explicit options, don't vary any more. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/powerpc/pr88233.c: Make some alignment strictness and calling conventions assumptions explicit. Restore uniform codegen expectations --- gcc/testsuite/gcc.target/powerpc/pr88233.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/pr88233.c b/gcc/testsuite/gcc.target/powerpc/pr88233.c index 27c73717a3f79..46a3ebfa28775 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr88233.c +++ b/gcc/testsuite/gcc.target/powerpc/pr88233.c @@ -1,5 +1,5 @@ /* { dg-require-effective-target lp64 } */ -/* { dg-options "-O2 -mdejagnu-cpu=power8" } */ +/* { dg-options "-O2 -mdejagnu-cpu=power8 -mno-strict-align -fpcc-struct-return" } */ typedef struct { double a[2]; } A; A @@ -9,6 +9,5 @@ foo (const A *a) } /* { dg-final { scan-assembler-not {\mmtvsr} } } */ -/* { dg-final { scan-assembler-times {\mlxvd2x\M} 1 { target { be } } } } */ -/* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 { target { be } } } } */ -/* { dg-final { scan-assembler-times {\mlfd\M} 2 { target { le } } } } */ +/* { dg-final { scan-assembler-times {\mlxvd2x\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] ppc: testsuite: vec-mul requires vsx runtime
Ping? https://gcc.gnu.org/pipermail/gcc-patches/2022-May/593947.html vec-mul is an execution test, but it only requires a powerpc_vsx_ok effective target, which is enough only for compile tests. In order to To check for runtime and execution environment support, we need to require vsx_hw. Make that a condition for execution, but still perform a compile test if the condition is not satisfied. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/powerpc/vec-mul.c: Run on target vsx_hw, just compile otherwise. --- gcc/testsuite/gcc.target/powerpc/vec-mul.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/powerpc/vec-mul.c b/gcc/testsuite/gcc.target/powerpc/vec-mul.c index bfcaf80719d1d..11da86159723f 100644 --- a/gcc/testsuite/gcc.target/powerpc/vec-mul.c +++ b/gcc/testsuite/gcc.target/powerpc/vec-mul.c @@ -1,4 +1,5 @@ -/* { dg-do run } */ +/* { dg-do compile { target { ! vsx_hw } } } */ +/* { dg-do run { target vsx_hw } } */ /* { dg-require-effective-target powerpc_vsx_ok } */ /* { dg-options "-mvsx -O3" } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] Request check for hw support in ppc run tests with -maltivec/-mvsx
From: Olivier Hainque Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/powerpc/swaps-p8-20.c: Change powerpc_altivec_ok require-effective-target test into vmx_hw. * gcc.target/powerpc/vsx-vector-5.c: Change powerpc_vsx_ok require-effective-target test into vsx_hw. --- gcc/testsuite/gcc.target/powerpc/swaps-p8-20.c |2 +- gcc/testsuite/gcc.target/powerpc/vsx-vector-5.c |5 + 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/swaps-p8-20.c b/gcc/testsuite/gcc.target/powerpc/swaps-p8-20.c index 564e8acb1f421..755519bfe847d 100644 --- a/gcc/testsuite/gcc.target/powerpc/swaps-p8-20.c +++ b/gcc/testsuite/gcc.target/powerpc/swaps-p8-20.c @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-require-effective-target vmx_hw } */ /* { dg-options "-O2 -mdejagnu-cpu=power8 -maltivec" } */ /* The expansion for vector character multiply introduces a vperm operation. diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-vector-5.c b/gcc/testsuite/gcc.target/powerpc/vsx-vector-5.c index dcc88b1f3a4c6..37a324b6f897d 100644 --- a/gcc/testsuite/gcc.target/powerpc/vsx-vector-5.c +++ b/gcc/testsuite/gcc.target/powerpc/vsx-vector-5.c @@ -1,11 +1,8 @@ /* { dg-do run { target lp64 } } */ /* { dg-skip-if "" { powerpc*-*-darwin* } } */ -/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-require-effective-target vsx_hw } */ /* { dg-options "-mvsx -O2" } */ -/* This will run, and someday we should add the support to test whether we are - running on VSX hardware. */ - #include #include -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] adjust vectorization expectations for ppc costmodel 76b
Ping? https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566525.html This test expects vectorization at power8+ because strict alignment is not required for vectors. For power7, vectorization is not to take place because it's not deemed profitable: 12 iterations would be required to make it so. But for power6 and below, the test's 10 iterations are enough to make vectorization profitable, but the test doesn't expect this. Assuming the decision is indeed appropriate, I'm adjusting the expectations. for gcc/testsuite/ChangeLog * gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c: Adjust expectations for cpus below power7. --- .../gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c index cbbfbb24658f8..0dab2c08acdb4 100644 --- a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c +++ b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-76b.c @@ -46,9 +46,10 @@ int main (void) return 0; } -/* Peeling to align the store is used. Overhead of peeling is too high. */ -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 0 "vect" { target { vector_alignment_reachable && {! vect_no_align} } } } } */ -/* { dg-final { scan-tree-dump-times "vectorization not profitable" 1 "vect" { target { vector_alignment_reachable && {! vect_hw_misalign} } } } } */ +/* Peeling to align the store is used. Overhead of peeling is too high + for power7, but acceptable for earlier architectures. */ +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 0 "vect" { target { has_arch_pwr7 && { vector_alignment_reachable && {! vect_no_align} } } } } } */ +/* { dg-final { scan-tree-dump-times "vectorization not profitable" 1 "vect" { target { has_arch_pwr7 && { vector_alignment_reachable && {! vect_hw_misalign} } } } } } */ /* Versioning to align the store is used. Overhead of versioning is not too high. */ -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_no_align || {! vector_alignment_reachable} } } } } */ +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_no_align || { {! vector_alignment_reachable} || {! has_arch_pwr7 } } } } } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] disable ldist for test, to restore vectorizing-candidate loop
Ping? https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566524.html The loop we're supposed to try to vectorize in gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c is turned into a memset before the vectorizer runs. Various other tests in this set have already run into this, and the solution has been to disable this loop distribution transformation, enabled at -O2, so that the vectorizer gets a chance to transform the loop and, in this testcase, fail to do so. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c: Disable ldist. --- .../gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c |1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c index 454a714a30916..90b5d5a7f400b 100644 --- a/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c +++ b/gcc/testsuite/gcc.dg/vect/costmodel/ppc/costmodel-vect-31a.c @@ -1,4 +1,5 @@ /* { dg-require-effective-target vect_int } */ +/* { dg-additional-options "-fno-tree-loop-distribute-patterns" } */ #include #include "../../tree-vect.h" -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [ppc64] expect error on vxworks too
These ppc lp64 tests check for errors or warnings on -mno-powerpc64. On powerpc64-*-vxworks* we get the same errors as on most other covered platforms, but the tests did not mark them as expected for this target. On powerpc-*-vxworks*, the tests are skipped because lp64 is not satisfied, so I'm naming powerpc*-*-vxworks* rather than something more specific. Regstrapped on x86_64-linux-gnu and ppc64el-linux-gnu. Also tested with gcc-13 on ppc64-vx7r2 and ppc-vx7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/powerpc/pr106680-1.c: Error on vxworks too. * gcc.target/powerpc/pr106680-2.c: Likewise. * gcc.target/powerpc/pr106680-3.c: Likewise. --- gcc/testsuite/gcc.target/powerpc/pr106680-1.c |2 +- gcc/testsuite/gcc.target/powerpc/pr106680-2.c |2 +- gcc/testsuite/gcc.target/powerpc/pr106680-3.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/gcc.target/powerpc/pr106680-1.c b/gcc/testsuite/gcc.target/powerpc/pr106680-1.c index d624d43230a7a..aadaa614cfeba 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr106680-1.c +++ b/gcc/testsuite/gcc.target/powerpc/pr106680-1.c @@ -8,6 +8,6 @@ int foo () return 1; } -/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* } 0 } */ +/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* powerpc*-*-vxworks* } 0 } */ /* { dg-warning "'-m64' requires PowerPC64 architecture, enabling" "PR106680" { target powerpc*-*-darwin* } 0 } */ /* { dg-warning "'-maix64' requires PowerPC64 architecture remain enabled" "PR106680" { target powerpc*-*-aix* } 0 } */ diff --git a/gcc/testsuite/gcc.target/powerpc/pr106680-2.c b/gcc/testsuite/gcc.target/powerpc/pr106680-2.c index a9ed73726ef0c..f0758e303350a 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr106680-2.c +++ b/gcc/testsuite/gcc.target/powerpc/pr106680-2.c @@ -9,6 +9,6 @@ int foo () return 1; } -/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* } 0 } */ +/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* powerpc*-*-vxworks* } 0 } */ /* { dg-warning "'-m64' requires PowerPC64 architecture, enabling" "PR106680" { target powerpc*-*-darwin* } 0 } */ /* { dg-warning "'-maix64' requires PowerPC64 architecture remain enabled" "PR106680" { target powerpc*-*-aix* } 0 } */ diff --git a/gcc/testsuite/gcc.target/powerpc/pr106680-3.c b/gcc/testsuite/gcc.target/powerpc/pr106680-3.c index b642d5c7a008d..bca012e2cf663 100644 --- a/gcc/testsuite/gcc.target/powerpc/pr106680-3.c +++ b/gcc/testsuite/gcc.target/powerpc/pr106680-3.c @@ -8,6 +8,6 @@ int foo () return 1; } -/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* } 0 } */ +/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc*-*-freebsd* powerpc-*-rtems* powerpc*-*-vxworks* } 0 } */ /* { dg-warning "'-m64' requires PowerPC64 architecture, enabling" "PR106680" { target powerpc*-*-darwin* } 0 } */ /* { dg-warning "'-maix64' requires PowerPC64 architecture remain enabled" "PR106680" { target powerpc*-*-aix* } 0 } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH v2] [testsuite] [arm] add effective target and options for pacbti tests
.c index 79dd8010d2dab..a34bb0842b632 100644 --- a/gcc/testsuite/gcc.target/arm/bti-1.c +++ b/gcc/testsuite/gcc.target/arm/bti-1.c @@ -1,7 +1,8 @@ /* Check that GCC does bti instruction. */ /* { dg-do compile } */ -/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ -/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ +/* { dg-add-options arm_arch_v8_1m_main } */ +/* { dg-additional-options "-mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ int main (void) diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c index 33910563849a4..e5bc4d5543a8d 100644 --- a/gcc/testsuite/gcc.target/arm/bti-2.c +++ b/gcc/testsuite/gcc.target/arm/bti-2.c @@ -1,8 +1,9 @@ /* { dg-do compile } */ /* -Os to create jump table. */ /* { dg-options "-Os" } */ -/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ -/* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ +/* { dg-add-options arm_arch_v8_1m_main } */ +/* { dg-additional-options "-mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ extern int f1 (void); extern int f2 (void); -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
On Apr 16, 2024, "Richard Earnshaw (lists)" wrote: > The require-effective-target flags test whether a specific set of > flags will make the compilation work, so they need to be used in > conjunction with the corresponding dg-add-options flags that then > apply those options. *nod*, that's the theory. Problem is the architectures suported by [add_options_for_]arm_arch_*[_ok] do not match exactly those expected by the tests, and I can't quite tell whether the subtle changes they would introduce would change what they intend to test, or even whether the differences are irrelevant, or would be sensible to add as variants to the dg machinery. I think it would take someone more familiar than I am with all of the ARM variants to do this correctly. I don't even know how these changes would need to be tested to be sure they remain correct. Would you be willing to take it from here, or would you accept the patch as an incremental yet imperfect improvement, or would you prefer to guide me in making it correct, and in verifying it (there are questions below)? I don't have a lot of cycles to put into this (we've already worked around the testsuite bugs we ran into), but it would be desirable to get a fix into GCC as well, if we can converge on one without unreasonably burdening anyone. v8_1m_main "-march=armv8.1-m.main+fp -mthumb" __ARM_ARCH_8M_MAIN__ v8_1m_main_pacbti "-march=armv8.1-m.main+pacbti+fp -mthumb" "__ARM_ARCH_8M_MAIN__ && __ARM_FEATURE_BTI && __ARM_FEATURE_PAUTH Why do these have +fp in -march but not in the v8_1m* arch name? gcc/testsuite/g++.target/arm/pac-1.C: /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */ v8_1m_main_pacbti plus +mve minus +fp. Do we need a dg arch for that? gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c: /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */ gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c: /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */ v8_1m_main_pacbti minus -mthumb. AFAICT the -mthumb is redundant. gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c: /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */ v8_1m_main minus -mthumb. AFAICT the -mthumb is redundant. gcc/testsuite/gcc.target/arm/bti-1.c: /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ gcc/testsuite/gcc.target/arm/bti-2.c: /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ v8_1m_main minus +fp. Can these be bumped to +fp, or do we need an extra dg arch? Are these missing +pacbti? Thanks, -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [libstdc++] introduce --disable-compat-libstdcxx-abi
On Apr 16, 2024, Jonathan Wakely wrote: >> +dnl >> +dnl Enable -Wabi=2 if not overridden by --disable-compat-libstdcxx-abi. >> +dnl >> +AC_DEFUN([GLIBCXX_ENABLE_WABI], [ >> + # Default. >> + WARN_FLAGS_WABI=\ -Wabi=2 >> + AC_MSG_CHECKING([for --disable-compat-libstdcxx-abi]) >> + AC_ARG_ENABLE([compat-libstdcxx-abi], > We have the GLIBCXX_ENABLE macro to simplify creating new --enable options. *nod*. There was some reason why I didn't use it at first. Maybe it can be used with the patch as it ended up. Will revisit. >> +AC_HELP_STRING([--disable-compat-libstdcxx-abi], >> + [Disable backward-compatibility ABI symbols)]), > There's a stray ')' here. Ugh, thanks >> --- a/libstdc++-v3/doc/html/manual/configure.html >> +++ b/libstdc++-v3/doc/html/manual/configure.html > This should be in doc/xml/manual/configure.xml too, which is used to > generate the HTML using docbook. Oh, right. Doh. So much for grepping for an existing option and jumping to edit the first match :-) > The description here in the docs (and the name of the configure > option) seem much too vague. Libstdc++ has dozens, probably hundreds, > of "backward-compatibility ABI symbols", and this only affects touches > a tiny handful of them. Just the aliases created automatically by the > compiler for mangling changes, right? Yeah. I had used --disable-libstdcxx-Wabi at some point, maybe that's better. FTR, we now have a binutils patch (thanks H.J.Lu) to address the underlying problem, so we'll probably no longer need the workaround that led me to propose this change. I wonder if there's interest in keeping it. I'd be equally happy to make the adjustments, or to withdraw it (or pretty much anything in between ;-). WDYT? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [testsuite] introduce strndup effective target
On Apr 16, 2024, Alexandre Oliva wrote: > * gcc.dg/builtin-dynamic-object-size-1.c: Likewise. > * gcc.dg/builtin-dynamic-object-size-2.c: Likewise. > * gcc.dg/builtin-dynamic-object-size-3.c: Likewise. > * gcc.dg/builtin-dynamic-object-size-4.c: Likewise. These hunks were missing from the patch I posted, sorry. I goofed when resolving the conflicts because the tests had been modified after gcc-13. I hope the intent was clear from the ChangeLog entry. Here they are. diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c index ffa59985024f5..76b4f704fed9c 100644 --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c @@ -1,7 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ -/* { dg-skip-if "no strndup" { hppa*-*-hpux* } } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #define __builtin_object_size __builtin_dynamic_object_size #include "builtin-object-size-1.c" diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c index fff32da7aea14..cb757a8d699cf 100644 --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c @@ -1,7 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ -/* { dg-skip-if "no strndup" { hppa*-*-hpux* } } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #define __builtin_object_size __builtin_dynamic_object_size #include "builtin-object-size-2.c" diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c index ac223d67b10a4..8a12f023f27bc 100644 --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c @@ -1,7 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ -/* { dg-skip-if "no strndup" { hppa*-*-hpux* } } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #define __builtin_object_size __builtin_dynamic_object_size #include "builtin-object-size-3.c" diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c index fdf4284ae1158..0efc2d9858422 100644 --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c @@ -1,7 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ -/* { dg-skip-if "no strndup" { hppa*-*-hpux* } } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #define __builtin_object_size __builtin_dynamic_object_size #include "builtin-object-size-4.c" -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [libstdc++] [testsuite] xfail double-prec from_chars for float128_t
On Apr 16, 2024, Alexandre Oliva wrote: > * testsuite/20_util/to_chars/float128-c++23.cc: Xfail run on > aarch64-vxworks. FTR, here's the fixed ChangeLog entry I'm putting in: (s/-/_/) * testsuite/20_util/to_chars/float128_c++23.cc: Xfail run on aarch64-vxworks. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [testsuite] [arm] accept empty init for bfloat16
On Apr 16, 2024, Mike Stump wrote: > Indeed, I kinda expect coverage already for that feature in > another test case. *nod*, jsm added gcc.dg/c11-empty-init-[123].c (and more) in the patch that implemented this c23 feature. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [strub] improve handling of indirected volatile parms [PR112938]
On Apr 16, 2024, Alexandre Oliva wrote: > I'm going to put it in momentarily. It had been approved for gcc 14, > before it branched off; should I install it there as well? Ermh, nevermind, I'm not sure how I got the idea that we'd already branched, but I was absolutely sure that this was the case. Doh! :-) -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [tree-prof] skip if errors were seen [PR113681]
On Mar 29, 2024, Alexandre Oliva wrote: > On Mar 22, 2024, Jeff Law wrote: >> On 3/9/24 2:11 AM, Alexandre Oliva wrote: >>> ipa_tree_profile asserts that the symtab is in IPA_SSA state, but we >>> don't reach that state and ICE if e.g. ipa-strub passes report errors. >>> Skip this pass if errors were seen. >>> Regstrapped on x86_64-linux-gnu. Ok to install? >>> >>> for gcc/ChangeLog >>> PR tree-optimization/113681 >>> * tree-profiling.cc (pass_ipa_tree_profile::gate): Skip if >>> seen_errors. >>> for gcc/testsuite/ChangeLog >>> PR tree-optimization/113681 >>> * c-c++-common/strub-pr113681.c: New. >> So I've really never dug into strub, but this would seem to imply that >> an error from strub is non-fatal? > Yeah. I believe that's no different from other passes. > Various other passes have seen_errors guards, but ipa-prof didn't. Specifically, pass_build_ssa_passes in passes.cc is gated with !seen_errors(), so we skip all the passes bundled in it, and don't advance the symtab state to IPA_SSA. So other passes that would require IPA_SSA need to be gated similarly. > I suppose the insertion point for the strubm pass was one where others > passes didn't previously issue errors, so that wasn't an issue for > ipa-prof. But now it is. The patch needed adjustments to resolve conflicts with unrelated changes. [tree-prof] skip if errors were seen [PR113681] ipa_tree_profile asserts that the symtab is in IPA_SSA state, but we don't reach that state and ICE if e.g. ipa-strub passes report errors. Skip this pass if errors were seen. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog PR tree-optimization/113681 * tree-profiling.cc (pass_ipa_tree_profile::gate): Skip if seen_errors. for gcc/testsuite/ChangeLog PR tree-optimization/113681 * c-c++-common/strub-pr113681.c: New. --- gcc/testsuite/c-c++-common/strub-pr113681.c | 22 ++ gcc/tree-profile.cc |3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/c-c++-common/strub-pr113681.c diff --git a/gcc/testsuite/c-c++-common/strub-pr113681.c b/gcc/testsuite/c-c++-common/strub-pr113681.c new file mode 100644 index 0..3ef9017b2eb70 --- /dev/null +++ b/gcc/testsuite/c-c++-common/strub-pr113681.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-fstrub=relaxed -fbranch-probabilities" } */ +/* { dg-require-effective-target strub } */ + +/* Same as torture/strub-inlineable1.c, but with -fbranch-probabilities, to + check that IPA tree-profiling won't ICE. It would when we refrained from + running passes that would take it to IPA_SSA, but ran the pass that asserted + for IPA_SSA. */ + +inline void __attribute__ ((strub ("internal"), always_inline)) +inl_int_ali (void) +{ + /* No internal wrapper, so this body ALWAYS gets inlined, + but it cannot be called from non-strub contexts. */ +} + +void +bat (void) +{ + /* Not allowed, not a strub context. */ + inl_int_ali (); /* { dg-error "context" } */ +} diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc index b87c121790c99..e4bb689cef589 100644 --- a/gcc/tree-profile.cc +++ b/gcc/tree-profile.cc @@ -2070,7 +2070,8 @@ pass_ipa_tree_profile::gate (function *) disabled. */ return (!in_lto_p && !flag_auto_profile && (flag_branch_probabilities || flag_test_coverage - || profile_arc_flag || condition_coverage_flag)); + || profile_arc_flag || condition_coverage_flag) + && !seen_error ()); } } // anon namespace -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [strub] improve handling of indirected volatile parms [PR112938]
On Mar 11, 2024, Richard Biener wrote: > On Sat, Mar 9, 2024 at 10:10 AM Alexandre Oliva wrote: >> >> >> The earlier patch for PR112938 arranged for volatile parms to be made >> indirect in internal strub wrapped bodies. >> >> The first problem that remained, more evident, was that the indirected >> parameter remained volatile, despite the indirection, but it wasn't >> regimplified, so indirecting it was malformed gimple. >> >> Regimplifying turned out not to be needed. The best course of action >> was to drop the volatility from the by-reference parm, that was being >> unexpectedly inherited from the original volatile parm. >> >> That exposed another problem: the dereferences would then lose their >> volatile status, so we had to bring volatile back to them. >> >> Regstrapped on x86_64-linux-gnu. Ok to install? > OK Thanks, sorry, I dropped the ball on this one. I'm going to put it in momentarily. It had been approved for gcc 14, before it branched off; should I install it there as well? >> >> for gcc/ChangeLog >> >> PR middle-end/112938 >> * ipa-strub.cc (pass_ipa_strub::execute): Drop volatility from >> indirected parm. >> (maybe_make_indirect): Restore volatility in dereferences. >> >> for gcc/testsuite/ChangeLog >> >> PR middle-end/112938 >> * g++.dg/strub-internal-pr112938.cc: New. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [i386] add -msse2 to tests that require it
Without -msse2, an i586-targeting toolchain fails bf16_short_warn.c because neither type __m128bh nor intrinsic _mm_cvtneps_pbh get declared. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/i386/bf16_short_warn.c: Add -msse2. --- gcc/testsuite/gcc.target/i386/bf16_short_warn.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.target/i386/bf16_short_warn.c b/gcc/testsuite/gcc.target/i386/bf16_short_warn.c index 3e47a815200c2..2e05624bc26f6 100644 --- a/gcc/testsuite/gcc.target/i386/bf16_short_warn.c +++ b/gcc/testsuite/gcc.target/i386/bf16_short_warn.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -msse2" } */ #include typedef struct { -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [i386] work around fails with --enable-frame-pointer
A few x86 tests get unexpected insn counts if the toolchain is configured with --enable-frame-pointer. Add explicit -fomit-frame-pointer so that the expected insn sequences are output. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/i386/pr107261.c: Add -fomit-frame-pointer. * gcc.target/i386/pr69482-1.c: Likewise. * gcc.target/i386/pr69482-2.c: Likewise. --- gcc/testsuite/gcc.target/i386/pr107261.c |2 +- gcc/testsuite/gcc.target/i386/pr69482-1.c |2 +- gcc/testsuite/gcc.target/i386/pr69482-2.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/gcc.target/i386/pr107261.c b/gcc/testsuite/gcc.target/i386/pr107261.c index eb1d232fbfc4b..b422af9cbf9a2 100644 --- a/gcc/testsuite/gcc.target/i386/pr107261.c +++ b/gcc/testsuite/gcc.target/i386/pr107261.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -msse2" } */ +/* { dg-options "-O2 -msse2 -fomit-frame-pointer" } */ typedef __bf16 v4bf __attribute__ ((vector_size (8))); typedef __bf16 v2bf __attribute__ ((vector_size (4))); diff --git a/gcc/testsuite/gcc.target/i386/pr69482-1.c b/gcc/testsuite/gcc.target/i386/pr69482-1.c index 99bb6ad5a377b..7ef0e71b17c8e 100644 --- a/gcc/testsuite/gcc.target/i386/pr69482-1.c +++ b/gcc/testsuite/gcc.target/i386/pr69482-1.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O3 -fno-stack-protector" } */ +/* { dg-options "-O3 -fno-stack-protector -fomit-frame-pointer" } */ static inline void memset_s(void* s, int n) { volatile unsigned char * p = s; diff --git a/gcc/testsuite/gcc.target/i386/pr69482-2.c b/gcc/testsuite/gcc.target/i386/pr69482-2.c index 58e89a7933364..6aabe4fb39399 100644 --- a/gcc/testsuite/gcc.target/i386/pr69482-2.c +++ b/gcc/testsuite/gcc.target/i386/pr69482-2.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O2" } */ +/* { dg-options "-O2 -fomit-frame-pointer" } */ void bar () { -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [arm] accept empty init for bfloat16
Complete r13-2205, adjusting an arm-specific test that expects a no-longer-issued error at an empty initializer. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.target/arm/bfloat16_scalar_typecheck.c: Accept C23 empty initializers. --- .../gcc.target/arm/bfloat16_scalar_typecheck.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_typecheck.c b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_typecheck.c index 8c80c55bc9f4c..04ede93bda152 100644 --- a/gcc/testsuite/gcc.target/arm/bfloat16_scalar_typecheck.c +++ b/gcc/testsuite/gcc.target/arm/bfloat16_scalar_typecheck.c @@ -42,7 +42,7 @@ bfloat16_t footest (bfloat16_t scalar0) short initi_1_4 = glob_bfloat; /* { dg-error {invalid conversion from type 'bfloat16_t'} } */ double initi_1_5 = glob_bfloat; /* { dg-error {invalid conversion from type 'bfloat16_t'} } */ - bfloat16_t scalar2_1 = {}; /* { dg-error {empty scalar initializer} } */ + bfloat16_t scalar2_1 = {}; bfloat16_t scalar2_2 = { glob_bfloat }; bfloat16_t scalar2_3 = { 0 }; /* { dg-error {invalid conversion to type 'bfloat16_t'} } */ bfloat16_t scalar2_4 = { 0.1 }; /* { dg-error {invalid conversion to type 'bfloat16_t'} } */ @@ -94,7 +94,7 @@ bfloat16_t footest (bfloat16_t scalar0) /* Compound literals. */ - (bfloat16_t) {}; /* { dg-error {empty scalar initializer} } */ + (bfloat16_t) {}; (bfloat16_t) { glob_bfloat }; (bfloat16_t) { 0 }; /* { dg-error {invalid conversion to type 'bfloat16_t'} } */ (bfloat16_t) { 0.1 }; /* { dg-error {invalid conversion to type 'bfloat16_t'} } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [libstdc++] [testsuite] disable SRA for compare_exchange_padding
On arm-vx7r2, the uses of as.load() as initializer get SRAed, so the padding bits in the tests are not what we might expect from full-word struct copies. I tried adding a function to perform bitwise copying, but even taking the as.load() argument by const&, we'd still construct a temporary with SRAed field-wise copying. Unable to find another way to ensure we wouldn't get a temporary, I went for disabling SRA. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for libstdc++-v3/ChangeLog * testsuite/29_atomics/atomic/compare_exchange_padding.cc: Disable SRA. --- .../29_atomics/atomic/compare_exchange_padding.cc |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc index 2f18d426e7f7e..a6081968ca869 100644 --- a/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc +++ b/libstdc++-v3/testsuite/29_atomics/atomic/compare_exchange_padding.cc @@ -1,6 +1,7 @@ // { dg-do run { target c++20 } } // { dg-require-atomic-cmpxchg-word "" } // { dg-add-options libatomic } +// { dg-additional-options "-fno-tree-sra" } #include #include @@ -26,10 +27,10 @@ main () s.s = 42; std::atomic as{ s }; - auto ts = as.load(); + auto ts = as.load(); // SRA might prevent copying of padding bits here. VERIFY( !compare_struct(s, ts) ); // padding cleared on construction as.exchange(s); - auto es = as.load(); + auto es = as.load(); // SRA might prevent copying of padding bits here. VERIFY( compare_struct(ts, es) ); // padding cleared on exchange S n; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [arm] require arm_v8_1m_main for pacbti tests
arm pac and bti tests that use -march=armv8.1-m.main get an implicit -mthumb, that is incompatible with vxworks kernel mode. Declaring the requirement for a 8.1-m.main-compatible toolchain is enough to avoid those fails, because the toolchain feature test fails in kernel mode. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * g++.target/arm/pac-1.C: Require arm_arch_v8_1m_main. * gcc.target/arm/acle/pacbti-m-predef-11.c: Likewise. * gcc.target/arm/acle/pacbti-m-predef-12.c: Likewise. * gcc.target/arm/acle/pacbti-m-predef-7.c: Likewise. * gcc.target/arm/bti-1.c: Likewise. * gcc.target/arm/bti-2.c: Likewise. --- gcc/testsuite/g++.target/arm/pac-1.C |1 + .../gcc.target/arm/acle/pacbti-m-predef-11.c |1 + .../gcc.target/arm/acle/pacbti-m-predef-12.c |1 + .../gcc.target/arm/acle/pacbti-m-predef-7.c|1 + gcc/testsuite/gcc.target/arm/bti-1.c |1 + gcc/testsuite/gcc.target/arm/bti-2.c |1 + 6 files changed, 6 insertions(+) diff --git a/gcc/testsuite/g++.target/arm/pac-1.C b/gcc/testsuite/g++.target/arm/pac-1.C index f671a27b048c6..f48ad6cc5cb65 100644 --- a/gcc/testsuite/g++.target/arm/pac-1.C +++ b/gcc/testsuite/g++.target/arm/pac-1.C @@ -2,6 +2,7 @@ /* { dg-do compile } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ /* { dg-options "-march=armv8.1-m.main+mve+pacbti -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard -g -O0" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ __attribute__((noinline)) void fn1 (int a, int b, int c) diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c index 6a5ae92c567f3..dba4f491cfea7 100644 --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" "-mfloat-abi=*" } } */ /* { dg-options "-march=armv8.1-m.main+fp+pacbti" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ #if (__ARM_FEATURE_BTI != 1) #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be defined to 1." diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c index db40b17c3b030..308a41eb4ba4c 100644 --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-12.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ /* { dg-options "-march=armv8-m.main+fp -mfloat-abi=softfp" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ #if defined (__ARM_FEATURE_BTI) #error "Feature test macro __ARM_FEATURE_BTI should not be defined." diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c index 1b25907635e24..10836a84bde56 100644 --- a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-7.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ /* { dg-additional-options "-march=armv8.1-m.main+pacbti+fp --save-temps -mfloat-abi=hard" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ #if defined (__ARM_FEATURE_BTI_DEFAULT) #error "Feature test macro __ARM_FEATURE_BTI_DEFAULT should be undefined." diff --git a/gcc/testsuite/gcc.target/arm/bti-1.c b/gcc/testsuite/gcc.target/arm/bti-1.c index 79dd8010d2dab..34655387b55f5 100644 --- a/gcc/testsuite/gcc.target/arm/bti-1.c +++ b/gcc/testsuite/gcc.target/arm/bti-1.c @@ -2,6 +2,7 @@ /* { dg-do compile } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ /* { dg-options "-march=armv8.1-m.main -mthumb -mfloat-abi=softfp -mbranch-protection=bti --save-temps" } */ +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */ int main (void) diff --git a/gcc/testsuite/gcc.target/arm/bti-2.c b/gcc/testsuite/gcc.target/arm/bti-2.c index 33910563849a4..3284aba3020cc 100644 --- a/gcc/testsuite/gcc.target/arm/bti-2.c +++ b/gcc/testsuite/gcc.target/arm/bti-2.c @@ -3,6 +3,7 @@ /* { dg-options "-Os" } */ /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" "-mcpu=*" } } */ /* { dg-options "
[PATCH] [testsuite] [i386] require fpic for pr111497.C
Fix another test that uses -fPIC without requiring fpic support. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? PS: This is neither the first nor the last such patch. Maybe the test harness could detect -fPIC et al in compile options and react intelligently to them, whether by warning if dg-require-effective-target fpic is missing, or adding it implicitly. We could have more such smarts in the testsuite machinery. WDYT? for gcc/testsuite/ChangeLog * g++.target/i386/pr111497.C: Require fpic support. --- gcc/testsuite/g++.target/i386/pr111497.C |1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/g++.target/i386/pr111497.C b/gcc/testsuite/g++.target/i386/pr111497.C index a645bb95907ee..30e2e0409ad0e 100644 --- a/gcc/testsuite/g++.target/i386/pr111497.C +++ b/gcc/testsuite/g++.target/i386/pr111497.C @@ -1,5 +1,6 @@ // { dg-do compile { target ia32 } } // { dg-options "-march=i686 -mtune=generic -fPIC -O2 -g" } +// { dg-require-effective-target fpic } class A; struct B { const char *b1; int b2; }; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [libstdc++] introduce --disable-compat-libstdcxx-abi
t; int main() { typedef short atomic_type; @@ -16252,7 +16255,7 @@ $as_echo "$glibcxx_cv_atomic_short" >&6; } rm -f conftest* cat > conftest.$ac_ext << EOF -#line 16255 "configure" +#line 16258 "configure" int main() { // NB: _Atomic_word not necessarily int. @@ -16288,7 +16291,7 @@ $as_echo "$glibcxx_cv_atomic_int" >&6; } rm -f conftest* cat > conftest.$ac_ext << EOF -#line 16291 "configure" +#line 16294 "configure" int main() { typedef long long atomic_type; @@ -16444,7 +16447,7 @@ $as_echo "mutex" >&6; } # unnecessary for this test. cat > conftest.$ac_ext << EOF -#line 16447 "configure" +#line 16450 "configure" int main() { _Decimal32 d1; @@ -16486,7 +16489,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu # unnecessary for this test. cat > conftest.$ac_ext << EOF -#line 16489 "configure" +#line 16492 "configure" template struct same { typedef T2 type; }; @@ -55671,7 +55674,35 @@ $as_echo "$gxx_include_dir" >&6; } # OPTIMIZE_CXXFLAGS = -O3 -fstrict-aliasing -fvtable-gc - WARN_FLAGS="-Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2" + WARN_FLAGS="-Wall -Wextra -Wwrite-strings -Wcast-qual" + + + + # Default. + WARN_FLAGS_WABI=\ -Wabi=2 + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for --disable-compat-libstdcxx-abi" >&5 +$as_echo_n "checking for --disable-compat-libstdcxx-abi... " >&6; } + # Check whether --enable-compat-libstdcxx-abi was given. +if test "${enable_compat_libstdcxx_abi+set}" = set; then : + enableval=$enable_compat_libstdcxx_abi; case "$enableval" in + yes) { $as_echo "$as_me:${as_lineno-$LINENO}: result: enabled$WARN_FLAGS_WABI" >&5 +$as_echo "enabled$WARN_FLAGS_WABI" >&6; } ;; + no) WARN_FLAGS_WABI= + { $as_echo "$as_me:${as_lineno-$LINENO}: result: disabled" >&5 +$as_echo "disabled" >&6; } ;; + *) { $as_echo "$as_me:${as_lineno-$LINENO}: result: unsupported" >&5 +$as_echo "unsupported" >&6; } + as_fn_error $? "Unsupported argument to enable/disable compat libstdc++ abi" "$LINENO" 5;; + esac +else + + { $as_echo "$as_me:${as_lineno-$LINENO}: result: defaulting to enabled$WARN_FLAGS_WABI" >&5 +$as_echo "defaulting to enabled$WARN_FLAGS_WABI" >&6; } + +fi + + + WARN_FLAGS="$WARN_FLAGS$WARN_FLAGS_WABI" diff --git a/libstdc++-v3/doc/html/manual/configure.html b/libstdc++-v3/doc/html/manual/configure.html index 346b5d345cd1b..8636b2360d9f0 100644 --- a/libstdc++-v3/doc/html/manual/configure.html +++ b/libstdc++-v3/doc/html/manual/configure.html @@ -108,6 +108,8 @@ then the [time.clock] implementation will use a system call to access the realtime and monotonic clocks, which is significantly slower than the C library's clock_gettime function. +--disable-compat-libstdcxx-abiDisables +backward-compatibility ABI symbols. --enable-libstdcxx-debugBuild separate debug libraries in addition to what is normally built. By default, the debug libraries are compiled with CXXFLAGS='-g3 -O0 -fno-inline' -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] xfail pr103798-2 in C++ on vxworks too [PR113706]
pr103798-2.c fails in C++ on targets that provide a ISO C++-compliant declaration of memchr, because it mismatches the C-compatible builtin, as per PR113706. Expect the C++ test to fail on vxworks as well. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog PR testsuite/113706 * c-c++-common/pr103798-2.c: XFAIL in C++ on vxworks too. --- gcc/testsuite/c-c++-common/pr103798-2.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/c-c++-common/pr103798-2.c b/gcc/testsuite/c-c++-common/pr103798-2.c index bc126c205e1e3..83cdfaa1660bb 100644 --- a/gcc/testsuite/c-c++-common/pr103798-2.c +++ b/gcc/testsuite/c-c++-common/pr103798-2.c @@ -28,4 +28,4 @@ main () } /* See PR c++/113706 for the xfail. */ -/* { dg-final { scan-assembler-not "memchr" { xfail { c++ && *-*-solaris2* } } } } */ +/* { dg-final { scan-assembler-not "memchr" { xfail { c++ && { *-*-solaris2* *-*-vxworks* } } } } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [analyzer] include sys/select.h if available
Test that calls select fails on vxworks because select is only declared in sys/select.h. Include that header if it's present. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c: Include sys/select.h if present. --- .../fd-glibc-byte-stream-connection-server.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c b/gcc/testsuite/gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c index fcbcc740170e6..f922a52238f90 100644 --- a/gcc/testsuite/gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c +++ b/gcc/testsuite/gcc.dg/analyzer/fd-glibc-byte-stream-connection-server.c @@ -8,6 +8,9 @@ #include #include #include +#if __has_include() +#include +#endif #include #include #include -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [vxworks] avoid mangling __STDC_VERSION_LIMITS_H__
The mangling of the macro name that guards limits.h from reinclusion was mangling a c23-required macro as well. Make the edit pattern stricter. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/ChangeLog * config/t-vxworks (vxw-glimits.h): Don't mangle c23-required __STDC_VERSION_LIMITS_H__ define. --- gcc/config/t-vxworks |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/t-vxworks b/gcc/config/t-vxworks index b89350c3c70f4..6063943e346e6 100644 --- a/gcc/config/t-vxworks +++ b/gcc/config/t-vxworks @@ -57,7 +57,7 @@ T_GLIMITS_H = vxw-glimits.h vxw-glimits.h: $(srcdir)/glimits.h ID=`echo $(BASEVER_c) | sed -e 's/\./_/g'` && \ - sed -e "s/_LIMITS_H__/_LIMITS_H__$${ID}_/" < $< > $@T + sed -e "s/_LIMITS_H___/_LIMITS_H__$${ID}_/" < $< > $@T mv $@T $@ # Arrange to "provide" a tailored version of stdint-gcc.h -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [analyzer] require fork where used
Mark tests that fail due to the lack of fork, as in vxworks kernel mode, as requiring fork. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/analyzer/pipe-glibc.c: Require fork. * gcc.dg/analyzer/pipe-manpages.c: Likewise. --- gcc/testsuite/gcc.dg/analyzer/pipe-glibc.c|5 +++-- gcc/testsuite/gcc.dg/analyzer/pipe-manpages.c |2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/analyzer/pipe-glibc.c b/gcc/testsuite/gcc.dg/analyzer/pipe-glibc.c index 60558a870b9d7..fe38ddef3959a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pipe-glibc.c +++ b/gcc/testsuite/gcc.dg/analyzer/pipe-glibc.c @@ -1,6 +1,7 @@ -/* Example of pipe usage from glibc manual. */ - /* { dg-skip-if "" { "avr-*-*" } } */ +/* { dg-require-fork "" } */ + +/* Example of pipe usage from glibc manual. */ #include #include diff --git a/gcc/testsuite/gcc.dg/analyzer/pipe-manpages.c b/gcc/testsuite/gcc.dg/analyzer/pipe-manpages.c index 6b9ae4d260281..ac5805fdba092 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pipe-manpages.c +++ b/gcc/testsuite/gcc.dg/analyzer/pipe-manpages.c @@ -1,3 +1,5 @@ +/* { dg-require-fork "" } */ + /* Example of "pipe" from release 5.13 of the Linux man-pages project. Copyright (C) 2005, 2008, Michael Kerrisk -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [analyzer] skip access-mode: O_ACCMODE on vxworks
O_ACCMODE is not defined on vxworks, and the test is meaningless and failing without it, so skip it. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/analyzer/fd-access-mode-target-headers.c: Skip on vxworks as well. --- .../analyzer/fd-access-mode-target-headers.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-access-mode-target-headers.c b/gcc/testsuite/gcc.dg/analyzer/fd-access-mode-target-headers.c index b57b9fa2279c2..9fc32638a3de4 100644 --- a/gcc/testsuite/gcc.dg/analyzer/fd-access-mode-target-headers.c +++ b/gcc/testsuite/gcc.dg/analyzer/fd-access-mode-target-headers.c @@ -1,5 +1,4 @@ -/* { dg-skip-if "" { powerpc*-*-aix* || newlib } } */ -/* { dg-skip-if "" { avr-*-* } } */ +/* { dg-skip-if "" { { powerpc*-*-aix* avr-*-* *-*-vxworks* } || newlib } } */ #include #include -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] [analyzer] avoid vxworks libc mode_t
Define macro that prevents mode_t from being defined by vxworks' headers as well. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * gcc.dg/analyzer/fd-4.c: Define macro to avoid mode_t on vxworks. --- gcc/testsuite/gcc.dg/analyzer/fd-4.c |1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c b/gcc/testsuite/gcc.dg/analyzer/fd-4.c index 880de3d789607..d104bfdad547f 100644 --- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c @@ -1,4 +1,5 @@ /* { dg-additional-options "-D_MODE_T_DECLARED=1" { target newlib } } */ +/* { dg-additional-options "-D_DEFINED_mode_t" { target *-*-vxworks* } } */ #if defined(_AIX) || defined(__hpux) #define _MODE_T #endif -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [testsuite] introduce strndup effective target
ite/gcc.dg/builtin-object-size-2.c index 37d3dcc6f5689..c28d72eee9bfe 100644 --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #include "builtin-object-size-common.h" @@ -536,7 +537,7 @@ test8 (unsigned cond) #endif } -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP /* Tests for strdup/strndup. */ size_t __attribute__ ((noinline)) @@ -624,7 +625,7 @@ test9 (void) FAIL (); free (res); } -#endif /* avr */ +#endif int main (void) @@ -639,7 +640,7 @@ main (void) test6 (); test7 (); test8 (1); -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP test9 (); #endif DONE (); diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c index f4d1ebf7027bf..3f58da3d500cd 100644 --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #include "builtin-object-size-common.h" @@ -628,7 +629,7 @@ test10 (void) } } -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP /* Tests for strdup/strndup. */ size_t __attribute__ ((noinline)) @@ -717,7 +718,7 @@ test11 (void) FAIL (); free (res); } -#endif /* avr */ +#endif int main (void) @@ -734,7 +735,7 @@ main (void) test8 (); test9 (1); test10 (); -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP test11 (); #endif DONE (); diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c index 2887dd150423b..b3eb36efb744d 100644 --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c @@ -1,6 +1,7 @@ /* { dg-do run } */ /* { dg-options "-O2 -Wno-stringop-overread" } */ /* { dg-require-effective-target alloca } */ +/* { dg-additional-options "-DSKIP_STRNDUP" { target { ! strndup } } } */ #include "builtin-object-size-common.h" @@ -509,7 +510,7 @@ test8 (unsigned cond) #endif } -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP /* Tests for strdup/strndup. */ size_t __attribute__ ((noinline)) @@ -597,7 +598,7 @@ test9 (void) FAIL (); free (res); } -#endif /* avr */ +#endif int main (void) @@ -612,7 +613,7 @@ main (void) test6 (); test7 (); test8 (1); -#if !defined(__AVR__) && !defined(__hpux__) /* avr and hpux have no strndup */ +#ifndef SKIP_STRNDUP test9 (); #endif DONE (); diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index edce672c0e21a..17c8382adf143 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -11580,6 +11580,17 @@ proc check_effective_target_stpcpy {} { return [check_function_available "stpcpy"] } +# Returns 1 if "strndup" is available on the target system. + +proc check_effective_target_strndup {} { +if { [istarget *-*-vxworks*] } { + # VxWorks doesn't have strndup but our way to test fails + # to detect as we're doing partial links for kernel modules. + return 0 +} +return [check_function_available "strndup"] +} + # Returns 1 if "sigsetjmp" is available on the target system. # Also check if "__sigsetjmp" is defined since that's what glibc # uses. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [c++] [testsuite] adjust contracts9.C for negative addresses
The test expected the address of a literal string, converted to long long, to yield a positive value. That expectation doesn't necessarily hold, and the test fails where it doesn't. Adjust the test to use a pointer that will compare as expected. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for gcc/testsuite/ChangeLog * g++.dg/contracts/contracts9.C: Don't assume string literals have non-negative addresses. --- gcc/testsuite/g++.dg/contracts/contracts9.C |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/g++.dg/contracts/contracts9.C b/gcc/testsuite/g++.dg/contracts/contracts9.C index 09a1a6532c5a0..58b60aca32057 100644 --- a/gcc/testsuite/g++.dg/contracts/contracts9.C +++ b/gcc/testsuite/g++.dg/contracts/contracts9.C @@ -27,7 +27,7 @@ int main() { fun1(1, -1); fun1(-1, 1.0); - fun1(-1, "test"); + fun1(-1, (const char *)0x1234); [[ assert: fun1(-1, -5) ]]; [[ assert: test::fun(10, -6) ]]; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [libstdc++] [testsuite] xfail double-prec from_chars for float128_t
Tests 20_util/from_chars/4.cc and 20_util/to_chars/long_double.cc were adjusted about a year ago to skip long double on some targets, because the fastfloat library was limited to 64-bit doubles. The same problem comes up in similar float128_t tests on aarch64-vxworks. This patch adjusts them similarly. Unlike the earlier tests, that got similar treatment for x86_64-vxworks, these haven't failed there. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for libstdc++-v3/ChangeLog * testsuite/20_util/from_chars/8.cc: Skip float128_t testing on aarch64-vxworks. * testsuite/20_util/to_chars/float128-c++23.cc: Xfail run on aarch64-vxworks. --- libstdc++-v3/testsuite/20_util/from_chars/8.cc |3 ++- .../testsuite/20_util/to_chars/float128_c++23.cc |1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libstdc++-v3/testsuite/20_util/from_chars/8.cc b/libstdc++-v3/testsuite/20_util/from_chars/8.cc index ee60d88c332db..a6343422c5a91 100644 --- a/libstdc++-v3/testsuite/20_util/from_chars/8.cc +++ b/libstdc++-v3/testsuite/20_util/from_chars/8.cc @@ -17,6 +17,7 @@ // { dg-do run { target c++23 } } // { dg-add-options ieee } +// { dg-additional-options "-DSKIP_LONG_DOUBLE" { target aarch64-*-vxworks* } } #include #include @@ -343,7 +344,7 @@ test06() #if defined(__STDCPP_FLOAT64_T__) && defined(_GLIBCXX_DOUBLE_IS_IEEE_BINARY64) test_max_mantissa(); #endif -#if defined(__GLIBCXX_TYPE_INT_N_0) \ +#if defined(__GLIBCXX_TYPE_INT_N_0) && !defined SKIP_LONG_DOUBLE \ && defined(__STDCPP_FLOAT128_T__) && defined(_GLIBCXX_LDOUBLE_IS_IEEE_BINARY128) test_max_mantissa(); #endif diff --git a/libstdc++-v3/testsuite/20_util/to_chars/float128_c++23.cc b/libstdc++-v3/testsuite/20_util/to_chars/float128_c++23.cc index 547632817b4bb..ca00761ee7c98 100644 --- a/libstdc++-v3/testsuite/20_util/to_chars/float128_c++23.cc +++ b/libstdc++-v3/testsuite/20_util/to_chars/float128_c++23.cc @@ -19,6 +19,7 @@ // { dg-require-effective-target ieee_floats } // { dg-require-effective-target size32plus } // { dg-add-options ieee } +// { dg-xfail-run-if "from_chars limited to double-precision" { aarch64-*-vxworks* } } #include #include -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [libstdc++] define zoneinfo_dir_override on vxworks
VxWorks fails to load kernel-mode modules with weak undefined symbols. In RTP mode modules, that undergo final linking, weak undefined symbols are not a problem. This patch adds kernel-mode VxWorks multilibs to the set of targets that don't support weak undefined symbols without special flags, in which tzdb's zoneinfo_dir_override is given a weak definition. Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? for libstdc++-v3/ChangeLog * src/c++20/tzdb.cc (__gnu_cxx::zoneinfo_dir_override): Define on VxWorks non-RTP. --- libstdc++-v3/src/c++20/tzdb.cc |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc index 890a4c53e2d44..639d1c440ba16 100644 --- a/libstdc++-v3/src/c++20/tzdb.cc +++ b/libstdc++-v3/src/c++20/tzdb.cc @@ -70,8 +70,9 @@ namespace __gnu_cxx #else [[gnu::weak]] const char* zoneinfo_dir_override(); -#if defined(__APPLE__) || defined(__hpux__) - // Need a weak definition for Mach-O. +#if defined(__APPLE__) || defined(__hpux__) \ + || (defined(__VXWORKS__) && !defined(__RTP__)) + // Need a weak definition for Mach-O et al. [[gnu::weak]] const char* zoneinfo_dir_override() { #ifdef _GLIBCXX_ZONEINFO_DIR -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[testsuite] [aarch64] Require fpic effective target
Regstrapped on x86_64-linux-gnu. Also tested with gcc-13 on arm-, aarch64-, x86- and x86_64-vxworks7r2. Ok to install? Co-authored-by: Olivier Hainque for gcc/testsuite/ChangeLog * gcc.target/aarch64/pr94201.c: Add missing dg-require-effective-target fpic. * gcc.target/aarch64/pr103085.c: Likewise. --- gcc/testsuite/gcc.target/aarch64/pr103085.c |1 + gcc/testsuite/gcc.target/aarch64/pr94201.c |1 + 2 files changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.target/aarch64/pr103085.c b/gcc/testsuite/gcc.target/aarch64/pr103085.c index dbc9c15b71f22..347280ed42b2d 100644 --- a/gcc/testsuite/gcc.target/aarch64/pr103085.c +++ b/gcc/testsuite/gcc.target/aarch64/pr103085.c @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-O2 -fstack-protector-strong -fPIC" } */ +/* { dg-require-effective-target fpic } */ void g(int*); void diff --git a/gcc/testsuite/gcc.target/aarch64/pr94201.c b/gcc/testsuite/gcc.target/aarch64/pr94201.c index 691761691868a..3b9b79059e02b 100644 --- a/gcc/testsuite/gcc.target/aarch64/pr94201.c +++ b/gcc/testsuite/gcc.target/aarch64/pr94201.c @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-mcmodel=tiny -mabi=ilp32 -fPIC" } */ +/* { dg-require-effective-target fpic } */ extern int bar (void *); extern long long a; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] [tree-prof] skip if errors were seen [PR113681]
On Mar 22, 2024, Jeff Law wrote: > On 3/9/24 2:11 AM, Alexandre Oliva wrote: >> ipa_tree_profile asserts that the symtab is in IPA_SSA state, but we >> don't reach that state and ICE if e.g. ipa-strub passes report errors. >> Skip this pass if errors were seen. >> Regstrapped on x86_64-linux-gnu. Ok to install? >> >> for gcc/ChangeLog >> PR tree-optimization/113681 >> * tree-profiling.cc (pass_ipa_tree_profile::gate): Skip if >> seen_errors. >> for gcc/testsuite/ChangeLog >> PR tree-optimization/113681 >> * c-c++-common/strub-pr113681.c: New. > So I've really never dug into strub, but this would seem to imply that > an error from strub is non-fatal? Yeah. I believe that's no different from other passes. Various other passes have seen_errors guards, but ipa-prof didn't. I suppose the insertion point for the strubm pass was one where others passes didn't previously issue errors, so that wasn't an issue for ipa-prof. But now it is. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [tree-prof] skip if errors were seen [PR113681]
ipa_tree_profile asserts that the symtab is in IPA_SSA state, but we don't reach that state and ICE if e.g. ipa-strub passes report errors. Skip this pass if errors were seen. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog PR tree-optimization/113681 * tree-profiling.cc (pass_ipa_tree_profile::gate): Skip if seen_errors. for gcc/testsuite/ChangeLog PR tree-optimization/113681 * c-c++-common/strub-pr113681.c: New. --- gcc/testsuite/c-c++-common/strub-pr113681.c | 22 ++ gcc/tree-profile.cc |3 ++- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/c-c++-common/strub-pr113681.c diff --git a/gcc/testsuite/c-c++-common/strub-pr113681.c b/gcc/testsuite/c-c++-common/strub-pr113681.c new file mode 100644 index 0..3ef9017b2eb70 --- /dev/null +++ b/gcc/testsuite/c-c++-common/strub-pr113681.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-fstrub=relaxed -fbranch-probabilities" } */ +/* { dg-require-effective-target strub } */ + +/* Same as torture/strub-inlineable1.c, but with -fbranch-probabilities, to + check that IPA tree-profiling won't ICE. It would when we refrained from + running passes that would take it to IPA_SSA, but ran the pass that asserted + for IPA_SSA. */ + +inline void __attribute__ ((strub ("internal"), always_inline)) +inl_int_ali (void) +{ + /* No internal wrapper, so this body ALWAYS gets inlined, + but it cannot be called from non-strub contexts. */ +} + +void +bat (void) +{ + /* Not allowed, not a strub context. */ + inl_int_ali (); /* { dg-error "context" } */ +} diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc index aed13e2b1bc30..d2bdbe3a08c2f 100644 --- a/gcc/tree-profile.cc +++ b/gcc/tree-profile.cc @@ -999,7 +999,8 @@ pass_ipa_tree_profile::gate (function *) disabled. */ return (!in_lto_p && !flag_auto_profile && (flag_branch_probabilities || flag_test_coverage - || profile_arc_flag)); + || profile_arc_flag) + && !seen_error ()); } } // anon namespace -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] [strub] improve handling of indirected volatile parms [PR112938]
The earlier patch for PR112938 arranged for volatile parms to be made indirect in internal strub wrapped bodies. The first problem that remained, more evident, was that the indirected parameter remained volatile, despite the indirection, but it wasn't regimplified, so indirecting it was malformed gimple. Regimplifying turned out not to be needed. The best course of action was to drop the volatility from the by-reference parm, that was being unexpectedly inherited from the original volatile parm. That exposed another problem: the dereferences would then lose their volatile status, so we had to bring volatile back to them. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog PR middle-end/112938 * ipa-strub.cc (pass_ipa_strub::execute): Drop volatility from indirected parm. (maybe_make_indirect): Restore volatility in dereferences. for gcc/testsuite/ChangeLog PR middle-end/112938 * g++.dg/strub-internal-pr112938.cc: New. --- gcc/ipa-strub.cc|7 +++ gcc/testsuite/g++.dg/strub-internal-pr112938.cc | 12 2 files changed, 19 insertions(+) create mode 100644 gcc/testsuite/g++.dg/strub-internal-pr112938.cc diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index dff94222351ad..8fa7bdf530023 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -1940,6 +1940,9 @@ maybe_make_indirect (indirect_parms_t _parms, tree op, int *rec) TREE_TYPE (TREE_TYPE (op)), op, build_int_cst (TREE_TYPE (op), 0)); + if (TYPE_VOLATILE (TREE_TYPE (TREE_TYPE (op))) + && !TREE_THIS_VOLATILE (ret)) + TREE_SIDE_EFFECTS (ret) = TREE_THIS_VOLATILE (ret) = 1; return ret; } } @@ -2894,6 +2897,10 @@ pass_ipa_strub::execute (function *) probably drop the TREE_ADDRESSABLE and keep the TRUE. */ tree ref_type = build_ref_type_for (nparm); + if (TREE_THIS_VOLATILE (nparm) + && TYPE_VOLATILE (TREE_TYPE (nparm)) + && !TYPE_VOLATILE (ref_type)) + TREE_SIDE_EFFECTS (nparm) = TREE_THIS_VOLATILE (nparm) = 0; DECL_ARG_TYPE (nparm) = TREE_TYPE (nparm) = ref_type; relayout_decl (nparm); TREE_ADDRESSABLE (nparm) = 0; diff --git a/gcc/testsuite/g++.dg/strub-internal-pr112938.cc b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc new file mode 100644 index 0..5a74becc2697e --- /dev/null +++ b/gcc/testsuite/g++.dg/strub-internal-pr112938.cc @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-fdump-tree-optimized -O2" } */ +/* { dg-require-effective-target strub } */ + +bool __attribute__ ((__strub__ ("internal"))) +f(bool i, volatile bool j) +{ + return (i ^ j) == j; +} + +/* Check for two dereferences of the indirected volatile j parm. */ +/* { dg-final { scan-tree-dump-times {={v} \*j_[0-9][0-9]*(D)} 2 "optimized" } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] arm: fix c23 0-named-args caller-side stdarg
On Mar 1, 2024, "Richard Earnshaw (lists)" wrote: > On 01/03/2024 04:38, Alexandre Oliva wrote: >> Thanks for the review. > For closure, Jakub has just pushed a patch to the generic code, so I > don't think we need this now. ACK. I see the c2x-stdarg-4.c test is now passing in our arm-eabi gcc-13 tree. Thank you all. Alas, the same nightly build showed a new riscv fail in c23-stdarg-6.c, that also got backported to gcc-13. Presumably it's failing in the trunk as well, both riscv32-elf and riscv64-elf. I haven't looked into whether it's a regression brought about by the patch or just a new failure mode that the new test exposed. Either way, I'm not sure whether to link this new failure to any of the associated PRs or to file a new one, but, FTR, I'm going to look into it. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] calls: Fix up TYPE_NO_NAMED_ARGS_STDARG_P handling [PR107453]
On Feb 27, 2024, Richard Earnshaw wrote: > This one has been festering for a while; both Alexandre and Torbjorn > have attempted to fix it recently, but I'm not sure either is really > right... *nod* xref https://gcc.gnu.org/pipermail/gcc-patches/2024-March/646926.html The patch I proposed was indeed far too limited in scope. > On Arm this is causing all anonymous arguments to be passed on the > stack, which is incorrect per the ABI. On a target that uses > 'pretend_outgoing_vararg_named', why is it correct to set n_named_args > to zero? Is it enough to guard both the statements you've added with > !targetm.calls.pretend_outgoing_args_named? ISTM that the change you suggest over Jakub's patch would address the inconsistency on ARM. Matthew suggested a patch along these lines in the other thread, that I xrefed above, that seems sound to me, but I also suspect it won't fix the ppc64le issue. My hunch is that we'll need a combination of both, possibly with further tweaks to adjust for Jakub's just-added test. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
Re: [PATCH] arm: fix c23 0-named-args caller-side stdarg
Hello, Matthew, Thanks for the review. On Feb 26, 2024, Matthew Malcomson wrote: > I think you're right that the AAPCS32 requires all arguments to be passed in > registers for this testcase. > (Nit on the commit-message: It says that your reading of the AAPCS32 > suggests > that the *caller* is correct -- I believe based on the change you > suggested you > meant *callee* is correct in expecting arguments in registers.) Ugh, yeah, sorry about the typo. > The approach you suggest looks OK to me -- I do notice that it doesn't > fix the > legacy ABI's of `atpcs` and `apcs` and guess it would be nicer to have them > working at the same time though would defer to maintainers on how > important that > is. > (For the benefit of others reading) I don't believe there is any ABI concern > with this since it's fixing something that is currently not working at > all and > only applies to c23 (so a change shouldn't have too much of an impact). > You mention you chose to make the change in the arm backend rather > than general > code due to hesitancy to change the generic ABI-affecting code. That makes > sense to me, certainly at this late stage in the development cycle. *nod* I wrote the patch in the following context: I hit the problem on the very first toolchain I started transitioning to gcc-13. I couldn't really fathom the notion that this breakage could have survived an entire release cycle if it affected many targets, and sort of held on to an assumption that the abi used by our arm-eabi toolchain had to be an uncommon one. All of this hypothesizing falls apart by the now apparent knowledge that the test is faling elsewhere as well, even on other ARM ABIs, it just hadn't been addressed yet. I'm glad we're getting there :-) > From a quick check on c23-stdarg-4.c it does look like the below > change ends up > with the same codegen as your patch (except in the case of those > legacy ABI's, > where the below does make the caller and callee ABI match AFAICT): > ``` > diff --git a/gcc/calls.cc b/gcc/calls.cc > index 01f44734743..0b302f633ed 100644 > --- a/gcc/calls.cc > +++ b/gcc/calls.cc > @@ -2970,14 +2970,15 @@ expand_call (tree exp, rtx target, int ignore) > we do not have any reliable way to pass unnamed args in > registers, so we must force them into memory. */ > - if (type_arg_types != 0 > + if ((type_arg_types != 0 || TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > && targetm.calls.strict_argument_naming (args_so_far)) > ; > else if (type_arg_types != 0 > && ! targetm.calls.pretend_outgoing_varargs_named > (args_so_far)) > /* Don't include the last named arg. */ > --n_named_args; > - else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype)) > + else if (TYPE_NO_NAMED_ARGS_STDARG_P (funtype) > + && ! targetm.calls.pretend_outgoing_varargs_named (args_so_far)) > n_named_args = 0; > else > /* Treat all args as named. */ > ``` > Do you agree that this makes sense (i.e. is there something I'm > completely missing)? Yeah, your argument is quite convincing, and the target knobs are indeed in line with the change you suggest, whereas the current code seems to deviate from them. With my ABI designer hat on, however, I see that there's room for ABIs to make decisions about 0-args stdargs that go differently from stdargs with leading named args, from prototyped functions, and even from prototypeless functions, and we might end up needing more knobs to deal with such custom decisions. We can cross that bridge if/when we get to it, though. > (lm32 mcore msp430 gcn cris fr30 frv h8300 arm v850 rx pru) Interesting that ppc64le is not on your list. There's PR107453 about that, and another thread is discussing a fix for it that is somewhat different from what you propose (presumably because the way the problem manifests on ppc64le is different), but it also tweaks expand_call. I'll copy you when following up there. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]
On Feb 23, 2024, Jason Merrill wrote: > The problem, as you say, comes when you want to both bootstrap and > build tools that aren't involved in the bootstrap process. It's more visible there, because those don't actively refrain from linking dynamically with libstdc++. But even bootstrapped matter that involves exceptions would have to link in libgcc_s, and that would bring about the same sort of issue. > To support that perhaps we want POSTBOOTSTRAP_HOST_EXPORTS for host > modules without the bootstrap tag, and add the TARGET_LIB_PATH > directories there? That would be welcome, but it doesn't really address the problem, does it? Namely, the problem that we may face two different kinds of scenarios, and each one calls for an opposite solution. 1. system tools used in the build depend on system libraries that are newer than the ones we're about to build This is the scenario that you're attempting to address with these patches. The problem here is that the libraries being built are older than the system libraries, and and system tools won't run if dynamically linked with the older libraries about to be built. 2. the libraries we're about to build are newer than corresponding system libraries, if any This is the scenario that the current build system aims for. Any build tools that rely on older system libraries are likely to work just as well with the newly built libraries. Any newly built libraries linked into programs used and run as part of the build have to be present in LD_LIBRARY_PATH lest we end up trying to use the older system libraries, which may seem to work in some settings, but is bound to break if the differences are large enough. For maximum clarity, consider a bootstrap with LTO using a linker plugin. The linker plugin is built against the newly-built libraries. The linker that attempts to load the plugin also requires the same libraries. Do you see how tending to 1. breaks 2., and vice-versa? Now add ASAN to the picture, for another set of newly-built libraries used during bootstrap. Also use a prebuilt linker with ASAN enabled, for maximum clarity of the problem I'm getting at. Do you see the problem? Do you agree that patching the build system to solve a problem in scenario 1. *will* cause problems in scenario 2., *unless* the fix can distinguish the two scenarios and behave accordingly, but that getting that right is triky and error prone? Do you agree that, until we get there, it's probably better to optimize for the more common scenario? Do you agree that the more common scenario is probably 2.? Do you agree that, until we get a solution that works for both 1. and 2. automatically, offering a reasonably simple workaround for 1., while aiming to work for 2., would be a desirable stopgap? Do you agree that adding support for users to prepend directories to the search path, enabling them to preempt build libraries with (symlinks to?) select newer system libraries, and documenting situations in which this could be needed, is a reasonably simple and desirable stopgap that enables 1. to work while defaulting to the presumed more common case 2.? Here's a patchlet that shows the crux of what I have in mind: (nevermind we'd make the change elsewhere, document it further elsewhere, set an empty default and arrange to passed down to sub-$(MAKE)s) diff --git a/Makefile.in b/Makefile.in index edb0c8a9a427f..10c7646ef98c4 100644 --- a/Makefile.in +++ b/Makefile.in @@ -771,7 +771,12 @@ TARGET_LIB_PATH_libatomic = $$r/$(TARGET_SUBDIR)/libatomic/.libs: # This is the list of directories that may be needed in RPATH_ENVVAR # so that programs built for the host machine work. -HOST_LIB_PATH = $(HOST_LIB_PATH_gmp)$(HOST_LIB_PATH_mpfr)$(HOST_LIB_PATH_mpc)$(HOST_LIB_PATH_isl) +# Users may set PREEMPT_HOST_LIB_PATH to a directory holding symlinks +# to system libraries required by build tools (say the linker) that +# are newer (as in higher-versioned) than the corresponding libraries +# we're building. If older libraries were to override the newer +# system libraries, that could prevent the build tools from running. +HOST_LIB_PATH = $(PREEMPT_HOST_LIB_PATH):$(HOST_LIB_PATH_gmp)$(HOST_LIB_PATH_mpfr)$(HOST_LIB_PATH_mpc)$(HOST_LIB_PATH_isl) # Define HOST_LIB_PATH_gcc here, for the sake of TARGET_LIB_PATH, ouch @if gcc Now, for a more general solution that doesn't require user intervention, configure could go about looking for system libraries in the default search path, or in RPATH_ENV_VAR, that share the soname with those we're about to build, identify preexisting libraries that are newer than those we're about to build, populate a build-tree directory with symlinks to them, and default PREEMPT_HOST_LIB_PATH to that directory. WDYT? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excludi
Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]
On Feb 21, 2024, Jason Merrill wrote: > So indeed I guess we still > want both prev and current libgcc directories in RPATH to handle the > case where we've removed the previous stage, as below. *nod*, thanks > I can't think of why we would need to depend on the current stage > target libraries, > If bootstrap doesn't actually need the target libraries. ISTM we may be miscommunicating. I'm not so worried about bootstrap itself as I am about post-bootstrap host tools. Those are unusual in that, after native bootstraps, they're built using the just-built (last-stage) compiler and target libraries, rather than the host compiler and system libraries. While configuring them, we need LD_LIBRARY_PATH (or similar) set up so that native execution tests can pass, at the very least; while building them, we may need LD_LIBRARY_PATH set up so that dependent libraries are found and link correctly. > I'm hoping for a fix that doesn't require individual users to know > about a workaround. I'm sure we'd all appreciate that, but AFAICT the conflicting requirements in scenarios of different freshness of libraries (system vs build) and both system tools and host tools dependencies on them would require a lot more intelligence in our build system to detect and react to the circumstances, deciding whether or not native tools used behind the scenes by the build will run with the just-built libraries, and whether or not the post-bootstrap host tools linked with the just-built libraries will run with the system libraries. There may even be scenarios in which these conflicting requirements would paint users into an inescapable corner AFAICT. The most worrying aspect is not libstdc++, but libgcc_s; it's needed as a dynamic library by languages that support exceptions, and depending on relative freshness of the toolchain being built and system libraries, newer symbols may be required by system programs and by just-built host tools. There doesn't seem to be an easy way around that. You have to either privilege one scenario over the other, like we do now, or introduce cleverness to detect and cope with such conflicting requirements, which we're not even trying to do. The approach in your patch changes the situation from privileging one scenario to privileging a mixed scenario (just-built libgcc but system libstdc++), and I'm pretty sure hunch that this mixed approach is likely to break down the road, even if not right now. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] RISC-V: Fix CTZ unnecessary sign extension [PR #106888]
On Feb 20, 2024, Jeff Law wrote: > On 2/19/24 21:26, Alexandre Oliva wrote: >> This backport for gcc-13 is required for pr90838.c to get the expected >> count of andi instructions on riscv64-elf . > In general, shouldn't backports be focused on correctness issues? *nod*. > It's unclear what the motivation is for backporting this change into > gcc-13. There's this unexpected fail in gcc-13 (pr90838.c), one out of a handful that we've hit while transitioning our riscv toolchains to gcc-13. I set out to understand them, I identified the patches that got them to pass in the trunk, and so I've proposed their backports to fix the fails in gcc-13. Surely there are other ways to address each one of the fails. But even if we choose to just xfail them, or leave them failing noisily, I've already gone through the process of identifying the fix, so I figured I might as well share it. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]
On Feb 16, 2024, Jason Merrill wrote: > So, for stage2+, let's add just prev- libgcc. I'm pretty sure this will break bootstrap-lean where libgcc_s isn't a system library, and we're building post-bootstrap host tools :-( We need the current stage lib after the prev stage is removed. I also doubt that TARGET_LIB_PATH was defined and used for no reason. My hunch is that bootstrap options and/or targets that don't have these libraries as system libraries will break in some obscure way without it. But I don't have the bandwidth to track down the history behind their inclusion. I insist that the entire approach of choosing the same set of target library directories regardless of the freshness relationship between e.g. a system libstdc++ and the one we're building can't possibly be an overall improvement, it's only trading problems in some scenarios (where we're building an older libstdc++) for problems in other scenarios (where we're building a newer libstdc++). The latter is unfortunately far more likely, which is reason enough for the current arrangement, but libstdc++ problems will likely only hit if the gap between system and being-built libraries is large enough (say, new symbols in the newer libstdc++ used by the compiler, but not available in the system library). I'm really uncomfortable with this change, especially at this stage. I'd much rather have a relatively obscure workaround for this relatively obscure problem, while keeping the defaults that have accumulated lots of testing on lots of configurations. An idea that occurred to me is to have some configure option or just a make variable that would be prepended to RPATH_ENVVAR, so that it would preempt TARGET_LIB_PATH. That would be a far more conservative change, that I think we could make even at this stage. WDYT? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] libcpp: Stabilize the location for macros restored after PCH load [PR105608]
This backport for gcc-13 is the second of two required for the g++.dg/pch/line-map-3.C test to stop hitting a variant of the known problem mentioned in that testcase: on riscv64-elf and riscv32-elf, after restoring the PCH, the location of the macros is mentioned as if they were on line 3 rather than 2, so even the existing xfails fail. I think this might be too much to backport, and I'm ready to use an xfail instead, but since this would bring more predictability, I thought I'd ask whether you'd find this backport acceptable. Regstrapped on x86_64-linux-gnu, along with other backports, and tested manually on riscv64-elf. Ok to install? From: Lewis Hyatt libcpp currently lacks the infrastructure to assign correct locations to macros that were defined prior to loading a PCH and then restored afterwards. While I plan to address that fully for GCC 15, this patch improves things by using at least a valid location, even if it's not the best one. Without this change, libcpp uses pfile->directive_line as the location for the restored macros, but this location_t applies to the old line map, not the one that was just restored from the PCH, so the resulting location is unpredictable and depends on what was stored in the line maps before. With this change, all restored macros get assigned locations at the line of the #include that triggered the PCH restore. A future patch will store the actual file name and line number of each definition and then synthesize locations in the new line map pointing to the right place. gcc/c-family/ChangeLog: PR preprocessor/105608 * c-pch.cc (c_common_read_pch): Adjust line map so that libcpp assigns a location to restored macros which is the same location that triggered the PCH include. libcpp/ChangeLog: PR preprocessor/105608 * pch.cc (cpp_read_state): Set a valid location for restored macros. (cherry picked from commit 019dc63819befb2b82077fb2d76b5dd670946f36) --- gcc/c-family/c-pch.cc | 23 +++ libcpp/pch.cc |9 - 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/gcc/c-family/c-pch.cc b/gcc/c-family/c-pch.cc index 9ee6f1790023c..d60972ba93084 100644 --- a/gcc/c-family/c-pch.cc +++ b/gcc/c-family/c-pch.cc @@ -318,6 +318,7 @@ c_common_read_pch (cpp_reader *pfile, const char *name, struct save_macro_data *smd; expanded_location saved_loc; bool saved_trace_includes; + int cpp_result; timevar_push (TV_PCH_RESTORE); @@ -343,20 +344,26 @@ c_common_read_pch (cpp_reader *pfile, const char *name, cpp_set_line_map (pfile, line_table); rebuild_location_adhoc_htab (line_table); line_table->trace_includes = saved_trace_includes; - linemap_add (line_table, LC_ENTER, 0, saved_loc.file, saved_loc.line); + + /* Set the current location to the line containing the #include (or the + #pragma GCC pch_preprocess) for the purpose of assigning locations to any + macros that are about to be restored. */ + linemap_add (line_table, LC_ENTER, 0, saved_loc.file, + saved_loc.line > 1 ? saved_loc.line - 1 : saved_loc.line); timevar_push (TV_PCH_CPP_RESTORE); - if (cpp_read_state (pfile, name, f, smd) != 0) -{ - fclose (f); - timevar_pop (TV_PCH_CPP_RESTORE); - goto end; -} - timevar_pop (TV_PCH_CPP_RESTORE); + cpp_result = cpp_read_state (pfile, name, f, smd); + /* Set the current location to the line following the #include, where we + were prior to processing the PCH. */ + linemap_line_start (line_table, saved_loc.line, 0); + timevar_pop (TV_PCH_CPP_RESTORE); fclose (f); + if (cpp_result != 0) +goto end; + /* Give the front end a chance to take action after a PCH file has been loaded. */ if (lang_post_pch_load) diff --git a/libcpp/pch.cc b/libcpp/pch.cc index a9f4ff19bf1e1..17e423f44b801 100644 --- a/libcpp/pch.cc +++ b/libcpp/pch.cc @@ -838,7 +838,14 @@ cpp_read_state (cpp_reader *r, const char *name, FILE *f, != NULL) { _cpp_clean_line (r); - if (!_cpp_create_definition (r, h, 0)) + + /* ??? Using r->line_table->highest_line is not ideal here, but we +do need to use some location that is relative to the new line +map just loaded, not the old one that was in effect when these +macros were lexed. The proper fix is to remember the file name +and line number where each macro was defined, and then add +these locations into the new line map. See PR105608. */ + if (!_cpp_create_definition (r, h, r->line_table->highest_line)) abort (); _cpp_pop_buffer (r); } -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and dive
[PATCH] libcpp: Improve location for macro names [PR66290]
This backport for gcc-13 is the first of two required for the g++.dg/pch/line-map-3.C test to stop hitting a variant of the known problem mentioned in that testcase: on riscv64-elf and riscv32-elf, after restoring the PCH, the location of the macros is mentioned as if they were on line 3 rather than 2, so even the existing xfails fail. I think this might be too much to backport, and I'm ready to use an xfail instead, but since this would bring more predictability, I thought I'd ask whether you'd find this backport acceptable. Regstrapped on x86_64-linux-gnu, along with other backports, and tested manually on riscv64-elf. Ok to install? From: Lewis Hyatt When libcpp reports diagnostics whose locus is a macro name (such as for -Wunused-macros), it uses the location in the cpp_macro object that was stored by _cpp_new_macro. This is currently set to pfile->directive_line, which contains the line number only and no column information. This patch changes the stored location to the src_loc for the token defining the macro name, which includes the location and range information. libcpp/ChangeLog: PR c++/66290 * macro.cc (_cpp_create_definition): Add location argument. * internal.h (_cpp_create_definition): Adjust prototype. * directives.cc (do_define): Pass new location argument to _cpp_create_definition. (do_undef): Stop passing inferior location to cpp_warning_with_line; the default from cpp_warning is better. (cpp_pop_definition): Pass new location argument to _cpp_create_definition. * pch.cc (cpp_read_state): Likewise. gcc/testsuite/ChangeLog: PR c++/66290 * c-c++-common/cpp/macro-ranges.c: New test. * c-c++-common/cpp/line-2.c: Adapt to check for column information on macro-related libcpp warnings. * c-c++-common/cpp/line-3.c: Likewise. * c-c++-common/cpp/macro-arg-count-1.c: Likewise. * c-c++-common/cpp/pr58844-1.c: Likewise. * c-c++-common/cpp/pr58844-2.c: Likewise. * c-c++-common/cpp/warning-zero-location.c: Likewise. * c-c++-common/pragma-diag-14.c: Likewise. * c-c++-common/pragma-diag-15.c: Likewise. * g++.dg/modules/macro-2_d.C: Likewise. * g++.dg/modules/macro-4_d.C: Likewise. * g++.dg/modules/macro-4_e.C: Likewise. * g++.dg/spellcheck-macro-ordering.C: Likewise. * gcc.dg/builtin-redefine.c: Likewise. * gcc.dg/cpp/Wunused.c: Likewise. * gcc.dg/cpp/redef2.c: Likewise. * gcc.dg/cpp/redef3.c: Likewise. * gcc.dg/cpp/redef4.c: Likewise. * gcc.dg/cpp/ucnid-11-utf8.c: Likewise. * gcc.dg/cpp/ucnid-11.c: Likewise. * gcc.dg/cpp/undef2.c: Likewise. * gcc.dg/cpp/warn-redefined-2.c: Likewise. * gcc.dg/cpp/warn-redefined.c: Likewise. * gcc.dg/cpp/warn-unused-macros-2.c: Likewise. * gcc.dg/cpp/warn-unused-macros.c: Likewise. (cherry picked from commit 4f3be7cbebce8ec9e0c5d9340b2772581454b862) --- gcc/testsuite/c-c++-common/cpp/line-2.c|2 gcc/testsuite/c-c++-common/cpp/line-3.c|2 gcc/testsuite/c-c++-common/cpp/macro-arg-count-1.c |4 gcc/testsuite/c-c++-common/cpp/macro-ranges.c | 52 ++ gcc/testsuite/c-c++-common/cpp/pr58844-1.c |4 gcc/testsuite/c-c++-common/cpp/pr58844-2.c |4 .../c-c++-common/cpp/warning-zero-location.c |2 gcc/testsuite/c-c++-common/pragma-diag-14.c|2 gcc/testsuite/c-c++-common/pragma-diag-15.c|2 gcc/testsuite/g++.dg/modules/macro-2_d.C |4 gcc/testsuite/g++.dg/modules/macro-4_d.C |4 gcc/testsuite/g++.dg/modules/macro-4_e.C |2 gcc/testsuite/g++.dg/spellcheck-macro-ordering.C |2 gcc/testsuite/gcc.dg/builtin-redefine.c| 18 - gcc/testsuite/gcc.dg/cpp/Wunused.c |6 gcc/testsuite/gcc.dg/cpp/redef2.c | 20 - gcc/testsuite/gcc.dg/cpp/redef3.c | 14 - gcc/testsuite/gcc.dg/cpp/redef4.c | 520 ++-- gcc/testsuite/gcc.dg/cpp/ucnid-11-utf8.c | 12 gcc/testsuite/gcc.dg/cpp/ucnid-11.c| 12 gcc/testsuite/gcc.dg/cpp/undef2.c |6 gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c| 10 gcc/testsuite/gcc.dg/cpp/warn-redefined.c | 10 gcc/testsuite/gcc.dg/cpp/warn-unused-macros-2.c|2 gcc/testsuite/gcc.dg/cpp/warn-unused-macros.c |2 libcpp/directives.cc | 13 - libcpp/internal.h |2 libcpp/macro.cc| 12 libcpp/pch.cc |2 29 files changed, 405 insertions(+), 342 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/cpp/macro-ranges.c diff --git a/gcc/testsuite/c-c++-common/cpp/line-2.c
[PATCH] RISC-V: Fix CTZ unnecessary sign extension [PR #106888]
This backport for gcc-13 is required for pr90838.c to get the expected count of andi instructions on riscv64-elf, rather than fail because of two extra andi insns in functions where it is not necessary. (On riscv32-elf, it passes). Regstrapped on x86_64-linux-gnu, along with other backports, and tested manually on riscv64-elf. Ok to install? From: Raphael Moreira Zinsly Changes since v1: - Remove subreg from operand 1. -- >8 -- We were not able to match the CTZ sign extend pattern on RISC-V because it gets optimized to zero extend and/or to ANDI patterns. For the ANDI case, combine scrambles the RTL and generates the extension by using subregs. gcc/ChangeLog: PR target/106888 * config/riscv/bitmanip.md (disi2): Match with any_extend. (disi2_sext): New pattern to match with sign extend using an ANDI instruction. gcc/testsuite/ChangeLog: PR target/106888 * gcc.target/riscv/pr106888.c: New test. * gcc.target/riscv/zbbw.c: Check for ANDI. (cherry picked from commit 9000da00dd70988f30d43806bae33b22ee6b9904) --- gcc/config/riscv/bitmanip.md | 13 - gcc/testsuite/gcc.target/riscv/pr106888.c | 12 gcc/testsuite/gcc.target/riscv/zbbw.c |1 + 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/riscv/pr106888.c diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md index 7aa591689ba87..cc55ca133c3fe 100644 --- a/gcc/config/riscv/bitmanip.md +++ b/gcc/config/riscv/bitmanip.md @@ -246,13 +246,24 @@ (define_insn "*si2" (define_insn "*disi2" [(set (match_operand:DI 0 "register_operand" "=r") -(sign_extend:DI +(any_extend:DI (clz_ctz_pcnt:SI (match_operand:SI 1 "register_operand" "r"] "TARGET_64BIT && TARGET_ZBB" "w\t%0,%1" [(set_attr "type" "bitmanip") (set_attr "mode" "SI")]) +;; A SImode clz_ctz_pcnt may be extended to DImode via subreg. +(define_insn "*disi2_sext" + [(set (match_operand:DI 0 "register_operand" "=r") +(and:DI (subreg:DI + (clz_ctz_pcnt:SI (match_operand:SI 1 "register_operand" "r")) 0) + (match_operand:DI 2 "const_int_operand")))] + "TARGET_64BIT && TARGET_ZBB && ((INTVAL (operands[2]) & 0x3f) == 0x3f)" + "w\t%0,%1" + [(set_attr "type" "bitmanip") + (set_attr "mode" "SI")]) + (define_insn "*di2" [(set (match_operand:DI 0 "register_operand" "=r") (clz_ctz_pcnt:DI (match_operand:DI 1 "register_operand" "r")))] diff --git a/gcc/testsuite/gcc.target/riscv/pr106888.c b/gcc/testsuite/gcc.target/riscv/pr106888.c new file mode 100644 index 0..77fb8e5b79c6b --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr106888.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gc_zbb -mabi=lp64" } */ + +int +ctz (int i) +{ + int res = __builtin_ctz (i); + return res&0x; +} + +/* { dg-final { scan-assembler-times "ctzw" 1 } } */ +/* { dg-final { scan-assembler-not "andi" } } */ diff --git a/gcc/testsuite/gcc.target/riscv/zbbw.c b/gcc/testsuite/gcc.target/riscv/zbbw.c index 709743c3b6807..f7b2b63853f40 100644 --- a/gcc/testsuite/gcc.target/riscv/zbbw.c +++ b/gcc/testsuite/gcc.target/riscv/zbbw.c @@ -23,3 +23,4 @@ popcount (int i) /* { dg-final { scan-assembler-times "clzw" 1 } } */ /* { dg-final { scan-assembler-times "ctzw" 1 } } */ /* { dg-final { scan-assembler-times "cpopw" 1 } } */ +/* { dg-final { scan-assembler-not "andi\t" } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] RISC-V: Fix error combine of pred_mov pattern
vd, m,vr,vr") +(if_then_else:V + (unspec: +[(match_operand: 1 "vector_mask_operand" "vmWc1, Wc1, vm, vmWc1, Wc1, Wc1") + (match_operand 4 "vector_length_operand" " rK,rK, rK,rK,rK,rK") + (match_operand 5 "const_int_operand" "i, i, i, i, i, i") + (match_operand 6 "const_int_operand" "i, i, i, i, i, i") + (match_operand 7 "const_int_operand" "i, i, i, i, i, i") + (reg:SI VL_REGNUM) + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) + (match_operand:V 3 "reg_or_mem_operand" "m, m, m,vr,vr,vr") + (match_operand:V 2 "vector_merge_operand""0,vu, vu,vu,vu, 0")))] + "(TARGET_VECTOR +&& (register_operand (operands[0], mode) +|| register_operand (operands[3], mode)))" "@ vle.v\t%0,%3%p1 vle.v\t%0,%3 vle.v\t%0,%3,%1.t vse.v\t%3,%0%p1 vmv.v.v\t%0,%3 - vmv.v.v\t%0,%3 - vmv.v.i\t%0,%v3 - vmv.v.i\t%0,%v3" + vmv.v.v\t%0,%3" "&& register_operand (operands[0], mode) && register_operand (operands[3], mode) && satisfies_constraint_vu (operands[2]) && INTVAL (operands[7]) == riscv_vector::VLMAX" [(set (match_dup 0) (match_dup 3))] "" - [(set_attr "type" "vlde,vlde,vlde,vste,vimov,vimov,vimov,vimov") + [(set_attr "type" "vlde,vlde,vlde,vste,vimov,vimov") (set_attr "mode" "")]) ;; Dedicated pattern for vse.v instruction since we can't reuse pred_mov pattern to include @@ -1367,6 +1359,26 @@ (define_insn "*pred_broadcast_zero" [(set_attr "type" "vimovxv,vimovxv") (set_attr "mode" "")]) +;; Because (vec_duplicate imm) will be converted to (const_vector imm), +;; This pattern is used to handle this case. +(define_insn "*pred_broadcast_imm" + [(set (match_operand:V 0 "register_operand" "=vr,vr") +(if_then_else:V + (unspec: +[(match_operand: 1 "vector_all_trues_mask_operand" " Wc1, Wc1") + (match_operand 4 "vector_length_operand" " rK, rK") + (match_operand 5 "const_int_operand" "i, i") + (match_operand 6 "const_int_operand" "i, i") + (match_operand 7 "const_int_operand" "i, i") + (reg:SI VL_REGNUM) + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) + (match_operand:V 3 "vector_const_int_or_double_0_operand" "viWc0, viWc0") + (match_operand:V 2 "vector_merge_operand" " vu, 0")))] + "TARGET_VECTOR" + "vmv.v.i\t%0,%v3" + [(set_attr "type" "vimov,vimov") + (set_attr "mode" "")]) + ;; --- ;; Predicated Strided loads/stores ;; ------- diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr110943.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110943.c new file mode 100644 index 0..8a6c00fc94d29 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr110943.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -march=rv64gcv -mabi=lp64d" } */ +/* { dg-final { check-function-bodies "**" "" } } */ + +#include + +/* +** foo9: +** vsetivli\tzero,1,e64,m2,t[au],m[au] +** ... +** vs2r.v\tv[0-9]+,0\([a-x0-9]+\) +** ret +*/ +void foo9 (void *base, void *out, size_t vl) +{ +int64_t scalar = *(int64_t*)(base + 100); +vint64m2_t v = __riscv_vmv_v_x_i64m2 (0, 1); +*(vint64m2_t*)out = v; +} + +/* +** foo10: +** vsetivli\tzero,1,e64,m2,t[au],m[au] +** ... +** vs2r.v\tv[0-9]+,0\([a-x0-9]+\) +** ret +*/ +void foo10 (void *base, void *out, size_t vl) +{ +int64_t scalar = *(int64_t*)(base + 100); +vint64m2_t v = __riscv_vmv_s_x_i64m2 (0, 1); +*(vint64m2_t*)out = v; +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] RISC-V: Revert the convert from vmv.s.x to vmv.v.i
*/ @@ -161,7 +161,7 @@ void foo11 (void *base, void *out, size_t vl) /* ** foo12: ** ... -** vmv.v.i\tv[0-9]+,\s*0 +** vmv.s.x\tv[0-9]+,\s*zero ** ... ** ret */ @@ -172,6 +172,20 @@ void foo12 (void *base, void *out, size_t vl) *(vfloat64m2_t*)out = v; } +/* +** foo12_1: +** ... +** vfmv.s.f\tv[0-9]+,\s*[a-x0-9]+ +** ... +** ret +*/ +void foo12_1 (void *base, void *out, size_t vl) +{ +vfloat64m2_t merge = *(vfloat64m2_t*) (base + 200); +vfloat64m2_t v = __riscv_vfmv_s_f_f64m2_tu (merge, 0.2, vl); +*(vfloat64m2_t*)out = v; +} + /* ** foo13: ** ... -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] RISC-V: Fix riscv/arch-19.c with different ISA spec version
This testcase is failing with riscv64-elf and riscv32-elf in the gcc-13 branch, if configured to use an assembler that supports -misa-spec; with an assembler that doesn't, the test passes both with and without the following backport from the trunk, so I'd like to install it in gcc-13. Regstrapped on x86_64-linux-gnu, along with other backports, and tested manually on riscv64-elf. Ok to install? From: Kito Cheng In newer ISA spec, F will implied zicsr, add that into -march option to prevent different test result on different default -misa-spec version. gcc/testsuite/ * gcc.target/riscv/arch-19.c: Add -misa-spec. (cherry picked from commit 9fde76a3be8e1717d9d38492c40675e742611e45) --- gcc/testsuite/gcc.target/riscv/arch-19.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.target/riscv/arch-19.c b/gcc/testsuite/gcc.target/riscv/arch-19.c index b042e1a49fe6f..95204ede26a69 100644 --- a/gcc/testsuite/gcc.target/riscv/arch-19.c +++ b/gcc/testsuite/gcc.target/riscv/arch-19.c @@ -1,4 +1,4 @@ /* { dg-do compile } */ -/* { dg-options "-march=rv64if_zfinx -mabi=lp64" } */ +/* { dg-options "-march=rv64if_zicsr_zfinx -mabi=lp64" } */ int foo() {} -/* { dg-error "'-march=rv64if_zfinx': z\\*inx conflicts with floating-point extensions" "" { target *-*-* } 0 } */ +/* { dg-error "'-march=rv64if_zicsr_zfinx': z\\*inx conflicts with floating-point extensions" "" { target *-*-* } 0 } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] gcc/Makefile.in: Fix install-info target if BUILD_INFO is empty
Hello, Christophe, On Feb 10, 2024, Christophe Lyon wrote: > gcc/ > * Makefile.in: Add no-info dependency. > * configure.ac: Set BUILD_INFO=no-info if makeinfo is not > available. > * configure: Regenerate. Thank you, this is ok. Now, this doesn't fix a regression, does it? I would support putting this in for GCC 14, but I would be overstepping my authority if I approved even such a small and well-contained improvement patch in the current stage, so I'm approving it for stage1, but maybe some global maintainer or release manager will chime in in support for earlier merging? (hint, hint ;-) -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]
On Feb 6, 2024, Jason Merrill wrote: > Reverting that hunk of the change fixed my problem with bubblestrapping GCC > 12 with ccache on a host with a newer system libstdc++. Did you have libcc1, gnattools and gotools enabled in your testing? Those non-bootstrapped tools are most likely to be problem spots. I have a vague recollection that, in native scenarios, when bootstrapping but not when not boostrapping, they are (or used to be?) built using the just-built (last-stage) compiler and target libraries, rather than system tools or previous stage (which I believe would fail bootstrap-lean). Just by looking at the current code I can't bring back to mind all the involved moving parts :-/ but my guess is that the need for TARGET_LIB_PATH comes about for this combination of circumstances, in which we would need to use the just-built native-target libstdc++ to link or to run these host tools using the just-built tools. Having a newer system libstdc++ would, I suspect, unfortunately mask the problem that those who had an older or no system libstdc++ would likely run into. OTOH, if the system linker requires the newer system library, and ld.so and ld overload LD_LIBRARY_PATH for both the libraries loaded for ld to run, the libraries ld searches to link executables, and the libraries loaded for so-linked programs to run, we seem to need some means to distinguish between these three circumstances so as to be able to set LD_LIBRARY_PATH correctly for each one, whether the libstdc++ we're building is newer or older than the system one. And there's an added complication that any single ld invocation involves two different and potentially conflicting uses of LD_LIBRARY_PATH. /me runs screaming :-) Perhaps the best we can do out of these conflicting requirements is to somehow detect a system libstdc++ newer than the one we're about to build, and fail early if we find that some of the tools depend on the newer libstdc++, or disable this env var setting for this case only and hope the newer system library is compatible enough. Holy nightmare, Batman! :-) :-( Perhaps the way to go is an explicit configure setting, recommended by the fail early, to disable the env var setting? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] gcc/Makefile.in: Fix install-info target if BUILD_INFO is empty
Hello, Christophe, Thanks for the patch. On Feb 5, 2024, Christophe Lyon wrote: > In order to save build time, our CI overrides BUILD_INFO="", which > works when invoking 'make all' but not for 'make install' in case some > info files need an update. Hmm, I don't think this would be desirable. We ship updated info files in release tarballs, and it would be desirable to install them even if makeinfo is not available in the build environment. > I noticed this when testing a patch posted on the gcc-patches list, > leading to an error at 'make install' time after updating tm.texi (the > build reported 'new text' in tm.texi and stopped). This is because > 'install' depends on 'install-info', which depends on > $(DESTDIR)$(infodir)/gccint.info (among others). Ideally, we'd detect and report info files that are out-of-date WRT their ultimate sources, especially to catch tm.texi.in changes, but doing so only at install time is clearly suboptimal. I mean, if we don't have the tools to build info files, it's fine if we skip their building, and even refrain from installing info files that are missing or outdated, but we should install prebuilt ones if they're available, and we should probably *not* refrain from trying to satisfy the dependencies for info files at build time, even if it turns out that we can't build the info files themselves. This suggests to me that, rather than setting BUILD_INFO to the empty string, we should set it to e.g. no-info, so that $(MAKEINFO) will not be run because x$(BUILD_INFO) != xinfo, but so that we still get the dependencies resolved, e.g. by making no-info depend on info. Or maybe make it info-check-deps, and insert that between info and its current deps. WDYT? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
Re: [PATCH] contrib: Fill in HOST{CC,CFLAGS,CXX,CXXFLAGS} in test_installed
On Feb 5, 2024, Jakub Jelinek wrote: > * test_installed: Fill in HOSTCC, HOSTCXX, HOSTCFLAGS and > HOSTCXXFLAGS. LGTM, thanks, -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100]
On Jan 19, 2024, Alexandre Oliva wrote: > On Jan 18, 2024, "Kewen.Lin" wrote: >> Not sure if I missed something in the testing, could you >> kindly double check if those test cases started to fail from r14-6275 on your >> env? > My guess is that they started to fail when David attempted to bypass the > strub tests by changing the dg proc that detects strub support. The > tests then detected the mismatch between the result of the proc and the > expected errors when strub is disabled properly. The patch below, that realigns stack scrubbing on sparc32 and sparc64, is still pending review (Ping?), but since your fix for ppc (that worsened scrubbing on sparc32) has long been in, I'm pushing the reversal of commit 9773ca519864e2e0706424b805c3314f1fbe7d10. > strub: introduce STACK_ADDRESS_OFFSET > Since STACK_POINTER_OFFSET is not necessarily at the boundary between > caller- and callee-owned stack, as desired by > __builtin_stack_address(), and using it as if it were or not causes > problems, introduce a new macro so that ports can define it suitably, > without modifying STACK_POINTER_OFFSET. > for gcc/ChangeLog > PR middle-end/112917 > PR middle-end/113100 > * builtins.cc (expand_builtin_stack_address): Use > STACK_ADDRESS_OFFSET. > * doc/extend.texi (__builtin_stack_address): Adjust. > * config/sparc/sparc.h (STACK_ADDRESS_OFFSET): Define. > * doc/tm.texi.in (STACK_ADDRESS_OFFSET): Document. > * doc/tm.texi.in: Rebuilt. Ping? https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643408.html -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] strub: drop nonaliased parm from build_ref_type_for [PR113394]
Variant type copies can't have their own alias sets any more, and it's not like setting them affected the pointed-to objects anyway. Regstrapped on x86_64-linux-gnu; also bootstrapped with -fstrub=internal enabled by default. Ok to install? for gcc/ChangeLog PR debug/113394 * ipa-strub.cc (build_ref_type_for): Drop nonaliased. Adjust caller. for gcc/testsuite/ChangeLog PR debug/113394 * gcc.dg/strub-internal-pr113394.c: New. --- gcc/ipa-strub.cc | 31 gcc/testsuite/gcc.dg/strub-internal-pr113394.c |6 + 2 files changed, 11 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/strub-internal-pr113394.c diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index e1b07016b8879..0ee063c9eddc4 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2052,36 +2052,17 @@ walk_regimplify_phi (gphi *stmt) return needs_commit; } -/* Create a reference type to use for PARM when turning it into a reference. - NONALIASED causes the reference type to gain its own separate alias set, so - that accessing the indirectly-passed parm won'will not add aliasing - noise. */ +/* Create a reference type to use for PARM when turning it into a + reference. */ static tree -build_ref_type_for (tree parm, bool nonaliased = true) +build_ref_type_for (tree parm) { gcc_checking_assert (TREE_CODE (parm) == PARM_DECL); tree ref_type = build_reference_type (TREE_TYPE (parm)); - if (!nonaliased) -return ref_type; - - /* Each PARM turned indirect still points to the distinct memory area at the - wrapper, and the reference in unchanging, so we might qualify it, but... - const is not really important, since we're only using default defs for the - reference parm anyway, and not introducing any defs, and restrict seems to - cause trouble. E.g., libgnat/s-concat3.adb:str_concat_3 has memmoves that, - if it's wrapped, the memmoves are deleted in dse1. Using a distinct alias - set seems to not run afoul of this problem, and it hopefully enables the - compiler to tell the pointers do point to objects that are not otherwise - aliased. */ - tree qref_type = build_variant_type_copy (ref_type); - - TYPE_ALIAS_SET (qref_type) = new_alias_set (); - record_alias_subset (TYPE_ALIAS_SET (qref_type), get_alias_set (ref_type)); - - return qref_type; + return ref_type; } /* Add cgraph edges from current_function_decl to callees in SEQ with frequency @@ -2909,9 +2890,7 @@ pass_ipa_strub::execute (function *) if with transparent reference, and the wrapper doesn't take any extra parms that could point into wrapper's parms. So we can probably drop the TREE_ADDRESSABLE and keep the TRUE. */ - tree ref_type = build_ref_type_for (nparm, - true - || !TREE_ADDRESSABLE (parm)); + tree ref_type = build_ref_type_for (nparm); DECL_ARG_TYPE (nparm) = TREE_TYPE (nparm) = ref_type; relayout_decl (nparm); diff --git a/gcc/testsuite/gcc.dg/strub-internal-pr113394.c b/gcc/testsuite/gcc.dg/strub-internal-pr113394.c new file mode 100644 index 0..d21c209cdf699 --- /dev/null +++ b/gcc/testsuite/gcc.dg/strub-internal-pr113394.c @@ -0,0 +1,6 @@ +/* { dg-do compile } */ +/* { dg-options "-fstrub=internal -g" } */ + +/* gcc.c-torture/compile/20020210-1.c */ +/* PR c/5615 */ +void f(int a, struct {int b[a];} c) {} /* { dg-warning "anonymous struct" } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] aarch64: enforce lane checking for intrinsics
On Jan 23, 2024, Richard Sandiford wrote: > Performing the check in expand is itself wrong *nod* > So I think we should enforce the immediate range within the frontend > instead, via TARGET_CHECK_BUILTIN_CALL. Sounds good. Can that accommodate the existing uses in always_inline wrappers? > Unfortunately that isn't suitable for stage 4 though. ACK. Is there a partial implementation of that? I might get a chance to take it to completion, even if it doesn't make gcc 14. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] testsuite: no dfp run without dfprt
On Jan 24, 2024, Jeff Law wrote: > OK Thanks. FTR, there were typos (s/ṕ/p/g) and a missing entry for builtin-snan-1.c in the ChangeLog entries, that the ChangeLog checker kindly pointed out. Fixed below, just pushed along with the otherwise-unchanged patch as r14-8505. for gcc/testsuite/ChangeLog * c-c++-common/dfp/pr36800.c: Drop dg-do overrider. * c-c++-common/dfp/pr39034.c: Likewise. * c-c++-common/dfp/pr39035.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d32-1.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d32-2.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d64-1.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d64-2.c: Likewise. * gcc.dg/dfp/builtin-snan-1.c: Likewise. * gcc.dg/dfp/builtin-tgmath-dfp.c: Likewise. * gcc.dg/dfp/c23-float-dfp-4.c: Likewise. * gcc.dg/dfp/c23-float-dfp-5.c: Likewise. * gcc.dg/dfp/c23-float-dfp-6.c: Likewise. * gcc.dg/dfp/c23-float-dfp-7.c: Likewise. * gcc.dg/dfp/pr108068.c: Likewise. * gcc.dg/dfp/pr97439.c: Likewise. * g++.dg/compat/decimal/pass-1_main.C: Require dfprt. * g++.dg/compat/decimal/pass-2_main.C: Likewise. * g++.dg/compat/decimal/pass-3_main.C: Likewise. * g++.dg/compat/decimal/pass-4_main.C: Likewise. * g++.dg/compat/decimal/pass-5_main.C: Likewise. * g++.dg/compat/decimal/pass-6_main.C: Likewise. * g++.dg/compat/decimal/return-1_main.C: Likewise. * g++.dg/compat/decimal/return-2_main.C: Likewise. * g++.dg/compat/decimal/return-3_main.C: Likewise. * g++.dg/compat/decimal/return-4_main.C: Likewise. * g++.dg/compat/decimal/return-5_main.C: Likewise. * g++.dg/compat/decimal/return-6_main.C: Likewise. * g++.dg/eh/dfp-1.C: Likewise. * g++.dg/eh/dfp-2.C: Likewise. * g++.dg/eh/dfp-saves-aarch64.C: Likewise. * gcc.c-torture/execute/pr80692.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d128-1.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d128-2.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d128-3.c: Likewise. * gcc.dg/dfp/bid-non-canonical-d128-4.c: Likewise. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] arm: fix c23 0-named-args caller-side stdarg
On Dec 5, 2023, Alexandre Oliva wrote: > arm: fix c23 0-named-args caller-side stdarg Ping? https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639472.html > The commit message doesn't name explicitly the fixed testsuite > failures. Here they are: > FAIL: gcc.dg/c23-stdarg-4.c execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O0 execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O1 execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O2 execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O2 -flto -fno-use-linker-plugin > -flto-partition=none execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O2 -flto -fuse-linker-plugin > -fno-fat-lto-objects execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -O3 -g execution test > FAIL: gcc.dg/torture/c23-stdarg-split-1a.c -Os execution test > Tested on arm-eabi. Ok to install? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] testsuite: require libc sym for -shared
Targets whose binutils support -shared, but that don't have a shared libc, and that can't add PDC (non-PIC) to shared libraries, may succeed at the effective target test for -shared, because it brings nothing from libc, but tests that rely on -shared and that use bits from libc, such as g++.dg/lto/pr108772, fail despite requiring the shared effective target. Extend the effective target test to bring malloc() from libc, that's likely to be present in libc and bring a substantial amount of code if no shared libc is available. Regstrapped on x86_64-linux-gnu, also tested on aarch64-elf with gcc-13, where the problem was observed. Ok to install? for gcc/testsuite/ChangeLog * lib/target-supports.exp (check_effective_target_shared): Check for a static-only libc. --- gcc/testsuite/lib/target-supports.exp | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 73360cd3a0d55..213dad355a6a5 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -1391,10 +1391,16 @@ proc check_effective_target_aarch64_tlsle32 { } { proc check_effective_target_shared { } { # Note that M68K has a multilib that supports -fpic but not # -fPIC, so we need to check both. We test with a program that -# requires GOT references. +# requires GOT references, and with a libc symbol that would +# bring in significant parts of a static-only libc. Absent a +# shared libc, this would make -shared tests fail, so we don't +# want to enable the shared effective target then. return [check_no_compiler_messages shared executable { + #include extern int foo (void); extern int bar; - int baz (void) { return foo () + bar; } + char *baz (void) { + return foo () + (char*) malloc (bar); + } } "-shared -fpic"] } -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] testsuite: no dfp run without dfprt
9002022 100644 --- a/gcc/testsuite/gcc.dg/dfp/pr108068.c +++ b/gcc/testsuite/gcc.dg/dfp/pr108068.c @@ -1,5 +1,4 @@ /* PR tree-optimization/108068 */ -/* { dg-do run } */ /* { dg-options "-O2" } */ int diff --git a/gcc/testsuite/gcc.dg/dfp/pr97439.c b/gcc/testsuite/gcc.dg/dfp/pr97439.c index 7fcf834043cb0..c651ec22e4314 100644 --- a/gcc/testsuite/gcc.dg/dfp/pr97439.c +++ b/gcc/testsuite/gcc.dg/dfp/pr97439.c @@ -1,4 +1,3 @@ -// { dg-do run } // { dg-options "-O1" } static int -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] aarch64: enforce lane checking for intrinsics
); - tree attrs = aarch64_get_attributes (d->flags, d->mode); + tree attrs = aarch64_get_attributes (d->flags, d->mode, lane_check); if (called_from_pragma) { -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100]
ning it like +@code{STACK_POINTER_OFFSET} may be appropriate for many machines, but +not all. + +On SPARC, for example, the register save area is *not* considered active +or used by the active function, but rather as akin to the area in which +call-preserved registers are saved by callees, so the stack address is +above that area, even though the (unbiased) stack pointer points below +it. This enables @code{__strub_leave} to clear what would otherwise +overlap with its own register save area. + +On PowerPC, @code{STACK_POINTER_OFFSET} also reserves space for a save +area, but that area is used by the caller rather than the callee, so the +boundary address is below it. + +If the address is computed too high or too low, parts of a stack range +that should be scrubbed may be left unscrubbed, scrubbing may corrupt +active portions of the stack frame, and stack ranges may be +doubly-scrubbed by caller and callee. +@end defmac + @defmac TARGET_STRUB_USE_DYNAMIC_ARRAY If defined to nonzero, @code{__strub_leave} will allocate a dynamic array covering the stack range that needs scrubbing before clearing it. diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 21343d4d1bf2f..658e1e63371e5 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -2688,6 +2688,35 @@ may reduce the size of debug information on some ports. @hook TARGET_HAVE_STRUB_SUPPORT_FOR +@defmac STACK_ADDRESS_OFFSET +Offset from the stack pointer register to the boundary address between +the stack area claimed by an active function, and stack ranges that +could get clobbered if it called another function. It should NOT +encompass any stack red zone, that is used in leaf functions. + +This value is added to the stack pointer register to compute the address +returned by @code{__builtin_stack_address}, and this is its only use. +If this macro is not defined, no offset is added. Defining it like +@code{STACK_POINTER_OFFSET} may be appropriate for many machines, but +not all. + +On SPARC, for example, the register save area is *not* considered active +or used by the active function, but rather as akin to the area in which +call-preserved registers are saved by callees, so the stack address is +above that area, even though the (unbiased) stack pointer points below +it. This enables @code{__strub_leave} to clear what would otherwise +overlap with its own register save area. + +On PowerPC, @code{STACK_POINTER_OFFSET} also reserves space for a save +area, but that area is used by the caller rather than the callee, so the +boundary address is below it. + +If the address is computed too high or too low, parts of a stack range +that should be scrubbed may be left unscrubbed, scrubbing may corrupt +active portions of the stack frame, and stack ranges may be +doubly-scrubbed by caller and callee. +@end defmac + @defmac TARGET_STRUB_USE_DYNAMIC_ARRAY If defined to nonzero, @code{__strub_leave} will allocate a dynamic array covering the stack range that needs scrubbing before clearing it. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100]
David, On Jan 7, 2024, "Kewen.Lin" wrote: > As PR113100 shows, the unbiasing introduced by r14-6737 can > cause the scrubbing to overrun and screw some critical data > on stack like saved toc base consequently cause segfault on > Power. I suppose this problem that Kewen fixed (thanks) was what caused you to install commit r14-6838. According to posted test results, strub worked on AIX until Dec 20, when the fixes for sparc that broke strub on ppc went in. I can't seem to find the email in which you posted the patch, and I'd have appreciated if you'd copied me. I wouldn't have missed it for so long if you had. Since I couldn't find that patch, I'm responding in this thread instead. The r14-6838 patch is actually very very broken. Disabling strub on a target is not a matter of changing only the testsuite. Your additions to the tests even broke the strub-unsupported testcases, that tested exactly the feature that enables ports to disable strub in a way that informs users in case they attempt to use it. I'd thus like to revert that patch. Kewen's patch needs a little additional cleanup, that I'm preparing now, to restore fully-functioning strub on sparc32. Please let me know in case you observe any other problems related with strub. I'd be happy to fix them, but I can only do so once I'm aware of them. In case the reversal or the upcoming cleanup has any negative impact, please make sure you let me know. Thanks, Happy GNU Year! -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100]
On Jan 12, 2024, "Kewen.Lin" wrote: >>> By checking PR112917, IMHO we should keep this unbiasing >>> guarded under SPARC_STACK_BOUNDARY_HACK (TARGET_ARCH64 && >>> TARGET_STACK_BIAS), similar to some existing code special >>> treating SPARC stack bias. >> >> I'm afraid this change will most certainly regress 32-bit sparc, because >> of the large register save area. > Oh, I read the comments and commit logs in PR112917, mainly > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112917#{c4,c5,c6}, > and the "sparc64" in subject of commit r14-6737 also implies > that this unbiasing is only required for sparc64, so I thought > it should be safe to guard with SPARC_STACK_BOUNDARY_HACK. It is safe, in a way, because that protects potentially active stack areas, but it's unsafe in that it may leak data that stack scrubbing was supposed to scrub. There's no conservative solution here, alas; we have to get it just right. Specifically on sparc32, if __builtin_scrub_leave allocated its own frame (it doesn't) with the large register-save area for its potential (but inexistent) callees to use, it could overlap with a large chunk of the very stack frame that it's supposed to clear. Unfortunately, this is slowly drifting away from the notion of stack address. I mean, all of the following could conceivably be returned by __builtin_stack_address: - the (biased) stack pointer - the address of the red zone - the unbiased stack pointer - the address of the save area reserved by callees for potential callees - the boundary between caller- and callee-used stack space The last one is what we need for stack scrubbing, so that's what I'm planning to implement, but I'm pondering whether to change __builtin_stack_address() to take an extra argument to select among the different possibilities, or of other means to query these various offsets. It feels like overthinking, so I'm trying to push these thoughts aside, but... Does anyone think that would be a desirable feature? We can always add it later. >> ISTM that PPC sets up a save area between the outgoing args and the > Yes, taking 64-bit PowerPC ELF abi 1.9 as example: *nod*, and that's a caller-used save area, as opposed to sparc's callee-used save area. Whereas the caller-used area needs to be preserved across a call, the callee-used one could conceivably even be used as scratch space by the caller. > Nice, thanks! Welcome back. :-) Thank you! -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100]
On Jan 7, 2024, "Kewen.Lin" wrote: > As PR113100 shows, the unbiasing introduced by r14-6737 can > cause the scrubbing to overrun and screw some critical data > on stack like saved toc base consequently cause segfault on > Power. Ugh. Sorry about the breakage, and thanks for addressing it during my absence. Happy GNU Year! :-) > By checking PR112917, IMHO we should keep this unbiasing > guarded under SPARC_STACK_BOUNDARY_HACK (TARGET_ARCH64 && > TARGET_STACK_BIAS), similar to some existing code special > treating SPARC stack bias. I'm afraid this change will most certainly regress 32-bit sparc, because of the large register save area. I had been hesitant to introduce yet another target configuration knob, but it looks like this is what we're going to have to do to accommodate all targets. > I also expect the culprit commit can > affect those ports with nonzero STACK_POINTER_OFFSET. IMHO it really shouldn't. STACK_POINTER_OFFSET should be the "Offset from the stack pointer register to the first location at which outgoing arguments are placed", which suggests to me that no data that the callee couldn't change should go in the area below (or above) %sp+S_P_O. ISTM that PPC sets up a save area between the outgoing args and the stack pointer; I don't think that's very common, but I suppose other targets that do so would also define STACK_POINTER_OFFSET to nonzero so as to reserve those bits. But whether they should be cleared by stack scrubbing, as on sparc, or preserved, as on ppc, depends on the ABI conventions, so we probably can't help yet another knob :-/ I'll take care of that, and update the corresponding documentation. Thanks, -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH 2/2 FYI] -finline-stringops: drop obsolete comment [PR112778]
On Dec 11, 2023, Richard Biener wrote: > On Sat, Dec 9, 2023 at 8:05 AM Alexandre Oliva wrote: >> PR target/112778 >> * builtins.cc (can_store_by_multiple_pieces): New. >> (try_store_by_multiple_pieces): Call it. >> +/* Check that store_by_pieces allows BITS + LEN (so that we don't >> + expand something too unreasonably long), and every power of 2 in >> + BITS. It is assumed that LEN has already been tested by >> + itself. */ >> +static bool >> +can_store_by_multiple_pieces (unsigned HOST_WIDE_INT bits, When fixing the PR, I failed to remove the comment that raised the very concern that the PR confirmed, and that the earlier patch for the PR fixed. I'm checking this in as obvious. for gcc/ChangeLog PR target/112778 * builtins.cc (try_store_by_multiple_pieces): Drop obsolete comment. --- gcc/builtins.cc |4 1 file changed, 4 deletions(-) diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 0f64feeedbad6..125ea158ebfad 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -4491,10 +4491,6 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len, if (max_len >> max_bits > min_len >> max_bits) tst_bits = max_bits; } - /* ??? Do we have to check that all powers of two lengths from - max_bits down to ctz_len pass can_store_by_pieces? As in, could - it possibly be that xlenest passes while smaller power-of-two - sizes don't? */ by_pieces_constfn constfun; void *constfundata; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] untyped calls: enable target switching [PR112334]
On Dec 20, 2023, Jeff Law wrote: > Sorry it was meant to be an ACK for the trunk for both patches. Aah, I see. So this is what I installed last night, upon seeing your message. untyped calls: use wrapper class type for implicit plus_one Instead of get and set macros to apply a delta, use a single macro that resorts to a temporary wrapper class to apply it. for gcc/ChangeLog * builtins.cc (delta_type): New template class. (set_apply_args_size, get_apply_args_size): Replace with... (saved_apply_args_size): ... this. (set_apply_result_size, get_apply_result_size): Replace with... (saved_apply_result_size): ... this. (apply_args_size, apply_result_size): Adjust. --- gcc/builtins.cc | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 7c2732ab79e6f..23cc6b6838a01 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -1403,16 +1403,24 @@ get_memory_rtx (tree exp, tree len) /* Built-in functions to perform an untyped call and return. */ -#define set_apply_args_size(x) \ - (this_target_builtins->x_apply_args_size_plus_one = 1 + (x)) -#define get_apply_args_size() \ - (this_target_builtins->x_apply_args_size_plus_one - 1) +/* Wrapper that implicitly applies a delta when getting or setting the + enclosed value. */ +template +class delta_type +{ + T T const delta; +public: + delta_type (T , T dlt) : value (val), delta (dlt) {} + operator T () const { return value + delta; } + T operator = (T val) const { value = val - delta; return val; } +}; + +#define saved_apply_args_size \ + (delta_type (this_target_builtins->x_apply_args_size_plus_one, -1)) #define apply_args_mode \ (this_target_builtins->x_apply_args_mode) -#define set_apply_result_size(x) \ - (this_target_builtins->x_apply_result_size_plus_one = 1 + (x)) -#define get_apply_result_size() \ - (this_target_builtins->x_apply_result_size_plus_one - 1) +#define saved_apply_result_size \ + (delta_type (this_target_builtins->x_apply_result_size_plus_one, -1)) #define apply_result_mode \ (this_target_builtins->x_apply_result_mode) @@ -1422,7 +1430,7 @@ get_memory_rtx (tree exp, tree len) static int apply_args_size (void) { - int size = get_apply_args_size (); + int size = saved_apply_args_size; int align; unsigned int regno; @@ -1456,7 +1464,7 @@ apply_args_size (void) else apply_args_mode[regno] = as_a (VOIDmode); - set_apply_args_size (size); + saved_apply_args_size = size; } return size; } @@ -1467,7 +1475,7 @@ apply_args_size (void) static int apply_result_size (void) { - int size = get_apply_result_size (); + int size = saved_apply_result_size; int align, regno; /* The values computed by this function never change. */ @@ -1500,7 +1508,7 @@ apply_result_size (void) size = APPLY_RESULT_SIZE; #endif - set_apply_result_size (size); + saved_apply_result_size = size; } return size; } -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: Ping Re: [PATCH] contrib: add git gcc-style alias
On Dec 19, 2023, Jason Merrill wrote: > On 12/11/23 22:00, Jason Merrill wrote: >> OK for trunk? > Ping. CCing Alex because this could plausibly be considered build > machinery, and he's had useful feedback on my sh code before. LGTM, thanks! -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH FYI] www: new AdaCore-contributed hardening features in gcc 13 and 14
On Nov 30, 2023, Richard Biener wrote: >> >> Here are changes.html entries for this and for the other newly-added >> >> features: >> >> > LGTM. (sorry, I should be following up two messages upthread, but I don't seem to have saved that one) I've finally put in the www changes. Mention hardening of conditionals (added in gcc 13), control flow redundancy, hardened booleans, and stack scrubbing. Also cover forced inlining of string operations while at that. --- htdocs/gcc-13/changes.html |6 ++ htdocs/gcc-14/changes.html | 29 + 2 files changed, 35 insertions(+) diff --git a/htdocs/gcc-13/changes.html b/htdocs/gcc-13/changes.html index ee6383a095706..d3bacc167cd30 100644 --- a/htdocs/gcc-13/changes.html +++ b/htdocs/gcc-13/changes.html @@ -168,6 +168,12 @@ You may also want to check out our been added, see also https://gcc.gnu.org/onlinedocs/gcc/Freestanding-Environments.html;>Profiling and Test Coverage in Freestanding Environments. + +New options -fharden-compares +and -fharden-conditional-branches to verify compares +and conditional branches, to detect some power-deprivation +hardware attacks, using reversed conditions. + diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 11c7ca7e6967f..24e6409a11b68 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -128,6 +128,35 @@ a work-in-progress. of hardening flags. The options it enables can be displayed using the --help=hardened option. + +New option -fharden-control-flow-redundancy, to +verify, at the end of functions, that the visited basic blocks +correspond to a legitimate execution path, so as to detect and +prevent attacks that transfer control into the middle of +functions. + + +New type attribute hardbool, for C and Ada. Hardened +booleans take user-specified representations for true +and false, presumably with higher hamming distance +than standard booleans, and get verified at every use, detecting +memory corruption and some malicious attacks. + + +New type attribute strub to control stack scrubbing +properties of functions and variables. The stack frame used by +functions marked with the attribute gets zeroed-out upon returning +or exception escaping. Scalar variables marked with the attribute +cause functions contaning or accessing them to get stack scrubbing +enabled implicitly. + + +New option -finline-stringops, to force inline +expansion of memcmp, memcpy, +memmove and memset, even when that is +not an optimization, to avoid relying on library +implementations. + New Languages and Language specific improvements -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] compare_tests: distinguish c-c++-common results by tool
On Dec 20, 2023, Jeff Law wrote: > So the strub tests in c-c++-common are problematical. They get run > twice, once for C, once for C++. Yet the name of the test is the same > in both runs. (by the name, I mean the name emitted into the dejagnu > summary and log files). > Thus if you have a test in there which passes in one context, but > fails in the other, comparison tools like contrib/compare_tests may > erroneously report the tests as both a test which now fails, but > passed before and a test which now passes but failed before. > It looks like some of the strub tests are currently known to fail with > C++ and are triggering this problem Yeah, type warnings/errors are different between C and C++, and this is noticeable with permissible conversions between strub types. > A third option would be to change the compare_tests tool to somehow > distinguish between the C and C++ tests. Not sure how feasible that > is. Most feasible among the possibilities ;-) I've tested the following by comparing my obj-x86_64-linux-gnu test tree with itself. Ok to install? When compare_tests compares both C and C++ tests in c-c++-common, they get the same identifier, so expected differences in results across languages become undesirably noisy. This patch adds tool identifiers to tests, so that runs by different tools are not confused by the compare logic. It also fixes a bug in reporting differences, that would attempt to print an undefined fname (the definitions are in subshell loops), and adjusts the target insertion to match tabs in addition to blanks after colons. for contrib/ChangeLog * compare_tests: Add tool to test lines. Match tabs besides blanks to insert tool and target. Don't print undefined fname. --- contrib/compare_tests |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/compare_tests b/contrib/compare_tests index 2dfa8640756a0..e09fc4f113a3f 100755 --- a/contrib/compare_tests +++ b/contrib/compare_tests @@ -96,7 +96,7 @@ if [ -d "$1" -a -d "$2" ] ; then ret=$? if [ $ret -ne 0 ]; then exit_status=`expr $exit_status + 1` - echo "## Differences found: $fname" + echo "## Differences found" fi if [ $exit_status -ne 0 ]; then echo "# $exit_status differences in $cmnsums common sum files found" @@ -108,8 +108,8 @@ elif [ -d "$1" -o -d "$2" ] ; then usage "Must specify either two directories or two files" fi -sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$1" | awk '/^Running target / {target = $3} { if (target != "unix") { sub(/: /, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp1 -sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$2" | awk '/^Running target / {target = $3} { if (target != "unix") { sub(/: /, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp2 +sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$1" | awk '/^[ ]*=== [^ ]* tests ===$/ {tool = $2} /^Running target / {target = $3} { if (tool != "") { sub(/:[ ]/, "&"tool": " ); }; if (target != "unix") { sub(/:[ ]/, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp1 +sed 's/^XFAIL/FAIL/; s/^ERROR/FAIL/; s/^XPASS/PASS/' < "$2" | awk '/^[ ]*=== [^ ]* tests ===$/ {tool = $2} /^Running target / {target = $3} { if (tool != "") { sub(/:[ ]/, "&"tool": " ); }; if (target != "unix") { sub(/:[ ]/, "&"target": " ); }; print $0; }' | cut -c1-2000 >$tmp2 before=$tmp1 now=$tmp2 -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
[PATCH] -finline-stringops: allow expansion into edges [PR113002]
Builtin expanders for memset and memcpy may involve conditionals and loops, but their sequences may be end up emitted in edges. Alas, commit_one_edge_insertion rejects sequences that end with a jump, a requirement that makes sense for insertions after expand, but not so much during expand. During expand, jumps may appear in the middle of the insert sequence as much as in the end, and it's only after committing edge insertions out of PHI nodes that we go through the entire function splitting blocks where needed, so relax the assert in commit_one_edge_insertion so that jumps are accepted during expand even at the end of the sequence. Regstrapping on x86_64-linux-gnu, after testing the testcase with a cross compiler to ppc64le-linux-gnu. Ok to install? Builtin expanders for memset and memcpy may involve conditionals and loops, but their sequences may be end up emitted in edges. Alas, commit_one_edge_insertion rejects sequences that end with a jump, a requirement that makes sense for insertions after expand, but not so much during expand. During expand, jumps may appear in the middle of the insert sequence as much as in the end, and it's only after committing edge insertions out of PHI nodes that we go through the entire function splitting blocks where needed, so relax the assert in commit_one_edge_insertion so that jumps are accepted during expand even at the end of the sequence. for gcc/ChangeLog PR rtl-optimization/113002 * cfgrtl.cc (commit_one_edge_insertion): Tolerate jumps in the inserted sequence during expand. for gcc/testsuite/ChangeLog PR rtl-optimization/113002 * gcc.dg/vect/pr113002.c: New. --- gcc/cfgrtl.cc|8 +++- gcc/testsuite/gcc.dg/vect/pr113002.c | 13 + 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/vect/pr113002.c diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc index 2a3f853eed59b..437eb3a86d5c2 100644 --- a/gcc/cfgrtl.cc +++ b/gcc/cfgrtl.cc @@ -2092,7 +2092,13 @@ commit_one_edge_insertion (edge e) delete_insn (before); } else -gcc_assert (!JUMP_P (last)); +/* Some builtin expanders, such as those for memset and memcpy, + may generate loops and conditionals, and those may get emitted + into edges. That's ok while expanding to rtl, basic block + boundaries will be identified and split afterwards. ??? Need + we check whether the destination labels of any inserted jumps + are also part of the inserted sequence? */ +gcc_assert (!JUMP_P (last) || currently_expanding_to_rtl); } /* Update the CFG for all queued instructions. */ diff --git a/gcc/testsuite/gcc.dg/vect/pr113002.c b/gcc/testsuite/gcc.dg/vect/pr113002.c new file mode 100644 index 0..186710f64b422 --- /dev/null +++ b/gcc/testsuite/gcc.dg/vect/pr113002.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target int128 } */ +/* { dg-options "-finline-stringops -Os" } */ + +typedef __int128 v64u128 __attribute__((vector_size(64))); +int c; +v64u128 u; +void foo() { + if (c) +u = (v64u128){0}; + else +u = (v64u128){1}; +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH FYI] -finline-stringops: copy timeout factor from memcmp-1.c test
Hi, Jeff, On Dec 18, 2023, Jeff Law wrote: > These are timing sporadically on the embedded platforms. Given they > include a test that has a timeout factor, it seems to me you should > duplicate the timeout factor in the new tests. > Remember when you include another file, the dg- directives in the > other file aren't applied. Thanks for the reminder. Sorry I missed most of them. I added some -finline-stringops tests that included memcmp-1.c, but carried over the timeout factor onto only one such test. Jeff Law kindly pointed that out (thanks!), so here's the fix. Testing on x86_64-linux-gnu. I'll check this in as obvious once testing is done. for gcc/testsuite/ChangeLog * gcc.dg/torture/inline-mem-cmp-1.c: Copy timeout factor from mem-cmp-1.c. * gcc.dg/torture/inline-mem-cpy-1.c: Likewise. --- gcc/testsuite/gcc.dg/torture/inline-mem-cmp-1.c |1 + gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c |1 + 2 files changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.dg/torture/inline-mem-cmp-1.c b/gcc/testsuite/gcc.dg/torture/inline-mem-cmp-1.c index a368f0741129d..4bc66597b35a6 100644 --- a/gcc/testsuite/gcc.dg/torture/inline-mem-cmp-1.c +++ b/gcc/testsuite/gcc.dg/torture/inline-mem-cmp-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-finline-stringops=memcmp -save-temps -g0 -fno-lto" } */ +/* { dg-timeout-factor 2 } */ #include "../memcmp-1.c" diff --git a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c index c98e903c1f169..f4952554dd011 100644 --- a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c +++ b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c @@ -1,5 +1,6 @@ /* { dg-do run } */ /* { dg-options "-finline-stringops=memcpy -save-temps -g0 -fno-lto" } */ +/* { dg-timeout-factor 2 } */ #include "../memcmp-1.c" /* Yeah, this memcmp test exercises plenty of memcpy, more than any of the -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Disinformation flourishes because many people care deeply about injustice but very few check the facts. Think Assange & Stallman. The empires strike back
Re: [PATCH] strub: avoid lto inlining
On Dec 15, 2023, Richard Biener wrote: > I think __noipa__ is more complete and will make the libgcc functions appear > as black boxes to callers. I was hesitant to use __noipa__ because I thought it might disable relevant optimizations even when not using LTO, but it is likely safer in the long run to use it, so I've added it, thanks. > OK your or this way. Here's what I'm checking in shortly. Regstrapped on x86_64-linux-gnu. strub: avoid lto inlining The strub builtins are not suited for cross-unit inlining, they should only be inlined by the builtin expanders, if at all. While testing on sparc64, it occurred to me that, if libgcc was built with LTO enabled, lto1 might inline them, and that would likely break things. So, make sure they're clearly marked as not inlinable. for libgcc/ChangeLog * strub.c (ATTRIBUTE_NOINLINE): New. (ATTRIBUTE_STRUB_CALLABLE): Add it. (__strub_dummy_force_no_leaf): Drop it. --- libgcc/strub.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libgcc/strub.c b/libgcc/strub.c index b0f990d9deebb..3b7cc26b3d8f0 100644 --- a/libgcc/strub.c +++ b/libgcc/strub.c @@ -36,7 +36,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # define TOPS < #endif -#define ATTRIBUTE_STRUB_CALLABLE __attribute__ ((__strub__ ("callable"))) +/* Make sure these builtins won't be inlined, even with LTO. */ +#define ATTRIBUTE_NOINLINE \ + __attribute__ ((__noinline__, __noclone__, __noipa__)) + +#define ATTRIBUTE_STRUB_CALLABLE \ + __attribute__ ((__strub__ ("callable"))) ATTRIBUTE_NOINLINE /* Enter a stack scrubbing context, initializing the watermark to the caller's stack address. */ @@ -72,7 +77,6 @@ __strub_update (void **watermark) /* Dummy function, called to force the caller to not be a leaf function, so that it can't use the red zone. */ static void ATTRIBUTE_STRUB_CALLABLE -__attribute__ ((__noinline__, __noipa__)) __strub_dummy_force_no_leaf (void) { } -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2v2/2] strub: sparc64: unbias the stack address [PR112917]
@node Stack Scrubbing -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] strub: use opt_for_fn during ipa
On Dec 15, 2023, Richard Biener wrote: > You have to be generally careful when working within IPA > with function bodies without push/pop_cfun around that, several APIs > have variants with struct function sepcified, using the wrong one > will get you a NULL cfun which _some_ of them handle gracefully and > "wrong", all EH stuff is amongst this for example. *nod*, I recall running into that, and finding some APIs that required push/pop_cfun, so since I was implementing strub so that it could be plugged into an existing compiler, I didn't give much thought to introducing alternate APIs that could. IIRC I first hit something about EH, and then I had to put in push/pop_cfun. That was very early on, so after that I may have used implicit-cfun APIs without getting ICEs. I suppose now that strub is in pursuing push/pop_cfun avoidance could be a nice cleanup. > I see you replace flag_exceptions with opt_for_fn (cfun->decl, > flag_exceptions), given that's 'cfun' this replacement is a no-op > given 'cfun' would be NULL in IPA context unless you pushed a function. > Looking at the 2nd hunk and the caller it seems the transform is > a no-op for indrect_calls but not callees, thus that hunk is OK. Yeah, I figured that was the reason behind your recommendation, but I guess adding explicit uses of cfun (rather than passing a function around) doesn't really make things much better, except inasmuchas it enables a future de-cfun-ification of strub passes to be a little more mechanical. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2/2] strub: sparc64: unbias the stack address [PR112917]
The stack pointer is biased by 2047 bytes on sparc64, so the range it delimits is way off. Unbias the addresses returned by __builtin_stack_address (), so that the strub builtins, inlined or not, can function correctly. I've considered introducing a new target macro, but using STACK_POINTER_OFFSET seems safe, and it enables the register save areas to be scrubbed as well. Because of the large fixed-size outgoing args area next to the register save area on sparc, we still need __strub_leave to not allocate its own frame, otherwise it won't be able to clear part of the frame it should. Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3. Ok to install? for gcc/ChangeLog PR middle-end/112917 * builtins.cc (expand_bultin_stack_address): Add STACK_POINTER_OFFSET. --- gcc/builtins.cc | 34 -- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 7c2732ab79e6f..4c8c514fe8618 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -5443,8 +5443,38 @@ expand_builtin_frame_address (tree fndecl, tree exp) static rtx expand_builtin_stack_address () { - return convert_to_mode (ptr_mode, copy_to_reg (stack_pointer_rtx), - STACK_UNSIGNED); + rtx ret = convert_to_mode (ptr_mode, copy_to_reg (stack_pointer_rtx), +STACK_UNSIGNED); + + /* Unbias the stack pointer, bringing it to the boundary between the + stack area claimed by the active function calling this builtin, + and stack ranges that could get clobbered if it called another + function. It should NOT encompass any stack red zone, that is + used in leaf functions. + + On SPARC, the register save area is *not* considered active or + used by the active function, but rather as akin to the area in + which call-preserved registers are saved by callees. This + enables __strub_leave to clear what would otherwise overlap with + its own register save area. + + If the address is computed too high or too low, parts of a stack + range that should be scrubbed may be left unscrubbed, scrubbing + may corrupt active portions of the stack frame, and stack ranges + may be doubly-scrubbed by caller and callee. + + In order for it to be just right, the area delimited by + @code{__builtin_stack_address} and @code{__builtin_frame_address + (0)} should encompass caller's registers saved by the function, + local on-stack variables and @code{alloca} stack areas. + Accumulated outgoing on-stack arguments, preallocated as part of + a function's own prologue, are to be regarded as part of the + (caller) function's active area as well, whereas those pushed or + allocated temporarily for a call are regarded as part of the + callee's stack range, rather than the caller's. */ + ret = plus_constant (ptr_mode, ret, STACK_POINTER_OFFSET); + + return force_reg (ptr_mode, ret); } /* Expand a call to builtin function __builtin_strub_enter. */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #1/2] strub: sparc: omit frame in strub_leave [PR112917]
If we allow __strub_leave to allocate a frame on sparc, it will overlap with a lot of the stack range we're supposed to scrub, because of the large fixed-size outgoing args and register save area. Unfortunately, setting up the PIC register seems to prevent the frame pointer from being omitted. Since the strub runtime doesn't issue calls or use global variables, at least on sparc, disabling PIC to compile strub.c seems to do the right thing. Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3. (but it will likely take forever on the cfarm machine; if someone with faster sparc machines could give this a spin and confirm, that would be appreciated.) Ok to install? IIRC this patch gets 32-bit sparc to pass all strub tests, but sparc64 still fails many of them; there's another one for sparc64 that fixes them, and that will improve sparc -m32 as well. for libgcc/ChangeLog PR middle-end/112917 * config.host (sparc, sparc64): Enable... * config/sparc/t-sparc: ... this new fragment. --- libgcc/config.host |2 ++ libgcc/config/sparc/t-sparc |4 2 files changed, 6 insertions(+) create mode 100644 libgcc/config/sparc/t-sparc diff --git a/libgcc/config.host b/libgcc/config.host index 694e3e9f54cad..54d06978a5d2c 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -199,9 +199,11 @@ riscv*-*-*) ;; sparc64*-*-*) cpu_type=sparc + tmake_file="${tmake_file} sparc/t-sparc" ;; sparc*-*-*) cpu_type=sparc + tmake_file="${tmake_file} sparc/t-sparc" ;; s390*-*-*) cpu_type=s390 diff --git a/libgcc/config/sparc/t-sparc b/libgcc/config/sparc/t-sparc new file mode 100644 index 0..fb1bf1fc29cc4 --- /dev/null +++ b/libgcc/config/sparc/t-sparc @@ -0,0 +1,4 @@ +# This is needed for __strub_leave to omit the frame pointer, without +# which it will allocate a register save area on the stack and leave +# it unscrubbed and most likely unused, because it's a leaf function. +CFLAGS-strub.c += -fno-PIC -fomit-frame-pointer -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] strub: use opt_for_fn during ipa
Instead of global optimization levels and flags, check per-function ones. Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3. Ok to install? (sorry, Richi, I dropped the ball and failed to fix this before the monster commit) for gcc/ChangeLog * ipa-strub.cc (gsi_insert_finally_seq_after_call): Likewise. (pass_ipa_strub::adjust_at_calls_call): Likewise. --- gcc/ipa-strub.cc |8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 943bb60996fc1..32e2869cf7840 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2132,7 +2132,7 @@ gsi_insert_finally_seq_after_call (gimple_stmt_iterator gsi, gimple_seq seq) || (call && gimple_call_nothrow_p (call)) || (eh_lp <= 0 && (TREE_NOTHROW (cfun->decl) - || !flag_exceptions))); + || !opt_for_fn (cfun->decl, flag_exceptions; if (noreturn_p && nothrow_p) return; @@ -2470,9 +2470,11 @@ pass_ipa_strub::adjust_at_calls_call (cgraph_edge *e, int named_args, /* If we're already within a strub context, pass on the incoming watermark pointer, and omit the enter and leave calls around the modified call, as an optimization, or as a means to satisfy a tail-call requirement. */ - tree swmp = ((optimize_size || optimize > 2 + tree swmp = ((opt_for_fn (e->caller->decl, optimize_size) + || opt_for_fn (e->caller->decl, optimize) > 2 || gimple_call_must_tail_p (ocall) - || (optimize == 2 && gimple_call_tail_p (ocall))) + || (opt_for_fn (e->caller->decl, optimize) == 2 + && gimple_call_tail_p (ocall))) ? strub_watermark_parm (e->caller->decl) : NULL_TREE); bool omit_own_watermark = swmp; -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] strub: avoid lto inlining
The strub builtins are not suited for cross-unit inlining, they should only be inlined by the builtin expanders, if at all. While testing on sparc64, it occurred to me that, if libgcc was built with LTO enabled, lto1 might inline them, and that would likely break things. So, make sure they're clearly marked as not inlinable. Regstrapped on x86_64-linux-gnu, also testing on sparc-solaris2.11.3. Ok to install? for libgcc/ChangeLog * strub.c (ATTRIBUTE_NOINLINE): New. (ATTRIBUTE_STRUB_CALLABLE): Add it. (__strub_dummy_force_no_leaf): Drop it. --- libgcc/strub.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libgcc/strub.c b/libgcc/strub.c index b0f990d9deebb..5062554d0e1e6 100644 --- a/libgcc/strub.c +++ b/libgcc/strub.c @@ -36,7 +36,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # define TOPS < #endif -#define ATTRIBUTE_STRUB_CALLABLE __attribute__ ((__strub__ ("callable"))) +/* Make sure these builtins won't be inlined, even with LTO. */ +#define ATTRIBUTE_NOINLINE \ + __attribute__ ((__noinline__, __noclone__)) + +#define ATTRIBUTE_STRUB_CALLABLE \ + __attribute__ ((__strub__ ("callable"))) ATTRIBUTE_NOINLINE /* Enter a stack scrubbing context, initializing the watermark to the caller's stack address. */ @@ -72,7 +77,6 @@ __strub_update (void **watermark) /* Dummy function, called to force the caller to not be a leaf function, so that it can't use the red zone. */ static void ATTRIBUTE_STRUB_CALLABLE -__attribute__ ((__noinline__, __noipa__)) __strub_dummy_force_no_leaf (void) { } -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] hardened: use LD_PIE_SPEC only if defined
sol2.h may define LINK_PIE_SPEC and leave LD_PIE_SPEC undefined, but gcc.cc will only provide a LD_PIE_SPEC definition if LINK_PIE_SPEC is not defined, and then it uses LD_PIE_SPEC guarded by #ifdef HAVE_LD_PIE only. Add LD_PIE_SPEC to the guard. Regstrapped on x86_64-linux-gnu; also testing on sparc-solaris2.11.3, where I hit the problem and couldn't build a baseline to compare with. Ok to install? gcc/ChangeLog * gcc.cc (process_command): Use LD_PIE_SPEC only if defined. --- gcc/gcc.cc |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/gcc.cc b/gcc/gcc.cc index 701f5cdfb59c8..d5e02c11cb05d 100644 --- a/gcc/gcc.cc +++ b/gcc/gcc.cc @@ -5008,7 +5008,7 @@ process_command (unsigned int decoded_options_count, { if (!any_link_options_p && !static_p) { -#ifdef HAVE_LD_PIE +#if defined HAVE_LD_PIE && defined LD_PIE_SPEC save_switch (LD_PIE_SPEC, 0, NULL, /*validated=*/true, /*known=*/false); #endif /* These are passed straight down to collect2 so we have to break -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2a/2] strub: indirect volatile parms in wrappers
[sorry that the previous, unfinished post got through] On Dec 12, 2023, Richard Biener wrote: > On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva wrote: >> DECL_NOT_GIMPLE_REG_P (arg) = 0; > I wonder why you clear this at all? That code seems to be inherited from expand_thunk. ISTR that flag was not negated when I started the strub implementation, back in gcc-10. >> +convert in separate statements. ??? Should >> +we drop volatile from the wrapper >> +instead? */ > volatile on function parameters are indeed odd beasts. You could > also force volatile arguments to be passed indirectly. Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of #1/2, now a cleanup that IMHO would still be desirable. Arrange for strub internal wrappers to pass volatile arguments by reference to the wrapped bodies. for gcc/ChangeLog PR middle-end/112938 * ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args by reference to internal strub wrapped bodies. for gcc/testsuite/ChangeLog PR middle-end/112938 * gcc.dg/strub-internal-volatile.c: Check indirection of volatile args. --- gcc/ipa-strub.cc | 19 +-- gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 + 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 45294b0b46bcb..943bb60996fc1 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *) parm = DECL_CHAIN (parm), nparm = DECL_CHAIN (nparm), nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE) - if (!(0 /* DECL_BY_REFERENCE (narg) */ - || is_gimple_reg_type (TREE_TYPE (nparm)) - || VECTOR_TYPE_P (TREE_TYPE (nparm)) - || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE - || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - <= 4 * UNITS_PER_WORD + if (TREE_THIS_VOLATILE (parm) + || !(0 /* DECL_BY_REFERENCE (narg) */ + || is_gimple_reg_type (TREE_TYPE (nparm)) + || VECTOR_TYPE_P (TREE_TYPE (nparm)) + || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE + || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + <= 4 * UNITS_PER_WORD { /* No point in indirecting pointer types. Presumably they won't ever pass the size-based test above, but check the @@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *) { tree tmp = arg; /* If ARG is e.g. volatile, we must copy and -convert in separate statements. ??? Should -we drop volatile from the wrapper -instead? */ +convert in separate statements. */ if (!is_gimple_val (arg)) { tmp = create_tmp_reg (TYPE_MAIN_VARIANT diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c index cdfca67616bc8..227406af245cc 100644 --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-options "-fdump-ipa-strub" } */ /* { dg-require-effective-target strub } */ void __attribute__ ((strub("internal"))) @@ -8,3 +9,7 @@ f(volatile short) { void g(void) { f(0); } + +/* We make volatile parms indirect in the wrapped f. */ +/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */ +/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2a/2]
On Dec 12, 2023, Richard Biener wrote: > On Tue, Dec 12, 2023 at 3:03 AM Alexandre Oliva wrote: >> DECL_NOT_GIMPLE_REG_P (arg) = 0; > I wonder why you clear this at all? That code seems to be inherited from expand_thunk. ISTR that flag was not negated when I started the strub implementation, back in gcc-10. >> +convert in separate statements. ??? Should >> +we drop volatile from the wrapper >> +instead? */ > volatile on function parameters are indeed odd beasts. You could > also force volatile arguments to be passed indirectly. Ooh, I like that, thanks! Regstrapped on x86_64-linux-gnu, on top of #1/2, now a cleanup that IMHO would still be desirable. strub: indirect volatile parms in wrappers Arrange for strub internal wrappers to pass volatile arguments by reference to the wrapped bodies. for gcc/ChangeLog PR middle-end/112938 * ipa-strub.cc (pass_ipa_strub::execute): Pass volatile args by reference to internal strub wrapped bodies. for gcc/testsuite/ChangeLog PR middle-end/112938 * gcc.dg/strub-internal-volatile.c: Check indirection of volatile args. --- gcc/ipa-strub.cc | 19 +-- gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 + 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 45294b0b46bcb..943bb60996fc1 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2881,13 +2881,14 @@ pass_ipa_strub::execute (function *) parm = DECL_CHAIN (parm), nparm = DECL_CHAIN (nparm), nparmt = nparmt ? TREE_CHAIN (nparmt) : NULL_TREE) - if (!(0 /* DECL_BY_REFERENCE (narg) */ - || is_gimple_reg_type (TREE_TYPE (nparm)) - || VECTOR_TYPE_P (TREE_TYPE (nparm)) - || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE - || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) - <= 4 * UNITS_PER_WORD + if (TREE_THIS_VOLATILE (parm) + || !(0 /* DECL_BY_REFERENCE (narg) */ + || is_gimple_reg_type (TREE_TYPE (nparm)) + || VECTOR_TYPE_P (TREE_TYPE (nparm)) + || TREE_CODE (TREE_TYPE (nparm)) == COMPLEX_TYPE + || (tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm))) + <= 4 * UNITS_PER_WORD { /* No point in indirecting pointer types. Presumably they won't ever pass the size-based test above, but check the @@ -3224,9 +3225,7 @@ pass_ipa_strub::execute (function *) { tree tmp = arg; /* If ARG is e.g. volatile, we must copy and -convert in separate statements. ??? Should -we drop volatile from the wrapper -instead? */ +convert in separate statements. */ if (!is_gimple_val (arg)) { tmp = create_tmp_reg (TYPE_MAIN_VARIANT diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c index cdfca67616bc8..227406af245cc 100644 --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-options "-fdump-ipa-strub" } */ /* { dg-require-effective-target strub } */ void __attribute__ ((strub("internal"))) @@ -8,3 +9,7 @@ f(volatile short) { void g(void) { f(0); } + +/* We make volatile parms indirect in the wrapped f. */ +/* { dg-final { scan-ipa-dump-times "volatile short" 2 "strub" } } */ +/* { dg-final { scan-ipa-dump-times "volatile short int &" 1 "strub" } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] untyped calls: enable target switching [PR112334]
On Dec 11, 2023, Jeff Law wrote: >> >> for gcc/ChangeLog >> PR target/112334 >> * builtins.h (target_builtins): Add fields for apply_args_size >> and apply_result_size. >> * builtins.cc (apply_args_size, apply_result_size): Cache >> results in fields rather than in static variables. >> (get_apply_args_size, set_apply_args_size): New. >> (get_apply_result_size, set_apply_result_size): New. > OK. Thanks, I put this bit in. >> untyped calls: use wrapper class type for implicit plus_one >> Instead of get and set macros to apply a delta, use a single macro >> that resorts to a temporary wrapper class to apply it. >> To be combined (or not) with the previous patch. > I'd be OK with this as well. That conditional made me doubt that this was meant as approval, so I did *not* put this one in ;-) If there's firmer/broader buy-in, I'd be glad to put it in as well. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #2/2] strub: drop volatile from wrapper args [PR112938]
On Dec 11, 2023, Alexandre Oliva wrote: > (there's a #2/2 followup coming up that addresses the ??? comment added > herein) Here it is. Also regstrapped on x86_64-linux-gnu, along with the previous patch (that had also been regstrapped by itself). I think this would be a desirable thing to do (maybe also with TYPE_QUAL_ATOMIC), but I'm a little worried about modifying the types of args of the original function decl, the one that is about to become a wrapper. This would be visible at least in debug information. OTOH, keeping the volatile in the wrapper would serve no useful purpose whatsoever, it would likely just make it slower, and such top-level qualifiers really only apply within the function body, which the wrapper isn't. Thoughts? Ok to install? Drop volatile from argument types in internal strub wrappers that are not made indirect. Their volatility is only relevant within the body of the function, and that body is moved to the wrapped function. for gcc/ChangeLog PR middle-end/112938 * ipa-strub.cc (pass_ipa_strub::execute): Drop volatile from internal strub wrapper args. for gcc/testsuite/ChangeLog PR middle-end/112938 * gcc.dg/strub-internal-volatile.c: Check for dropped volatile in wrapper. --- gcc/ipa-strub.cc | 14 +++--- gcc/testsuite/gcc.dg/strub-internal-volatile.c |5 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 45294b0b46bcb..bab20c386bb01 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -2922,6 +2922,16 @@ pass_ipa_strub::execute (function *) if (nparmt) adjust_ftype++; } + else if (TREE_THIS_VOLATILE (parm)) + { + /* Drop volatile from wrapper's arguments, they're just +temporaries copied to the wrapped function. ??? Should +we drop TYPE_QUAL_ATOMIC as well? */ + TREE_TYPE (parm) = build_qualified_type (TREE_TYPE (parm), + TYPE_QUALS (TREE_TYPE (parm)) + & ~TYPE_QUAL_VOLATILE); + TREE_THIS_VOLATILE (parm) = 0; + } /* Also adjust the wrapped function type, if needed. */ if (adjust_ftype) @@ -3224,9 +3234,7 @@ pass_ipa_strub::execute (function *) { tree tmp = arg; /* If ARG is e.g. volatile, we must copy and -convert in separate statements. ??? Should -we drop volatile from the wrapper -instead? */ +convert in separate statements. */ if (!is_gimple_val (arg)) { tmp = create_tmp_reg (TYPE_MAIN_VARIANT diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c index cdfca67616bc8..0ffa98d799d32 100644 --- a/gcc/testsuite/gcc.dg/strub-internal-volatile.c +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-options "-fdump-ipa-strub" } */ /* { dg-require-effective-target strub } */ void __attribute__ ((strub("internal"))) @@ -8,3 +9,7 @@ f(volatile short) { void g(void) { f(0); } + +/* We drop volatile from the wrapper, and keep it in the wrapped f, so + the count remains 1. */ +/* { dg-final { scan-ipa-dump-times "volatile" 1 "strub" } } */ -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH #1/2] strub: handle volatile promoted args in internal strub [PR112938]
When generating code for an internal strub wrapper, don't clear the DECL_NOT_GIMPLE_REG_P flag of volatile args, and gimplify them both before and after any conversion. While at that, move variable TMP into narrower scopes so that it's more trivial to track where ARG lives. Regstrapped on x86_64-linux-gnu. Ok to install? (there's a #2/2 followup coming up that addresses the ??? comment added herein) for gcc/ChangeLog PR middle-end/112938 * ipa-strub.cc (pass_ipa_strub::execute): Handle promoted volatile args in internal strub. Simplify. for gcc/testsuite/ChangeLog PR middle-end/112938 * gcc.dg/strub-internal-volatile.c: New. --- gcc/ipa-strub.cc | 29 +--- gcc/testsuite/gcc.dg/strub-internal-volatile.c | 10 2 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/strub-internal-volatile.c diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc index 8ec6824e8a802..45294b0b46bcb 100644 --- a/gcc/ipa-strub.cc +++ b/gcc/ipa-strub.cc @@ -3203,7 +3203,6 @@ pass_ipa_strub::execute (function *) i++, arg = DECL_CHAIN (arg), nparm = DECL_CHAIN (nparm)) { tree save_arg = arg; - tree tmp = arg; /* Arrange to pass indirectly the parms, if we decided to do so, and revert its type in the wrapper. */ @@ -3211,10 +3210,9 @@ pass_ipa_strub::execute (function *) { tree ref_type = TREE_TYPE (nparm); TREE_ADDRESSABLE (arg) = true; - tree addr = build1 (ADDR_EXPR, ref_type, arg); - tmp = arg = addr; + arg = build1 (ADDR_EXPR, ref_type, arg); } - else + else if (!TREE_THIS_VOLATILE (arg)) DECL_NOT_GIMPLE_REG_P (arg) = 0; /* Convert the argument back to the type used by the calling @@ -3223,16 +3221,31 @@ pass_ipa_strub::execute (function *) double to be passed on unchanged to the wrapped function. */ if (TREE_TYPE (nparm) != DECL_ARG_TYPE (nparm)) - arg = fold_convert (DECL_ARG_TYPE (nparm), arg); + { + tree tmp = arg; + /* If ARG is e.g. volatile, we must copy and +convert in separate statements. ??? Should +we drop volatile from the wrapper +instead? */ + if (!is_gimple_val (arg)) + { + tmp = create_tmp_reg (TYPE_MAIN_VARIANT + (TREE_TYPE (arg)), "arg"); + gimple *stmt = gimple_build_assign (tmp, arg); + gsi_insert_after (, stmt, GSI_NEW_STMT); + } + arg = fold_convert (DECL_ARG_TYPE (nparm), tmp); + } if (!is_gimple_val (arg)) { - tmp = create_tmp_reg (TYPE_MAIN_VARIANT - (TREE_TYPE (arg)), "arg"); + tree tmp = create_tmp_reg (TYPE_MAIN_VARIANT +(TREE_TYPE (arg)), "arg"); gimple *stmt = gimple_build_assign (tmp, arg); gsi_insert_after (, stmt, GSI_NEW_STMT); + arg = tmp; } - vargs.quick_push (tmp); + vargs.quick_push (arg); arg = save_arg; } /* These strub arguments are adjusted later. */ diff --git a/gcc/testsuite/gcc.dg/strub-internal-volatile.c b/gcc/testsuite/gcc.dg/strub-internal-volatile.c new file mode 100644 index 0..cdfca67616bc8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/strub-internal-volatile.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target strub } */ + +void __attribute__ ((strub("internal"))) +f(volatile short) { +} + +void g(void) { + f(0); +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: Introduce -finline-stringops
On Dec 11, 2023, Sam James wrote: > Alexandre Oliva via Gcc-patches writes: >> On Jun 2, 2023, Alexandre Oliva wrote: >> >>> Introduce -finline-stringops >> >> Ping? https://gcc.gnu.org/pipermail/gcc-patches/2023-June/620472.html > Should the docs for the x86-specific -minline-all-stringops refer to > the new -finline-stringops? I wouldn't oppose it, but I find it might be somewhat misleading. -minline-all-stringops seems to be presented as an optimization option, because on x86 that's viable, whereas on some targets -finline-stringops will often generate horribly inefficient code just to avoid some dependencies on libc. Now, it's undeniable that there is a connection between them, in terms of offered functionality if not in goal and framing/presentation. -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] multiflags: fix doc warning properly
On Dec 11, 2023, Joseph Myers wrote: > On Fri, 8 Dec 2023, Alexandre Oliva wrote: >> @@ -20589,7 +20589,7 @@ allocation before or after interprocedural >> optimization. >> This option enables multilib-aware @code{TFLAGS} to be used to build >> target libraries with options different from those the compiler is >> configured to use by default, through the use of specs (@xref{Spec >> -Files}) set up by compiler internals, by the target, or by builders at >> +Files}.) set up by compiler internals, by the target, or by builders at > The proper change in this context is to use @pxref instead of @xref. Oooh, nice! Thank you! Here's a presumably proper fix on top of the earlier one, then. Tested on x86_64-linux-gnu. Ok to install? Rather than a dubious fix for a dubious warning, namely adding a period after a parenthesized @xref because the warning demands it, use @pxref that is meant for exactly this case. Thanks to Joseph Myers for introducing me to it. for gcc/ChangeLog * doc/invoke.texi (multiflags): Drop extraneous period, use @pxref instead. --- gcc/doc/invoke.texi |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 7d15cf94821e3..ce4bb025d5144 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -20588,8 +20588,8 @@ allocation before or after interprocedural optimization. @item -fmultiflags This option enables multilib-aware @code{TFLAGS} to be used to build target libraries with options different from those the compiler is -configured to use by default, through the use of specs (@xref{Spec -Files}.) set up by compiler internals, by the target, or by builders at +configured to use by default, through the use of specs (@pxref{Spec +Files}) set up by compiler internals, by the target, or by builders at configure time. Like @code{TFLAGS}, this allows the target libraries to be built for -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH v2 FYI] -finline-stringops: avoid too-wide smallest_int_mode_for_size [PR112784]
On Dec 11, 2023, Richard Biener wrote: > you can use .exists (_mode) here to ... Aah, yeah, and that should help avoid the noisy as_a conversions too, that I could replace with require(), or drop altogether. >> + || (GET_MODE_BITSIZE (as_a (int_move_mode)) >> + != incr * BITS_PER_UNIT)) Unfortunately, here it can't quite be dropped, GET_MODE_SIZE on a machine_mode isn't suitable for the !=, but with .require() we know it's a scalar_int_mode and thus != on its bitsize is fine. > I'll note that int_mode_for_size and smallest_int_mode_for_size > are not semantically equivalent in what they can return. In > particular it seems you are incrementing by iter_incr even when > formerly smallest_int_mode_for_size would have returned a > larger than necessary mode, resulting in at least inefficient > code, copying/comparing pieces multiple times. If we get a mode that isn't exactly the expected size, we go for BLKmode, so it should be fine and efficient. Unless machine modes are not powers of two multiples of BITS_PER_UNIT, then things may get a little weird, not so much because of repeated copying/comparing, but because of inefficiencies in the block copy/compare operations with block sizes that are not a good fit for such hypothetical (?) GCC targets. I guess we can cross that bridge if we ever get to it. > So int_mode_for_size looks more correct. *nod*. IIRC I had it at first (very long ago), and went for the smallest_ when transitioning to the machine_mode type hierarchy revamp. > OK with the above change. Here's what I've regstrapped on x86_64-linux-gnu, and will install shortly. Thanks! smallest_int_mode_for_size may abort when the requested mode is not available. Call int_mode_for_size instead, that signals the unsatisfiable request in a more graceful way. for gcc/ChangeLog PR middle-end/112784 * expr.cc (emit_block_move_via_loop): Call int_mode_for_size for maybe-too-wide sizes. (emit_block_cmp_via_loop): Likewise. for gcc/testsuite/ChangeLog PR middle-end/112784 * gcc.target/i386/avx512cd-inline-stringops-pr112784.c: New. --- gcc/expr.cc| 20 +--- .../i386/avx512cd-inline-stringops-pr112784.c | 12 2 files changed, 21 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c diff --git a/gcc/expr.cc b/gcc/expr.cc index 6da51f2aca296..076ba706537aa 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -2449,15 +2449,14 @@ emit_block_move_via_loop (rtx x, rtx y, rtx size, } emit_move_insn (iter, iter_init); - scalar_int_mode int_move_mode -= smallest_int_mode_for_size (incr * BITS_PER_UNIT); - if (GET_MODE_BITSIZE (int_move_mode) != incr * BITS_PER_UNIT) + opt_scalar_int_mode int_move_mode += int_mode_for_size (incr * BITS_PER_UNIT, 1); + if (!int_move_mode.exists (_mode) + || GET_MODE_BITSIZE (int_move_mode.require ()) != incr * BITS_PER_UNIT) { move_mode = BLKmode; gcc_checking_assert (can_move_by_pieces (incr, align)); } - else -move_mode = int_move_mode; x_addr = force_operand (XEXP (x, 0), NULL_RTX); y_addr = force_operand (XEXP (y, 0), NULL_RTX); @@ -2701,16 +2700,15 @@ emit_block_cmp_via_loop (rtx x, rtx y, rtx len, tree len_type, rtx target, iter = gen_reg_rtx (iter_mode); emit_move_insn (iter, iter_init); - scalar_int_mode int_cmp_mode -= smallest_int_mode_for_size (incr * BITS_PER_UNIT); - if (GET_MODE_BITSIZE (int_cmp_mode) != incr * BITS_PER_UNIT - || !can_compare_p (NE, int_cmp_mode, ccp_jump)) + opt_scalar_int_mode int_cmp_mode += int_mode_for_size (incr * BITS_PER_UNIT, 1); + if (!int_cmp_mode.exists (_mode) + || GET_MODE_BITSIZE (int_cmp_mode.require ()) != incr * BITS_PER_UNIT + || !can_compare_p (NE, cmp_mode, ccp_jump)) { cmp_mode = BLKmode; gcc_checking_assert (incr != 1); } - else -cmp_mode = int_cmp_mode; /* Save the base addresses. */ x_addr = force_operand (XEXP (x, 0), NULL_RTX); diff --git a/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c b/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c new file mode 100644 index 0..c81f99c693c24 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512cd -finline-stringops" } */ + +struct S { + int e; +} __attribute__((aligned(128))); + +int main() { + struct S s1; + struct S s2; + int v = __builtin_memcmp(, , sizeof(s1)); +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH] -finline-stringops: don't assume ptr_mode ptr in memset [PR112804]
On Dec 10, 2023, Jeff Law wrote: > On 12/8/23 19:25, Alexandre Oliva wrote: >> On aarch64 -milp32, and presumably on other such targets, ptr can be >> in a different mode than ptr_mode in the testcase. Cope with it. >> Regstrapped on x86_64-linux-gnu, also tested the new test on >> aarch64-elf. Ok to install? >> >> for gcc/ChangeLog >> PR target/112804 >> * builtins.cc (try_store_by_multiple_pieces): Use ptr's mode >> for the increment. >> for gcc/testsuite/ChangeLog >> PR target/112804 >> * gcc.target/aarch64/inline-mem-set-pr112804.c: New. > Hmmm. I'd like a little more information here I wouldn't expect those > modes to differ even for ILP32 on a 64bit target. Are we just > papering over a problem elsewhere? Possibly. I've run into Pmode/ptr_mode issues before, so I didn't give it much thought. Despite ptr_mode's being SImode, and DEST being a 32-bit pointer, expand_builtin_memset_args gets from get_memory_rtx a (MEM:BLK (REG:DI)) The expansion of dest to RTL is convoluted. It is first computed in: (set (reg:DI 102) (plus:DI (reg/f:DI 95 virtual-stack-vars) (const_int -264 [0xfef8]))) in DImode because virtual-stack-vars is DImode, then it's SUBREG-narrowed to SImode in expand_expr_addr_expr_1 because of new_tmode (== pointer_mode), and that's the address stored in addr and passed to memory_address in get_memory_rtx. But then something unexpected happens: memory_address_addr_space extends it back to DImode because address_mode = targetm.addr_space.address_mode () = Pmode: (set (reg:DI 103) (zero_extend:DI (subreg:SI (reg:DI 102) 0))) and it's a MEM with (reg:DI 103) as the address that gets passed to clear_storage_hints, and then try_store_by_multiple_pieces, where the address is copied from the MEM into ptr, and then, in the update_needed case I modified in the proposed patch, it gets incremented. So it looks like my hunch that this was a Pmode/ptr_mode issue was correct, and I don't see anything particularly fishy in the conversions above. Do you? -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH v2] -finline-stringops: check base blksize for memset [PR112778]
break; } if (blksize - && can_store_by_pieces (xlenest, - builtin_memset_read_str, - , align, true)) + && can_store_by_multiple_pieces (xlenest, + builtin_memset_read_str, + , align, true, 0)) { max_len += blksize; min_len += blksize; diff --git a/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c new file mode 100644 index 0..fdfc5b6f28c8e --- /dev/null +++ b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-finline-stringops" } */ + +char buf[3]; + +int +f () +{ + __builtin_memset (buf, 'v', 3); +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] multiflags: fix doc warning
Comply with dubious doc warning that after an @xref there must be a comma or a period, not a close parentheses. Build-testing on x86_64-linux-gnu now. Ok to install? for gcc/ChangeLog * doc/invoke.texi (multiflags): Add period after @xref to silence warning. --- gcc/doc/invoke.texi |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index d4e689b64c010..4e67c95dbf85a 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -20589,7 +20589,7 @@ allocation before or after interprocedural optimization. This option enables multilib-aware @code{TFLAGS} to be used to build target libraries with options different from those the compiler is configured to use by default, through the use of specs (@xref{Spec -Files}) set up by compiler internals, by the target, or by builders at +Files}.) set up by compiler internals, by the target, or by builders at configure time. Like @code{TFLAGS}, this allows the target libraries to be built for -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] -finline-stringops: check base blksize for memset [PR112778]
, align, true, 0)) { max_len += blksize; min_len += blksize; diff --git a/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c new file mode 100644 index 0..fdfc5b6f28c8e --- /dev/null +++ b/gcc/testsuite/gcc.dg/inline-mem-cmp-pr112778.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-finline-stringops" } */ + +char buf[3]; + +int +f () +{ + __builtin_memset (buf, 'v', 3); +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] -finline-stringops: don't assume ptr_mode ptr in memset [PR112804]
On aarch64 -milp32, and presumably on other such targets, ptr can be in a different mode than ptr_mode in the testcase. Cope with it. Regstrapped on x86_64-linux-gnu, also tested the new test on aarch64-elf. Ok to install? for gcc/ChangeLog PR target/112804 * builtins.cc (try_store_by_multiple_pieces): Use ptr's mode for the increment. for gcc/testsuite/ChangeLog PR target/112804 * gcc.target/aarch64/inline-mem-set-pr112804.c: New. --- gcc/builtins.cc|2 +- .../gcc.target/aarch64/inline-mem-set-pr112804.c |7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 38b0acff13124..12a535d313f12 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -4519,7 +4519,7 @@ try_store_by_multiple_pieces (rtx to, rtx len, unsigned int ctz_len, to = change_address (to, QImode, 0); emit_move_insn (to, val); if (update_needed) - next_ptr = plus_constant (ptr_mode, ptr, blksize); + next_ptr = plus_constant (GET_MODE (ptr), ptr, blksize); } else { diff --git a/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c b/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c new file mode 100644 index 0..fe8414559864d --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/inline-mem-set-pr112804.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-finline-stringops -mabi=ilp32 -ftrivial-auto-var-init=zero" } */ + +short m(unsigned k) { + const unsigned short *n[65]; + return 0; +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
[PATCH] -finline-stringops: avoid too-wide smallest_int_mode_for_size [PR112784]
smallest_int_mode_for_size may abort when the requested mode is not available. Call int_mode_for_size instead, that signals the unsatisfiable request in a more graceful way. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog PR middle-end/112784 * expr.cc (emit_block_move_via_loop): Call int_mode_for_size for maybe-too-wide sizes. (emit_block_cmp_via_loop): Likewise. for gcc/testsuite/ChangeLog PR middle-end/112784 * gcc.target/i386/avx512cd-inline-stringops-pr112784.c: New. --- gcc/expr.cc| 22 .../i386/avx512cd-inline-stringops-pr112784.c | 12 +++ 2 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c diff --git a/gcc/expr.cc b/gcc/expr.cc index 6da51f2aca296..178b3ec6d5adb 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -2449,15 +2449,17 @@ emit_block_move_via_loop (rtx x, rtx y, rtx size, } emit_move_insn (iter, iter_init); - scalar_int_mode int_move_mode -= smallest_int_mode_for_size (incr * BITS_PER_UNIT); - if (GET_MODE_BITSIZE (int_move_mode) != incr * BITS_PER_UNIT) + opt_scalar_int_mode int_move_mode += int_mode_for_size (incr * BITS_PER_UNIT, 1); + if (!int_move_mode.exists () + || (GET_MODE_BITSIZE (as_a (int_move_mode)) + != incr * BITS_PER_UNIT)) { move_mode = BLKmode; gcc_checking_assert (can_move_by_pieces (incr, align)); } else -move_mode = int_move_mode; +move_mode = as_a (int_move_mode); x_addr = force_operand (XEXP (x, 0), NULL_RTX); y_addr = force_operand (XEXP (y, 0), NULL_RTX); @@ -2701,16 +2703,18 @@ emit_block_cmp_via_loop (rtx x, rtx y, rtx len, tree len_type, rtx target, iter = gen_reg_rtx (iter_mode); emit_move_insn (iter, iter_init); - scalar_int_mode int_cmp_mode -= smallest_int_mode_for_size (incr * BITS_PER_UNIT); - if (GET_MODE_BITSIZE (int_cmp_mode) != incr * BITS_PER_UNIT - || !can_compare_p (NE, int_cmp_mode, ccp_jump)) + opt_scalar_int_mode int_cmp_mode += int_mode_for_size (incr * BITS_PER_UNIT, 1); + if (!int_cmp_mode.exists () + || (GET_MODE_BITSIZE (as_a (int_cmp_mode)) + != incr * BITS_PER_UNIT) + || !can_compare_p (NE, as_a (int_cmp_mode), ccp_jump)) { cmp_mode = BLKmode; gcc_checking_assert (incr != 1); } else -cmp_mode = int_cmp_mode; +cmp_mode = as_a (int_cmp_mode); /* Save the base addresses. */ x_addr = force_operand (XEXP (x, 0), NULL_RTX); diff --git a/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c b/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c new file mode 100644 index 0..c81f99c693c24 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512cd-inline-stringops-pr112784.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512cd -finline-stringops" } */ + +struct S { + int e; +} __attribute__((aligned(128))); + +int main() { + struct S s1; + struct S s2; + int v = __builtin_memcmp(, , sizeof(s1)); +} -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive
Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
On Nov 22, 2023, Jason Merrill wrote: > Tested x86_64-pc-linux-gnu, OK for trunk? FYI, Linaro's regression tester let me know that my patch reversal, that expected this patch to go in instead, caused two "regressions". https://linaro.atlassian.net/browse/GNU-1067 -- Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive