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));
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
