From a03b686805ae33af88237536489c62d73e901f8d Mon Sep 17 00:00:00 2001 From: Aitor Camacho Date: Mon, 1 Dec 2025 19:13:11 +0900 Subject: [PATCH] kk: Enable fragmentStoresAndAtomics Metal will prematurely discard fragments with side effects even if those side effects happen before the discard. Work around this by making said discards "optional". Reviewed-by: Arcady Goldmints-Orlov Signed-off-by: Aitor Camacho Part-of: --- docs/drivers/kosmickrisp/workarounds.rst | 22 ++++++++++++++-- .../compiler/msl_nir_lower_common.c | 26 +++++++++++++++++++ src/kosmickrisp/compiler/nir_to_msl.h | 1 + src/kosmickrisp/vulkan/kk_physical_device.c | 8 +----- src/kosmickrisp/vulkan/kk_shader.c | 4 +++ 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/docs/drivers/kosmickrisp/workarounds.rst b/docs/drivers/kosmickrisp/workarounds.rst index 960e4a1fced..d391a664eb5 100644 --- a/docs/drivers/kosmickrisp/workarounds.rst +++ b/docs/drivers/kosmickrisp/workarounds.rst @@ -49,6 +49,24 @@ info on what was updated. Workarounds =========== +KK_WORKAROUND_5 +--------------- +| macOS version: 26.0.1 +| Metal ticket: Not reported +| Metal ticket status: +| CTS test failure: ``dEQP-VK.fragment_operations.early_fragment.discard_no_early_fragment_tests_depth`` +| Comments: + +Fragment shaders that have side effects (like writing to a buffer) will be +prematurely discarded if there is a ``discard_fragment`` that will always +execute. To work around this, we just make the discard "optional" by moving +it inside a run time conditional that will always be true (such as is the +fragment a helper?). This tricks the MSL compiler into not optimizing it into +a premature discard. + +| Log: +| 2025-12-01: Workaround implemented + KK_WORKAROUND_4 --------------- | macOS version: 26.0.1 @@ -98,8 +116,8 @@ The way to fix this is by changing the conditional to: KK_WORKAROUND_2 --------------- | macOS version: 15.4.x -| Metal ticket: Not reported -| Metal ticket status: +| Metal ticket: FB21065475 (@aitor) +| Metal ticket status: Waiting resolution | CTS test crash: ``dEQP-VK.graphicsfuzz.cov-nested-loops-never-change-array-element-one`` and ``dEQP-VK.graphicsfuzz.disc-and-add-in-func-in-loop`` | Comments: diff --git a/src/kosmickrisp/compiler/msl_nir_lower_common.c b/src/kosmickrisp/compiler/msl_nir_lower_common.c index bed6cb2f9f0..12a9582ed1e 100644 --- a/src/kosmickrisp/compiler/msl_nir_lower_common.c +++ b/src/kosmickrisp/compiler/msl_nir_lower_common.c @@ -283,6 +283,32 @@ msl_nir_vs_io_types(nir_shader *nir) NULL); } +static bool +fake_guard_for_discards(nir_builder *b, nir_intrinsic_instr *intrin, void *data) +{ + if (intrin->intrinsic != nir_intrinsic_demote) + return false; + + b->cursor = nir_before_instr(&intrin->instr); + nir_def *helper = nir_is_helper_invocation(b, 1); + nir_demote_if(b, nir_inot(b, helper)); + nir_instr_remove(&intrin->instr); + return true; +} + +bool +msl_nir_fake_guard_for_discards(struct nir_shader *nir) +{ + assert(nir->info.stage == MESA_SHADER_FRAGMENT); + + /* No side effects, no lowering needed */ + if (!nir->info.writes_memory) + return false; + + return nir_shader_intrinsics_pass(nir, fake_guard_for_discards, + nir_metadata_control_flow, NULL); +} + static bool lower_clip_distance(nir_builder *b, nir_intrinsic_instr *intr, void *data) { diff --git a/src/kosmickrisp/compiler/nir_to_msl.h b/src/kosmickrisp/compiler/nir_to_msl.h index 6919c18f2cb..89947a46b42 100644 --- a/src/kosmickrisp/compiler/nir_to_msl.h +++ b/src/kosmickrisp/compiler/nir_to_msl.h @@ -55,4 +55,5 @@ bool msl_ensure_depth_write(nir_shader *nir); bool msl_ensure_vertex_position_output(nir_shader *nir); bool msl_nir_fs_io_types(nir_shader *nir); bool msl_nir_vs_io_types(nir_shader *nir); +bool msl_nir_fake_guard_for_discards(struct nir_shader *nir); void msl_lower_nir_late(nir_shader *nir); diff --git a/src/kosmickrisp/vulkan/kk_physical_device.c b/src/kosmickrisp/vulkan/kk_physical_device.c index 58dd08d8bb8..abecafdf507 100644 --- a/src/kosmickrisp/vulkan/kk_physical_device.c +++ b/src/kosmickrisp/vulkan/kk_physical_device.c @@ -155,13 +155,7 @@ kk_get_device_features( .depthClamp = true, .drawIndirectFirstInstance = true, .dualSrcBlend = true, - /* TODO_KOSMICKRISP - * Enabling fragmentStoresAndAtomics fails the following CTS tests, need - * to investigate: - * dEQP-VK.fragment_operations.early_fragment.discard_no_early_fragment_tests_depth - * dEQP-VK.robustness.image_robustness.bind.notemplate.*i.unroll.nonvolatile.sampled_image.no_fmt_qual.img.samples_1.*d_array.frag - */ - .fragmentStoresAndAtomics = false, + .fragmentStoresAndAtomics = true, .fullDrawIndexUint32 = true, .imageCubeArray = true, .independentBlend = true, diff --git a/src/kosmickrisp/vulkan/kk_shader.c b/src/kosmickrisp/vulkan/kk_shader.c index 1b4598cabbd..1dabf5cc605 100644 --- a/src/kosmickrisp/vulkan/kk_shader.c +++ b/src/kosmickrisp/vulkan/kk_shader.c @@ -66,6 +66,7 @@ kk_get_nir_options(struct vk_physical_device *vk_pdev, mesa_shader_stage stage, .lower_doubles_options = (nir_lower_doubles_options)(~0), .lower_int64_options = nir_lower_ufind_msb64 | nir_lower_subgroup_shuffle64, + .io_options = nir_io_mediump_is_32bit, }; return &options; } @@ -417,6 +418,9 @@ kk_lower_fs(struct kk_device *dev, nir_shader *nir, nir->info.fs.needs_coarse_quad_helper_invocations) NIR_PASS(_, nir, msl_lower_static_sample_mask, 0xFFFFFFFF); + /* KK_WORKAROUND_5 */ + if (!(dev->disabled_workarounds & BITFIELD64_BIT(5))) + NIR_PASS(_, nir, msl_nir_fake_guard_for_discards); /* KK_WORKAROUND_4 */ if (!(dev->disabled_workarounds & BITFIELD64_BIT(4))) { NIR_PASS(_, nir, nir_lower_helper_writes, true);