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