[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM modulo comment nits.



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1622
     EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock,
-                         getProfileCount(CondOp), Weights);
+                         getProfileCount(CondOp), Stmt::LH_None);
 
----------------
Mordante wrote:
> aaron.ballman wrote:
> > Why `LH_None` in this case? (Also, given that it's the default value, should you skip passing this at all? Or should the parameter not have a default value, perhaps?)
> This should be `LH_None` since this is the likelihood of `C ? T : F` and there's no likelihood for this expression. When used in `if(C ? T : F)` the if can have an likelihood attribute for the `if`. That's why the other two calls in this block have a likelihood. I'll add comment to make it clear why this is correct. I felt using the explicit value here instead of the defaulted value was clearer. This would be the only call in this function not to explicitly use a value.
> 
> Since `EmitBranchOnBoolExpr` is also used at other places I prefer to keep the default argument.
Thank you for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88363