]> granicus.if.org Git - postgresql/commitdiff
New method for preventing compile-time calculation of degree constants.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 25 Apr 2016 19:21:04 +0000 (15:21 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 25 Apr 2016 19:21:04 +0000 (15:21 -0400)
Commit 65abaab547a5758b tried to prevent the scaling constants used in
the degree-based trig functions from being precomputed at compile time,
because some compilers do that with functions that don't yield results
identical-to-the-last-bit to what you get at runtime.  A report from
Peter Eisentraut suggests that some recent compilers are smart enough
to see through that trick, though.  Instead, let's put the inputs to
these calculations into non-const global variables, which should be a
more reliable way of convincing the compiler that it can't assume that
they are compile-time constants.  (If we really get desperate, we could
mark these variables "volatile", but I do not believe we should have to.)

src/backend/utils/adt/float.c

index c7c0b5842b112ed8097b417540ced6541f000964..a95ebe58f3ab4cb332a632bea586780a018512c2 100644 (file)
@@ -77,15 +77,24 @@ static float8 atan_1_0 = 0;
 static float8 tan_45 = 0;
 static float8 cot_45 = 0;
 
+/*
+ * These are intentionally not static; don't "fix" them.  They will never
+ * be referenced by other files, much less changed; but we don't want the
+ * compiler to know that, else it might try to precompute expressions
+ * involving them.  See comments for init_degree_constants().
+ */
+float8         degree_c_thirty = 30.0;
+float8         degree_c_forty_five = 45.0;
+float8         degree_c_sixty = 60.0;
+float8         degree_c_one_half = 0.5;
+float8         degree_c_one = 1.0;
+
 /* Local function prototypes */
 static int     float4_cmp_internal(float4 a, float4 b);
 static int     float8_cmp_internal(float8 a, float8 b);
 static double sind_q1(double x);
 static double cosd_q1(double x);
-
-/* This is INTENTIONALLY NOT STATIC.  Don't "fix" it. */
-void init_degree_constants(float8 thirty, float8 forty_five, float8 sixty,
-                                         float8 one_half, float8 one);
+static void init_degree_constants(void);
 
 #ifndef HAVE_CBRT
 /*
@@ -1814,35 +1823,31 @@ dtan(PG_FUNCTION_ARGS)
  * compilers out there that will precompute expressions such as sin(constant)
  * using a sin() function different from what will be used at runtime.  If we
  * want exact results, we must ensure that none of the scaling constants used
- * in the degree-based trig functions are computed that way.
- *
- * The whole approach fails if init_degree_constants() gets inlined into the
- * call sites, since then constant-folding can happen anyway.  Currently it
- * seems sufficient to declare it non-static to prevent that.  We have no
- * expectation that other files will call this, but don't tell gcc that.
+ * in the degree-based trig functions are computed that way.  To do so, we
+ * compute them from the variables degree_c_thirty etc, which are also really
+ * constants, but the compiler cannot assume that.
  *
  * Other hazards we are trying to forestall with this kluge include the
  * possibility that compilers will rearrange the expressions, or compute
  * some intermediate results in registers wider than a standard double.
  */
-void
-init_degree_constants(float8 thirty, float8 forty_five, float8 sixty,
-                                         float8 one_half, float8 one)
-{
-       sin_30 = sin(thirty * RADIANS_PER_DEGREE);
-       one_minus_cos_60 = 1.0 - cos(sixty * RADIANS_PER_DEGREE);
-       asin_0_5 = asin(one_half);
-       acos_0_5 = acos(one_half);
-       atan_1_0 = atan(one);
-       tan_45 = sind_q1(forty_five) / cosd_q1(forty_five);
-       cot_45 = cosd_q1(forty_five) / sind_q1(forty_five);
+static void
+init_degree_constants(void)
+{
+       sin_30 = sin(degree_c_thirty * RADIANS_PER_DEGREE);
+       one_minus_cos_60 = 1.0 - cos(degree_c_sixty * RADIANS_PER_DEGREE);
+       asin_0_5 = asin(degree_c_one_half);
+       acos_0_5 = acos(degree_c_one_half);
+       atan_1_0 = atan(degree_c_one);
+       tan_45 = sind_q1(degree_c_forty_five) / cosd_q1(degree_c_forty_five);
+       cot_45 = cosd_q1(degree_c_forty_five) / sind_q1(degree_c_forty_five);
        degree_consts_set = true;
 }
 
 #define INIT_DEGREE_CONSTANTS() \
 do { \
        if (!degree_consts_set) \
-               init_degree_constants(30.0, 45.0, 60.0, 0.5, 1.0); \
+               init_degree_constants(); \
 } while(0)