erichkeane added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2197
+ const Expr *ProbArg = E->getArg(2);
+ ProbArg->EvaluateAsFloat(Probability, CGM.getContext());
+ llvm::Type *Ty = ConvertType(ProbArg->getType());
----------------
You likely need to assert on the return value (as Richard suggested).
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2202
+ // won't use it for anything.
+ // Note, we still IRGen ExpectedValue because it could have side-effects.
+ if (CGM.getCodeGenOpts().OptimizationLevel == 0)
----------------
I believe in EvaluateAsFloat you need to pass 'allow side effects', because by
default it does not.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204
+ if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+ return RValue::get(ArgValue);
+
----------------
Do you still need to evaluate this for side-effects even when called with O1?
I think this code only evaluates side-effects in O0 mode at the moment.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+ const Expr *ProbArg = TheCall->getArg(2);
+ if (ProbArg->isValueDependent())
+ return ExprError();
----------------
Why is value-dependent an error? The expression should be able to be a
template parameter. For example:
struct S {
static constexpr float value = 1.1;
};
template<typename T>
void f(bool b) {
__builtin_expect_with_probability(b, 1, T::value);
}
should work.
Additionally, in C++20(though not yet implemented in clang it seems):
template<float F>
void f(bool b) {
__builtin_expect_with_probability(b, 1, F);
}
Additionally, how about an integer type? It would seem that I should be able
ot do:
template<int I>
void f(bool b) {
__builtin_expect_with_probability(b, 1, I); // probability is obviously 0
or 1
}
================
Comment at: clang/lib/Sema/SemaChecking.cpp:1812
+ Context)) ||
+ !Eval.Val.isFloat()) {
+ Diag(ProbArg->getLocStart(), diag::err_probability_not_constant_float)
----------------
BTW, this check should be valid (ensuring its a float), since normal
conversions should happen as a part of the function call.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79830/new/
https://reviews.llvm.org/D79830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
