[PATCH][AArch64] Separate shrink wrapping hooks implementation

[James Greenhalgh]

On Wed, Nov 30, 2016 at 02:07:58PM +0000, Kyrill Tkachov wrote:
> 
> On 29/11/16 20:29, Segher Boessenkool wrote:
> >Hi James, Kyrill,
> >
> >On Tue, Nov 29, 2016 at 10:57:33AM +0000, James Greenhalgh wrote:
> >>>+static sbitmap
> >>>+aarch64_components_for_bb (basic_block bb)
> >>>+{
> >>>+  bitmap in = DF_LIVE_IN (bb);
> >>>+  bitmap gen = &DF_LIVE_BB_INFO (bb)->gen;
> >>>+  bitmap kill = &DF_LIVE_BB_INFO (bb)->kill;
> >>>+
> >>>+  sbitmap components = sbitmap_alloc (V31_REGNUM + 1);
> >>>+  bitmap_clear (components);
> >>>+
> >>>+  /* GPRs are used in a bb if they are in the IN, GEN, or KILL sets.  */
> >>>+  for (unsigned regno = R0_REGNUM; regno <= V31_REGNUM; regno++)
> >>The use of R0_REGNUM and V31_REGNUM scare me a little bit, as we're hardcoding
> >>where the end of the register file is (does this, for example, fall apart
> >>with the SVE work that was recently posted). Something like a
> >>LAST_HARDREG_NUM might work?
> >Components and registers aren't the same thing (you can have components
> >for things that aren't just a register save, e.g. the frame setup, stack
> >alignment, save of some non-GPR via a GPR, PIC register setup, etc.)
> >The loop here should really only cover the non-volatile registers, and
> >there should be some translation from register number to component number
> >(it of course is convenient to have a 1-1 translation for the GPRs and
> >floating point registers).  For rs6000 many things in the backend already
> >use non-symbolic numbers for the FPRs and GPRs, so that is easier there.
> 
> Anyway, here's the patch with James's comments implemented.
> I've introduced LAST_SAVED_REGNUM which is used to delimit the registers
> considered for shrink-wrapping.
> 
> aarch64_process_components is introduced and used to implement the
> emit_prologue_components and emit_epilogue_components functions in a single
> place.

OK.

> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> 
> Thanks,
> Kyrill
> 
> 2016-11-30  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/aarch64/aarch64.h (machine_function): Add
>     reg_is_wrapped_separately field.
>     * config/aarch64/aarch64.md (LAST_SAVED_REGNUM): Define new constant.
>     * config/aarch64/aarch64.c (emit_set_insn): Change return type to
>     rtx_insn *.
>     (aarch64_save_callee_saves): Don't save registers that are wrapped
>     separately.
>     (aarch64_restore_callee_saves): Don't restore registers that are
>     wrapped separately.
>     (offset_9bit_signed_unscaled_p, offset_12bit_unsigned_scaled_p,
>     aarch64_offset_7bit_signed_scaled_p): Move earlier in the file.
>     (aarch64_get_separate_components): New function.
>     (aarch64_get_next_set_bit): Likewise.
>     (aarch64_components_for_bb): Likewise.
>     (aarch64_disqualify_components): Likewise.
>     (aarch64_emit_prologue_components): Likewise.
>     (aarch64_emit_epilogue_components): Likewise.
>     (aarch64_set_handled_components): Likewise.
>     (aarch64_process_components): Likewise.
>     (TARGET_SHRINK_WRAP_GET_SEPARATE_COMPONENTS,
>     TARGET_SHRINK_WRAP_COMPONENTS_FOR_BB,
>     TARGET_SHRINK_WRAP_DISQUALIFY_COMPONENTS,
>     TARGET_SHRINK_WRAP_EMIT_PROLOGUE_COMPONENTS,
>     TARGET_SHRINK_WRAP_EMIT_EPILOGUE_COMPONENTS,
>     TARGET_SHRINK_WRAP_SET_HANDLED_COMPONENTS): Define.
> >>>+static void
> >>>+aarch64_disqualify_components (sbitmap, edge, sbitmap, bool)
> >>>+{
> >>>+}
> >>Is there no default "do nothing" hook for this?
> >I can make the shrink-wrap code do nothing here if this hook isn't
> >defined, if you want?
> 
> I don't mind either way.
> If you do it I'll then remove the empty implementation in aarch64.

If there is no empty default, this is fine.

Thanks,
James