This is an archive of the discontinued LLVM Phabricator instance.

Add element-atomic mem intrinsic canary tests for InstCombine.
ClosedPublic

Authored by dneilson on Jul 17 2017, 1:16 PM.

Details

Summary

Add canary tests to verify that InstCombine currently does nothing with the element atomic memory intrinsics for memmove and memset.

Placeholder tests that will fail once element atomic @llvm.mem[move|set] instrinsics have been added to the MemIntrinsic class hierarchy. These will act as a reminder to verify that inst combine handles these intrinsics properly once they have been added to that class hierarchy.

Diff Detail

Repository
rL LLVM

Event Timeline

dneilson created this revision.Jul 17 2017, 1:16 PM
dneilson edited the summary of this revision. (Show Details)Jul 17 2017, 1:29 PM
reames requested changes to this revision.Jul 17 2017, 1:30 PM

FYI, Daniel and I are talking about this one offline and are going to iterate quickly on testing style. You'll see a couple rounds of updated patches on this within the next few minutes.

This revision now requires changes to proceed.Jul 17 2017, 1:30 PM
dneilson updated this revision to Diff 106939.Jul 17 2017, 1:46 PM
dneilson edited edge metadata.
dneilson edited the summary of this revision. (Show Details)

Refactor tests to make intent clearer.

dneilson edited the summary of this revision. (Show Details)Jul 17 2017, 1:55 PM
dneilson updated this revision to Diff 106943.Jul 17 2017, 1:57 PM

Header comment was ambiguous about memcpy/memmove being element atomic forms.

reames accepted this revision.Jul 17 2017, 2:02 PM

LGTM w/minor comment optional

test/Transforms/InstCombine/element-atomic-memintrins.ll
10 ↗(On Diff #106943)

minor: I'd add "(yet)" to each of your comments.

20 ↗(On Diff #106943)

I mentioned to Daniel that instcombine also contains trivial DSE which should eventually trigger here, but given both cases are covered, I didn't ask for a separate test.

This revision is now accepted and ready to land.Jul 17 2017, 2:02 PM
This revision was automatically updated to reflect the committed changes.