Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIR-V] static variable in template structures cannot pass to other functions with library profile #7049

Closed
GinShio opened this issue Jan 8, 2025 · 4 comments
Labels
bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V
Milestone

Comments

@GinShio
Copy link

GinShio commented Jan 8, 2025

Description

Hi, I want to use a series template structures as trait.
When I pass the static constant variables to function or return it in function.
I get the undef value.

Of course, if profile is normal stage (e.g. fragment) it looks good, but it failed if profile is library.

Steps to Reproduce

HLL

template <typename S> struct Trait;
template <> struct Trait<half> {
  using type = half;
  static const uint size = 2;
};

uint get_size() { return Trait<half>::size; }
float cvt(uint x) { return float(x); }

export float testcase(int x) {
  if (x == 2)
    return float(Trait<half>::size); // return 2.0
  else if (x == 4)
    return cvt(Trait<half>::size); // return undef
  else if (x == 8)
    return get_size(); // return undef
  return 0.f;
}

compilation

dxc -O3 -T lib_6_8 -HV 2021 -spirv -fcgl -fspv-target-env=universal1.5 -Fo test.spv test.hlsl

Actual Behavior

%size = OpVariable %_ptr_Private_uint Private
   %testcase = OpFunction %float None %11
          %x = OpFunctionParameter %_ptr_Function_int
   %bb_entry = OpLabel
%param_var_x = OpVariable %_ptr_Function_uint Function
         %26 = OpIEqual %bool %25 %int_4
               OpSelectionMerge %if_merge_0 None
               OpBranchConditional %26 %if_true_0 %if_false_0
  %if_true_0 = OpLabel
         %30 = OpLoad %uint %size
               OpStore %param_var_x %30
         %31 = OpFunctionCall %float %cvt %param_var_x
               OpReturnValue %31

%size is not initialized.

Environment

  • DXC version libdxcompiler.so: 1.8(dev;4662-416fab6b); libdxil.so: 1.8
  • Host Operating System openSUSE Tumbleweed x86_64 6.12.6-1-default
@GinShio GinShio added bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V labels Jan 8, 2025
@s-perron
Copy link
Collaborator

s-perron commented Jan 8, 2025

Having a function with the export keyword is not a complete feature in HLSL. In general, we do not have rules on how the shaders should be linked. In fact, it is not possible to call testcase for another HLSL file without defining it. The extern keyword is not part of HLSL.

In general, the way we handle initialization of private variable is to initialize them at the start of the entry point. This is working see https://godbolt.org/z/reGonbsG1.

So I consider this working as far as the HLSL spec is concerned. Since development of DXC is winding down, we will not support a new work flow in DXC. If you want to follow up on this, you can open an issue in https://github.com/microsoft/hlsl-specs/ to define how HLSL compilation unit should be linked, and it could be implemented in clang.

@s-perron s-perron closed this as completed Jan 8, 2025
@github-project-automation github-project-automation bot moved this to Triaged in HLSL Triage Jan 8, 2025
@GinShio
Copy link
Author

GinShio commented Jan 10, 2025

Having a function with the export keyword is not a complete feature in HLSL. In general, we do not have rules on how the shaders should be linked. In fact, it is not possible to call testcase for another HLSL file without defining it. The extern keyword is not part of HLSL.

Yes, you're right.
A tricky method is hacking spir-v assemble... We can use linkage attribute to mark function is external.

               OpCapability Shader
               OpCapability Linkage
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %main "main" %gl_LocalInvocationIndex %_
               OpExecutionMode %main LocalSize 1 1 1
               OpSource GLSL 450
               OpDecorate %gl_LocalInvocationIndex BuiltIn LocalInvocationIndex
               OpDecorate %_runtimearr_float ArrayStride 4
               OpDecorate %Buffer Block
               OpMemberDecorate %Buffer 0 Offset 0
               OpDecorate %_ Binding 0
               OpDecorate %_ DescriptorSet 0
               OpDecorate %testcase_i1_ LinkageAttributes "testcase" Import
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
        %int = OpTypeInt 32 1
%_ptr_Function_int = OpTypePointer Function %int
      %float = OpTypeFloat 32
          %9 = OpTypeFunction %float %_ptr_Function_int
       %uint = OpTypeInt 32 0
%_ptr_Function_uint = OpTypePointer Function %uint
%_ptr_Input_uint = OpTypePointer Input %uint
%gl_LocalInvocationIndex = OpVariable %_ptr_Input_uint Input
%_runtimearr_float = OpTypeRuntimeArray %float
     %Buffer = OpTypeStruct %_runtimearr_float
%_ptr_StorageBuffer_Buffer = OpTypePointer StorageBuffer %Buffer
          %_ = OpVariable %_ptr_StorageBuffer_Buffer StorageBuffer
      %int_0 = OpConstant %int 0
%_ptr_StorageBuffer_float = OpTypePointer StorageBuffer %float
%testcase_i1_ = OpFunction %float None %9
          %x = OpFunctionParameter %_ptr_Function_int
               OpFunctionEnd
       %main = OpFunction %void None %3
          %5 = OpLabel
         %id = OpVariable %_ptr_Function_uint Function
      %param = OpVariable %_ptr_Function_int Function
         %22 = OpLoad %uint %gl_LocalInvocationIndex
               OpStore %id %22
         %28 = OpLoad %uint %id
         %29 = OpLoad %uint %id
         %30 = OpBitcast %int %29
               OpStore %param %30
         %32 = OpFunctionCall %float %testcase_i1_ %param
         %34 = OpAccessChain %_ptr_StorageBuffer_float %_ %int_0 %28
               OpStore %34 %32
               OpReturn
               OpFunctionEnd

And spirv-link can handling it.

spirv-link --target-env vulkan1.3 -o linked.spv test.spv lib.spv

OTOH, the result is correct if compile to dxil. Please see https://godbolt.org/z/hzsMejhET. I think this is a bug on the spir-v part.

Could we reopen it now?

@s-perron s-perron added this to the Dormant milestone Jan 10, 2025
@s-perron
Copy link
Collaborator

I think this is a bug on the spir-v part.

I disagree that this is a bug because we do not support that work flow. You are asking us to support link of different HLSL modules, which is not an HLSL feature.

Could we reopen it now?

I will reopen it, but place it in the dormant milestone. If you want to find someone to fix it, we can review the change. As I mentioned earlier, DXC development is winding down, and we are focusing more on clang. The maintainers of the SPIR-V backend will not be fixing this issue. It is not a priority for us.

@s-perron s-perron reopened this Jan 10, 2025
@GinShio
Copy link
Author

GinShio commented Jan 11, 2025

I disagree that this is a bug because we do not support that work flow. You are asking us to support link of different HLSL modules, which is not an HLSL feature.

Thanks to reply.

As I mentioned earlier, DXC development is winding down, and we are focusing more on clang.

Oh, sorry, I see. I like this. (Sorry, I didn't notice your comments migrated to clang, please forgive me.)

I will reopen it, but place it in the dormant milestone. If you want to find someone to fix it, we can review the change.

I guess this is not a issue when I use clang, and should fix it in clang if issue exists. I think it should be closed....

@github-project-automation github-project-automation bot moved this from New to Done in HLSL Roadmap Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V
Projects
Status: Done
Archived in project
Development

No branches or pull requests

2 participants