rsmith added a comment.
Looking specifically for attributes in the 'then' and 'else' cases of an `if`
seems like a fine first pass at this, but I think this is the wrong way to
lower these attributes in the longer term: we should have a uniform treatment
of them that looks for the most recent prior branch and annotates it with
weights. We could do that either by generating LLVM intrinsic calls that an
LLVM pass lowers, or by having the frontend look backwards for the most recent
branch and annotate it. I suppose it's instructive to consider a case such as:
void mainloop() noexcept; // probably terminates by calling `exit`
void f() {
mainloop();
[[unlikely]];
somethingelse();
}
... where the `[[unlikely]];` should probably cause us to split the
`somethingelse()` out into a separate basic block that we mark as cold, but
should not cause `f()` itself or its call to `mainloop()` to be considered
unlikely -- except in the case where `mainloop()` can be proven to always
terminate, in which case the implication is that it's unlikely that `f()` is
invoked. Cases like this probably need the LLVM intrinsic approach to model
them properly.
================
Comment at: clang/include/clang/AST/Stmt.h:175-178
+ /// The likelihood of taking the ThenStmt.
+ /// One of the enumeration values in Stmt::Likelihood.
+ unsigned ThenLikelihood : 2;
+
----------------
Do we need to store this here? The "then" and "else" cases should be an
`AttributedStmt` holding the likelihood attribute, so duplicating that info
here seems redundant.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:578
+static std::pair<Stmt::Likelihood, const Attr *>
+getLikelihood(const Stmt *Stmt) {
+ if (const auto *AS = dyn_cast<AttributedStmt>(Stmt))
----------------
Sema doesn't care about any of this; can you move this code to CodeGen and
remove the code that stores likelihood data in the AST?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85091/new/
https://reviews.llvm.org/D85091
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
