reorg.c (fill_slots_from_thread): Improve for TARGET_FLAGS_REGNUM targets

[Richard Sandiford]

Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > @@ -2411,6 +2411,21 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
>> >    CLEAR_RESOURCE (&needed);
>> >    CLEAR_RESOURCE (&set);
>> >  
>> > +  /* Handle the flags register specially, to be able to accept a
>> > +     candidate that clobbers it.  See also fill_simple_delay_slots.  */
>> > +  bool filter_flags
>> > +    = (slots_to_fill == 1
>> > +       && targetm.flags_regnum != INVALID_REGNUM
>> > +       && find_regno_note (insn, REG_DEAD, targetm.flags_regnum));
>> > +  struct resources fset;
>> > +  struct resources flags_res;
>> > +  if (filter_flags)
>> > +    {
>> > +      CLEAR_RESOURCE (&fset);
>> > +      CLEAR_RESOURCE (&flags_res);
>> > +      SET_HARD_REG_BIT (flags_res.regs, targetm.flags_regnum);
>> > +    }
>> > +
>> >    /* If we do not own this thread, we must stop as soon as we find
>> >       something that we can't put in a delay slot, since all we can do
>> >       is branch into THREAD at a later point.  Therefore, labels stop
>> > @@ -2439,8 +2454,18 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
>> >        /* If TRIAL conflicts with the insns ahead of it, we lose.  Also,
>> >  	 don't separate or copy insns that set and use CC0.  */
>> >        if (! insn_references_resource_p (trial, &set, true)
>> > -	  && ! insn_sets_resource_p (trial, &set, true)
>> > +	  && ! insn_sets_resource_p (trial, filter_flags ? &fset : &set, true)
>> >  	  && ! insn_sets_resource_p (trial, &needed, true)
>> > +	  /* If we're handling sets to the flags register specially, we
>> > +	     only allow an insn into a delay-slot, if it either:
>> > +	     - doesn't set the flags register,
>> > +	     - the "set" of the flags register isn't used (clobbered),
>> > +	     - insns between the delay-slot insn and the trial-insn
>> > +	     as accounted in "set", have not affected the flags register.  */
>> > +	  && (! filter_flags
>> > +	      || ! insn_sets_resource_p (trial, &flags_res, true)
>> > +	      || find_regno_note (trial, REG_UNUSED, targetm.flags_regnum)
>> > +	      || ! TEST_HARD_REG_BIT (set.regs, targetm.flags_regnum))
>> >  	  && (!HAVE_cc0 || (! (reg_mentioned_p (cc0_rtx, pat)
>> >  			      && (! own_thread || ! sets_cc0_p (pat)))))
>> >  	  && ! can_throw_internal (trial))
>> > @@ -2618,6 +2643,16 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
>> >        lose = 1;
>> >        mark_set_resources (trial, &set, 0, MARK_SRC_DEST_CALL);
>> >        mark_referenced_resources (trial, &needed, true);
>> > +      if (filter_flags)
>> > +	{
>> > +	  mark_set_resources (trial, &fset, 0, MARK_SRC_DEST_CALL);
>> > +
>> > +	  /* Groups of flags-register setters with users should not
>> > +	     affect opportunities to move flags-register-setting insns
>> > +	     (clobbers) into the delay-slot.  */
>> > +	  CLEAR_HARD_REG_BIT (needed.regs, targetm.flags_regnum);
>> 
>> If we do this, what stops us from trying to move a flags-register user
>> ahead of the associated setter, when the user doesn't itself set the
>> flags register?
>
> First, all sets are still in set (set.regs) including any that
> set the flags register between the insn with the delay-slot and
> the trial.
>
> (That's why it's separate from fset (fset.regs).  I used the
> same naming scheme as in the named commit, but perhaps a better
> name would be set_with_flags_filtered or something.)
>
>>  Feels like there should be some test involving
>> insn_references_resource_p (trial, &flags_res, true) in the
>> (! filter_flags || ...) condition above.
>
> There is: the "! insn_references_resource_p (trial, &set, true)"
> (pre-existing in the context above the patch), so we don't move
> anything that references anything set (only additional flags
> register setters where the result is unused).

Ah, right, I think I was getting them the wrong way around.

Looks OK to me, but give Eric 24 hrs to object/comment.
Like you say, he's better qualified to review this, I was just
stepping in because it looked like the patch had fallen through
the cracks.

Thanks,
Richard