[Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Jani Nikula]

is_power_of_2() only works for types <= sizeof(unsigned long) and it's
also not a constant expression. There are a number of places in kernel
where is_power_of_2() is called on u64, which fails on 32-bit
builds. Try to remedy that. While at it, make it a constant expression
when possible.

I admit I've only lightly tested this, and I haven't tried it with
allmodconfig.


Jani Nikula (4):
  log2: add helper __IS_POWER_OF_2()
  log2: have is_power_of_2() support bigger types than unsigned long
  log2: allow use of is_power_of_2() in constant expressions
  drm/i915/reg: use is_power_of_2() from log2.h

 drivers/gpu/drm/i915/i915_reg_defs.h |  7 +------
 include/linux/log2.h                 | 25 ++++++++++++++++++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

-- 
2.39.2

[Intel-gfx] [PATCH 1/4] log2: add helper __IS_POWER_OF_2()

[Jani Nikula]

Add a helper to avoid duplication in the subsequent changes.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: David Gow <davidgow@google.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/linux/log2.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/log2.h b/include/linux/log2.h
index 9f30d087a128..19e773116ae3 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -33,6 +33,8 @@ int __ilog2_u64(u64 n)
 }
 #endif
 
+#define __IS_POWER_OF_2(n) ((n) != 0 && (((n) & ((n) - 1)) == 0))
+
 /**
  * is_power_of_2() - check if a value is a power of two
  * @n: the value to check
@@ -44,7 +46,7 @@ int __ilog2_u64(u64 n)
 static inline __attribute__((const))
 bool is_power_of_2(unsigned long n)
 {
-	return (n != 0 && ((n & (n - 1)) == 0));
+	return __IS_POWER_OF_2(n);
 }
 
 /**
-- 
2.39.2

[Intel-gfx] [PATCH 2/4] log2: have is_power_of_2() support bigger types than unsigned long

[Jani Nikula]

is_power_of_2() does not work properly for e.g. u64 in 32-bit
builds. Choose an unsigned long long version if the argument is bigger
than unsigned long.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: David Gow <davidgow@google.com>
Link: https://lore.kernel.org/r/20230329065532.2122295-2-davidgow@google.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/linux/log2.h | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/log2.h b/include/linux/log2.h
index 19e773116ae3..4027d1eecd7f 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -35,6 +35,18 @@ int __ilog2_u64(u64 n)
 
 #define __IS_POWER_OF_2(n) ((n) != 0 && (((n) & ((n) - 1)) == 0))
 
+static inline __attribute__((const))
+bool __is_power_of_2_ull(unsigned long long n)
+{
+	return __IS_POWER_OF_2(n);
+}
+
+static inline __attribute__((const))
+bool __is_power_of_2(unsigned long n)
+{
+	return __IS_POWER_OF_2(n);
+}
+
 /**
  * is_power_of_2() - check if a value is a power of two
  * @n: the value to check
@@ -43,11 +55,10 @@ int __ilog2_u64(u64 n)
  * *not* considered a power of two.
  * Return: true if @n is a power of 2, otherwise false.
  */
-static inline __attribute__((const))
-bool is_power_of_2(unsigned long n)
-{
-	return __IS_POWER_OF_2(n);
-}
+#define is_power_of_2(n)						\
+	__builtin_choose_expr(sizeof(n) > sizeof(unsigned long),	\
+			      __is_power_of_2_ull(n),			\
+			      __is_power_of_2(n))
 
 /**
  * __roundup_pow_of_two() - round up to nearest power of two
-- 
2.39.2

[Intel-gfx] [PATCH 3/4] log2: allow use of is_power_of_2() in constant expressions

[Jani Nikula]

Even though the static inline functions are __attribute__((const)) you
can't use them in constant expressions. Make is_power_of_2() a constant
expression if possible.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christian König <christian.koenig@amd.com>
Cc: David Gow <davidgow@google.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/linux/log2.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/log2.h b/include/linux/log2.h
index 4027d1eecd7f..447a85102de0 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -56,9 +56,11 @@ bool __is_power_of_2(unsigned long n)
  * Return: true if @n is a power of 2, otherwise false.
  */
 #define is_power_of_2(n)						\
-	__builtin_choose_expr(sizeof(n) > sizeof(unsigned long),	\
-			      __is_power_of_2_ull(n),			\
-			      __is_power_of_2(n))
+	__builtin_choose_expr(__is_constexpr(n),			\
+			      __IS_POWER_OF_2(n),			\
+			      __builtin_choose_expr(sizeof(n) > sizeof(unsigned long), \
+						    __is_power_of_2_ull(n), \
+						    __is_power_of_2(n)))
 
 /**
  * __roundup_pow_of_two() - round up to nearest power of two
-- 
2.39.2

[Intel-gfx] [PATCH 4/4] drm/i915/reg: use is_power_of_2() from log2.h

[Jani Nikula]

Now that log2.h is_power_of_2() supports constant expressions, use it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg_defs.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
index be43580a6979..44e99a9a381f 100644
--- a/drivers/gpu/drm/i915/i915_reg_defs.h
+++ b/drivers/gpu/drm/i915/i915_reg_defs.h
@@ -52,11 +52,6 @@
 				 __is_constexpr(__low) &&		\
 				 ((__low) < 0 || (__high) > 63 || (__low) > (__high)))))
 
-/*
- * Local integer constant expression version of is_power_of_2().
- */
-#define IS_POWER_OF_2(__x)		((__x) && (((__x) & ((__x) - 1)) == 0))
-
 /**
  * REG_FIELD_PREP() - Prepare a u32 bitfield value
  * @__mask: shifted mask defining the field's length and position
@@ -71,7 +66,7 @@
 	((u32)((((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) +	\
 	       BUILD_BUG_ON_ZERO(!__is_constexpr(__mask)) +		\
 	       BUILD_BUG_ON_ZERO((__mask) == 0 || (__mask) > U32_MAX) +		\
-	       BUILD_BUG_ON_ZERO(!IS_POWER_OF_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
+	       BUILD_BUG_ON_ZERO(!is_power_of_2((__mask) + (1ULL << __bf_shf(__mask)))) + \
 	       BUILD_BUG_ON_ZERO(__builtin_choose_expr(__is_constexpr(__val), (~((__mask) >> __bf_shf(__mask)) & (__val)), 0))))
 
 /**
-- 
2.39.2

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Christian König]

Am 30.03.23 um 12:42 schrieb Jani Nikula:
> is_power_of_2() only works for types <= sizeof(unsigned long) and it's
> also not a constant expression. There are a number of places in kernel
> where is_power_of_2() is called on u64, which fails on 32-bit
> builds. Try to remedy that. While at it, make it a constant expression
> when possible.
>
> I admit I've only lightly tested this, and I haven't tried it with
> allmodconfig.

Looks good to me from a one mile high level pov. But I'm really not an 
expert on that type of compiler magic, so only:

Acked-by: Christian König <christian.koenig@amd.com>

for the series.

Regards,
Christian.

>
>
> Jani Nikula (4):
>    log2: add helper __IS_POWER_OF_2()
>    log2: have is_power_of_2() support bigger types than unsigned long
>    log2: allow use of is_power_of_2() in constant expressions
>    drm/i915/reg: use is_power_of_2() from log2.h
>
>   drivers/gpu/drm/i915/i915_reg_defs.h |  7 +------
>   include/linux/log2.h                 | 25 ++++++++++++++++++++-----
>   2 files changed, 21 insertions(+), 11 deletions(-)
>

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for log2: make is_power_of_2() more generic

[Patchwork]

== Series Details ==

Series: log2: make is_power_of_2() more generic
URL   : https://patchwork.freedesktop.org/series/115863/
State : warning

== Summary ==

Error: dim checkpatch failed
5c5c2de058cb log2: add helper __IS_POWER_OF_2()
-:24: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#24: FILE: include/linux/log2.h:36:
+#define __IS_POWER_OF_2(n) ((n) != 0 && (((n) & ((n) - 1)) == 0))

total: 0 errors, 0 warnings, 1 checks, 16 lines checked
66a1c1b93361 log2: have is_power_of_2() support bigger types than unsigned long
-:28: WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __attribute_const__ over __attribute__((const))
#28: FILE: include/linux/log2.h:38:
+static inline __attribute__((const))

-:34: WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __attribute_const__ over __attribute__((const))
#34: FILE: include/linux/log2.h:44:
+static inline __attribute__((const))

-:52: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
#52: FILE: include/linux/log2.h:58:
+#define is_power_of_2(n)						\
+	__builtin_choose_expr(sizeof(n) > sizeof(unsigned long),	\
+			      __is_power_of_2_ull(n),			\
+			      __is_power_of_2(n))

total: 0 errors, 2 warnings, 1 checks, 33 lines checked
1d19fef56ddc log2: allow use of is_power_of_2() in constant expressions
385585e52a78 drm/i915/reg: use is_power_of_2() from log2.h

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for log2: make is_power_of_2() more generic

[Patchwork]

== Series Details ==

Series: log2: make is_power_of_2() more generic
URL   : https://patchwork.freedesktop.org/series/115863/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✓ Fi.CI.BAT: success for log2: make is_power_of_2() more generic

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 10598 bytes --]

== Series Details ==

Series: log2: make is_power_of_2() more generic
URL   : https://patchwork.freedesktop.org/series/115863/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12939 -> Patchwork_115863v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/index.html

Participating hosts (37 -> 36)
------------------------------

  Additional (1): bat-rpls-2 
  Missing    (2): fi-rkl-11600 fi-snb-2520m 

Known issues
------------

  Here are the changes found in Patchwork_115863v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-2:         NOTRUN -> [SKIP][1] ([i915#7456])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@read:
    - bat-rpls-2:         NOTRUN -> [SKIP][2] ([i915#2582]) +4 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@fbdev@read.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-rpls-2:         NOTRUN -> [SKIP][3] ([i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][4] ([i915#3282])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-rpls-2:         NOTRUN -> [SKIP][5] ([i915#7561])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-11:         [PASS][6] -> [FAIL][7] ([i915#8308])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/bat-dg2-11/igt@i915_pm_rps@basic-api.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-dg2-11/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [PASS][8] -> [DMESG-FAIL][9] ([i915#5334] / [i915#7872])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
    - fi-glk-j4005:       [PASS][10] -> [DMESG-FAIL][11] ([i915#5334])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][12] ([i915#4258])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
    - bat-dg2-9:          [PASS][13] -> [DMESG-FAIL][14] ([i915#6998] / [i915#7913])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/bat-dg2-9/igt@i915_selftest@live@hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-dg2-9/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@migrate:
    - bat-dg2-11:         [PASS][15] -> [DMESG-WARN][16] ([i915#7699])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/bat-dg2-11/igt@i915_selftest@live@migrate.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-dg2-11/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][17] ([i915#6367] / [i915#7913])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@i915_selftest@live@slpc.html
    - bat-rpls-1:         NOTRUN -> [DMESG-FAIL][18] ([i915#6367] / [i915#7996])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][19] ([i915#7978])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@i915_suspend@basic-s3-without-i915.html
    - bat-rpls-1:         NOTRUN -> [ABORT][20] ([i915#6687] / [i915#7978])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_busy@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][21] ([i915#1845]) +14 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_busy@basic.html

  * igt@kms_chamelium_edid@hdmi-edid-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][22] ([i915#7828]) +7 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_chamelium_edid@hdmi-edid-read.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - bat-rpls-2:         NOTRUN -> [SKIP][23] ([i915#3637]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-rpls-2:         NOTRUN -> [SKIP][24] ([fdo#109285])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][25] ([i915#1849])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rpls-2:         NOTRUN -> [SKIP][26] ([i915#1072]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-2:         NOTRUN -> [SKIP][27] ([i915#3555])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-rpls-2:         NOTRUN -> [SKIP][28] ([fdo#109295] / [i915#1845] / [i915#3708])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][29] ([fdo#109295] / [i915#3708]) +3 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-2/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [DMESG-FAIL][30] ([i915#5334]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@requests:
    - fi-kbl-soraka:      [ABORT][32] ([i915#7913]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/fi-kbl-soraka/igt@i915_selftest@live@requests.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/fi-kbl-soraka/igt@i915_selftest@live@requests.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [ABORT][34] ([i915#4983]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/bat-rpls-1/igt@i915_selftest@live@reset.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rpls-1/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - bat-rplp-1:         [DMESG-FAIL][36] ([i915#6367] / [i915#7913]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/bat-rplp-1/igt@i915_selftest@live@slpc.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/bat-rplp-1/igt@i915_selftest@live@slpc.html

  
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6998]: https://gitlab.freedesktop.org/drm/intel/issues/6998
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
  [i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308


Build changes
-------------

  * Linux: CI_DRM_12939 -> Patchwork_115863v1

  CI-20190529: 20190529
  CI_DRM_12939: 67f2a2f9902dad389232eee48db7073d66011582 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7227: 5dfa3675c36bab82cdda056db7ef622363c324ee @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115863v1: 67f2a2f9902dad389232eee48db7073d66011582 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

ed7b2f352bc5 drm/i915/reg: use is_power_of_2() from log2.h
97bc6258c1fc log2: allow use of is_power_of_2() in constant expressions
2d4a91afcc50 log2: have is_power_of_2() support bigger types than unsigned long
90cce1adad92 log2: add helper __IS_POWER_OF_2()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/index.html

[-- Attachment #2: Type: text/html, Size: 12640 bytes --]

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Andrew Morton]

On Thu, 30 Mar 2023 13:42:39 +0300 Jani Nikula <jani.nikula@intel.com> wrote:

> is_power_of_2() only works for types <= sizeof(unsigned long) and it's
> also not a constant expression. There are a number of places in kernel
> where is_power_of_2() is called on u64, which fails on 32-bit
> builds. Try to remedy that. While at it, make it a constant expression
> when possible.

Yes, the current `is_power_of_2(unsigned long n)' isn't very general.

But wouldn't all these issues be addressed by simply doing

#define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))

?

(With suitable tweaks to avoid evaluating `n' more than once)

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[David Laight]

From: Andrew Morton
> Sent: 30 March 2023 20:51
> 
> On Thu, 30 Mar 2023 13:42:39 +0300 Jani Nikula <jani.nikula@intel.com> wrote:
> 
> > is_power_of_2() only works for types <= sizeof(unsigned long) and it's
> > also not a constant expression. There are a number of places in kernel
> > where is_power_of_2() is called on u64, which fails on 32-bit
> > builds. Try to remedy that. While at it, make it a constant expression
> > when possible.
> 
> Yes, the current `is_power_of_2(unsigned long n)' isn't very general.
> 
> But wouldn't all these issues be addressed by simply doing
> 
> #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
> 
> ?
> 
> (With suitable tweaks to avoid evaluating `n' more than once)

I think you need to use the 'horrid tricks' from min() to get
a constant expression from constant inputs.

For non-constants this looks ok (see https://godbolt.org/z/G73MTr9jn)

	David

static inline int lisp2(unsigned long n)
{
    return n && !(n & (n - 1));
}

static inline int llisp2(unsigned long long lln)
{
#if 0  // I think this looks worse, esp. for gcc on x86
    return lln && !(lln & (lln - 1));
#else
    unsigned long n = lln;
    if (lln >= 1ull << 32) {
        if (n)
            return 0;
        n = lln >> 32; 
    }
    return lisp2(n);
#endif
}

#define isp2(n) (sizeof ((n)+0) == sizeof (long) ? lisp2(n) : llisp2(n))

int is12(unsigned long long  i)
{
    return isp2(i);
}

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Andrew Morton]

On Thu, 30 Mar 2023 21:53:03 +0000 David Laight <David.Laight@ACULAB.COM> wrote:

> > But wouldn't all these issues be addressed by simply doing
> > 
> > #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
> > 
> > ?
> > 
> > (With suitable tweaks to avoid evaluating `n' more than once)
> 
> I think you need to use the 'horrid tricks' from min() to get
> a constant expression from constant inputs.

This

--- a/include/linux/log2.h~a
+++ a/include/linux/log2.h
@@ -41,11 +41,11 @@ int __ilog2_u64(u64 n)
  * *not* considered a power of two.
  * Return: true if @n is a power of 2, otherwise false.
  */
-static inline __attribute__((const))
-bool is_power_of_2(unsigned long n)
-{
-	return (n != 0 && ((n & (n - 1)) == 0));
-}
+#define is_power_of_2(_n)				\
+	({						\
+		typeof(_n) n = (_n);			\
+		n != 0 && ((n & (n - 1)) == 0);		\
+	})
 
 /**
  * __roundup_pow_of_two() - round up to nearest power of two
_

worked for me in a simple test.

--- a/fs/open.c~b
+++ a/fs/open.c
@@ -1564,3 +1564,10 @@ int stream_open(struct inode *inode, str
 }
 
 EXPORT_SYMBOL(stream_open);
+
+#include <linux/log2.h>
+
+int foo(void)
+{
+	return is_power_of_2(43);
+}
_


foo:
# fs/open.c:1573: }
	xorl	%eax, %eax	#
	ret	


Is there some more tricky situation where it breaks?

[Intel-gfx] ✓ Fi.CI.IGT: success for log2: make is_power_of_2() more generic

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 11799 bytes --]

== Series Details ==

Series: log2: make is_power_of_2() more generic
URL   : https://patchwork.freedesktop.org/series/115863/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12939_full -> Patchwork_115863v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/index.html

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_115863v1_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_frontbuffer_tracking@fbc-indfb-scaledprimary:
    - {shard-dg1}:        [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-dg1-14/igt@kms_frontbuffer_tracking@fbc-indfb-scaledprimary.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-dg1-16/igt@kms_frontbuffer_tracking@fbc-indfb-scaledprimary.html

  
Known issues
------------

  Here are the changes found in Patchwork_115863v1_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_fair@basic-deadline:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#2846])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-apl4/igt@gem_exec_fair@basic-deadline.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-apl4/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([i915#2842])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([i915#2842])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-glk3/igt@gem_exec_fair@basic-pace@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk8/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_lmem_swapping@random:
    - shard-glk:          NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#4613]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk7/igt@gem_lmem_swapping@random.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-glk:          NOTRUN -> [WARN][10] ([i915#2658])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk7/igt@gem_pwrite@basic-exhaustion.html

  * igt@i915_pm_rps@reset:
    - shard-snb:          [PASS][11] -> [INCOMPLETE][12] ([i915#7790])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-snb2/igt@i915_pm_rps@reset.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-snb5/igt@i915_pm_rps@reset.html

  * igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#3886]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk7/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-hdmi-a-1:
    - shard-glk:          NOTRUN -> [SKIP][14] ([fdo#109271]) +48 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk7/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-hdmi-a-1.html

  
#### Possible fixes ####

  * igt@drm_read@short-buffer-wakeup:
    - shard-snb:          [SKIP][15] ([fdo#109271]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-snb5/igt@drm_read@short-buffer-wakeup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-snb7/igt@drm_read@short-buffer-wakeup.html

  * igt@gem_exec_endless@dispatch@vecs0:
    - {shard-tglu}:       [TIMEOUT][17] ([i915#3778]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-tglu-10/igt@gem_exec_endless@dispatch@vecs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-tglu-5/igt@gem_exec_endless@dispatch@vecs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - {shard-tglu}:       [FAIL][19] ([i915#2842]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-tglu-3/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-tglu-10/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_render_copy@y-tiled-ccs-to-y-tiled@smem:
    - shard-glk:          [DMESG-WARN][21] ([i915#118]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-glk7/igt@gem_render_copy@y-tiled-ccs-to-y-tiled@smem.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk2/igt@gem_render_copy@y-tiled-ccs-to-y-tiled@smem.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][23] ([i915#2346]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [FAIL][25] ([i915#2346]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-apl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-apl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [FAIL][27] ([i915#4767]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-apl2/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1:
    - shard-glk:          [FAIL][29] ([i915#79]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12939/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1257]: https://gitlab.freedesktop.org/drm/intel/issues/1257
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
  [i915#4884]: https://gitlab.freedesktop.org/drm/intel/issues/4884
  [i915#4885]: https://gitlab.freedesktop.org/drm/intel/issues/4885
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7790]: https://gitlab.freedesktop.org/drm/intel/issues/7790
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981


Build changes
-------------

  * Linux: CI_DRM_12939 -> Patchwork_115863v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12939: 67f2a2f9902dad389232eee48db7073d66011582 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7227: 5dfa3675c36bab82cdda056db7ef622363c324ee @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_115863v1: 67f2a2f9902dad389232eee48db7073d66011582 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_115863v1/index.html

[-- Attachment #2: Type: text/html, Size: 9917 bytes --]

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[David Laight]

From: Andrew Morton
> Sent: 30 March 2023 23:19
> 
> On Thu, 30 Mar 2023 21:53:03 +0000 David Laight <David.Laight@ACULAB.COM> wrote:
> 
> > > But wouldn't all these issues be addressed by simply doing
> > >
> > > #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
> > >
> > > ?
> > >
> > > (With suitable tweaks to avoid evaluating `n' more than once)
> >
> > I think you need to use the 'horrid tricks' from min() to get
> > a constant expression from constant inputs.
> 
> This
> 
> --- a/include/linux/log2.h~a
> +++ a/include/linux/log2.h
> @@ -41,11 +41,11 @@ int __ilog2_u64(u64 n)
>   * *not* considered a power of two.
>   * Return: true if @n is a power of 2, otherwise false.
>   */
> -static inline __attribute__((const))
> -bool is_power_of_2(unsigned long n)
> -{
> -	return (n != 0 && ((n & (n - 1)) == 0));
> -}
> +#define is_power_of_2(_n)				\
> +	({						\
> +		typeof(_n) n = (_n);			\
> +		n != 0 && ((n & (n - 1)) == 0);		\
> +	})
> 
>  /**
>   * __roundup_pow_of_two() - round up to nearest power of two
> _
> 
> worked for me in a simple test.
> 
> --- a/fs/open.c~b
> +++ a/fs/open.c
> @@ -1564,3 +1564,10 @@ int stream_open(struct inode *inode, str
>  }
> 
>  EXPORT_SYMBOL(stream_open);
> +
> +#include <linux/log2.h>
> +
> +int foo(void)
> +{
> +	return is_power_of_2(43);
> +}
> _
> 
> 
> foo:
> # fs/open.c:1573: }
> 	xorl	%eax, %eax	#
> 	ret
> 
> 
> Is there some more tricky situation where it breaks?

Try:
static int x = is_power_of_2(43);

I suspect that some (all?) of the compile-time assert checks won't
like ({...}) either.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Jani Nikula]

On Thu, 30 Mar 2023, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 30 Mar 2023 21:53:03 +0000 David Laight <David.Laight@ACULAB.COM> wrote:
>
>> > But wouldn't all these issues be addressed by simply doing
>> > 
>> > #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
>> > 
>> > ?
>> > 
>> > (With suitable tweaks to avoid evaluating `n' more than once)
>> 
>> I think you need to use the 'horrid tricks' from min() to get
>> a constant expression from constant inputs.
>
> This
>
> --- a/include/linux/log2.h~a
> +++ a/include/linux/log2.h
> @@ -41,11 +41,11 @@ int __ilog2_u64(u64 n)
>   * *not* considered a power of two.
>   * Return: true if @n is a power of 2, otherwise false.
>   */
> -static inline __attribute__((const))
> -bool is_power_of_2(unsigned long n)
> -{
> -	return (n != 0 && ((n & (n - 1)) == 0));
> -}
> +#define is_power_of_2(_n)				\
> +	({						\
> +		typeof(_n) n = (_n);			\
> +		n != 0 && ((n & (n - 1)) == 0);		\
> +	})
>  
>  /**
>   * __roundup_pow_of_two() - round up to nearest power of two
> _
>
> worked for me in a simple test.
>
> --- a/fs/open.c~b
> +++ a/fs/open.c
> @@ -1564,3 +1564,10 @@ int stream_open(struct inode *inode, str
>  }
>  
>  EXPORT_SYMBOL(stream_open);
> +
> +#include <linux/log2.h>
> +
> +int foo(void)
> +{
> +	return is_power_of_2(43);
> +}
> _
>
>
> foo:
> # fs/open.c:1573: }
> 	xorl	%eax, %eax	#
> 	ret	
>
>
> Is there some more tricky situation where it breaks?

It doesn't work with BUILD_BUG_ON_ZERO().

test.c:
#define IS_POWER_OF_2(_n)				\
	({						\
		typeof(_n) n = (_n);			\
		n != 0 && ((n & (n - 1)) == 0);		\
	})

#define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))

#define FOO(n) ((n) + BUILD_BUG_ON_ZERO(!IS_POWER_OF_2(n)))

int main(void)
{
	return FOO(2);
}

$ gcc test.c
test.c: In function ‘main’:
test.c:16:51: error: bit-field ‘<anonymous>’ width not an integer constant
   16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
      |                                                   ^
test.c:18:23: note: in expansion of macro ‘BUILD_BUG_ON_ZERO’
   18 | #define FOO(n) ((n) + BUILD_BUG_ON_ZERO(!IS_POWER_OF_2(n)))
      |                       ^~~~~~~~~~~~~~~~~
test.c:22:9: note: in expansion of macro ‘FOO’
   22 |  return FOO(2);
      |         ^~~


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH 0/4] log2: make is_power_of_2() more generic

[Steven Price]

On 31/03/2023 09:31, Jani Nikula wrote:
> On Thu, 30 Mar 2023, Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Thu, 30 Mar 2023 21:53:03 +0000 David Laight <David.Laight@ACULAB.COM> wrote:
>>
>>>> But wouldn't all these issues be addressed by simply doing
>>>>
>>>> #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
>>>>
>>>> ?
>>>>
>>>> (With suitable tweaks to avoid evaluating `n' more than once)
>>>
>>> I think you need to use the 'horrid tricks' from min() to get
>>> a constant expression from constant inputs.
>>
>> This
>>
>> --- a/include/linux/log2.h~a
>> +++ a/include/linux/log2.h
>> @@ -41,11 +41,11 @@ int __ilog2_u64(u64 n)
>>   * *not* considered a power of two.
>>   * Return: true if @n is a power of 2, otherwise false.
>>   */
>> -static inline __attribute__((const))
>> -bool is_power_of_2(unsigned long n)
>> -{
>> -	return (n != 0 && ((n & (n - 1)) == 0));
>> -}
>> +#define is_power_of_2(_n)				\
>> +	({						\
>> +		typeof(_n) n = (_n);			\
>> +		n != 0 && ((n & (n - 1)) == 0);		\
>> +	})
>>  
>>  /**
>>   * __roundup_pow_of_two() - round up to nearest power of two
>> _
>>
>> worked for me in a simple test.
>>
>> --- a/fs/open.c~b
>> +++ a/fs/open.c
>> @@ -1564,3 +1564,10 @@ int stream_open(struct inode *inode, str
>>  }
>>  
>>  EXPORT_SYMBOL(stream_open);
>> +
>> +#include <linux/log2.h>
>> +
>> +int foo(void)
>> +{
>> +	return is_power_of_2(43);
>> +}
>> _
>>
>>
>> foo:
>> # fs/open.c:1573: }
>> 	xorl	%eax, %eax	#
>> 	ret	
>>
>>
>> Is there some more tricky situation where it breaks?
> 
> It doesn't work with BUILD_BUG_ON_ZERO().

Like most programming problems, you just need another layer of
indirection! The below works for me in all the cases I could think of
(including __uint128_t).


#define __IS_POWER_OF_2(n) (n != 0 && ((n & (n - 1)) == 0))

#define _IS_POWER_OF_2(n, unique_n)				\
	({							\
		typeof(n) unique_n = (n);			\
		__IS_POWER_OF_2(unique_n);			\
	})

#define is_power_of_2(n)					\
	__builtin_choose_expr(__is_constexpr((n)),		\
			      __IS_POWER_OF_2((n)),		\
			      _IS_POWER_OF_2(n, __UNIQUE_ID(_n)))


Although Jani's original might be easier to understand.

Steve

Re: [PATCH 0/4] log2: make is_power_of_2() more generic

[Jani Nikula]

On Wed, 05 Apr 2023, Steven Price <steven.price@arm.com> wrote:
> On 31/03/2023 09:31, Jani Nikula wrote:
>> On Thu, 30 Mar 2023, Andrew Morton <akpm@linux-foundation.org> wrote:
>>> On Thu, 30 Mar 2023 21:53:03 +0000 David Laight <David.Laight@ACULAB.COM> wrote:
>>>
>>>>> But wouldn't all these issues be addressed by simply doing
>>>>>
>>>>> #define is_power_of_2(n) (n != 0 && ((n & (n - 1)) == 0))
>>>>>
>>>>> ?
>>>>>
>>>>> (With suitable tweaks to avoid evaluating `n' more than once)
>>>>
>>>> I think you need to use the 'horrid tricks' from min() to get
>>>> a constant expression from constant inputs.
>>>
>>> This
>>>
>>> --- a/include/linux/log2.h~a
>>> +++ a/include/linux/log2.h
>>> @@ -41,11 +41,11 @@ int __ilog2_u64(u64 n)
>>>   * *not* considered a power of two.
>>>   * Return: true if @n is a power of 2, otherwise false.
>>>   */
>>> -static inline __attribute__((const))
>>> -bool is_power_of_2(unsigned long n)
>>> -{
>>> -	return (n != 0 && ((n & (n - 1)) == 0));
>>> -}
>>> +#define is_power_of_2(_n)				\
>>> +	({						\
>>> +		typeof(_n) n = (_n);			\
>>> +		n != 0 && ((n & (n - 1)) == 0);		\
>>> +	})
>>>  
>>>  /**
>>>   * __roundup_pow_of_two() - round up to nearest power of two
>>> _
>>>
>>> worked for me in a simple test.
>>>
>>> --- a/fs/open.c~b
>>> +++ a/fs/open.c
>>> @@ -1564,3 +1564,10 @@ int stream_open(struct inode *inode, str
>>>  }
>>>  
>>>  EXPORT_SYMBOL(stream_open);
>>> +
>>> +#include <linux/log2.h>
>>> +
>>> +int foo(void)
>>> +{
>>> +	return is_power_of_2(43);
>>> +}
>>> _
>>>
>>>
>>> foo:
>>> # fs/open.c:1573: }
>>> 	xorl	%eax, %eax	#
>>> 	ret	
>>>
>>>
>>> Is there some more tricky situation where it breaks?
>> 
>> It doesn't work with BUILD_BUG_ON_ZERO().
>
> Like most programming problems, you just need another layer of
> indirection! The below works for me in all the cases I could think of
> (including __uint128_t).
>
>
> #define __IS_POWER_OF_2(n) (n != 0 && ((n & (n - 1)) == 0))
>
> #define _IS_POWER_OF_2(n, unique_n)				\
> 	({							\
> 		typeof(n) unique_n = (n);			\
> 		__IS_POWER_OF_2(unique_n);			\
> 	})
>
> #define is_power_of_2(n)					\
> 	__builtin_choose_expr(__is_constexpr((n)),		\
> 			      __IS_POWER_OF_2((n)),		\
> 			      _IS_POWER_OF_2(n, __UNIQUE_ID(_n)))
>
>
> Although Jani's original might be easier to understand.

I dropped the ball since I couldn't make heads or tails what I should be
doing. And a year has passed. I'll note that the kernel has a number of
helpers for "is power of 2" for u64 and for constant expressions,
outside of log2.h.

I tried to make is_power_of_2() work for all the cases. Would it be more
palatable if I just added all the variants separately to log2.h?

- Leave is_power_of_2() as is
- Add is_power_of_2_u64() for 32-bit build compatible 64-bit checks
- Add IS_POWER_OF_2() macro for constant expressions

Please just tell me what to do and I'll do it.

BR,
Jani.


-- 
Jani Nikula, Intel