This is an archive of the discontinued LLVM Phabricator instance.

[MemCpyOpt] Turn memcpy from just-memset'd source into memset.
ClosedPublic

Authored by ab on May 11 2015, 2:49 PM.

Details

Summary

There's no point in copying around constants, so, when all else fails, I think we can still transform memcpy of memset into two independent memsets. To quote the example, we can turn:

memset(dst1, c, dst1_size);
memcpy(dst2, dst1, dst2_size);

into:

memset(dst1, c, dst1_size);
memset(dst2, c, dst2_size);

When dst2_size <= dst1_size.

Like D498 for copy constructors, I think this pattern can occur in move constructors.

Diff Detail

Repository
rL LLVM

Event Timeline

ab updated this revision to Diff 25513.May 11 2015, 2:49 PM
ab retitled this revision from to [MemCpyOpt] Turn memcpy from just-memset'd source into memset..
ab updated this object.
ab edited the test plan for this revision. (Show Details)
ab added reviewers: jmolloy, dberlin.
ab set the repository for this revision to rL LLVM.
ab added a subscriber: Unknown Object (MLST).
dberlin added inline comments.May 12 2015, 12:54 PM
lib/Transforms/Scalar/MemCpyOptimizer.cpp
931 ↗(On Diff #25513)

So, this looks like a valid optimization, and in fact, i expect things like GVN would like this form better than the original.

But I am curious how often it actually happens.

Do you have real programs where this triggers?

ab added inline comments.May 12 2015, 2:49 PM
lib/Transforms/Scalar/MemCpyOptimizer.cpp
931 ↗(On Diff #25513)

From clang, I don't think it happens very often.

I saw this on C++ move constructors implemented in terms of std::swap. For instance:

#include <string>
#include <utility>

struct A {
  std::string a;
  A(A &&other);
};

A::A(A &&other)  {
  std::swap(a, other.a);
}

Which I think is a controversial idiom in the C++ world, but still gets written ;)
It's also easy to have an artificial testcase:

void bar(int *);

struct B {
  int b[100];
  B() : b() {}
};

void foo() {
  B x;
  B y = x;
  bar(x.b);
  bar(y.b);
}

But I can't say I've seen that one (not that I looked; perhaps I should).

dberlin edited edge metadata.May 14 2015, 1:46 PM

SGTM.
It is almost zero cost to check and optimize this case, so if it
actually happens, i'm fine with it.

I did get around to testing, and it's definitely the case that GVN is
much happier with them as memsets. It can often pull the constant
value directly out of the memset, whereas when they are memcpy, it
often just ends up with a use of a load or a load widening.

This revision was automatically updated to reflect the committed changes.
ab added a comment.May 15 2015, 6:37 PM

Thanks for looking into it Daniel!

r237506. FWIW, I realized that std::swap move constructors might me more frequent than I thought, since I don't think there's another way to move a class for which no move constructors was provided (if you can't change it, but have an std::swap specialisation).

-Ahmed