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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
