This is an archive of the discontinued LLVM Phabricator instance.

Restore original intent of memset instcombine test
ClosedPublic

Authored by dneilson on Jun 26 2017, 1:14 PM.

Details

Summary

The original intent of test/Transforms/InstCombine/memset.ll was to test for lowering of llvm.memset into stores when the size of the memset is 1, 2, 4, or 8. Sometime between then and now the test has stopped testing for that, but remained passing due to testing for the absence of llvm.memset calls rather than the presence of store instructions. Right now this test ends up with an empty function body because the alloca is eliminated as safe-to-remove, which results in the llvm.memset calls's being eliminated due to their pointer args being undef; so it is not testing for conversion of llvm.memset into store instructions at all.

This change alters the test to verify that store instructions are created, and moves the target of the memset to an arg of the proc to avoid it being eliminated as unused.

Diff Detail

Repository
rL LLVM

Event Timeline

dneilson created this revision.Jun 26 2017, 1:14 PM
efriedma edited reviewers, added: efriedma; removed: lattner.Jun 26 2017, 1:25 PM
efriedma added a subscriber: efriedma.
efriedma added inline comments.
test/Transforms/InstCombine/memset.ll
14 ↗(On Diff #104010)

Your CHECK lines aren't checking that this memset was eliminated. (Maybe you can do something with CHECK-NEXT?)

dneilson added inline comments.Jun 26 2017, 1:26 PM
test/Transforms/InstCombine/memset.ll
14 ↗(On Diff #104010)

Good point. CHECK-NOT, perhaps?

dneilson updated this revision to Diff 104013.Jun 26 2017, 1:32 PM

Add CHECK-NOT to ensure that the memset calls have been removed.

dneilson marked an inline comment as done.Jun 26 2017, 1:32 PM
efriedma added inline comments.Jun 26 2017, 4:57 PM
test/Transforms/InstCombine/memset.ll
14 ↗(On Diff #104010)

CHECK-NOT isn't really doing what you want it to here... it doesn't apply to the whole file, just the bit after the last CHECK line.

Hence my suggestion to use CHECK-NEXT: if every check is a CHECK-NEXT, you can verify that there aren't any unexpected instructions in the IR.

dneilson updated this revision to Diff 104158.Jun 27 2017, 7:08 AM

Use CHECK-NEXT to verify test contents.

This revision is now accepted and ready to land.Jun 27 2017, 10:47 AM
This revision was automatically updated to reflect the committed changes.