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

Calling methods on large structs may lead to numerous copies #4037

Closed
dubiousconst282 opened this issue Apr 25, 2024 · 2 comments
Closed

Calling methods on large structs may lead to numerous copies #4037

dubiousconst282 opened this issue Apr 25, 2024 · 2 comments
Assignees
Labels
goal:client support Feature or fix needed for a current slang user. goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang

Comments

@dubiousconst282
Copy link
Contributor

Repro:

// slangc -target spirv-asm -emit-spirv-directly -profile glsl_460 -lang slang -O0 test.slang
struct WorkData {
    float A[16 * 16];
    float B[16 * 16];

    float Foo(uint i) { return A[i] * B[i]; }
};
ConstantBuffer<WorkData> input;
RWStructuredBuffer<float> dest;

[numthreads(64, 1, 1)]
void ComputeMain(uint tid: SV_DispatchThreadID)
{
    dest[tid] = input.Foo(tid);
}

The generated SPIRV contains numerous OpCompositeExtract instructions followed by a OpCompositeConstruct, which I'd hope to be redundant. Enabling optimizations makes them go away but I think only because of inlining, so I would prefer not to rely on it.

This is not specific to ConstantBuffer and also happens with pointers.

@bmillsNV bmillsNV added goal:client support Feature or fix needed for a current slang user. goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang labels Apr 30, 2024
@csyonghe csyonghe added this to the Q2 2024 (Spring) milestone Apr 30, 2024
@csyonghe
Copy link
Collaborator

The copy is actually correct by the semantic meaning of the code.
In Slang, if you call a function that takes a struct as parameter, it semantically means the argument is passed by value (copying the value in), and it is expected that the compiler optimization will take place to remove unnecessary copies.

General support of passing by reference is unfortunately not possible in many platforms. For example, HLSL and GLSL does not have a notion of pointers, and pointers is only valid in SPIRV with restrictions.

This code when compiled with DXC is producing similar results with -O0.

It is possible for us to do a better job when targeting SPIRV, but it is not clear to me that will result in more performant code after optimization, since the optimization passes are designed to be good at removing this kind of copies.

@csyonghe
Copy link
Collaborator

I am closing this issue for now, since there is not much we can do in the short term. We cannot generate code that makes pervasive use of pointers in parameter passing given the restrictions of many targets just yet.

@csyonghe csyonghe closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:client support Feature or fix needed for a current slang user. goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang
Projects
None yet
Development

No branches or pull requests

3 participants