Mordante marked 2 inline comments as done.
Mordante added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1622
EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock,
- getProfileCount(CondOp), Weights);
+ getProfileCount(CondOp), Stmt::LH_None);
----------------
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.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1678-1679
// Create branch weights based on the number of times we get here and the
// number of times the condition should be true.
+ llvm::MDNode *Weights = nullptr;
----------------
aaron.ballman wrote:
> This comment is now a bit stale.
Agreed, I'll remove it.
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
