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.
|
Checked these locally again after bumping to main and I think this should be fine to merge after a test run. |
|
I've gone through all the failures on the previous test run - the only one related to the tests in this PR were the failures on Metal with Clang. I've XFAILed the configuration as it is blocked by the same missing intrinsic lowering as other Clang DXIL targets. |
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.