Fix Ada comparison failure on SPARC

Message ID yddef1sovkt.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • Fix Ada comparison failure on SPARC
Related show

Commit Message

Rainer Orth Aug. 11, 2019, 10:17 a.m.
Between 20190806 (r274144) and 20190807 (r274169), Solaris/SPARC
bootstrap with Ada began to fail with a comparion failure:

Bootstrap comparison failure!
gcc/ada/bindo-graphs.o differs

The differences look like this:

-prev-gcc/ada/bindo-graphs.o.stripped:     file format elf32-sparc-sol2
+gcc/ada/bindo-graphs.o.stripped:     file format elf32-sparc-sol2
@@ -22716 +22716 @@
-   162d0:      81 aa 0a ca     fcmped  %f8, %f10
+   162d0:      83 aa 0a ca     fcmped  %fcc1, %f8, %f10
@@ -22718 +22718 @@
-   162d8:      85 61 20 01     movl  %fcc0, 1, %g2
+   162d8:      85 61 28 01     movl  %fcc1, 1, %g2

and a couple (ca. 40) more.

The presence or absence of the failure is quite sensitive to both the
toolchain used (32-bit gas/ld and gas/gld fail, 64-bit gas/ld and 32 and
64-bit as/ld pass) and the exact srcdir.

A reghunt identified this patch as the culprit:

2019-08-07  Richard Sandiford  <richard.sandiford@arm.com>

	* data-streamer.h (streamer_write_poly_uint64): Declare.
	(streamer_read_poly_uint64): Likewise.
	* data-streamer-in.c (streamer_read_poly_uint64): New function.
	* data-streamer-out.c (streamer_write_poly_uint64): Likewise.
	* ipa-predicate.h (condition::size): Turn into a poly_int64.
	(add_condition): Take a poly_int64 size.
	* ipa-predicate.c (add_condition): Likewise.
[...]

Looking through it, I noticed this snippet

to maybe_eq as in the following patch fixed the comparison failure.

Bootstrapped without regressions on sparc-sun-solaris2.11 and
i386-pc-solaris2.11.  Ok for mainline?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2019-08-11  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* ipa-predicate.c (add_condition): Fix typo.

Comments

Jakub Jelinek Aug. 11, 2019, 10:20 a.m. | #1
On Sun, Aug 11, 2019 at 12:17:06PM +0200, Rainer Orth wrote:
> 2019-08-07  Richard Sandiford  <richard.sandiford@arm.com>

> 

> 	* data-streamer.h (streamer_write_poly_uint64): Declare.

> 	(streamer_read_poly_uint64): Likewise.

> 	* data-streamer-in.c (streamer_read_poly_uint64): New function.

> 	* data-streamer-out.c (streamer_write_poly_uint64): Likewise.

> 	* ipa-predicate.h (condition::size): Turn into a poly_int64.

> 	(add_condition): Take a poly_int64 size.

> 	* ipa-predicate.c (add_condition): Likewise.

> [...]

> 

> Looking through it, I noticed this snippet

> 

> diff --git a/gcc/ipa-predicate.c b/gcc/ipa-predicate.c

> --- a/gcc/ipa-predicate.c

> +++ b/gcc/ipa-predicate.c

> @@ -549,7 +549,7 @@ add_condition (class ipa_fn_summary *sum

>    for (i = 0; vec_safe_iterate (summary->conds, i, &c); i++)

>      {

>        if (c->operand_num == operand_num

> -         && c->size == size

> +         && maybe_ne (c->size, size)

> 

> 

> where changing == to maybe_ne didn't seem right.  And indeed changing it

> to maybe_eq as in the following patch fixed the comparison failure.


Shouldn't that be known_eq instead?  Of course, it could make a difference
right now solely on aarch64 SVE.

	Jakub
Richard Biener Aug. 11, 2019, 12:21 p.m. | #2
On August 11, 2019 12:20:31 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Sun, Aug 11, 2019 at 12:17:06PM +0200, Rainer Orth wrote:

>> 2019-08-07  Richard Sandiford  <richard.sandiford@arm.com>

>> 

>> 	* data-streamer.h (streamer_write_poly_uint64): Declare.

>> 	(streamer_read_poly_uint64): Likewise.

>> 	* data-streamer-in.c (streamer_read_poly_uint64): New function.

>> 	* data-streamer-out.c (streamer_write_poly_uint64): Likewise.

>> 	* ipa-predicate.h (condition::size): Turn into a poly_int64.

>> 	(add_condition): Take a poly_int64 size.

>> 	* ipa-predicate.c (add_condition): Likewise.

>> [...]

>> 

>> Looking through it, I noticed this snippet

>> 

>> diff --git a/gcc/ipa-predicate.c b/gcc/ipa-predicate.c

>> --- a/gcc/ipa-predicate.c

>> +++ b/gcc/ipa-predicate.c

>> @@ -549,7 +549,7 @@ add_condition (class ipa_fn_summary *sum

>>    for (i = 0; vec_safe_iterate (summary->conds, i, &c); i++)

>>      {

>>        if (c->operand_num == operand_num

>> -         && c->size == size

>> +         && maybe_ne (c->size, size)

>> 

>> 

>> where changing == to maybe_ne didn't seem right.  And indeed changing

>it

>> to maybe_eq as in the following patch fixed the comparison failure.

>

>Shouldn't that be known_eq instead?  Of course, it could make a

>difference

>right now solely on aarch64 SVE.


OK with using known_eq. 

Richard. 

>	Jakub

Patch

# HG changeset patch
# Parent  2ca1e7660c8c156605bb706a911e971840fca6d4
Fix Ada comparison failure on SPARC

diff --git a/gcc/ipa-predicate.c b/gcc/ipa-predicate.c
--- a/gcc/ipa-predicate.c
+++ b/gcc/ipa-predicate.c
@@ -549,7 +549,7 @@  add_condition (class ipa_fn_summary *sum
   for (i = 0; vec_safe_iterate (summary->conds, i, &c); i++)
     {
       if (c->operand_num == operand_num
-	  && maybe_ne (c->size, size)
+	  && maybe_eq (c->size, size)
 	  && c->code == code
 	  && c->val == val
 	  && c->agg_contents == agg_contents