Skip to content

[HLSL] Implement export keyword #96823

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 3 commits into from
Jul 1, 2024
Merged

[HLSL] Implement export keyword #96823

merged 3 commits into from
Jul 1, 2024

Conversation

hekota
Copy link
Member

@hekota hekota commented Jun 26, 2024

Implements export keyword in HLSL.

There are two ways the export keyword can be used:

  1. On individual function declarations
export void f() {}
  1. On a group of function declaration:
export {
   void f1();
   void f2() {}
}

Functions declared with the export keyword have external linkage. The implementation does not include validation of when a function can or cannot be exported, such as when it has resource argument or semantic annotations. That will be covered by #93330.

Currently all function declarations in global or named namespaces have external linkage by default so there are no specific code changes required right now to make sure exported function have external linkage as well. That will change as part of #92071. Any additional changes to make sure exported functions still have external linkage will be done as part of this work item.

Fixes #92812

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules HLSL HLSL Language Support labels Jun 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2024

@llvm/pr-subscribers-hlsl
@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Helena Kotas (hekota)

Changes

Implements export keyword in HLSL.

There are two ways the export keyword can be used:

  1. On individual function declarations
export void f() {}
  1. On a group of function declaration:
export {
   void f1();
   void f2() {}
}

Functions declared with the export keyword have external linkage. The implementation does not include validation of when a function can or cannot be exported, such as when it has resource argument or semantic annotations. That will be covered by llvm/llvm-project#93330.

Currently all function declarations in global or named namespaces have external linkage by default so there are no specific code changes required right now to make sure exported function have external linkage as well. That will change as part of llvm/llvm-project#92071. Any additional changes to make sure exported functions still have external linkage will be done as part of this work item.

Fixes #92812


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

7 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+8)
  • (modified) clang/lib/Parse/Parser.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaModule.cpp (+32)
  • (added) clang/test/AST/HLSL/export.hlsl (+23)
  • (added) clang/test/CodeGenHLSL/export.hlsl (+20)
  • (added) clang/test/SemaHLSL/export.hlsl (+50)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 25a87078a5709..a2465ecc936e0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12265,6 +12265,9 @@ def warn_hlsl_availability_unavailable :
   Warning<err_unavailable.Summary>,
   InGroup<HLSLAvailability>, DefaultError;
 
+def err_hlsl_export_not_on_function : Error<
+  "export declaration can only be used on functions">;
+
 // Layout randomization diagnostics.
 def err_non_designated_init_used : Error<
   "a randomized struct can only be initialized with a designated initializer">;
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 5e3ee5f0579aa..226377e93fe56 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -445,6 +445,14 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
 ///         'export' declaration
 ///         'export' '{' declaration-seq[opt] '}'
 ///
+/// HLSL: Parse export function declaration.
+///
+///      export-function-declaration: 
+///         'export' function-declaration
+/// 
+///      export-declaration-group:
+///         'export' '{' function-declaration-seq[opt] '}'
+///
 Decl *Parser::ParseExportDeclaration() {
   assert(Tok.is(tok::kw_export));
   SourceLocation ExportLoc = ConsumeToken();
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 6d0cf7b174e50..ddc8aa9b49e64 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -970,7 +970,7 @@ Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
     SingleDecl = ParseModuleImport(SourceLocation(), IS);
   } break;
   case tok::kw_export:
-    if (getLangOpts().CPlusPlusModules) {
+    if (getLangOpts().CPlusPlusModules || getLangOpts().HLSL) {
       ProhibitAttributes(Attrs);
       SingleDecl = ParseExportDeclaration();
       break;
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index ad118ac90e4aa..e920b880ecb4d 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -851,6 +851,21 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
   CurContext->addDecl(D);
   PushDeclContext(S, D);
 
+  if (getLangOpts().HLSL) {
+    // exported functions cannot be in an unnamed namespace
+    for (const DeclContext *DC = CurContext; DC; DC = DC->getLexicalParent()) {
+      if (const auto *ND = dyn_cast<NamespaceDecl>(DC)) {
+        if (ND->isAnonymousNamespace()) {
+          Diag(ExportLoc, diag::err_export_within_anonymous_namespace);
+          Diag(ND->getLocation(), diag::note_anonymous_namespace);
+          D->setInvalidDecl();
+          return D;
+        }
+      }
+    }
+    return D;
+  }
+
   // C++2a [module.interface]p1:
   //   An export-declaration shall appear only [...] in the purview of a module
   //   interface unit. An export-declaration shall not appear directly or
@@ -924,6 +939,23 @@ static bool checkExportedDeclContext(Sema &S, DeclContext *DC,
 /// Check that it's valid to export \p D.
 static bool checkExportedDecl(Sema &S, Decl *D, SourceLocation BlockStart) {
 
+  // HLSL: export declaration is valid only on functions
+  if (S.getLangOpts().HLSL) {
+    auto *FD = dyn_cast<FunctionDecl>(D);
+    if (!FD) {
+      if (auto *ED2 = dyn_cast<ExportDecl>(D)) {
+        S.Diag(ED2->getBeginLoc(), diag::err_export_within_export);
+        if (auto *ED1 = dyn_cast<ExportDecl>(D->getDeclContext()))
+          S.Diag(ED1->getBeginLoc(), diag::note_export);
+      }
+      else {
+        S.Diag(D->getBeginLoc(), diag::err_hlsl_export_not_on_function);
+      }
+      D->setInvalidDecl();
+      return false;
+    }
+  }
+
   //  C++20 [module.interface]p3:
   //   [...] it shall not declare a name with internal linkage.
   bool HasName = false;
diff --git a/clang/test/AST/HLSL/export.hlsl b/clang/test/AST/HLSL/export.hlsl
new file mode 100644
index 0000000000000..69c4fb2b457ac
--- /dev/null
+++ b/clang/test/AST/HLSL/export.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK:ExportDecl 0x{{[0-9a-f]+}} <{{.*}}> col:1
+// CHECK:FunctionDecl 0x{{[0-9a-f]+}} <{{.*}}> col:13 used f1 'void ()'
+// CHECK:CompoundStmt 0x{{[0-9a-f]+}} <{{.*}}>
+export void f1() {}
+
+// CHECK:NamespaceDecl 0x{{[0-9a-f]+}} <{{.*}}>
+// CHECK:ExportDecl 0x{{[0-9a-f]+}} <{{.*}}> col:3
+// CHECK:FunctionDecl 0x{{[0-9a-f]+}} <{{.*}}> col:15 used f2 'void ()'
+// CHECK:CompoundStmt 0x{{[0-9a-f]+}} <{{.*}}>
+namespace MyNamespace {
+  export void f2() {}
+}
+
+// CHECK:ExportDecl 0x{{[0-9a-f]+}} <{{.*}}>
+// CHECK:FunctionDecl 0x{{[0-9a-f]+}} <{{.*}}> col:10 used f3 'void ()'
+// CHECK:FunctionDecl 0x{{[0-9a-f]+}} <{{.*}}> col:10 used f4 'void ()'
+// CHECK:CompoundStmt 0x{{[0-9a-f]+}} <{{.*}}>
+export {
+    void f3() {}
+    void f4() {}
+}
diff --git a/clang/test/CodeGenHLSL/export.hlsl b/clang/test/CodeGenHLSL/export.hlsl
new file mode 100644
index 0000000000000..cea8875684b9e
--- /dev/null
+++ b/clang/test/CodeGenHLSL/export.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: define void @"?f1@@YAXXZ"()
+export void f1() {
+}
+
+// CHECK: define void @"?f2@MyNamespace@@YAXXZ"()
+namespace MyNamespace {
+  export void f2() {
+  }
+}
+
+export {
+// CHECK: define void @"?f3@@YAXXZ"()
+// CHECK: define void @"?f4@@YAXXZ"()    
+    void f3() {}
+    void f4() {}
+}
\ No newline at end of file
diff --git a/clang/test/SemaHLSL/export.hlsl b/clang/test/SemaHLSL/export.hlsl
new file mode 100644
index 0000000000000..0cb9248f3f589
--- /dev/null
+++ b/clang/test/SemaHLSL/export.hlsl
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify
+
+export void f1();
+
+export void f1() {}
+
+namespace { // expected-note {{anonymous namespace begins here}}
+    export void f2(); // expected-error {{export declaration appears within anonymous namespace}}
+}
+
+export void f3();
+
+export { // expected-note {{export block begins here}}
+    void f4() {}
+    export void f5() {} // expected-error {{export declaration appears within another export declaration}}
+    int A; // expected-error {{export declaration can only be used on functions}}
+    namespace ns { // expected-error {{export declaration can only be used on functions}}
+        void f6();
+    }
+}
+
+void export f7() {} // expected-error {{expected unqualified-id}}
+
+export static void f8() {} // expected-error {{declaration of 'f8' with internal linkage cannot be exported}}
+
+export void f9(); // expected-note {{previous declaration is here}}
+static void f9(); // expected-error {{static declaration of 'f9' follows non-static declaration}}
+
+static void f10(); // expected-note {{previous declaration is here}}
+export void f10(); // expected-error {{cannot export redeclaration 'f10' here since the previous declaration has internal linkage}}
+
+export float V1; // expected-error {{export declaration can only be used on functions}}
+
+static export float V2; // expected-error{{expected unqualified-id}}
+
+export static float V3 = 0; // expected-error {{export declaration can only be used on functions}}
+
+export groupshared float V4; // expected-error {{export declaration can only be used on functions}}
+
+void f6() {
+  export int i;  // expected-error {{expected expression}}
+}
+
+export cbuffer CB { // expected-error {{export declaration can only be used on functions}}
+    int a;
+}
+
+export template<typename T> void tf1(T t) {} // expected-error {{export declaration can only be used on functions}}
+
+void f5() export {} // expected-error {{expected function body after function declarator}}
\ No newline at end of file

@ChuanqiXu9
Copy link
Member

The grammer looks exactly the same with modules. May it be compatible with modules?

@llvm-beanz
Copy link
Collaborator

The grammer looks exactly the same with modules. May it be compatible with modules?

The grammar is the same (although HLSL only supports the export keyword in a subset of the places C++ does), but the behavior isn't the same. We may look to shift HLSL's behavior to match C++ in the future but for compatibility with existing code we need to support the HLSL behavior.


export { // expected-note {{export block begins here}}
void f4() {}
export void f5() {} // expected-error {{export declaration appears within another export declaration}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale for prohibiting nested exports?

I notice that C++ allows nested extern 'C's, for example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each export creates a new ExportDecl node in the AST three and limiting it to just one make it cleaner. I am not aware of any specific reason other than maybe this and to keep the syntax and parsing code identical to C++ modules export.

@hekota hekota merged commit 938cbdb into llvm:main Jul 1, 2024
7 checks passed
hekota added a commit that referenced this pull request Jul 2, 2024
Implements availability diagnostic on `export` functions. 

For shader libraries the HLSL availability diagnostic should run on all
entry points and export functions. Now that the `export` keyword is
implemented (#96823), we can detect which functions are
exported and run the diagnostic on them.

Exported functions can be nested in namespaces and in export
declarations so we need to scan not just the current translation unit
but also namespace and export declarations contexts.

Fixes #92073
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
Implements `export` keyword in HLSL.

There are two ways the `export` keyword can be used:
1. On individual function declarations
```
export void f() {}
```
2. On a group of function declaration:
```
export {
   void f1();
   void f2() {}
}
```

Functions declared with the `export` keyword have external linkage. The
implementation does not include validation of when a function can or
cannot be exported, such as when it has resource argument or semantic
annotations. That will be covered by llvm#93330.

Currently all function declarations in global or named namespaces have
external linkage by default so there are no specific code changes
required right now to make sure exported function have external linkage
as well. That will change as part of llvm#92071. Any
additional changes to make sure exported functions still have external
linkage will be done as part of this work item.

Fixes llvm#92812
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
Implements availability diagnostic on `export` functions. 

For shader libraries the HLSL availability diagnostic should run on all
entry points and export functions. Now that the `export` keyword is
implemented (llvm#96823), we can detect which functions are
exported and run the diagnostic on them.

Exported functions can be nested in namespaces and in export
declarations so we need to scan not just the current translation unit
but also namespace and export declarations contexts.

Fixes llvm#92073
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Implements `export` keyword in HLSL.

There are two ways the `export` keyword can be used:
1. On individual function declarations
```
export void f() {}
```
2. On a group of function declaration:
```
export {
   void f1();
   void f2() {}
}
```

Functions declared with the `export` keyword have external linkage. The
implementation does not include validation of when a function can or
cannot be exported, such as when it has resource argument or semantic
annotations. That will be covered by llvm#93330.

Currently all function declarations in global or named namespaces have
external linkage by default so there are no specific code changes
required right now to make sure exported function have external linkage
as well. That will change as part of llvm#92071. Any
additional changes to make sure exported functions still have external
linkage will be done as part of this work item.

Fixes llvm#92812
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Implements availability diagnostic on `export` functions. 

For shader libraries the HLSL availability diagnostic should run on all
entry points and export functions. Now that the `export` keyword is
implemented (llvm#96823), we can detect which functions are
exported and run the diagnostic on them.

Exported functions can be nested in namespaces and in export
declarations so we need to scan not just the current translation unit
but also namespace and export declarations contexts.

Fixes llvm#92073
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:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[HLSL] Implement export keyword
6 participants