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

Compilation error in an atomic function parameter from a pre-compiled module #5941

Open
cezneluk opened this issue Dec 27, 2024 · 4 comments · May be fixed by #5945
Open

Compilation error in an atomic function parameter from a pre-compiled module #5941

cezneluk opened this issue Dec 27, 2024 · 4 comments · May be fixed by #5945
Assignees
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang

Comments

@cezneluk
Copy link

An Atomic<T> argument in a function from another module works only if the module is loaded from a source code, but not if the module was pre-compiled.

Current result:

Module file test_module.slang

module "test_module";

public void atomicFunc(Atomic<int>* ptr) {
    ptr.add(1);
}

Main file test.slang

import "test_module";

RWStructuredBuffer<Atomic<int>> input0;

[shader("compute")]
[numthreads(1,1,1)]
void computeMain(uint3 workGroup : SV_GroupID)
{
    atomicFunc(&input0[0]);
}

Compilation from plain source code works fine:

$ rm test_module.slang-module
$ slangc test.slang -o test.spv

But compilation with precompiled module

$ slangc test_module.slang -o test_module.slang-module
$ slangc test.slang -o test.spv

leads to a weird error:

test.slang(9): error 30019: expected an expression of type 'Ptr<Atomic<int>>', got 'Ptr<Atomic<int>>'
    atomicFunc(&input0[0]);
               ^
test.slang(9): note: explicit conversion from 'Ptr<Atomic<int>>' to 'Ptr<Atomic<int>>' is possible

Tested on binaries from the latest release (v2024.17) and built from the current master (7cecc51).

Expected result:

Shader will be successfully compiled and produce the same result as compiling without a pre-compiled module.

@csyonghe csyonghe self-assigned this Dec 28, 2024
@csyonghe csyonghe added the goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang label Dec 28, 2024
@csyonghe csyonghe added this to the Q4 2024 (Fall) milestone Dec 28, 2024
@csyonghe
Copy link
Collaborator

The type system error is fixed in #5945.

However the code your provide contains an invalid use of pointers. Taking an address of a structured buffer element is not something we can support, and will result in invalid SPIRV. Slang currently only support pointers to memory in PhysicalStorageBuffer space, but structured buffers are in StorageBuffer address space, so forming a pointer to such elements shouldn't be allowed, and we need to add diagnostics to prevent user from doing so.

@csyonghe
Copy link
Collaborator

To pass a reference to an Atomic<T> object to a different function, you can use __ref parameter direction:

module "test_module";

public void atomicFunc(__ref Atomic<int> ptr) {
    ptr.add(1);
}
import "test_module";

RWStructuredBuffer<Atomic<int>> input0;

[shader("compute")]
[numthreads(1,1,1)]
void computeMain(uint3 workGroup : SV_GroupID)
{
    atomicFunc(input0[0]);
}

@cezneluk
Copy link
Author

Thank you for the quick response.

This invalid pointer usage was introduced when looking for a minimal reproducible example from my original source code (in which I use PhysicalStorageBuffer), I apologize for that.

Usage of __ref is more straightforward in any case, I didn't know about it. Is it documented anywhere?

@csyonghe
Copy link
Collaborator

This is not yet documented because it hasn't been tested in user code enough, and the way we support it today on spirv is still via inlining. We will document it for public use once we can generate real pointer code in SPIRV with future extensions and confirm it is stable enough. Before that it will continue to be internal/experimental feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants