[Patch, Fortran] Fix handling of polymorphic coarrays and coarray components
This is patch is the first change from Fortran-caf which I want to merge to the trunk; it only affects -fcoarray=lib. Build and regtested xon 86-64-gnu-linux. OK for the trunk? Tobias PS: I am currently looking at another issue with polymophic coarrays, which also affects -fcoarray=single; that issue I want to solve first on the trunk – and then merge into the branch. I might pick one or the other patch from the branch for the trunk, but the main work should first stabilize on the branch before I want to submit it to the trunk. 2014-04-27 Tobias Burnus bur...@net-b.de * trans-expr.c (get_tree_for_caf_expr): Fix handling of polymorphic and derived-type coarrays. 2014-04-27 Tobias Burnus bur...@net-b.de * gfortran.dg/coarray_poly_4.f90: New. * gfortran.dg/coarray_poly_5.f90: New. diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index d6f820c..f0e5b7d 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -1387,25 +1387,42 @@ gfc_get_expr_charlen (gfc_expr *e) static tree get_tree_for_caf_expr (gfc_expr *expr) { - tree caf_decl = NULL_TREE; - gfc_ref *ref; + tree caf_decl; + bool found; + gfc_ref *ref; - gcc_assert (expr expr-expr_type == EXPR_VARIABLE); - if (expr-symtree-n.sym-attr.codimension) - caf_decl = expr-symtree-n.sym-backend_decl; + gcc_assert (expr expr-expr_type == EXPR_VARIABLE); - for (ref = expr-ref; ref; ref = ref-next) - if (ref-type == REF_COMPONENT) - { + caf_decl = expr-symtree-n.sym-backend_decl; + gcc_assert (caf_decl); + if (expr-symtree-n.sym-ts.type == BT_CLASS) +caf_decl = gfc_class_data_get (caf_decl); + if (expr-symtree-n.sym-attr.codimension) +return caf_decl; + + /* The following code assumes that the coarray is a component reachable via + only scalar components/variables; the Fortran standard guarantees this. */ + + for (ref = expr-ref; ref; ref = ref-next) +if (ref-type == REF_COMPONENT) + { gfc_component *comp = ref-u.c.component; -if (comp-attr.pointer || comp-attr.allocatable) - caf_decl = NULL_TREE; - if (comp-attr.codimension) - caf_decl = comp-backend_decl; - } - gcc_assert (caf_decl != NULL_TREE); - return caf_decl; + if (POINTER_TYPE_P (TREE_TYPE (caf_decl))) + caf_decl = build_fold_indirect_ref_loc (input_location, caf_decl); + caf_decl = fold_build3_loc (input_location, COMPONENT_REF, +TREE_TYPE (comp-backend_decl), caf_decl, +comp-backend_decl, NULL_TREE); + if (comp-ts.type == BT_CLASS) + caf_decl = gfc_class_data_get (caf_decl); + if (comp-attr.codimension) + { + found = true; + break; + } + } + gcc_assert (found caf_decl); + return caf_decl; } diff --git a/gcc/testsuite/gfortran.dg/coarray_poly_4.f90 b/gcc/testsuite/gfortran.dg/coarray_poly_4.f90 new file mode 100644 index 000..ceb1c85 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray_poly_4.f90 @@ -0,0 +1,23 @@ +! { dg-do compile } +! { dg-options -fcoarray=lib -fdump-tree-original } + +subroutine test(i) +type t + real, allocatable :: x[:] +end type t + +interface + subroutine sub(y) +import +real :: y[*] + end subroutine sub +end interface + +integer :: i +type(t), save :: var +allocate(var%x[*]) +call sub(var%x) +end subroutine test + +! { dg-final { scan-tree-dump-times sub \\(\\(real\\(kind=4\\) \\*\\) var.x.data, var.x.token, 0\\); 1 original } } +! { dg-final { cleanup-tree-dump original } } diff --git a/gcc/testsuite/gfortran.dg/coarray_poly_5.f90 b/gcc/testsuite/gfortran.dg/coarray_poly_5.f90 new file mode 100644 index 000..29c9c8c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/coarray_poly_5.f90 @@ -0,0 +1,14 @@ +! { dg-do compile } +! { dg-options -fcoarray=lib -fdump-tree-original } + +subroutine test(x) +type t + real, allocatable :: x[:] +end type t + +class(t) :: x +allocate(x%x[*]) +end subroutine test + +! { dg-final { scan-tree-dump-times x-_data-x.data = _gfortran_caf_register \\(4, 1, x-_data-x.token, 0B, 0B, 0\\); 1 original } } +! { dg-final { cleanup-tree-dump original } }
Re: [DOC PATCH] Rewrite docs for inline asm
On 04/26/2014 10:33 PM, Gerald Pfeifer wrote: +any symbols it references. This may result in those symbols getting discarded +by GCC as unreferenced. We can omit by GCC here. We can, but we should not. We should avoid the passive voice like the plague in technical documentation, even if doing so leads to some slight redundancy. Andrew.
[wwwdocs] Buildstat update for 4.4
Latest results for 4.4.x -tgc Testresults for 4.4.7: i386-pc-solaris2.7 Index: buildstat.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.4/buildstat.html,v retrieving revision 1.28 diff -u -r1.28 buildstat.html --- buildstat.html 2 Oct 2013 08:15:38 - 1.28 +++ buildstat.html 27 Apr 2014 09:15:56 - @@ -184,6 +184,14 @@ /tr tr +tdi386-pc-solaris2.7/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01925.html;4.4.7/a +/td +/tr + +tr tdi386-pc-solaris2.8/td tdnbsp;/td tdTest results:
[wwwdocs] Buildstat update for 4.6
Latest results for 4.6.x -tgc Testresults for 4.6.4: alphaev68-dec-osf5.1a Index: buildstat.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.6/buildstat.html,v retrieving revision 1.15 diff -u -r1.15 buildstat.html --- buildstat.html 4 Aug 2013 19:31:51 - 1.15 +++ buildstat.html 27 Apr 2014 09:13:43 - @@ -35,6 +35,7 @@ tdalphaev68-dec-osf5.1a/td tdnbsp;/td tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00049.html;4.6.4/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02465.html;4.6.3/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02464.html;4.6.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02472.html;4.6.1/a,
[wwwdocs] Buildstat update for 4.7
Latest results for 4.7.x -tgc Testresults for 4.7.3: alphaev68-dec-osf5.1a Index: buildstat.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/buildstat.html,v retrieving revision 1.11 diff -u -r1.11 buildstat.html --- buildstat.html 13 Oct 2013 23:50:32 - 1.11 +++ buildstat.html 27 Apr 2014 09:10:35 - @@ -42,6 +42,7 @@ tdalphaev68-dec-osf5.1a/td tdnbsp;/td tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00048.html;4.7.3/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02474.html;4.7.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02469.html;4.7.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2012-09/msg02475.html;4.7.1/a,
[wwwdocs] Buildstat update for 4.8
Latest results for 4.8.x -tgc Testresults for 4.8.2: aarch64-unknown-linux-gnu (new) hppa-unknown-linux-gnu (new) x86_64-unknown-linux-gnu Index: buildstat.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/buildstat.html,v retrieving revision 1.6 diff -u -r1.6 buildstat.html --- buildstat.html 16 Dec 2013 14:42:43 - 1.6 +++ buildstat.html 27 Apr 2014 09:03:29 - @@ -23,6 +23,14 @@ table tr +tdaarch64-unknown-linux-gnu/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-03/msg02123.html;4.8.2/a +/td +/tr + +tr tdarm-unknown-linux-gnueabi/td tdnbsp;/td tdTest results: @@ -33,6 +41,14 @@ /tr tr +tdhppa-unknown-linux-gnu/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2013-12/msg01923.html;4.8.2/a +/td +/tr + +tr tdhppa1.1-hp-hpux10.20/td tdnbsp;/td tdTest results: @@ -283,6 +299,7 @@ tdx86_64-unknown-linux-gnu/td tdnbsp;/td tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-02/msg01608.html;4.8.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2013-11/msg00373.html;4.8.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2013-10/msg01854.html;4.8.2/a, a href=http://gcc.gnu.org/ml/gcc-testresults/2013-10/msg01777.html;4.8.2/a,
[wwwdocs] Buildstat update for 4.9
Latest results for 4.9.x -tgc Testresults for 4.9.0: i386-pc-solaris2.9 (2) i386-pc-solaris2.10 i386-pc-solaris2.11 sparc-sun-solaris2.9 (2) sparc-sun-solaris2.11 sparc64-sun-solaris2.9 x86_64-unknown-linux-gnu (2) Index: buildstat.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.9/buildstat.html,v retrieving revision 1.1 diff -u -r1.1 buildstat.html --- buildstat.html 11 Apr 2014 13:36:53 - 1.1 +++ buildstat.html 27 Apr 2014 09:29:22 - @@ -20,5 +20,68 @@ a href=http://gcc.gnu.org/install/finalinstall.html; Installing GCC: Final Installation/a./p +table + +tr +tdi386-pc-solaris2.9/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg02112.html;4.9.0/a, +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01785.html;4.9.0/a +/td +/tr + +tr +tdi386-pc-solaris2.10/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01761.html;4.9.0/a +/td +/tr + +tr +tdi386-pc-solaris2.11/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01758.html;4.9.0/a +/td +/tr + +tr +tdsparc-sun-solaris2.9/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg02113.html;4.9.0/a, +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01832.html;4.9.0/a +/td +/tr + +tr +tdsparc-sun-solaris2.11/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01826.html;4.9.0/a +/td +/tr + +tr +tdsparc64-sun-solaris2.9/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg02114.html;4.9.0/a +/td +/tr + +tr +tdx86_64-unknown-linux-gnu/td +tdnbsp;/td +tdTest results: +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01757.html;4.9.0/a, +a href=http://gcc.gnu.org/ml/gcc-testresults/2014-04/msg01741.html;4.9.0/a +/td +/tr + +/table + /body /html
Re: -fuse-caller-save - Enable for MIPS
Tom de Vries tom_devr...@mentor.com writes: 2014-01-12 Radovan Obradovic robrado...@mips.com Tom de Vries t...@codesourcery.com * config/mips/mips-protos.h (mips_emit_call_insn): Declare. * config/mips/mips.h (POST_CALL_TMP_REG): Define. * config/mips/mips.c (mips_emit_call_insn): Remove static. Use last_call_insn. Add POST_CALL_TMP_REG clobber (mips_split_call): Use POST_CALL_TMP_REG. (TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS): Redefine to true. * config/mips/mips.md (define_expand untyped_call): Use mips_emit_call_insn. * gcc.target/mips/mips.exp: Add use-caller-save to -ffoo/-fno-foo options. * gcc.target/mips/fuse-caller-save.h: New include file. * gcc.target/mips/fuse-caller-save.c: New test. * gcc.target/mips/fuse-caller-save-mips16.c: Same. * gcc.target/mips/fuse-caller-save-micromips.c: Same. Sorry, a couple of things, but this is looking pretty good: mips_emit_call_insn (rtx pattern, rtx orig_addr, rtx addr, bool lazy_p) { rtx insn, reg; - insn = emit_call_insn (pattern); + emit_call_insn (pattern); + insn = last_call_insn (); if (TARGET_MIPS16 mips_use_pic_fn_addr_reg_p (orig_addr)) { This change isn't necessary; emit_call_insn is defined to return a CALL_INSN. @@ -2843,6 +2844,16 @@ mips_emit_call_insn (rtx pattern, rtx orig_addr, rtx addr, bool lazy_p) gen_rtx_REG (Pmode, GOT_VERSION_REGNUM)); emit_insn (gen_update_got_version ()); } + + if (TARGET_MIPS16 + TARGET_EXPLICIT_RELOCS + TARGET_CALL_CLOBBERED_GP + !find_reg_note (insn, REG_NORETURN, 0)) +{ + rtx post_call_tmp_reg = gen_rtx_REG (word_mode, POST_CALL_TMP_REG); + clobber_reg (CALL_INSN_FUNCTION_USAGE (insn), post_call_tmp_reg); +} The REG_NORETURN note won't be around yet, so we might as well drop that line. I'm not sure how useful it would be anyway since values are never live across a noreturn call. +/* Temporary register that is used after a call. $4 and $5 are used for Might as well make it ...used when restoring $gp after a call, now that it's not as obvious from context. + returning complex double values in soft-float code, so $6 is the first + suitable candidate for !TARGET_MIPS16. For TARGET_MIPS16, we use + PIC_OFFSET_TABLE_REGNUM instead. */ !TARGET_MIPS16 and TARGET_MIPS16 are the wrong way around: suitable candidate for TARGET_MIPS16. For !TARGET_MIPS16 we can use $gp itself as the temporary. */ +/* The scan of the sp-relative saves will fail for -O0 and -O1. + For -flto, scans will fail because there's no code in the .s file. */ +/* { dg-skip-if { *-*-* } { -O0 -O1 -flto} } */ The -flto thing is handled automatically by the testsuite (see force_conventional_output_for) so that one should be left out. I'm a bit surprised that it doesn't work at -O1 for a simple test like this though. What goes wrong? Thanks, Richard
Re: [wide-int 3/5] Fix large widths in shifted_mask
Kenneth Zadeck zad...@naturalbridge.com writes: I think that this patch is fine as is.but in looking at the surrounding code, i saw something that appears to be somewhat troubling. I am worried about the two asserts. Given that we now require that some users write code similar to the code in tree-vrp.c:2628, it seems that these asserts are only latent land mines. Yeah, they should probably go. OK to remove them? Thanks, Richard
RFA: Avoid creating garbage CONSTs in cselib.c
It looks like ~1.5% to ~2% of all rtl created comes from wrap_constant in cselib.c. Not a huge amount, but it's easy to avoid. Tested on x86_64-linux-gnu. OK to install? Thanks, Richard gcc/ * cselib.c (find_slot_memmode): Delete. (cselib_hasher): Change compare_type to a struct. (cselib_hasher::equal): Update accordingly. Don't expect wrapped constants. (preserve_constants_and_equivs): Adjust for new compare_type. (cselib_find_slot): Likewise. Take the mode of the rtx as argument. (wrap_constant): Delete. (cselib_lookup_mem, cselib_lookup_1): Update calls to cselib_find_slot. Index: gcc/cselib.c === --- gcc/cselib.c2014-04-26 13:05:40.546106651 +0100 +++ gcc/cselib.c2014-04-26 13:18:09.183434172 +0100 @@ -49,9 +49,6 @@ struct elt_list { cselib_val *elt; }; -/* See the documentation of cselib_find_slot below. */ -static enum machine_mode find_slot_memmode; - static bool cselib_record_memory; static bool cselib_preserve_constants; static bool cselib_any_perm_equivs; @@ -94,7 +91,14 @@ static rtx cselib_expand_value_rtx_1 (rt struct cselib_hasher : typed_noop_remove cselib_val { typedef cselib_val value_type; - typedef rtx_def compare_type; + struct compare_type { +/* The rtx value and its mode (needed separately for constant + integers). */ +enum machine_mode mode; +rtx x; +/* The mode of the contaning MEM, if any, otherwise VOIDmode. */ +enum machine_mode memmode; + }; static inline hashval_t hash (const value_type *); static inline bool equal (const value_type *, const compare_type *); }; @@ -118,27 +122,20 @@ cselib_hasher::hash (const value_type *v cselib_hasher::equal (const value_type *v, const compare_type *x_arg) { struct elt_loc_list *l; - rtx x = CONST_CAST_RTX (x_arg); - enum machine_mode mode = GET_MODE (x); - - gcc_assert (!CONST_SCALAR_INT_P (x) GET_CODE (x) != CONST_FIXED); + rtx x = x_arg-x; + enum machine_mode mode = x_arg-mode; + enum machine_mode memmode = x_arg-memmode; if (mode != GET_MODE (v-val_rtx)) return false; - /* Unwrap X if necessary. */ - if (GET_CODE (x) == CONST - (CONST_SCALAR_INT_P (XEXP (x, 0)) - || GET_CODE (XEXP (x, 0)) == CONST_FIXED)) -x = XEXP (x, 0); - if (GET_CODE (x) == VALUE) return x == v-val_rtx; /* We don't guarantee that distinct rtx's have different hash values, so we need to do a comparison. */ for (l = v-locs; l; l = l-next) -if (rtx_equal_for_cselib_1 (l-loc, x, find_slot_memmode)) +if (rtx_equal_for_cselib_1 (l-loc, x, memmode)) { promote_debug_loc (l); return true; @@ -498,8 +495,11 @@ preserve_constants_and_equivs (cselib_va if (invariant_or_equiv_p (v)) { + cselib_hasher::compare_type lookup = { + GET_MODE (v-val_rtx), v-val_rtx, VOIDmode + }; cselib_val **slot - = cselib_preserved_hash_table.find_slot_with_hash (v-val_rtx, + = cselib_preserved_hash_table.find_slot_with_hash (lookup, v-hash, INSERT); gcc_assert (!*slot); *slot = v; @@ -572,22 +572,19 @@ cselib_get_next_uid (void) /* Search for X, whose hashcode is HASH, in CSELIB_HASH_TABLE, INSERTing if requested. When X is part of the address of a MEM, - MEMMODE should specify the mode of the MEM. While searching the - table, MEMMODE is held in FIND_SLOT_MEMMODE, so that autoinc RTXs - in X can be resolved. */ + MEMMODE should specify the mode of the MEM. */ static cselib_val ** -cselib_find_slot (rtx x, hashval_t hash, enum insert_option insert, - enum machine_mode memmode) +cselib_find_slot (enum machine_mode mode, rtx x, hashval_t hash, + enum insert_option insert, enum machine_mode memmode) { cselib_val **slot = NULL; - find_slot_memmode = memmode; + cselib_hasher::compare_type lookup = { mode, x, memmode }; if (cselib_preserve_constants) -slot = cselib_preserved_hash_table.find_slot_with_hash (x, hash, +slot = cselib_preserved_hash_table.find_slot_with_hash (lookup, hash, NO_INSERT); if (!slot) -slot = cselib_hash_table.find_slot_with_hash (x, hash, insert); - find_slot_memmode = VOIDmode; +slot = cselib_hash_table.find_slot_with_hash (lookup, hash, insert); return slot; } @@ -1042,18 +1039,6 @@ rtx_equal_for_cselib_1 (rtx x, rtx y, en return 1; } -/* We need to pass down the mode of constants through the hash table - functions. For that purpose, wrap them in a CONST of the appropriate - mode. */ -static rtx -wrap_constant (enum machine_mode mode, rtx x) -{ - if (!CONST_SCALAR_INT_P (x) GET_CODE (x) != CONST_FIXED) -return x; - gcc_assert (mode != VOIDmode); - return gen_rtx_CONST (mode, x); -} - /* Hash an
Re: [patch, fortran] Fix PR 59604
Hi Tobias, Am 12.04.2014 21:33, schrieb Thomas Koenig: please find attached a patch for PR 59604. The patch makes sure that, if -fno-range-check is specified, using int on an overflowing boz constant yields the same result for compile-time simplification and run-time execution. OK for trunk? Looks good to me. At a glance, it looks as if it also fixes the original bug 58003 - or is something still missing? You're right - the original PR is also fixed. I have included a modification of the original test case, and mentioned PR 58003 in the ChangeLog. Thanks! Thomas
Re: [DOC PATCH] Rewrite docs for inline asm
any symbols it references. This may result in those symbols getting discarded by GCC as unreferenced. We can omit by GCC here. We can, but we should not. We should avoid the passive voice like the plague in technical documentation, even if doing so leads to some slight redundancy. I agree, but that's still passive voice (you need not omit the actor to be using passive voice)! Active voice (which is indeed preferred) is This may result in GCC discarding those symbols as unreferenced. (For those who don't know, a good test of whether something is the passive voice is that you *could* remove the actor and the sentence would still be grammatically correct. That's the case above.) Writing in the passive voice with the actor present is indeed better than the passive voice with the actor removed, but the active voice is still the preferred style.
Re: [patch, libgfortran] Wrong result for UTF-8/UCS-4 list-directed and namelist read and nml write
Jerry DeLisle jvdeli...@charter.net writes: +static void +push_char4 (st_parameter_dt *dtp, gfc_char4_t c) +{ + gfc_char4_t *new, *p = (gfc_char4_t *) dtp-u.p.saved_string; + + if (p == NULL) +{ + dtp-u.p.saved_string = xcalloc (SCRATCH_SIZE, sizeof (gfc_char4_t)); + dtp-u.p.saved_length = SCRATCH_SIZE; + dtp-u.p.saved_used = 0; + p = (gfc_char4_t *) dtp-u.p.saved_string; +} + + if (dtp-u.p.saved_used = dtp-u.p.saved_length) +{ + dtp-u.p.saved_length = 2 * dtp-u.p.saved_length; + new = realloc (p, dtp-u.p.saved_length); That's a buffer overflow. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function? Trev signature.asc Description: Digital signature
Re: [Patch 2/3] Make regex's infinite loop detection accurate
On 26/04/14 22:34 -0400, Tim Shen wrote: On Sat, Apr 26, 2014 at 1:00 PM, Jonathan Wakely jwak...@redhat.com wrote: Maybe a dumb question (I don't understand a lot of the regex code!) but is it correct to set this to 1 in the case where __rep_count.first != _M_current ? Could that result in the count going downwards from 2 to 1? (Maybe that's correct?) As I said in the comment, __rep_count records how many times (__rep_count.second) this node is visited *under certain input iterator* (__rep_count.first). That is to say, if the current iterator (_M_current) is not changing, but the executor keeps visiting this node, times of visit needs to be recorded. Once the _M_current changed, the count should be reset to 0. We simply set it to 1 for the set to 0 then increase by 1 operations. Oh of course, that makes perfect sense now, thanks! This patch is OK to commit. About the overall regex code, I think I can compose a blog entry to explain it, if necessary? If you do have time to do that it would be fantastic, I'm sure I wouldn't be the only person who would find it valuable.
Re: [Patch 3/3] Separate `repeating` node from `alternative` node in regex
On 25/04/14 18:04 -0400, Tim Shen wrote: * include/bits/regex_automaton.h (_NFA::_M_insert_repeat): Add _S_opcode_repeat support to distingush a loop from _S_opcode_alternative. * include/bits/regex_automaton.tcc (_State_base::_M_print, _State_base::_M_dot, _NFA::_M_eliminate_dummy, _StateSeq::_M_clone): Likewise. * include/bits/regex_compiler.tcc (_Compiler::_M_quantifier): Likewise. * include/bits/regex_executor.tcc (_Executor::_M_dfs): Likewise. * include/bits/regex_scanner.tcc (_Scanner::_M_eat_escape_ecma): Uglify local variable __i. * include/bits/regex_compiler.h (_BracketMatcher::_M_make_cache): Use size_t instead of int to compare with vector::size(). This one is OK to commit too - thanks.
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function? No, shall change that to inline function. Trev
Re: [Patch 2/3] Make regex's infinite loop detection accurate
On 27/04/14 13:28 +0100, Jonathan Wakely wrote: On 26/04/14 22:34 -0400, Tim Shen wrote: On Sat, Apr 26, 2014 at 1:00 PM, Jonathan Wakely jwak...@redhat.com wrote: Maybe a dumb question (I don't understand a lot of the regex code!) but is it correct to set this to 1 in the case where __rep_count.first != _M_current ? Could that result in the count going downwards from 2 to 1? (Maybe that's correct?) As I said in the comment, __rep_count records how many times (__rep_count.second) this node is visited *under certain input iterator* (__rep_count.first). That is to say, if the current iterator (_M_current) is not changing, but the executor keeps visiting this node, times of visit needs to be recorded. Once the _M_current changed, the count should be reset to 0. We simply set it to 1 for the set to 0 then increase by 1 operations. Oh of course, that makes perfect sense now, thanks! This patch is OK to commit. ... with the fix for the un-uglified variable name.
Re: debug container patch
On 17/04/14 22:43 +0200, François Dumont wrote: Hi Here is a patch to globally enhance debug containers implementation. François, sorry for the delay, this is a large patch and I wanted to give it the time it deserves to review properly. I have isolated all code of special functions in a base class so that in C++11 we can use default implementations for the debug containers. This way implementation is simpler and inherit from the noexcept qualifications. I like this approach, the result is simpler and less repetitive, but I'm slightly concerned by inverting the inheritance ... _Safe_container is now using the debug base class (_Safe_sequence, _Safe_node_sequence, _Safe_forward_list...) as a policy describing what must be done as _M_invalidate_all or _M_swap. Note that I needed to invert inheritance so that I can use this base type to do checks with allocators before they get potentially moved by the normal code. I understand why this is needed, but it changes the layout of the classes in very significant ways, meaning the debug containers will not be compatible across GCC releases. I'm OK with that now, but from the next major GCC release I'd like to avoid that in future. This is the case for the allocator aware move constructor and move assignment operator. With the allocator aware move constructor it is in fact fixing a bug, iterators were not correctly swap when they had to. Great, I had been meaning to fully review the iterator handling in those kind of cases, thanks for finding and fixing this. I had to put a _IsCpp11AllocatorAware template parameter to this new type for types that are not yet C++11 allocator aware. We will be able to simplify it later. Yes, that should become unnecessary by the end of stage 1, but for now it serves a purpose. As I said in my other email, please rename it to _IsCxx11AllocatorAware, for consistency with the rest of the library. I noticed also that in std/c++11/debug.cc we have some methods qualified with noexcept while in a C++03 user code those methods will have a throw() qualification. Is that fine ? As I said in my last mail, yes, those specifications are compatible. But I don't think your changes are doing what you think they are doing in all cases. Using _GLIBCXX_NOEXCEPT does not expand to throw() in C++03 mode, it expands to nothing. If you want a macro that expands to either throw() or noexcept, then you should be using _GLIBCXX_USE_NOEXCEPT. * include/debug/safe_sequence.tcc * include/debug/safe_unordered_base.h * include/debug/safe_unordered_container.h (_Safe_unordered_container_base()): Add noexcept. (~_Safe_unordered_container_base()): Likewise. (_M_swap(_Safe_unordered_container_base)): Likewise. * include/debug/safe_unordered_container.tcc N.B. This file has no changes listed in the changelog entry. @@ -69,8 +75,26 @@ // 23.2.1.1 construct/copy/destroy: - deque() : _Base() { } +#if __cplusplus 201103L + deque() + : _Base() { } + deque(const deque __x) + : _Base(__x) { } + + ~deque() _GLIBCXX_NOEXCEPT { } In C++03 mode the _GLIBCXX_NOEXCEPT macro expands to an empty string, so it is useless in this chunk of code, which is only compiled for C++03 mode. It should probably just be removed here (and in all the other debug containers which use it in C++03-only code). Index: include/debug/forward_list === --- include/debug/forward_list (revision 209446) +++ include/debug/forward_list (working copy) @@ -33,8 +33,113 @@ #include forward_list #include debug/safe_sequence.h +#include debug/safe_container.h #include debug/safe_iterator.h +namespace __gnu_debug +{ + /// Special iterators swap and invalidation for forward_list because of the + /// before_begin iterator. + templatetypename _SafeSequence +class _Safe_forward_list +: public _Safe_sequence_SafeSequence +{ + _SafeSequence + _M_this() noexcept + { return *static_cast_SafeSequence*(this); } + + static void + _M_swap_aux(_Safe_sequence_base __lhs, + _Safe_iterator_base* __lhs_iterators, + _Safe_sequence_base __rhs, + _Safe_iterator_base* __rhs_iterators); + +protected: + void + _M_invalidate_all() + { + using _Base_const_iterator = __decltype(_M_this()._M_base().cend()); + this-_M_invalidate_if([this](_Base_const_iterator __it) + { + return __it != _M_this()._M_base().cbefore_begin() +__it != _M_this()._M_base().cend(); }); + } + + void _M_swap(_Safe_sequence_base) noexcept; +}; + + templatetypename _SafeSequence +void +_Safe_forward_list_SafeSequence:: +_M_swap_aux(_Safe_sequence_base __lhs, + _Safe_iterator_base* __lhs_iterators, + _Safe_sequence_base __rhs, + _Safe_iterator_base* __rhs_iterators) +{ +
Re: [patch, libgfortran] Wrong result for UTF-8/UCS-4 list-directed and namelist read and nml write
On 04/27/2014 04:57 AM, Andreas Schwab wrote: Jerry DeLisle jvdeli...@charter.net writes: +static void +push_char4 (st_parameter_dt *dtp, gfc_char4_t c) +{ + gfc_char4_t *new, *p = (gfc_char4_t *) dtp-u.p.saved_string; + + if (p == NULL) +{ + dtp-u.p.saved_string = xcalloc (SCRATCH_SIZE, sizeof (gfc_char4_t)); + dtp-u.p.saved_length = SCRATCH_SIZE; + dtp-u.p.saved_used = 0; + p = (gfc_char4_t *) dtp-u.p.saved_string; +} + + if (dtp-u.p.saved_used = dtp-u.p.saved_length) +{ + dtp-u.p.saved_length = 2 * dtp-u.p.saved_length; + new = realloc (p, dtp-u.p.saved_length); That's a buffer overflow. Do you mean it should be? new = realloc (p, dtp-u.p.saved_length * sizeof (gfc_char4_t)); jerry
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Trev Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function? No, shall change that to inline function. Trev signature.asc Description: Digital signature
[Patch, Fortran] (port branch to trunk) this_image()/num_images() changes for TS18508 and minor cleanup
This patch ports another* patch from the Fortran-caf branch to the trunk; it only affects -fcoarray=lib. An earlier patch was posted before,** remains unreviewed and is replaced by this patch (re-diffed, minutely enhanced). Besides some minor clean-up in libgfortran/caf, the patch changes the handling of this_image()/num_images(). The current code on the trunk calls caf_init in the main program - and uses this to obtain the value of this_images()/num_images(). The values are then stored in global variables. That procedure has two disadvantages: a) If one uses coarrays only in some parts of the program (e.g. in a library) and not in the main program, caf_init will never be called and one refers to a nonexisting variable (link-time failure). [Okay, one could require that that the main program is compiled with -fcoarray=lib or the user calls caf_init manually, e.g. if the code is some C code linking to a Fortran program.] b) The Technical Specification (TS) 18508 Additional Parallel Features in Fortran***, which extends the coarray support, will support teams. When changing to a new team (CHANGE TEAM), this_image and num_images change but the current scheme does not support this. Thus, the patch removes the static variables and calls a library function. The only down side is that this will lead to some missed optimization cases, when one calls multiple times to this_image()/num_images() in an expression. But I think that can be better be solved in the front-end optimization pass. Note that TS18508's this_image() is not only able to return the image index of the current team but also its index in the n-th ancestor team (optional distance argument). Similarly, num_images() takes an optional distance argument - and an optional Boolean argument, which requests to return the number of failed images. Thus, to prepare for TS18508, the attached patch also adds the additional arguments. (The optional coarray array argument of this_image() was and will be handled in the front-end itself.) Built and regtested on x86-64-gnu-linux. OK for the trunk? Tobias * The first patch was: http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01769.html ** http://gcc.gnu.org/ml/fortran/2014-03/msg00030.html *** Current draft: ftp://ftp.nag.co.uk/sc22wg5/N2001-N2050/N2007.pdf ; latest status (also known as ballot): ftp://ftp.nag.co.uk/sc22wg5/N2001-N2050/N2013.txt - however, those comments do not affect this patch. PS: I am aware of at least three patches which someone (I?) should review; I will try to find some time for those. 2014-04-27 Tobias Burnus bur...@net-b.de * gfortran.h (gfc_init_coarray_decl): Remove. * parse.c (translate_all_program_units): Remove call to it. (gfc_parse_file): Update call. * trans.h (gfor_fndecl_caf_this_image, gfor_fndecl_caf_num_images): Add. (gfort_gvar_caf_num_images, gfort_gvar_caf_this_image): Remove. * trans-decl.c (gfor_fndecl_caf_this_image, gfor_fndecl_caf_num_images): Add. (gfort_gvar_caf_num_images, gfort_gvar_caf_this_image): Remove. (gfc_build_builtin_function_decls): Init new decl. (gfc_init_coarray_dec): Remove. (create_main_function): Change calls. * trans-intrinsic.c (trans_this_image, trans_image_index, conv_intrinsic_cobound): Generate call to new library function instead of to a static variable. * trans-stmt.c (gfc_trans_sync): Ditto. 2014-04-27 Tobias Burnus bur...@net-b.de * gfortran.dg/coarray_lib_this_image_1.f90: New. * gfortran.dg/coarray_lib_this_image_2.f90: New. 2014-03-08 Tobias Burnus bur...@net-b.de * caf/libcaf.h (_gfortran_caf_this_image, _gfortran_caf_num_images): New prototypes. (_gfortran_caf_init): Change prototype. (mpi_token_t): New typedef. (TOKEN): New define. * caf/mpi.c (_gfortran_caf_this_image, _gfortran_caf_num_images): New functions. (_gfortran_caf_init): Update. (_gfortran_caf_finalize, _gfortran_caf_register, _gfortran_caf_deregister): Use mpi_token_t. * caf/single.c (_gfortran_caf_this_image, _gfortran_caf_num_images): New functions. (_gfortran_caf_init): Update. (_gfortran_caf_finalize, _gfortran_caf_register, _gfortran_caf_deregister): Use mpi_token_t, simplify. diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index f0eed80..0707b58 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2948,7 +2948,6 @@ bool gfc_convert_to_structure_constructor (gfc_expr *, gfc_symbol *, /* trans.c */ void gfc_generate_code (gfc_namespace *); void gfc_generate_module_code (gfc_namespace *); -void gfc_init_coarray_decl (bool); /* trans-intrinsic.c */ bool gfc_inline_intrinsic_function_p (gfc_expr *); diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c index 0faf47a..7766715 100644 --- a/gcc/fortran/parse.c +++ b/gcc/fortran/parse.c @@ -4495,19 +4495,13 @@ clean_up_modules (gfc_gsymbol *gsym) /* Translate all the program units. This could be in a different order to resolution if there are forward references in the file. */ static void
[Patch, Fortran] Fix ucobound checking bug and -fcoarray=lib argument passing
This patch fixes two issues: a) ucobound(x) of an assumed-size array was wrongly rejected. b) For assumed-shape arrays, the dummy argument expected an array descriptor, which contains the codimension. However, only for allocatable coarrays, the coshape is stored in the descriptor - for all others, the corank and the coshape is defined at dummy declaration time. - Consequently, we also only passed for assumed-shape dummies the shape - and used the local coshape. Except: the tree decl to the dummy variable expected a descriptor of rank+corank size. Fortunately, simply fixing the typedecl of the descriptor was sufficient. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: The last lines of the patch require http://gcc.gnu.org/ml/fortran/2014-04/msg00091.html to be applied as they modify the dump of the latter. 2014-04-27 Tobias Burnus bur...@net-b.de * resolve.c (resolve_function): Don't do assumed-size check for lcobound/ucobound. * trans-types.c (gfc_build_array_type): Only build an array descriptor with codimensions for allocatable coarrays. 2014-04-27 Tobias Burnus bur...@net-b.de * gfortran.dg/coarray_lib_this_image_2.f90: Update dump. * gfortran.dg/coarray_lib_token_4.f90: Ditto. * gfortran.dg/coarray/codimension.f90: New. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 38755fe..15c9463 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -2942,6 +2942,8 @@ resolve_function (gfc_expr *expr) else if (expr-value.function.actual != NULL expr-value.function.isym != NULL GENERIC_ID != GFC_ISYM_LBOUND + GENERIC_ID != GFC_ISYM_LCOBOUND + GENERIC_ID != GFC_ISYM_UCOBOUND GENERIC_ID != GFC_ISYM_LEN GENERIC_ID != GFC_ISYM_LOC GENERIC_ID != GFC_ISYM_C_LOC diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c index 243feb7..f693712 100644 --- a/gcc/fortran/trans-types.c +++ b/gcc/fortran/trans-types.c @@ -1303,7 +1303,14 @@ gfc_build_array_type (tree type, gfc_array_spec * as, { tree lbound[GFC_MAX_DIMENSIONS]; tree ubound[GFC_MAX_DIMENSIONS]; - int n; + int n, corank; + + /* Assumed-shape arrays do not have codimension information stored in the + descriptor. */ + corank = as-corank; + if (as-type == AS_ASSUMED_SHAPE || + (as-type == AS_ASSUMED_RANK akind == GFC_ARRAY_ALLOCATABLE)) +corank = 0; if (as-type == AS_ASSUMED_RANK) for (n = 0; n GFC_MAX_DIMENSIONS; n++) @@ -1322,14 +1329,14 @@ gfc_build_array_type (tree type, gfc_array_spec * as, ubound[n] = gfc_conv_array_bound (as-upper[n]); } - for (n = as-rank; n as-rank + as-corank; n++) + for (n = as-rank; n as-rank + corank; n++) { if (as-type != AS_DEFERRED as-lower[n] == NULL) lbound[n] = gfc_index_one_node; else lbound[n] = gfc_conv_array_bound (as-lower[n]); - if (n as-rank + as-corank - 1) + if (n as-rank + corank - 1) ubound[n] = gfc_conv_array_bound (as-upper[n]); } @@ -1341,7 +1348,7 @@ gfc_build_array_type (tree type, gfc_array_spec * as, : GFC_ARRAY_ASSUMED_RANK; return gfc_get_array_type_bounds (type, as-rank == -1 ? GFC_MAX_DIMENSIONS : as-rank, -as-corank, lbound, +corank, lbound, ubound, 0, akind, restricted); } diff --git a/gcc/testsuite/gfortran.dg/coarray_lib_token_4.f90 b/gcc/testsuite/gfortran.dg/coarray_lib_token_4.f90 index 43da9f4..9e445f4 100644 --- a/gcc/testsuite/gfortran.dg/coarray_lib_token_4.f90 +++ b/gcc/testsuite/gfortran.dg/coarray_lib_token_4.f90 @@ -35,9 +35,9 @@ end program test_caf ! { dg-final { scan-tree-dump-times expl \\(integer\\(kind=4\\).0:. . restrict z, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+\\) 1 original } } ! -! { dg-final { scan-tree-dump-times bar \\(struct array2_integer\\(kind=4\\) restrict y, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+\\) 1 original } } +! { dg-final { scan-tree-dump-times bar \\(struct array1_integer\\(kind=4\\) restrict y, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+\\) 1 original } } ! -! { dg-final { scan-tree-dump-times foo \\(struct array2_integer\\(kind=4\\) restrict x, struct array2_integer\\(kind=4\\) restrict y, integer\\(kind=4\\) restrict test, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+\\) 1 original } } +! { dg-final { scan-tree-dump-times foo \\(struct array1_integer\\(kind=4\\) restrict x, struct array1_integer\\(kind=4\\) restrict y, integer\\(kind=4\\) restrict test, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+, void . restrict caf_token.\[0-9\]+, integer\\(kind=.\\) caf_offset.\[0-9\]+\\) 1 original } } ! ! { dg-final { scan-tree-dump-times bar \\(parm.\[0-9\]+, caf_token.\[0-9\]+, \\(\\(integer\\(kind=.\\)\\) parm.\[0-9\]+.data -
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Trev Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function? No, shall change that to inline function. Trev Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + bool is_array_parm; }; struct GTY(()) tree_decl_with_vis { Index: gcc/tree.h === --- gcc/tree.h (revision 209800) +++ gcc/tree.h (working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define
[PATCH] testsuite: Register loaded libs
libffi/ChangeLog 2014-04-27 Sebastian Huber sebastian.hu...@embedded-brains.de * testsuite/lib/libffi.exp (load_gcc_lib): Register loaded libs. libjava/ChangeLog 2014-04-27 Sebastian Huber sebastian.hu...@embedded-brains.de * testsuite/lib/libjava.exp (load_gcc_lib): Register loaded libs. libstdc++-v3/ChangeLog 2014-04-27 Sebastian Huber sebastian.hu...@embedded-brains.de * testsuite/lib/libstdc++.exp (load_gcc_lib): Register loaded libs. --- libffi/testsuite/lib/libffi.exp | 4 +++- libjava/testsuite/lib/libjava.exp| 4 +++- libstdc++-v3/testsuite/lib/libstdc++.exp | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libffi/testsuite/lib/libffi.exp b/libffi/testsuite/lib/libffi.exp index 3c61baa..25efce2 100644 --- a/libffi/testsuite/lib/libffi.exp +++ b/libffi/testsuite/lib/libffi.exp @@ -15,8 +15,10 @@ # http://www.gnu.org/licenses/. proc load_gcc_lib { filename } { -global srcdir +global srcdir loaded_libs + load_file $srcdir/../../gcc/testsuite/lib/$filename +set loaded_libs($filename) } load_lib dg.exp diff --git a/libjava/testsuite/lib/libjava.exp b/libjava/testsuite/lib/libjava.exp index 0de823b..0cfb253 100644 --- a/libjava/testsuite/lib/libjava.exp +++ b/libjava/testsuite/lib/libjava.exp @@ -2,8 +2,10 @@ # Free Software Foundation proc load_gcc_lib { filename } { -global srcdir +global srcdir loaded_libs + load_file $srcdir/../../gcc/testsuite/lib/$filename +set loaded_libs($filename) } load_lib libgloss.exp diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp index 95954d8..a23ea3b 100644 --- a/libstdc++-v3/testsuite/lib/libstdc++.exp +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp @@ -38,8 +38,10 @@ # ~/.dejagnurc or $DEJAGNU. proc load_gcc_lib { filename } { -global srcdir +global srcdir loaded_libs + load_file $srcdir/../../gcc/testsuite/lib/$filename +set loaded_libs($filename) } # system routines -- 1.8.1.4
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? Thanks, Andrew [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Trev Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function? No, shall change that to inline function. Trev sizeof-array-argument.patch
Re: patch fortran, pr 59746, internal compiler error : segmentation fault
Given that we are now in stage 1: Mikael and Bud, what's the status of this patch? http://gcc.gnu.org/ml/fortran/2014-03/msg00098.html Tobias On January 13, 2014 22:18, Mikael Morin wrote: Hello, Le 10/03/2014 03:15, jimmie.da...@l-3com.com a écrit : Index: gcc/gcc/fortran/symbol.c === --- gcc/gcc/fortran/symbol.c(revision 208437) +++ gcc/gcc/fortran/symbol.c(working copy) @@ -3069,56 +3069,56 @@ FOR_EACH_VEC_ELT (latest_undo_chgset-syms, i, p) { - if (p-gfc_new) + /* Symbol was new. Or was old and just put in common. */ Now the comment needs updating as just put in common also applies to the new case. Or you can also remove it (just put in common is somewhat redundant with the other comment anyway). + if ( p-attr.in_common p-common_block p-common_block-head + (p-gfc_new || !p-old_symbol-attr.in_common)) { - /* Symbol was new. */ - if (p-attr.in_common p-common_block p-common_block-head) - { - /* If the symbol was added to any common block, it -needs to be removed to stop the resolver looking -for a (possibly) dead symbol. */ + /* If the symbol was added to any common block, it + needs to be removed to stop the resolver looking + for a (possibly) dead symbol. */ needs should be aligned with If like it was before; same for for. Now we are in pretty good shape. The ICE happens with invalid code after reporting an error, correct? Then I agree, this should rather wait for stage 1. Thanks Mikael
[Patch, Fortran] Fix an issue with CLASS and -fcoarray=lib on the trunk
First, I would be really delighted if someone could review my coarray patches for the trunk as it makes simpler to develop patches on top of it: * http://gcc.gnu.org/ml/fortran/2014-04/msg00087.html * http://gcc.gnu.org/ml/fortran/2014-04/msg00091.html * http://gcc.gnu.org/ml/fortran/2014-04/msg00092.html Secondly, attached is a patch which fixes an ICE - and prepares for some additional class-related coarray patches. In particular, the patch ensures that for nonallocatable *polymorphic* coarrays, the coarray token and offset are passed. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: There is still something wrong (for both -fcoarray=single and -fcoarray=lib) with lcobound/ucobounds and polymorphic coarrays and with using them with select type and associated. That's something I would like to tackle next. If that's done, I probably should really concentrate on reviewing a few patches and doing some other bug fixes before continue working on coarrays. 2014-04-27 Tobias Burnus bur...@net-b.de * trans-decl.c (create_function_arglist): Add hidden coarray arguments also for polymorphic coarrays. * trans-expr.c (gfc_conv_procedure_call): Pass hidden coarray arguments also for polymorphic coarrays. 2014-04-27 Tobias Burnus bur...@net-b.de * gfortran.dg/coarray_poly_7.f90 * gfortran.dg/coarray_poly_8.f90 * gfortran.dg/coarray_poly_9.f90 diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index c835a3b..ee6c7e3 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -2234,9 +2234,12 @@ create_function_arglist (gfc_symbol * sym) /* Coarrays which are descriptorless or assumed-shape pass with -fcoarray=lib the token and the offset as hidden arguments. */ - if (f-sym-attr.codimension - gfc_option.coarray == GFC_FCOARRAY_LIB - !f-sym-attr.allocatable) + if (gfc_option.coarray == GFC_FCOARRAY_LIB + ((f-sym-ts.type != BT_CLASS f-sym-attr.codimension + !f-sym-attr.allocatable) + || (f-sym-ts.type == BT_CLASS + CLASS_DATA (f-sym)-attr.codimension + !CLASS_DATA (f-sym)-attr.allocatable))) { tree caf_type; tree token; @@ -2244,13 +2247,18 @@ create_function_arglist (gfc_symbol * sym) gcc_assert (f-sym-backend_decl != NULL_TREE !sym-attr.is_bind_c); - caf_type = TREE_TYPE (f-sym-backend_decl); + caf_type = f-sym-ts.type == BT_CLASS + ? TREE_TYPE (CLASS_DATA (f-sym)-backend_decl) + : TREE_TYPE (f-sym-backend_decl); token = build_decl (input_location, PARM_DECL, create_tmp_var_name (caf_token), build_qualified_type (pvoid_type_node, TYPE_QUAL_RESTRICT)); - if (f-sym-as-type == AS_ASSUMED_SHAPE) + if ((f-sym-ts.type != BT_CLASS + f-sym-as-type != AS_DEFERRED) + || (f-sym-ts.type == BT_CLASS + CLASS_DATA (f-sym)-as-type != AS_DEFERRED)) { gcc_assert (DECL_LANG_SPECIFIC (f-sym-backend_decl) == NULL || GFC_DECL_TOKEN (f-sym-backend_decl) == NULL_TREE); @@ -2275,7 +2283,10 @@ create_function_arglist (gfc_symbol * sym) create_tmp_var_name (caf_offset), gfc_array_index_type); - if (f-sym-as-type == AS_ASSUMED_SHAPE) + if ((f-sym-ts.type != BT_CLASS + f-sym-as-type != AS_DEFERRED) + || (f-sym-ts.type == BT_CLASS + CLASS_DATA (f-sym)-as-type != AS_DEFERRED)) { gcc_assert (GFC_DECL_CAF_OFFSET (f-sym-backend_decl) == NULL_TREE); diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index f0e5b7d..6b93537 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -4783,19 +4783,24 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* For descriptorless coarrays and assumed-shape coarray dummies, we pass the token and the offset as additional arguments. */ - if (fsym fsym-attr.codimension - gfc_option.coarray == GFC_FCOARRAY_LIB - !fsym-attr.allocatable - e == NULL) + if (fsym e == NULL gfc_option.coarray == GFC_FCOARRAY_LIB + ((fsym-ts.type != BT_CLASS fsym-attr.codimension + !fsym-attr.allocatable) + || (fsym-ts.type == BT_CLASS + CLASS_DATA (fsym)-attr.codimension + !CLASS_DATA (fsym)-attr.allocatable))) { /* Token and offset. */ vec_safe_push (stringargs, null_pointer_node); vec_safe_push (stringargs, build_int_cst (gfc_array_index_type, 0)); gcc_assert (fsym-attr.optional); } - else if (fsym fsym-attr.codimension - !fsym-attr.allocatable - gfc_option.coarray == GFC_FCOARRAY_LIB) + else if (fsym gfc_option.coarray == GFC_FCOARRAY_LIB + ((fsym-ts.type != BT_CLASS fsym-attr.codimension + !fsym-attr.allocatable) + || (fsym-ts.type == BT_CLASS + CLASS_DATA (fsym)-attr.codimension + !CLASS_DATA (fsym)-attr.allocatable))) { tree caf_decl, caf_type; tree offset, tmp2; @@ -4837,22 +4842,30 @@ gfc_conv_procedure_call (gfc_se * se,
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 11:22 PM, pins...@gmail.com wrote: On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? I guess the warning would be shared by c-family languages, so I had added the field to tree_parm_decl. This patch is C-only (added the member to c-lang.h:lang_type instead). [gcc/c] * c-decl.c (push_parm_decl): Check if declarator is array parameter. * c-lang.h (lang_type): Add new member is_array_parm. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Trev Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_FIELDS(NODE) \ @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree #define DECL_INCOMING_RTL(NODE) \ (PARM_DECL_CHECK (NODE)-parm_decl.incoming_rtl) +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ + (PARM_DECL_CHECK (NODE)-parm_decl.is_array_parm = (val)) if we're adding more stuff here is there a reason it needs to be a macro not a inline function?
Re: [wwwdocs] Buildstat update for 4.4
On Sun, 27 Apr 2014, Tom G. Christensen wrote: Testresults for 4.4.7: i386-pc-solaris2.7 Wow. Solaris 7/x86? That's really old. :-) Applied. Gerald
Re: [wwwdocs] Buildstat update for 4.6
On Sun, 27 Apr 2014, Tom G. Christensen wrote: Testresults for 4.6.4: alphaev68-dec-osf5.1a Applied, thanks. Gerald
Re: [wwwdocs] Buildstat update for 4.7
On Sun, 27 Apr 2014, Tom G. Christensen wrote: Testresults for 4.7.3: alphaev68-dec-osf5.1a That may be the last report for this platform we'll get ever, except for a GCC 4.7.4 perhaps. Thanks, applied. Gerald
Re: [wwwdocs] Buildstat update for 4.8
On Sun, 27 Apr 2014, Tom G. Christensen wrote: Testresults for 4.8.2: aarch64-unknown-linux-gnu (new) hppa-unknown-linux-gnu (new) x86_64-unknown-linux-gnu Thanks, applied. Gerald
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 12:50 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 11:22 PM, pins...@gmail.com wrote: On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? I guess the warning would be shared by c-family languages, so I had added the field to tree_parm_decl. This patch is C-only (added the member to c-lang.h:lang_type instead). That was not talking about. I was talking about DECL_LANG_FLAG_* which is already there for your usage. You should be able to use DECL_LANG_FLAG_2 as it is unused for both C and C++ for PARM_DECLs. This should also reduce the size of the patch too. Thanks, Andrew Pinski [gcc/c] * c-decl.c (push_parm_decl): Check if declarator is array parameter. * c-lang.h (lang_type): Add new member is_array_parm. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Trev Index: gcc/tree.h === --- gcc/tree.h(revision 209800) +++ gcc/tree.h(working copy) @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location #define TYPE_LANG_SPECIFIC(NODE) \ (TYPE_CHECK (NODE)-type_with_lang_specific.lang_specific) + #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)-type_non_common.values) #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)-type_non_common.values) #define
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Mon, Apr 28, 2014 at 1:31 AM, Andrew Pinski pins...@gmail.com wrote: On Sun, Apr 27, 2014 at 12:50 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 11:22 PM, pins...@gmail.com wrote: On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? I guess the warning would be shared by c-family languages, so I had added the field to tree_parm_decl. This patch is C-only (added the member to c-lang.h:lang_type instead). That was not talking about. I was talking about DECL_LANG_FLAG_* which is already there for your usage. You should be able to use DECL_LANG_FLAG_2 as it is unused for both C and C++ for PARM_DECLs. This should also reduce the size of the patch too. Thanks for pointing it out, I have modified the patch. [gcc/c] * c-decl.c (push_parm_decl): Set DECL_LANG_FLAG_2 (decl) if declarator is array parameter. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew Pinski [gcc/c] * c-decl.c (push_parm_decl): Check if declarator is array parameter. * c-lang.h (lang_type): Add new member is_array_parm. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Trev Index: gcc/tree.h
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Sun, Apr 27, 2014 at 1:25 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Mon, Apr 28, 2014 at 1:31 AM, Andrew Pinski pins...@gmail.com wrote: On Sun, Apr 27, 2014 at 12:50 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 11:22 PM, pins...@gmail.com wrote: On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? I guess the warning would be shared by c-family languages, so I had added the field to tree_parm_decl. This patch is C-only (added the member to c-lang.h:lang_type instead). That was not talking about. I was talking about DECL_LANG_FLAG_* which is already there for your usage. You should be able to use DECL_LANG_FLAG_2 as it is unused for both C and C++ for PARM_DECLs. This should also reduce the size of the patch too. Thanks for pointing it out, I have modified the patch. [gcc/c] * c-decl.c (push_parm_decl): Set DECL_LANG_FLAG_2 (decl) if declarator is array parameter. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Can you add a new macro in c-tree.h for this new usage of DECL_LANG_FLAG_2 so it is easier to figure out what the used lang flag bits are in use and to understand what that flag bit means? Thanks, Andrew Pinski Thanks and Regards, Prathamesh Thanks, Andrew Pinski [gcc/c] * c-decl.c (push_parm_decl): Check if declarator is array parameter. * c-lang.h (lang_type): Add new member is_array_parm. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew [gcc] * tree-core.h (tree_parm_decl): Add new member bool is_array_parm * tree.h (set_parm_decl_is_array): New function. (parm_decl_array_p): New function. [gcc/c] * c-decl.c
Re: RFA: Avoid creating garbage CONSTs in cselib.c
Steven Bosscher stevenb@gmail.com writes: It looks like ~1.5% to ~2% of all rtl created comes from wrap_constant in cselib.c. Not a huge amount, but it's easy to avoid. Tested on x86_64-linux-gnu. OK to install? Nice patch. OK after also testing this on an autoinc target. OK, installed after comparing the asm output before and after the patch on gcc.torture, gcc.dg and g++.dg for arm-eabi. There were no differences. Thanks, Richard
Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument
On Mon, Apr 28, 2014 at 2:13 AM, Andrew Pinski pins...@gmail.com wrote: On Sun, Apr 27, 2014 at 1:25 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Mon, Apr 28, 2014 at 1:31 AM, Andrew Pinski pins...@gmail.com wrote: On Sun, Apr 27, 2014 at 12:50 PM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 11:22 PM, pins...@gmail.com wrote: On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni bilbotheelffri...@gmail.com wrote: On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders tsaund...@mozilla.com wrote: On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: Hi, Shall it a good idea to add new warning -Wsizeof-array-argument that warns when sizeof is applied on parameter declared as an array ? Seems reasonable enough. Similar to clang's -Wsizeof-array-argument: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html This was also reported as PR6940: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 I have attached a patch that adds the warning to C front-end. if we're doing this for C, we should probably do it for C++ too. I implemented it by adding a new member BOOL_BITFIELD is_array_parm to tree_parm_decl. Not sure if that's a good approach. I'm about the last one who should comment on this, but it seems pretty crazy you can't use data that's already stored. AFAIU, the information about declarator is stored in c_declarator. c_declarator-kind == cdk_array holds true if the declarator is an array. However in push_parm_decl, call to grokdeclarator returns decl of pointer_type, corresponding to array declarator if the array is parameter (TREE_TYPE (decl) is pointer_type). So I guess we lose that information there. I guess that sort of makes sense, so I'll shut up ;) Index: gcc/tree-core.h === --- gcc/tree-core.h (revision 209800) +++ gcc/tree-core.h (working copy) @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { struct GTY(()) tree_parm_decl { struct tree_decl_with_rtl common; rtx incoming_rtl; + BOOL_BITFIELD is_array_parm; BOOL_BITFIELD only makes sense if you declare it as an actually bitfield with size less than that of unisgned int, otherwise you might as well use that directly. On the other hand I wonder if we can't just nuke BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being a builtin type? Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? you can certainly do |bool x;| as struct fields, that's already all over. However its not entirely clear to me if |bool x : 1;| will work everywhere and take the single bit you'd expect, istr there being compilers that do stupid things if you use multiple types next to each other in bitfields, but I'm not sure if we need to care about any of those. Changed to bool is_array_parm; and from macros to inline functions. I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? I guess the warning would be shared by c-family languages, so I had added the field to tree_parm_decl. This patch is C-only (added the member to c-lang.h:lang_type instead). That was not talking about. I was talking about DECL_LANG_FLAG_* which is already there for your usage. You should be able to use DECL_LANG_FLAG_2 as it is unused for both C and C++ for PARM_DECLs. This should also reduce the size of the patch too. Thanks for pointing it out, I have modified the patch. [gcc/c] * c-decl.c (push_parm_decl): Set DECL_LANG_FLAG_2 (decl) if declarator is array parameter. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Can you add a new macro in c-tree.h for this new usage of DECL_LANG_FLAG_2 so it is easier to figure out what the used lang flag bits are in use and to understand what that flag bit means? Is name C_ARRAY_PARM ok ? Bootstrapped on x86_64-unknown-linux-gnu [gcc/c] * c-tree.h (C_ARRAY_PARM): New macro, alias for DECL_LANG_FLAG_2. * c-decl.c (push_parm_decl): Set C_ARRAY_PARM (decl) if declarator is an array parameter. * c-typeck.c (c_expr_sizeof_expr): Check for sizeof-array-argument warning. [gcc/c-family] * c.opt (-Wsizeof-array-argument): New option. [gcc/testsuite/gcc.dg] * sizeof-array-argument.c: New test-case. Thanks and Regards, Prathamesh Thanks, Andrew Pinski Thanks and Regards, Prathamesh Thanks, Andrew Pinski [gcc/c] * c-decl.c (push_parm_decl):
Re: [gofrontend-dev] libgo patch committed: Fix madvise on systems with page size != 4096
'Ian Lance Taylor i...@google.com' via gofrontend-dev gofrontend-...@googlegroups.com writes: This patch from Anton Blanchard fixes libgo to adjust to the system page size when calling madvise. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.9 branch. Hi, I think this patch will make my Canonical colleagues very happy (and me when I get around to building a 64k page arm64 kernel...). It looks to me like this is also a problem in the gc runtime library though -- should the patch be sent there too? (Apologies if it already has and I missed it, I did look though). Cheers, mwh Ian -- You received this message because you are subscribed to the Google Groups gofrontend-dev group. To unsubscribe from this group and stop receiving emails from it, send an email to gofrontend-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. diff -r 3a53301d24d7 libgo/runtime/mheap.c --- a/libgo/runtime/mheap.c Tue Apr 22 16:43:35 2014 -0700 +++ b/libgo/runtime/mheap.c Thu Apr 24 21:18:35 2014 -0700 @@ -387,7 +387,7 @@ static uintptr scavengelist(MSpan *list, uint64 now, uint64 limit) { - uintptr released, sumreleased; + uintptr released, sumreleased, start, end, pagesize; MSpan *s; if(runtime_MSpanList_IsEmpty(list)) @@ -400,7 +400,17 @@ mstats.heap_released += released; sumreleased += released; s-npreleased = s-npages; - runtime_SysUnused((void*)(s-start PageShift), s-npages PageShift); + + start = s-start PageShift; + end = start + (s-npages PageShift); + + // Round start up and end down to ensure we + // are acting on entire pages. + pagesize = getpagesize(); + start = ROUND(start, pagesize); + end = ~(pagesize - 1); + if(end start) + runtime_SysUnused((void*)start, end - start); } } return sumreleased;
Re: [DOC PATCH] Rewrite docs for inline asm
Please take my comments below into account for an updated patch, and once Andrew and Richard have signed of, this is then good to commit. You did raise a technical question I'd like to get Andrew, Richard or someone to comment on: How does the user know what is dialect #0? Same for the others? When I originally wrote that section, I didn't know the answer (which is why I left it vague). Now I think I do, but I'd like someone to confirm. On my builds of gcc, the dialects are listed (in dialect order) under Known assembler dialects in gcc --target-help. Can I rely on this enough to put it in the docs? Is there some better source? (Patches of this size really are tough to review. That surely has contributed to the delay getting this in the tree most significantly. True enough. The goal of this patch was to rewrite section 6.41. That's no simple task, since 6.41 was ~10 very full pages. Unfortunately the current text is so random, I couldn't think of any way to update it piecemeal. Once I started, it became clear to me that I needed to split the text into two sections (Basic Extended), which meant I needed a sub-menu. Once I had the sub-menu, it seemed logical to move the rest of the asm-related topics here as well. It was not (and is not) my intent to try to re-work all these asm-related topics as part of this patch. Some changes have leaked in, but I actually have separate files where I have started work on new patches for these related sections. So while the text in these related sections may be perfectly adequate for the moment, I plan on taking another shot at them. In fact, I fell asleep over my notebook thrice reviewing this Dang, I knew I should have put more car chases and explosions in this thing. Maybe a love interest? On last point for Gerald: A number of your concerns seem to stem from reviewing this text only by looking at the patch. I think your questions might answered by looking at the actual generated output. While I'm sure I don't need to teach the doc maintainer how to generate the files, if you don't have your build environment handy, you can see the output (with all the changes below) at: http://www.LimeGreenSocks.com/gcc/Using-Assembly-Language-with-C.html +@menu +* Basic Asm:: Inline assembler with no operands. +* Extended Asm:: Inline assembler with operands. +* Constraints::Constraints for asm operands Should this be Asm operands based on the other references here or, better @code{asm} in all cases here? Changed to Constraints for @code{asm} operands. +* Explicit Reg Vars:: Defining variables residing in specified registers. Should this be Register instead of just Reg? And Variables instead of just Vars? Since reg vars wasn't really the focus of this patch, I mostly left the existing text. It may look new in the patch because it got moved into this new menu. I don't know why this was originally written this way (sounds like laziness). Register and Variables make more sense to me. That said, unless you feel strongly, I'd suggest doing this in my upcoming RegVars patch. +* Size of an asm:: How GCC calculates the size of an asm block. @code{asm} ? Changed to: Size of an asm:: How GCC calculates the size of an @code{asm} block. +@subsection Basic Asm - Assembler Instructions with No Operands --- instead of - Changed. +To create headers compatible with ISO C, write @code{__asm__} instead of +@code{asm} (@pxref{Alternate Keywords}). Here it's just ISO C, whereas in case of __inline__ you had ISO C90. Would it make sense to just use ISO C throughout? Wow, good catch. The change to __inline__ was a mistake. I must have modified this bit in Inline while searching for similar text in the asm section. I have undone this change to inline. As for the larger issue of ISO C vs ISO C90, that's a tougher question. Now that I've undone this change, all the inline asm stuff says ISO C, which is what the original text used. ISO C90 and ISO C99 are sprinkled throughout the other sections in extend.texi, and I'm reluctant to make wholesale changes to the rest of the file as part of this patch. +By definition, a Basic @code{asm} statement is one with no operands. +@code{asm} statements that contain one or more colons (used to delineate +operands) are considered to be Extended (for example, @code{asm(int $3)} +is Basic, and @code{asm(int $3 : )} is Extended). @xref{Extended Asm}. At this point the reader does not yet know the concept of those colons, does she? So that can be seen as a bit confusing. I understand what you mean, however I don't see a practical solution here. Describing the use of colons in Basic asm, only to follow it by saying now that you understand all that, none of it applies here seems like it would be even more confusing. Saying that colons are part of Extended asm, and providing a link to Extended asm which does describe them
Re: [gofrontend-dev] libgo patch committed: Fix madvise on systems with page size != 4096
On Sun, Apr 27, 2014 at 2:45 PM, Michael Hudson-Doyle michael.hud...@linaro.org wrote: 'Ian Lance Taylor i...@google.com' via gofrontend-dev gofrontend-...@googlegroups.com writes: This patch from Anton Blanchard fixes libgo to adjust to the system page size when calling madvise. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.9 branch. Hi, I think this patch will make my Canonical colleagues very happy (and me when I get around to building a 64k page arm64 kernel...). It looks to me like this is also a problem in the gc runtime library though -- should the patch be sent there too? (Apologies if it already has and I missed it, I did look though). It's a potential problem in the gc runtime library, but I think it's not a real problem at this point, since gc only runs on systems with 4k pages anyhow. Ian
Re: -fuse-caller-save - Enable for MIPS
On 27-04-14 12:27, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: 2014-01-12 Radovan Obradovic robrado...@mips.com Tom de Vries t...@codesourcery.com * config/mips/mips-protos.h (mips_emit_call_insn): Declare. * config/mips/mips.h (POST_CALL_TMP_REG): Define. * config/mips/mips.c (mips_emit_call_insn): Remove static. Use last_call_insn. Add POST_CALL_TMP_REG clobber (mips_split_call): Use POST_CALL_TMP_REG. (TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS): Redefine to true. * config/mips/mips.md (define_expand untyped_call): Use mips_emit_call_insn. * gcc.target/mips/mips.exp: Add use-caller-save to -ffoo/-fno-foo options. * gcc.target/mips/fuse-caller-save.h: New include file. * gcc.target/mips/fuse-caller-save.c: New test. * gcc.target/mips/fuse-caller-save-mips16.c: Same. * gcc.target/mips/fuse-caller-save-micromips.c: Same. Sorry, a couple of things, but this is looking pretty good: mips_emit_call_insn (rtx pattern, rtx orig_addr, rtx addr, bool lazy_p) { rtx insn, reg; - insn = emit_call_insn (pattern); + emit_call_insn (pattern); + insn = last_call_insn (); if (TARGET_MIPS16 mips_use_pic_fn_addr_reg_p (orig_addr)) { This change isn't necessary; emit_call_insn is defined to return a CALL_INSN. I dropped this change, as well as the change in the untyped_call expand, I realized it's unnecessary. @@ -2843,6 +2844,16 @@ mips_emit_call_insn (rtx pattern, rtx orig_addr, rtx addr, bool lazy_p) gen_rtx_REG (Pmode, GOT_VERSION_REGNUM)); emit_insn (gen_update_got_version ()); } + + if (TARGET_MIPS16 + TARGET_EXPLICIT_RELOCS + TARGET_CALL_CLOBBERED_GP + !find_reg_note (insn, REG_NORETURN, 0)) +{ + rtx post_call_tmp_reg = gen_rtx_REG (word_mode, POST_CALL_TMP_REG); + clobber_reg (CALL_INSN_FUNCTION_USAGE (insn), post_call_tmp_reg); +} The REG_NORETURN note won't be around yet, so we might as well drop that line. I'm not sure how useful it would be anyway since values are never live across a noreturn call. Done. +/* Temporary register that is used after a call. $4 and $5 are used for Might as well make it ...used when restoring $gp after a call, now that it's not as obvious from context. + returning complex double values in soft-float code, so $6 is the first + suitable candidate for !TARGET_MIPS16. For TARGET_MIPS16, we use + PIC_OFFSET_TABLE_REGNUM instead. */ !TARGET_MIPS16 and TARGET_MIPS16 are the wrong way around: suitable candidate for TARGET_MIPS16. For !TARGET_MIPS16 we can use $gp itself as the temporary. */ Fixed, thanks for catching that. +/* The scan of the sp-relative saves will fail for -O0 and -O1. + For -flto, scans will fail because there's no code in the .s file. */ +/* { dg-skip-if { *-*-* } { -O0 -O1 -flto} } */ The -flto thing is handled automatically by the testsuite (see force_conventional_output_for) so that one should be left out. Ah, I see. Removed. I'm a bit surprised that it doesn't work at -O1 for a simple test like this though. What goes wrong? AFAIU now the problem is that the optimization doesn't trigger for -O0 and -01, because the register allocator behaves more conservatively. OK for trunk, if re-testing succeeds? Thanks, - Tom 2014-01-12 Radovan Obradovic robrado...@mips.com Tom de Vries t...@codesourcery.com * config/mips/mips-protos.h (mips_emit_call_insn): Declare. * config/mips/mips.h (POST_CALL_TMP_REG): Define. * config/mips/mips.c (mips_emit_call_insn): Remove static. Add POST_CALL_TMP_REG clobber. (mips_split_call): Use POST_CALL_TMP_REG. (TARGET_CALL_FUSAGE_CONTAINS_NON_CALLEE_CLOBBERS): Redefine to true. * gcc.target/mips/mips.exp: Add use-caller-save to -ffoo/-fno-foo options. * gcc.target/mips/fuse-caller-save.h: New include file. * gcc.target/mips/fuse-caller-save.c: New test. * gcc.target/mips/fuse-caller-save-mips16.c: Same. * gcc.target/mips/fuse-caller-save-micromips.c: Same. --- gcc/config/mips/mips-protos.h| 1 + gcc/config/mips/mips.c | 20 +++- gcc/config/mips/mips.h | 7 +++ .../gcc.target/mips/fuse-caller-save-micromips.c | 17 + .../gcc.target/mips/fuse-caller-save-mip16.c | 17 + gcc/testsuite/gcc.target/mips/fuse-caller-save.c | 17 + gcc/testsuite/gcc.target/mips/fuse-caller-save.h | 17 + gcc/testsuite/gcc.target/mips/mips.exp | 1 + 8 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.target/mips/fuse-caller-save-micromips.c create mode 100644 gcc/testsuite/gcc.target/mips/fuse-caller-save-mip16.c create mode 100644 gcc/testsuite/gcc.target/mips/fuse-caller-save.c
Re: [Patch 1/3] Use manual regex algorithm switching
On Sat, Apr 26, 2014 at 10:50 AM, Jonathan Wakely jwak...@redhat.com wrote: This patch is OK, thanks very much. Committed. Thanks! -- Regards, Tim Shen
Re: [Patch 2/3] Make regex's infinite loop detection accurate
On Sun, Apr 27, 2014 at 8:30 AM, Jonathan Wakely jwak...@redhat.com wrote: This patch is OK to commit. ... with the fix for the un-uglified variable name. Committed. Thanks! -- Regards, Tim Shen
Re: [Patch 3/3] Separate `repeating` node from `alternative` node in regex
On Sun, Apr 27, 2014 at 8:31 AM, Jonathan Wakely jwak...@redhat.com wrote: This one is OK to commit too - thanks. Committed. Thanks! -- Regards, Tim Shen
Re: [PATCH] testsuite: Register loaded libs
On Apr 27, 2014, at 10:45 AM, Sebastian Huber sebastian.hu...@embedded-brains.de wrote: 2014-04-27 Sebastian Huber sebastian.hu...@embedded-brains.de * testsuite/lib/libffi.exp (load_gcc_lib): Register loaded libs. So, I didn’t see anything that strikes me as wrong, but, I’m curious why you want this? I didn’t see any uses?