> Am 17.07.2024 um 23:13 schrieb Richard Sandiford <richard.sandif...@arm.com>:
>
> Andrew Pinski <pins...@gmail.com> writes:
>>> On Wed, Jul 17, 2024 at 1:03 PM Tamar Christina <tamar.christ...@arm.com>
>>> wrote:
>>>
>>>> -----Original Message-----
>>>> From: Richard Sandiford <richard.sandif...@arm.com>
>>>> Sent: Wednesday, July 17, 2024 8:55 PM
>>>> To: Richard Biener <richard.guent...@gmail.com>
>>>> Cc: pan2...@intel.com; gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai;
>>>> kito.ch...@gmail.com; Tamar Christina <tamar.christ...@arm.com>;
>>>> jeffreya...@gmail.com; rdapp....@gmail.com; hongtao....@intel.com
>>>> Subject: Re: [PATCH v1] Match: Bugfix .SAT_TRUNC honor types has no mode
>>>> precision [PR115961]
>>>>
>>>> Richard Biener <richard.guent...@gmail.com> writes:
>>>>> On Wed, Jul 17, 2024 at 11:48 AM <pan2...@intel.com> wrote:
>>>>>>
>>>>>> From: Pan Li <pan2...@intel.com>
>>>>>>
>>>>>> The .SAT_TRUNC matching doesn't check the type has mode precision. Thus
>>>>>> when bitfield like below will be recog as .SAT_TRUNC.
>>>>>>
>>>>>> struct e
>>>>>> {
>>>>>> unsigned pre : 12;
>>>>>> unsigned a : 4;
>>>>>> };
>>>>>>
>>>>>> __attribute__((noipa))
>>>>>> void bug (e * v, unsigned def, unsigned use) {
>>>>>> e & defE = *v;
>>>>>> defE.a = min_u (use + 1, 0xf);
>>>>>> }
>>>>>>
>>>>>> This patch would like to add type_has_mode_precision_p for the
>>>>>> .SAT_TRUNC matching to get rid of this.
>>>>>>
>>>>>> The below test suites are passed for this patch:
>>>>>> 1. The rv64gcv fully regression tests.
>>>>>> 2. The x86 bootstrap tests.
>>>>>> 3. The x86 fully regression tests.
>>>>>
>>>>> Hmm, rather than restricting the matching the issue is the optab query or
>>>>> in this case how *_optab_supported_p blindly uses TYPE_MODE without
>>>>> either asserting the type has mode precision or failing the query in this
>>>>> case.
>>>>>
>>>>> I think it would be simplest to adjust direct_optab_supported_p
>>>>> (and convert_optab_supported_p) to reject such operations? Richard, do
>>>>> you agree or should callers check this instead?
>>>>
>>>> Sounds good to me, although I suppose it should go:
>>>>
>>>> bool
>>>> direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
>>>> optimization_type opt_type)
>>>> {
>>>> // <--- Here
>>>> switch (fn)
>>>> {
>>>>
>>>> }
>>>> }
>>>>
>>>> until we know of a specific case where that's wrong.
>>>>
>>>> Is type_has_mode_precision_p meaningful for all types?
>>>>
>>>
>>> I was wondering about that, wouldn't VECTOR_BOOLEAN_TYPE_P types fail?
>>> e.g. on AVX where the type precision is 1 but the mode precision QImode?
>>>
>>> Unless I misunderstood the predicate.
>>
>> So type_has_mode_precision_p only works with scalar integral types
>> (maybe scalar real types too) since it uses TYPE_PRECISION directly
>> and not element_precision (the precision field is overloaded for
>> vectors for the number of elements and TYPE_PRECISION on a vector type
>> will cause an ICE since r14-2150-gfe48f2651334bc).
>> So I suspect you need to check !VECTOR_TYPE_P (type) before calling
>> type_has_mode_precision_p .
>
> I think for VECTOR_TYPE_P it would be worth checking VECTOR_MODE_P instead,
> if we're not requiring callers to check this kind of thing.
>
> So something like:
>
> bool
> mode_describes_type_p (const_tree type)
> {
> if (VECTOR_TYPE_P (type))
> return VECTOR_MODE_P (TREE_TYPE (type));
That’s a good point and another mistake that keeps popping up. I hope the
generic vector support passes in a scalar type when checking for support on the
underlying integer mode…
>
> if (INTEGRAL_TYPE_P (type))
> return type_has_mode_precision_p (type);
>
> if (SCALAR_FLOAT_TYPE_P (type))
> return true;
>
> return false;
> }
>
> ? Possibly also with complex handling if we need that.
>
> Richard
