Implement for metal SV_GroupIndex
#4385
Conversation
1. If we don't have `sv_GroupThreadId` available we create one using `SV_GroupIndex`s location data. 2. We emit code emulating `sv_GroupThreadId` from the same logic that CUDA/CPP uses.
| auto groupIndexCalc = emitCalcGroupThreadIndex(svBuilder, groupThreadId, computeExtent); | ||
| svBuilder.emitStore(newGroupIndex, groupIndexCalc); | ||
|
|
||
| param->replaceUsesWith(newGroupIndex); |
There was a problem hiding this comment.
What type does param have? Isn't it a int instead of int*? If so we should just replace it with groupIndexCalc? Why do we need to replace a param which is local to the entrypoint, with a global variable? We should just insert the computation of the group index within the entrypoint and use the SSA value of the index as the param?
There was a problem hiding this comment.
I see your point, I did not need to make a globalVar
|
You may also want to enable all the tests that this PR fixes. Do you have a mac to test locally? |
There was a problem hiding this comment.
I think you may want to create a new struct like MetalEntryPointVaryingParamLegalizeContext that inherits from EntryPointVaryingParamLegalizeContext and move the implemention from source/slang/slang-ir-metal-legalize.cpp.
If you do it, you wouldn't need to expose emitCalcGroupExtents() and emitCalcGroupThreadIndex() outside of "slang-ir-legalize-varying-params.cpp".
It seems strange that the functionality that Cuda and CPP use in "slang-ir-legalize-varying-params.cpp" is need in another file just for Metal.
There was a problem hiding this comment.
Reusing EntryPointVaryingParamLegalizeContext here should also allow us to fix #4375, because we can just use the logic here to SOA-ify all the varying inputs and hoist them all to top level.
I do not have a Mac locally setup, |
Addressed all but two: [1](shader-slang#4385 (comment)) and [2](shader-slang#4385 (comment)) I want to enable tests and be sure there is no bugs using CI before I redesign the code so I have a working fallback.
enable now functioning tests due to `SV_GroupIndex` working with metal
did `= param` instead of `= i.param`
|
I am going to merge this PR as is. It seems that a natural next step following this is to investigate how we can reuse the entrypoint param legalization logic for metal, so that we can convert all vertex attribute inputs and fragment stage_in inputs buried inside nested structs into top level entry point parameters. |
|
Yes, we will do the refactoring as a part of #4375. |
v2024.1.22 c00f461 remove inline that crashes old glibc version (shader-slang#4398) fdef653 Improve Direct SPIRV Backend Test Coverage (shader-slang#4396) 33e81a0 [Metal] Fix global constant array emit. (shader-slang#4392) a38a4fb Make unknown attributes a warning instead of an error. (shader-slang#4391) a210091 [Metal] Support SV_TargetN. (shader-slang#4390) 2cc9690 Implement for metal `SV_GroupIndex` (shader-slang#4385) a6b8348 fix the clang/gcc warning (shader-slang#4382) cfef0c6 Metal: misc fixes and enable more tests. (shader-slang#4374) 2407966 SPIR-V `SV_InstanceId` support in pixel shader (shader-slang#4368) fba316f Remove `IRHLSLExportDecoration` and `IRKeepAliveDecoration` for non-CUDA/Torch targets (shader-slang#4364) f0d40ad (origin/master, origin/HEAD, master) capture/replay: implement infrastructure for capture (shader-slang#4372) ecc6ecb Fix cuda/cpp/metal crash for when using GLSL style shader inputs (shader-slang#4378) 0bf0bf7 Implement Sampler2D for CPP target (shader-slang#4371) b970b88 Enable full test on macos. (shader-slang#4327) 0574dca Delete glsl_vulkan and glsl_vulkan_one_desc targets. (shader-slang#4361) 085d1a6 Fix emit logic for getElementPtr. (shader-slang#4362) 8813c61 Capability System: Implicit capability upgrade warning/error (shader-slang#4241) 7447fca Add constant folding for % operator. (shader-slang#4359) 8bf7d11 Fix merge error. (shader-slang#4358) b7e8243 Add slangc flag to `-zero-initialize` all variables (shader-slang#3987) ccc26c2 Extend the COM-based API to support whole program compilation. (shader-slang#4355) 318adcc Add compiler option to treat enum types as unscoped. (shader-slang#4354) ec35feb Fix incorrect drop of decoration when translating glsl global var to entrypoint param. (shader-slang#4353) c194af8 Fix crash on invalid entrypoint varying parameter. (shader-slang#4349) 7a4757d Implicit register binding for hlsl to non-hlsl targets (shader-slang#4338) 180d6b1 Fix duplicate SPIRV decorations. (shader-slang#4346) fa8c11e Add option to preserve shader parameter declaration in output SPIRV. (shader-slang#4344) 3fe4a77 Fix crash when using optional type in a generic. (shader-slang#4341) 5da06d4 Fix global value inlining for spirv_asm blocks. (shader-slang#4339) 7e79669 [gfx] Metal improvements (shader-slang#4337) 6909d65 SPIRV backend: add support for tessellation stages, (shader-slang#4336) ef20d93 Test more texture types in Metal (shader-slang#4333) 5a28968 [gfx] Metal texture fixes (shader-slang#4331) df0a201 Support integer typed textures for GLSL (shader-slang#4329) 51d3585 Remove duplicate `VkPhysicalDeviceComputeShaderDerivativesFeaturesNV` extension structure in vk-api.h (shader-slang#4335) 6d5ef9b Fix `GetAttributeAtVertex` for spirv and glsl targets. (shader-slang#4334) 21bbebb Address glslang ordering requirments for 'derivative_group_*NV' (shader-slang#4323) 72016f9 Partial implementation of static_assert (shader-slang#4294) 712ce65 enable more metal tests (shader-slang#4326) 38c0bac Fix SPIRV emit for `Flat` decoration and TessLevel builtin. (shader-slang#4318) b5cdd83 Support all integer typed indices in StructuredBuffer Load/Store/[]. (shader-slang#4311) 6857dd5 [gfx] Metal graphics support (shader-slang#4324) 0974463 Fix typos in the docs (shader-slang#4322) 9a23a9a SPIRV `Block` decoration fixes. (shader-slang#4303) bc680e7 Add initial draft auto-diff basics and IR overview documents (shader-slang#4216) ee812d1 Disallow certain types of decls in `interface` to provide better diagnostic message. (shader-slang#4312) 65928af Metal system value overhaul. (shader-slang#4308) e39ceab Adding functional test for GLSL texture functions (shader-slang#4306) 2a45bc3 Support HLSL `.mips` syntax. (shader-slang#4310) 056a4b9 Small SPIRV emit cleanup around vector element extract. (shader-slang#4309) f83fe55 Make CTS failure report more obvious (shader-slang#4302) 78d34f3 Improve documentation and example formatting consistency (shader-slang#4299) 7c6faf6 Precompute UIntSet from individual capabilities inside generator (shader-slang#4269) 004fe27 Metal compute tests (shader-slang#4292) 72f10a8 Fixed profile string per request in pr#4268 (shader-slang#4297) a709e02 Update capability-generator-main.cpp (shader-slang#4295) d301267 Typo in 06-interfaces-generics.md (shader-slang#4284) fa664d1 Fix build warnings and treat warnings as error on CI (shader-slang#4276) f149052 Remove unnecessary call to __requireComputeDerivative (shader-slang#4283)
fixes: #4370
SV_GroupThreadIdavailable we create one usingSV_GroupIndexs location data sinceSV_GroupIndexis not an actual SV for metal.SV_GroupThreadIdfrom the same logic that CUDA/CPP uses.note: added
SV_GroupIdtranslation tothreadgroup_position_in_gridso I can get aSV_GroupIndextest to run