diff mbox

[rs6000] Fix PR65914

Message ID 1434753359.2747.70.camel@gnopaine
State New
Headers show

Commit Message

Bill Schmidt June 19, 2015, 10:35 p.m. UTC 
Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65914 demonstrates that we
fail to match vector predicates with pseudos that are in the virtual
stack register range.  The reduced test case provided with the bug
report, when compiled for the C++14 standard, demonstrates that we need
to be able to do this.  This patch loosens the restriction for the
vector predicates so that all pseudos are accepted.  Thanks to Uli
Weigand for his insights on this bug.

As a side benefit, applying this patch fixes a number of libgomp tests
that have been failing recently.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?  After it burns in, I would propose
to backport it to GCC 5.1 and GCC 4.9 (when the tree is open).

Thanks,
Bill


[gcc]

2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/predicates.md (altivec_register_operand): Permit
	virtual stack registers.
	(vsx_register_operand): Likewise.
	(vfloat_operand): Likewise.
	(vint_operand): Likewise.
	(vlogical_operand): Likewise.

[gcc/testsuite]

2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* g++.dg/torture/pr65419.C:  New.

Comments

David Edelsohn June 20, 2015, 12:36 a.m. UTC | #1 
On Fri, Jun 19, 2015 at 6:35 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65914 demonstrates that we
> fail to match vector predicates with pseudos that are in the virtual
> stack register range.  The reduced test case provided with the bug
> report, when compiled for the C++14 standard, demonstrates that we need
> to be able to do this.  This patch loosens the restriction for the
> vector predicates so that all pseudos are accepted.  Thanks to Uli
> Weigand for his insights on this bug.
>
> As a side benefit, applying this patch fixes a number of libgomp tests
> that have been failing recently.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?  After it burns in, I would propose
> to backport it to GCC 5.1 and GCC 4.9 (when the tree is open).
>
> Thanks,
> Bill
>
>
> [gcc]
>
> 2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * config/rs6000/predicates.md (altivec_register_operand): Permit
>         virtual stack registers.
>         (vsx_register_operand): Likewise.
>         (vfloat_operand): Likewise.
>         (vint_operand): Likewise.
>         (vlogical_operand): Likewise.

Okay.

> [gcc/testsuite]
>
> 2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * g++.dg/torture/pr65419.C:  New.

Maybe you should ask Richi or Jakub about the testcase because you are
placing it in a non-target-specific location.  It should succeed on
all targets, but it may expose latent bugs on other targets.

Thanks, David
Mike Stump June 20, 2015, 8:37 p.m. UTC | #2 
On Jun 19, 2015, at 5:36 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> Maybe you should ask Richi or Jakub about the testcase because you are
> placing it in a non-target-specific location.  It should succeed on
> all targets, but it may expose latent bugs on other targets.

A latent bug is one that is broken, but appears to work, and then a change in the compiler is made to expose the bug that was always there but didn’t show a failure.  It only applies to the compiler proper.

In this case, since the change is to add a test case, the test case can only show bugs that aren’t latent (or failures in the test case).
Bill Schmidt June 21, 2015, 1:01 p.m. UTC | #3 
On Sat, 2015-06-20 at 13:37 -0700, Mike Stump wrote:
> On Jun 19, 2015, at 5:36 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> > Maybe you should ask Richi or Jakub about the testcase because you are
> > placing it in a non-target-specific location.  It should succeed on
> > all targets, but it may expose latent bugs on other targets.
> 
> A latent bug is one that is broken, but appears to work, and then a change in the compiler is made to expose the bug that was always there but didn’t show a failure.  It only applies to the compiler proper.
> 
> In this case, since the change is to add a test case, the test case can only show bugs that aren’t latent (or failures in the test case).

Yes, I felt that in this case we seem to have C++14 adding some new code
paths that could well cause surprises on other targets besides Power.
It seems like the sort of thing that language-specific torture tests are
for, but please let me know if that's wrong and I'll move it to a target
directory.

Thanks,
Bill
Bill Schmidt June 22, 2015, 1:47 a.m. UTC | #4 
On Fri, 2015-06-19 at 20:36 -0400, David Edelsohn wrote:
> On Fri, Jun 19, 2015 at 6:35 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65914 demonstrates that we
> > fail to match vector predicates with pseudos that are in the virtual
> > stack register range.  The reduced test case provided with the bug
> > report, when compiled for the C++14 standard, demonstrates that we need
> > to be able to do this.  This patch loosens the restriction for the
> > vector predicates so that all pseudos are accepted.  Thanks to Uli
> > Weigand for his insights on this bug.
> >
> > As a side benefit, applying this patch fixes a number of libgomp tests
> > that have been failing recently.
> >
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> > regressions.  Is this ok for trunk?  After it burns in, I would propose
> > to backport it to GCC 5.1 and GCC 4.9 (when the tree is open).
> >
> > Thanks,
> > Bill
> >
> >
> > [gcc]
> >
> > 2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >
> >         * config/rs6000/predicates.md (altivec_register_operand): Permit
> >         virtual stack registers.
> >         (vsx_register_operand): Likewise.
> >         (vfloat_operand): Likewise.
> >         (vint_operand): Likewise.
> >         (vlogical_operand): Likewise.
> 
> Okay.
> 
> > [gcc/testsuite]
> >
> > 2015-05-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >
> >         * g++.dg/torture/pr65419.C:  New.
> 
> Maybe you should ask Richi or Jakub about the testcase because you are
> placing it in a non-target-specific location.  It should succeed on
> all targets, but it may expose latent bugs on other targets.

OK, thanks.  Richi, Jakub, is this ok as a general C++ torture test?

Thanks,
Bill

> 
> Thanks, David
>
Jakub Jelinek June 22, 2015, 5:29 a.m. UTC | #5 
On Sun, Jun 21, 2015 at 08:47:10PM -0500, Bill Schmidt wrote:
> > >         * g++.dg/torture/pr65419.C:  New.
> > 
> > Maybe you should ask Richi or Jakub about the testcase because you are
> > placing it in a non-target-specific location.  It should succeed on
> > all targets, but it may expose latent bugs on other targets.
> 
> OK, thanks.  Richi, Jakub, is this ok as a general C++ torture test?

Yes, of course, there is nothing rs6000 specific on the testcase.
And, if it exposes latent bugs, at least we'll know it, that is what we have
tests for, don't we?

	Jakub
diff mbox

Patch

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 224671)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -41,7 +41,7 @@ 
   if (!REG_P (op))
     return 0;
 
-  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+  if (REGNO (op) >= FIRST_PSEUDO_REGISTER)
     return 1;
 
   return ALTIVEC_REGNO_P (REGNO (op));
@@ -57,7 +57,7 @@ 
   if (!REG_P (op))
     return 0;
 
-  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+  if (REGNO (op) >= FIRST_PSEUDO_REGISTER)
     return 1;
 
   return VSX_REGNO_P (REGNO (op));
@@ -74,7 +74,7 @@ 
   if (!REG_P (op))
     return 0;
 
-  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+  if (REGNO (op) >= FIRST_PSEUDO_REGISTER)
     return 1;
 
   return VFLOAT_REGNO_P (REGNO (op));
@@ -91,7 +91,7 @@ 
   if (!REG_P (op))
     return 0;
 
-  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+  if (REGNO (op) >= FIRST_PSEUDO_REGISTER)
     return 1;
 
   return VINT_REGNO_P (REGNO (op));
@@ -108,7 +108,7 @@ 
   if (!REG_P (op))
     return 0;
 
-  if (REGNO (op) > LAST_VIRTUAL_REGISTER)
+  if (REGNO (op) >= FIRST_PSEUDO_REGISTER)
     return 1;
 
   return VLOGICAL_REGNO_P (REGNO (op));
Index: gcc/testsuite/g++.dg/torture/pr65419.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr65419.C	(revision 0)
+++ gcc/testsuite/g++.dg/torture/pr65419.C	(working copy)
@@ -0,0 +1,70 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-std=c++14" } */
+
+enum expression_template_option { et_on };
+template <class, expression_template_option = et_on> class A;
+template <class, class, class, class = void, class = void> struct expression;
+template <class T> struct B { typedef const T &type; };
+template <class tag, class A1, class A2, class A3, class A4>
+struct B<expression<tag, A1, A2, A3, A4>> {
+  typedef expression<tag, A1, A2> type;
+};
+template <class tag, class Arg1, class Arg2>
+struct expression<tag, Arg1, Arg2> {
+  expression(Arg1 p1, const Arg2 &p2) : arg1(p1), arg2(p2) {}
+  typename B<Arg1>::type arg1;
+  typename B<Arg2>::type arg2;
+};
+template <class Backend> expression<int, int, A<Backend>> sin(A<Backend>) {
+  return expression<int, int, A<Backend>>(0, 0);
+}
+template <class tag, class A1, class A2, class A3, class A4>
+expression<int, int, expression<tag, A1, A2>>
+  asin(expression<tag, A1, A2, A3, A4> p1) {
+  return expression<int, int, expression<tag, A1, A2>>(0, p1);
+}
+template <class B, expression_template_option ET, class tag, class Arg1,
+	  class Arg2, class Arg3, class Arg4>
+expression<int, A<B>, expression<tag, Arg1, Arg2>>
+  operator+(A<B, ET>, expression<tag, Arg1, Arg2, Arg3, Arg4> p2) {
+  return expression<int, A<B>, expression<tag, Arg1, Arg2>>(0, p2);
+}
+template <class tag, class Arg1, class Arg2, class Arg3, class Arg4, class tag2,
+	  class Arg1b, class Arg2b, class Arg3b, class Arg4b>
+expression<int, expression<tag, Arg1, Arg2>, expression<tag2, Arg1b, Arg2b>>
+  operator*(expression<tag, Arg1, Arg2, Arg3, Arg4> p1,
+	    expression<tag2, Arg1b, Arg2b, Arg3b, Arg4b> p2) {
+  return expression<int, expression<tag, Arg1, Arg2>,
+		    expression<tag2, Arg1b, Arg2b>>(p1, p2);
+}
+template <class B> expression<int, A<B>, A<B>> operator/(A<B>, A<B>) {
+  return expression<int, A<B>, A<B>>(0, 0);
+}
+template <class tag, class Arg1, class Arg2, class Arg3, class Arg4, class V>
+void operator/(expression<tag, Arg1, Arg2, Arg3, Arg4>, V);
+template <class, expression_template_option> class A {
+public:
+  A() {}
+  template <class V> A(V) {}
+};
+template <class T, class Policy> void jacobi_recurse(T, T, Policy) {
+  T a, b, c;
+  (a+asin(b/c) * sin(a)) / 0.1;
+}
+template <class T, class Policy> void jacobi_imp(T p1, Policy) {
+  T x;
+  jacobi_recurse(x, p1, 0);
+}
+template <class T, class U, class V, class Policy>
+void jacobi_elliptic(T, U, V, Policy) {
+  jacobi_imp(static_cast<T>(0), 0);
+}
+template <class U, class T, class Policy> void jacobi_sn(U, T, Policy) {
+  jacobi_elliptic(static_cast<T>(0), 0, 0, 0);
+}
+template <class U, class T> void jacobi_sn(U, T p2) { jacobi_sn(0, p2, 0); }
+template <class T> void test_extra(T) {
+  T d;
+  jacobi_sn(0, d);
+}
+void foo() { test_extra(A<int>()); }