This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch: Revert find_reloads part of the IRA merge

On 07/14/2010 09:03 PM, Jeff Law wrote:
> On 07/14/10 05:28, Bernd Schmidt wrote:
>> Attached is a patch which tries to solve the problem in a different way;
>> I fixed some other problems that showed up in testing on ARM. 
>> Specifically:
>>    * Immediately marking used_spill_regs as ever_live can cause a
>>      situation where the next iteration will use a different set of
>>      hard regs, and we end up pushing registers that are never used.
>>      If we delay settings regs_ever_live until nothing else has
>>      changed, we can avoid that, at the cost of a little bit of
>>      complexity, and the risk of requiring more iterations.  Not
>>      sure this is worth it.
> How often have you seen this occur in practice?  The complexity is
> trivial, so if it's occuring with any regularity, I'd say go for it.

Anecdotally, I seem to recall seeing this every now and then, and it's
been bugging me for a while.  Practically, in the last test run I think
it only affected one testcase out of the many I have collected for
comparing assembly output.  I've added this code because one of the two
other changes made it show up.

>>    * When evaluating costs of alternatives, reload isn't taking into
>>      account whether something is going to need secondary reloads.
> I'm all for more accurate cost modeling.

Yes.  I think I'll put this in (after a bit more thought and some
testing on its own), it seems like the right thing to do.

>>    * Use live_throughout sets to determine if a reload will force
>>      a spill.  This seems to work better than the previous heuristic,
>>      but it can still fail (dead_or_set isn't taken into account
>>      since it's quite a useless set, the test isn't run on address
>>      reloads, etc.)
> Well, you're detecting one of (many) cases where we know a particular
> reload is going to cause a spill.  While it'd be nice to catch the other
> cases, I don't think detecting all the cases where we can prove that a
> reload is going to cause a spill is necessary.  What you're proposing is
> a clear incremental improvement IMHO.

Not really, it suffers from the same conceptual problem as the previous
attempt I reverted.  If we can't get completely accurate answers (and at
this point, we can't), it can happen that we prefer an alternative for
the wrong reasons (e.g. both of them will force a spill, but we can
prove it only for one of them here).

In practice it seems to work reasonably well, at least on ARM.  More
testing would be good.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]