On Thu, Jul 1, 2021 at 6:08 PM Zebediah Figura (she/her) zfigura@codeweavers.com wrote:
On 7/1/21 4:29 AM, Matteo Bruni wrote:
On Thu, Jul 1, 2021 at 4:55 AM Zebediah Figura zfigura@codeweavers.com wrote:
@@ -1524,12 +1515,12 @@ static void write_sm1_type(struct bytecode_buffer *buffer, struct hlsl_type *typ write_sm1_type(buffer, field->type, ctab_start); }
fields_offset = (buffer->count - ctab_start) * sizeof(*buffer->data);
fields_offset = get_buffer_size(buffer) - ctab_start;This isn't quite what I was expecting when I proposed the helper. You access the current buffer offset whenever you want to reference it afterwards, either in some range computation or to overwrite the buffer in that location with the final value. In all cases (I believe?) what you actually want is the aligned offset, i.e. the location where you're going to start writing whatever is going to come next in the buffer. Which is why, in principle, every access to the current buffer offset needs to take the alignment (specifically of the next thing that's going into the buffer, whenever it might make a difference) into consideration.
Renaming align_buffer to get_buffer_offset() (and getting rid of get_buffer_size()) would do the trick, except it seems a bit ugly to have a get_ function modify the buffer. So it's probably better to only align the returned offset there without touching the buffer. That means though that you still need to explicitly align the buffer after the very last thing you're going to write (i.e. the current use of align_buffer()). Unless...
I'm not sure off the top of my head but it might be the case that the alignment is a property of the buffer (e.g. always 4 for SM1 DXBC). In that case it might make sense to store the alignment right in struct bytecode_buffer instead and, probably, align after writing the data in put_bytes() instead of before.
That's essentially what we were doing before. The actual rule seems to be that alignment is always 4, *except* for strings, which as it turns out don't need to be aligned (in SM1 or SM4).
Most of SM1/SM4 deals with bytecode offsets anyway, which means that theoretically the native parser could handle unaligned offsets. Since almost everything is aligned anyway it's a moot point.
I guess I can dodge the issue by just leaving the alignment behaviour alone, though.
Okay, I wasn't entirely sure that the string alignment change was intended. That means my last suggestion (i.e. my last paragraph above) is invalid. The rest should still apply, so if at some point you want to have another shot at changing the alignment handling, I guess you could give them a try. No particular hurry of course.