KhronosGroup / KhronosGroup/SPIRV-Cross

`[[clang::optnone]]` causes miscompiles

Open
#2,156 2 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
GLSL
Stars
2.5k
Forks
713
Avg merge
2d 18h
Merged PRs (30d)
16

Description

This is mostly an Apple compiler bug, but we might want to consider avoiding using things that both aren't in the MSL spec and change lots of otherwise unused (and therefore probably completely untested) options in the compiler

To reproduce:
- Download and unzip [OptNoneFail.zip](https://github.com/KhronosGroup/SPIRV-Cross/files/11580188/OptNoneFail.zip)
- Run `make -j2 run`

The test will do a two-pass render, first with a depth-only pass and then with a compare zequal color pass that writes red to the output. It will then compare the two images and count the number of pixels that wrote depth but failed the zequal test. This should be zero, but on Apple GPUs, 2211 pixels will fail. You can go into `main.swift` and uncomment the commented lines to take a capture if you're curious (change the destination and add an output if you don't want to import this project into Xcode).

The shaders VS0 and VS1 are lightly modified from spirv-cross, with as much `[[clang::optnone]]` usage as possible removed while still preserving the bug.

I disassembled the compiled shaders to see why the bug happens, for anyone curious:

Shader disassembly and analysis

For anyone following along, apply [this patch](https://github.com/KhronosGroup/SPIRV-Cross/files/11580265/0001-OptNoneFail-tester.patch) to [the apple gpu disassembler](https://github.com/dougallj/applegpu.git) and run `python3 compiler_explorer.py OptNoneFail/VS0.metal --no-fast-math`

Full shader decompilation

[GitHub seems to have a comment length limit of 65536 characters, so have a text file instead](https://github.com/KhronosGroup/SPIRV-Cross/files/11580261/VS0.txt)

The two outlined functions are as follows:
```
; @ 0x-80, spvFAdd
0: 2aad56a22500 fadd32 r11, r11, r13
6: 2ab158c22500 fadd32 r12, r12, r14
c: 1402 ret r1
; @ 0x-40, spvFAdd
0: 2aad56c22500 fadd32 r11, r11, r14
6: 2ab158e22500 fadd32 r12, r12, r15
c: 2ab55a022600 fadd32 r13, r13, r16
12: 1402 ret r1
```

Looking at the area around the two function calls (since we know that's where things are breaking), you can see this:
```
1706: 421000000000 push_exec r0l, 2
170c: 10c074e8ffff call 0x-80
1712: d21600000000 pop_exec r0l.cache, 2
1718: 02be8e2229a401308015 fcmpsel lt, r47h, r39.cache, r41.cache, u82l, 0
1722: 02bc8e2229a6513aa015 fcmpsel lt, r47l, r39.cache, r41.cache, u83l, u82h
172c: 02908e2229a5613aa005 fcmpsel lt, r4l, r39.cache, r41.cache, u82h, u83l
1736: 82944e222500413a2005 fcmpsel lt, r5l.cache, r39, r41, 0, u82l
1740: e22d00000000 mov_imm r11.cache, 0
1746: aa9dca000002 fadd32 r7.cache, r5l.discard, -0.0
174c: 621500000000 mov_imm r5, 0
1752: 7e19d80a8000 mov r6, r12.discard
1758: 62350000803f mov_imm r13, 1065353216
175e: 7e39920a8000 mov r14, r9.cache
1764: 7e3d920a8000 mov r15, r9.cache
176a: 7e41d20a8000 mov r16, r9.discard
1770: 7e31ce0a8000 mov r12, r7.discard
1776: 421000000000 push_exec r0l, 2
177c: 10c044e8ffff call 0x-40
1782: 521600000000 pop_exec r0l, 2
```
The calling convention seems to put inputs and outputs starting at r11
Weirdly, offset 1740 seems to overwrite r11 (the first output of the first function call) without saving it first. Checking the shader, it is used:
```metal
r8.xy = spvONFAdd(r1.xy, -r8.xy);
// much later...
r0.z = dot(r8.xy, r8.xy);
```
r12 does get saved into r6, so if we search for the next use of that...
```
1c7a: 9a9d4aa22400 fmul32 r7.cache, r5, r5
1c80: 9aad4cc22400 fmul32 r11.cache, r6, r6
1c86: 2a99dce22d10 fadd32 r38, r14.discard, r15.discard
1c8c: aaadce622d00 fadd32 r11.cache, r7.discard, r11.discard
```
It seems the compiler thinks r8.x is in r5, which got zeroed in offset 174c
I guess something about the next function's input being zero confused it?

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with OptNoneFail.zip and run `make -j2 run`; inspect main.swift and the VS0/VS1 shaders, then compare behavior with and without the `[[clang::optnone]]` usage. The issue provides disassembly and reproduction details, but does not define a project-side change or a clear completion condition; a maintainer must decide whether an SPIRV-Cross mitigation is appropriate.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
compilers, computer-graphics
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.