Re: [AArch64] Enable generation of FRINTNZ instructions

[Richard Sandiford <richard.sandiford@arm.com>]

"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> On 17/12/2021 12:44, Richard Sandiford wrote:
>>
>>> @@ -3252,16 +3257,31 @@ vectorizable_call (vec_info *vinfo,
>>>         rhs_type = unsigned_type_node;
>>>       }
>>>   
>>> -  int mask_opno = -1;
>>> +  /* The argument that is not of the same type as the others.  */
>>> +  int diff_opno = -1;
>>> +  bool masked = false;
>>>     if (internal_fn_p (cfn))
>>> -    mask_opno = internal_fn_mask_index (as_internal_fn (cfn));
>>> +    {
>>> +      if (cfn == CFN_FTRUNC_INT)
>>> +	/* For FTRUNC this represents the argument that carries the type of the
>>> +	   intermediate signed integer.  */
>>> +	diff_opno = 1;
>>> +      else
>>> +	{
>>> +	  /* For masked operations this represents the argument that carries the
>>> +	     mask.  */
>>> +	  diff_opno = internal_fn_mask_index (as_internal_fn (cfn));
>>> +	  masked = diff_opno >=  0;
>>> +	}
>>> +    }
>> I think it would be cleaner not to process argument 1 at all for
>> CFN_FTRUNC_INT.  There's no particular need to vectorise it.
>
> I agree with this,  will change the loop to continue for argument 1 when 
> dealing with non-masked CFN's.
>
>>>   	}
>>> […]
>>> diff --git a/gcc/tree.c b/gcc/tree.c
>>> index 845228a055b2cfac0c9ca8c0cda1b9df4b0095c6..f1e9a1eb48769cb11aa69730e2480ed5522f78c1 100644
>>> --- a/gcc/tree.c
>>> +++ b/gcc/tree.c
>>> @@ -6645,11 +6645,11 @@ valid_constant_size_p (const_tree size, cst_size_error *perr /* = NULL */)
>>>     return true;
>>>   }
>>>   
>>> -/* Return the precision of the type, or for a complex or vector type the
>>> -   precision of the type of its elements.  */
>>> +/* Return the type, or for a complex or vector type the type of its
>>> +   elements.  */
>>>   
>>> -unsigned int
>>> -element_precision (const_tree type)
>>> +tree
>>> +element_type (const_tree type)
>>>   {
>>>     if (!TYPE_P (type))
>>>       type = TREE_TYPE (type);
>>> @@ -6657,7 +6657,16 @@ element_precision (const_tree type)
>>>     if (code == COMPLEX_TYPE || code == VECTOR_TYPE)
>>>       type = TREE_TYPE (type);
>>>   
>>> -  return TYPE_PRECISION (type);
>>> +  return (tree) type;
>> I think we should stick a const_cast in element_precision and make
>> element_type take a plain “type”.  As it stands element_type is an
>> implicit const_cast for many cases.
>>
>> Thanks,
> Was just curious about something here, I thought the purpose of having 
> element_precision (before) and element_type (now) take a const_tree as 
> an argument was to make it clear we aren't changing the input type. I 
> understand that as it stands element_type could be an implicit 
> const_cast (which I should be using rather than the '(tree)' cast), but 
> that's only if 'type' is a type that isn't complex/vector, either way, 
> we are conforming to the promise that we aren't changing the incoming 
> type, what the caller then does with the result is up to them no?
>
> I don't mind making the changes, just trying to understand the reasoning 
> behind it.

The problem with the above is that functions like the following become
well-typed:

void
foo (const_tree t)
{
  TYPE_MODE (element_type (t)) = VOIDmode;
}

even though element_type (t) could well be t.

One of the points of const_tree (and const pointer targets in general)
is to use the type system to enforce the promise that the value isn't
changed.

I guess the above is similar to the traditional problem with functions
like index and strstr, which take a const char * but return a char *.
So for example:

void
foo (const char *x)
{
  index (x, '.') = 0;
}

is well-typed.  But the equivalent C++ code (using iterators) would be
rejected.  If C allowed overloading them the correct prototypes would be:

    const char *index (const char *, int);
    char *index (char *, int);

And I think the same applies here.  Either we should provide two functions:

    const_tree element_type (const_tree);
    tree element_type (tree);

or just the latter.

Thanks,
Richard