On Thu, Feb 6, 2025 at 11:40 PM Vladimir Makarov <vmaka...@redhat.com> wrote: > > > On 2/6/25 4:54 PM, Richard Sandiford wrote: > > Vladimir Makarov <vmaka...@redhat.com> writes: > > This is a complicated problem resulted in many tries to fix it in some > general way. > > In general I am agree with Surya's approach to scale cost of reg > saves/restores somehow. But the general approach, although solved some > problems, also created a lot of new ones. May be because IRA does not > take some other aspects of using callee saved regs. And some of them > were addressed by other patches. e.g. recently proposed by Surya and one > for PR118497. > > I also agree with Richard Sandiford's comment that we should avoid > introducing the new hooks for RA and I actually tried to stick to this > policy for a long time. But I don't see another solution to introducing > the new hook in this case. It is hard to figure out generally in RA > that saves/restores will be insns different from ld/st (e.g. x86 > push/pop) and that they will be cheaper. > > So after some time to think about the patch I decided to approve the RA > part of the patch. I also hope that the work on this problem will > continue (e.g. improving default and target hook implementations and > documentation how to better use it). > > For the record, I strongly object to this. The hook just seems like a > complete hack to me. Even if we accept that there is target-specific > information in play, the hook isn't providing that information. > > In contrast to what you said above, my objection isn't to having hooks > -- those are often needed and good. What bothers me is that the hook > isn't well designed. Hooks should provide information rather than > override code by brute force. > > On the other hand, I accept that you're (rightly!) the maintainer. > > I also don't like the hook implementation for x86-64 (although this is a > matter of target maintainers). All these costs look voodoo and random to me. > > But this problem is longing for more than half year. I spent a lot of time > too on this. Patches were submitted and reverted and nobody did find so far > any solution satisfying all GCC tests. If somebody finds a solution without > the hook, I will be glad to get rid off the hook. Also the related PRs are > marked as P1 ones, it means people think they are important (I am not sure > about this myself). Without fixing them (or downgrading them) there will be > no GCC release. So I am in difficult situation with these PRs and need some > resolution.
Just to chime in as the one who likely made those PRs P1. 'P1' here is really about the testsuite FAIL, how to resolve it is up to target maintainers - P1 should make them look, and adjusting the testcase (or even XFAILing it) is a valid resolution of the P1 regression. I do agree with Richards comment on the proposed hook to be badly designed and I do like Honzas suggestion to instead hookize the biasing. Richard. > >
