This is an archive of the discontinued LLVM Phabricator instance.

[MemCpyOpt] Teach memcpyopt to handle loads from the constant memory.
ClosedPublic

Authored by hliao on Aug 5 2021, 2:55 PM.

Details

Summary
  • Loads from the constant memory (either explicit one or as the source of memory transfer intrinsics) won't alias any stores.

Diff Detail

Event Timeline

hliao created this revision.Aug 5 2021, 2:55 PM
hliao requested review of this revision.Aug 5 2021, 2:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2021, 2:55 PM

Makes sense.

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
1571

Is it possible to write this using getModRefInfo()? Something like if (isModSet(AA->getModRefInfo(M, MemoryLocation::getForSource(M)))).

llvm/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
21

Please don't use undef here, or in the other tests. It might lead to confusing results in the future.

hliao updated this revision to Diff 364677.Aug 5 2021, 7:34 PM

Check isModSet instead.

hliao marked 2 inline comments as done.Aug 5 2021, 7:34 PM
hliao added inline comments.
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
1571

Good point!

hliao marked an inline comment as done.Aug 5 2021, 7:35 PM
efriedma accepted this revision.Aug 5 2021, 10:43 PM

LGTM

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
1176–1179

I think the original code was trying to do something slightly different... but I guess there isn't any test coverage? Not completely sure how the difference could matter, anyway; maybe we could do something funny with pointer alignment in certain edge cases.

This revision is now accepted and ready to land.Aug 5 2021, 10:43 PM
asbirlea accepted this revision.Aug 6 2021, 12:36 AM

lgtm.

hliao added inline comments.Aug 6 2021, 7:58 AM
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
1176–1179

not regression with all targets enabled, I will check any regression due to this change.