[{"id":1776575,"web_url":"http://patchwork.ozlabs.org/comment/1776575/","msgid":"<20170927195043.GA15193@ravnborg.org>","list_archive_url":null,"date":"2017-09-27T19:50:43","subject":"Re: [PATCH v2 1/2] sparc64: Define SPARC default fls and __fls","submitter":{"id":1009,"url":"http://patchwork.ozlabs.org/api/people/1009/","name":"Sam Ravnborg","email":"sam@ravnborg.org"},"content":"Hi Vijay.\n\nSome feedback - see below.\nThe comment about ENTRY() ENDPROC() is also valid for patch 2/2\n\n\tSam\n\n> \n> diff --git a/arch/sparc/include/asm/bitops_64.h b/arch/sparc/include/asm/bitops_64.h\n> index 2d52240..946c236 100644\n> --- a/arch/sparc/include/asm/bitops_64.h\n> +++ b/arch/sparc/include/asm/bitops_64.h\n> @@ -22,11 +22,12 @@\n>  void clear_bit(unsigned long nr, volatile unsigned long *addr);\n>  void change_bit(unsigned long nr, volatile unsigned long *addr);\n>  \n> +#define fls64(word)  (((word)?(__fls(word) + 1):0))\nThis macro could result in unwanted sideeffects.\nIf I use:\n\n\tfls64(i++)\n\nfor some obscure reason, then i will be incremented twice if i != 0.\nUsing the asm-generic version would be better.\n\n> +int fls(unsigned int word);\n> +int __fls(unsigned long word);\n> +\n>  #include <asm-generic/bitops/non-atomic.h>\n>  \n> -#include <asm-generic/bitops/fls.h>\n> -#include <asm-generic/bitops/__fls.h>\n> -#include <asm-generic/bitops/fls64.h>\n>  \n>  #ifdef __KERNEL__\n>  \n> diff --git a/arch/sparc/lib/Makefile b/arch/sparc/lib/Makefile\n> index 07c03e7..eefbb9c 100644\n> --- a/arch/sparc/lib/Makefile\n> +++ b/arch/sparc/lib/Makefile\n> @@ -16,6 +16,7 @@ lib-$(CONFIG_SPARC64) += atomic_64.o\n>  lib-$(CONFIG_SPARC32) += lshrdi3.o ashldi3.o\n>  lib-$(CONFIG_SPARC32) += muldi3.o bitext.o cmpdi2.o\n>  lib-$(CONFIG_SPARC64) += multi3.o\n> +lib-$(CONFIG_SPARC64) += fls.o\n>  \n>  lib-$(CONFIG_SPARC64) += copy_page.o clear_page.o bzero.o\n>  lib-$(CONFIG_SPARC64) += csum_copy.o csum_copy_from_user.o csum_copy_to_user.o\n> diff --git a/arch/sparc/lib/fls.S b/arch/sparc/lib/fls.S\n> new file mode 100644\n> index 0000000..a19bff2\n> --- /dev/null\n> +++ b/arch/sparc/lib/fls.S\n> @@ -0,0 +1,126 @@\n> +/* fls.S: SPARC default fls and __fls definitions.\n> + *\n> + * SPARC default fls and __fls definitions, which follows the same\n> + * algorithm as in generic fls() and __fls(). These functions will\n> + * be boot time patched on T4 and onward.\n> + */\n> +\n> +#include <asm/bitsperlong.h>\n> +#include <asm/export.h>\n> +\n> +\t.text\n> +\t.align\t32\n> +\n> +\t.global\tfls, __fls\n> +\t.type\tfls,\t#function\n> +\t.type\t__fls,\t#function\n> +\n> +\t.register\t%g2, #scratch\n> +\t.register\t%g3, #scratch\n> +\n> +EXPORT_SYMBOL(__fls)\n> +EXPORT_SYMBOL(fls)\n> +\n> +fls:\nUse ENTRY(), ENDPROC() for assembler functions.\n> +\tbrz,pn\t%o0, 6f\n> +\t mov\t0, %o1\n> +\tsethi\t%hi(0xffff0000), %g3\n> +\tmov\t%o0, %g2\n> +\tandcc\t%o0, %g3, %g0\n> +\tbe,pt\t%icc, 8f\n> +\t mov\t32, %o1\n> +\tsethi\t%hi(0xff000000), %g3\n> +\tandcc\t%g2, %g3, %g0\n> +\tbne,pt\t%icc, 3f\n> +\t sethi\t%hi(0xf0000000), %g3\n> +\tsll\t%o0, 8, %o0\n> +1:\n> +\tadd\t%o1, -8, %o1\n> +\tsra\t%o0, 0, %o0\n> +\tmov\t%o0, %g2\n> +2:\n> +\tsethi\t%hi(0xf0000000), %g3\n> +3:\n> +\tandcc\t%g2, %g3, %g0\n> +\tbne,pt\t%icc, 4f\n> +\t sethi\t%hi(0xc0000000), %g3\n> +\tsll\t%o0, 4, %o0\n> +\tadd\t%o1, -4, %o1\n> +\tsra\t%o0, 0, %o0\n> +\tmov\t%o0, %g2\n> +4:\n> +\tandcc\t%g2, %g3, %g0\n> +\tbe,a,pt\t%icc, 7f\n> +\t sll\t%o0, 2, %o0\n> +5:\n> +\txnor\t%g0, %o0, %o0\n> +\tsrl\t%o0, 31, %o0\n> +\tsub\t%o1, %o0, %o1\n> +6:\n> +\tjmp\t%o7 + 8\n> +\t sra\t%o1, 0, %o0\n> +7:\n> +\tadd\t%o1, -2, %o1\n> +\tba,pt\t%xcc, 5b\n> +\t sra\t%o0, 0, %o0\n> +8:\n> +\tsll\t%o0, 16, %o0\n> +\tsethi\t%hi(0xff000000), %g3\n> +\tsra\t%o0, 0, %o0\n> +\tmov\t%o0, %g2\n> +\tandcc\t%g2, %g3, %g0\n> +\tbne,pt\t%icc, 2b\n> +\t mov\t16, %o1\n> +\tba,pt\t%xcc, 1b\n> +\t sll\t%o0, 8, %o0\n> +\t.size\tfls, .-fls\n> +\n> +__fls:\nSame here, use ENTRY(), ENDPROC()\n> +#if BITS_PER_LONG == 64\n> +\tmov\t-1, %g2\n> +\tsllx\t%g2, 32, %g2\n> +\tand\t%o0, %g2, %g2\n> +\tbrnz,pt\t%g2, 1f\n> +\t mov\t63, %g1\n> +\tsllx\t%o0, 32, %o0\n> +#endif\n\nTestign for BITS_PER_LONG seems not necessary as long as this is sparc64 only.\nAnd sparc32 has no optimized bit operations not even LEON\nso this would not make sense in sparc32 land anyway.\n\n> +\tmov\t31, %g1\n> +1:\n> +\tmov\t-1, %g2\n> +\tsllx\t%g2, (BITS_PER_LONG-16), %g2\nspaces around operators please. It is no excuse that the source did not have so.\n\n> +\tand\t%o0, %g2, %g2\n> +\tbrnz,pt\t%g2, 2f\n> +\t mov\t-1, %g2\n> +\tsllx\t%o0, 16, %o0\n> +\tadd\t%g1, -16, %g1\n> +2:\n> +\tmov\t-1, %g2\n> +\tsllx\t%g2, (BITS_PER_LONG-8), %g2\n> +\tand\t%o0, %g2, %g2\n> +\tbrnz,pt\t%g2, 3f\n> +\t mov\t-1, %g2\n> +\tsllx\t%o0, 8, %o0\n> +\tadd\t%g1, -8, %g1\n> +3:\n> +\tsllx\t%g2, (BITS_PER_LONG-4), %g2\n> +\tand\t%o0, %g2, %g2\n> +\tbrnz,pt\t%g2, 4f\n> +\t mov\t-1, %g2\n> +\tsllx\t%o0, 4, %o0\n> +\tadd\t%g1, -4, %g1\n> +4:\n> +\tsllx\t%g2, (BITS_PER_LONG-2), %g2\n> +\tand\t%o0, %g2, %g2\n> +\tbrnz,pt\t%g2, 5f\n> +\t mov\t-1, %g3\n> +\tsllx\t%o0, 2, %o0\n> +\tadd\t%g1, -2, %g1\n> +5:\n> +\tmov\t0, %g2\n> +\tsllx\t%g3, (BITS_PER_LONG-1), %g3\n> +\tand\t%o0, %g3, %o0\n> +\tmovre\t%o0, 1, %g2\n> +\tsub\t%g1, %g2, %g1\n> +\tjmp\t%o7+8\n> +\t sra\t%g1, 0, %o0\n> +\t.size\t__fls, .-__fls\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2T2d0Yjcz9t30\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 05:50:49 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751362AbdI0Tus (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 15:50:48 -0400","from asavdk4.altibox.net ([109.247.116.15]:35514 \"EHLO\n\tasavdk4.altibox.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751345AbdI0Tur (ORCPT\n\t<rfc822; sparclinux@vger.kernel.org>); Wed, 27 Sep 2017 15:50:47 -0400","from ravnborg.org (unknown [188.228.89.252])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby asavdk4.altibox.net (Postfix) with ESMTPS id A5C73802FC;\n\tWed, 27 Sep 2017 21:50:44 +0200 (CEST)"],"Date":"Wed, 27 Sep 2017 21:50:43 +0200","From":"Sam Ravnborg <sam@ravnborg.org>","To":"Vijay Kumar <vijay.ac.kumar@oracle.com>","Cc":"davem@davemloft.net, linux-kernel@vger.kernel.org,\n\tsparclinux@vger.kernel.org, babu.moger@oracle.com, rob.gardner@oracle.com","Subject":"Re: [PATCH v2 1/2] sparc64: Define SPARC default fls and __fls","Message-ID":"<20170927195043.GA15193@ravnborg.org>","References":"<1506540326-9604-1-git-send-email-vijay.ac.kumar@oracle.com>\n\t<1506540326-9604-2-git-send-email-vijay.ac.kumar@oracle.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1506540326-9604-2-git-send-email-vijay.ac.kumar@oracle.com>","User-Agent":"Mutt/1.5.21 (2010-09-15)","X-CMAE-Score":"0","X-CMAE-Analysis":"v=2.2 cv=eqGd9chX c=1 sm=1 tr=0\n\ta=Ij76tQDYWdb01v2+RnYW5w==:117 a=Ij76tQDYWdb01v2+RnYW5w==:17\n\ta=kj9zAlcOel0A:10 a=bpxEyb3N_zgcQIJegpkA:9 a=CjuIK1q_8ugA:10","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1776577,"web_url":"http://patchwork.ozlabs.org/comment/1776577/","msgid":"<20170927195922.GA22664@ravnborg.org>","list_archive_url":null,"date":"2017-09-27T19:59:22","subject":"Re: [PATCH v2 1/2] sparc64: Define SPARC default fls and __fls","submitter":{"id":1009,"url":"http://patchwork.ozlabs.org/api/people/1009/","name":"Sam Ravnborg","email":"sam@ravnborg.org"},"content":"> > +\n> > +EXPORT_SYMBOL(__fls)\n> > +EXPORT_SYMBOL(fls)\n> > +\n> > +fls:\n> Use ENTRY(), ENDPROC() for assembler functions.\nAnd locate EXPORT_SYMBOL() right after ENDPROC().\n\n\tSam\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2TDb10T5z9t4b\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 05:59:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751136AbdI0T70 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 15:59:26 -0400","from asavdk4.altibox.net ([109.247.116.15]:35680 \"EHLO\n\tasavdk4.altibox.net\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750932AbdI0T70 (ORCPT\n\t<rfc822; sparclinux@vger.kernel.org>); Wed, 27 Sep 2017 15:59:26 -0400","from ravnborg.org (unknown [188.228.89.252])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby asavdk4.altibox.net (Postfix) with ESMTPS id 0F008802FD;\n\tWed, 27 Sep 2017 21:59:24 +0200 (CEST)"],"Date":"Wed, 27 Sep 2017 21:59:22 +0200","From":"Sam Ravnborg <sam@ravnborg.org>","To":"Vijay Kumar <vijay.ac.kumar@oracle.com>","Cc":"davem@davemloft.net, linux-kernel@vger.kernel.org,\n\tsparclinux@vger.kernel.org, babu.moger@oracle.com, rob.gardner@oracle.com","Subject":"Re: [PATCH v2 1/2] sparc64: Define SPARC default fls and __fls","Message-ID":"<20170927195922.GA22664@ravnborg.org>","References":"<1506540326-9604-1-git-send-email-vijay.ac.kumar@oracle.com>\n\t<1506540326-9604-2-git-send-email-vijay.ac.kumar@oracle.com>\n\t<20170927195043.GA15193@ravnborg.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927195043.GA15193@ravnborg.org>","User-Agent":"Mutt/1.5.21 (2010-09-15)","X-CMAE-Score":"0","X-CMAE-Analysis":"v=2.2 cv=eqGd9chX c=1 sm=1 tr=0\n\ta=Ij76tQDYWdb01v2+RnYW5w==:117 a=Ij76tQDYWdb01v2+RnYW5w==:17\n\ta=kj9zAlcOel0A:10 a=31tsVcWsjbqEky76TD4A:9 a=CjuIK1q_8ugA:10","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}}]