Re: [PATCH V2 2/8] bpf: new GCC port

[Richard Sandiford <richard dot sandiford at arm dot com>]

jose.marchesi@oracle.com (Jose E. Marchesi) writes:
> Hi Richard!
>
> Many thanks for the deep review.  I'm addressing some of your questions
> below.
>
>     > [...]
>     > +/* Override options and do some other initialization.  */
>     > +
>     > +static void
>     > +bpf_option_override (void)
>     > +{
>     > +  /* Set the default target kernel if no -mkernel was specified.  */
>     > +  if (!global_options_set.x_bpf_kernel)
>     > +    bpf_kernel = LINUX_LATEST;
>     
>     LINUX_LATEST is the default in the .opt file, so when is this needed?
>
> It is an idiom I got from sparc.c:
>
>   /* Set the default CPU if no -mcpu option was specified.  */
>   if (!global_options_set.x_sparc_cpu_and_features)
>     {
>       ...
>     }
>
> Maybe the code in sparc.c predates the Init() directive in sparc.opt
> file?

Might be wrong, but it looks like the SPARC code is overriding the .opt
default with the configure-time one.  I don't think it's needed when
the default is fixed.

>     > [...]
>     > +/* Return a RTX indicating whether a function argument is passed in a
>     > +   register and if so, which register.  */
>     > +
>     > +static rtx
>     > +bpf_function_arg (cumulative_args_t ca, enum machine_mode mode ATTRIBUTE_UNUSED,
>     > +                  const_tree type ATTRIBUTE_UNUSED, bool named ATTRIBUTE_UNUSED)
>     > +{
>     > +  CUMULATIVE_ARGS *cum = get_cumulative_args (ca);
>     > +
>     > +  if (*cum < 5)
>     > +    return gen_rtx_REG (mode, *cum + 1);
>     > +  else
>     > +    /* An error have been emitted for this in
>     > +       bpf_function_arg_advance.  */
>     > +    return NULL_RTX;
>    
>     This hook is called first, so "will be" rather than "has been".
>
>     (BTW, I just submitted a series of patches to change this interface,
>     but it should be a trivial change for whichever of us gets to make it.)
>
> I see your interface change was approved yesterday, so I will just adapt
> in my next rebase :)

Thanks!

Richard