Bug 108275

Summary:	Breaking out of loop creates broken code on RADV
Product:	Mesa	Reporter:	maister
Component:	Drivers/Vulkan/radeon	Assignee:	mesa-dev
Status:	RESOLVED FIXED	QA Contact:	mesa-dev
Severity:	normal
Priority:	medium
Version:	18.2
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Fossilize dump workaround

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.