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

[HLSL][RootSignature] Make Root Signature lexer keywords case-insensitive #132967

Merged
merged 2 commits into from
Mar 28, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented Mar 25, 2025

From the corrections to the Root Signature specification here: llvm/wg-hlsl#192. It was denoted that keywords are also case-insensitive in DXC.

This pr adjusts the lexer to adhere to the updated spec.

We also have a NFC to add a missing license to a file while in the area.

Finn Plummer added 2 commits March 25, 2025 16:23
- from dxc testing, it was shown that keywords could be specified in a
case-insensitive manner. the current implementation was case sensitive
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Mar 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 25, 2025

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Finn Plummer (inbelic)

Changes

From the corrections to the Root Signature specification here: llvm/wg-hlsl#192. It was denoted that keywords are also case-insensitive in DXC.

This pr adjusts the lexer to adhere to the updated spec.

We also have a NFC to add a missing license to a file while in the area.


Full diff: https://github.com/llvm/llvm-project/pull/132967.diff

2 Files Affected:

  • (modified) clang/lib/Lex/LexHLSLRootSignature.cpp (+9-1)
  • (modified) clang/unittests/Lex/LexHLSLRootSignatureTest.cpp (+30)
diff --git a/clang/lib/Lex/LexHLSLRootSignature.cpp b/clang/lib/Lex/LexHLSLRootSignature.cpp
index 8344aad15a9bc..fb4aab20c7275 100644
--- a/clang/lib/Lex/LexHLSLRootSignature.cpp
+++ b/clang/lib/Lex/LexHLSLRootSignature.cpp
@@ -1,3 +1,11 @@
+//=== LexHLSLRootSignature.cpp - Lex Root Signature -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
 #include "clang/Lex/LexHLSLRootSignature.h"
 
 namespace clang {
@@ -87,7 +95,7 @@ RootSignatureToken RootSignatureLexer::LexToken() {
 
   // Define a large string switch statement for all the keywords and enums
   auto Switch = llvm::StringSwitch<TokenKind>(TokSpelling);
-#define KEYWORD(NAME) Switch.Case(#NAME, TokenKind::kw_##NAME);
+#define KEYWORD(NAME) Switch.CaseLower(#NAME, TokenKind::kw_##NAME);
 #define ENUM(NAME, LIT) Switch.CaseLower(LIT, TokenKind::en_##NAME);
 #include "clang/Lex/HLSLRootSignatureTokenKinds.def"
 
diff --git a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
index 0576f08c4c276..d72a842922f98 100644
--- a/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Lex/LexHLSLRootSignatureTest.cpp
@@ -120,6 +120,36 @@ TEST_F(LexHLSLRootSignatureTest, ValidLexAllTokensTest) {
   CheckTokens(Lexer, Tokens, Expected);
 }
 
+TEST_F(LexHLSLRootSignatureTest, ValidCaseInsensitiveKeywordsTest) {
+  // This test will check that we can lex keywords in an case-insensitive
+  // manner
+  const llvm::StringLiteral Source = R"cc(
+    DeScRiPtOrTaBlE
+
+    CBV srv UAV sampler
+    SPACE visibility FLAGS
+    numDescriptors OFFSET
+  )cc";
+  auto TokLoc = SourceLocation();
+  hlsl::RootSignatureLexer Lexer(Source, TokLoc);
+
+  SmallVector<hlsl::RootSignatureToken> Tokens;
+  SmallVector<hlsl::TokenKind> Expected = {
+      hlsl::TokenKind::kw_DescriptorTable,
+      hlsl::TokenKind::kw_CBV,
+      hlsl::TokenKind::kw_SRV,
+      hlsl::TokenKind::kw_UAV,
+      hlsl::TokenKind::kw_Sampler,
+      hlsl::TokenKind::kw_space,
+      hlsl::TokenKind::kw_visibility,
+      hlsl::TokenKind::kw_flags,
+      hlsl::TokenKind::kw_numDescriptors,
+      hlsl::TokenKind::kw_offset,
+  };
+
+  CheckTokens(Lexer, Tokens, Expected);
+}
+
 TEST_F(LexHLSLRootSignatureTest, ValidLexPeekTest) {
   // This test will check that we the peek api is correctly used
   const llvm::StringLiteral Source = R"cc(

@inbelic inbelic merged commit 83c4cb3 into llvm:main Mar 28, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants