[PATCH] D18013: Using MPI for Profiling Data Reduction

Hal Finkel via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 9 17:07:32 PST 2016


hfinkel added inline comments.

================
Comment at: lib/profile/InstrProfilingReduce.c:30
@@ +29,3 @@
+              0, MPI_COMM_WORLD);
+   return PMPI_Finalize();
+}
----------------
davidxl wrote:
> Should return be used for Rank !=0 cases too?
It is used in both cases, however this is not visually obvious because the else statement has no braces.

Please add braces around the else body (for consistency with the if body).

================
Comment at: lib/profile/InstrProfilingStub.c:14
@@ +13,2 @@
+void MPI_Reduce() { abort(); }
+void PMPI_Finalize() { abort(); }
----------------
silvas wrote:
> davidxl wrote:
> > 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
> Why do we need these stubs?
I'm not sure defining these as weak solves the dependency-on-link-order problem because, except for PMPI_Finalize, they're weak in the actual MPI library too.

Also, to be more specific, we're depending on the fact that the linker won't use any symbols from this object file because nothing in this object file is required (when there is an MPI implementation that also provides these symbols). This might be a link order effect, or it could just be the fact that the MPI library is required to resolve other symbols, I'm not sure which.



http://reviews.llvm.org/D18013





More information about the llvm-commits mailing list