[PATCH 1/2] drm/i915: Squelch smatch warning for statement with no effect

[PATCH 1/2] drm/i915: Squelch smatch warning for statement with no effect

[Chris Wilson]

The sgt iterators cause an

drivers/gpu/drm/i915/i915_gpu_error.c:846 i915_error_object_create() warn: statement has no effect 7

everywhere they are used. If we change the code slightly, we can achieve
the same increment without altering the output or raising a warning.

   text	   data	    bss	    dec	    hex	filename
1267906	  20587	   3168	1291661	 13b58d	before
1267906	  20587	   3168	1291661	 13b58d	after

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1b340eb0db19..ebe2f6c737f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,8 +2620,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 #define for_each_sgt_dma(__dmap, __iter, __sgt)				\
 	for ((__iter) = __sgt_iter((__sgt)->sgl, true);			\
 	     ((__dmap) = (__iter).dma + (__iter).curr);			\
-	     (((__iter).curr += PAGE_SIZE) < (__iter).max) ||		\
-	     ((__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0))
+	     (((__iter).curr += PAGE_SIZE) >= (__iter).max) ?		\
+	     (__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0 : 0)
 
 /**
  * for_each_sgt_page - iterate over the pages of the given sg_table
@@ -2633,8 +2633,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 	for ((__iter) = __sgt_iter((__sgt)->sgl, false);		\
 	     ((__pp) = (__iter).pfn == 0 ? NULL :			\
 	      pfn_to_page((__iter).pfn + ((__iter).curr >> PAGE_SHIFT))); \
-	     (((__iter).curr += PAGE_SIZE) < (__iter).max) ||		\
-	     ((__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0))
+	     (((__iter).curr += PAGE_SIZE) >= (__iter).max) ?		\
+	     (__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0 : 0)
 
 static inline const struct intel_device_info *
 intel_info(const struct drm_i915_private *dev_priv)
-- 
2.11.0

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

[PATCH 2/2] drm/i915: Silence smatch for cmdparser

[Chris Wilson]

drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue

If we move the shift into each case not only do we kill the warning from
smatch, but we shrink the code slightly:

   text	   data	    bss	    dec	    hex	filename
1267906	  20587	   3168	1291661	 13b58d	before
1267890	  20587	   3168	1291645	 13b57d	after

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 420bbc220f35..650ee34f5786 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -800,22 +800,15 @@ struct cmd_node {
  */
 static inline u32 cmd_header_key(u32 x)
 {
-	u32 shift;
-
 	switch (x >> INSTR_CLIENT_SHIFT) {
 	default:
 	case INSTR_MI_CLIENT:
-		shift = STD_MI_OPCODE_SHIFT;
-		break;
+		return x >> STD_MI_OPCODE_SHIFT;
 	case INSTR_RC_CLIENT:
-		shift = STD_3D_OPCODE_SHIFT;
-		break;
+		return x >> STD_3D_OPCODE_SHIFT;
 	case INSTR_BC_CLIENT:
-		shift = STD_2D_OPCODE_SHIFT;
-		break;
+		return x >> STD_2D_OPCODE_SHIFT;
 	}
-
-	return x >> shift;
 }
 
 static int init_hash_table(struct intel_engine_cs *engine,
-- 
2.11.0

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

Re: [PATCH 2/2] drm/i915: Silence smatch for cmdparser

[Chris Wilson]

On Fri, Jan 20, 2017 at 09:45:30PM +0000, Chris Wilson wrote:
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> 
> If we move the shift into each case not only do we kill the warning from
> smatch, but we shrink the code slightly:
> 
>    text	   data	    bss	    dec	    hex	filename
> 1267906	  20587	   3168	1291661	 13b58d	before
> 1267890	  20587	   3168	1291645	 13b57d	after
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Any takers for this pair?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Silence smatch for cmdparser

[Jani Nikula]

On Fri, 20 Jan 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:808:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:811:23: error: not an lvalue
> drivers/gpu/drm/i915/i915_cmd_parser.c:814:23: error: not an lvalue

I don't understand the error, but the change is sane

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> If we move the shift into each case not only do we kill the warning from
> smatch, but we shrink the code slightly:
>
>    text	   data	    bss	    dec	    hex	filename
> 1267906	  20587	   3168	1291661	 13b58d	before
> 1267890	  20587	   3168	1291645	 13b57d	after
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 420bbc220f35..650ee34f5786 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -800,22 +800,15 @@ struct cmd_node {
>   */
>  static inline u32 cmd_header_key(u32 x)
>  {
> -	u32 shift;
> -
>  	switch (x >> INSTR_CLIENT_SHIFT) {
>  	default:
>  	case INSTR_MI_CLIENT:
> -		shift = STD_MI_OPCODE_SHIFT;
> -		break;
> +		return x >> STD_MI_OPCODE_SHIFT;
>  	case INSTR_RC_CLIENT:
> -		shift = STD_3D_OPCODE_SHIFT;
> -		break;
> +		return x >> STD_3D_OPCODE_SHIFT;
>  	case INSTR_BC_CLIENT:
> -		shift = STD_2D_OPCODE_SHIFT;
> -		break;
> +		return x >> STD_2D_OPCODE_SHIFT;
>  	}
> -
> -	return x >> shift;
>  }
>  
>  static int init_hash_table(struct intel_engine_cs *engine,

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Squelch smatch warning for statement with no effect

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Squelch smatch warning for statement with no effect
URL   : https://patchwork.freedesktop.org/series/18326/
State : success

== Summary ==

Series 18326v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/18326/revisions/1/mbox/


fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:79   pass:66   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

b32b21b117a110873013e5e6d4fa880b6a81a404 drm-tip: 2017y-01m-20d-20h-00m-57s UTC integration manifest
7670182 drm/i915: Silence smatch for cmdparser
a7c25bf drm/i915: Squelch smatch warning for statement with no effect

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3566/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Squelch smatch warning for statement with no effect

[Jani Nikula]

On Fri, 20 Jan 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The sgt iterators cause an
>
> drivers/gpu/drm/i915/i915_gpu_error.c:846 i915_error_object_create() warn: statement has no effect 7
>
> everywhere they are used. If we change the code slightly, we can achieve
> the same increment without altering the output or raising a warning.

I've never liked using logical OR short circuiting for assignments like
this anyway, so I think this is an improvement no matter what.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
>    text	   data	    bss	    dec	    hex	filename
> 1267906	  20587	   3168	1291661	 13b58d	before
> 1267906	  20587	   3168	1291661	 13b58d	after
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1b340eb0db19..ebe2f6c737f7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2620,8 +2620,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
>  #define for_each_sgt_dma(__dmap, __iter, __sgt)				\
>  	for ((__iter) = __sgt_iter((__sgt)->sgl, true);			\
>  	     ((__dmap) = (__iter).dma + (__iter).curr);			\
> -	     (((__iter).curr += PAGE_SIZE) < (__iter).max) ||		\
> -	     ((__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0))
> +	     (((__iter).curr += PAGE_SIZE) >= (__iter).max) ?		\
> +	     (__iter) = __sgt_iter(__sg_next((__iter).sgp), true), 0 : 0)
>  
>  /**
>   * for_each_sgt_page - iterate over the pages of the given sg_table
> @@ -2633,8 +2633,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
>  	for ((__iter) = __sgt_iter((__sgt)->sgl, false);		\
>  	     ((__pp) = (__iter).pfn == 0 ? NULL :			\
>  	      pfn_to_page((__iter).pfn + ((__iter).curr >> PAGE_SHIFT))); \
> -	     (((__iter).curr += PAGE_SIZE) < (__iter).max) ||		\
> -	     ((__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0))
> +	     (((__iter).curr += PAGE_SIZE) >= (__iter).max) ?		\
> +	     (__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0 : 0)
>  
>  static inline const struct intel_device_info *
>  intel_info(const struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx