This is an archive of the discontinued LLVM Phabricator instance.

[DSE] Handle memcpy/memset with equal non-const sizes.
ClosedPublic

Authored by fhahn on Mar 9 2021, 12:43 PM.

Details

Summary

Currently DSE misses cases where the size is a non-const IR value, even
if they match. For example, this means that llvm.memcpy/llvm.memset
calls are not eliminated, even if they write the same number of bytes.

This patch extends isOverwite to try to get IR values for the number of
bytes written from the analyzed instructions. If the values match,
alias checks are performed and the result is returned.

At the moment this only covers llvm.memcpy/llvm.memset. In the future,
we may enable MemoryLocation to also track variable sizes, but this
simple approach should allow us to cover the important cases in DSE.

Diff Detail

Event Timeline

fhahn created this revision.Mar 9 2021, 12:43 PM
fhahn requested review of this revision.Mar 9 2021, 12:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2021, 12:43 PM

Can you add test like:

llvm.memcpy(dst, src, len)
llvm.memset(dst, 0, len)

?

asbirlea accepted this revision.Mar 9 2021, 2:03 PM

Changes looks good.
+1 on the additional testcase.

This revision is now accepted and ready to land.Mar 9 2021, 2:03 PM
nikic added inline comments.Mar 9 2021, 2:29 PM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
393

I'd directly call AA.isMustAlias(Ealier, Later) here and forego the manual pointer cast stripping.

llvm/test/Transforms/DeadStoreElimination/memory-intrinsics-sizes.ll
47

Is it actually necessary that the parameters are noalias? This looks valid to me even if src and dst alias.

This revision was landed with ongoing or failed builds.Mar 10 2021, 2:14 AM
This revision was automatically updated to reflect the committed changes.
fhahn added inline comments.Mar 10 2021, 6:37 AM
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
393

Good point, I initially kept it symmetric with the code below, but it's not really needed. I adjusted that in the committed version