search

microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.11k stars 698 forks source link

Passing string literal to templated function causes validation errors when optimizations are disabled #6154

Open mateeeeeee opened 10 months ago

mateeeeeee commented 10 months ago

Description Passing string literal to templated function causes validation errors when optimizations are disabled (using -Od) but works fine with optimizations enabled.

Steps to Reproduce Minimal reproducible example: godbolt

I am not sure what is expected behavior, should the dxc reject or accept the code but it should behave the same despite different optimization flags.

Environment

llvm-beanz commented 10 months ago

This only "works" with optimized code because we optimize the string away entirely. Strings are not supported in HLSL, but our enforcement of that is post-optimization. We should error about strings earlier.

KStocky commented 9 months ago

I assume if you were to error about strings earlier then strings used for rootsignatures would need to be treated as a special case then?

llvm-beanz commented 9 months ago

I assume if you were to error about strings earlier then strings used for rootsignatures would need to be treated as a special case then?

Not any more than they already are. String-literal arguments to attributes aren't really strings (i.e. you can't declare a string variable and pass it to an attribute). Here's a C++ example using MSVC's UUID attribute.

Attribute grammars aren't yet documented in the HLSL draft specification, but I have written down the parsing grammar for the C++ attributes that I want to include in HLSL 202x here (https://github.com/microsoft/hlsl-specs/pull/65). The really relevant bit is that attribute arguments are balanced-token which means they're grammar tokens not typed literal expressions.

That all said, I don't think we're likely to change how we handle strings in DXC (even if it isn't strictly accurate), because we do want to someday support string literals for printf. We're resource constrained enough that I'd rather we leave this bug lie around for a while and resolve issues with strings as part of larger feature work.