[PATCH] D70256: [FPEnv] clang support for constrained FP builtins
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 18 13:26:07 PST 2019
rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:357
+
+ if (ConstrainedIntrinsicID != Intrinsic::not_intrinsic &&
+ CGF.Builder.getIsFPConstrained()) {
----------------
kpn wrote:
> rjmccall wrote:
> > I don't see any situation where `not_intrinsic` is passed in here; why all these checks?
> I tend to be conservative, and I'm not good at predicting the future. I can remove the checks easily in the next round.
I just can't imagine why you would ever call this function without a constrained intrinsic ID when its only purpose is to choose between a constrained and unconstrained intrinsic. It's always better to start with stronger preconditions and then generalize later if necessary.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2302
+ return RValue::get(Builder.CreateCall(F, {Base, Exponent}));
+ }
}
----------------
kpn wrote:
> rjmccall wrote:
> > What can this not use the standard path?
> I'll take another look. It wasn't using the standard path before so I made my added code match. If it can use the standard path in both cases then maybe it should?
Yeah. It's possible the type-heterogeneity (the power always being an `int`) stops that from working for some reason, but if not, it's always nicer to re-use more code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70256/new/
https://reviews.llvm.org/D70256
More information about the cfe-commits
mailing list