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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to