[SPIR-V] Honor inline spir-v attributes on functions#8616
Conversation
The [[vk::ext_decorate]], [[vk::ext_capability]], and [[vk::ext_extension]] attributes were only applied to variables, parameters, stage variables, entry-point functions, and functions carrying [[vk::ext_instruction]]. A plain function that carried them was skipped, so the attributes were silently dropped: no OpDecorate was emitted for its OpFunction, and no OpCapability / OpExtension was added to the module. Apply them when the function is registered (getOrRegisterFn), next to the existing linkage decoration. This reuses the same helpers as the variable and parameter paths: - the decoration goes through a new SpirvFunction overload of decorateWithIntrinsicAttrs, and - the capability/extension loops are factored into registerCapabilitiesAndExtensionsForDecl, now shared with the variable path. Only the literal form of ext_decorate is supported on functions; the id and string variants have no SpirvFunction-target decoration and are now diagnosed rather than silently dropped.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| srcLoc, targetFunc, static_cast<spv::Decoration>(decorate), literals); | ||
| assert(decor != nullptr); | ||
| mod->addDecoration(decor); | ||
| } |
There was a problem hiding this comment.
addDecoration used to be used only by decorateLinkage, which always encodes the function name into params. But now two functions both carrying the same decoration will make us drop the second one. This is failing for me:
// RUN: %dxc -T cs_6_0 -E main -fcgl %s -spirv | FileCheck %s
[[vk::ext_decorate(0)]]
[noinline] uint Foo(uint x) { return x; }
[[vk::ext_decorate(0)]]
[noinline] uint Bar(uint x) { return x + 1; }
RWStructuredBuffer<uint> buf;
[numthreads(1, 1, 1)]
void main(uint3 tid : SV_DispatchThreadID) {
buf[0] = Foo(tid.x) + Bar(tid.x);
}
// CHECK-DAG: OpDecorate %Foo RelaxedPrecision
// CHECK-DAG: OpDecorate %Bar RelaxedPrecision
The second CHECK-DAG can't find the RelaxedPrecision decoration. I think adding getTargetFunc() in the hash would fix this.
There was a problem hiding this comment.
Yep. Fixed, and added a second function in the test to demonstrate.
|
|
||
| [[vk::ext_decorate_string(/* UserTypeGOOGLE */ 5636, "myType")]] | ||
| [noinline] uint Identity(uint x) { return x; } | ||
|
|
There was a problem hiding this comment.
Could you add a second function that exercises ext_decorate_id?
There was a problem hiding this comment.
Yes. I called it DecorateId. An apt name, but maybe on the nose.
| @@ -0,0 +1,28 @@ | |||
| // RUN: %dxc -T cs_6_0 -E main -fcgl -Vd %s -spirv | FileCheck %s | |||
There was a problem hiding this comment.
If Identity is decorated with a Kernel capability, you probably don't need to disable spirv-val here.
There was a problem hiding this comment.
Dropped -Vd as suggested. Note that Kernel specifically cannot be used since spirv-val rejects OpCapability Kernel for the Vulkan target. Instead, the test now decorates with RelaxedPrecision and declares Int8 to validate cleanly. I had to drop the Alignment case since that requires Kernel, but there are other tests dedicated for decorated function operands.
Admittedly, RelaxedPrecision isn't an ideal choice. spirv-val accepts it, but it isn't listed on the SPIR-V spec as a valid target. The problem is that the only solidly valid decoration on an OpFunction is LinkageAttributes, which DXC manages itself. So we can have "no -Vd" OR "an honest, spec-clean decoration," but not both. Take your pick.
The [[vk::ext_decorate]], [[vk::ext_capability]], and [[vk::ext_extension]] attributes were only applied to variables, parameters, stage variables, entry-point functions, and functions carrying [[vk::ext_instruction]]. A plain function that carried them was skipped, so the attributes were silently dropped: no OpDecorate was emitted for its OpFunction, and no OpCapability / OpExtension was added to the module.
Apply them when the function is registered (getOrRegisterFn), next to the existing linkage decoration. This reuses the same helpers as the variable and parameter paths:
Only the literal form of ext_decorate is supported on functions; the id and string variants have no SpirvFunction-target decoration and are now diagnosed rather than silently dropped.