array->data and ret->data are not divisible by four in this test case, yet we use ints for copying. Not too bad on a i686, very bad on systems that enforce alignment. $ cat cshift-char.f90 program main character(len=2) :: c2 character(len=4), dimension(2,2) :: a, b, c, d ! Force misalignment of a or b common /foo/ a, c, c2, b, d a = 'aa' b = 'bb' d = cshift(b,1) c = cshift(a,1) end program main $ gfortran -g cshift-char.f90 $ gdb ./a.out GNU gdb 6.8-debian Copyright (C) 2008 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i486-linux-gnu"... (gdb) b _gfortran_cshift0_4_char Function "_gfortran_cshift0_4_char" not defined. Make breakpoint pending on future shared library load? (y or [n]) y Breakpoint 1 (_gfortran_cshift0_4_char) pending. (gdb) r Starting program: /home/ig25/Krempel/Char/a.out Breakpoint 1, *_gfortran_cshift0_4_char (ret=0xbfc41720, ret_length=4, array=0xbfc41744, pshift=0x8048904, pdim=0x0, array_length=4) at ../../../../gcc/trunk/libgfortran/intrinsics/cshift0.c:364 364 DEFINE_CSHIFT (4); (gdb) p ret->data $1 = 0x8049ad2 "" (gdb) p array->data $2 = 0x8049ac2 "bb bb bb bb " (gdb)
Related to PR 32972 (optimizing cshift the way that other array intrinsics have been done will cure this).
Created attachment 16000 [details] proposed patch for cshift0 Here's something that works for cshift0. cshift1 still to do...
Created attachment 16001 [details] better patch This one is better :-)
> Created an attachment (id=16001) [edit] > better patch I think some of the run-time checks (with -fbounds-checks) for PR 36874 can be best done in libgfortran. How about including in your patch a check that the shape of ARRAY (minus dimension DIM) is conformable with the SHIFT argument? (i.e. that for every dimension SHIFT has the same number of elements as ARRAY.) I was additionally wondering whether one should change error code such as: if (which < 0 || (which + 1) > GFC_DESCRIPTOR_RANK (array)) into if (__builtin_expect (which < 0 || (which + 1) > GFC_DESCRIPTOR_RANK (array), 0)) to speed up the run time by a tiny bit.
(In reply to comment #4) > > Created an attachment (id=16001) [edit] > > better patch > > I think some of the run-time checks (with -fbounds-checks) for PR 36874 can be > best done in libgfortran. How about including in your patch a check that the > shape of ARRAY (minus dimension DIM) is conformable with the SHIFT argument? I've done so for other intrinsics (see PR 34670), and I plan to do so for cshift as well. Not with this patch, though; I like to do one thing at a time :-) > I was additionally wondering whether one should change error code such as: > if (which < 0 || (which + 1) > GFC_DESCRIPTOR_RANK (array)) > into > if (__builtin_expect (which < 0 || (which + 1) > GFC_DESCRIPTOR_RANK (array), > 0)) > to speed up the run time by a tiny bit. Sure, we could do that for all of the run-time checks.
Subject: Bug 36886 Author: tkoenig Date: Thu Aug 14 18:31:32 2008 New Revision: 139111 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=139111 Log: 2008-08-14 Thomas Koenig <tkoenig@gcc.gnu.org> PR libfortran/36886 * Makefile.am: Added $(i_cshift0_c). Added $(i_cshift0_c) to gfor_built_specific_src. Add rule to build from cshift0.m4. * Makefile.in: Regenerated. * libgfortran.h: Addedd prototypes for cshift0_i1, cshift0_i2, cshift0_i4, cshift0_i8, cshift0_i16, cshift0_r4, cshift0_r8, cshift0_r10, cshift0_r16, cshift0_c4, cshift0_c8, cshift0_c10, cshift0_c16. Define Macros GFC_UNALIGNED_C4 and GFC_UNALIGNED_C8. * intrinsics/cshift0.c: Remove helper functions for the innter shift loop. (cshift0): Call specific functions depending on type of array argument. Only call specific functions for correct alignment for other types. * m4/cshift0.m4: New file. * generated/cshift0_i1.c: New file. * generated/cshift0_i2.c: New file. * generated/cshift0_i4.c: New file. * generated/cshift0_i8:.c New file. * generated/cshift0_i16.c: New file. * generated/cshift0_r4.c: New file. * generated/cshift0_r8.c: New file. * generated/cshift0_r10.c: New file. * generated/cshift0_r16.c: New file. * generated/cshift0_c4.c: New file. * generated/cshift0_c8.c: New file. * generated/cshift0_c10.c: New file. * generated/cshift0_c16.c: New file. 2008-08-14 Thomas Koenig <tkoenig@gcc.gnu.org> PR libfortran/36886 * gfortran.dg/cshift_char_3.f90: New test case. * gfortran.dg/cshift_nan_1.f90: New test case. Added: trunk/gcc/testsuite/gfortran.dg/char_cshift_3.f90 trunk/gcc/testsuite/gfortran.dg/cshift_nan_1.f90 trunk/libgfortran/generated/cshift0_c10.c trunk/libgfortran/generated/cshift0_c16.c trunk/libgfortran/generated/cshift0_c4.c trunk/libgfortran/generated/cshift0_c8.c trunk/libgfortran/generated/cshift0_i1.c trunk/libgfortran/generated/cshift0_i16.c trunk/libgfortran/generated/cshift0_i2.c trunk/libgfortran/generated/cshift0_i4.c trunk/libgfortran/generated/cshift0_i8.c trunk/libgfortran/generated/cshift0_r10.c trunk/libgfortran/generated/cshift0_r16.c trunk/libgfortran/generated/cshift0_r4.c trunk/libgfortran/generated/cshift0_r8.c trunk/libgfortran/m4/cshift0.m4 Modified: trunk/gcc/testsuite/ChangeLog trunk/libgfortran/ChangeLog trunk/libgfortran/Makefile.am trunk/libgfortran/Makefile.in trunk/libgfortran/intrinsics/cshift0.c trunk/libgfortran/libgfortran.h
Fixed on trunk. Closing.