This is equivalent in terms of LLVM IR semantics, but we want to transition away from using MaybeAlign to represent the alignment of these instructions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks, this is indeed much better and the direction again in-line with what i've raised in some attributor patch.
LG to me in general, but please wait for someone else to take a look, too.
llvm/include/llvm/IR/IRBuilder.h | ||
---|---|---|
1597 | I was thinking we might want to switch CreateAlignedLoad to take an Align instead of a MaybeAlign: I'm not sure there's any good reason to use a MaybeAlign once we clean up the getters on load/store/alloca. But I guess we can do that later, if it turns out to actually be feasible; I'll change it. |
Actually address all the review comments. Fix CreateAlloca to use the pref alignment instead of the ABI alignment, like instcombine and selectiondag.
This revision has broken an MLIR test case that tests conversion to LLVM IR (test/Target/llvmir.mlir - an llvm.alloca in MLIR with its alignment attribute set to 0 now converts into an LLVM IR alloca with alignment automatically set to 4 - I think the fix is trivial; the check there is expecting to see no align attribute attached to the resulting alloca). Please build with -DLLVM_ENABLE_PROJECTS=mlir to reproduce, but revisions submitted via arcanist will already show the failed test cases.
Can't we just pawn of the alignment stuff to CreateAlignedLoad? We cal lit with 0 here and we determine an alignment there anyway?
Nit: BB->getModule() should work.