Add support for explicit register space bindings (#542)

This change adds support for specifying explicit register spaces, like:

```hlsl
// Bind to texture register #2 in space #1
Texture2D t : register(t2, space1);
```

I added a test case to confirm that the register space is properly propagated through the Slang reflection API.

This change also adds proper error messages for some error/unsupported cases that weren't being diagnosed:

* Specifying a completely bogus register "class" (e.g., `register(bad99)`)
* Failing to specify a register index (`register(u)`)
* Specifying a component mask (`register(t0.x)`)
* Using `packoffset` bindings

I added test cases to cover all of these, as well as the new errors around support for register `space` bindings.

In order to get the existing tests to pass, I had to remove explicit `packoffset` bindings from some DXSDK test shaders.
None of these `packoffset` bindings were semantically significant (they matched what the compiler would do anyway, for both Slang and the standard HLSL compiler). Removing them is required for Slang now that we give an explicit error about our lack of `packoffset` support.
In a future change we might add logic to either detect semantically insignificant `packoffset`s, or to just go ahead and support them properly (as a general feature on `struct` types).

6 files changed, 188 insertions, 41 deletions

diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index 8f564cc59..f8e048bdb 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -253,18 +253,6 @@ DIAGNOSTIC(39999, Error, tooManyArguments, "too many arguments to call (got $0,
 DIAGNOSTIC(39999, Error, invalidIntegerLiteralSuffix, "invalid suffix '$0' on integer literal")
 DIAGNOSTIC(39999, Error, invalidFloatingPOintLiteralSuffix, "invalid suffix '$0' on floating-point literal")
 
-DIAGNOSTIC(39999, Error, conflictingExplicitBindingsForParameter, "conflicting explicit bindings for parameter '$0'")
-DIAGNOSTIC(39999, Warning, parameterBindingsOverlap, "explicit binding for parameter '$0' overlaps with parameter '$1'")
-
-
-DIAGNOSTIC(39999, Error, shaderParameterDeclarationsDontMatch, "declarations of shader parameter '$0' in different translation units don't match")
-
-DIAGNOSTIC(39999, Note, shaderParameterTypeMismatch, "type is declared as '$0' in one translation unit, and '$0' in another")
-DIAGNOSTIC(39999, Note, fieldTypeMisMatch, "type of field '$0' is declared as '$1' in one translation unit, and '$2' in another")
-DIAGNOSTIC(39999, Note, fieldDeclarationsDontMatch, "type '$0' is declared with different fields in each translation unit")
-DIAGNOSTIC(39999, Note, usedInDeclarationOf, "used in declaration of '$0'")
-
-
 
 
 
@@ -287,6 +275,26 @@ DIAGNOSTIC(38021, Error, typeArgumentDoesNotConformToInterface, "type argument `
 DIAGNOSTIC(38200, Error, recursiveModuleImport, "module `$0` recursively imports itself")
 DIAGNOSTIC(39999, Fatal, errorInImportedModule, "error in imported module, compilation ceased.")
 
+// 39xxx - Type layout and parameter binding.
+
+DIAGNOSTIC(39000, Error, conflictingExplicitBindingsForParameter, "conflicting explicit bindings for parameter '$0'")
+DIAGNOSTIC(39001, Warning, parameterBindingsOverlap, "explicit binding for parameter '$0' overlaps with parameter '$1'")
+
+
+DIAGNOSTIC(39002, Error, shaderParameterDeclarationsDontMatch, "declarations of shader parameter '$0' in different translation units don't match")
+
+DIAGNOSTIC(39003, Note, shaderParameterTypeMismatch, "type is declared as '$0' in one translation unit, and '$0' in another")
+DIAGNOSTIC(39004, Note, fieldTypeMisMatch, "type of field '$0' is declared as '$1' in one translation unit, and '$2' in another")
+DIAGNOSTIC(39005, Note, fieldDeclarationsDontMatch, "type '$0' is declared with different fields in each translation unit")
+DIAGNOSTIC(39006, Note, usedInDeclarationOf, "used in declaration of '$0'")
+
+DIAGNOSTIC(39007, Error, unknownRegisterClass, "unknown register class: '$0'")
+DIAGNOSTIC(39008, Error, expectedARegisterIndex, "expected a register index after '$0'")
+DIAGNOSTIC(39009, Error, expectedSpace, "expected 'space', got '$0'")
+DIAGNOSTIC(39010, Error, expectedSpaceIndex, "expected a register space index after 'space'")
+DIAGNOSTIC(39011, Error, componentMaskNotSupported, "explicit register component masks are not yet supported in Slang")
+DIAGNOSTIC(39012, Error, packOffsetNotSupported, "explicit 'packoffset' bindings are not yet supported in Slang")
+
 //
 // 4xxxx - IL code generation.
 //
diff --git a/source/slang/diagnostics.cpp b/source/slang/diagnostics.cpp
index 03acf50dc..77473b95f 100644
--- a/source/slang/diagnostics.cpp
+++ b/source/slang/diagnostics.cpp
@@ -39,6 +39,12 @@ void printDiagnosticArg(StringBuilder& sb, Slang::String const& str)
     sb << str;
 }
 
+void printDiagnosticArg(StringBuilder& sb, Slang::UnownedStringSlice const& str)
+{
+    sb.append(str);
+}
+
+
 void printDiagnosticArg(StringBuilder& sb, Name* name)
 {
     sb << getText(name);
diff --git a/source/slang/diagnostics.h b/source/slang/diagnostics.h
index 9bc3b3d8c..a12de03e4 100644
--- a/source/slang/diagnostics.h
+++ b/source/slang/diagnostics.h
@@ -77,6 +77,7 @@ namespace Slang
     void printDiagnosticArg(StringBuilder& sb, int val);
     void printDiagnosticArg(StringBuilder& sb, UInt val);
     void printDiagnosticArg(StringBuilder& sb, Slang::String const& str);
+    void printDiagnosticArg(StringBuilder& sb, Slang::UnownedStringSlice const& str);
     void printDiagnosticArg(StringBuilder& sb, Name* name);
     void printDiagnosticArg(StringBuilder& sb, Decl* decl);
     void printDiagnosticArg(StringBuilder& sb, Type* type);
diff --git a/source/slang/modifier-defs.h b/source/slang/modifier-defs.h
index 6212a244e..f9b3b6675 100644
--- a/source/slang/modifier-defs.h
+++ b/source/slang/modifier-defs.h
@@ -164,7 +164,9 @@ SYNTAX_CLASS(HLSLLayoutSemantic, HLSLSemantic)
 END_SYNTAX_CLASS()
 
 // An HLSL `register` semantic
-SIMPLE_SYNTAX_CLASS(HLSLRegisterSemantic, HLSLLayoutSemantic)
+SYNTAX_CLASS(HLSLRegisterSemantic, HLSLLayoutSemantic)
+    FIELD(Token, spaceName)
+END_SYNTAX_CLASS()
 
 // TODO(tfoley): `packoffset`
 SIMPLE_SYNTAX_CLASS(HLSLPackOffsetSemantic, HLSLLayoutSemantic)
diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index 4378cb06b..e0cc26f2b 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -288,8 +288,34 @@ struct LayoutSemanticInfo
     // TODO: need to deal with component-granularity binding...
 };
 
+static bool isDigit(char c)
+{
+    return (c >= '0') && (c <= '9');
+}
+
+/// Given a string that specifies a name and index (e.g., `COLOR0`),
+/// split it into slices for the name part and the index part.
+static void splitNameAndIndex(
+    String const&       text,
+    UnownedStringSlice& outName,
+    UnownedStringSlice& outDigits)
+{
+    char const* nameBegin = text.begin();
+    char const* digitsEnd = text.end();
+
+    char const* nameEnd = digitsEnd;
+    while( nameEnd != nameBegin && isDigit(*(nameEnd - 1)) )
+    {
+        nameEnd--;
+    }
+    char const* digitsBegin = nameEnd;
+
+    outName = UnownedStringSlice(nameBegin, nameEnd);
+    outDigits = UnownedStringSlice(digitsBegin, digitsEnd);
+}
+
 LayoutSemanticInfo ExtractLayoutSemanticInfo(
-    ParameterBindingContext*    /*context*/,
+    ParameterBindingContext*    context,
     HLSLLayoutSemantic*         semantic)
 {
     LayoutSemanticInfo info;
@@ -297,12 +323,31 @@ LayoutSemanticInfo ExtractLayoutSemanticInfo(
     info.index = 0;
     info.kind = LayoutResourceKind::None;
 
-    auto registerName = semantic->registerName.Content;
+    String registerName = semantic->registerName.Content;
     if (registerName.Length() == 0)
         return info;
 
+    // The register name is expected to be in the form:
+    //
+    //      identifier-char+ digit+
+    //
+    // where the identifier characters name a "register class"
+    // and the digits identify a register index within that class.
+    //
+    // We are going to split the string the user gave us
+    // into these constituent parts:
+    //
+    UnownedStringSlice registerClassName;
+    UnownedStringSlice registerIndexDigits;
+    splitNameAndIndex(registerName, registerClassName, registerIndexDigits);
+
+    // All of the register classes we support are single ASCII characters,
+    // so we really just care about the first byte, but we want to be
+    // careful and only look at it if the register class name is one
+    // byte long.
+    char registerClassChar = registerClassName.size() == 1 ? *registerClassName.begin() : 0;
     LayoutResourceKind kind = LayoutResourceKind::None;
-    switch (registerName[0])
+    switch (registerClassChar)
     {
     case 'b':
         kind = LayoutResourceKind::ConstantBuffer;
@@ -321,29 +366,59 @@ LayoutSemanticInfo ExtractLayoutSemanticInfo(
         break;
 
     default:
-        // TODO: issue an error here!
+        getSink(context)->diagnose(semantic->registerName, Diagnostics::unknownRegisterClass, registerClassName);
         return info;
     }
 
-    // TODO: need to parse and handle `space` binding
-    int space = 0;
+    // For a `register` semantic, the register index is not optional (unlike
+    // how it works for varying input/output semantics).
+    if( registerIndexDigits.size() == 0 )
+    {
+        getSink(context)->diagnose(semantic->registerName, Diagnostics::expectedARegisterIndex, registerClassName);
+    }
 
     UInt index = 0;
-    for (UInt ii = 1; ii < registerName.Length(); ++ii)
+    for(auto c : registerIndexDigits)
     {
-        int c = registerName[ii];
-        if (c >= '0' && c <= '9')
-        {
-            index = index * 10 + (c - '0');
-        }
-        else
+        SLANG_ASSERT(isDigit(c));
+        index = index * 10 + (c - '0');
+    }
+
+
+    UInt space = 0;
+    if( auto registerSemantic = dynamic_cast<HLSLRegisterSemantic*>(semantic) )
+    {
+        auto const& spaceName = registerSemantic->spaceName.Content;
+        if(spaceName.Length() != 0)
         {
-            // TODO: issue an error here!
-            return info;
+            UnownedStringSlice spaceSpelling;
+            UnownedStringSlice spaceDigits;
+            splitNameAndIndex(spaceName, spaceSpelling, spaceDigits);
+
+            if( spaceSpelling != UnownedTerminatedStringSlice("space") )
+            {
+                getSink(context)->diagnose(semantic->registerName, Diagnostics::expectedSpace, spaceSpelling);
+            }
+            else if( spaceDigits.size() == 0 )
+            {
+                getSink(context)->diagnose(semantic->registerName, Diagnostics::expectedSpaceIndex);
+            }
+            else
+            {
+                for(auto c : spaceDigits)
+                {
+                    SLANG_ASSERT(isDigit(c));
+                    space = space * 10 + (c - '0');
+                }
+            }
         }
     }
 
     // TODO: handle component mask part of things...
+    if( semantic->componentMask.Content.Length() != 0 )
+    {
+        getSink(context)->diagnose(semantic->componentMask, Diagnostics::componentMaskNotSupported);
+    }
 
     info.kind = kind;
     info.index = (int) index;
diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp
index 5d1b254d7..ab95be5ac 100644
--- a/source/slang/parser.cpp
+++ b/source/slang/parser.cpp
@@ -1835,24 +1835,76 @@ namespace Slang
         }
     }
 
-    //
-    // layout-semantic ::= (register | packoffset) '(' register-name component-mask? ')'
-    // register-name ::= identifier
-    // component-mask ::= '.' identifier
-    //
-    static void ParseHLSLLayoutSemantic(
-        Parser*				parser,
-        HLSLLayoutSemantic*	semantic)
+    /// Parse the "register name" part of a `register` or `packoffset` semantic.
+    ///
+    /// The syntax matched is:
+    ///
+    ///     register-name-and-component-mask ::= register-name component-mask?
+    ///     register-name ::= identifier
+    ///     component-mask ::= '.' identifier
+    ///
+    static void parseHLSLRegisterNameAndOptionalComponentMask(
+        Parser*             parser,
+        HLSLLayoutSemantic* semantic)
     {
-        semantic->name = parser->ReadToken(TokenType::Identifier);
-
-        parser->ReadToken(TokenType::LParent);
         semantic->registerName = parser->ReadToken(TokenType::Identifier);
         if (AdvanceIf(parser, TokenType::Dot))
         {
             semantic->componentMask = parser->ReadToken(TokenType::Identifier);
         }
+    }
+
+    /// Parse an HLSL `register` semantic.
+    ///
+    /// The syntax matched is:
+    ///
+    ///     register-semantic ::= 'register' '(' register-name-and-component-mask register-space? ')'
+    ///     register-space ::= ',' identifier
+    ///
+    static void parseHLSLRegisterSemantic(
+        Parser*                 parser,
+        HLSLRegisterSemantic*   semantic)
+    {
+        // Read the `register` keyword
+        semantic->name = parser->ReadToken(TokenType::Identifier);
+
+        // Expect a parenthized list of additional arguments
+        parser->ReadToken(TokenType::LParent);
+
+        // First argument is a required register name and optional component mask
+        parseHLSLRegisterNameAndOptionalComponentMask(parser, semantic);
+
+        // Second argument is an optional register space
+        if(AdvanceIf(parser, TokenType::Comma))
+        {
+            semantic->spaceName = parser->ReadToken(TokenType::Identifier);
+        }
+
+        parser->ReadToken(TokenType::RParent);
+    }
+
+    /// Parse an HLSL `packoffset` semantic.
+    ///
+    /// The syntax matched is:
+    ///
+    ///     packoffset-semantic ::= 'packoffset' '(' register-name-and-component-mask ')'
+    ///
+    static void parseHLSLPackOffsetSemantic(
+        Parser*                 parser,
+        HLSLPackOffsetSemantic* semantic)
+    {
+        // Read the `packoffset` keyword
+        semantic->name = parser->ReadToken(TokenType::Identifier);
+
+        // Expect a parenthized list of additional arguments
+        parser->ReadToken(TokenType::LParent);
+
+        // First and only argument is a required register name and optional component mask
+        parseHLSLRegisterNameAndOptionalComponentMask(parser, semantic);
+
         parser->ReadToken(TokenType::RParent);
+
+        parser->sink->diagnose(semantic, Diagnostics::packOffsetNotSupported);
     }
 
     //
@@ -1864,18 +1916,21 @@ namespace Slang
         if (parser->LookAheadToken("register"))
         {
             RefPtr<HLSLRegisterSemantic> semantic = new HLSLRegisterSemantic();
-            ParseHLSLLayoutSemantic(parser, semantic.Ptr());
+            parser->FillPosition(semantic);
+            parseHLSLRegisterSemantic(parser, semantic.Ptr());
             return semantic;
         }
         else if (parser->LookAheadToken("packoffset"))
         {
             RefPtr<HLSLPackOffsetSemantic> semantic = new HLSLPackOffsetSemantic();
-            ParseHLSLLayoutSemantic(parser, semantic.Ptr());
+            parser->FillPosition(semantic);
+            parseHLSLPackOffsetSemantic(parser, semantic.Ptr());
             return semantic;
         }
         else if (parser->LookAheadToken(TokenType::Identifier))
         {
             RefPtr<HLSLSimpleSemantic> semantic = new HLSLSimpleSemantic();
+            parser->FillPosition(semantic);
             semantic->name = parser->ReadToken(TokenType::Identifier);
             return semantic;
         }