public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Support g++ 4.8 as a host compiler.
@ 2023-10-04 22:19 Roger Sayle
  2023-10-05  5:52 ` Xi Ruoyao
  2023-10-07 16:19 ` Jeff Law
  0 siblings, 2 replies; 13+ messages in thread
From: Roger Sayle @ 2023-10-04 22:19 UTC (permalink / raw)
  To: gcc-patches; +Cc: 'Richard Sandiford'

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]


The recent patch to remove poly_int_pod triggers a bug in g++ 4.8.5's
C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
constructor.  This in turn prohibits it from being used as a member in
a union (rtxunion) that constructed statically, resulting in a (fatal)
error during stage 1.  A workaround is to add an explicit constructor
to the problematic union, which allows mainline to be bootstrapped with
the system compiler on older RedHat 7 systems.

This patch has been tested on x86_64-pc-linux-gnu where it allows a
bootstrap to complete when using g++ 4.8.5 as the host compiler.
Ok for mainline?


2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* rtl.h (rtx_def::u): Add explicit constructor to workaround
	issue using g++ 4.8 as a host compiler.


[-- Attachment #2: patchbs.txt --]
[-- Type: text/plain, Size: 390 bytes --]

diff --git a/gcc/rtl.h b/gcc/rtl.h
index 6850281..a7667f5 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -451,6 +451,9 @@ struct GTY((desc("0"), tag("0"),
     struct fixed_value fv;
     struct hwivec_def hwiv;
     struct const_poly_int_def cpi;
+#if defined(__GNUC__) && GCC_VERSION < 5000
+    u () {}
+#endif
   } GTY ((special ("rtx_def"), desc ("GET_CODE (&%0)"))) u;
 };
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-04 22:19 [PATCH] Support g++ 4.8 as a host compiler Roger Sayle
@ 2023-10-05  5:52 ` Xi Ruoyao
  2023-10-07 16:19 ` Jeff Law
  1 sibling, 0 replies; 13+ messages in thread
From: Xi Ruoyao @ 2023-10-05  5:52 UTC (permalink / raw)
  To: Roger Sayle, gcc-patches; +Cc: 'Richard Sandiford'

On Wed, 2023-10-04 at 23:19 +0100, Roger Sayle wrote:
> 
> The recent patch to remove poly_int_pod triggers a bug in g++ 4.8.5's
> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
> constructor.  This in turn prohibits it from being used as a member in
> a union (rtxunion) that constructed statically, resulting in a (fatal)
> error during stage 1.  A workaround is to add an explicit constructor
> to the problematic union, which allows mainline to be bootstrapped with
> the system compiler on older RedHat 7 systems.
> 
> This patch has been tested on x86_64-pc-linux-gnu where it allows a
> bootstrap to complete when using g++ 4.8.5 as the host compiler.
> Ok for mainline?
> 
> 
> 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
> 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
> 	issue using g++ 4.8 as a host compiler.

AFAIK G++ 5.1 also has a bug (https://gcc.gnu.org/PR65801) breaking
building recent GCC.  I don't think it's really "maintainable" to ensure
current GCC able to be built with a buggy host compiler.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-04 22:19 [PATCH] Support g++ 4.8 as a host compiler Roger Sayle
  2023-10-05  5:52 ` Xi Ruoyao
@ 2023-10-07 16:19 ` Jeff Law
  2023-10-07 21:30   ` Sam James
  2023-10-15 10:50   ` Roger Sayle
  1 sibling, 2 replies; 13+ messages in thread
From: Jeff Law @ 2023-10-07 16:19 UTC (permalink / raw)
  To: Roger Sayle, gcc-patches; +Cc: 'Richard Sandiford'



On 10/4/23 16:19, Roger Sayle wrote:
> 
> The recent patch to remove poly_int_pod triggers a bug in g++ 4.8.5's
> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
> constructor.  This in turn prohibits it from being used as a member in
> a union (rtxunion) that constructed statically, resulting in a (fatal)
> error during stage 1.  A workaround is to add an explicit constructor
> to the problematic union, which allows mainline to be bootstrapped with
> the system compiler on older RedHat 7 systems.
> 
> This patch has been tested on x86_64-pc-linux-gnu where it allows a
> bootstrap to complete when using g++ 4.8.5 as the host compiler.
> Ok for mainline?
> 
> 
> 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
> 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
> 	issue using g++ 4.8 as a host compiler.
I think the bigger question is whether or not we're going to step 
forward on the minimum build requirements.

My recollection was we settled on gcc-4.8 for the benefit of RHEL 7 and 
Centos 7 which are rapidly approaching EOL (June 2024).

I would certainly support stepping forward to a more modern compiler for 
the build requirements, which might make this patch obsolete.

Jeff

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-07 16:19 ` Jeff Law
@ 2023-10-07 21:30   ` Sam James
  2023-10-08  4:40     ` Jeff Law
  2023-10-15 10:50   ` Roger Sayle
  1 sibling, 1 reply; 13+ messages in thread
From: Sam James @ 2023-10-07 21:30 UTC (permalink / raw)
  To: Jeff Law
  Cc: Roger Sayle, 'Richard Sandiford',
	gcc-patches, Jakub Jelinek, Richard Biener


Jeff Law <jeffreyalaw@gmail.com> writes:

> On 10/4/23 16:19, Roger Sayle wrote:
>> The recent patch to remove poly_int_pod triggers a bug in g++
>> 4.8.5's
>> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
>> constructor.  This in turn prohibits it from being used as a member in
>> a union (rtxunion) that constructed statically, resulting in a (fatal)
>> error during stage 1.  A workaround is to add an explicit constructor
>> to the problematic union, which allows mainline to be bootstrapped with
>> the system compiler on older RedHat 7 systems.
>> This patch has been tested on x86_64-pc-linux-gnu where it allows a
>> bootstrap to complete when using g++ 4.8.5 as the host compiler.
>> Ok for mainline?
>> 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
>> gcc/ChangeLog
>> 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
>> 	issue using g++ 4.8 as a host compiler.
> I think the bigger question is whether or not we're going to step
> forward on the minimum build requirements.
>
> My recollection was we settled on gcc-4.8 for the benefit of RHEL 7
> and Centos 7 which are rapidly approaching EOL (June 2024).
>
> I would certainly support stepping forward to a more modern compiler
> for the build requirements, which might make this patch obsolete.

See also richi and jakub's comments at https://inbox.sourceware.org/gcc-patches/mpt5y3ppio0.fsf@arm.com/T/#m985295bedaadb47aa0b9ba63b7cb69a660a108bb.

>
> Jeff


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-07 21:30   ` Sam James
@ 2023-10-08  4:40     ` Jeff Law
  2023-10-08  8:08       ` Iain Sandoe
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Law @ 2023-10-08  4:40 UTC (permalink / raw)
  To: Sam James
  Cc: Roger Sayle, 'Richard Sandiford',
	gcc-patches, Jakub Jelinek, Richard Biener



On 10/7/23 15:30, Sam James wrote:
> 
> Jeff Law <jeffreyalaw@gmail.com> writes:
> 
>> On 10/4/23 16:19, Roger Sayle wrote:
>>> The recent patch to remove poly_int_pod triggers a bug in g++
>>> 4.8.5's
>>> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
>>> constructor.  This in turn prohibits it from being used as a member in
>>> a union (rtxunion) that constructed statically, resulting in a (fatal)
>>> error during stage 1.  A workaround is to add an explicit constructor
>>> to the problematic union, which allows mainline to be bootstrapped with
>>> the system compiler on older RedHat 7 systems.
>>> This patch has been tested on x86_64-pc-linux-gnu where it allows a
>>> bootstrap to complete when using g++ 4.8.5 as the host compiler.
>>> Ok for mainline?
>>> 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
>>> gcc/ChangeLog
>>> 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
>>> 	issue using g++ 4.8 as a host compiler.
>> I think the bigger question is whether or not we're going to step
>> forward on the minimum build requirements.
>>
>> My recollection was we settled on gcc-4.8 for the benefit of RHEL 7
>> and Centos 7 which are rapidly approaching EOL (June 2024).
>>
>> I would certainly support stepping forward to a more modern compiler
>> for the build requirements, which might make this patch obsolete.
> 
> See also richi and jakub's comments at https://inbox.sourceware.org/gcc-patches/mpt5y3ppio0.fsf@arm.com/T/#m985295bedaadb47aa0b9ba63b7cb69a660a108bb.
Yea.  As Jakub notes, there's the cfarm situation, but I've had good 
success with DTS on Centos 7 systems (I have to support some of those 
internally within Ventana).  It quite literally "just works" though 
users would have to enable it.

Alternately, update the cfarm hosts?

Jeff

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-08  4:40     ` Jeff Law
@ 2023-10-08  8:08       ` Iain Sandoe
  0 siblings, 0 replies; 13+ messages in thread
From: Iain Sandoe @ 2023-10-08  8:08 UTC (permalink / raw)
  To: Jeff Law
  Cc: Sam James, Roger Sayle, Richard Sandiford, GCC Patches,
	Jakub Jelinek, Richard Biener



> On 8 Oct 2023, at 05:40, Jeff Law <jeffreyalaw@gmail.com> wrote:
> On 10/7/23 15:30, Sam James wrote:
>> Jeff Law <jeffreyalaw@gmail.com> writes:
>>> On 10/4/23 16:19, Roger Sayle wrote:
>>>> The recent patch to remove poly_int_pod triggers a bug in g++
>>>> 4.8.5's
>>>> C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
>>>> constructor.  This in turn prohibits it from being used as a member in
>>>> a union (rtxunion) that constructed statically, resulting in a (fatal)
>>>> error during stage 1.  A workaround is to add an explicit constructor
>>>> to the problematic union, which allows mainline to be bootstrapped with
>>>> the system compiler on older RedHat 7 systems.
>>>> This patch has been tested on x86_64-pc-linux-gnu where it allows a
>>>> bootstrap to complete when using g++ 4.8.5 as the host compiler.
>>>> Ok for mainline?
>>>> 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
>>>> gcc/ChangeLog
>>>> 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
>>>> 	issue using g++ 4.8 as a host compiler.
>>> I think the bigger question is whether or not we're going to step
>>> forward on the minimum build requirements.
>>> 
>>> My recollection was we settled on gcc-4.8 for the benefit of RHEL 7
>>> and Centos 7 which are rapidly approaching EOL (June 2024).
>>> 
>>> I would certainly support stepping forward to a more modern compiler
>>> for the build requirements, which might make this patch obsolete.
>> See also richi and jakub's comments at https://inbox.sourceware.org/gcc-patches/mpt5y3ppio0.fsf@arm.com/T/#m985295bedaadb47aa0b9ba63b7cb69a660a108bb.
> Yea.  As Jakub notes, there's the cfarm situation, but I've had good success with DTS on Centos 7 systems (I have to support some of those internally within Ventana).  It quite literally "just works" though users would have to enable it.
> 
> Alternately, update the cfarm hosts?

In practice, if one wants to test Ada and D, a newer toolchain is needed anyway - so at least some of us are already using self-built bootstrap toolchains.

Is there some blocker to installing a project-built toolchain on /opt, for example?  (admittedly it then becomes a point that someone has to take responsibility for providing it).
Iain


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-07 16:19 ` Jeff Law
  2023-10-07 21:30   ` Sam James
@ 2023-10-15 10:50   ` Roger Sayle
  2023-10-15 11:43     ` Richard Sandiford
  1 sibling, 1 reply; 13+ messages in thread
From: Roger Sayle @ 2023-10-15 10:50 UTC (permalink / raw)
  To: 'Jeff Law', gcc-patches
  Cc: 'Richard Sandiford', 'Richard Biener',
	'Jakub Jelinek'


I'd like to ping my patch for restoring bootstrap using g++ 4.8.5
(the system compiler on RHEL 7 and later systems).
https://gcc.gnu.org/pipermail/gcc-patches/2023-October/632008.html

Note the preprocessor #ifs can be removed; they are only there to document
why the union u must have an explicit, empty (but not default) constructor.

I completely agree with the various opinions that we might consider
upgrading the minimum host compiler for many good reasons (Ada,
D, newer C++ features etc.).  It's inevitable that older compilers and
systems can't be supported indefinitely.

Having said that I don't think that this unintentional trivial breakage,
that has a safe one-line work around is sufficient cause (or non-neglible
risk or support burden), to inconvenice a large number of GCC users
(the impact/disruption to cfarm has already been mentioned).

Interestingly, "scl enable devtoolset-XX" to use a newer host compiler,
v10 or v11, results in a significant increase (100+) in unexpected failures I see
during mainline regression testing using "make -k check" (on RedHat 7.9).
(Older) system compilers, despite their flaws, are selected for their
(overall) stability and maturity.

If another patch/change hits the compiler next week that reasonably
means that 4.8.5 can no longer be supported, so be it, but its an
annoying (and unnecessary?) inconvenience in the meantime.

Perhaps we should file a Bugzilla PR indicating that the documentation
and release notes need updating, if my fix isn't considered acceptable?

Why this patch is an trigger issue (that requires significant discussion
and deliberation) is somewhat of a mystery.

Thanks in advance.
Roger
> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: 07 October 2023 17:20
> To: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org
> Cc: 'Richard Sandiford' <richard.sandiford@arm.com>
> Subject: Re: [PATCH] Support g++ 4.8 as a host compiler.
> 
> 
> 
> On 10/4/23 16:19, Roger Sayle wrote:
> >
> > The recent patch to remove poly_int_pod triggers a bug in g++ 4.8.5's
> > C++ 11 support which mistakenly believes poly_uint16 has a non-trivial
> > constructor.  This in turn prohibits it from being used as a member in
> > a union (rtxunion) that constructed statically, resulting in a (fatal)
> > error during stage 1.  A workaround is to add an explicit constructor
> > to the problematic union, which allows mainline to be bootstrapped
> > with the system compiler on older RedHat 7 systems.
> >
> > This patch has been tested on x86_64-pc-linux-gnu where it allows a
> > bootstrap to complete when using g++ 4.8.5 as the host compiler.
> > Ok for mainline?
> >
> >
> > 2023-10-04  Roger Sayle  <roger@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> > 	* rtl.h (rtx_def::u): Add explicit constructor to workaround
> > 	issue using g++ 4.8 as a host compiler.
> I think the bigger question is whether or not we're going to step forward on the
> minimum build requirements.
> 
> My recollection was we settled on gcc-4.8 for the benefit of RHEL 7 and Centos 7
> which are rapidly approaching EOL (June 2024).
> 
> I would certainly support stepping forward to a more modern compiler for the
> build requirements, which might make this patch obsolete.
> 
> Jeff


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-15 10:50   ` Roger Sayle
@ 2023-10-15 11:43     ` Richard Sandiford
  2023-10-18  8:37       ` Eric Gallager
  2023-10-18 10:17       ` Jakub Jelinek
  0 siblings, 2 replies; 13+ messages in thread
From: Richard Sandiford @ 2023-10-15 11:43 UTC (permalink / raw)
  To: Roger Sayle
  Cc: 'Jeff Law', gcc-patches, 'Richard Biener',
	'Jakub Jelinek'

"Roger Sayle" <roger@nextmovesoftware.com> writes:
> I'd like to ping my patch for restoring bootstrap using g++ 4.8.5
> (the system compiler on RHEL 7 and later systems).
> https://gcc.gnu.org/pipermail/gcc-patches/2023-October/632008.html
>
> Note the preprocessor #ifs can be removed; they are only there to document
> why the union u must have an explicit, empty (but not default) constructor.
>
> I completely agree with the various opinions that we might consider
> upgrading the minimum host compiler for many good reasons (Ada,
> D, newer C++ features etc.).  It's inevitable that older compilers and
> systems can't be supported indefinitely.
>
> Having said that I don't think that this unintentional trivial breakage,
> that has a safe one-line work around is sufficient cause (or non-neglible
> risk or support burden), to inconvenice a large number of GCC users
> (the impact/disruption to cfarm has already been mentioned).
>
> Interestingly, "scl enable devtoolset-XX" to use a newer host compiler,
> v10 or v11, results in a significant increase (100+) in unexpected failures I see
> during mainline regression testing using "make -k check" (on RedHat 7.9).
> (Older) system compilers, despite their flaws, are selected for their
> (overall) stability and maturity.
>
> If another patch/change hits the compiler next week that reasonably
> means that 4.8.5 can no longer be supported, so be it, but its an
> annoying (and unnecessary?) inconvenience in the meantime.
>
> Perhaps we should file a Bugzilla PR indicating that the documentation
> and release notes need updating, if my fix isn't considered acceptable?
>
> Why this patch is an trigger issue (that requires significant discussion
> and deliberation) is somewhat of a mystery.

It seemed like there was considerable support for bumping the minimum
to beyond 4.8.  I think we should wait until a decision has been made
before adding more 4.8 workarounds.

Having a conditional explicit constructor is dangerous because it changes
semantics.  E.g. consider:

  #include <new>

  union u { int x; };
  void f(u *ptr) { new(ptr) u; }
  void g(u *ptr) { new(ptr) u(); }

g(ptr) zeros ptr->x whereas f(ptr) doesn't.  If we add "u() {}" then g()
does not zero ptr->x.

So if we did add the workaround, it would need to be unconditional,
like you say.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-15 11:43     ` Richard Sandiford
@ 2023-10-18  8:37       ` Eric Gallager
  2023-10-18 10:17       ` Jakub Jelinek
  1 sibling, 0 replies; 13+ messages in thread
From: Eric Gallager @ 2023-10-18  8:37 UTC (permalink / raw)
  To: Roger Sayle, Jeff Law, gcc-patches, Richard Biener,
	Jakub Jelinek, richard.sandiford

On Sun, Oct 15, 2023 at 7:43 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> "Roger Sayle" <roger@nextmovesoftware.com> writes:
> > I'd like to ping my patch for restoring bootstrap using g++ 4.8.5
> > (the system compiler on RHEL 7 and later systems).
> > https://gcc.gnu.org/pipermail/gcc-patches/2023-October/632008.html
> >
> > Note the preprocessor #ifs can be removed; they are only there to document
> > why the union u must have an explicit, empty (but not default) constructor.
> >
> > I completely agree with the various opinions that we might consider
> > upgrading the minimum host compiler for many good reasons (Ada,
> > D, newer C++ features etc.).  It's inevitable that older compilers and
> > systems can't be supported indefinitely.
> >
> > Having said that I don't think that this unintentional trivial breakage,
> > that has a safe one-line work around is sufficient cause (or non-neglible
> > risk or support burden), to inconvenice a large number of GCC users
> > (the impact/disruption to cfarm has already been mentioned).
> >
> > Interestingly, "scl enable devtoolset-XX" to use a newer host compiler,
> > v10 or v11, results in a significant increase (100+) in unexpected failures I see
> > during mainline regression testing using "make -k check" (on RedHat 7.9).
> > (Older) system compilers, despite their flaws, are selected for their
> > (overall) stability and maturity.
> >
> > If another patch/change hits the compiler next week that reasonably
> > means that 4.8.5 can no longer be supported, so be it, but its an
> > annoying (and unnecessary?) inconvenience in the meantime.
> >
> > Perhaps we should file a Bugzilla PR indicating that the documentation
> > and release notes need updating, if my fix isn't considered acceptable?
> >
> > Why this patch is an trigger issue (that requires significant discussion
> > and deliberation) is somewhat of a mystery.
>
> It seemed like there was considerable support for bumping the minimum
> to beyond 4.8.  I think we should wait until a decision has been made
> before adding more 4.8 workarounds.
>
> Having a conditional explicit constructor is dangerous because it changes
> semantics.  E.g. consider:
>
>   #include <new>
>
>   union u { int x; };
>   void f(u *ptr) { new(ptr) u; }
>   void g(u *ptr) { new(ptr) u(); }
>
> g(ptr) zeros ptr->x whereas f(ptr) doesn't.  If we add "u() {}" then g()
> does not zero ptr->x.
>
> So if we did add the workaround, it would need to be unconditional,
> like you say.
>
> Thanks,
> Richard

I personally would prefer it if GCC would continue to maintain support
for gcc 4.8 for a host compiler. One of the things I like about GCC is
how it's generally tried to keep support for older host tools for
longer than other projects do, meaning that with GCC, you don't get
stuck on the upgrade treadmill of having to compile a whole string of
consecutive compilers just to be able to compile the latest one like
you have to do with clang. Please just apply Roger's patch; it already
exists and is relatively simple.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-15 11:43     ` Richard Sandiford
  2023-10-18  8:37       ` Eric Gallager
@ 2023-10-18 10:17       ` Jakub Jelinek
  2023-10-18 10:23         ` Richard Sandiford
  1 sibling, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2023-10-18 10:17 UTC (permalink / raw)
  To: Roger Sayle, 'Jeff Law',
	gcc-patches, 'Richard Biener',
	richard.sandiford

On Sun, Oct 15, 2023 at 12:43:10PM +0100, Richard Sandiford wrote:
> It seemed like there was considerable support for bumping the minimum
> to beyond 4.8.  I think we should wait until a decision has been made
> before adding more 4.8 workarounds.

I think adding a workaround until that decision is made and perhaps
removing it afterwards will make life easier for people still using gcc 4.8.

> Having a conditional explicit constructor is dangerous because it changes
> semantics.  E.g. consider:
> 
>   #include <new>
> 
>   union u { int x; };
>   void f(u *ptr) { new(ptr) u; }
>   void g(u *ptr) { new(ptr) u(); }
> 
> g(ptr) zeros ptr->x whereas f(ptr) doesn't.  If we add "u() {}" then g()
> does not zero ptr->x.
> 
> So if we did add the workaround, it would need to be unconditional,
> like you say.

What about using more directed workaround then?

Like (just stage1 build tested, perhaps with comment why we do that)
below?  Seems at least in stage1 it is the only problematic spot.

--- a/gcc/cse.cc
+++ b/gcc/cse.cc
@@ -4951,8 +4951,14 @@ cse_insn (rtx_insn *insn)
          && is_a <scalar_int_mode> (mode, &int_mode)
          && (extend_op = load_extend_op (int_mode)) != UNKNOWN)
        {
+#if GCC_VERSION >= 5000
          struct rtx_def memory_extend_buf;
          rtx memory_extend_rtx = &memory_extend_buf;
+#else
+         alignas (alignof (rtx_def)) unsigned char
+           memory_extended_buf[sizeof (rtx_def)];
+         rtx memory_extend_rtx = (rtx) &memory_extended_buf[0];
+#endif
 
          /* Set what we are trying to extend and the operation it might
             have been extended with.  */


	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-18 10:17       ` Jakub Jelinek
@ 2023-10-18 10:23         ` Richard Sandiford
  2023-10-18 11:33           ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Sandiford @ 2023-10-18 10:23 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Roger Sayle, 'Jeff Law', gcc-patches, 'Richard Biener'

Jakub Jelinek <jakub@redhat.com> writes:
> On Sun, Oct 15, 2023 at 12:43:10PM +0100, Richard Sandiford wrote:
>> It seemed like there was considerable support for bumping the minimum
>> to beyond 4.8.  I think we should wait until a decision has been made
>> before adding more 4.8 workarounds.
>
> I think adding a workaround until that decision is made and perhaps
> removing it afterwards will make life easier for people still using gcc 4.8.
>
>> Having a conditional explicit constructor is dangerous because it changes
>> semantics.  E.g. consider:
>> 
>>   #include <new>
>> 
>>   union u { int x; };
>>   void f(u *ptr) { new(ptr) u; }
>>   void g(u *ptr) { new(ptr) u(); }
>> 
>> g(ptr) zeros ptr->x whereas f(ptr) doesn't.  If we add "u() {}" then g()
>> does not zero ptr->x.
>> 
>> So if we did add the workaround, it would need to be unconditional,
>> like you say.
>
> What about using more directed workaround then?
>
> Like (just stage1 build tested, perhaps with comment why we do that)
> below?  Seems at least in stage1 it is the only problematic spot.
>
> --- a/gcc/cse.cc
> +++ b/gcc/cse.cc
> @@ -4951,8 +4951,14 @@ cse_insn (rtx_insn *insn)
>           && is_a <scalar_int_mode> (mode, &int_mode)
>           && (extend_op = load_extend_op (int_mode)) != UNKNOWN)
>         {
> +#if GCC_VERSION >= 5000
>           struct rtx_def memory_extend_buf;
>           rtx memory_extend_rtx = &memory_extend_buf;
> +#else
> +         alignas (alignof (rtx_def)) unsigned char
> +           memory_extended_buf[sizeof (rtx_def)];

Looks like the simpler "alignas (rtx_def)" should work.

LGTM otherwise FWIW.

Richard

> +         rtx memory_extend_rtx = (rtx) &memory_extended_buf[0];
> +#endif
>  
>           /* Set what we are trying to extend and the operation it might
>              have been extended with.  */
>
>
> 	Jakub

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-18 10:23         ` Richard Sandiford
@ 2023-10-18 11:33           ` Jakub Jelinek
  2023-10-18 11:51             ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2023-10-18 11:33 UTC (permalink / raw)
  To: Roger Sayle, 'Jeff Law',
	gcc-patches, 'Richard Biener',
	richard.sandiford

On Wed, Oct 18, 2023 at 11:23:49AM +0100, Richard Sandiford wrote:
> > --- a/gcc/cse.cc
> > +++ b/gcc/cse.cc
> > @@ -4951,8 +4951,14 @@ cse_insn (rtx_insn *insn)
> >           && is_a <scalar_int_mode> (mode, &int_mode)
> >           && (extend_op = load_extend_op (int_mode)) != UNKNOWN)
> >         {
> > +#if GCC_VERSION >= 5000
> >           struct rtx_def memory_extend_buf;
> >           rtx memory_extend_rtx = &memory_extend_buf;
> > +#else
> > +         alignas (alignof (rtx_def)) unsigned char
> > +           memory_extended_buf[sizeof (rtx_def)];
> 
> Looks like the simpler "alignas (rtx_def)" should work.

It does.

> LGTM otherwise FWIW.

Here is what I'm bootstrapping/regtesting on gcc112 now (i.e. with 4.8.5
as system compiler), added details what bug we are working around.
The reduced testcase on which I've bisected it is:
struct poly_int {
  poly_int() = default;
  template <typename ...T> poly_int(const T &...) {}
};
union rtunion {
  poly_int rt_subregrt_rtx;
};
struct rtx_def {
  rtunion fld;
};
void cse_insn() { rtx_def memory_extend_buf; }
or even with just
  template <typename> poly_int();
line in there.  Bet gcc 4.8/4.9 was unhappy about the
template variadic ctor accepting empty pack and being like
the default ctor but not defaulted in that case.
Making it guaranteed that it has at least one argument say through
  template <typename U, typename ...T> poly_int(const U &, const T &...) {}
fixes it for 4.8/4.9 as well.

2023-10-18  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/111852
	* cse.cc (cse_insn): Add workaround for GCC 4.8-4.9, instead of
	using rtx_def type for memory_extend_buf, use unsigned char
	arrayy with size of rtx_def and its alignment.

--- gcc/cse.cc.jj	2023-06-20 08:57:38.339505245 +0200
+++ gcc/cse.cc	2023-10-18 13:20:30.555836778 +0200
@@ -4951,8 +4951,15 @@ cse_insn (rtx_insn *insn)
 	  && is_a <scalar_int_mode> (mode, &int_mode)
 	  && (extend_op = load_extend_op (int_mode)) != UNKNOWN)
 	{
+#if GCC_VERSION >= 5000
 	  struct rtx_def memory_extend_buf;
 	  rtx memory_extend_rtx = &memory_extend_buf;
+#else
+	  /* Workaround GCC < 5 bug, fixed in r5-3834 as part of PR63362
+	     fix.  */
+	  alignas (rtx_def) unsigned char memory_extended_buf[sizeof (rtx_def)];
+	  rtx memory_extend_rtx = (rtx) &memory_extended_buf[0];
+#endif
 
 	  /* Set what we are trying to extend and the operation it might
 	     have been extended with.  */


	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Support g++ 4.8 as a host compiler.
  2023-10-18 11:33           ` Jakub Jelinek
@ 2023-10-18 11:51             ` Jakub Jelinek
  0 siblings, 0 replies; 13+ messages in thread
From: Jakub Jelinek @ 2023-10-18 11:51 UTC (permalink / raw)
  To: Roger Sayle, 'Jeff Law',
	gcc-patches, 'Richard Biener',
	richard.sandiford

On Wed, Oct 18, 2023 at 01:33:40PM +0200, Jakub Jelinek wrote:
> Making it guaranteed that it has at least one argument say through
>   template <typename U, typename ...T> poly_int(const U &, const T &...) {}
> fixes it for 4.8/4.9 as well.

So, perhaps (but so far totally untested, the other bootstrap is still
running):

2023-10-18  Jakub Jelinek  <jakub@redhat.com>

	* poly-int.h (poly_int::poly_int): Ensure the const Cs &...
	argument ctor has at least one argument.

--- gcc/poly-int.h.jj	2023-10-13 19:34:44.112832389 +0200
+++ gcc/poly-int.h	2023-10-18 13:49:29.038751482 +0200
@@ -379,8 +379,8 @@ public:
   template<typename Ca>
   poly_int (const poly_int<N, Ca> &);
 
-  template<typename ...Cs>
-  constexpr poly_int (const Cs &...);
+  template<typename C0, typename ...Cs>
+  constexpr poly_int (const C0 &, const Cs &...);
 
   poly_int &operator = (const poly_int &) = default;
 
@@ -446,11 +446,11 @@ poly_int<N, C>::poly_int (const poly_int
 }
 
 template<unsigned int N, typename C>
-template<typename ...Cs>
+template<typename C0, typename ...Cs>
 inline constexpr
-poly_int<N, C>::poly_int (const Cs &... cs)
+poly_int<N, C>::poly_int (const C0 &c0, const Cs &... cs)
   : poly_int (typename poly_int_fullness<sizeof... (Cs) >= N>::type (),
-	      cs...) {}
+	      c0, cs...) {}
 
 /* Initialize with c0, cs..., and some trailing zeros.  */
 template<unsigned int N, typename C>


	Jakub


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-10-18 11:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-04 22:19 [PATCH] Support g++ 4.8 as a host compiler Roger Sayle
2023-10-05  5:52 ` Xi Ruoyao
2023-10-07 16:19 ` Jeff Law
2023-10-07 21:30   ` Sam James
2023-10-08  4:40     ` Jeff Law
2023-10-08  8:08       ` Iain Sandoe
2023-10-15 10:50   ` Roger Sayle
2023-10-15 11:43     ` Richard Sandiford
2023-10-18  8:37       ` Eric Gallager
2023-10-18 10:17       ` Jakub Jelinek
2023-10-18 10:23         ` Richard Sandiford
2023-10-18 11:33           ` Jakub Jelinek
2023-10-18 11:51             ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).