On 24/05/2019 09:33, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-05-24 09:20:47)
On 24/05/2019 08:25, Chris Wilson wrote:
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@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
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]));
This seems wrong - is bound to trigger.
Why does it seem wrong? That's the calculation used previously, and the
ptr->engines[] was meant to be packed in order for
sizeof(ptr->engines[count]) == count * sizeof(ptr->engines[0]). Anyway,
I threw it in there to check if the calculation was sane.
Because sizeof(ptr->engines[0]) == sizeof(ptr->engines[N]), since the
code is not declaring N big array, just referencing the element N. So
for more than one engine I expect it explodes. Unless I am way wrong.. I
guess someone needs to run it.. :)
+ return sizeof(*ptr) + sizeof(ptr->engines[count]);
So size of of engine needs to be multiplied by count.
(Just note this is the what the current VLA evaluates to :)
+}
+
+static struct i915_context_engines_load_balance *
+alloc_load_balance(int count)
+{
+ return calloc(1, sizeof_load_balance(count));
How about alloca so cleanup is simpler? Or is alloca also on the
unpopular list?
I don't mind. Would shave a few lines indeed, but we need the memsets
back. #define alloca0()?
And a helper macro to generically deal with struct header + engines
array so it doesn't need to be repeated three times. Yadayada too much
work.. :) ...
Or possibly what Simon suggested, just a large temporary stack arrays
would be enough and easiest diff. Just with an assert that it fits.
I don't think that is as clean for the long term.
... this should be just fine for now so I'd vote for it.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
