Bug 29410 - [4.2 only] bug with TRANSFER() and -O2
Summary: [4.2 only] bug with TRANSFER() and -O2
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.2.0
Assignee: Andrew Pinski
URL:
Keywords: alias, wrong-code
Depends on: 29951
Blocks:
  Show dependency treegraph
 
Reported: 2006-10-10 09:13 UTC by Dominique d'Humieres
Modified: 2007-01-23 05:13 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-10-18 20:50:31


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique d'Humieres 2006-10-10 09:13:13 UTC
The following code:

program chop
  integer ix, iy
  real x, y
  x = 1.
  y = x
  ix = transfer(x,ix)
  iy = transfer(y,iy)
  print '(2z20.8)', ix, iy
  if (ix /= iy) call abort
end program chop

when compiled with -O1 (or below) on PPC OSX 10.3, gives the correct result:

            3F800000            3F800000

but when compiled with -O2 (or above) gives:

            8FE15C3C            8FE52E41

This bug does not show up on Linux AMD64, gfortran 4.2.0 20061009.

Note 1, if I replace

  real x, y

by

  real o, t, x, y

or

  real o, t, td, x, y

I get (with -O2) respectively:

            BFFFEFA0            8FE52E41

or

            8FE52E41            BFFFEFA0

Note 2, on OSX 10.4, gfortran 4.2.0 20060610, I get

            00002D44            003006D0

TIA

Dominique
Comment 1 Francois-Xavier Coudert 2006-10-18 20:50:15 UTC
Confirmed, and Andrew will soon have a patch for this.
Comment 2 Andrew Pinski 2006-10-29 22:15:59 UTC
The problem is that gfc_conv_intrinsic_transfer does *(int*)&float_var which causes a violation of aliasing rules.
Comment 3 Andrew Pinski 2006-10-29 22:36:16 UTC
Here is the fix which I am testing right now:
Index: trans-intrinsic.c
===================================================================
--- trans-intrinsic.c   (revision 118159)
+++ trans-intrinsic.c   (working copy)
@@ -2914,7 +2914,7 @@ gfc_conv_intrinsic_array_transfer (gfc_s


 /* Scalar transfer statement.
-   TRANSFER (source, mold) = *(typeof<mold> *)&source.  */
+   TRANSFER (source, mold) = VIEW_CONVERT_EXPR<typeof<mold> >source.  */

 static void
 gfc_conv_intrinsic_transfer (gfc_se * se, gfc_expr * expr)
@@ -2939,9 +2939,9 @@ gfc_conv_intrinsic_transfer (gfc_se * se

   arg = arg->next;
   type = gfc_typenode_for_spec (&expr->ts);
-  ptr = convert (build_pointer_type (type), ptr);
   if (expr->ts.type == BT_CHARACTER)
     {
+      ptr = convert (build_pointer_type (type), ptr);
       gfc_init_se (&argse, NULL);
       gfc_conv_expr (&argse, arg->expr);
       gfc_add_block_to_block (&se->pre, &argse.pre);
@@ -2951,7 +2951,8 @@ gfc_conv_intrinsic_transfer (gfc_se * se
     }
   else
     {
-      se->expr = build_fold_indirect_ref (ptr);
+      tree tmp = build_fold_indirect_ref (ptr);
+      se->expr = fold_build1 (VIEW_CONVERT_EXPR, type, tmp);
     }
 }

Comment 4 Andrew Pinski 2006-10-30 16:15:24 UTC
Subject: Bug 29410

Author: pinskia
Date: Mon Oct 30 16:15:09 2006
New Revision: 118186

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=118186
Log:
2006-10-30  Andrew Pinski  <pinskia@gmail.com>

        PR fortran/29410
        * trans-intrinsic.c (gfc_conv_intrinsic_array_transfer):
        Change over to create VIEW_CONVERT_EXPR instead of using an
        ADDR_EXPR, a cast and then an indirect reference
2006-10-30  Andrew Pinski  <pinskia@gmail.com>

        PR Fortran/29410
        * gfortran.fortran-torture/execute/transfer1.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.fortran-torture/execute/transfer1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog

Comment 5 Andrew Pinski 2006-11-07 19:34:07 UTC
apinski hi
apinski PaulT: VCE means *(type*)&a without the need for addressable and does not confuse aliasing
PaulT apinski: Hi, yes I answered the question myself - I missed it in tree.def by only looking in tree.h
apinski PaulT: this is mention in tree.def :)
PaulT apinski: It's undefined if the types have different lengths?
apinski yes
apinski actually is transfer ok for different size types?
PaulT Yes, I was just racing for the standard.
apinski ok
apinski I guess I have to change my patch then
apinski to include a size comparision
apinski The physical representation of the result is the same as SOURCE, truncated if the result is smaller or with an undefined trailing portion if the result is larger.
apinski that is from IBM's docs about transfer
PaulT apinski: Gee, yes. I missed that completely.
apinski I can improve the truncated part also
PaulT If I change the integers in your example to integer(8), I get a core dump. 
apinski during running?
PaulT g95 and ifort do the right thing.
PaulT Shucks, I'm sorry about that Andrew, I missed it completely.
apinski I figured it was going to be wrong
apinski lucky I only applied to the mainline :)
apinski can you write something in the bug report so I don't forget to fix it this weekend?
PaulT apinski: I keep telling people that that is what it is for!
PaulT apinski: Sure thing - I suppose that you can safely do a kind conversion?
apinski I know how to fix this correctly now
apinski use __builtin_memcpy and hope that gets optimized correctly in the future :)
PaulT apinski: make type of the same length as source, so that the assignment does the rest?
apinski no
apinski PaulT: do memcpy (dst, src, min(size1, size0))
PaulT apinski: No... you're right that won't present a bit-wise transfer.
Comment 6 Andrew Pinski 2006-11-07 19:34:48 UTC
My patch is incorrect for transfer with different sizes.
Comment 7 Andrew Pinski 2006-11-07 19:35:46 UTC
In fact I think the orginal way is incorrect for cases where the transfer size is larger than the orginal size of the decl.
Comment 8 Andrew Pinski 2006-11-12 01:31:43 UTC
Actually it turns out using VIEW_CONVERT_EXPR is ok and works correctly as far as I can tell.
Comment 9 Andrew Pinski 2006-11-24 22:48:23 UTC
It turns out my fix caused PR 29951 which I am testing a fix for PR 29951 now.  My new patch does not break this testcase which is a good sign.
Comment 10 Paul Thomas 2007-01-14 11:19:27 UTC
(In reply to comment #9)
> It turns out my fix caused PR 29951 which I am testing a fix for PR 29951 now. 
> My new patch does not break this testcase which is a good sign.
> 
Andrew,

Were you going to apply this to 4.2, together with revision 119211, or will you close them as fixed on 4.3?

Paul
Comment 11 pinskia@gmail.com 2007-01-22 01:08:41 UTC
Subject: Re:  [4.2 only] bug with TRANSFER() and -O2

On Sun, 2007-01-14 at 11:19 +0000, pault at gcc dot gnu dot org wrote:
> 
> Were you going to apply this to 4.2, together with revision 119211, or
> will you
> close them as fixed on 4.3? 

I am going to test it for 4.2 once I finish testing an objective-C
patch.

-- Pinski

Comment 12 Andrew Pinski 2007-01-23 05:13:41 UTC
Fixed.
Comment 13 Andrew Pinski 2007-01-23 05:15:34 UTC
Subject: Bug 29410

Author: pinskia
Date: Tue Jan 23 05:15:21 2007
New Revision: 121076

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121076
Log:
2007-01-22  Andrew Pinski  <pinskia@gmail.com>

        PR fortran/29951
        * gfortran.fortran-torture/execute/transfer2.f90: New test.

        PR Fortran/29410
        * gfortran.fortran-torture/execute/transfer1.f90: New test.
2007-01-22 Andrew Pinski  <pinskia@gmail.com>

        PR fortran/29951
        * trans-intrinsic.c (gfc_conv_intrinsic_transfer): Change to
        call memcpy instead of creating a VIEW_CONVERT_EXRP.

        PR fortran/29410
        * trans-intrinsic.c (gfc_conv_intrinsic_array_transfer):
        Change over to create VIEW_CONVERT_EXPR instead of using an
        ADDR_EXPR, a cast and then an indirect reference.


Added:
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.fortran-torture/execute/transfer1.f90
      - copied unchanged from r118186, trunk/gcc/testsuite/gfortran.fortran-torture/execute/transfer1.f90
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.fortran-torture/execute/transfer2.f90
      - copied unchanged from r119211, trunk/gcc/testsuite/gfortran.fortran-torture/execute/transfer2.f90
Modified:
    branches/gcc-4_2-branch/gcc/fortran/ChangeLog
    branches/gcc-4_2-branch/gcc/fortran/trans-intrinsic.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog