[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

Fri Oct 14 12:24:03 PDT 2022

beanz added inline comments.

================
Comment at: clang/include/clang/Basic/HLSLRuntime.h:31
+// NOTE: keep sync with ResourceBase::Kinds in DirectX backend.
+enum class ResourceKind : uint32_t {
+  Invalid = 0,
----------------
If this is only used in the clangCodeGen library, we can move it into libLLVMFrontendHLSL to share instead of keeping them in sync.

================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:222
+  IRBuilder<> B(Ctx);
+  ResourceMD->addOperand(MDNode::get(
+      Ctx, {ValueAsMetadata::get(GV), MDString::get(Ctx, TyName),
----------------
Shouldn't this be using `FrontendResource`?

================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:253
+  addBufferResourceAnnotation(GV, QT.getAsString(),
+                              static_cast<hlsl::ResourceClass>(RC), Binding);
+}
----------------
since we're relying on these two enums basically being identical, can you add `static_assert`s up near the top of this file to ensure that they are the same? It would be unfortunate if in the future one got updated and the other didn't.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130951/new/

https://reviews.llvm.org/D130951