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

Reading from a ConstantBuffer within a template fails compilation in a non-library profile #3972

Closed
sndels opened this issue Sep 27, 2021 · 1 comment · Fixed by #3991
Closed

Comments

@sndels
Copy link

sndels commented Sep 27, 2021

Reading from a ConstantBuffer within a template causes error: External function used in non-library profile: : \01??$Fn@N@@YANN@Z. Making the read within a separate function seems to be a workaround. This could be tied to #3731, but the error is quite different.

Minimal example, reproduces with dndxc.exe from de6a8ed's AppVeyor build
Fails: -enable-templates -DFAILURE_CASE
Compiles: -enable-templates

struct ConstantsStruct
{
    float Val : Val;
};
ConstantBuffer<ConstantsStruct> Constants;

#ifdef FAILURE_CASE
template<typename T>
T Fn(T input)
{
    T ret = input - Constants.Val;

    return ret;
}

#else

float ReadConstant()
{
    return Constants.Val;
}
template<typename T>
T Fn(T input)
{
    T ret = input - ReadConstant();

    return ret;
}

#endif

void main()
{
    Fn(1.f);
}

Edit: Made example more minimal

llvm-beanz added a commit to llvm-beanz/DirectXShaderCompiler that referenced this issue Oct 6, 2021
ConstantBuffers and TextureBuffers need to behave at the AST level as
if they implicitly convert to the underlying buffer data type.

We do this when generting the AST, but we don't have the proper
overrides in place in `Sema::PerformObjectMemberConversion`.

This is the root cause of issue microsoft#3327, so this change should be layered
on a revert of 2903170.

This also addresses microsoft#3972.

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl
@llvm-beanz
Copy link
Collaborator

To throw some context in here as breadcrumbs. When you dump the AST for the failure case, the template instantiation is incomplete. The cause of this is a failure in the tree transformation copying the AST from the template into the instantiation. The failure occurs because the tree transformation tries to convert the member access following C++ casting rules, which really only support casting along inheritance lines for member access. Because this member access is a ConstantBuffer, we need to support FlatConversion casting when transforming the AST.

llvm-beanz added a commit that referenced this issue Oct 11, 2021
…ure buffers (#3991)

* Revert "Fix crash in Sema::CheckDerivedToBaseConversion when Paths is
empty. (#3327)"

This reverts commit 2903170, but keeps
the test case, as the test case is important.

* Generate FlatConversion for buffer member access

ConstantBuffers and TextureBuffers need to behave at the AST level as
if they implicitly convert to the underlying buffer data type.

We do this when generting the AST, but we don't have the proper
overrides in place in `Sema::PerformObjectMemberConversion`.

This is the root cause of issue #3327, so this change should be layered
on a revert of 2903170.

This also addresses #3972.

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl

'beanz/beanz/constant-buffer-template-issue-3972'.
../tools/clang/test/HLSLFileCheck/hlsl/template/BufferInExpansion.hlsl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants