Add Graphics tests for ddx/y_fine #510
Conversation
test/Graphics/ddx_fine.test
Outdated
| ... | ||
| #--- end | ||
|
|
||
| # UNSUPPORTED: Clang |
There was a problem hiding this comment.
Didn't you merge the clang changes?
There was a problem hiding this comment.
ddx/y_coarse was merged, but _fine is waiting for review.
Regardless, all the graphics tests current fail on clang, including the original SimpleTriangle.test, because of other issues.
The output from clang-d3d12-graphics for all tests is
# .---command stderr------------
# | error: Unsupported intrinsic llvm.dx.load.input.v4f32 for DXIL lowering
# | error: Unsupported intrinsic llvm.dx.store.output.v4f32 for DXIL lowering
# | 2 errors generated.
# `-----------------------------
# error: command failed with exit status: 1
There was a problem hiding this comment.
Interesting can you file an issue and I'll triage it.
There was a problem hiding this comment.
Did some more testing and created #540 (comment).
I'm happy to look into the failure if you'd like
There was a problem hiding this comment.
Since it's only failing with DirectX + Clang, you should do this instead:
| # UNSUPPORTED: Clang | |
| # Bug https://github.com/llvm/offload-test-suite/issues/540 | |
| # XFAIL: Clang && DirectX |
There was a problem hiding this comment.
the CI indicates it is failing on vulkan too. The only runner that is passing is the metal one. I agree these need to be changed to XFAIL. But maybe it should be Clang && (DirectX || Vulkan)?
There was a problem hiding this comment.
I think the runners failed because it couldn't find the image in the golden-images repo. For this and the other graphics tests I've added to run the corresponding PRs for the image comparison will need merged first.
I've updated this to XFAIL: Clang && DirectX as I believe once llvm/offload-golden-images#5 is merged it will pass on Vulkan. I'm unsure what will happen with Metal.
There was a problem hiding this comment.
I don’t have permissions in that repository. Will follow up with @llvm-beanz
test/Graphics/ddy_fine.test
Outdated
| ... | ||
| #--- end | ||
|
|
||
| # UNSUPPORTED: Clang |
There was a problem hiding this comment.
Didn't you merge the clang changes?
| # RUN: %dxc_target -T vs_6_0 -Fo %t-vertex.o %t/vertex.hlsl | ||
| # RUN: %dxc_target -T ps_6_0 -Fo %t-pixel.o %t/pixel.hlsl | ||
| # RUN: %offloader %t/pipeline.yaml %t-vertex.o %t-pixel.o -r Output -o %t/Output.png | ||
| # RUN: imgdiff %t/Output.png %goldenimage_dir/hlsl/Graphics/DdxFine.png -rules %t/rules.yaml |
There was a problem hiding this comment.
I don't see an uploaded image in this PR. How are you testing this?
There was a problem hiding this comment.
The graphics test images are in a different repo. The PR for them is here llvm/offload-golden-images#5
| # RUN: %dxc_target -T vs_6_0 -Fo %t-vertex.o %t/vertex.hlsl | ||
| # RUN: %dxc_target -T ps_6_0 -Fo %t-pixel.o %t/pixel.hlsl | ||
| # RUN: %offloader %t/pipeline.yaml %t-vertex.o %t-pixel.o -r Output -o %t/Output.png | ||
| # RUN: imgdiff %t/Output.png %goldenimage_dir/hlsl/Graphics/DdyFine.png -rules %t/rules.yaml |
farzonl
added
the
test-all
farzonl
left a comment
farzonl
left a comment
There was a problem hiding this comment.
Approval conditional on switching to XFAIL. I've also market this PR as test-all so we know how it impacts all IHV GPUs.
bogner
left a comment
bogner
left a comment
There was a problem hiding this comment.
A couple of stylistic notes, but otherwise this looks good.
We should also add compute tests for these with SM6.6+ (See https://microsoft.github.io/DirectX-Specs/d3d/HLSL_SM_6_6_Derivatives.html). That can probably be done separately, but could you make sure we at least have an issue tracking it?
test/Graphics/ddx_fine.test
Outdated
| struct PSInput | ||
| { | ||
| float4 position : SV_POSITION; | ||
| }; |
There was a problem hiding this comment.
Please follow llvm-style in these tests. So here, we'd want to use braces like so:
| struct PSInput | |
| { | |
| float4 position : SV_POSITION; | |
| }; | |
| struct PSInput { | |
| float4 position : SV_POSITION; | |
| }; |
It's a bit annoying because of how the files are embedded, but you can run clang-format on the split files in the build directory or copy the code to a temp file to do the same, which makes it easier to be consistent.
test/Graphics/ddx_fine.test
Outdated
| #--- pixel.hlsl | ||
|
|
||
| // Offset into the image of our circle | ||
| #define offset 128.0 |
There was a problem hiding this comment.
Arguably better to either name this OFFSET following the usual preprocessor macro naming conventions or make it a static const float instead of using a define.
4 participants
Relies on llvm/offload-golden-images#5
Adds Graphics tests for ddx_coarse and ddy_coarse intrinsics. The tests use the dd_coarse intrinsics to apply a very basic anti-aliasing style blur to the edges of a circle in either the X or Y dimension.