Bug 19543 - [4.1 only] fortran LOGICAL*8 not consistently distinguished from 32 bit integers
Summary: [4.1 only] fortran LOGICAL*8 not consistently distinguished from 32 bit integers
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.1.1
Assignee: Not yet assigned to anyone
URL:
Keywords: patch, wrong-code
: 20066 (view as bug list)
Depends on: 20066
Blocks:
  Show dependency treegraph
 
Reported: 2005-01-20 10:31 UTC by Francois-Xavier Coudert
Modified: 2006-03-14 23:08 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.0
Known to fail: 4.1.0 4.2.0
Last reconfirmed: 2005-12-30 19:00:47


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Francois-Xavier Coudert 2005-01-20 10:31:32 UTC
When outputing a logical(8) variable, the output can be 'T' even if the variable
is set to .FALSE.! Here is a testcase!

program test
  implicit none
  logical(4), parameter :: l4 = .FALSE.
  logical(8), parameter :: l8 = .FALSE.

  print *, l4, l8
  print *, 'foo', l8
  if (l8) print *, 'l8 is true'
end

This produces the two lines:
 F T
 foo T

which is, of course, not right! What is troubling me is that removing any of the
print statements induces a correct output for the other (and, in fact, removing
the 'foo' string induces correct output).

This bug occurs on i686-linux. This reduced snippet has been worked out from a
code that fails in a similar way on i686-mingw
(http://home.comcast.net/~kmbtib/gfortran_bugs/REF_JVB_GFTN_002.html).
Comment 1 Andrew Pinski 2005-01-20 15:35:42 UTC
  int4 C.417 = 0;
  int4 C.416 = 0;

  _gfortran_transfer_logical (&C.417, 8);

Comment 2 Tobias Schlüter 2005-01-21 21:57:31 UTC
This only happens for parameters.
Comment 3 Tobias Schlüter 2005-01-21 22:21:00 UTC
Ugh. This is ugly.

When building the I/O statement, we create a temporary because the argument is
passed by reference (why?).  The type of the temporary is determined as
TREE_TYPE(se->expr), where se->expr is the logical constant.

The type for a logical constant is built like so:
  new_type = make_unsigned_type (bit_size);
  TREE_SET_CODE (new_type, BOOLEAN_TYPE);
  TYPE_MAX_VALUE (new_type) = build_int_cst (new_type, 1);
  TYPE_PRECISION (new_type) = 1;

So, why is a temporary of 32 bits width generated instead of one bit width?

This wrong behavior can be seen in the dumps (with my patch which instruments
the dumper to print the widths of the integer constants). for this source:
   LOGICAL*8 v
   LOGICAL(4), parameter :: l4 = .FALSE.
   LOGICAL(8), parameter :: l8 = .false.

   v = l8
   print *, v
   print *, l4, l8
.t03.original looks like this, after cutting out the I/O related stuff:
  v = 0_U1;  <----------- right
   <snip>
  {
    int4 C.454 = 0_S32; <----------- right

    _gfortran_transfer_logical (&C.454, 4_S32);
  }
  {
    int4 C.455 = 0_S32; <------------ wrong

    _gfortran_transfer_logical (&C.455, 8_S32);
  }
Comment 4 Tobias Schlüter 2005-01-21 22:37:26 UTC
Weird, the build hadn't picked up my patch, which had led me to do the analysis
given above.

Patch: http://gcc.gnu.org/ml/fortran/2005-01/msg00239.html
Comment 5 Tobias Schlüter 2005-01-21 22:58:09 UTC
Unfortunately, this patch is not sufficient to fix this issue, as the logical
constants are not dealt with correctly by the middle-end.

The following testcase shouldn't abort:
program test
  implicit none
  logical(1), parameter :: t1 = .TRUE., f1 = .FALSE.
  logical(2), parameter :: t2 = .TRUE., f2 = .FALSE.
  logical(4), parameter :: t4 = .TRUE., f4 = .FALSE.
  logical(8), parameter :: t8 = .TRUE., f8 = .FALSE.
  character*2 :: t(4), f(4)

  write(t(1),*) t1
  write(f(1),*) f1
  write(t(2),*) t2
  write(f(2),*) f2
  write(t(3),*) t4
  write(f(3),*) f4
  write(t(4),*) t8
  write(f(4),*) f8

  if (any(t .ne. " T")) call abort
  if (any(f .ne. " F")) call abort
end

It does, with or without my patch.  Without my patch it only fails for f(4),
i.e. for LOGICAL*8, with my patch it fails for f(2) through to f(4), i.e. for
all logical types which are not byte-sized.

While the .original-dump looks correct with my patch, and not correct without
it, the logical constants get merged in an unhealthy way:
        .align 4
.LC1:
        .long   1
        .align 4
.LC2:
        .long   0
(without my patch, with my patch the alignments disappear, which of course
explains why this fails for anything wider than 32 bits, or byte respectively)

To the potential middle-end hacker looking into this: Fortran's LOGICAL is a
boolean type which is defined as:
  new_type = make_unsigned_type (bit_size);
  TREE_SET_CODE (new_type, BOOLEAN_TYPE);
  TYPE_MAX_VALUE (new_type) = build_int_cst (new_type, 1);
  TYPE_PRECISION (new_type) = 1;
(from trans-types.c, around line 420)
bit_size is one of 8, 16, 32, or 64.
Comment 6 Tobias Schlüter 2005-01-21 23:32:11 UTC
Forgot to say this: an obvious fix would be not setting the type precision to 1
bit, but to the full width of the type.  This would of course somewhat defeat
the purpose of giving logicals a type of their own.
Comment 7 GCC Commits 2005-01-22 00:30:28 UTC
Subject: Bug 19543

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	tobi@gcc.gnu.org	2005-01-22 00:29:36

Modified files:
	gcc/fortran    : ChangeLog trans-const.c 

Log message:
	PR fortran/19543
	* trans-const.c (gfc_conv_constant_to_tree): Give logical
	constants the correct type.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/ChangeLog.diff?cvsroot=gcc&r1=1.303&r2=1.304
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/fortran/trans-const.c.diff?cvsroot=gcc&r1=1.20&r2=1.21

Comment 8 Tobias Schlüter 2005-01-22 00:52:51 UTC
Patch which implements the obvious workaround mentioned above:
http://gcc.gnu.org/ml/fortran/2005-01/msg00242.html

This doesn't fix the backend issue, so no patch keyword.
Comment 9 Jerry DeLisle 2006-02-04 05:55:03 UTC
Propose patch submitted to list for review.
Comment 10 Tobias Schlüter 2006-02-04 19:27:58 UTC
I'm changing the summary, and pushing this to component middle-end.  See comments #3 and #5 for an analysis of what's going wrong.
Comment 11 Roger Sayle 2006-02-20 00:34:17 UTC
Subject: Bug 19543

Author: sayle
Date: Mon Feb 20 00:34:12 2006
New Revision: 111294

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111294
Log:

	PR middle-end/19543
	* varasm.c (compare_constant) <INTEGER_CST>: Integer constants are
	only equivalent if the have both the same mode and precision.

	* gfortran.dg/logical_1.f90: New test case.


Added:
    trunk/gcc/testsuite/gfortran.dg/logical_1.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/varasm.c

Comment 12 Francois-Xavier Coudert 2006-02-20 16:13:00 UTC
*** Bug 20066 has been marked as a duplicate of this bug. ***
Comment 13 Roger Sayle 2006-03-14 19:25:29 UTC
Subject: Bug 19543

Author: sayle
Date: Tue Mar 14 19:25:25 2006
New Revision: 112065

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=112065
Log:

	PR middle-end/19543
	Backport from mainline.
	* varasm.c (compare_constant) <INTEGER_CST>: Integer constants are
	only equivalent if the have both the same mode and precision.

	* gfortran.dg/logical_1.f90: New test case.


Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/logical_1.f90
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/varasm.c

Comment 14 Andrew Pinski 2006-03-14 23:08:17 UTC
Fixed, thanks Roger for fixing this.