On 23.03.2017 13:41, Timothy Arceri wrote:
On 23/03/17 23:19, Nicolai Hähnle wrote:
On 23.03.2017 13:06, Timothy Arceri wrote:
On 23/03/17 22:52, Nicolai Hähnle wrote:
On 23.03.2017 07:32, Timothy Arceri wrote:
+void GLAPIENTRY
+_mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid
* data,
+ GLenum usage)
+{
+ GET_CURRENT_CONTEXT(ctx);
+ size_t cmd_size =
+ sizeof(struct marshal_cmd_BufferData) + (data ? size *2 : 0);
Why the *2?
That's a a typo I thought, I fixed it before sending.
Also, I didn't notice that before, but I'm concerned that this can
break
the alignment of subsequent marshal_cmd structs. It's probably best to
force the alignment in _mesa_glthread_allocate_command, so that this
problem cannot be missed later on in similar situations.
You mean as a follow up patch right? We don't want to make a copy here.
Well, better as a preparatory patch, so that the alignment problem is
never introduced in the first place, but yes.
This code (and I assume other similar code) is already generated
automatically by gl_marshal.py so there is no regression.
I have a feeling we're talking past each other. At least I don't see how
what you write relates to what I wrote :/
The issue that concerns me is when size is not a multiple of 8 bytes (or
perhaps sizeof(void *)? -- but there are commands with 8 byte-arguments
even on 32-bit platforms).
In that case, cmd_size % 8 != 0, and _mesa_glthread_allocate_command
will happily advance glthread->batch->used to an amount that is not a
multiple of 8. As a result, subsequent command structs will no longer be
aligned.
I suppose this isn't a noticeable issue on x86 (perhaps some memory
accesses will be slower when crossing a cache line), but (1) sanitizers
_will_ complain about this, and (2) it's likely to break other
architectures.
My suggested solution would be to round size up in
_mesa_glthread_allocate_command, to catch this problem in a single
place. Alternatively (e.g. if that degrades the assembly generated by
the compiler), we could simply add an assert in
_mesa_glthread_allocate_command and do the alignment in the custom-coded
mesa_marshal_* functions that need it.
Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
