Page MenuHomePhabricator

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

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

Diff Detail

Unit TestsFailed

3,940 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.AddFiles
Note: Google Test filter = DirectoryWatcherTest.AddFiles [==========] Running 1 test from 1 test case.
3,720 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.DeleteFile
Note: Google Test filter = DirectoryWatcherTest.DeleteFile [==========] Running 1 test from 1 test case.
3,730 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.ModifyFile
Note: Google Test filter = DirectoryWatcherTest.ModifyFile [==========] Running 1 test from 1 test case.

Event Timeline

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

LGTM, thanks!


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.Oct 16 2020, 1:44 AM
This revision was landed with ongoing or failed builds.Oct 16 2020, 2:38 AM
This revision was automatically updated to reflect the committed changes.
xbolva00 marked an inline comment as done.Oct 16 2020, 2:39 AM
fhahn added inline comments.Oct 21 2020, 2:35 AM

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.Oct 21 2020, 2:52 AM

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