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:
And change the way we handle alignment.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
v2: use uint8_t; add align_buffer and get_buffer_size helpers; fix the uniform table size calculation
libs/vkd3d-shader/hlsl.h | 4 +- libs/vkd3d-shader/hlsl_codegen.c | 104 +++++++++++++++---------------- 2 files changed, 51 insertions(+), 57 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e0045acf..54273ac6 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -126,7 +126,7 @@ struct hlsl_type } e;
unsigned int reg_size;
- unsigned int bytecode_offset;
size_t bytecode_offset; };
struct hlsl_semantic
@@ -144,7 +144,7 @@ struct hlsl_struct_field struct hlsl_semantic semantic; unsigned int reg_offset;
- unsigned int name_bytecode_offset;
size_t name_bytecode_offset; };
struct hlsl_reg
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 9afa590a..8e842bd1 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1328,72 +1328,62 @@ static struct hlsl_reg hlsl_reg_from_deref(const struct hlsl_deref *deref, const struct bytecode_buffer { struct hlsl_ctx *ctx;
- uint32_t *data;
- size_t count, size;
- uint8_t *data;
- size_t size, capacity; int status; };
-/* Returns the token index. */ -static unsigned int put_dword(struct bytecode_buffer *buffer, uint32_t value) +static size_t put_bytes(struct bytecode_buffer *buffer, const void *bytes, size_t size, size_t alignment) {
- unsigned int index = buffer->count;
size_t prev_size = buffer->size;
size_t offset = align(prev_size, alignment);
if (buffer->status)
return index;
return offset;
- if (!hlsl_array_reserve(buffer->ctx, (void **)&buffer->data, &buffer->size,
buffer->count + 1, sizeof(*buffer->data)))
- if (!hlsl_array_reserve(buffer->ctx, (void **)&buffer->data, &buffer->capacity, offset + size, 1)) { buffer->status = VKD3D_ERROR_OUT_OF_MEMORY;
return index;
return offset; }
- buffer->data[buffer->count++] = value;
- memset(buffer->data + prev_size, 0xab, offset - prev_size);
- memcpy(buffer->data + offset, bytes, size);
- buffer->size = offset + size;
- return offset;
+}
- return index;
+static size_t put_dword(struct bytecode_buffer *buffer, uint32_t value) +{
- return put_bytes(buffer, &value, sizeof(value), sizeof(value)); }
-/* Returns the token index. */ -static unsigned int put_float(struct bytecode_buffer *buffer, float value) +static size_t put_float(struct bytecode_buffer *buffer, float value) {
- union
- {
float f;uint32_t u;- } u;
- u.f = value;
- return put_dword(buffer, u.u);
- return put_bytes(buffer, &value, sizeof(value), sizeof(value)); }
-static void set_dword(struct bytecode_buffer *buffer, unsigned int index, uint32_t value) +static void set_dword(struct bytecode_buffer *buffer, size_t offset, uint32_t value) { if (buffer->status) return;
- assert(index < buffer->count);
- buffer->data[index] = value;
- assert(offset + sizeof(value) <= buffer->size);
- memcpy(buffer->data + offset, &value, sizeof(value)); }
-/* Returns the token index. */ -static unsigned int put_string(struct bytecode_buffer *buffer, const char *str) +static size_t put_string(struct bytecode_buffer *buffer, const char *string) {
- unsigned int index = buffer->count;
- size_t len = strlen(str) + 1;
- unsigned int token_count = (len + 3) / sizeof(*buffer->data);
- if (buffer->status)
return index;
- return put_bytes(buffer, string, strlen(string) + 1, 1);
+}
- if (!hlsl_array_reserve(buffer->ctx, (void **)&buffer->data, &buffer->size,
buffer->count + token_count, sizeof(*buffer->data)))- {
buffer->status = E_OUTOFMEMORY;return index;- }
+static size_t align_buffer(struct bytecode_buffer *buffer, uint32_t alignment) +{
- return put_bytes(buffer, NULL, 0, alignment);
+}
- buffer->data[buffer->count + token_count - 1] = 0xabababab;
- memcpy(buffer->data + buffer->count, str, len);
- buffer->count += token_count;
- return index;
+static size_t get_buffer_size(struct bytecode_buffer *buffer) +{
return buffer->size; }
static uint32_t sm1_version(enum vkd3d_shader_type type, unsigned int major, unsigned int minor)
@@ -1509,9 +1499,10 @@ static unsigned int get_array_size(const struct hlsl_type *type) static void write_sm1_type(struct bytecode_buffer *buffer, struct hlsl_type *type, unsigned int ctab_start) { const struct hlsl_type *array_type = get_array_type(type);
- unsigned int fields_offset = 0, field_count = 0; unsigned int array_size = get_array_size(type); struct hlsl_struct_field *field;
unsigned int field_count = 0;
size_t fields_offset = 0;
if (type->bytecode_offset) return;
@@ -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.