[PATCH] D41059: [PGO] MST min edge selection heuristic to ensure non-zero entry count

[Rong Xu via Phabricator via llvm-commits]

xur added a comment.

This is a good improvement.



================
Comment at: lib/Transforms/Instrumentation/CFGMST.h:183
+    uint64_t EntryInWeight = EntryWeight;
+    if (EntryInWeight < MaxExitOutWeight &&
+        MaxEntryOutWeight < MaxExitInWeight)
----------------
Will this ever be true? EntryInWeight should be always greater than or equal to weight of Exit BB. 


================
Comment at: lib/Transforms/Instrumentation/CFGMST.h:198
+    }
   }
 
----------------
Do we need to make sure EntryInWeight less than MaxEntryOutWeight? 

If Entry BB has more than one successors,  and Exit BB has more than one predecessors, 
even after adjustment, EntryIncoming->Weight might be greater than ExitOutgoing->Weight.

EntryIncoming ExitOutgoing and ExitIncoming will be in MST (thus not instrumented).
EntryOutgoing  will be instrumented. But since it has two successors, we will instrumented the target BB. 
(i.e. entryBB is not instrumented). 

But I'm sure  how this relates to the problem this patch wants to solve. It seems to me that choice b/w instrumenting entry and exit only applies the case of single successor/predecessor for entry and exit nodes. 


https://reviews.llvm.org/D41059