[PATCH, fortran/52879] RANDOM_SEED revisited
All,
Here is a potentially contentious patch for PR fortran/52879.
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52879
As demonstrated in that PR, it is possible to provide RANDOM_SEED
with sets of seeds that result in a very poor sequences of PRN.
Gfortran's RANDOM_NUMBER uses 3 independent KISS PRNG to generate
the bits of the significands of the real PRN. Each KISS PRNG
requires 4 seeds. This patch removes the need for a user to
specify 12 seeds. It uses a Lehmer linear congruent generator
to generate the 12 KISS seeds. This LCG requires a single seed.
I have selected to have seed(1)=0 correspond to the current
default set of KISS seeds. Internally, the LCG declares the
seed as a GFC_UINTEGER_8 (ie., uint64_t) type, so for gfortran's
default integer kind one has 2**31-1 possible seeds and if one
use -fdefault-integer-8 then one has 2**32 possible seeds.
It is possible to extend the patch to allow a skip count such that
for example seed(2)=10 would use every tenth value in the LCG
sequence. I haven't pursued this as it appears to be an unneeded
complication.
The only testsuite failure is for gfortran.dg/random_seed_1.f90,
which is a test designed to specifically test for the 12 KISS
seeds. This test will be removed if the patch is OK.
--
Steve
Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 207633)
+++ gcc/fortran/check.c (working copy)
@@ -4925,16 +4925,9 @@ gfc_check_random_number (gfc_expr *harve
bool
gfc_check_random_seed (gfc_expr *size, gfc_expr *put, gfc_expr *get)
{
- unsigned int nargs = 0, kiss_size;
+ unsigned int nargs = 0;
locus *where = NULL;
mpz_t put_size, get_size;
- bool have_gfc_real_16; /* Try and mimic HAVE_GFC_REAL_16 in libgfortran. */
-
- have_gfc_real_16 = gfc_validate_kind (BT_REAL, 16, true) != -1;
-
- /* Keep the number of bytes in sync with kiss_size in
- libgfortran/intrinsics/random.c. */
- kiss_size = (have_gfc_real_16 ? 48 : 32) / gfc_default_integer_kind;
if (size != NULL)
{
@@ -4975,13 +4968,6 @@ gfc_check_random_seed (gfc_expr *size, g
if (!kind_value_check (put, 1, gfc_default_integer_kind))
return false;
-
- if (gfc_array_size (put, &put_size)
- && mpz_get_ui (put_size) < kiss_size)
- gfc_error ("Size of '%s' argument of '%s' intrinsic at %L "
- "too small (%i/%i)",
- gfc_current_intrinsic_arg[1]->name, gfc_current_intrinsic,
- where, (int) mpz_get_ui (put_size), kiss_size);
}
if (get != NULL)
@@ -5007,13 +4993,6 @@ gfc_check_random_seed (gfc_expr *size, g
if (!kind_value_check (get, 2, gfc_default_integer_kind))
return false;
-
- if (gfc_array_size (get, &get_size)
- && mpz_get_ui (get_size) < kiss_size)
- gfc_error ("Size of '%s' argument of '%s' intrinsic at %L "
- "too small (%i/%i)",
- gfc_current_intrinsic_arg[2]->name, gfc_current_intrinsic,
- where, (int) mpz_get_ui (get_size), kiss_size);
}
/* RANDOM_SEED may not have more than one non-optional argument. */
Index: libgfortran/intrinsics/random.c
===
--- libgfortran/intrinsics/random.c (revision 207633)
+++ libgfortran/intrinsics/random.c (working copy)
@@ -224,9 +224,20 @@ KISS algorithm. */
z=0,c=0 and z=2^32-1,c=698769068
should be avoided. */
-/* Any modifications to the seeds that change kiss_size below need to be
- reflected in check.c (gfc_check_random_seed) to enable correct
- compile-time checking of PUT size for the RANDOM_SEED intrinsic. */
+/* The 3 KISS generators require 3 sets for 4 seeds. It is possible for a
+ a user to specify a set of seeds that is inadequate for reseeding the
+ generators. For example, change only the value of one of the 12 seeds
+ may not reset the PRNG sequences. To work-around this issue, use a
+ simple Lehmer linear congruential generator that requires only a single
+ seed to generate the 12 KISS seeds. */
+
+static const GFC_INTEGER_4 lehmer_lcg_size = 1;
+static GFC_UINTEGER_8 lehmer_lcg_seed = 0;
+static GFC_UINTEGER_4
+lehmer_lcg(GFC_UINTEGER_4 a)
+{
+return ((GFC_UINTEGER_8)a * 279470273UL) % 4294967291UL;
+}
#define KISS_DEFAULT_SEED_1 123456789, 362436069, 521288629, 316191069
#define KISS_DEFAULT_SEED_2 987654321, 458629013, 582859209, 438195021
@@ -636,28 +647,6 @@ arandom_r16 (gfc_array_r16 *x)
#endif
-
-static void
-scramble_seed (unsigned char *dest, unsigned char *src, int size)
-{
- int i;
-
- for (i = 0; i < size; i++)
-dest[(i % 2) * (size / 2) + i / 2] = src[i];
-}
-
-
-static void
-unscramble_seed (unsigned char *dest, unsigned char *src, int size)
-{
- int i;
-
- for (i = 0; i < size; i++)
-dest[i] = src[(i % 2) * (size / 2) + i / 2];
-}
-
-
-
/* random_seed is used to seed the PRNG with either a default
Re: [Patch, fortran] PR34928 - Extension: volatile common blocks
2014-02-08 22:47 GMT+01:00 Steve Kargl : > On Sat, Feb 08, 2014 at 10:28:24PM +0100, Dominique Dhumieres wrote: >> > I can't remember. Do you have commit privilege? >> >> No. >> >> > If not, why? >> >> Probably because I did not asked for it explicitly. >> I have only hinted a couple time that I have the FSF papers >> signed. > > I suppose we can fix that issue. I forget the procedure on > getting 'write after approval' access. >From http://gcc.gnu.org/svnwrite.html#policies: "The steering committee or a well-established GCC maintainer (including reviewers) can approve for write access any person with GNU copyright assignment papers in place and known to submit good patches." That sounds like approval of any Fortran reviewer should be sufficient (which includes Steve and myself). I definitely support Dominique's approval as a write-after-approval contributor. Dominique, please add yourself to the corresponding section of the MAINTAINERS file! Cheers, Janus
Re: [Patch, fortran] PR34928 - Extension: volatile common blocks
On Sat, Feb 08, 2014 at 10:28:24PM +0100, Dominique Dhumieres wrote: > > I can't remember. Do you have commit privilege? > > No. > > > If not, why? > > Probably because I did not asked for it explicitly. > I have only hinted a couple time that I have the FSF papers > signed. > I suppose we can fix that issue. I forget the procedure on getting 'write after approval' access. I'll ask on IRC later (need to re-install xchat). -- Steve
Re: [Patch, fortran] PR34928 - Extension: volatile common blocks
> I can't remember. Do you have commit privilege? No. > If not, why? Probably because I did not asked for it explicitly. I have only hinted a couple time that I have the FSF papers signed. I'll do the changes. Thanks for the quick review, Dominique
Re: [PATCH, rs6000] Remove most -mcall- options from consideration in ENDIAN_SELECT
On Sat, Feb 8, 2014 at 11:44 AM, Ulrich Weigand wrote:
> Hello,
>
> using the -mcall-linux option on powerpc64le-linux (which should be a no-op)
> currently causes the compiler driver to pass -mbig to the linker. Likewise,
> using -mcall-aixdesc (which is allowed when also using -mabi=elfv1, like
> certain versions of the Linux kernel do), causes the compiler driver to
> pass -mbig to the linker.
>
> While in both cases the incorrect effect can be worked around by using an
> explicit -mlittle-endian, this should not be required in a compiler built
> for the powerpc64le-linux target.
>
> The reason for this behaviour is use of an ENDIAN_SELECT statement in
> ASM_SPEC as defined in sysv4.h, and this ENDIAN_SELECT macro hard-codes
> certain byte orders in response to various -mcall- options:
> big-endian for -mcall-aixdesc, -mcall-{free,net,open}bsd and -mcall-linux;
> little-endian for -mcall-i960-old.
>
> Interestingly enough, for the big-endian cases above, this affects only
> the *assembler*; the compiler itself will not switch endianness. However,
> for -mcall-i960-old, there is code in SUBTARGET_OVERRIDE_OPTIONS to also
> switch the compiler's byte order.
>
> I'm not 100% sure what to do about -mcall-i960-old, but at least the
> behaviour is self-consistent, so I left it alone for now.
>
> However, -mcall-aixdesc, -mcall-linux (and possibly in the future
> also -mcall-*bsd) should clearly not imply a particular byte order,
> and don't work in a consistent manner on little-endian systems today
> anyway, so the patch below removes this clause from ENDIAN_SELECT.
>
> As a side-effect, I noticed that there apparently was some attempt
> in the past to pass endian options to cc1, as evidenced by specs
> substrings like %cc1_endian_default. However, the current set of
> header files never defines any of those specs to any non-empty
> value, so this patch removes all of them as well.
>
> Tested on powerpc64-linux (-m64/-m32) and powerpc64le-linux.
>
> OK for mainline?
>
> Bye,
> Ulrich
>
>
> ChangeLog:
>
> * config/rs6000/sysv4.h (ENDIAN_SELECT): Do not attempt to enforce
> big-endian mode for -mcall-aixdesc, -mcall-freebsd, -mcall-netbsd,
> -mcall-openbsd, or -mcall-linux.
> (CC1_ENDIAN_BIG_SPEC): Remove.
> (CC1_ENDIAN_LITTLE_SPEC): Remove.
> (CC1_ENDIAN_DEFAULT_SPEC): Remove.
> (CC1_SPEC): Remove (always empty) %cc1_endian_... spec.
> (SUBTARGET_EXTRA_SPECS): Remove %cc1_endian_big, %cc1_endian_little,
> and %cc1_endian_default.
> * config/rs6000/sysv4le.h (CC1_ENDIAN_DEFAULT_SPEC): Remove.
Okay.
Thanks for cleaning this up.
Thanks, David
Re: [Patch, fortran] PR34928 - Extension: volatile common blocks
On Sat, Feb 08, 2014 at 10:07:23PM +0100, Dominique Dhumieres wrote:
> Is the following patch OK?
>
> Dominique
>
> 2014-02-08 Dominique d'Humieres
>
> PR fortran/34928
> * fortran/gfortran.texi: Document Volatile COMMON as not
> suppoerted.
s/suppoerted/supported
> --- ../_clean/gcc/fortran/gfortran.texi 2014-01-04 15:51:42.0
> +0100
> +++ gcc/fortran/gfortran.texi 2014-02-03 15:33:50.0 +0100
> @@ -1990,6 +1990,7 @@ code that uses them running with the GNU
> @c * CARRIAGECONTROL, DEFAULTFILE, DISPOSE and RECORDTYPE I/O specifiers::
> @c * Omitted arguments in procedure call::
> * Alternate complex function syntax::
> +* Volatile COMMON blocks::
> @end menu
>
>
> @@ -2184,6 +2185,18 @@ extensions. @command{gfortran} accepts
> common, but not the former.
>
>
> +@node Volatile COMMON blocks
> +@subsection Volatile @code{COMMON} blocks
> +@cindex @code{VOLATILE}
> +@cindex @code{COMMON}
> +
> +Some Fortran compilers, including @command{g77}, let the user declare
> +@code{COMMON} with the @code{VOLATILE} attribute. This is
> +invalid standard Fortran 77/90/95/2003/2008 syntax and is not
I would remove 77/90/95/2003/2008.
> +supported by @command{gfortran}. Note that @command{gfortran} accepts
> +VOLATILE variables in COMMON blocks since revision 4.3.
With these minor changes, looks fine to me.
I can't remember. Do you have commit privilege? If not, why?
You've certainly proven yourself with your attention to bugs
and testing.
--
Steve
[Patch, fortran] PR34928 - Extension: volatile common blocks
Is the following patch OK?
Dominique
2014-02-08 Dominique d'Humieres
PR fortran/34928
* fortran/gfortran.texi: Document Volatile COMMON as not
suppoerted.
--- ../_clean/gcc/fortran/gfortran.texi 2014-01-04 15:51:42.0 +0100
+++ gcc/fortran/gfortran.texi 2014-02-03 15:33:50.0 +0100
@@ -1990,6 +1990,7 @@ code that uses them running with the GNU
@c * CARRIAGECONTROL, DEFAULTFILE, DISPOSE and RECORDTYPE I/O specifiers::
@c * Omitted arguments in procedure call::
* Alternate complex function syntax::
+* Volatile COMMON blocks::
@end menu
@@ -2184,6 +2185,18 @@ extensions. @command{gfortran} accepts
common, but not the former.
+@node Volatile COMMON blocks
+@subsection Volatile @code{COMMON} blocks
+@cindex @code{VOLATILE}
+@cindex @code{COMMON}
+
+Some Fortran compilers, including @command{g77}, let the user declare
+@code{COMMON} with the @code{VOLATILE} attribute. This is
+invalid standard Fortran 77/90/95/2003/2008 syntax and is not
+supported by @command{gfortran}. Note that @command{gfortran} accepts
+VOLATILE variables in COMMON blocks since revision 4.3.
+
+
@c -
@c Mixed-Language Programming
Re: [Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
Le 08/02/2014 18:48, Paul Richard Thomas a écrit : > Dear All, > > This is another corner of a corner case that is not a regression but > fixes a wrong-code-on-valid. > > At present, an actual argument of an elemental procedure with the > VALUE attribute is not passed by value. The fix is obvious. > > Bootstraps and regtests on FC17/x86_64 - OK for trunk, 4.8 and 4.7? > ... and gfc_conv_procedure_call continues to grow. :-( It's OK from a fortran point of view. Mikael
[Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
Dear All,
This is another corner of a corner case that is not a regression but
fixes a wrong-code-on-valid.
At present, an actual argument of an elemental procedure with the
VALUE attribute is not passed by value. The fix is obvious.
Bootstraps and regtests on FC17/x86_64 - OK for trunk, 4.8 and 4.7?
Cheers
Paul
2014-02-08 Paul Thomas
PR fortran/59026
* trans-expr.c (gfc_conv_procedure_call): Pass the value of the
actual argument to a formal argument with the value attribute
in an elemental procedure.
2014-02-08 Paul Thomas
PR fortran/59026
* gfortran.dg/elemental_by_value_1.f90 : New test
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c(revision 205031)
--- gcc/fortran/trans-expr.c(working copy)
*** gfc_conv_procedure_call (gfc_se * se, gf
*** 4050,4056
--- 4050,4060
gfc_init_se (&parmse, se);
parm_kind = ELEMENTAL;
+ if (fsym && fsym->attr.value)
+ gfc_conv_expr (&parmse, e);
+ else
gfc_conv_expr_reference (&parmse, e);
+
if (e->ts.type == BT_CHARACTER && !e->rank
&& e->expr_type == EXPR_FUNCTION)
parmse.expr = build_fold_indirect_ref_loc (input_location,
Index: gcc/testsuite/gfortran.dg/elemental_by_value_1.f90
===
*** gcc/testsuite/gfortran.dg/elemental_by_value_1.f90 (revision 0)
--- gcc/testsuite/gfortran.dg/elemental_by_value_1.f90 (working copy)
***
*** 0
--- 1,22
+ ! { dg-do run }
+ !
+ ! PR fortran/59026
+ !
+ ! Contributed by F-X Coudert
+ !
+ ! Failed to dereference the argument in scalarized loop.
+ !
+ elemental integer function foo(x)
+ integer, value :: x
+ foo = x + 1
+ end function
+
+ interface
+ elemental integer function foo(x)
+ integer, value :: x
+ end function
+ end interface
+
+ if (foo(42) .ne. 43) call abort
+ if (any (foo([0,1]) .ne. [1,2])) call abort
+ end
Re: [RFC] PR 59776 - esra vs gimple_debug
Hi,
On Fri, Feb 07, 2014 at 04:37:22PM -0800, Richard Henderson wrote:
> On 02/07/2014 03:12 PM, Richard Biener wrote:
> > On February 7, 2014 8:35:16 PM GMT+01:00, Richard Henderson
> > wrote:
> >> In the testcases with the PR, we have a bit of type punning going on,
> >>
> >> *(int *) &s2.f = 0;
> >> s2 = s1;
> >>
> >> which SRA trasforms to
> >>
> >> # DEBUG s2 => 0
> >> MEM[(int *)&s2] = 0;
> >> # DEBUG s2 => s1$f_7
> >> # DEBUG s2$g => s1$g_6
> >> s2 ={v} {CLOBBER};
> >>
> >> Note that it has chosen not to expand s1.f like s1.g, but to expand
> >> that field
> >> as the type-punned integer. Which means that "s2 => s1$f_7" has
> >> mismatched
> >> types across lhs and rhs: SI => SF. Which understandibly ICEs during
> >> rtl
> >> expansion.
> >>
> >> I'm not really sure how this is avoided for the actual code generation,
> >> but
> >> this minimal patch (aka hack) simply drops the debug info to avoid the
> >> ICE.
> >>
> >> Thoughts on how this might really be solved?
> >
> > Add a VIEW_CONVERT_EXPR around the rhs of the debug statement.
>
> Well, ok, though I'm pretty sure that the debug info will pretty much barf on
> that immediately.
>
> What I really meant is: where's a better place to put this check, since such a
> check _must_ exist somewhere else for the regular code generation.
>
That is what SRA does for regular code, see the useless_type_conversion_p
check earlier in the same function. If you want me to, I can prepare
the patch (at some point next week).
Thanks for analysis of the bug,
Martin
Re: [Patch, microblaze]: Add optimized lshrsi3
On 11/25/13 23:53, David Holsgrove wrote: Add optimized lshrsi3 instruction, to be used when optimizing for size with immediate values over 5 Changelog 2013-11-26 Nagaraju Mekala * gcc/config/microblaze/microblaze.md: Add size optimized lshrsi3 insn. David -- Please put the description of the patch in the text of the email, rather than hiding it within an attached patch. The patch describes a very specific situation where this patch will have an effect. Please provide a test case. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
[PATCH, rs6000] Remove most -mcall- options from consideration in ENDIAN_SELECT
Hello,
using the -mcall-linux option on powerpc64le-linux (which should be a no-op)
currently causes the compiler driver to pass -mbig to the linker. Likewise,
using -mcall-aixdesc (which is allowed when also using -mabi=elfv1, like
certain versions of the Linux kernel do), causes the compiler driver to
pass -mbig to the linker.
While in both cases the incorrect effect can be worked around by using an
explicit -mlittle-endian, this should not be required in a compiler built
for the powerpc64le-linux target.
The reason for this behaviour is use of an ENDIAN_SELECT statement in
ASM_SPEC as defined in sysv4.h, and this ENDIAN_SELECT macro hard-codes
certain byte orders in response to various -mcall- options:
big-endian for -mcall-aixdesc, -mcall-{free,net,open}bsd and -mcall-linux;
little-endian for -mcall-i960-old.
Interestingly enough, for the big-endian cases above, this affects only
the *assembler*; the compiler itself will not switch endianness. However,
for -mcall-i960-old, there is code in SUBTARGET_OVERRIDE_OPTIONS to also
switch the compiler's byte order.
I'm not 100% sure what to do about -mcall-i960-old, but at least the
behaviour is self-consistent, so I left it alone for now.
However, -mcall-aixdesc, -mcall-linux (and possibly in the future
also -mcall-*bsd) should clearly not imply a particular byte order,
and don't work in a consistent manner on little-endian systems today
anyway, so the patch below removes this clause from ENDIAN_SELECT.
As a side-effect, I noticed that there apparently was some attempt
in the past to pass endian options to cc1, as evidenced by specs
substrings like %cc1_endian_default. However, the current set of
header files never defines any of those specs to any non-empty
value, so this patch removes all of them as well.
Tested on powerpc64-linux (-m64/-m32) and powerpc64le-linux.
OK for mainline?
Bye,
Ulrich
ChangeLog:
* config/rs6000/sysv4.h (ENDIAN_SELECT): Do not attempt to enforce
big-endian mode for -mcall-aixdesc, -mcall-freebsd, -mcall-netbsd,
-mcall-openbsd, or -mcall-linux.
(CC1_ENDIAN_BIG_SPEC): Remove.
(CC1_ENDIAN_LITTLE_SPEC): Remove.
(CC1_ENDIAN_DEFAULT_SPEC): Remove.
(CC1_SPEC): Remove (always empty) %cc1_endian_... spec.
(SUBTARGET_EXTRA_SPECS): Remove %cc1_endian_big, %cc1_endian_little,
and %cc1_endian_default.
* config/rs6000/sysv4le.h (CC1_ENDIAN_DEFAULT_SPEC): Remove.
Index: gcc/config/rs6000/sysv4le.h
===
--- gcc/config/rs6000/sysv4le.h (revision 207578)
+++ gcc/config/rs6000/sysv4le.h (working copy)
@@ -22,9 +22,6 @@
#undef TARGET_DEFAULT
#define TARGET_DEFAULT MASK_LITTLE_ENDIAN
-#undef CC1_ENDIAN_DEFAULT_SPEC
-#defineCC1_ENDIAN_DEFAULT_SPEC "%(cc1_endian_little)"
-
#undef DEFAULT_ASM_ENDIAN
#defineDEFAULT_ASM_ENDIAN " -mlittle"
Index: gcc/config/rs6000/sysv4.h
===
--- gcc/config/rs6000/sysv4.h (revision 207578)
+++ gcc/config/rs6000/sysv4.h (working copy)
@@ -522,8 +522,6 @@
#define ENDIAN_SELECT(BIG_OPT, LITTLE_OPT, DEFAULT_OPT)\
"%{mlittle|mlittle-endian:"LITTLE_OPT ";" \
"mbig|mbig-endian:" BIG_OPT";" \
- "mcall-aixdesc|mcall-freebsd|mcall-netbsd|" \
- "mcall-openbsd|mcall-linux:" BIG_OPT";" \
"mcall-i960-old:"LITTLE_OPT ";" \
":" DEFAULT_OPT "}"
@@ -536,20 +534,12 @@
%{memb|msdata=eabi: -memb}" \
ENDIAN_SELECT(" -mbig", " -mlittle", DEFAULT_ASM_ENDIAN)
-#defineCC1_ENDIAN_BIG_SPEC ""
-
-#defineCC1_ENDIAN_LITTLE_SPEC ""
-
-#defineCC1_ENDIAN_DEFAULT_SPEC "%(cc1_endian_big)"
-
#ifndef CC1_SECURE_PLT_DEFAULT_SPEC
#define CC1_SECURE_PLT_DEFAULT_SPEC ""
#endif
-/* Pass -G xxx to the compiler and set correct endian mode. */
+/* Pass -G xxx to the compiler. */
#defineCC1_SPEC "%{G*} %(cc1_cpu)" \
- ENDIAN_SELECT(" %(cc1_endian_big)", " %(cc1_endian_little)", \
- " %(cc1_endian_default)") \
"%{meabi: %{!mcall-*: -mcall-sysv }} \
%{!meabi: %{!mno-eabi: \
%{mrelocatable: -meabi } \
@@ -903,9 +893,6 @@
{ "link_os_netbsd", LINK_OS_NETBSD_SPEC }, \
{ "link_os_openbsd", LINK_OS_OPENBSD_SPEC }, \
{ "link_os_default", LINK_OS_DEFAULT_SPEC }, \
- { "cc1_endian_big", CC1_ENDIAN_BIG_SPEC }, \
- { "cc1_endian_little", CC1_ENDIAN_LITTLE_SPEC }, \
- { "cc1_endian_default", CC1_ENDIAN_DEFAULT_SPEC }, \
{ "cc1_secure_plt_default", CC1_SECURE_PLT_DEFAULT_SPEC }, \
{ "cpp_os_ads", CPP_OS_ADS_SPEC }, \
{ "cpp_os_yellowknife", CPP_OS_YELLOWKNIFE_SPEC }, \
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
ulrich.weig...@de.
[Patch, fortran] PR57522 - [F03] ASSOCIATE construct creates array descriptor with incorrect stride for derived type array component
Dear All,
I am aware that we are in stage 4 but this wrong-code-on-valid PR is
confined to a corner of a corner and so I am certain that it can be
safely applied - OK, Richie?
This patch follows the same route as has been used for pointer array
assignment to components of derived type arrays by rolling out the
auxilliary 'span' variable for the associate-name to give it the
correct extent. Once the array descriptor reform has been completed,
this fix should be removed. All such code can be found by grepping on
'subref_array', so the excision will be swift and painless :-)
Note that this is not needed for SELECT_TYPE since it is inaccessible
there: "component to the right of an array reference may not have
ALLOCATABLE or POINTER attribute" && "class component must be
ALLOCATABLE or POINTER" = false for any selector that I have been able
to construct.
Bootstraps and regtests on FC17/x86_64 - OK for trunk and 4.8?
Cheers
Paul
2014-02-08 Paul Thomas
PR fortran/57522
* resolve.c (resolve_assoc_var): Set the subref_array_pointer
attribute for the 'associate-name' if necessary.
* trans-stmt.c (trans_associate_var): If the 'associate-name'
is a subref_array_pointer, assign the element size of the
associate variable to 'span'.
2014-02-08 Paul Thomas
PR fortran/57522
* gfortran.dg/associated_target_5.f03 : New test
Index: gcc/fortran/resolve.c
===
*** gcc/fortran/resolve.c (revision 207612)
--- gcc/fortran/resolve.c (working copy)
*** resolve_assoc_var (gfc_symbol* sym, bool
*** 8269,8274
--- 8269,8276
sym->attr.target = tsym->attr.target
|| gfc_expr_attr (target).pointer;
+ if (is_subref_array (target))
+ sym->attr.subref_array_pointer = 1;
}
/* Get type if this was not already set. Note that it can be
Index: gcc/fortran/trans-stmt.c
===
*** gcc/fortran/trans-stmt.c(revision 207612)
--- gcc/fortran/trans-stmt.c(working copy)
*** trans_associate_var (gfc_symbol *sym, gf
*** 1190,1195
--- 1190,1206
dim, gfc_index_one_node);
}
+ /* If this is a subreference array pointer associate name use the
+associate variable element size for the value of 'span'. */
+ if (sym->attr.subref_array_pointer)
+ {
+ gcc_assert (e->expr_type == EXPR_VARIABLE);
+ tmp = e->symtree->n.sym->backend_decl;
+ tmp = gfc_get_element_type (TREE_TYPE (tmp));
+ tmp = fold_convert (gfc_array_index_type, size_in_bytes (tmp));
+ gfc_add_modify (&se.pre, GFC_DECL_SPAN(desc), tmp);
+ }
+
/* Done, register stuff as init / cleanup code. */
gfc_add_init_cleanup (block, gfc_finish_block (&se.pre),
gfc_finish_block (&se.post));
Index: gcc/testsuite/gfortran.dg/associated_target_5.f03
===
*** gcc/testsuite/gfortran.dg/associated_target_5.f03 (revision 0)
--- gcc/testsuite/gfortran.dg/associated_target_5.f03 (working copy)
***
*** 0
--- 1,32
+ ! { dg-do run }
+ ! Test the fix for PR57522, in which the associate name had a
+ ! 'span' of an INTEGER rather than that of 'mytype'.
+ !
+ ! Contributed by A Briolat
+ !
+ program test_associate
+ type mytype
+ integer :: a, b
+ end type
+ type(mytype) :: t(4)
+ integer :: c(4)
+ t%a = [0, 1, 2, 3]
+ t%b = [4, 5, 6, 7]
+ associate (a => t%a)
+ ! Test 'a' is OK on lhs and/or rhs of assignments
+ c = a - 1
+ if (any (c .ne. [-1,0,1,2])) call abort
+ a = a + 1
+ if (any (a .ne. [1,2,3,4])) call abort
+ a = t%b
+ if (any (a .ne. t%b)) call abort
+ ! Test 'a' is OK as an actual argument
+ c = foo(a)
+ if (any (c .ne. t%b + 10)) call abort
+ end associate
+ contains
+ function foo(arg) result(res)
+ integer :: arg(4), res(4)
+ res = arg + 10
+ end function
+ end program
Re: [PATCH] Fix various issues in vect_analyze_data_refs (PR middle-end/59150)
On Sat, Feb 8, 2014 at 10:25 AM, Jakub Jelinek wrote: > On Thu, Feb 06, 2014 at 10:04:00AM +0100, Richard Biener wrote: >> > PS, just looking at the patch now again, in the light of PR59594 >> > the reordering of datarefs before goto again looks also wrong, we IMHO have >> > to perform a vector ordered remove in that case, ok to handle it as a >> > follow-up? >> >> Yeah... > > Here it is. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? Ok. Thanks, Richard. > 2014-02-08 Jakub Jelinek > > * tree-vect-data-refs.c (vect_analyze_data_refs): For clobbers > not at the end of datarefs vector use ordered_remove to avoid > reordering datarefs vector. > > --- gcc/tree-vect-data-refs.c.jj2014-02-06 18:04:15.0 +0100 > +++ gcc/tree-vect-data-refs.c 2014-02-07 21:55:01.278701818 +0100 > @@ -3303,7 +3303,8 @@ again: > datarefs.pop (); > break; > } > - datarefs[i] = dr = datarefs.pop (); > + datarefs.ordered_remove (i); > + dr = datarefs[i]; > goto again; > } > > > > Jakub
Re: FRE may run out of memory
On Sat, Feb 8, 2014 at 8:29 AM, dxq wrote: > hi all, > > We found that gcc would run out of memory on Windows when compiling a *big* > function (10 lines). > > More investigation shows that gcc crashes at the function *compute_avail*, > in tree-fre pass. *compute_avail* collects information from basic blocks, > so memory is allocated to record informantion. > However, if there are huge number of basic blocks, the memory would be > exhausted and gcc would crash down, especially for Windows PC, only 2G or 4G > memory generally. It's ok On linux, and *compute_avail* allocates *2.4G* > memory. I guess some optimization passes in gcc like FRE didn't consider the > extreme > case. This was fixed for GCC 4.8, FRE no longer uses compute_avail (but PRE still does). Basically GCC 4.8 should (at -O1) compile most extreme cases just fine. Richard. > When disable tree-fre pass, gcc crashes at IRA pass. I will do more > investigation about that. > > Any suggestions? > > Thanks! > > danxiaoqiang > > > > -- > View this message in context: > http://gcc.1065356.n5.nabble.com/FRE-may-run-out-of-memory-tp1009578.html > Sent from the gcc - patches mailing list archive at Nabble.com.
Re: [RFC] PR 59776 - esra vs gimple_debug
On Sat, Feb 8, 2014 at 1:37 AM, Richard Henderson wrote:
> On 02/07/2014 03:12 PM, Richard Biener wrote:
>> On February 7, 2014 8:35:16 PM GMT+01:00, Richard Henderson
>> wrote:
>>> In the testcases with the PR, we have a bit of type punning going on,
>>>
>>> *(int *) &s2.f = 0;
>>> s2 = s1;
>>>
>>> which SRA trasforms to
>>>
>>> # DEBUG s2 => 0
>>> MEM[(int *)&s2] = 0;
>>> # DEBUG s2 => s1$f_7
>>> # DEBUG s2$g => s1$g_6
>>> s2 ={v} {CLOBBER};
>>>
>>> Note that it has chosen not to expand s1.f like s1.g, but to expand
>>> that field
>>> as the type-punned integer. Which means that "s2 => s1$f_7" has
>>> mismatched
>>> types across lhs and rhs: SI => SF. Which understandibly ICEs during
>>> rtl
>>> expansion.
>>>
>>> I'm not really sure how this is avoided for the actual code generation,
>>> but
>>> this minimal patch (aka hack) simply drops the debug info to avoid the
>>> ICE.
>>>
>>> Thoughts on how this might really be solved?
>>
>> Add a VIEW_CONVERT_EXPR around the rhs of the debug statement.
>
> Well, ok, though I'm pretty sure that the debug info will pretty much barf on
> that immediately.
>
> What I really meant is: where's a better place to put this check, since such a
> check _must_ exist somewhere else for the regular code generation.
What do you mean with "check"? We have stmt verifiers in tree-cfg.c but they
skip debug statement contents because "those may contain anything" (including
crap it seems ;)). That was an early design decision appearantly - given the
above ICE maybe not the best one.
IIRC the debug stmt expander is the one that is supposed to throw away anything
it cannot handle (and thus it has to expect any sort of werid stuff).
Richard.
>
> r~
Re: minor help message fix
On Fri, Feb 7, 2014 at 6:15 PM, Xinliang David Li wrote: > On Fri, Feb 7, 2014 at 1:22 AM, Richard Biener > wrote: >> On Thu, Feb 6, 2014 at 10:30 PM, Xinliang David Li >> wrote: >>> Hi the following patch removes the 'state' print for >>> -ftree-tree-vectorize option which does not make sense anymore. Ok for >>> trunk? >> >> Hmm, isn't it more appropriate to remove 'Report' from ftree-vectorize >> in common.opt? > > For a supported option, we should probably report it. Do we want to > deprecate it in the future? > >> Or simply treat the [enabled]/[disabled] literally? > > Not clear what you mean. If -ftree-vectorize was not specified then it's not enabled (even when both -ftree-slp-vectorize and -ftree-loop-vectorize are). > I was thinking putting something like [see > -ftree-loop-vectorize and -ftree-slp-vectorize] but it wraps around > and looks bad. > >> Or simply also enable (redundantly) OPT_ftree_vectorize at -O3? > > This does not feel right. The flag does not represent one single > optimization. Imaging we have a default level where loop vectorize is > on, but slp is off (O2 will likely end up like that), what will the > enable/disable state for tree-vectorize? off I don't have a very good suggestion on how to treat these compound opts. What do we do with -ffast-math or other cases? I suppose the compound opts should be marked specially in the opt file so we can list it and its "sub" options in a group. Richard. > David > > >> >> Richard. >> >>> thanks, >>> >>> David
Re: [Patch, Fortran] PR 58470: [4.9 Regression] [OOP] ICE on invalid with FINAL procedure and type extension
Hi Mikael,
thanks for your suggestions and the approval of the patch.
>> maybe add gcc_assert to
>> make it clear that fini->proc_tree should be set at this point.
> Or better: a comment ;-)
I'm going for the gcc_assert *plus* a comment ;)
The thusly updated patch in the attachment regtests cleanly. Will
commit later today.
Cheers,
Janus
Index: gcc/fortran/class.c
===
--- gcc/fortran/class.c (revision 207631)
+++ gcc/fortran/class.c (working copy)
@@ -1880,8 +1880,7 @@ generate_finalization_wrapper (gfc_symbol *derived
for (fini = derived->f2k_derived->finalizers; fini; fini = fini->next)
{
- if (!fini->proc_tree)
- fini->proc_tree = gfc_find_sym_in_symtree (fini->proc_sym);
+ gcc_assert (fini->proc_tree); /* Should have been set in
gfc_resolve_finalizers. */
if (fini->proc_tree->n.sym->attr.elemental)
{
fini_elem = fini;
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (revision 207631)
+++ gcc/fortran/resolve.c (working copy)
@@ -12455,10 +12455,6 @@ resolve_fl_derived0 (gfc_symbol *sym)
/* Add derived type to the derived type list. */
add_dt_to_dt_list (sym);
- /* Check if the type is finalizable. This is done in order to ensure that the
- finalization wrapper is generated early enough. */
- gfc_is_finalizable (sym, NULL);
-
return true;
}
Re: [RFA][rtl-optimization/52714] Do not allow combine to create invalid RTL
> This is then combined into:
>
> newpat = (parallel [
> (set (pc)
> (pc))
> (set (reg/v/f:SI 34 [ stack ])
> (reg/f:SI 15 %sp))
> ])
>
> This isn't a recognized insn, and it needs to be split. Since it's just
> a parallel of independent sets, combine tries to split it into a pair of
> assignments which look like:
>
> (insn 16 14 17 2 (set (pc)
> (pc)) pr52714.c:10 2147483647 {NOOP_MOVE}
> (nil))
> (jump_insn 17 16 18 2 (set (reg/v/f:SI 34 [ stack ])
> (reg/f:SI 15 %sp)) pr52714.c:10 38 {*movsi_m68k2}
> (int_list:REG_BR_PROB 1014 (nil))
> -> 40)
>
>
> Note how the second is a JUMP_INSN, but it doesn't modify the PC. Opps.
>
>
> ISTM the code in combine which tries to rip apart a PARALLEL like that
> needs to ensure that I2 and I3 are both INSNs.
That seems to be an unnecessary pessimization given that the combination looks
perfectly valid if you swap the insns. Can't we enhance the code just below
which chooses the order of the insns after splitting?
--
Eric Botcazou
Re: [PATCH] Fix up __builtin_setjmp_receiver handling (PR c++/60082)
> Looks like I have to build a cross to a sjlj EH target ... Let's clear a common misconception here: the SJLJ EH scheme implemented in C++ doesn't explicitly use __builtin_setjmp/__buitin_longjmp so it won't help to understand how things work for the case at hand; it's instead a hybrid scheme, i.e. EH machinery and __builtin_setjmp/__buitin_longjmp at the RTL level. If you want to experiment with __builtin_setjmp/__buitin_longjmp without writing them explicitly, you can use the SJLJ EH scheme implemented in Ada, because it is entirely piggybacked on __builtin_setjmp/__buitin_longjmp: gigi lowers all the EH constructs to use them (and consequently doesn't use the EH machinery at all, for example there is not a single EH edge in the CFG). And, of course, the canonical setjmp/longjmp are something else and are never turned into __builtin_setjmp/__buitin_longjmp because the semantics is not the same. They were recently changed to make use of the existing machinery for __builtin_setjmp/__buitin_longjmp under the hood (AB edges then AB dispatcher) but they are still optimization blockers because of cfun->calls_setjmp, which is set neither for __builtin_setjmp/__buitin_longjmp nor for SJLJ EH schemes. -- Eric Botcazou
Re: [PATCH] Fix PR52289, a typoed word in an error message
Jeff Law writes:
> On 02/06/14 13:39, Benno Schulenberg wrote:
>>
>> [Oops, had a wrong bug number in the subject line.]
>>
>> Below patch fixes another miswording in an error message,
>> reported by Roland Stigge. Please apply.
>>
>>
>> 2014-02-06 Benno Schulenberg
>>
>> PR translation/52289
>> * fortran/resolve.c (resolve_ordinary_assign): Fix typoed word
>> in an error message.
> Thanks. Installed.
Sometimes even such a trivial patch needs to be regtested. Installed.
Andreas.
PR translation/52289
* gfortran.dg/coarray_8.f90: Update dg-error match.
diff --git a/gcc/testsuite/gfortran.dg/coarray_8.f90
b/gcc/testsuite/gfortran.dg/coarray_8.f90
index 6defc1a..91d6e9a 100644
--- a/gcc/testsuite/gfortran.dg/coarray_8.f90
+++ b/gcc/testsuite/gfortran.dg/coarray_8.f90
@@ -112,7 +112,7 @@ contains
type(t),allocatable :: x[:]
type(t) :: y
x = y
-x[1] = y ! { dg-error "must not be have an allocatable ultimate component"
}
+x[1] = y ! { dg-error "must not have an allocatable ultimate component" }
end subroutine assign2
end module mmm3
--
1.8.5.4
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Re: [PATCH] Fix various issues in vect_analyze_data_refs (PR middle-end/59150)
On Thu, Feb 06, 2014 at 10:04:00AM +0100, Richard Biener wrote: > > PS, just looking at the patch now again, in the light of PR59594 > > the reordering of datarefs before goto again looks also wrong, we IMHO have > > to perform a vector ordered remove in that case, ok to handle it as a > > follow-up? > > Yeah... Here it is. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2014-02-08 Jakub Jelinek * tree-vect-data-refs.c (vect_analyze_data_refs): For clobbers not at the end of datarefs vector use ordered_remove to avoid reordering datarefs vector. --- gcc/tree-vect-data-refs.c.jj2014-02-06 18:04:15.0 +0100 +++ gcc/tree-vect-data-refs.c 2014-02-07 21:55:01.278701818 +0100 @@ -3303,7 +3303,8 @@ again: datarefs.pop (); break; } - datarefs[i] = dr = datarefs.pop (); + datarefs.ordered_remove (i); + dr = datarefs[i]; goto again; } Jakub
[committed] Fix #pragma omp simd with local address taken variables (PR c/59984)
Hi!
If a non-static variable declared inside of #pragma {omp,} simd
body is addressable and we would still like to be able to vectorize
it, we must treat it like if it were private variable, i.e. create
omp simd array for it, otherwise all iterations might get address of
the same variable, rather than each SIMD lane it's own copy.
Fixed by the gimplify.c change. The rest is that I've noticed that
for any #pragma omp for/simd loop we actually duplicated the block
and bind vars and e.g. in the debug info emitted them twice.
As for actual vectorization, if it turns out to be no longer addressable,
we vectorize it just fine, otherwise we are missing vectorization of
statements like:
_27 = GOMP_SIMD_LANE (...);
a_5 = &array[_27];
which can be vectorized as
vec_cst_29 = { &array[0], &array[1], ..., &array[vf - 1] };
(supposedly hoisted before the loop), and then it would be nice
to also handle this specially when passing such an address to
a linear parameter (so that we pass &array[0]). Note that foo in
the testcase (the one not marked as linear) will be only vectorizable
when we have scatter support in the vectorizer.
I think the vectorizer changes need to wait for 5.0 though.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2014-02-08 Jakub Jelinek
PR c/59984
* gimplify.c (gimplify_bind_expr): In ORT_SIMD region
mark local addressable non-static vars as GOVD_PRIVATE
instead of GOVD_LOCAL.
* omp-low.c (lower_omp_for): Move gimple_bind_vars
and BLOCK_VARS of gimple_bind_block to new_stmt rather
than copying them.
* gcc.dg/vect/pr59984.c: New test.
--- gcc/gimplify.c.jj 2014-02-06 23:06:49.0 +0100
+++ gcc/gimplify.c 2014-02-07 16:03:57.095077331 +0100
@@ -1042,7 +1042,14 @@ gimplify_bind_expr (tree *expr_p, gimple
&& (! DECL_SEEN_IN_BIND_EXPR_P (t)
|| splay_tree_lookup (ctx->variables,
(splay_tree_key) t) == NULL))
- omp_add_variable (gimplify_omp_ctxp, t, GOVD_LOCAL | GOVD_SEEN);
+ {
+ if (ctx->region_type == ORT_SIMD
+ && TREE_ADDRESSABLE (t)
+ && !TREE_STATIC (t))
+ omp_add_variable (ctx, t, GOVD_PRIVATE | GOVD_SEEN);
+ else
+ omp_add_variable (ctx, t, GOVD_LOCAL | GOVD_SEEN);
+ }
DECL_SEEN_IN_BIND_EXPR_P (t) = 1;
--- gcc/omp-low.c.jj2014-02-06 23:06:49.449786481 +0100
+++ gcc/omp-low.c 2014-02-07 21:44:02.902176938 +0100
@@ -9406,8 +9406,14 @@ lower_omp_for (gimple_stmt_iterator *gsi
if (!gimple_seq_empty_p (omp_for_body)
&& gimple_code (gimple_seq_first_stmt (omp_for_body)) == GIMPLE_BIND)
{
- tree vars = gimple_bind_vars (gimple_seq_first_stmt (omp_for_body));
+ gimple inner_bind = gimple_seq_first_stmt (omp_for_body);
+ tree vars = gimple_bind_vars (inner_bind);
gimple_bind_append_vars (new_stmt, vars);
+ /* bind_vars/BLOCK_VARS are being moved to new_stmt/block, don't
+keep them on the inner_bind and it's block. */
+ gimple_bind_set_vars (inner_bind, NULL_TREE);
+ if (gimple_bind_block (inner_bind))
+ BLOCK_VARS (gimple_bind_block (inner_bind)) = NULL_TREE;
}
if (gimple_omp_for_combined_into_p (stmt))
--- gcc/testsuite/gcc.dg/vect/pr59984.c.jj 2014-02-07 22:17:20.447769447
+0100
+++ gcc/testsuite/gcc.dg/vect/pr59984.c 2014-02-07 22:17:40.639785545 +0100
@@ -0,0 +1,64 @@
+/* PR c/59984 */
+/* { dg-additional-options "-fopenmp-simd" } */
+
+#include "tree-vect.h"
+
+#define N 128
+
+int a[N];
+
+#pragma omp declare simd
+__attribute__((noinline)) void
+foo (int in, int *out1, int *out2)
+{
+ *out1 = in * in - 1;
+ *out2 = in * in + 1;
+}
+
+#pragma omp declare simd linear (out1, out2)
+__attribute__((noinline)) void
+bar (int in, int *out1, int *out2)
+{
+ *out1 = in * in - 1;
+ *out2 = in * in + 1;
+}
+
+__attribute__((noinline)) void
+test (void)
+{
+ int i;
+ for (i = 0; i < N; i++)
+a[i] = i;
+#pragma omp simd
+ for (i = 0; i < N; i++)
+{
+ int v1, v2;
+ foo (a[i], &v1, &v2);
+ a[i] = v1 * v2;
+}
+ for (i = 0; i < N; i++)
+if (a[i] != i * i * i * i - 1)
+ __builtin_abort ();
+ for (i = 0; i < N; i++)
+a[i] = i;
+#pragma omp simd
+ for (i = 0; i < N; i++)
+{
+ int v1, v2;
+ bar (a[i], &v1, &v2);
+ a[i] = v1 * v2;
+}
+ for (i = 0; i < N; i++)
+if (a[i] != i * i * i * i - 1)
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ check_vect ();
+ test ();
+ return 0;
+}
+
+/* { dg-final { cleanup-tree-dump "vect" } } */
Jakub
