[PATCH] D15258: [PGO] Remove data races on Data->Values field

David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 17 15:46:13 PST 2015


davidxl added inline comments.

================
Comment at: lib/profile/InstrProfilingValue.c:159
@@ -184,4 +158,3 @@
   for (I = (__llvm_profile_data *)DataBegin; I != DataEnd; ++I) {
-
     DEF_VALUE_RECORD(R, I->NumValueSites, I->Values);
 
----------------
betulb wrote:
> As an overall comment, I'm not too fond of using macros this heavily. Specific to this line, DEF_VALUE_RECORD is used only once and there is a macro definition for its contents. It's not necessary. Secondly, R is passed to the macro w/o being declared in this context. I'd rather have preferred that R is declared first and then passed to the macro (if macro usage is ever a requisite).
yes, after the cleanup, this one can be removed -- as it is not reused anywhere else.

================
Comment at: lib/profile/InstrProfilingWriter.c:76
@@ +75,3 @@
+          {ValueDataArray[I], sizeof(char), ValueDataArray[I]->TotalSize}};
+      if (Writer(IOVec2, sizeof(IOVec2) / sizeof(*IOVec2), &WriterCtx))
+        return -1;
----------------
betulb wrote:
> I think there is an overhead of making the syscall to write as many times as there are profile data variables. I think it was one of the reasons why the other profiling constants/variables were merged into a single global variable and into linker sections to reduce the many iterations needed to write the data.
We can do the write in batches of course. I will do some measurement to see if that is needed.


http://reviews.llvm.org/D15258





More information about the llvm-commits mailing list