Page MenuHomePhabricator

[loop-idiom] Support memcpy instructions for memmove-like loops
AbandonedPublic

Authored by zhuhan0 on Aug 19 2021, 12:24 AM.

Details

Summary

Following https://reviews.llvm.org/D104464. In addition to load and store
instructions, also handle memcpy intrinsics in loop body. The constraints are
same as before (same base pointer, etc.).

Fixed the FIXME added in D104464, and added two more tests.

Diff Detail

Event Timeline

zhuhan0 created this revision.Aug 19 2021, 12:24 AM
zhuhan0 requested review of this revision.Aug 19 2021, 12:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2021, 12:24 AM

@zhuhan0: Thanks for following up my memmove change! Actually there is analogous follow up but it didn't catch lot of attention: https://reviews.llvm.org/D107075.
Maybe you can reuse some parts of that change. I feel you could at least copy most unit tests from there for better coverage.
And last but not least - there is one ongoing small but important fix for https://reviews.llvm.org/D104464 which can be found here: https://reviews.llvm.org/D107964
It would be great if you and @hoy could take a look since your changes depends on it.

yurai007 added inline comments.Aug 19 2021, 7:12 AM
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
1356

IIRC I think I had 'kind of' similar idea to use 4th mayLoopAccessLocation but since it seems expensive I ended up in 3 calls: https://reviews.llvm.org/D107075 I admit I didn't measure impact on compile time. Maybe it's not so bad.

zhuhan0 added a comment.EditedAug 20 2021, 5:01 PM

@zhuhan0: Thanks for following up my memmove change! Actually there is analogous follow up but it didn't catch lot of attention: https://reviews.llvm.org/D107075.
Maybe you can reuse some parts of that change. I feel you could at least copy most unit tests from there for better coverage.
And last but not least - there is one ongoing small but important fix for https://reviews.llvm.org/D104464 which can be found here: https://reviews.llvm.org/D107964
It would be great if you and @hoy could take a look since your changes depends on it.

Oops I didn't know you had a follow-up patch already! Is it really okay for me to copy your diff? I can also review your diff and land that. Which do you prefer?

Oops I didn't know you had a follow-up patch already! Is it really okay for me to copy your diff? I can also review your diff and land that. Which do you prefer?

Well, it's up to you :) I don't have problem with abandoning https://reviews.llvm.org/D107075. But if you see there are benefits in (re)using it then go ahead.