Steve Ellcey <sell...@cavium.com> writes:
> This is the third of three patches for Aarch64 SIMD ABI support.  This
> patch is not fully tested yet but I want to post it to get comments.
>
> This is the only patch of the three that touches non-aarch64 specific
> code.  The changes here are made to allow GCC to have better information
> about what registers are clobbered by functions.  With the new SIMD
> ABI on Aarch64 the registers clobbered by a SIMD function is a subset
> of the registers clobbered by a normal (non-SIMD) function.  This can
> result in the caller saving and restoring more registers than is necessary.
>
> This patch addresses that by passing information about the call insn to 
> various routines so that they can check on what type of function is being
> called and modify the clobbered register set based on that information.
>
> As an example, this code:
>
>   __attribute__ ((__simd__ ("notinbranch"))) extern double sin (double __x);
>   __attribute__ ((__simd__ ("notinbranch"))) extern double log (double __x);
>   __attribute__ ((__simd__ ("notinbranch"))) extern double exp (double __x);
>
>   double foo(double * __restrict__ x, double * __restrict__ y,
>              double * __restrict__ z, int n)
>   {
>       int i;
>       double a = 0.0;
>       for (i = 0; i < n; i++)
>               a = a + sin(x[i]) + log(y[i]) + exp (z[i]);
>       return a;
>   }
>
> Will generate stores inside the main vectorized loop to preserve registers
> without this patch, but after the patch, will not do any stores and will
> use registers it knows the vector sin/log/exp functions do not clobber.
>
> Comments?

I think it'd be better to keep the get_call_reg_set_usage interface
the same, since:

  get_call_reg_set_usage (insn, &used_regs, call_used_reg_set);

is easier to read than:

  get_call_reg_set_usage (insn, &used_regs, true);

I don't think it would be possible as things stand for an ABI variant
to *add* call-clobbered registers here.  Instead it would need to
clobber the registers explicitly in CALL_INSN_FUNCTION_USAGE.
So I think in practice the new hook can only remove call-clobbered
registers.  It might then be better to have it filter out registers
from the set that it's given.  I.e. something like:

  targetm.remove_extra_call_preserved_regs (insn, &used_regs);

which get_call_reg_set_usage could call immediately before
returning.

The change to targetm.hard_regno_call_part_clobbered looks good.

I guess targetm.check_part_clobbered is probably an expedient
fix for now.  It might be better to call it something like
honor_part_clobbered_p, to make it clearer that the hook doesn't
actually do the checking itself.

When you submit the final patch, could you split off each hook and
interface change?  That would make things easier to review.

Thanks,
Richard

Reply via email to