[Bug fortran/33759] New: Unequal character lengths in MERGE intrinsic not detected in contained function.

[Bug fortran/33759] New: Unequal character lengths in MERGE intrinsic not detected in contained function.

[dominiq at lps dot ens dot fr]

In my comment 31 of PR31608, I have questionned the validity of the test case
in comment 28.  Now I think the test is invalid, but not detected. First
consider the code:

character(len=20) :: string
character(len=1)  :: tmp(20)
string = ""
tmp = ""
tmp = transfer(string,"x",len(string))
tmp = merge(tmp, string, .true.)
end

gfortran gives:

tmp = merge(tmp, string, .true.)
           1
Error: Unequal character lengths (1 and 20) in MERGE intrinsic at (1)

Note that an error based on the shape would probably better. When using arrays
of real, I get:

Error: Different shape for arguments 'tsource' and 'fsource' for intrinsic
'merge' at (1) on dimension 1 (3 and 2)

Now consider the code

  character(len=1)  :: string = "z"
  character(len=20) :: tmp = ""
  tmp = Upper ("abcdefg")
  print *, "'", tmp, "'"
 contains
  Character (len=20) Function Upper (string)
    Character(len=*) string
    character(len=1) :: tmp(20) = ""
    tmp = transfer(string,"x",len(string))
    tmp = merge(tmp, string, .true.)
    Upper = transfer(tmp, "x")
    return
  end function Upper
end

gfortran compiles it without complain and gives

 'a                   '

I think the above code is invalid in two counts:

(1) transfer(string,"x",len(string)) is a rank one array of size 7 and should
not be assigned to an array of size 20.  I think this is quite difficult to
detect at compile time, but it would be nice to have it at runtime.

(2) merge(tmp, string, .true.) try to merge a rank one character array with a
rank zero string. If this invalid and detected in the first test above, it
should also be detected in the function, unless I am missing something.


-- 
           Summary: Unequal character lengths in MERGE intrinsic not
                    detected in contained function.
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: dominiq at lps dot ens dot fr
 GCC build triplet: powerpc-apple-darwin8
  GCC host triplet: powerpc-apple-darwin8
GCC target triplet: powerpc-apple-darwin8


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[burnus at gcc dot gnu dot org]

------- Comment #1 from burnus at gcc dot gnu dot org  2007-10-12 20:54 -------
> (1) transfer(string,"x",len(string)) is a rank one array of size 7 and should
> not be assigned to an array of size 20.  I think this is quite difficult to
> detect at compile time, but it would be nice to have it at runtime.

NAG f95 agrees with this and prints at run time:
  Rank 1 of array operand has extent 7 instead of 20
  In UPPER, line 9 of test.f90

> (2) merge(tmp, string, .true.) try to merge a rank one character array with a
> rank zero string. If this invalid and detected in the first test above, it
> should also be detected in the function, unless I am missing something.

I think the rank is not a problem as MERGE is an ELEMENTAL function and a
scalar ("string") is conformable with any array (such as "tmp"). However, there
is the problem that the string length is not the same (1 and 7); NAG f95 prints
at run time:

  Differing character lengths (1 and 7) in MERGE
  In UPPER, line 10 of test.f90


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |27766
              nThis|                            |
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
  GCC build triplet|powerpc-apple-darwin8       |
   GCC host triplet|powerpc-apple-darwin8       |
 GCC target triplet|powerpc-apple-darwin8       |
           Keywords|                            |diagnostic
   Last reconfirmed|0000-00-00 00:00:00         |2007-10-12 20:54:19
               date|                            |
            Summary|Unequal character lengths in|Unequal character lengths in
                   |MERGE intrinsic not detected|MERGE intrinsic not detected
                   |in contained function.      |at run time


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[dominiq at lps dot ens dot fr]

------- Comment #2 from dominiq at lps dot ens dot fr  2007-10-12 22:18 -------
Subject: Re:  Unequal character lengths in MERGE intrinsic
 not detected at run time

> scalar ("string") is conformable with any array (such as "tmp")

Yes, I missed that, so if the length of string is only known at runtime,
it can only be caught at run time, however the Portland Group Fortran
detects it at compile time (constant folding?).

Now I have another odd result: the following code

  character(len=1)  :: string = "z"
  character(len=20) :: tmp = ""
  tmp = Upper ("abcdefgh")
  print *, tmp
 contains
  Character (len=20) Function Upper (string)
    Character(len=*) string
    print *, len(string)
    print *, size(transfer(string,"xy",len(string)))
    Upper = ""
    Upper(1:2) =                                                               
&
     transfer(merge(transfer(string,"xy",len(string)),    &
       string(1:2), .true.), "xy")
    return
  end function Upper
end

gives

 8
 8
 ab                  

with xlf, g95, Intel and PGF, but gfortran gives:

           8
           4
 ab                  

a bug or an ambiguity of the transfer definition?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[pault at gcc dot gnu dot org]

------- Comment #3 from pault at gcc dot gnu dot org  2007-10-23 15:52 -------
(In reply to comment #2)
...snip...
> Now I have another odd result: the following code
...snip...
>     print *, size(transfer(string,"xy",len(string)))

I believe that gfortran is incorrect:

In implementing "If the physical representation of the result is longer than
that of SOURCE, the physical representation of the leading part is that of
SOURCE and the remainder is undefined." I took the number of elements in result
to be the physical size of source.

However, "Case (iii): If SIZE is present, the result is array valued of rank
one and size SIZE." is nothing if not clear!

I'll figure out how to fix it.

Cheers

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2007-10-12 20:54:19         |2007-10-23 15:52:17
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[pault at gcc dot gnu dot org]

------- Comment #4 from pault at gcc dot gnu dot org  2007-10-23 16:08 -------
(In reply to comment #3)
> (In reply to comment #2)

> I'll figure out how to fix it.

This does the job:

Index: gcc/fortran/trans-intrinsic.c
===================================================================
*** gcc/fortran/trans-intrinsic.c       (revision 129504)
--- gcc/fortran/trans-intrinsic.c       (working copy)
*************** gfc_conv_intrinsic_array_transfer (gfc_s
*** 3171,3178 ****
      {
        tmp = fold_build2 (MULT_EXPR, gfc_array_index_type,
                         tmp, dest_word_len);
-       tmp = fold_build2 (MIN_EXPR, gfc_array_index_type,
-                        tmp, source_bytes);
      }
    else
      tmp = source_bytes;
--- 3171,3176 ----
*************** gfc_conv_intrinsic_array_transfer (gfc_s
*** 3229,3235 ****
                         3,
                         tmp,
                         fold_convert (pvoid_type_node, source),
!                        size_bytes);
    gfc_add_expr_to_block (&se->pre, tmp);

    se->expr = info->descriptor;
--- 3227,3233 ----
                         3,
                         tmp,
                         fold_convert (pvoid_type_node, source),
!                        source_bytes);
    gfc_add_expr_to_block (&se->pre, tmp);

    se->expr = info->descriptor;

and survives "dg.exp=gfortran/transfer*".  It is going to be a while before I
can deal with this, so please feel free.  (I cannot svn commit from where I
am.)

Paul


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[dominiq at lps dot ens dot fr]

------- Comment #5 from dominiq at lps dot ens dot fr  2007-10-24 11:36 -------
With the patch in comment #4, the code in #2 yields the same results as other
compilers without regression.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[dominiq at lps dot ens dot fr]

------- Comment #6 from dominiq at lps dot ens dot fr  2007-10-25 09:48 -------
> and survives "dg.exp=gfortran/transfer*".

Let me just point out that gfortran.dg/transfer_simplify_4.f90 fails
with any optimization starting at -O1 due to the failure of the reduced test:

! { dg-do run }
! { dg-options "-O0" }
! Tests that the in-memory representation of a transferred variable
! propagates properly.
!
  implicit none

  integer, parameter :: ip1 = 42
  logical, parameter :: ap1 = transfer(ip1, .true.)
  integer, parameter :: ip2 = transfer(ap1, 0)

  logical :: a
  integer :: i

  i = ip1
  a = transfer(i, .true.)
  i = transfer(a, 0)
  print *, i, ip1
  if (i .ne. ip1) call abort ()

end

[karma] f90/bug% gfc -O1 transfer_simplify_4_red.f90
[karma] f90/bug% a.out
           0          42
Abort


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[pault at gcc dot gnu dot org]

------- Comment #7 from pault at gcc dot gnu dot org  2007-11-12 15:03 -------
(In reply to comment #6)
> > and survives "dg.exp=gfortran/transfer*".
> Let me just point out that gfortran.dg/transfer_simplify_4.f90 fails
> with any optimization starting at -O1 due to the failure of the reduced test:

Note that this failure occurs without the patch and is a problem with the
scalar branch of transfer.  This patch is OK.

A peculiar feature of this failure is that 'i' can be changed to INTEGER(1,2,8)
and the test succeeds at any level of optimisation.  It is only at the default
logical length that it fails!

Puzzled

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[pault at gcc dot gnu dot org]

------- Comment #8 from pault at gcc dot gnu dot org  2007-11-12 15:34 -------
(In reply to comment #7)

> A peculiar feature of this failure is that 'i' can be changed to INTEGE(1,2,8)
> and the test succeeds at any level of optimisation.  It is only at the default
> logical length that it fails!
> Puzzled

For -O1 and above, .true. == 1_defaultint, whereas it is  .true. ==
>0_defaultint for no optimization.  Try setting ip1 to 1.

Paul


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[dominiq at lps dot ens dot fr]

------- Comment #9 from dominiq at lps dot ens dot fr  2007-11-12 16:48 -------
Subject: Re:  Unequal character lengths in MERGE intrinsic
 not detected at run time

> This patch is OK.

Yes indeed, I have applied it a long time ago. 

I have only pointed to the last bug on transfer I know of.

> It is only at the default logical length that it fails!

What is the default logical length with -m64? because it fails
only for integer(4) with or without -m64 (PPC Darwin8)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] Unequal character lengths in MERGE intrinsic not detected at run time

[pault at gcc dot gnu dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]



------- Comment #10 from pault at gcc dot gnu dot org  2007-11-13 12:46 -------
(In reply to comment #9)

> > This patch is OK.
> Yes indeed, I have applied it a long time ago.

As I found out minutes after I posted this note - thanks!

> I have only pointed to the last bug on transfer I know of.
> > It is only at the default logical length that it fails!

The optimizer is doing something horrible to the code in
transfer_simplify_4.f90; I have CC'd Andrew in the hope that he can shed some
light on it:

As an experiment, I tried the following patch to circumvent the mashing up of
the logical by BUILT_IN_MEMCPY when transferring between LOGICAL and INTEGER:

Index: gcc/fortran/trans-intrinsic.c
===================================================================
*** gcc/fortran/trans-intrinsic.c       (révision 130095)
--- gcc/fortran/trans-intrinsic.c       (copie de travail)
*************** gfc_conv_intrinsic_transfer (gfc_se * se
*** 3277,3282 ****
--- 3277,3291 ----
        se->expr = ptr;
        se->string_length = argse.string_length;
      }
+   else if (expr->value.function.actual->expr->ts.kind == expr->ts.kind
+            && ((expr->value.function.actual->expr->ts.type == BT_LOGICAL
+                       && expr->ts.type == BT_INTEGER)
+            ||  (expr->value.function.actual->expr->ts.type == BT_INTEGER
+                       && expr->ts.type == BT_LOGICAL)))
+     {
+       ptr = convert (build_pointer_type (type), ptr);
+       se->expr = build_fold_indirect_ref (ptr);
+     }
    else
      {
        tree moldsize;

The example in comment #6 now works at any level of optimization, even without
the PRINT statement (the reason why I mention this will be apparent in a
moment).

transfer_simplify_4.f90 itself fails in the third block, at -O2 and higher,
whereas it failed at all levels of optimization withhout the patch.

  a = transfer(ip1, .true.)
  i = transfer(a, 0)
  if (i .ne. ip1) call abort ()

Adding a PRINT *, a; or a PRINT *, i; anywhere in the program allows it to work
at any level of optimization.  This is true, even if the PRINT replaces one of
the call abort's.

I have looked to see what the i/o might be doing but cannot understand it at
all.

Cheers

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[pault at gcc dot gnu dot org]

------- Comment #11 from pault at gcc dot gnu dot org  2007-11-19 07:44 -------
I kept losing this because the title is no longer applicable.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Unequal character lengths in|ICE in
                   |MERGE intrinsic not detected|transfer_simplify_4.f90 at
                   |at run time                 |any level of optimization


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[pault at gcc dot gnu dot org]

------- Comment #12 from pault at gcc dot gnu dot org  2008-02-26 12:44 -------
Have changed the keyword to represent comment #6

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|diagnostic                  |ice-on-valid-code


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[fxcoudert at gcc dot gnu dot org]

------- Comment #13 from fxcoudert at gcc dot gnu dot org  2008-03-04 23:47 -------
It will probably be useless to others, who have already dug into this PR, but I
did have to research a bit to understand it, so I paste here the results of my
research... in case it can help.

-----------------------------------------
For the following reduced code:

  integer :: i
  i = 42
  print *, transfer(i, .true.)
  print *, transfer (transfer(i, .true.), 0)
  end

a.f90.013t.cfg has the (still valid):

  i.6 = i;
  transfer.4 = (logical(kind=4)) i.6;
  transfer.8 = transfer.4;
  D.876 = transfer.8;
  D.882 = D.876;
  transfer.5 = (integer(kind=4)) D.882;
  transfer.9 = transfer.5;
  D.878 = transfer.9;

a.f90.014t.cplxlower0 has made it into a pair of memcpy, but
a.f90.015t.veclower has them back in the form of two casts. a.f90.036t.dce1 has
them simplified into:

  transfer.4_9 = (logical(kind=4)) i_7;
  transfer.5_11 = (integer(kind=4)) transfer.4_9; 
  D.878 ={v} transfer.5_11;

a.f90.055t.copyrename2 still has that same form:

  transfer.4_9 = (logical(kind=4)) i_7;
  transfer.5_11 = (integer(kind=4)) transfer.4_9;
  D.878 = transfer.5_11;

but a.f90.056t.ccp2 definitely looses it:

  transfer.4_9 = 0;
  transfer.5_11 = 0;
  D.878 = 0;


So, the question is, is cpp2 right to change:

  i_7 = 42;
  transfer.4_9 = (logical(kind=4)) i_7;
  transfer.5_11 = (integer(kind=4)) transfer.4_9;
  D.878 = transfer.5_11;

into

  transfer.4_9 = 0;
  transfer.5_11 = 0;
  D.878 = 0;

I'm not sure what the semantics are for the logical kinds in the middle-end,
but it may be that only the least significant bit is used (if you change 42
into 43, you don't get the failure).

According to the related thread at
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/b29957a2f57fec25/27c2b4eca91c511f?lnk=gst&q=transfer+logical#27c2b4eca91c511f
correctly (comp.lang.fortran thread named "Transfer and variables that don't
use all their storage space.", started by... Tobias Burnus),
TRANSFER(TRANSFER(A,B),A) is legal and it's result should always be A (there
are some restrictions, but they are fulfilled in this precise case).
-----------------------------------------

Paul, to comment on your patch, it's not only integer and logical that are
concerned. You can do the same thing with logical and any other type, I'm
sure... and I'm even sure you could manage to do the same thing with a
real(kind=10), which has "unused bits" like a real type. (Or maybe, by playing
with NaN canonicalization, you could do it with any other floating-point type;
ie, choose an integer value that will TRANSFER into a given NaN, and the
middle-end might canonicalize this NaN or optimize it in sort weird way.) Also,
maybe the optimizer can sometimes be clever enough to see through your added
indirection level, can't it?

The only one way I see to do that would be to not create intermediate
variables. This is, after all, the meaning of the Fortran standard, that
although assignment can normalize (and array transfers are excluded from the
TRANSFER(TRANSFER(A,B),A) == A rule), scalar TRANSFERs without assignment don't
normalize. 


-- 

fxcoudert at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fxcoudert at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[pault at gcc dot gnu dot org]

------- Comment #14 from pault at gcc dot gnu dot org  2008-10-21 06:28 -------
This one keeps falling off the pending pile - unassigning myself for now.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|pault at gcc dot gnu dot org|unassigned at gcc dot gnu
                   |                            |dot org
             Status|ASSIGNED                    |NEW


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[jakub at gcc dot gnu dot org]

------- Comment #15 from jakub at gcc dot gnu dot org  2008-11-12 17:03 -------
Subject: Bug 33759

Author: jakub
Date: Wed Nov 12 17:01:51 2008
New Revision: 141790

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141790
Log:
        PR target/35366
        PR fortran/33759
        * fold-const.c (native_encode_string): New function.
        (native_encode_expr): Use it for STRING_CST.

        * trans-const.c (gfc_conv_constant_to_tree): Warn when
        converting an integer outside of LOGICAL's range to
        LOGICAL.
        * trans-intrinsic.c (gfc_conv_intrinsic_function,
        gfc_conv_intrinsic_array_transfer, gfc_conv_intrinsic_transfer):
        Use INTEGER_TYPE instead of BOOLEAN_TYPE for TRANSFER as
        argument of another TRANSFER.

        * gfortran.dg/hollerith.f90: Don't assume a 32-bit value
        stored into logical variable will be preserved.
        * gfortran.dg/transfer_simplify_4.f90: Remove undefined
        cases.  Run at all optimization levels.  Add a couple of
        new tests.
        * gfortran.dg/hollerith5.f90: New test.
        * gfortran.dg/hollerith_legacy.f90: Add dg-warning.

Added:
    trunk/gcc/testsuite/gfortran.dg/hollerith5.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-const.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/hollerith.f90
    trunk/gcc/testsuite/gfortran.dg/hollerith_legacy.f90
    trunk/gcc/testsuite/gfortran.dg/transfer_simplify_4.f90


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[pault at gcc dot gnu dot org]

------- Comment #16 from pault at gcc dot gnu dot org  2008-11-13 11:24 -------
Fixed on trunk

Thanks, Jakub

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759

[Bug fortran/33759] ICE in transfer_simplify_4.f90 at any level of optimization

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33759