[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

> (IEEEdouble, IEEEdouble) -> (uint64_t, uint64_t) -> PPCDoubleDoubleImpl, then run the old algorithm.

We need to document in the code what is going on here and why it works. Just looking at a bunch of code like this:

  if (Semantics == &semPPCDoubleDouble) {
    APFloat Tmp(semPPCDoubleDoubleImpl, bitcastToAPInt());
    auto Ret = Tmp.next(nextDown);
    *this = DoubleAPFloat(semPPCDoubleDouble, Tmp.bitcastToAPInt());
    return Ret;
  }

it is not at all obvious what is going on (i.e. why are we casting to integers?). Maybe semPPCDoubleDoubleImpl needs a better name now? It seems confusing to have semPPCDoubleDoubleImpl and semPPCDoubleDouble where one is not really an "implementation" class of the other.



================
Comment at: llvm/include/llvm/ADT/APFloat.h:791
   void makeNaN(bool SNaN, bool Neg, const APInt *fill) {
-    getIEEE().makeNaN(SNaN, Neg, fill);
+    if (usesLayout<IEEEFloat>(getSemantics()))
+      return U.IEEE.makeNaN(SNaN, Neg, fill);
----------------
I realize that some of this existed before, but is there any way to refactor this so that we don't have so much boiler plate? Even using a utility macro is probably better than this.


https://reviews.llvm.org/D27872