Re: [RFC 14/15] drm/i915: Use new IS_GEN range helpers

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Feb 08, 2018 at 05:13:02PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Tvrtko Ursulin (2018-02-08 14:34:38)
> >> 
> >> On 08/02/2018 14:22, Ville Syrjälä wrote:
> >> > On Thu, Feb 08, 2018 at 01:06:05PM +0000, Tvrtko Ursulin wrote:
> >> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> >>
> >> >> Coccinelle transformation:
> >> >>
> >> >>   @@
> >> >>   expression p, g;
> >> >>   @@
> >> >>   (
> >> >>   -INTEL_GEN(p) > g
> >> >>   +IS_GEN_GT(p, g)
> >> > 
> >> > I think this stuff makes the code pretty close to illegible.
> >> > In this particular case even more so because "GT" actually
> >> > means something very different to us.
> >> 
> >> Oh how true! And I did not realize it at all while writing it! :)
> >> 
> >> Anyway, something like this, regardless of a name, is needed if people 
> >> want this to be effective. Since the checks have to be moved to known at 
> >> compile time. Or a completely different approach will be needed.
> >
> > IS_GEN_RANGE() doesn't cut it?
> >
> 
> IS_GEN_RANGE(8,9);
> 
> short and readable

'if (IS_GEN_RANGE(...))' reads funny. IS_GEN_IN_RANGE() would be more
englishy perhaps, but it looks a bit off to me for whatever reason.

And it still doesn't tell you anything about inclusive vs. exlusive.
So it just forces you to waste brain cells on mundane details when
reading the code. IMO that's a fairly bad tradeoff.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx