Extended opcodes (modifiers) are primarily used for sign-extension, yet there are some additional modifiers we can parse at least.
Information taken from dxvk implementation!
There should be no functional change beyond the FIXME -> TRACE change for the newly acknowledge modifiers.
Signed-off-by: Tobias Klausmann tobias.johannes.klausmann@mni.thm.de --- dlls/wined3d/shader_sm4.c | 98 +++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 29 deletions(-)
diff --git a/dlls/wined3d/shader_sm4.c b/dlls/wined3d/shader_sm4.c index b119e8feaa..ad0ac9e80a 100644 --- a/dlls/wined3d/shader_sm4.c +++ b/dlls/wined3d/shader_sm4.c @@ -25,14 +25,19 @@ WINE_DEFAULT_DEBUG_CHANNEL(d3d_shader); WINE_DECLARE_DEBUG_CHANNEL(d3d_bytecode);
#define WINED3D_SM4_INSTRUCTION_MODIFIER (0x1u << 31) +#define WINED3D_SM4_INSTRUCTION_MODIFIER_BIT 31
-#define WINED3D_SM4_MODIFIER_AOFFIMMI 0x1 -#define WINED3D_SM4_AOFFIMMI_U_SHIFT 9 -#define WINED3D_SM4_AOFFIMMI_U_MASK (0xfu << WINED3D_SM4_AOFFIMMI_U_SHIFT) -#define WINED3D_SM4_AOFFIMMI_V_SHIFT 13 -#define WINED3D_SM4_AOFFIMMI_V_MASK (0xfu << WINED3D_SM4_AOFFIMMI_V_SHIFT) -#define WINED3D_SM4_AOFFIMMI_W_SHIFT 17 -#define WINED3D_SM4_AOFFIMMI_W_MASK (0xfu << WINED3D_SM4_AOFFIMMI_W_SHIFT) +#define WINED3D_SM4_MODIFIER_FIELD_BIT_LOW 0 +#define WINED3D_SM4_MODIFIER_FIELD_BIT_HIGH 5 + +#define WINED3D_SM4_AOFFIMMI_U_BIT_LOW 9 +#define WINED3D_SM4_AOFFIMMI_U_BIT_HIGH 12 + +#define WINED3D_SM4_AOFFIMMI_V_BIT_LOW 13 +#define WINED3D_SM4_AOFFIMMI_V_BIT_HIGH 16 + +#define WINED3D_SM4_AOFFIMMI_W_BIT_LOW 17 +#define WINED3D_SM4_AOFFIMMI_W_BIT_HIGH 20
#define WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT 24 #define WINED3D_SM4_INSTRUCTION_LENGTH_MASK (0x1fu << WINED3D_SM4_INSTRUCTION_LENGTH_SHIFT) @@ -304,6 +309,13 @@ enum wined3d_sm4_opcode WINED3D_SM5_OP_DCL_GS_INSTANCES = 0xce, };
+enum wined3d_sm4_ext_opcode { + WINED3D_SM4_EXT_OP_EMPTY = 0, + WINED3D_SM4_EXT_OP_SAMPLE_CONTROLS, + WINED3D_SM4_EXT_OP_RESOURCE_DIM, + WINED3D_SM4_EXT_OP_RESOURCE_RETURN_TYPE, +}; + enum wined3d_sm4_register_type { WINED3D_SM4_RT_TEMP = 0x00, @@ -1222,6 +1234,10 @@ static enum wined3d_data_type map_data_type(char t) } }
+inline DWORD bit_extract32(DWORD value, DWORD fst, DWORD lst) { + return (value >> fst) & ~(~0 << (lst - fst + 1)); +} + enum wined3d_shader_type wined3d_get_sm4_shader_type(const DWORD *byte_code, size_t byte_code_size) { DWORD shader_type; @@ -1610,32 +1626,56 @@ static BOOL shader_sm4_read_dst_param(struct wined3d_sm4_data *priv, const DWORD
static void shader_sm4_read_instruction_modifier(DWORD modifier, struct wined3d_shader_instruction *ins) { - static const DWORD recognized_bits = WINED3D_SM4_INSTRUCTION_MODIFIER - | WINED3D_SM4_MODIFIER_AOFFIMMI - | WINED3D_SM4_AOFFIMMI_U_MASK - | WINED3D_SM4_AOFFIMMI_V_MASK - | WINED3D_SM4_AOFFIMMI_W_MASK; - - if (modifier & ~recognized_bits) + if (bit_extract32(modifier, + WINED3D_SM4_INSTRUCTION_MODIFIER_BIT, + WINED3D_SM4_INSTRUCTION_MODIFIER_BIT)) { - FIXME("Unhandled modifier 0x%08x.\n", modifier); + const DWORD extOpcode = bit_extract32(modifier, + WINED3D_SM4_MODIFIER_FIELD_BIT_LOW, + WINED3D_SM4_MODIFIER_FIELD_BIT_HIGH); + + switch (extOpcode) + { + case WINED3D_SM4_EXT_OP_SAMPLE_CONTROLS: { + /* Bit fields are used for sign extension */ + struct + { + int u : 4; + int v : 4; + int w : 4; + } + aoffimmi; + + aoffimmi.u = bit_extract32(modifier, + WINED3D_SM4_AOFFIMMI_U_BIT_LOW, + WINED3D_SM4_AOFFIMMI_U_BIT_HIGH); + aoffimmi.v = bit_extract32(modifier, + WINED3D_SM4_AOFFIMMI_V_BIT_LOW, + WINED3D_SM4_AOFFIMMI_V_BIT_HIGH); + aoffimmi.w = bit_extract32(modifier, + WINED3D_SM4_AOFFIMMI_W_BIT_LOW, + WINED3D_SM4_AOFFIMMI_W_BIT_HIGH); + + ins->texel_offset.u = aoffimmi.u; + ins->texel_offset.v = aoffimmi.v; + ins->texel_offset.w = aoffimmi.w; + } + break; + + case WINED3D_SM4_EXT_OP_RESOURCE_DIM: + case WINED3D_SM4_EXT_OP_RESOURCE_RETURN_TYPE: + TRACE("Unhandled opcode extension 0x%08x -> %d\n", modifier, + extOpcode); + break; // part of resource description + + default: + FIXME("Unhandled opcode extension 0x%08x\n", modifier); + break; + } } else { - /* Bit fields are used for sign extension */ - struct - { - int u : 4; - int v : 4; - int w : 4; - } - aoffimmi; - aoffimmi.u = (modifier & WINED3D_SM4_AOFFIMMI_U_MASK) >> WINED3D_SM4_AOFFIMMI_U_SHIFT; - aoffimmi.v = (modifier & WINED3D_SM4_AOFFIMMI_V_MASK) >> WINED3D_SM4_AOFFIMMI_V_SHIFT; - aoffimmi.w = (modifier & WINED3D_SM4_AOFFIMMI_W_MASK) >> WINED3D_SM4_AOFFIMMI_W_SHIFT; - ins->texel_offset.u = aoffimmi.u; - ins->texel_offset.v = aoffimmi.v; - ins->texel_offset.w = aoffimmi.w; + FIXME("Unhandled modifier 0x%08x\n", modifier); } }
On 9 September 2018 at 02:10, Tobias Klausmann tobias.johannes.klausmann@mni.thm.de wrote:
Extended opcodes (modifiers) are primarily used for sign-extension, yet there are some additional modifiers we can parse at least.
Information taken from dxvk implementation!
There should be no functional change beyond the FIXME -> TRACE change for the newly acknowledge modifiers.
Thank you for the patch, but unless there's a good reason not to, please stick to the same structure and style as the rest of the code.
On 09.09.18 14:42, Henri Verbeet wrote:
On 9 September 2018 at 02:10, Tobias Klausmann tobias.johannes.klausmann@mni.thm.de wrote:
Extended opcodes (modifiers) are primarily used for sign-extension, yet there are some additional modifiers we can parse at least.
Information taken from dxvk implementation!
There should be no functional change beyond the FIXME -> TRACE change for the newly acknowledge modifiers.
Thank you for the patch, but unless there's a good reason not to, please stick to the same structure and style as the rest of the code.
Hi,
i'm not sure which structure you really mean, the FIXME -> TRACE, or the bitextract instead of the bitmasks & shift
Greetings,
Tobias
On 9 September 2018 at 17:37, Tobias Klausmann tobias.johannes.klausmann@mni.thm.de wrote:
i'm not sure which structure you really mean, the FIXME -> TRACE, or the bitextract instead of the bitmasks & shift
The latter, mostly. I think changing e.g. AOFFIMMI_U_SHIFT/AOFFIMMI_U_MASK to AOFFIMMI_U_BIT_LOW/AOFFIMMI_U_BIT_HIGH is gratuitous.
Looking a little more closely at the patch though, you're not really parsing the RESOURCE_DIM and RESOURCE_RETURN_TYPE modifiers either, so I think a FIXME would still be appropriate there. So both, I guess.
On 09.09.18 15:33, Henri Verbeet wrote:
On 9 September 2018 at 17:37, Tobias Klausmann tobias.johannes.klausmann@mni.thm.de wrote:
i'm not sure which structure you really mean, the FIXME -> TRACE, or the bitextract instead of the bitmasks & shift
The latter, mostly. I think changing e.g. AOFFIMMI_U_SHIFT/AOFFIMMI_U_MASK to AOFFIMMI_U_BIT_LOW/AOFFIMMI_U_BIT_HIGH is gratuitous.
Looking a little more closely at the patch though, you're not really parsing the RESOURCE_DIM and RESOURCE_RETURN_TYPE modifiers either, so I think a FIXME would still be appropriate there. So both, I guess.
Actually i would prefer keeping the bit extract thingy, as it seems more straight forward to me in these cases. If you worry about consistency, converting more of would be a thing i could do!
On 9 September 2018 at 19:14, Tobias Klausmann tobias.johannes.klausmann@mni.thm.de wrote:
On 09.09.18 15:33, Henri Verbeet wrote:
The latter, mostly. I think changing e.g. AOFFIMMI_U_SHIFT/AOFFIMMI_U_MASK to AOFFIMMI_U_BIT_LOW/AOFFIMMI_U_BIT_HIGH is gratuitous.
Looking a little more closely at the patch though, you're not really parsing the RESOURCE_DIM and RESOURCE_RETURN_TYPE modifiers either, so I think a FIXME would still be appropriate there. So both, I guess.
Actually i would prefer keeping the bit extract thingy, as it seems more straight forward to me in these cases. If you worry about consistency, converting more of would be a thing i could do!
Consistency is the primary concern, yes. I'm also not all that convinced about the merits of that particular change though—it seems largely a matter of personal preference. I could be convinced if you were a major contributor to the code in question, but not on your first patch I'm afraid.
-
Henri Verbeet -
Tobias Klausmann