public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] fortran: Fix o'...' boz to integer/real conversions [PR96859]
@ 2020-09-02  8:18 Jakub Jelinek
  2020-09-02  9:20 ` Tobias Burnus
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2020-09-02  8:18 UTC (permalink / raw)
  To: gcc-patches, fortran

Hi!

The standard says that excess digits from boz are truncated.
For hexadecimal or binary, the routines copy just the number of digits
that will be needed, but for octal we copy number of digits that
contain one extra bit (for 8-bit, 32-bit or 128-bit, i.e. kind 1, 4 and 16)
or two extra bits (for 16-bit or 64-bit, i.e. kind 2 and 8).
The clearing of the first bit is done correctly by changing the first digit
if it is 4-7 to one smaller by 4 (i.e. modulo 4).
The clearing of the first two bits is done by changing 4 or 6 to 0
and 5 or 7 to 1, which is incorrect, because we really want to change the
first digit to 0 if it was even, or to 1 if it was odd, so digits
2 and 3 are mishandled by keeping them as is, rather than changing 2 to 0
and 3 to 1.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk and release branches?

2020-09-02  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/96859
	* check.c (gfc_boz2real, gfc_boz2int): When clearing first two bits,
	change also '2' to '0' and '3' to '1' rather than just handling '4'
	through '7'.

	* gfortran.dg/pr96859.f90: New test.

--- gcc/fortran/check.c.jj	2020-08-24 10:00:01.424256990 +0200
+++ gcc/fortran/check.c	2020-09-01 17:15:19.882311053 +0200
@@ -340,9 +340,9 @@ gfc_boz2real (gfc_expr *x, int kind)
       /* Clear first two bits.  */
       else
 	{
-	  if (buf[0] == '4' || buf[0] == '6')
+	  if (buf[0] == '2' || buf[0] == '4' || buf[0] == '6')
 	    buf[0] = '0';
-	  else if (buf[0] == '5' || buf[0] == '7')
+	  else if (buf[0] == '3' || buf[0] == '5' || buf[0] == '7')
 	    buf[0] = '1';
 	}
     }
@@ -429,9 +429,9 @@ gfc_boz2int (gfc_expr *x, int kind)
       /* Clear first two bits.  */
       else
 	{
-	  if (buf[0] == '4' || buf[0] == '6')
+	  if (buf[0] == '2' || buf[0] == '4' || buf[0] == '6')
 	    buf[0] = '0';
-	  else if (buf[0] == '5' || buf[0] == '7')
+	  else if (buf[0] == '3' || buf[0] == '5' || buf[0] == '7')
 	    buf[0] = '1';
 	}
     }
--- gcc/testsuite/gfortran.dg/pr96859.f90.jj	2020-09-01 15:26:26.448799264 +0200
+++ gcc/testsuite/gfortran.dg/pr96859.f90	2020-09-01 15:26:18.492914701 +0200
@@ -0,0 +1,25 @@
+! PR fortran/96859
+! { dg-do run }
+
+program pr96859
+  if (merge_bits(32767_2, o'1234567', 32767_2).ne.32767_2) stop 1
+  if (merge_bits(o'1234567', 32767_2, o'1234567').ne.32767_2) stop 2
+  if (merge_bits(32767_2, o'1234567', b'010101').ne.14711_2) stop 3
+  if (merge_bits(32767_2, o'1234567', z'12345678').ne.32639_2) stop 4
+  if (int (o'1034567', 2).ne.14711_2) stop 5
+  if (int (o'1234567', 2).ne.14711_2) stop 6
+  if (int (o'1434567', 2).ne.14711_2) stop 7
+  if (int (o'1634567', 2).ne.14711_2) stop 8
+  if (int (o'1134567', 2).ne.-18057_2) stop 9
+  if (int (o'1334567', 2).ne.-18057_2) stop 10
+  if (int (o'1534567', 2).ne.-18057_2) stop 11
+  if (int (o'1734567', 2).ne.-18057_2) stop 12
+  if (int (o'70123456776543211234567', 8).ne.1505855851274254711_8) stop 13
+  if (int (o'72123456776543211234567', 8).ne.1505855851274254711_8) stop 14
+  if (int (o'74123456776543211234567', 8).ne.1505855851274254711_8) stop 15
+  if (int (o'76123456776543211234567', 8).ne.1505855851274254711_8) stop 16
+  if (int (o'71123456776543211234567', 8).ne.-7717516185580521097_8) stop 17
+  if (int (o'73123456776543211234567', 8).ne.-7717516185580521097_8) stop 18
+  if (int (o'75123456776543211234567', 8).ne.-7717516185580521097_8) stop 19
+  if (int (o'77123456776543211234567', 8).ne.-7717516185580521097_8) stop 20
+end

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] fortran: Fix o'...' boz to integer/real conversions [PR96859]
  2020-09-02  8:18 [PATCH] fortran: Fix o'...' boz to integer/real conversions [PR96859] Jakub Jelinek
@ 2020-09-02  9:20 ` Tobias Burnus
  0 siblings, 0 replies; 2+ messages in thread
From: Tobias Burnus @ 2020-09-02  9:20 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches, fortran

On 9/2/20 10:18 AM, Jakub Jelinek via Gcc-patches wrote:

> The standard says that excess digits from boz are truncated.
> For hexadecimal or binary, the routines copy just the number of digits
> that will be needed, but for octal we copy number of digits that
> contain one extra bit (for 8-bit, 32-bit or 128-bit, i.e. kind 1, 4 and 16)
> or two extra bits (for 16-bit or 64-bit, i.e. kind 2 and 8).
> The clearing of the first bit is done correctly by changing the first digit
> if it is 4-7 to one smaller by 4 (i.e. modulo 4).
> The clearing of the first two bits is done by changing 4 or 6 to 0
> and 5 or 7 to 1, which is incorrect, because we really want to change the
> first digit to 0 if it was even, or to 1 if it was odd, so digits
> 2 and 3 are mishandled by keeping them as is, rather than changing 2 to 0
> and 3 to 1.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk and release branches?

The code part of the patch looks okay – as does the testcase.
→ LGTM.

What confused me a while when looking at the patch is the comment
"Clear first two bits"
This does not really apply to neither o'2' = b'11' nor
o'3' = b'11' as only the first ('1') bit needs to be removed (ignoring
'0' padding on left). However, unless you have better wording,
we can leave it as is.

Thanks for the patch!

Tobias


> 2020-09-02  Jakub Jelinek  <jakub@redhat.com>
>
>       PR fortran/96859
>       * check.c (gfc_boz2real, gfc_boz2int): When clearing first two bits,
>       change also '2' to '0' and '3' to '1' rather than just handling '4'
>       through '7'.
>
>       * gfortran.dg/pr96859.f90: New test.
>
> --- gcc/fortran/check.c.jj    2020-08-24 10:00:01.424256990 +0200
> +++ gcc/fortran/check.c       2020-09-01 17:15:19.882311053 +0200
> @@ -340,9 +340,9 @@ gfc_boz2real (gfc_expr *x, int kind)
>         /* Clear first two bits.  */
>         else
>       {
> -       if (buf[0] == '4' || buf[0] == '6')
> +       if (buf[0] == '2' || buf[0] == '4' || buf[0] == '6')
>           buf[0] = '0';
> -       else if (buf[0] == '5' || buf[0] == '7')
> +       else if (buf[0] == '3' || buf[0] == '5' || buf[0] == '7')
>           buf[0] = '1';
>       }
>       }
> @@ -429,9 +429,9 @@ gfc_boz2int (gfc_expr *x, int kind)
>         /* Clear first two bits.  */
>         else
>       {
> -       if (buf[0] == '4' || buf[0] == '6')
> +       if (buf[0] == '2' || buf[0] == '4' || buf[0] == '6')
>           buf[0] = '0';
> -       else if (buf[0] == '5' || buf[0] == '7')
> +       else if (buf[0] == '3' || buf[0] == '5' || buf[0] == '7')
>           buf[0] = '1';
>       }
>       }
> --- gcc/testsuite/gfortran.dg/pr96859.f90.jj  2020-09-01 15:26:26.448799264 +0200
> +++ gcc/testsuite/gfortran.dg/pr96859.f90     2020-09-01 15:26:18.492914701 +0200
> @@ -0,0 +1,25 @@
> +! PR fortran/96859
> +! { dg-do run }
> +
> +program pr96859
> +  if (merge_bits(32767_2, o'1234567', 32767_2).ne.32767_2) stop 1
> +  if (merge_bits(o'1234567', 32767_2, o'1234567').ne.32767_2) stop 2
> +  if (merge_bits(32767_2, o'1234567', b'010101').ne.14711_2) stop 3
> +  if (merge_bits(32767_2, o'1234567', z'12345678').ne.32639_2) stop 4
> +  if (int (o'1034567', 2).ne.14711_2) stop 5
> +  if (int (o'1234567', 2).ne.14711_2) stop 6
> +  if (int (o'1434567', 2).ne.14711_2) stop 7
> +  if (int (o'1634567', 2).ne.14711_2) stop 8
> +  if (int (o'1134567', 2).ne.-18057_2) stop 9
> +  if (int (o'1334567', 2).ne.-18057_2) stop 10
> +  if (int (o'1534567', 2).ne.-18057_2) stop 11
> +  if (int (o'1734567', 2).ne.-18057_2) stop 12
> +  if (int (o'70123456776543211234567', 8).ne.1505855851274254711_8) stop 13
> +  if (int (o'72123456776543211234567', 8).ne.1505855851274254711_8) stop 14
> +  if (int (o'74123456776543211234567', 8).ne.1505855851274254711_8) stop 15
> +  if (int (o'76123456776543211234567', 8).ne.1505855851274254711_8) stop 16
> +  if (int (o'71123456776543211234567', 8).ne.-7717516185580521097_8) stop 17
> +  if (int (o'73123456776543211234567', 8).ne.-7717516185580521097_8) stop 18
> +  if (int (o'75123456776543211234567', 8).ne.-7717516185580521097_8) stop 19
> +  if (int (o'77123456776543211234567', 8).ne.-7717516185580521097_8) stop 20
> +end
>
>       Jakub
>
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-02  9:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02  8:18 [PATCH] fortran: Fix o'...' boz to integer/real conversions [PR96859] Jakub Jelinek
2020-09-02  9:20 ` Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).