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.tkac...@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