jema...@gnu.org (Jose E. Marchesi) writes:
> +/* Return true if an argument at the position indicated by CUM should
> + be passed by reference. If the hook returns true, a copy of that
> + argument is made in memory and a pointer to the argument is passed
> + instead of the argument itself. */
> +
> +static bool
> +bpf_pass_by_reference (cumulative_args_t cum ATTRIBUTE_UNUSED,
> + const function_arg_info &arg)
> +{
> + poly_int64 mode_size = GET_MODE_SIZE (arg.mode);
> + unsigned num_bytes
> + = (arg.type ? arg.type_size_in_bytes() : mode_size);
Sorry, I meant replace the whole expression with arg.type_size_in_bytes ():
static bool
bpf_pass_by_reference (cumulative_args_t cum ATTRIBUTE_UNUSED,
const function_arg_info &arg)
{
unsigned num_bytes = arg.type_size_in_bytes ();
> +
> + /* Pass aggregates and values bigger than 5 words by reference.
> + Everything else is passed by copy. */
> + return ((arg.type && arg.aggregate_type_p ())
> + || (num_bytes > 8*5));
Here too:
return (arg.aggregate_type_p ()
|| (num_bytes > 8*5));
(although it now fits easily on one line).
> +/* Update the summarizer variable pointed by CA to advance past an
> + argument in the argument list. */
> +
> +static void
> +bpf_function_arg_advance (cumulative_args_t ca,
> + const function_arg_info &arg)
> +{
> + CUMULATIVE_ARGS *cum = get_cumulative_args (ca);
> + poly_int64 mode_size = GET_MODE_SIZE (arg.mode);
> + unsigned num_bytes = (arg.type
> + ? arg.type_size_in_bytes () : mode_size);
Same here.
> + unsigned num_words
> + = CEIL (num_bytes, UNITS_PER_WORD);
Nit: line break seems unnecessary.
> +
> + if (*cum <= 5 && *cum + num_words > 5)
> + error ("too many function arguments for eBPF");
> + else
> + *cum += num_words;
For this to work as intended (i.e. only report an error once), I think
you need to do "*cum += num_words;" even when reporting the error.
> +/* Return the appropriate instruction to CALL to a function. TARGET
> + is a RTX denoting the address of the called function.
an RTX
> + if (GET_CODE (op0) == REG
> + && (GET_CODE (op1) == LABEL_REF
> + || GET_CODE (op1) == SYMBOL_REF
> + || GET_CODE (op1) == CONST_INT
> + || GET_CODE (op1) == CONST))
> + {
> + fprintf (file, "[%s+", reg_names[REGNO (op0)]);
> + output_addr_const (file, op1);
> + fputs ("]", file);
> + }
> + else
> + fatal_insn ("invalid address in operand", addr);
Do you support all those codes on the rhs of a PLUS? I thought it was
just CONST_INT.
> +(define_predicate "reg_or_memory_operand"
> + (ior (match_operand 0 "register_operand")
> + (match_operand 0 "memory_operand")))
> +
> +(define_predicate "mov_dst_operand"
> + (ior (match_operand 0 "register_operand")
> + (match_operand 0 "memory_operand")))
These are both equivalent to the standard nonimmediate_operand.
It'd be better to use that instead if possible.
Looks good otherwise.
Thanks,
Richard
