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

[Tony Jiang via Phabricator via llvm-commits]

jtony added inline comments.


================
Comment at: llvm/include/llvm/ADT/APFloat.h:800
 
-  void makeLargest(bool Neg) { getIEEE().makeLargest(Neg); }
+  void makeLargest(bool Neg) {
+    if (usesLayout<IEEEFloat>(getSemantics())) {
----------------
I know it is allowed to return a void function call inside a void function, but I think this reduces the code readability in general and causes some confusing to some people. Maybe it is better to avoid using this kind of coding style. I think we can simply call each function in each branch without the 'return' keyword, by default, the program will reach the end of function and return.  

One possible equivalent code:

void makeNaN(bool SNaN, bool Neg, const APInt *fill) {
    if (usesLayout<IEEEFloat>(getSemantics())) {
      U.IEEE.makeNaN(SNaN, Neg, fill);
    } else if (usesLayout<DoubleAPFloat>(getSemantics())) {
      U.Double.makeNaN(SNaN, Neg, fill);
    } else {
      llvm_unreachable("Unexpected semantics");
    }
  }


================
Comment at: llvm/include/llvm/ADT/APFloat.h:811
+  void makeSmallest(bool Neg) {
+    if (usesLayout<IEEEFloat>(getSemantics())) {
+      return U.IEEE.makeSmallest(Neg);
----------------
Same here.


================
Comment at: llvm/include/llvm/ADT/APFloat.h:821
   void makeSmallestNormalized(bool Neg) {
-    getIEEE().makeSmallestNormalized(Neg);
+    if (usesLayout<IEEEFloat>(getSemantics())) {
+      return U.IEEE.makeSmallestNormalized(Neg);
----------------
Same here.


================
Comment at: llvm/include/llvm/ADT/APFloat.h:1100
 
-  void changeSign() { getIEEE().changeSign(); }
-  void clearSign() { getIEEE().clearSign(); }
-  void copySign(const APFloat &RHS) { getIEEE().copySign(RHS.getIEEE()); }
+  void changeSign() {
+    if (usesLayout<IEEEFloat>(getSemantics())) {
----------------
Same here.


https://reviews.llvm.org/D27872