-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[NFC][RootSignature] Move RootSignature util functions #142491
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
Conversation
- this commit moves the helper utilities functions/helper objects to a seperate file, HLSLRootSignatureUtils.[h|cpp]
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang Author: Finn Plummer (inbelic) Changes
However, there many users of the structs that don't require any of the helper methods. This requires us to link the For instance:
This change allows the struct definitions to be kept in a single header file and to then have the Full diff: https://github.com/llvm/llvm-project/pull/142491.diff 7 Files Affected:
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index 112e902dfb374..1b84b8824047b 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -24,7 +24,7 @@
#include "clang/Basic/Specifiers.h"
#include "clang/Basic/TypeTraits.h"
#include "llvm/ADT/StringExtras.h"
-#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
#include <algorithm>
#include <utility>
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index cfe9dc1192d9d..dbfdd0e1ecfbb 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -23,6 +23,7 @@
#include "clang/AST/Type.h"
#include "clang/Basic/TargetOptions.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/GlobalVariable.h"
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index ca383a828b5cc..2f028817b45b6 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -6,25 +6,19 @@
//
//===----------------------------------------------------------------------===//
///
-/// \file This file contains helper objects for working with HLSL Root
-/// Signatures.
+/// \file This file contains structure definitions of HLSL Root Signature
+/// objects.
///
//===----------------------------------------------------------------------===//
#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
-#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/DXILABI.h"
-#include "llvm/Support/raw_ostream.h"
#include <variant>
namespace llvm {
-class LLVMContext;
-class MDNode;
-class Metadata;
-
namespace hlsl {
namespace rootsig {
@@ -195,8 +189,6 @@ struct DescriptorTable {
uint32_t NumClauses = 0;
};
-LLVM_ABI raw_ostream &operator<<(raw_ostream &OS, const DescriptorTable &Table);
-
static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
@@ -225,9 +217,6 @@ struct DescriptorTableClause {
}
};
-LLVM_ABI raw_ostream &operator<<(raw_ostream &OS,
- const DescriptorTableClause &Clause);
-
struct StaticSampler {
Register Reg;
SamplerFilter Filter = SamplerFilter::Anisotropic;
@@ -264,32 +253,6 @@ using RootElement =
std::variant<RootFlags, RootConstants, RootDescriptor, DescriptorTable,
DescriptorTableClause, StaticSampler>;
-LLVM_ABI void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
-
-class MetadataBuilder {
-public:
- MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
- : Ctx(Ctx), Elements(Elements) {}
-
- /// Iterates through the elements and dispatches onto the correct Build method
- ///
- /// Accumulates the root signature and returns the Metadata node that is just
- /// a list of all the elements
- LLVM_ABI MDNode *BuildRootSignature();
-
-private:
- /// Define the various builders for the different metadata types
- MDNode *BuildRootFlags(const RootFlags &Flags);
- MDNode *BuildRootConstants(const RootConstants &Constants);
- MDNode *BuildRootDescriptor(const RootDescriptor &Descriptor);
- MDNode *BuildDescriptorTable(const DescriptorTable &Table);
- MDNode *BuildDescriptorTableClause(const DescriptorTableClause &Clause);
-
- llvm::LLVMContext &Ctx;
- ArrayRef<RootElement> Elements;
- SmallVector<Metadata *> GeneratedMetadata;
-};
-
} // namespace rootsig
} // namespace hlsl
} // namespace llvm
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h
new file mode 100644
index 0000000000000..365197a4dfdb5
--- /dev/null
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignatureUtils.h
@@ -0,0 +1,65 @@
+//===- HLSLRootSignatureUtils.h - HLSL Root Signature helpers -------------===//
+//
+// 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 contains helper obejcts for working with HLSL Root
+/// Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATUREUTILS_H
+#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATUREUTILS_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace llvm {
+class LLVMContext;
+class MDNode;
+class Metadata;
+
+namespace hlsl {
+namespace rootsig {
+
+LLVM_ABI raw_ostream &operator<<(raw_ostream &OS,
+ const DescriptorTableClause &Clause);
+
+LLVM_ABI raw_ostream &operator<<(raw_ostream &OS, const DescriptorTable &Table);
+
+LLVM_ABI void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
+
+class MetadataBuilder {
+public:
+ MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
+ : Ctx(Ctx), Elements(Elements) {}
+
+ /// Iterates through the elements and dispatches onto the correct Build method
+ ///
+ /// Accumulates the root signature and returns the Metadata node that is just
+ /// a list of all the elements
+ LLVM_ABI MDNode *BuildRootSignature();
+
+private:
+ /// Define the various builders for the different metadata types
+ MDNode *BuildRootFlags(const RootFlags &Flags);
+ MDNode *BuildRootConstants(const RootConstants &Constants);
+ MDNode *BuildRootDescriptor(const RootDescriptor &Descriptor);
+ MDNode *BuildDescriptorTable(const DescriptorTable &Table);
+ MDNode *BuildDescriptorTableClause(const DescriptorTableClause &Clause);
+
+ llvm::LLVMContext &Ctx;
+ ArrayRef<RootElement> Elements;
+ SmallVector<Metadata *> GeneratedMetadata;
+};
+
+} // namespace rootsig
+} // namespace hlsl
+} // namespace llvm
+
+#endif // LLVM_FRONTEND_HLSL_HLSLROOTSIGNATUREUTILS_H
diff --git a/llvm/lib/Frontend/HLSL/CMakeLists.txt b/llvm/lib/Frontend/HLSL/CMakeLists.txt
index dd987544fe80b..dfebe354807bf 100644
--- a/llvm/lib/Frontend/HLSL/CMakeLists.txt
+++ b/llvm/lib/Frontend/HLSL/CMakeLists.txt
@@ -1,7 +1,7 @@
add_llvm_component_library(LLVMFrontendHLSL
CBuffer.cpp
HLSLResource.cpp
- HLSLRootSignature.cpp
+ HLSLRootSignatureUtils.cpp
ADDITIONAL_HEADER_DIRS
${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
similarity index 98%
rename from llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
rename to llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
index b8d6e229006d0..f533d8d3dd8ed 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignatureUtils.cpp
@@ -1,4 +1,4 @@
-//===- HLSLRootSignature.cpp - HLSL Root Signature helper objects ---------===//
+//===- HLSLRootSignatureUtils.cpp - HLSL Root Signature helpers -----------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -10,9 +10,9 @@
///
//===----------------------------------------------------------------------===//
-#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/bit.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
diff --git a/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
index 3f92fa0f05794..90e6cd0a80d6b 100644
--- a/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
+++ b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/Frontend/HLSL/HLSLRootSignatureUtils.h"
#include "gtest/gtest.h"
using namespace llvm::hlsl::rootsig;
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/11956 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/13551 Here is the relevant piece of the build log for the reference
|
HLSLRootSignature.h
was originally created to hold the struct definitions of anllvm::hlsl::rootsig::RootElement
and some helper functions for it.However, there many users of the structs that don't require any of the helper methods. This requires us to link the
FrontendHLSL
library, where we otherwise wouldn't need to.For instance:
FrontendHLSL
libraryHLSLRootSignatureVersion
enum. Ideally this could live with the root signature struct defs, but we don't want to link the helper objects intoclang/Basic/TargetOptions.h
This change allows the struct definitions to be kept in a single header file and to then have the
FrontendHLSL
library only be linked when required.