self-review: misc clean up

Author: inbelic

clang/lib/CodeGen/CGHLSLRuntime.cpp

@@ -68,15 +68,14 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) {
   DXILValMD->addOperand(Val);
 }
 
-void addRootSignature(
-    ArrayRef<llvm::hlsl::rootsig::RootElement> Elements,
-    llvm::Function *Fn, llvm::Module &M) {
+void addRootSignature(ArrayRef<llvm::hlsl::rootsig::RootElement> Elements,
+                      llvm::Function *Fn, llvm::Module &M) {
   auto &Ctx = M.getContext();
 
   llvm::hlsl::rootsig::MetadataBuilder Builder(Ctx, Elements);
   MDNode *RootSignature = Builder.BuildRootSignature();
-  MDNode *FnPairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
-                                        RootSignature});
+  MDNode *FnPairing =
+      MDNode::get(Ctx, {ValueAsMetadata::get(Fn), RootSignature});
 
   StringRef RootSignatureValKey = "dx.rootsignatures";
   auto *RootSignatureValMD = M.getOrInsertNamedMetadata(RootSignatureValKey);
@@ -443,7 +442,8 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
   const AttrVec &Attrs = FD->getAttrs();
   for (const Attr *Attr : Attrs) {
     if (const auto *RSAttr = dyn_cast<RootSignatureAttr>(Attr))
-      addRootSignature(RSAttr->getSignatureDecl()->getRootElements(), EntryFn, M);
+      addRootSignature(RSAttr->getSignatureDecl()->getRootElements(), EntryFn,
+                       M);
   }
 }
 

clang/test/CodeGenHLSL/RootSignature.hlsl

@@ -19,7 +19,7 @@ void FirstEntry() {}
 [numthreads(1,1,1)]
 void SecondEntry() {}
 
-// Sanity test to ensure to root is added for this function
+// Sanity test to ensure no root is added for this function
 [shader("compute")]
 [numthreads(1,1,1)]
 void ThirdEntry() {}

llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h

@@ -131,15 +131,18 @@ using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable,
 void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
 
 class MetadataBuilder {
- public:
+public:
   MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
-   : Ctx(Ctx), Elements(Elements) {}
+      : Ctx(Ctx), Elements(Elements) {}
 
-  // Iterates through the elements and builds the respective nodes
+  /// 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
   MDNode *BuildRootSignature();
 
- private:
-  // Define the various builders for the different metadata types
+private:
+  /// Define the various builders for the different metadata types
   MDNode *BuildDescriptorTable(const DescriptorTable &Table);
   MDNode *BuildDescriptorTableClause(const DescriptorTableClause &Clause);
 

llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp

@@ -10,8 +10,8 @@
 ///
 //===----------------------------------------------------------------------===//
 
-#include "llvm/ADT/bit.h"
 #include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/ADT/bit.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
@@ -198,17 +198,27 @@ MDNode *MetadataBuilder::BuildRootSignature() {
 MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
   IRBuilder<> B(Ctx);
   SmallVector<Metadata *> TableOperands;
+  // Set the mandatory arguments
   TableOperands.push_back(MDString::get(Ctx, "DescriptorTable"));
-  TableOperands.push_back(ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Table.Visibility))));
-
-  assert(Table.NumClauses <= GeneratedMetadata.size() && "Table expected all owned clauses to be generated already");
-  TableOperands.append(GeneratedMetadata.end() - Table.NumClauses, GeneratedMetadata.end());
+  TableOperands.push_back(ConstantAsMetadata::get(
+      B.getInt32(llvm::to_underlying(Table.Visibility))));
+
+  // Remaining operands are references to the table's clauses. The in-memory
+  // representation of the Root Elements created from parsing will ensure that
+  // the previous N elements are the clauses for this table.
+  assert(Table.NumClauses <= GeneratedMetadata.size() &&
+         "Table expected all owned clauses to be generated already");
+  // So, add a refence to each clause to our operands
+  TableOperands.append(GeneratedMetadata.end() - Table.NumClauses,
+                       GeneratedMetadata.end());
+  // Then, remove those clauses from the general list of Root Elements
   GeneratedMetadata.pop_back_n(Table.NumClauses);
 
   return MDNode::get(Ctx, TableOperands);
 }
 
-MDNode *MetadataBuilder::BuildDescriptorTableClause(const DescriptorTableClause &Clause) {
+MDNode *MetadataBuilder::BuildDescriptorTableClause(
+    const DescriptorTableClause &Clause) {
   IRBuilder<> B(Ctx);
   return MDNode::get(Ctx, {
     ClauseTypeToName(Ctx, Clause.Type),