[PATCH] D68257: [Support] Add mathematical constants

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4998
+        DAG.getNode(ISD::FMUL, dl, MVT::f32, Exp,
+                    DAG.getConstantFP(numbers::ln2, dl, MVT::f32));
 
----------------
evandro wrote:
> efriedma wrote:
> > I don't like taking a 64-bit constant and truncating it to 32 bits.  For most common mathematical constants, it probably produces the right result, but it's not correctly rounded in general.
> Suggestions?
You can have two constants: ln2 and ln2f, which are double and float respectively.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:1943
     if (ArgLb == ExpLb || ArgID == Intrinsic::exp)
-      // FIXME: The Euler number should be M_E, but it's place of definition
-      // is not quite standard.
-      Eul = ConstantFP::get(Log->getType(), 2.7182818284590452354);
+      Eul = ConstantFP::get(Log->getType(), numbers::e);
     else if (ArgLb == Exp2Lb || ArgID == Intrinsic::exp2)
----------------
evandro wrote:
> efriedma wrote:
> > Do we actually have enough precision here if the type is long double?
> The method is defined as `static Constant *get(Type* Ty, double V)`.  Likewise, the POSIX constant `M_E` is double precision only.
I guess this isn't making the existing problem worse, but please leave a FIXME, at least.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68257/new/

https://reviews.llvm.org/D68257