Signed-off-by: Francisco Casas fcasas@codeweavers.com --- libs/vkd3d-shader/hlsl_sm4.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 2458018f..07bbd376 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -727,6 +727,46 @@ static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_typ } }
+struct sm4_instruction_modifier +{ + enum vkd3d_sm4_instruction_modifier type; + + union + { + struct + { + int u,v,w; + } aoffimmi; + }; +}; + +static uint32_t sm4_encode_instruction_modifier(struct sm4_instruction_modifier imod) +{ + uint32_t word = 0; + + word |= VKD3D_SM4_MODIFIER_MASK & imod.type; + + switch (imod.type) + { + case VKD3D_SM4_MODIFIER_AOFFIMMI: + assert(-8 <= imod.aoffimmi.u && imod.aoffimmi.u <= 7); + assert(-8 <= imod.aoffimmi.v && imod.aoffimmi.v <= 7); + assert(-8 <= imod.aoffimmi.w && imod.aoffimmi.w <= 7); + word |= ((uint32_t)imod.aoffimmi.u & 0xf) << VKD3D_SM4_AOFFIMMI_U_SHIFT; + word |= ((uint32_t)imod.aoffimmi.v & 0xf) << VKD3D_SM4_AOFFIMMI_V_SHIFT; + word |= ((uint32_t)imod.aoffimmi.w & 0xf) << VKD3D_SM4_AOFFIMMI_W_SHIFT; + break; + + default: + FIXME("Unhandled instruction modifier %#x.\n", imod.type); + return 0; + break; + } + + return word; +} + + struct sm4_register { enum vkd3d_sm4_register_type type; @@ -741,6 +781,9 @@ struct sm4_instruction { enum vkd3d_sm4_opcode opcode;
+ struct sm4_instruction_modifier modifiers[1]; + unsigned int modifier_count; + struct { struct sm4_register reg; @@ -913,6 +956,7 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st uint32_t token = instr->opcode; unsigned int size = 1, i, j;
+ size += instr->modifier_count; for (i = 0; i < instr->dst_count; ++i) size += sm4_register_order(&instr->dsts[i].reg); for (i = 0; i < instr->src_count; ++i) @@ -920,8 +964,18 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st size += instr->idx_count;
token |= (size << VKD3D_SM4_INSTRUCTION_LENGTH_SHIFT); + + token |= ((0 < instr->modifier_count) << 31); + put_u32(buffer, token);
+ for (i = 0; i < instr->modifier_count; i++) + { + token = sm4_encode_instruction_modifier(instr->modifiers[i]); + token |= ((i + 1 < instr->modifier_count) << 31); + put_u32(buffer, token); + } + for (i = 0; i < instr->dst_count; ++i) { token = sm4_encode_register(&instr->dsts[i].reg);
On 12/17/21 13:12, Francisco Casas wrote:
Signed-off-by: Francisco Casas fcasas@codeweavers.com
libs/vkd3d-shader/hlsl_sm4.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 2458018f..07bbd376 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -727,6 +727,46 @@ static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_typ } }
+struct sm4_instruction_modifier +{
- enum vkd3d_sm4_instruction_modifier type;
- union
- {
struct{int u,v,w;} aoffimmi;- };
+};
+static uint32_t sm4_encode_instruction_modifier(struct sm4_instruction_modifier imod)
This should be a const pointer instead, I think.
+{
- uint32_t word = 0;
- word |= VKD3D_SM4_MODIFIER_MASK & imod.type;
- switch (imod.type)
- {
case VKD3D_SM4_MODIFIER_AOFFIMMI:assert(-8 <= imod.aoffimmi.u && imod.aoffimmi.u <= 7);assert(-8 <= imod.aoffimmi.v && imod.aoffimmi.v <= 7);assert(-8 <= imod.aoffimmi.w && imod.aoffimmi.w <= 7);word |= ((uint32_t)imod.aoffimmi.u & 0xf) << VKD3D_SM4_AOFFIMMI_U_SHIFT;word |= ((uint32_t)imod.aoffimmi.v & 0xf) << VKD3D_SM4_AOFFIMMI_V_SHIFT;word |= ((uint32_t)imod.aoffimmi.w & 0xf) << VKD3D_SM4_AOFFIMMI_W_SHIFT;break;default:FIXME("Unhandled instruction modifier %#x.\n", imod.type);return 0;break;- }
- return word;
+}
- struct sm4_register { enum vkd3d_sm4_register_type type;
@@ -741,6 +781,9 @@ struct sm4_instruction { enum vkd3d_sm4_opcode opcode;
- struct sm4_instruction_modifier modifiers[1];
- unsigned int modifier_count;
struct { struct sm4_register reg;@@ -913,6 +956,7 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st uint32_t token = instr->opcode; unsigned int size = 1, i, j;
- size += instr->modifier_count; for (i = 0; i < instr->dst_count; ++i) size += sm4_register_order(&instr->dsts[i].reg); for (i = 0; i < instr->src_count; ++i)
@@ -920,8 +964,18 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st size += instr->idx_count;
token |= (size << VKD3D_SM4_INSTRUCTION_LENGTH_SHIFT);
- token |= ((0 < instr->modifier_count) << 31);
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Same thing in the loop below.
put_u32(buffer, token);
- for (i = 0; i < instr->modifier_count; i++)
- {
token = sm4_encode_instruction_modifier(instr->modifiers[i]);token |= ((i + 1 < instr->modifier_count) << 31);put_u32(buffer, token);- }
for (i = 0; i < instr->dst_count; ++i) { token = sm4_encode_register(&instr->dsts[i].reg);
December 17, 2021 6:21 PM, "Zebediah Figura (she/her)" zfigura@codeweavers.com wrote:
On 12/17/21 13:12, Francisco Casas wrote:
Signed-off-by: Francisco Casas fcasas@codeweavers.com
libs/vkd3d-shader/hlsl_sm4.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 2458018f..07bbd376 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -727,6 +727,46 @@ static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_typ } }
+struct sm4_instruction_modifier
+{
- enum vkd3d_sm4_instruction_modifier type;
- union
- {
- struct
- {
- int u,v,w;
- } aoffimmi;
- };
+};
+static uint32_t sm4_encode_instruction_modifier(struct sm4_instruction_modifier imod)
This should be a const pointer instead, I think.
Okay, Is there any reason besides avoiding copying in the pass-by-value? Or is it because it looks more consistent with the rest of the code?
+{
- uint32_t word = 0;
- word |= VKD3D_SM4_MODIFIER_MASK & imod.type;
- switch (imod.type)
- {
- case VKD3D_SM4_MODIFIER_AOFFIMMI:
- assert(-8 <= imod.aoffimmi.u && imod.aoffimmi.u <= 7);
- assert(-8 <= imod.aoffimmi.v && imod.aoffimmi.v <= 7);
- assert(-8 <= imod.aoffimmi.w && imod.aoffimmi.w <= 7);
- word |= ((uint32_t)imod.aoffimmi.u & 0xf) << VKD3D_SM4_AOFFIMMI_U_SHIFT;
- word |= ((uint32_t)imod.aoffimmi.v & 0xf) << VKD3D_SM4_AOFFIMMI_V_SHIFT;
- word |= ((uint32_t)imod.aoffimmi.w & 0xf) << VKD3D_SM4_AOFFIMMI_W_SHIFT;
- break;
- default:
- FIXME("Unhandled instruction modifier %#x.\n", imod.type);
- return 0;
- break;
- }
- return word;
+}
struct sm4_register { enum vkd3d_sm4_register_type type; @@ -741,6 +781,9 @@ struct sm4_instruction { enum vkd3d_sm4_opcode opcode;
- struct sm4_instruction_modifier modifiers[1];
- unsigned int modifier_count;
struct { struct sm4_register reg; @@ -913,6 +956,7 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st uint32_t token = instr->opcode; unsigned int size = 1, i, j;
- size += instr->modifier_count;
for (i = 0; i < instr->dst_count; ++i) size += sm4_register_order(&instr->dsts[i].reg); for (i = 0; i < instr->src_count; ++i) @@ -920,8 +964,18 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st size += instr->idx_count;
token |= (size << VKD3D_SM4_INSTRUCTION_LENGTH_SHIFT);
- token |= ((0 < instr->modifier_count) << 31);
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Same thing in the loop below.
put_u32(buffer, token);
- for (i = 0; i < instr->modifier_count; i++)
- {
- token = sm4_encode_instruction_modifier(instr->modifiers[i]);
- token |= ((i + 1 < instr->modifier_count) << 31);
- put_u32(buffer, token);
- }
for (i = 0; i < instr->dst_count; ++i) { token = sm4_encode_register(&instr->dsts[i].reg);
Sure.
On 12/20/21 06:25, Francisco Casas wrote:
December 17, 2021 6:21 PM, "Zebediah Figura (she/her)" zfigura@codeweavers.com wrote:
On 12/17/21 13:12, Francisco Casas wrote:
Signed-off-by: Francisco Casas fcasas@codeweavers.com
libs/vkd3d-shader/hlsl_sm4.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 2458018f..07bbd376 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -727,6 +727,46 @@ static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_typ } }
+struct sm4_instruction_modifier
+{
- enum vkd3d_sm4_instruction_modifier type;
- union
- {
- struct
- {
- int u,v,w;
- } aoffimmi;
- };
+};
+static uint32_t sm4_encode_instruction_modifier(struct sm4_instruction_modifier imod)
This should be a const pointer instead, I think.
Okay, Is there any reason besides avoiding copying in the pass-by-value? Or is it because it looks more consistent with the rest of the code?
Yes, I think in general we want to avoid pass-by-value. There are some examples of it elsewhere in the code but they should probably be fixed.
On Fri, Dec 17, 2021 at 10:21 PM Zebediah Figura (she/her) zfigura@codeweavers.com wrote:
On 12/17/21 13:12, Francisco Casas wrote:
Signed-off-by: Francisco Casas fcasas@codeweavers.com
libs/vkd3d-shader/hlsl_sm4.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 2458018f..07bbd376 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -727,6 +727,46 @@ static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_typ } }
+struct sm4_instruction_modifier +{
- enum vkd3d_sm4_instruction_modifier type;
- union
- {
struct{int u,v,w;} aoffimmi;- };
+};
+static uint32_t sm4_encode_instruction_modifier(struct sm4_instruction_modifier imod)
This should be a const pointer instead, I think.
+{
- uint32_t word = 0;
- word |= VKD3D_SM4_MODIFIER_MASK & imod.type;
- switch (imod.type)
- {
case VKD3D_SM4_MODIFIER_AOFFIMMI:assert(-8 <= imod.aoffimmi.u && imod.aoffimmi.u <= 7);assert(-8 <= imod.aoffimmi.v && imod.aoffimmi.v <= 7);assert(-8 <= imod.aoffimmi.w && imod.aoffimmi.w <= 7);word |= ((uint32_t)imod.aoffimmi.u & 0xf) << VKD3D_SM4_AOFFIMMI_U_SHIFT;word |= ((uint32_t)imod.aoffimmi.v & 0xf) << VKD3D_SM4_AOFFIMMI_V_SHIFT;word |= ((uint32_t)imod.aoffimmi.w & 0xf) << VKD3D_SM4_AOFFIMMI_W_SHIFT;break;default:FIXME("Unhandled instruction modifier %#x.\n", imod.type);return 0;break;- }
- return word;
+}
- struct sm4_register { enum vkd3d_sm4_register_type type;
@@ -741,6 +781,9 @@ struct sm4_instruction { enum vkd3d_sm4_opcode opcode;
- struct sm4_instruction_modifier modifiers[1];
- unsigned int modifier_count;
struct { struct sm4_register reg;@@ -913,6 +956,7 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st uint32_t token = instr->opcode; unsigned int size = 1, i, j;
- size += instr->modifier_count; for (i = 0; i < instr->dst_count; ++i) size += sm4_register_order(&instr->dsts[i].reg); for (i = 0; i < instr->src_count; ++i)
@@ -920,8 +964,18 @@ static void write_sm4_instruction(struct vkd3d_bytecode_buffer *buffer, const st size += instr->idx_count;
token |= (size << VKD3D_SM4_INSTRUCTION_LENGTH_SHIFT);
- token |= ((0 < instr->modifier_count) << 31);
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Or even
if (instr->modifier_count) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Hi,
Il 19/01/22 13:02, Matteo Bruni ha scritto:
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;Or even
if (instr->modifier_count) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Not a big deal, but between these two I like Zeb's version more, in that it makes more explicit that we're dealing with a number and not with a boolean condition.
Giovanni.
On Wed, 19 Jan 2022 at 13:48, Giovanni Mascellani gmascellani@codeweavers.com wrote:
Il 19/01/22 13:02, Matteo Bruni ha scritto:
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;Or even
if (instr->modifier_count) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Not a big deal, but between these two I like Zeb's version more, in that it makes more explicit that we're dealing with a number and not with a boolean condition.
Actually, that's the reason I prefer the other variant; "Does the instruction have modifiers?" vs "Is the number of modifiers larger than <x>?".
Hi,
Il 19/01/22 14:06, Henri Verbeet ha scritto:
On Wed, 19 Jan 2022 at 13:48, Giovanni Mascellani gmascellani@codeweavers.com wrote:
Il 19/01/22 13:02, Matteo Bruni ha scritto:
Ech, this is hard to read, how about a simpler:
if (instr->modifier_count > 0) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;Or even
if (instr->modifier_count) token |= VKD3D_SM4_INSTRUCTION_MODIFIER;
Not a big deal, but between these two I like Zeb's version more, in that it makes more explicit that we're dealing with a number and not with a boolean condition.
Actually, that's the reason I prefer the other variant; "Does the instruction have modifiers?" vs "Is the number of modifiers larger than <x>?".
Well, I'd use "!=" instead of ">".
Giovanni.
-
Francisco Casas -
Giovanni Mascellani -
Henri Verbeet -
Matteo Bruni -
Zebediah Figura (she/her)