[PATCH i-g-t 1/3] lib: Add swap() macro

[PATCH i-g-t 1/3] lib: Add swap() macro

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

swap() will swap its two arguments while keeping the required
tmp variable hidden. Makes for neater code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_aux.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 6c83c53..63e1b06 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
 #define min(a, b) ((a) < (b) ? (a) : (b))
 #define max(a, b) ((a) > (b) ? (a) : (b))
 
+#define swap(a, b) do {		\
+	typeof(a) _tmp = (a);	\
+	(a) = (b);		\
+	(b) = _tmp;		\
+} while (0)
+
 #endif /* IGT_AUX_H */
-- 
2.0.4

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

[PATCH i-g-t 2/3] lib/igt.cocci: Deal with min/max/swap

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Replace open coded min/max/swap with the macro invocation.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt.cocci | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/lib/igt.cocci b/lib/igt.cocci
index adebb31..0d337bf 100644
--- a/lib/igt.cocci
+++ b/lib/igt.cocci
@@ -91,3 +91,39 @@ expression E;
 @@
 - assert(E);
 + igt_assert(E);
+
+// Replace open-coded swap()
+@@
+type T;
+T a, b, tmp;
+@@
+- tmp = a;
+- a = b;
+- b = tmp;
++ swap(a, b);
+
+// Replace open-coded min()
+@@
+expression a;
+expression b;
+@@
+(
+- ((a) < (b) ? (a) : (b))
++ min(a, b)
+|
+- ((a) <= (b) ? (a) : (b))
++ min(a, b)
+)
+
+// Replace open-coded max()
+@@
+expression a;
+expression b;
+@@
+(
+- ((a) > (b) ? (a) : (b))
++ max(a, b)
+|
+- ((a) >= (b) ? (a) : (b))
++ max(a, b)
+)
-- 
2.0.4

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

[PATCH i-g-t 3/3] tests: Run lib/igt.cocci

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Found some open coded min()/max()/swap() macros.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/drv_hangman.c     |  4 ++--
 tests/eviction_common.c |  5 +----
 tests/gem_seqno_wrap.c  |  5 +----
 tests/gem_stress.c      |  7 ++-----
 tests/kms_flip.c        | 12 +++---------
 5 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index cdbded2..ec28c9d 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -33,6 +33,7 @@
 
 #include "intel_chipset.h"
 #include "drmtest.h"
+#include "igt_aux.h"
 #include "igt_debugfs.h"
 #include "ioctl_wrappers.h"
 
@@ -117,8 +118,7 @@ static void read_dfs(const char *fname, char *d, int maxlen)
 static void _assert_dfs_entry(const char *fname, const char *s, bool inverse)
 {
 	char tmp[1024];
-	const int l = strlen(s) < sizeof(tmp) ?
-		strlen(s) : sizeof(tmp);
+	const int l = min(strlen(s), sizeof(tmp));
 
 	read_dfs(fname, tmp, l + 1);
 	if (!inverse) {
diff --git a/tests/eviction_common.c b/tests/eviction_common.c
index 52578e2..4a12dcb 100644
--- a/tests/eviction_common.c
+++ b/tests/eviction_common.c
@@ -54,11 +54,8 @@ struct igt_eviction_test_ops
 static void exchange_uint32_t(void *array, unsigned i, unsigned j)
 {
 	uint32_t *i_arr = array;
-	uint32_t i_tmp;
 
-	i_tmp = i_arr[i];
-	i_arr[i] = i_arr[j];
-	i_arr[j] = i_tmp;
+	swap(i_arr[i], i_arr[j]);
 }
 
 static int minor_evictions(int fd, struct igt_eviction_test_ops *ops,
diff --git a/tests/gem_seqno_wrap.c b/tests/gem_seqno_wrap.c
index be4ab3d..7da7fdf 100644
--- a/tests/gem_seqno_wrap.c
+++ b/tests/gem_seqno_wrap.c
@@ -171,11 +171,8 @@ static void render_copyfunc(struct igt_buf *src,
 static void exchange_uint(void *array, unsigned i, unsigned j)
 {
 	unsigned *i_arr = array;
-	unsigned i_tmp;
 
-	i_tmp = i_arr[i];
-	i_arr[i] = i_arr[j];
-	i_arr[j] = i_tmp;
+	swap(i_arr[i], i_arr[j]);
 }
 
 static void run_sync_test(int num_buffers, bool verify)
diff --git a/tests/gem_stress.c b/tests/gem_stress.c
index 6e3a64c..9f20bde 100644
--- a/tests/gem_stress.c
+++ b/tests/gem_stress.c
@@ -570,11 +570,8 @@ static void init_set(unsigned set)
 static void exchange_uint(void *array, unsigned i, unsigned j)
 {
 	unsigned *i_arr = array;
-	unsigned i_tmp;
 
-	i_tmp = i_arr[i];
-	i_arr[i] = i_arr[j];
-	i_arr[j] = i_tmp;
+	swap(i_arr[i], i_arr[j]);
 }
 
 static void copy_tiles(unsigned *permutation)
@@ -741,7 +738,7 @@ static void init(void)
 
 	if (options.num_buffers == 0) {
 		tmp = gem_aperture_size(drm_fd);
-		tmp = tmp > 256*(1024*1024) ? 256*(1024*1024) : tmp;
+		tmp = min(256 * (1024 * 1024), tmp);
 		num_buffers = 2 * tmp / options.scratch_buf_size / 3;
 		num_buffers /= 2;
 		igt_info("using %u buffers\n", num_buffers);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 041f46a..e579ce0 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -165,7 +165,7 @@ static unsigned long gettime_us(void)
 static void emit_dummy_load__bcs(struct test_output *o)
 {
 	int i, limit;
-	drm_intel_bo *dummy_bo, *target_bo, *tmp_bo;
+	drm_intel_bo *dummy_bo, *target_bo;
 	struct igt_fb *fb_info = &o->fb_info[o->current_fb_id];
 	unsigned pitch = fb_info->stride;
 
@@ -197,9 +197,7 @@ static void emit_dummy_load__bcs(struct test_output *o)
 			ADVANCE_BATCH();
 		}
 
-		tmp_bo = dummy_bo;
-		dummy_bo = target_bo;
-		target_bo = tmp_bo;
+		swap(dummy_bo, target_bo);
 	}
 	intel_batchbuffer_flush(batch);
 
@@ -282,16 +280,12 @@ static void emit_dummy_load__rcs(struct test_output *o)
 	dst = &sb[1];
 
 	for (i = 0; i < limit; i++) {
-		struct igt_buf *tmp;
-
 		copyfunc(batch, NULL,
 			 src, 0, 0,
 			 o->fb_width, o->fb_height,
 			 dst, 0, 0);
 
-		tmp = src;
-		src = dst;
-		dst = tmp;
+		swap(src, dst);
 	}
 	intel_batchbuffer_flush(batch);
 
-- 
2.0.4

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

Re: [PATCH i-g-t 1/3] lib: Add swap() macro

[Jani Nikula]

On Fri, 05 Dec 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> swap() will swap its two arguments while keeping the required
> tmp variable hidden. Makes for neater code.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_aux.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6c83c53..63e1b06 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
>  #define min(a, b) ((a) < (b) ? (a) : (b))
>  #define max(a, b) ((a) > (b) ? (a) : (b))
>  
> +#define swap(a, b) do {		\
> +	typeof(a) _tmp = (a);	\
> +	(a) = (b);		\
> +	(b) = _tmp;		\
> +} while (0)

Nitpick, make the macro take pointers instead, and amend the cocci patch
accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
obvious than swap(a, b)...

Jani.


> +
>  #endif /* IGT_AUX_H */
> -- 
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH i-g-t 1/3] lib: Add swap() macro

[Ville Syrjälä]

On Fri, Dec 05, 2014 at 06:44:19PM +0200, Jani Nikula wrote:
> On Fri, 05 Dec 2014, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > swap() will swap its two arguments while keeping the required
> > tmp variable hidden. Makes for neater code.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_aux.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index 6c83c53..63e1b06 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
> >  #define min(a, b) ((a) < (b) ? (a) : (b))
> >  #define max(a, b) ((a) > (b) ? (a) : (b))
> >  
> > +#define swap(a, b) do {		\
> > +	typeof(a) _tmp = (a);	\
> > +	(a) = (b);		\
> > +	(b) = _tmp;		\
> > +} while (0)
> 
> Nitpick, make the macro take pointers instead, and amend the cocci patch
> accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
> obvious than swap(a, b)...

But then it's different than the kernel swap() we all love.

> 
> Jani.
> 
> 
> > +
> >  #endif /* IGT_AUX_H */
> > -- 
> > 2.0.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

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

Re: [PATCH i-g-t 1/3] lib: Add swap() macro

[Jani Nikula]

On Fri, 05 Dec 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Dec 05, 2014 at 06:44:19PM +0200, Jani Nikula wrote:
>> On Fri, 05 Dec 2014, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > swap() will swap its two arguments while keeping the required
>> > tmp variable hidden. Makes for neater code.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  lib/igt_aux.h | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>> > index 6c83c53..63e1b06 100644
>> > --- a/lib/igt_aux.h
>> > +++ b/lib/igt_aux.h
>> > @@ -90,4 +90,10 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
>> >  #define min(a, b) ((a) < (b) ? (a) : (b))
>> >  #define max(a, b) ((a) > (b) ? (a) : (b))
>> >  
>> > +#define swap(a, b) do {		\
>> > +	typeof(a) _tmp = (a);	\
>> > +	(a) = (b);		\
>> > +	(b) = _tmp;		\
>> > +} while (0)
>> 
>> Nitpick, make the macro take pointers instead, and amend the cocci patch
>> accordingly? To a grumpy old C coder, swap(&a, &b) is so much more
>> obvious than swap(a, b)...
>
> But then it's different than the kernel swap() we all love.

Fair enough. I'll cry a little. I'll feel better soon.


>
>> 
>> Jani.
>> 
>> 
>> > +
>> >  #endif /* IGT_AUX_H */
>> > -- 
>> > 2.0.4
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>
> -- 
> Ville Syrjälä
> Intel OTC

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