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?
Richard
>
> So, instead of match.pd the check would need to be in vector pattern matching
> and SSA math opts. Or alternatively in internal-fn.cc as laid out above.
>
> Richard.
>
>> PR target/115961
>>
>> gcc/ChangeLog:
>>
>> * match.pd: Add type_has_mode_precision_p check for .SAT_TRUNC.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.target/i386/pr115961-run-1.C: New test.
>> * g++.target/riscv/rvv/base/pr115961-run-1.C: New test.
>>
>> Signed-off-by: Pan Li <pan2...@intel.com>
>> ---
>> gcc/match.pd | 4 +--
>> .../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
>> .../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
>> 3 files changed, 70 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/g++.target/i386/pr115961-run-1.C
>> create mode 100644 gcc/testsuite/g++.target/riscv/rvv/base/pr115961-run-1.C
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 24a0bbead3e..8121ec09f53 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -3240,7 +3240,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> (bit_ior:c (negate (convert (gt @0 INTEGER_CST@1)))
>> (convert @0))
>> (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
>> - && TYPE_UNSIGNED (TREE_TYPE (@0)))
>> + && TYPE_UNSIGNED (TREE_TYPE (@0)) && type_has_mode_precision_p (type))
>> (with
>> {
>> unsigned itype_precision = TYPE_PRECISION (TREE_TYPE (@0));
>> @@ -3255,7 +3255,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> (match (unsigned_integer_sat_trunc @0)
>> (convert (min @0 INTEGER_CST@1))
>> (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
>> - && TYPE_UNSIGNED (TREE_TYPE (@0)))
>> + && TYPE_UNSIGNED (TREE_TYPE (@0)) && type_has_mode_precision_p (type))
>> (with
>> {
>> unsigned itype_precision = TYPE_PRECISION (TREE_TYPE (@0));
>> diff --git a/gcc/testsuite/g++.target/i386/pr115961-run-1.C
>> b/gcc/testsuite/g++.target/i386/pr115961-run-1.C
>> new file mode 100644
>> index 00000000000..b8c8aef3b17
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/i386/pr115961-run-1.C
>> @@ -0,0 +1,34 @@
>> +/* PR target/115961 */
>> +/* { dg-do run } */
>> +/* { dg-options "-O3 -fdump-rtl-expand-details" } */
>> +
>> +struct e
>> +{
>> + unsigned pre : 12;
>> + unsigned a : 4;
>> +};
>> +
>> +static unsigned min_u (unsigned a, unsigned b)
>> +{
>> + return (b < a) ? b : a;
>> +}
>> +
>> +__attribute__((noipa))
>> +void bug (e * v, unsigned def, unsigned use) {
>> + e & defE = *v;
>> + defE.a = min_u (use + 1, 0xf);
>> +}
>> +
>> +__attribute__((noipa, optimize(0)))
>> +int main(void)
>> +{
>> + e v = { 0xded, 3 };
>> +
>> + bug(&v, 32, 33);
>> +
>> + if (v.a != 0xf)
>> + __builtin_abort ();
>> +
>> + return 0;
>> +}
>> +/* { dg-final { scan-rtl-dump-not ".SAT_TRUNC " "expand" } } */
>> diff --git a/gcc/testsuite/g++.target/riscv/rvv/base/pr115961-run-1.C
>> b/gcc/testsuite/g++.target/riscv/rvv/base/pr115961-run-1.C
>> new file mode 100644
>> index 00000000000..b8c8aef3b17
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/riscv/rvv/base/pr115961-run-1.C
>> @@ -0,0 +1,34 @@
>> +/* PR target/115961 */
>> +/* { dg-do run } */
>> +/* { dg-options "-O3 -fdump-rtl-expand-details" } */
>> +
>> +struct e
>> +{
>> + unsigned pre : 12;
>> + unsigned a : 4;
>> +};
>> +
>> +static unsigned min_u (unsigned a, unsigned b)
>> +{
>> + return (b < a) ? b : a;
>> +}
>> +
>> +__attribute__((noipa))
>> +void bug (e * v, unsigned def, unsigned use) {
>> + e & defE = *v;
>> + defE.a = min_u (use + 1, 0xf);
>> +}
>> +
>> +__attribute__((noipa, optimize(0)))
>> +int main(void)
>> +{
>> + e v = { 0xded, 3 };
>> +
>> + bug(&v, 32, 33);
>> +
>> + if (v.a != 0xf)
>> + __builtin_abort ();
>> +
>> + return 0;
>> +}
>> +/* { dg-final { scan-rtl-dump-not ".SAT_TRUNC " "expand" } } */
>> --
>> 2.34.1
>>
