[IRBuilder] Don't crash when creating alloca/load/store in unparented block.

[rudkx]

Summary

Previously it was possible to create allocas/loads/stores in blocks
that had not previously been inserted in the function. A recent
change (https://reviews.llvm.org/D77984) uses getModule() on
the block to look up the DataLayout and automatically compute
an alignment in the cases where it isn't passed in. That results
in a crash in cases where the block hasn't already been inserted
into a function.

Instead, we'll now check that we successfully retrieved the
module before dereferencing it, and if we aren't able to retrieve
it we'll fall back on having no explicit alignment.

In one implementation of CreateAlloca we were also looking up
the default address space from the DataLayout. We don't have
have a way to avoid looking up the Module in order to get the
DataLayout, and don't have a way to otherwise default the address
space, so that implementation will still crash if called on
a block that hasn't yet been inserted into a function.

[efriedma]

The direction we're moving is that load/store instructions will store an "Align", not a "MaybeAlign". So it will be impossible to leave the alignment unspecified.

It would be possible to store the DataLayout as a member of the IRBuilder, so it's possible to query the datalayout even if the basic block doesn't have a parent, I guess. Not sure what the use-case would be, though; I can't imagine creating a basic block without knowing what function it will be attached to.

[rudkx]

In the particular out-of-tree code I'm trying to fix up the issue isn't that you don't know what function the blocks will go into, more that you don't know at the point when the blocks are created whether they will end up getting inserted into the function at all, or where. Imagine something like:

struct Blocks { BasicBlock *First, *Second, etc. };
switch (condition) {
  case OneOfSeveral:
    Blocks = createBlocksForOptionOne(...);
    break;
 case TwoOfSeveral:
   Blocks = ...;
}
// ...
// under the right conditions, call the function that inserts the blocks appropriately
if (...) {
  InsertBlocks(Blocks);
}

An alternative would be to insert all the blocks arbitrarily when they are created, the move them later.

If the general direction here is to just always require an alignment (which FWIW I think makes perfect sense), then I'll just do that in this code and move on.

Thanks for taking a look.

llvm/include/llvm/IR/IRBuilder.h

Value *CreateNAryOp(unsigned Opc, ArrayRef<Value *> Ops,
                       const Twine &Name = "", MDNode *FPMathTag = nullptr);
 
   //===--------------------------------------------------------------------===//
   // Instruction creation methods: Memory Instructions
   //===--------------------------------------------------------------------===//
 
   AllocaInst *CreateAlloca(Type *Ty, unsigned AddrSpace,
                            Value *ArraySize = nullptr, const Twine &Name = "") {
-    const DataLayout &DL = BB->getModule()->getDataLayout();
-    Align AllocaAlign = DL.getPrefTypeAlign(Ty);
+    Module *Mod = BB->getModule();
+    MaybeAlign AllocaAlign;
+    if (Mod) {
+      const DataLayout &DL = Mod->getDataLayout();
+      AllocaAlign = DL.getPrefTypeAlign(Ty);
+    }
     return Insert(new AllocaInst(Ty, AddrSpace, ArraySize, AllocaAlign), Name);
   }
 
+  /// Only valid for builders created with blocks that have already been
+  /// inserted into a function. It otherwise necessarily crashes when trying to
+  /// retrieve the module to get the alignment and address space.
   AllocaInst *CreateAlloca(Type *Ty, Value *ArraySize = nullptr,
                            const Twine &Name = "") {
     const DataLayout &DL = BB->getModule()->getDataLayout();
     Align AllocaAlign = DL.getPrefTypeAlign(Ty);
     unsigned AddrSpace = DL.getAllocaAddrSpace();
     return Insert(new AllocaInst(Ty, AddrSpace, ArraySize, AllocaAlign), Name);
   }
 
LLVM_ATTRIBUTE_DEPRECATED(LoadInst *CreateAlignedLoad(Type *Ty, Value *Ptr,
                                                         bool isVolatile,
                                                         const Twine &Name = ""),
                             "Use the version that takes MaybeAlign instead") {
     return CreateAlignedLoad(Ty, Ptr, MaybeAlign(Align), isVolatile, Name);
   }
   LoadInst *CreateAlignedLoad(Type *Ty, Value *Ptr, MaybeAlign Align,
                               bool isVolatile, const Twine &Name = "") {
     if (!Align) {
-      const DataLayout &DL = BB->getModule()->getDataLayout();
+      Module *Mod = BB->getModule();
+      if (Mod) {
+        const DataLayout &DL = Mod->getDataLayout();
         Align = DL.getABITypeAlign(Ty);
       }
+    }
     return Insert(new LoadInst(Ty, Ptr, Twine(), isVolatile, *Align), Name);
   }
 
   // Deprecated [opaque pointer types]
   LLVM_ATTRIBUTE_DEPRECATED(LoadInst *CreateAlignedLoad(Value *Ptr,
                                                         unsigned Align,
                                                         const char *Name),
                             "Use the version that takes MaybeAlign instead") {
LLVM_ATTRIBUTE_DEPRECATED(
       StoreInst *CreateAlignedStore(Value *Val, Value *Ptr, unsigned Align,
                                     bool isVolatile = false),
       "Use the version that takes MaybeAlign instead") {
     return CreateAlignedStore(Val, Ptr, MaybeAlign(Align), isVolatile);
   }
   StoreInst *CreateAlignedStore(Value *Val, Value *Ptr, MaybeAlign Align,
                                 bool isVolatile = false) {
     if (!Align) {
-      const DataLayout &DL = BB->getModule()->getDataLayout();
+      Module *Mod = BB->getModule();
+      if (Mod) {
+        const DataLayout &DL = Mod->getDataLayout();
         Align = DL.getABITypeAlign(Val->getType());
       }
+    }
     return Insert(new StoreInst(Val, Ptr, isVolatile, Align));
   }
   FenceInst *CreateFence(AtomicOrdering Ordering,
                          SyncScope::ID SSID = SyncScope::System,
                          const Twine &Name = "") {
     return Insert(new FenceInst(Context, Ordering, SSID), Name);
   }