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

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:46
 public:
+  struct ResBinding {
+    llvm::Optional<unsigned> Reg;
----------------
Does this apply to buffers only? In which case it might be better to either nest this into Buffer definition or rename into something like  `BufferResBinding`. Also adding some documenting comments would help here, even if they could just refer to the language documentation.


================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:64
   CodeGenModule &CGM;
   uint32_t ResourceCounters[static_cast<uint32_t>(
       hlsl::ResourceClass::NumClasses)] = {0};
----------------
This is not part of this change but any reason why it is a protected member and not private?


================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:77
 private:
+  void addResourceAnnotation(llvm::GlobalVariable *GV, llvm::StringRef TyName,
+                             hlsl::ResourceClass RC, ResBinding &Binding);
----------------
Similarly - is this buffer specific? Then you could either rename to `addBufferResourceAnnotation` or make this a member of a Buffer instead...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130951