On 6/12/25 4:04 AM, Richard Sandiford wrote:
Richard, Jeff, it does not seem appropriate to merge this patch now,
given that it breaks avr and or1k. Let me know if such experiment is
desired despite the known breakages.
It's a bit of a judgment call -- we don't require changes to be clean
across every target, that would be an undue testing burden on contributors.
But knowing it's causing an avr fail means we definitely need to be a
bit more cautious. An argument could be made that this is a bug in the
avr port -- the code in question looks quite bogus and the fact that the
"fix" is to stop using reload.
I think a reasonable step forward would be to put this into my tester
and see if anything else pops out. If nothing else pops out then I
would suggest we flip the AVR port to LRA and install the patch. If
other things pop out, then we'll have to revise the plan.
So I'll throw it into my tester. We'll have data on the crosses
tomorrow AM my time.
Thanks, sounds good to me FWIW. I agree that we shouldn't let dubious
port-specific code hold the patch back. But the cross-port testing
would help detect whether there is a legitimate corner case that we
haven't thought about. (Neither the avr nor the or1k cases are
legitimate corner cases, IMO.)
Nothing else popped from that test, either in the runtime builds or new
testsuite fails.
So I figured it would be advisable to test LRA on the AVR port. Many
many months ago I did a sweep through the reload targets and converted
those which trivially worked with LRA (ie, everything builds, no
testsuite regressions). The fact that AVR still defaults to reload
indicates I either missed testing AVR or that it failed. I'm pretty
sure it was the latter given it just failed :(
./../../..//gcc/libgcc/libgcc2.c: In function '__ucmpdi2':
../../../..//gcc/libgcc/libgcc2.c:2060:1: internal compiler error: in
update_reg_eliminate, at lra-eliminations.cc:1200
So now the question is do we let this patch go forward now as the two
issues exposed (or1k, avr) are both target specific issues. Or do we
force this patch to wait.
I'm inclined to wait a week or so to give the port maintainers some time
to dive into the issues, then go forward.
Jeff
