Metal 3.1 introduced a Metal regression bug which causes an infinite recursion
crash during Metal's analysis of an entry point input structure that itself
contains internal recursion. This patch works around this by replacing the
recursive input declaration with a alternate variable of type void*, and
then casting to the correct type at the top of the entry point function.
- Add CompilerMSL::Options::replace_recursive_inputs to enable
replacing recursive input.
- Add Compiler::type_contains_recursion() to determine if a struct
contains internal recursion, and add custom Decorations to mark
such structs, to short-cut future similar checks.
- Replace recursive input struct declarations with void*,
and emit a recast to correct type at top of entry function.
- Add unit test.
- Compiler::type_is_top_level_block() remove hardcode reference to spirv_cross
namespace, as it interferes with configurable namespaces (unrelated).
Normally, I wouldn't have bothered with this, given that we already
support the Vulkan 1.1 subgroup functionality, but a client asked for
the legacy extensions.
It is possible in SPIR-V to declare multiple specialization constants
with the same constant ID. The most common cause of this in GLSL is
defining a spec constant, then declaring the workgroup size to use that
spec constant by its ID. But, MSL forbids defining multiple function
constants with the same function constant ID. So, we must only emit one
definition of the actual function constant (with the
`[[function_constant(id)]]` attribute); but we can point the other
variables at this one definition.
Fixes three tests in the Vulkan CTS under
`dEQP-VK.compute.basic.max_local_size_*`.
Some Metal devices have a bug with depth array textures using comparison
with explicit LoD, where the LoD given will be biased by some amount.
For these devices, we can use a gradient instead, which does not exhibit
this problem. As with the fragment demote workaround, this is only
expected to be needed until the bug is fixed in Metal.
Undef values may be of struct type and may be used in constants.
Therefore, they must be interleaved with constants and types.
Fixes the rest of the Vulkan CTS test
`dEQP-VK.spirv_assembly.instruction.compute.opundef.undefined_spec_constant_composite`.
(Please excuse the churn in the reference output; it's an inevitable
result of this change.)
Fixes the CTS test
`dEQP-VK.spirv_assembly.instruction.compute.opundef.undefined_constant_composite`
and helps with another,
`dEQP-VK.spirv_assembly.instruction.compute.opundef.undefined_spec_constant_composite`.
Unfortunately, fixing the latter requires another change.
Some Metal devices have a bug where storage resources can still be
written to even if the fragment is discarded. This is obviously a bug in
Metal, but bothering Apple to fix it will only fix it for newer
versions; therefore, a workaround is needed for older versions. I have
made this an option so that, in case the bug is ever fixed, the
workaround can be disabled.
This workaround is simple: if a fragment shader may discard its fragment
and writes to a storage resource, a variable representing the
`HelperInvocation` built-in is created and passed to all functions. The
flag is checked on all resource writes; writes do not occur when
`HelperInvocation` is `true`. This relies on the earlier workaround to
update `HelperInvocation` when the fragment is discarded.
Fixes at least 3 failures in the CTS.
It is possible to pass unsigned integers to `OpSMulExtended`. In that
case, we want to do a signed multiply with sign extension, so make sure
the operands are forced to be interpreted as signed.
This was an oversight on my part when I added these instructions.
Fixes the CTS test
`dEQP-VK.spirv_assembly.instruction.compute.signed_op.uint_smulextended`.
Some Metal devices have a bug where `simd_is_helper_thread()` won't
return true after a fragment has been discarded. We can work around this
by manually setting `gl_HelperInvocation` upon discarding a fragment.
This is fairly unintrusive, so it is enabled by default. I've made it an
option so that, when the bug is fixed, we can disable it.
This op creates a new composite constant with one element replaced. So,
we reconstruct the `SPIRConstant` for the composite constant, but with
one of the IDs replaced. Constant initializer lists are memoized for
when the result of a `CompositeInsert` is used in another
`CompositeInsert`.
(I wanted to add a test case for GLSL as well, but for two things:
1. `glslang` in Vulkan mode chokes on the first constant array,
insisting that its initializer needs to be a constant. [Bug in
glslang?]
2. The declarations for the buffers used by the shader aren't emitted,
regardless of whether Vulkan mode is enabled.)
Fixes five tests under
`dEQP-VK.spirv_assembly.instruction.*.opspecconstantop.vector_related`.
Flattening doesn't play well with dynamic indices. In this case, it's
better to leave it as an array of structs.
(I wanted to do this for named blocks generally. Trouble is, the builtin
`gl_out` block is *also* a named block...)
Fixes six more CTS tests, under
`dEQP-VK.tessellation.user_defined_io.per_patch_block_array.*`.
Using vertex-style stage input is complex, and it doesn't support
nesting of structures or arrays. By using raw buffer input instead, we
get this support "for free," and everything becomes much simpler.
Arguably, this is the way I should've done this in the first place.
Eventually, I'd like to make this the default, and then remove the
option altogether. (And I still need to do that with
`multi_patch_workgroup`...)
Should help fix 66 tests in the Vulkan CTS, under the following trees:
- `dEQP-VK.pipeline.*.interface_matching.*`
- `dEQP-VK.tessellation.user_defined_io.*`
- `dEQP-VK.clipping.user_defined.*`
It is allowed to redeclare descriptors with different types in Vulkan.
MSL in general does not allow this, but for raw buffers, we can cast the
reference type at the very least.
For typed resources we are kinda hosed. Without descriptor indexing's
PARTIALLY_BOUND_BIT, descriptors must be valid if they are statically
accessed, so it would not be valid to access differently typed aliases
unless that flag is used. There might be a way to reinterpret cast
descriptors, but that seems very sketchy.
Implements support for:
- Single discrete descriptor
- Single argument buffer descriptor
- Array of argument buffer descriptors
Other cases are unimplemented for now since they are extremely painful
to unroll.
The standard `matrix` type in MSL lacked a constructor in the
`threadgroup` AS. This means that it was impossible to declare a
`threadgroup` variable that contains a matrix. This appears to have been
an oversight that was corrected in macOS 13/Xcode 14 beta 4. This
workaround continues to be required, however, for older systems.
To avoid changing interfaces unnecessarily (which shouldn't be a problem
regardless because the old and new types take up the same amount of
storage), only do this for structs if the struct is positively
identified as being used for workgroup storage.
I'm entirely aware this is inconsistent with the way packed matrices are
handled. One of them should be changed to match the other. Not sure
which one.
Fixes 23 CTS tests under `dEQP-VK.memory_model.shared`.
Since `bool` is a logical type, it cannot be used in uniform or storage
buffers. Therefore, replacing it in structures should not change the
shader interface.
We leave it alone for builtins. (FIXME: Should we also leave it for
I/O varyings?)
Fixes 24 CTS tests under `dEQP-VK.memory_model.shared`.
- Assign ulongn physical type to buffer pointers in short arrays
when array stride is larger than pointer size.
- Support GL_EXT_buffer_reference_uvec2 casting
buffer reference pointers to and from uvec2 values.
- When packing structs, include structs inside physical buffers.
- Update mechanism for traversing pointer arrays when calculating type sizes.
- Added unit test shaders.
- Determine sizing and alignments of pointers to types as
distinct from the size and alignment of the types themselves.
- Declare all buffer pointers in the MSL device address space.
- Support struct pointer recursion, where structs can
contain pointers to themselves or to a parent struct.
- Add SPIRType::was_forward_referenced to track if a type was forward
referenced, to help emit MSL structs in the correct dependency order.
- Handle pointers to pointers that are not just arrays of arrays.
HLSL is very fussy about fallthrough in switch blocks, so promote
Unreachable blocks to breaks if they are inside a switch construct.
Some false positives are possible in weird multi-break scenarios, but
this is benign.
Test for array presence using is_array() instead of element count.
Add shaders-msl/vert/interface-block-single-element-array.vert regression test.
Fixes a regression error introduced in 3d4daab.
This allows two variables of the same struct type to be flattened
into the same interface struct without a member name conflict.
Add shaders-msl/frag/in_block_with_multiple_structs_of_same_type.frag
unit test shader to demonstrate this.
Makes codegen from typical D3D emulation SPIR-V more readable.
Also makes cross compilation with NotEqual more sensible.
It's very rare to actually need the strict NaN-checks in practice.
Also, glslang now emits UnordNotEqual by default it seems, so give up
trying to assume OrdNotEqual. Harmonize for UnordNotEqual as the sane
default.
Fixes numerous CTS tests of types
dEQP-VK.pipeline.interface_matching.vector_length.member_of_*,
passing complex nested structs between stages as stage I/O.
- Make add_composite_member_variable_to_interface_block() recursive to allow
struct members to contain nested structs, building up member names and access
chains recursively, and only add the resulting flattened leaf members to the
synthetic input and output interface blocks.
- Recursively generate individual location numbers for the flattened members
of the input/output block.
- Replace to_qualified_member_name() with append_member_name().
- Update add_variable_to_interface_block() to support arrays as struct members,
adding a member to input and output interface blocks for each element of the array.
- Pass name qualifiers to add_plain_member_variable_to_interface_block() to allow
struct members to be arrays of structs, building up member names and access chains,
and adding multiple distinct flattened leaf members to the synthetic input and
output interface blocks.
- Generate individual location numbers for the individual array members
of the input/output block.
- SPIRVCrossDecorationInterfaceMemberIndex references the index of a member
of a variable that is a struct type. The value is relative to the variable,
and for structs nested within that top-level struct, the index value needs
to take into consideration the members within those nested structs.
- Pass var_mbr_idx to add_plain_member_variable_to_interface_block() and
add_composite_member_variable_to_interface_block(), start at zero for each
variable, and increment for each member or nested member within that variable.
- Add unit test shaders-msl/vert/out-block-with-nested-struct-array.vert
- Add unit test shaders-msl/vert/out-block-with-struct-array.vert
- Add unit test shaders-msl/tese/in-block-with-nested-struct.tese
Fragment shaders that require explicit early fragment tests are incompatible
with specifying depth and stencil values within the shader. If explicit early
fragment tests is specified, remove the depth and stencil outputs from the
output structure, and replace them with dummy local variables.
Add CompilerMSL:uses_explicit_early_fragment_test() function to consolidate
testing for whether early fragment tests are required.
Add two unit tests for depth-out with, and without, early fragment tests.
SPIR-V allows an image to be marked as a depth image, but with a non-depth
format. Such images should be read or sampled as vectors instead of scalars,
except when they are subject to compare operations.
Don't mark an OpSampledImage as using a compare operation just because the
image contains a depth marker. Instead, require that a compare operation
is actually used on that image.
Compiler::image_is_comparison() was really testing whether an image is a
depth image, since it incorporates the depth marker. Rename that function
to is_depth_image(), to clarify what it is really testing.
In Compiler::is_depth_image(), do not treat an image as a depth image
if it has been explicitly marked with a color format, unless the image
is subject to compare operations.
In CompilerMSL::to_function_name(), test for compare operations
specifically, rather than assuming them from the depth-image marker.
CompilerGLSL and CompilerMSL still contain a number of internal tests that
use is_depth_image() both for testing for a depth image, and for testing
whether compare operations are being used. I've left these as they are
for now, but these should be cleaned up at some point.
Add unit tests for fetch/sample depth images with color formats and no compare ops.
Previous test for SPIRVCrossDecorationPhysicalTypePacked on parent struct
when unpacking member struct was too restrictive, and not needed as long
as padding compensates.
Populate member_type_index_redirection as reverse lookup, not forward lookup.
Move use of member_type_index_redirection from CompilerMSL::to_member_reference()
to CompilerGLSL::access_chain_internal() to access all redirected type info,
not just name.
Promote to short instead and do simple casts on load/store instead.
Not 100% complete fix since structs can contain booleans, but this is
getting into pretty ridiculously complicated territory.
Add test shader for new functionality.
Add legacy test reference shader for unrelated buffer-bitcast
test, that doesn't seem to have been added previously.
When gl_Position is defined by SPIR-V, but neither used nor initialized,
it appeared twice in the MSL output, as gl_Position and glPosition_1.
The existing tests for whether an output is active check only that it is
used by an op, or initialized. Adding the implicit gl_Position also marked
the existing gl_Position as active, duplicating the output variable.
Fix is that when checking for the need to add an implicit gl_Position
output, also check if the var is already defined in the shader,
and just needs to be marked as active.
Add test shader.
Vulkan specifies that the Sample Mask Test occurs before fragment shading.
This means gl_SampleMaskIn should be influenced by both sample-shading and
VkPipelineMultisampleStateCreateInfo::pSampleMask.
CTS tests dEQP-VK.pipeline.multisample_shader_builtin.* bear this out.
For sample-shading, gl_SampleMaskIn should only have a single bit set,
Since Metal does not filter for this, apply a bitmask based on gl_SampleID.
For a fixed sample mask, since Metal is unaware of
VkPipelineMultisampleStateCreateInfo::pSampleMask, we need to ensure that
we apply it to both gl_SampleMaskIn and gl_SampleMask. This has the side
effect of a redundant application of pSampleMask if the shader already
includes gl_SampleMaskIn when setting gl_SampleMask, but I don't see an
easy way around this.
Also, simplify the logic for including the fixed sample mask in gl_ShaderMask,
and print the fixed sample mask as a hex value for readability of bits.
We'll need to force a temporary and mark it as precise.
MSL is a little weird here, but we can piggyback on top of the invariant
float math option here to force fma() operations everywhere.
