Skip to content

Allowing use of const static variables in library exports#3780

Merged
tex3d merged 1 commit intomicrosoft:masterfrom
nv-jdeligiannis:master
May 29, 2021
Merged

Allowing use of const static variables in library exports#3780
tex3d merged 1 commit intomicrosoft:masterfrom
nv-jdeligiannis:master

Conversation

@nv-jdeligiannis
Copy link
Copy Markdown
Contributor

@nv-jdeligiannis nv-jdeligiannis commented May 18, 2021

We're currently in the process of converting already existing shader assets to raytracing equivalent versions. While doing so we run in to the following compile error when using global static variables in our library export: "static global resource use is disallowed in library exports."

shadercode:

Texture2D<float2> RainDepth : register(t1, space1);

SamplerState samplerPoint_BorderWhite 
{ 
    Filter = MIN_MAG_MIP_POINT ; 
    AddressU = BORDER ; 
    AddressV = BORDER ; 
    AddressW = BORDER ; 
    MipLODBias = 0 ; 
    MaxAnisotropy = 1 ; 
    BorderColor = float4 ( 1 , 1 , 1 , 1 ) ; 
} ; 

static const SamplerState samplerMIN_MAG_MIP_POINTBORDERBORDERWHITE = samplerPoint_BorderWhite ; 

static const SamplerState samplerStaticRainDepth = samplerMIN_MAG_MIP_POINTBORDERBORDERWHITE ; 

struct Payload{
	uint value;
};

struct Attributes
{
 float2 Barycentrics;
};

[shader("closesthit")]
void CHSMain(inout Payload payload, Attributes attributes)
{
 float val = RainDepth.SampleLevel ( samplerStaticRainDepth , float2(0,0), 0 ) ; 
}

While there is an explanation for why the use of global static variables is disallowed in DxilPromoteResourcePasses.cpp it remains unclear if const variables are disallowed by design, or if we can loosen the restrictions a bit to let global static const variables through?

Note: changing from global static to preprocessor defines would work for the problem at hand, but it's a non trivial task given the amount of already existing assets in the engine.

@AppVeyorBot
Copy link
Copy Markdown

@nv-jdeligiannis
Copy link
Copy Markdown
Contributor Author

nv-jdeligiannis commented May 27, 2021

The comment reads:

// Read/write to global static resource is disallowed for libraries:
// Resource use needs to be resolved to a single real global resource,
// but it may not be possible since any external function call may re-enter
// at any other library export, which could modify the global static
// between write and read.
// While it could work for certain cases, describing the boundary at
// the HLSL level is difficult, so at this point it's better to disallow.
// example of what could work:
// After inlining, exported functions must have writes to static globals
// before reads, and must not have any external function calls between
// writes and subsequent reads, such that the static global may be
// optimized away for the exported function.

It appears to be a problem related to read and write of global static variables. I propose that if a global static variable is marked as read only (const), the above concerns do not apply.

Copy link
Copy Markdown
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good.

@tex3d tex3d merged commit bb8ea37 into microsoft:master May 29, 2021
nv-jdeligiannis added a commit to nv-jdeligiannis/DirectXShaderCompiler that referenced this pull request May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants