[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:131
   case Decl::RequiresExprBody:
+  case Decl::HLSLBuffer:
     // None of these decls require codegen support.
----------------
I'm a little confused by this.  If it's possible to declare an HLSLBuffer inside a function, why don't you need to handle it?  If it isn't possible to declare an HLSLBuffer this way, can you just move this to use the llvm_unreachable()?


================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:112
+    // Replace.
+    GV->replaceAllUsesWith(GEP);
+    // Erase GV.
----------------
Messing with globals like this feels a little weird, but I guess it's fine if nothing actually tries to use the erased globals after this code runs.  I'm a little concerned that someone might accidentally rearrange the relevant code in the future (CodeGenModule has a bunch of maps which aren't cleared before this code runs).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130131