mbox series

[00/13] Pass an argument descriptor to target hooks

Message ID mpt1rxhdwbz.fsf@arm.com
Headers show
Series Pass an argument descriptor to target hooks | expand

Message

Richard Sandiford Aug. 19, 2019, 3:11 p.m. UTC
For the SVE calling conventions, function_arg and function_arg_advance
need to know whether an argument is being passed by reference or not.
The simplest way of providing that information would have been to add a
new parameter, or convert the "named" parameter into a bitmask.  But it
seemed cleaner to wrap the argument information in a struct/class instead.

The bulk of this series therefore adds a function_arg_info that
initially records the type, mode and named flag.  The final patch then
adds the pass-by-reference information as well, which becomes a small
change on its own.

The new struct/class inherits a couple of non-obvious points of the
current interface:

(a) For some hooks the mode is the mode of the type.  For others it's
    the mode after promotion.  On targets that use argument promotion,
    this affects the choice between the mode and the type when both
    pieces of information are specified.

(a) Traditionally the type is null when calling libgcc support functions.
    But I think the argument conceptually still has a type (most of the
    functions are written in C after all).  It's just that the type is
    inferred from the unpromoted mode rather than being specified directly.
    So when we have access to the unpromoted mode, we can still query some
    properties of the type even if we don't have access to the type itself.

    (I remember it was said years ago that we should clean this up and
    call libgcc functions like any other function.  But TBH I can't see
    that ever happening.)

Of course, the ABI support is one of the most sensitive areas of the
compiler and it would be very easy to introduce a silent ABI break.
I've therefore tried to be conservative and stick to the following
changes:

(1) Replace uses of the old parameters with the corresponding fields
    of the function_arg_info (which hold the same values as before).

(2) In cases where the mode is the unpromoted mode, replace the calculation:

      type ? int_size_in_bytes (type) : GET_MODE_SIZE (mode)

    with a new type_size_in_bytes (), which computes the same thing
    in the same way.

    [The name is based on (b) above -- every argument has a type,
    even if we aren't given it explicitly.]

(3) In cases where the mode is the promoted mode, replace the calculation:

      mode == BLKmode ? int_size_in_bytes (type) : GET_MODE_SIZE (mode)

    with a new promoted_size_in_bytes (), which computes the same thing
    in the same way.

    Not all the affected targets use argument promotion, but that's
    what the calculation is logically providing in the affected contexts.

    [The only case I found in which the calculation was used for an
    unpromoted mode was aarch64_pass_by_reference.  Other targets use
    the expression in (2) here, and the later:

      /* Aggregates are passed by reference based on their size.  */
      if (type && AGGREGATE_TYPE_P (type))
	{
	  size = int_size_in_bytes (type);
	}

    suggests that that might have been the intention for aarch64 too.
    The series just leaves the aarch64 calculation as-is.]

(4) In cases where the mode is the promoted mode, replace the calculation:

      mode == BLKmode && type ? int_size_in_bytes (type) : GET_MODE_SIZE (mode)

    with promoted_size_in_bytes (), as for (3).  This means that the
    compiler will now ICE (via a segfault) on the invalid combination:

      mode == BLKmode && !type

    which I think is preferable to letting the error slip past.

(5) Replace the common test:

      type && AGGREGRATE_TYPE_P (type)

    with a new aggregate_type_p (), which computes the same thing
    in the same way.  This is again based on point (b) above.

(6) In function_arg only, replace:

      mode == VOIDmode

    and

      type == void_type_node

    (or both) with end_marker_p ().

Bootstrapped & regression-tested on aarch64-linux-gnu and x86_64-linux-gnu
(all languages for the latter).  I also tested each individual patch in
the series by compiling at least one target per CPU directory, checking
for no new warnings, and checking that there were no changes in assembly
output for gcc.c-torture, gcc.dg and g++.dg at -O0.

diffstat for series:

 66 files changed, 1676 insertions(+), 2246 deletions(-)

although admittedly a lot of that comes from culling out-of-date comments.

Richard