[igt-dev] [PATCH i-g-t] benchmarks/gem_wsim: Heap allocate VLA structs

Chris Wilson chris at chris-wilson.co.uk
Fri May 24 07:25:51 UTC 2019


Apparently VLA structs (e.g. struct { int array[count] }) is a gcc
extension that clang refuses to support as handling memory layout is too
difficult for it.

Move the on-stack VLA to the heap.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
 benchmarks/gem_wsim.c | 146 +++++++++++++++++++++++++++---------------
 1 file changed, 95 insertions(+), 51 deletions(-)

diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
index e2ffb93a9..0a0032bff 100644
--- a/benchmarks/gem_wsim.c
+++ b/benchmarks/gem_wsim.c
@@ -1441,6 +1441,48 @@ set_ctx_sseu(struct ctx *ctx, uint64_t slice_mask)
 	return slice_mask;
 }
 
+static size_t sizeof_load_balance(int count)
+{
+	struct i915_context_engines_load_balance *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_engines_load_balance *
+alloc_load_balance(int count)
+{
+	return calloc(1, sizeof_load_balance(count));
+}
+
+static size_t sizeof_param_engines(int count)
+{
+	struct i915_context_param_engines *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_param_engines *
+alloc_param_engines(int count)
+{
+	return calloc(1, sizeof_param_engines(count));
+}
+
+static size_t sizeof_engines_bond(int count)
+{
+	struct i915_context_engines_bond *ptr;
+
+	assert(sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]));
+	return sizeof(*ptr) + sizeof(ptr->engines[count]);
+}
+
+static struct i915_context_engines_bond *
+alloc_engines_bond(int count)
+{
+	return calloc(1, sizeof_engines_bond(count));
+}
+
 static int
 prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 {
@@ -1676,66 +1718,54 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 		}
 
 		if (ctx->engine_map) {
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  ctx->engine_map_count + 1);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 ctx->engine_map_count);
+			struct i915_context_param_engines *set_engines =
+				alloc_param_engines(ctx->engine_map_count + 1);
+			struct i915_context_engines_load_balance *load_balance =
+				alloc_load_balance(ctx->engine_map_count);
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(ctx->engine_map_count + 1),
+				.value = to_user_pointer(set_engines),
 			};
+			struct i915_context_engines_bond *last = NULL;
 
 			if (ctx->wants_balance) {
-				set_engines.extensions =
-					to_user_pointer(&load_balance);
+				set_engines->extensions =
+					to_user_pointer(load_balance);
 
-				memset(&load_balance, 0, sizeof(load_balance));
-				load_balance.base.name =
+				load_balance->base.name =
 					I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-				load_balance.num_siblings =
+				load_balance->num_siblings =
 					ctx->engine_map_count;
 
 				for (j = 0; j < ctx->engine_map_count; j++)
-					load_balance.engines[j] =
+					load_balance->engines[j] =
 						get_engine(ctx->engine_map[j]);
-			} else {
-				set_engines.extensions = 0;
 			}
 
 			/* Reserve slot for virtual engine. */
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
 
 			for (j = 1; j <= ctx->engine_map_count; j++)
-				set_engines.engines[j] =
+				set_engines->engines[j] =
 					get_engine(ctx->engine_map[j - 1]);
 
+			last = NULL;
 			for (j = 0; j < ctx->bond_count; j++) {
 				unsigned long mask = ctx->bonds[j].mask;
-				I915_DEFINE_CONTEXT_ENGINES_BOND(bond,
-								 __builtin_popcount(mask));
-				struct i915_context_engines_bond *p = NULL, *prev;
+				struct i915_context_engines_bond *bond =
+					alloc_engines_bond(__builtin_popcount(mask));
 				unsigned int b, e;
 
-				prev = p;
-				p = alloca(sizeof(bond));
-				assert(p);
-				memset(p, 0, sizeof(bond));
-
-				if (j == 0)
-					load_balance.base.next_extension =
-						to_user_pointer(p);
-				else if (j < (ctx->bond_count - 1))
-					prev->base.next_extension =
-						to_user_pointer(p);
+				bond->base.next_extension = to_user_pointer(last);
+				bond->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
 
-				p->base.name = I915_CONTEXT_ENGINES_EXT_BOND;
-				p->virtual_index = 0;
-				p->master = get_engine(ctx->bonds[j].master);
+				bond->virtual_index = 0;
+				bond->master = get_engine(ctx->bonds[j].master);
 
 				for (b = 0, e = 0; mask; e++, mask >>= 1) {
 					unsigned int idx;
@@ -1743,44 +1773,58 @@ prepare_workload(unsigned int id, struct workload *wrk, unsigned int flags)
 					if (!(mask & 1))
 						continue;
 
-					idx = find_engine(&set_engines.engines[1],
+					idx = find_engine(&set_engines->engines[1],
 							  ctx->engine_map_count,
 							  e);
-					p->engines[b++] =
-						set_engines.engines[1 + idx];
+					bond->engines[b++] =
+						set_engines->engines[1 + idx];
 				}
+
+				last = bond;
 			}
+			load_balance->base.next_extension = to_user_pointer(last);
 
 			gem_context_set_param(fd, &param);
+
+			while (last) {
+				struct i915_context_engines_bond *next =
+					from_user_pointer(last->base.next_extension);
+				free(last);
+				last = next;
+			}
+			free(load_balance);
+			free(set_engines);
 		} else if (ctx->wants_balance) {
 			const unsigned int count = num_engines_in_class(VCS);
-			I915_DEFINE_CONTEXT_ENGINES_LOAD_BALANCE(load_balance,
-								 count);
-			I915_DEFINE_CONTEXT_PARAM_ENGINES(set_engines,
-							  count + 1);
+			struct i915_context_engines_load_balance *load_balance =
+				alloc_load_balance(count);
+			struct i915_context_param_engines *set_engines =
+				alloc_param_engines(count + 1);
 			struct drm_i915_gem_context_param param = {
 				.ctx_id = ctx_id,
 				.param = I915_CONTEXT_PARAM_ENGINES,
-				.size = sizeof(set_engines),
-				.value = to_user_pointer(&set_engines),
+				.size = sizeof_param_engines(count + 1),
+				.value = to_user_pointer(set_engines),
 			};
 
-			set_engines.extensions = to_user_pointer(&load_balance);
+			set_engines->extensions = to_user_pointer(load_balance);
 
-			set_engines.engines[0].engine_class =
+			set_engines->engines[0].engine_class =
 				I915_ENGINE_CLASS_INVALID;
-			set_engines.engines[0].engine_instance =
+			set_engines->engines[0].engine_instance =
 				I915_ENGINE_CLASS_INVALID_NONE;
-			fill_engines_class(&set_engines.engines[1], VCS);
+			fill_engines_class(&set_engines->engines[1], VCS);
 
-			memset(&load_balance, 0, sizeof(load_balance));
-			load_balance.base.name =
+			load_balance->base.name =
 				I915_CONTEXT_ENGINES_EXT_LOAD_BALANCE;
-			load_balance.num_siblings = count;
+			load_balance->num_siblings = count;
 
-			fill_engines_class(&load_balance.engines[0], VCS);
+			fill_engines_class(&load_balance->engines[0], VCS);
 
 			gem_context_set_param(fd, &param);
+
+			free(set_engines);
+			free(load_balance);
 		}
 
 		if (wrk->sseu) {
-- 
2.20.1



More information about the igt-dev mailing list