This is an archive of the discontinued LLVM Phabricator instance.

[x86, CGP] reduce memcmp() expansion to 2 load pairs (PR33914)
ClosedPublic

Authored by RKSimon on Jul 25 2017, 3:46 AM.

Details

Summary

D35067/rL308322 attempted to support up to 4 load pairs for memcmp inlining which resulted in some regressions for some optimized libc memcmp implementations (PR33914).

Until we can match these more optimal cases, this patch reduces the memcmp() expansion to a maximum of 2 load pairs (which matches what we do for -Os).

This patch should be considered for the 5.0.0 release branch as well

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Jul 25 2017, 3:46 AM
zvi edited edge metadata.Jul 25 2017, 5:42 AM

Thanks for the timely fix, Simon. Can you please wait another day or two until we get this tested?

In D35830#819967, @zvi wrote:

Thanks for the timely fix, Simon. Can you please wait another day or two until we get this tested?

No problem, the only time constraint is the clang 5 release schedule.

zvi accepted this revision.Jul 25 2017, 8:10 AM

LGTM. Just got word this patch fixes the regression in our internal benchmark.
Would be great if this patch will be merged to the release branch.
Thanks!

This revision is now accepted and ready to land.Jul 25 2017, 8:10 AM
This revision was automatically updated to reflect the committed changes.