wwwdocs: Note that old reload is deprecated

Hi!

Happy new year everyone.

Is this patch okay to commit?


Segher

---
 htdocs/gcc-13/changes.html | 2 ++
 1 file changed, 2 insertions(+)

Does this mean that targets that have an option to use LRA or not should now default to LRA?  Some currently default to old reload.

	paul


> On Jan 5, 2023, at 2:27 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> Hi!
> 
> Happy new year everyone.
> 
> Is this patch okay to commit?
> 
> 
> Segher
> 
> ---
> htdocs/gcc-13/changes.html | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/htdocs/gcc-13/changes.html b/htdocs/gcc-13/changes.html
> index 3876ad77543a..954469cdcfa4 100644
> --- a/htdocs/gcc-13/changes.html
> +++ b/htdocs/gcc-13/changes.html
> @@ -39,6 +39,8 @@ a work-in-progress.</p>
>     <li>Legacy debug info compression option <code>-gz=zlib-gnu</code> was removed
>       and the option is ignored right now.</li>
>     <li>New debug info compression option value <code>-gz=zstd</code> has been added.</li>
> +    <li>Support for old reload is deprecated. It will be removed in a future
> +      release. Every target will always use LRA from then on.</li>
> </ul>
> 
> 
> -- 
> 1.8.3.1
>

On Thu, Jan 05, 2023 at 02:54:04PM -0500, Paul Koning wrote:
> > On Jan 5, 2023, at 2:27 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> > +    <li>Support for old reload is deprecated. It will be removed in a future
> > +      release. Every target will always use LRA from then on.</li>

> Does this mean that targets that have an option to use LRA or not should now default to LRA?  Some currently default to old reload.

It would be a good idea to do that in GCC 13 already, yes.


Segher

On Thu, 5 Jan 2023, Segher Boessenkool wrote:
> Happy new year everyone.
> 
> Is this patch okay to commit?

From a wwwdocs perspective, yes. 

Are you also *asking* from an architectural/"strategic" perspective, 
or simply *informing*? :-)  The former I cannot approve, the latter I 
certainly can.

Gerald

On Wed, Jan 11, 2023 at 3:22 PM Gerald Pfeifer <gerald@pfeifer.com> wrote:
>
> On Thu, 5 Jan 2023, Segher Boessenkool wrote:
> > Happy new year everyone.
> >
> > Is this patch okay to commit?
>
> From a wwwdocs perspective, yes.
>
> Are you also *asking* from an architectural/"strategic" perspective,
> or simply *informing*? :-)  The former I cannot approve, the latter I
> certainly can.

Note this is more info for port maintainers not for users and
changes.html is for users.  "In a future release" is also quite vague.

Richard.

>
> Gerald

On Wed, Jan 11, 2023 at 03:34:45PM +0100, Richard Biener wrote:
> On Wed, Jan 11, 2023 at 3:22 PM Gerald Pfeifer <gerald@pfeifer.com> wrote:
> >
> > On Thu, 5 Jan 2023, Segher Boessenkool wrote:
> > > Happy new year everyone.
> > >
> > > Is this patch okay to commit?
> >
> > From a wwwdocs perspective, yes.
> >
> > Are you also *asking* from an architectural/"strategic" perspective,
> > or simply *informing*? :-)  The former I cannot approve, the latter I
> > certainly can.

Strategic, yes.  Good way of phrasing it, thanks :-)

> Note this is more info for port maintainers not for users and
> changes.html is for users.

And users will notice some ports will have to be removed, because those
ports are not maintained / not maintained enough.  Some ports will not
work with LRA, most will be easy to fix, but someone will have to do
that.  If no one does so the port works sufficiently well it will have
to be removed before release.

> "In a future release" is also quite vague.

It's what we usually say in changes.html .  "In GCC 14" if you want?

I can add some stuff on how this will benefit users?


Segher

> Am 11.01.2023 um 16:17 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
> 
> On Wed, Jan 11, 2023 at 03:34:45PM +0100, Richard Biener wrote:
>>> On Wed, Jan 11, 2023 at 3:22 PM Gerald Pfeifer <gerald@pfeifer.com> wrote:
>>> 
>>> On Thu, 5 Jan 2023, Segher Boessenkool wrote:
>>>> Happy new year everyone.
>>>> 
>>>> Is this patch okay to commit?
>>> 
>>> From a wwwdocs perspective, yes.
>>> 
>>> Are you also *asking* from an architectural/"strategic" perspective,
>>> or simply *informing*? :-)  The former I cannot approve, the latter I
>>> certainly can.
> 
> Strategic, yes.  Good way of phrasing it, thanks :-)
> 
>> Note this is more info for port maintainers not for users and
>> changes.html is for users.
> 
> And users will notice some ports will have to be removed, because those
> ports are not maintained / not maintained enough.  Some ports will not
> work with LRA, most will be easy to fix, but someone will have to do
> that.  If no one does so the port works sufficiently well it will have
> to be removed before release.
> 
>> "In a future release" is also quite vague.
> 
> It's what we usually say in changes.html .  "In GCC 14" if you want?
> 
> I can add some stuff on how this will benefit users?

I guess listing the ports without LRA support might be a first step for clarification?


> 
> 
> Segher

On Wed, Jan 11, 2023 at 05:27:36PM +0100, Richard Biener wrote:
> > Am 11.01.2023 um 16:17 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
> >> Note this is more info for port maintainers not for users and
> >> changes.html is for users.
> > 
> > And users will notice some ports will have to be removed, because those
> > ports are not maintained / not maintained enough.  Some ports will not
> > work with LRA, most will be easy to fix, but someone will have to do
> > that.  If no one does so the port works sufficiently well it will have
> > to be removed before release.
> > 
> >> "In a future release" is also quite vague.
> > 
> > It's what we usually say in changes.html .  "In GCC 14" if you want?
> > 
> > I can add some stuff on how this will benefit users?
> 
> I guess listing the ports without LRA support might be a first step for clarification?

Every port has LRA support.

Some ports will not build later when we delete old reload, because they
use some functions and/or data structures unique to that.

But all that is easily fixed (for the port maintainers at least,
assuming they understand what their code does ;-) ).  The bigger problem
is that if the port has never been tested with LRA the chances of it
working in all cases are not great (say 50%), so likely some attention
will be needed to get the compiler back to release quality.  And some
ports will even not work for the simplest pieces of source code.  Those
are the problematic cases.

Usually not hard to fix -- all the more complicated targets already run
LRA always, the hard work is done already -- but it still requires a
target maintainer (with a suitable testing environment, hopefully even
hardware) to do the work.  This is what I want to alert people to, and
get agreement that this will happen next major release.


Segher

> Am 11.01.2023 um 19:34 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
> 
> On Wed, Jan 11, 2023 at 05:27:36PM +0100, Richard Biener wrote:
>>>> Am 11.01.2023 um 16:17 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
>>>>> Note this is more info for port maintainers not for users and
>>>>> changes.html is for users.
>>> 
>>> And users will notice some ports will have to be removed, because those
>>> ports are not maintained / not maintained enough.  Some ports will not
>>> work with LRA, most will be easy to fix, but someone will have to do
>>> that.  If no one does so the port works sufficiently well it will have
>>> to be removed before release.
>>> 
>>>> "In a future release" is also quite vague.
>>> 
>>> It's what we usually say in changes.html .  "In GCC 14" if you want?
>>> 
>>> I can add some stuff on how this will benefit users?
>> 
>> I guess listing the ports without LRA support might be a first step for clarification?
> 
> Every port has LRA support.
> 
> Some ports will not build later when we delete old reload, because they
> use some functions and/or data structures unique to that.
> 
> But all that is easily fixed (for the port maintainers at least,
> assuming they understand what their code does ;-) ).  The bigger problem
> is that if the port has never been tested with LRA the chances of it
> working in all cases are not great (say 50%), so likely some attention
> will be needed to get the compiler back to release quality.  And some
> ports will even not work for the simplest pieces of source code.  Those
> are the problematic cases.

Like if they cannot even build their target libraries aka their build will fail.  It would be nice to identify those and, say, make at least -mlra available to all ports that currently do not have a way to enable LRA?

Richard 

> Usually not hard to fix -- all the more complicated targets already run
> LRA always, the hard work is done already -- but it still requires a
> target maintainer (with a suitable testing environment, hopefully even
> hardware) to do the work.  This is what I want to alert people to, and
> get agreement that this will happen next major release.
> 
> 
> Segher

> On Jan 11, 2023, at 1:32 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Wed, Jan 11, 2023 at 05:27:36PM +0100, Richard Biener wrote:
>>> Am 11.01.2023 um 16:17 schrieb Segher Boessenkool <segher@kernel.crashing.org>:
>>>> Note this is more info for port maintainers not for users and
>>>> changes.html is for users.
>>> 
>>> And users will notice some ports will have to be removed, because those
>>> ports are not maintained / not maintained enough.  Some ports will not
>>> work with LRA, most will be easy to fix, but someone will have to do
>>> that.  If no one does so the port works sufficiently well it will have
>>> to be removed before release.
>>> 
>>>> "In a future release" is also quite vague.
>>> 
>>> It's what we usually say in changes.html .  "In GCC 14" if you want?
>>> 
>>> I can add some stuff on how this will benefit users?
>> 
>> I guess listing the ports without LRA support might be a first step for clarification?
> 
> Every port has LRA support.
> 
> Some ports will not build later when we delete old reload, because they
> use some functions and/or data structures unique to that.

Or, as in my case, because building with LRA as the default triggers an ICE that I don't understand.  I posted a note to the GCC list about what I saw, but have received no reaction.

If anyone can help me understand how LRA can generate RTL with register choices that violate the constraints listed in the MD file, I would be grateful.

	paul

On Wed, Jan 11, 2023 at 07:39:29PM +0100, Richard Biener wrote:
> Like if they cannot even build their target libraries aka their build will fail.  It would be nice to identify those and, say, make at least -mlra available to all ports that currently do not have a way to enable LRA?

It is up to the target maintainers to make such support, it is a machine
flag after all (-m are machine flags, -f are more general flags).

There has been ample warning, see <https://gcc.gnu.org/wiki/LRAIsDefault>
for example.  GCC 13 release will be six years after that, I'd hope that
that is enough.

Just using
  targetm.lra_p = default_lra_p;
is enough to test.  I don't have a setup to build all targets (that
requires target headers, to begin with), and it is up to the target
maintainers to decide how they want things fixed anyway.

I'll put up a preliminary branch for the generic patches, but let me
update it to trunk first :-)


Segher

On Wed, Jan 11, 2023 at 01:42:22PM -0500, Paul Koning wrote:
> Or, as in my case, because building with LRA as the default triggers an ICE that I don't understand.  I posted a note to the GCC list about what I saw, but have received no reaction.

<https://gcc.gnu.org/pipermail/gcc/2023-January/240497.html>?

I would say your predicates are way too lenient here (general_operand),
but this needs more info.  A testcase to reproduce the problem, to
start with :-)


Segher

> On Jan 11, 2023, at 2:28 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Wed, Jan 11, 2023 at 01:42:22PM -0500, Paul Koning wrote:
>> Or, as in my case, because building with LRA as the default triggers an ICE that I don't understand.  I posted a note to the GCC list about what I saw, but have received no reaction.
> 
> <https://gcc.gnu.org/pipermail/gcc/2023-January/240497.html>?

I just saw that, thanks!

> I would say your predicates are way too lenient here (general_operand),
> but this needs more info.  A testcase to reproduce the problem, to
> start with :-)

I'll try to trim it down.

What do you mean "too lenient"?  The first input operand (which is supposed to be the same as the output since the instruction set is 2-address) is "general_operand".  The destination is "nonimmediate_operand" which fits the constraints applied to it.

	paul

On Wed, Jan 11, 2023 at 8:09 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Wed, Jan 11, 2023 at 07:39:29PM +0100, Richard Biener wrote:
> > Like if they cannot even build their target libraries aka their build will fail.  It would be nice to identify those and, say, make at least -mlra available to all ports that currently do not have a way to enable LRA?
>
> It is up to the target maintainers to make such support, it is a machine
> flag after all (-m are machine flags, -f are more general flags).
>
> There has been ample warning, see <https://gcc.gnu.org/wiki/LRAIsDefault>
> for example.  GCC 13 release will be six years after that, I'd hope that
> that is enough.
>
> Just using
>   targetm.lra_p = default_lra_p;
> is enough to test.  I don't have a setup to build all targets (that
> requires target headers, to begin with), and it is up to the target
> maintainers to decide how they want things fixed anyway.
>
> I'll put up a preliminary branch for the generic patches, but let me
> update it to trunk first :-)

Just saying that the changes.html note has not much information but instead will
spread FUD without indicating which ports would be dysfunctional after removing
reload support (aka will even no longer build).  So I'd say we don't want this
note in changes.html in the proposed form.

Richard.

>
>
> Segher

On Thu, Jan 12, 2023 at 8:44 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Wed, Jan 11, 2023 at 8:09 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Wed, Jan 11, 2023 at 07:39:29PM +0100, Richard Biener wrote:
> > > Like if they cannot even build their target libraries aka their build will fail.  It would be nice to identify those and, say, make at least -mlra available to all ports that currently do not have a way to enable LRA?
> >
> > It is up to the target maintainers to make such support, it is a machine
> > flag after all (-m are machine flags, -f are more general flags).
> >
> > There has been ample warning, see <https://gcc.gnu.org/wiki/LRAIsDefault>
> > for example.  GCC 13 release will be six years after that, I'd hope that
> > that is enough.
> >
> > Just using
> >   targetm.lra_p = default_lra_p;
> > is enough to test.  I don't have a setup to build all targets (that
> > requires target headers, to begin with), and it is up to the target
> > maintainers to decide how they want things fixed anyway.
> >
> > I'll put up a preliminary branch for the generic patches, but let me
> > update it to trunk first :-)
>
> Just saying that the changes.html note has not much information but instead will
> spread FUD without indicating which ports would be dysfunctional after removing
> reload support (aka will even no longer build).  So I'd say we don't want this
> note in changes.html in the proposed form.

Btw, the following is the ports that default to reload and have no command line
option to switch to LRA.

config/alpha/alpha.cc:#define TARGET_LRA_P hook_bool_void_false
config/avr/avr.cc:#define TARGET_LRA_P hook_bool_void_false
config/bfin/bfin.cc:#define TARGET_LRA_P hook_bool_void_false
config/c6x/c6x.cc:#define TARGET_LRA_P hook_bool_void_false
config/cris/cris.cc:#define TARGET_LRA_P hook_bool_void_false
config/epiphany/epiphany.cc:#define TARGET_LRA_P hook_bool_void_false
config/fr30/fr30.cc:#define TARGET_LRA_P hook_bool_void_false
config/frv/frv.cc:#define TARGET_LRA_P hook_bool_void_false
config/h8300/h8300.cc:#define TARGET_LRA_P hook_bool_void_false
config/ia64/ia64.cc:#define TARGET_LRA_P hook_bool_void_false
config/iq2000/iq2000.cc:#define TARGET_LRA_P hook_bool_void_false
config/lm32/lm32.cc:#define TARGET_LRA_P hook_bool_void_false
config/m32c/m32c.cc:#define TARGET_LRA_P hook_bool_void_false
config/m32r/m32r.cc:#define TARGET_LRA_P hook_bool_void_false
config/m68k/m68k.cc:#define TARGET_LRA_P hook_bool_void_false
config/mcore/mcore.cc:#define TARGET_LRA_P hook_bool_void_false
config/microblaze/microblaze.cc:#define TARGET_LRA_P hook_bool_void_false
config/mmix/mmix.cc:#define TARGET_LRA_P hook_bool_void_false
config/mn10300/mn10300.cc:#define TARGET_LRA_P hook_bool_void_false
config/moxie/moxie.cc:#define TARGET_LRA_P hook_bool_void_false
config/msp430/msp430.cc:#define TARGET_LRA_P hook_bool_void_false
config/nvptx/nvptx.cc:#define TARGET_LRA_P hook_bool_void_false
config/pa/pa.cc:#define TARGET_LRA_P hook_bool_void_false
config/rl78/rl78.cc:#define TARGET_LRA_P hook_bool_void_false
config/stormy16/stormy16.cc:#define TARGET_LRA_P hook_bool_void_false
config/visium/visium.cc:#define TARGET_LRA_P hook_bool_void_false

On Wed, Jan 11, 2023 at 05:07:47PM -0500, Paul Koning wrote:
> > On Jan 11, 2023, at 2:28 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> > I would say your predicates are way too lenient here (general_operand),
> > but this needs more info.  A testcase to reproduce the problem, to
> > start with :-)
> 
> I'll try to trim it down.
> 
> What do you mean "too lenient"?  The first input operand (which is supposed to be the same as the output since the instruction set is 2-address) is "general_operand".  The destination is "nonimmediate_operand" which fits the constraints applied to it.

I mean general_operand accepts that sp thing you saw.  But your
constraints do not?  (I don't know your target well, maybe this isn't
true).  Things like this should be sorted out by reload, but you get
better code (and fewer problems ;-) ) if you make code that fits
earlier.  The L in LRA means "local": it "just" makes things fit, it
does not emphasise optimising your code.


Segher

> On Jan 12, 2023, at 4:50 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Wed, Jan 11, 2023 at 05:07:47PM -0500, Paul Koning wrote:
>>> On Jan 11, 2023, at 2:28 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>>> I would say your predicates are way too lenient here (general_operand),
>>> but this needs more info.  A testcase to reproduce the problem, to
>>> start with :-)
>> 
>> I'll try to trim it down.
>> 
>> What do you mean "too lenient"?  The first input operand (which is supposed to be the same as the output since the instruction set is 2-address) is "general_operand".  The destination is "nonimmediate_operand" which fits the constraints applied to it.
> 
> I mean general_operand accepts that sp thing you saw.  But your
> constraints do not?  (I don't know your target well, maybe this isn't
> true).  Things like this should be sorted out by reload, but you get
> better code (and fewer problems ;-) ) if you make code that fits
> earlier.  The L in LRA means "local": it "just" makes things fit, it
> does not emphasise optimising your code.

The destination is "nonimmediate_operand" which matches what the machine actually does.  It's like VAX and M68k, instruction operands in general can be registers, memory references, register indirect or memory indirect, memory at register with offset, or autoinc/autodec off any register.

As far as operand type is concerned, SP is certainly a valid operand for an add operation, that isn't the problem.  The problem is that it's a two operand machine: the first operand of the add instruction is both source and destination.  And LRA assigned the source register to be the destination register as required, but then after doing so it replaced the destination (an FP reference) by a different register (SP reference), violating the constraint after having satisfied it earlier.

Interesting to know what LRA stands for, I didn't know that.

	paul

On Thu, Jan 12, 2023 at 09:17:31AM -0500, Paul Koning wrote:
> > On Jan 12, 2023, at 4:50 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> > I mean general_operand accepts that sp thing you saw.  But your
> > constraints do not?  (I don't know your target well, maybe this isn't
> > true).  Things like this should be sorted out by reload, but you get
> > better code (and fewer problems ;-) ) if you make code that fits
> > earlier.  The L in LRA means "local": it "just" makes things fit, it
> > does not emphasise optimising your code.
> 
> The destination is "nonimmediate_operand" which matches what the machine actually does.  It's like VAX and M68k, instruction operands in general can be registers, memory references, register indirect or memory indirect, memory at register with offset, or autoinc/autodec off any register.
> 
> As far as operand type is concerned, SP is certainly a valid operand for an add operation, that isn't the problem.  The problem is that it's a two operand machine: the first operand of the add instruction is both source and destination.  And LRA assigned the source register to be the destination register as required, but then after doing so it replaced the destination (an FP reference) by a different register (SP reference), violating the constraint after having satisfied it earlier.

Ah okay.  Yes, something does not verify if the instructions are valid
before doing some replacement.  Something around ELIMINABLE_REGS it
looks like?  Maybe the dump file says more, or maybe the dump file can
be improved.

> Interesting to know what LRA stands for, I didn't know that.

First line of lra.cc:
/* LRA (local register allocator) driver and LRA utilities.

:-)


Segher

> On Jan 12, 2023, at 9:40 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Thu, Jan 12, 2023 at 09:17:31AM -0500, Paul Koning wrote:
>>> On Jan 12, 2023, at 4:50 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>>> I mean general_operand accepts that sp thing you saw.  But your
>>> constraints do not?  (I don't know your target well, maybe this isn't
>>> true).  Things like this should be sorted out by reload, but you get
>>> better code (and fewer problems ;-) ) if you make code that fits
>>> earlier.  The L in LRA means "local": it "just" makes things fit, it
>>> does not emphasise optimising your code.
>> 
>> The destination is "nonimmediate_operand" which matches what the machine actually does.  It's like VAX and M68k, instruction operands in general can be registers, memory references, register indirect or memory indirect, memory at register with offset, or autoinc/autodec off any register.
>> 
>> As far as operand type is concerned, SP is certainly a valid operand for an add operation, that isn't the problem.  The problem is that it's a two operand machine: the first operand of the add instruction is both source and destination.  And LRA assigned the source register to be the destination register as required, but then after doing so it replaced the destination (an FP reference) by a different register (SP reference), violating the constraint after having satisfied it earlier.
> 
> Ah okay.  Yes, something does not verify if the instructions are valid
> before doing some replacement.  Something around ELIMINABLE_REGS it
> looks like?  Maybe the dump file says more, or maybe the dump file can
> be improved.

The Reload dump file mentions in a couple of places that it sees r5 (FP) as eliminable, replaced by r6 (SP).  The IRA dump file says nothing.

Yes, the ELIMINABLE_REGS macro says FP can be eliminated, which is correct.  In fact, if it weren't for that, it would be wrong for the register allocator to pick it (R5) as a general register to hold the result of that addhi3.  So the trouble is that the replacement is made after that register allocation, and given the constraint the allocator actually needs to generate an additional instruction, a move from SP to R5 so it can then do the two-operand add the constraint requires.

This feels like a bug; should I file a bug report?  Or is there something the target code can do to make this work?  I looked through the internals manual a bit, it doesn't show anything helpful.  The register elimination hooks are rather generic and don't give me any handle to recognize cases like this one.

	paul