Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug
Bug#: 19543
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Francois-Xavier Coudert <fxcoudert@gcc.gnu.org>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 19543 depends on: 20066 Show dependency tree
Show dependency graph
Bug 19543 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2005-12-30 19:00 Opened: 2005-01-20 10:31
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 From Andrew Pinski 2005-01-20 15:35 -------
  int4 C.417 = 0;
  int4 C.416 = 0;

  _gfortran_transfer_logical (&C.417, 8);


------- Comment #2 From Tobias Schlüter 2005-01-21 21:57 -------
This only happens for parameters.

------- Comment #3 From Tobias Schlüter 2005-01-21 22:21 -------
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 From Tobias Schlüter 2005-01-21 22:37 -------
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 From Tobias Schlüter 2005-01-21 22:58 -------
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 From Tobias Schlüter 2005-01-21 23:32 -------
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 From CVS Commits 2005-01-22 00:30 -------
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 From Tobias Schlüter 2005-01-22 00:52 -------
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 From Jerry DeLisle 2006-02-04 05:55 -------
Propose patch submitted to list for review.

------- Comment #10 From Tobias Schlüter 2006-02-04 19:27 -------
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 From Roger Sayle 2006-02-20 00:34 -------
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 From Francois-Xavier Coudert 2006-02-20 16:13 -------
*** Bug 20066 has been marked as a duplicate of this bug. ***

------- Comment #13 From Roger Sayle 2006-03-14 19:25 -------
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 From Andrew Pinski 2006-03-14 23:08 -------
Fixed, thanks Roger for fixing this.

Bug List: (This bug is not in your last search results)   Show last search results      Search page      Enter new bug