Re: Add a compatible_vector_types_p target hook

2020-01-09 Thread Richard Biener
On Tue, Jan 7, 2020 at 11:33 AM Richard Sandiford
 wrote:
>
> Richard Sandiford  writes:
> > Richard Biener  writes:
> >> On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford 
> >>  wrote:
> >>>Richard Biener  writes:
>  On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
> >>> wrote:
> >Richard Biener  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 (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) == 

Re: Add a compatible_vector_types_p target hook

2020-01-07 Thread Richard Sandiford
Richard Sandiford  writes:
> Richard Biener  writes:
>> On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford 
>>  wrote:
>>>Richard Biener  writes:
 On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
>>> wrote:
>Richard Biener  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 (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)))
>>> 

Re: Add a compatible_vector_types_p target hook

2019-12-16 Thread Richard Sandiford
Richard Biener  writes:
> On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford 
>  wrote:
>>Richard Biener  writes:
>>> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
>> wrote:
Richard Biener  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 (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
>>&& 

Re: Add a compatible_vector_types_p target hook

2019-12-14 Thread Richard Biener
On December 14, 2019 11:43:48 AM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford
> wrote:
>>>Richard Biener  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 (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 

Re: Add a compatible_vector_types_p target hook

2019-12-14 Thread Richard Sandiford
Richard Biener  writes:
> On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford 
>  wrote:
>>Richard Biener  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 (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 

Re: Add a compatible_vector_types_p target hook

2019-12-13 Thread Richard Sandiford
Richard Biener  writes:
>>> 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 (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. 

Hmm, OK.  Any suggestions for how the varargs type should be
represented?  We can't just change (int, ...) to (int, foo, bar),
since varargs can be passed differently from non-varargs.  We'd need
something like "(int, ...) used as (int, foo, bar)" instead.

Currently TYPE_ARG_TYPES ends with void_list_node for non-varargs
and null for varargs.  Perhaps we could instead add a "..." marker, so:

:
  unprototyped function

:
  (type1, ...) prototype

:
  (type1, type2) prototype

:
  unprototyped function called with type1

:
  (type1, ...) prototype called with (type1, type2)

If so, would something like that be OK during stage3?

Richard


Re: Add a compatible_vector_types_p target hook

2019-12-13 Thread Richard Biener
On December 13, 2019 10:12:40 AM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  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 (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. 

>> Btw, how does STRIP_NOPS preserve the argument type if a conversion
>> happens in the call and generic folding applies? IIRC that puns down
>> to modes as well (which was the reason to make
>> useless_type_conversion_p do the same).
>
>These are VIEW_CONVERT_EXPRs, which don't get stripped.

Ah, OK. Guess you're lucky there then. 

>> Sorry for the vague and late answers, I'm already in christmas mode
>;) 
>
>Wish I was too :-)

Only a few days left ;) 

Richard. 

>Thanks,
>Richard



Re: Add a compatible_vector_types_p target hook

2019-12-13 Thread Richard Sandiford
Richard Biener  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 (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.

> Btw, how does STRIP_NOPS preserve the argument type if a conversion
> happens in the call and generic folding applies? IIRC that puns down
> to modes as well (which was the reason to make
> useless_type_conversion_p do the same).

These are VIEW_CONVERT_EXPRs, which don't get stripped.

> Sorry for the vague and late answers, I'm already in christmas mode ;) 

Wish I was too :-)

Thanks,
Richard


Re: Add a compatible_vector_types_p target hook

2019-12-13 Thread Richard Biener
On December 12, 2019 7:15:36 PM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On December 12, 2019 5:44:25 PM GMT+01:00, Richard Sandiford
> wrote:
>>>Richard Biener  writes:
 On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford
>>> wrote:
>One problem with adding an N-bit vector extension to an existing
>architecture is to decide how N-bit vectors should be passed to
>functions and returned from functions.  Allowing all N-bit vector
>types to be passed in registers breaks backwards compatibility,
>since N-bit vectors could be used (and emulated) before the vector
>extension was added.  But always passing N-bit vectors on the
>stack would be inefficient for things like vector libm functions.
>
>For SVE we took the compromise position of predefining new SVE
>vector
>types that are distinct from all existing vector types, including
>GNU-style vectors.  The new types are passed and returned in an
>efficient way while existing vector types are passed and returned
>in the traditional way.  In the right circumstances, the two types
>are inter-convertible.
>
>The SVE types are created using:
>
>  vectype = build_distinct_type_copy (vectype);
>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>  TYPE_ARTIFICIAL (vectype) = 1;
>
>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>to convert from one type to the other.  However, the distinction
>can
>be lost during gimple, which treats two vector types with the same
>mode, number of elements, and element type as equivalent.  And for
>most targets that's the right thing to do.

 And why's that a problem? The difference appears only in the
>function
>>>call ABI which is determined by the function signature rather than
>>>types or modes of the actual arguments? 
>>>
>>>We use the type of the actual arguments when deciding how arguments
>>>should be passed to functions:
>>>
>>>/* I counts args in order (to be) pushed; ARGPOS counts in order
>>>written.  */
>>>  for (argpos = 0; argpos < num_actuals; i--, argpos++)
>>>{
>>>  tree type = TREE_TYPE (args[i].tree_value);
>>>  [...]
>>>   /* See if this argument should be passed by invisible reference. 
>*/
>>>  function_arg_info arg (type, argpos < n_named_args);
>>>
>>>And it has to be that way for calls to unprototyped functions,
>>>or for varargs.
>>
>> So even for varargs the passing is different? Also we have
>CALL_EXPR_FNTYPE which you could populate specially even for
>unprototyped or varargs functions.
>>
>> I realize we now look at the type of values but you have to realize
>that differences that are not relevant for values are discarded. 
>Artificially preserving such non-real differences everywhere(!) while
>it only matters at call boundaries doesn't look correct. 
>
>But isn't this similar to the way that we preserve the difference
>between:
>
>  struct s1 { int i; };
>  struct s2 { int i; };
>
>?  They're the same value as far as the target machine is concerned,
>but we preserve the difference for other reasons.

With LTO we don't. Both get the same TYPE_CANONICAL. 

>>>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. 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 

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Sandiford
Richard Biener  writes:
> On December 12, 2019 5:44:25 PM GMT+01:00, Richard Sandiford 
>  wrote:
>>Richard Biener  writes:
>>> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford
>> wrote:
One problem with adding an N-bit vector extension to an existing
architecture is to decide how N-bit vectors should be passed to
functions and returned from functions.  Allowing all N-bit vector
types to be passed in registers breaks backwards compatibility,
since N-bit vectors could be used (and emulated) before the vector
extension was added.  But always passing N-bit vectors on the
stack would be inefficient for things like vector libm functions.

For SVE we took the compromise position of predefining new SVE vector
types that are distinct from all existing vector types, including
GNU-style vectors.  The new types are passed and returned in an
efficient way while existing vector types are passed and returned
in the traditional way.  In the right circumstances, the two types
are inter-convertible.

The SVE types are created using:

  vectype = build_distinct_type_copy (vectype);
  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
  TYPE_ARTIFICIAL (vectype) = 1;

The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
to convert from one type to the other.  However, the distinction can
be lost during gimple, which treats two vector types with the same
mode, number of elements, and element type as equivalent.  And for
most targets that's the right thing to do.
>>>
>>> And why's that a problem? The difference appears only in the function
>>call ABI which is determined by the function signature rather than
>>types or modes of the actual arguments? 
>>
>>We use the type of the actual arguments when deciding how arguments
>>should be passed to functions:
>>
>>/* I counts args in order (to be) pushed; ARGPOS counts in order
>>written.  */
>>  for (argpos = 0; argpos < num_actuals; i--, argpos++)
>>{
>>  tree type = TREE_TYPE (args[i].tree_value);
>>  [...]
>>   /* See if this argument should be passed by invisible reference.  */
>>  function_arg_info arg (type, argpos < n_named_args);
>>
>>And it has to be that way for calls to unprototyped functions,
>>or for varargs.
>
> So even for varargs the passing is different? Also we have CALL_EXPR_FNTYPE 
> which you could populate specially even for unprototyped or varargs functions.
>
> I realize we now look at the type of values but you have to realize that 
> differences that are not relevant for values are discarded.  Artificially 
> preserving such non-real differences everywhere(!) while it only matters at 
> call boundaries doesn't look correct. 

But isn't this similar to the way that we preserve the difference
between:

  struct s1 { int i; };
  struct s2 { int i; };

?  They're the same value as far as the target machine is concerned,
but we preserve the difference for other reasons.

>>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.

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.

Thanks,
Richard


Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Biener
On December 12, 2019 5:44:25 PM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford
> wrote:
>>>One problem with adding an N-bit vector extension to an existing
>>>architecture is to decide how N-bit vectors should be passed to
>>>functions and returned from functions.  Allowing all N-bit vector
>>>types to be passed in registers breaks backwards compatibility,
>>>since N-bit vectors could be used (and emulated) before the vector
>>>extension was added.  But always passing N-bit vectors on the
>>>stack would be inefficient for things like vector libm functions.
>>>
>>>For SVE we took the compromise position of predefining new SVE vector
>>>types that are distinct from all existing vector types, including
>>>GNU-style vectors.  The new types are passed and returned in an
>>>efficient way while existing vector types are passed and returned
>>>in the traditional way.  In the right circumstances, the two types
>>>are inter-convertible.
>>>
>>>The SVE types are created using:
>>>
>>>  vectype = build_distinct_type_copy (vectype);
>>>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>>  TYPE_ARTIFICIAL (vectype) = 1;
>>>
>>>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>>>to convert from one type to the other.  However, the distinction can
>>>be lost during gimple, which treats two vector types with the same
>>>mode, number of elements, and element type as equivalent.  And for
>>>most targets that's the right thing to do.
>>
>> And why's that a problem? The difference appears only in the function
>call ABI which is determined by the function signature rather than
>types or modes of the actual arguments? 
>
>We use the type of the actual arguments when deciding how arguments
>should be passed to functions:
>
>/* I counts args in order (to be) pushed; ARGPOS counts in order
>written.  */
>  for (argpos = 0; argpos < num_actuals; i--, argpos++)
>{
>  tree type = TREE_TYPE (args[i].tree_value);
>  [...]
>   /* See if this argument should be passed by invisible reference.  */
>  function_arg_info arg (type, argpos < n_named_args);
>
>And it has to be that way for calls to unprototyped functions,
>or for varargs.

So even for varargs the passing is different? Also we have CALL_EXPR_FNTYPE 
which you could populate specially even for unprototyped or varargs functions.

I realize we now look at the type of values but you have to realize that 
differences that are not relevant for values are discarded.  Artificially 
preserving such non-real differences everywhere(!) while it only matters at 
call boundaries doesn't look correct. 

>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?

Richard. 

>Thanks,
>Richard
>
>>
>> Richard. 
>>
>>>This patch therefore adds a hook that lets the target choose
>>>whether such vector types are indeed equivalent.
>>>
>>>Note that the new tests fail for -mabi=ilp32 in the same way as other
>>>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>>>
>>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>>
>>>Richard
>>>
>>>
>>>2019-12-12  Richard Sandiford  
>>>
>>>gcc/
>>> * target.def (compatible_vector_types_p): New target hook.
>>> * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>>> * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>>> * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>>> * doc/tm.texi: Regenerate.
>>> * gimple-expr.c: Include target.h.
>>> (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>>> * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>>> function.
>>> (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>>> * config/aarch64/aarch64-sve-builtins.cc
>>>(gimple_folder::convert_pred):
>>> Use the original predicate if it already has a suitable type.
>>>
>>>gcc/testsuite/
>>> * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>>> * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>>>
>>>Index: gcc/target.def
>>>===
>>>--- gcc/target.def   2019-11-30 18:48:18.531984101 +
>>>+++ gcc/target.def   2019-12-12 15:07:43.960415368 +
>>>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>>>  hook_bool_mode_false)
>>> 
>>> DEFHOOK
>>>+(compatible_vector_types_p,
>>>+ "Return true if there is no target-specific reason for treating\n\
>>>+vector types @var{type1} and @var{type2} as distinct types.  The
>>>caller\n\
>>>+has already checked 

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Sandiford
Richard Biener  writes:
> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford 
>  wrote:
>>One problem with adding an N-bit vector extension to an existing
>>architecture is to decide how N-bit vectors should be passed to
>>functions and returned from functions.  Allowing all N-bit vector
>>types to be passed in registers breaks backwards compatibility,
>>since N-bit vectors could be used (and emulated) before the vector
>>extension was added.  But always passing N-bit vectors on the
>>stack would be inefficient for things like vector libm functions.
>>
>>For SVE we took the compromise position of predefining new SVE vector
>>types that are distinct from all existing vector types, including
>>GNU-style vectors.  The new types are passed and returned in an
>>efficient way while existing vector types are passed and returned
>>in the traditional way.  In the right circumstances, the two types
>>are inter-convertible.
>>
>>The SVE types are created using:
>>
>>  vectype = build_distinct_type_copy (vectype);
>>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>  TYPE_ARTIFICIAL (vectype) = 1;
>>
>>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>>to convert from one type to the other.  However, the distinction can
>>be lost during gimple, which treats two vector types with the same
>>mode, number of elements, and element type as equivalent.  And for
>>most targets that's the right thing to do.
>
> And why's that a problem? The difference appears only in the function call 
> ABI which is determined by the function signature rather than types or modes 
> of the actual arguments? 

We use the type of the actual arguments when deciding how arguments
should be passed to functions:

  /* I counts args in order (to be) pushed; ARGPOS counts in order written.  */
  for (argpos = 0; argpos < num_actuals; i--, argpos++)
{
  tree type = TREE_TYPE (args[i].tree_value);
  [...]
  /* See if this argument should be passed by invisible reference.  */
  function_arg_info arg (type, argpos < n_named_args);

And it has to be that way for calls to unprototyped functions,
or for varargs.

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.

Thanks,
Richard

>
> Richard. 
>
>>This patch therefore adds a hook that lets the target choose
>>whether such vector types are indeed equivalent.
>>
>>Note that the new tests fail for -mabi=ilp32 in the same way as other
>>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>>
>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>
>>Richard
>>
>>
>>2019-12-12  Richard Sandiford  
>>
>>gcc/
>>  * target.def (compatible_vector_types_p): New target hook.
>>  * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>>  * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>>  * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>>  * doc/tm.texi: Regenerate.
>>  * gimple-expr.c: Include target.h.
>>  (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>>  * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>>  function.
>>  (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>>  * config/aarch64/aarch64-sve-builtins.cc
>>(gimple_folder::convert_pred):
>>  Use the original predicate if it already has a suitable type.
>>
>>gcc/testsuite/
>>  * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>>  * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>>
>>Index: gcc/target.def
>>===
>>--- gcc/target.def2019-11-30 18:48:18.531984101 +
>>+++ gcc/target.def2019-12-12 15:07:43.960415368 +
>>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>>  hook_bool_mode_false)
>> 
>> DEFHOOK
>>+(compatible_vector_types_p,
>>+ "Return true if there is no target-specific reason for treating\n\
>>+vector types @var{type1} and @var{type2} as distinct types.  The
>>caller\n\
>>+has already checked for target-independent reasons, meaning that
>>the\n\
>>+types are known to have the same mode, to have the same number of
>>elements,\n\
>>+and to have what the caller considers to be compatible element
>>types.\n\
>>+\n\
>>+The main reason for defining this hook is to reject pairs of types\n\
>>+that are handled differently by the target's calling convention.\n\
>>+For example, when a new @var{N}-bit vector architecture is added\n\
>>+to a target, the target may want to handle normal @var{N}-bit\n\
>>+@code{VECTOR_TYPE} arguments and return values in the same way as\n\
>>+before, to maintain backwards compatibility.  

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Biener
On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford 
 wrote:
>One problem with adding an N-bit vector extension to an existing
>architecture is to decide how N-bit vectors should be passed to
>functions and returned from functions.  Allowing all N-bit vector
>types to be passed in registers breaks backwards compatibility,
>since N-bit vectors could be used (and emulated) before the vector
>extension was added.  But always passing N-bit vectors on the
>stack would be inefficient for things like vector libm functions.
>
>For SVE we took the compromise position of predefining new SVE vector
>types that are distinct from all existing vector types, including
>GNU-style vectors.  The new types are passed and returned in an
>efficient way while existing vector types are passed and returned
>in the traditional way.  In the right circumstances, the two types
>are inter-convertible.
>
>The SVE types are created using:
>
>  vectype = build_distinct_type_copy (vectype);
>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>  TYPE_ARTIFICIAL (vectype) = 1;
>
>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>to convert from one type to the other.  However, the distinction can
>be lost during gimple, which treats two vector types with the same
>mode, number of elements, and element type as equivalent.  And for
>most targets that's the right thing to do.

And why's that a problem? The difference appears only in the function call ABI 
which is determined by the function signature rather than types or modes of the 
actual arguments? 

Richard. 

>This patch therefore adds a hook that lets the target choose
>whether such vector types are indeed equivalent.
>
>Note that the new tests fail for -mabi=ilp32 in the same way as other
>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
>Richard
>
>
>2019-12-12  Richard Sandiford  
>
>gcc/
>   * target.def (compatible_vector_types_p): New target hook.
>   * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>   * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>   * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>   * doc/tm.texi: Regenerate.
>   * gimple-expr.c: Include target.h.
>   (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>   * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>   function.
>   (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>   * config/aarch64/aarch64-sve-builtins.cc
>(gimple_folder::convert_pred):
>   Use the original predicate if it already has a suitable type.
>
>gcc/testsuite/
>   * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>   * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>
>Index: gcc/target.def
>===
>--- gcc/target.def 2019-11-30 18:48:18.531984101 +
>+++ gcc/target.def 2019-12-12 15:07:43.960415368 +
>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>  hook_bool_mode_false)
> 
> DEFHOOK
>+(compatible_vector_types_p,
>+ "Return true if there is no target-specific reason for treating\n\
>+vector types @var{type1} and @var{type2} as distinct types.  The
>caller\n\
>+has already checked for target-independent reasons, meaning that
>the\n\
>+types are known to have the same mode, to have the same number of
>elements,\n\
>+and to have what the caller considers to be compatible element
>types.\n\
>+\n\
>+The main reason for defining this hook is to reject pairs of types\n\
>+that are handled differently by the target's calling convention.\n\
>+For example, when a new @var{N}-bit vector architecture is added\n\
>+to a target, the target may want to handle normal @var{N}-bit\n\
>+@code{VECTOR_TYPE} arguments and return values in the same way as\n\
>+before, to maintain backwards compatibility.  However, it may also\n\
>+provide new, architecture-specific @code{VECTOR_TYPE}s that are
>passed\n\
>+and returned in a more efficient way.  It is then important to
>maintain\n\
>+a distinction between the ``normal'' @code{VECTOR_TYPE}s and the
>new\n\
>+architecture-specific ones.\n\
>+\n\
>+The default implementation returns true, which is correct for most
>targets.",
>+ bool, (const_tree type1, const_tree type2),
>+ hook_bool_const_tree_const_tree_true)
>+
>+DEFHOOK
> (vector_alignment,
> "This hook can be used to define the alignment for a vector of type\n\
>@var{type}, in order to comply with a platform ABI.  The default is
>to\n\
>Index: gcc/hooks.h
>===
>--- gcc/hooks.h2019-11-04 21:13:57.727755548 +
>+++ gcc/hooks.h2019-12-12 15:07:43.960415368 +
>@@ -45,6 +45,7 @@ extern bool hook_bool_uint_uint_mode_fal
> extern bool hook_bool_uint_mode_true (unsigned int, machine_mode);
> extern bool hook_bool_tree_false (tree);
>