[clang] Introduce `SemaHLSL` #87912

Endilll · 2024-04-07T09:45:02Z

This patch introduces SemaHLSL class, and moves some HLSL-related functions there. No functional changes intended.

Removing "HLSL" from function names inside SemaHLSL is left for a subsequent PR by HLSL contributors, if they deem that desirable.

This is a part of the effort to split Sema into smaller manageable parts, and follows the example of OpenACC. See #82217, #84184, #87634 for additional context.

llvmbot · 2024-04-07T09:45:20Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Vlad Serebrennikov (Endilll)

Changes

This patch introduces SemaHLSL class, and moves some HLSL-related functions there. No functional changes intended.

This is a part of the effort to split Sema into smaller manageable parts, and follows the example of OpenACC. See #82217, #84184, #87634 for additional context.

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

5 Files Affected:

(modified) clang/include/clang/Sema/Sema.h (+14-26)
(added) clang/include/clang/Sema/SemaHLSL.h (+37)
(modified) clang/lib/Parse/ParseHLSL.cpp (+6-5)
(modified) clang/lib/Sema/Sema.cpp (+2)
(modified) clang/lib/Sema/SemaHLSL.cpp (+14-10)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f49bc724c96c89..a3318c55c6ced0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -182,6 +182,7 @@ class Preprocessor;
 class PseudoDestructorTypeStorage;
 class PseudoObjectExpr;
 class QualType;
+class SemaHLSL;
 class SemaOpenACC;
 class StandardConversionSequence;
 class Stmt;
@@ -465,9 +466,8 @@ class Sema final : public SemaBase {
   // 36. FixIt Helpers (SemaFixItUtils.cpp)
   // 37. Name Lookup for RISC-V Vector Intrinsic (SemaRISCVVectorLookup.cpp)
   // 38. CUDA (SemaCUDA.cpp)
-  // 39. HLSL Constructs (SemaHLSL.cpp)
-  // 40. OpenMP Directives and Clauses (SemaOpenMP.cpp)
-  // 41. SYCL Constructs (SemaSYCL.cpp)
+  // 39. OpenMP Directives and Clauses (SemaOpenMP.cpp)
+  // 40. SYCL Constructs (SemaSYCL.cpp)
 
   /// \name Semantic Analysis
   /// Implementations are in Sema.cpp
@@ -964,6 +964,11 @@ class Sema final : public SemaBase {
   /// CurContext - This is the current declaration context of parsing.
   DeclContext *CurContext;
 
+  SemaHLSL &HLSL() {
+    assert(HLSLPtr);
+    return *HLSLPtr;
+  }
+
   SemaOpenACC &OpenACC() {
     assert(OpenACCPtr);
     return *OpenACCPtr;
@@ -999,6 +1004,7 @@ class Sema final : public SemaBase {
 
   mutable IdentifierInfo *Ident_super;
 
+  std::unique_ptr<SemaHLSL> HLSLPtr;
   std::unique_ptr<SemaOpenACC> OpenACCPtr;
 
   ///@}
@@ -1967,6 +1973,11 @@ class Sema final : public SemaBase {
   bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
                          const FunctionProtoType *Proto);
 
+  bool SemaBuiltinVectorMath(CallExpr *TheCall, QualType &Res);
+  bool SemaBuiltinVectorToScalarMath(CallExpr *TheCall);
+
+  bool CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
+
 private:
   void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
                         const ArraySubscriptExpr *ASE = nullptr,
@@ -13148,29 +13159,6 @@ class Sema final : public SemaBase {
   //
   //
 
-  /// \name HLSL Constructs
-  /// Implementations are in SemaHLSL.cpp
-  ///@{
-
-public:
-  Decl *ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
-                             SourceLocation KwLoc, IdentifierInfo *Ident,
-                             SourceLocation IdentLoc, SourceLocation LBrace);
-  void ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace);
-
-  bool CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
-
-  bool SemaBuiltinVectorMath(CallExpr *TheCall, QualType &Res);
-  bool SemaBuiltinVectorToScalarMath(CallExpr *TheCall);
-
-  ///@}
-
-  //
-  //
-  // -------------------------------------------------------------------------
-  //
-  //
-
   /// \name OpenMP Directives and Clauses
   /// Implementations are in SemaOpenMP.cpp
   ///@{
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
new file mode 100644
index 00000000000000..acc675963c23a5
--- /dev/null
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -0,0 +1,37 @@
+//===----- SemaHLSL.h ----- Semantic Analysis for HLSL constructs ---------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file declares semantic analysis for HLSL constructs.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SEMA_SEMAHLSL_H
+#define LLVM_CLANG_SEMA_SEMAHLSL_H
+
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Sema/Scope.h"
+#include "clang/Sema/SemaBase.h"
+
+namespace clang {
+
+class SemaHLSL : public SemaBase {
+public:
+  SemaHLSL(Sema &S);
+
+  Decl *ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
+                             SourceLocation KwLoc, IdentifierInfo *Ident,
+                             SourceLocation IdentLoc, SourceLocation LBrace);
+  void ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace);
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_SEMA_SEMAHLSL_H
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index 4fc6a2203cec36..5afc958600fa55 100644
--- a/clang/lib/Parse/ParseHLSL.cpp
+++ b/clang/lib/Parse/ParseHLSL.cpp
@@ -15,6 +15,7 @@
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
+#include "clang/Sema/SemaHLSL.h"
 
 using namespace clang;
 
@@ -71,9 +72,9 @@ Decl *Parser::ParseHLSLBuffer(SourceLocation &DeclEnd) {
     return nullptr;
   }
 
-  Decl *D = Actions.ActOnStartHLSLBuffer(getCurScope(), IsCBuffer, BufferLoc,
-                                         Identifier, IdentifierLoc,
-                                         T.getOpenLocation());
+  Decl *D = Actions.HLSL().ActOnStartHLSLBuffer(
+      getCurScope(), IsCBuffer, BufferLoc, Identifier, IdentifierLoc,
+      T.getOpenLocation());
 
   while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) {
     // FIXME: support attribute on constants inside cbuffer/tbuffer.
@@ -87,7 +88,7 @@ Decl *Parser::ParseHLSLBuffer(SourceLocation &DeclEnd) {
       T.skipToEnd();
       DeclEnd = T.getCloseLocation();
       BufferScope.Exit();
-      Actions.ActOnFinishHLSLBuffer(D, DeclEnd);
+      Actions.HLSL().ActOnFinishHLSLBuffer(D, DeclEnd);
       return nullptr;
     }
   }
@@ -95,7 +96,7 @@ Decl *Parser::ParseHLSLBuffer(SourceLocation &DeclEnd) {
   T.consumeClose();
   DeclEnd = T.getCloseLocation();
   BufferScope.Exit();
-  Actions.ActOnFinishHLSLBuffer(D, DeclEnd);
+  Actions.HLSL().ActOnFinishHLSLBuffer(D, DeclEnd);
 
   Actions.ProcessDeclAttributeList(Actions.CurScope, D, Attrs);
   return D;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 6b8e88e8850035..04eadb5f3b8ae6 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -42,6 +42,7 @@
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "clang/Sema/SemaHLSL.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/SemaOpenACC.h"
 #include "clang/Sema/TemplateDeduction.h"
@@ -198,6 +199,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
       LateTemplateParser(nullptr), LateTemplateParserCleanup(nullptr),
       OpaqueParser(nullptr), CurContext(nullptr), ExternalSource(nullptr),
       CurScope(nullptr), Ident_super(nullptr),
+      HLSLPtr(std::make_unique<SemaHLSL>(*this)),
       OpenACCPtr(std::make_unique<SemaOpenACC>(*this)),
       MSPointerToMemberRepresentationMethod(
           LangOpts.getMSPointerToMemberRepresentationMethod()),
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index cf82cc9bccdf51..099c71bd72f636 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -8,27 +8,31 @@
 // This implements Semantic Analysis for HLSL constructs.
 //===----------------------------------------------------------------------===//
 
+#include "clang/Sema/SemaHLSL.h"
 #include "clang/Sema/Sema.h"
 
 using namespace clang;
 
-Decl *Sema::ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
-                                 SourceLocation KwLoc, IdentifierInfo *Ident,
-                                 SourceLocation IdentLoc,
-                                 SourceLocation LBrace) {
+SemaHLSL::SemaHLSL(Sema &S) : SemaBase(S) {};
+
+Decl *SemaHLSL::ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
+                                     SourceLocation KwLoc,
+                                     IdentifierInfo *Ident,
+                                     SourceLocation IdentLoc,
+                                     SourceLocation LBrace) {
   // For anonymous namespace, take the location of the left brace.
-  DeclContext *LexicalParent = getCurLexicalContext();
+  DeclContext *LexicalParent = SemaRef.getCurLexicalContext();
   HLSLBufferDecl *Result = HLSLBufferDecl::Create(
-      Context, LexicalParent, CBuffer, KwLoc, Ident, IdentLoc, LBrace);
+      getASTContext(), LexicalParent, CBuffer, KwLoc, Ident, IdentLoc, LBrace);
 
-  PushOnScopeChains(Result, BufferScope);
-  PushDeclContext(BufferScope, Result);
+  SemaRef.PushOnScopeChains(Result, BufferScope);
+  SemaRef.PushDeclContext(BufferScope, Result);
 
   return Result;
 }
 
-void Sema::ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace) {
+void SemaHLSL::ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace) {
   auto *BufDecl = cast<HLSLBufferDecl>(Dcl);
   BufDecl->setRBraceLoc(RBrace);
-  PopDeclContext();
+  SemaRef.PopDeclContext();
 }

github-actions · 2024-04-07T09:47:45Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Endilll · 2024-04-07T09:55:15Z

clang-format 18.1.1 that we use in the workflow is complaining, because it's missing #82097.

cor3ntin · 2024-04-08T06:41:13Z

clang/include/clang/Sema/Sema.h

+  bool BuiltinVectorMath(CallExpr *TheCall, QualType &Res);
+  bool BuiltinVectorToScalarMath(CallExpr *TheCall);
+
+  bool CheckHLSLBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);


Shouldn't that be moved to SemaHLSL?

I tried, but it depends on a lot of static functions inside SemaChecking.cpp, so I decided to leave it for later.

cor3ntin · 2024-04-08T06:45:15Z

Should these be moved?

 void ActOnHLSLTopLevelFunction(FunctionDecl *FD);
  void CheckHLSLEntryPoint(FunctionDecl *FD);
  void CheckHLSLSemanticAnnotation(FunctionDecl *EntryPoint, const Decl *Param,
                                   const HLSLAnnotationAttr *AnnotationAttr);
  void DiagnoseHLSLAttrStageMismatch(
      const Attr *A, HLSLShaderAttr::ShaderType Stage,
      std::initializer_list<HLSLShaderAttr::ShaderType> AllowedStages);

What about

HLSLNumThreadsAttr *mergeHLSLNumThreadsAttr(Decl *D,
                                              const AttributeCommonInfo &AL,
                                              int X, int Y, int Z);
  HLSLShaderAttr *mergeHLSLShaderAttr(Decl *D, const AttributeCommonInfo &AL,
                                      HLSLShaderAttr::ShaderType ShaderType);
  HLSLParamModifierAttr *
  mergeHLSLParamModifierAttr(Decl *D, const AttributeCommonInfo &AL,
                             HLSLParamModifierAttr::Spelling Spelling);

There may be other functions hlsl specific that do not have 'HLSL" in their name

Endilll · 2024-04-08T06:50:17Z

Should these be moved?

Given their names, they are clearly on the list. However, I'm keeping the scope of this patch limited. I'd like to focus on getting HLSL contributors on board first.

llvm-beanz

I really like this direction. Thank you for taking the initiative on cleaning Sema up!

I had one small comment below, but you can take it or leave it.

llvm-beanz · 2024-04-08T13:20:05Z

clang/include/clang/Sema/SemaHLSL.h

+  Decl *ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
+                             SourceLocation KwLoc, IdentifierInfo *Ident,
+                             SourceLocation IdentLoc, SourceLocation LBrace);
+  void ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace);


nit: Since we're moving these into a class that has HLSL in the name which is accessed through a method named HLSL() should we maybe remove HLSL from the method names?

While HLSL() has to be used to access them from outside, it's not going to be there when accessed from within SemaHLSL. As I know very little about HLSL, I decided to leave the renaming to a subsequent PR.

AaronBallman

LGTM!

A follow-up to #87912. I'm moving more HLSL-related functions from `Sema` to `SemaHLSL`. I'm also dropping `HLSL` from their names in the process.

[clang] Introduce SemaHLSL

4a60eb0

Endilll added clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Apr 7, 2024

Endilll requested review from bogner, llvm-beanz, python3kgae and mgehre-amd April 7, 2024 09:45

llvmbot added the clang Clang issues not falling into any other category label Apr 7, 2024

Endilll added 2 commits April 7, 2024 16:45

Merge remote-tracking branch 'upstream/main' into semahlsl

7bec564

Remove unnecessary semicolon

7ce37f0

cor3ntin reviewed Apr 8, 2024

View reviewed changes

llvm-beanz approved these changes Apr 8, 2024

View reviewed changes

AaronBallman approved these changes Apr 8, 2024

View reviewed changes

Endilll merged commit d345f6a into llvm:main Apr 8, 2024
4 checks passed

Endilll deleted the semahlsl branch April 8, 2024 17:32

Endilll mentioned this pull request Apr 11, 2024

[clang][NFC] Move more functions to SemaHLSL #88354

Merged

Endilll added a commit that referenced this pull request Apr 12, 2024

[clang][NFC] Move more functions to SemaHLSL (#88354)

b45c9c3

A follow-up to #87912. I'm moving more HLSL-related functions from `Sema` to `SemaHLSL`. I'm also dropping `HLSL` from their names in the process.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Introduce `SemaHLSL` #87912

[clang] Introduce `SemaHLSL` #87912

Endilll commented Apr 7, 2024 •

edited

Loading

llvmbot commented Apr 7, 2024 •

edited

Loading

github-actions bot commented Apr 7, 2024 •

edited

Loading

Endilll commented Apr 7, 2024

cor3ntin Apr 8, 2024

Endilll Apr 8, 2024

cor3ntin commented Apr 8, 2024

Endilll commented Apr 8, 2024

llvm-beanz left a comment

llvm-beanz Apr 8, 2024

Endilll Apr 8, 2024

AaronBallman left a comment

[clang] Introduce SemaHLSL #87912

[clang] Introduce SemaHLSL #87912

Conversation

Endilll commented Apr 7, 2024 • edited Loading

llvmbot commented Apr 7, 2024 • edited Loading

github-actions bot commented Apr 7, 2024 • edited Loading

Endilll commented Apr 7, 2024

cor3ntin Apr 8, 2024

Choose a reason for hiding this comment

Endilll Apr 8, 2024

Choose a reason for hiding this comment

cor3ntin commented Apr 8, 2024

Endilll commented Apr 8, 2024

llvm-beanz left a comment

Choose a reason for hiding this comment

llvm-beanz Apr 8, 2024

Choose a reason for hiding this comment

Endilll Apr 8, 2024

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

[clang] Introduce `SemaHLSL` #87912

[clang] Introduce `SemaHLSL` #87912

Endilll commented Apr 7, 2024 •

edited

Loading

llvmbot commented Apr 7, 2024 •

edited

Loading

github-actions bot commented Apr 7, 2024 •

edited

Loading