Add a compatible_vector_types_p target hook

[Richard Biener]

On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Richard Biener <richard.guenther@gmail.com> writes:
>> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
><richard.sandiford@arm.com> wrote:
>>>Richard Biener <richard.guenther@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?

How does this get around the LTO canonical type merging machinery? That is, how are those types streamed and how are they identified by the backend? Just by means of being pointer equal to some statically built type in the backend? 
Or does the type have some attribute on it or on the component? How does the middle end build a related type with the same ABI, like a vector with the half number of elements? 

Richard. 

>Thanks,
>Richard