Skip to content

Revert "[HLSL][RootSignature] Define and integrate HLSLRootSignatureAttr" #134273

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

Merged
merged 1 commit into from
Apr 3, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented Apr 3, 2025

Reverts #134124

The build is failing again to a linking error: here. Again the error was not present locally or any of the pre-merge builds and must have been transitively linked in these build environments...

@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 Apr 3, 2025
@inbelic inbelic merged commit 73e8d67 into main Apr 3, 2025
11 of 16 checks passed
@llvmbot
Copy link
Member

llvmbot commented Apr 3, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Finn Plummer (inbelic)

Changes

Reverts llvm/llvm-project#134124

The build is failing again to a linking error: here. Again the error was not present locally or any of the pre-merge builds and must have been transitively linked in these build environments...


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

8 Files Affected:

  • (modified) clang/include/clang/AST/Attr.h (-1)
  • (modified) clang/include/clang/Basic/Attr.td (-19)
  • (modified) clang/include/clang/Basic/AttrDocs.td (-11)
  • (modified) clang/include/clang/Sema/SemaHLSL.h (-1)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (-3)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (-35)
  • (removed) clang/test/AST/HLSL/RootSignatures-AST.hlsl (-24)
  • (removed) clang/test/SemaHLSL/RootSignature-err.hlsl (-9)
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 37c3f8bbfb5f9..994f236337b99 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -26,7 +26,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Support/Compiler.h"
 #include "llvm/Frontend/HLSL/HLSLResource.h"
-#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/VersionTuple.h"
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 9ef4f2b6b91ed..fd9e686485552 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4710,25 +4710,6 @@ def Error : InheritableAttr {
   let Documentation = [ErrorAttrDocs];
 }
 
-def HLSLRootSignature : Attr {
-  /// [RootSignature(Signature)]
-  let Spellings = [Microsoft<"RootSignature">];
-  let Args = [StringArgument<"Signature">];
-  let Subjects = SubjectList<[Function],
-                             ErrorDiag, "'function'">;
-  let LangOpts = [HLSL];
-  let Documentation = [HLSLRootSignatureDocs];
-  let AdditionalMembers = [{
-private:
-  ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements;
-public:
-  void setElements(ArrayRef<llvm::hlsl::rootsig::RootElement> Elements) {
-    RootElements = Elements;
-  }
-  auto getElements() const { return RootElements; }
-}];
-}
-
 def HLSLNumThreads: InheritableAttr {
   let Spellings = [Microsoft<"numthreads">];
   let Args = [IntArgument<"X">, IntArgument<"Y">, IntArgument<"Z">];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 1b969e456b910..c8b371280e35d 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8145,17 +8145,6 @@ and https://microsoft.github.io/hlsl-specs/proposals/0013-wave-size-range.html
   }];
 }
 
-def HLSLRootSignatureDocs : Documentation {
-  let Category = DocCatFunction;
-  let Content = [{
-The ``RootSignature`` attribute applies to HLSL entry functions to define what
-types of resources are bound to the graphics pipeline.
-
-For details about the use and specification of Root Signatures please see here:
-https://learn.microsoft.com/en-us/windows/win32/direct3d12/root-signatures
-  }];
-}
-
 def NumThreadsDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index 1bd35332612cd..f333fe30e8da0 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -118,7 +118,6 @@ class SemaHLSL : public SemaBase {
                                        bool IsCompAssign);
   void emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS, BinaryOperatorKind Opc);
 
-  void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL);
   void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL);
   void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL);
   void handleSV_DispatchThreadIDAttr(Decl *D, const ParsedAttr &AL);
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index b36d327f5bd0a..0b844b44930b9 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7498,9 +7498,6 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
     break;
 
   // HLSL attributes:
-  case ParsedAttr::AT_HLSLRootSignature:
-    S.HLSL().handleRootSignatureAttr(D, AL);
-    break;
   case ParsedAttr::AT_HLSLNumThreads:
     S.HLSL().handleNumThreadsAttr(D, AL);
     break;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index bed14c111b544..fe600386e6fa9 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -28,7 +28,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
-#include "clang/Parse/ParseHLSLRootSignature.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/ParsedAttr.h"
 #include "clang/Sema/Sema.h"
@@ -942,40 +941,6 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS,
       << NewFnName << FixItHint::CreateReplacement(FullRange, OS.str());
 }
 
-void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
-  if (AL.getNumArgs() != 1) {
-    Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1;
-    return;
-  }
-
-  StringRef Signature;
-  if (!SemaRef.checkStringLiteralArgumentAttr(AL, 0, Signature))
-    return;
-
-  SourceLocation Loc = AL.getArgAsExpr(0)->getExprLoc();
-  // TODO(#126565): pass down below to lexer when fp is supported
-  // llvm::RoundingMode RM = SemaRef.CurFPFeatures.getRoundingMode();
-  hlsl::RootSignatureLexer Lexer(Signature, Loc);
-  SmallVector<llvm::hlsl::rootsig::RootElement> Elements;
-  hlsl::RootSignatureParser Parser(Elements, Lexer, SemaRef.getPreprocessor());
-
-  if (Parser.parse())
-    return;
-
-  // Allocate elements onto AST context
-  unsigned N = Elements.size();
-  auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>(
-      ::new (getASTContext()) llvm::hlsl::rootsig::RootElement[N], N);
-  for (unsigned I = 0; I < N; ++I)
-    RootElements[I] = Elements[I];
-
-  // Set elements
-  auto *Result = ::new (getASTContext())
-      HLSLRootSignatureAttr(getASTContext(), AL, Signature);
-  Result->setElements(ArrayRef<llvm::hlsl::rootsig::RootElement>(RootElements));
-  D->addAttr(Result);
-}
-
 void SemaHLSL::handleNumThreadsAttr(Decl *D, const ParsedAttr &AL) {
   llvm::VersionTuple SMVersion =
       getASTContext().getTargetInfo().getTriple().getOSVersion();
diff --git a/clang/test/AST/HLSL/RootSignatures-AST.hlsl b/clang/test/AST/HLSL/RootSignatures-AST.hlsl
deleted file mode 100644
index 948f2484ff5d0..0000000000000
--- a/clang/test/AST/HLSL/RootSignatures-AST.hlsl
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -ast-dump \
-// RUN:  -disable-llvm-passes -o - %s | FileCheck %s
-
-// This test ensures that the sample root signature is parsed without error and
-// the Attr AST Node is created succesfully. If an invalid root signature was
-// passed in then we would exit out of Sema before the Attr is created.
-
-#define SampleRS \
-  "DescriptorTable( " \
-  "  CBV(), " \
-  "  SRV(), " \
-  "  UAV()" \
-  "), " \
-  "DescriptorTable(Sampler())"
-
-// CHECK:      HLSLRootSignatureAttr
-// CHECK-SAME: "DescriptorTable(
-// CHECK-SAME:   CBV(),
-// CHECK-SAME:   SRV(),
-// CHECK-SAME:   UAV()
-// CHECK-SAME: ),
-// CHECK-SAME: DescriptorTable(Sampler())"
-[RootSignature(SampleRS)]
-void main() {}
diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl
deleted file mode 100644
index 647a4ba2470a7..0000000000000
--- a/clang/test/SemaHLSL/RootSignature-err.hlsl
+++ /dev/null
@@ -1,9 +0,0 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
-
-// Attr test
-
-[RootSignature()] // expected-error {{'RootSignature' attribute takes one argument}}
-void bad_root_signature_0() {}
-
-[RootSignature("Arg1", "Arg2")] // expected-error {{'RootSignature' attribute takes one argument}}
-void bad_root_signature_1() {}

@inbelic inbelic deleted the revert-134124-inbelic/rootsig-attr branch April 3, 2025 16:40
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
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: Closed
Development

Successfully merging this pull request may close these issues.

2 participants