diff mbox series

, PR middle_end/82333, Make long double/_Float128 constants not hash to the same value on the PowerPC

Message ID 20171109215206.GA8602@ibm-tiger.the-meissners.org
State New
Headers show
Series , PR middle_end/82333, Make long double/_Float128 constants not hash to the same value on the PowerPC | expand

Commit Message

Michael Meissner Nov. 9, 2017, 9:52 p.m. UTC 
This patch fixes PR middle_end/82333.

The bug is due to compare_constant thinking two floating point constants are
the same if the floating point size and the internal value are the same.  On
the PowerPC, long double and _Float128 both are 128-bits, but they have
different internal representations.

The bug shows up when you try to inline two functions, one that returns 0
converted to long double _Complex and the other that returns 0 converted to
_Float128 _Complex.

The function compare_constant in varasm.c thinks that these two constants are
the same, and assigns them to the same hash.  When inliner tries to replace the
inline function (that returns 0) with the constant, it does moves of the real
part and the imaginary part.  In the second function, the real/imaginary parts
have type KFmode, while the first function (that has the saved constant) the
real/imaginary parts have type TFmode.

The fix is to consider the type along with the precision when doing hash of the
constants.

I have done bootstraps on x86-64 and little endian power8 systems.  Both
systems bootstrapped fine and there were no regressions in the test suite.  I
verified that the new test case in the powerpc test directory passes.  Can I
check this into the trunk?  Can I also check this info the active branches (gcc
6/7) providing it causes no regressions?

[gcc]
2017-11-09  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR middle_end/82333
	* varasm.c (compare_constant): Take the mode of the constants into
	account when comparing floating point constants.

[gcc/testsuite]
2017-11-09  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR middle_end/82333
	* gcc.target/powerpc/pr82333.c: New test.

Comments

Joseph Myers Nov. 27, 2017, 4:31 p.m. UTC | #1 
On Thu, 9 Nov 2017, Michael Meissner wrote:

> [gcc]
> 2017-11-09  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR middle_end/82333
> 	* varasm.c (compare_constant): Take the mode of the constants into
> 	account when comparing floating point constants.
> 
> [gcc/testsuite]
> 2017-11-09  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR middle_end/82333
> 	* gcc.target/powerpc/pr82333.c: New test.

OK, though I think it should be sufficient just to test the mode without 
also testing precision.
diff mbox series

Patch

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 254556)
+++ gcc/varasm.c	(working copy)
@@ -3118,10 +3118,16 @@  compare_constant (const tree t1, const t
       return tree_int_cst_equal (t1, t2);
 
     case REAL_CST:
-      /* Real constants are the same only if the same width of type.  */
+      /* Real constants are the same only if the same width of type.  In
+	 addition to the same width, we need to check whether the modes are the
+	 same.  There might be two floating point modes that are the same size
+	 but have different representations, such as the PowerPC that has 2
+	 different 128-bit floating point types (IBM extended double and IEEE
+	 128-bit floating point).  */
       if (TYPE_PRECISION (TREE_TYPE (t1)) != TYPE_PRECISION (TREE_TYPE (t2)))
 	return 0;
-
+      if (TYPE_MODE (TREE_TYPE (t1)) != TYPE_MODE (TREE_TYPE (t2)))
+	return 0;
       return real_identical (&TREE_REAL_CST (t1), &TREE_REAL_CST (t2));
 
     case FIXED_CST:
Index: gcc/testsuite/gcc.target/powerpc/pr82333.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr82333.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr82333.c	(revision 0)
@@ -0,0 +1,34 @@ 
+/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-require-effective-target ppc_float128_sw } */
+/* { dg-require-effective-target vsx_hw } */
+/* { dg-options "-mvsx -O2 -mabi=ibmlongdouble -Wno-psabi" } */
+
+/* PR 82333 was an internal compiler abort where the compiler thought that a
+   long double _Complex constant was the same as __float128 _Complex.  */
+
+_Complex long double vld;
+_Complex _Float128 vf128;
+
+_Complex long double
+fld (_Complex long double arg0)
+{
+  return 0;
+}
+
+_Complex _Float128
+ff128 (_Complex _Float128 arg0)
+{
+  return 0;
+}
+
+void
+tld (void)
+{
+  vld = fld (vld);
+}
+
+void
+tf128 (void)
+{
+  vf128 = ff128 (vf128);
+}