> Am 24.08.2024 um 13:33 schrieb pan2...@intel.com:
>
> From: Pan Li <pan2...@intel.com>
>
> This patch would like to add strict check for imm operand of .SAT_ADD
> matching. We have no type checking for imm operand in previous, which
> may result in unexpected IL to be catched by .SAT_ADD pattern.
>
> However, things may become more complicated due to the int promotion.
> This means any const_int without any suffix will be promoted to int
> before matching. For example as below.
>
> uint8_t a;
> uint8_t sum = .SAT_ADD (a, 12);
>
> The second operand will be (const_int 12) with int type when try to
> match .SAT_ADD. Thus, to support int8/int16 .SAT_ADD, only the
> int32 and int64 will be strictly checked.
>
> The below test suite are passed for this patch:
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> gcc/ChangeLog:
>
> * match.pd:
> * match.pd: Add strict type check for .SAT_ADD imm operand.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/sat_u_add_imm-11.c: Adjust test case for imm.
> * gcc.target/riscv/sat_u_add_imm-12.c: Ditto.
> * gcc.target/riscv/sat_u_add_imm-15.c: Ditto.
> * gcc.target/riscv/sat_u_add_imm-16.c: Ditto.
> * gcc.target/riscv/sat_u_add_imm_type_check-1.c: New test.
> * gcc.target/riscv/sat_u_add_imm_type_check-2.c: New test.
> * gcc.target/riscv/sat_u_add_imm_type_check-3.c: New test.
> * gcc.target/riscv/sat_u_add_imm_type_check-4.c: New test.
> * gcc.target/riscv/sat_u_add_imm_type_check-5.c: New test.
> * gcc.target/riscv/sat_u_add_imm_type_check-6.c: New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/match.pd | 11 ++++++++++-
> gcc/testsuite/gcc.target/riscv/sat_u_add_imm-11.c | 4 ++--
> gcc/testsuite/gcc.target/riscv/sat_u_add_imm-12.c | 4 ++--
> gcc/testsuite/gcc.target/riscv/sat_u_add_imm-15.c | 4 ++--
> gcc/testsuite/gcc.target/riscv/sat_u_add_imm-16.c | 4 ++--
> .../gcc.target/riscv/sat_u_add_imm_type_check-1.c | 9 +++++++++
> .../gcc.target/riscv/sat_u_add_imm_type_check-2.c | 9 +++++++++
> .../gcc.target/riscv/sat_u_add_imm_type_check-3.c | 9 +++++++++
> .../gcc.target/riscv/sat_u_add_imm_type_check-4.c | 9 +++++++++
> .../gcc.target/riscv/sat_u_add_imm_type_check-5.c | 9 +++++++++
> .../gcc.target/riscv/sat_u_add_imm_type_check-6.c | 9 +++++++++
> 11 files changed, 72 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-2.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-3.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-4.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-5.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-6.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 65a3aae2243..f695790629e 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3190,7 +3190,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (cond^ (ne (imagpart (IFN_ADD_OVERFLOW@2 @0 INTEGER_CST@1)) integer_zerop)
> integer_minus_onep (realpart @2))
> (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
> - && types_match (type, @0))))
> + && types_match (type, @0))
> + (with
> + {
> + unsigned precision = TYPE_PRECISION (type);
> + unsigned int_precision = HOST_BITS_PER_INT;
> + }
> + /* The const_int will perform int promotion, the const_int will have at
> + least the int_precision. Thus, type less than int_precision will be
> + skipped the type match checking. */
> + (if (precision < int_precision || types_match (type, @1))))))
I don’t think this is an improvement. What are the constraints on the type so
that the saturating operation is correcly matched?
If you let through wrong types here you will still have to deal with that in
the consumers.
Note there’s int_fits_type which might come handy- it’s just not clear which
constants form a valid saturating operation.
Richard
>
> /* Unsigned saturation sub, case 1 (branch with gt):
> SAT_U_SUB = X > Y ? X - Y : 0 */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-11.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-11.c
> index 43f34b5f3c9..a246e9b1857 100644
> --- a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-11.c
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-11.c
> @@ -5,7 +5,7 @@
> #include "sat_arith.h"
>
> /*
> -** sat_u_add_imm7_uint32_t_fmt_3:
> +** sat_u_add_imm7u_uint32_t_fmt_3:
> ** slli\s+[atx][0-9]+,\s*a0,\s*32
> ** srli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*32
> ** addi\s+[atx][0-9]+,\s*a0,\s*7
> @@ -17,6 +17,6 @@
> ** sext.w\s+a0,\s*a0
> ** ret
> */
> -DEF_SAT_U_ADD_IMM_FMT_3(uint32_t, 7)
> +DEF_SAT_U_ADD_IMM_FMT_3(uint32_t, 7u)
>
> /* { dg-final { scan-rtl-dump-times ".SAT_ADD " 2 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-12.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-12.c
> index 561c127f5fa..143f14c3af0 100644
> --- a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-12.c
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-12.c
> @@ -5,13 +5,13 @@
> #include "sat_arith.h"
>
> /*
> -** sat_u_add_imm8_uint64_t_fmt_3:
> +** sat_u_add_imm8ull_uint64_t_fmt_3:
> ** addi\s+[atx][0-9]+,\s*a0,\s*8
> ** sltu\s+[atx][0-9]+,\s*[atx][0-9]+,\s*[atx][0-9]+
> ** neg\s+[atx][0-9]+,\s*[atx][0-9]+
> ** or\s+a0,\s*[atx][0-9]+,\s*[atx][0-9]+
> ** ret
> */
> -DEF_SAT_U_ADD_IMM_FMT_3(uint64_t, 8)
> +DEF_SAT_U_ADD_IMM_FMT_3(uint64_t, 8ull)
>
> /* { dg-final { scan-rtl-dump-times ".SAT_ADD " 2 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-15.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-15.c
> index eeea574bba2..995a02bffff 100644
> --- a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-15.c
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-15.c
> @@ -5,7 +5,7 @@
> #include "sat_arith.h"
>
> /*
> -** sat_u_add_imm7_uint32_t_fmt_4:
> +** sat_u_add_imm7u_uint32_t_fmt_4:
> ** slli\s+[atx][0-9]+,\s*a0,\s*32
> ** srli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*32
> ** addi\s+[atx][0-9]+,\s*a0,\s*7
> @@ -17,6 +17,6 @@
> ** sext.w\s+a0,\s*a0
> ** ret
> */
> -DEF_SAT_U_ADD_IMM_FMT_4(uint32_t, 7)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint32_t, 7u)
>
> /* { dg-final { scan-rtl-dump-times ".SAT_ADD " 2 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-16.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-16.c
> index 307b81589ee..65e5a4a99bb 100644
> --- a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-16.c
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm-16.c
> @@ -5,13 +5,13 @@
> #include "sat_arith.h"
>
> /*
> -** sat_u_add_imm8_uint64_t_fmt_4:
> +** sat_u_add_imm8ull_uint64_t_fmt_4:
> ** addi\s+[atx][0-9]+,\s*a0,\s*8
> ** sltu\s+[atx][0-9]+,\s*[atx][0-9]+,\s*[atx][0-9]+
> ** neg\s+[atx][0-9]+,\s*[atx][0-9]+
> ** or\s+a0,\s*[atx][0-9]+,\s*[atx][0-9]+
> ** ret
> */
> -DEF_SAT_U_ADD_IMM_FMT_4(uint64_t, 8)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint64_t, 8ull)
>
> /* { dg-final { scan-rtl-dump-times ".SAT_ADD " 2 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-1.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-1.c
> new file mode 100644
> index 00000000000..81117a3e6de
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint8_t, 9)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint8_t, 9)
> +
> +/* { dg-final { scan-rtl-dump-times ".SAT_ADD " 4 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-2.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-2.c
> new file mode 100644
> index 00000000000..d5de0960a96
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint16_t, 91)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint16_t, 91)
> +
> +/* { dg-final { scan-rtl-dump-times ".SAT_ADD " 4 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-3.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-3.c
> new file mode 100644
> index 00000000000..a3391ca460a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-3.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint32_t, 191u)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint32_t, 191u)
> +
> +/* { dg-final { scan-rtl-dump-times ".SAT_ADD " 4 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-4.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-4.c
> new file mode 100644
> index 00000000000..bc12e0e346f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-4.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint64_t, 191)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint64_t, 191)
> +
> +/* { dg-final { scan-rtl-dump-not ".SAT_ADD " "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-5.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-5.c
> new file mode 100644
> index 00000000000..9b92d9624b5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-5.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint64_t, 191ull)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint64_t, 191ull)
> +
> +/* { dg-final { scan-rtl-dump-times ".SAT_ADD " 4 "expand" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-6.c
> b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-6.c
> new file mode 100644
> index 00000000000..a64f417bda3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/sat_u_add_imm_type_check-6.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64d -O3 -fdump-rtl-expand-details" }
> */
> +
> +#include "sat_arith.h"
> +
> +DEF_SAT_U_ADD_IMM_FMT_3(uint32_t, 191ull)
> +DEF_SAT_U_ADD_IMM_FMT_4(uint32_t, 191ull)
> +
> +/* { dg-final { scan-rtl-dump-not ".SAT_ADD " "expand" } } */
> --
> 2.43.0
>
