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 <tzun...@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 <nat...@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
>
>
