[PGO] assignment operator does not get profile data

[davidxl]

Summary

For compiler generated assignment operator that is not trivial (calling base class operator=()), Clang FE assign region counters to the function body but does not emit profile counter increment for the function entry. This leads to many problems:

1. the operator body does not have profile data generated leading to warning in profile-use
2. the size of the function body may be large and lack of profile data may lead to wrong inlining decisions
3. when FE assign region counters to the function, it also emit coverage mapping data for the function -- but it has no coverage data which is confusing (currently the llvm-cov tool will report malformed format (as the name of the operator is not put into the right name section).

[davidxl]

Updated test case.

Another test case in profile-rt (pending) is : http://reviews.llvm.org/D16974

[vsk]

Thanks! This is definitely the right fix, since it fixes up implicitly-generated move constructors as well. However, I think the test can be simpler (while also checking that move constructors are handled correctly). E.g:

struct A {
  void operator=(const A &a) {}
  void operator=(const A &&a) {}
};
struct B {
  A a;
  B &operator=(const B &b) = default;
  B &operator=(B &&b) = default;
};
int main() {
  B b1, b2;
  b1 = b2;
  b2 = static_cast<B &&>(b1);
  return 0;
}

[davidxl]

Simplified test case suggested by Vedant.

[vsk]

Lgtm.

[dblaikie]

This looks like a change to clang - could you test it in clang (& review it
on cfe-commits instead of llvm-commits)?

[davidxl]

Further simplify tests according to David B's comment.

[dblaikie]

cfe/trunk/test/Profile/def-assignop.cpp

27 ↗

(On Diff #47351)

This doesn't need to be main or have an int return. Just make it a void function (with some generic name) & drop the "return 0" to keep it simple.

lib/CodeGen/CGClass.cpp

 
   LexicalScope Scope(*this, RootCS->getSourceRange());
 
+  incrementProfileCounter(RootCS);
   AssignmentMemcpyizer AM(*this, AssignOp, Args);
   for (auto *I : RootCS->body())
     AM.emitAssignment(I);

test/Profile/def-assignop.cpp

+// RUN: %clang_cc1 -x c++ -std=c++11 %s -triple x86_64-unknown-linux-gnu -main-file-name def-assignop.cpp -o - -emit-llvm -fprofile-instrument=clang | FileCheck --check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -x c++ -std=c++11 %s -triple x86_64-unknown-linux-gnu -main-file-name def-assignop.cpp -o - -emit-llvm -fprofile-instrument=clang -fcoverage-mapping | FileCheck --check-prefix=COVMAP %s
+
+struct B {
+  B& operator=(const B &b);
+  B& operator=(const B &&b);
+};
+
+struct A {
+  A &operator=(const A &) = default;
+  // PGOGEN: define {{.*}}@_ZN1AaSERKS_(
+  // PGOGEN: %pgocount = load {{.*}} @__profc__ZN1AaSERKS_
+  // PGOGEN: {{.*}}add{{.*}}%pgocount, 1
+  // PGOGEN: store{{.*}}@__profc__ZN1AaSERKS_
+  A &operator=(A &&) = default;
+  // PGOGEN: define {{.*}}@_ZN1AaSEOS_
+  // PGOGEN: %pgocount = load {{.*}} @__profc__ZN1AaSEOS_
+  // PGOGEN: {{.*}}add{{.*}}%pgocount, 1
+  // PGOGEN: store{{.*}}@__profc__ZN1AaSEOS_
+
+  // Check that coverage mapping includes 6 function records including the
+  // defaulted copy and move operators: A::operator=
+  // COVMAP: @__llvm_coverage_mapping = {{.*}} { { i32, i32, i32, i32 }, [3 x <{{.*}}>],
+  B b;
+};
+
+int main() {
+  A a1, a2;
+  a1 = a2;
+  a2 = static_cast<A &&>(a1);
+  return 0;
+}