* [PATCH] i386: Fix emissing of __builtin_cpu_supports. @ 2021-12-14 9:55 Martin Liška 2021-12-14 10:28 ` Jakub Jelinek 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2021-12-14 9:55 UTC (permalink / raw) To: gcc-patches; +Cc: Stefan Kneifel The patch fixes __builtin_cpu_supports("avx512vbmi2") which returns a negative value (that's not allowed in the documentation). I also checked ppc target that does the same, and __builtin_cpu_is, which are fine. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin PR target/103661 gcc/ChangeLog: * config/i386/i386-builtins.c (fold_builtin_cpu): Compare to 0 as API expects that non-zero values are returned. For "avx512vbmi2" argument, we return now 1 << 31, which is a negative integer value. --- gcc/config/i386/i386-builtins.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/config/i386/i386-builtins.c b/gcc/config/i386/i386-builtins.c index 0fb14b55712..7e57b665c1e 100644 --- a/gcc/config/i386/i386-builtins.c +++ b/gcc/config/i386/i386-builtins.c @@ -2353,7 +2353,8 @@ fold_builtin_cpu (tree fndecl, tree *args) /* Return __cpu_model.__cpu_features[0] & field_val */ final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, build_int_cstu (unsigned_type_node, field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); + return build2 (NE_EXPR, integer_type_node, final, + build_int_cst (unsigned_type_node, 0)); } gcc_unreachable (); } -- 2.34.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: Fix emissing of __builtin_cpu_supports. 2021-12-14 9:55 [PATCH] i386: Fix emissing of __builtin_cpu_supports Martin Liška @ 2021-12-14 10:28 ` Jakub Jelinek 2021-12-14 15:07 ` Martin Liška 0 siblings, 1 reply; 14+ messages in thread From: Jakub Jelinek @ 2021-12-14 10:28 UTC (permalink / raw) To: Martin Liška; +Cc: gcc-patches, Stefan Kneifel On Tue, Dec 14, 2021 at 10:55:01AM +0100, Martin Liška wrote: > The patch fixes __builtin_cpu_supports("avx512vbmi2") which returns a negative > value (that's not allowed in the documentation). > > I also checked ppc target that does the same, and __builtin_cpu_is, which > are fine. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > PR target/103661 > > gcc/ChangeLog: > > * config/i386/i386-builtins.c (fold_builtin_cpu): Compare to 0 > as API expects that non-zero values are returned. For > "avx512vbmi2" argument, we return now 1 << 31, which is a > negative integer value. > --- > gcc/config/i386/i386-builtins.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/i386/i386-builtins.c b/gcc/config/i386/i386-builtins.c > index 0fb14b55712..7e57b665c1e 100644 > --- a/gcc/config/i386/i386-builtins.c > +++ b/gcc/config/i386/i386-builtins.c > @@ -2353,7 +2353,8 @@ fold_builtin_cpu (tree fndecl, tree *args) > /* Return __cpu_model.__cpu_features[0] & field_val */ > final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, > build_int_cstu (unsigned_type_node, field_val)); > - return build1 (CONVERT_EXPR, integer_type_node, final); > + return build2 (NE_EXPR, integer_type_node, final, > + build_int_cst (unsigned_type_node, 0)); > } > gcc_unreachable (); > } Wouldn't this be better done only if field_val has the msb set and keep the CONVERT_EXPR otherwise (why isn't it NOP_EXPR?)? Jakub ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: Fix emissing of __builtin_cpu_supports. 2021-12-14 10:28 ` Jakub Jelinek @ 2021-12-14 15:07 ` Martin Liška 2021-12-14 16:12 ` Jakub Jelinek 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2021-12-14 15:07 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Stefan Kneifel [-- Attachment #1: Type: text/plain, Size: 448 bytes --] On 12/14/21 11:28, Jakub Jelinek wrote: > Wouldn't this be better done only if field_val has the msb set Yes, updated in the attached patch. > and keep the CONVERT_EXPR otherwise (why isn't it NOP_EXPR?)? Dunno, but I can prepare a separate patch (likely stage1 material, right)? Note that are other places that also use CONVERT_EXPR. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin [-- Attachment #2: 0001-i386-Fix-emissing-of-__builtin_cpu_supports.patch --] [-- Type: text/x-patch, Size: 1342 bytes --] From 227450e9f3a506fdfcff67aa45135fe31f3f91f6 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Mon, 13 Dec 2021 15:34:30 +0100 Subject: [PATCH] i386: Fix emissing of __builtin_cpu_supports. PR target/103661 gcc/ChangeLog: * config/i386/i386-builtins.c (fold_builtin_cpu): Compare to 0 as API expects that non-zero values are returned (do that it mask == 31). For "avx512vbmi2" argument, we return now 1 << 31, which is a negative integer value. --- gcc/config/i386/i386-builtins.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gcc/config/i386/i386-builtins.c b/gcc/config/i386/i386-builtins.c index 0fb14b55712..bca244fc011 100644 --- a/gcc/config/i386/i386-builtins.c +++ b/gcc/config/i386/i386-builtins.c @@ -2353,7 +2353,11 @@ fold_builtin_cpu (tree fndecl, tree *args) /* Return __cpu_model.__cpu_features[0] & field_val */ final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, build_int_cstu (unsigned_type_node, field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); + if (isa_names_table[i].feature == 31) + return build2 (NE_EXPR, integer_type_node, final, + build_int_cst (unsigned_type_node, 0)); + else + return build1 (CONVERT_EXPR, integer_type_node, final); } gcc_unreachable (); } -- 2.34.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: Fix emissing of __builtin_cpu_supports. 2021-12-14 15:07 ` Martin Liška @ 2021-12-14 16:12 ` Jakub Jelinek 2021-12-15 9:57 ` [PATCH] i386: simplify cpu_feature handling Martin Liška 0 siblings, 1 reply; 14+ messages in thread From: Jakub Jelinek @ 2021-12-14 16:12 UTC (permalink / raw) To: Martin Liška; +Cc: gcc-patches, Stefan Kneifel On Tue, Dec 14, 2021 at 04:07:55PM +0100, Martin Liška wrote: > On 12/14/21 11:28, Jakub Jelinek wrote: > > Wouldn't this be better done only if field_val has the msb set > > Yes, updated in the attached patch. > > > and keep the CONVERT_EXPR otherwise (why isn't it NOP_EXPR?)? > > Dunno, but I can prepare a separate patch (likely stage1 material, > right)? Note that are other places that also use CONVERT_EXPR. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > From 227450e9f3a506fdfcff67aa45135fe31f3f91f6 Mon Sep 17 00:00:00 2001 > From: Martin Liska <mliska@suse.cz> > Date: Mon, 13 Dec 2021 15:34:30 +0100 > Subject: [PATCH] i386: Fix emissing of __builtin_cpu_supports. > > PR target/103661 > > gcc/ChangeLog: > > * config/i386/i386-builtins.c (fold_builtin_cpu): Compare to 0 > as API expects that non-zero values are returned (do that > it mask == 31). > For "avx512vbmi2" argument, we return now 1 << 31, which is a > negative integer value. > --- > gcc/config/i386/i386-builtins.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/i386/i386-builtins.c b/gcc/config/i386/i386-builtins.c > index 0fb14b55712..bca244fc011 100644 > --- a/gcc/config/i386/i386-builtins.c > +++ b/gcc/config/i386/i386-builtins.c > @@ -2353,7 +2353,11 @@ fold_builtin_cpu (tree fndecl, tree *args) > /* Return __cpu_model.__cpu_features[0] & field_val */ > final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, > build_int_cstu (unsigned_type_node, field_val)); > - return build1 (CONVERT_EXPR, integer_type_node, final); > + if (isa_names_table[i].feature == 31) > + return build2 (NE_EXPR, integer_type_node, final, > + build_int_cst (unsigned_type_node, 0)); > + else > + return build1 (CONVERT_EXPR, integer_type_node, final); > } > gcc_unreachable (); > } I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. Jakub ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i386: simplify cpu_feature handling 2021-12-14 16:12 ` Jakub Jelinek @ 2021-12-15 9:57 ` Martin Liška 2021-12-16 20:58 ` Stefan Kneifel 2022-01-03 11:43 ` Martin Liška 0 siblings, 2 replies; 14+ messages in thread From: Martin Liška @ 2021-12-15 9:57 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Stefan Kneifel [-- Attachment #1: Type: text/plain, Size: 395 bytes --] On 12/14/21 17:12, Jakub Jelinek wrote: > I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. Installed with that change, thanks. Moreover, I'm suggesting a simplification: The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR that can be simplified with NOP_EXPR. Survives i386.exp tests, may I install the patch after testing or is it a stage1 material? Thanks, Martin [-- Attachment #2: 0001-i386-simplify-cpu_feature-handling.patch --] [-- Type: text/x-patch, Size: 7216 bytes --] From 9fa89df81b3e6cb56f6ab59b0993168e7a048489 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Wed, 15 Dec 2021 10:54:23 +0100 Subject: [PATCH] i386: simplify cpu_feature handling The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR that can be simplified with NOP_EXPR. gcc/ChangeLog: * common/config/i386/cpuinfo.h (has_cpu_feature): Directly compute index in cpu_features2. (set_cpu_feature): Likewise. * config/i386/i386-builtins.c (fold_builtin_cpu): Also remove loop for cpu_features2 and use NOP_EXPRs. --- gcc/common/config/i386/cpuinfo.h | 50 +++++++++++--------- gcc/config/i386/i386-builtins.c | 79 ++++++++++++++++---------------- 2 files changed, 67 insertions(+), 62 deletions(-) diff --git a/gcc/common/config/i386/cpuinfo.h b/gcc/common/config/i386/cpuinfo.h index bbf29bdb116..dd321920108 100644 --- a/gcc/common/config/i386/cpuinfo.h +++ b/gcc/common/config/i386/cpuinfo.h @@ -55,43 +55,49 @@ struct __processor_model2 static inline int has_cpu_feature (struct __processor_model *cpu_model, unsigned int *cpu_features2, - enum processor_features f) + enum processor_features feature) { - unsigned int i; + unsigned index, offset; + unsigned f = feature; + if (f < 32) { /* The first 32 features. */ - return cpu_model->__cpu_features[0] & (1U << (f & 31)); + return cpu_model->__cpu_features[0] & (1U << f); + } + else + { + /* The rest of features. cpu_features2[i] contains features from + (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ + f -= 32; + index = f / 32; + offset = f % 32; + return cpu_features2[index] & (1U << offset); } - /* The rest of features. cpu_features2[i] contains features from - (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ - for (i = 0; i < SIZE_OF_CPU_FEATURES; i++) - if (f < (32 + 32 + i * 32)) - return cpu_features2[i] & (1U << ((f - (32 + i * 32)) & 31)); - gcc_unreachable (); } static inline void set_cpu_feature (struct __processor_model *cpu_model, unsigned int *cpu_features2, - enum processor_features f) + enum processor_features feature) { - unsigned int i; + unsigned index, offset; + unsigned f = feature; + if (f < 32) { /* The first 32 features. */ - cpu_model->__cpu_features[0] |= (1U << (f & 31)); - return; + cpu_model->__cpu_features[0] |= (1U << f); + } + else + { + /* The rest of features. cpu_features2[i] contains features from + (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ + f -= 32; + index = f / 32; + offset = f % 32; + cpu_features2[index] |= (1U << offset); } - /* The rest of features. cpu_features2[i] contains features from - (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ - for (i = 0; i < SIZE_OF_CPU_FEATURES; i++) - if (f < (32 + 32 + i * 32)) - { - cpu_features2[i] |= (1U << ((f - (32 + i * 32)) & 31)); - return; - } - gcc_unreachable (); } /* Get the specific type of AMD CPU and return AMD CPU name. Return diff --git a/gcc/config/i386/i386-builtins.c b/gcc/config/i386/i386-builtins.c index 4b30cf75c26..31e034e1bc9 100644 --- a/gcc/config/i386/i386-builtins.c +++ b/gcc/config/i386/i386-builtins.c @@ -2275,7 +2275,7 @@ fold_builtin_cpu (tree fndecl, tree *args) /* Check the value. */ final = build2 (EQ_EXPR, unsigned_type_node, ref, build_int_cstu (unsigned_type_node, field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); + return build1 (NOP_EXPR, integer_type_node, final); } else if (fn_code == IX86_BUILTIN_CPU_SUPPORTS) { @@ -2300,7 +2300,8 @@ fold_builtin_cpu (tree fndecl, tree *args) return integer_zero_node; } - if (isa_names_table[i].feature >= 32) + unsigned feature = isa_names_table[i].feature; + if (feature >= INT_TYPE_SIZE) { if (ix86_cpu_features2_var == nullptr) { @@ -2318,46 +2319,44 @@ fold_builtin_cpu (tree fndecl, tree *args) varpool_node::add (ix86_cpu_features2_var); } - for (unsigned int j = 0; j < SIZE_OF_CPU_FEATURES; j++) - if (isa_names_table[i].feature < (32 + 32 + j * 32)) - { - field_val = (1U << (isa_names_table[i].feature - - (32 + j * 32))); - tree index = size_int (j); - array_elt = build4 (ARRAY_REF, unsigned_type_node, - ix86_cpu_features2_var, - index, NULL_TREE, NULL_TREE); - /* Return __cpu_features2[index] & field_val */ - final = build2 (BIT_AND_EXPR, unsigned_type_node, - array_elt, - build_int_cstu (unsigned_type_node, - field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); - } + feature -= INT_TYPE_SIZE; + field_val = 1U << (feature % INT_TYPE_SIZE); + tree index = size_int (feature / INT_TYPE_SIZE); + array_elt = build4 (ARRAY_REF, unsigned_type_node, + ix86_cpu_features2_var, + index, NULL_TREE, NULL_TREE); + /* Return __cpu_features2[index] & field_val */ + final = build2 (BIT_AND_EXPR, unsigned_type_node, + array_elt, + build_int_cstu (unsigned_type_node, + field_val)); + return build1 (NOP_EXPR, integer_type_node, final); } - - field = TYPE_FIELDS (ix86_cpu_model_type_node); - /* Get the last field, which is __cpu_features. */ - while (DECL_CHAIN (field)) - field = DECL_CHAIN (field); - - /* Get the appropriate field: __cpu_model.__cpu_features */ - ref = build3 (COMPONENT_REF, TREE_TYPE (field), ix86_cpu_model_var, - field, NULL_TREE); - - /* Access the 0th element of __cpu_features array. */ - array_elt = build4 (ARRAY_REF, unsigned_type_node, ref, - integer_zero_node, NULL_TREE, NULL_TREE); - - field_val = (1U << isa_names_table[i].feature); - /* Return __cpu_model.__cpu_features[0] & field_val */ - final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, - build_int_cstu (unsigned_type_node, field_val)); - if (isa_names_table[i].feature == (INT_TYPE_SIZE - 1)) - return build2 (NE_EXPR, integer_type_node, final, - build_int_cst (unsigned_type_node, 0)); else - return build1 (CONVERT_EXPR, integer_type_node, final); + { + field = TYPE_FIELDS (ix86_cpu_model_type_node); + /* Get the last field, which is __cpu_features. */ + while (DECL_CHAIN (field)) + field = DECL_CHAIN (field); + + /* Get the appropriate field: __cpu_model.__cpu_features */ + ref = build3 (COMPONENT_REF, TREE_TYPE (field), ix86_cpu_model_var, + field, NULL_TREE); + + /* Access the 0th element of __cpu_features array. */ + array_elt = build4 (ARRAY_REF, unsigned_type_node, ref, + integer_zero_node, NULL_TREE, NULL_TREE); + + field_val = (1U << feature); + /* Return __cpu_model.__cpu_features[0] & field_val */ + final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, + build_int_cstu (unsigned_type_node, field_val)); + if (feature == (INT_TYPE_SIZE - 1)) + return build2 (NE_EXPR, integer_type_node, final, + build_int_cst (unsigned_type_node, 0)); + else + return build1 (NOP_EXPR, integer_type_node, final); + } } gcc_unreachable (); } -- 2.34.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2021-12-15 9:57 ` [PATCH] i386: simplify cpu_feature handling Martin Liška @ 2021-12-16 20:58 ` Stefan Kneifel 2021-12-17 8:50 ` Martin Liška 2022-01-03 11:43 ` Martin Liška 1 sibling, 1 reply; 14+ messages in thread From: Stefan Kneifel @ 2021-12-16 20:58 UTC (permalink / raw) To: Martin Liška, Jakub Jelinek; +Cc: gcc-patches Am 15.12.21 um 10:57 schrieb Martin Liška: > On 12/14/21 17:12, Jakub Jelinek wrote: >> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. > > Installed with that change, thanks. > > Moreover, I'm suggesting a simplification: > > The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR > that can be simplified with NOP_EXPR. > > Survives i386.exp tests, may I install the patch after testing or > is it a stage1 material? > > Thanks, > Martin The loops indeed seem to be unnecessary. For safety reasons: what would you think about throwing an ICE if (index >= SIZE_OF_CPU_FEATURES) ? This should not happen - however, a lot of things shouldn't happen... and it might facilitiate locating a potential bug at a later time. Regards, Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2021-12-16 20:58 ` Stefan Kneifel @ 2021-12-17 8:50 ` Martin Liška 0 siblings, 0 replies; 14+ messages in thread From: Martin Liška @ 2021-12-17 8:50 UTC (permalink / raw) To: Stefan Kneifel, Jakub Jelinek; +Cc: gcc-patches On 12/16/21 21:58, Stefan Kneifel wrote: > Am 15.12.21 um 10:57 schrieb Martin Liška: >> On 12/14/21 17:12, Jakub Jelinek wrote: >>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >> >> Installed with that change, thanks. >> >> Moreover, I'm suggesting a simplification: >> >> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >> that can be simplified with NOP_EXPR. >> >> Survives i386.exp tests, may I install the patch after testing or >> is it a stage1 material? >> >> Thanks, >> Martin > > The loops indeed seem to be unnecessary. > > For safety reasons: what would you think about throwing an ICE if (index >= SIZE_OF_CPU_FEATURES) ? > This should not happen - however, a lot of things shouldn't happen... and it might facilitiate locating a potential bug at a later time. Hello. Well, I see your point, but I don't think it's necessary as the macro is well defined. Note we have a ASAN and UBSAN bootstrap that would caught such an error. Cheers, Martin > > Regards, Stefan > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2021-12-15 9:57 ` [PATCH] i386: simplify cpu_feature handling Martin Liška 2021-12-16 20:58 ` Stefan Kneifel @ 2022-01-03 11:43 ` Martin Liška 2022-03-31 7:01 ` Martin Liška 1 sibling, 1 reply; 14+ messages in thread From: Martin Liška @ 2022-01-03 11:43 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Stefan Kneifel, gcc-patches PING: Jakub? On 12/15/21 10:57, Martin Liška wrote: > On 12/14/21 17:12, Jakub Jelinek wrote: >> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. > > Installed with that change, thanks. > > Moreover, I'm suggesting a simplification: > > The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR > that can be simplified with NOP_EXPR. > > Survives i386.exp tests, may I install the patch after testing or > is it a stage1 material? > > Thanks, > Martin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-01-03 11:43 ` Martin Liška @ 2022-03-31 7:01 ` Martin Liška 2022-05-02 7:57 ` Martin Liška 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2022-03-31 7:01 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Stefan Kneifel, gcc-patches @Jakub: May I install it once stage1 opens? Cheers, Martin On 1/3/22 12:43, Martin Liška wrote: > PING: Jakub? > > On 12/15/21 10:57, Martin Liška wrote: >> On 12/14/21 17:12, Jakub Jelinek wrote: >>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >> >> Installed with that change, thanks. >> >> Moreover, I'm suggesting a simplification: >> >> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >> that can be simplified with NOP_EXPR. >> >> Survives i386.exp tests, may I install the patch after testing or >> is it a stage1 material? >> >> Thanks, >> Martin > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-03-31 7:01 ` Martin Liška @ 2022-05-02 7:57 ` Martin Liška 2022-05-11 8:19 ` Martin Liška 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2022-05-02 7:57 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Stefan Kneifel, gcc-patches On 3/31/22 09:01, Martin Liška wrote: > @Jakub: May I install it once stage1 opens? May I please ping this? Thanks, Martin > > Cheers, > Martin > > On 1/3/22 12:43, Martin Liška wrote: >> PING: Jakub? >> >> On 12/15/21 10:57, Martin Liška wrote: >>> On 12/14/21 17:12, Jakub Jelinek wrote: >>>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >>> >>> Installed with that change, thanks. >>> >>> Moreover, I'm suggesting a simplification: >>> >>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >>> that can be simplified with NOP_EXPR. >>> >>> Survives i386.exp tests, may I install the patch after testing or >>> is it a stage1 material? >>> >>> Thanks, >>> Martin >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-05-02 7:57 ` Martin Liška @ 2022-05-11 8:19 ` Martin Liška 2022-05-11 8:27 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2022-05-11 8:19 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Stefan Kneifel, gcc-patches, Uros Bizjak On 5/2/22 09:57, Martin Liška wrote: > On 3/31/22 09:01, Martin Liška wrote: >> @Jakub: May I install it once stage1 opens? > > May I please ping this? > > Thanks, > Martin > >> >> Cheers, >> Martin >> >> On 1/3/22 12:43, Martin Liška wrote: >>> PING: Jakub? >>> >>> On 12/15/21 10:57, Martin Liška wrote: >>>> On 12/14/21 17:12, Jakub Jelinek wrote: >>>>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >>>> >>>> Installed with that change, thanks. >>>> >>>> Moreover, I'm suggesting a simplification: >>>> >>>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >>>> that can be simplified with NOP_EXPR. >>>> >>>> Survives i386.exp tests, may I install the patch after testing or >>>> is it a stage1 material? >>>> >>>> Thanks, >>>> Martin >>> >> > CCing Uros. May I install the patch? Martin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-05-11 8:19 ` Martin Liška @ 2022-05-11 8:27 ` Uros Bizjak 2022-05-11 8:50 ` Martin Liška 0 siblings, 1 reply; 14+ messages in thread From: Uros Bizjak @ 2022-05-11 8:27 UTC (permalink / raw) To: Martin Liška; +Cc: Jakub Jelinek, Stefan Kneifel, gcc-patches On Wed, May 11, 2022 at 10:19 AM Martin Liška <mliska@suse.cz> wrote: > > On 5/2/22 09:57, Martin Liška wrote: > > On 3/31/22 09:01, Martin Liška wrote: > >> @Jakub: May I install it once stage1 opens? > > > > May I please ping this? > > > > Thanks, > > Martin > > > >> > >> Cheers, > >> Martin > >> > >> On 1/3/22 12:43, Martin Liška wrote: > >>> PING: Jakub? > >>> > >>> On 12/15/21 10:57, Martin Liška wrote: > >>>> On 12/14/21 17:12, Jakub Jelinek wrote: > >>>>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. > >>>> > >>>> Installed with that change, thanks. > >>>> > >>>> Moreover, I'm suggesting a simplification: > >>>> > >>>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR > >>>> that can be simplified with NOP_EXPR. > >>>> > >>>> Survives i386.exp tests, may I install the patch after testing or > >>>> is it a stage1 material? > >>>> > >>>> Thanks, > >>>> Martin > >>> > >> > > > > CCing Uros. > > May I install the patch? Can you please attach the latest version of the patch? I only found the version from Dec. 15. 2021, which I'm not sure is the latest, Thanks, Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-05-11 8:27 ` Uros Bizjak @ 2022-05-11 8:50 ` Martin Liška 2022-05-11 8:57 ` Uros Bizjak 0 siblings, 1 reply; 14+ messages in thread From: Martin Liška @ 2022-05-11 8:50 UTC (permalink / raw) To: Uros Bizjak; +Cc: Jakub Jelinek, Stefan Kneifel, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1243 bytes --] On 5/11/22 10:27, Uros Bizjak wrote: > On Wed, May 11, 2022 at 10:19 AM Martin Liška <mliska@suse.cz> wrote: >> >> On 5/2/22 09:57, Martin Liška wrote: >>> On 3/31/22 09:01, Martin Liška wrote: >>>> @Jakub: May I install it once stage1 opens? >>> >>> May I please ping this? >>> >>> Thanks, >>> Martin >>> >>>> >>>> Cheers, >>>> Martin >>>> >>>> On 1/3/22 12:43, Martin Liška wrote: >>>>> PING: Jakub? >>>>> >>>>> On 12/15/21 10:57, Martin Liška wrote: >>>>>> On 12/14/21 17:12, Jakub Jelinek wrote: >>>>>>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. >>>>>> >>>>>> Installed with that change, thanks. >>>>>> >>>>>> Moreover, I'm suggesting a simplification: >>>>>> >>>>>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR >>>>>> that can be simplified with NOP_EXPR. >>>>>> >>>>>> Survives i386.exp tests, may I install the patch after testing or >>>>>> is it a stage1 material? >>>>>> >>>>>> Thanks, >>>>>> Martin >>>>> >>>> >>> >> >> CCing Uros. >> >> May I install the patch? > > Can you please attach the latest version of the patch? I only found > the version from Dec. 15. 2021, which I'm not sure is the latest, > > Thanks, > Uros. Sure, there's the rebased version of the patch. Thanks, Martin [-- Attachment #2: 0001-i386-simplify-cpu_feature-handling.patch --] [-- Type: text/x-patch, Size: 7221 bytes --] From 72aa0459f84baa845fef7f9f7f0d04edda410d87 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Wed, 15 Dec 2021 10:54:23 +0100 Subject: [PATCH] i386: simplify cpu_feature handling The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR that can be simplified with NOP_EXPR. gcc/ChangeLog: * common/config/i386/cpuinfo.h (has_cpu_feature): Directly compute index in cpu_features2. (set_cpu_feature): Likewise. * config/i386/i386-builtins.cc (fold_builtin_cpu): Also remove loop for cpu_features2 and use NOP_EXPRs. --- gcc/common/config/i386/cpuinfo.h | 50 +++++++++++--------- gcc/config/i386/i386-builtins.cc | 79 ++++++++++++++++---------------- 2 files changed, 67 insertions(+), 62 deletions(-) diff --git a/gcc/common/config/i386/cpuinfo.h b/gcc/common/config/i386/cpuinfo.h index 239759dc766..6d6171f4555 100644 --- a/gcc/common/config/i386/cpuinfo.h +++ b/gcc/common/config/i386/cpuinfo.h @@ -55,43 +55,49 @@ struct __processor_model2 static inline int has_cpu_feature (struct __processor_model *cpu_model, unsigned int *cpu_features2, - enum processor_features f) + enum processor_features feature) { - unsigned int i; + unsigned index, offset; + unsigned f = feature; + if (f < 32) { /* The first 32 features. */ - return cpu_model->__cpu_features[0] & (1U << (f & 31)); + return cpu_model->__cpu_features[0] & (1U << f); + } + else + { + /* The rest of features. cpu_features2[i] contains features from + (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ + f -= 32; + index = f / 32; + offset = f % 32; + return cpu_features2[index] & (1U << offset); } - /* The rest of features. cpu_features2[i] contains features from - (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ - for (i = 0; i < SIZE_OF_CPU_FEATURES; i++) - if (f < (32 + 32 + i * 32)) - return cpu_features2[i] & (1U << ((f - (32 + i * 32)) & 31)); - gcc_unreachable (); } static inline void set_cpu_feature (struct __processor_model *cpu_model, unsigned int *cpu_features2, - enum processor_features f) + enum processor_features feature) { - unsigned int i; + unsigned index, offset; + unsigned f = feature; + if (f < 32) { /* The first 32 features. */ - cpu_model->__cpu_features[0] |= (1U << (f & 31)); - return; + cpu_model->__cpu_features[0] |= (1U << f); + } + else + { + /* The rest of features. cpu_features2[i] contains features from + (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ + f -= 32; + index = f / 32; + offset = f % 32; + cpu_features2[index] |= (1U << offset); } - /* The rest of features. cpu_features2[i] contains features from - (32 + i * 32) to (31 + 32 + i * 32), inclusively. */ - for (i = 0; i < SIZE_OF_CPU_FEATURES; i++) - if (f < (32 + 32 + i * 32)) - { - cpu_features2[i] |= (1U << ((f - (32 + i * 32)) & 31)); - return; - } - gcc_unreachable (); } /* Get the specific type of AMD CPU and return AMD CPU name. Return diff --git a/gcc/config/i386/i386-builtins.cc b/gcc/config/i386/i386-builtins.cc index 8c6d0fe9631..59c7da25a14 100644 --- a/gcc/config/i386/i386-builtins.cc +++ b/gcc/config/i386/i386-builtins.cc @@ -2280,7 +2280,7 @@ fold_builtin_cpu (tree fndecl, tree *args) /* Check the value. */ final = build2 (EQ_EXPR, unsigned_type_node, ref, build_int_cstu (unsigned_type_node, field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); + return build1 (NOP_EXPR, integer_type_node, final); } else if (fn_code == IX86_BUILTIN_CPU_SUPPORTS) { @@ -2305,7 +2305,8 @@ fold_builtin_cpu (tree fndecl, tree *args) return integer_zero_node; } - if (isa_names_table[i].feature >= 32) + unsigned feature = isa_names_table[i].feature; + if (feature >= INT_TYPE_SIZE) { if (ix86_cpu_features2_var == nullptr) { @@ -2323,46 +2324,44 @@ fold_builtin_cpu (tree fndecl, tree *args) varpool_node::add (ix86_cpu_features2_var); } - for (unsigned int j = 0; j < SIZE_OF_CPU_FEATURES; j++) - if (isa_names_table[i].feature < (32 + 32 + j * 32)) - { - field_val = (1U << (isa_names_table[i].feature - - (32 + j * 32))); - tree index = size_int (j); - array_elt = build4 (ARRAY_REF, unsigned_type_node, - ix86_cpu_features2_var, - index, NULL_TREE, NULL_TREE); - /* Return __cpu_features2[index] & field_val */ - final = build2 (BIT_AND_EXPR, unsigned_type_node, - array_elt, - build_int_cstu (unsigned_type_node, - field_val)); - return build1 (CONVERT_EXPR, integer_type_node, final); - } + feature -= INT_TYPE_SIZE; + field_val = 1U << (feature % INT_TYPE_SIZE); + tree index = size_int (feature / INT_TYPE_SIZE); + array_elt = build4 (ARRAY_REF, unsigned_type_node, + ix86_cpu_features2_var, + index, NULL_TREE, NULL_TREE); + /* Return __cpu_features2[index] & field_val */ + final = build2 (BIT_AND_EXPR, unsigned_type_node, + array_elt, + build_int_cstu (unsigned_type_node, + field_val)); + return build1 (NOP_EXPR, integer_type_node, final); } - - field = TYPE_FIELDS (ix86_cpu_model_type_node); - /* Get the last field, which is __cpu_features. */ - while (DECL_CHAIN (field)) - field = DECL_CHAIN (field); - - /* Get the appropriate field: __cpu_model.__cpu_features */ - ref = build3 (COMPONENT_REF, TREE_TYPE (field), ix86_cpu_model_var, - field, NULL_TREE); - - /* Access the 0th element of __cpu_features array. */ - array_elt = build4 (ARRAY_REF, unsigned_type_node, ref, - integer_zero_node, NULL_TREE, NULL_TREE); - - field_val = (1U << isa_names_table[i].feature); - /* Return __cpu_model.__cpu_features[0] & field_val */ - final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, - build_int_cstu (unsigned_type_node, field_val)); - if (isa_names_table[i].feature == (INT_TYPE_SIZE - 1)) - return build2 (NE_EXPR, integer_type_node, final, - build_int_cst (unsigned_type_node, 0)); else - return build1 (CONVERT_EXPR, integer_type_node, final); + { + field = TYPE_FIELDS (ix86_cpu_model_type_node); + /* Get the last field, which is __cpu_features. */ + while (DECL_CHAIN (field)) + field = DECL_CHAIN (field); + + /* Get the appropriate field: __cpu_model.__cpu_features */ + ref = build3 (COMPONENT_REF, TREE_TYPE (field), ix86_cpu_model_var, + field, NULL_TREE); + + /* Access the 0th element of __cpu_features array. */ + array_elt = build4 (ARRAY_REF, unsigned_type_node, ref, + integer_zero_node, NULL_TREE, NULL_TREE); + + field_val = (1U << feature); + /* Return __cpu_model.__cpu_features[0] & field_val */ + final = build2 (BIT_AND_EXPR, unsigned_type_node, array_elt, + build_int_cstu (unsigned_type_node, field_val)); + if (feature == (INT_TYPE_SIZE - 1)) + return build2 (NE_EXPR, integer_type_node, final, + build_int_cst (unsigned_type_node, 0)); + else + return build1 (NOP_EXPR, integer_type_node, final); + } } gcc_unreachable (); } -- 2.36.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i386: simplify cpu_feature handling 2022-05-11 8:50 ` Martin Liška @ 2022-05-11 8:57 ` Uros Bizjak 0 siblings, 0 replies; 14+ messages in thread From: Uros Bizjak @ 2022-05-11 8:57 UTC (permalink / raw) To: Martin Liška; +Cc: Jakub Jelinek, Stefan Kneifel, gcc-patches On Wed, May 11, 2022 at 10:50 AM Martin Liška <mliska@suse.cz> wrote: > > On 5/11/22 10:27, Uros Bizjak wrote: > > On Wed, May 11, 2022 at 10:19 AM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 5/2/22 09:57, Martin Liška wrote: > >>> On 3/31/22 09:01, Martin Liška wrote: > >>>> @Jakub: May I install it once stage1 opens? > >>> > >>> May I please ping this? > >>> > >>> Thanks, > >>> Martin > >>> > >>>> > >>>> Cheers, > >>>> Martin > >>>> > >>>> On 1/3/22 12:43, Martin Liška wrote: > >>>>> PING: Jakub? > >>>>> > >>>>> On 12/15/21 10:57, Martin Liška wrote: > >>>>>> On 12/14/21 17:12, Jakub Jelinek wrote: > >>>>>>> I'd use INT_TYPE_SIZE - 1 instead of 31. Otherwise LGTM. > >>>>>> > >>>>>> Installed with that change, thanks. > >>>>>> > >>>>>> Moreover, I'm suggesting a simplification: > >>>>>> > >>>>>> The patch removes unneeded loops for cpu_features2 and CONVERT_EXPR > >>>>>> that can be simplified with NOP_EXPR. > >>>>>> > >>>>>> Survives i386.exp tests, may I install the patch after testing or > >>>>>> is it a stage1 material? > >>>>>> > >>>>>> Thanks, > >>>>>> Martin > >>>>> > >>>> > >>> > >> > >> CCing Uros. > >> > >> May I install the patch? > > > > Can you please attach the latest version of the patch? I only found > > the version from Dec. 15. 2021, which I'm not sure is the latest, > > > > Thanks, > > Uros. > > Sure, there's the rebased version of the patch. LGTM. Thanks, Uros. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-05-11 8:57 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-14 9:55 [PATCH] i386: Fix emissing of __builtin_cpu_supports Martin Liška 2021-12-14 10:28 ` Jakub Jelinek 2021-12-14 15:07 ` Martin Liška 2021-12-14 16:12 ` Jakub Jelinek 2021-12-15 9:57 ` [PATCH] i386: simplify cpu_feature handling Martin Liška 2021-12-16 20:58 ` Stefan Kneifel 2021-12-17 8:50 ` Martin Liška 2022-01-03 11:43 ` Martin Liška 2022-03-31 7:01 ` Martin Liška 2022-05-02 7:57 ` Martin Liška 2022-05-11 8:19 ` Martin Liška 2022-05-11 8:27 ` Uros Bizjak 2022-05-11 8:50 ` Martin Liška 2022-05-11 8:57 ` Uros Bizjak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).