HLSL 6.0 ... 6.9 and GPUTexture for d3d12#9077
Conversation
|
Is that possible to have a maintainer trigger the CI for this one please? |
|
We don't seem to have any d3d12compute coverage on the buildbots. I'll need to add that prior to reviewing/merging this. |
Added this to tests HLSL 6.x features: |
|
I've modified the buildbots to test d3d12compute_sm65, which is the highest version both Windows bots support. We'll see how this goes! I notice that several features here are gated behind higher versions than that. I've tagged this |
|
Many tests require |
|
@soufianekhiat -- can you look at the codegen failures here? There seem to be two classes of bug:
|
Do we have tests for codegen? I just run locally the one I think are relevant. |
We don't have (many) compile-only tests... I'm just looking at the buildbot failures here; the issues are with invalid code being generated which is why I called them "codegen failures". Also, I'm noticing that there's some signed/unsigned mismatch happening somewhere. https://buildbot.halide-lang.org/master/#/builders/391/builds/385 |
|
@soufianekhiat - I've opened #9080 to test patches that are generally needed to fix the D3D12 backend, even for HLSL <6.0. That should provide a more stable foundation for this PR, which should be scoped to extending support to 6.0-6.9, rather than also including fundamental bug-fixes. |
|
@soufianekhiat - the HLSL 5.1 fixes are in, so please rebase/merge with main. |
|
Thanks |
|
Updated the validation script for that PR found new issues: |
|
@alexreinking @shoaibkamil @slomp Let me know if that changes fits. |
| Upload, | ||
| ReadBack | ||
| ReadBack, | ||
| Texture |
There was a problem hiding this comment.
I think retrofitting textures into the buffer interface will lead to trouble eventually.
Technically, things like Upload/Readback and ReadOnly/ReadWrite are properties of the buffer that are also applicable (with their own quirks) to textures.
Is this how the other Halide back-ends implement textures (by just piggy-backing on the buffer interface)?
|
I'll give you and the other reviewers some time to discuss/address my points above, and once that's resolved, I'll give the runtime code another look. I think the kernel argument packing logic which I wrote originally has some fundamental flaws that I want to explore, and adding/mixing 64bits to it has raised some concerns I need to verify in the meanwhile. |
|
@alexreinking could you help for the current state of build machines? |
|
I had some worries about the byte straddling rules for (If someone else would like to double-check and verify the argument packing rules, now would be a good time!) |
|
@soufianekhiat - how is this going? Seems @slomp had a few more comments |
|
@alexreinking still on it. Need more time, I really want to land this one. So ti answer: WIP |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9077 +/- ##
=======================================
Coverage ? 69.40%
=======================================
Files ? 255
Lines ? 78139
Branches ? 18694
=======================================
Hits ? 54229
Misses ? 18468
Partials ? 5442 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Breaking changes
Details:
Checklist