Re: [RFA 2/n] Don't lift loads above register using jumps in postreload-gcse.c

[Matthew Gretton-Dann <matthew dot gretton-dann at linaro dot org>]

On 5 September 2012 13:45, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 05/09/12 13:02, Steven Bosscher wrote:
>> On Wed, Sep 5, 2012 at 1:42 PM, Matthew Gretton-Dann wrote:
>>> Whilst this fix works for this particular case I am not sure it is the
>>> best fix for the general issue, and so if others have a better idea how
>>> to fix this I would be very happy.
>>
>> postreload-gcse.c is broken in "interesting" ways. Look at this gem for example:
>>
>> static bool
>> reg_changed_after_insn_p (rtx x, int cuid)
>> {
>>   unsigned int regno, end_regno;
>>
>>   regno = REGNO (x);
>>   end_regno = END_HARD_REGNO (x);
>>   do
>>     if (reg_avail_info[regno] > cuid)
>>       return true;
>>   while (++regno < end_regno);
>>   return false;
>> }
>>
>> So the more conservative the fix, the better :-)

I suppose removing the pass is too conservative :-)

>> The patch looks correct to me. But perhaps the pass should just punt
>> on blocks not ending in a simple jump in
>> bb_has_well_behaved_predecessors?

By 'simple jump' you mean any block with at most only EDGE_FALLTHRU on the edge?

> That sort of makes sense.  Why would we ever want to hoist an insn out
> of a cold block into a hot one?  I could see it making sense to do the
> reverse on occasion, but clearly care is needed here.

So whilst testing -freorder-blocks-and-partition has caused this
behaviour to be exhibited, I believe there is nothing stopping this
happening with any indirect jump - not just crossing jumps.

Thanks,

Matt

-- 
Matthew Gretton-Dann
Linaro Toolchain Working Group
matthew.gretton-dann@linaro.org