Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Fri, 16 Feb 2024, Maciej W. Rozycki wrote:

> On Fri, 16 Feb 2024, Jakub Jelinek wrote:
> 
> > >  There is no function prologue to optimise in the VAX case, because all 
> > > the frame setup has already been made by the CALLS instruction itself in 
> > > the caller.  The first machine instruction of the callee is technically 
> > > already past the "prologue".  And then RET serves as the whole function 
> > > "epilogue".
> > 
> > So, what is the problem with DWARF unwinding?  Just make sure to emit
> > appropriate instructions describing the saving of the corresponding
> > registers at specific points based on CFA at the start of the function
> > (so that it appears in CIE instructions) and that should be all that is
> > needed, no?
> 
>  I may not remember all the issues correctly offhand as it's been a while 
> since I looked into it, but as I recall DWARF handling code has not been 
> prepared for all the frame to have been already allocated and initialised 
> at a function's entry point, and also at least DWARF-4 is IIRC required to 
> have statics at offsets positive from FP (for a stack growing downwards).

 There is a further complication actually where lazy binding is in use.  
In that case a function that has been jumped to indirectly from the lazy 
resolver will often have a different number of statics saved in the frame 
from where the function has been called directly via a fully resolved PLT 
GOT entry.

 This is because at the time the lazy resolver is being called it is not 
known what statics the ultimate callee wants to save, as it is not a part 
of the ABI.  Therefore the worst condition is assumed and the resolver 
requests all the statics (R6-R11) to be saved, observing that saving more 
statics than required makes no change to code semantics, it just hurts 
performance (but calls to the lazy resolver are rare, so this is not a big 
deal).  Conversely when the function has been already resolved, the PLT 
GOT entry points at the callee instead, which will then only save the 
statics it has requested itself, knowing them to be used.

 Obviously a frame that has all the statics saved will have a different 
size of its variable part and slots will have been assigned differently 
there from the case where only some statics have been saved.  Of course it 
does not matter for regular code execution as RET will always correctly 
interpret a stack frame and restore exactly these statics that have been 
saved in the frame, but for unwinding actual frame contents have to be 
interpreted.

 I am not sure if this run-time dependent frame layout can be described in 
DWARF terms even, so I am leaning towards concluding a native unwinder is 
the only feasible way to go.

 For those who are unaware how information as to what statics are to be 
saved is made available by functions with VAX hardware: it is embedded at 
the function's address in a form of a 16-bit data quantity, which is a 
register save bitmask (an entry mask in VAX-speak) for registers R0-R11;
1 in the mask requests that the corresponding register be saved in the 
callee's frame by the CALLS instruction.  Once the frame has been built by 
CALLS, control is then passed to the location immediately following the 
bitmask, which is the function's actual entry point, i.e. the PC is set 
to the function's address + 2.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Fri, 16 Feb 2024, Jakub Jelinek wrote:

> >  There is no function prologue to optimise in the VAX case, because all 
> > the frame setup has already been made by the CALLS instruction itself in 
> > the caller.  The first machine instruction of the callee is technically 
> > already past the "prologue".  And then RET serves as the whole function 
> > "epilogue".
> 
> So, what is the problem with DWARF unwinding?  Just make sure to emit
> appropriate instructions describing the saving of the corresponding
> registers at specific points based on CFA at the start of the function
> (so that it appears in CIE instructions) and that should be all that is
> needed, no?

 I may not remember all the issues correctly offhand as it's been a while 
since I looked into it, but as I recall DWARF handling code has not been 
prepared for all the frame to have been already allocated and initialised 
at a function's entry point, and also at least DWARF-4 is IIRC required to 
have statics at offsets positive from FP (for a stack growing downwards).

 There was an issue about restoring the caller's value of SP too, which is 
callee's AP+4*(*AP)+4 (AP being the argument pointer, which is a hardreg, 
pointing at #arguments), or alternatively it can be calculated as the sum 
of FP, the fixed frame size (20), the variable frame size (4*#statics, 
recorded as a register mask in a 12-bit field at *(FP+6)), the #arguments 
slot (4), and the argument space size (4*#arguments).  Obviously the 
former calculation is easier to make (#arguments has to be zero-extended 
from bits 7:0).

 FAOD the #arguments slot is filled by the CALLS instruction at call time 
and can vary between callers obviously for varargs, but also cases such as 
open(2), so to reconstruct the caller's SP you always need to examine the 
actual stack frame, and it does not appear to be supported by libgcc DWARF 
EH code as it stands.

 I've left out stack realignment stuff or CALLG instruction support from 
the consideration above, as we don't handle these features in our psABI, 
all of which have provisions for from hardware in the stack frame.

 As I say I may have missed or confused something, and anyway I'll take 
another stab at it, hopefully soon.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Paul Koning]

> On Feb 16, 2024, at 6:34 AM, Maciej W. Rozycki  wrote:
> 
> On Thu, 15 Feb 2024, Paul Koning wrote:
> 
>>> On May 15, 2023, at 5:09 PM, Maciej W. Rozycki  wrote:
>>> 
>>> ...
>>> 
>>> I may choose to implement a non-DWARF unwinder instead, as the VAX stack 
>>> frame is always fully described by the hardware and there is never ever a 
>>> need for debug information to be able to decode any VAX stack frame (the 
>>> RET machine instruction uses the stack frame information to restore the 
>>> previous PC, FP, SP, AP and any static registers saved by CALLS).
>> 
>> That would make sense; it's like the heuristic unwinder found in some 
>> other targets (I remember the MIPS one, which worked fairly well and 
>> allowed debugging without useable debug data).
> 
> Not really, in particular because EH unwinding has to be reliable and 
> heuristics inherently is not.

Fair enough, but what I meant is only that it's conceptually similar: unwind 
based on the code and machine state, rather than auxiliary information like 
debug data.  And I think your point was that on VAX this is indeed a reliable 
technique by virtue of the iSA.  In fact, the standard way to do exeception 
handling unwinding is part of the originail VAX architecture (via the otherwise 
unused first word (I think) of the call frame).

paul

Re: [PATCH] Turn on LRA on all targets

[Jakub Jelinek]

On Fri, Feb 16, 2024 at 02:23:54PM +, Maciej W. Rozycki wrote:
> On Fri, 16 Feb 2024, Segher Boessenkool wrote:
> 
> > >  Conversely no heuristics is required to unwind VAX frames, because they 
> > > are fixed in layout by hardware, fully self-described, and with the 
> > > hardware frame pointer always available.
> > 
> > The downside of the VAX situation of course is that the compiler has no
> > freedom to optimise the frame and *logue code at all, let alone well.
> > This may not matter so much on narrow ucoded in-order machines, there
> > are different balances there :-)
> 
>  There is no function prologue to optimise in the VAX case, because all 
> the frame setup has already been made by the CALLS instruction itself in 
> the caller.  The first machine instruction of the callee is technically 
> already past the "prologue".  And then RET serves as the whole function 
> "epilogue".

So, what is the problem with DWARF unwinding?  Just make sure to emit
appropriate instructions describing the saving of the corresponding
registers at specific points based on CFA at the start of the function
(so that it appears in CIE instructions) and that should be all that is
needed, no?

Jakub

Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Fri, 16 Feb 2024, Segher Boessenkool wrote:

> >  Conversely no heuristics is required to unwind VAX frames, because they 
> > are fixed in layout by hardware, fully self-described, and with the 
> > hardware frame pointer always available.
> 
> The downside of the VAX situation of course is that the compiler has no
> freedom to optimise the frame and *logue code at all, let alone well.
> This may not matter so much on narrow ucoded in-order machines, there
> are different balances there :-)

 There is no function prologue to optimise in the VAX case, because all 
the frame setup has already been made by the CALLS instruction itself in 
the caller.  The first machine instruction of the callee is technically 
already past the "prologue".  And then RET serves as the whole function 
"epilogue".

 A discussion happened at the VAX/NetBSD mailing list as to a change to 
the calling convention to make it more RISC-like and replace the procedure 
call (CALLS and CALLG; the latter unused in our psABI) and return (RET) 
instructions with the subroutine branch (BSB), jump (JSB) and return (RSB) 
instructions, which are similar in semantics to say x86 CALL (for BSB/JSB) 
and RET (for RSB) instructions.  Should that happen we'd have fine-grained 
control over the function prologues and epilogues.

 It's not clear however what the consequences such a change would have on 
performance, and it would surely increase code size.  And the final VAX 
microarchitecture implementations (NVAX/NVAX+, dating back to 1991) have 
provisions in hardware, unsurprisingly, for optimising the execution speed 
of the CALL* and RET instructions.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Fri, Feb 16, 2024 at 11:34:55AM +, Maciej W. Rozycki wrote:
>  Not really, in particular because EH unwinding has to be reliable and 
> heuristics inherently is not.

Yup.  Which is why I did 0359465c703a for rs6000 six years ago (how time
flies!)  The commit message for that includes

To find out where on-entry register values live at any point in a
program, GDB currently tries to parse to parse the executable code.
This does not work very well, for example it gets confused if some
accesses to the stack use the frame pointer (r31) and some use the
stack pointer (r1).  A symptom is that backtraces can be cut short.

and the patch does

+  /* By default, always emit DWARF-2 unwind info.  This allows debugging
+ without maintaining a stack frame back-chain.  It also allows the
+ debugger to find out where on-entry register values are stored at any
+ point in a function, without having to analyze the executable code (which
+ isn't even possible to do in the general case).  */
+#ifdef OBJECT_FORMAT_ELF
+  opts->x_flag_asynchronous_unwind_tables = 1;
+#endif

We went through very many refinements of the heuristics through the
years, but at some point you just have to give up: heuristics never
can make up for missing information.

>  Consequently the more aggressive the compiler has become to schedule
> function body instructions within a function's prologue the more lost the 
> machine code interpreter has become.  Ultimately it would have to become a 
> full-fledged CPU simulator to do its heuristics.

Yup, exactly.

> In reality it means the 
> unwinder may fail to produce acceptable results, which will happen at any 
> frequency between hardly ever to most often, depending on the exact 
> circumstances.

Yes.  If the compiler optimises the *logue code well, there is no way
heuristics can follow that.

>  Conversely no heuristics is required to unwind VAX frames, because they 
> are fixed in layout by hardware, fully self-described, and with the 
> hardware frame pointer always available.

The downside of the VAX situation of course is that the compiler has no
freedom to optimise the frame and *logue code at all, let alone well.
This may not matter so much on narrow ucoded in-order machines, there
are different balances there :-)


Segher

Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Thu, 15 Feb 2024, Paul Koning wrote:

> > On May 15, 2023, at 5:09 PM, Maciej W. Rozycki  wrote:
> > 
> > ...
> > 
> > I may choose to implement a non-DWARF unwinder instead, as the VAX stack 
> > frame is always fully described by the hardware and there is never ever a 
> > need for debug information to be able to decode any VAX stack frame (the 
> > RET machine instruction uses the stack frame information to restore the 
> > previous PC, FP, SP, AP and any static registers saved by CALLS).
> 
> That would make sense; it's like the heuristic unwinder found in some 
> other targets (I remember the MIPS one, which worked fairly well and 
> allowed debugging without useable debug data).

 Not really, in particular because EH unwinding has to be reliable and 
heuristics inherently is not.

 The MIPS heuristic unwinder continues living in GDB; I have extended it
to the microMIPS ISA at one point.  It has a major flaw though: the MIPS 
psABI uses a variable frame layout, with the frame maintained solely by 
software and with no fixed hardware frame pointer, so to analyse it in the 
absence of debug information the instruction sequence of the function's in 
question prologue has to be decoded to discover the location of individual 
frame slots.

 Consequently the more aggressive the compiler has become to schedule
function body instructions within a function's prologue the more lost the 
machine code interpreter has become.  Ultimately it would have to become a 
full-fledged CPU simulator to do its heuristics.  In reality it means the 
unwinder may fail to produce acceptable results, which will happen at any 
frequency between hardly ever to most often, depending on the exact 
circumstances.

 A mixed approach by interpreting lightweight PDR (Procedure Description 
Record) information inherited from the ECOFF Mdebug format combined with 
function prologue scanning might be more reliable, because in that case 
frame slot positions are known and the only unknown is the code locations 
they are initialised each at.  So all the prologue scanner has to know it 
is the store machine instructions and any that set a frame pointer from 
the stack pointer.  All the other instructions can be simply ignored.  And 
then only in the innermost frame, because any outer frames must have been 
fully set up already.  But I never got to implementing it and remnants of 
the PDR unwinder have long been removed from GDB.

 Conversely no heuristics is required to unwind VAX frames, because they 
are fixed in layout by hardware, fully self-described, and with the 
hardware frame pointer always available.  Therefore to unwind a VAX frame 
steps similar to those made by hardware on a function return (machine RET 
instruction) can simply be recreated from information produced by hardware 
at the function call and recorded in the stack frame and registers.  There 
is room reserved in the stack frame for a pointer to an exception handler 
even ("condition handler" in VAX-speak), preset to zero (a null pointer) 
by hardware at function entry.

 It does seem really attractive (and saves some storage space, which VAX 
hardware users will likely appreciate), but implies dedicated libgcc code 
as opposed to reusing common bits, which may or may not be welcome by the 
community for such an exotic corner case target.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Thu, Feb 15, 2024 at 08:41:42PM -0500, Paul Koning wrote:
> > On Feb 15, 2024, at 5:56 PM, Segher Boessenkool 
> >  wrote:
> > 
> > On Thu, Feb 15, 2024 at 07:34:32PM +, Sam James wrote:
> >> I have now started doing this in PR113932.
> > 
> > Thank you!
> > 
> > Segher
> 
> Presumably this isn't for version 14 since it's in a late stage, right?  I 
> have my bits about ready to go in but I'll wait for State 1 to open.  Correct?

Absolutely.  It was decided early in stage 1 that this wasn't going to
happen for 14.

It appears most of the anywhere near hard targets have not done anything
though.  I'll just push very hard for 15.  But you will be fine :-)


Segher

Re: [PATCH] Turn on LRA on all targets

[Paul Koning]

> On Feb 15, 2024, at 5:56 PM, Segher Boessenkool  
> wrote:
> 
> On Thu, Feb 15, 2024 at 07:34:32PM +, Sam James wrote:
>> I have now started doing this in PR113932.
> 
> Thank you!
> 
> Segher

Presumably this isn't for version 14 since it's in a late stage, right?  I have 
my bits about ready to go in but I'll wait for State 1 to open.  Correct?

paul

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Thu, Feb 15, 2024 at 07:34:32PM +, Sam James wrote:
> I have now started doing this in PR113932.

Thank you!


Segher

Re: [PATCH] Turn on LRA on all targets

[Paul Koning]

> On May 15, 2023, at 5:09 PM, Maciej W. Rozycki  wrote:
> 
> ...
> 
> I may choose to implement a non-DWARF unwinder instead, as the VAX stack 
> frame is always fully described by the hardware and there is never ever a 
> need for debug information to be able to decode any VAX stack frame (the 
> RET machine instruction uses the stack frame information to restore the 
> previous PC, FP, SP, AP and any static registers saved by CALLS).

That would make sense; it's like the heuristic unwinder found in some other 
targets (I remember the MIPS one, which worked fairly well and allowed 
debugging without useable debug data).

paul

Re: [PATCH] Turn on LRA on all targets

[Sam James]

Sam James  writes:

> [[PGP Signed Part:Undecided]]
>
> "Maciej W. Rozycki"  writes:
>
>> On Sun, 23 Apr 2023, Segher Boessenkool wrote:
>>
>>> >  There are extra ICEs in regression testing and code quality is poor; cf. 
>>> > .  
>>> 
>>> Do you have something you can show for this?  Maybe in a PR?
>>
>>  I have filed no PRs as I didn't assess the collateral damage at the time 
>> I looked at it.  I only ran regression-testing with `-mlra' shortly after 
>> I completed MODE_CC conversion and added the option, to see what lies 
>> beyond.  And I only added `-mlra' and made minimal changes to make the 
>> compiler build again just to make it easier to proceed towards LRA.
>
> I think before moving forward with the plan in general, a PR is ideally
> needed for each target anyway. Not all machine maintainers actively watch the
> MLs.

I have now started doing this in PR113932.

>
> [[End of PGP Signed Part]]



signature.asc
Description: PGP signature

Re: [PATCH] Turn on LRA on all targets

[Sam James via Gcc-patches]

"Maciej W. Rozycki"  writes:

> On Sun, 23 Apr 2023, Segher Boessenkool wrote:
>
>> >  There are extra ICEs in regression testing and code quality is poor; cf. 
>> > .  
>> 
>> Do you have something you can show for this?  Maybe in a PR?
>
>  I have filed no PRs as I didn't assess the collateral damage at the time 
> I looked at it.  I only ran regression-testing with `-mlra' shortly after 
> I completed MODE_CC conversion and added the option, to see what lies 
> beyond.  And I only added `-mlra' and made minimal changes to make the 
> compiler build again just to make it easier to proceed towards LRA.

I think before moving forward with the plan in general, a PR is ideally
needed for each target anyway. Not all machine maintainers actively watch the
MLs.


signature.asc
Description: PGP signature

Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Sun, 23 Apr 2023, Segher Boessenkool wrote:

> >  There are extra ICEs in regression testing and code quality is poor; cf. 
> > .  
> 
> Do you have something you can show for this?  Maybe in a PR?

 I have filed no PRs as I didn't assess the collateral damage at the time 
I looked at it.  I only ran regression-testing with `-mlra' shortly after 
I completed MODE_CC conversion and added the option, to see what lies 
beyond.  And I only added `-mlra' and made minimal changes to make the 
compiler build again just to make it easier to proceed towards LRA.

> And, are the ICEs in the generic code, or something vax-specific?

 At least some were in generic code, e.g.:

during RTL pass: combine
.../gcc/testsuite/gcc.c-torture/compile/pr101562.c: In function 'foo':
.../gcc/testsuite/gcc.c-torture/compile/pr101562.c:12:1: internal compiler 
error: in insert, at wide-int.cc:682
Please submit a full bug report,
with preprocessed source if appropriate.
See  for instructions.
compiler exited with status 1
FAIL: gcc.c-torture/compile/pr101562.c   -O1  (internal compiler error)
FAIL: gcc.c-torture/compile/pr101562.c   -O1  (test for excess errors)

(coming from `gcc_checking_assert (precision >= width)'), or:

In file included from .../gcc/testsuite/g++.dg/modules/xtreme-header-2.h:10,
 from .../gcc/testsuite/g++.dg/modules/xtreme-header-2_a.H:4:
.../vax-netbsdelf/libstdc++-v3/include/regex:42: internal compiler error: in 
set_filename, at cp/module.cc:19134
Please submit a full bug report,
with preprocessed source if appropriate.
See  for instructions.
compiler exited with status 1
FAIL: g++.dg/modules/xtreme-header-2_a.H -std=c++2b (internal compiler error)
FAIL: g++.dg/modules/xtreme-header-2_a.H -std=c++2b (test for excess errors)

(from `gcc_checking_assert (!filename)').  As I say, I did not assess this 
at all back then and the logs are dated Nov 2021 (I had to chase them).

 Also I'm not going to dedicate any time now to switch the VAX backend to 
LRA, because old reload continues working while we have a non-functional 
exception unwinder that never ever worked, as I have recently discovered, 
which breaks lots of C++ code, including in particular native VAX/NetBSD 
GDB and `gdbserver' (my newly-ported implementation of), which is a bit of 
a problem (native VAX/NetBSD GCC has been spared owing to the decision not 
to use exceptions).

 And fixing the unwinder is going to be a major effort due to how the VAX 
CALLS machine instruction works and the stack frame has been consequently 
structured; it is unlike any other ELF target, and even if it can be 
expressed in DWARF terms (which I'm not entirely sure about), it is going 
to require a dedicated handler like with ARM or IA64.

 I may choose to implement a non-DWARF unwinder instead, as the VAX stack 
frame is always fully described by the hardware and there is never ever a 
need for debug information to be able to decode any VAX stack frame (the 
RET machine instruction uses the stack frame information to restore the 
previous PC, FP, SP, AP and any static registers saved by CALLS).

 So implementing a working exception unwinder has to take precedence over 
LRA and I do hope to complete it during this release cycle, but I may not 
have any time left for LRA.

 Please keep this in mind with any plans to drop old reload.  I'll highly 
appreciate that and I do keep LRA on my radar as the next item to address 
after the unwinder, by any means it's not been lost.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Jeff Law via Gcc-patches]

On 4/23/23 10:47, Segher Boessenkool wrote:

This minimal patch enables LRA for all targets.  It does not clean up
the target code, nor does it do anything to generic code: it just
deletes all target definitions of TARGET_LRA_P.

There are three kinds of changes:

1) Targets that already always have LRA, but that redefine the hook
anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
really changes for these targets with this patch (but later patches
will delete the superfluous hook implementations).
2) Targets that have LRA selectable.  Some of those are probably fine,
since they default to using LRA (arc, mips, s390).  Two others don't
though, maybe because there are problems (ft32 and sh).  I'd love to
hear from all targets in this category what the status is, how easy it
was to convert, etc.
3) Targets that as of yet never used LRA.  Many of those will be fine,
but some others will need a little tuning, and a few might need some
actual improvements to LRA itself.  These are cris, epiphany, fr30,
frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
find out how many of those targets are ever tested, and how many of
those work with LRA without further changes, and how well.

I send this patch now so that people can start testing.  I don't plan to
commit this for another week at least, for a week after GCC 13 release I
guess?  How does that plan sound to people?

This is an RFC, so no changelog, no one can accidentally commit it :-)
I thought about Cc:ing lots and lots of people, should I do that?ISTM that we ought to go through the non-LRA targets one by one to see 
which can be trivially converted and will still work.  Any that meet 
that criteria should just be converted.


While we may have some performance deltas, I would argue we just move 
forward.  The maintainers can and should be participating in getting us 
moved away from reload.


Of the list you mentioned, I would just remove m32c.  It hasn't been 
able to even build newlib in years and while I did spend some time 
debugging it, my conclusion was it was not salvagable.  It should just 
be deprecated.


For epiphany, it faults semi-randomly in reload last I looked and I 
haven't even tried to have the tester build newlib on that platform in 
eons.  I think epiphany should be deprecated as well.



Jeff

Re: [PATCH] Turn on LRA on all targets

[Jeff Law via Gcc-patches]

On 4/29/23 07:37, Roger Sayle wrote:


Segher Boessenkool wrote:

I send this patch now so that people can start testing.

diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc
index 89349dae9e62..e32f17377525 100644
--- a/gcc/config/nvptx/nvptx.cc
+++ b/gcc/config/nvptx/nvptx.cc
@@ -7601,9 +7601,6 @@ nvptx_asm_output_def_from_decls (FILE *stream, tree

name, tree value)

#undef TARGET_ATTRIBUTE_TABLE
#define TARGET_ATTRIBUTE_TABLE nvptx_attribute_table

-#undef TARGET_LRA_P
-#define TARGET_LRA_P hook_bool_void_false
-
#undef TARGET_LEGITIMATE_ADDRESS_P
#define TARGET_LEGITIMATE_ADDRESS_P nvptx_legitimate_address_p



I've tested Segher's patch on nvptx-none with make and make -k check and
can confirm there are no new regressions.  Nvptx is unique in that it
doesn't
use register allocation, i.e. GCC's only TARGET_NO_REGISTER_ALLOCATION
target,
so it's a little odd that it specifies which register allocator it doesn't
use.

I hope this helps,
It does.  Consider a patch which flips the nvptx port to LRA as 
pre-approved.


I tried the FRV just for fun.  It faulted all over the place :(

jeff

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

Hi!

On Mon, Apr 24, 2023 at 11:46:50AM +0200, Uros Bizjak wrote:
> On Mon, Apr 24, 2023 at 11:19 AM Segher Boessenkool
>  wrote:
> > We still need someone to test this on alpha now, years later, and give
> > a final okay, but hearing this is encouraging :-)
> 
> Please note that bootstrap worked on alpha*EV6*, not plain alpha.
> 
> Plain alpha is !BWX architecture and uses {un,}aligned_memory_operand
> predicates that call resolve_reload_operand function. Unfortunately,
> this function peeks deep into reload internals (reg_equiv_memory_loc)
> that has no equivalent in LRA. As said in the comment, this internal
> function resolves what reload is going to do with OP if it is a
> register.

Bootstrap works with everything I tried, but building Linux fails with a
few things like
/home/segher/src/kernel/drivers/tty/serial/serial_core.c:1029:1: internal 
compiler error: maximum number of generated reload insns per insn achieved (90)
(it uses -mcpu=ev5 there; to reproduce just (try to) build a defconfig).


Segher

Re: [PATCH] Turn on LRA on all targets

[Roger Sayle]

Segher Boessenkool wrote:
> I send this patch now so that people can start testing.
>
> diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc
> index 89349dae9e62..e32f17377525 100644
> --- a/gcc/config/nvptx/nvptx.cc
> +++ b/gcc/config/nvptx/nvptx.cc
> @@ -7601,9 +7601,6 @@ nvptx_asm_output_def_from_decls (FILE *stream, tree
name, tree value)
> #undef TARGET_ATTRIBUTE_TABLE
> #define TARGET_ATTRIBUTE_TABLE nvptx_attribute_table
>
>-#undef TARGET_LRA_P
>-#define TARGET_LRA_P hook_bool_void_false
>-
> #undef TARGET_LEGITIMATE_ADDRESS_P
> #define TARGET_LEGITIMATE_ADDRESS_P nvptx_legitimate_address_p


I've tested Segher's patch on nvptx-none with make and make -k check and
can confirm there are no new regressions.  Nvptx is unique in that it
doesn't
use register allocation, i.e. GCC's only TARGET_NO_REGISTER_ALLOCATION
target,
so it's a little odd that it specifies which register allocator it doesn't
use.

I hope this helps,
Roger
--

Re: [PATCH] Turn on LRA on all targets

[Uros Bizjak via Gcc-patches]

On Mon, Apr 24, 2023 at 11:19 AM Segher Boessenkool
 wrote:
>
> On Sun, Apr 23, 2023 at 11:06:41PM +0200, Uros Bizjak wrote:
> > > I send this patch now so that people can start testing.  I don't plan to
> > > commit this for another week at least, for a week after GCC 13 release I
> > > guess?  How does that plan sound to people?
> >
> > An old patch to enable Alpha is at
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66207
>
> Ah, good to hear it worked way back then already!
>
> We still need someone to test this on alpha now, years later, and give
> a final okay, but hearing this is encouraging :-)

Please note that bootstrap worked on alpha*EV6*, not plain alpha.

Plain alpha is !BWX architecture and uses {un,}aligned_memory_operand
predicates that call resolve_reload_operand function. Unfortunately,
this function peeks deep into reload internals (reg_equiv_memory_loc)
that has no equivalent in LRA. As said in the comment, this internal
function resolves what reload is going to do with OP if it is a
register.

Uros.

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Mon, Apr 24, 2023 at 10:19:23AM +0200, Richard Biener wrote:
> On Sun, Apr 23, 2023 at 6:48 PM Segher Boessenkool
>  wrote:
> >
> > This minimal patch enables LRA for all targets.  It does not clean up
> > the target code, nor does it do anything to generic code: it just
> > deletes all target definitions of TARGET_LRA_P.
> >
> > There are three kinds of changes:
> >
> > 1) Targets that already always have LRA, but that redefine the hook
> > anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> > really changes for these targets with this patch (but later patches
> > will delete the superfluous hook implementations).
> > 2) Targets that have LRA selectable.  Some of those are probably fine,
> > since they default to using LRA (arc, mips, s390).  Two others don't
> > though, maybe because there are problems (ft32 and sh).  I'd love to
> > hear from all targets in this category what the status is, how easy it
> > was to convert, etc.

(Note I misqualified a bunch under 1) that should be 2)).

> I think 1) and 2) where the default is LRA already should be changed to
> use the existing default and the ability to switch back to reload removed.
> That could be done independently and soon.

Sure.  Those categories of backends are easy to handle anyway.  Cat. 3
is the problematic category.

> > 3) Targets that as of yet never used LRA.  Many of those will be fine,
> > but some others will need a little tuning, and a few might need some
> > actual improvements to LRA itself.  These are cris, epiphany, fr30,
> > frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
> > mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
> > find out how many of those targets are ever tested, and how many of
> > those work with LRA without further changes, and how well.
> >
> > I send this patch now so that people can start testing.  I don't plan to
> > commit this for another week at least, for a week after GCC 13 release I
> > guess?  How does that plan sound to people?
> 
> I think it's important to check that the targets still build, including a
> sensible set of their target libraries in a sensible set of their
> multilib configurations so they are not completely cut off from
> testing.
> 
> "Converting" targets one-by-one this way is sensible and appreciated.

Yes.  I sent this all-in-one simple-to-apply simple-to-test RFC patch
to try and get things moving, especially for all those targets where all
previous attempts have done nothing.

> > This is an RFC, so no changelog, no one can accidentally commit it :-)
> > I thought about Cc:ing lots and lots of people, should I do that?
> 
> Did we announce a cut-off for reload going away?

I tried to do that in 

but it was decided we should not announce this in user documentation.
There has been talk about this on the developer mailing lists (and
elsewhere, irc for example) for ages, so it shouldn't come as a surprise
to anyone.

> Knowing the set of
> targets that fail to "auto-convert" within the above constraint would be
> nice.

Yes, but the response from most of the category 3 targets has been
thunderous silence :-/

I'll test how many targets fall down at my boring "build Linux" (so, the
kernel, not a distro :-) ) test, already.  A bit scared of doing that,
but it is time I guess.

> Thanks for pushing!

"It's a dirty job", etc.

So many ancient shortcomings of GCC cannot reasonably be solved while we
still allow old reload to be used (subregs of mem anyone?  But that is
just one example).  Let's see where this goes!


Segher

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Sun, Apr 23, 2023 at 11:06:41PM +0200, Uros Bizjak wrote:
> > I send this patch now so that people can start testing.  I don't plan to
> > commit this for another week at least, for a week after GCC 13 release I
> > guess?  How does that plan sound to people?
> 
> An old patch to enable Alpha is at
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66207

Ah, good to hear it worked way back then already!

We still need someone to test this on alpha now, years later, and give
a final okay, but hearing this is encouraging :-)


Segher

Re: [PATCH] Turn on LRA on all targets

[Andreas Schwab]

Here is the result for m68k:

libtool: compile:  /daten/aranym/gcc/gcc-20230424/Build/./gcc/xgcc 
-shared-libgcc -B/daten/aranym/gcc/gcc-20230424/Build/./gcc -nostdinc++ 
-L/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/src 
-L/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/src/.libs 
-L/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/libsupc++/.libs 
-B/daten/cross/m68k-linux/m68k-linux/bin/ 
-B/daten/cross/m68k-linux/m68k-linux/lib/ -isystem 
/daten/cross/m68k-linux/m68k-linux/include -isystem 
/daten/cross/m68k-linux/m68k-linux/sys-include 
-I/daten/aranym/gcc/gcc-20230424/libstdc++-v3/../libgcc 
-I/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include/m68k-linux
 -I/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include 
-I/daten/aranym/gcc/gcc-20230424/libstdc++-v3/libsupc++ -std=gnu++11 
-D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra -Wwrite-strings 
-Wcast-qual -Wabi=2 -fdiagnostics-show-location=once -ffunction-sections 
-fdata-sections -frandom-seed=wlocale-inst.lo -g -O2 -D_GNU_SOURCE -c 
../../../../../libstdc++-v3/src/c++11/wlocale-inst.cc  -fPIC -DPIC 
-D_GLIBCXX_SHARED -o wlocale-inst.o
during RTL pass: reload
In file included from 
/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include/bits/locale_facets.h:2687,
 from 
/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include/locale:42,
 from ../../../../../libstdc++-v3/src/c++11/locale-inst.cc:38,
 from ../../../../../libstdc++-v3/src/c++11/wlocale-inst.cc:35:
/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include/bits/locale_facets.tcc:
 In member function '_InIter std::num_get<_CharT, _InIter>::do_get(iter_type, 
iter_type, std::ios_base&, std::ios_base::iostate&, bool&) const [with _CharT = 
wchar_t; _InIter = std::istreambuf_iterator 
>]':
/daten/aranym/gcc/gcc-20230424/Build/m68k-linux/libstdc++-v3/include/bits/locale_facets.tcc:686:5:
 internal compiler error: maximum number of generated reload insns per insn 
achieved (90)
  686 | }
  | ^
0xe8a5e8 lra_constraints(bool)
../../gcc/lra-constraints.cc:5319
0xe71f32 lra(_IO_FILE*)
../../gcc/lra.cc:2375
0xe21731 do_reload
../../gcc/ira.cc:5967
0xe21731 execute
../../gcc/ira.cc:6153

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] Turn on LRA on all targets

[Richard Biener via Gcc-patches]

On Sun, Apr 23, 2023 at 6:48 PM Segher Boessenkool
 wrote:
>
> This minimal patch enables LRA for all targets.  It does not clean up
> the target code, nor does it do anything to generic code: it just
> deletes all target definitions of TARGET_LRA_P.
>
> There are three kinds of changes:
>
> 1) Targets that already always have LRA, but that redefine the hook
> anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> really changes for these targets with this patch (but later patches
> will delete the superfluous hook implementations).
> 2) Targets that have LRA selectable.  Some of those are probably fine,
> since they default to using LRA (arc, mips, s390).  Two others don't
> though, maybe because there are problems (ft32 and sh).  I'd love to
> hear from all targets in this category what the status is, how easy it
> was to convert, etc.

I think 1) and 2) where the default is LRA already should be changed to
use the existing default and the ability to switch back to reload removed.
That could be done independently and soon.

> 3) Targets that as of yet never used LRA.  Many of those will be fine,
> but some others will need a little tuning, and a few might need some
> actual improvements to LRA itself.  These are cris, epiphany, fr30,
> frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
> mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
> find out how many of those targets are ever tested, and how many of
> those work with LRA without further changes, and how well.
>
> I send this patch now so that people can start testing.  I don't plan to
> commit this for another week at least, for a week after GCC 13 release I
> guess?  How does that plan sound to people?

I think it's important to check that the targets still build, including a
sensible set of their target libraries in a sensible set of their
multilib configurations so they are not completely cut off from
testing.

"Converting" targets one-by-one this way is sensible and appreciated.

>
> This is an RFC, so no changelog, no one can accidentally commit it :-)
> I thought about Cc:ing lots and lots of people, should I do that?

Did we announce a cut-off for reload going away?  Knowing the set of
targets that fail to "auto-convert" within the above constraint would be
nice.

Thanks for pushing!
Richard.

>
>
> Segher
> ---
>  gcc/config/alpha/alpha.cc   | 3 ---
>  gcc/config/arc/arc.cc   | 2 --
>  gcc/config/avr/avr.cc   | 3 ---
>  gcc/config/bfin/bfin.cc | 3 ---
>  gcc/config/c6x/c6x.cc   | 3 ---
>  gcc/config/cris/cris.cc | 3 ---
>  gcc/config/epiphany/epiphany.cc | 2 --
>  gcc/config/fr30/fr30.cc | 3 ---
>  gcc/config/frv/frv.cc   | 3 ---
>  gcc/config/ft32/ft32.cc | 3 ---
>  gcc/config/gcn/gcn.cc   | 2 --
>  gcc/config/h8300/h8300.cc   | 3 ---
>  gcc/config/ia64/ia64.cc | 3 ---
>  gcc/config/iq2000/iq2000.cc | 3 ---
>  gcc/config/lm32/lm32.cc | 2 --
>  gcc/config/m32c/m32c.cc | 3 ---
>  gcc/config/m32r/m32r.cc | 3 ---
>  gcc/config/m68k/m68k.cc | 3 ---
>  gcc/config/mcore/mcore.cc   | 3 ---
>  gcc/config/microblaze/microblaze.cc | 3 ---
>  gcc/config/mips/mips.cc | 2 --
>  gcc/config/mmix/mmix.cc | 3 ---
>  gcc/config/mn10300/mn10300.cc   | 3 ---
>  gcc/config/msp430/msp430.cc | 3 ---
>  gcc/config/nvptx/nvptx.cc   | 3 ---
>  gcc/config/pa/pa.cc | 3 ---
>  gcc/config/pdp11/pdp11.cc   | 3 ---
>  gcc/config/rl78/rl78.cc | 3 ---
>  gcc/config/rx/rx.cc | 3 ---
>  gcc/config/s390/s390.cc | 3 ---
>  gcc/config/sh/sh.cc | 3 ---
>  gcc/config/sparc/sparc.cc   | 3 ---
>  gcc/config/stormy16/stormy16.cc | 3 ---
>  gcc/config/vax/vax.cc   | 3 ---
>  gcc/config/visium/visium.cc | 3 ---
>  gcc/config/xtensa/xtensa.cc | 3 ---
>  36 files changed, 103 deletions(-)
>
> diff --git a/gcc/config/alpha/alpha.cc b/gcc/config/alpha/alpha.cc
> index 1d826085198e..784348dbe787 100644
> --- a/gcc/config/alpha/alpha.cc
> +++ b/gcc/config/alpha/alpha.cc
> @@ -10094,9 +10094,6 @@ alpha_can_change_mode_class (machine_mode from, 
> machine_mode to,
>  #define TARGET_MANGLE_TYPE alpha_mangle_type
>  #endif
>
> -#undef TARGET_LRA_P
> -#define TARGET_LRA_P hook_bool_void_false
> -
>  #undef TARGET_LEGITIMATE_ADDRESS_P
>  #define TARGET_LEGITIMATE_ADDRESS_P alpha_legitimate_address_p
>
> diff --git a/gcc/config/arc/arc.cc b/gcc/config/arc/arc.cc
> index 22eb2e9cb459..8cd5a794073f 100644
> --- a/gcc/config/arc/arc.cc
> +++ b/gcc/config/arc/arc.cc
> @@ -804,8 +804,6 @@ static rtx arc_legitimize_address_0 (rtx, rtx, 
> machine_mode mode);
>  #define TARGET_NO_SPECULATION_IN_DELAY_SLOTS_P \
>arc_no_speculation_in_delay_slots_p
>
> -#undef TARGET_LRA_P
> -#define TARGET_

Re: [PATCH] Turn on LRA on all targets

[Uros Bizjak via Gcc-patches]

On Sun, Apr 23, 2023 at 6:48 PM Segher Boessenkool
 wrote:
>
> This minimal patch enables LRA for all targets.  It does not clean up
> the target code, nor does it do anything to generic code: it just
> deletes all target definitions of TARGET_LRA_P.
>
> There are three kinds of changes:
>
> 1) Targets that already always have LRA, but that redefine the hook
> anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> really changes for these targets with this patch (but later patches
> will delete the superfluous hook implementations).
> 2) Targets that have LRA selectable.  Some of those are probably fine,
> since they default to using LRA (arc, mips, s390).  Two others don't
> though, maybe because there are problems (ft32 and sh).  I'd love to
> hear from all targets in this category what the status is, how easy it
> was to convert, etc.
> 3) Targets that as of yet never used LRA.  Many of those will be fine,
> but some others will need a little tuning, and a few might need some
> actual improvements to LRA itself.  These are cris, epiphany, fr30,
> frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
> mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
> find out how many of those targets are ever tested, and how many of
> those work with LRA without further changes, and how well.
>
> I send this patch now so that people can start testing.  I don't plan to
> commit this for another week at least, for a week after GCC 13 release I
> guess?  How does that plan sound to people?

An old patch to enable Alpha is at

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66207

Uros.

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

On Sun, Apr 23, 2023 at 07:56:56PM +0100, Maciej W. Rozycki wrote:
> On Sun, 23 Apr 2023, Segher Boessenkool wrote:
> > 1) Targets that already always have LRA, but that redefine the hook
> > anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> > really changes for these targets with this patch (but later patches
> > will delete the superfluous hook implementations).
> 
>  Umm, no, VAX has LRA selectable and for a reason it defaults to off:

I read TARGET_LRA as something that denotes LRA is selected :-)  See the
other mail.  So, vax is in category 2).

>  There are extra ICEs in regression testing and code quality is poor; cf. 
> .  

Do you have something you can show for this?  Maybe in a PR?

And, are the ICEs in the generic code, or something vax-specific?


Segher

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

(You didn't leave me in Cc: on the reply.  Maybe you did a
reply-to-only-one-person?)

On Sun, Apr 23, 2023 at 11:01:05AM -0600, Jeff Law via Gcc-patches wrote:
> On 4/23/23 10:47, Segher Boessenkool wrote:
> >3) Targets that as of yet never used LRA.  Many of those will be fine,
> >but some others will need a little tuning, and a few might need some
> >actual improvements to LRA itself.  These are cris, epiphany, fr30,
> >frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
> >mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
> >find out how many of those targets are ever tested, and how many of
> >those work with LRA without further changes, and how well.
> These test daily except for m68k and pa which test weekly ;-)

Ah, very good!  But still none of them are tested with LRA at all, after
so many years :-(


Segher

Re: [PATCH] Turn on LRA on all targets

[Segher Boessenkool]

Hi!

On Sun, Apr 23, 2023 at 02:36:05PM -0400, Paul Koning wrote:
> > On Apr 23, 2023, at 12:47 PM, Segher Boessenkool 
> >  wrote:
> > 1) Targets that already always have LRA, but that redefine the hook
> > anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> > really changes for these targets with this patch (but later patches
> > will delete the superfluous hook implementations).
> 
> I thought that the existing coding for pdp11 makes LRA selectable (via -mlra) 
> and defaults to off.

static bool
pdp11_lra_p (void)
{
  return TARGET_LRA;
}

Ah, that is a target flag, not an enum value or such?  Aha!

mlra
Target Mask(LRA)
Use LRA register allocator.

This is true for sparc and vax and xtensa as well, and rx with
TARGET_ENABLE_LRA.  gcn does in fact do
#define TARGET_LRA_P hook_bool_void_true
which is a funny way to have the same effect as not defining it at all.

So these five targets are category 2).  Thanks for correcting me!

> I had planned to change it to default to on but leave it selectable.  I 
> suppose just having it on is ok too, although the code from LRA wasn't as 
> efficient as the old last I looked (which is a while ago).

The plan is to delete old reload completely, with all follow-up
simplifications and cleanups.


Segher

Re: [PATCH] Turn on LRA on all targets

[Maciej W. Rozycki]

On Sun, 23 Apr 2023, Segher Boessenkool wrote:

> 1) Targets that already always have LRA, but that redefine the hook
> anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> really changes for these targets with this patch (but later patches
> will delete the superfluous hook implementations).

 Umm, no, VAX has LRA selectable and for a reason it defaults to off:

> diff --git a/gcc/config/vax/vax.cc b/gcc/config/vax/vax.cc
> index 82a176d3bfc9..17fdc4483797 100644
> --- a/gcc/config/vax/vax.cc
> +++ b/gcc/config/vax/vax.cc
> @@ -114,9 +114,6 @@ static HOST_WIDE_INT vax_starting_frame_offset (void);
>  #undef TARGET_STRUCT_VALUE_RTX
>  #define TARGET_STRUCT_VALUE_RTX vax_struct_value_rtx
>  
> -#undef TARGET_LRA_P
> -#define TARGET_LRA_P vax_lra_p
> -

 There are extra ICEs in regression testing and code quality is poor; cf. 
.  
I'll have a look into it sometime, but it may not be soon and the broken 
VAX exception unwinder is more important and will have to take precedence 
anyway.  I have fixing the unwinder scheduled hopefully for this release 
cycle, but LRA will have to wait.

  Maciej

Re: [PATCH] Turn on LRA on all targets

[Paul Koning via Gcc-patches]

> On Apr 23, 2023, at 12:47 PM, Segher Boessenkool  
> wrote:
> 
> This minimal patch enables LRA for all targets.  It does not clean up
> the target code, nor does it do anything to generic code: it just
> deletes all target definitions of TARGET_LRA_P.
> 
> There are three kinds of changes:
> 
> 1) Targets that already always have LRA, but that redefine the hook
> anyway.  These are gcn, pdp11, rx, sparc, vax, and xtensa.  Nothing
> really changes for these targets with this patch (but later patches
> will delete the superfluous hook implementations).

I thought that the existing coding for pdp11 makes LRA selectable (via -mlra) 
and defaults to off.  I had planned to change it to default to on but leave it 
selectable.  I suppose just having it on is ok too, although the code from LRA 
wasn't as efficient as the old last I looked (which is a while ago).

paul

Re: [PATCH] Turn on LRA on all targets

[Jeff Law via Gcc-patches]

On 4/23/23 10:47, Segher Boessenkool wrote:

3) Targets that as of yet never used LRA.  Many of those will be fine,
but some others will need a little tuning, and a few might need some
actual improvements to LRA itself.  These are cris, epiphany, fr30,
frv, h8300, ia64, iq2000, lm32, m32c, m32r, m68k, mcore, microblaze,
mmix, mn10300, msp430, nvptx, pa, rl78, stormy16, and visium.  We'll
find out how many of those targets are ever tested, and how many of
those work with LRA without further changes, and how well.

These test daily except for m68k and pa which test weekly ;-)

Jeff