Richard Biener <richard.guent...@gmail.com> writes:
> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>Richard Biener <richard.guent...@gmail.com> writes:
>>>>>>The AArch64 port emits an error if calls pass values of SVE type to
>>>>an
>>>>>>unprototyped function. To do that we need to know whether the
>>value
>>>>>>really is an SVE type rathr than a plain vector.
>>>>>>
>>>>>>For varags the ABI is the same for 256 bits+. But we'll have the
>>>>>>same problem there once we support -msve-vector-bits=128, since the
>>>>>>layout of SVE and Advanced SIMD vectors differ for big-endian.
>>>>>
>>>>> But then why don't you have different modes?
>>>>
>>>>Yeah, true, modes will probably help for the Advanced SIMD/SVE
>>>>difference. But from a vector value POV, a vector of 4 ints is a
>>>>vector
>>>>of 4 ints, so even distinguishing based on the mode is artificial.
>>>
>>> True.
>>>
>>>>SVE is AFAIK the first target to have different modes for potentially
>>>>the "same" vector type, and I had to add new infrastructure to allow
>>>>targets to define multiple modes of the same size. So the fact that
>>>>gimple distinguishes otherwise identical vectors based on mode is a
>>>>relatively recent thing. AFAIK it just fell out in the wash rather
>>>>than being deliberately planned. It happens to be convenient in this
>>>>context, but it hasn't been important until now.
>>>>
>>>>The hook doesn't seem any worse than distinguishing based on the
>>mode.
>>>>Another way to avoid this would have been to define separate SVE
>>modes
>>>>for the predefined vectors. The big downside of that is that we'd
>>end
>>>>up doubling the number of SVE patterns.
>>>>
>>>>Extra on-the-side metadata is going to be easy to drop accidentally,
>>>>and this is something we need for correctness rather than
>>optimisation.
>>>
>>> Still selecting the ABI during call expansion only and based on
>>values types at that point is fragile.
>>
>>Agreed. But it's fragile in general, not just for this case. Changing
>>something as fundamental as that would be a lot of work and seems
>>likely
>>to introduce accidental ABI breakage.
>>
>>> The frontend are in charge of specifying the actual argument type and
>>> at that point the target may fix the ABI. The ABI can be recorded in
>>> the calls fntype, either via its TYPE_ARG_TYPES or in more awkward
>>> ways for varargs functions (in full generality that would mean
>>> attaching varargs ABI meta to each call).
>>>
>>> The alternative is to have an actual argument type vector associated
>>> with each call.
>>
>>I think multiple pieces of gimple code would then have to cope with
>>that
>>as a special case. E.g. if:
>>
>> void foo (int, ...);
>>
>> type1 a;
>> b = VIEW_CONVERT_EXPR<type2> (a);
>> if (a)
>> foo (1, a);
>> else
>> foo (1, b);
>>
>>gets converted to:
>>
>> if (a)
>> foo (1, a);
>> else
>> foo (1, a);
>>
>>on the basis that type1 and type2 are "the same" despite having
>>different calling conventions, we have to be sure that the calls
>>are not treated as equivalent:
>>
>> foo (1, a);
>>
>>Things like IPA clones would also need to handle this specially.
>>Anything that generates new calls based on old ones will need
>>to copy this information too.
>>
>>This also sounds like it would be fragile and seems a bit too
>>invasive for stage 3.
>
> But we are already relying on this to work (fntype non-propagation) because
> function pointer conversions are dropped on the floor.
>
> The real change would be introducing (per call) fntype for calls to
> unprototyped functions and somehow dealing with varargs.
It looks like this itself relies on useless_type_conversion_p,
is that right? E.g. we have things like:
bool
func_checker::compare_gimple_call (gcall *s1, gcall *s2)
{
...
tree fntype1 = gimple_call_fntype (s1);
tree fntype2 = gimple_call_fntype (s2);
if ((fntype1 && !fntype2)
|| (!fntype1 && fntype2)
|| (fntype1 && !types_compatible_p (fntype1, fntype2)))
return return_false_with_msg ("call function types are not compatible");
and useless_type_conversion_p has:
else if ((TREE_CODE (inner_type) == FUNCTION_TYPE
|| TREE_CODE (inner_type) == METHOD_TYPE)
&& TREE_CODE (inner_type) == TREE_CODE (outer_type))
{
tree outer_parm, inner_parm;
/* If the return types are not compatible bail out. */
if (!useless_type_conversion_p (TREE_TYPE (outer_type),
TREE_TYPE (inner_type)))
return false;
/* Method types should belong to a compatible base class. */
if (TREE_CODE (inner_type) == METHOD_TYPE
&& !useless_type_conversion_p (TYPE_METHOD_BASETYPE (outer_type),
TYPE_METHOD_BASETYPE (inner_type)))
return false;
/* A conversion to an unprototyped argument list is ok. */
if (!prototype_p (outer_type))
return true;
/* If the unqualified argument types are compatible the conversion
is useless. */
if (TYPE_ARG_TYPES (outer_type) == TYPE_ARG_TYPES (inner_type))
return true;
for (outer_parm = TYPE_ARG_TYPES (outer_type),
inner_parm = TYPE_ARG_TYPES (inner_type);
outer_parm && inner_parm;
outer_parm = TREE_CHAIN (outer_parm),
inner_parm = TREE_CHAIN (inner_parm))
if (!useless_type_conversion_p
(TYPE_MAIN_VARIANT (TREE_VALUE (outer_parm)),
TYPE_MAIN_VARIANT (TREE_VALUE (inner_parm))))
return false;
So it looks like we'd still need to distinguish the vector types in
useless_type_conversion_p even if we went the fntype route. The difference
is that the fntype route would give us the option of only distinguishing
the vectors for return and argument types and not in general.
But if we are going to have to distinguish the vectors here anyway
in some form, could we go with the patch as-is for stage 3 and leave
restricting this to just return and argument types as a follow-on
optimisation?
Thanks,
Richard
