On 02/05/2017 15:45, Chris Wilson wrote:
On Tue, May 02, 2017 at 01:24:58PM +0100, Tvrtko Ursulin wrote:
On 28/04/2017 20:02, Chris Wilson wrote:
+ if (!p->height) {
+ for (bits = p->bitmap; (i = ffs(bits)); bits &= ~0u << i) {
Would for_each_set_bit be more readable?
Downside is that we have to cast bitmap to unsigned long:
Something like:
diff --git a/drivers/gpu/drm/i915/selftests/i915_syncmap.c b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
index 1f8b594b4157..9fbc9e144833 100644
--- a/drivers/gpu/drm/i915/selftests/i915_syncmap.c
+++ b/drivers/gpu/drm/i915/selftests/i915_syncmap.c
@@ -33,7 +33,7 @@ __sync_print(struct i915_syncmap *p,
unsigned int idx)
{
unsigned long len;
- unsigned i, bits, X;
+ unsigned i, X;
if (depth) {
unsigned int d;
@@ -61,7 +61,7 @@ __sync_print(struct i915_syncmap *p,
*sz -= len;
if (!p->height) {
- for (bits = p->bitmap; (i = ffs(bits)); bits &= ~0u << i) {
+ for_each_set_bit(i, (unsigned long *)&p->bitmap, KSYNCMAP) {
len = scnprintf(buf, *sz, " %x:%x,",
i - 1, __sync_seqno(p)[i - 1]);
buf += len;
@@ -76,11 +76,11 @@ __sync_print(struct i915_syncmap *p,
*sz -= len;
if (p->height) {
- for (bits = p->bitmap; (i = ffs(bits)); ) {
- bits &= ~0u << i;
+ for_each_set_bit(i, (unsigned long *)&p->bitmap, KSYNCMAP) {
buf = __sync_print(__sync_child(p)[i - 1],
buf, sz,
- depth + 1, (last << 1) | !!bits,
+ depth + 1, (last << 1) |
+ !!(p->bitmap & (~0u << i)),
i - 1);
}
}
Its a bit smaller line shrink in this version so I am not sure. Most
importantly it is not getting rid of the ~0u << i business. Have it as
you prefer it.
And thank you for not suggesting to use the horrible code generation of
for_each_set_bit() outside of the pretty printer. :)
P.S. Latest ascii graphs:
0xXXXXXXXXXXXXXXXX
0-> 0x0000000000XXXXXX
| 0-> 0x0000000000000XXX
| | 0-> 0x00000000000000XX
| | | 0-> 0x000000000000000X 0:0, 1:1, 2:2
| | | 1-> 0x000000000000001X 0:10, 1:11
| | 2-> 0x000000000000020X 0:200, 1:201
| 5-> 0x000000000050XXXX
| 0-> 0x000000000050000X 0:500000, 1:500001
| 3-> 0x000000000050300X 0:503000, 1:503001
e-> 0xe00000000000000X e:e
I think that's better. And thank you for adding the graph!
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx