Skip to content

Commit

Permalink
Handle case insensitive FXC HLSL keywords. (#2347)
Browse files Browse the repository at this point in the history
There are a few keywords like "pass" in HLSL that are actually case-insensitive for FXC. This can be disabled with strict mode, but even if you do that FXC will continue to give an error if you try to use them in identifiers (at all, with any casing).

This changes the namer code to escape these keywords even if the casing is different.

If you're wondering where I got the list from: I looked at the list of strings in D3DCompiler_47.dll.
  • Loading branch information
PJB3005 authored May 31, 2023
1 parent 5206c59 commit 544ccf8
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 12 deletions.
8 changes: 7 additions & 1 deletion src/back/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,13 @@ impl<'a, W: Write> Writer<'a, W> {
// Generate a map with names required to write the module
let mut names = crate::FastHashMap::default();
let mut namer = proc::Namer::default();
namer.reset(module, keywords::RESERVED_KEYWORDS, &["gl_"], &mut names);
namer.reset(
module,
keywords::RESERVED_KEYWORDS,
&[],
&["gl_"],
&mut names,
);

// Build the instance
let mut this = Self {
Expand Down
22 changes: 15 additions & 7 deletions src/back/hlsl/keywords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,23 @@ HLSL Reserved Words
- <https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-appendix-reserved-words>
*/

// When compiling with FXC without strict mode, these keywords are actually case insensitive.
// If you compile with strict mode and specify a different casing like "Pass" instead in an identifier, FXC will give this error:
// "error X3086: alternate cases for 'pass' are deprecated in strict mode"
// This behavior is not documented anywhere, but as far as I can tell this is the full list.
pub const RESERVED_CASE_INSENSITIVE: &[&str] = &[
"asm",
"decl",
"pass",
"technique",
"Texture1D",
"Texture2D",
"Texture3D",
"TextureCube",
];

pub const RESERVED: &[&str] = &[
"AppendStructuredBuffer",
"asm",
"asm_fragment",
"BlendState",
"bool",
Expand Down Expand Up @@ -68,7 +82,6 @@ pub const RESERVED: &[&str] = &[
"out",
"OutputPatch",
"packoffset",
"pass",
"pixelfragment",
"PixelShader",
"point",
Expand Down Expand Up @@ -101,18 +114,13 @@ pub const RESERVED: &[&str] = &[
"switch",
"StructuredBuffer",
"tbuffer",
"technique",
"technique10",
"technique11",
"texture",
"Texture1D",
"Texture1DArray",
"Texture2D",
"Texture2DArray",
"Texture2DMS",
"Texture2DMSArray",
"Texture3D",
"TextureCube",
"TextureCubeArray",
"true",
"typedef",
Expand Down
9 changes: 7 additions & 2 deletions src/back/hlsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,13 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {

fn reset(&mut self, module: &Module) {
self.names.clear();
self.namer
.reset(module, super::keywords::RESERVED, &[], &mut self.names);
self.namer.reset(
module,
super::keywords::RESERVED,
super::keywords::RESERVED_CASE_INSENSITIVE,
&[],
&mut self.names,
);
self.entry_point_io.clear();
self.named_expressions.clear();
self.wrapped.clear();
Expand Down
2 changes: 1 addition & 1 deletion src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3033,7 +3033,7 @@ impl<W: Write> Writer<W> {
) -> Result<TranslationInfo, Error> {
self.names.clear();
self.namer
.reset(module, super::keywords::RESERVED, &[], &mut self.names);
.reset(module, super::keywords::RESERVED, &[], &[], &mut self.names);
self.struct_member_pads.clear();

writeln!(
Expand Down
1 change: 1 addition & 0 deletions src/back/wgsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ impl<W: Write> Writer<W> {
module,
crate::keywords::wgsl::RESERVED,
// an identifier must not start with two underscore
&[],
&["__"],
&mut self.names,
);
Expand Down
48 changes: 47 additions & 1 deletion src/proc/namer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{arena::Handle, FastHashMap, FastHashSet};
use std::borrow::Cow;
use std::hash::{Hash, Hasher};

pub type EntryPointIndex = u16;
const SEPARATOR: char = '_';
Expand All @@ -25,6 +26,7 @@ pub struct Namer {
/// The last numeric suffix used for each base name. Zero means "no suffix".
unique: FastHashMap<String, u32>,
keywords: FastHashSet<String>,
keywords_case_insensitive: FastHashSet<AsciiUniCase<&'static str>>,
reserved_prefixes: Vec<String>,
}

Expand Down Expand Up @@ -102,7 +104,12 @@ impl Namer {
}
None => {
let mut suffixed = base.to_string();
if base.ends_with(char::is_numeric) || self.keywords.contains(base.as_ref()) {
if base.ends_with(char::is_numeric)
|| self.keywords.contains(base.as_ref())
|| self
.keywords_case_insensitive
.contains(&AsciiUniCase(base.as_ref()))
{
suffixed.push(SEPARATOR);
}
debug_assert!(!self.keywords.contains(&suffixed));
Expand Down Expand Up @@ -137,6 +144,7 @@ impl Namer {
&mut self,
module: &crate::Module,
reserved_keywords: &[&str],
reserved_keywords_case_insensitive: &[&'static str],
reserved_prefixes: &[&str],
output: &mut FastHashMap<NameKey, String>,
) {
Expand All @@ -148,6 +156,17 @@ impl Namer {
self.keywords.clear();
self.keywords
.extend(reserved_keywords.iter().map(|string| (string.to_string())));

debug_assert!(reserved_keywords_case_insensitive
.iter()
.all(|s| s.is_ascii()));
self.keywords_case_insensitive.clear();
self.keywords_case_insensitive.extend(
reserved_keywords_case_insensitive
.iter()
.map(|string| (AsciiUniCase(*string))),
);

let mut temp = String::new();

for (ty_handle, ty) in module.types.iter() {
Expand Down Expand Up @@ -252,6 +271,33 @@ impl Namer {
}
}

/// A string wrapper type with an ascii case insensitive Eq and Hash impl
struct AsciiUniCase<S: AsRef<str> + ?Sized>(S);

impl<S: AsRef<str>> PartialEq<Self> for AsciiUniCase<S> {
#[inline]
fn eq(&self, other: &Self) -> bool {
self.0.as_ref().eq_ignore_ascii_case(other.0.as_ref())
}
}

impl<S: AsRef<str>> Eq for AsciiUniCase<S> {}

impl<S: AsRef<str>> Hash for AsciiUniCase<S> {
#[inline]
fn hash<H: Hasher>(&self, hasher: &mut H) {
for byte in self
.0
.as_ref()
.as_bytes()
.iter()
.map(|b| b.to_ascii_lowercase())
{
hasher.write_u8(byte);
}
}
}

#[test]
fn test() {
let mut namer = Namer::default();
Expand Down
6 changes: 6 additions & 0 deletions tests/in/hlsl-keyword.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@fragment
fn fs_main() -> @location(0) vec4f {
// Make sure case-insensitive keywords are escaped in HLSL.
var Pass = vec4(1.0,1.0,1.0,1.0);
return Pass;
}
9 changes: 9 additions & 0 deletions tests/out/hlsl/hlsl-keyword.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

float4 fs_main() : SV_Target0
{
float4 Pass_ = (float4)0;

Pass_ = float4(1.0, 1.0, 1.0, 1.0);
float4 _expr6 = Pass_;
return _expr6;
}
3 changes: 3 additions & 0 deletions tests/out/hlsl/hlsl-keyword.hlsl.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
vertex=()
fragment=(fs_main:ps_5_1 )
compute=()
1 change: 1 addition & 0 deletions tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ fn convert_wgsl() {
("force_point_size_vertex_shader_webgl", Targets::GLSL),
("invariant", Targets::GLSL),
("ray-query", Targets::SPIRV | Targets::METAL),
("hlsl-keyword", Targets::HLSL),
];

for &(name, targets) in inputs.iter() {
Expand Down

0 comments on commit 544ccf8

Please sign in to comment.