[PATCH] D56582: [InstCombine]Avoid introduction of unaligned mem access
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 15 14:34:12 PST 2019
reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: test/Transforms/InstCombine/element-atomic-memintrins.ll:15
define void @test_memset_to_store(i8* %dest) {
; CHECK-LABEL: @test_memset_to_store(
----------------
reames wrote:
> I think these changes break the spirit of the tests. I'd recommend just providing the required alignment information on the dest argument. And then add a separate test for the unaligned case.
I was suggesting that you add an align(8) to the %dest argument. This would preserve the transform for the existing tests, and you could add a new test showing the inhibited transform w/o the alignment. Reviewing your tests, I think you do have the coverage, just not quite the way I was suggesting. So, consider this an optional comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56582/new/
https://reviews.llvm.org/D56582
More information about the llvm-commits
mailing list