https://bugs.winehq.org/show_bug.cgi?id=54832
Bug ID: 54832 Summary: Dereference with non-constant offset of type HLSL_IR_EXPR Product: vkd3d Version: 1.7 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: hlsl Assignee: wine-bugs@winehq.org Reporter: flibitijibibo@gmail.com Distribution: ---
Created attachment 74335 --> https://bugs.winehq.org/attachment.cgi?id=74335 EG2 Vertex Shader
MojoShader uses this for arrays, as an example:
--- float4 uniforms_float4[13]; int4 m_v6 : BLENDINDICES0;
const int ARRAYBASE_8 = 0; a0.x = int(floor(abs(v6.x) + 0.5) * sign(v6.x)); r0.xy = lerp(v1.xy, v1.zw, uniforms_float4[ARRAYBASE_8 + a0.x].xy); ---
A complete example from Escape Goat 2 is attached, but this shader depends on #54826 getting fixed first.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #1 from Ethan Lee flibitijibibo@gmail.com --- I've uploaded a beta branch of Escape Goat 2 on Steam for Linux that reproduces this issue. The password is "codeweaversrulez".
This is the same game as the default build, but it includes dxvk-native and vkd3d in the ./lib64/ folder, so launching the game with "/gldevice:D3D11" as a launch option should be enough to get it to run (and immediately crash).
The debug logs will be at /tmp/EscapeGoat_DebugLog.txt, and you should be able to spot the compiler error messages toward the bottom.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #2 from Ethan Lee flibitijibibo@gmail.com --- Played with this a bit today... long story short, the hlsl_fixme is exactly where it should be (big surprise), but I'm definitely out of my league here as I'm not totally sure what a fix for this would entail. I can see how everything works for IR_CONSTANT offsets, but IR_EXPR and IR_LOAD (or maybe IR_INDEX?) seem to complicate things a _ton_ because without knowing the offset ahead of time, it can't figure out where to load or store anything from, and it doesn't seem like there's an easy way to defer that work to the shader itself. The asm isn't too helpful either:
HLSL:
const int ARRAYBASE_8 = 0; r0.xy = lerp(v1.xy, v1.zw, uniforms_float4[ARRAYBASE_8 + a0.x].xy);
SM4 asm:
add r0.yzw, -v1.yyxy, v1.wwzw mad r1.xy, cb0[r0.x + 0].xyxx, r0.zwzz, v1.xyxx
Like I said though, this is definitely uncharted waters for me, so maybe I'm missing something obvious here.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #3 from Ethan Lee flibitijibibo@gmail.com --- Submitted a unit test, still with no fix, but should at least be useful for someone that's smarter than me!
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/189
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #4 from Ethan Lee flibitijibibo@gmail.com --- Created attachment 74416 --> https://bugs.winehq.org/attachment.cgi?id=74416 Awful non-const patch that sucks
Attached is a very very bad diff that gets non-const offsets to compile. It fails a bunch of "unrelated" unit tests and is riddled with FIXMEs!
It occurred to me that the HLSL compiler might not support this, but the DXBC parser still could - and it does. So I turned this from a compiler task into more of a reverse-engineering task, and this patch is the result.
The core of this feature lives in the TPF emitter; IR_EXPR needs to take a separate path from IR_CONSTANT, doing a number of _very_ specific things:
1. The register being returned by sm4_register_from_deref should something other than TEMP (in our case, CONSTBUFFER) 2. The index, rather than a literal, is a temp register, which is handed to us via deref->offset.node->reg 3. The index information is shoved at the end of the src info; it's basically a nested src info inside a src info 4. With the nested src info, we need to mark the base src as having an additional order with relative addressing enabled; this also means we have to add the nested src length to the total size of the instruction
The big issues with this patch: - I couldn't figure out where some of the data comes from - a lot of these seems like it should be provided by the hlsl_ir_expr, and I'm sure it's in there _somewhere_, but I wasn't able to find it myself - The change to existing tokens is a mess - grep "has_deref" and try not to bleed too much afterward - The added fields to sm4_register seems to have introduced some uninitialized memory issues, which is what I'm guessing is causing all the broken tests (has_deref, as ugly as it is, is there to try and isolate the ugliness to this one specific bit of input data)
I got a whole lot further than I expected, but I still think this will need a maintainer's touch to get all the details right.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #5 from Ethan Lee flibitijibibo@gmail.com --- First MR that will eventually fix this: https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/207
Currently testing with the full patch series and it does appear to work with my tests - will report back if anything comes up but it looks like this will be resolved when the final patch is approved.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #6 from Ethan Lee flibitijibibo@gmail.com --- A unit test has been upstreamed and can be found here:
https://gitlab.winehq.org/wine/vkd3d/-/blob/master/tests/array-index-expr.sh...
More tests will be added with the current MR as well.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #7 from Ethan Lee flibitijibibo@gmail.com --- The following commit fixes this error for vectors: https://gitlab.winehq.org/wine/vkd3d/-/commit/ebf7573571d4bfcd3f38846886106f...
Once the patch for SM4 relative addressing is merged this should be fully resolved for arrays as well.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #8 from Ethan Lee flibitijibibo@gmail.com --- This MR should fully resolve this issue: https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/229
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #9 from Ethan Lee flibitijibibo@gmail.com --- Previous MR has been replaced with a new series, starting with this part:
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/396
I've tested the full series against some FNA titles and my MojoShader database and all came out clean!
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #10 from Ethan Lee flibitijibibo@gmail.com --- MR that resolves this issue take 2: https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/435
https://bugs.winehq.org/show_bug.cgi?id=54832
Ethan Lee flibitijibibo@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Fixed by SHA1| |0ef25ad137d076490b87edabfd5 | |73546ae9b8ff4 Resolution|--- |FIXED
--- Comment #11 from Ethan Lee flibitijibibo@gmail.com --- Fixed in the latest Git revision!
https://bugs.winehq.org/show_bug.cgi?id=54832
Nikolay Sivov bunglehead@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #12 from Nikolay Sivov bunglehead@gmail.com --- Closing bugs fixed in 1.10.
https://bugs.winehq.org/show_bug.cgi?id=54832
--- Comment #13 from Andrey Gusev andrey.goosev@gmail.com --- Created attachment 76484 --> https://bugs.winehq.org/attachment.cgi?id=76484 error
Weird, I still get this error with 1.11-556-g9c83caed
-
WineHQ Bugzilla