Re: [PATCH] drm/i915/pmu: Fix build error with GCOV and AutoFDO enabled

[Nathan Chancellor <nathan@kernel.org>]

On Thu, May 29, 2025 at 04:29:10AM +0000, Tzung-Bi Shih wrote:
> i915_pmu.c may fail to build with GCOV and AutoFDO enabled.
> 
> ../drivers/gpu/drm/i915/i915_pmu.c:116:3: error: call to '__compiletime_assert_487' declared with 'error' attribute: BUILD_BUG_ON failed: bit > BITS_PER_TYPE(typeof_member(struct i915_pmu, enable)) - 1
>   116 |                 BUILD_BUG_ON(bit >
>       |                 ^
> 
> Here is a way to reproduce the issue:
> $ git checkout v6.15
> $ mkdir build
> $ ./scripts/kconfig/merge_config.sh -O build -n -m <(cat <<EOF
> CONFIG_DRM=y
> CONFIG_PCI=y
> CONFIG_DRM_I915=y
> 
> CONFIG_PERF_EVENTS=y
> 
> CONFIG_DEBUG_FS=y
> CONFIG_GCOV_KERNEL=y
> CONFIG_GCOV_PROFILE_ALL=y
> 
> CONFIG_AUTOFDO_CLANG=y
> EOF
> )
> $ PATH=${PATH}:${HOME}/llvm-20.1.5-x86_64/bin make LLVM=1 O=build \
>        olddefconfig
> $ PATH=${PATH}:${HOME}/llvm-20.1.5-x86_64/bin make LLVM=1 O=build \
>        CLANG_AUTOFDO_PROFILE=...PATH_TO_SOME_AFDO_PROFILE... \
>        drivers/gpu/drm/i915/i915_pmu.o
> 
> Although not super sure what happened, by reviewing the code, it should
> depend on `__builtin_constant_p(bit)` directly instead of assuming
> `__builtin_constant_p(config)` makes `bit` a builtin constant.
> 
> Also fix a nit, to reuse the `bit` local variable.
> 
> Fixes: a644fde77ff7 ("drm/i915/pmu: Change bitmask of enabled events to u32")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>

This seems like a reasonable fix, as it is likely that these
configurations cause config_bit() not to be inlined due to
instrumentation being added. If config_bit() is not inlined, bit will
not be known at compile time, triggering the compiletime error because
the condition cannot be proven false at that point.

Marking config_bit(), is_engine_config(), engine_config_sample(),
other_bit(), config_counter(), and config_gt_id() all as __always_inline
might resolve this as well but I cannot say if that is worth it. I guess
it depends on how often this check is likely to fire.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index e5a188ce3185..990bfaba3ce4 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -112,7 +112,7 @@ static u32 config_mask(const u64 config)
>  {
>  	unsigned int bit = config_bit(config);
>  
> -	if (__builtin_constant_p(config))
> +	if (__builtin_constant_p(bit))
>  		BUILD_BUG_ON(bit >
>  			     BITS_PER_TYPE(typeof_member(struct i915_pmu,
>  							 enable)) - 1);
> @@ -121,7 +121,7 @@ static u32 config_mask(const u64 config)
>  			     BITS_PER_TYPE(typeof_member(struct i915_pmu,
>  							 enable)) - 1);
>  
> -	return BIT(config_bit(config));
> +	return BIT(bit);
>  }
>  
>  static bool is_engine_event(struct perf_event *event)
> -- 
> 2.49.0.1266.g31b7d2e469-goog
> 
>