Created attachment 138037 [details] [review] Patch for tessellation demo to reproduce the issue As mentioned in the title, reading a per-patch output variable that was written by a different invocation produces undefined results even after a barrier() call. The attached patch changes the tessellation control shader of the 'tessellation' demo from Sascha Willems' Vulkan samples in that it reads the tessellation levels from a per-patch output array. Note that the shader needs to be recompiled manually in order to reproduce the issue.
Created attachment 138038 [details] Witcher 3 hull shader which may suffer from the same issue FWIW, the tessellation demo works correctly with AMDVLK with the patched shader. On RADV, the tessellation levels are seemingly random, and its behaviour changes by just changing the location number. A similar issue occurs in The Witcher 3 when run with DXVK, where water surfaces have incorrect tessellation factors applied on RADV. It reportedly renders correctly on Nvidia. In this case however, there is only one single invocation writing per-patch outputs. A workaround that makes this particular shader work correctly on RADV is to write all per-patch outputs to an array with storage class Private first, reading the tessellation factors from that array, and finally copying the contents of the temporary array to the output array, which tells me that reading from the output array again returns incorrect results.
Created attachment 138044 [details] Tessellation demo screenshot
https://patchwork.freedesktop.org/series/39918/ Should fix this, thanks for the report and reproducer.
Created attachment 138092 [details] Tessellation demo screenshot with LLVM-svn Can confirm that this fixes the tessellation demo on Polaris with LLVM 5.0.1. There are still issues when building against LLVM-svn (see attached screenshot). Not sure if this is in any way related, but it looks like the TES is receiving incorrect/random data now, which can be reproduced even with the original TCS (without my patch) *and* without the TCS. This may not be reproducible on Vega.
Typo: *and* without the TCS fix.
I can confirm that the tessellation demo is broken without Philip's patch on mesa 03e37ec6d7 and llvm-svn 327550 on R9 Fury.
A possible fix https://reviews.llvm.org/D44974
Should be fixed with SVN r329591.
Note that r329591 has been reverted because it broke two AMDGPU tests. Fixed version has been pushed, r329764 fixes the issue.
Hi Samuel, We are observing performance regression on SGEMM on fiji with https://reviews.llvm.org/D44974. Shortening the vector length from 128 to 64 is causing the regression. I wanted to analyze the stability regression that you observed with 128bit vector length. Is there any reproducer? Or can you please tell me where I can get tessellation shader and the steps to run it? Thanks- Farhana
Hi, Well, you can still re-enable the previous behaviour with "enable-ds128" and I expect performance to be similar. Otherwise, if you want to reproduce the problem you have to clone https://github.com/SaschaWillems/Vulkan and to run the tessellationshader demo using RADV.
Created attachment 139738 [details] [review] radv testing patch Hey Samuel, I re-tested this (on Tonga) with the latest LLVM trunk and the attached patch. Using RADV_DEBUG=nocache,shaders,ds128 shows assembly that uses the 128-bit instructions without rendering artifacts. Looks like we could try enabling this in LLVM by default again, any objections?
The tessellation demo looks good on vega here. What changed in LLVM since last time? If you really want to enable this by default, I would like to run CTS before.
I don't know. Farhana asked me to test this since I have a Mesa setup. I'd appreciate if you could give a CTS run a shot.
Yeah, I will do.
No CTS regressions, I'm fine with it.
Great, thanks for testing!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.