[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. That fix looks right; thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Uday Bondhugula via Phabricator via cfe-commits
bondhugula added a comment. In D77984#1979644 , @bondhugula wrote: > This revision has broken an MLIR test case that test conversion to LLVM IR > (test/Target/llvmir.mlir - an llvm.alloca in MLIR with its alignment > attribute set to 0 now converts into

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Uday Bondhugula via Phabricator via cfe-commits
bondhugula added a comment. This revision has broken MLIR test cases that test 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). P

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG89e0662dee5f: Make IRBuilder automatically set alignment on load/store/alloca. (authored by efriedma). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/ne

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 _

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 257063. efriedma added a comment. Actually address all the review comments. Fix CreateAlloca to use the pref alignment instead of the ABI alignment, like instcombine and selectiondag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma planned changes to this revision. efriedma added a comment. Err, didn't quite address all the review comments. New version soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 __

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 257059. efriedma added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77984/new/ https://reviews.llvm.org/D77984 Files: clang/test/CodeGen/arm_neon_intrinsics.c llvm/incl

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma marked an inline comment as done. efriedma added inline comments. Comment at: llvm/include/llvm/IR/IRBuilder.h:1600 +return CreateAlignedLoad(Ty, Ptr, DL.getABITypeAlign(Ty), isVolatile, Name); } jdoerfert wrote: > Can't we just pawn of the ali

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. I think this is fine. Two comments to consider below. Comment at: llvm/include/llvm/IR/IRBuilder.h:1600 +return CreateAlignedLoad(Ty, Ptr, DL.getABITypeAlign(Ty), isVolatile, Name); } Can't we just pawn of the alignment stuf

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

2020-04-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision. efriedma added reviewers: jdoerfert, lebedev.ri, spatel. Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely. Herald added a reviewer: bollu. Herald added a project: clang. This is equivalent in terms of LLVM IR semantics, but we want to transition aw