Bernd Schmidt <ber...@codesourcery.com> writes:
> +               CLEAR_HARD_REG_SET (set_regs);
> +               note_stores (PATTERN (scan), record_hard_reg_sets,
> +                            &set_regs);
> +               if (CALL_P (scan))
> +                 IOR_HARD_REG_SET (set_regs, call_used_reg_set);
> +               for (link = REG_NOTES (scan); link; link = XEXP (link, 1))
> +                 if (REG_NOTE_KIND (link) == REG_INC)
> +                   record_hard_reg_sets (XEXP (link, 0), NULL, &set_regs);
> +
> +               if (TEST_HARD_REG_BIT (set_regs, srcreg)
> +                   || reg_referenced_p (SET_DEST (set),
> +                                        PATTERN (scan)))
> +                 {
> +                   scan = NULL_RTX;
> +                   break;
> +                 }
> +               if (CALL_P (scan))
> +                 {
> +                   rtx link = CALL_INSN_FUNCTION_USAGE (scan);
> +                   while (link)
> +                     {
> +                       rtx tmp = XEXP (link, 0);
> +                       if (GET_CODE (tmp) == USE
> +                           && reg_referenced_p (SET_DEST (set), tmp))
> +                         break;
> +                       link = XEXP (link, 1);
> +                     }
> +                   if (link)
> +                     {
> +                       scan = NULL_RTX;
> +                       break;
> +                     }
> +                 }

Could we use DF_REF_USES/DEFS here instead?

I'd like to get a stage where new code should treat DF_REF_USES/DEFS
as the default way of testing for register usage.  I think this sort
of ad-hoc liveness stuff should be a last resort, and should have a
comment saying why DF_REF_USES/DEFS isn't suitable.

Rather than walk all the instructions of intermediate blocks,
you could just test DF_LR_BB_INFO (bb)->use to see whether the
block uses the register at all.

> +  FOR_BB_INSNS_SAFE (entry_block, insn, curr)

Is there any particular reason for doing a forward walk rather than
a backward walk?  A backward walk would allow chains to be moved,
and would avoid the need for the quadraticness in the current:

  for each insn I in bb
    for each insn J after I in bb

since you could then keep an up-to-date record of what registers
are used or set by the instructions that you aren't moving.

Also:

+/* Look for sets of call-saved registers in the first block of the
+   function, and move them down into successor blocks if the register
+   is used only on one path.  This exposes more opportunities for
+   shrink-wrapping.
+   These kinds of sets often occur when incoming argument registers are
+   moved to call-saved registers because their values are live across
+   one or more calls during the function.  */
+
+static void
+prepare_shrink_wrap (basic_block entry_block)

There's no check for call-savedness.  Is that deliberate?  I'm seeing
a case where we have:

   (set (reg 2) (reg 15))
   (set (reg 15) (reg 2))

(yes, it's silly, but bear with me) for call-clobbered registers 2 and 15.
We move the second instruction as far as possible, making 2 live for
much longer.  So if the prologue uses 2 as a temporary register, this
would actually prevent shrink-wrapping.

The reason I'm suddenly "reviewing" the code now is that it
doesn't prevent shrink-wrapping, because nothing adds register 2
to the liveness info of the affected blocks.  The temporary prologue
value of register 2 is then moved into register 15.

Testcase is gcc.c-torture/execute/920428-2.c on mips64-linux-gnu cc1
(although any MIPS should do) with:

    -O2 -mabi=32 -mips16 -mno-shared -mabicalls -march=mips32

Richard

Reply via email to