KhronosGroup / KhronosGroup/SPIRV-Cross
[MSL] SPIRV-Cross generates undefined behaviour for OpSMod
- Dominant language
- GLSL
- Stars
- 2.5k
- Forks
- 713
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 16
Description
Given the following shader that computes the mod by 32 of a number provided as an input:
```
; Magic: 0x07230203 (SPIR-V)
; Version: 0x00010300 (Version: 1.3.0)
; Generator: 0x00220001 (SPIRVSmith)
; Schema: 0
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpDecorate %struct_t Block
OpDecorate %struct_variable DescriptorSet 0
OpDecorate %struct_variable Binding 0
OpMemberDecorate %struct_t 0 Offset 0
%void_t = OpTypeVoid
%main_t = OpTypeFunction %void_t
%int_t = OpTypeInt 32 1
%struct_t = OpTypeStruct %int_t
%int_ptr_t = OpTypePointer StorageBuffer %int_t
%struct_ptr_t = OpTypePointer StorageBuffer %struct_t
%const_mod_operand = OpConstant %int_t 32
%const_zero = OpConstant %int_t 0
%struct_variable = OpVariable %struct_ptr_t StorageBuffer
%main = OpFunction %void_t DontInline %main_t
%main_label = OpLabel
%struct_element_ptr = OpAccessChain %int_ptr_t %struct_variable %const_zero
%struct_element = OpLoad %int_t %struct_element_ptr
%op_mod = OpSMod %int_t %struct_element %const_mod_operand
OpStore %struct_element_ptr %op_mod
OpReturn
OpFunctionEnd
```
SPIRV-Cross generates the following MSL equivalent:
```
#include
#include
using namespace metal;
struct _2
{
int _m0;
};
kernel void main0(device _2& _3 [[buffer(0)]])
{
_3._m0 %= 32;
}
```
This is undefined if the input is negative, as specified in [MSL Spec Section 3.1.2](https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf). Could we have a safe wrapper for `OpSMod`, similar to the one generated for `OpFMod`?
Note that this is an issue even in the constant case where we don't perform the mod on an operand from the input buffer:
```
; Magic: 0x07230203 (SPIR-V)
; Version: 0x00010300 (Version: 1.3.0)
; Generator: 0x00220001 (SPIRVSmith)
; Schema: 0
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %main "main"
OpExecutionMode %main LocalSize 1 1 1
OpDecorate %struct_t Block
OpDecorate %struct_variable DescriptorSet 0
OpDecorate %struct_variable Binding 0
OpMemberDecorate %struct_t 0 Offset 0
%void_t = OpTypeVoid
%main_t = OpTypeFunction %void_t
%int_t = OpTypeInt 32 1
%struct_t = OpTypeStruct %int_t
%int_ptr_t = OpTypePointer StorageBuffer %int_t
%struct_ptr_t = OpTypePointer StorageBuffer %struct_t
%const_mod_operand1 = OpConstant %int_t 32
%const_mod_operand2 = OpConstant %int_t -22
%const_zero = OpConstant %int_t 0
%struct_variable = OpVariable %struct_ptr_t StorageBuffer
%main = OpFunction %void_t DontInline %main_t
%main_label = OpLabel
%struct_element_ptr = OpAccessChain %int_ptr_t %struct_variable %const_zero
%op_mod = OpSMod %int_t %const_mod_operand1 %const_mod_operand2
OpStore %struct_element_ptr %op_mod
OpReturn
OpFunctionEnd
```
Translates to:
```
#include
#include
using namespace metal;
struct _2
{
int _m0;
};
kernel void main0(device _2& _3 [[buffer(0)]])
{
_3._m0 = (-22) % 32;
}
```
As far as my understanding of the SPIR-V spec goes, taking the mod of a negative number is legal and doesn't produce UB but I'd be happy if anyone can point to me something that says otherwise.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by locating the OpSMod handling in the MSL backend and compare it with the existing OpFMod wrapper. Use the two SPIR-V examples in this issue to check negative input and constant operands; done means generated MSL preserves the defined SPIR-V signed-modulo behavior without undefined operations.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100