Re: [RFC] drm/i915: Introduce GEM_MAYBE_BUILD_BUG_ON

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/04/2017 14:57, Michal Wajdeczko wrote:
> This is slightly weaker version of MAYBE_BUILD_BUG_ON that allows
> us to avoid adding implicit BUG but still detect as much as possible
> during the build. With this new macro we can fix the problem with
> GCC 4.4.7 that wrongly triggers build break in wait_for_atomic()
> when invoked with non-const parameter.
> 
> Fixes: 1d1a9774 ("drm/i915: Extend intel_wait_for_register_fw() with fast timeout")
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.h  | 9 +++++++++
>  drivers/gpu/drm/i915/intel_drv.h | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 5a49487..ce23cf1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -42,6 +42,15 @@
>  #define GEM_DEBUG_BUG_ON(expr)
>  #endif
>  
> +#define GEM_MAYBE_BUILD_BUG_ON(expr) \
> +	do { \
> +		if (__builtin_constant_p((expr))) \
> +			BUILD_BUG_ON(expr); \
> +		else \
> +			GEM_BUG_ON(expr); \
> +	} while (0)
> +
> +
>  #define I915_NUM_ENGINES 5
>  
>  #endif /* __I915_GEM_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7bc0c25..ce50ec4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -88,7 +88,7 @@
>  	int cpu, ret, timeout = (US) * 1000; \
>  	u64 base; \
>  	_WAIT_FOR_ATOMIC_CHECK(ATOMIC); \
> -	BUILD_BUG_ON((US) > 50000); \
> +	GEM_MAYBE_BUILD_BUG_ON((US) > 50000); \
>  	if (!(ATOMIC)) { \
>  		preempt_disable(); \
>  		cpu = smp_processor_id(); \
> 

The choice is between this and this:

-#define wait_for_atomic(COND, MS)	_wait_for_atomic((COND), (MS) * 1000, 1)
-#define wait_for_atomic_us(COND, US)	_wait_for_atomic((COND), (US), 1)
+#define wait_for_atomic(COND, MS) \
+({ \
+	int ret__; \
+	BUILD_BUG_ON((MS) > 50); \
+	ret__ = _wait_for_atomic((COND), (MS) * 1000, 1); \
+	ret__; \
+})
+
+#define wait_for_atomic_us(COND, US) \
+({ \
+	int ret__; \
+	BUILD_BUG_ON((US) > 50000); \
+	ret__ = _wait_for_atomic((COND), (US), 1); \
+	ret__; \
+})

Correct? Both would fix the GCC 4.4 issue?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx