[trafodion] branch master updated: [TRAFODION-3284] Make overflow processing for float SUM consistent

[db...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dbirdsall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafodion.git


The following commit(s) were added to refs/heads/master by this push:
     new ef9d56a  [TRAFODION-3284] Make overflow processing for float SUM consistent
     new 5d85d08  Merge pull request #1812 from DaveBirdsall/FloatingPointOverflows
ef9d56a is described below

commit ef9d56a7e929189bf22ca2c4e0072f8f3f5145c1
Author: Dave Birdsall <db...@apache.org>
AuthorDate: Mon Mar 11 19:32:10 2019 +0000

    [TRAFODION-3284] Make overflow processing for float SUM consistent
---
 core/sql/exp/exp_eval.cpp | 26 +++++++++++++++++++++++---
 core/sql/exp/exp_ieee.cpp |  8 +++++++-
 core/sql/exp/exp_ieee.h   |  2 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/core/sql/exp/exp_eval.cpp b/core/sql/exp/exp_eval.cpp
index 00fbcc4..84757ed 100644
--- a/core/sql/exp/exp_eval.cpp
+++ b/core/sql/exp/exp_eval.cpp
@@ -54,6 +54,7 @@
 
 #include <unistd.h>
 #include <sys/mman.h>
+#include <fenv.h>  // floating point environment stuff
 
 #include "exp_ovfl_ptal.h"
 
@@ -72,6 +73,10 @@ double MathConvReal64ToReal64(double op1, Int16 * ov);
 #define EXP_CLEAR_SIGNBIT(result) \
   ( ((result) & 0x80000000) ? ((result) ^ 0x80000002) : (result) )
 
+// macros for branch prediction hints in gcc
+#define LIKELY(x)       __builtin_expect((x),1)
+#define UNLIKELY(x)     __builtin_expect((x),0)
+
 
 void setVCLength(char * VCLen, Int32 VCLenSize, UInt32 value);
 
@@ -5019,11 +5024,26 @@ ex_expr::exp_return_type ex_expr::evalPCode(PCodeBinary* pCode32,
       FLT64ASSIGN(&flt64_1, (stack[pCode[0]] + pCode[1]));
       FLT64ASSIGN(&flt64_2, (stack[pCode[2]] + pCode[3]));
 
+      // The following is an inlining of most of MathReal64Add (from exp/exp_ieee.cpp), 
+      // done for performance reasons. For the SUM aggregate, in large queries we expect 
+      // millions or billions of rows; the inlining makes a difference at that scale.
+
+      unsigned int mxcsr;
+      __asm__ ("stmxcsr %0" : "=m" (*&mxcsr)); 
+      /* Clear the relevant bits.  */ 
+      mxcsr &= ~(FE_OVERFLOW | FE_INVALID | FE_DIVBYZERO | FE_UNDERFLOW);
+      /* And put them into effect.  */
+      __asm__ ("ldmxcsr %0" : : "m" (*&mxcsr));
+  
       flt64_0 = flt64_1 + flt64_2;
-      if ((flt64_0 < -DBL_MAX) || (flt64_0 > DBL_MAX) ||
-          ((flt64_0 != 0) && (flt64_0 < DBL_MIN) &&
-           (flt64_0 > -DBL_MIN)))
+  
+      unsigned int cs;
+      __asm__ ("stmxcsr %0" : "=m" (*&cs));
+      cs &= (FE_UNDERFLOW | FE_OVERFLOW | FE_DIVBYZERO | FE_INVALID);
+      if (UNLIKELY(cs))  // if there was an error (this is unlikely)
         {
+          ov = 0;
+          MathEvalException(flt64_0, cs, &ov);  // don't bother inlining error path
           goto Error1_;
         }
 
diff --git a/core/sql/exp/exp_ieee.cpp b/core/sql/exp/exp_ieee.cpp
index 6c32f4b..9dc130d 100644
--- a/core/sql/exp/exp_ieee.cpp
+++ b/core/sql/exp/exp_ieee.cpp
@@ -107,6 +107,12 @@ double doConvReal64ToReal64(double op1)
   return temp;
 }
 
+// Note: A copy of this function's logic has been inlined
+// in exp_eval.cpp. So if you change this, change that logic
+// also. (We may ultimately want to inline this function 
+// everywhere, or even all the functions in this module, but 
+// that requires a more complete study of performance impacts 
+// than I am prepared to invest at the moment.)
 
 double MathReal64Add(double x, double y, short *ov)
 {
@@ -202,4 +208,4 @@ float MathConvReal64ToReal32(double x, short * ov)
       MathEvalExceptionConv1(res, exc, ov);
   }
   return res;
-}
\ No newline at end of file
+}
diff --git a/core/sql/exp/exp_ieee.h b/core/sql/exp/exp_ieee.h
index eddf994..a16ac1a 100644
--- a/core/sql/exp/exp_ieee.h
+++ b/core/sql/exp/exp_ieee.h
@@ -40,4 +40,6 @@ double MathConvReal64ToReal64(double x, short * ov);
 }
 #endif
 
+void MathEvalException(double result, unsigned long exc, short * ov); // so exp_eval.cpp can get it
+
 #endif