> 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 certainly understand the pain with heuirstics changes distubring testsuite and bringing some regressions and some improvements making it difficult to weight overall impact. With inliner heuristics this happens often. We are good on tracking regressions, but if something improves, it goes without saying. If some heuristics is formely wrong and fixing it introduces some problems, we are kind of biassed towards keeping old, broken one. Consistency is good, but I think in this specific case we should see overall improvements. The testcases where we previously shrink-wrapped and now use caller saved registers are IMO not necessarily a problem. We had bug in cost model that made us to use callee saved registers too often and shrink-wrapping helped to mitigate it. Using caller saved register has same effect if frequency of caller saving sums to 1. So perhaps adjusting the testcases to have more calls will restore original behaviour in cases where they are intendd to verify that shrink-wrapping works. Naturally it would be nice to make the cost model anticipate the shrink-wrapping but that looks like quite hard problem to me, since precise conditions are known only post-reload. Perhaps one can special case some early exit conditions where shrink-wrapping is most important. I will prepare patch for noreturns. They are probably not too important in practice but easy to get right. Honza
