Page MenuHomePhabricator

[MemLoc] Support memchr/memccpy in MemoryLocation::getForArgument
ClosedPublic

Authored by xbolva00 on Tue, Oct 13, 8:29 AM.

Diff Detail

Event Timeline

xbolva00 created this revision.Tue, Oct 13, 8:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Oct 13, 8:29 AM
xbolva00 requested review of this revision.Tue, Oct 13, 8:29 AM
fhahn accepted this revision.Fri, Oct 16, 1:44 AM

LGTM, thanks!

llvm/test/Transforms/DeadStoreElimination/MSSA/libcalls.ll
277

nit: noalias should not be needed here and the other tests, as %stack.ptr is to an alloca and LLVM should be able to determine noalias without the extra help.

This revision is now accepted and ready to land.Fri, Oct 16, 1:44 AM
This revision was landed with ongoing or failed builds.Fri, Oct 16, 2:38 AM
This revision was automatically updated to reflect the committed changes.
xbolva00 marked an inline comment as done.Fri, Oct 16, 2:39 AM
fhahn added inline comments.Wed, Oct 21, 2:35 AM
llvm/lib/Analysis/MemoryLocation.cpp
262

Thinking about this again, I am not sure if this is a good idea to do for the destination argument, because it gives the impression that exactly LenCI bytes are written, when this is not the case.

I think for reads the larger size is the conservative option and fine, but for writing it could mis-lead clients into thinking the call writes more bytes than it actually does, leading to potentially invalid optimizations.

xbolva00 added inline comments.Wed, Oct 21, 2:52 AM
llvm/lib/Analysis/MemoryLocation.cpp
262

Makes sense, I will prepare fix to skip doing this for dst.