[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 Andrew Pinski changed: What|Removed |Added Target Milestone|--- |11.3
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 Dominique d'Humieres changed: What|Removed |Added Status|WAITING |RESOLVED Resolution|--- |FIXED --- Comment #14 from Dominique d'Humieres --- > Anything left to do or can the PR be closed? I think so. Please reopen if I missed something.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 anlauf at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|WAITING --- Comment #13 from anlauf at gcc dot gnu.org --- Anything left to do or can the PR be closed?
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 --- Comment #12 from CVS Commits --- The releases/gcc-10 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:e24dbeeb7864dc4c673f57383c3fa329d1d16e71 commit r10-8729-ge24dbeeb7864dc4c673f57383c3fa329d1d16e71 Author: Jakub Jelinek Date: Wed Sep 2 12:18:46 2020 +0200 fortran: Fix o'...' boz to integer/real conversions [PR96859] 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. 2020-09-02 Jakub Jelinek 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. (cherry picked from commit b567d3bd302933adb253aba9069fd8120c485441)
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 --- Comment #11 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:b567d3bd302933adb253aba9069fd8120c485441 commit r11-2978-gb567d3bd302933adb253aba9069fd8120c485441 Author: Jakub Jelinek Date: Wed Sep 2 12:18:46 2020 +0200 fortran: Fix o'...' boz to integer/real conversions [PR96859] 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. 2020-09-02 Jakub Jelinek 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.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859
--- Comment #10 from Steve Kargl ---
On Tue, Sep 01, 2020 at 03:20:20PM +, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859
>
> --- Comment #9 from Jakub Jelinek ---
> I've read that. But I think in this case it is an obvious bug (just, what
> I've
> missed in the patch, there is another copy of the same bug in another
> routine).
> The
> /* Clear first bit. */
> if (kind == 1 || kind == 4 || kind == 16)
> {
> if (buf[0] == '4')
> buf[0] = '0';
> else if (buf[0] == '5')
> buf[0] = '1';
> else if (buf[0] == '6')
> buf[0] = '2';
> else if (buf[0] == '7')
> buf[0] = '3';
> }
> part looks correct, for kind 1, 4 and 16 the calculated len times 3 is 1
> larger
> than the number of bits it needs, so the above ensures that the first digit is
> 0-3 even if it is 4-7 by subtracting 4.
> But the:
> /* Clear first two bits. */
> else
> {
> if (buf[0] == '4' || buf[0] == '6')
> buf[0] = '0';
> else if (buf[0] == '5' || buf[0] == '7')
> buf[0] = '1';
> }
> which is needed for kind 2 and 8, when he calculated len times 3 is 2 larger
> than the number of bits it needs, is only correct for digits 0-1 an 4-7, for
> which it ensures the digit is 0 if it is even and 1 if it is odd. But 2 and 3
> are kept as is, while they don't fit into 1 bit.
>
It is certainly possible I have/had an off-by-one in handwritten
conversions I was doing; in particular, I don't often work with
octal numbers.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859
--- Comment #9 from Jakub Jelinek ---
I've read that. But I think in this case it is an obvious bug (just, what I've
missed in the patch, there is another copy of the same bug in another routine).
The
/* Clear first bit. */
if (kind == 1 || kind == 4 || kind == 16)
{
if (buf[0] == '4')
buf[0] = '0';
else if (buf[0] == '5')
buf[0] = '1';
else if (buf[0] == '6')
buf[0] = '2';
else if (buf[0] == '7')
buf[0] = '3';
}
part looks correct, for kind 1, 4 and 16 the calculated len times 3 is 1 larger
than the number of bits it needs, so the above ensures that the first digit is
0-3 even if it is 4-7 by subtracting 4.
But the:
/* Clear first two bits. */
else
{
if (buf[0] == '4' || buf[0] == '6')
buf[0] = '0';
else if (buf[0] == '5' || buf[0] == '7')
buf[0] = '1';
}
which is needed for kind 2 and 8, when he calculated len times 3 is 2 larger
than the number of bits it needs, is only correct for digits 0-1 an 4-7, for
which it ensures the digit is 0 if it is even and 1 if it is odd. But 2 and 3
are kept as is, while they don't fit into 1 bit.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 --- Comment #8 from Steve Kargl --- On Tue, Sep 01, 2020 at 12:52:49PM +, jakub at gcc dot gnu.org wrote: > > --- Comment #5 from Jakub Jelinek --- > I think this boils down to: > program foo > print *, int(o'1234567',2), int(o'1234567',4) > end program foo > I believe ifort prints > 14711 342391 > while gfortran prints > -18057 342391 > Since 01234567 in C is 0x53977, after that is cast to 16-bit integer that > should be 0x3977 and thus 14711. > The Fortran standard states how the BOZ string of bits is padded or truncated if it contains too few or too many bits. See F2018, 16.3.3. Also no that the handling of the sign bit is processor dependent. -- Steve
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 Jakub Jelinek changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org Status|NEW |ASSIGNED --- Comment #7 from Jakub Jelinek --- Created attachment 49164 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49164=edit gcc11-pr96859.patch Full untested patch.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859
--- Comment #6 from Jakub Jelinek ---
Untested fix:
--- gcc/fortran/check.c.jj 2020-08-24 10:00:01.424256990 +0200
+++ gcc/fortran/check.c 2020-09-01 15:06:45.428938642 +0200
@@ -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';
}
}
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org Status|WAITING |NEW --- Comment #5 from Jakub Jelinek --- I think this boils down to: program foo print *, int(o'1234567',2), int(o'1234567',4) end program foo I believe ifort prints 14711 342391 while gfortran prints -18057 342391 Since 01234567 in C is 0x53977, after that is cast to 16-bit integer that should be 0x3977 and thus 14711.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 --- Comment #4 from zhen...@compiler-dev.com --- (In reply to kargl from comment #2) > gfortran appears to be correct. Writing an actual program with your > examples, I have > > program foo >print '(I0,1X,I0)', & >& merge_bits(32767_2,o'1234567',32767_2), & >& ior(iand(32767_2,32767_2),iand(o'1234567',not(32767_2))) > >print '(I0,1X,I0)', & >& merge_bits(o'1234567',32767_2,o'1234567'), & >& > ior(iand(o'1234567',int(o'1234567',2)),iand(32767_2,not(int(o'1234567',2 > >print '(I0,1X,I0)', & >& merge_bits(32767_2,o'1234567',b'010101'), & >& ior(iand(32767_2,b'010101'),iand(o'1234567', not(int(b'010101',2 > >print '(I0,1X,I0)', & >& merge_bits(32767_2,o'1234567',z'12345678'), & >& ior(iand(32767_2,z'12345678'), iand(o'1234567',not(int( > z'12345678',2 > end program foo I compiled your code with gfortran-10, getting result: -1 -1 -1 -1 -18057 -18057 -129 -129 The right one should be: 32767 32767 32767 32767 14711 14711 32639 32639
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 --- Comment #3 from zhen...@compiler-dev.com --- (In reply to Dominique d'Humieres from comment #1) > What results do you expect? merge_bits(32767_2, o'1234567', 32767_2) merge_bits(o'1234567', 32767_2, o'1234567') merge_bits(32767_2, o'1234567', b'010101') merge_bits(32767_2, o'1234567', z'12345678') The four corresponding results should be: 32767 32767 14711 32639 I checked the Fortran 2008 standard and confirmed the results with hand calculation.
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 kargl at gcc dot gnu.org changed: What|Removed |Added CC||kargl at gcc dot gnu.org --- Comment #2 from kargl at gcc dot gnu.org --- gfortran appears to be correct. Writing an actual program with your examples, I have program foo print '(I0,1X,I0)', & & merge_bits(32767_2,o'1234567',32767_2), & & ior(iand(32767_2,32767_2),iand(o'1234567',not(32767_2))) print '(I0,1X,I0)', & & merge_bits(o'1234567',32767_2,o'1234567'), & & ior(iand(o'1234567',int(o'1234567',2)),iand(32767_2,not(int(o'1234567',2 print '(I0,1X,I0)', & & merge_bits(32767_2,o'1234567',b'010101'), & & ior(iand(32767_2,b'010101'),iand(o'1234567', not(int(b'010101',2 print '(I0,1X,I0)', & & merge_bits(32767_2,o'1234567',z'12345678'), & & ior(iand(32767_2,z'12345678'), iand(o'1234567',not(int( z'12345678',2 end program foo
[Bug fortran/96859] Wrong answer with intrinsic merge_bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96859 Dominique d'Humieres changed: What|Removed |Added Last reconfirmed||2020-08-31 Status|UNCONFIRMED |WAITING Ever confirmed|0 |1 --- Comment #1 from Dominique d'Humieres --- What results do you expect?
