* [PATCH] asm-generic: fix -Wtype-limits compiler warnings
@ 2019-07-23 20:49 Qian Cai
2019-07-23 20:49 ` Qian Cai
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Qian Cai @ 2019-07-23 20:49 UTC (permalink / raw)
To: akpm
Cc: davem, arnd, dhowells, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel, Qian Cai
The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
compilation warning because "rx_frag_size" is an "ushort" while
PAGE_SHIFT here is 16. The commit changed the get_order() to be a
multi-line macro where compilers insist to check all statements in the
macro even when __builtin_constant_p(rx_frag_size) will return false as
"rx_frag_size" is a module parameter.
In file included from ./arch/powerpc/include/asm/page_64.h:107,
from ./arch/powerpc/include/asm/page.h:242,
from ./arch/powerpc/include/asm/mmu.h:132,
from ./arch/powerpc/include/asm/lppaca.h:47,
from ./arch/powerpc/include/asm/paca.h:17,
from ./arch/powerpc/include/asm/current.h:13,
from ./include/linux/thread_info.h:21,
from ./arch/powerpc/include/asm/processor.h:39,
from ./include/linux/prefetch.h:15,
from drivers/net/ethernet/emulex/benet/be_main.c:14:
drivers/net/ethernet/emulex/benet/be_main.c: In function
'be_rx_cqs_create':
./include/asm-generic/getorder.h:54:9: warning: comparison is always
true due to limited range of data type [-Wtype-limits]
(((n) < (1UL << PAGE_SHIFT)) ? 0 : \
^
drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
of macro 'get_order'
adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
^~~~~~~~~
Fix it by moving almost all of this multi-line macro into a proper
function __get_order(), and leave get_order() as a single-line macro in
order to avoid compilation errors.
Fixes: d66acc39c7ce ("bitops: Optimise get_order()")
Signed-off-by: Qian Cai <cai@lca.pw>
---
include/asm-generic/getorder.h | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/asm-generic/getorder.h b/include/asm-generic/getorder.h
index c64bea7a52be..c6a6d3cd7007 100644
--- a/include/asm-generic/getorder.h
+++ b/include/asm-generic/getorder.h
@@ -15,6 +15,16 @@ int __get_order(unsigned long size)
{
int order;
+ if (__builtin_constant_p(size)) {
+ if (!size)
+ return BITS_PER_LONG - PAGE_SHIFT;
+
+ if (size < (1UL << PAGE_SHIFT))
+ return 0;
+
+ return ilog2((size) - 1) - PAGE_SHIFT + 1;
+ }
+
size--;
size >>= PAGE_SHIFT;
#if BITS_PER_LONG == 32
@@ -49,11 +59,6 @@ int __get_order(unsigned long size)
*/
#define get_order(n) \
( \
- __builtin_constant_p(n) ? ( \
- ((n) == 0UL) ? BITS_PER_LONG - PAGE_SHIFT : \
- (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
- ilog2((n) - 1) - PAGE_SHIFT + 1) \
- ) : \
__get_order(n) \
)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-23 20:49 [PATCH] asm-generic: fix -Wtype-limits compiler warnings Qian Cai
@ 2019-07-23 20:49 ` Qian Cai
2019-07-23 21:27 ` Andrew Morton
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Qian Cai @ 2019-07-23 20:49 UTC (permalink / raw)
To: akpm
Cc: davem, arnd, dhowells, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel, Qian Cai
The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
compilation warning because "rx_frag_size" is an "ushort" while
PAGE_SHIFT here is 16. The commit changed the get_order() to be a
multi-line macro where compilers insist to check all statements in the
macro even when __builtin_constant_p(rx_frag_size) will return false as
"rx_frag_size" is a module parameter.
In file included from ./arch/powerpc/include/asm/page_64.h:107,
from ./arch/powerpc/include/asm/page.h:242,
from ./arch/powerpc/include/asm/mmu.h:132,
from ./arch/powerpc/include/asm/lppaca.h:47,
from ./arch/powerpc/include/asm/paca.h:17,
from ./arch/powerpc/include/asm/current.h:13,
from ./include/linux/thread_info.h:21,
from ./arch/powerpc/include/asm/processor.h:39,
from ./include/linux/prefetch.h:15,
from drivers/net/ethernet/emulex/benet/be_main.c:14:
drivers/net/ethernet/emulex/benet/be_main.c: In function
'be_rx_cqs_create':
./include/asm-generic/getorder.h:54:9: warning: comparison is always
true due to limited range of data type [-Wtype-limits]
(((n) < (1UL << PAGE_SHIFT)) ? 0 : \
^
drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
of macro 'get_order'
adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
^~~~~~~~~
Fix it by moving almost all of this multi-line macro into a proper
function __get_order(), and leave get_order() as a single-line macro in
order to avoid compilation errors.
Fixes: d66acc39c7ce ("bitops: Optimise get_order()")
Signed-off-by: Qian Cai <cai@lca.pw>
---
include/asm-generic/getorder.h | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/asm-generic/getorder.h b/include/asm-generic/getorder.h
index c64bea7a52be..c6a6d3cd7007 100644
--- a/include/asm-generic/getorder.h
+++ b/include/asm-generic/getorder.h
@@ -15,6 +15,16 @@ int __get_order(unsigned long size)
{
int order;
+ if (__builtin_constant_p(size)) {
+ if (!size)
+ return BITS_PER_LONG - PAGE_SHIFT;
+
+ if (size < (1UL << PAGE_SHIFT))
+ return 0;
+
+ return ilog2((size) - 1) - PAGE_SHIFT + 1;
+ }
+
size--;
size >>= PAGE_SHIFT;
#if BITS_PER_LONG == 32
@@ -49,11 +59,6 @@ int __get_order(unsigned long size)
*/
#define get_order(n) \
( \
- __builtin_constant_p(n) ? ( \
- ((n) == 0UL) ? BITS_PER_LONG - PAGE_SHIFT : \
- (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
- ilog2((n) - 1) - PAGE_SHIFT + 1) \
- ) : \
__get_order(n) \
)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-23 20:49 [PATCH] asm-generic: fix -Wtype-limits compiler warnings Qian Cai
2019-07-23 20:49 ` Qian Cai
@ 2019-07-23 21:27 ` Andrew Morton
2019-07-23 21:27 ` Andrew Morton
2019-07-24 2:29 ` Nathan Chancellor
2019-07-24 7:49 ` David Howells
3 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2019-07-23 21:27 UTC (permalink / raw)
To: Qian Cai
Cc: davem, arnd, dhowells, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel
On Tue, 23 Jul 2019 16:49:46 -0400 Qian Cai <cai@lca.pw> wrote:
> The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
> compilation warning because "rx_frag_size" is an "ushort" while
> PAGE_SHIFT here is 16. The commit changed the get_order() to be a
> multi-line macro where compilers insist to check all statements in the
> macro even when __builtin_constant_p(rx_frag_size) will return false as
> "rx_frag_size" is a module parameter.
>
> In file included from ./arch/powerpc/include/asm/page_64.h:107,
> from ./arch/powerpc/include/asm/page.h:242,
> from ./arch/powerpc/include/asm/mmu.h:132,
> from ./arch/powerpc/include/asm/lppaca.h:47,
> from ./arch/powerpc/include/asm/paca.h:17,
> from ./arch/powerpc/include/asm/current.h:13,
> from ./include/linux/thread_info.h:21,
> from ./arch/powerpc/include/asm/processor.h:39,
> from ./include/linux/prefetch.h:15,
> from drivers/net/ethernet/emulex/benet/be_main.c:14:
> drivers/net/ethernet/emulex/benet/be_main.c: In function
> 'be_rx_cqs_create':
> ./include/asm-generic/getorder.h:54:9: warning: comparison is always
> true due to limited range of data type [-Wtype-limits]
> (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> ^
> drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
> of macro 'get_order'
> adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
> ^~~~~~~~~
>
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
>
> ...
>
> --- a/include/asm-generic/getorder.h
> +++ b/include/asm-generic/getorder.h
> @@ -15,6 +15,16 @@ int __get_order(unsigned long size)
> {
> int order;
>
> + if (__builtin_constant_p(size)) {
> + if (!size)
> + return BITS_PER_LONG - PAGE_SHIFT;
> +
> + if (size < (1UL << PAGE_SHIFT))
> + return 0;
> +
> + return ilog2((size) - 1) - PAGE_SHIFT + 1;
> + }
> +
> size--;
> size >>= PAGE_SHIFT;
> #if BITS_PER_LONG == 32
> @@ -49,11 +59,6 @@ int __get_order(unsigned long size)
> */
> #define get_order(n) \
> ( \
> - __builtin_constant_p(n) ? ( \
> - ((n) == 0UL) ? BITS_PER_LONG - PAGE_SHIFT : \
> - (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> - ilog2((n) - 1) - PAGE_SHIFT + 1) \
> - ) : \
> __get_order(n) \
> )
So we can remove __get_order() altogether now?
--- a/include/asm-generic/getorder.h~asm-generic-fix-wtype-limits-compiler-warnings-fix
+++ a/include/asm-generic/getorder.h
@@ -7,11 +7,29 @@
#include <linux/compiler.h>
#include <linux/log2.h>
-/*
- * Runtime evaluation of get_order()
+/**
+ * get_order - Determine the allocation order of a memory size
+ * @size: The size for which to get the order
+ *
+ * Determine the allocation order of a particular sized block of memory. This
+ * is on a logarithmic scale, where:
+ *
+ * 0 -> 2^0 * PAGE_SIZE and below
+ * 1 -> 2^1 * PAGE_SIZE to 2^0 * PAGE_SIZE + 1
+ * 2 -> 2^2 * PAGE_SIZE to 2^1 * PAGE_SIZE + 1
+ * 3 -> 2^3 * PAGE_SIZE to 2^2 * PAGE_SIZE + 1
+ * 4 -> 2^4 * PAGE_SIZE to 2^3 * PAGE_SIZE + 1
+ * ...
+ *
+ * The order returned is used to find the smallest allocation granule required
+ * to hold an object of the specified size.
+ *
+ * The result is undefined if the size is 0.
+ *
+ * This function may be used to initialise variables with compile time
+ * evaluations of constants.
*/
-static inline __attribute_const__
-int __get_order(unsigned long size)
+static inline __attribute_const__ int get_order(unsigned long size)
{
int order;
@@ -35,33 +53,6 @@ int __get_order(unsigned long size)
return order;
}
-/**
- * get_order - Determine the allocation order of a memory size
- * @size: The size for which to get the order
- *
- * Determine the allocation order of a particular sized block of memory. This
- * is on a logarithmic scale, where:
- *
- * 0 -> 2^0 * PAGE_SIZE and below
- * 1 -> 2^1 * PAGE_SIZE to 2^0 * PAGE_SIZE + 1
- * 2 -> 2^2 * PAGE_SIZE to 2^1 * PAGE_SIZE + 1
- * 3 -> 2^3 * PAGE_SIZE to 2^2 * PAGE_SIZE + 1
- * 4 -> 2^4 * PAGE_SIZE to 2^3 * PAGE_SIZE + 1
- * ...
- *
- * The order returned is used to find the smallest allocation granule required
- * to hold an object of the specified size.
- *
- * The result is undefined if the size is 0.
- *
- * This function may be used to initialise variables with compile time
- * evaluations of constants.
- */
-#define get_order(n) \
-( \
- __get_order(n) \
-)
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_GENERIC_GETORDER_H */
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-23 21:27 ` Andrew Morton
@ 2019-07-23 21:27 ` Andrew Morton
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Morton @ 2019-07-23 21:27 UTC (permalink / raw)
To: Qian Cai
Cc: davem, arnd, dhowells, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel
On Tue, 23 Jul 2019 16:49:46 -0400 Qian Cai <cai@lca.pw> wrote:
> The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
> compilation warning because "rx_frag_size" is an "ushort" while
> PAGE_SHIFT here is 16. The commit changed the get_order() to be a
> multi-line macro where compilers insist to check all statements in the
> macro even when __builtin_constant_p(rx_frag_size) will return false as
> "rx_frag_size" is a module parameter.
>
> In file included from ./arch/powerpc/include/asm/page_64.h:107,
> from ./arch/powerpc/include/asm/page.h:242,
> from ./arch/powerpc/include/asm/mmu.h:132,
> from ./arch/powerpc/include/asm/lppaca.h:47,
> from ./arch/powerpc/include/asm/paca.h:17,
> from ./arch/powerpc/include/asm/current.h:13,
> from ./include/linux/thread_info.h:21,
> from ./arch/powerpc/include/asm/processor.h:39,
> from ./include/linux/prefetch.h:15,
> from drivers/net/ethernet/emulex/benet/be_main.c:14:
> drivers/net/ethernet/emulex/benet/be_main.c: In function
> 'be_rx_cqs_create':
> ./include/asm-generic/getorder.h:54:9: warning: comparison is always
> true due to limited range of data type [-Wtype-limits]
> (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> ^
> drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
> of macro 'get_order'
> adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
> ^~~~~~~~~
>
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
>
> ...
>
> --- a/include/asm-generic/getorder.h
> +++ b/include/asm-generic/getorder.h
> @@ -15,6 +15,16 @@ int __get_order(unsigned long size)
> {
> int order;
>
> + if (__builtin_constant_p(size)) {
> + if (!size)
> + return BITS_PER_LONG - PAGE_SHIFT;
> +
> + if (size < (1UL << PAGE_SHIFT))
> + return 0;
> +
> + return ilog2((size) - 1) - PAGE_SHIFT + 1;
> + }
> +
> size--;
> size >>= PAGE_SHIFT;
> #if BITS_PER_LONG == 32
> @@ -49,11 +59,6 @@ int __get_order(unsigned long size)
> */
> #define get_order(n) \
> ( \
> - __builtin_constant_p(n) ? ( \
> - ((n) == 0UL) ? BITS_PER_LONG - PAGE_SHIFT : \
> - (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> - ilog2((n) - 1) - PAGE_SHIFT + 1) \
> - ) : \
> __get_order(n) \
> )
So we can remove __get_order() altogether now?
--- a/include/asm-generic/getorder.h~asm-generic-fix-wtype-limits-compiler-warnings-fix
+++ a/include/asm-generic/getorder.h
@@ -7,11 +7,29 @@
#include <linux/compiler.h>
#include <linux/log2.h>
-/*
- * Runtime evaluation of get_order()
+/**
+ * get_order - Determine the allocation order of a memory size
+ * @size: The size for which to get the order
+ *
+ * Determine the allocation order of a particular sized block of memory. This
+ * is on a logarithmic scale, where:
+ *
+ * 0 -> 2^0 * PAGE_SIZE and below
+ * 1 -> 2^1 * PAGE_SIZE to 2^0 * PAGE_SIZE + 1
+ * 2 -> 2^2 * PAGE_SIZE to 2^1 * PAGE_SIZE + 1
+ * 3 -> 2^3 * PAGE_SIZE to 2^2 * PAGE_SIZE + 1
+ * 4 -> 2^4 * PAGE_SIZE to 2^3 * PAGE_SIZE + 1
+ * ...
+ *
+ * The order returned is used to find the smallest allocation granule required
+ * to hold an object of the specified size.
+ *
+ * The result is undefined if the size is 0.
+ *
+ * This function may be used to initialise variables with compile time
+ * evaluations of constants.
*/
-static inline __attribute_const__
-int __get_order(unsigned long size)
+static inline __attribute_const__ int get_order(unsigned long size)
{
int order;
@@ -35,33 +53,6 @@ int __get_order(unsigned long size)
return order;
}
-/**
- * get_order - Determine the allocation order of a memory size
- * @size: The size for which to get the order
- *
- * Determine the allocation order of a particular sized block of memory. This
- * is on a logarithmic scale, where:
- *
- * 0 -> 2^0 * PAGE_SIZE and below
- * 1 -> 2^1 * PAGE_SIZE to 2^0 * PAGE_SIZE + 1
- * 2 -> 2^2 * PAGE_SIZE to 2^1 * PAGE_SIZE + 1
- * 3 -> 2^3 * PAGE_SIZE to 2^2 * PAGE_SIZE + 1
- * 4 -> 2^4 * PAGE_SIZE to 2^3 * PAGE_SIZE + 1
- * ...
- *
- * The order returned is used to find the smallest allocation granule required
- * to hold an object of the specified size.
- *
- * The result is undefined if the size is 0.
- *
- * This function may be used to initialise variables with compile time
- * evaluations of constants.
- */
-#define get_order(n) \
-( \
- __get_order(n) \
-)
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_GENERIC_GETORDER_H */
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-23 20:49 [PATCH] asm-generic: fix -Wtype-limits compiler warnings Qian Cai
2019-07-23 20:49 ` Qian Cai
2019-07-23 21:27 ` Andrew Morton
@ 2019-07-24 2:29 ` Nathan Chancellor
2019-07-24 2:29 ` Nathan Chancellor
2019-07-24 7:49 ` David Howells
3 siblings, 1 reply; 14+ messages in thread
From: Nathan Chancellor @ 2019-07-24 2:29 UTC (permalink / raw)
To: Qian Cai
Cc: akpm, davem, arnd, dhowells, jakub, ndesaulniers, morbo,
jyknight, linux-arch, linux-kernel
On Tue, Jul 23, 2019 at 04:49:46PM -0400, Qian Cai wrote:
> The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
> compilation warning because "rx_frag_size" is an "ushort" while
> PAGE_SHIFT here is 16. The commit changed the get_order() to be a
> multi-line macro where compilers insist to check all statements in the
> macro even when __builtin_constant_p(rx_frag_size) will return false as
> "rx_frag_size" is a module parameter.
>
> In file included from ./arch/powerpc/include/asm/page_64.h:107,
> from ./arch/powerpc/include/asm/page.h:242,
> from ./arch/powerpc/include/asm/mmu.h:132,
> from ./arch/powerpc/include/asm/lppaca.h:47,
> from ./arch/powerpc/include/asm/paca.h:17,
> from ./arch/powerpc/include/asm/current.h:13,
> from ./include/linux/thread_info.h:21,
> from ./arch/powerpc/include/asm/processor.h:39,
> from ./include/linux/prefetch.h:15,
> from drivers/net/ethernet/emulex/benet/be_main.c:14:
> drivers/net/ethernet/emulex/benet/be_main.c: In function
> 'be_rx_cqs_create':
> ./include/asm-generic/getorder.h:54:9: warning: comparison is always
> true due to limited range of data type [-Wtype-limits]
> (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> ^
> drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
> of macro 'get_order'
> adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
> ^~~~~~~~~
>
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
Wouldn't it just be better to rename __get_order to get_order?
> Fixes: d66acc39c7ce ("bitops: Optimise get_order()")
> Signed-off-by: Qian Cai <cai@lca.pw>
Other than that, LGTM.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 2:29 ` Nathan Chancellor
@ 2019-07-24 2:29 ` Nathan Chancellor
0 siblings, 0 replies; 14+ messages in thread
From: Nathan Chancellor @ 2019-07-24 2:29 UTC (permalink / raw)
To: Qian Cai
Cc: akpm, davem, arnd, dhowells, jakub, ndesaulniers, morbo,
jyknight, linux-arch, linux-kernel
On Tue, Jul 23, 2019 at 04:49:46PM -0400, Qian Cai wrote:
> The commit d66acc39c7ce ("bitops: Optimise get_order()") introduced a
> compilation warning because "rx_frag_size" is an "ushort" while
> PAGE_SHIFT here is 16. The commit changed the get_order() to be a
> multi-line macro where compilers insist to check all statements in the
> macro even when __builtin_constant_p(rx_frag_size) will return false as
> "rx_frag_size" is a module parameter.
>
> In file included from ./arch/powerpc/include/asm/page_64.h:107,
> from ./arch/powerpc/include/asm/page.h:242,
> from ./arch/powerpc/include/asm/mmu.h:132,
> from ./arch/powerpc/include/asm/lppaca.h:47,
> from ./arch/powerpc/include/asm/paca.h:17,
> from ./arch/powerpc/include/asm/current.h:13,
> from ./include/linux/thread_info.h:21,
> from ./arch/powerpc/include/asm/processor.h:39,
> from ./include/linux/prefetch.h:15,
> from drivers/net/ethernet/emulex/benet/be_main.c:14:
> drivers/net/ethernet/emulex/benet/be_main.c: In function
> 'be_rx_cqs_create':
> ./include/asm-generic/getorder.h:54:9: warning: comparison is always
> true due to limited range of data type [-Wtype-limits]
> (((n) < (1UL << PAGE_SHIFT)) ? 0 : \
> ^
> drivers/net/ethernet/emulex/benet/be_main.c:3138:33: note: in expansion
> of macro 'get_order'
> adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
> ^~~~~~~~~
>
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
Wouldn't it just be better to rename __get_order to get_order?
> Fixes: d66acc39c7ce ("bitops: Optimise get_order()")
> Signed-off-by: Qian Cai <cai@lca.pw>
Other than that, LGTM.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-23 20:49 [PATCH] asm-generic: fix -Wtype-limits compiler warnings Qian Cai
` (2 preceding siblings ...)
2019-07-24 2:29 ` Nathan Chancellor
@ 2019-07-24 7:49 ` David Howells
2019-07-24 7:49 ` David Howells
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: David Howells @ 2019-07-24 7:49 UTC (permalink / raw)
To: Qian Cai
Cc: dhowells, akpm, davem, arnd, jakub, ndesaulniers, morbo,
jyknight, natechancellor, linux-arch, linux-kernel
Qian Cai <cai@lca.pw> wrote:
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
The idea was that you could compile-time initialise a global variable with
get_order():
int a = get_order(SOME_MACRO);
This is the same reason that ilog2() is a macro:
int a = ilog2(SOME_MACRO);
See the banner comment on get_order():
* This function may be used to initialise variables with compile time
* evaluations of constants.
If you're moving the constant branch into __get_order(), an inline function,
then we'll no longer be able to do this and you need to modify the comment
too. In fact, would there still be a point in having the get_order() macro?
Also, IIRC, older versions of gcc see __builtin_constant_p(n) == 0 inside an
function, inline or otherwise, even if the passed-in argument *is* constant.
David
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 7:49 ` David Howells
@ 2019-07-24 7:49 ` David Howells
2019-07-24 18:45 ` Qian Cai
2019-07-24 21:49 ` David Howells
2 siblings, 0 replies; 14+ messages in thread
From: David Howells @ 2019-07-24 7:49 UTC (permalink / raw)
To: Qian Cai
Cc: dhowells, akpm, davem, arnd, jakub, ndesaulniers, morbo,
jyknight, natechancellor, linux-arch, linux-kernel
Qian Cai <cai@lca.pw> wrote:
> Fix it by moving almost all of this multi-line macro into a proper
> function __get_order(), and leave get_order() as a single-line macro in
> order to avoid compilation errors.
The idea was that you could compile-time initialise a global variable with
get_order():
int a = get_order(SOME_MACRO);
This is the same reason that ilog2() is a macro:
int a = ilog2(SOME_MACRO);
See the banner comment on get_order():
* This function may be used to initialise variables with compile time
* evaluations of constants.
If you're moving the constant branch into __get_order(), an inline function,
then we'll no longer be able to do this and you need to modify the comment
too. In fact, would there still be a point in having the get_order() macro?
Also, IIRC, older versions of gcc see __builtin_constant_p(n) == 0 inside an
function, inline or otherwise, even if the passed-in argument *is* constant.
David
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 7:49 ` David Howells
2019-07-24 7:49 ` David Howells
@ 2019-07-24 18:45 ` Qian Cai
2019-07-24 18:45 ` Qian Cai
2019-07-24 21:49 ` David Howells
2 siblings, 1 reply; 14+ messages in thread
From: Qian Cai @ 2019-07-24 18:45 UTC (permalink / raw)
To: David Howells
Cc: akpm, davem, arnd, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel
On Wed, 2019-07-24 at 08:49 +0100, David Howells wrote:
> Qian Cai <cai@lca.pw> wrote:
>
> > Fix it by moving almost all of this multi-line macro into a proper
> > function __get_order(), and leave get_order() as a single-line macro in
> > order to avoid compilation errors.
>
> The idea was that you could compile-time initialise a global variable with
> get_order():
>
> int a = get_order(SOME_MACRO);
>
> This is the same reason that ilog2() is a macro:
>
> int a = ilog2(SOME_MACRO);
>
> See the banner comment on get_order():
>
> * This function may be used to initialise variables with compile time
> * evaluations of constants.
>
> If you're moving the constant branch into __get_order(), an inline function,
> then we'll no longer be able to do this and you need to modify the comment
> too. In fact, would there still be a point in having the get_order() macro?
>
> Also, IIRC, older versions of gcc see __builtin_constant_p(n) == 0 inside an
> function, inline or otherwise, even if the passed-in argument *is* constant.
I have GCC 8.2.1 which works fine.
# cat const.c
#include <stdio.h>
static int i = 0;
static inline void check()
{
if (__builtin_constant_p(i))
printf("i is a const.\n");
}
void main()
{
check();
}
# gcc -O2 const.c -o const
# ./const
i is a const.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 18:45 ` Qian Cai
@ 2019-07-24 18:45 ` Qian Cai
0 siblings, 0 replies; 14+ messages in thread
From: Qian Cai @ 2019-07-24 18:45 UTC (permalink / raw)
To: David Howells
Cc: akpm, davem, arnd, jakub, ndesaulniers, morbo, jyknight,
natechancellor, linux-arch, linux-kernel
On Wed, 2019-07-24 at 08:49 +0100, David Howells wrote:
> Qian Cai <cai@lca.pw> wrote:
>
> > Fix it by moving almost all of this multi-line macro into a proper
> > function __get_order(), and leave get_order() as a single-line macro in
> > order to avoid compilation errors.
>
> The idea was that you could compile-time initialise a global variable with
> get_order():
>
> int a = get_order(SOME_MACRO);
>
> This is the same reason that ilog2() is a macro:
>
> int a = ilog2(SOME_MACRO);
>
> See the banner comment on get_order():
>
> * This function may be used to initialise variables with compile time
> * evaluations of constants.
>
> If you're moving the constant branch into __get_order(), an inline function,
> then we'll no longer be able to do this and you need to modify the comment
> too. In fact, would there still be a point in having the get_order() macro?
>
> Also, IIRC, older versions of gcc see __builtin_constant_p(n) == 0 inside an
> function, inline or otherwise, even if the passed-in argument *is* constant.
I have GCC 8.2.1 which works fine.
# cat const.c
#include <stdio.h>
static int i = 0;
static inline void check()
{
if (__builtin_constant_p(i))
printf("i is a const.\n");
}
void main()
{
check();
}
# gcc -O2 const.c -o const
# ./const
i is a const.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 7:49 ` David Howells
2019-07-24 7:49 ` David Howells
2019-07-24 18:45 ` Qian Cai
@ 2019-07-24 21:49 ` David Howells
2019-07-24 21:49 ` David Howells
2019-07-25 3:22 ` Qian Cai
2 siblings, 2 replies; 14+ messages in thread
From: David Howells @ 2019-07-24 21:49 UTC (permalink / raw)
To: Qian Cai
Cc: dhowells, akpm, davem, arnd, jakub, ndesaulniers, morbo,
jyknight, natechancellor, linux-arch, linux-kernel
Qian Cai <cai@lca.pw> wrote:
> I have GCC 8.2.1 which works fine.
But you need to check the minimum version, i.e. 4.6:
#if GCC_VERSION < 40600
David
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 21:49 ` David Howells
@ 2019-07-24 21:49 ` David Howells
2019-07-25 3:22 ` Qian Cai
1 sibling, 0 replies; 14+ messages in thread
From: David Howells @ 2019-07-24 21:49 UTC (permalink / raw)
To: Qian Cai
Cc: dhowells, akpm, davem, arnd, jakub, ndesaulniers, morbo,
jyknight, natechancellor, linux-arch, linux-kernel
Qian Cai <cai@lca.pw> wrote:
> I have GCC 8.2.1 which works fine.
But you need to check the minimum version, i.e. 4.6:
#if GCC_VERSION < 40600
David
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-24 21:49 ` David Howells
2019-07-24 21:49 ` David Howells
@ 2019-07-25 3:22 ` Qian Cai
2019-07-25 3:22 ` Qian Cai
1 sibling, 1 reply; 14+ messages in thread
From: Qian Cai @ 2019-07-25 3:22 UTC (permalink / raw)
To: David Howells
Cc: Andrew Morton, David Miller, Arnd Bergmann, Jakub Jelinek,
Nick Desaulniers, morbo, jyknight, natechancellor, linux-arch,
linux-kernel
> On Jul 24, 2019, at 5:49 PM, David Howells <dhowells@redhat.com> wrote:
>
> Qian Cai <cai@lca.pw> wrote:
>
>> I have GCC 8.2.1 which works fine.
>
> But you need to check the minimum version, i.e. 4.6:
>
> #if GCC_VERSION < 40600
>
I did check gcc version 4.1.2 20080704 and it works fine.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] asm-generic: fix -Wtype-limits compiler warnings
2019-07-25 3:22 ` Qian Cai
@ 2019-07-25 3:22 ` Qian Cai
0 siblings, 0 replies; 14+ messages in thread
From: Qian Cai @ 2019-07-25 3:22 UTC (permalink / raw)
To: David Howells
Cc: Andrew Morton, David Miller, Arnd Bergmann, Jakub Jelinek,
Nick Desaulniers, morbo, jyknight, natechancellor, linux-arch,
linux-kernel
> On Jul 24, 2019, at 5:49 PM, David Howells <dhowells@redhat.com> wrote:
>
> Qian Cai <cai@lca.pw> wrote:
>
>> I have GCC 8.2.1 which works fine.
>
> But you need to check the minimum version, i.e. 4.6:
>
> #if GCC_VERSION < 40600
>
I did check gcc version 4.1.2 20080704 and it works fine.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-07-25 3:22 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23 20:49 [PATCH] asm-generic: fix -Wtype-limits compiler warnings Qian Cai
2019-07-23 20:49 ` Qian Cai
2019-07-23 21:27 ` Andrew Morton
2019-07-23 21:27 ` Andrew Morton
2019-07-24 2:29 ` Nathan Chancellor
2019-07-24 2:29 ` Nathan Chancellor
2019-07-24 7:49 ` David Howells
2019-07-24 7:49 ` David Howells
2019-07-24 18:45 ` Qian Cai
2019-07-24 18:45 ` Qian Cai
2019-07-24 21:49 ` David Howells
2019-07-24 21:49 ` David Howells
2019-07-25 3:22 ` Qian Cai
2019-07-25 3:22 ` Qian Cai
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 NNTP newsgroup(s).