Re: [committed] Fix #pragma omp simd with local address taken variables (PR c/59984)
Jakub, The test gcc.dg/vect/pr59984.c fails on targets using an as that does not support avx instructions (e.g., darwin). TIA Dominique
Re: [Patch, fortran] PR34928 - Extension: volatile common blocks
Janus Weil wrote: 2014-02-08 22:47 GMT+01:00 Steve Kargl s...@troutmask.apl.washington.edu: 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: Towards the top is the following link: https://sourceware.org/cgi-bin/pdw/ps_form.cgi Dominique: Simply fill out that form and choose a sponsor (Steve, Janus or me, choose one). Tobias, who hopes that he has now again a bit more time to work on GCC.
Re: [Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
On February 8, 2014 6:48:08 PM GMT+01:00, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: 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? Ok from a RM perspective. Richard. Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org 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 pa...@gcc.gnu.org PR fortran/59026 * gfortran.dg/elemental_by_value_1.f90 : New test
[MIPS] Update libstdc++-v3 baseline symbols
David Edelsohn dje@gmail.com writes: On Sun, Jan 26, 2014 at 11:12 AM, Richard Sandiford rdsandif...@googlemail.com wrote: [adding libstdc++@] Bill Schmidt wschm...@linux.vnet.ibm.com writes: It was recently pointed out to me that our new powerpc64le-linux-gnu target does not yet have a corresponding directory in libstdc ++-v3/config/abi/post/ to hold a baseline_symbols.txt for the platform. I've been looking around and haven't found any documentation for how the minimum baseline symbols file should be generated. Can someone please enlighten me about the process? Yeah, I'd like to know this too. abi_check has been failing for mips*-linux-gnu for a long while but I was never sure what to do about it. libstdc++/testsuite Makefile has a new-abi-baseline target. # Use 'new-abi-baseline' to create an initial symbol file. Then run # 'check-abi' to test for changes against that file. Thanks. I finally got around to doing this for mips64-linux-gnu, as below. It looks like the current symbols were taken in the middle of the 3.4.11 window so the only changes were to add new symbols to that and future versions. 32/ logically follows mips-linux-gnu, but I see x86_64-linux-gnu also has a 32/. Tested on mips64-linux-gnu and applied. Richard libstdc++-v3/ * config/abi/post/mips64-linux-gnu/32/baseline_symbols.txt: New file. * config/abi/post/mips64-linux-gnu/baseline_symbols.txt: Update. * config/abi/post/mips64-linux-gnu/64/baseline_symbols.txt: Likewise. libstdcxx-baseline.diff.bz2 Description: BZip2 compressed data
Re: [PATCH, fortran/52879] RANDOM_SEED revisited
Steve, First, it is needed to remove the line mpz_t put_size, get_size; in gcc/fortran/check.c, otherwise compiling the file with -Werror (default) fails. Second, the patch breaks the interface of RANDOM_SEED(GET/PUT...) as shown for gfortran.dg/random_seed_1.f90 and the original test in pr52879. This likely makes the patch stage 1 material. AFAIU my tests in pr52879, comment 1, the generated sequence for real does not depend on the four first integers. From a user point of view, this surprising (may be undocumented, I did not check). So I think the seed generation should keep the present properties and only add a way to randomize the 12 integers. Thanks for working on it. Dominique
print quotes around )
A trivial fix to print quotes around ) in libcpp/macro.c OK for trunk ? * macro.c (parse_params): print quote around ) in call to cpp_error() in case CPP_EOF Index: libcpp/macro.c === --- libcpp/macro.c(revision 207627) +++ libcpp/macro.c(working copy) @@ -2815,7 +2815,7 @@ parse_params (cpp_reader *pfile, cpp_mac /* Fall through. */ case CPP_EOF: - cpp_error (pfile, CPP_DL_ERROR, missing ')' in macro parameter list); + cpp_error (pfile, CPP_DL_ERROR, missing %)% in macro parameter list); return false; } }
Re: print quotes around )
On Sun, Feb 9, 2014 at 8:31 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: A trivial fix to print quotes around ) in libcpp/macro.c OK for trunk ? Will not work for if pfile-cb.error callback does not recognize % and % (maybe clients other than c, c++ front-ends ?). So we cannot include % and % in the error message ? * macro.c (parse_params): print quote around ) in call to cpp_error() in case CPP_EOF Index: libcpp/macro.c === --- libcpp/macro.c(revision 207627) +++ libcpp/macro.c(working copy) @@ -2815,7 +2815,7 @@ parse_params (cpp_reader *pfile, cpp_mac /* Fall through. */ case CPP_EOF: - cpp_error (pfile, CPP_DL_ERROR, missing ')' in macro parameter list); + cpp_error (pfile, CPP_DL_ERROR, missing %)% in macro parameter list); return false; } }
Re: [MIPS] Update libstdc++-v3 baseline symbols
On 09/02/14 14:19 +, Richard Sandiford wrote: Thanks. I finally got around to doing this for mips64-linux-gnu, as below. It looks like the current symbols were taken in the middle of the 3.4.11 window so the only changes were to add new symbols to that and future versions. 32/ logically follows mips-linux-gnu, but I see x86_64-linux-gnu also has a 32/. Tested on mips64-linux-gnu and applied. Richard libstdc++-v3/ * config/abi/post/mips64-linux-gnu/32/baseline_symbols.txt: New file. * config/abi/post/mips64-linux-gnu/baseline_symbols.txt: Update. * config/abi/post/mips64-linux-gnu/64/baseline_symbols.txt: Likewise. Thanks, Richard. Sorry it took so long to answer your question, I've bookmarked David's mail so I know the answer myself in future! (And thanks, David, for answering the question.)
Re: [PATCH, fortran/52879] RANDOM_SEED revisited
On Sun, Feb 09, 2014 at 03:31:09PM +0100, Dominique Dhumieres wrote: Steve, First, it is needed to remove the line mpz_t put_size, get_size; in gcc/fortran/check.c, otherwise compiling the file with -Werror (default) fails. OK. Second, the patch breaks the interface of RANDOM_SEED(GET/PUT...) as shown for gfortran.dg/random_seed_1.f90 and the original test in pr52879. This likely makes the patch stage 1 material. Technically, it doesn't change the interface as SIZE should be used to determine the shape of GET/PUT. random_seed_1.f90 was specifically crafted with the assumption that SIZE=12. But, yes, I know gfortran users will have hard coded 12 into their codes. As far as stage 1, it was low hanging fruit and I had a little time to look at a gfortran PR. AFAIU my tests in pr52879, comment 1, the generated sequence for real does not depend on the four first integers. From a user point of view, this surprising (may be undocumented, I did not check). I could not remember what your comment 1 was trying to convey. But. yes the 12 seeds were originally split into 3 groups of 4. The first set went to the first 32-bits, the second set was used for the next 32-bits, and the last 4 give the next 49 bits. Then, ... So I think the seed generation should keep the present properties and only add a way to randomize the 12 integers. FX added the scramble_seed() and unscramble_seed() functions to try to randomize the seeds. But, as shown in comment 0, it is possible to give random_seed() a set of seeds that defeats FX's scrambling. The scrambling still yields the grouping as described above. This gives the nice property of program rnd real(4) a04 real(8) a08 real(10) a10 real(16) a16 call random_seed() ; call random_number(a04) ; print *, a04 call random_seed() ; call random_number(a08) ; print *, a08 call random_seed() ; call random_number(a10) ; print *, a10 call random_seed() ; call random_number(a16) ; print *, a16 end program rnd % gfc4x -o z -O rnd1.f90 ./z 0.997559547 0.99755959009261719 0.997559590092617187729 0.997559590092617200441811501464004185 To go beyond the scrambling that FX gave us, I think would require a caching the user defined seeds (so GET will recover PUT) and much more complicated mashing up of 12 numbers. -- Steve
Re: [Patch, fortran] PR59026 - ELEMENTAL procedure with VALUE arguments emits wrong code
Dear All, Committed as revision 207645 - thanks for review and RM OK. Cheers Paul On 9 February 2014 13:55, Richard Biener rguent...@suse.de wrote: On February 8, 2014 6:48:08 PM GMT+01:00, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: 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? Ok from a RM perspective. Richard. Cheers Paul 2014-02-08 Paul Thomas pa...@gcc.gnu.org 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 pa...@gcc.gnu.org PR fortran/59026 * gfortran.dg/elemental_by_value_1.f90 : New test -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Use [warning enabled by default] for default warnings
We print [-Wfoo] after a warning that was enabled by the -Wfoo option, which is pretty clear. But for warnings that have no -W option we just print [enabled by default], which leads to the question of _what_ is enabled by default. As shown by: http://gcc.gnu.org/ml/gcc/2014-01/msg00234.html it invites the wrong interpretation for things like: warning: non-static data member initializers only available with -std=c++11 or -std=gnu++11 [enabled by default] IMO the natural assumption is that gnu++11 is enabled by default, which is how Lars also read it. There seemed to be support for using warning enabled by default instead, so this patch does that. Tested on x86_64-linux-gnu. OK to install? I'll post an Ada patch separately. Thanks, Richard gcc/ * opts.c (option_name): Use warning enabled by default rather than just enabled by default. gcc/testsuite/ * gcc.dg/gomp/simd-clones-5.c: Update comment for new warning message. Index: gcc/opts.c === --- gcc/opts.c 2014-02-09 12:07:06.237317560 + +++ gcc/opts.c 2014-02-09 12:07:06.371318597 + @@ -,7 +,7 @@ option_name (diagnostic_context *context if (context-warning_as_error_requested) return xstrdup (cl_options[OPT_Werror].opt_text); else - return xstrdup (_(enabled by default)); + return xstrdup (_(warning enabled by default)); } else return NULL; Index: gcc/testsuite/gcc.dg/gomp/simd-clones-5.c === --- gcc/testsuite/gcc.dg/gomp/simd-clones-5.c 2014-02-09 12:07:06.237317560 + +++ gcc/testsuite/gcc.dg/gomp/simd-clones-5.c 2014-02-09 12:07:06.371318597 + @@ -3,7 +3,7 @@ /* ?? The -w above is to inhibit the following warning for now: a.c:2:6: warning: AVX vector argument without AVX enabled changes - the ABI [enabled by default]. */ + the ABI [warning enabled by default]. */ #pragma omp declare simd notinbranch simdlen(4) void foo (int *a)
[Ada] Use [warning enabled by default] for default warnings
This switches Ada from using [enabled by default] to [warning enabled
by default] for consistency with:
http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00549.html
Tested on x86_64-linux-gnu. OK if the above patch goes in?
Thanks,
Richard
gcc/ada/
* erroutc.adb (Output_Msg_Text): Use [warning enabled by default].
* err_vars.ads, errout.ads, gnat_ugn.texi: Update comments and
documentation accordingly.
Index: gcc/ada/erroutc.adb
===
--- gcc/ada/erroutc.adb 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/erroutc.adb 2014-02-09 20:02:58.640471235 +
@@ -456,7 +456,7 @@ package body Erroutc is
if Warn and then Warn_Chr /= ' ' then
if Warn_Chr = '?' then
-Warn_Tag := new String'( [enabled by default]);
+Warn_Tag := new String'( [warning enabled by default]);
elsif Warn_Chr in 'a' .. 'z' then
Warn_Tag := new String'( [-gnatw Warn_Chr ']');
Index: gcc/ada/err_vars.ads
===
--- gcc/ada/err_vars.ads2014-02-09 20:02:00.971968883 +
+++ gcc/ada/err_vars.ads2014-02-09 20:02:58.639471226 +
@@ -141,8 +141,8 @@ package Err_Vars is
-- Setting is irrelevant if no insertion character is present. Note
-- that it is not necessary to reset this after using it, since the proper
-- procedure is always to set it before issuing such a message. Note that
- -- the warning documentation tag is always [enabled by default] in the
- -- case where this flag is True.
+ -- the warning documentation tag is always [warning enabled by default]
+ -- in the case where this flag is True.
Error_Msg_String : String (1 .. 4096);
Error_Msg_Strlen : Natural;
Index: gcc/ada/errout.ads
===
--- gcc/ada/errout.ads 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/errout.ads 2014-02-09 20:02:58.639471226 +
@@ -287,8 +287,8 @@ package Errout is
--Insertion character ?? (Two question marks: default warning)
-- Like ?, but if the flag Warn_Doc_Switch is True, adds the string
- -- [enabled by default] at the end of the warning message. For
- -- continuations, use this in each continuation message.
+ -- [warning enabled by default] at the end of the warning message.
+ -- For continuations, use this in each continuation message.
--Insertion character ?x? (warning with switch)
-- Like ?, but if the flag Warn_Doc_Switch is True, adds the string
Index: gcc/ada/gnat_ugn.texi
===
--- gcc/ada/gnat_ugn.texi 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/gnat_ugn.texi 2014-02-09 20:02:58.644471270 +
@@ -5055,8 +5055,8 @@ indexed components, slices, and selected
@cindex @option{-gnatw.d} (@command{gcc})
If this switch is set, then warning messages are tagged, either with
the string ``@option{-gnatw?}'' showing which switch controls the warning,
-or with ``[enabled by default]'' if the warning is not under control of a
-specific @option{-gnatw?} switch. This mode is off by default, and is not
+or with ``[warning enabled by default]'' if the warning is not under control
+of a specific @option{-gnatw?} switch. This mode is off by default, and is not
affected by the use of @code{-gnatwa}.
@item -gnatw.D
Re: Use [warning enabled by default] for default warnings
On 2/9/2014 3:00 PM, Richard Sandiford wrote: We print [-Wfoo] after a warning that was enabled by the -Wfoo option, which is pretty clear. But for warnings that have no -W option we just print [enabled by default], which leads to the question of _what_ is enabled by default. As shown by: http://gcc.gnu.org/ml/gcc/2014-01/msg00234.html it invites the wrong interpretation for things like: warning: non-static data member initializers only available with -std=c++11 or -std=gnu++11 [enabled by default] IMO the natural assumption is that gnu++11 is enabled by default, which is how Lars also read it. There seemed to be support for using warning enabled by default instead, so this patch does that. Tested on x86_64-linux-gnu. OK to install? Sounds like an earthquake patch from the point of view of test suite baselines! I'll post an Ada patch separately. Will definitely have a big impact on the Ada test suite. Fine to post the Ada patch (which is of course trivial as a patch), but we will have to coordinate installing it with a pass through test base lines.
Re: [Ada] Use [warning enabled by default] for default warnings
On 2/9/2014 3:03 PM, Richard Sandiford wrote:
This switches Ada from using [enabled by default] to [warning enabled
by default] for consistency with:
http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00549.html
Tested on x86_64-linux-gnu. OK if the above patch goes in?
I would say hold off on this until we can find the time
to coordinate updating our test suite, which we will do
as fast as possible.
Thanks,
Richard
gcc/ada/
* erroutc.adb (Output_Msg_Text): Use [warning enabled by default].
* err_vars.ads, errout.ads, gnat_ugn.texi: Update comments and
documentation accordingly.
Index: gcc/ada/erroutc.adb
===
--- gcc/ada/erroutc.adb 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/erroutc.adb 2014-02-09 20:02:58.640471235 +
@@ -456,7 +456,7 @@ package body Erroutc is
if Warn and then Warn_Chr /= ' ' then
if Warn_Chr = '?' then
-Warn_Tag := new String'( [enabled by default]);
+Warn_Tag := new String'( [warning enabled by default]);
elsif Warn_Chr in 'a' .. 'z' then
Warn_Tag := new String'( [-gnatw Warn_Chr ']');
Index: gcc/ada/err_vars.ads
===
--- gcc/ada/err_vars.ads2014-02-09 20:02:00.971968883 +
+++ gcc/ada/err_vars.ads2014-02-09 20:02:58.639471226 +
@@ -141,8 +141,8 @@ package Err_Vars is
-- Setting is irrelevant if no insertion character is present. Note
-- that it is not necessary to reset this after using it, since the proper
-- procedure is always to set it before issuing such a message. Note that
- -- the warning documentation tag is always [enabled by default] in the
- -- case where this flag is True.
+ -- the warning documentation tag is always [warning enabled by default]
+ -- in the case where this flag is True.
Error_Msg_String : String (1 .. 4096);
Error_Msg_Strlen : Natural;
Index: gcc/ada/errout.ads
===
--- gcc/ada/errout.ads 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/errout.ads 2014-02-09 20:02:58.639471226 +
@@ -287,8 +287,8 @@ package Errout is
--Insertion character ?? (Two question marks: default warning)
-- Like ?, but if the flag Warn_Doc_Switch is True, adds the string
- -- [enabled by default] at the end of the warning message. For
- -- continuations, use this in each continuation message.
+ -- [warning enabled by default] at the end of the warning message.
+ -- For continuations, use this in each continuation message.
--Insertion character ?x? (warning with switch)
-- Like ?, but if the flag Warn_Doc_Switch is True, adds the string
Index: gcc/ada/gnat_ugn.texi
===
--- gcc/ada/gnat_ugn.texi 2014-02-09 20:02:00.971968883 +
+++ gcc/ada/gnat_ugn.texi 2014-02-09 20:02:58.644471270 +
@@ -5055,8 +5055,8 @@ indexed components, slices, and selected
@cindex @option{-gnatw.d} (@command{gcc})
If this switch is set, then warning messages are tagged, either with
the string ``@option{-gnatw?}'' showing which switch controls the warning,
-or with ``[enabled by default]'' if the warning is not under control of a
-specific @option{-gnatw?} switch. This mode is off by default, and is not
+or with ``[warning enabled by default]'' if the warning is not under control
+of a specific @option{-gnatw?} switch. This mode is off by default, and is not
affected by the use of @code{-gnatwa}.
@item -gnatw.D
Re: Use [warning enabled by default] for default warnings
IMO the natural assumption is that gnu++11 is enabled by default, which is how Lars also read it. There seemed to be support for using warning enabled by default instead, so this patch does that. Tested on x86_64-linux-gnu. OK to install? I'll post an Ada patch separately. FWIW this doesn't seem desirable to me, this will make the diagnostic longer. For Ada this wouldn't really disambiguate things, and some users may be dependent on the current format, so changing it isn't very friendly. Arno
Re: [Ada] Use [warning enabled by default] for default warnings
This switches Ada from using [enabled by default] to [warning enabled by default] for consistency with: http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00549.html Tested on x86_64-linux-gnu. OK if the above patch goes in? As I just mentioned, this isn't OK at first sight. Arno
Re: [Ada] Use [warning enabled by default] for default warnings
Robert Dewar de...@adacore.com writes: On 2/9/2014 3:03 PM, Richard Sandiford wrote: This switches Ada from using [enabled by default] to [warning enabled by default] for consistency with: http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00549.html Tested on x86_64-linux-gnu. OK if the above patch goes in? I would say hold off on this until we can find the time to coordinate updating our test suite, which we will do as fast as possible. Which testsuite do you mean? I did test this with Ada enabled and there were no regressions. If you mean an external testsuite then I certainly don't mind holding off the Ada part. I hope the non-Ada part could still go in without it though. Thanks, Richard
Re: Use [warning enabled by default] for default warnings
On 2/9/2014 3:09 PM, Arnaud Charlet wrote: IMO the natural assumption is that gnu++11 is enabled by default, which is how Lars also read it. There seemed to be support for using warning enabled by default instead, so this patch does that. Tested on x86_64-linux-gnu. OK to install? I'll post an Ada patch separately. FWIW this doesn't seem desirable to me, this will make the diagnostic longer. For Ada this wouldn't really disambiguate things, and some users may be dependent on the current format, so changing it isn't very friendly. Arno can't we just reword the one warning where there is an ambiguity to avoid the confusion, rather than creating such an earthquake, which as Arno says, really has zero advantages to Ada programmers, and clear disadvantages .. to me [enabled by default] is already awfully long!
Re: [Ada] Use [warning enabled by default] for default warnings
On 2/9/2014 3:10 PM, Richard Sandiford wrote: Which testsuite do you mean? I did test this with Ada enabled and there were no regressions. If you mean an external testsuite then I certainly don't mind holding off the Ada part. I hope the non-Ada part could still go in without it though. I mean many external test suites, many of our users maintain their own test suites, and base lines for their codes, and any change like this is very disruptive.
Re: {GRAPHITE] Replacement of isl_int by isl_val
On 02/09/2014 04:47 PM, Mircea Namolaru wrote:
Patch for replacement of the isl_int (obsolete) by isl_val.
No regressions for c/c++/fortran on x86-64 Linux.
Hi Mircae,
thanks a lot for looking into this.
Just for people not aware of the idea of this patch. Cloog recently
upgraded to the latest version of isl (isl-0.12.2) and with this
release isl deprecated the isl_int interface in favor of the isl_val
interface. Both are interfaces for arbitrary precision numbers. However,
the old isl_int interface was mostly a proxy to gmp, whereas the new
interface hides the implementation and will later allow optimizations
for the common use case of small integers.
We really want to get to the latest version of isl as the newer releases
(and development branches) provide very useful features
including the possibility to bound the amount of computation before
we fall back to a more conservative solution. This feature should help
us to fix some of the newer bugs.
I have doubts if we can still push this patch to graphite at this stage
of trunk, but at the very least we should have it ready for stage-1.
Regarding your patch. It looks generally very nice. Just a couple of
smaller comments:
- Can you adapt configure to only allow isl/cloog versions that actually
support the new interface?
- Can you updated the documentation that lists the supported isl/cloog
versions.
Besides some inline comments:
Index: gcc/graphite-optimize-isl.c
===
--- gcc/graphite-optimize-isl.c (revision 207298)
+++ gcc/graphite-optimize-isl.c (working copy)
@@ -260,6 +260,7 @@
DimToVectorize can be devided by VectorWidth. The default VectorWidth is
currently constant and not yet target specific. This function does not
reason
about parallelism. */
+
This change looks unrelated. Can you submit a separate fix?
static isl_map *
getPrevectorMap (isl_ctx *ctx, int DimToVectorize,
int ScheduleDimensions,
@@ -273,8 +274,9 @@
isl_aff *Aff;
int PointDimension; /* ip */
int TileDimension; /* it */
- isl_int VectorWidthMP;
+ isl_val *VectorWidthMP;
int i;
+ isl_ctx *ct;
We normally use 'ctx' as abbreviation for context.
/* assert (0 = DimToVectorize DimToVectorize ScheduleDimensions);*/
@@ -304,10 +306,10 @@
Aff = isl_aff_zero_on_domain (LocalSpaceRange);
Aff = isl_aff_set_constant_si (Aff, VectorWidth);
Aff = isl_aff_set_coefficient_si (Aff, isl_dim_in, TileDimension, 1);
- isl_int_init (VectorWidthMP);
- isl_int_set_si (VectorWidthMP, VectorWidth);
- Aff = isl_aff_mod (Aff, VectorWidthMP);
- isl_int_clear (VectorWidthMP);
+
+ ct = isl_aff_get_ctx (Aff);
+ VectorWidthMP = isl_val_int_from_si (ct, VectorWidth);
+ Aff = isl_aff_mod_val (Aff, VectorWidthMP);
Modulo = isl_pw_aff_zero_set (isl_pw_aff_from_aff (Aff));
TilingMap = isl_map_intersect_range (TilingMap, Modulo);
Index: gcc/graphite-sese-to-poly.c
===
--- gcc/graphite-sese-to-poly.c (revision 207298)
+++ gcc/graphite-sese-to-poly.c (working copy)
@@ -26,8 +26,15 @@
#include isl/union_map.h
#include isl/constraint.h
#include isl/aff.h
+#include isl/val.h
+#if defined(__cplusplus)
+extern C {
+#endif
+#include isl/val_gmp.h
+#if defined(__cplusplus)
+}
We should put a comment why this is extern C is necessary. Or better,
we should check if Sven can get release us an isl 0.12.3 that includes
the extern C in the relevant header
+#endif
#include cloog/cloog.h
-#include cloog/cloog.h
This seems to be an unrelated fix. Can you submit a separate patch?
#include cloog/isl/domain.h
#endif
@@ -67,7 +74,6 @@
#include graphite-poly.h
#include graphite-sese-to-poly.h
-
This change looks unrelated. Can you submit a separate fix?
/* Assigns to RES the value of the INTEGER_CST T. */
static inline void
@@ -481,13 +487,11 @@
int i;
int nb_iterators = pbb_dim_iter_domain (pbb);
int used_scattering_dimensions = nb_iterators * 2 + 1;
- isl_int val;
+ isl_val *val;
isl_space *dc, *dm;
gcc_assert (scattering_dimensions = used_scattering_dimensions);
- isl_int_init (val);
-
dc = isl_set_get_space (pbb-domain);
dm = isl_space_add_dims (isl_space_from_domain (dc),
isl_dim_out, scattering_dimensions);
@@ -501,12 +505,10 @@
isl_constraint *c = isl_equality_alloc
(isl_local_space_from_space (isl_map_get_space (pbb-schedule)));
- if (0 != isl_aff_get_coefficient (static_sched, isl_dim_in,
- i / 2, val))
- gcc_unreachable ();
+ val = isl_aff_get_coefficient_val (static_sched, isl_dim_in, i / 2);
- isl_int_neg (val, val);
- c = isl_constraint_set_constant (c, val);
+ val = isl_val_neg (val);
+ c = isl_constraint_set_constant_val (c, val);
c =
Re: Use [warning enabled by default] for default warnings
Robert Dewar de...@adacore.com writes: On 2/9/2014 3:09 PM, Arnaud Charlet wrote: IMO the natural assumption is that gnu++11 is enabled by default, which is how Lars also read it. There seemed to be support for using warning enabled by default instead, so this patch does that. Tested on x86_64-linux-gnu. OK to install? I'll post an Ada patch separately. FWIW this doesn't seem desirable to me, this will make the diagnostic longer. For Ada this wouldn't really disambiguate things, and some users may be dependent on the current format, so changing it isn't very friendly. Arno can't we just reword the one warning where there is an ambiguity to avoid the confusion, rather than creating such an earthquake, which as Arno says, really has zero advantages to Ada programmers, and clear disadvantages .. to me [enabled by default] is already awfully long! Well, since the Ada part has been rejected I think we just need to consider this from the non-Ada perspective. And IMO there's zero chance that each new warning will be audited for whether the [enabled by default] will be unambiguous. The fact that this particular warning caused confusion and someone actually reported it doesn't mean that there are no other warnings like that. E.g.: -fprefetch-loop-arrays is not supported with -Os [enabled by default] could also be misunderstood, especially if working on an existing codebase with an existing makefile. And the effect for: pragma simd ignored because -fcilkplus is not enabled [enabled by default] is a bit unfortunate. Those were just two examples -- I'm sure I could pick more. Thanks, Richard
Re: Use [warning enabled by default] for default warnings
On 2/9/2014 3:23 PM, Richard Sandiford wrote: can't we just reword the one warning where there is an ambiguity to avoid the confusion, rather than creating such an earthquake, which as Arno says, really has zero advantages to Ada programmers, and clear disadvantages .. to me [enabled by default] is already awfully long! Well, since the Ada part has been rejected I think we just need to consider this from the non-Ada perspective. And IMO there's zero chance that each new warning will be audited for whether the [enabled by default] will be unambiguous. The fact that this particular warning caused confusion and someone actually reported it doesn't mean that there are no other warnings like that. E.g.: -fprefetch-loop-arrays is not supported with -Os [enabled by default] could also be misunderstood, especially if working on an existing codebase with an existing makefile. And the effect for: pragma simd ignored because -fcilkplus is not enabled [enabled by default] is a bit unfortunate. Those were just two examples -- I'm sure I could pick more. Indeed, worrisome examples, a shorter substitute would be [default warning] ??? Thanks, Richard
Re: [Patch, fortran] PR57522 - [F03] ASSOCIATE construct creates array descriptor with incorrect stride for derived type array component
Dear All, Committed as revision 207646. Thanks for the review and RM OK. Cheers Paul On 9 February 2014 13:56, Richard Biener rguent...@suse.de wrote: On February 8, 2014 5:08:52 PM GMT+01:00, Paul Richard Thomas paul.richard.tho...@gmail.com wrote: 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? Ok with me. Richard. 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 pa...@gcc.gnu.org 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 pa...@gcc.gnu.org PR fortran/57522 * gfortran.dg/associated_target_5.f03 : New test -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH 1/6] [GOMP4] OpenACC 1.0+ support in fortran front-end
Ilmir Usmanov wrote:
OpenACC 1.0 support to fortran FE -- core.
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -1031,9 +1031,22 @@ show_omp_node (int level, gfc_code *c)
{
gfc_omp_clauses *omp_clauses = NULL;
const char *name = NULL;
+ bool is_oacc = false;
I'd also update the comment before the function and mention OpenACC
there. It currently reads:
/* Show a single OpenMP directive node and everything underneath it
if necessary. */
+ fprintf (dumpfile, !$%s %s, is_oacc?ACC:OMP, name);
Add spaces around ? and :
@@ -1215,7 +1334,7 @@ show_omp_node (int level, gfc_code *c)
- fprintf (dumpfile, !$OMP END %s, name);
+ fprintf (dumpfile, !$%s END %s, is_oacc?ACC:OMP, name);
Ditto.
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
+/* Likewise to gfc_namelist, but contains expressions. */
+typedef struct gfc_exprlist
+{
+ struct gfc_expr *expr;
+ struct gfc_exprlist *next;
+}
+gfc_exprlist;
I don't feel strong about it, but I think it is more GCC / gfortran
style to use gfc_expr_list instead of gfc_exprlist.
+ /* OpenACC. */
+ struct gfc_expr *async_expr;
+ struct gfc_expr *gang_expr;
+ struct gfc_expr *worker_expr;
+ struct gfc_expr *vector_expr;
+ struct gfc_expr *num_gangs_expr;
+ struct gfc_expr *num_workers_expr;
+ struct gfc_expr *vector_length_expr;
+ struct gfc_expr *non_clause_wait_expr;
+ gfc_exprlist *waitlist;
+ gfc_exprlist *tilelist;
+ bool async, gang, worker, vector, seq, independent;
+ bool wait, par_auto, gang_static;
I wonder whether it would make sense to use bit fields here.
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -2595,6 +2595,33 @@ match_exit_cycle (gfc_statement st, gfc_exec_op op)
if (cnt 0
o != NULL
o-state == COMP_OMP_STRUCTURED_BLOCK
+ (o-head-op == EXEC_OACC_LOOP
+ || o-head-op == EXEC_OACC_PARALLEL_LOOP))
+{
+ int collapse = 1;
+ gcc_assert (o-head-next != NULL
+ (o-head-next-op == EXEC_DO
+ || o-head-next-op == EXEC_DO_WHILE)
Two questions:
a) Does this part work well when both -fopenmp and -fopenacc is used? I
mean: !$acc loop followed/preceded by !$omp do? I could imagine that
there could be clashes, especially when - e.g. - collapse doesn't match.
b) Do you also handle DO CONCURRENT - either by rejecting it or by
accepting it? Namely,
!$acc loop
do concurrent(i=1:5)
end do
!$acc end loop
end
Side remark, with -fopenmp, it does ICE:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60127
Talking about !$acc loop: I vaguely remember that OpenACC 1.0's spec
doesn't have !$acc end loop while I have seen OpenACC programs which
use it. How do you handle !$acc end loop?
Looking at parse.c, it seems to simply error out. I wonder whether one
should be a bit more graceful. For instance, the following examples use
!$acc end loop:
http://devblogs.nvidia.com/parallelforall/openacc-example-part-2/ [If I
remember correctly, pgf95 and Cray ftn silently accepts end loop while
pathf95 accepts it with a warning.]
And looking at the spec of OpenACC 1.0 and 2.0a, the end loop seems to
be invalid. How about following PathScale's ENZO and accepting end
loop with a warning? Or at least error out with a good error message.
+ if (st == ST_EXIT cnt = collapse)
+{
+ gfc_error (EXIT statement at %C terminating !$ACC LOOP loop);
+ return MATCH_ERROR;
+}
+ if (st == ST_CYCLE cnt collapse)
+{
+ gfc_error (CYCLE statement at %C to non-innermost collapsed
+ !$ACC LOOP loop);
+ return MATCH_ERROR;
+}
+}
I wonder whether one should include for OpenMP and OpenACC some
additional checks as done for DO CONCURRENT or whether those aren't
required. I think image controll statements like Fortran 2008's CRITICAL
block might cause problems with OpenACC/OpenMP concurrency. (Given that
OpenACC/OpenMP likely ignores Fortran 2008's coarrays, one might defer
this until OpenACC/OpenMP has been updated for Fortran 2008.)
+ if (gfc_pure (NULL))
+{
+ gfc_error_now (OpenACC directives at %C may not appear in PURE
+ or ELEMENTAL procedures);
Using gfc_pure() you do not check for ELEMENTAL: Since Fortran 2008,
there are also IMPURE ELEMENTAL procedures. I don't know the spec, but I
don't really see a reason why OpenACC shouldn't be permitted in IMPURE
ELEMENTAL procedures. (BTW: ELEMENTAL implies PURE unless an explicit
IMPURE is used.)
In any case, either drop or ELEMENTAL or also check for the elemental
attribute.
+ if (gfc_implicit_pure (NULL))
+gfc_current_ns-proc_name-attr.implicit_pure = 0;
I believe that will fail with BLOCK - cf. gfc_implicit_pure()
real function foo(n)
integer, value :: n
BLOCK
integer i
real sum
!$acc loop reduce(+:sum)
do i=1, n
sum += sin(real(i))
end do
END BLOCK
end
Re: [PATCH 2/6] [GOMP4] OpenACC 1.0+ support in fortran front-end
Ilmir Usmanov wrote:
OpenACC 1.0 fortran FE support -- matching and resolving.
+ return MATCH_ERROR;
+}
+
+static match
+match_oacc_clause_gang (gfc_omp_clauses *cp)
+{
For consistency, can you add another empty line before the function?
+#define OMP_CLAUSE_SEQ (1ll 32)
I think you should use 1LL instead of 1ll - that should be more readable
and matches gcc/{configure,fold-const.c,glimits.h,simplify-rtx.c}
+static void
+resolve_oacc_scalar_int_expr (gfc_expr *expr, const char *clause)
+{
+ if (!gfc_resolve_expr (expr)
+ || expr-ts.type != BT_INTEGER || expr-rank != 0)
+gfc_error (%s clause at %L requires a scalar INTEGER expression,
+ clause, expr-where);
+}
I'd use integer instead of INTEGER as it is not a 'reserved' word
+gfc_warning (INTEGER expression of %s clause at %L must be positive,
+ clause, expr-where);
+}
Ditto.
@@ -800,10 +1366,14 @@ resolve_omp_clauses (gfc_code *code)
static const char *clause_names[]
= { PRIVATE, FIRSTPRIVATE, LASTPRIVATE, COPYPRIVATE, SHARED,
- COPYIN, REDUCTION };
+ COPYIN, COPY, COPYIN, COPYOUT, CREATE, DELETE,
+PRESENT, PRESENT_OR_COPY, PRESENT_OR_COPYIN,
PRESENT_OR_COPYOUT,
+PRESENT_OR_CREATE, DEVICEPTR, USE_DEVICE, DEVICE_RESIDENT,
+HOST, DEVICE, CACHE, REDUCTION};
Indention: Should be a tab not spaces.
@@ -933,8 +1503,43 @@ resolve_omp_clauses (gfc_code *code)
else
gcc_unreachable ();
+ if (list = OMP_LIST_DATA_CLAUSE_FIRST
+ list = OMP_LIST_DATA_CLAUSE_LAST)
+{
+ if (n-sym-ts.type == BT_DERIVED
+ n-sym-attr.allocatable)
+gfc_error (ALLOCATABLE object '%s' of DERIVED type in %s clause at
%L,
+ n-sym-name, name, code-loc);
+ if (n-sym-ts.type == BT_DERIVED
+ n-sym-attr.pointer)
+gfc_error (POINTER object '%s' of DERIVED type in %s clause at %L,
+ n-sym-name, name, code-loc);
I'd use derived type for the same reason as above.
Shouldn't you also reject polymorphic types (BT_CLASS and
BT_ASSUMED)? [BT_CLASS = class(derived_type_name) or class(*);
BT_ASSUMED = type(*)]
+ if (n-sym-attr.pointer)
+gfc_error (POINTER object '%s' in %s clause at %L,
+ n-sym-name, name, code-loc);
Actually, here and probably elsewhere: Do you need to take care of
derived-type components? I mean something like
clause(derived_type%comp)
If so, note that symtree-n.sym-attr.pointer only checks for
derived_type - even if you are interested in comp's attributes. You
probably should use gfc_expr_attr() in this and similar cases.
+ if (n-sym-as n-sym-as-type == AS_ASSUMED_SIZE)
+gfc_error (Assumed size array '%s' in %s clause at %L,
+ n-sym-name, name, code-loc);
+}
Do you also need to reject AS_ASSUMED_RANK (new since Fortran Technical
Specification ISO/IEC TS 29113:2012)? I mean code like:
subroutine foo(x)
integer, DIMENSION(..) :: x
... openacc_clause(x)
Side note: One should also check how OpenMP handles those.
+ case OMP_LIST_DEVICEPTR:
+ if (n-sym-attr.pointer)
+ gfc_error (POINTER object '%s' in %s clause at %L,
+n-sym-name, name, code-loc);
Talking about pointers, you probably also want to reject Cray pointers
(attr.cray_pointee - and cray_pointee). [One should also check what
OpenMP does; I think it does handle it correctly.]
And another point: I think you have to check whether the argument is a
named constant (PARAMETER, attr.flavor == FL_PARAMETER), I think those
you cannot put them there either.
What happens if you try to use a literal such as deviceptr(5)?
+}
+
+static void
+resolve_oacc_params_in_parallel (gfc_code *code, const char *clause)
+{
Another empty line for consistency.
+ if (oacc_is_parallel (code))
+gfc_error (LOOP %s in PARALLEL section allows no argument or static at
%L,
+ clause, code-loc);
I am not sure whether I understand the error - in particular what
static means. (It might be obvious after looking at the spec - if not,
the error message should be improved.) In any case, I think you want to
use nor instead of or.
+ if (code-ext.omp_clauses-seq)
+{
+ if (code-ext.omp_clauses-independent)
+gfc_error (Both SEQ and INDEPENDENT are not allowed in %L,
code-loc);
Somehow, I don't like the wording; it sounds to me a bit as if SEQ is
not permitted and INDEPENDENT is not permitted while you mean that only
using them simultaneously is wrong. I don't have a very good suggestion,
but here are some: SEQ clause conflicts with INDEPENDENT at %L, SEQ
shall not be used together with INDEPENDENT at %L or ...
+ resolve_oacc_positive_int_expr (el-expr, TILE);
+ if (el-expr-expr_type != EXPR_CONSTANT)
+gfc_error (TILE requires constant expression at %L,
Re: [PATCH 3/6] [GOMP4] OpenACC 1.0+ support in fortran front-end
Ilmir Usmanov wrote:
OpenACC 1.0 fortran FE support -- translation to GENERIC.
+static tree
+gfc_trans_oacc_loop (gfc_code *, gfc_omp_clauses *)
+{
+ gfc_error (Unimplemented);
+ return NULL_TREE;
+}
I think that should be a bit more explicit: First, there should be a
location (%L); secondly, it should state what is unimplemented
(presumably OpenACC LOOP).
Tobias
Re: [PATCH 5/6] [GOMP4] OpenACC 1.0+ support in fortran front-end
Some general questions to the patch set:
* I miss -fopenacc. Is the support already in the branch? I assume
that part is then in c-family/c.opt fortran/lang.opt?
* Documentation: Do you also need to update fortran/gfortran.texi and/or
fortran/invoke.texi? (I assume that -fopenacc is already documented in
docs/invoke.texi) [Search for openmp to find possible spots.]
* Intrinsic module openacc and openacc_lib.h: I assume that those
will be created as follow up - or do they already exist? If so, do you
need to document something in fortran/intrinsic.texi? Or in libgomp.texi?
Ilmir Usmanov wrote:
OpenACC 1.0 fortran FE support -- tests.
gcc/testsuite/gfortran.dg/goacc/
* goacc.exp: New test directory.
+++ b/gcc/testsuite/gfortran.dg/goacc/branch.f95
@@ -0,0 +1,55 @@
+! { dg-do compile }
+! { dg-options -fopenacc }
Is there a reason that you don't automatically add that flag via goacc.exp?
+! { dg-final { scan-tree-dump pragma acc data original } }
+! { dg-final { scan-tree-dump if original } }
This one looks rather general. Shouldn't use narrow it down a bit, e.g.
by using scan-tree-dump-times?
+! { dg-final { scan-tree-dump to original } }
+! { dg-final { scan-tree-dump from original } }
+! { dg-final { scan-tree-dump alloc original } }
Ditto. Also spaces before/after the pattern should make it more unique.
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/goacc.exp
@@ -0,0 +1,36 @@
+# Load support procs.
+load_lib gfortran-dg.exp
+
+if ![check_effective_target_fopenmp] {
+ return
+}
I assume that this should be indeed fopenmp here and not fopenacc as
both share libgomp?
+# Main loop.
+gfortran-dg-runtest [lsort \
+ [find $srcdir/$subdir *.\[fF\]{,90,95,03,08} ] ] -fopenacc
-fdump-parse-tree
As you use -fopenacc here, you probably can get rid of it in dg-options.
Can't you? I am not sure whether -fdump-parse-tree is needed; on the
other hand, it just clutters the *log files.
As -fopenmp seemingly can be mixed with -fopenacc, I think it would be
nice to have some test cases where !$omp and !$acc are both placed - in
either order - before the same Fortran statement.
Tobias
Re: {GRAPHITE] Replacement of isl_int by isl_val
Tobias Grosser wrote: On 02/09/2014 04:47 PM, Mircea Namolaru wrote: Patch for replacement of the isl_int (obsolete) by isl_val. Just for people not aware of the idea of this patch. Cloog recently upgraded to the latest version of isl (isl-0.12.2) and with this release isl deprecated the isl_int interface in favor of the isl_val interface. Is isl_int already in ISL 0.10 and/or in ISL 0.11.1? 0.10 is still supported by configure and 0.11.1 is *the* version listed in prerequisites. Cf. http://gcc.gnu.org/ml/gcc/2014-01/msg00288.html If it is only in 0.11.1, I think configure can be simply updated without further ado. Tobias PS: I think we should apply the following patch: --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -386 +386 @@ installed but it is not in your default library search path, the -@item ISL Library version 0.11.1 +@item ISL Library version 0.11.1 (or later)
[PATCH, WWW] [AVX-512] Add news about AVX-512.
Hello,
This patch adds news about AVX-512 to GCC main page
and entry to 4.9's changes.html.
Is it ok?
PS: I am not native speaker, any corrections are welcome!
--
Thanks, K
Index: htdocs/index.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v
retrieving revision 1.903
diff -p -r1.903 index.html
*** htdocs/index.html 3 Feb 2014 10:02:51 - 1.903
--- htdocs/index.html 7 Feb 2014 07:46:01 -
*** mission statement/a./p
*** 53,58
--- 53,63
dl class=news
+ dtspanIntel AVX-512 support/span
+ span class=date[2014-02-07]/span/dt
+ ddIntel AVX-512 support was added to GCC. That includes inline assembly
+ support, extended existing and new registers, intrinsics set (covered by
+ corresponding testsuite), and basic autovectorization./dd
dtspanAltera Nios II support/span
span class=date[2013-12-31]/span/dt
ddA port for Altera Nios II has been contributed by Mentor
Graphics./dd
Index: htdocs/gcc-4.9/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.9/changes.html,v
retrieving revision 1.54
diff -p -r1.54 changes.html
*** htdocs/gcc-4.9/changes.html 28 Jan 2014 23:57:49 - 1.54
--- htdocs/gcc-4.9/changes.html 7 Feb 2014 07:46:02 -
*** auto incr = [](auto x) { return x++; };
*** 387,392
--- 387,401
h3 id=x86IA-32/x86-64/h3
ul
+ liGCC now supports Intel Advanced Vector Extensions 512 instructions
+ (AVX-512), including inline assembly support, extended existing and
+ new registers, intrinsics set (covered by corresponding testsuite) and
+ basic autovectorization. AVX-512 instructions are available via
+ following GCC switches: AVX-512 foundamental instructions
+ code-mavx512f/code, AVX-512 prefetch instructions
code-mavx512pf/code,
+ AVX-512 exponential and reciprocal instructions code-mavx512er/code,
+ AVX-512 conflict detection instructions code-mavx512cd/code.
+ /li
li It is now possible to call x86 intrinsics from select functions in
a file that are tagged with the corresponding target attribute without
having to compile the entire file with the code-mxxx/code option.
Re: [patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
Ping?
On Tue, Feb 4, 2014 at 5:08 PM, Paul Pluzhnikov ppluzhni...@google.com wrote:
+cc jakub
On Tue, Feb 4, 2014 at 4:59 PM, Paul Pluzhnikov ppluzhni...@google.com
wrote:
Greetings,
The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
gold, but not when linked with BFD ld.
The problem appears to be off-by-one error causing array out of bounds
access, fixed by attached patch.
Alternate fix (used in no-vfa-vect-depend-3.c):
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -5,8 +5,8 @@
#define N 17
-int ia[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
-int ib[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ia[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ib[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
int res[N] = {48,192,180,168,156,144,132,120,108,96,84,72,60,48,36,24,12};
__attribute__ ((noinline))
OK for trunk?
Thanks,
gcc/testsuite/ChangeLog:
2014-02-04 Paul Pluzhnikov ppluzhni...@google.com
* gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
overflow.
Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
===
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -15,7 +15,7 @@
int i;
/* Not vectorizable due to data dependence: dependence distance 1. */
- for (i = N - 1; i = 0; i--)
+ for (i = N - 2; i = 0; i--)
{
ia[i] = ia[i+1] * 4;
}
@@ -28,7 +28,7 @@
}
/* Vectorizable. Dependence distance -1. */
- for (i = N - 1; i = 0; i--)
+ for (i = N - 2; i = 0; i--)
{
ib[i+1] = ib[i] * 4;
}
--
Paul Pluzhnikov
--
Paul Pluzhnikov
Re: [RFA][rtl-optimization/52714] Do not allow combine to create invalid RTL
On 02/08/14 03:44, Eric Botcazou wrote:
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?
I pondered changing the condition for swapping the insn order, but it
didn't seem worth it. I doubt we see many 3-2 combinations where I3 is
a JUMP_INSN, the result turns into two simple sets and the insn swapping
code you wrote decides it needs to swap the insns.
I also pondered identifying the nop set within the parallel taking it
apart again. Again, I rejected as I suspect it doesn't happen enough in
practice to make it worth the effort.
I pondered adding the guard for the newi2pat = set0; newpat = set1 case
where we actually swap the insns. But realized we could run into
similar problems in the prior conditional if I2 was a CALL_INSN that we
somehow simplified.
It seems to me that as long as we're re-using the existing insns to
contain the simple sets that we have to ensure that they're INSNs, not
CALL_INSNs or JUMP_INSNs.
I also pondered resetting the code on each insn via SET_CODE (whatever,
INSN). It's rather hacky, but the formats are close enough that it
would work. Then I realized that we can still undo_all later and didn't
want to add compensation code to put the original code back.
Jeff
[RFA][middle-end/52306] Fix reload from creating invalid RTL
As mentioned in the PR, we emit_input_reload_insns has this little
optimization:
/* If we are reloading a pseudo-register that was set by the previous
insn, see if we can get rid of that pseudo-register entirely
by redirecting the previous insn into our reload register. */
When this optimization applies we change SET_DEST of that previous insn
to be RELOADREG for the current insn's input reload.
Here's the relevant insns:
(insn 246 70 247 10 (set (reg:SI 8 %a0)
(reg:SI 54 [ ivtmp.11 ])) j.c:44 -1
(nil))
(insn 247 246 71 10 (set (reg:SI 54 [ ivtmp.11 ])
(plus:SI (reg:SI 54 [ ivtmp.11 ])
(const_int 4 [0x4]))) j.c:44 141 {*addsi3_internal}
(nil))
(insn 71 247 240 10 (set (reg/f:SI 48 [ D.1497 ])
(mem/f:SI (post_inc:SI (reg:SI 8 %a0)) [3 MEM[base: 0B, index:
ivtmp.11_45, offset: 0B]+0 S4 A16])) j.c:44 39 {*movsi_m68k2}
(expr_list:REG_INC (reg:SI 8 %a0)
(expr_list:REG_INC (reg:SI 8 %a0)
(nil
(note 240 71 73 NOTE_INSN_DELETED)
(insn 73 240 74 10 (set (cc0)
(compare (reg/f:SI 0 %d0 [orig:46 D.1493 ] [46])
(mem/f:SI (reg/f:SI 48 [ D.1497 ]) [3 _30-prefix+0 S4
A16]))) j.c:44 16 {*m68k.md:492}
(expr_list:REG_DEAD (reg/f:SI 48 [ D.1497 ])
(nil)))
Insns 246 and 247 are the reloads for the MEM inside insn 71 which has
an autoincrement addressing mode. Note carefully that insn 247 also
does an increment and will set the equivalent memory location for the
pseudo -- the autoincrement left in insn 71 is put there by reload in
the hopes the value will be useful.
Anyway, we're processing the input reload for insn 73. We see that insn
71's SET_DEST is the same as the input reload. The input reload's
reloadreg is a0. Replacing (reg:SI 48) with a0 in insn 71 produces an
insn which is recognized and which satisfies its constraints. However,
we have a0 used within an increment addressing mode and elsewhere in the
same insn, which is invalid RTL. Eventually we blow up in cselib due to
the invalid RTL.
This shows up in both the testcases in that BZ as well as an m68k bootstrap.
m68k bootstrap is in progress and will probably take a very long time.
I've verified the resulting RTL for the reduced testcase in the PR looks
good and the m68k bootstrap progresses further than it did before (still
in the stage2 build)
OK for the trunk?
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1237904..84c0ba1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2014-02-09 Jeff Law l...@redhat.com
+
+ PR middle-end/52306
+ * reload1.c (emit_input_reload_insns): Do not create invalid RTL
+ when changing the SET_DEST of a prior insn to avoid an input
+ reload.
+
2014-02-07 Jeff Law l...@redhat.com
PR target/40977
diff --git a/gcc/reload1.c b/gcc/reload1.c
index bb761fe..b789ee8 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -7362,9 +7362,18 @@ emit_input_reload_insns (struct insn_chain *chain,
struct reload *rl,
/* Store into the reload register instead of the pseudo. */
SET_DEST (PATTERN (temp)) = reloadreg;
- /* Verify that resulting insn is valid. */
+ /* Verify that resulting insn is valid.
+
+Note that we have replaced the destination of TEMP with
+RELOADREG. If TEMP references RELOADREG within an
+autoincrement addressing mode, then the resulting insn
+is ill-formed and we must reject this optimization. */
extract_insn (temp);
- if (constrain_operands (1))
+ if (constrain_operands (1)
+#ifdef AUTO_INC_DEC
+ ! find_reg_note (temp, REG_INC, reloadreg)
+#endif
+ )
{
/* If the previous insn is an output reload, the source is
a reload register, and its spill_reg_store entry will
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a50fa4b..214259c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-02-09 Jeff Law l...@redhat.com
+
+ PR middle-end-52306
+ * gcc.c-torture/compile/pr52306.c: New test.
+
2014-02-07 Jakub Jelinek ja...@redhat.com
PR preprocessor/56824
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52306.c
b/gcc/testsuite/gcc.c-torture/compile/pr52306.c
new file mode 100644
index 000..e82cb2a
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr52306.c
@@ -0,0 +1,84 @@
+/* PR middle-end/52306 */
+
+struct xmlNs {
+const unsigned char *prefix;
+};
+
+struct xmlNode {
+const unsigned char *name;
+struct xmlNs *ns;
+struct xmlNs *nsDef;
+};
+
+struct xsltTemplate {
+const unsigned char *name;
+int inheritedNsNr;
+void *inheritedNs;
+};
+
+struct xsltTemplate *xsltNewTemplate(void);
+void xsltGetQNameURI(unsigned char**);
+long xmlMalloc(unsigned long);
+void xsltGenericDebug(void);
+int xmlStrEqual(const unsigned char*,
