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

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added inline comments.


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:131
   case Decl::RequiresExprBody:
+  case Decl::HLSLBuffer:
     // None of these decls require codegen support.
----------------
efriedma wrote:
> 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()?
Nice catch.
Fixed.


================
Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:112
+    // Replace.
+    GV->replaceAllUsesWith(GEP);
+    // Erase GV.
----------------
efriedma wrote:
> 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).
These globals should not be in any map except Buf.Constants which is used here.
If another map has these globals, we should remove them from the map.
Cannot see any other map has these globals now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130131