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.
> (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
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?
(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
(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
With the patch in comment #4, the code in #2 yields the same results as other compilers without regression.
> 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
(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
(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
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)
(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
I kept losing this because the title is no longer applicable. Paul
Have changed the keyword to represent comment #6 Paul
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.
This one keeps falling off the pending pile - unassigning myself for now. Paul
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
Fixed on trunk Thanks, Jakub Paul