Re: [PATCH/RFC] C++ FE: expression ranges (v2)
On Sat, Nov 21, 2015 at 02:16:49AM -0500, Jason Merrill wrote: > On 11/19/2015 03:46 PM, Jason Merrill wrote: > >On 11/15/2015 12:01 AM, David Malcolm wrote: > >>As with the C frontend, there's an issue with tree nodes that > >>don't have locations: VAR_DECL, INTEGER_CST, etc: > >> > >> int test (int foo) > >> { > >> return foo * 100; > >>^^^ ^^^ > >> } > >> > >>where we'd like to access the source spelling ranges of the expressions > >>during parsing, so that we can use them when reporting parser errors. > > > >Hmm, I had been thinking to address this in the C++ front end by > >wrapping uses in another tree: NOP_EXPR for rvalues, VIEW_CONVERT_EXPR > >for lvalues. > > On the other hand, my direction seems likely to cause more issues, > especially with code that doesn't yet know how to handle VIEW_CONVERT_EXPR, > and could create ambiguity with explicit conversions. So I guess your > approach seems reasonable. But your approach would allow better diagnostics even in places where you don't have the structures with tree, location_t pairs around. With that it will be limited solely to the parser and nothing else, so even template instantiation if it is something that can be only detected when instantiating would be too late. I think using a new tree (rather than using NOP_EXPR/VIEW_CONVERT_EXPR) that would be just some expression with location and teaching the FE and folder about it might be even better. Jakub
New Chinese (simplified) PO file for 'gcc' (version 5.2.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Chinese (simplified) team of translators. The file is available at: http://translationproject.org/latest/gcc/zh_CN.po (This file, 'gcc-5.2.0.zh_CN.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH, 10/16] Add pass_oacc_kernels pass group in passes.def
On 20/11/15 11:28, Richard Biener wrote: On Thu, 19 Nov 2015, Tom de Vries wrote: >On 17/11/15 15:53, Tom de Vries wrote: > > >And the above LIM example > > >is none for why you need two LIM passes... > > > >Indeed. I'm planning a separate reply to explain in more detail the need > >for the two pass_lims. > >I. > >I managed to get rid of the two pass_lims for the motivating example that I >used until now (goacc/kernels-double-reduction.c). I found that by adding a >pass_dominator instance after pass_ch, I could get rid of the second pass_lim >(and pass_copyprop as well). > >But... then I wrote a counter example (goacc/kernels-double-reduction-n.c), >and I'm back at two pass_lims (and two pass_dominators). >Also I've split the pass group into a bit before and after pass_fre. > >So, the current pass group looks like: >... >NEXT_PASS (pass_build_ealias); > >/* Pass group that runs when the function is an offloaded function >containing oacc kernels loops. Part 1. */ >NEXT_PASS (pass_oacc_kernels); >PUSH_INSERT_PASSES_WITHIN (pass_oacc_kernels) > /* We need pass_ch here, because pass_lim has no effect on >exit-first loops (PR65442). Ideally we want to remove both >this pass instantiation, and the reverse transformation >transform_to_exit_first_loop_alt, which is done in >pass_parallelize_loops_oacc_kernels. */ > NEXT_PASS (pass_ch); >POP_INSERT_PASSES () > >NEXT_PASS (pass_fre); > >/* Pass group that runs when the function is an offloaded function >containing oacc kernels loops. Part 2. */ >NEXT_PASS (pass_oacc_kernels2); >PUSH_INSERT_PASSES_WITHIN (pass_oacc_kernels2) > /* We use pass_lim to rewrite in-memory iteration and reduction >variable accesses in loops into local variables accesses. */ > NEXT_PASS (pass_lim); > NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */); > NEXT_PASS (pass_lim); > NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */); > NEXT_PASS (pass_dce); > NEXT_PASS (pass_parallelize_loops_oacc_kernels); > NEXT_PASS (pass_expand_omp_ssa); >POP_INSERT_PASSES () >NEXT_PASS (pass_merge_phi); >... > > >II. > >The motivating test-case kernels-double-reduction-n.c: >... >#include > >#define N 500 > >unsigned int a[N][N]; > >void __attribute__((noinline,noclone)) >foo (unsigned int n) >{ > int i, j; > unsigned int sum = 1; > >#pragma acc kernels copyin (a[0:n]) copy (sum) > { > for (i = 0; i < n; ++i) > for (j = 0; j < n; ++j) > sum += a[i][j]; > } > > if (sum != 5001) > abort (); >} >... > > >III. > >Before first pass_lim. Note no phis on inner or outer loop header for >iteration varables or reduction variable: >... > : > _5 = *.omp_data_i_4(D).i; > *_5 = 0; > _44 = *.omp_data_i_4(D).n; > _45 = *_44; > if (_45 != 0) > goto ; > else > goto ; > > : outer loop header > _12 = *.omp_data_i_4(D).j; > *_12 = 0; > if (_45 != 0) > goto ; > else > goto ; > > : inner loop header, latch > _19 = *.omp_data_i_4(D).a; > _21 = *_5; > _23 = *_12; > _24 = *_19[_21][_23]; > _25 = *.omp_data_i_4(D).sum; > sum.0_26 = *_25; > sum.1_27 = _24 + sum.0_26; > *_25 = sum.1_27; > _33 = _23 + 1; > *_12 = _33; > j.2_16 = (unsigned int) _33; > if (j.2_16 < _45) > goto ; > else > goto ; > > : outer loop latch > _36 = *_5; > _38 = _36 + 1; > *_5 = _38; > i.3_9 = (unsigned int) _38; > if (i.3_9 < _45) > goto ; > else > goto ; > > : > return; >... > > >IV. > >After first pass_lim/pass_dom pair. Note there are phis on the inner loop >header for the reduction and the iteration variable, but not on the outer loop >header: >... > : > _5 = *.omp_data_i_4(D).i; > *_5 = 0; > _44 = *.omp_data_i_4(D).n; > _45 = *_44; > if (_45 != 0) > goto ; > else > goto ; > > : > _12 = *.omp_data_i_4(D).j; > _19 = *.omp_data_i_4(D).a; > D__lsm.10_50 = *_12; > D__lsm.11_51 = 0; > _25 = *.omp_data_i_4(D).sum; > > : outer loop header > D__lsm.10_20 = 0; > D__lsm.11_22 = 1; > _21 = *_5; > D__lsm.12_28 = *_25; > D__lsm.13_30 = 0; > goto ; > > : inner loop header, latch > # D__lsm.10_47 = PHI <0(5), _33(7)> > # D__lsm.12_49 = PHI > _23 = D__lsm.10_47; > _24 = *_19[_21][D__lsm.10_47]; > sum.0_26 = D__lsm.12_49; > sum.1_27 = _24 + D__lsm.12_49; > D__lsm.12_31 = sum.1_27; > D__lsm.13_32 = 1; > _33 = D__lsm.10_47 + 1; > D__lsm.10_14 = _33; > D__lsm.11_15 = 1; > j.2_16 = (unsigned int) _33; > if (j.2_16 < _45) > goto ; > else > goto ; > > : outer loop latch > # D__lsm.10_35 = PHI <_33(7)> > # D__lsm.11_37 = PHI <1(7)> > # D__lsm.12_7 = PHI > # D__lsm.13_8 = PHI <1(7)> > *_25 = sum.1_27; > _36 = *_5; > _38 = _36 + 1; > *_5 = _38; > i.3_9 = (unsigned int) _38; > if (i.3_9 < _45) > goto ; > else > goto ; > > : > # D__lsm.10_10 = PHI <_33(8)> > # D__lsm.11_11 = PHI <1(8)> > *_12 = _33; >
Re: ICF fixes
> this patchs fixes few issues in ipa-icf. First I drop the use of > TYPE_CANONICAL because that is no longer part of type compatibility > machinery. That doesn't seem right, as the check on TYPE_CANONICAL was restored for aggregate types because of the serious issues we found. > Second I also hash TYPE_MODE for aggregates, becuase we now always require a > match and I check that we don't match types that are incomplete where this > would give false negatives. We clearly know that TYPE_MODE is far from being sufficient for aggregates. -- Eric Botcazou
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
Hi Steve, Just a couple of small typos: "Unexpected expr_type cause an ICE" ; causes? "! An array of derived types workd too." ; works? Apart from that it's OK for trunk. Thanks for the patch Cheers Paul On 20 November 2015 at 21:09, Steve Kargl wrote: > On Thu, Nov 19, 2015 at 04:58:36PM -0800, Steve Kargl wrote: >> >> 2015-11-19 Steven G. Kargl >> >> * intrinsic.h: Prototype for gfc_simplify_cshift >> * intrinsic.c (add_functions): Use gfc_simplify_cshift. >> * simplify.c (gfc_simplify_cshift): Implement simplification of CSHIFT. >> (gfc_simplify_spread): Remove a FIXME and add error condition. >> >> 2015-11-19 Steven G. Kargl >> >> * gfortran.dg/simplify_cshift_1.f90: New test. >> > > I've attached an updated patch. The changes consists of > 1) better/more comments > 2) re-organize code to reduce copying of the array. > 3) add optimization for a left/right shift that >returns the original array. > 4) Don't leak memory. > > -- > Steve -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [PATCH, 4/16] Implement -foffload-alias
On 13/11/15 12:39, Jakub Jelinek wrote: On Fri, Nov 13, 2015 at 12:29:51PM +0100, Richard Biener wrote: thanks for the explanation. Filed as PR68331 - '[meta-bug] fipa-pta issues'. Any feedback on the '#pragma GCC offload-alias=' bit above? Is that sort of what you had in mind? Yes. Whether that makes sense is another question of course. You can annotate memory references with MR_DEPENDENCE_BASE/CLIQUE yourself as well if you know dependences without the users intervention. I really don't like even the GCC offload-alias, I just don't see anything special on the offload code. Not to mention that the same issue is already with other outlined functions, like OpenMP tasks or parallel regions, those aren't offloaded, yet they can suffer from worse alias/points-to analysis too. AFAIU there is one aspect that is different for offloaded code: the setup of the data on the device. Consider this example: ... unsigned int a[N]; unsigned int b[N]; unsigned int c[N]; int main (void) { ... #pragma acc kernels copyin (a) copyin (b) copyout (c) { for (COUNTERTYPE ii = 0; ii < N; ii++) c[ii] = a[ii] + b[ii]; } ... ... At gimple level, we have: ... #pragma omp target oacc_kernels \ map(force_from:c [len: 2097152]) \ map(force_to:b [len: 2097152]) \ map(force_to:a [len: 2097152]) ... [ The meaning of the force_from/force_to mappings is given in include/gomp-constants.h: ... /* Allocate. */ GOMP_MAP_FORCE_ALLOC = (GOMP_MAP_FLAG_FORCE | GOMP_MAP_ALLOC), /* ..., and copy to device. */ GOMP_MAP_FORCE_TO = (GOMP_MAP_FLAG_FORCE | GOMP_MAP_TO), /* ..., and copy from device. */ GOMP_MAP_FORCE_FROM = (GOMP_MAP_FLAG_FORCE | GOMP_MAP_FROM), /* ..., and copy to and from device. */ GOMP_MAP_FORCE_TOFROM = (GOMP_MAP_FLAG_FORCE | GOMP_MAP_TOFROM), ... ] So before calling the offloaded function, a separate alloc is done for a, b and c, and the base pointers of the newly allocated objects are passed to the offloaded function. This means we can mark those base pointers as restrict in the offloaded function. Attached proof-of-concept patch implements that. We simply have some compiler internal interface between the caller and callee of the outlined regions, each interface in between those has its own structure type used to communicate the info; we can attach attributes on the fields, or some flags to indicate some properties interesting from aliasing POV. We don't really need to perform full IPA-PTA, perhaps it would be enough to a) record somewhere in cgraph the relationship in between such callers and callees (for offloading regions we already have "omp target entrypoint" attribute on the callee and a singler caller), tell LTO if possible not to split those into different partitions if easily possible, and then just for these pairs perform aliasing/points-to analysis in the caller and the result record using cliques/special attributes/whatever to the callee side, so that the callee (outlined OpenMP/OpenACC/Cilk+ region) can then improve its alias analysis. As a start, is the approach of this patch OK? It will allow us to commit the oacc kernels patch series with the ability to parallelize non-trivial testcases, and work on improving the alias bit after that. Thanks, - Tom Mark pointers to allocated target vars as restricted, if possible --- gcc/omp-low.c | 67 ++- 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 268b67b..0ce822d 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -1372,7 +1372,8 @@ build_sender_ref (tree var, omp_context *ctx) /* Add a new field for VAR inside the structure CTX->SENDER_DECL. */ static void -install_var_field (tree var, bool by_ref, int mask, omp_context *ctx) +install_var_field_1 (tree var, bool by_ref, int mask, omp_context *ctx, + bool base_pointers_restrict) { tree field, type, sfield = NULL_TREE; splay_tree_key key = (splay_tree_key) var; @@ -1396,7 +1397,11 @@ install_var_field (tree var, bool by_ref, int mask, omp_context *ctx) type = build_pointer_type (build_pointer_type (type)); } else if (by_ref) -type = build_pointer_type (type); +{ + type = build_pointer_type (type); + if (base_pointers_restrict) + type = build_qualified_type (type, TYPE_QUAL_RESTRICT); +} else if ((mask & 3) == 1 && is_reference (var)) type = TREE_TYPE (type); @@ -1460,6 +1465,12 @@ install_var_field (tree var, bool by_ref, int mask, omp_context *ctx) splay_tree_insert (ctx->sfield_map, key, (splay_tree_value) sfield); } +static void +install_var_field (tree var, bool by_ref, int mask, omp_context *ctx) +{ + install_var_field_1 (var, by_ref, mask, ctx, false); +} + static tree install_var_local (tree var, omp_context *ctx) { @@ -1816,7 +1827,8 @@ fixup_child_record_type (omp_context *ctx) specified by CLAUSES. */ static void -scan_sharing_c
Re: [PATCH] g++.dg/init/vbase1.C and g++.dg/cpp/ucn-1.C
> I fixed this with the below patch. Tested on x86_64 linux, x86_64 darwin and > my port. If you want to > list aarch64/arn and mips, please do. > > * g++.dg/init/vbase1.C: Only run on x86_64-*-* as this testcase > isn't portable. I have added i?86-*-* to the list. 2015-11-21 Uros Bizjak * g++.dg/init/vbase1.C: Also run on i?86-*-*. Tested on x86_64-linux-gnu {,-m32} and committed to mainline SVN. Uros. Index: g++.dg/init/vbase1.C === --- g++.dg/init/vbase1.C(revision 230701) +++ g++.dg/init/vbase1.C(working copy) @@ -42,4 +42,4 @@ // Verify that the SubB() mem-initializer is storing 0 directly into // this->D.whatever rather than into a stack temp that is then copied into the // base field. -// { dg-final { scan-rtl-dump "set \[^\n\]*\n\[^\n\]*this\[^\n\]*\n\[^\n\]*const_int 0" "expand" { target { x86_64-*-* } } } } +// { dg-final { scan-rtl-dump "set \[^\n\]*\n\[^\n\]*this\[^\n\]*\n\[^\n\]*const_int 0" "expand" { target { i?86-*-* x86_64-*-* } } } }
Re: C++11 support still experimental?
On 21 November 2015 at 10:35, Uros Bizjak wrote: > [1] still says in its third paragraph: > > --q-- > Important: GCC's support for C++11 is still experimental. Some > features were implemented based on early proposals, and no attempt > will be made to maintain backward compatibility when they are updated > to match the final C++11 standard. > --/q-- > > [1] https://gcc.gnu.org/projects/cxx0x.html I posted a patch for it https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00531.html but never made the changes Gerald requested. I've committed the first part now, as attached. Index: cxx0x.html === RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx0x.html,v retrieving revision 1.69 diff -u -r1.69 cxx0x.html --- cxx0x.html 30 Sep 2015 09:01:43 - 1.69 +++ cxx0x.html 21 Nov 2015 12:31:42 - @@ -27,10 +27,10 @@ GCC 4.7 and later support -std=c++11 and -std=gnu++11 as well. - Important: GCC's support for C++11 is still + Important: Before GCC 5.1 support for C++11 was experimental. Some features were implemented based on - early proposals, and no attempt will be made to maintain backward - compatibility when they are updated to match the final C++11 + early proposals, and no attempt was made to maintain backward + compatibility when they were updated to match the final C++11 standard. C++11 Language Features
Re: [wwwdocs] Update C++ conformance status
On 06/10/15 12:39 -0400, Gerald Pfeifer wrote: On Tue, 6 Oct 2015, Jonathan Wakely wrote: People are being scared off by the experimental status on https://gcc.gnu.org/projects/cxx0x.html e.g. https://gcc.gnu.org/ml/gcc/2015-10/msg00025.html This makes it clear C++11 in 5.1 is no longer experimental. Nice! We also have a "Standard Conformance" section for G++ in https://gcc.gnu.org/bugs/ which says "Two milestones in standard conformance are GCC 3.0 (including a major overhaul of the standard library) and the 3.4.0 version (with its new C++ parser)." I've added some more recent milestones, although maybe std::lib conformance doesn't need to be mentioned in this context? How about removing those references to GCC 3.x in bugs/index.html? That page is supposed to provide instructions on bugs and bug reporting, and I don't think we've got all that many users still interesting in those versions, do we? (And it makes this documentation more concise.) If you agree, I'll be happy to make this change. Just let me know. I forgot to respond to this, and never committed the patch, sorry. I've committed the changes to htdocs/projects/cxx0x.html now, but not the htdocs/bugs/index.html change. I agree that the 3.x info is not useful on that page. Maybe we should just drop the whole "Common problems when upgrading the compiler" section, because info on 3.x is outdated, nearly everybody understands that C++ compilers conform to the standard these days (even MS got in on that act eventually ;-) and the info about breaking the C++ ABI with every major release is just wrong!
[ptx] Add more whitespace
In investigating some failures, I noticed the PTX output is rather dense. Committed this to add some blank lines before DECL and DEF lines. Also fixed a few formatting inconsistencies I noticed on the way. nathan 2015-11-21 Nathan Sidwell * config/nvptx/nvptx.c (write_function_decl_and_comment): Print leading blank line. (write_func_decl_from_insn): Likewise. (init_output_initializer, nvptx_assemble_undefined_decl): Likewise. (nvptx_file_end): Likewise. (nvptx_function_end): Undent output. (nvptx_expand_call): Fix formatting. (nvptx_output_call_insn): Indent output. * config/nvptx/nvptx.h (ASM_OUTPUT_ALIGNED_DECL_COMMON, ASM__OUTPUT_ALIGNED_DECL_LOCAL): Print leading blank line. Index: config/nvptx/nvptx.c === --- config/nvptx/nvptx.c (revision 230704) +++ config/nvptx/nvptx.c (working copy) @@ -515,7 +515,7 @@ walk_args_for_param (FILE *file, tree ar static void write_function_decl_and_comment (std::stringstream &s, const char *name, const_tree decl) { - s << "// BEGIN"; + s << "\n// BEGIN"; if (TREE_PUBLIC (decl)) s << " GLOBAL"; s << " FUNCTION DECL: "; @@ -765,7 +765,7 @@ write_func_decl_from_insn (std::stringst { name = XSTR (callee, 0); name = nvptx_name_replacement (name); - s << "// BEGIN GLOBAL FUNCTION DECL: " << name << "\n"; + s << "\n// BEGIN GLOBAL FUNCTION DECL: " << name << "\n"; } s << (callprototype ? "\t.callprototype\t" : "\t.extern .func "); @@ -820,7 +820,7 @@ write_func_decl_from_insn (std::stringst void nvptx_function_end (FILE *file) { - fprintf (file, "\t}\n"); + fprintf (file, "}\n"); } /* Decide whether we can make a sibling call to a function. For ptx, we @@ -965,7 +965,7 @@ nvptx_expand_call (rtx retval, rtx addre } if (varargs) - XVECEXP (pat, 0, vec_pos++) = gen_rtx_USE (VOIDmode, varargs); +XVECEXP (pat, 0, vec_pos++) = gen_rtx_USE (VOIDmode, varargs); gcc_assert (vec_pos = XVECLEN (pat, 0)); @@ -1726,7 +1726,7 @@ static void init_output_initializer (FILE *file, const char *name, const_tree type, bool is_public) { - fprintf (file, "// BEGIN%s VAR DEF: ", is_public ? " GLOBAL" : ""); + fprintf (file, "\n// BEGIN%s VAR DEF: ", is_public ? " GLOBAL" : ""); assemble_name_raw (file, name); fputc ('\n', file); @@ -1809,7 +1809,8 @@ nvptx_assemble_undefined_decl (FILE *fil if (TREE_CODE (decl) != VAR_DECL) return; const char *section = nvptx_section_for_decl (decl); - fprintf (file, "// BEGIN%s VAR DECL: ", TREE_PUBLIC (decl) ? " GLOBAL" : ""); + fprintf (file, "\n// BEGIN%s VAR DECL: ", + TREE_PUBLIC (decl) ? " GLOBAL" : ""); assemble_name_raw (file, name); fputs ("\n", file); HOST_WIDE_INT size = int_size_in_bytes (TREE_TYPE (decl)); @@ -1934,7 +1935,7 @@ nvptx_output_call_insn (rtx_insn *insn, } fprintf (asm_out_file, ";\n"); if (result != NULL_RTX) -return "ld.param%t0\t%0, [%%retval_in];\n\t}"; +return "\tld.param%t0\t%0, [%%retval_in];\n\t}"; return "}"; } @@ -3979,7 +3980,7 @@ nvptx_file_end (void) worker_bcast_size = (worker_bcast_size + worker_bcast_align - 1) & ~(worker_bcast_align - 1); - fprintf (asm_out_file, "// BEGIN VAR DEF: %s\n", worker_bcast_name); + fprintf (asm_out_file, "\n// BEGIN VAR DEF: %s\n", worker_bcast_name); fprintf (asm_out_file, ".shared .align %d .u8 %s[%d];\n", worker_bcast_align, worker_bcast_name, worker_bcast_size); @@ -3992,7 +3993,7 @@ nvptx_file_end (void) worker_red_size = ((worker_red_size + worker_red_align - 1) & ~(worker_red_align - 1)); - fprintf (asm_out_file, "// BEGIN VAR DEF: %s\n", worker_red_name); + fprintf (asm_out_file, "\n// BEGIN VAR DEF: %s\n", worker_red_name); fprintf (asm_out_file, ".shared .align %d .u8 %s[%d];\n", worker_red_align, worker_red_name, worker_red_size); Index: config/nvptx/nvptx.h === --- config/nvptx/nvptx.h (revision 230704) +++ config/nvptx/nvptx.h (working copy) @@ -304,7 +304,7 @@ struct GTY(()) machine_function #define ASM_OUTPUT_ALIGNED_DECL_COMMON(FILE, DECL, NAME, SIZE, ALIGN) \ do \ { \ - fprintf (FILE, "// BEGIN%s VAR DEF: ",\ + fprintf (FILE, "\n// BEGIN%s VAR DEF: ",\ TREE_PUBLIC (DECL) ? " GLOBAL" : ""); \ assemble_name_raw (FILE, NAME); \ fputc ('\n', FILE); \ @@ -322,7 +322,7 @@ struct GTY(()) machine_function #define ASM_OUTPUT_ALIGNED_DECL_LOCAL(FILE, DECL, NAME, SIZE, ALIGN) \ do \ { \ - fprintf (FILE, "// BEGIN VAR DEF: ");\ + fprintf (FILE, "\n// BEGIN VAR DEF: ");\ assemble_name_raw (FILE, NAME); \ fputc ('\n', FILE); \ const char *sec = nvptx_section_for_decl (DECL); \
[ptx] fix CLZ
This patch fixes CLZ. It always returns SImode, we should look at the input operand to determine the type. Fixes cc.c-torture/execute/builtin-bitops-1.c committed. nathan 2015-11-21 Nathan Sidwell * config/nvptx/nvptx.md (clz2): Use operand 1 for type. Index: config/nvptx/nvptx.md === --- config/nvptx/nvptx.md (revision 230704) +++ config/nvptx/nvptx.md (working copy) @@ -731,7 +731,7 @@ [(set (match_operand:SI 0 "nvptx_register_operand" "=R") (clz:SI (match_operand:SDIM 1 "nvptx_register_operand" "R")))] "" - "%.\\tclz.b%T0\\t%0, %1;") + "%.\\tclz.b%T1\\t%0, %1;") (define_expand "ctz2" [(set (match_operand:SI 0 "nvptx_register_operand" "")
Fix atomic test
I've committed this to fix gcc.dg/atomic-generic.c. It was calling memcmp without a declaration in scope, and passing a plain int as the 3rd argument instead of directly using sizeof or casting to size_t. This blew up PTX with a type mismatch. nathan 2015-11-21 Nathan Sidwell * gcc.dg/atomic-generic.c: Include . Index: testsuite/gcc.dg/atomic-generic.c === --- testsuite/gcc.dg/atomic-generic.c (revision 230704) +++ testsuite/gcc.dg/atomic-generic.c (working copy) @@ -10,6 +10,7 @@ #include #include +#include extern void abort();
Re: [wwwdocs] Update C++ conformance status
On Sat, 21 Nov 2015, Jonathan Wakely wrote: > I forgot to respond to this, and never committed the patch, sorry. > > I've committed the changes to htdocs/projects/cxx0x.html now, but > not the htdocs/bugs/index.html change. I wasn't opposed to the bugs/index.html change, mind. Only wondering about the 3.x info. > I agree that the 3.x info is not useful on that page. Maybe we should > just drop the whole "Common problems when upgrading the compiler" > section, because info on 3.x is outdated, nearly everybody understands > that C++ compilers conform to the standard these days (even MS got in > on that act eventually ;-) and the info about breaking the C++ ABI with > every major release is just wrong! Version-specific changes like the ones described here, and how to cope with them, are usually covered in gcc-*/porting_to.html these days, perhaps we should add pointers from bugs.html? I agree with your thoughts and went ahead and made a first set of changes along these lines (patch below). Absolutely go ahead and trim (or remove) this further. There is also a section about "C++ non-bugs" where I am not sure the current contents still makes a lot of sense? Gerald Index: index.html === RCS file: /cvs/gcc/wwwdocs/htdocs/bugs/index.html,v retrieving revision 1.117 diff -u -r1.117 index.html --- index.html 8 Oct 2015 15:06:00 - 1.117 +++ index.html 21 Nov 2015 15:40:43 - @@ -688,8 +688,7 @@ Standard conformance With each release, we try to make G++ conform closer to the http://www.open-std.org/jtc1/sc22/wg21/";>ISO C++ standard. -We have also implemented some of the core and library defect reports. +"http://www.open-std.org/jtc1/sc22/wg21/";>ISO C++ standard. Non-conforming legacy code that worked with older versions of GCC may be rejected by more recent compilers. There is no command-line switch to ensure @@ -698,58 +697,6 @@ However, some non-conforming constructs are allowed when the command-line option -fpermissive is used. -Two milestones in standard conformance are GCC 3.0 (including a major -overhaul of the standard library) and the 3.4.0 version (with its new C++ -parser). - -New in GCC 3.0 - - - -The standard library is much more conformant, and uses the -std:: namespace (which is now a real namespace, not an -alias for ::). - -The standard header files for the c library don't end with -.h, but begin with c (i.e. -rather than ). -The .h names are still available, but are deprecated. - - is deprecated, use - instead. - -streambuf::seekoff & -streambuf::seekpos are private, instead use -streambuf::pubseekoff & -streambuf::pubseekpos respectively. - -If std::operator << (std::ostream &, long long) -doesn't exist, you need to recompile libstdc++ with ---enable-long-long. - - - -If you get lots of errors about things like cout not being -found, you've most likely forgotten to tell the compiler to look in the -std:: namespace. There are several ways to do this: - - - -Say std::cout at the call. This is the most explicit -way of saying what you mean. - -Say using std::cout; somewhere before the call. You -will need to do this for each function or type you wish to use from the -standard library. - -Say using namespace std; somewhere before the call. -This is the quick-but-dirty fix. This brings the whole of the -std:: namespace into scope. Never do this in a -header file, as every user of your header file will be affected by this -decision. - - - New in GCC 3.4.0 The new parser brings a lot of improvements, especially concerning
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 11:41:51AM +0100, Paul Richard Thomas wrote: > > Just a couple of small typos: > "Unexpected expr_type cause an ICE" ; causes? > "! An array of derived types workd too." ; works? > > Apart from that it's OK for trunk. > > Thanks for the patch > Thanks for the the review. I don't have a clue as to how to do simplification for rank > 2. :( -- Steve
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 8:26 AM, Steve Kargl wrote: > On Sat, Nov 21, 2015 at 11:41:51AM +0100, Paul Richard Thomas wrote: >> >> Just a couple of small typos: >> "Unexpected expr_type cause an ICE" ; causes? >> "! An array of derived types workd too." ; works? >> >> Apart from that it's OK for trunk. >> >> Thanks for the patch >> > > Thanks for the the review. I don't have a clue as > to how to do simplification for rank > 2. :( > It breaks bootstrap: int dm; /* DIM is only useful for rank > 1, but deal with it here as one can set DIM = 1 for rank = 1. */ if (dim) { if (!gfc_is_constant_expr (dim)) return NULL; dm = mpz_get_si (dim->value.integer); } else dm = 1; dm is set, but never used. H.J.
Re: ICF fixes
> > this patchs fixes few issues in ipa-icf. First I drop the use of > > TYPE_CANONICAL because that is no longer part of type compatibility > > machinery. > > That doesn't seem right, as the check on TYPE_CANONICAL was restored for > aggregate types because of the serious issues we found. > > > Second I also hash TYPE_MODE for aggregates, becuase we now always require a > > match and I check that we don't match types that are incomplete where this > > would give false negatives. > > We clearly know that TYPE_MODE is far from being sufficient for aggregates. Yes, this is just a hash. It should give same equivalence as: /* Return true if types are compatible from perspective of ICF. */ bool func_checker::compatible_types_p (tree t1, tree t2) { if (TREE_CODE (t1) != TREE_CODE (t2)) return return_false_with_msg ("different tree types"); if (TYPE_RESTRICT (t1) != TYPE_RESTRICT (t2)) return return_false_with_msg ("restrict flags are different"); if (!types_compatible_p (t1, t2)) return return_false_with_msg ("types are not compatible"); if (get_alias_set (t1) != get_alias_set (t2)) return return_false_with_msg ("alias sets are different"); return true; } which does the TYPE_CANONICAL test via types_compatible_p and additionally check alias set sequivalence. (this checking needs to be dismantled for semantic equivalence done via operand_equal_p and TBAA equivalcne done for loads/stores, but only next stage1). The hash needs to be stable from compilation to WPA to ltrans. The problem here is that TYPE_CANONICAL changes between compilatoin stage and WPA and thus we end up with different hashes for otherwise same types. (such as TYPE_CANONICAL of ptrdiff_t being ptrdiff_t at compile stage and size_t at lto stage). This corrupts the hashtable. The function does deep recurse into aggregates and hash in the fields later on. Honza > > -- > Eric Botcazou
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 10:07:35AM -0800, H.J. Lu wrote: > On Sat, Nov 21, 2015 at 8:26 AM, Steve Kargl > wrote: > > On Sat, Nov 21, 2015 at 11:41:51AM +0100, Paul Richard Thomas wrote: > >> > >> Just a couple of small typos: > >> "Unexpected expr_type cause an ICE" ; causes? > >> "! An array of derived types workd too." ; works? > >> > >> Apart from that it's OK for trunk. > >> > >> Thanks for the patch > >> > > > > Thanks for the the review. I don't have a clue as > > to how to do simplification for rank > 2. :( > > > > It breaks bootstrap: > > int dm; > > /* DIM is only useful for rank > 1, but deal with it here as one can > set DIM = 1 for rank = 1. */ > if (dim) > { > if (!gfc_is_constant_expr (dim)) > return NULL; > dm = mpz_get_si (dim->value.integer); > } > else > dm = 1; > > dm is set, but never used. > Perhaps, bootstrap needs to set appropriate warning levels. -- Steve
Fix lto-symtab ICE during Ada LTO bootstrap
Hi, this patch fixes an ICE seen with Ada LTO bootstrap in reporting type mismatches and it also makes us to stop complaining about C++ ODR violation. The warnings are however correct. I looked at few: ../../libiberty/xstrerror.c:40:14: warning: type of �strerror� does not match original declaration [-Wlto-type-mismatch] extern char *strerror (int); ^ ../../gcc/ada/s-os_lib.adb:1007:16: note: return value type mismatch function strerror (errnum : Integer) return System.Address; ^ ../../gcc/ada/s-os_lib.adb:1007:16: note: �system__os_lib__errno_message__strerror� was previously declared here Here we have function returning pointer WRT function returning integer: public unsigned DI size unit size align 64 symtab 0 alias set -1 structural equality context pointer_to_this > QI size unit size align 8 symtab 0 alias set 0 structural equality arg-types chain >> pointer_to_this > addressable public external QI file ../../libiberty/xstrerror.c line 40 col 14 align 8 context > unit size align 64 symtab 0 alias set -1 canonical type 0x76adb930 precision 64 min max context pointer_to_this chain > QI size unit size align 8 symtab 0 alias set 0 structural equality arg-types chain >> pointer_to_this > addressable public external QI file ../../gcc/ada/s-os_lib.adb line 1007 col 16 align 8 context > $7 = void : warning: type of �__builtin_strlen� does not match original declaration [-Wlto-type-mismatch] ../../gcc/ada/osint.adb:422:19: note: return value type mismatch ../../gcc/ada/osint.adb:422:19: note: type �integer� should match type �long unsigned int� Here the signedness of integer does not match: constant 64> unit size constant 8> align 64 symtab 0 alias set -1 canonical type 0x76adb930 precision 64 min max pointer_to_this > unit size align 32 symtab 0 alias set -1 canonical type 0x76adb7e0 precision 32 min max pointer_to_this > public SI size unit size align 32 symtab 0 alias set -1 canonical type 0x76adb7e0 precision 32 min max context pointer_to_this chain > $17 = void ../../gcc/ada/s-os_lib.ads:1053:4: warning: type of �system__os_lib__directory_separator� does not match original declaration [-Wlto-type-mismatch] Directory_Separator : constant Character; ^ ../../gcc/ada/adaint.c:225:6: note: type �char� should match type �volatile character� char __gnat_dir_separator = DIR_SEPARATOR; ^ Here we get difference in signedness and volatility: unit size align 8 symtab 0 alias set 0 canonical type 0x76adb5e8 precision 8 min max pointer_to_this reference_to_this > addressable public static QI file ../../gcc/ada/adaint.c line 225 col 6 size unit size align 8 context initial > $18 = void (gdb) p debug_tree (decl) unit size align 8 symtab 0 alias set -1 canonical type 0x76adb5e8 precision 8 min max context pointer_to_this > side-effects addressable volatile public unsigned external QI file ../../gcc/ada/s-os_lib.ads line 1053 col 4 size unit size align 8 context > $19 = void All those types will lead to wrong code if ever written to same memory location because of TBAA. Eric, does Ada need all this types to be TBAA compatible? If so, we need to implement more strict globbing as we did for Fortran and we probably finally need to make the globbing aware of languages involved (we definitly don't want to glob pointers and integers for C/C++ programs) Eric, it would be great to have a stand alone testcases in style of gcc/testsuite/gfortran.dg/lto/bind_c-* which stores and reads the same memory location in same alias set and thus trigger the undefined behvaiour. Bootstrapped/regtested x86_64-linux, will commit it shortly. Honza * lto-symtab.c (warn_type_compatibility_p): Do not set ODR mismatch flag for types that are not ODR; fix loop walking parameters. Index: lto-symtab.c === --- lto-symtab.c(revision 230683) +++ lto-symtab.c(working copy) @@ -180,16 +180,26 @@ lto_varpool_replace_node (varpool_node * /* Return non-zero if we want to output waring about T1 and T2. Return value is a bitmask of reasons of violation: Bit 0 indicates that types are not compatible of memory layout. - Bot 1 indicates that types are not compatible because of C++ ODR rule. */ + Bit 1 indicates that types are not compatible because of C++ ODR rule. */ static int warn_type_compatibility_p (tree prevailing_type, tree type) { int lev = 0; + + /* Get complete type. + ??? We might want to emit a warning here if type qualification +
[PATCH] Mark by_ref mem_ref in build_receiver_ref as non-trapping
[ was: Re: [Committed] Mark *.omp_data_i as non-trapping ] On 13/07/15 11:49, Tom de Vries wrote: [ was: Re: [gomp4, committed] Handle nested loops in kernels regions ] On 13/07/15 10:36, Jakub Jelinek wrote: On Mon, Jul 13, 2015 at 10:19:56AM +0200, Thomas Schwinge wrote: We rely on pass_lim to move the *.omp_data_i loads out of the loop nest. For the test-case, pass_lim was managing to move the load out of the inner loop, but not the outer loop, because the load was classified as 'MOVE_PRESERVE_EXECUTION'. By marking the *.omp_data_i load non-trapping, it's now classified as 'MOVE_POSSIBLE', and moved out of the loop nest. This follow-up patch also marks the 'by_ref' mem_ref in build_receiver_ref as non-trapping. Bootstrapped and reg-tested on x86_64. OK for stage3 (because it's needed for the oacc kernels support)? Thanks, - Tom Mark by_ref mem_ref in build_receiver_ref as non-trapping 2015-11-21 Tom de Vries * omp-low.c (build_receiver_ref): Mark by_ref mem_ref as non-trapping. --- gcc/omp-low.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 830db75..78f2853 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -1249,7 +1249,10 @@ build_receiver_ref (tree var, bool by_ref, omp_context *ctx) TREE_THIS_NOTRAP (x) = 1; x = omp_build_component_ref (x, field); if (by_ref) -x = build_simple_mem_ref (x); +{ + x = build_simple_mem_ref (x); + TREE_THIS_NOTRAP (x) = 1; +} return x; }
Re: More Division optimization using match and simplify
On Tue, 17 Nov 2015, Hurugalawadi, Naveen wrote: Please find attached the patch that moves some more division optimizations from fold-const using match and simplify. First, note that we are in stage 3, so this all may have to wait until sometime around March or April next year unless it is fixing some bug. +/* Simplify A / (B << N) where A and B are positive and B is a power of 2, + to A >> (N + log2(B)). */ +(simplify + (floor_div (convert? @0) (lshift @1 INTEGER_CST@2)) I don't think the fold-const code was asking for @2 to be a constant. You will never see (lshift cst1 cst2) in match.pd, it has already been folded to a constant. + (if ((TYPE_UNSIGNED (type) + || tree_expr_nonnegative_warnv_p (@0, NULL)) + && integer_pow2p (@1) Note that you can write integer_pow2p@1 in the pattern directly if you want. + && tree_int_cst_sgn (@1) > 0 + && tree_nop_conversion_p (type, TREE_TYPE (@0))) + (with + { tree pow2 = build_int_cst (TREE_TYPE (@2), wi::exact_log2 (@1)); + tree temp = const_binop (PLUS_EXPR, TREE_TYPE (@2), @2, pow2); } Hmm, maybe you could do the log and plus using wi and only build a single tree at the end, for a slight economy? + (rshift (convert @0) { temp; } + +/* Convert A/B to A/B */ +(for div (CEIL_DIV_EXPR FLOOR_DIV_EXPR) + (simplify + (div (convert? @0) (convert? @1)) + (if (wi::multiple_of_p (@0, @1, TYPE_SIGN (type))) + (exact_div (convert @0) (convert @1) Sorry, using wi::multiple_of_p makes no sense here. It can only work on constants, but for constants we have constant propagation. -- Marc Glisse
Re: [PATCH, rs6000, gcc 5 backport] Fix PR target/67808, LRA ICE on double to long double conversion
On Fri, Nov 20, 2015 at 6:47 PM, Michael Meissner wrote: > On Fri, Oct 02, 2015 at 02:04:48PM -0500, Peter Bergner wrote: >> PR67808 exposes a problem with the constraints in the *extenddftf2_internal >> pattern, in that it allows TFmode operands to occupy Altivec registers >> which they are not allowed to do. Reload was able to work around the >> problem, but LRA is more pedantic and it caused it to go into an infinite >> spill loop until it ICEd. The following patch from Mike changes the TFmode >> output operand to use the "d" constraint instead of "ws". It also allows >> using the "ws" constraint for the two input operands, since that is allowed >> for DFmode operands. >> >> This passed bootstraps (with reload on by default and lra on by default) >> and shows no testsuite regressions. Is this ok for trunk? >> >> The bug is also present in the FSF 5 branch (4.9 is ok), is this ok for >> that too, assuming my bootstrap/regtesting there are clean? > > The following patch backports the fix to GCC 5.x. There were no regressions > in > doing the bootstrap/make check on both a big endian power7 system and a little > endian power8 system. Is it ok to apply the patch to the gcc-5 branch? > > 2015-10-20 Michael Meissner > > Back port from trunk: > 2015-10-05 Michael Meissner > Peter Bergner > > PR target/67808 > * config/rs6000/rs6000.md (extenddftf2): In the expander, only > allow registers, but provide insns for the combiner to create for > loads from memory. Separate VSX code from non-VSX code. For > non-VSX code, combine extenddftf2_fprs into extenddftf2 and rename > externaldftf2_internal to externaldftf2_fprs. Reorder constraints > so that registers come before memory operations. Drop support from > converting DFmode to TFmode, if the DFmode value is in a GPR > register. > (extenddftf2_fprs): Likewise. > (extenddftf2_internal): Likewise. > (extenddftf2_vsx): Likewise. > (extendsftf2): In the expander, only allow registers, but provide > insns for the combiner to create for stores and loads. Okay. Thanks, David
Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm wrote: > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote: >> On 11/17/2015 04:13 PM, David Malcolm wrote: >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote: >> >> >> >> Should c_expr perhaps acquire a constructor so that this problem is >> >> avoided in the future? The whole thing seems somewhat error-prone. >> > >> > I agree that it's error prone, and the ctor approach is what I've been >> > trying for the C++ FE [1] but I suspect that touching that in the C FE >> > would be a much more invasive patch (unless we simply give it a default >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?). >> >> The UNKNOWN_LOCATIONS pair would have been my approach, yes. >> >> > This case gains a pair of locals: start_loc and end_loc (so that we can >> > track the spelling range whilst retaining the "loc" used for the caret), >> > and I preferred to confine their scope to within the case, hence the >> > extra braced block. Omitting the braced block leads to: >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label [-fpermissive] >> >case RID_OFFSETOF: >> > ^ >> > ../../src/gcc/c/c-parser.c:7472:17: error: crosses initialization of >> > ‘location_t end_loc’ >> >location_t end_loc = c_parser_peek_token (parser)->get_finish (); >> > ^ >> > etc. >> >> Hmm, odd, I tried placing just the location_t start_loc line into the >> switch and that appeared to compile fine. But I guess this is not a huge >> problem. >> > >> > Is the combination of the 3 patches OK for trunk? (assuming >> > bootstrap®rest; it's only the braced-init tweak that hasn't been). >> >> Yes. > > Thanks. I've committed the 3 patches to trunk as r230497, which should > fix the worst of the regressions caused by r230331 seen on AIX. I'll > continue to investigate as per the discussion above. Hi, David The new stret-1.m Objective C failure on AIX shows the same symptoms. Is there another fix needed for Objective C? #1 0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x7000, line=991) at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991 991 linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set)); Thanks, David
Re: RFA: PATCH to match.pd for c++/68385
On Sat, 21 Nov 2015, Richard Biener wrote: On November 20, 2015 8:58:15 PM GMT+01:00, Jason Merrill wrote: In this bug, we hit the (A & sign-bit) != 0 -> A < 0 transformation. Because of delayed folding, the operands aren't fully folded yet, so we have NOP_EXPRs around INTEGER_CSTs, and so calling wi::only_sign_bit_p ICEs. We've been seeing several similar bugs, where code calls integer_zerop and therefore assumes that they have an INTEGER_CST, but in fact integer_zerop does STRIP_NOPS. This patch changes the pattern to only match if the operand is actually an INTEGER_CST. Alternatively we could call tree_strip_nop_conversions on the operand, but I would expect that to have issues when the conversion changes the signedness of the type. OK if testing passes? What happens if we remove the nops stripping from integer_zerop? I had the same reaction. Do other integer predicates strip nops? Yes, they do. I believe I added one or two of those, and the reason I added STRIP_NOPS is because they started as a copy-paste of integer_zerop... -- Marc Glisse
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 10:20 AM, Steve Kargl wrote: > On Sat, Nov 21, 2015 at 10:07:35AM -0800, H.J. Lu wrote: >> On Sat, Nov 21, 2015 at 8:26 AM, Steve Kargl >> wrote: >> > On Sat, Nov 21, 2015 at 11:41:51AM +0100, Paul Richard Thomas wrote: >> >> >> >> Just a couple of small typos: >> >> "Unexpected expr_type cause an ICE" ; causes? >> >> "! An array of derived types workd too." ; works? >> >> >> >> Apart from that it's OK for trunk. >> >> >> >> Thanks for the patch >> >> >> > >> > Thanks for the the review. I don't have a clue as >> > to how to do simplification for rank > 2. :( >> > >> >> It breaks bootstrap: >> >> int dm; >> >> /* DIM is only useful for rank > 1, but deal with it here as one can >> set DIM = 1 for rank = 1. */ >> if (dim) >> { >> if (!gfc_is_constant_expr (dim)) >> return NULL; >> dm = mpz_get_si (dim->value.integer); >> } >> else >> dm = 1; >> >> dm is set, but never used. >> > > Perhaps, bootstrap needs to set appropriate warning levels. https://gcc.gnu.org/ml/gcc-regression/2015-11/msg00648.html -- H.J.
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 11:19:22AM -0800, H.J. Lu wrote: > On Sat, Nov 21, 2015 at 10:20 AM, Steve Kargl > wrote: > > On Sat, Nov 21, 2015 at 10:07:35AM -0800, H.J. Lu wrote: > >> On Sat, Nov 21, 2015 at 8:26 AM, Steve Kargl > >> wrote: > >> > On Sat, Nov 21, 2015 at 11:41:51AM +0100, Paul Richard Thomas wrote: > >> >> > >> >> Just a couple of small typos: > >> >> "Unexpected expr_type cause an ICE" ; causes? > >> >> "! An array of derived types workd too." ; works? > >> >> > >> >> Apart from that it's OK for trunk. > >> >> > >> >> Thanks for the patch > >> >> > >> > > >> > Thanks for the the review. I don't have a clue as > >> > to how to do simplification for rank > 2. :( > >> > > >> > >> It breaks bootstrap: > >> > >> int dm; > >> > >> /* DIM is only useful for rank > 1, but deal with it here as one can > >> set DIM = 1 for rank = 1. */ > >> if (dim) > >> { > >> if (!gfc_is_constant_expr (dim)) > >> return NULL; > >> dm = mpz_get_si (dim->value.integer); > >> } > >> else > >> dm = 1; > >> > >> dm is set, but never used. > >> > > > > Perhaps, bootstrap needs to set appropriate warning levels. > > https://gcc.gnu.org/ml/gcc-regression/2015-11/msg00648.html > See 5 lines up. -- Steve
Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
On Sat, 2015-11-21 at 13:54 -0500, David Edelsohn wrote: > On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm wrote: > > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote: > >> On 11/17/2015 04:13 PM, David Malcolm wrote: > >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote: > >> >> > >> >> Should c_expr perhaps acquire a constructor so that this problem is > >> >> avoided in the future? The whole thing seems somewhat error-prone. > >> > > >> > I agree that it's error prone, and the ctor approach is what I've been > >> > trying for the C++ FE [1] but I suspect that touching that in the C FE > >> > would be a much more invasive patch (unless we simply give it a default > >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?). > >> > >> The UNKNOWN_LOCATIONS pair would have been my approach, yes. > >> > >> > This case gains a pair of locals: start_loc and end_loc (so that we can > >> > track the spelling range whilst retaining the "loc" used for the caret), > >> > and I preferred to confine their scope to within the case, hence the > >> > extra braced block. Omitting the braced block leads to: > >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label > >> > [-fpermissive] > >> >case RID_OFFSETOF: > >> > ^ > >> > ../../src/gcc/c/c-parser.c:7472:17: error: crosses initialization of > >> > ‘location_t end_loc’ > >> >location_t end_loc = c_parser_peek_token (parser)->get_finish (); > >> > ^ > >> > etc. > >> > >> Hmm, odd, I tried placing just the location_t start_loc line into the > >> switch and that appeared to compile fine. But I guess this is not a huge > >> problem. > >> > > >> > Is the combination of the 3 patches OK for trunk? (assuming > >> > bootstrap®rest; it's only the braced-init tweak that hasn't been). > >> > >> Yes. > > > > Thanks. I've committed the 3 patches to trunk as r230497, which should > > fix the worst of the regressions caused by r230331 seen on AIX. I'll > > continue to investigate as per the discussion above. > > Hi, David > > The new stret-1.m Objective C failure on AIX shows the same symptoms. > Is there another fix needed for Objective C? > > #1 0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x7000, line=991) > at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991 > 991 linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set)); I believe this one is fixed by the patch I posted here: "[PATCH] Fix PR objc/68438 (uninitialized source ranges)" https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02536.html (it runs cleanly under valgrind on x86_64 with that patch applied)
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
> > > > > > Perhaps, bootstrap needs to set appropriate warning levels. > > > > https://gcc.gnu.org/ml/gcc-regression/2015-11/msg00648.html > > > > See 5 lines up. > Committed. Index: ChangeLog === --- ChangeLog (revision 230709) +++ ChangeLog (working copy) @@ -1,5 +1,10 @@ 2015-11-21 Steven G. Kargl + * simplify.c (gfc_simplify_cshift): Work around bootstrap issues + due to inappropriate warning options. + +2015-11-21 Steven G. Kargl + * simplify.c (gfc_simplify_cshift): Implement simplification of CSHIFT for rank=1 arrays. (gfc_simplify_spread): Remove a FIXME and add error condition. Index: simplify.c === --- simplify.c (revision 230709) +++ simplify.c (working copy) @@ -1869,6 +1869,15 @@ gfc_simplify_cshift (gfc_expr *array, gf else { /* FIXME: Deal with rank > 1 arrays. For now, don't leak memory. */ + + /* GCC bootstrap is too stupid to realize that the above code for dm +is correct. First, dim can be specified for a rank 1 array. It is +not needed in this nor used here. Second, the code is simply waiting +for someone to implement rank > 1 simplification. For now, add a +pessimization to the code that has a zero valid reason to be here. */ + if (dm > array->rank) + gcc_unreachable (); + gfc_free_expr (a); } -- Steve
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
‘dm’ is actually not used, the building problem is fixed by the patch (I did not rearrange the nested ‘if’s) --- ../_clean/gcc/fortran/simplify.c2015-11-21 20:59:57.0 +0100 +++ gcc/fortran/simplify.c 2015-11-21 21:06:30.0 +0100 @@ -1792,7 +1792,6 @@ gfc_expr * gfc_simplify_cshift (gfc_expr *array, gfc_expr *shift, gfc_expr *dim) { gfc_expr *a, *result; - int dm; /* DIM is only useful for rank > 1, but deal with it here as one can set DIM = 1 for rank = 1. */ @@ -1800,10 +1799,7 @@ gfc_simplify_cshift (gfc_expr *array, gf { if (!gfc_is_constant_expr (dim)) return NULL; - dm = mpz_get_si (dim->value.integer); } - else -dm = 1; /* Copy array into 'a', simplify it, and then test for a constant array. An unexpected expr_type causes an ICE. */ Dominique
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 09:22:40PM +0100, Dominique d'Humi??res wrote: > ???dm??? is actually not used, the building problem is fixed by the patch (I > did not rearrange the nested ???if???s) > > --- ../_clean/gcc/fortran/simplify.c 2015-11-21 20:59:57.0 +0100 > +++ gcc/fortran/simplify.c2015-11-21 21:06:30.0 +0100 > @@ -1792,7 +1792,6 @@ gfc_expr * > gfc_simplify_cshift (gfc_expr *array, gfc_expr *shift, gfc_expr *dim) > { >gfc_expr *a, *result; > - int dm; > >/* DIM is only useful for rank > 1, but deal with it here as one can > set DIM = 1 for rank = 1. */ > @@ -1800,10 +1799,7 @@ gfc_simplify_cshift (gfc_expr *array, gf > { >if (!gfc_is_constant_expr (dim)) > return NULL; > - dm = mpz_get_si (dim->value.integer); > } > - else > -dm = 1; > >/* Copy array into 'a', simplify it, and then test for a constant array. > An unexpected expr_type causes an ICE. */ > I already fix the problem. Your patch unfixes the valid code that is needed for the rank>2 case. -- Steve
Re: [PATCH] Fix uninitialized src_range within c_expr (Re: libcpp/C FE source range patch committed (r230331))
On Sat, Nov 21, 2015 at 3:00 PM, David Malcolm wrote: > On Sat, 2015-11-21 at 13:54 -0500, David Edelsohn wrote: >> On Tue, Nov 17, 2015 at 3:12 PM, David Malcolm wrote: >> > On Tue, 2015-11-17 at 16:24 +0100, Bernd Schmidt wrote: >> >> On 11/17/2015 04:13 PM, David Malcolm wrote: >> >> > On Mon, 2015-11-16 at 22:34 +0100, Bernd Schmidt wrote: >> >> >> >> >> >> Should c_expr perhaps acquire a constructor so that this problem is >> >> >> avoided in the future? The whole thing seems somewhat error-prone. >> >> > >> >> > I agree that it's error prone, and the ctor approach is what I've been >> >> > trying for the C++ FE [1] but I suspect that touching that in the C FE >> >> > would be a much more invasive patch (unless we simply give it a default >> >> > ctor that makes the src_range be a pair of UNKNOWN_LOCATIONS?). >> >> >> >> The UNKNOWN_LOCATIONS pair would have been my approach, yes. >> >> >> >> > This case gains a pair of locals: start_loc and end_loc (so that we can >> >> > track the spelling range whilst retaining the "loc" used for the caret), >> >> > and I preferred to confine their scope to within the case, hence the >> >> > extra braced block. Omitting the braced block leads to: >> >> > ../../src/gcc/c/c-parser.c:7494:7: error: jump to case label >> >> > [-fpermissive] >> >> >case RID_OFFSETOF: >> >> > ^ >> >> > ../../src/gcc/c/c-parser.c:7472:17: error: crosses initialization of >> >> > ‘location_t end_loc’ >> >> >location_t end_loc = c_parser_peek_token (parser)->get_finish (); >> >> > ^ >> >> > etc. >> >> >> >> Hmm, odd, I tried placing just the location_t start_loc line into the >> >> switch and that appeared to compile fine. But I guess this is not a huge >> >> problem. >> >> > >> >> > Is the combination of the 3 patches OK for trunk? (assuming >> >> > bootstrap®rest; it's only the braced-init tweak that hasn't been). >> >> >> >> Yes. >> > >> > Thanks. I've committed the 3 patches to trunk as r230497, which should >> > fix the worst of the regressions caused by r230331 seen on AIX. I'll >> > continue to investigate as per the discussion above. >> >> Hi, David >> >> The new stret-1.m Objective C failure on AIX shows the same symptoms. >> Is there another fix needed for Objective C? >> >> #1 0x10016794 in _Z14linemap_lookupP9line_mapsj (set=0x7000, line=991) >> at /nasfarm/edelsohn/src/src/libcpp/line-map.c:991 >> 991 linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set)); > > I believe this one is fixed by the patch I posted here: > "[PATCH] Fix PR objc/68438 (uninitialized source ranges)" >https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02536.html > > (it runs cleanly under valgrind on x86_64 with that patch applied) Yep, thanks. I missed the follow-on patch. Thanks, David
[PATCH] GCC system.h and Graphite header order
Graphite relies on the ISL library and includes multiple ISL headers. The ISL headers refer to identifiers that are poisoned for use in GCC. The source files for Graphite were organized to include the ISL headers first, to avoid the identifier poisoning, which breaks some platforms because GCC header features are disabled. This patch reorganizes the graphite*.c header file inclusion order to list ISL header files near the end, just before the graphite header files on which they rely. A new macro, USES_ISL, is defined, which skips the relevant identifier poisoning, similar to logic for Flex and Bison. This patch also removes early inclusion of stddef.h for ISL because it now should be provided by GCC system.h This has been bootstrapped on powerpc-ibm-aix7.1.0.0 Okay for trunk? Thanks, David * system.h: Don't poison calloc and strdup if USES_ISL is defined. * graphite-dependences.c: Define USES_ISL. Include ISL header files after GCC header files and before graphite header files. * graphite-dependences.c: Same. * graphite-isl-ast-to-gimple.c: Same. * graphite-optimize-isl.c: Same. * graphite-poly.c: Same. * graphite-scop-detection.c: Same. * graphite-sese-to-poly.c: Same. * graphite.c: Same. GG Description: Binary data
[PATCH] c++/42121 - diagnose invalid flexible array members
Bug 42121 - g++ should warn or error on internal 0 size array in struct, is a request to diagnose declarations of flexible array members that aren't last in the enclosing struct, such as in the following: struct S { int a; char b[]; // invalid int c; }; The C front end diagnoses such cases because they are invalid in standard C. Comment 8 on the bug points out that flexible array members should not be treated identically to zero-size arrays (they're not in C). The attached patch implements the requested diagnostic, keeping comment 8 in mind. It also issues a diagnostic for flexible array members in unions (which are also diagnosed as invalid in C mode). The patch found a number of instances of invalid flexible array members in the C++ test suites. I corrected them. Since the C++ front end doesn't distinguish between flexible array members and zero-size arrays (both are considered to have an upper bound of SIZE_MAX), and since determining whether or not a declaration of such a member is valid cannot be done until the whole containing struct has been processed, the patch makes use one of the DECL_LANG_FLAGs to temporarily remember which is which (I somewhat arbitrarily picked DECL_LANG_FLAG_1), before clearing it. There might be a better flag to use, and it might be appropriate to define a descriptive macro for this purpose in cp-tree.h, along the same lines as the macros already defined for other such purposes. Martin gcc/ 2015-11-20 Martin Sebor PR c++/42121 * c/c-decl.c (grokdeclarator): Mention type size in a diagnostic. (finish_struct): Same. gcc/cp/ 2015-11-20 Martin Sebor PR c++/42121 * cp/class.c (layout_class_type): Mention type size in a diagnostic. (all_bases_empty_p, field_nonempty_p, check_flexarrays): New helper functions. (finish_struct_1): Call check_flexarrays. * cp/decl.c (compute_array_index_type): Add argument. Enhance diagnostic. (grokdeclarator): Reject flexible array members in unions. Distinguish flexible array members from zero-size arrays. gcc/testsuite/ 2015-11-20 Martin Sebor PR c++/42121 * g++.dg/ext/flexary2.C: Adjust and enhance. * g++.dg/ext/flexary3.C: Adjust. * g++.dg/ext/flexary3.C: New test. * g++.dg/torture/pr64280.C: Adjust. * g++.dg/torture/pr64312.C: Adjust. * g++.dg/parse/pr43765.C: Adjust. * g++.dg/torture/pr64280.C: Adjust. diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 7b9ab8a..e55471f 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -5868,10 +5868,12 @@ grokdeclarator (const struct c_declarator *declarator, && !int_fits_type_p (size, index_type)) { if (name) - error_at (loc, "size of array %qE is too large", - name); + error_at (loc, "size of array %qE is too large " +"(%qE bytes)", + name, size); else - error_at (loc, "size of unnamed array is too large"); + error_at (loc, "size of unnamed array is too large" +" (%qE bytes)", size); type = error_mark_node; continue; } @@ -7701,7 +7703,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, && TREE_CODE (TYPE_SIZE_UNIT (t)) == INTEGER_CST && !TREE_OVERFLOW (TYPE_SIZE_UNIT (t)) && !valid_constant_size_p (TYPE_SIZE_UNIT (t))) -error ("type %qT is too large", t); +error ("size of type %qT is too large (%qE bytes)", t, TYPE_SIZE_UNIT (t)); /* Give bit-fields their proper types and rewrite the type of array fields with scalar component if the enclosing type has reverse storage order. */ diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 216a301..4803f55 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "convert.h" #include "dumpfile.h" #include "gimplify.h" +#include "intl.h" /* The number of nested classes being processed. If we are not in the scope of any class, this is zero. */ @@ -6531,7 +6532,7 @@ layout_class_type (tree t, tree *virtuals_p) && TREE_CODE (TYPE_SIZE_UNIT (t)) == INTEGER_CST && !TREE_OVERFLOW (TYPE_SIZE_UNIT (t)) && !valid_constant_size_p (TYPE_SIZE_UNIT (t))) -error ("type %qT is too large", t); +error ("size of type %qT is too large (%qE bytes)", t, TYPE_SIZE_UNIT (t)); /* Warn about bases that can't be talked about due to ambiguity. */ warn_about_ambiguous_bases (t); @@ -6598,12 +6599,196 @@ sorted_fields_type_new (int n) } +/* Return true when all base classes of class T (a class type) are + empty, false otherwise. */ + +static bool +all_bases_empty_p (tree t) +{ + int i; + tree binfo; + tree base_binfo; + + for (binfo = TYPE_BINFO (t), i = 0; + BINFO_BASE_ITERATE (binfo, i, base_binfo); i++) +{ + tree basetype = TREE_TYPE (base_binfo); + gcc_assert (COMPLETE_TYPE_P (basetype)); + + if (!is_empty_class (basetype)) + return false; +} + + return true; +} + +/* Helper of finish_struct_1. Retu
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
> + * simplify.c (gfc_simplify_cshift): Work around bootstrap issues > + due to inappropriate warning options. The warning options are appropriate, any dead code can potentially hide a bug and should be flagged; every static analyzer will also do it and we would soon have PRs opened with bugzilla if we tolerated it. -- Eric Botcazou
Re: [PATCH] (Partial) Implementation of simplificaiton of CSHIFT
On Sat, Nov 21, 2015 at 11:26:17PM +0100, Eric Botcazou wrote: > > + * simplify.c (gfc_simplify_cshift): Work around bootstrap issues > > + due to inappropriate warning options. > > The warning options are appropriate, any dead code can potentially hide a bug > and should be flagged; every static analyzer will also do it and we would > soon > have PRs opened with bugzilla if we tolerated it. > I disgree. If bootstrap is going to enforce -Werror -Wunused-*, then this should be the default for any possible invocation of make. -- Steve
[PATCH] lround for PowerPC
PowerPC was missing a definition of the lroundMN pattern, which can be implemented with VSX instructions available in Power7. Below is a first draft. - David * config/rs6000/rs6000.md (*xsrdpidf2): New define_insn. (lrounddfdi2): New define_expand. diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 8c53c40..eadbe1d 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -77,6 +77,7 @@ UNSPEC_FRIN UNSPEC_FRIP UNSPEC_FRIZ + UNSPEC_XSRDPI UNSPEC_LD_MPIC ; load_macho_picbase UNSPEC_RELD_MPIC; re-load_macho_picbase UNSPEC_MPIC_CORRECT ; macho_correct_pic @@ -5245,6 +5246,27 @@ [(set_attr "type" "fp") (set_attr "fp_type" "fp_addsub_")]) +(define_insn "*xsrdpidf2" + [(set (match_operand:DF 0 "gpc_reg_operand" "=") + (unspec:DF [(match_operand:DF 1 "gpc_reg_operand" "")] + UNSPEC_XSRDPI))] + "TARGET_DF_FPR && TARGET_POPCNTD" + "xsrdpi %0,%1" + [(set_attr "type" "fp")]) + +(define_expand "lrounddfdi2" + [(set (match_dup 2) + (unspec:DF [(match_operand:DF 1 "gpc_reg_operand" "")] + UNSPEC_XSRDPI)) + (set (match_operand:DI 0 "gpc_reg_operand" "=d") + (unspec:DI [(match_dup 2)] + UNSPEC_FCTID))] + "TARGET_DF_FPR && TARGET_POPCNTD + && flag_unsafe_math_optimizations && !flag_trapping_math" +{ + operands[2] = gen_reg_rtx (DFmode); +}) + ; An UNSPEC is used so we don't have to support SImode in FP registers. (define_insn "stfiwx" [(set (match_operand:SI 0 "memory_operand" "=Z")
Re: [PATCH] GCC system.h and Graphite header order
On Sat, Nov 21, 2015 at 4:03 PM, David Edelsohn wrote: > Graphite relies on the ISL library and includes multiple ISL headers. > The ISL headers refer to identifiers that are poisoned for use in GCC. > The source files for Graphite were organized to include the ISL > headers first, to avoid the identifier poisoning, which breaks some > platforms because GCC header features are disabled. > > This patch reorganizes the graphite*.c header file inclusion order to > list ISL header files near the end, just before the graphite header > files on which they rely. A new macro, USES_ISL, is defined, which > skips the relevant identifier poisoning, similar to logic for Flex and > Bison. > > This patch also removes early inclusion of stddef.h for ISL because it > now should be provided by GCC system.h > > This has been bootstrapped on powerpc-ibm-aix7.1.0.0 > > Okay for trunk? > > Thanks, David > > * system.h: Don't poison calloc and strdup if USES_ISL is defined. > * graphite-dependences.c: Define USES_ISL. Include ISL header files > after GCC header files and before graphite header files. > * graphite-dependences.c: Same. > * graphite-isl-ast-to-gimple.c: Same. > * graphite-optimize-isl.c: Same. > * graphite-poly.c: Same. > * graphite-scop-detection.c: Same. > * graphite-sese-to-poly.c: Same. > * graphite.c: Same. The patch looks good to me. Thanks David for fixing this. Sebastian
Re: [PATCH] lround for PowerPC
On November 22, 2015 2:52:53 AM GMT+01:00, David Edelsohn wrote: >PowerPC was missing a definition of the lroundMN pattern, which can be >implemented with VSX instructions available in Power7. Below is a >first draft. > >- David > >* config/rs6000/rs6000.md (*xsrdpidf2): New define_insn. >(lrounddfdi2): New define_expand. > >diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md >index 8c53c40..eadbe1d 100644 >--- a/gcc/config/rs6000/rs6000.md >+++ b/gcc/config/rs6000/rs6000.md >@@ -77,6 +77,7 @@ >UNSPEC_FRIN >UNSPEC_FRIP >UNSPEC_FRIZ >+ UNSPEC_XSRDPI >UNSPEC_LD_MPIC ; load_macho_picbase >UNSPEC_RELD_MPIC; re-load_macho_picbase >UNSPEC_MPIC_CORRECT ; macho_correct_pic >@@ -5245,6 +5246,27 @@ > [(set_attr "type" "fp") >(set_attr "fp_type" "fp_addsub_")]) > >+(define_insn "*xsrdpidf2" >+ [(set (match_operand:DF 0 "gpc_reg_operand" "=") >+ (unspec:DF [(match_operand:DF 1 "gpc_reg_operand" "")] >+ UNSPEC_XSRDPI))] >+ "TARGET_DF_FPR && TARGET_POPCNTD" >+ "xsrdpi %0,%1" >+ [(set_attr "type" "fp")]) >+ >+(define_expand "lrounddfdi2" >+ [(set (match_dup 2) >+ (unspec:DF [(match_operand:DF 1 "gpc_reg_operand" "")] >+ UNSPEC_XSRDPI)) >+ (set (match_operand:DI 0 "gpc_reg_operand" "=d") >+ (unspec:DI [(match_dup 2)] >+ UNSPEC_FCTID))] >+ "TARGET_DF_FPR && TARGET_POPCNTD >+ && flag_unsafe_math_optimizations && !flag_trapping_math" Why unsafe-math? Richard. >+{ >+ operands[2] = gen_reg_rtx (DFmode); >+}) >+ >; An UNSPEC is used so we don't have to support SImode in FP registers. > (define_insn "stfiwx" > [(set (match_operand:SI 0 "memory_operand" "=Z")