| Message ID | 20180502172142.3661-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-05-02 20:21:42) > I caught the compiler emitting the if(!NULL) guard at the start of > dma_fence_put(); on the request it should know for certain is already > non-NULL. Mark up the function as non-null and tell the compiler that > the request pointer doesn't change: > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5 (-5) > Function old new delta > i915_request_retire 1782 1777 -5 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
Quoting Joonas Lahtinen (2018-05-03 14:59:35) > Quoting Chris Wilson (2018-05-02 20:21:42) > > I caught the compiler emitting the if(!NULL) guard at the start of > > dma_fence_put(); on the request it should know for certain is already > > non-NULL. Mark up the function as non-null and tell the compiler that > > the request pointer doesn't change: > > > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5 (-5) > > Function old new delta > > i915_request_retire 1782 1777 -5 > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Heh, Tvrtko out Joonased Jonas, as he complained that this was doing the compiler's job: "where does it end?" -Chris
On 03/05/2018 15:06, Chris Wilson wrote: > Quoting Joonas Lahtinen (2018-05-03 14:59:35) >> Quoting Chris Wilson (2018-05-02 20:21:42) >>> I caught the compiler emitting the if(!NULL) guard at the start of >>> dma_fence_put(); on the request it should know for certain is already >>> non-NULL. Mark up the function as non-null and tell the compiler that >>> the request pointer doesn't change: >>> >>> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5 (-5) >>> Function old new delta >>> i915_request_retire 1782 1777 -5 >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Heh, Tvrtko out Joonased Jonas, as he complained that this was doing the > compiler's job: "where does it end?" Yeah, Joonas is getting soft. ;) My concern was that we could then go around annotating many function arguments which cannot take NULLs. (Which I think we should not do.) So it seemed random to annotate just one for one branch saved, with potentially just one compiler version/whatever. Just my 2c. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 3bcb75742110..d54c99211c8c 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -405,7 +405,8 @@ static void __retire_engine_upto(struct intel_engine_cs *engine, } while (tmp != rq); } -static void i915_request_retire(struct i915_request *request) +__attribute__((nonnull)) +static void i915_request_retire(struct i915_request * const request) { struct i915_gem_active *active, *next;
I caught the compiler emitting the if(!NULL) guard at the start of dma_fence_put(); on the request it should know for certain is already non-NULL. Mark up the function as non-null and tell the compiler that the request pointer doesn't change: add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-5 (-5) Function old new delta i915_request_retire 1782 1777 -5 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_request.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)