[v2] Internal-fn: Only allow type matches mode for internal fn[PR115961]
Checks
| Context |
Check |
Description |
| rivoscibot/toolchain-ci-rivos-apply-patch |
success
|
Patch applied
|
| rivoscibot/toolchain-ci-rivos-lint |
success
|
Lint passed
|
| rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--newlib-rv64gc-lp64d-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--newlib-rv32imc_zba_zbb_zbc_zbs-ilp32d-non-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--linux-rv32gc_zba_zbb_zbc_zbs-ilp32d-non-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-non-multilib |
success
|
Build passed
|
| rivoscibot/toolchain-ci-rivos-test |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
From: Pan Li <pan2.li@intel.com>
The direct_internal_fn_supported_p has no restrictions for the type
modes. For example the 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 check strictly for the direct_internal_fn_supported_p,
and only allows the type matches mode for ifn type tree pair.
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.
PR target/115961
gcc/ChangeLog:
* internal-fn.cc (type_strictly_matches_mode_p): Add new func
impl to check type strictly matches mode or not.
(type_pair_strictly_matches_mode_p): Ditto but for tree type
pair.
(direct_internal_fn_supported_p): Add above check for the tree
type pair.
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.li@intel.com>
---
gcc/internal-fn.cc | 32 +++++++++++++++++
.../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
.../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
3 files changed, 100 insertions(+)
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
Comments
On Fri, Jul 19, 2024 at 4:11 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> The direct_internal_fn_supported_p has no restrictions for the type
> modes. For example the 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 check strictly for the direct_internal_fn_supported_p,
> and only allows the type matches mode for ifn type tree pair.
>
> 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.
Note I just ran into basically the same bug with BIT_ANDC/BIT_IORC
(which will be renamed to BIT_ANDN/BIT_IORN).
>
> PR target/115961
>
> gcc/ChangeLog:
>
> * internal-fn.cc (type_strictly_matches_mode_p): Add new func
> impl to check type strictly matches mode or not.
> (type_pair_strictly_matches_mode_p): Ditto but for tree type
> pair.
> (direct_internal_fn_supported_p): Add above check for the tree
> type pair.
>
> 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.li@intel.com>
> ---
> gcc/internal-fn.cc | 32 +++++++++++++++++
> .../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
> .../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
> 3 files changed, 100 insertions(+)
> 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/internal-fn.cc b/gcc/internal-fn.cc
> index 95946bfd683..5c21249318e 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4164,6 +4164,35 @@ direct_internal_fn_optab (internal_fn fn)
> gcc_unreachable ();
> }
>
> +/* Return true if TYPE's mode has the same format as TYPE, and if there is
> + a 1:1 correspondence between the values that the mode can store and the
> + values that the type can store. */
> +
> +static bool
> +type_strictly_matches_mode_p (const_tree type)
> +{
> + if (VECTOR_TYPE_P (type))
> + return VECTOR_MODE_P (TYPE_MODE (type));
> +
> + if (INTEGRAL_TYPE_P (type))
> + return type_has_mode_precision_p (type);
> +
> + if (SCALAR_FLOAT_TYPE_P (type) || COMPLEX_FLOAT_TYPE_P (type))
> + return true;
> +
> + return false;
> +}
> +
> +/* Return true if both the first and the second type of tree pair are
> + strictly matches their modes, or return false. */
Just a slight comment improvement:
/* Returns true if both types of TYPE_PAIR strictly match their modes,
else returns false. */
> +
> +static bool
> +type_pair_strictly_matches_mode_p (tree_pair type_pair)
> +{
> + return type_strictly_matches_mode_p (type_pair.first)
> + && type_strictly_matches_mode_p (type_pair.second);
> +}
> +
> /* Return true if FN is supported for the types in TYPES when the
> optimization type is OPT_TYPE. The types are those associated with
> the "type0" and "type1" fields of FN's direct_internal_fn_info
> @@ -4173,6 +4202,9 @@ bool
> direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
> optimization_type opt_type)
> {
> + if (!type_pair_strictly_matches_mode_p (types))
> + return false;
> +
> switch (fn)
> {
> #define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) \
> 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
This testcase could go in g++.dg/torture/ without the -O3 option.
> @@ -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" } } */
Since we are scanning for the negative it should pass on all targets
even ones without SAT_TRUNC support. And then you should not need the
other testcase either.
Thanks,
Andrew Pinski
> 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
>
On Fri, Jul 19, 2024 at 1:10 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> The direct_internal_fn_supported_p has no restrictions for the type
> modes. For example the 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 check strictly for the direct_internal_fn_supported_p,
> and only allows the type matches mode for ifn type tree pair.
>
> 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.
LGTM unless Richard S. has any more comments.
Richard.
> PR target/115961
>
> gcc/ChangeLog:
>
> * internal-fn.cc (type_strictly_matches_mode_p): Add new func
> impl to check type strictly matches mode or not.
> (type_pair_strictly_matches_mode_p): Ditto but for tree type
> pair.
> (direct_internal_fn_supported_p): Add above check for the tree
> type pair.
>
> 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.li@intel.com>
> ---
> gcc/internal-fn.cc | 32 +++++++++++++++++
> .../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
> .../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
> 3 files changed, 100 insertions(+)
> 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/internal-fn.cc b/gcc/internal-fn.cc
> index 95946bfd683..5c21249318e 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -4164,6 +4164,35 @@ direct_internal_fn_optab (internal_fn fn)
> gcc_unreachable ();
> }
>
> +/* Return true if TYPE's mode has the same format as TYPE, and if there is
> + a 1:1 correspondence between the values that the mode can store and the
> + values that the type can store. */
> +
> +static bool
> +type_strictly_matches_mode_p (const_tree type)
> +{
> + if (VECTOR_TYPE_P (type))
> + return VECTOR_MODE_P (TYPE_MODE (type));
> +
> + if (INTEGRAL_TYPE_P (type))
> + return type_has_mode_precision_p (type);
> +
> + if (SCALAR_FLOAT_TYPE_P (type) || COMPLEX_FLOAT_TYPE_P (type))
> + return true;
> +
> + return false;
> +}
> +
> +/* Return true if both the first and the second type of tree pair are
> + strictly matches their modes, or return false. */
> +
> +static bool
> +type_pair_strictly_matches_mode_p (tree_pair type_pair)
> +{
> + return type_strictly_matches_mode_p (type_pair.first)
> + && type_strictly_matches_mode_p (type_pair.second);
> +}
> +
> /* Return true if FN is supported for the types in TYPES when the
> optimization type is OPT_TYPE. The types are those associated with
> the "type0" and "type1" fields of FN's direct_internal_fn_info
> @@ -4173,6 +4202,9 @@ bool
> direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
> optimization_type opt_type)
> {
> + if (!type_pair_strictly_matches_mode_p (types))
> + return false;
> +
> switch (fn)
> {
> #define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) \
> 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
>
Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Jul 19, 2024 at 1:10 PM <pan2.li@intel.com> wrote:
>>
>> From: Pan Li <pan2.li@intel.com>
>>
>> The direct_internal_fn_supported_p has no restrictions for the type
>> modes. For example the 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 check strictly for the direct_internal_fn_supported_p,
>> and only allows the type matches mode for ifn type tree pair.
>>
>> 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.
>
> LGTM unless Richard S. has any more comments.
LGTM too with Andrew's comments addressed.
Thanks,
Richard
>
> Richard.
>
>> PR target/115961
>>
>> gcc/ChangeLog:
>>
>> * internal-fn.cc (type_strictly_matches_mode_p): Add new func
>> impl to check type strictly matches mode or not.
>> (type_pair_strictly_matches_mode_p): Ditto but for tree type
>> pair.
>> (direct_internal_fn_supported_p): Add above check for the tree
>> type pair.
>>
>> 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.li@intel.com>
>> ---
>> gcc/internal-fn.cc | 32 +++++++++++++++++
>> .../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
>> .../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
>> 3 files changed, 100 insertions(+)
>> 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/internal-fn.cc b/gcc/internal-fn.cc
>> index 95946bfd683..5c21249318e 100644
>> --- a/gcc/internal-fn.cc
>> +++ b/gcc/internal-fn.cc
>> @@ -4164,6 +4164,35 @@ direct_internal_fn_optab (internal_fn fn)
>> gcc_unreachable ();
>> }
>>
>> +/* Return true if TYPE's mode has the same format as TYPE, and if there is
>> + a 1:1 correspondence between the values that the mode can store and the
>> + values that the type can store. */
>> +
>> +static bool
>> +type_strictly_matches_mode_p (const_tree type)
>> +{
>> + if (VECTOR_TYPE_P (type))
>> + return VECTOR_MODE_P (TYPE_MODE (type));
>> +
>> + if (INTEGRAL_TYPE_P (type))
>> + return type_has_mode_precision_p (type);
>> +
>> + if (SCALAR_FLOAT_TYPE_P (type) || COMPLEX_FLOAT_TYPE_P (type))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +/* Return true if both the first and the second type of tree pair are
>> + strictly matches their modes, or return false. */
>> +
>> +static bool
>> +type_pair_strictly_matches_mode_p (tree_pair type_pair)
>> +{
>> + return type_strictly_matches_mode_p (type_pair.first)
>> + && type_strictly_matches_mode_p (type_pair.second);
>> +}
>> +
>> /* Return true if FN is supported for the types in TYPES when the
>> optimization type is OPT_TYPE. The types are those associated with
>> the "type0" and "type1" fields of FN's direct_internal_fn_info
>> @@ -4173,6 +4202,9 @@ bool
>> direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
>> optimization_type opt_type)
>> {
>> + if (!type_pair_strictly_matches_mode_p (types))
>> + return false;
>> +
>> switch (fn)
>> {
>> #define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) \
>> 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
>>
> Just a slight comment improvement:
> /* Returns true if both types of TYPE_PAIR strictly match their modes,
> else returns false. */
> This testcase could go in g++.dg/torture/ without the -O3 option.
> Since we are scanning for the negative it should pass on all targets
> even ones without SAT_TRUNC support. And then you should not need the
> other testcase either.
Thanks all, will address above comments and commit it if no surprise from test.
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandiford@arm.com>
Sent: Tuesday, July 23, 2024 10:03 PM
To: Richard Biener <richard.guenther@gmail.com>
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; tamar.christina@arm.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com
Subject: Re: [PATCH v2] Internal-fn: Only allow type matches mode for internal fn[PR115961]
Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Jul 19, 2024 at 1:10 PM <pan2.li@intel.com> wrote:
>>
>> From: Pan Li <pan2.li@intel.com>
>>
>> The direct_internal_fn_supported_p has no restrictions for the type
>> modes. For example the 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 check strictly for the direct_internal_fn_supported_p,
>> and only allows the type matches mode for ifn type tree pair.
>>
>> 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.
>
> LGTM unless Richard S. has any more comments.
LGTM too with Andrew's comments addressed.
Thanks,
Richard
>
> Richard.
>
>> PR target/115961
>>
>> gcc/ChangeLog:
>>
>> * internal-fn.cc (type_strictly_matches_mode_p): Add new func
>> impl to check type strictly matches mode or not.
>> (type_pair_strictly_matches_mode_p): Ditto but for tree type
>> pair.
>> (direct_internal_fn_supported_p): Add above check for the tree
>> type pair.
>>
>> 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.li@intel.com>
>> ---
>> gcc/internal-fn.cc | 32 +++++++++++++++++
>> .../g++.target/i386/pr115961-run-1.C | 34 +++++++++++++++++++
>> .../riscv/rvv/base/pr115961-run-1.C | 34 +++++++++++++++++++
>> 3 files changed, 100 insertions(+)
>> 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/internal-fn.cc b/gcc/internal-fn.cc
>> index 95946bfd683..5c21249318e 100644
>> --- a/gcc/internal-fn.cc
>> +++ b/gcc/internal-fn.cc
>> @@ -4164,6 +4164,35 @@ direct_internal_fn_optab (internal_fn fn)
>> gcc_unreachable ();
>> }
>>
>> +/* Return true if TYPE's mode has the same format as TYPE, and if there is
>> + a 1:1 correspondence between the values that the mode can store and the
>> + values that the type can store. */
>> +
>> +static bool
>> +type_strictly_matches_mode_p (const_tree type)
>> +{
>> + if (VECTOR_TYPE_P (type))
>> + return VECTOR_MODE_P (TYPE_MODE (type));
>> +
>> + if (INTEGRAL_TYPE_P (type))
>> + return type_has_mode_precision_p (type);
>> +
>> + if (SCALAR_FLOAT_TYPE_P (type) || COMPLEX_FLOAT_TYPE_P (type))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +/* Return true if both the first and the second type of tree pair are
>> + strictly matches their modes, or return false. */
>> +
>> +static bool
>> +type_pair_strictly_matches_mode_p (tree_pair type_pair)
>> +{
>> + return type_strictly_matches_mode_p (type_pair.first)
>> + && type_strictly_matches_mode_p (type_pair.second);
>> +}
>> +
>> /* Return true if FN is supported for the types in TYPES when the
>> optimization type is OPT_TYPE. The types are those associated with
>> the "type0" and "type1" fields of FN's direct_internal_fn_info
>> @@ -4173,6 +4202,9 @@ bool
>> direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
>> optimization_type opt_type)
>> {
>> + if (!type_pair_strictly_matches_mode_p (types))
>> + return false;
>> +
>> switch (fn)
>> {
>> #define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) \
>> 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
>>
@@ -4164,6 +4164,35 @@ direct_internal_fn_optab (internal_fn fn)
gcc_unreachable ();
}
+/* Return true if TYPE's mode has the same format as TYPE, and if there is
+ a 1:1 correspondence between the values that the mode can store and the
+ values that the type can store. */
+
+static bool
+type_strictly_matches_mode_p (const_tree type)
+{
+ if (VECTOR_TYPE_P (type))
+ return VECTOR_MODE_P (TYPE_MODE (type));
+
+ if (INTEGRAL_TYPE_P (type))
+ return type_has_mode_precision_p (type);
+
+ if (SCALAR_FLOAT_TYPE_P (type) || COMPLEX_FLOAT_TYPE_P (type))
+ return true;
+
+ return false;
+}
+
+/* Return true if both the first and the second type of tree pair are
+ strictly matches their modes, or return false. */
+
+static bool
+type_pair_strictly_matches_mode_p (tree_pair type_pair)
+{
+ return type_strictly_matches_mode_p (type_pair.first)
+ && type_strictly_matches_mode_p (type_pair.second);
+}
+
/* Return true if FN is supported for the types in TYPES when the
optimization type is OPT_TYPE. The types are those associated with
the "type0" and "type1" fields of FN's direct_internal_fn_info
@@ -4173,6 +4202,9 @@ bool
direct_internal_fn_supported_p (internal_fn fn, tree_pair types,
optimization_type opt_type)
{
+ if (!type_pair_strictly_matches_mode_p (types))
+ return false;
+
switch (fn)
{
#define DEF_INTERNAL_FN(CODE, FLAGS, FNSPEC) \
new file mode 100644
@@ -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" } } */
new file mode 100644
@@ -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" } } */