[PATCH] D77454: LoadInst should store Align, not MaybeAlign.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I'll accept tomorrow if no one raises a concern.



================
Comment at: llvm/include/llvm/IR/Instructions.h:184
-  LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr = "",
-           Instruction *InsertBefore = nullptr);
   LoadInst(Type *Ty, Value *Ptr, const Twine &NameStr, BasicBlock *InsertAtEnd);
----------------
efriedma wrote:
> jdoerfert wrote:
> > efriedma wrote:
> > > jdoerfert wrote:
> > > > Why do we want to remove the default values?
> > > The four constructors that don't explicitly specify an alignment now grab the DataLayout from the last argument.  So it can't be null.
> > Makes sense. TBH, I would make it a reference so it breaks at compile time not runtime when null is passed explicitly.
> Maybe it should be a reference... but I  I don't really want to sign up to change every constructor for every Instruction to pass a reference instead of a pointer.
Yeah... Interesting that no in-tree user passes a nullptr. Maybe people don't do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77454