On Mon, 2026-03-02 at 08:41 -0700, Jeffrey Law wrote: > > On 3/2/2026 5:59 AM, Oleg Endo wrote: > > Ping. > > > > I'd like to try to merge the pending SH LRA changes before the GCC 16 > > release. The debian guys have been using a patchedGCC version with SH LRA > > enabled for a while now and it seems feasible, at least from this point of > > view. > > > > I'd appreciate some feedback on this.
Thanks for your attention, Jeff! Firstly, I forgot to mention the original PR in the subject line https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117182 > I wanted to look at it deeper before commenting, but my first impression > was this patch is a bad idea. > I can relate to your concern. In my first message I wrote: "Eventually LRA should be taught to check that any substitution is valid....". Below the expanded version of it. > It sounds like it's papering over a > target issue, but again, I really want to look at it more deeply. As Kaz mentions in the PR, the relevant SH insn "movsf_ie_ra" has alternatives that have fp-mode requirements and alternatives that don't, i.e. that insn alternative is valid only when cpu/fpu is in the specified mode. The problem is that LRA changes the alternative in the insn without checking whether it would violate the cpu/fpu-mode setting for the insn. The next problem, according to my understanding, is that there is actually nothing there that is storing/tracking the current cpu/fpu-mode across insns. It seems it's not even directly possible to determine the cpu/fpu- mode state for a given insn directly outside of mode-switching.cc. We can try to _guess_ it by looking at which alternative is currently selected, but it still can't give a definitive answer. So once mode-switching.cc is done (before RA) the insn lists become rather fragile in regard to this. Given an insn, nobody really knows whether it's safe to change the mode/alternative after the initial mode-switching.cc has been done. Right now, the only way for anything outside the backend to tell whether an insn is affected by cpu/fpu-mode switching is through "targetm.mode_switching.needed". And the only place where that is used is mode-switching.cc. Hence my conclusion that this is not actually a backend issue but rather an issue in the whole cpu/fpu-mode switching approach. It worked OK before because there was nothing there that would change insn alternatives after cpu/fpu-mode switching in the way LRA does now. I guess the proper check could be added _somewhere_ in lra- constraints.cc:process_alt_operands. But we'd also have to add cpu/fp-mode state info to each insn first. > Given we're not actually removing reload and non-LRA targets this > release (sigh) we have a bit of time. > There have been recent requests to improve support for SH4 FPU utilization. In addition to the single/double precision fp-mode the thing has another mode that controls whether fp loads/stores are 32-bit or 64-bit. Adding insult to injury, the modes are interdependent. Currently, the SH backend is using only the single/double cpu/fpu-mode for switching. I was thinking to extend this cover the fp-move-mode-thing as well. Although I have no idea what to expect in terms of resulting code quality. In any case, now I realize it would would just amplify the issue at hand. The whole SH move insn stuff turned into an unmaintainable gibberish, imo. Lockstepping through the insn alternatives and trying to guess what it's supposed to accomplish is just too tedious and error prone. Adding the LRA thing on top makes it even worse, as it runs into issues like this one. Perhaps it'd be better to split the patterns into smaller bites and also chop off some old cruft that is no longer needed/valid. However, I'd do that only after switching to LRA. Please let me know what you find out and your thoughts on this. Best regards, Oleg Endo
