[HLSL] Allow resource annotations to specify only register space (llvm#135287)

Specifying only `space` in a `register` annotation means the compiler
should implicitly assign a register slot to the resource from the
provided virtual register space.

Closes llvm#133346

Author: hekota

clang/include/clang/Basic/Attr.td

@@ -4752,20 +4752,25 @@ def HLSLResourceBinding: InheritableAttr {
 
   private:
       RegisterType RegType;
-      unsigned SlotNumber;
+      std::optional<unsigned> SlotNumber;
       unsigned SpaceNumber;
 
   public:
-      void setBinding(RegisterType RT, unsigned SlotNum, unsigned SpaceNum) {
+      void setBinding(RegisterType RT, std::optional<unsigned> SlotNum, unsigned SpaceNum) {
         RegType = RT;
         SlotNumber = SlotNum;
         SpaceNumber = SpaceNum;
       }
+      bool isImplicit() const {
+        return !SlotNumber.has_value();
+      }
       RegisterType getRegisterType() const {
+        assert(!isImplicit() && "binding does not have register slot");
         return RegType;
       }
       unsigned getSlotNumber() const {
-        return SlotNumber;
+        assert(!isImplicit() && "binding does not have register slot");
+        return SlotNumber.value();
       }
       unsigned getSpaceNumber() const {
         return SpaceNumber;

clang/lib/CodeGen/CGHLSLRuntime.cpp

@@ -261,7 +261,7 @@ void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
       BufDecl->getAttr<HLSLResourceBindingAttr>();
   // FIXME: handle implicit binding if no binding attribute is found
   // (llvm/llvm-project#110722)
-  if (RBA)
+  if (RBA && !RBA->isImplicit())
     initializeBufferFromBinding(CGM, BufGV, RBA->getSlotNumber(),
                                 RBA->getSpaceNumber());
 }

clang/lib/Parse/ParseHLSL.cpp

@@ -163,11 +163,16 @@ void Parser::ParseHLSLAnnotations(ParsedAttributes &Attrs,
     SourceLocation SlotLoc = Tok.getLocation();
     ArgExprs.push_back(ParseIdentifierLoc());
 
-    // Add numeric_constant for fix-it.
-    if (SlotStr.size() == 1 && Tok.is(tok::numeric_constant))
+    if (SlotStr.size() == 1) {
+      if (!Tok.is(tok::numeric_constant)) {
+        Diag(Tok.getLocation(), diag::err_expected) << tok::numeric_constant;
+        SkipUntil(tok::r_paren, StopAtSemi); // skip through )
+        return;
+      }
+      // Add numeric_constant for fix-it.
       fixSeparateAttrArgAndNumber(SlotStr, SlotLoc, Tok, ArgExprs, *this,
                                   Actions.Context, PP);
-
+    }
     if (Tok.is(tok::comma)) {
       ConsumeToken(); // consume comma
       if (!Tok.is(tok::identifier)) {

clang/lib/Sema/SemaHLSL.cpp

@@ -1533,72 +1533,80 @@ void SemaHLSL::handleResourceBindingAttr(Decl *TheDecl, const ParsedAttr &AL) {
                                     diag::err_incomplete_type))
       return;
   }
-  StringRef Space = "space0";
+
   StringRef Slot = "";
+  StringRef Space = "";
+  SourceLocation SlotLoc, SpaceLoc;
 
   if (!AL.isArgIdent(0)) {
     Diag(AL.getLoc(), diag::err_attribute_argument_type)
         << AL << AANT_ArgumentIdentifier;
     return;
   }
-
   IdentifierLoc *Loc = AL.getArgAsIdent(0);
-  StringRef Str = Loc->getIdentifierInfo()->getName();
-  SourceLocation ArgLoc = Loc->getLoc();
 
-  SourceLocation SpaceArgLoc;
-  bool SpecifiedSpace = false;
   if (AL.getNumArgs() == 2) {
-    SpecifiedSpace = true;
-    Slot = Str;
+    Slot = Loc->getIdentifierInfo()->getName();
+    SlotLoc = Loc->getLoc();
     if (!AL.isArgIdent(1)) {
       Diag(AL.getLoc(), diag::err_attribute_argument_type)
           << AL << AANT_ArgumentIdentifier;
       return;
     }
-
-    IdentifierLoc *Loc = AL.getArgAsIdent(1);
+    Loc = AL.getArgAsIdent(1);
     Space = Loc->getIdentifierInfo()->getName();
-    SpaceArgLoc = Loc->getLoc();
+    SpaceLoc = Loc->getLoc();
   } else {
-    Slot = Str;
+    StringRef Str = Loc->getIdentifierInfo()->getName();
+    if (Str.starts_with("space")) {
+      Space = Str;
+      SpaceLoc = Loc->getLoc();
+    } else {
+      Slot = Str;
+      SlotLoc = Loc->getLoc();
+      Space = "space0";
+    }
   }
 
-  RegisterType RegType;
-  unsigned SlotNum = 0;
+  RegisterType RegType = RegisterType::SRV;
+  std::optional<unsigned> SlotNum;
   unsigned SpaceNum = 0;
 
-  // Validate.
+  // Validate slot
   if (!Slot.empty()) {
     if (!convertToRegisterType(Slot, &RegType)) {
-      Diag(ArgLoc, diag::err_hlsl_binding_type_invalid) << Slot.substr(0, 1);
+      Diag(SlotLoc, diag::err_hlsl_binding_type_invalid) << Slot.substr(0, 1);
       return;
     }
     if (RegType == RegisterType::I) {
-      Diag(ArgLoc, diag::warn_hlsl_deprecated_register_type_i);
+      Diag(SlotLoc, diag::warn_hlsl_deprecated_register_type_i);
       return;
     }
-
     StringRef SlotNumStr = Slot.substr(1);
-    if (SlotNumStr.getAsInteger(10, SlotNum)) {
-      Diag(ArgLoc, diag::err_hlsl_unsupported_register_number);
+    unsigned N;
+    if (SlotNumStr.getAsInteger(10, N)) {
+      Diag(SlotLoc, diag::err_hlsl_unsupported_register_number);
       return;
     }
+    SlotNum = N;
   }
 
+  // Validate space
   if (!Space.starts_with("space")) {
-    Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;
+    Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
     return;
   }
   StringRef SpaceNumStr = Space.substr(5);
   if (SpaceNumStr.getAsInteger(10, SpaceNum)) {
-    Diag(SpaceArgLoc, diag::err_hlsl_expected_space) << Space;
+    Diag(SpaceLoc, diag::err_hlsl_expected_space) << Space;
     return;
   }
 
-  if (!DiagnoseHLSLRegisterAttribute(SemaRef, ArgLoc, TheDecl, RegType,
-                                     SpecifiedSpace))
-    return;
+  // If we have slot, diagnose it is the right register type for the decl
+  if (SlotNum.has_value())
+    if (!DiagnoseHLSLRegisterAttribute(SemaRef, SlotLoc, TheDecl, RegType,
+                                       !SpaceLoc.isInvalid()))
+      return;
 
   HLSLResourceBindingAttr *NewAttr =
       HLSLResourceBindingAttr::Create(getASTContext(), Slot, Space, AL);
@@ -1971,7 +1979,7 @@ void SemaHLSL::ActOnEndOfTranslationUnit(TranslationUnitDecl *TU) {
     for (const Decl *VD : DefaultCBufferDecls) {
       const HLSLResourceBindingAttr *RBA =
           VD->getAttr<HLSLResourceBindingAttr>();
-      if (RBA &&
+      if (RBA && !RBA->isImplicit() &&
           RBA->getRegisterType() == HLSLResourceBindingAttr::RegisterType::C) {
         DefaultCBuffer->setHasValidPackoffset(true);
         break;
@@ -3262,7 +3270,7 @@ static bool initVarDeclWithCtor(Sema &S, VarDecl *VD,
 
 static bool initGlobalResourceDecl(Sema &S, VarDecl *VD) {
   HLSLResourceBindingAttr *RBA = VD->getAttr<HLSLResourceBindingAttr>();
-  if (!RBA)
+  if (!RBA || RBA->isImplicit())
     // FIXME: add support for implicit binding (llvm/llvm-project#110722)
     return false;
 
@@ -3347,7 +3355,7 @@ void SemaHLSL::processExplicitBindingsOnDecl(VarDecl *VD) {
   bool HasBinding = false;
   for (Attr *A : VD->attrs()) {
     HLSLResourceBindingAttr *RBA = dyn_cast<HLSLResourceBindingAttr>(A);
-    if (!RBA)
+    if (!RBA || RBA->isImplicit())
       continue;
     HasBinding = true;
 

clang/test/AST/HLSL/resource_binding_attr.hlsl

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -Wno-hlsl-implicit-binding -triple dxil-pc-shadermodel6.3-library -finclude-default-header -ast-dump -o - %s | FileCheck %s
 
 // CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 4]]:9 cbuffer CB
 // CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
@@ -30,6 +30,10 @@ RWBuffer<float> UAV : register(u3);
 // CHECK: HLSLResourceBindingAttr {{.*}} "u4" "space0"
 RWBuffer<float> UAV1 : register(u2), UAV2 : register(u4);
 
+// CHECK: VarDecl {{.*}} UAV3 'RWBuffer<float>':'hlsl::RWBuffer<float>'
+// CHECK: HLSLResourceBindingAttr {{.*}} "" "space5"
+RWBuffer<float> UAV3 : register(space5);
+
 //
 // Default constants ($Globals) layout annotations
 

clang/test/SemaHLSL/resource_binding_attr_error.hlsl

@@ -22,15 +22,18 @@ cbuffer c : register(bf, s2) {
 // expected-error@+1 {{expected identifier}}
 cbuffer A : register() {}
 
-// expected-error@+1 {{register number should be an integer}}
-cbuffer B : register(space1) {}
+// expected-error@+1 {{invalid space specifier 'space' used; expected 'space' followed by an integer, like space1}}
+cbuffer B : register(space) {}
 
 // expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}}
 cbuffer C : register(b 2) {}
 
 // expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}}
 cbuffer D : register(b 2, space3) {}
 
+// expected-error@+1 {{expected <numeric_constant>}}
+cbuffer E : register(u-1) {};
+
 // expected-error@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
 static MyTemplatedSRV<float> U : register(u5);
 

clang/test/SemaHLSL/resource_binding_implicit.hlsl

@@ -14,9 +14,8 @@ RWBuffer<int> c;
 // No warning - explicit binding.
 RWBuffer<float> d : register(u0);
 
-// TODO: Add this test once #135287 lands
-// TODO: ... @+1 {{resource has implicit register binding}}
-// TODO: RWBuffer<float> dd : register(space1);
+// expected-warning@+1 {{resource has implicit register binding}}
+RWBuffer<float> dd : register(space1);
 
 // No warning - explicit binding.
 RWBuffer<float> ddd : register(u3, space4);