[PATCH] D79968: StoreInst should store Align, not MaybeAlign

Johannes Doerfert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 14 21:13:31 PDT 2020


jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM, two minor things below.



================
Comment at: clang/lib/CodeGen/CGExpr.cpp:130
+  auto *Store = new llvm::StoreInst(Init, Var.getPointer(), false,
+                                    Var.getAlignment().getAsAlign());
   llvm::BasicBlock *Block = AllocaInsertPt->getParent();
----------------
Nit: I guess `/* volatile */ false`


================
Comment at: llvm/lib/IR/Instructions.cpp:1422
+    : StoreInst(val, addr, isVolatile,
+                computeLoadAlign(val->getType(), InsertBefore), InsertBefore) {}
 
----------------
Minor: Do we want toe keep the name? `computeAccessAlign` maybe? Or a shallow `computeStoreAlign` wrapper? No strong feelings, just odd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79968





More information about the cfe-commits mailing list