<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">davidxl updated this revision to Diff 47217.<br>
davidxl added a comment.<br>
<br>
Simplified test case suggested by Vedant.<br>
<span class=""><br>
<br>
<a href="http://reviews.llvm.org/D16947" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16947</a><br>
<br>
Files:<br>
lib/CodeGen/CGClass.cpp<br>
test/Profile/def-assignop.cpp<br>
<br>
Index: test/Profile/def-assignop.cpp<br>
===================================================================<br>
--- test/Profile/def-assignop.cpp<br>
+++ test/Profile/def-assignop.cpp<br>
</span>@@ -0,0 +1,32 @@<br>
<span class="">+// 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<br>
+// 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<br>
+<br>
</span>+struct B {<br>
+ void operator=(const B &b) {}<br>
+ void operator=(const B &&b) {}<br></blockquote><div><br></div><div>Probably best to make these canonical to avoid confusion:<br><br>B &operator=(const B&);<br>B &operator=(B&&);<br><br>(& they don't need definitions - just declarations)</div><div><br>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)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+};<br>
+<br>
+struct A {<br>
<span class="">+ A &operator=(const A &) = default;<br></span></blockquote><div><br></div><div>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)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">
</span><span class="">+ // PGOGEN: define {{.*}}@_ZN1AaSERKS_(<br>
</span>+ // PGOGEN: %pgocount = load {{.*}} @__profc__ZN1AaSERKS_<br>
+ // PGOGEN: {{.*}}add{{.*}}%pgocount, 1<br>
+ // PGOGEN: store{{.*}}@__profc__ZN1AaSERKS_<br>
+ A &operator=(A &&) = default; </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ // PGOGEN: define {{.*}}@_ZN1AaSEOS_<br>
+ // PGOGEN: %pgocount = load {{.*}} @__profc__ZN1AaSEOS_<br>
<span class="">+ // PGOGEN: {{.*}}add{{.*}}%pgocount, 1<br>
</span>+ // PGOGEN: store{{.*}}@__profc__ZN1AaSEOS_<br>
<span class="">+<br>
+ // Check that coverage mapping includes 6 function records including the<br>
</span>+ // defaulted copy and move operators: A::operator=<br>
+ // COVMAP: @__llvm_coverage_mapping = {{.*}} { { i32, i32, i32, i32 }, [5 x <{{.*}}>],<br>
+ B b;<br>
+};<br>
+<br>
+int main() {<br>
+ A a1, a2;<br>
+ a1 = a2;<br>
+ a2 = static_cast<A &&>(a1);<br></blockquote><div><br>An option, though not necessarily better, would be to just take the address of the special members:<br><br>auto (B::*x)(const B&) = &bar::operator=;<br>auto (B::*x)(B&&) = &bar::operator=;</div><div><br></div><div>In short, what I'm picturing, in total:<br><br>struct A {<br> A &operator=(const A&);<br> A &operator=(A&&);<br>};<br><br>struct B {<br> A a;<br>};<br><br>auto (B::*x)(const B&) = &B::operator=;<br>auto (B::*x)(B&&) = &B::operator=;<br><br>Also, this test should probably be in clang, since it's a clang code change/fix.<br><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5">+ return 0;<br>
+}<br>
Index: lib/CodeGen/CGClass.cpp<br>
===================================================================<br>
--- lib/CodeGen/CGClass.cpp<br>
+++ lib/CodeGen/CGClass.cpp<br>
@@ -1608,6 +1608,7 @@<br>
<br>
LexicalScope Scope(*this, RootCS->getSourceRange());<br>
<br>
+ incrementProfileCounter(RootCS);<br>
AssignmentMemcpyizer AM(*this, AssignOp, Args);<br>
for (auto *I : RootCS->body())<br>
AM.emitAssignment(I);<br>
<br>
<br>
</div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>