[PATCH] D18013: Using MPI for Profiling Data Reduction

David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 15:39:13 PST 2016


davidxl added inline comments.

================
Comment at: lib/profile/InstrProfiling.h:16
@@ -15,1 +15,3 @@
 
+COMPILER_RT_VISIBILITY extern int __llvm_write_prof_data;
+
----------------
Make the name conform to the 'name space' convention:

--> __llvm_profile_write_data

================
Comment at: lib/profile/InstrProfilingMPI.c:43
@@ +42,3 @@
+/* Included here so regression test has a definition to use */
+COMPILER_RT_WEAK int MPI_Finalize() { return PMPI_Finalize(); }
+#endif
----------------
Why not put the stub def in the test case itself? 

#ifndef HAVE_MPI_H
...
#endif

================
Comment at: lib/profile/InstrProfilingStubs.c:14
@@ +13,3 @@
+
+COMPILER_RT_WEAK int PMPI_Finalize() { abort(); }
+COMPILER_RT_WEAK int PMPI_Comm_rank(void *comm, int *rank) { abort(); }
----------------
what are the issues of merging this into InstrProfilingMPI.c?

================
Comment at: test/profile/instrprof-reduce.c:16
@@ +15,3 @@
+int PMPI_Finalize() { return 0; }
+int MPI_Finalize();
+
----------------
With the stub definition for MPI_Finalize when HAVE_MPI_H is not defined, this test case will link, but the run will still fail -- so it will won't work.

I suggest adding the stub definition of MPI_Finalize like this:

... MPI_Finalize() {
     if (ThisRank != 0) {
      __llvm_write_prof_data = 0;
     }
    return 0;

}


http://reviews.llvm.org/D18013





More information about the llvm-commits mailing list