[PATCH] D31042: Allow DataLayout to specify addrspace for allocas.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: include/llvm/IR/IRBuilder.h:1098
+  AllocaInst *CreateAlloca(const DataLayout &DL, Type *Ty,
+                           Value *ArraySize = nullptr, const Twine &Name = "") {
+    return Insert(new AllocaInst(Ty, DL.getStackAddrSpace(), ArraySize), Name);
----------------
efriedma wrote:
> I'm not sure this overload is a good idea; it saves a little typing, but it hides the reason we need the datalayout.  
I'm not sure that's a problem here. This is the most common version, and the point of IRBuilder is to be the convenient way of creating IR. A DataLayout argument is less error prone than an unsigned argument, where it easier to accidentally pass the alignment or something else.

It would also be able to hide this more by getting the parent module's DataLayout from the insertion block (which is what the CHERI patches did). However there are already functions using DataLayout input arguments, so I figured there might be some reason to pass it in rather than doing that.


https://reviews.llvm.org/D31042