On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote:
> davidxl updated this revision to Diff 47217. > davidxl added a comment. > > Simplified test case suggested by Vedant. > > > http://reviews.llvm.org/D16947 > > Files: > lib/CodeGen/CGClass.cpp > test/Profile/def-assignop.cpp > > Index: test/Profile/def-assignop.cpp > =================================================================== > --- test/Profile/def-assignop.cpp > +++ test/Profile/def-assignop.cpp > @@ -0,0 +1,32 @@ > +// 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 { > + void operator=(const B &b) {} > + void operator=(const B &&b) {} > Probably best to make these canonical to avoid confusion: B &operator=(const B&); B &operator=(B&&); (& they don't need definitions - just declarations) Also, neither of these are the move /constructor/, just the move operator. Not sure if Vedant just used the wrong terminology, or whether it's worth testing the move/copy ctors too, to check that they do the right thing as well. (if all of these things use the same codepath, I don't see a great benefit in having separate tests for them (but you can add them here if you like) - I'm just suggesting a manual verification in case those need a separate fix) > +}; > + > +struct A { > + A &operator=(const A &) = default; > Is the fix/codepath specifically about explicitly defaulted ops? Or just any compiler-generated ones? (you could drop these lines if it's about any compiler-generated ones, might be simpler/more obvious that it's not about the "= default" feature) > + // 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 }, > [5 x <{{.*}}>], > + B b; > +}; > + > +int main() { > + A a1, a2; > + a1 = a2; > + a2 = static_cast<A &&>(a1); > An option, though not necessarily better, would be to just take the address of the special members: auto (B::*x)(const B&) = &bar::operator=; auto (B::*x)(B&&) = &bar::operator=; In short, what I'm picturing, in total: struct A { A &operator=(const A&); A &operator=(A&&); }; struct B { A a; }; auto (B::*x)(const B&) = &B::operator=; auto (B::*x)(B&&) = &B::operator=; Also, this test should probably be in clang, since it's a clang code change/fix. > + return 0; > +} > Index: lib/CodeGen/CGClass.cpp > =================================================================== > --- lib/CodeGen/CGClass.cpp > +++ lib/CodeGen/CGClass.cpp > @@ -1608,6 +1608,7 @@ > > LexicalScope Scope(*this, RootCS->getSourceRange()); > > + incrementProfileCounter(RootCS); > AssignmentMemcpyizer AM(*this, AssignOp, Args); > for (auto *I : RootCS->body()) > AM.emitAssignment(I); > > > > _______________________________________________ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
