This is an archive of the discontinued LLVM Phabricator instance.

[DSE] Teach the pass about partial overwrite of atomic memory intrinsics
ClosedPublic

Authored by dneilson on Apr 12 2018, 12:09 PM.

Details

Summary

This change teaches DSE that the atomic memory intrinsics can be overwriten
partially in the same way as the non-atomic forms. Specifically, that the
atomic memcpy & memset can be shortened at the end and that the atomic memset
can be shortened at the beginning, if they partially overwritten
by later stores.

Diff Detail

Repository
rL LLVM

Event Timeline

dneilson created this revision.Apr 12 2018, 12:09 PM

ping - any reviewers able to take a crack at this?

I don't see any problems with the code (other than the nit I've pointed out), but I do not know this logic well enough to make the final judgement. Adding some people who contributed to DSE recently. Can you guys take a look?

lib/Transforms/Scalar/DeadStoreElimination.cpp
917 ↗(On Diff #142230)

There are already existing functions for this, like isPowerOf2_32, can you please use one of them?

dneilson added inline comments.May 3 2018, 11:57 AM
lib/Transforms/Scalar/DeadStoreElimination.cpp
917 ↗(On Diff #142230)

Perhaps a misleading comment. The line below this is checking whether NewLength is an integer multiple of ElementSize (i.e. whether NewLength % ElementSize == 0), but I did it via a bit operation because we know that ElementSize must be a power of 2 (via the spec for the atomic memory intrinsics).

I'll just use the modulo operator; less potential for confusion.

dneilson updated this revision to Diff 145069.May 3 2018, 12:20 PM
  • Use mod operator instead of bit twiddling

ping - any takers to review this? I'm fairly certain it's correct, but a sanity check would be nice.

efriedma accepted this revision.May 9 2018, 3:16 PM

LGTM

This revision is now accepted and ready to land.May 9 2018, 3:16 PM
This revision was automatically updated to reflect the committed changes.