[PATCH] D110257: [CFE][Codegen] Make sure to maintain the contiguity of all the static allocas

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 11 22:49:42 PDT 2021


rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:392
+  ///     order immediately after all static allocas.
+  llvm::BasicBlock::iterator AllocaAddrSpaceInsertPt;
+
----------------
Please call this something like `PostAllocaInsertPt`.  Instead of eagerly creating it, please create it lazily: add an accessor like `getPostAllocaInsertPoint()` which creates (and saves here) an instruction that immediately follows `AllocaInsertPt` if this is currently null.  I think you should make your own instruction so that we don't mess up any code that might rely on temporarily creating and then removing dead instructions.

Please use an `llvm::AssertingVH<llvm::Instruction>`.  I think that can handle holding a null value.

The comment here should be more general, like "a place in the prologue where code can be inserted that will be dominated by all the static allocas."  You don't need to talk about  `addrspacecast`s specifically; that's just one possible use case for this.

Also, it's either "`addrspacecast`" (using the IR name of the operation) or "address space cast" (writing out the operation in normal English words); don't run together `addressspace` as if it were a keyword when it isn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257



More information about the cfe-commits mailing list