Page MenuHomePhabricator

[LoopIdiom] Transform loop containing memcpy into memmove
ClosedPublic

Authored by yurai007 on Jul 29 2021, 7:19 AM.

Details

Summary

The purpose of patch is to learn Loop idiom recognition pass how to recognize simple memmove patterns
in similar way like GCC does: https://godbolt.org/z/dKjGvTGff
It's follow-up of following change: https://reviews.llvm.org/D104464

Diff Detail

Event Timeline

yurai007 created this revision.Jul 29 2021, 7:19 AM
yurai007 requested review of this revision.Jul 29 2021, 7:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2021, 7:19 AM

Sorry for the delay. I've been occupied with other projects.

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1275–1279

Not sure I fully understand the FIXME. Why not drop the equals case?

1429

Nit: please fix lint.

1431

I feel the logic here is a bit hard to read. Whether it's memcpy or not, it both requires IsSameObject to be true. Maybe change this to

bool UseMemMove = IsMemCpy || LoopAccessStore;

And then move the IsSameObject check into MemmoveVerifier?

Thank you for looking into this change. Since I'm busy with other projects too I will address comments and update patch tomorrow morning.

yurai007 updated this revision to Diff 377499.Oct 6 2021, 4:30 AM
yurai007 marked 2 inline comments as done.

Rebased and addressed comments.

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1275–1279

Right, apparently I didn't think enough about that case so left fixme without good reason.

1431

I agree logic is not very straightforward here. Your idea would work except isMemcpy and !IsSameObject case (like in memcpy-intrinsic.ll).
For that case we just want to hoist body memcpy to big header memcpy without reaching MemmoveVerifier.
We could still move some checks there to make main logic more readable but then that wouldn't be "MemmoveVerifier" anymore I guess.

yurai007 marked an inline comment as done.Oct 6 2021, 4:34 AM
yurai007 added inline comments.
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1262

Added some cosmetic improvements - more consts, refs instead pointers etc.

zhuhan0 accepted this revision.Oct 7 2021, 9:34 AM

LGTM.

This revision is now accepted and ready to land.Oct 7 2021, 9:34 AM
This revision was automatically updated to reflect the committed changes.