| Message ID | 20161202132314.14878-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote: > In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this > will enable us to freely add warnings which our CI will hopefully catch > but without fear of impacting production machines. Impacting performance? > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > index 51ec793f2e20..04c777e6d0bf 100644 > --- a/drivers/gpu/drm/i915/i915_gem.h > +++ b/drivers/gpu/drm/i915/i915_gem.h > @@ -27,8 +27,10 @@ > > #ifdef CONFIG_DRM_I915_DEBUG_GEM > #define GEM_BUG_ON(expr) BUG_ON(expr) > +#define GEM_WARN_ON(expr) WARN_ON(expr) > #else > #define GEM_BUG_ON(expr) do { } while (0) > +#define GEM_WARN_ON(expr) do { } while (0) > #endif > > #define I915_NUM_ENGINES 5 > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote: > In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this > will enable us to freely add warnings which our CI will hopefully catch > but without fear of impacting production machines. > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > index 51ec793f2e20..04c777e6d0bf 100644 > --- a/drivers/gpu/drm/i915/i915_gem.h > +++ b/drivers/gpu/drm/i915/i915_gem.h > @@ -27,8 +27,10 @@ > > #ifdef CONFIG_DRM_I915_DEBUG_GEM > #define GEM_BUG_ON(expr) BUG_ON(expr) > +#define GEM_WARN_ON(expr) WARN_ON(expr) > #else > #define GEM_BUG_ON(expr) do { } while (0) > +#define GEM_WARN_ON(expr) do { } while (0) #define GEM_WARN_ON 0 i.e. so that if (GEM_WARN_ON(insane_condition()) compiles out correctly without DEBUG_GEM enabled. -Chris
On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote: >> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this >> will enable us to freely add warnings which our CI will hopefully catch >> but without fear of impacting production machines. >> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h >> index 51ec793f2e20..04c777e6d0bf 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.h >> +++ b/drivers/gpu/drm/i915/i915_gem.h >> @@ -27,8 +27,10 @@ >> >> #ifdef CONFIG_DRM_I915_DEBUG_GEM >> #define GEM_BUG_ON(expr) BUG_ON(expr) >> +#define GEM_WARN_ON(expr) WARN_ON(expr) >> #else >> #define GEM_BUG_ON(expr) do { } while (0) >> +#define GEM_WARN_ON(expr) do { } while (0) > > #define GEM_WARN_ON 0 Oops. As an alternative, would you be offended with something like: #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0) Then the compiler will still check the validity of the expression, but will still compile everything out, and then we don't accidentally break the build when compiling without DEBUG_GEM ?
On Fri, Dec 02, 2016 at 03:11:05PM +0000, Matthew Auld wrote: > On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote: > >> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this > >> will enable us to freely add warnings which our CI will hopefully catch > >> but without fear of impacting production machines. > >> > >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > >> Signed-off-by: Matthew Auld <matthew.auld@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_gem.h | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > >> index 51ec793f2e20..04c777e6d0bf 100644 > >> --- a/drivers/gpu/drm/i915/i915_gem.h > >> +++ b/drivers/gpu/drm/i915/i915_gem.h > >> @@ -27,8 +27,10 @@ > >> > >> #ifdef CONFIG_DRM_I915_DEBUG_GEM > >> #define GEM_BUG_ON(expr) BUG_ON(expr) > >> +#define GEM_WARN_ON(expr) WARN_ON(expr) > >> #else > >> #define GEM_BUG_ON(expr) do { } while (0) > >> +#define GEM_WARN_ON(expr) do { } while (0) > > > > #define GEM_WARN_ON 0 > Oops. As an alternative, would you be offended with something like: > > #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0) > > Then the compiler will still check the validity of the expression, but > will still compile everything out, and then we don't accidentally > break the build when compiling without DEBUG_GEM ? Ooh, new shiny. Uses sizeof()! Can you prepare a patch to fixup GEM_BUG_ON as well? -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index 51ec793f2e20..04c777e6d0bf 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -27,8 +27,10 @@ #ifdef CONFIG_DRM_I915_DEBUG_GEM #define GEM_BUG_ON(expr) BUG_ON(expr) +#define GEM_WARN_ON(expr) WARN_ON(expr) #else #define GEM_BUG_ON(expr) do { } while (0) +#define GEM_WARN_ON(expr) do { } while (0) #endif #define I915_NUM_ENGINES 5
In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this will enable us to freely add warnings which our CI will hopefully catch but without fear of impacting production machines. Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_gem.h | 2 ++ 1 file changed, 2 insertions(+)