Re: [PATCH][AArch64] Separate shrink wrapping hooks implementation

[Segher Boessenkool <segher@kernel.crashing.org>]

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.

> > +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?


Segher