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

[Matthew Gretton-Dann]

On Wednesday 05 September 2012 17:40:23 Steven Bosscher wrote:
> On Wed, Sep 5, 2012 at 3:18 PM, Matthew Gretton-Dann
> 
> <matthew.gretton-d...@linaro.org> wrote:
> > On 5 September 2012 13:45, Richard Earnshaw <rearn...@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?
> No, I mean using the onlyjump_p predicate.

Again sorry for the delay.  Attached is an updated patch using the onlyjump_p 
predicate as suggested by Steven.

Tested cross arm-none-linux-gnueabi with QEmu.

OK for trunk?

Thanks,

Matt

gcc/ChangeLog:

2012-09-25  Matthew Gretton-Dann  <matthew.gretton-d...@arm.com>

        * postreload-gcse.c (bb_has_well_behaved_predecessors): Don't handle 
blocks
        that end in a non-simple jump.

-- 
Matthew Gretton-Dann
Linaro Toolchain Working Group
matthew.gretton-d...@linaro.org
diff --git a/gcc/postreload-gcse.c b/gcc/postreload-gcse.c
index b9e9f25..412c8fc 100644
--- a/gcc/postreload-gcse.c
+++ b/gcc/postreload-gcse.c
@@ -925,6 +925,9 @@ bb_has_well_behaved_predecessors (basic_block bb)
 
       if (JUMP_TABLE_DATA_P (BB_END (pred->src)))
        return false;
+
+      if (onlyjump_p (BB_END (pred->src)))
+       return false;
     }
   return true;
 }