[PATCH] D80044: AllocaInst should store Align instead of MaybeAlign.

[Eli Friedman via Phabricator via llvm-commits]

efriedma marked 2 inline comments as done.
efriedma added inline comments.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:7071
+  SmallPtrSet<Type *, 4> Visited;
+  if (!Alignment && !Ty->isSized(&Visited))
     return Error(ExplicitTypeLoc, "loading unsized types is not allowed");
----------------
I guess it's worth briefly noting what this is about: if isSized is passed a SmallPtrSet, it uses that set to catch infinitely recursive types (for example, a struct that has itself as a member).  Otherwise, it just crashes on such types.

An existing test for alloca caught this, so while I was here I extended the same check to load/store.


================
Comment at: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp:390
+          AgTy, DL.getAllocaAddrSpace(), nullptr,
+          I->getParamAlign().getValueOr(DL.getPrefTypeAlign(AgTy)), "",
+          InsertPt);
----------------
I'm not completely confident this is correct; LangRef says the default alignment of a byval argument is target-dependent (not encoded in the datalayout), so it could theoretically by higher than getPrefTypeAlign().  But this matches the current behavior of the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80044