counted_by attribute and type compatibility

[Martin Uecker]

Hi Quin and Joseph,

I saw that there is now new code in tu_tagged_types_compatible
which makes structure type incompatible depending on whether
there is ac counted_by attribute.  Is this what we want?  I think a
warning might make more sense as this types are fundamentally
still compatible.

I also wonder if this could be handled generically by
comp_type_attributes?


Martin

[patch, fortran] Fix UNSIGNED ICE on matchting

[Thomas Koenig]

Hello world,

I have just committed the attached patch as obvious - just recognize
that an UNSIGNED is an error in a complex part.

Fix an ICE with UNSIGNED in match_sym_complex_part.

gcc/fortran/ChangeLog:

PR fortran/117225
* primary.cc (match_sym_complex_part): An UNSIGNED in
a complex part is an error.

gcc/testsuite/ChangeLog:

PR fortran/117225
* gfortran.dg/unsigned_38.f90: New test.
diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc
index c11359a559b..e114bf1375f 100644
--- a/gcc/fortran/primary.cc
+++ b/gcc/fortran/primary.cc
@@ -1471,6 +1471,9 @@ match_sym_complex_part (gfc_expr **result)
 	goto error;
   break;
 
+case BT_UNSIGNED:
+  goto error;
+
 default:
   gfc_internal_error ("gfc_match_sym_complex_part(): Bad type");
 }
diff --git a/gcc/testsuite/gfortran.dg/unsigned_38.f90 b/gcc/testsuite/gfortran.dg/unsigned_38.f90
new file mode 100644
index 000..0d169ac92d3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unsigned_38.f90
@@ -0,0 +1,6 @@
+! { dg-do compile }
+! { dg-options "-funsigned" }
+program main
+  unsigned, parameter :: u = 7u
+  print *,mod(-(u+1u),u)
+end program main

Re: [WIP RFC] libstdc++: add module std

[Florian Weimer]

* Jason Merrill:

> This patch is not ready for integration, but I'd like to get feedback on the
> approach (and various specific questions below).
>
> -- 8< --
>
> This patch introduces an installed source form of module std and std.compat.
> To find them, we install a libstdc++.modules.json file alongside
> libstdc++.so, which tells the build system where the files are and any
> special flags it should use when compiling them (none, in our case).  The
> format is from a proposal in SG15.
>
> The build system can find this file with
> gcc -print-file-name=libstdc++.modules.json

Is it possible to use specs file to redirect this to a different file
based on C++ compiler flags, should that become necessary?

[PATCH] testsuite: arm: Relax register selection [PR116623]

[Torbjörn SVENSSON]

With r15-1618-g9f168b412f4, I get the following asm generated for the test case:

.align  1
.align  2
.global test5
.syntax unified
.thumb
.thumb_func
.type   test5, %function
test5:
@ args = 4, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
push{r4, r5, r6, lr}
ldr r4, [sp, #16]
cmp r4, #0
ble .L37
sub ip, r4, #16
addsr6, r2, r4
addsr5, r1, r4
add r0, r0, r4
dlstp.8 lr, r4
.L39:
subsr2, r5, r4
subsr1, r0, r4
vldrb.8 q3, [r1]
vldrb.8 q2, [r2]
subsr2, r6, r4
mov r4, ip
sub ip, ip, #16
vadd.i8 q3, q3, q2
vstrb.8 q3, [r2]
vstrb.8 q3, [r3]
letplr, .L39
.L37:
pop {r4, r5, r6, pc}
.size   test5, .-test5

...

.align  1
.align  2
.global test8
.syntax unified
.thumb
.thumb_func
.type   test8, %function
test8:
@ args = 4, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
push{r4, lr}
ldr r4, [sp, #8]
cmp r3, #0
ble .L59
dlstp.32lr, r3
.L61:
vldrw.32q3, [r0], #16
vctp.32 r4
vpst
vldrwt.32   q2, [r1], #16
addsr4, r4, #1
vadd.i32q3, q3, q2
vstrw.32q3, [r2], #16
letplr, .L61
.L59:
pop {r4, pc}
.size   test8, .-test8




With r15-1619-g3b9b8d6cfdf, I instead get:

.align  1
.align  2
.global test5
.syntax unified
.thumb
.thumb_func
.type   test5, %function
test5:
@ args = 4, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
push{r4, r5, r6, lr}
ldr ip, [sp, #16]
cmp ip, #0
ble .L37
mov r6, r3
sub r3, ip, #16
add r5, r2, ip
add r4, r1, ip
add r0, r0, ip
dlstp.8 lr, ip
.L39:
sub r2, r4, ip
sub r1, r0, ip
vldrb.8 q3, [r1]
vldrb.8 q2, [r2]
sub r2, r5, ip
mov ip, r3
subsr3, r3, #16
vadd.i8 q3, q3, q2
vstrb.8 q3, [r2]
vstrb.8 q3, [r6]
letplr, .L39
.L37:
pop {r4, r5, r6, pc}
.size   test5, .-test5

...

.align  1
.align  2
.global test8
.syntax unified
.thumb
.thumb_func
.type   test8, %function
test8:
@ args = 4, pretend = 0, frame = 0
@ frame_needed = 0, uses_anonymous_args = 0
push{lr}
ldr ip, [sp, #4]
cmp r3, #0
ble .L59
dlstp.32lr, r3
.L61:
vldrw.32q3, [r0], #16
vctp.32 ip
vpst
vldrwt.32   q2, [r1], #16
add ip, ip, #1
vadd.i32q3, q3, q2
vstrw.32q3, [r2], #16
letplr, .L61
.L59:
ldr pc, [sp], #4
.size   test8, .-test8



As can be seen, with r15-1619-g3b9b8d6cfdf, it now uses ip in ways that it did
not before. I think this part is fine.
It also, for some reason, decides to move r3 into r6 in test5 and then use
that later for the vstrb.8. While I suppose it does work, it will consume one
extra mov, so it's slightly bigger.

With below patch, I no longer see any failure reported for arm-none-eabi.

Even with the slight size increase for test5, is it ok for trunk?

--

Since r15-1619-g3b9b8d6cfdf, test5 and test8 fails due to that "ip"
might be used and r3 might be moved to another register for later
dereference.

gcc/testsuite/ChangeLog:

PR testsuite/116623
* gcc.target/arm/mve/dlstp-compile-asm-2.c: Align test5 and
test8 with changes in r15-1619-g3b9b8d6cfdf.

Signed-off-by: Torbjörn SVENSSON 
---
 gcc/testsuite/gcc.target/arm/mve/dlstp-compile-asm-2.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/mve/dlstp-compile-asm-2.c 
b/gcc/testsuite/gcc.target/arm/mve/dlstp-compile-asm-2.c
index 84f4a2fc4f9..c62f592a60d 100644
--- a/gcc/testsuite/gcc.target/arm/mve/dlstp-compile-asm-2.c
+++ b/gcc/testsuite/gcc.target/arm/mve/dlstp-compile-asm-2.c
@@ -147,15 +147,17 @@ void test5 (uint8_t *a, uint8_t *b, uint8_t *c,  uint8_t 
*d, int n)
 /*
 ** test5:
 **...
-** dlstp.8 lr, r[0-9]+
+** (?:mov  (r[0-9]+), r3)?
+**...
+** dlstp.8 lr, (?:r[0-9]+|ip)
 **...
 ** vldrb.8 q[0-9]+, \[r1\]
 ** vldrb.8 q[0-9]+, \[r2\]
 **...
 ** vadd.i8 (q[0-9]+), q[0-9]+, q[0-9]+
 **...
-** vstrb.8 \1, \[r2\]
-** vstrb.8 \1, \[r3\]
+** vstrb.8 \2, \[r2\]
+** vstrb.8 \2, \[(r3|\1)\]
 ** letplr, .*
 **...
 */
@@ -247,7 +249,7 @@ void test8 (int32_t *a, int32_

Re: [PATCH] phiopt: do factor_out_conditional_operation for all phis [PR112418]

[Jeff Law]

On 10/18/24 7:41 PM, Andrew Pinski wrote:

Sometimes factor_out_conditional_operation can factor out
an operation that causes a phi node to become the same element.
Other times, we want to factor out a binary operator because
it can improve code generation, an example is PR 110015 (openjpeg).

Note this includes a heuristic to decide if factoring out the operation
is profitable or not. It can be expanded to include a better live range
extend detector. Right now it has a simple one where if it is live on a
dominating path, it is considered a live or if there are a small # of
assign statements (defaults to 5), then it does not extend the live range
too much.

Bootstrapped and tested on x86_64-linux-gnu.

PR tree-optimization/112418

gcc/ChangeLog:

* tree-ssa-phiopt.cc (is_factor_profitable): New function.
(factor_out_conditional_operation): Add merge argument. Remove
arg0/arg1 arguments. Return bool instead of the new phi.
Early return for virtual ops. Call is_factor_profitable to
check if the factoring would be profitable.
(pass_phiopt::execute): Call factor_out_conditional_operation
on all phis instead of just singleton phi.
* doc/invoke.texi (--param phiopt-factor-max-stmts-live=): Document.
* params.opt (--param=phiopt-factor-max-stmts-live=): New opt.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/factor_op_phi-1.c: New test.
* gcc.dg/tree-ssa/factor_op_phi-2.c: New test.
* gcc.dg/tree-ssa/factor_op_phi-3.c: New test.
* gcc.dg/tree-ssa/factor_op_phi-4.c: New test.

Dregding up all kinds of old memories.

Not terribly happy with the new param, but a magic constant would 
probably be even worse.. So I won't object to that.


OK for the trunk.  I wouldn't be terribly surprised if we see light 
fallout on some of the crosses, but I don't think that should inherently 
block the patch, we'll just fix them up.


Jeff

Re: [PATCH] regenerate-opt-urls.py: fix transposed values for "vax" and "v850"

[Maciej W. Rozycki]

On Wed, 12 Jun 2024, Maciej W. Rozycki wrote:

> > Hence we decided to check for it in CI instead.
> > 
> > Hope the trade-off sounds reasonable
> 
>  I have reviewed the thread referred and I note that a concern such as 
> mine has already been raised in response to which you have added the 
> `regenerate-opt-urls' make target (thanks!).

 So I have now tried to run this make target on a modification I made to 
options and all I got it is this:

make: Entering directory '.../obj/gcc/gcc'
.../src/gcc/gcc/regenerate-opt-urls.py .../obj/gcc/gcc/HTML/gcc-15.0.0 
.../src/gcc
Traceback (most recent call last):
  File ".../src/gcc/gcc/regenerate-opt-urls.py", line 397, in 
main(args)
  File ".../src/gcc/gcc/regenerate-opt-urls.py", line 378, in main
optfile = OptFile(opt_path, rel_path)
  File ".../src/gcc/gcc/regenerate-opt-urls.py", line 203, in __init__
assert rel_path.startswith('gcc')
AssertionError
make: *** [Makefile:3697: regenerate-opt-urls] Error 1
make: Leaving directory '.../obj/gcc/gcc'

The invocation was:

$ make -C .../obj/gcc/gcc regenerate-opt-urls

(I've edited the shared absolute path prefix for the source/build tree for 
brevity).

I double-checked the GCC internals manual and all it says is:

 There files are generated from the '.opt' files and the generated
 HTML documentation by 'regenerate-opt-urls.py', and should be
 regenerated when adding new options, via manually invoking 'make
 regenerate-opt-urls'.

So what is wrong here, how am I supposed to use it?

  Maciej

[PATCH] c++: Relax checking assert about elision to support -fno-elide-constructors [PR114619]

[Simon Martin]

We currently ICE in checking mode with cxx_dialect < 17 on the following
valid code

=== cut here ===
struct X {
  X(const X&) {}
};
extern X x;
void foo () {
  new X[1]{x};
}
=== cut here ===

The problem is that cp_gimplify_expr gcc_checking_asserts that a
TARGET_EXPR is not TARGET_EXPR_ELIDING_P (or cannot be elided), while in
this case with cxx_dialect < 17, it is TARGET_EXPR_ELIDING_P but we have
not even tried to elide.

This patch relaxes that gcc_checking_assert to not fail when using
cxx_dialect < 17 and -fno-elide-constructors (I considered being more
clever at setting TARGET_EXPR_ELIDING_P appropriately but it looks more
risky and not worth the extra complexity for a checking assert).

Successfully tested on x86_64-pc-linux-gnu.

PR c++/114619

gcc/cp/ChangeLog:

* cp-gimplify.cc (cp_gimplify_expr): Relax gcc_checking_assert
to support the usage of -fno-elide-constructors with c++ < 17.

gcc/testsuite/ChangeLog:

* g++.dg/init/no-elide3.C: New test.

---
 gcc/cp/cp-gimplify.cc |  7 ++-
 gcc/testsuite/g++.dg/init/no-elide3.C | 11 +++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/init/no-elide3.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 003e68f1ea7..354ea73c63b 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -908,7 +908,12 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, 
gimple_seq *post_p)
 gimplify_init_ctor_preeval can materialize subobjects of a CONSTRUCTOR
 on the rhs of an assignment, as in constexpr-aggr1.C.  */
   gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
-  || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
+  || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p))
+  /* If we explicitly asked not to elide and it's not
+ required by the standard, we can't expect elision
+ to have happened.  */
+  || (cxx_dialect < cxx17
+  && !flag_elide_constructors));
   ret = GS_UNHANDLED;
   break;
 
diff --git a/gcc/testsuite/g++.dg/init/no-elide3.C 
b/gcc/testsuite/g++.dg/init/no-elide3.C
new file mode 100644
index 000..9377d9f0161
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/no-elide3.C
@@ -0,0 +1,11 @@
+// PR c++/114619
+// { dg-do "compile" { target c++11 } }
+// { dg-options "-fno-elide-constructors" }
+
+struct X {
+  X(const X&) {}
+};
+extern X x;
+void foo () {
+  new X[1]{x};
+}
-- 
2.44.0

Re: [PATCH] c++: Fix crash during NRV optimization with invalid input [PR117099]

[Simon Martin]

On 18 Oct 2024, at 10:55, Sam James wrote:

> Simon Martin  writes:
>
>> Hi Sam,
>
> Hi Simon,
>
>>
>> On 16 Oct 2024, at 22:06, Sam James wrote:
>>
>>> Simon Martin  writes:
>>>
 We ICE upon the following invalid code because we end up calling
 finalize_nrv_r with a RETURN_EXPR with no operand.

 === cut here ===
 struct X {
   ~X();
 };
 X test(bool b) {
   {
 X x;
 return x;
   }
   if (!(b)) return;
 }
 === cut here ===

 This patch fixes this by simply returning error_mark_node when
 detecting
 a void return in a function returning non-void.

 Successfully tested on x86_64-pc-linux-gnu.

PR c++/117099

 gcc/cp/ChangeLog:

* typeck.cc (check_return_expr): Return error_mark_node upon
void return for function returning non-void.

 gcc/testsuite/ChangeLog:

* g++.dg/parse/crash77.C: New test.

 ---
  gcc/cp/typeck.cc |  1 +
  gcc/testsuite/g++.dg/parse/crash77.C | 14 ++
  2 files changed, 15 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C

 diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
 index 71d879abef1..22a6ec9a185 100644
 --- a/gcc/cp/typeck.cc
 +++ b/gcc/cp/typeck.cc
 @@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool
 *no_warning, bool *dangling)
 RETURN_EXPR to avoid control reaches end of non-void function
 warnings in tree-cfg.cc.  */
*no_warning = true;
 +  return error_mark_node;
  }
/* Check for a return statement with a value in a function that
   isn't supposed to return a value.  */
 diff --git a/gcc/testsuite/g++.dg/parse/crash77.C
 b/gcc/testsuite/g++.dg/parse/crash77.C
 new file mode 100644
 index 000..d3f0ae6a877
 --- /dev/null
 +++ b/gcc/testsuite/g++.dg/parse/crash77.C
 @@ -0,0 +1,14 @@
 +// PR c++/117099
 +// { dg-compile }
>>>
>>> dg-do compile
>>>
>> Aarg, of course, thanks for spotting this! Fixed in the attached
>> version.
>>
 +
 +struct X {
 +  ~X();
 +};
 +
 +X test(bool b) {
 +  {
 +X x;
 +return x;
 +  }
 +  if (!(b)) return; // { dg-error "return-statement with no value" }
 +}
 -- 
 2.44.0

>>>
>>> BTW, the line-endings on this seem a bit odd. Did you use
>>> git-send-email?
>> I did use git-send-email indeed. What oddities do you see with line
>> endings?
>> cat -A over the patch file looks good.
>>
>
> Weird -- if I open your original email in mu4e, I see a bunch of ^M at
> the end of the lines.
Strange. FWIW I’m generating and sending the patches from a MacOS box,
and there might be some weirdness coming from that. I’ll check and try
to fix.

Simon

[PATCH] testsuite: arm: Use effective-target for data-intrinsics-assembly test

[Torbjörn SVENSSON]

Ok for trunk and releases/gcc-14?

--

The expected assembler in the test case assumes -marm, so explicily
require it.

gcc/testsuite/ChangeLog:

* gcc.target/arm/acle/data-intrinsics-assembly.c: Use
effective-target arm_arch_v6_arm.

Signed-off-by: Torbjörn SVENSSON 
---
 gcc/testsuite/gcc.target/arm/acle/data-intrinsics-assembly.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/acle/data-intrinsics-assembly.c 
b/gcc/testsuite/gcc.target/arm/acle/data-intrinsics-assembly.c
index 478cbde1600..f0f583734e8 100644
--- a/gcc/testsuite/gcc.target/arm/acle/data-intrinsics-assembly.c
+++ b/gcc/testsuite/gcc.target/arm/acle/data-intrinsics-assembly.c
@@ -1,9 +1,9 @@
 /* Test the ACLE data intrinsics get expanded to the correct instructions on a 
specific architecture  */
 /* { dg-do assemble } */
 /* { dg-require-effective-target arm_softfp_ok } */
-/* { dg-require-effective-target arm_arch_v6_ok } */
+/* { dg-require-effective-target arm_arch_v6_arm_ok } */
 /* { dg-additional-options "--save-temps -O1" } */
-/* { dg-add-options arm_arch_v6 } */
+/* { dg-add-options arm_arch_v6_arm } */
 /* { dg-final { check-function-bodies "**" "" "" } } */
 
 #include "arm_acle.h"
-- 
2.25.1

[PATCH] testsuite: arm: Relax cbranch tests to accept inverted branches

[Torbjörn SVENSSON]

Ok for trunk and releases/gcc-14?

--

Similar to PR113502, but for non-aarch64 test.
The test started to fail after r14-7243-gafac1bd3365.

gcc/testsuite/ChangeLog:

* gcc.target/arm/vect-early-break-cbranch.c: Ignore exact
branch.

Signed-off-by: Torbjörn SVENSSON 
---
 .../gcc.target/arm/vect-early-break-cbranch.c| 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.target/arm/vect-early-break-cbranch.c 
b/gcc/testsuite/gcc.target/arm/vect-early-break-cbranch.c
index d5c6d56ec86..334e064a239 100644
--- a/gcc/testsuite/gcc.target/arm/vect-early-break-cbranch.c
+++ b/gcc/testsuite/gcc.target/arm/vect-early-break-cbranch.c
@@ -20,7 +20,7 @@ int b[N] = {0};
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
@@ -45,7 +45,7 @@ void f1 ()
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
@@ -70,7 +70,7 @@ void f2 ()
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
@@ -96,7 +96,7 @@ void f3 ()
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
@@ -121,7 +121,7 @@ void f4 ()
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
@@ -146,7 +146,7 @@ void f5 ()
 ** cmp r[0-9]+, #0
 ** bne \.L[0-9]+
 ** |
-** cbnzr[0-9]+, \.L.+
+** cbn?z   r[0-9]+, \.L.+
 ** )
 ** ...
 */
-- 
2.25.1

Re: [PATCH] regenerate-opt-urls.py: fix transposed values for "vax" and "v850"

[Mark Wielaard]

Hi Maciej,

On Sat, Oct 19, 2024 at 04:54:06PM +0100, Maciej W. Rozycki wrote:
> On Wed, 12 Jun 2024, Maciej W. Rozycki wrote:
> 
> > > Hence we decided to check for it in CI instead.
> > > 
> > > Hope the trade-off sounds reasonable
> > 
> >  I have reviewed the thread referred and I note that a concern such as 
> > mine has already been raised in response to which you have added the 
> > `regenerate-opt-urls' make target (thanks!).
> 
>  So I have now tried to run this make target on a modification I made to 
> options and all I got it is this:
> 
> make: Entering directory '.../obj/gcc/gcc'
> .../src/gcc/gcc/regenerate-opt-urls.py .../obj/gcc/gcc/HTML/gcc-15.0.0 
> .../src/gcc
> Traceback (most recent call last):
>   File ".../src/gcc/gcc/regenerate-opt-urls.py", line 397, in 
> main(args)
>   File ".../src/gcc/gcc/regenerate-opt-urls.py", line 378, in main
> optfile = OptFile(opt_path, rel_path)
>   File ".../src/gcc/gcc/regenerate-opt-urls.py", line 203, in __init__
> assert rel_path.startswith('gcc')
> AssertionError
> make: *** [Makefile:3697: regenerate-opt-urls] Error 1
> make: Leaving directory '.../obj/gcc/gcc'
> 
> The invocation was:
> 
> $ make -C .../obj/gcc/gcc regenerate-opt-urls
> 
> (I've edited the shared absolute path prefix for the source/build tree for 
> brevity).
> 
> I double-checked the GCC internals manual and all it says is:
> 
>  There files are generated from the '.opt' files and the generated
>  HTML documentation by 'regenerate-opt-urls.py', and should be
>  regenerated when adding new options, via manually invoking 'make
>  regenerate-opt-urls'.
> 
> So what is wrong here, how am I supposed to use it?

That is how you are supposed to use it.
Or instead of make -C do cd .../obj/gcc; make regenerate-opt-urls.

That assert triggering is odd.  Could you add or change that into a
print(rel_path) so we can see what it really is?

Thanks,

Mark

Re: [PATCH] c++: Fix crash during NRV optimization with invalid input [PR117099]

[Iain Sandoe]

> On 19 Oct 2024, at 10:16, Simon Martin  wrote:
> 
> On 18 Oct 2024, at 10:55, Sam James wrote:
> 
>> Simon Martin  writes:
>> 
>>> Hi Sam,
>> 
>> Hi Simon,
>> 
>>> 
>>> On 16 Oct 2024, at 22:06, Sam James wrote:
>>> 
 Simon Martin  writes:
 
> We ICE upon the following invalid code because we end up calling
> finalize_nrv_r with a RETURN_EXPR with no operand.
> 
> === cut here ===
> struct X {
>  ~X();
> };
> X test(bool b) {
>  {
>X x;
>return x;
>  }
>  if (!(b)) return;
> }
> === cut here ===
> 
> This patch fixes this by simply returning error_mark_node when
> detecting
> a void return in a function returning non-void.
> 
> Successfully tested on x86_64-pc-linux-gnu.
> 
>   PR c++/117099
> 
> gcc/cp/ChangeLog:
> 
>   * typeck.cc (check_return_expr): Return error_mark_node upon
>   void return for function returning non-void.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/parse/crash77.C: New test.
> 
> ---
> gcc/cp/typeck.cc |  1 +
> gcc/testsuite/g++.dg/parse/crash77.C | 14 ++
> 2 files changed, 15 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C
> 
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 71d879abef1..22a6ec9a185 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool
> *no_warning, bool *dangling)
>RETURN_EXPR to avoid control reaches end of non-void function
>warnings in tree-cfg.cc.  */
>   *no_warning = true;
> +  return error_mark_node;
> }
>   /* Check for a return statement with a value in a function that
>  isn't supposed to return a value.  */
> diff --git a/gcc/testsuite/g++.dg/parse/crash77.C
> b/gcc/testsuite/g++.dg/parse/crash77.C
> new file mode 100644
> index 000..d3f0ae6a877
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/crash77.C
> @@ -0,0 +1,14 @@
> +// PR c++/117099
> +// { dg-compile }
 
 dg-do compile
 
>>> Aarg, of course, thanks for spotting this! Fixed in the attached
>>> version.
>>> 
> +
> +struct X {
> +  ~X();
> +};
> +
> +X test(bool b) {
> +  {
> +X x;
> +return x;
> +  }
> +  if (!(b)) return; // { dg-error "return-statement with no value" }
> +}
> -- 
> 2.44.0
> 
 
 BTW, the line-endings on this seem a bit odd. Did you use
 git-send-email?
>>> I did use git-send-email indeed. What oddities do you see with line
>>> endings?
>>> cat -A over the patch file looks good.
>>> 
>> 
>> Weird -- if I open your original email in mu4e, I see a bunch of ^M at
>> the end of the lines.
> Strange. FWIW I’m generating and sending the patches from a MacOS box,
> and there might be some weirdness coming from that. I’ll check and try
> to fix.

macOS Mail messes up whitespace, for sure (but I’ve not seen it append ).
AFAIK  “git send-email” works fine (at least no-one has reported a problem).

maybe the editor in use thinks the target format is Windows and has changed
line endings accordingly?

Iain


> 
> Simon

Re: [PATCH 4/7] RISC-V: Honour -mrvv-max-lmul in riscv_vector::expand_block_move

[Jeff Law]

On 10/18/24 7:12 AM, Craig Blackmore wrote:

Unlike the other vector string ops, expand_block_move was using max LMUL
m8 regardless of TARGET_MAX_LMUL.
Yea.  In retrospect I should have caught this when we integrated the 
original patches.




The check for whether to generate inline vector code for movmem has been
moved from movmem to riscv_vector::expand_block_move to avoid
maintaining multiple versions of similar logic.  They already differed
on the minimum length for which they would generate vector code.  Now
that the expand_block_move value is used, movmem will be generated for
smaller lengths.

ACK.




Limiting memcpy to m1 caused some memcpy loops to be generated in
the calling convention tests which makes it awkward to add suitable scan
assembler tests checking the return value being set, so
-mrvv-max-lmul=m8 has been added to these tests.  Other tests have been
adjusted to expect the new memcpy m1 generation where reasonably
straight-forward, otherwise -mrvv-max-lmul=m8 has been added.

Seems quite reasonable.



pr111720-[0-9].c regressed because a memcpy loop is generated instead
of straight-line.  This reveals an existing issue where a redundant
straight-line memcpy gets eliminated but a memcpy loop does not
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117205).
Hard to argue with Richi's comment though; opaque intrinsics are going 
to be tough for the gimple phases to do much with.  My hope is that not 
too many developers make heavy use of the intrinsics interface.  I 
wouldn't consider that a major problem.






I have added -mrvv-max-lmul=m8 to pr111720-[0-9].c so that we continue
to test the elimination of straight-line memcpy.

ACK.



gcc/ChangeLog:

* config/riscv/riscv-protos.h (get_lmul_mode): New prototype.
(expand_block_move): Add bool parameter for movmem_p.
* config/riscv/riscv-string.cc (riscv_expand_block_move_scalar):
Pass movmem_p as false to riscv_vector::expand_block_move.
(expand_block_move): Add movmem_p parameter.  Return false if
loop needed and movmem_p is true.  Respect TARGET_MAX_LMUL.
* config/riscv/riscv-v.cc (get_lmul_mode): New function.
* config/riscv/riscv.md (movmem): Move checking for
whether to generate inline vector code to
riscv_vector::expand_block_move by passing movmem_p as true.

Generally OK as well.

Robin is on PTO for the next few days.  But I take his comment as a 
request for a minor cleanup/enhancement that could be used in the rest 
of the inline str*/mem* expanders, not as a "this has to be fixed to 
move forward".


So I'll go ahead and push this to the trunk now.  Please review Robin's 
comment on this patch and take appropriate action.


Thanks,
jeff

Re: [PATCH 6/7] RISC-V: Make vectorized memset handle more cases

[Jeff Law]

On 10/18/24 7:12 AM, Craig Blackmore wrote:

`expand_vec_setmem` only generated vectorized memset if it fitted into a
single vector store.  Extend it to generate a loop for longer and
unknown lengths.

The test cases now use -O1 so that they are not sensitive to scheduling.

gcc/ChangeLog:

* config/riscv/riscv-string.cc
(use_vector_stringop_p): Add comment.
(expand_vec_setmem): Use use_vector_stringop_p instead of
check_vectorise_memory_operation.  Add loop generation.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/base/setmem-1.c: Use -O1.  Expect a loop
instead of a libcall.  Add test for unknown length.
* gcc.target/riscv/rvv/base/setmem-2.c: Likewise.
* gcc.target/riscv/rvv/base/setmem-3.c: Likewise and expect smaller
lmul.
So why handle memset differently than the other mem* routines where we 
limit ourselves to what we can handle without needing loops?


My suspicion is that once we're moving enough data that we can't do it 
with a single big lmul store that calling out to the library variant 
probably isn't a big deal for memset.  Do you have data which suggests 
otherwise?


Jeff

Re: [PATCH 7/7] RISC-V: Disable by pieces for vector setmem length > UNITS_PER_WORD

[Jeff Law]

On 10/18/24 7:13 AM, Craig Blackmore wrote:

For fast unaligned access targets, by pieces uses up to UNITS_PER_WORD
size pieces resulting in more store instructions than needed.  For
example gcc.target/riscv/rvv/base/setmem-1.c:f1 built with
`-O3 -march=rv64gcv -mtune=thead-c906`:
```
f1:
 vsetivlizero,8,e8,mf2,ta,ma
 vmv.v.x v1,a1
 vsetivlizero,0,e32,mf2,ta,ma
 sb  a1,14(a0)
 vmv.x.s a4,v1
 vsetivlizero,8,e16,m1,ta,ma
 vmv.x.s a5,v1
 vse8.v  v1,0(a0)
 sw  a4,8(a0)
 sh  a5,12(a0)
 ret
```

The slow unaligned access version built with `-O3 -march=rv64gcv` used
15 sb instructions:
```
f1:
 sb  a1,0(a0)
 sb  a1,1(a0)
 sb  a1,2(a0)
 sb  a1,3(a0)
 sb  a1,4(a0)
 sb  a1,5(a0)
 sb  a1,6(a0)
 sb  a1,7(a0)
 sb  a1,8(a0)
 sb  a1,9(a0)
 sb  a1,10(a0)
 sb  a1,11(a0)
 sb  a1,12(a0)
 sb  a1,13(a0)
 sb  a1,14(a0)
 ret
```

After this patch, the following is generated in both cases:
```
f1:
 vsetivlizero,15,e8,m1,ta,ma
 vmv.v.x v1,a1
 vse8.v  v1,0(a0)
 ret
```

gcc/ChangeLog:

* config/riscv/riscv.cc (riscv_use_by_pieces_infrastructure_p):
New function.
(TARGET_USE_BY_PIECES_INFRASTRUCTURE_P): Define.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/autovec/pr113469.c: Expect mf2 setmem.
* gcc.target/riscv/rvv/base/setmem-2.c: Update f1 to expect
straight-line vector memset.
* gcc.target/riscv/rvv/base/setmem-3.c: Likewise.
This looks independent of 6/7, so I went ahead and pushed it.  There's a 
slight chance we'll have a test goof if there's an unexpected dependency.


Thanks,
Jeff

[r15-4494 Regression] FAIL: gfortran.dg/unsigned_38.f90 -O (test for excess errors) on Linux/x86_64

[haochen.jiang]

On Linux/x86_64,

4f9b1735ab5eaf93d07d65c81d83cd123a8f3478 is the first bad commit
commit 4f9b1735ab5eaf93d07d65c81d83cd123a8f3478
Author: Thomas Koenig 
Date:   Sat Oct 19 10:26:17 2024 +0200

Fix an ICE with UNSIGNED in match_sym_complex_part.

caused

FAIL: gfortran.dg/unsigned_38.f90   -O  (test for excess errors)

with GCC configured with

../../gcc/configure 
--prefix=/export/users/haochenj/src/gcc-bisect/master/master/r15-4494/usr 
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
--enable-libmpx x86_64-linux --disable-bootstrap

To reproduce:

$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/unsigned_38.f90 --target_board='unix{-m32}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/unsigned_38.f90 --target_board='unix{-m32\ 
-march=cascadelake}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/unsigned_38.f90 --target_board='unix{-m64}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/unsigned_38.f90 --target_board='unix{-m64\ 
-march=cascadelake}'"

(Please do not reply to this email, for question about this report, contact me 
at haochen dot jiang at intel.com.)
(If you met problems with cascadelake related, disabling AVX512F in command 
line might save that.)
(However, please make sure that there is no potential problems with AVX512.)

Re: [PATCH 5/7] RISC-V: Move vector memcpy decision making to separate function [NFC]

[Jeff Law]

On 10/18/24 7:12 AM, Craig Blackmore wrote:

This moves the code for deciding whether to generate a vectorized
memcpy, what vector mode to use and whether a loop is needed out of
riscv_vector::expand_block_move and into a new function
riscv_vector::use_stringop_p so that it can be reused for other string
operations.

gcc/ChangeLog:

* config/riscv/riscv-string.cc (struct stringop_info): New.
(expand_block_move): Move decision making code to...
(use_vector_stringop_p): ...here.

Thanks.  I've pushed this to the trunk.
jeff

Re: [RFC/RFA] [PATCH v5 09/12] Add symbolic execution support.

[Jeff Law]

On 10/18/24 9:01 AM, Mariam Arutunian wrote:

Gives an opportunity to execute the code on bit level,
assigning symbolic values to the variables which don't have initial values.
Supports only CRC specific operations.

Example:

uint8_t crc;
uint8_t pol = 1;
crc = crc ^ pol;

during symbolic execution crc's value will be:
crc(8), crc(7), ... crc(1), crc(0) ^ 1

   gcc/

     * Makefile.in (OBJS): Add sym-exec/sym-exec-expression.o,
     sym-exec/sym-exec-state.o, sym-exec/sym-exec-condition.o.
     * configure (sym-exec): New subdir.

   gcc/sym-exec/

     * sym-exec-condition.cc: New file.
     * sym-exec-condition.h: New file.
     * sym-exec-expression-is-a-helper.h: New file.
     * sym-exec-expression.cc: New file.
     * sym-exec-expression.h: New file.
     * sym-exec-state.cc: New file.
     * sym-exec-state.h: New file.
The new files all need a copyright notice.  Just grab one from another 
file and copy it in verbatim.


New functions all need a function comment.  Many have them, but some 
don't.  Just a quick pass over the new files for missing function 
comments please.


Jeff

Re: [RFC/RFA][PATCH v5 00/12] CRC optimization.

[Jeff Law]

On 10/18/24 9:00 AM, Mariam Arutunian wrote:

Hello,

This patch series is a respin of the following: https://gcc.gnu.org/ 
pipermail/gcc-patches/2024-September/662961.html . Although I sent 
[PATCH v4 00/12] to the mailing list, it didn’t appear in the archives, 
so I've provided the link to the first patch ([PATCH v4 01/12]). The 
original patch set can be found here: https://gcc.gnu.org/pipermail/gcc- 
patches/2024-May/652610.html .


I have addressed all the feedback from the previous series, except for 
the |emit_crc| function in patch 01/12, which I am still working on.
So there's some minor comments on patch #9, but none which would 
directly result in code changes, just comment additions.


I think once you fix those plus the emit_crc stuff from the last round, 
this will be ready go commit!


Jeff

Re: [PATCH][v5] RISC-V: add option -m(no-)autovec-segment

[Jeff Law]

On 10/16/24 6:36 PM, Edwin Lu wrote:

Add option -m(no-)autovec-segment to enable/disable autovectorizer
from emitting vector segment load/store instructions. This is useful for
performance experiments.

gcc/ChangeLog:
* config/riscv/autovec.md (vec_mask_len_load_lanes, 
vec_mask_len_store_lanes):
  Predicate with TARGET_VECTOR_AUTOVEC_SEGMENT
* gcc/config/riscv/riscv-opts.h (TARGET_VECTOR_AUTOVEC_SEGMENT): New 
macro.
* gcc/config/riscv/riscv.opt (-m(no-)autovec-segment): New option.
* testsuite/gcc.target/riscv/rvv/autovec/struct/*_noseg*.c,
testsuite/gcc.target/riscv/rvv/autovec/no-segment.c: New tests.

Had to fixup the ChangeLog to get past the commit hooks.  Done and pushed.

jeff

Re: [PATCH v4] RISC-V: add option -m(no-)autovec-segment

[Jeff Law]

On 10/15/24 12:05 PM, Robin Dapp wrote:

Quick question.  We did something like this to aid internal
testing/bringup.  Our variant adjusted a ton of the mode iterators in
vector-iterators.md and the TUPLE_ENTRY stuff in riscv-vector-switch.def.

Robin, do you remember why you had to adjust all the iterators?  Was it
that LTO issue we had with the early variants, or something else?


I don't fully remember but the LTO issue was likely part of the problem.
The other part might have been "better safe than sorry".
Thanks.  We got a clean run through the tester, so I went ahead and 
fixed up the ChangeLog and pushed the change.  Going to assume it was 
the latter ;-)



jeff

Re: [RFC/RFA][PATCH v5 00/12] CRC optimization.

[Mariam Arutunian]

On Sat, Oct 19, 2024, 17:58 Jeff Law  wrote:

>
>
> On 10/18/24 9:00 AM, Mariam Arutunian wrote:
> > Hello,
> >
> > This patch series is a respin of the following: https://gcc.gnu.org/
> > pipermail/gcc-patches/2024-September/662961.html  > pipermail/gcc-patches/2024-September/662961.html>. Although I sent
> > [PATCH v4 00/12] to the mailing list, it didn’t appear in the archives,
> > so I've provided the link to the first patch ([PATCH v4 01/12]). The
> > original patch set can be found here: https://gcc.gnu.org/pipermail/gcc-
> > patches/2024-May/652610.html  > patches/2024-May/652610.html>.
> >
> > I have addressed all the feedback from the previous series, except for
> > the |emit_crc| function in patch 01/12, which I am still working on.
> So there's some minor comments on patch #9, but none which would
> directly result in code changes, just comment additions.
>
> I think once you fix those plus the emit_crc stuff from the last round,
> this will be ready go commit!
>


Ok. Thank you very much!

Mariam


> Jeff
>
>

Re: [PATCH] diagnostics: libcpp: Improve locations for _Pragma lexing diagnostics [PR114423]

[Lewis Hyatt]

On Fri, Oct 18, 2024 at 6:21 PM David Malcolm  wrote:
>
> On Fri, 2024-10-18 at 13:58 -0400, Lewis Hyatt wrote:
> > On Fri, Oct 18, 2024 at 11:25 AM David Malcolm 
> > wrote:
> > > >if (!pfile->cb.diagnostic)
> > > >  abort ();
> > > > -  ret = pfile->cb.diagnostic (pfile, level, reason, richloc,
> > > > _(msgid), ap);
> > > > -
> > > > -  return ret;
> > > > +  if (pfile->diagnostic_override_loc && level != CPP_DL_NOTE)
> > > > +{
> > > > +  rich_location rc2{pfile->line_table, pfile-
> > > > > diagnostic_override_loc};
> > > > +  return pfile->cb.diagnostic (pfile, level, reason, &rc2,
> > > > _(msgid), ap);
> > > > +}
> > >
> > > This will effectively override the primary location in the
> > > rich_location, but by using a second rich_location instance it will
> > > also ignore any secondary locations and fix-it hints.
> > >
> > > This might will be what we want here, but did you consider
> > >   richloc.set_range (0, pfile->diagnostic_override_loc,
> > >  SHOW_RANGE_WITH_CARET);
> > > to reset the primary location?
> > >
> > > Otherwise, looks good to me.
> > >
> > > [...snip...]
> > >
> > > Thanks
> > > Dave
> > >
> >
> > Thanks for taking a look! My thinking was, when libcpp produces
> > tokens
> > from a _Pragma string, basically every location_t that it generates
> > is
> > wrong and shouldn't be used. Because it doesn't actually set up the
> > line_map, it gets something random that's just sorta close to
> > reasonable. So I think it makes sense to discard fixits and secondary
> > locations too.
>
> Fair enough.  I think the patch is OK, then, assuming usual testing.

...

> > For this particular case I could improve it with a one line addition
> > like
> >
> > rc2.set_escape_on_output (richloc->escape_on_output_p ());
> >
> > and that would actually handle all currently needed cases since we
> > don't use a lot of rich_locations in libcpp. It would just become
> > stale if some other option gets added to rich_location in the future
> > that we also should preserve.
> > I think to fix it in a fully general
> > way, it would be necessary to add a new interface to class
> > rich_location. It already has a method to delete all the fixit hints,
> > it would also need a method to delete all the ranges. Then we could
> > make a copy of the richloc and just delete everything we don't want
> > to
> > preserve. Do you have a preference one way or the other?
>
> Do we know which diagnostics are likely to be emitted when we override
> the location?  I suspect that anywhere that's overriding the location
> is an awkward case where we're unlikely to have fix-it hints and
> secondary ranges.
>
> I don't have a strong preference here.
>
> >
> > By the way, your suggestion was to directly modify richloc... these
> > functions do take the richloc by non-const pointer, but is it OK to
> > modify it or is a copy needed? The functions like cpp_warning_at()
> > are
> > exposed in the public header file, although at the moment, all call
> > sites are within libcpp and don't look like they would notice if the
> > argument was modified. I wasn't sure what is the expected interface
> > here.
>
> The diagnostic-reporting functions take non-const rich_location because
> the Fortran frontend doesn't set the locations on its diagnostics, but
> instead uses formatting codes %C and %L which write back locations into
> the rich_location when pp_format is called.  So there's some precedent
> for non-const rich_location, FWIW
>

OK cool, thanks for the explanation. I just noticed that the C++
frontend modifies the richloc too. (Sometimes it calls into libcpp
after lexing all the tokens, like for string concatenation, and then
it needs to set the location to the original one from when the token
was obtained.) I'll keep that in mind for the future, in the meantime
I have pushed it with the simple approach.

-Lewis

PING: [PATCH] sibcall: Adjust BLKmode argument size for alignment padding

[H.J. Lu]

On Sun, Oct 13, 2024, 10:07 AM H.J. Lu  wrote:

> Adjust BLKmode argument size for parameter alignment for sibcall check.
>
> gcc/
>
> PR middle-end/117098
> * calls.cc (store_one_arg): Adjust BLKmode argument size for
> alignment padding for sibcall check.
>
> gcc/testsuite/
>
> PR middle-end/117098
> * gcc.dg/sibcall-12.c: New test.
>
> OK for master?
>

PING.


>
> H.J.
>

[PATCH 3/3] gcc: Add --enable-multilib-space option

[Keith Packard]

This option adds a per-multilib variant that specifies -Os
instead of the default.

Signed-off-by: Keith Packard 
---
 config-ml.in |  2 +-
 gcc/Makefile.in  | 32 +++-
 gcc/configure| 13 +
 gcc/configure.ac |  7 +++
 gcc/doc/install.texi | 10 ++
 5 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/config-ml.in b/config-ml.in
index 645cac822fd..21345e38bee 100644
--- a/config-ml.in
+++ b/config-ml.in
@@ -175,7 +175,7 @@ eval scan_arguments "${ac_configure_args}"
 unset scan_arguments
 
 # Only do this if --enable-multilib.
-if [ "${enable_multilib}" = yes ]; then
+if [ "${enable_multilib}" = yes -o "${enable_multilib_space}" = yes ]; then
 
 # Compute whether this is the library's top level directory
 # (ie: not a multilib subdirectory, and not a subdirectory like newlib/src).
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 059cf2e8f79..7faf6d2fdb0 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2304,25 +2304,47 @@ libgcc.mvars: config.status Makefile specs xgcc$(exeext)
 
mv tmp-libgcc.mvars libgcc.mvars
 
+ifeq (@enable_multilib_space@,yes)
+MULTILIB_OPTIONS   += Os
+MULTILIB_DIRNAMES  += space
+MULTILIB_MATCHES   += Os=Oz
+
+MULTILIB_OSDIRNAMES_SPACE = $(MULTILIB_OSDIRNAMES)\
+   $(if $(findstring =,$(MULTILIB_OSDIRNAMES)),\
+ $(foreach OSD,$(MULTILIB_OSDIRNAMES),$(subst =,/Os=,$(OSD))/space),\
+ $(if $(MULTILIB_OSDIRNAMES),space,))
+MULTILIB_REQUIRED_SPACE = $(if $(MULTILIB_REQUIRED),Os $(foreach REQ, 
$(MULTILIB_REQUIRED), $(REQ) $(REQ)/Os),)
+MULTILIB_EXCEPTIONS_SPACE = $(foreach EXC, $(MULTILIB_EXCEPTIONS), $(EXC) 
$(EXC)/Os)
+MULTILIB_REUSE_SPACE = $(foreach REU, $(MULTILIB_REUSE), $(REU) $(subst 
=,/Os=,$(REU))/Os)
+MULTILIB_ENABLE = yes
+else
+MULTILIB_OSDIRNAMES_SPACE = $(MULTILIB_OSDIRNAMES)
+MULTILIB_REQUIRED_SPACE = $(MULTILIB_REQUIRED)
+MULTILIB_EXCEPTIONS_SPACE = $(MULTILIB_EXCEPTIONS)
+MULTILIB_REUSE_SPACE = $(MULTILIB_REUSE)
+MULTILIB_ENABLE = @multilib@
+endif
+
 # Use the genmultilib shell script to generate the information the gcc
 # driver program needs to select the library directory based on the
 # switches.
 multilib.h: s-mlib; @true
 s-mlib: $(srcdir)/genmultilib Makefile
if test @enable_multilib@ = yes \
+   || test @enable_multilib_space@ = yes \
   || test -n "$(MULTILIB_OSDIRNAMES)"; then \
  $(SHELL) $(srcdir)/genmultilib \
"$(MULTILIB_OPTIONS)" \
"$(MULTILIB_DIRNAMES)" \
"$(MULTILIB_MATCHES)" \
-   "$(MULTILIB_EXCEPTIONS)" \
+   "$(MULTILIB_EXCEPTIONS_SPACE)" \
"$(MULTILIB_EXTRA_OPTS)" \
"$(MULTILIB_EXCLUSIONS)" \
-   "$(MULTILIB_OSDIRNAMES)" \
-   "$(MULTILIB_REQUIRED)" \
+   "$(MULTILIB_OSDIRNAMES_SPACE)" \
+   "$(MULTILIB_REQUIRED_SPACE)" \
"$(if $(MULTILIB_OSDIRNAMES),,$(MULTIARCH_DIRNAME))" \
-   "$(MULTILIB_REUSE)" \
-   "@enable_multilib@" \
+   "$(MULTILIB_REUSE_SPACE)" \
+   "$(MULTILIB_ENABLE)" \
> tmp-mlib.h; \
else \
  $(SHELL) $(srcdir)/genmultilib '' '' '' '' '' '' '' '' \
diff --git a/gcc/configure b/gcc/configure
index 5acc42c1e4d..2a12a7cd224 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -856,6 +856,7 @@ DEFAULT_MATCHPD_PARTITIONS
 with_float
 with_cpu
 enable_multiarch
+enable_multilib_space
 enable_multilib
 coverage_flags
 valgrind_command
@@ -977,6 +978,7 @@ enable_coverage
 enable_gather_detailed_mem_stats
 enable_valgrind_annotations
 enable_multilib
+enable_multilib_space
 enable_multiarch
 with_stack_clash_protection_guard_size
 with_matchpd_partitions
@@ -1718,6 +1720,7 @@ Optional Features:
   --enable-valgrind-annotations
   enable valgrind runtime interaction
   --enable-multilib   enable library support for multiple ABIs
+  --enable-multilib-space enable extra -Os variant for every multilib ABI
   --enable-multiarch  enable support for multiarch paths
   --enable-__cxa_atexit   enable __cxa_atexit for C++
   --enable-decimal-float={no,yes,bid,dpd}
@@ -7834,6 +7837,16 @@ fi
 
 
 
+# Determine whether or not -Os multilibs are enabled.
+# Check whether --enable-multilib-space was given.
+if test "${enable_multilib_space+set}" = set; then :
+  enableval=$enable_multilib_space;
+else
+  enable_multilib_space=yes
+fi
+
+
+
 # Determine whether or not multiarch is enabled.
 # Check whether --enable-multiarch was given.
 if test "${enable_multiarch+set}" = set; then :
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 23f4884eff9..84faf3a7645 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -848,6 +848,13 @@ AC_ARG_ENABLE(multilib,
 [], [enable_multilib=yes])
 AC_SUBST(enable_multilib)
 
+# Determine whether or not -Os multilibs are enabled.
+AC_ARG_ENABLE(multilib-space,
+[AS_HELP_STRING([--enable-multilib-space],
+   [enable extra -Os variant for ev

[PATCH 1/3] libgcc: Use -Os/-Oz from CC or CFLAGS

[Keith Packard]

Override other optimization settings with any -Os or -Oz found in CC
or CFLAGS.

Signed-off-by: Keith Packard 
---
 libgcc/Makefile.in | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index ffc45f21267..25710636938 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -294,16 +294,20 @@ override CFLAGS := $(filter-out -fprofile-generate 
-fprofile-use,$(CFLAGS))
 # CFLAGS first is not perfect; normally setting CFLAGS should override any
 # options in LIBGCC2_CFLAGS.  But LIBGCC2_CFLAGS may contain -g0, and CFLAGS
 # will usually contain -g, so for the moment CFLAGS goes first.  We must
-# include CFLAGS - that's where multilib options live.
+# include CFLAGS - that's where multilib options live. If CC or CFLAGS
+# specify -Os or -Oz, we want that to override our local options as that
+# could be a multilib flag.
 INTERNAL_CFLAGS = $(CFLAGS) $(LIBGCC2_CFLAGS) $(HOST_LIBGCC2_CFLAGS) \
- $(INCLUDES) @set_have_cc_tls@ @set_use_emutls@
+ $(INCLUDES) @set_have_cc_tls@ @set_use_emutls@ \
+ $(filter -Os -Oz,$(CC) $(CFLAGS))
 
 # Options to use when compiling crtbegin/end.
 CRTSTUFF_CFLAGS = -O2 $(GCC_CFLAGS) $(INCLUDES) $(MULTILIB_CFLAGS) -g0 \
   $(NO_PIE_CFLAGS) -finhibit-size-directive -fno-inline -fno-exceptions \
   -fno-zero-initialized-in-bss -fno-toplevel-reorder -fno-tree-vectorize \
   -fbuilding-libgcc -fno-stack-protector $(FORCE_EXPLICIT_EH_REGISTRY) \
-  $(INHIBIT_LIBC_CFLAGS) $(USE_TM_CLONE_REGISTRY)
+  $(INHIBIT_LIBC_CFLAGS) $(USE_TM_CLONE_REGISTRY) \
+  $(filter -Os -Oz,$(CC) $(CFLAGS))
 
 # Extra flags to use when compiling crt{begin,end}.o.
 CRTSTUFF_T_CFLAGS =
-- 
2.45.2

[PATCH 2/3] arm: Add missing multilib default values

[Keith Packard]

The arm multilib configuration includes two more parameters which
affect multilib selection, marm/mthumb and mfloat-abi. Without those,
the default multilib selection is mis-specified and the only reason it
works is because '.' is the fall-back path.

Add "marm" and "mfloat-abi=soft" to MULTILIB_DEFAULTS to actually
match when the compiler is run without any target parameters.

This hasn't caused any problems in practice because there are no
non-default multilib options which can be applied to the default
-march target as it has neither an FPU nor any branch protection
support. Specifying another cpu or architecture always sets -marm and
-mfloat-abi and so those multilib configuration don't rely on the
defaults.

Signed-off-by: Keith Packard 
---
 gcc/config/arm/arm-mlib.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/arm/arm-mlib.h b/gcc/config/arm/arm-mlib.h
index e3552d8f065..dac38b9acc9 100644
--- a/gcc/config/arm/arm-mlib.h
+++ b/gcc/config/arm/arm-mlib.h
@@ -19,4 +19,4 @@
along with GCC; see the file COPYING3.  If not see
.  */
 
-#define MULTILIB_DEFAULTS { "mbranch-protection=none" }
+#define MULTILIB_DEFAULTS { "marm", "mfloat-abi=soft", 
"mbranch-protection=none" }
-- 
2.45.2

[PATCH 0/3, v2] Automate creation of -O2 and -Os multilib variants

[Keith Packard]

This is the second version of this patch series.

Changes from the previous versions:

 * Add documentation for --enable-multilib-space configuration
   option. Thanks to Joseph Myers for flagging this.

 * Fixed targets using MULTILIB_OSDIRNAMES using '=' signs; instead of
   simply adding a new 'space' name, each OSDIRNAMES entry gets
   replicated with the second version adding the OS and space values.

 * Added support for configurations not also using
   --enable-multilib. This affected how the libraries were built; we
   need to also check for --enable-multilib-space when deciding
   whether to build multiple variants of each one.

 Description of the V2 series 

When building toolchains for embedded development, some projects will
want to optimize for speed while others are much more concerned about
overall code size. We can do this using the existing GCC multilib
infrastructure, adding suitable speed and size variants as another
dimension of the multilib table.

This could replace the current Debian arm-none-eabi toolchain practice
of duplicating the C library build into 'regular' and 'release'
variants and then requiring application developers select which to use
with a new GCC flag, --picolibc-buildtype=release, which is processed
in a custom .specs file. This mechanism only provides space/speed
optimized versions of the C library, and not libgcc or libstdc++.

The first patch in this series allows multilib configurations to
enable -Os or -Oz while building libgcc by pulling any -Os or -Oz
setting out of the CC and CFLAGS variables and appending them to the
end of the compiler flags used for libgcc and other targets,
overriding numerous uses of -O2 throughout the build system.

The second patch fixes a bug in the ARM multilib configuration
settings; the default values marm and mfloat-abi=soft were not
specified; when -Os was the only option provided to the driver, the
missing default values would cause the multilib selection code to fail
to match the Os version.

The third patch enables a new configuration option,
--enable-multilib-space, which adds -Os as one of the multilib options
and then extends any existing MULTILIB_EXCEPTIONS, MULTILIB_REQUIRED,
MULTILIB_REUSE and MULTILIB_OSDIRNAMES settings, adding Os variants of
each entry.

This has been tested by building all 28 Zephyr toolchains and running
a set of Zephyr tests using the resulting compilers.

As an alternative, we could alter the multilib settings for each
target, but that would touch many more files. If this is the preferred
approach, we'll still need the first patch in this series which allows
a multilib configuration that includes -Os to override the existing
optimization settings used while building libgcc.