Bug 108275 - Breaking out of loop creates broken code on RADV
Summary: Breaking out of loop creates broken code on RADV
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Vulkan/radeon (show other bugs)
Version: 18.2
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-08 18:52 UTC by maister
Modified: 2018-12-17 13:56 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Fossilize dump (197.34 KB, application/json)
2018-10-08 18:52 UTC, maister
Details
workaround (564 bytes, patch)
2018-11-05 17:02 UTC, Samuel Pitoiset
Details | Splinter Review

Description maister 2018-10-08 18:52:36 UTC
Created attachment 141940 [details]
Fossilize dump

I have a test case where adding a break to a loop creates broken code, and Vulkan renders something complete bogus.

The code comes from spirv-opt and works fine on all other implementations I've tested.

The original GLSL looks like this: https://github.com/Themaister/Granite/blob/master/assets/shaders/ocean/cull_blocks.comp
To workaround the issue, I removed the "break" on line 54, which for some reason fixed the issue.

My hunch is that the SelectionMerge inside the loop is merging to the loop's continue block, and this is causing some weirdness.

To build Fossilize for repro:

git clone git://github.com/Themaister/Fossilize
cd Fossilize
git submodule update --init
mkdir build
cd build
cmake .. -DCMAKE_BUILD_TYPE=Debug
make -j16

To disassemble the failing SPIR-V pipeline:

GLSL (SPIRV-Cross):
./cli/fossilize-disasm fossilize.json --compute-pipeline 3 --target glsl
SPIR-V asm:
... --target asm
AMD ISA (VK_AMD_shader_info):
... --target amd

For the workaround case, use --compute-pipeline 29 instead.
Comment 1 maister 2018-10-08 19:07:15 UTC
To replay the pipeline (for debugging):

./cli/fossilize-replay fossilize.json --filter-compute 3
Comment 2 Ian Romanick 2018-10-08 20:18:40 UTC
(In reply to maister from comment #0)
> Created attachment 141940 [details]
> Fossilize dump
> 
> I have a test case where adding a break to a loop creates broken code, and
> Vulkan renders something complete bogus.
> 
> The code comes from spirv-opt and works fine on all other implementations
> I've tested.

Does that include the Intel Vulkan driver in Mesa?  That may help narrow down the location of the problem.
Comment 3 maister 2018-10-08 21:10:08 UTC
Seems to work just fine on Intel (Anvil).
Comment 4 Samuel Pitoiset 2018-10-09 07:36:45 UTC
Just compiled your stuff, how do I see the bogus rendering?
Comment 5 maister 2018-10-09 10:26:11 UTC
To see the result, you would need a repro. I can create a RenderDoc capture if that helps (I run this on an RX470), although "bogus" in this case is actually that nothing is rendered.

The shader in question is used to frustum cull blocks, but everything is falsely culled.
Comment 6 Samuel Pitoiset 2018-10-09 12:02:55 UTC
Yes, please record a renderdoc capture.
Comment 7 maister 2018-10-09 17:29:20 UTC
Broken capture: https://drive.google.com/file/d/1qdUNsJqLE0kzBKz_h7kPqb6ivbVLEH9P/view?usp=sharing

Working capture (without spirv-opt on the offending compute shader):
https://drive.google.com/file/d/16gKUa0XtrFS0buRZRKFOB5kIX8R4t9z_/view?usp=sharing

The offending event is EID 52.
Comment 8 maister 2018-10-09 17:30:46 UTC
So, apparently not *everything* is falsely culled, just most of it, depending on view angle.
Comment 9 Samuel Pitoiset 2018-11-05 17:02:11 UTC
Created attachment 142373 [details] [review]
workaround

This workaround appears to fix the problem on my side. I will investigate more.
Comment 10 Samuel Pitoiset 2018-12-14 11:09:37 UTC
Can you try latest master? Apparently, https://cgit.freedesktop.org/mesa/mesa/commit/?id=5921a19d4b0c6491b3535b5154d585384c9ea144 fixes the problem for some reasons.

I wonder if LLVM was confused with the IR.
Comment 11 Timothy Arceri 2018-12-14 12:47:03 UTC
(In reply to Samuel Pitoiset from comment #10)
> Can you try latest master? Apparently,
> https://cgit.freedesktop.org/mesa/mesa/commit/
> ?id=5921a19d4b0c6491b3535b5154d585384c9ea144 fixes the problem for some
> reasons.
> 
> I wonder if LLVM was confused with the IR.

I've found LLVM touchy when it comes to loops in the past [1].

[1] https://cgit.freedesktop.org/mesa/mesa/commit/?id=b42633db8e3711e54a5bd10495b1436b8e362801
Comment 12 maister 2018-12-14 13:14:55 UTC
Yes, with latest master, it works as expected.
Comment 13 Samuel Pitoiset 2018-12-17 13:56:23 UTC
Yeah, I agree with Timothy. Closing as this problem is fixed with latest master.
Thanks for the report.


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.