Re: [RFC] Fix recent popcount change is breaking

2018-11-23 Thread Bin.Cheng
On Sat, Jul 28, 2018 at 7:36 AM Kugan Vivekanandarajah
 wrote:
>
> Hi,
>
> On 28 July 2018 at 01:13, Richard Biener  wrote:
> > On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška"  
> > 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
> >


Re: [RFC] Fix recent popcount change is breaking

2018-07-27 Thread Kugan Vivekanandarajah
Hi,

On 28 July 2018 at 01:13, Richard Biener  wrote:
> On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška"  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
>


Re: [RFC] Fix recent popcount change is breaking

2018-07-27 Thread Richard Biener
On July 27, 2018 3:33:59 PM GMT+02:00, "Martin Liška"  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



Re: [RFC] Fix recent popcount change is breaking

2018-07-27 Thread Martin Liška
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


Re: [RFC] Fix recent popcount change is breaking

2018-07-11 Thread Richard Biener
On Wed, Jul 11, 2018 at 1:26 PM Kugan Vivekanandarajah
 wrote:
>
> Hi Andrew,
>
> On 11 July 2018 at 15:43, Andrew Pinski  wrote:
> > On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah
> >  wrote:
> >>
> >> Hi Andrew,
> >>
> >> On 11 July 2018 at 11:19, Andrew Pinski  wrote:
> >> > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah
> >> >  wrote:
> >> >>
> >> >> On 10 July 2018 at 23:17, Richard Biener  
> >> >> wrote:
> >> >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
> >> >> >  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  
> >> >> >>
> >> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
> >> >> >> if libfunc for popcount is available.


Re: [RFC] Fix recent popcount change is breaking

2018-07-11 Thread Kugan Vivekanandarajah
Hi Andrew,

On 11 July 2018 at 15:43, Andrew Pinski  wrote:
> On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah
>  wrote:
>>
>> Hi Andrew,
>>
>> On 11 July 2018 at 11:19, Andrew Pinski  wrote:
>> > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah
>> >  wrote:
>> >>
>> >> On 10 July 2018 at 23:17, Richard Biener  
>> >> wrote:
>> >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
>> >> >  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  
>> >> >>
>> >> >> * 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" } } */


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Andrew Pinski
On Tue, Jul 10, 2018 at 6:35 PM Kugan Vivekanandarajah
 wrote:
>
> Hi Andrew,
>
> On 11 July 2018 at 11:19, Andrew Pinski  wrote:
> > On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah
> >  wrote:
> >>
> >> On 10 July 2018 at 23:17, Richard Biener  
> >> wrote:
> >> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
> >> >  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  
> >> >>
> >> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
> >> >> if libfunc for popcount is available.


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Kugan Vivekanandarajah
Hi Andrew,

On 11 July 2018 at 11:19, Andrew Pinski  wrote:
> On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah
>  wrote:
>>
>> On 10 July 2018 at 23:17, Richard Biener  wrote:
>> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
>> >  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  
>> >>
>> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
>> >> if libfunc for popcount is available.


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Andrew Pinski
On Tue, Jul 10, 2018 at 6:14 PM Kugan Vivekanandarajah
 wrote:
>
> On 10 July 2018 at 23:17, Richard Biener  wrote:
> > On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
> >  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  
> >>
> >> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
> >> if libfunc for popcount is available.


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Kugan Vivekanandarajah
On 10 July 2018 at 23:17, Richard Biener  wrote:
> On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
>  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  
>>
>> * 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;


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Jeff Law
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


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Jeff Law
On 07/10/2018 07:17 AM, Richard Biener wrote:
> On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
>  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


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Richard Biener
On Tue, Jul 10, 2018 at 3:27 PM 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?

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


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Jakub Jelinek
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


Re: [RFC] Fix recent popcount change is breaking

2018-07-10 Thread Richard Biener
On Tue, Jul 10, 2018 at 3:06 PM Kugan Vivekanandarajah
 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  
>
> * tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
> if libfunc for popcount is available.


[RFC] Fix recent popcount change is breaking

2018-07-10 Thread Kugan Vivekanandarajah
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  

* tree-ssa-loop-niter.c (number_of_iterations_popcount): Check
if libfunc for popcount is available.
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;