[PATCH] D48538: Make __gcov_flush flush counters for all shared libraries

David Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 3 10:14:47 PDT 2018


davidxl added inline comments.


================
Comment at: lib/profile/GCDAProfiling.c:116
+    prev = list->head;
+    while (prev->next != element);
+    prev->next = element->next;
----------------
this loop is still wrong.

Also it is possible to simply the code by removing the if-then-else:

    struct fn_node** prev = &list->head;
     while ((*prev) != element) 
          prev = &(*prev)->next;
     (*prev) = element->next;


================
Comment at: lib/profile/GCDAProfiling.c:537
+  while (curr) {
+    fn_list_remove(&writeout_fn_list_current, curr);
+    fn_list_remove(&writeout_fn_list_shared, curr);
----------------
It suffices to do this outside the loop by making current list's head to null.


================
Comment at: lib/profile/GCDAProfiling.c:538
+    fn_list_remove(&writeout_fn_list_current, curr);
+    fn_list_remove(&writeout_fn_list_shared, curr);
+    curr = curr->next;
----------------
The second one has quadratic behavior.

If the current list is remembered in the fn_node object, you can simply do a one sweep by comparing the recorded list with the current list.




================
Comment at: test/profile/Inputs/instrprof-dlopen-func.c.gcov:6
+// CHECK-NEXT:        -:    0:Programs:1
+// CHECK-NEXT:        3:    1:void func(int K) { if (K) {} }
----------------
marco-c wrote:
> davidxl wrote:
> > this is not correct. func is executed only once.
> I think this is pre-existing behavior unfortunately, I'll test again without the patch.
Have you examined why this is wrong? If it is not regression, it is worth fixing as a follow up.


https://reviews.llvm.org/D48538





More information about the llvm-commits mailing list