| Message ID | CAELXzTOijCYcUUuhUjvsXQMDahdYRx4RhHWGjeAAz5FiHRvW+A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] Fix recent popcount change is breaking | expand |
On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote: > > Hi, > > Jeff told me that the recent popcount built-in detection is causing > kernel build issues as > ERROR: "__popcountsi2" > [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > > I could also reproduce this. AFIK, we should check if the libfunc is > defined while checking popcount? > > I am testing the attached RFC patch. Is this reasonable? It doesn't work that way, all targets have this libfunc in libgcc. This means the kernel has to provide it. The only thing you could do is restrict replacement of CALL_EXPRs (in SCEV cprop) to those the target natively supports. Richard. > Thanks, > Kugan > > gcc/ChangeLog: > > 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> > > * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check > if libfunc for popcount is available.
On Tue, Jul 10, 2018 at 03:17:48PM +0200, Richard Biener wrote: > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > > Jeff told me that the recent popcount built-in detection is causing > > kernel build issues as > > ERROR: "__popcountsi2" > > [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > > > > I could also reproduce this. AFIK, we should check if the libfunc is > > defined while checking popcount? > > > > I am testing the attached RFC patch. Is this reasonable? > > It doesn't work that way, all targets have this libfunc in libgcc. This means > the kernel has to provide it. The only thing you could do is restrict > replacement of CALL_EXPRs (in SCEV cprop) to those the target > natively supports. Yeah, that is what we've done in the past in other cases, e.g. PR82981 is somewhat similar. So perhaps just check the optab and use it only if it is supported? Jakub
On Tue, Jul 10, 2018 at 3:27 PM Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Jul 10, 2018 at 03:17:48PM +0200, Richard Biener wrote: > > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > > > Jeff told me that the recent popcount built-in detection is causing > > > kernel build issues as > > > ERROR: "__popcountsi2" > > > [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > > > > > > I could also reproduce this. AFIK, we should check if the libfunc is > > > defined while checking popcount? > > > > > > I am testing the attached RFC patch. Is this reasonable? > > > > It doesn't work that way, all targets have this libfunc in libgcc. This means > > the kernel has to provide it. The only thing you could do is restrict > > replacement of CALL_EXPRs (in SCEV cprop) to those the target > > natively supports. > > Yeah, that is what we've done in the past in other cases, e.g. PR82981 > is somewhat similar. So perhaps just check the optab and use it only if it > is supported? Btw, given that we do not want to fail niter analysis we'd have to audit all places that eventually code-generate it which isn't only SCEV-cprop ... So not sure if it isn't better to user-control this in a way not depending on target HW support... Richard. > > Jakub
On 07/10/2018 07:17 AM, Richard Biener wrote: > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > <kugan.vivekanandarajah@linaro.org> wrote: >> >> Hi, >> >> Jeff told me that the recent popcount built-in detection is causing >> kernel build issues as >> ERROR: "__popcountsi2" >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! >> >> I could also reproduce this. AFIK, we should check if the libfunc is >> defined while checking popcount? >> >> I am testing the attached RFC patch. Is this reasonable? > > It doesn't work that way, all targets have this libfunc in libgcc. This means > the kernel has to provide it. The only thing you could do is restrict > replacement of CALL_EXPRs (in SCEV cprop) to those the target > natively supports. I can certainly live with that, but I think we should reach out to the kernel developers to proactively make them aware of the requirement to provide popcount. Jeff
On 07/10/2018 07:27 AM, Jakub Jelinek wrote: > On Tue, Jul 10, 2018 at 03:17:48PM +0200, Richard Biener wrote: >> On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah >>> Jeff told me that the recent popcount built-in detection is causing >>> kernel build issues as >>> ERROR: "__popcountsi2" >>> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! >>> >>> I could also reproduce this. AFIK, we should check if the libfunc is >>> defined while checking popcount? >>> >>> I am testing the attached RFC patch. Is this reasonable? >> >> It doesn't work that way, all targets have this libfunc in libgcc. This means >> the kernel has to provide it. The only thing you could do is restrict >> replacement of CALL_EXPRs (in SCEV cprop) to those the target >> natively supports. > > Yeah, that is what we've done in the past in other cases, e.g. PR82981 > is somewhat similar. So perhaps just check the optab and use it only if it > is supported? And I could live with this too. Essentially I'm just looking to get the issue raised and addressed now rather than waiting for stage3/stage4 :-) jeff
On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > <kugan.vivekanandarajah@linaro.org> wrote: >> >> Hi, >> >> Jeff told me that the recent popcount built-in detection is causing >> kernel build issues as >> ERROR: "__popcountsi2" >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! >> >> I could also reproduce this. AFIK, we should check if the libfunc is >> defined while checking popcount? >> >> I am testing the attached RFC patch. Is this reasonable? > > It doesn't work that way, all targets have this libfunc in libgcc. This means > the kernel has to provide it. The only thing you could do is restrict > replacement of CALL_EXPRs (in SCEV cprop) to those the target > natively supports. How about restricting it in expression_expensive_p ? Is that what you wanted. Attached patch does this. Bootstrap and regression testing progressing. Thanks, Kugan > > Richard. > >> Thanks, >> Kugan >> >> gcc/ChangeLog: >> >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check >> if libfunc for popcount is available. diff --git a/gcc/testsuite/gcc.target/aarch64/popcount4.c b/gcc/testsuite/gcc.target/aarch64/popcount4.c index e69de29..ee55b2e 100644 --- a/gcc/testsuite/gcc.target/aarch64/popcount4.c +++ b/gcc/testsuite/gcc.target/aarch64/popcount4.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized -mgeneral-regs-only" } */ + +int PopCount (long b) { + int c = 0; + + while (b) { + b &= b - 1; + c++; + } + return c; +} + +/* { dg-final { scan-tree-dump-times "__builtin_popcount" 0 "optimized" } } */ diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c index 69122f2..ec8e4ec 100644 --- a/gcc/tree-scalar-evolution.c +++ b/gcc/tree-scalar-evolution.c @@ -257,7 +257,9 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "backend.h" +#include "target.h" #include "rtl.h" +#include "optabs-query.h" #include "tree.h" #include "gimple.h" #include "ssa.h" @@ -282,6 +284,7 @@ along with GCC; see the file COPYING3. If not see #include "gimple-fold.h" #include "tree-into-ssa.h" #include "builtins.h" +#include "case-cfn-macros.h" static tree analyze_scalar_evolution_1 (struct loop *, tree); static tree analyze_scalar_evolution_for_address_of (struct loop *loop, @@ -3500,6 +3503,23 @@ expression_expensive_p (tree expr) { tree arg; call_expr_arg_iterator iter; + tree fndecl = get_callee_fndecl (expr); + + if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) + { + combined_fn cfn = as_combined_fn (DECL_FUNCTION_CODE (fndecl)); + switch (cfn) + { + CASE_CFN_POPCOUNT: + /* Check if opcode for popcount is available. */ + if (optab_handler (popcount_optab, + TYPE_MODE (TREE_TYPE (CALL_EXPR_ARG (expr, 0)))) + == CODE_FOR_nothing) + return true; + default: + break; + } + } if (!is_inexpensive_builtin (get_callee_fndecl (expr))) return true;
On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote: > > On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: > > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > > <kugan.vivekanandarajah@linaro.org> wrote: > >> > >> Hi, > >> > >> Jeff told me that the recent popcount built-in detection is causing > >> kernel build issues as > >> ERROR: "__popcountsi2" > >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > >> > >> I could also reproduce this. AFIK, we should check if the libfunc is > >> defined while checking popcount? > >> > >> I am testing the attached RFC patch. Is this reasonable? > > > > It doesn't work that way, all targets have this libfunc in libgcc. This means > > the kernel has to provide it. The only thing you could do is restrict > > replacement of CALL_EXPRs (in SCEV cprop) to those the target > > natively supports. > > How about restricting it in expression_expensive_p ? Is that what you > wanted. Attached patch does this. > Bootstrap and regression testing progressing. Seems like that should go into is_inexpensive_builtin instead which is just tested right below. Thanks, Andrew > > Thanks, > Kugan > > > > > Richard. > > > >> Thanks, > >> Kugan > >> > >> gcc/ChangeLog: > >> > >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> > >> > >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check > >> if libfunc for popcount is available.
Hi Andrew, On 11 July 2018 at 11:19, Andrew Pinski <pinskia@gmail.com> wrote: > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah > <kugan.vivekanandarajah@linaro.org> wrote: >> >> On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah >> > <kugan.vivekanandarajah@linaro.org> wrote: >> >> >> >> Hi, >> >> >> >> Jeff told me that the recent popcount built-in detection is causing >> >> kernel build issues as >> >> ERROR: "__popcountsi2" >> >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! >> >> >> >> I could also reproduce this. AFIK, we should check if the libfunc is >> >> defined while checking popcount? >> >> >> >> I am testing the attached RFC patch. Is this reasonable? >> > >> > It doesn't work that way, all targets have this libfunc in libgcc. This means >> > the kernel has to provide it. The only thing you could do is restrict >> > replacement of CALL_EXPRs (in SCEV cprop) to those the target >> > natively supports. >> >> How about restricting it in expression_expensive_p ? Is that what you >> wanted. Attached patch does this. >> Bootstrap and regression testing progressing. > > Seems like that should go into is_inexpensive_builtin instead which > is just tested right below. I hought about that. is_inexpensive_builtin is used in various other places including some inlining decision so wasn't sure if it is the right thing. Happy to change it if that is the right thing to do. Thanks, Kugan > > Thanks, > Andrew > >> >> Thanks, >> Kugan >> >> > >> > Richard. >> > >> >> Thanks, >> >> Kugan >> >> >> >> gcc/ChangeLog: >> >> >> >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check >> >> if libfunc for popcount is available.
On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote: > > Hi Andrew, > > On 11 July 2018 at 11:19, Andrew Pinski <pinskia@gmail.com> wrote: > > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah > > <kugan.vivekanandarajah@linaro.org> wrote: > >> > >> On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: > >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > >> > <kugan.vivekanandarajah@linaro.org> wrote: > >> >> > >> >> Hi, > >> >> > >> >> Jeff told me that the recent popcount built-in detection is causing > >> >> kernel build issues as > >> >> ERROR: "__popcountsi2" > >> >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > >> >> > >> >> I could also reproduce this. AFIK, we should check if the libfunc is > >> >> defined while checking popcount? > >> >> > >> >> I am testing the attached RFC patch. Is this reasonable? > >> > > >> > It doesn't work that way, all targets have this libfunc in libgcc. This means > >> > the kernel has to provide it. The only thing you could do is restrict > >> > replacement of CALL_EXPRs (in SCEV cprop) to those the target > >> > natively supports. > >> > >> How about restricting it in expression_expensive_p ? Is that what you > >> wanted. Attached patch does this. > >> Bootstrap and regression testing progressing. > > > > Seems like that should go into is_inexpensive_builtin instead which > > is just tested right below. > > I hought about that. is_inexpensive_builtin is used in various other > places including some inlining decision so wasn't sure if it is the > right thing. Happy to change it if that is the right thing to do. I audited all of the users (and their users if it is used in a wrapper) and found that is_inexpensive_builtin should return false for this builtin if it is a function call in the end; there are other builtins which should be checked the similar way but I think we should not going to force you to do the similar thing for those builtins. Thanks, Andrew > > Thanks, > Kugan > > > > Thanks, > > Andrew > > > >> > >> Thanks, > >> Kugan > >> > >> > > >> > Richard. > >> > > >> >> Thanks, > >> >> Kugan > >> >> > >> >> gcc/ChangeLog: > >> >> > >> >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> > >> >> > >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check > >> >> if libfunc for popcount is available.
Hi Andrew, On 11 July 2018 at 15:43, Andrew Pinski <pinskia@gmail.com> wrote: > On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah > <kugan.vivekanandarajah@linaro.org> wrote: >> >> Hi Andrew, >> >> On 11 July 2018 at 11:19, Andrew Pinski <pinskia@gmail.com> wrote: >> > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah >> > <kugan.vivekanandarajah@linaro.org> wrote: >> >> >> >> On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: >> >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah >> >> > <kugan.vivekanandarajah@linaro.org> wrote: >> >> >> >> >> >> Hi, >> >> >> >> >> >> Jeff told me that the recent popcount built-in detection is causing >> >> >> kernel build issues as >> >> >> ERROR: "__popcountsi2" >> >> >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! >> >> >> >> >> >> I could also reproduce this. AFIK, we should check if the libfunc is >> >> >> defined while checking popcount? >> >> >> >> >> >> I am testing the attached RFC patch. Is this reasonable? >> >> > >> >> > It doesn't work that way, all targets have this libfunc in libgcc. This means >> >> > the kernel has to provide it. The only thing you could do is restrict >> >> > replacement of CALL_EXPRs (in SCEV cprop) to those the target >> >> > natively supports. >> >> >> >> How about restricting it in expression_expensive_p ? Is that what you >> >> wanted. Attached patch does this. >> >> Bootstrap and regression testing progressing. >> > >> > Seems like that should go into is_inexpensive_builtin instead which >> > is just tested right below. >> >> I hought about that. is_inexpensive_builtin is used in various other >> places including some inlining decision so wasn't sure if it is the >> right thing. Happy to change it if that is the right thing to do. > > I audited all of the users (and their users if it is used in a > wrapper) and found that is_inexpensive_builtin should return false for > this builtin if it is a function call in the end; there are other > builtins which should be checked the similar way but I think we should > not going to force you to do the similar thing for those builtins. Attached patch does this. Testing is progressing. Is This OK if no regression. Thanks, Kugan > > Thanks, > Andrew > >> >> Thanks, >> Kugan >> > >> > Thanks, >> > Andrew >> > >> >> >> >> Thanks, >> >> Kugan >> >> >> >> > >> >> > Richard. >> >> > >> >> >> Thanks, >> >> >> Kugan >> >> >> >> >> >> gcc/ChangeLog: >> >> >> >> >> >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> >> >> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check >> >> >> if libfunc for popcount is available. diff --git a/gcc/builtins.c b/gcc/builtins.c index 820d6c2..59cf567 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -10619,6 +10619,18 @@ is_inexpensive_builtin (tree decl) else if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL) switch (DECL_FUNCTION_CODE (decl)) { + case BUILT_IN_POPCOUNT: + case BUILT_IN_POPCOUNTL: + case BUILT_IN_POPCOUNTLL: + { + tree arg = TYPE_ARG_TYPES (TREE_TYPE (decl)); + /* Check if opcode for popcount is available. */ + if (optab_handler (popcount_optab, + TYPE_MODE (TREE_VALUE (arg))) + == CODE_FOR_nothing) + return false; + } + return true; case BUILT_IN_ABS: CASE_BUILT_IN_ALLOCA: case BUILT_IN_BSWAP16: @@ -10670,10 +10682,7 @@ is_inexpensive_builtin (tree decl) case BUILT_IN_VA_COPY: case BUILT_IN_TRAP: case BUILT_IN_SAVEREGS: - case BUILT_IN_POPCOUNTL: - case BUILT_IN_POPCOUNTLL: case BUILT_IN_POPCOUNTIMAX: - case BUILT_IN_POPCOUNT: case BUILT_IN_PARITYL: case BUILT_IN_PARITYLL: case BUILT_IN_PARITYIMAX: diff --git a/gcc/testsuite/gcc.target/aarch64/popcount4.c b/gcc/testsuite/gcc.target/aarch64/popcount4.c index e69de29..ee55b2e 100644 --- a/gcc/testsuite/gcc.target/aarch64/popcount4.c +++ b/gcc/testsuite/gcc.target/aarch64/popcount4.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized -mgeneral-regs-only" } */ + +int PopCount (long b) { + int c = 0; + + while (b) { + b &= b - 1; + c++; + } + return c; +} + +/* { dg-final { scan-tree-dump-times "__builtin_popcount" 0 "optimized" } } */
On Wed, Jul 11, 2018 at 1:26 PM Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote: > > Hi Andrew, > > On 11 July 2018 at 15:43, Andrew Pinski <pinskia@gmail.com> wrote: > > On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah > > <kugan.vivekanandarajah@linaro.org> wrote: > >> > >> Hi Andrew, > >> > >> On 11 July 2018 at 11:19, Andrew Pinski <pinskia@gmail.com> wrote: > >> > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah > >> > <kugan.vivekanandarajah@linaro.org> wrote: > >> >> > >> >> On 10 July 2018 at 23:17, Richard Biener <richard.guenther@gmail.com> wrote: > >> >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah > >> >> > <kugan.vivekanandarajah@linaro.org> wrote: > >> >> >> > >> >> >> Hi, > >> >> >> > >> >> >> Jeff told me that the recent popcount built-in detection is causing > >> >> >> kernel build issues as > >> >> >> ERROR: "__popcountsi2" > >> >> >> [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! > >> >> >> > >> >> >> I could also reproduce this. AFIK, we should check if the libfunc is > >> >> >> defined while checking popcount? > >> >> >> > >> >> >> I am testing the attached RFC patch. Is this reasonable? > >> >> > > >> >> > It doesn't work that way, all targets have this libfunc in libgcc. This means > >> >> > the kernel has to provide it. The only thing you could do is restrict > >> >> > replacement of CALL_EXPRs (in SCEV cprop) to those the target > >> >> > natively supports. > >> >> > >> >> How about restricting it in expression_expensive_p ? Is that what you > >> >> wanted. Attached patch does this. > >> >> Bootstrap and regression testing progressing. > >> > > >> > Seems like that should go into is_inexpensive_builtin instead which > >> > is just tested right below. > >> > >> I hought about that. is_inexpensive_builtin is used in various other > >> places including some inlining decision so wasn't sure if it is the > >> right thing. Happy to change it if that is the right thing to do. > > > > I audited all of the users (and their users if it is used in a > > wrapper) and found that is_inexpensive_builtin should return false for > > this builtin if it is a function call in the end; there are other > > builtins which should be checked the similar way but I think we should > > not going to force you to do the similar thing for those builtins. > > Attached patch does this. Testing is progressing. Is This OK if no regression. As said this isn't a complete fix given others may code-generate expressions with niter, for example vectorization. Also the table-based popcount implementation in libgcc is probably faster and the popcount call at least smaller than an open-coded variant. So I'm not sure if this is an appropriate fix. Why not simply make popcountdi available in the kernel? They do have implementations for other libgcc functions IIRC. Richard. > Thanks, > Kugan > > > > > > Thanks, > > Andrew > > > >> > >> Thanks, > >> Kugan > >> > > >> > Thanks, > >> > Andrew > >> > > >> >> > >> >> Thanks, > >> >> Kugan > >> >> > >> >> > > >> >> > Richard. > >> >> > > >> >> >> Thanks, > >> >> >> Kugan > >> >> >> > >> >> >> gcc/ChangeLog: > >> >> >> > >> >> >> 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> > >> >> >> > >> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check > >> >> >> if libfunc for popcount is available.
On 07/11/2018 02:31 PM, Richard Biener wrote: > Why not simply make popcountdi available in the kernel? They do have > implementations for other libgcc functions IIRC. Can you please Kugan create Linux kernel bug for that? So that discussion can happen? Thanks, Martin
On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška" <mliska@suse.cz> wrote: >On 07/11/2018 02:31 PM, Richard Biener wrote: >> Why not simply make popcountdi available in the kernel? They do have >> implementations for other libgcc functions IIRC. > >Can you please Kugan create Linux kernel bug for that? So that >discussion >can happen? There's no discussion necessary, libgcc is the core compiler runtime. If you choose not to use it you have to provide your own implementation. Richard. >Thanks, >Martin
Hi, On 28 July 2018 at 01:13, Richard Biener <richard.guenther@gmail.com> wrote: > On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška" <mliska@suse.cz> wrote: >>On 07/11/2018 02:31 PM, Richard Biener wrote: >>> Why not simply make popcountdi available in the kernel? They do have >>> implementations for other libgcc functions IIRC. >> >>Can you please Kugan create Linux kernel bug for that? So that >>discussion >>can happen? > > There's no discussion necessary, libgcc is the core compiler runtime. If you choose not to use it you have to provide your own implementation. Created a bug against kernel at https://bugzilla.kernel.org/show_bug.cgi?id=200671 Thanks, Kugan > > Richard. > >>Thanks, >>Martin >
On Sat, Jul 28, 2018 at 7:36 AM Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> wrote: > > Hi, > > On 28 July 2018 at 01:13, Richard Biener <richard.guenther@gmail.com> wrote: > > On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška" <mliska@suse.cz> wrote: > >>On 07/11/2018 02:31 PM, Richard Biener wrote: > >>> Why not simply make popcountdi available in the kernel? They do have > >>> implementations for other libgcc functions IIRC. > >> > >>Can you please Kugan create Linux kernel bug for that? So that > >>discussion > >>can happen? > > > > There's no discussion necessary, libgcc is the core compiler runtime. If you choose not to use it you have to provide your own implementation. > > Created a bug against kernel at > https://bugzilla.kernel.org/show_bug.cgi?id=200671 Looks like it's concerned whether GCC would generate more inefficient code now. If that's true, it might be better to disable this somehow for some cases? otherwise, could someone reason about it in that kernel bug in order to push forward a fix in kernel please? Thanks, bin > > Thanks, > Kugan > > > > Richard. > > > >>Thanks, > >>Martin > >
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c index f6fa2f7..2e2b9c6 100644 --- a/gcc/tree-ssa-loop-niter.c +++ b/gcc/tree-ssa-loop-niter.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "backend.h" +#include "target.h" #include "rtl.h" #include "tree.h" #include "gimple.h" @@ -42,6 +43,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-chrec.h" #include "tree-scalar-evolution.h" #include "params.h" +#include "optabs-libfuncs.h" #include "tree-dfa.h" @@ -2570,6 +2572,10 @@ number_of_iterations_popcount (loop_p loop, edge exit, (long_long_integer_type_node)) fn = builtin_decl_implicit (BUILT_IN_POPCOUNTLL); + /* Check if libfunc for popcount is available. */ + if (!optab_libfunc (popcount_optab, + TYPE_MODE (TREE_TYPE (src)))) + return false; /* ??? Support promoting char/short to int. */ if (!fn) return false;
Hi, Jeff told me that the recent popcount built-in detection is causing kernel build issues as ERROR: "__popcountsi2" [drivers/net/wireless/broadcom/brcm80211/brcmfmac/brcmfmac.ko] undefined! I could also reproduce this. AFIK, we should check if the libfunc is defined while checking popcount? I am testing the attached RFC patch. Is this reasonable? Thanks, Kugan gcc/ChangeLog: 2018-07-10 Kugan Vivekanandarajah <kuganv@linaro.org> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check if libfunc for popcount is available.