[PATCH] D98898: [SimplifyCFG] use profile metadata to refine merging branch conditions

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Can prof metadata and `MD_unpredictable` be mixed together?
What should we do here in presence of `MD_unpredictable`?



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:2846
 // and deduce a glue that we need to use to join branch's conditions
 // to arrive at the common destination.
 static Optional<std::pair<Instruction::BinaryOps, bool>>
----------------
... and check that it wouldn't be an obviously unprofitable thing to do as per the prof metadata.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:2861-2862
+        BranchProbability::getBranchProbability(TWeight, TWeight + FWeight);
+    Likely = BranchProbability::getBranchProbability(
+        LikelyBranchWeight, LikelyBranchWeight + UnlikelyBranchWeight);
+  }
----------------
And now that i actually start reviewing this, i see what was irking me.
While it seems like we should be using the weights from `LowerExpectIntrinsic`,
why is that the right threshold for profile-driven weights?

There's `TLI->getPredictableBranchThreshold()`,
shouldn't we use that?

Because the `LikelyBranchWeight`/`UnlikelyBranchWeight` appears to only be used in 
`CodeGenFunction.cpp` and `LowerExpectIntrinsic.cpp`, none of the transforms use them,
unlike `TLI->getPredictableBranchThreshold()`. Which to me looks like
that they are `LowerExpectIntrinsic`'s implementation detail
that is unintentionally overexposed.


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

https://reviews.llvm.org/D98898