<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><br><hr id="zwchr"><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Xinliang David Li" <davidxl@google.com><br><b>To: </b>"Hal Finkel" <hfinkel@anl.gov><br><b>Cc: </b>"Daniel Waters" <dwatersg@gmail.com>, "Vedant Kumar" <vsk@apple.com>, "Eric Christopher" <echristo@gmail.com>, "Duncan P. N. Exon Smith" <dexonsmith@apple.com>, "Sean Silva" <chisophugis@gmail.com>, "llvm-commits" <llvm-commits@lists.llvm.org>, reviews+D18013+public+36299b239a77f675@reviews.llvm.org<br><b>Sent: </b>Wednesday, March 9, 2016 6:59:39 PM<br><b>Subject: </b>Re: [PATCH] D18013: Using MPI for Profiling Data Reduction<br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 9, 2016 at 4:47 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div style="font-family: arial,helvetica,sans-serif; font-size: 10pt; color: rgb(0, 0, 0);"><br><br><hr><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Xinliang David Li" <<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>><br><b>To: </b><a href="mailto:reviews%2BD18013%2Bpublic%2B36299b239a77f675@reviews.llvm.org" target="_blank">reviews+D18013+public+36299b239a77f675@reviews.llvm.org</a><br><b>Cc: </b>"Daniel Waters" <<a href="mailto:dwatersg@gmail.com" target="_blank">dwatersg@gmail.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>, "Vedant Kumar" <<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>>, "Eric Christopher" <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>>, "Duncan P. N. Exon Smith" <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>>, "Sean Silva" <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>>, "llvm-commits" <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br><b>Sent: </b>Wednesday, March 9, 2016 6:41:45 PM<br><b>Subject: </b>Re: [PATCH] D18013: Using MPI for Profiling Data Reduction<div><div class="h5"><br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 9, 2016 at 4:35 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">silvas added a subscriber: silvas.<br>
<br>
================<br>
Comment at: lib/profile/InstrProfilingFile.c:86<br>
@@ -74,2 +85,3 @@<br>
const char *Filename;<br>
+#ifdef _MSC_VER<br>
FILE *File;<br>
----------------<br>
Let's split this change into a separate patch for a focused discussion on the requirements and possible solutions.<br>
<br>
================<br>
Comment at: lib/profile/InstrProfilingReduce.c:1<br>
@@ +1,2 @@<br>
+/*===- InstrProfilingReduce.c - Support library for applications using MPI ===*\<br>
+|*<br>
----------------<br>
Let's call this InstrProfilingMPI.c consistent with the comment.<br>
<span><br>
================<br>
Comment at: lib/profile/InstrProfilingStub.c:14<br>
@@ +13,2 @@<br>
+void MPI_Reduce() { abort(); }<br>
+void PMPI_Finalize() { abort(); }<br>
----------------<br>
</span><span>davidxl wrote:<br>
> Is the MPI library defining thse functions static or a DSO? If the former, then perhaps declare those stubs weak so that we don't rely on the link order of the runtime library<br>
</span>Why do we need these stubs?<br>
<br></blockquote><div><br></div><div>I thought it is needed when the binary is not linked with mpi library, but InstrProfilingReduce.o is linked in. Now that you asked, it seems that for that case, InstrProfilingReduce.o should not be linked in at all.</div><div><br></div><div>I think the culprit is that __llvm_write_profile_data flag is defined in that file. With my suggestion to move it to InstrProfiling.c, I think there is no need for the stub file. Good catch.</div></div></div></div></div></div></blockquote>It is __llvm_write_profile_data, but that's on purpose. The problem is that we need a hard dependency on InstrProfilingReduce.o in order to force the linker to bring it in. Otherwise, because there is already a weak definition of MPI_Finalize in the MPI library, the linker would not bring in InstrProfilingReduce.o at all and our MPI_Finalize implementation would never be called.<br><br>You're correct that we then need the stubs to make linking succeed when we're not actually linking in an MPI library (e.g. for every regression test).<br><br></div></div></blockquote><div><br></div><div id="DWT14314">ok. Can those stubs be declared weak and merged into InstrProfilingReduce.c ?</div></div></div></div></blockquote>I don't think so because the MPI functions in the actual MPI library themselves are weak and the weak vs. weak resolving is likely ill-defined.<br><br>One option might be that, in our MPI_Finalize implementation, we *only* use the "strong" PMPI_* variants. Then having weak stubs might work. What do you think?<br><br> -Hal<br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div style="font-family: arial,helvetica,sans-serif; font-size: 10pt; color: rgb(0, 0, 0);">Thanks again,<br>Hal<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div><br></div><div>David</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><br>
<a href="http://reviews.llvm.org/D18013" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18013</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div><span class="HOEnZb"><font color="#888888">
</font></span></blockquote><span class="HOEnZb"><font color="#888888"><br><br><br>-- <br><div><span></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span></span><br></div></font></span></div></div></blockquote></div><br></div></div>
</blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></body></html>