[PATCH] D159336: Statically analyze likely and unlikely blocks based on metadata

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:128
+
+  // [[unlikely]] and builtin_expect assigns 1/2000 as cold branch probability.
+  auto ColdProb = BranchProbability(1, std::min(2000, ColdBranchProbDenom.getValue()));
----------------
Is there an internal option that determines the value used there that can be used in the code here? Looks like it is controlled by LikelyBranchWeight - can that be made a global option and referenced here so that they don't get out of sync?


================
Comment at: llvm/test/Transforms/HotColdSplit/split-static-profile.ll:4
+
+; Check that the unlikely branch is outlined.
+; int cold(const char*);
----------------
Add baz code here too?


================
Comment at: llvm/test/Transforms/HotColdSplit/split-static-profile.ll:26
+
+; CHECK: internal void @foo.cold.1() #[[ATTR0:[0-9]+]]
+; CHECK-NEXT: newFuncRoot
----------------
I think these cases should also be the same in the CHECK-PROB case - probably want a common check label to check the common cases between the two invocations, and a different check label just for the different parts.

Also, probably want to ensure that the hot calls/funcs are all above the cold calls/funcs - the checks below don't guarantee that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159336/new/

https://reviews.llvm.org/D159336