Re: [cxx-conversion] New Hash Table (issue6244048)
My main concern is that the precise collector we have in place now requires substantial care to use. It is supported by a tool that does not really understand C, let alone C++. We avoid the problems when annotations are unnecessary. We can do that in one of two ways, either by upgrading GTY to understand more of the language so that it infers the right things from regular code, or by changing the implementation strategy so that the garbage collector does not need type information. I prefer the latter approach, as it makes our experience closer to our customers' experience. Are you really saying that, because our customers might have different ways of doing garbage collection in their code, we should drop our implementation that might well be superior to theirs? That would be a weak argument IMO, all the more so that, in the end, this might degrade the GCC experience for them. -- Eric Botcazou
Re: [patch] disintegrate integrate.[ch]
The attached patch moves the code from integrate.c to (what I hope you agree to be) better places: * inliner code goes to tree-inline.c * functions only called from dwarf2out.c are moved there. * allocate_initial_values is moved to ira.c * the initial-value stuff is moved to function.c The rest is just mechanical updates: Don't include integrate.h anywhere, and include function.h if something is needed from there. The files integrate.c and integrate.h can be removed after this change. Please just drop the reference to integrate.c in expmed.c, there will be no point in keeping it after the remnants of the old inliner are eliminated. -- Eric Botcazou
[PATCH, WWWDOCS] Document AArch64 branch.
This patch documents the AArch64 branch in wwwdocs/htdocs/svn.html. OK? /Marcus Proposed ChangeLog: * htdocs/svn.html: Document aarch64 branch.diff -u -p -r1.173 svn.html --- svn.html12 Apr 2012 01:26:33 - 1.173 +++ svn.html29 May 2012 07:28:17 - @@ -395,6 +395,12 @@ the command codesvn log --stop-on-copy standard and tracks the gcc-4.4 development. This branch is maintained by Richard Earnshaw./dd + dtARM/aarch64-branch/dt + ddThis branch adds support for the AArch64 architecture and will track +trunk until such time as the port is merged into trunk. Patches to this +branch should be tagged [AARCH64] and must be approved by ARM +personnel./dd + dtcell-4_3-branch/dt ddThe goal of this branch is to add fixes and additional features required for the Cell/B.E. processor (both PPE and SPE) to GCC 4.3.x. This branch
[PATCH][AARCH64] Remove t-aarch64 from libgcc
Since in 4.7, libgcc/config/aarch64/t-aarch64 only contains makefile rules for crti.o and crtn.o and these rules are automatically added by the generic make system, we can remove it. I have verified that ctri.o and ctrn.o are still generated correctly. Addition to libgcc/ChangeLog: 2012-05-28 Jim MacArthurjim.macart...@arm.com * config/aarch64/t-aarch64: Delete. * config.host (aarch64*-*-elf): Remove reference to t-aarch64. diff --git a/libgcc/config.host b/libgcc/config.host index ccd0fa1..56beddd 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -291,7 +291,6 @@ case ${host} in ;; aarch64*-*-elf) extra_parts=$extra_parts crtbegin.o crtend.o crti.o crtn.o - tmake_file=${tmake_file} ${cpu_type}/t-aarch64 tmake_file=${tmake_file} t-softfp-sfdf t-softfp-excl tmake_file=${tmake_file} ${cpu_type}/t-softfp t-softfp ;; diff --git a/libgcc/config/aarch64/t-aarch64 b/libgcc/config/aarch64/t-aarch64 deleted file mode 100644 index d80fc49..000 --- a/libgcc/config/aarch64/t-aarch64 +++ /dev/null @@ -1,28 +0,0 @@ -# Machine description for AArch64 architecture. -# Copyright (C) 2009, 2010, 2011, 2012 Free Software Foundation, Inc. -# Contributed by ARM Ltd. -# -# This file is part of GCC. -# -# GCC is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3, or (at your option) -# any later version. -# -# GCC is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with GCC; see the file COPYING3. If not see -# http://www.gnu.org/licenses/. - -# Assemble startup files. -crti.o: $(libgcc_topdir)/libgcc/config/aarch64/crti.S - $(CC) $(compile_deps) -I. -I$(gcc_objdir) -c -x assembler-with-cpp \ - -o $@ $(libgcc_topdir)/libgcc/config/aarch64/crti.S - -crtn.o: $(libgcc_topdir)/libgcc/config/aarch64/crtn.S - $(CC) $(compile_deps) -I. -I$(gcc_objdir) -c -x assembler-with-cpp \ - -o $@ $(libgcc_topdir)/libgcc/config/aarch64/crtn.S
Re: [patch] disintegrate integrate.[ch]
On Tue, May 29, 2012 at 8:43 AM, Eric Botcazou ebotca...@adacore.com wrote: The attached patch moves the code from integrate.c to (what I hope you agree to be) better places: * inliner code goes to tree-inline.c * functions only called from dwarf2out.c are moved there. * allocate_initial_values is moved to ira.c * the initial-value stuff is moved to function.c The rest is just mechanical updates: Don't include integrate.h anywhere, and include function.h if something is needed from there. The files integrate.c and integrate.h can be removed after this change. Please just drop the reference to integrate.c in expmed.c, there will be no point in keeping it after the remnants of the old inliner are eliminated. Yes, I realize that, but it looks like that code is not doing something because old integrate.c choked on it. Quoting that part of the patch: Index: expmed.c === --- expmed.c(revision 187936) +++ expmed.c(working copy) @@ -1866,6 +1866,9 @@ extract_fixed_bit_field (enum machine_mode tmode, shift it so it does. */ /* Maybe propagate the target for the shift. */ /* But not if we will return it--could confuse integrate.c. */ + /* ??? What does the above comment mean in relation to the code +below? NB, integrate.c is no more, so I guess it can't be +confused by anything anymore... */ rtx subtarget = (target != 0 REG_P (target) ? target : 0); if (tmode != mode) subtarget = 0; op0 = expand_shift (RSHIFT_EXPR, mode, op0, bitpos, subtarget, 1); Does that But not if... comment refer to the if (tmode != mode) subtarget = 0; line? Of so, can/should we drop that line (as well as the comment)? I don't know this part of the compiler well enough to tell. You're much more into this than me, so perhaps you can help ;-) Is the patch otherwise OK? Ciao! Steven
[patch] alias.c TLC
Hello, The attached patch does some maintenance on alias.c: * Change reg_known_value from rtx * to a VEC(rtx). * Change reg_known_equiv_p from char * to an sbitmap. Nothing spectacular, just maintenance. Bootstrapped and regtested on x86_64-unknown-linux-gnu. OK? Ciao! Steven alias_tlc.diff Description: Binary data
Re: [driver, LTO Patch]: Resurrect user specs support
On 05/28/2012 06:27 PM, Joseph S. Myers wrote: On Mon, 28 May 2012, Christian Bruel wrote: On 05/28/2012 01:11 PM, Joseph S. Myers wrote: On Mon, 28 May 2012, Christian Bruel wrote: I shared the same concern, however, after playing bits with spec toys, I couldn't a find a way to get a % switch recognition failure, since the switches passed on the command line at this point are already validated if necessary. Suppose with the existing sources an option (in a .opt file) is matched by a $ spec, and not by any other spec. Will it be rejected by the driver? It shouldn't be. indeed, it's not rejected if it is present in a .opt file. I was concerned that it will not be rejected even if not in any .opt (or now in --specs). Which was what the validated setting seemed to imply. Should it be rejected ? probably. But this is not implied by the --spec changes. The existing rule is supposed to be: options are only accepted if in *both* a .opt file *and* a spec. If not in a .opt file, the common machinery will reject them; if in a .opt file but not a spec, the driver's own validation machinery will reject them. Thanks for confirming this, the question was more specifically for %options. Today, with the current implementation, I see two uses cases: 1) The flag appears in a % spec but is not in a .opt file - It is *not* rejected. It is just ignored. 2) The flag appears in a user switch and in a % spec, and not in a .opt file. - It is rejected. To refocus on the original question from the patch. I'm still not convinced after our discussions and testings that the propagation of the user flag to the do_spec functions is required to keep the same semantic. If there is an issue with the current % handling, could we handle this separately ? my primary focus was in matching --spec user options behavior with the .opt internal ones. If the driver's own validation machinery isn't rejecting them, that indicates that some spec has handled them. It's possible there's more than one piece of code relating to accepting such options and some such code is redundant. (This can't be tested with options starting -f or -m because of the specs passing all such options to cc1.) The new semantics are supposed to be, I think: an option in a .opt file is accepted if any spec matches it (same as now), an option not in a .opt file is only accepted if a user spec matches it and not simply because of a match from a built-in spec (where built-in specs are considered to include those generated by some of GCC's own runtime libraries). I agree. I believe my patch implements this, my focus was on not changing the current behavior for switches internally defined in a .opt (or now in a --spec file). error are still generated for other cases. Many thanks Christian
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
domi...@lps.ens.fr (Dominique Dhumieres) writes: I have regstapped r187893 with the following patch [karma] gcc/darwin_buildw% diff -up ../_gcc_clean/libcpp/lex.c ../work/libcpp/lex.c --- ../_gcc_clean/libcpp/lex.c2012-05-25 08:54:05.0 +0200 +++ ../work/libcpp/lex.c 2012-05-27 13:25:08.0 +0200 @@ -592,7 +592,8 @@ search_line_fast (const uchar *s, const union { vc v; - unsigned long l[N]; + /* Statically assert that N is 2 or 4. */ + unsigned long l[(N == 2 || N == 4) ? N : -1]; } u; unsigned long l, i = 0; without related regression. Thank you, Dominique. So, dear maintainers, is the patch below OK? It bootstraps on x86_64-unknown-linux-gnu as well. PR bootstrap/53459 * lex.c (search_line_fast): Avoid unused local typedefs to simulate a static assertion. --- libcpp/ChangeLog |6 ++ libcpp/lex.c |4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libcpp/lex.c b/libcpp/lex.c index c4dd603..98ee4e9 100644 --- a/libcpp/lex.c +++ b/libcpp/lex.c @@ -590,10 +590,10 @@ search_line_fast (const uchar *s, const uchar *end ATTRIBUTE_UNUSED) { #define N (sizeof(vc) / sizeof(long)) -typedef char check_count[(N == 2 || N == 4) * 2 - 1]; union { vc v; - unsigned long l[N]; + /* Statically assert that N is 2 or 4. */ + unsigned long l[(N == 2 || N == 4) ? N : -1]; } u; unsigned long l, i = 0; -- Dodji
Re: [PATCH] PR bootstrap/53459 - unused local typedef when building on altivec
On Tue, May 29, 2012 at 10:57:28AM +0200, Dodji Seketeli wrote: So, dear maintainers, is the patch below OK? It bootstraps on x86_64-unknown-linux-gnu as well. PR bootstrap/53459 * lex.c (search_line_fast): Avoid unused local typedefs to simulate a static assertion. --- libcpp/ChangeLog |6 ++ libcpp/lex.c |4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/libcpp/lex.c b/libcpp/lex.c index c4dd603..98ee4e9 100644 --- a/libcpp/lex.c +++ b/libcpp/lex.c @@ -590,10 +590,10 @@ search_line_fast (const uchar *s, const uchar *end ATTRIBUTE_UNUSED) { #define N (sizeof(vc) / sizeof(long)) -typedef char check_count[(N == 2 || N == 4) * 2 - 1]; union { vc v; - unsigned long l[N]; + /* Statically assert that N is 2 or 4. */ + unsigned long l[(N == 2 || N == 4) ? N : -1]; } u; unsigned long l, i = 0; Ok. Jakub
Ping: [Patch]: Fix call to end_prologue debug hook
Ping for: On May 15, 2012, at 11:26 AM, Tristan Gingold wrote: Hi, the end_prologue debug hook (only used by dwarf on VMS) is currently called at the NOTE_INSN_FUNCTION_BEG. This is not what its name implies, neither what the VMS debugger expect and neither what a comment in dwarf2out.c says: /* Recall that this end-of-prologue indication is *not* the same thing as the end_prologue debug hook. The NOTE_INSN_PROLOGUE_END note, to which the hook corresponds, follows the last insn that was emitted by gen_prologue. What we need is to preceed the first insn that had been emitted after NOTE_INSN_FUNCTION_BEG, i.e. the first insn that corresponds to something the user wrote. These may be very different locations once scheduling is enabled. */ This is fixed by this patch. No regressions on x86_64 GNU/Linux. Ok for trunk ? Tristan. 2012-05-15 Tristan Gingold ging...@adacore.com * final.c (final_scan_insn): Move call to end_prologue debug hook to the NOTE_INSN_PROLOGUE_END. diff --git a/gcc/final.c b/gcc/final.c index 718caf1..6efa06f 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1961,6 +1961,8 @@ final_scan_insn (rtx insn, FILE *file, int optimize_p ATTR case NOTE_INSN_PROLOGUE_END: targetm.asm_out.function_end_prologue (file); profile_after_prologue (file); + if (!DECL_IGNORED_P (current_function_decl)) + debug_hooks-end_prologue (last_linenum, last_filename); if ((*seen (SEEN_EMITTED | SEEN_NOTE)) == SEEN_NOTE) { @@ -1989,8 +1991,6 @@ final_scan_insn (rtx insn, FILE *file, int optimize_p ATTR case NOTE_INSN_FUNCTION_BEG: app_disable (); - if (!DECL_IGNORED_P (current_function_decl)) - debug_hooks-end_prologue (last_linenum, last_filename); if ((*seen (SEEN_EMITTED | SEEN_NOTE)) == SEEN_NOTE) {
Ping: [Patch]: Fix ICE by expand_expr_addr_expr_1
Ping for: On May 15, 2012, at 10:59 AM, Tristan Gingold wrote: Hi, I got ICE in plus_constant (after the assertions were added) due to expand_expr_addr_expr_1 during build on ia64/Openvms. This function is called with TMODE == SImode (32 bit pointers) but EXP designating a variable on the frame (whose register is DImode). Hence the ICE. Fixed by the following patch. No regression on x86-64 GNU/Linux. Ok for trunk ? Tristan. 2012-05-15 Tristan Gingold ging...@adacore.com * expr.c (expand_expr_addr_expr_1): Call convert_memory_address_addr_space. diff --git a/gcc/expr.c b/gcc/expr.c index 3edb4a2..1b0ad8d 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -7600,6 +7600,7 @@ expand_expr_addr_expr_1 (tree exp, rtx target, enum machin TYPE_USER_ALIGN (TREE_TYPE (inner)) = 1; } result = expand_expr_addr_expr_1 (inner, subtarget, tmode, modifier, as); + result = convert_memory_address_addr_space (tmode, result, as); if (offset) {
Re: [patch] disintegrate integrate.[ch]
Yes, I realize that, but it looks like that code is not doing something because old integrate.c choked on it. Quoting that part of the patch: Index: expmed.c === --- expmed.c (revision 187936) +++ expmed.c (working copy) @@ -1866,6 +1866,9 @@ extract_fixed_bit_field (enum machine_mode tmode, shift it so it does. */ /* Maybe propagate the target for the shift. */ /* But not if we will return it--could confuse integrate.c. */ + /* ??? What does the above comment mean in relation to the code + below? NB, integrate.c is no more, so I guess it can't be + confused by anything anymore... */ rtx subtarget = (target != 0 REG_P (target) ? target : 0); if (tmode != mode) subtarget = 0; op0 = expand_shift (RSHIFT_EXPR, mode, op0, bitpos, subtarget, 1); Does that But not if... comment refer to the if (tmode != mode) subtarget = 0; line? Of so, can/should we drop that line (as well as the comment)? I don't know this part of the compiler well enough to tell. I don't think it's a direct reference. There is the same pattern at the end of the function, where TARGET has already been zeroed if mode != tmode. If the mode don't match, expand_shift cannot reuse [SUB]TARGET as-is, so the code consistently avoids to pass it. I'd just drop the dangling reference to integrate.c and reformat the line if (tmode != mode) subtarget = 0; into if (mode != mode) subtarget = 0; to make it more closely ressemble the other instance. -- Eric Botcazou
Re: RFA: temp slot TLC [3/3]
On Sun, May 27, 2012 at 2:48 AM, Michael Matz m...@suse.de wrote: Hi, and this is a further small cleanup. pop_temp_slots is now the same as free_temp_slots (modulo the level-- of course), so there's no need in calling both. (and preserve_temp_slots(NULL) is useless now). Regstrapped on x86_64-linux together with [1/3] and [2/3], all languages (+Ada,+obj-c++). Okay for trunk? Ok for all three patches. Thanks, Richard. Ciao, Michael. -- * function.c (pop_temp_slots): Call free_temp_slots. * calls.c (store_one_arg): Don't call preserve_temp_slots or free_temp_slots. * expr.c (expand_assignment): Don't call free_temp_slots. Index: calls.c === --- calls.c.orig 2012-05-26 21:47:02.0 +0200 +++ calls.c 2012-05-26 23:42:57.0 +0200 @@ -4721,11 +4721,7 @@ store_one_arg (struct arg_data *arg, rtx be deferred during the rest of the arguments. */ NO_DEFER_POP; - /* Free any temporary slots made in processing this argument. Show - that we might have taken the address of something and pushed that - as an operand. */ - preserve_temp_slots (NULL_RTX); - free_temp_slots (); + /* Free any temporary slots made in processing this argument. */ pop_temp_slots (); return sibcall_failure; Index: expr.c === --- expr.c.orig 2012-05-26 21:47:02.0 +0200 +++ expr.c 2012-05-26 23:43:23.0 +0200 @@ -4836,7 +4836,6 @@ expand_assignment (tree to, tree from, b if (result) preserve_temp_slots (result); - free_temp_slots (); pop_temp_slots (); return; } @@ -4884,7 +4883,6 @@ expand_assignment (tree to, tree from, b emit_move_insn (to_rtx, value); } preserve_temp_slots (to_rtx); - free_temp_slots (); pop_temp_slots (); return; } @@ -4911,7 +4909,6 @@ expand_assignment (tree to, tree from, b emit_move_insn (to_rtx, temp); preserve_temp_slots (to_rtx); - free_temp_slots (); pop_temp_slots (); return; } @@ -4941,7 +4938,6 @@ expand_assignment (tree to, tree from, b TYPE_MODE (sizetype)); preserve_temp_slots (to_rtx); - free_temp_slots (); pop_temp_slots (); return; } @@ -4951,7 +4947,6 @@ expand_assignment (tree to, tree from, b push_temp_slots (); result = store_expr (from, to_rtx, 0, nontemporal); preserve_temp_slots (result); - free_temp_slots (); pop_temp_slots (); return; } Index: function.c === --- function.c.orig 2012-05-26 23:41:39.0 +0200 +++ function.c 2012-05-26 23:42:27.0 +0200 @@ -1200,22 +1200,7 @@ push_temp_slots (void) void pop_temp_slots (void) { - struct temp_slot *p, *next; - bool some_available = false; - - for (p = *temp_slots_at_level (temp_slot_level); p; p = next) - { - next = p-next; - make_slot_available (p); - some_available = true; - } - - if (some_available) - { - remove_unused_temp_slot_addresses (); - combine_temp_slots (); - } - + free_temp_slots (); temp_slot_level--; }
Re: RFA: Speedup expand_used_vars by 30 times (PR38474)
On Sun, May 27, 2012 at 5:03 AM, Michael Matz m...@suse.de wrote: Hi, [for certain test cases :-) ] the temp slot cleanups I just sent where actually motivated by PR38474. It exposes many slownesses in the compiler, but at -O0 the only remaining one is the expand phase. expanding variables is the one thing (on my machine here, with -O0 -g f951): 30 seconds for the reduced testcase. expand itself (the statements) then needs 5.5 seconds and is the other thing. Overall 77 seconds compile time. The problem with expanding variables in this case is the add_alias_set_conflicts routine. It walks all NxN/2 stack variable pairs, looks if pair (i,j) can share the same stack slot from a type perspective (canshare(i,j)) and adds a conflict if not so. Before asking that question it doesn't look if pair (i,j) maybe already conflict for other reasons. Now, we could simply add the conflicts(i,j) test in that loop, before asking canshare(i,j) and adding a new conflict. But adding conflicts comes with a cost, so we can do better. The answer to canshare(i,j) depends on only all components already merged into [i] and the type of j. With carefully choosing a type to represent all components of [i] and updating it in union_stack_vars we can simply ask canshare(i,j) right before we actually want to merge j into [i]. That's the least amount of work possible as long as we want to have the same results. This change brings the 30 seconds for var expansion down to 0.8 seconds. The other change in function.c further improves the temp slot machinery. While expanding free_temp_slots is called after each statement, and if it is able to free a slot it needs to update the RTX-slot mapping (a htab_t). The freeing of slots was done incorrectly, it overwrote the slot in question with NULL, but it should have used htab_clear_slot. That looks like a bugfix applicable to branches? This meant that the hashtable kept growing and growing even with all elements being empty because the size statistics aren't correct. Additionally it breaks hash chains if there is one striding the nullified slot. And then a microoptimization: if we know there aren't any slots in use at all (happens often in normal circumstances) we can as well use htab_empty instead of traversing the hash table for the right slots. This brings the expansion time (i.e. for expanding the statements) down from 5.5 to 2.8 seconds. All changes together reduce the compile time for the reduced testcase from ~ 77 seconds to ~ 44. A variant of this regstrapped on x86_64-linux, all languages+Ada+objc++, but after some changes I'm redoing that regstrap. Okay for trunk if that passes? The volatile handling is very odd - the objects_must_conflict_p code basically says that two volatile vars may always share stack slots?! What's the reasoning for this, or handling volatiles special in any way? I'd rather remove this fishy code (and if at all, ensure that volatile automatics are never coalesced with any other stack slot). Thanks, Richard. Ciao, Michael. --- PR middle-end/38474 * cfgexpand.c (struct stack_var): Add slot_type member. (add_stack_var): Initialize it. (add_alias_set_conflicts): Remove. (merge_stack_vars_p, more_specific_type): New functions. (nonconflicting_type): New static data. (union_stack_vars): Use more_specific_type to update slot_type. (partition_stack_vars): Call merge_stack_vars_p ... (expand_used_vars): ... instead of add_alias_set_conflicts. (fini_vars_expansion): Clear nonconflicting_type. * function.c (n_temp_slots_in_use): New static data. (make_slot_available, assign_stack_temp_for_type): Update it. (init_temp_slots): Zero it. (remove_unused_temp_slot_addresses): Use it for quicker removal. (remove_unused_temp_slot_addresses_1): Use htab_clear_slot. Index: cfgexpand.c === --- cfgexpand.c.orig 2012-05-27 01:33:55.0 +0200 +++ cfgexpand.c 2012-05-27 04:37:18.0 +0200 @@ -177,6 +177,11 @@ struct stack_var /* The next stack variable in the partition, or EOC. */ size_t next; + /* The most specific type of all variables merged with the slot + if this is a representative. Initially this is simply the + TREE_TYPE for the variable. */ + tree slot_type; + /* The numbers of conflicting stack variables. */ bitmap conflicts; }; @@ -285,6 +290,7 @@ add_stack_var (tree decl) /* All variables are initially in their own partition. */ v-representative = stack_vars_num; v-next = EOC; + v-slot_type = TREE_TYPE (decl); /* All variables initially conflict with no other. */ v-conflicts = NULL; @@ -353,45 +359,40 @@ aggregate_contains_union_type (tree type return false; } -/* A subroutine of expand_used_vars. If two variables X and Y have
Re: [patch] alias.c TLC
On Tue, May 29, 2012 at 10:46 AM, Steven Bosscher stevenb@gmail.com wrote: Hello, The attached patch does some maintenance on alias.c: * Change reg_known_value from rtx * to a VEC(rtx). * Change reg_known_equiv_p from char * to an sbitmap. Nothing spectacular, just maintenance. Bootstrapped and regtested on x86_64-unknown-linux-gnu. OK? Ok! Thanks, Richard. Ciao! Steven
Re: [cxx-conversion] New Hash Table (issue6244048)
On Fri, May 25, 2012 at 8:43 PM, Mike Stump mikest...@comcast.net wrote: On May 25, 2012, at 10:50 AM, Lawrence Crowl wrote: Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, If portability to other C++ compilers wasn't a concern, we could extend out g++ to make supporting GTY better, so that we can simplify and refine the GTY stuff. I fear we need some light weight reflection, might make a nice language feature for a future C++ standard, if done well. Seconded. It points the finger of my #1 concern with the C++ conversion - our GC. We need a GC scheme that allows us to use standard library containers, and the scheme that was outlined earlier would work. Are the TR1 hash table implementations using any non-C++98/03 features? If not then I would suggest to use our TR1 hash tables. Richard.
Re: [C++ Patch] PR 25137 (no -Wmissing-braces in -Wall version)
On 05/28/2012 06:39 PM, Jason Merrill wrote: OK. Thanks. I also applied as obvious the below, provided by Manuel, and changing this warning too to use the new LangEnabledBy. Paolo. / 2012-05-29 Manuel López-Ibáñez m...@gcc.gnu.org * c.opt (Wmissing-braces): Use LangEnabledBy(C ObjC,Wall). * c-opts.c (c_common_handle_option): Remove code handling warn_missing_braces. Index: c.opt === --- c.opt (revision 187936) +++ c.opt (working copy) @@ -463,7 +463,7 @@ C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning Warn about suspicious declarations of \main\ Wmissing-braces -C ObjC C++ ObjC++ Var(warn_missing_braces) Warning +C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall) Warn about possibly missing braces around initializers Wmissing-declarations Index: c-opts.c === --- c-opts.c(revision 187937) +++ c-opts.c(working copy) @@ -401,8 +401,6 @@ c_common_handle_option (size_t scode, const char * done in c_common_post_options. */ if (warn_enum_compare == -1) warn_enum_compare = value; - - warn_missing_braces = value; } else {
[PATCH] PR preprocessor/53469 - argument tokens of _Pragma miss virtual location
Hello, Consider this short test snippet: -8--- #define STRINGIFY(x) #x #define TEST(x) \ _Pragma(STRINGIFY(GCC diagnostic ignored -Wunused-local-typedefs)) \ typedef int myint; void bar () { TEST(myint) } -8--- The _Pragma is effectively ignored, and compiling with -Wunused-local-typedefs warns on the local typedef, even though the pragma should have prevented the warning to be emitted. This is because when the preprocessor sees the _Pragma operator and then goes to handle the first token ('GCC' here) that makes up its operands, it retains the spelling location of that token, not its virtual location. Later when diagnostic_report_diagnostic is called to emit the warning (or ignore it because of the pragma), it compares the location of the first operand of the pragma with the location of the unused location, (by calling linemap_location_before_p) and that comparison fails because in this case, both locations should be virtual. This patch fixes the issue by teaching the pragma handling to use virtual locations. Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk. libcpp/ PR preprocessor/53469 * directives.c (do_pragma): Use the virtual location for the pragma token, instead of its spelling location. gcc/testsuite/ PR preprocessor/53469 * gcc.dg/cpp/_Pragma7.c: New test case. --- gcc/testsuite/ChangeLog |5 + gcc/testsuite/gcc.dg/cpp/_Pragma7.c | 14 ++ libcpp/ChangeLog|6 ++ libcpp/directives.c |8 +--- 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/_Pragma7.c diff --git a/gcc/testsuite/gcc.dg/cpp/_Pragma7.c b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c new file mode 100644 index 000..a7a5b1b --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/_Pragma7.c @@ -0,0 +1,14 @@ +/* + Origin: PR preprocessor/53469 + { dg-do compile } + */ + +#define STRINGIFY(x) #x +#define TEST(x) \ + _Pragma(STRINGIFY(GCC diagnostic ignored -Wunused-local-typedefs)) \ + typedef int myint; + +void bar () +{ + TEST(myint) +} diff --git a/libcpp/directives.c b/libcpp/directives.c index e46280e..cd880f1 100644 --- a/libcpp/directives.c +++ b/libcpp/directives.c @@ -1347,13 +1347,15 @@ static void do_pragma (cpp_reader *pfile) { const struct pragma_entry *p = NULL; - const cpp_token *token, *pragma_token = pfile-cur_token; + const cpp_token *token, *pragma_token; + source_location pragma_token_virt_loc = 0; cpp_token ns_token; unsigned int count = 1; pfile-state.prevent_expansion++; - token = cpp_get_token (pfile); + pragma_token = token = cpp_get_token_with_location (pfile, + pragma_token_virt_loc); ns_token = *token; if (token-type == CPP_NAME) { @@ -1407,7 +1409,7 @@ do_pragma (cpp_reader *pfile) { if (p-is_deferred) { - pfile-directive_result.src_loc = pragma_token-src_loc; + pfile-directive_result.src_loc = pragma_token_virt_loc; pfile-directive_result.type = CPP_PRAGMA; pfile-directive_result.flags = pragma_token-flags; pfile-directive_result.val.pragma = p-u.ident; -- Dodji
Re: [driver, LTO Patch]: Resurrect user specs support
On Tue, 29 May 2012, Christian Bruel wrote: The existing rule is supposed to be: options are only accepted if in *both* a .opt file *and* a spec. If not in a .opt file, the common machinery will reject them; if in a .opt file but not a spec, the driver's own validation machinery will reject them. Thanks for confirming this, the question was more specifically for %options. Today, with the current implementation, I see two uses cases: 1) The flag appears in a % spec but is not in a .opt file - It is *not* rejected. It is just ignored. I don't really see that as a use case; it's more a matter of an internal consistency check that could be done but isn't. I'm only concerned about cases where the option is actually passed on the command line to the driver. 2) The flag appears in a user switch and in a % spec, and not in a .opt file. - It is rejected. There are also cases: * The option, passed by the user, is in a .opt file, a % spec but no other spec. (Should be accepted.) * The option, passed by the user, is in a .opt file and no spec at all. (Should be rejected.) To refocus on the original question from the patch. I'm still not convinced after our discussions and testings that the propagation of the user flag to the do_spec functions is required to keep the same semantic. With the proposed change to the rules for when a spec serves to validate an option, all settings of the validated field need to be reviewed to make sure that they are in accordance with the new rules - and that it is transparent to human readers of the code that they are in accordance with the new rules. If you don't think propagation is needed to do_spec functions, then it should be possible to remove the validated setting in there, with a proper explanation of the order in which the various relevant functions are called and where validated will previously or subsequently be set. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] PR preprocessor/53469 - argument tokens of _Pragma miss virtual location
Hi, On 05/29/2012 12:19 PM, Dodji Seketeli wrote: The _Pragma is effectively ignored, hi Dodji, and sorry for taking the occasion to mention other issues with pragmas which definitely can be handled separately, but I can't resist ;) In fact we have a number of open PRs: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47347 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48914 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431 all having to do with #pragma GCC diagnostic ignored and since you looked into this code I'm wondering if you can see a pattern, something which may explain all of them at once? (too good to be true, eh?) Thanks anyway, Paolo.
[PATCH] Fix massive memory leak in read_line (PR middle-end/53510)
Hi! As soon as line length goes over 200 bytes, read_line starts leaking memory (for line length x where x 200 bytes it leaks ((1UL ((x - 199) / 2)) - 1) * 200 bytes). This patch fixes it. As noted by Manuel, he copied that buggy code from gcov.c, so this fixes gcov too. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2012-05-29 Jakub Jelinek ja...@redhat.com PR middle-end/53510 * input.c (read_line): Use XRESIZEVEC instead of XNEWVEC to avoid leaking memory. No need to handle memory allocation failure. Double string_len on each reallocation instead of adding 2. * gcov.c (read_line): Likewise. --- gcc/input.c.jj 2012-05-02 09:38:42.0 +0200 +++ gcc/input.c 2012-05-29 09:22:05.123856883 +0200 @@ -105,15 +105,8 @@ read_line (FILE *file) return string; } pos += len; - ptr = XNEWVEC (char, string_len * 2); - if (ptr) - { - memcpy (ptr, string, pos); - string = ptr; - string_len += 2; - } - else - pos = 0; + string = XRESIZEVEC (char, string, string_len * 2); + string_len *= 2; } return pos ? string : NULL; --- gcc/gcov.c.jj 2012-01-13 21:47:35.719634891 +0100 +++ gcc/gcov.c 2012-05-29 10:30:21.862814065 +0200 @@ -2219,15 +2219,8 @@ read_line (FILE *file) return string; } pos += len; - ptr = XNEWVEC (char, string_len * 2); - if (ptr) - { - memcpy (ptr, string, pos); - string = ptr; - string_len += 2; - } - else - pos = 0; + string = XRESIZEVEC (char, string, string_len * 2); + string_len *= 2; } return pos ? string : NULL; Jakub
Re: [PR tree-optimization/52558]: RFC: questions on store data race
On Mon, 21 May 2012, Aldy Hernandez wrote: On 05/16/12 07:53, Richard Guenther wrote: On Mon, 7 May 2012, Aldy Hernandez wrote: [Sorry for the delay; I was on vacation.] I am forgoing the load avoidance code altogether to simplify things. Thanks. + /* Emit the load code into the latch, so that we are sure it will + be processed after all dependencies. */ + latch_edge = loop_latch_edge (loop); inserting code into the latch block is bad - the vectorizer will be confused by non-empty latches. The code as is on trunk inserts the load on the latch. Does it? Ok ... That's why I also inserted the supporting flag checking code there. Do you want me to put the supporting code somewhere else? Technically there isn't a good other place that does not add a partial redundancy. Your ChangeLog mentions changes that are no longer necessary (the target hook). Whoops. Fixed. I didn't quite get the store order issue - we _do_ preserve store ordering right now, do we? So how come introducing the flags change that? The current scheme on trunk works because it inserts one load with gsi_insert_on_edge() and subsequent ones get appended correctly, whereas my patch has to split the edge (which happens at the top of the block), so subsequent code inserts happen in reverse order (at the top). If I remember correctly, that is... I can look again and report if it's still unclear. Hmm, the old code (well, and the new one ...) walks stores to move by walking a bitmap. I suppose we rely on computing uids in the original program order here then. (flag_tm loop_preheader_edge (loop)-src-flags BB_IN_TRANSACTION) can you encapsulate this into a predicate? Like block_in_transaction () that also checks flag_tm? + /* ?? FIXME TESTING TESTING ?? */ + multi_threaded_model_p=true; + /* ?? FIXME TESTING TESTING ?? */ that of course needs fixing ;) (and re-testing) New patch attached. Tested on x86-64 Linux. No regressions. Ok with the new predicate in basic-block.h and re-testing without the fixme above. Thanks, Richard.
Re: [PATCH] Fix massive memory leak in read_line (PR middle-end/53510)
On Tue, May 29, 2012 at 1:12 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! As soon as line length goes over 200 bytes, read_line starts leaking memory (for line length x where x 200 bytes it leaks ((1UL ((x - 199) / 2)) - 1) * 200 bytes). This patch fixes it. As noted by Manuel, he copied that buggy code from gcov.c, so this fixes gcov too. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. 2012-05-29 Jakub Jelinek ja...@redhat.com PR middle-end/53510 * input.c (read_line): Use XRESIZEVEC instead of XNEWVEC to avoid leaking memory. No need to handle memory allocation failure. Double string_len on each reallocation instead of adding 2. * gcov.c (read_line): Likewise. --- gcc/input.c.jj 2012-05-02 09:38:42.0 +0200 +++ gcc/input.c 2012-05-29 09:22:05.123856883 +0200 @@ -105,15 +105,8 @@ read_line (FILE *file) return string; } pos += len; - ptr = XNEWVEC (char, string_len * 2); - if (ptr) - { - memcpy (ptr, string, pos); - string = ptr; - string_len += 2; - } - else - pos = 0; + string = XRESIZEVEC (char, string, string_len * 2); + string_len *= 2; } return pos ? string : NULL; --- gcc/gcov.c.jj 2012-01-13 21:47:35.719634891 +0100 +++ gcc/gcov.c 2012-05-29 10:30:21.862814065 +0200 @@ -2219,15 +2219,8 @@ read_line (FILE *file) return string; } pos += len; - ptr = XNEWVEC (char, string_len * 2); - if (ptr) - { - memcpy (ptr, string, pos); - string = ptr; - string_len += 2; - } - else - pos = 0; + string = XRESIZEVEC (char, string, string_len * 2); + string_len *= 2; } return pos ? string : NULL; Jakub
Re: [driver, LTO Patch]: Resurrect user specs support
On 05/29/2012 12:50 PM, Joseph S. Myers wrote: On Tue, 29 May 2012, Christian Bruel wrote: The existing rule is supposed to be: options are only accepted if in *both* a .opt file *and* a spec. If not in a .opt file, the common machinery will reject them; if in a .opt file but not a spec, the driver's own validation machinery will reject them. Thanks for confirming this, the question was more specifically for %options. Today, with the current implementation, I see two uses cases: 1) The flag appears in a % spec but is not in a .opt file - It is *not* rejected. It is just ignored. I don't really see that as a use case; it's more a matter of an internal consistency check that could be done but isn't. I'm only concerned about cases where the option is actually passed on the command line to the driver. 2) The flag appears in a user switch and in a % spec, and not in a .opt file. - It is rejected. There are also cases: * The option, passed by the user, is in a .opt file, a % spec but no other spec. (Should be accepted.) yes, it is. * The option, passed by the user, is in a .opt file and no spec at all. (Should be rejected.) yes, it is. To refocus on the original question from the patch. I'm still not convinced after our discussions and testings that the propagation of the user flag to the do_spec functions is required to keep the same semantic. With the proposed change to the rules for when a spec serves to validate an option, all settings of the validated field need to be reviewed to make sure that they are in accordance with the new rules - and that it is transparent to human readers of the code that they are in accordance with the new rules. If you don't think propagation is needed to do_spec functions, then it should be possible to remove the validated setting in there, with a proper explanation of the order in which the various relevant functions are called and where validated will previously or subsequently be set. I agree. I see two potential settings of the validated field. The % that we just reviewed, and the case : /* We have Xno-YYY, search for XYYY. */ It seems possible to remove those, I've just checked this during lunch :-) with the scenarios described above (without use m* f* specs). If it is a prerequisite before my --spec patch, I'd like to propose it as an independent one, since this is a valid modification regardless of the --spec implementation (although it makes it clearer). Many thanks Christian
Re: [cxx-conversion] New Hash Table (issue6244048)
On May 25, 2012, at 2:42 PM, Lawrence Crowl wrote: On 5/24/12, Jakub Jelinek ja...@redhat.com wrote: You haven't looked at the most important problem of that approach - code bloat. Are you claiming that the size of the binary is more important than run-time performance, type safety, and source code size? I just look at it as a nice commoning problem in search of a nice solution.
Re: [cxx-conversion] New Hash Table (issue6244048)
On May 26, 2012, at 11:25 PM, Gabriel Dos Reis wrote: how do I want the codebase to look like in 10, 15, 20, 25 years. Gee, if the next 25 look just like the last 25, it will look very similar to what it looks like today. :-)
Re: RFA: Speedup expand_used_vars by 30 times (PR38474)
Hi, On Tue, 29 May 2012, Richard Guenther wrote: The other change in function.c further improves the temp slot machinery. While expanding free_temp_slots is called after each statement, and if it is able to free a slot it needs to update the RTX-slot mapping (a htab_t). The freeing of slots was done incorrectly, it overwrote the slot in question with NULL, but it should have used htab_clear_slot. That looks like a bugfix applicable to branches? Perhaps, though the results of this bug are only worse code generation. When the chains are broken due to the bug some RTX-slot mappings aren't found anymore, but the code deals conservatively with that. A variant of this regstrapped on x86_64-linux, all languages+Ada+objc++, but after some changes I'm redoing that regstrap. Okay for trunk if that passes? The volatile handling is very odd - the objects_must_conflict_p code basically says that two volatile vars may always share stack slots?! That's correct. Basically everything can share a stack slot that conflicts already for other reasons than address-based considerations. Two volatile accesses always conflict and hence they'll never be swapped (which is what creates the problems with stack slot sharing). What's the reasoning for this, or handling volatiles special in any way? I'd rather remove this fishy code (and if at all, ensure that volatile automatics are never coalesced with any other stack slot). Volatility is no reason for not sharing stack slots, it's not fishy code. Ciao, Michael.
[PATCH][7/7] add-referenced-vars TLC
This puts checking code into place that we do not put globals into referenced-vars (nor allocate var-annotations for them). It also fixes remaining cases in mudflap and matrix-reorg. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2012-05-29 Richard Guenther rguent...@suse.de * tree-dfa.c (find_vars_r): Do not call add_referenced_vars for globals. (add_referenced_var_1): Re-organize. Assert we are not called for globals. (remove_referenced_var): Likewise. * varpool.c (add_new_static_var): Use create_tmp_var_raw. * tree-mudflap.c (execute_mudflap_function_ops): Do not call add_referenced_var on globals. * matrix-reorg.c (transform_access_sites): Likewise. Index: gcc/tree-dfa.c === *** gcc/tree-dfa.c (revision 187830) --- gcc/tree-dfa.c (working copy) *** find_vars_r (tree *tp, int *walk_subtree *** 430,436 /* If T is a regular variable that the optimizers are interested in, add it to the list of variables. */ ! else if (SSA_VAR_P (*tp)) add_referenced_var_1 (*tp, fn); /* Type, _DECL and constant nodes have no interesting children. --- 430,439 /* If T is a regular variable that the optimizers are interested in, add it to the list of variables. */ ! else if ((TREE_CODE (*tp) == VAR_DECL !!is_global_var (*tp)) ! || TREE_CODE (*tp) == PARM_DECL ! || TREE_CODE (*tp) == RESULT_DECL) add_referenced_var_1 (*tp, fn); /* Type, _DECL and constant nodes have no interesting children. *** add_referenced_var_1 (tree var, struct f *** 560,581 || TREE_CODE (var) == PARM_DECL || TREE_CODE (var) == RESULT_DECL); ! if (!(TREE_CODE (var) == VAR_DECL !VAR_DECL_IS_VIRTUAL_OPERAND (var)) !is_global_var (var)) ! return false; ! if (!*DECL_VAR_ANN_PTR (var)) ! *DECL_VAR_ANN_PTR (var) = ggc_alloc_cleared_var_ann_d (); ! ! /* Insert VAR into the referenced_vars hash table if it isn't present. */ if (referenced_var_check_and_insert (var, fn)) ! return true; return false; } ! /* Remove VAR from the list. */ void remove_referenced_var (tree var) --- 563,586 || TREE_CODE (var) == PARM_DECL || TREE_CODE (var) == RESULT_DECL); ! gcc_checking_assert ((TREE_CODE (var) == VAR_DECL !VAR_DECL_IS_VIRTUAL_OPERAND (var)) ! || !is_global_var (var)); ! /* Insert VAR into the referenced_vars hash table if it isn't present ! and allocate its var-annotation. */ if (referenced_var_check_and_insert (var, fn)) ! { ! gcc_checking_assert (!*DECL_VAR_ANN_PTR (var)); ! *DECL_VAR_ANN_PTR (var) = ggc_alloc_cleared_var_ann_d (); ! return true; ! } return false; } ! /* Remove VAR from the list of referenced variables and clear its !var-annotation. */ void remove_referenced_var (tree var) *** remove_referenced_var (tree var) *** 585,598 void **loc; unsigned int uid = DECL_UID (var); ! /* Preserve var_anns of globals. */ ! if (!is_global_var (var) !(v_ann = var_ann (var))) ! { ! ggc_free (v_ann); ! *DECL_VAR_ANN_PTR (var) = NULL; ! } ! gcc_assert (DECL_P (var)); in.uid = uid; loc = htab_find_slot_with_hash (gimple_referenced_vars (cfun), in, uid, NO_INSERT); --- 590,605 void **loc; unsigned int uid = DECL_UID (var); ! gcc_checking_assert (TREE_CODE (var) == VAR_DECL ! || TREE_CODE (var) == PARM_DECL ! || TREE_CODE (var) == RESULT_DECL); ! ! gcc_checking_assert (!is_global_var (var)); ! ! v_ann = var_ann (var); ! ggc_free (v_ann); ! *DECL_VAR_ANN_PTR (var) = NULL; ! in.uid = uid; loc = htab_find_slot_with_hash (gimple_referenced_vars (cfun), in, uid, NO_INSERT); Index: gcc/varpool.c === *** gcc/varpool.c (revision 187830) --- gcc/varpool.c (working copy) *** add_new_static_var (tree type) *** 449,455 tree new_decl; struct varpool_node *new_node; ! new_decl = create_tmp_var (type, NULL); DECL_NAME (new_decl) = create_tmp_var_name (NULL); TREE_READONLY (new_decl) = 0; TREE_STATIC (new_decl) = 1; --- 449,455 tree new_decl; struct varpool_node *new_node; ! new_decl = create_tmp_var_raw (type, NULL); DECL_NAME (new_decl) = create_tmp_var_name (NULL); TREE_READONLY (new_decl) = 0; TREE_STATIC (new_decl) = 1; Index: gcc/tree-mudflap.c === *** gcc/tree-mudflap.c (revision
Re: FYI: 1500+ typos, with suggested fixes
Also note: the line numbers listed below work for me with yesterday's up-to-date trunk, but if you want to use these commands, you should rerun the commands above so that the line numbers reflect your actual sources. ) This is just a heads up. I'm not volunteering to make these changes. FWIW, there are many false positives on the gcc/ada directory (missing Ada specific notions and acronyms). A few real typo fixes also of course, but the ratio does not look very good. Arno
Re: FYI: 1500+ typos, with suggested fixes
On May 29, 2012, at 2:49 PM, Jim Meyering wrote: Running the following command spots over 1500 typos, and suggests fixes: $ git ls-files|misspellings -f -|grep -v '^ERROR:'|perl -pe \ 's/^(.*?)\[(\d+)\]: (\w+) - (.*?)$/sed -i '\''${2}s!$3!$4!'\'' $1/' k The misspellings command comes from here: http://github.com/lyda/misspell-check From the list below, I've excluded any that affect ChangeLog or .po files: $ grep -vE 'ChangeLog|\.po$' k k2 (Note that these sed commands are just suggestions that do not take into account any context, and that some of them have two or more alternates, so that applying the sed command as-is would be invalid. Also note: the line numbers listed below work for me with yesterday's up-to-date trunk, but if you want to use these commands, you should rerun the commands above so that the line numbers reflect your actual sources. ) This is just a heads up. I'm not volunteering to make these changes. Some of them are wrong for Ada: sed -i '80s!withing!within!' gcc/ada/a-calend-vms.adb sed -i '79s!withing!within!' gcc/ada/a-calend.adb sed -i '1488s!withing!within!' gcc/ada/a-calend.adb withing is the action of 'with' a unit. sed -i '41s!STPO!Stop!' gcc/ada/a-dynpri.adb STPO is the acronym of System.Task_Primitives.Operations Tristan.
Re: [google/gcc-4_6] Fix -gfission ICEs with pubnames and .debug_addr table (issue6254054)
On 12-05-25 18:29 , Cary Coutant wrote: 2012-05-25 Sterling Augustinesaugust...@google.com Cary Coutantccout...@google.com * gcc/dwarf2out.c (remove_loc_list_addr_table_entries): New function. (is_class_die): Return TRUE for DW_TAG_structure_type. (resolve_addr): Remove address table entries when replacing a location list. OK. Diego.
Re: [driver, LTO Patch]: Resurrect user specs support
On Tue, 29 May 2012, Christian Bruel wrote: I agree. I see two potential settings of the validated field. The % that we just reviewed, and the case : /* We have Xno-YYY, search for XYYY. */ It seems possible to remove those, I've just checked this during lunch :-) with the scenarios described above (without use m* f* specs). If it is a prerequisite before my --spec patch, I'd like to propose it as an independent one, since this is a valid modification regardless of the --spec implementation (although it makes it clearer). Please do send such a patch (with an explanation in each case of how validated still gets set with that redundant setting removed). -- Joseph S. Myers jos...@codesourcery.com
Re: FYI: 1500+ typos, with suggested fixes
Arnaud Charlet wrote: Also note: the line numbers listed below work for me with yesterday's up-to-date trunk, but if you want to use these commands, you should rerun the commands above so that the line numbers reflect your actual sources. ) This is just a heads up. I'm not volunteering to make these changes. FWIW, there are many false positives on the gcc/ada directory (missing Ada specific notions and acronyms). A few real typo fixes also of course, but the ratio does not look very good. In any list of that size, at least a few are guaranteed to be false positives. That's why I wrote this part: (Note that these sed commands are just suggestions that do not take into account any context ... The figure of 1500+ was rounded down from the actual total of 1586 suggestions. Eliminating the FPs matching /TJE|TEH|STPO|withing/i reduces that to 1501.
Re: FYI: 1500+ typos, with suggested fixes
A lot of the changes are to files that GCC either imports strictly verbatim with no local changes at all, or will make changes locally to only for serious issues relating to integration with GCC; in either case, such typos cannot justify making changes locally in GCC so if you want those issues fixed you'll need to send them to the relevant upstream projects. On Tue, 29 May 2012, Jim Meyering wrote: sed -i '711s!Cant!Cannot,Can not,Can't!' boehm-gc/allchblk.c boehm-gc is externally maintained so such changes would need to go upstream. sed -i '130s!RELA!Real!' gcc/config/stormy16/stormy-abi In general this looks spurious in anything dealing with ELF. sed -i '26s!ASLO!Also!' gcc/testsuite/ada/acats/tests/a/ae3002g.ada ACATS is an external project so changes would need to go upstream. sed -i '1s!throught!thought,through,throughout!' gcc/testsuite/c-c++-common/pr41779.c I don't think it's particularly worthwhile fixing typos in testcases. sed -i '323s!loosing!losing!' intl/dcigettext.c sed -i '720s!thru!through!' intl/plural.c libintl comes from gettext so changes would need to go upstream. sed -i '87s!relevent!relevant!' libffi/src/arm/gentramp.sh libffi is externally maintained so it's best for changes to go upstream. sed -i '37s!truely!truly!' libgcc/soft-fp/op-common.h soft-fp is part of glibc so changes should go upstream. sed -i '25s!sence!since,sense!' libgo/go/compress/flate/deflate.go libgo is maintained elsehwere so changes should go upstream. sed -i '576s!sould!could,soul,should,sold!' libjava/classpath/NEWS classpath is an external project so changes should go upstream. sed -i '5133s!Ammend!Amend!' libstdc++-v3/doc/html/ext/lwg-active.html These files are maintained by WG21 so changes should go there. sed -i '34s!untill!until!' zlib/contrib/ada/zlib-streams.ads zlib is an external project so changes should go upstream. -- Joseph S. Myers jos...@codesourcery.com
Re: FYI: 1500+ typos, with suggested fixes
On Tue, 29 May 2012, Jim Meyering wrote: sed -i '464s!occurence!occurrence!' gcc/doc/include/texinfo.tex sed -i '4802s!achive!archive,achieve!' gcc/doc/include/texinfo.tex I missed these when first listing files from upstream projects; anyway, I've now updated texinfo.tex from upstream to fix these. 2012-05-29 Joseph Myers jos...@codesourcery.com * doc/include/texinfo.tex: Update to version 2012-05-16.16. Tested with make pdf. Diffs omitted. -- Joseph S. Myers jos...@codesourcery.com
[PATCH] Fix PR53516
This fixes PR53516 - we are happily vectorizing/memsetting stores to bitfields. That's obviously wrong. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. Richard. 2012-05-29 Richard Guenther rguent...@suse.de PR tree-optimization/53516 * tree-data-ref.c (stmt_with_adjacent_zero_store_dr_p): Reject bitfield accesses. * tree-vect-data-refs.c (vect_analyze_data_refs): Likewise. * gcc.dg/torture/pr53516.c: New testcase. Index: gcc/tree-data-ref.c === *** gcc/tree-data-ref.c (revision 187943) --- gcc/tree-data-ref.c (working copy) *** stores_from_loop (struct loop *loop, VEC *** 5255,5280 bool stmt_with_adjacent_zero_store_dr_p (gimple stmt) { ! tree op0, op1; bool res; struct data_reference *dr; if (!stmt || !gimple_vdef (stmt) ! || !is_gimple_assign (stmt) ! || !gimple_assign_single_p (stmt) ! || !(op1 = gimple_assign_rhs1 (stmt)) ! || !(integer_zerop (op1) || real_zerop (op1))) return false; dr = XCNEW (struct data_reference); - op0 = gimple_assign_lhs (stmt); DR_STMT (dr) = stmt; ! DR_REF (dr) = op0; res = dr_analyze_innermost (dr, loop_containing_stmt (stmt)) ! stride_of_unit_type_p (DR_STEP (dr), TREE_TYPE (op0)); free_data_ref (dr); return res; --- 5255,5287 bool stmt_with_adjacent_zero_store_dr_p (gimple stmt) { ! tree lhs, rhs; bool res; struct data_reference *dr; if (!stmt || !gimple_vdef (stmt) ! || !gimple_assign_single_p (stmt)) ! return false; ! ! lhs = gimple_assign_lhs (stmt); ! rhs = gimple_assign_rhs1 (stmt); ! ! /* If this is a bitfield store bail out. */ ! if (TREE_CODE (lhs) == COMPONENT_REF !DECL_BIT_FIELD (TREE_OPERAND (lhs, 1))) ! return false; ! ! if (!(integer_zerop (rhs) || real_zerop (rhs))) return false; dr = XCNEW (struct data_reference); DR_STMT (dr) = stmt; ! DR_REF (dr) = lhs; res = dr_analyze_innermost (dr, loop_containing_stmt (stmt)) ! stride_of_unit_type_p (DR_STEP (dr), TREE_TYPE (lhs)); free_data_ref (dr); return res; Index: gcc/tree-vect-data-refs.c === *** gcc/tree-vect-data-refs.c (revision 187943) --- gcc/tree-vect-data-refs.c (working copy) *** vect_analyze_data_refs (loop_vec_info lo *** 2972,2981 return false; } - base = unshare_expr (DR_BASE_ADDRESS (dr)); - offset = unshare_expr (DR_OFFSET (dr)); - init = unshare_expr (DR_INIT (dr)); - if (stmt_can_throw_internal (stmt)) { if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS)) --- 2972,2977 *** vect_analyze_data_refs (loop_vec_info lo *** 2997,3002 --- 2993,3024 return false; } + if (TREE_CODE (DR_REF (dr)) == COMPONENT_REF + DECL_BIT_FIELD (TREE_OPERAND (DR_REF (dr), 1))) + { + if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS)) + { + fprintf (vect_dump, not vectorized: statement is bitfield +access ); + print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM); + } + + if (bb_vinfo) + { + STMT_VINFO_VECTORIZABLE (stmt_info) = false; + stop_bb_analysis = true; + continue; + } + + if (gather) + free_data_ref (dr); + return false; + } + + base = unshare_expr (DR_BASE_ADDRESS (dr)); + offset = unshare_expr (DR_OFFSET (dr)); + init = unshare_expr (DR_INIT (dr)); + if (is_gimple_call (stmt)) { if (vect_print_dump_info (REPORT_UNVECTORIZED_LOCATIONS)) Index: gcc/testsuite/gcc.dg/torture/pr53516.c === *** gcc/testsuite/gcc.dg/torture/pr53516.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr53516.c (revision 0) *** *** 0 --- 1,32 + /* { dg-do run } */ + /* { dg-options -ftree-vectorize -ftree-loop-distribute-patterns } */ + + extern void abort (void); + + struct Foo + { + char a : 1; + char b : 7; + }; + + struct Foo x[256]; + int y[256]; + + void __attribute__((noinline,noclone)) bar (int n) + { + int i; + for (i = 0; i n; ++i) + { + x[i].a = 0; + y[i] = 3; + } + } + + int main() + { + x[5].b = 7; + bar (256); + if (x[5].b != 7) + abort (); + return 0; + }
Re: FYI: 1500+ typos, with suggested fixes
Joseph S. Myers wrote: I've applied this patch to fix what seemed to be some of the more straightforward cases covered by gcc/ChangeLog. Thanks. I've run the same thing on gettext and reported the results upstream.
s%funcs%ifuncs typo in libatomic/acinclude.m4
Hi! The subject already says it all. It got me confused what »funcs« that might be. OK to commit with regenerating configure? diff --git a/libatomic/acinclude.m4 b/libatomic/acinclude.m4 index 282a992..a236a6d 100644 --- a/libatomic/acinclude.m4 +++ b/libatomic/acinclude.m4 @@ -188,7 +188,7 @@ dnl dnl Check whether the target supports ifuncs. dnl AC_DEFUN([LIBAT_CHECK_IFUNC], [ - AC_CACHE_CHECK([whether the target supports funcs], libat_cv_have_ifunc, [ + AC_CACHE_CHECK([whether the target supports ifuncs], libat_cv_have_ifunc, [ save_CFLAGS=$CFLAGS CFLAGS=$CFLAGS -Werror AC_TRY_LINK([ Grüße, Thomas pgpp0sg9UUiLi.pgp Description: PGP signature
Re: [patch] disintegrate integrate.[ch]
On Tue, May 29, 2012 at 11:17 AM, Eric Botcazou ebotca...@adacore.com wrote: Yes, I realize that, but it looks like that code is not doing something because old integrate.c choked on it. Quoting that part of the patch: Index: expmed.c === --- expmed.c (revision 187936) +++ expmed.c (working copy) @@ -1866,6 +1866,9 @@ extract_fixed_bit_field (enum machine_mode tmode, shift it so it does. */ /* Maybe propagate the target for the shift. */ /* But not if we will return it--could confuse integrate.c. */ + /* ??? What does the above comment mean in relation to the code + below? NB, integrate.c is no more, so I guess it can't be + confused by anything anymore... */ rtx subtarget = (target != 0 REG_P (target) ? target : 0); if (tmode != mode) subtarget = 0; op0 = expand_shift (RSHIFT_EXPR, mode, op0, bitpos, subtarget, 1); Does that But not if... comment refer to the if (tmode != mode) subtarget = 0; line? Of so, can/should we drop that line (as well as the comment)? I don't know this part of the compiler well enough to tell. I don't think it's a direct reference. There is the same pattern at the end of the function, where TARGET has already been zeroed if mode != tmode. If the mode don't match, expand_shift cannot reuse [SUB]TARGET as-is, so the code consistently avoids to pass it. I'd just drop the dangling reference to integrate.c and reformat the line if (tmode != mode) subtarget = 0; into if (mode != mode) subtarget = 0; to make it more closely ressemble the other instance. Done, updated diff attached. OK for trunk? Ciao! Steven disintegrate_integrate.diff Description: Binary data
Re: FYI: 1500+ typos, with suggested fixes
On Tue, 29 May 2012, Jim Meyering wrote: sed -i '167s!Millenial!Millennial!' libgcc/config/libbid/bid128_string.c sed -i '30s!guranteed!guaranteed!' libgcc/config/libbid/bid64_fma.c sed -i '174s!oposite!opposite!' libgcc/config/libbid/bid64_fma.c sed -i '218s!oposite!opposite!' libgcc/config/libbid/bid64_fma.c sed -i '154s!Millenial!Millennial!' libgcc/config/libbid/bid64_string.c sed -i '1825s!pased!passed!' libgcc/config/libbid/bid_internal.h libbid is also externally maintained, see http://gcc.gnu.org/codingconventions.html. I've applied this patch with fixes for some of the more straightforward issues in the rest of libgcc and in libcpp. I suggest sending an updated typo list for current sources - minus the parts for directories identified as maintained elsewhere, and possibly split by directory - as smaller lists are easier for people to go through. For libjava and gcc/java changes, CC j...@gcc.gnu.org on the list (they may well be able to deal with the classpath typos although properly it's a separate project). For libstdc++-v3 changes (other than libstdc++-v3/doc/html/ext/ which are files from WG21), CC libstd...@gcc.gnu.org. Index: libgcc/configure.ac === --- libgcc/configure.ac (revision 187963) +++ libgcc/configure.ac (working copy) @@ -315,7 +315,7 @@ AC_SUBST(vis_hide) # See if we have thread-local storage. We can only test assembler -# sicne link-time and run-time tests require the newly built +# since link-time and run-time tests require the newly built # gcc, which can't be used to build executable due to that libgcc # is yet to be built here. GCC_CHECK_CC_TLS Index: libgcc/ChangeLog === --- libgcc/ChangeLog(revision 187963) +++ libgcc/ChangeLog(working copy) @@ -1,3 +1,33 @@ +2012-05-29 Joseph Myers jos...@codesourcery.com + + * config/arm/ieee754-df.S: Fix typos. + * config/arm/ieee754-sf.S: Fix typos. + * config/c6x/libunwind.S: Fix typos. + * config/epiphany/udivsi3-float.c: Fix typos. + * config/microblaze/muldi3_hard.S: Fix typos. + * config/picochip/adddi3.S: Fix typos. + * config/picochip/ashlsi3.S: Fix typos. + * config/picochip/ashrsi3.S: Fix typos. + * config/picochip/clzsi2.S: Fix typos. + * config/picochip/cmpsi2.S: Fix typos. + * config/picochip/divmod15.S: Fix typos. + * config/picochip/divmodhi4.S: Fix typos. + * config/picochip/divmodsi4.S: Fix typos. + * config/picochip/longjmp.S: Fix typos. + * config/picochip/lshrsi3.S: Fix typos. + * config/picochip/parityhi2.S: Fix typos. + * config/picochip/popcounthi2.S: Fix typos. + * config/picochip/setjmp.S: Fix typos. + * config/picochip/subdi3.S: Fix typos. + * config/picochip/ucmpsi2.S: Fix typos. + * config/picochip/udivmodhi4.S: Fix typos. + * config/picochip/udivmodsi4.S: Fix typos. + * config/spu/divv2df3.c: Fix typos. + * config/spu/mfc_multi_tag_release.c: Fix typos. + * config/spu/mfc_tag_release.c: Fix typos. + * configure.ac: Fix typos. + * configure: Regenerate. + 2012-05-25 Ian Lance Taylor i...@google.com * config/i386/morestack.S (__morestack_non_split): Check whether Index: libgcc/config/c6x/libunwind.S === --- libgcc/config/c6x/libunwind.S (revision 187963) +++ libgcc/config/c6x/libunwind.S (working copy) @@ -113,7 +113,7 @@ # Zero demand saved flags mvk .s1 0, A0 stw .d2t1 A0, *+B15[1] - # Save return address, setup additional argument and call fucntion + # Save return address, setup additional argument and call function stw .d2t2 B3, *+B15[35] add .d\argside B15, 4, \argreg do_call __gnu\name Index: libgcc/config/spu/mfc_tag_release.c === --- libgcc/config/spu/mfc_tag_release.c (revision 187963) +++ libgcc/config/spu/mfc_tag_release.c (working copy) @@ -25,7 +25,7 @@ extern vector unsigned int __mfc_tag_table; /* Release the specified DMA tag from exclusive use. Once released, the - tag is available for future reservation. Upon sucessful release, + tag is available for future reservation. Upon successful release, MFC_DMA_TAG_VALID is returned. If the specified tag is not in the range 0 to 31, or had not been reserved, no action is taken and MFC_DMA_TAG_INVALID is returned. */ Index: libgcc/config/spu/mfc_multi_tag_release.c === --- libgcc/config/spu/mfc_multi_tag_release.c (revision 187963) +++ libgcc/config/spu/mfc_multi_tag_release.c (working copy) @@ -26,7 +26,7 @@ /* Release a sequential group of tags from exclusive use. The sequential group of
Re: FYI: 1500+ typos, with suggested fixes
On Tue, 29 May 2012, Joseph S. Myers wrote: For libjava and gcc/java changes, CC j...@gcc.gnu.org on the list (they may well be able to deal with the classpath typos although properly it's a separate project). For libstdc++-v3 changes (other than libstdc++-v3/doc/html/ext/ which are files from WG21), CC libstd...@gcc.gnu.org. Also, for changes to gcc/fortran/ and libgfortran/, CC fort...@gcc.gnu.org. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch] disintegrate integrate.[ch]
Done, updated diff attached. You also need to adjust the ChangeLog: * expmed.c (extract_fixed_bit_field): Add ??? for seemingly outdated comment about integrate.c. OK for trunk? I can only formally approve the cse.c and expmed.c hunks, so you probably need to run this by a GR. Nice cleanup in any case! -- Eric Botcazou
Re: [patch] disintegrate integrate.[ch]
On 12-05-29 11:09 , Eric Botcazou wrote: I can only formally approve the cse.c and expmed.c hunks, so you probably need to run this by a GR. The other changes are OK. Nice cleanup in any case! Indeed. Though you probably made my next cxx-conversion merge more difficult ;) Thanks for doing this. Diego.
Re: s%funcs%ifuncs typo in libatomic/acinclude.m4
Hi! On Tue, 29 May 2012 16:35:31 +0200, I wrote: The subject already says it all. It got me confused what »funcs« that might be. OK to commit with regenerating configure? diff --git a/libatomic/acinclude.m4 b/libatomic/acinclude.m4 index 282a992..a236a6d 100644 --- a/libatomic/acinclude.m4 +++ b/libatomic/acinclude.m4 @@ -188,7 +188,7 @@ dnl dnl Check whether the target supports ifuncs. dnl AC_DEFUN([LIBAT_CHECK_IFUNC], [ - AC_CACHE_CHECK([whether the target supports funcs], libat_cv_have_ifunc, [ + AC_CACHE_CHECK([whether the target supports ifuncs], libat_cv_have_ifunc, [ save_CFLAGS=$CFLAGS CFLAGS=$CFLAGS -Werror AC_TRY_LINK([ Actually, I'm getting additional/unrelated changes in configure -- probably someone forgot to regenerated this earlier on, or used a version different from 2.64? Anyway, OK to commit all this? diff --git a/libatomic/configure b/libatomic/configure index 9a7e5a5..c936ece 100755 --- a/libatomic/configure +++ b/libatomic/configure @@ -655,7 +655,6 @@ CCAS am__fastdepCC_FALSE am__fastdepCC_TRUE CCDEPMODE -am__nodep AMDEPBACKSLASH AMDEP_FALSE AMDEP_TRUE @@ -3044,11 +3043,11 @@ MAKEINFO=${MAKEINFO-${am_missing_run}makeinfo} # We need awk for the check target. The system awk is bad on # some platforms. -# Always define AMTAR for backward compatibility. Yes, it's still used -# in the wild :-( We should find a proper way to deprecate it ... -AMTAR='$${TAR-tar}' +# Always define AMTAR for backward compatibility. -am__tar='$${TAR-tar} chof - $$tardir' am__untar='$${TAR-tar} xf -' +AMTAR=${AMTAR-${am_missing_run}tar} + +am__tar='${AMTAR} chof - $$tardir'; am__untar='${AMTAR} xf -' @@ -3946,7 +3945,6 @@ fi if test x$enable_dependency_tracking != xno; then am_depcomp=$ac_aux_dir/depcomp AMDEPBACKSLASH='\' - am__nodep='_no' fi if test x$enable_dependency_tracking != xno; then AMDEP_TRUE= @@ -3971,7 +3969,6 @@ else # instance it was reported that on HP-UX the gcc test will end up # making a dummy file named `D' -- because `-MD' means `put the output # in D'. - rm -rf conftest.dir mkdir conftest.dir # Copy depcomp to subdir because otherwise we won't find it if we're # using a relative directory. @@ -4031,7 +4028,7 @@ else break fi ;; -msvc7 | msvc7msys | msvisualcpp | msvcmsys) +msvisualcpp | msvcmsys) # This compiler won't grok `-c -o', but also, the minuso test has # not run yet. These depmodes are late enough in the game, and # so weak that their functioning should not be impacted. @@ -4106,7 +4103,6 @@ else # instance it was reported that on HP-UX the gcc test will end up # making a dummy file named `D' -- because `-MD' means `put the output # in D'. - rm -rf conftest.dir mkdir conftest.dir # Copy depcomp to subdir because otherwise we won't find it if we're # using a relative directory. @@ -4164,7 +4160,7 @@ else break fi ;; -msvc7 | msvc7msys | msvisualcpp | msvcmsys) +msvisualcpp | msvcmsys) # This compiler won't grok `-c -o', but also, the minuso test has # not run yet. These depmodes are late enough in the game, and # so weak that their functioning should not be impacted. @@ -11015,7 +11011,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 11018 configure +#line 11014 configure #include confdefs.h #if HAVE_DLFCN_H @@ -11121,7 +7,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat conftest.$ac_ext _LT_EOF -#line 11124 configure +#line 11120 configure #include confdefs.h #if HAVE_DLFCN_H @@ -14540,8 +14536,8 @@ $as_echo #define HAVE_ATTRIBUTE_ALIAS 1 confdefs.h fi if test x$try_ifunc = xyes; then - { $as_echo $as_me:${as_lineno-$LINENO}: checking whether the target supports funcs 5 -$as_echo_n checking whether the target supports funcs... 6; } + { $as_echo $as_me:${as_lineno-$LINENO}: checking whether the target supports ifuncs 5 +$as_echo_n checking whether the target supports ifuncs... 6; } if test ${libat_cv_have_ifunc+set} = set; then : $as_echo_n (cached) 6 else Grüße, Thomas pgpiEMgvzUTIo.pgp Description: PGP signature
Regenerating fixincludes/configure
Hi! While looking at libatomic's, I noticed that fixincludes' configure can't be regenerated without errors. Here is a patch, in spirit of 749dea2a0549c126a0e992a6dd8e9b5eb28e1cee. OK to commit? fixincludes/ * configure.ac: Use GCC_AC_FUNC_MMAP_BLACKLIST instead of gcc_AC_FUNC_MMAP_BLACKLIST. * aclocal.m4: Regenerate. * configure: Regenerate. diff --git a/fixincludes/aclocal.m4 b/fixincludes/aclocal.m4 index b23541c..7237922 100644 --- a/fixincludes/aclocal.m4 +++ b/fixincludes/aclocal.m4 @@ -12,6 +12,6 @@ # PARTICULAR PURPOSE. m4_include([../config/acx.m4]) +m4_include([../config/mmap.m4]) m4_include([../config/override.m4]) m4_include([../config/warnings.m4]) -m4_include([../gcc/acinclude.m4]) diff --git a/fixincludes/configure b/fixincludes/configure index ea889b8..4836cd8 100755 --- a/fixincludes/configure +++ b/fixincludes/configure @@ -5222,7 +5222,7 @@ else # read() to the same fd. The only system known to have a problem here # is VMS, where text files have record structure. case $host_os in - vms* | ultrix*) + *vms* | ultrix*) gcc_cv_func_mmap_file=no ;; *) gcc_cv_func_mmap_file=yes;; @@ -5246,7 +5246,7 @@ else # Systems known to be in this category are Windows (all variants), # VMS, and Darwin. case $host_os in - vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) + *vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) gcc_cv_func_mmap_dev_zero=no ;; *) gcc_cv_func_mmap_dev_zero=yes;; @@ -5303,7 +5303,7 @@ else # above for use of /dev/zero. # Systems known to be in this category are Windows, VMS, and SCO Unix. case $host_os in - vms* | cygwin* | pe | mingw* | sco* | udk* ) + *vms* | cygwin* | pe | mingw* | sco* | udk* ) gcc_cv_func_mmap_anon=no ;; *) gcc_cv_func_mmap_anon=yes;; diff --git a/fixincludes/configure.ac b/fixincludes/configure.ac index f1fb2ff..f8f352f 100644 --- a/fixincludes/configure.ac +++ b/fixincludes/configure.ac @@ -96,7 +96,7 @@ AC_CHECK_DECLS(m4_split(m4_normalize(fixincludes_UNLOCKED_FUNCS))) AC_C_CONST # Checks for library functions. -gcc_AC_FUNC_MMAP_BLACKLIST +GCC_AC_FUNC_MMAP_BLACKLIST AC_MSG_CHECKING([whether to enable maintainer-specific portions of Makefiles]) AC_ARG_ENABLE(maintainer-mode, Grüße, Thomas pgpxihRJqk25r.pgp Description: PGP signature
[AVR,Committed]: Re: How to run a compiled C program?
Ian Lance Taylor wrote: Georg-Johann Lay a...@gjlay.de writes: The avr backend auto-generates a part of the texi documentation by means of a small C program. The relevant part of t-avr reads: s-avr-mmcu-texi: gen-avr-mmcu-texi$(build_exeext) $(RUN_GEN) $ | sed -e 's:\r::g' avr-mmcu.texi There was a problem report that the executable cannot be found. When I wrote that I checked other uses of RUN_GEN (which is empty) in $(builddir)/gcc/Makefile that do calls like $(RUN_GEN) build/... What is the correct way to call the executable? Add ./ like so? s-avr-mmcu-texi: gen-avr-mmcu-texi$(build_exeext) $(RUN_GEN) ./$ | sed -e 's:\r::g' avr-mmcu.texi Yes. Ian Thanks, applied here: http://gcc.gnu.org/viewcvs?view=revisionrevision=187968 Johann
_FORTIFY_SOURCE for std::vector
This patch evaluates _FORTIFY_SOURCE in a way similar to GNU libc. If set, std::vector::operator[] throws if the index is out of bounds. This is compliant with the standard because such usage triggers undefined behavior. _FORTIFY_SOURCE users expect some performance hit. Okay for trunk? 2012-05-29 Florian Weimer fwei...@redhat.com * include/bits/stl_vector.h (vector::_M_fortify_range_check): New. * (vector::operator[]): Call it. * testsuite/23_containers/vector/element_access/2.cc: New. -- Florian Weimer / Red Hat Product Security Team Index: libstdc++-v3/include/bits/stl_vector.h === --- libstdc++-v3/include/bits/stl_vector.h (revision 187951) +++ libstdc++-v3/include/bits/stl_vector.h (working copy) @@ -768,7 +768,10 @@ */ reference operator[](size_type __n) - { return *(this-_M_impl._M_start + __n); } + { + _M_fortify_range_check(__n); + return *(this-_M_impl._M_start + __n); + } /** * @brief Subscript access to the data contained in the %vector. @@ -783,7 +786,10 @@ */ const_reference operator[](size_type __n) const - { return *(this-_M_impl._M_start + __n); } + { + _M_fortify_range_check(__n); + return *(this-_M_impl._M_start + __n); + } protected: /// Safety check used only from at(). @@ -794,6 +800,16 @@ __throw_out_of_range(__N(vector::_M_range_check)); } + /// Range check used by operator[]. + /// No-op unless _FORTIFY_SOURCE is enabled. + void + _M_fortify_range_check(size_type __n) const + { +#if defined _FORTIFY_SOURCE _FORTIFY_SOURCE 0 + _M_range_check(__n); +#endif + } + public: /** * @brief Provides access to the data contained in the %vector. Index: libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc === --- libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc (revision 0) +++ libstdc++-v3/testsuite/23_containers/vector/element_access/2.cc (revision 0) @@ -0,0 +1,71 @@ +// Copyright (C) 2012 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// http://www.gnu.org/licenses/. + +// 23.2.4 vector + +// { dg-add-options no_pch } + +#undef _FORTIFY_SOURCE +#define _FORTIFY_SOURCE 2 + +#include vector +#include stdexcept +#include testsuite_hooks.h + +void test01() +{ + std::vectorint v(5); + try +{ + v[5]; + VERIFY( false ); +} + catch(std::out_of_range err) +{ + VERIFY( true ); +} + catch(...) +{ + VERIFY( false ); +} +} + +void test02() +{ + std::vectorint v(5); + const std::vectorint u(v); + try +{ + u[5]; + VERIFY( false ); +} + catch(std::out_of_range err) +{ + VERIFY( true ); +} + catch(...) +{ + VERIFY( false ); +} +} + +int main() +{ + test01(); + test02(); + return 0; +}
Re: Regenerating fixincludes/configure
On May 29, 2012, at 5:27 PM, Thomas Schwinge wrote: Hi! While looking at libatomic's, I noticed that fixincludes' configure can't be regenerated without errors. Here is a patch, in spirit of 749dea2a0549c126a0e992a6dd8e9b5eb28e1cee. OK to commit? Humm, looks like the same as Tobias patch: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01465.html That I commented in: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01467.html Tristan.
Re: FYI: 1500+ typos, with suggested fixes
On Tue, 29 May 2012, Jim Meyering wrote: sed -i '8s!compability!compatibility!' config/mt-sde sed -i '21s!enviroments!environments!' config/stdint.m4 sed -i '293s!arbitary!arbitrary!' config/tcl.m4 sed -i '376s!arbitary!arbitrary!' config/tcl.m4 sed -i '818s!apropriate!appropriate!' config/tcl.m4 I've applied this patch to config/ and resynced config/ in the src repository from the GCC copy (there didn't appear to be any src-local changes not applied to GCC). Index: mt-sde === --- mt-sde (revision 187967) +++ mt-sde (working copy) @@ -5,6 +5,6 @@ # as they have the D-to-I redirect for PC-relative loads. -mno-gpopt # has two purposes: it allows libraries to be used in situations where # $gp != our _gp, and it allows them to be built with -G8 while -# retaining link compability with -G0 and -G4. +# retaining link compatibility with -G0 and -G4. CFLAGS_FOR_TARGET += -Os -minterlink-mips16 -mcode-xonly -mno-gpopt CXXFLAGS_FOR_TARGET += -Os -minterlink-mips16 -mcode-xonly -mno-gpopt Index: stdint.m4 === --- stdint.m4 (revision 187967) +++ stdint.m4 (working copy) @@ -18,7 +18,7 @@ dnl existence of an include file stdint.h that defines a set of dnl typedefs, especially uint8_t,int32_t,uintptr_t. dnl Many older installations will not provide this file, but some will -dnl have the very same definitions in inttypes.h. In other enviroments +dnl have the very same definitions in inttypes.h. In other environments dnl we can use the inet-types in sys/types.h which would define the dnl typedefs int8_t and u_int8_t respectivly. dnl Index: ChangeLog === --- ChangeLog (revision 187967) +++ ChangeLog (working copy) @@ -1,3 +1,9 @@ +2012-05-29 Joseph Myers jos...@codesourcery.com + + * mt-sde: Fix typos. + * stdint.m4: Fix typos. + * tcl.m4: Fix typos. + 2012-04-03 Tristan Gingold ging...@adacore.com * mmap.m4: Use *vms* instead of vms*. Index: tcl.m4 === --- tcl.m4 (revision 187967) +++ tcl.m4 (working copy) @@ -290,7 +290,7 @@ elif test `uname -s` = Darwin; then # If Tcl was built as a framework, attempt to use the libraries # from the framework at the given location so that linking works - # against Tcl.framework installed in an arbitary location. + # against Tcl.framework installed in an arbitrary location. case ${TCL_DEFS} in *TCL_FRAMEWORK*) if test -f ${TCL_BIN_DIR}/${TCL_LIB_FILE}; then @@ -373,7 +373,7 @@ elif test `uname -s` = Darwin; then # If Tk was built as a framework, attempt to use the libraries # from the framework at the given location so that linking works - # against Tk.framework installed in an arbitary location. + # against Tk.framework installed in an arbitrary location. case ${TK_DEFS} in *TK_FRAMEWORK*) if test -f ${TK_BIN_DIR}/${TK_LIB_FILE}; then @@ -815,7 +815,7 @@ # # Defines the following variable: # -# MAN_FLAGS - The apropriate flags for installManPage +# MAN_FLAGS - The appropriate flags for installManPage # according to the user's selection. # # -- Joseph S. Myers jos...@codesourcery.com
[C++] Reject variably modified types in operator new
This patch flags operator new on variably modified types as an error. If this is acceptable, this will simplify the implementation of the C++11 requirement to throw std::bad_array_new_length instead of allocating a memory region which is too short. Okay for trunk? Or should I guard this with -fpermissive? 2012-05-29 Florian Weimer fwei...@redhat.com * init.c (build_new): Reject variably modified types. 2012-05-29 Florian Weimer fwei...@redhat.com * g++.dg/init/new33.C: New. -- Florian Weimer / Red Hat Product Security Team Index: gcc/cp/init.c === --- gcc/cp/init.c (revision 187951) +++ gcc/cp/init.c (working copy) @@ -2844,6 +2844,13 @@ if (!complete_type_or_maybe_complain (type, NULL_TREE, complain)) return error_mark_node; + if (variably_modified_type_p (type, NULL_TREE)) +{ + if (complain tf_error) + error (new cannot be applied to a variably modified type); + return error_mark_node; +} + rval = build_new_1 (placement, type, nelts, init, use_global_new, complain); if (rval == error_mark_node) return error_mark_node; Index: gcc/testsuite/g++.dg/init/new33.C === --- gcc/testsuite/g++.dg/init/new33.C (revision 0) +++ gcc/testsuite/g++.dg/init/new33.C (revision 0) @@ -0,0 +1,10 @@ +// { dg-do compile } + +int +main (int argc, char **argv) +{ + typedef char A[argc]; + new A; // { dg-error variably modified } + new A[0]; // { dg-error variably modified } + new A[5]; // { dg-error variably modified } +}
Re: [driver, LTO Patch]: Resurrect user specs support
Please do send such a patch (with an explanation in each case of how validated still gets set with that redundant setting removed). 'validated' can only be removed for user --specs switches, this is why I think it doesn't need to be propagated to do_specs. My mistake from the previous mail: It can't be split out of the whole patch. For the explanation, We should see a difference in the way define a flag as a .opt internal, and a option in a spec file. %{S: as a substitute doesn't really define a new flag. But we want to allow them. This why it works. a --spec file option is validated as soon as a user switch matches a substitute spec string. It can't possibly be an invalid option and a user substitute rule in the same time, so there is no need to check in do_specs. validate_all_switches is enough. Of course for .opt flags it is different, and the current handling is unchanged. But I need the .known guard, because we must make sure that we are not caught by any * matching. If the switch is defined by a substitute option in a user spec rule, then the switch is validated when we see it. Since there is necessarily a spec rule, it is necessarily used in a spec. So all other checks in do_specs are not needed. The only risk is that we might mark a switch as SWITCH_FALSE even if it's not valid. Which would be the case for instance if we use a %flag rule in a internal GCC spec that is only defined in a --spec file..., but that would be a forbidden use, caught by an error. So I tested the following semantics, with the ones that you pointed. 1) undefined switch === unchanged: passing a switch that is not declared in a .opt internal file and not defined in a --spec file is an error. If it is not declared in a .opt but defined in a --spec file it is accepted 2) defined unchanged a) passing a switch that is declared in a .opt internal file, but is not in a spec is an error b) passing a switch that is declared in a .opt internal file, and used in any spec rule is accepted. even it it is just to ignore it % such switch are marked with the .know field of the 'struct switchstr' when it is not decoded as OPT_SPECIAL_unknown. 3) defined changed == a) passing a switch that is defined in a --spec file is accepted. Because the driver will necessary process the spec definition rule at some point (starting from EXTRA_SPECS). It there is no definition, then it is an error b) if the switch is used in the * case it is accepted * A switch handled in the * case that is defined in a --spec file doesn't not need to be validated at this point, because it will be validated when processing the spec option. * If there is no spec rule for this switch this will be an error. This implementation should change the current semantics when --spec is not used, or when it is used only new substitutes should be allowed. I'm attaching here a revised version of the patch that contains the removal of the 'validated' field in do_specs for non internal flags, thanks for your feedback. Christian Index: gcc/gcc.c === --- gcc/gcc.c (revision 187927) +++ gcc/gcc.c (working copy) @@ -190,8 +190,8 @@ static void store_arg (const char *, int, int); static void insert_wrapper (const char *); static char *load_specs (const char *); -static void read_specs (const char *, int); -static void set_spec (const char *, const char *); +static void read_specs (const char *, bool, bool); +static void set_spec (const char *, const char *, bool); static struct compiler *lookup_compiler (const char *, size_t, const char *); static char *build_search_list (const struct path_prefix *, const char *, bool, bool); @@ -227,9 +227,9 @@ static void do_self_spec (const char *); static const char *find_file (const char *); static int is_directory (const char *, bool); -static const char *validate_switches (const char *); +static const char *validate_switches (const char *, bool); static void validate_all_switches (void); -static inline void validate_switches_from_spec (const char *); +static inline void validate_switches_from_spec (const char *, bool); static void give_switch (int, int); static int used_arg (const char *, int); static int default_arg (const char *, int); @@ -1171,11 +1171,12 @@ const char **ptr_spec; /* pointer to the spec itself. */ struct spec_list *next; /* Next spec in linked list. */ int name_len; /* length of the name */ - int alloc_p; /* whether string was allocated */ + bool user_p; /* whether string come from file spec. */ + bool alloc_p; /* whether string was allocated */ }; #define INIT_STATIC_SPEC(NAME,PTR) \ -{ NAME, NULL, PTR, (struct spec_list *) 0, sizeof (NAME) - 1, 0 } + { NAME, NULL, PTR, (struct spec_list *) 0, sizeof (NAME) - 1, false, false } /* List of statically defined specs. */ static struct spec_list static_specs[] = @@ -1479,7 +1480,7 @@ current
PATCH: Always create a new language function for nested functions
Hi All, Last week I went to build a mips-gnu-linux toolchain and the built compiler seg faulted while building glibc. Bisecting the change pointed to r187757. The problem really goes back to r178692, which added support for -Wunused-local-typedefs. r187757 just made it easier to hit by enabling the warning more aggressively. The problem is that in r187757 the following changes were applied: @@ -8455,9 +8479,11 @@ void c_push_function_context (void) { - struct language_function *p; - p = ggc_alloc_language_function (); - cfun-language = p; + struct language_function *p = cfun-language; + /* cfun-language might have been already allocated by the use of + -Wunused-local-typedefs. In that case, just re-use it. */ + if (p == NULL) +cfun-language = p = ggc_alloc_cleared_language_function (); p-base.x_stmt_tree = c_stmt_tree; c_stmt_tree.x_cur_stmt_list @@ -8483,7 +8509,11 @@ pop_function_context (); p = cfun-language; - cfun-language = NULL; + /* When -Wunused-local-typedefs is in effect, cfun-languages is + used to store data throughout the life time of the current cfun, + So don't deallocate it. */ + if (!warn_unused_local_typedefs) +cfun-language = NULL; This causes problems with nested functions because the following scenario might happen (and did happen in the cause of building glibc for MIPS): 1. A nested function is found while compiling with -Wunused-local-typedefs. 2. c_push_function_context reuses the outer functions cfun-language instance. 3. c_push_function_context saves a reference to the current functions statement tree: p-base.x_stmt_tree = c_stmt_tree; 4. c_pop_function_context is executed after processing the nested function. Note the c_stmt_tree value is still saved per step (3). 5. The outer function continues to be parsed and upon encountering more statements the statement tree is resized. This puts the original statement tree memory back in the free pool. Therefore cfun-language-base.x_stmt_tree is pointing to free memory. 6. The memory that was previously associated with the statement tree gets allocated to something else and written. 7. finish_function is called for the outer function and the garbage collector is invoked. The garbage collector crashes trying to walk the memory associated with the x_stmt_tree field, which is now owned by something else. The attached patch fixes the problem by always allocating a new language function when going into a new function context (it reverts back to the original code). I suppose another option would be to clear all the saved fields in c_pop_function_context, but that seems like more trouble than it is worth. I wasn't able to devise a simplified reproduction case for this problem. I could only reproduce it by building glibc. The patch was tested by building mips-linux-gnu and bootstrapping and running the full test suite for i686-pc-linux-gnu. OK? P.S. If it is OK, then can someone commit for me (I don't have write access)? 2012-05-29 Meador Inge mead...@codesourcery.com * c-decl.c (c_push_function_context): Always create a new language function. (c_pop_function_context): Clear the language function created in c_push_function_context. -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software Index: gcc/c-decl.c === --- gcc/c-decl.c (revision 187923) +++ gcc/c-decl.c (working copy) @@ -8579,11 +8579,9 @@ check_for_loop_decls (location_t loc, bo void c_push_function_context (void) { - struct language_function *p = cfun-language; - /* cfun-language might have been already allocated by the use of - -Wunused-local-typedefs. In that case, just re-use it. */ - if (p == NULL) -cfun-language = p = ggc_alloc_cleared_language_function (); + struct language_function *p; + p = ggc_alloc_language_function (); + cfun-language = p; p-base.x_stmt_tree = c_stmt_tree; c_stmt_tree.x_cur_stmt_list @@ -8609,11 +8607,7 @@ c_pop_function_context (void) pop_function_context (); p = cfun-language; - /* When -Wunused-local-typedefs is in effect, cfun-languages is - used to store data throughout the life time of the current cfun, - So don't deallocate it. */ - if (!warn_unused_local_typedefs) -cfun-language = NULL; + cfun-language = NULL; if (DECL_STRUCT_FUNCTION (current_function_decl) == 0 DECL_SAVED_TREE (current_function_decl) == NULL_TREE)
Re: [C++] Reject variably modified types in operator new
On Tue, May 29, 2012 at 11:00 AM, Florian Weimer fwei...@redhat.com wrote: This patch flags operator new on variably modified types as an error. If this is acceptable, this will simplify the implementation of the C++11 requirement to throw std::bad_array_new_length instead of allocating a memory region which is too short. Okay for trunk? Or should I guard this with -fpermissive? I must say that ideally this should go in. However, this having been accepted in previous releases, I think people would like one release of deprecation. So my suggestion is: -- make it an error unless -fpermissive. -- if -fpermissive, make it unconditionally deprecated. -- schedule for entire removal in 4.9. 2012-05-29 Florian Weimer fwei...@redhat.com * init.c (build_new): Reject variably modified types. 2012-05-29 Florian Weimer fwei...@redhat.com * g++.dg/init/new33.C: New. -- Florian Weimer / Red Hat Product Security Team
Re: _FORTIFY_SOURCE for std::vector
Hi, This patch evaluates _FORTIFY_SOURCE in a way similar to GNU libc. If set, std::vector::operator[] throws if the index is out of bounds. This is compliant with the standard because such usage triggers undefined behavior. _FORTIFY_SOURCE users expect some performance hit. Indeed. But at the moment I don't clearly see how this kind of check relates to debug-mode. Library patches should go to the library mailing list too (especially so when controversial ;) Paolo
Re: [Patch ARM] Fix off by one error in neon_evpc_vrev.
On 05/26/2012 01:27 AM, Ramana Radhakrishnan wrote: - for (i = 0; i nelt; i += diff) + for (i = 0; i nelt ; i += (diff + 1)) for (j = 0; j= diff; j += 1) - if (d-perm[i + j] != i + diff - j) - return false; + { + /* This is guaranteed to be true as the value of diff + is 7, 3, 1 and we should have enough elements in the + queue to generate this. Getting a vector mask with a + value of diff other than these values implies that + something is wrong by the time we get here. */ + gcc_assert ((i + j) nelt); Yep, that all looks correct. Unnecessary () in both lines though. r~
[C++ Patch] PR 26155
Hi, in this pretty old issue we crash for this testcase: namespace N { namespace M = N; namespace M {}// { dg-error namespace alias } } after the error message, because error recovery after error fails. We try to do: error (namespace alias %qD not allowed here, assuming %qD, d, DECL_NAMESPACE_ALIAS (d)); d = DECL_NAMESPACE_ALIAS (d); but then we crash in resume_scope because: /* Also, resuming a non-directly nested namespace is a no-no. */ gcc_assert (b-level_chain == current_binding_level); Thus, in the first patch below I changed push_namespace to stay away from this nasty situation and produce and simpler diagnostics and no special error recovery in this case: a false is returned, and the caller, cp_parser_namespace_definition, makes sure to not call a matching pop_namespace. Diagnostics is good, patch passes testing. Alternately, the second patch below does the latter unconditionally, without even trying the assuming thing. Same diagnostics for the testcase at issue, no regressions in this case too. I don't have a strong personal preference, I suppose the current assuming thing may lead to fewer cascading errors, but I don't know for sure, certainly the second patch is simpler. Thanks, Paolo. /cp 2012-05-29 Paolo Carlini paolo.carl...@oracle.com PR c++/26155 * name-lookup.c (push_namespace): Simply return false after error when error recovery is impossible. * name-lookup.h (push_namespace): Update prototype. * parser.c (cp_parser_namespace_definition): Chech push_namespace return value and don't call pop_namespace when false. /testsuite 2012-05-29 Paolo Carlini paolo.carl...@oracle.com PR c++/26155 * g++.dg/parse/namespace-alias-1.C: New. Index: testsuite/g++.dg/parse/namespace-alias-1.C === --- testsuite/g++.dg/parse/namespace-alias-1.C (revision 0) +++ testsuite/g++.dg/parse/namespace-alias-1.C (revision 0) @@ -0,0 +1,7 @@ +// PR c++/26155 + +namespace N +{ + namespace M = N; + namespace M {}// { dg-error namespace alias } +} Index: cp/parser.c === --- cp/parser.c (revision 187968) +++ cp/parser.c (working copy) @@ -14738,6 +14738,7 @@ cp_parser_namespace_definition (cp_parser* parser) tree identifier, attribs; bool has_visibility; bool is_inline; + bool ok; if (cp_lexer_next_token_is_keyword (parser-lexer, RID_INLINE)) { @@ -14766,7 +14767,7 @@ cp_parser_namespace_definition (cp_parser* parser) /* Look for the `{' to start the namespace. */ cp_parser_require (parser, CPP_OPEN_BRACE, RT_OPEN_BRACE); /* Start the namespace. */ - push_namespace (identifier); + ok = push_namespace (identifier); /* inline namespace is equivalent to a stub namespace definition followed by a strong using directive. */ @@ -14792,7 +14793,8 @@ cp_parser_namespace_definition (cp_parser* parser) pop_visibility (1); /* Finish the namespace. */ - pop_namespace (); + if (ok) +pop_namespace (); /* Look for the final `}'. */ cp_parser_require (parser, CPP_CLOSE_BRACE, RT_CLOSE_BRACE); } Index: cp/name-lookup.c === --- cp/name-lookup.c(revision 187968) +++ cp/name-lookup.c(working copy) @@ -3512,9 +3512,10 @@ handle_namespace_attrs (tree ns, tree attributes) } /* Push into the scope of the NAME namespace. If NAME is NULL_TREE, then we - select a name that is unique to this compilation unit. */ + select a name that is unique to this compilation unit. Return false if + something goes very badly wrong, true otherwise. */ -void +bool push_namespace (tree name) { tree d = NULL_TREE; @@ -3544,12 +3545,23 @@ push_namespace (tree name) d = IDENTIFIER_NAMESPACE_VALUE (name); if (d != NULL_TREE TREE_CODE (d) == NAMESPACE_DECL) { + tree dna = DECL_NAMESPACE_ALIAS (d); need_new = 0; - if (DECL_NAMESPACE_ALIAS (d)) + if (dna) { - error (namespace alias %qD not allowed here, assuming %qD, -d, DECL_NAMESPACE_ALIAS (d)); - d = DECL_NAMESPACE_ALIAS (d); + if (NAMESPACE_LEVEL (dna)-level_chain + == current_binding_level) + { + error (namespace alias %qD not allowed here, +assuming %qD, d, dna); + d = dna; + } + else + { + error (namespace alias %qD not allowed here, d); + timevar_cond_stop (TV_NAME_LOOKUP, subtime); + return false; + } } } } @@ -3583,6 +3595,7 @@ push_namespace (tree name) current_namespace = d; timevar_cond_stop
Re: [PATCH] Atom: Scheduler improvements for better imul placement
Hi, Uros! Sorry, I didn't realize that patch was missed. I attached new version. Changelog: 2012-05-29 Yuri Rumyantsev yuri.s.rumyant...@intel.com * config/i386/i386.c (x86_sched_reorder): New function. Added new function x86_sched_reorder. As for multiply modes, currently we handled most frequent case for Atom and in the future this could be enhanced. Thanks, Igor Hello! Ping? Please at least add and URL to the patch, it took me some time to found the latest version [1], I'm not even sure if it is the latest version... I assume that you cleared all issues with middle-end and scheduler maintainers, it is not clear from the message. + (1) IMUL instrction is on the top of list; Typo above. + static int issue_rate = -1; + int n_ready = *pn_ready; + rtx insn; + rtx insn1; + rtx insn2; Please put three definitions above on the same line. + int i; + sd_iterator_def sd_it; + dep_t dep; + int index = -1; + + /* set up issue rate */ + if (issue_rate 0) + issue_rate = ix86_issue_rate(); Please set issue_rate unconditionally here. Also, please follow the GNU style of comments (Full sentence with two spaces after the dot) everywhere, e.g: /* Set up issue rate. */ + if (!(GET_CODE (SET_SRC (insn)) == MULT + GET_MODE (SET_SRC (insn)) == SImode)) + return issue_rate; Is it correct that only SImode multiplies are checked against SImode multiplies? Can't we use DImode or HImode multiply (or other long-latency insns) to put them into the shadow of the first multiply insn? As proposed in [2], there are many other fine-tuning approaches proposed by the scheduler maintainer. OTOH, even the big hammer approach in the proposed patch makes things better, so it is the step in the right direction - and it is existing practice anyway. Under this rationale, I think that the patch should be committed to mainline. But please also consider proposed fine-tunings to refine the scheduling accuracy. So, OK for mainline, if there are no objections from other maintainers in next two days. [1] http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00964.html [2] http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00806.html Thanks, Uros. imul_reordering.patch Description: Binary data
Re: [driver, LTO Patch]: Resurrect user specs support
On Tue, 29 May 2012, Christian Bruel wrote: So I tested the following semantics, with the ones that you pointed. Thanks for the testing. The patch is OK with the ChangeLog conflict markers removed (and the dates on log entries updated to the date of commit). -- Joseph S. Myers jos...@codesourcery.com
Re: PATCH: Always create a new language function for nested functions
On Tue, 29 May 2012, Meador Inge wrote: 2012-05-29 Meador Inge mead...@codesourcery.com * c-decl.c (c_push_function_context): Always create a new language function. (c_pop_function_context): Clear the language function created in c_push_function_context. Thanks, committed. -- Joseph S. Myers jos...@codesourcery.com
Re: FYI: 1500+ typos, with suggested fixes
On Tue, 29 May 2012, Jim Meyering wrote: Running the following command spots over 1500 typos, and suggests fixes: $ git ls-files|misspellings -f -|grep -v '^ERROR:'|perl -pe \ 's/^(.*?)\[(\d+)\]: (\w+) - (.*?)$/sed -i '\''${2}s!$3!$4!'\'' $1/' k This command has some bugs in how it produces fixes, fixing them would make the output more useful in some cases: sed -i '1811s!MISCELANEOUS!Miscellaneous!' gcc/config/mn10300/mn10300.md Should preserve the case of all-uppercase words. sed -i '2102s!hasnt!hasn't!' gcc/config/picochip/picochip.c Needs to allow for the ' in the replacement string. -- Joseph S. Myers jos...@codesourcery.com
Re: FYI: 1500+ typos, with suggested fixes
I've applied these fixes for three miscellaneous directories with a single typo each to fix (gcc/c-family, libmudflap, lto-plugin). Index: libmudflap/mf-impl.h === --- libmudflap/mf-impl.h(revision 187979) +++ libmudflap/mf-impl.h(working copy) @@ -46,7 +46,7 @@ /* Private definitions related to mf-runtime.h */ -#define __MF_TYPE_MAX_CEM __MF_TYPE_STACK /* largest type# for the cemetary */ +#define __MF_TYPE_MAX_CEM __MF_TYPE_STACK /* largest type# for the cemetery */ #define __MF_TYPE_MAX __MF_TYPE_GUESS Index: libmudflap/ChangeLog === --- libmudflap/ChangeLog(revision 187979) +++ libmudflap/ChangeLog(working copy) @@ -1,3 +1,7 @@ +2012-05-29 Joseph Myers jos...@codesourcery.com + + * mf-impl.h: Fix typo. + 2012-05-16 H.J. Lu hongjiu...@intel.com * configure: Regenerated. Index: lto-plugin/ChangeLog === --- lto-plugin/ChangeLog(revision 187979) +++ lto-plugin/ChangeLog(working copy) @@ -1,3 +1,7 @@ +2012-05-29 Joseph Myers jos...@codesourcery.com + + * lto-plugin.c: Fix typo. + 2012-05-16 H.J. Lu hongjiu...@intel.com * configure: Regenerated. Index: lto-plugin/lto-plugin.c === --- lto-plugin/lto-plugin.c (revision 187979) +++ lto-plugin/lto-plugin.c (working copy) @@ -754,7 +754,7 @@ conflicts-syms = xmalloc (sizeof (struct ld_plugin_symbol) * outlen); conflicts-aux = xmalloc (sizeof (struct sym_aux) * outlen); - /* Move all duplicate symbols into the auxillary conflicts table. */ + /* Move all duplicate symbols into the auxiliary conflicts table. */ out = 0; for (i = 0; i t-nsyms; i++) { Index: gcc/c-family/ChangeLog === --- gcc/c-family/ChangeLog (revision 187979) +++ gcc/c-family/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2012-05-29 Joseph Myers jos...@codesourcery.com + + * c-common.c: Fix typo. + 2012-05-29 Michael Matz m...@suse.de * c-common.h (c_expand_decl): Remove prototype. Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 187979) +++ gcc/c-family/c-common.c (working copy) @@ -3960,7 +3960,7 @@ /* Replace the integer argument with a suitable product by the object size. Do this multiplication as signed, then convert to the appropriate type - for the pointer operation and disregard an overflow that occured only + for the pointer operation and disregard an overflow that occurred only because of the sign-extension change in the latter conversion. */ { tree t = build_binary_op (loc, -- Joseph S. Myers jos...@codesourcery.com
Re: PowerPC prologue and epilogue 6
Alan, I think the following patch --- ../_gcc_clean/gcc/testsuite/gcc.target/powerpc/powerpc.exp 2012-05-02 14:25:40.0 +0200 +++ ../work/gcc/testsuite/gcc.target/powerpc/powerpc.exp2012-05-29 21:14:48.0 +0200 @@ -47,4 +47,5 @@ set-torture-options $SAVRES_TEST_OPTS gcc-dg-runtest [list $srcdir/$subdir/savres.c] $alti # All done. +torture-finish dg-finish is required to avoid the errors of the kind ERROR: tcl error sourcing /home/gccbuild/gcc_trunk_anonsvn/gcc/gcc/testsuite/gcc.target/powerpc/powerpc.exp. ERROR: torture-init: torture_without_loops is not empty as expected (see http://gcc.gnu.org/ml/gcc-testresults/2012-05/msg02608.html ). In addition the tests of savres.c fails on powerpc-apple-darwin9 with FAIL: gcc.target/powerpc/savres.c (test for excess errors) Excess errors: /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:109:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:123:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:135:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:170:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:180:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:212:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:222:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:251:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:259:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:289:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:303:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:315:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:350:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:360:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:392:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:402:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:431:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:439:3: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:472:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:491:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:508:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:558:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:573:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:620:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:635:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:679:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:692:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:737:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:756:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:773:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:823:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:838:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:885:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:900:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:944:5: error: PIC register clobbered by 'r31' in 'asm' /opt/gcc/work/gcc/testsuite/gcc.target/powerpc/savres.c:957:5: error: PIC register clobbered by 'r31' in 'asm' WARNING: gcc.target/powerpc/savres.c compilation failed to produce executable However I am not able to say if this generic or due to
Re: Fix fixinclude's configure{,.ac}
Hi! On Tue, 22 May 2012 11:38:55 +0200, Tristan Gingold ging...@adacore.com wrote: On May 22, 2012, at 11:20 AM, Tobias Burnus wrote: an --enable-maintainers-build fails here with: configure.ac:99: error: possibly undefined macro: gcc_AC_FUNC_MMAP_BLACKLIST If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. make[3]: *** [fixincludes/configure] Error 1 Looking at Tristan's commit Rev. 186106 of 2012-04-03, the change gcc_ - GCC_ is missing. It also looks as if the fixinclude/ config files where not regenerated, even though there was a later patch by Tristan which touches fixinclude's config files (Rev. 186759 of 2012-04-24). I encountered the same issue, and came up with the same fix, http://news.gmane.org/find-root.php?message_id=%3c87ehq39fe7@schwinge.name%3E. Indeed, I failed to notice that fixinclude was using ../gcc/config.m4. I think it would also make sense to remove '-I ../gcc' from ACLOCAL_AMFLAGS of fixincludes/Makefile.in I agree. Thank you for having noticed and fixed this bug, Tristan. [ Bruce CC as he is the maintained of fixinclude] Thusly fixed: fixincludes/ * configure.ac: Use GCC_AC_FUNC_MMAP_BLACKLIST instead of gcc_AC_FUNC_MMAP_BLACKLIST. * Makefile.in (ACLOCAL_AMFLAGS): Don't include ../gcc. * aclocal.m4: Regenerate. * configure: Regenerate. diff --git a/fixincludes/Makefile.in b/fixincludes/Makefile.in index b9857b9..92b365c 100644 --- a/fixincludes/Makefile.in +++ b/fixincludes/Makefile.in @@ -66,7 +66,7 @@ mkinstalldirs=$(SHELL) $(srcdir)/../mkinstalldirs AUTOCONF = autoconf AUTOHEADER = autoheader ACLOCAL = aclocal -ACLOCAL_AMFLAGS = -I ../gcc -I .. -I ../config +ACLOCAL_AMFLAGS = -I .. -I ../config default : all diff --git a/fixincludes/aclocal.m4 b/fixincludes/aclocal.m4 index b23541c..7237922 100644 --- a/fixincludes/aclocal.m4 +++ b/fixincludes/aclocal.m4 @@ -12,6 +12,6 @@ # PARTICULAR PURPOSE. m4_include([../config/acx.m4]) +m4_include([../config/mmap.m4]) m4_include([../config/override.m4]) m4_include([../config/warnings.m4]) -m4_include([../gcc/acinclude.m4]) diff --git a/fixincludes/configure b/fixincludes/configure index ea889b8..4836cd8 100755 --- a/fixincludes/configure +++ b/fixincludes/configure @@ -5222,7 +5222,7 @@ else # read() to the same fd. The only system known to have a problem here # is VMS, where text files have record structure. case $host_os in - vms* | ultrix*) + *vms* | ultrix*) gcc_cv_func_mmap_file=no ;; *) gcc_cv_func_mmap_file=yes;; @@ -5246,7 +5246,7 @@ else # Systems known to be in this category are Windows (all variants), # VMS, and Darwin. case $host_os in - vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) + *vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) gcc_cv_func_mmap_dev_zero=no ;; *) gcc_cv_func_mmap_dev_zero=yes;; @@ -5303,7 +5303,7 @@ else # above for use of /dev/zero. # Systems known to be in this category are Windows, VMS, and SCO Unix. case $host_os in - vms* | cygwin* | pe | mingw* | sco* | udk* ) + *vms* | cygwin* | pe | mingw* | sco* | udk* ) gcc_cv_func_mmap_anon=no ;; *) gcc_cv_func_mmap_anon=yes;; diff --git a/fixincludes/configure.ac b/fixincludes/configure.ac index f1fb2ff..f8f352f 100644 --- a/fixincludes/configure.ac +++ b/fixincludes/configure.ac @@ -96,7 +96,7 @@ AC_CHECK_DECLS(m4_split(m4_normalize(fixincludes_UNLOCKED_FUNCS))) AC_C_CONST # Checks for library functions. -gcc_AC_FUNC_MMAP_BLACKLIST +GCC_AC_FUNC_MMAP_BLACKLIST AC_MSG_CHECKING([whether to enable maintainer-specific portions of Makefiles]) AC_ARG_ENABLE(maintainer-mode, Grüße, Thomas pgpv63WNqNmhE.pgp Description: PGP signature
Re: Fix stable_sort to work on iterators returning rvalue
Attached patch applied then. 2012-05-29 François Dumont fdum...@gcc.gnu.org * include/bits/stl_tempbuf.h (__uninitialized_construct_buf) (__uninitialized_construct_buf_dispatch::__ucr): Fix to work with iterator returning rvalue. * testsuite/25_algorithms/stable_sort/3.cc: New. François On 05/28/2012 09:59 PM, Christopher Jefferson wrote: On 28 May 2012, at 20:00, François Dumont wrote: On 05/28/2012 12:11 PM, Christopher Jefferson wrote: My main concern is one I have mentioned previously. I'm unsure all our code works with things like move_iterator, even when it correctly compiles. We previously took out internal uses of move_iterator, because while the code compiled it did not work correctly. Look at bug : http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48038 for any example. With this change, code which uses move_iterator, and operator which pass by value, will cause values to be 'eaten' during the sort. Now, the standard in my opinion doesn't say this would be a bug, but it certainly seems like it would be unpleasant. How to fix this? The simplest way might be to wrap all predicates / operator in a wrapper which just takes lvalue references. Not sure if it is worth going to that much pain just to fix this. At the very least, I would want to see a bunch of tests which make sure we don't have any other code which accidentally 'eats' users data. Make sure you catch all the various bits of sort, some of which get triggered rarely. I realise that is putting other people to a higher standard than I put myself, but we have learnt this is a tricky area :) Chris Does it mean that you refuse the patch ? The patch purpose is not to make the code compatible with move_iterator but to make it compatible with iterator types that return pure rvalue through their dereference operator. As a side effect std::move_iterator are going to compile too which might be bad but is it really a reason to forbid other kind of iterators ? Of course we should find a good way to handle move_iterator, and I can spend some time on it, but I think that it should be the subject of a dedicated patch. Sorry, just to clarify (also, I have been having problems with my mail client). I believe this patch is good, I withdraw any complaint. I believe there is a serious issue with move_iterator in use with almost all standard libraries, but it is disconnected from this patch. Chris Index: include/bits/stl_tempbuf.h === --- include/bits/stl_tempbuf.h (revision 187979) +++ include/bits/stl_tempbuf.h (working copy) @@ -1,7 +1,7 @@ // Temporary buffer implementation -*- C++ -*- // Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, -// 2010, 2011 +// 2010, 2011, 2012 // Free Software Foundation, Inc. // // This file is part of the GNU ISO C++ Library. This library is free @@ -182,25 +182,25 @@ templatebool struct __uninitialized_construct_buf_dispatch { - templatetypename _ForwardIterator, typename _Tp + templatetypename _Pointer, typename _ForwardIterator static void -__ucr(_ForwardIterator __first, _ForwardIterator __last, - _Tp __value) +__ucr(_Pointer __first, _Pointer __last, + _ForwardIterator __seed) { if(__first == __last) return; - _ForwardIterator __cur = __first; + _Pointer __cur = __first; __try { std::_Construct(std::__addressof(*__first), - _GLIBCXX_MOVE(__value)); - _ForwardIterator __prev = __cur; + _GLIBCXX_MOVE(*__seed)); + _Pointer __prev = __cur; ++__cur; for(; __cur != __last; ++__cur, ++__prev) std::_Construct(std::__addressof(*__cur), _GLIBCXX_MOVE(*__prev)); - __value = _GLIBCXX_MOVE(*__prev); + *__seed = _GLIBCXX_MOVE(*__prev); } __catch(...) { @@ -213,9 +213,9 @@ template struct __uninitialized_construct_buf_dispatchtrue { - templatetypename _ForwardIterator, typename _Tp + templatetypename _Pointer, typename _ForwardIterator static void -__ucr(_ForwardIterator, _ForwardIterator, _Tp) { } +__ucr(_Pointer, _Pointer, _ForwardIterator) { } }; // Constructs objects in the range [first, last). @@ -223,23 +223,22 @@ // their exact value is not defined. In particular they may // be 'moved from'. // - // While __value may altered during this algorithm, it will have + // While *__seed may be altered during this algorithm, it will have // the same value when the algorithm finishes, unless one of the // constructions throws. // - // Requirements: _ForwardIterator::value_type(_Tp) is valid. - templatetypename _ForwardIterator, typename _Tp + // Requirements: _Pointer::value_type(_Tp) is valid. + templatetypename _Pointer, typename _ForwardIterator inline void -__uninitialized_construct_buf(_ForwardIterator
Add myself to write-after-approval section of MAINTAINERS file
I have just committed the patch below to add myself to the write-after-approval section of the MAINTAINERS file. Thanks, Edmar ChangeLog: 2012-05-29 Edmar Wienskoskied...@freescale.com * MAINTAINERS (Write After Approval): Add myself. Index: MAINTAINERS === --- MAINTAINERS (revision 187985) +++ MAINTAINERS (working copy) @@ -523,6 +523,7 @@ Florian Weimer f...@deneb.enyo.de Zack Weinberg za...@panix.com Mark Wielaard m...@gcc.gnu.org +Edmar Wienskoski ed...@freescale.com Ollie Wild a...@google.com Kevin Williams kevin.willi...@inria.fr Carlo Wood ca...@alinoe.com
Ping: [RFA:] doc: TARGET_LEGITIMIZE_ADDRESS needs to be defined for native TLS
From: Hans-Peter Nilsson h...@axis.com Date: Wed, 16 May 2012 02:24:02 +0200 Ping... An old patch I finally came around to submit. Verified that the DVI and info output looks ok. Ok to commit with inherent relicensing and whatever? gcc: * doc/tm.texi.in (Addressing Modes) TARGET_LEGITIMIZE_ADDRESS: Mention that this hook needs to be defined for native TLS. * doc/tm.texi: Regenerate. Index: doc/tm.texi.in === --- doc/tm.texi.in(revision 187568) +++ doc/tm.texi.in(working copy) @@ -5465,8 +5465,10 @@ The code of the hook should not alter th @var{x}. If it transforms @var{x} into a more legitimate form, it should return the new @var{x}. -It is not necessary for this hook to come up with a legitimate address. -The compiler has standard ways of doing so in all cases. In fact, it +It is not necessary for this hook to come up with a legitimate address, +with the exception of native TLS addresses (@pxref{Emulated TLS}). +The compiler has standard ways of doing so in all cases. In fact, if +the target supports only emulated TLS, it is safe to omit this hook or make it return @var{x} if it cannot find a valid way to legitimize the address. But often a machine-dependent strategy can generate better code. brgds, H-P
Re: FYI: 1500+ typos, with suggested fixes
Joseph S. Myers wrote: On Tue, 29 May 2012, Jim Meyering wrote: Running the following command spots over 1500 typos, and suggests fixes: $ git ls-files|misspellings -f -|grep -v '^ERROR:'|perl -pe \ 's/^(.*?)\[(\d+)\]: (\w+) - (.*?)$/sed -i '\''${2}s!$3!$4!'\'' $1/' k This command has some bugs in how it produces fixes, fixing them would make the output more useful in some cases: sed -i '1811s!MISCELANEOUS!Miscellaneous!' gcc/config/mn10300/mn10300.md Should preserve the case of all-uppercase words. I suppose that would be useful. sed -i '2102s!hasnt!hasn't!' gcc/config/picochip/picochip.c Needs to allow for the ' in the replacement string. I can do that on the command line, but it may not be worth it. It's getting pretty ugly: git ls-files|misspellings -f -|grep -v '^ERROR:'|perl -p \ -e '/^(.*?)\[(\d+)\]: (\w+) - (.*?)$/ or next;' \ -e '($file,$N,$L,$R)=($1,$2,$3,$4); $q='\''; $R=~s/$q/$q\\$q$q/g;' \ -e 's/.*/sed -i $q${N}s!$L!$R!$q $file/' I.e., it would print this: sed -i '2102s!hasnt!hasn'\''t!' gcc/config/picochip/picochip.c
[gfortran/ssp/quadmath] symvers config tweaks
As per libstdc++/52700, this fixes the configure bits for libgfortran/libssp/libquadmath. With these fixes, I believe all the libs are safe for --enable-symvers=gnu* variants. Super simple patches... I intend to put this on the 4.7 branch as well. tested x86/linux -benjamin 2012-05-29 Benjamin Kosnik b...@redhat.com PR libstdc++/51007 * configure.ac: Allow gnu, gnu* variants for --enable-symvers argument. * configure: Regenerated. diff --git a/libquadmath/configure b/libquadmath/configure index e5c3de6..8beb1a6 100755 --- a/libquadmath/configure +++ b/libquadmath/configure @@ -12355,7 +12355,7 @@ else quadmath_use_symver=yes fi -if test x$quadmath_use_symver = xyes; then +if test x$quadmath_use_symver != xno; then if test x$gcc_no_link = xyes; then # If we cannot link, we cannot build shared libraries, so do not use # symbol versioning. diff --git a/libquadmath/configure.ac b/libquadmath/configure.ac index 512b9f8..d3bfb04 100644 --- a/libquadmath/configure.ac +++ b/libquadmath/configure.ac @@ -169,7 +169,7 @@ AS_HELP_STRING([--disable-symvers], [disable symbol versioning for libquadmath]), quadmath_use_symver=$enableval, quadmath_use_symver=yes) -if test x$quadmath_use_symver = xyes; then +if test x$quadmath_use_symver != xno; then if test x$gcc_no_link = xyes; then # If we cannot link, we cannot build shared libraries, so do not use # symbol versioning. 2012-05-29 Benjamin Kosnik b...@redhat.com PR libstdc++/51007 * configure.ac: Allow gnu, gnu* variants for --enable-symvers argument. * configure: Regenerated. diff --git a/libssp/configure.ac b/libssp/configure.ac index 0eee36c..93dfa8d 100644 --- a/libssp/configure.ac +++ b/libssp/configure.ac @@ -77,7 +77,7 @@ AS_HELP_STRING([--disable-symvers], [disable symbol versioning for libssp]), ssp_use_symver=$enableval, ssp_use_symver=yes) -if test x$ssp_use_symver = xyes; then +if test x$ssp_use_symver != xno; then if test x$gcc_no_link = xyes; then # If we cannot link, we cannot build shared libraries, so do not use # symbol versioning. 2012-05-29 Benjamin Kosnik b...@redhat.com PR libstdc++/51007 * configure.ac: Allow gnu, gnu* variants for --enable-symvers argument. * configure: Regenerated. diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index fc58a5c..97b337e 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -157,7 +157,7 @@ AS_HELP_STRING([--disable-symvers], [disable symbol versioning for libgfortran]), gfortran_use_symver=$enableval, gfortran_use_symver=yes) -if test x$gfortran_use_symver = xyes; then +if test x$gfortran_use_symver != xno; then save_LDFLAGS=$LDFLAGS LDFLAGS=$LDFLAGS -fPIC -shared -Wl,--version-script,./conftest.map cat conftest.map EOF
Re: [C++ Patch] Produce canonical names for debug info without changing normal pretty-printing (issue6215052)
On Wed, May 16, 2012 at 1:03 PM, Sterling Augustine saugust...@google.com wrote: This patch adds new flags and defines such that the C++ decl pretty printer prints both canonical dwarf names for decls without perturbing normal error message output. It addresses the issues with the earlier patches submitted as: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00516.html http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00512.html Which are withdrawn. This patch requires no changes to the testsuite and does not produce visible changes to gcc's output except to dwarf consumers, which will now all agree on the names of functions. Tested with a full bootstrap. OK for mainline? Sterling 2012-05-16 Sterling Augustine saugust...@google.com * gcc/c-family/c-pretty-print.h (pp_c_flag_gnu_v3): New enumerator. * gcc/c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Check it at both the start and end of the function. * gcc/cp/cp-tree.h (TFF_MATCH_GNU_V3_DEMANGLER): Define and comment. * gcc/cp/error.c (dump_decl): Print appropriate string for anonymous namespace based on pp_c_flag_gnu_v3. (decl_as_string): Set cxx_pp-flags based on TFF_MATCH_GNU_V3_DEMANGLER. (lang_decl_name): Handle unnamed namespace decls. * gcc/cp/cp-lang.c (cxx_dwarf_name): Call decl_as_string for namespace decls. Index: gcc/c-family/c-pretty-print.c === --- gcc/c-family/c-pretty-print.c (revision 187603) +++ gcc/c-family/c-pretty-print.c (working copy) @@ -446,8 +446,9 @@ pp_c_specifier_qualifier_list (c_pretty_printer *p { const enum tree_code code = TREE_CODE (t); - if (TREE_CODE (t) != POINTER_TYPE) + if (!(pp-flags pp_c_flag_gnu_v3) TREE_CODE (t) != POINTER_TYPE) pp_c_type_qualifier_list (pp, t); + switch (code) { case REFERENCE_TYPE: @@ -494,6 +495,8 @@ pp_c_specifier_qualifier_list (c_pretty_printer *p pp_simple_type_specifier (pp, t); break; } + if ((pp-flags pp_c_flag_gnu_v3) TREE_CODE (t) != POINTER_TYPE) + pp_c_type_qualifier_list (pp, t); } /* parameter-type-list: Index: gcc/c-family/c-pretty-print.h === --- gcc/c-family/c-pretty-print.h (revision 187603) +++ gcc/c-family/c-pretty-print.h (working copy) @@ -30,7 +30,8 @@ along with GCC; see the file COPYING3. If not see typedef enum { pp_c_flag_abstract = 1 1, - pp_c_flag_last_bit = 2 + pp_c_flag_last_bit = 2, + pp_c_flag_gnu_v3 = 4 } pp_c_pretty_print_flags; Index: gcc/cp/error.c === --- gcc/cp/error.c (revision 187603) +++ gcc/cp/error.c (working copy) @@ -1028,7 +1028,12 @@ dump_decl (tree t, int flags) dump_scope (CP_DECL_CONTEXT (t), flags); flags = ~TFF_UNQUALIFIED_NAME; if (DECL_NAME (t) == NULL_TREE) - pp_cxx_ws_string (cxx_pp, M_({anonymous})); + { + if (!(pp_c_base (cxx_pp)-flags pp_c_flag_gnu_v3)) + pp_cxx_ws_string (cxx_pp, M_({anonymous})); + else + pp_cxx_ws_string (cxx_pp, M_((anonymous namespace))); + } else pp_cxx_tree_identifier (cxx_pp, DECL_NAME (t)); } @@ -2561,6 +2566,8 @@ decl_as_string (tree decl, int flags) { reinit_cxx_pp (); pp_translate_identifiers (cxx_pp) = false; + if (flags TFF_MATCH_GNU_V3_DEMANGLER) + pp_c_base (cxx_pp)-flags |= pp_c_flag_gnu_v3; dump_decl (decl, flags); return pp_formatted_text (cxx_pp); } @@ -2596,6 +2603,9 @@ lang_decl_name (tree decl, int v, bool translate) if (TREE_CODE (decl) == FUNCTION_DECL) dump_function_name (decl, TFF_PLAIN_IDENTIFIER); + else if ((DECL_NAME (decl) == NULL_TREE) + TREE_CODE (decl) == NAMESPACE_DECL) + dump_decl (decl, TFF_PLAIN_IDENTIFIER); else dump_decl (DECL_NAME (decl), TFF_PLAIN_IDENTIFIER); Index: gcc/cp/cp-lang.c === --- gcc/cp/cp-lang.c (revision 187603) +++ gcc/cp/cp-lang.c (working copy) @@ -120,8 +120,14 @@ cxx_dwarf_name (tree t, int verbosity) if (verbosity = 2) return decl_as_string (t, TFF_DECL_SPECIFIERS | TFF_UNQUALIFIED_NAME - | TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS); + | TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS + | TFF_MATCH_GNU_V3_DEMANGLER); + /* decl_as_string handles namespaces--especially anonymous ones--more + appropriately for debugging than cxx_printable_name. But + cxx_printable_name handles templates and global ctors and dtors better. */ + if (TREE_CODE (t) == NAMESPACE_DECL) + return decl_as_string (t,
Re: [C++ Patch] PR 26155
.. Ah! I think I have a much better tentative fix, which, as it happens, also leads to a diagnostics quite close to that produced by the EDG front-end. The idea is checking for the offending case and not handling it in any special way besides turning need_new = 1, as if for a normal new declaration: actually in this case it conflicts with the existing one from the namespace alias declaration, and leads to a guaranteed error a bit later. Patch passes testing. Thanks, Paolo. /// Index: testsuite/g++.dg/parse/namespace-alias-1.C === --- testsuite/g++.dg/parse/namespace-alias-1.C (revision 0) +++ testsuite/g++.dg/parse/namespace-alias-1.C (revision 0) @@ -0,0 +1,7 @@ +// PR c++/26155 + +namespace N +{ + namespace M = N; // { dg-error previous declaration } + namespace M {}// { dg-error declaration of namespace } +} Index: cp/name-lookup.c === --- cp/name-lookup.c(revision 187991) +++ cp/name-lookup.c(working copy) @@ -3544,12 +3544,20 @@ push_namespace (tree name) d = IDENTIFIER_NAMESPACE_VALUE (name); if (d != NULL_TREE TREE_CODE (d) == NAMESPACE_DECL) { + tree dna = DECL_NAMESPACE_ALIAS (d); need_new = 0; - if (DECL_NAMESPACE_ALIAS (d)) + if (dna) { - error (namespace alias %qD not allowed here, assuming %qD, -d, DECL_NAMESPACE_ALIAS (d)); - d = DECL_NAMESPACE_ALIAS (d); + if (NAMESPACE_LEVEL (dna)-level_chain + == current_binding_level) + { + error (namespace alias %qD not allowed here, +assuming %qD, d, dna); + d = dna; + } + else + /* We error out below. */ + need_new = 1; } } }
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
Yes, I understand that's broken, but there are no consumers at this point that make any use of that offset. Would it be acceptable if we just put 0 there? (Given that I expect .debug_pub* to go away soon, I don't think it's worth the trouble of filling in the offset with anything more meaningful.) I wouldn't expect it to be much harder to put the skeleton there than plain 0. I was concerned that we might not always have a skeleton type to point to, but I confess I haven't looked closely enough to know whether that's a valid concern. At the time we emit the pubtypes table, we have a pointer to the DIE that has been moved to the type unit, and there's no mapping from that back to the skeleton DIE. As it stands, we don't even emit a skeleton DIE unless one of its descendants is a declaration, so we can't count on always having a skeleton DIE to point to. In the case of enumeration constants, if we did have a skeleton DIE, it would only be for the parent enumeration type. How about we modify the patch to just emit a 0 for the DIE offset in a pubtype entry? -cary
Re: FYI: 1500+ typos, with suggested fixes
On May 29, 2012, at 5:49 AM, Jim Meyering wrote: Running the following command spots over 1500 typos, and suggests fixes: $ git ls-files|misspellings -f -|grep -v '^ERROR:'|perl -pe \ 's/^(.*?)\[(\d+)\]: (\w+) - (.*?)$/sed -i '\''${2}s!$3!$4!'\'' $1/' k Hum, maybe a make rule (only run by hand), with a file that lists a gcc whitelist of things we like, withing, such a nice word...
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/24/12, Gabriel Dos Reis g...@integrable-solutions.net wrote: On May 24, 2012 Lawrence Crowl cr...@google.com wrote: Add a type-safe hash table, typed_htab. Uses of this table replace uses of libiberty's htab_t. The benefits include less boiler-plate code, full type safety, and improved performance. Lawrence, is there any chance you could just call it hash_table? After the conversion, we will be living most of the time in a typed world, so the typed_ prefix will be redundant if not confusing :-) The name hash_table is already taken in libcpp/include/symtab.h. Do you have any other suggestions? -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/28/12, Eric Botcazou ebotca...@adacore.com wrote: My main concern is that the precise collector we have in place now requires substantial care to use. It is supported by a tool that does not really understand C, let alone C++. We avoid the problems when annotations are unnecessary. We can do that in one of two ways, either by upgrading GTY to understand more of the language so that it infers the right things from regular code, or by changing the implementation strategy so that the garbage collector does not need type information. I prefer the latter approach, as it makes our experience closer to our customers' experience. Are you really saying that, because our customers might have different ways of doing garbage collection in their code, we should drop our implementation that might well be superior to theirs? That would be a weak argument IMO, all the more so that, in the end, this might degrade the GCC experience for them. No, I am suggesting that the current GTY approach requires more manual care than is desireable. There are several ways to fix that. One way is to improve GTY so that less manual effort is needed. However, any work we spend on GTY would not affect our customers except, perhaps, in the data size of the compiler. Another way is to use C++ containers that manage the storage for us, and to back that up with a conservative collector. If we do so, then we are operating in the same environment as our customers. We can fix performance issues in the compiler by generating better code. That better code helps our customers with their code. The intent in living as our customers do is to improve the overall experience of our customers. Having said that, let me be clear. There is no proposal as yet to change anything about garbage collection in gcc other than upgrade it as we need. (See Diego's vec patch.) We are just having a friendly philosophical discussion. :-) -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
On 5/29/12, Richard Guenther richard.guent...@gmail.com wrote: On May 25, 2012 Mike Stump mikest...@comcast.net wrote: On May 25, 2012, at 10:50 AM, Lawrence Crowl wrote: Diego and I looked long and hard at this issue. It all came down to a sequence of problems. First, libstdc++ isn't rigged for GTY, If portability to other C++ compilers wasn't a concern, we could extend out g++ to make supporting GTY better, so that we can simplify and refine the GTY stuff. I fear we need some light weight reflection, might make a nice language feature for a future C++ standard, if done well. Seconded. It points the finger of my #1 concern with the C++ conversion - our GC. We need a GC scheme that allows us to use standard library containers, and the scheme that was outlined earlier would work. Using the standard library containers is not a prerequisite to using C++, nor is it the source of primary benefit. You can build the compiler completely independently of the standard library. There are even good reasons to do so. More importantly, the standard library containers have different operations, different semantics, and different performance implications from the existing containers. Replacing any existing container uses with standard library uses is actually many tasks, which should be done incrementally after the infrastructure is in place. More importantly, some of that replacement would not be trivial, and the specialists in particular data structures might be better suited to it. Are the TR1 hash table implementations using any non-C++98/03 features? If not then I would suggest to use our TR1 hash tables. Almost by definition they are not using non-C++03 features. I do not think we should try to make that change in one step, but make it an eventual goal. -- Lawrence Crowl
Re: [cxx-conversion] New Hash Table (issue6244048)
Hi, On Sun, 27 May 2012, Gabriel Dos Reis wrote: people actually working on it and used to that style. We don't want to have a mixture of several different styles in the compiler. I (and I expect many others) don't want anyone working around the latter by going over the whole source base and reindent everything. Hence inventing a new coding standard for GCC-in-C++ (by reusing existing ones or doing something new) that isn't mostly the same as GCC-in-C isn't going to fly. if this coding standard is going to be adopted as a GNU coding convention, then you have to be flexible and allow yourself to see beyond the past written in C. You have to ask yourself: how do I want the codebase to look like in 10, 15, 20, 25 years. No, I don't have to ask that myself. I don't have to be flexible (just to be clear: I am flexible in many cases, just not because you tell me to be). And no, I don't have to allow myself to see beyond the past you imply. I am seeing beyond the past whenever I see fit. Thanks for fathering my feelings, but please stop doing so. And thanks for making clear what the whole GCC-in-c++ stunt is about. ( I didn't know beforehand that the C++ proponents for GCC set out to start a new GNU coding standard for C++ and wanted to make GCC the case at hand for that adventure including all kinds of niceties that c++ perhaps delivers. If that would have been their open goal from the start (which obviously it wasn't; rather it was hidden in feel-good rhetoric about how c++ would magically increase the contributor base, which of course will never happen; I mean how naive is that thinking, a new, more complex language, driving more contributors to a project; hello?) I would have opposed it even stronger from the start. Of course without any result (because a global reviewer is part of the club). You made a reference to the coding standard as to be influenced in the way it is because of the Lisp way, implying that this is the obsolete way, suggesting from wording that this is the old style that's somehow to be overcome. (No doubt you'll now claim that this is not what you said explicitely; which is literally true but doesn't matter because you're very well aware of the mechanics of your rhetoric): That's obviously complete bollocks. While the backend IR is lispy (and even that only on the outset) the coding standard itself has nothing to do with that whatsoever. Not even the RTL routines are written in a lispy style. And the tree/gimple routines obviously have never seen the honor of being read by you. Seeing lisp style in _those_ involves major inventarism. So, if you make the suggestion that I should think and program beyond my (implied) small cubicle of lispy influenced old-style GNU coding standard C code (so as to, in a way, expand my programming capabilities), then I can only say: Gabriel, thanks for your suggestion, but you obviously don't have the slightest idea what you're talking about. Why do you even have the guts to take part in this discussion (apart from being in the c++ fan-boy club, of course)? When was the last submission of yours to the code base in areas that we're talking about? ) Namely useless noise and source change activity for the sake of it. Only marginally interested in an answer, Michael.
Re: PATCH for to use tree clobbers for c++/51060 (temporary re-use)
On Thu, May 3, 2012 at 3:39 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sat, Nov 12, 2011 at 4:42 PM, Jason Merrill ja...@redhat.com wrote: Now that we have a way of explicitly marking a variable as dead, we can use that to indicate the end of a temporary's lifetime by adding it as a cleanup for that temporary. Since gimple_push_cleanup still deals in trees I needed to tweak a couple of places to avoid trying to treat a clobber as a real CONSTRUCTOR, but the changes were small. One somewhat surprising thing that showed up as a result of this change were some failures in the libstdc++ testsuite. When I investigated, I found that the tests were relying on temporaries living longer than they should: const int x = std::max(1, 2); Since std::max takes its arguments by const reference and returns one of them, x ends up as a dangling reference to the temporary containing 2, which dies at the end of the declaration-statement. This patch breaks the testcase because now the temporary stack slots get reused by the next statement, so by the time we look at x it no longer points to a 2. I've fixed the tests by removing the references on the variables. Tested x86_64-pc-linux-gnu, applying to trunk. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53220 -- H.J. This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53356 -- H.J.
Re: [Patch ARM] Fix off by one error in neon_evpc_vrev.
On 29 May 2012 18:30, Richard Henderson r...@redhat.com wrote: On 05/26/2012 01:27 AM, Ramana Radhakrishnan wrote: - for (i = 0; i nelt; i += diff) + for (i = 0; i nelt ; i += (diff + 1)) for (j = 0; j= diff; j += 1) - if (d-perm[i + j] != i + diff - j) - return false; + { + /* This is guaranteed to be true as the value of diff + is 7, 3, 1 and we should have enough elements in the + queue to generate this. Getting a vector mask with a + value of diff other than these values implies that + something is wrong by the time we get here. */ + gcc_assert ((i + j) nelt); Yep, that all looks correct. Unnecessary () in both lines though. Bah - Thanks - don't know why I put those in :( .Committed to trunk with those changes and I would like to backport this to the 4.7 branch after a couple of weeks to allow the auto-testers to pick this up as it really turns on this functionality in this particular case if the release managers don't object. This is a significant performance issue in 4.7 for cases where we reverse vectors and would be nice to fix there. ( 2 loads + 2 generic permutes vs a single reverse instruciton) regards, Ramana 2012-05-30 Ramana Radhakrishnan ramana.radhakrish...@linaro.org * config/arm/arm.c (arm_evpc_neon_vrev): Adjust off by one error. * gcc.target/arm/neon-vrev..c: New. r~ Index: gcc/testsuite/gcc.target/arm/neon-vrev.c === --- gcc/testsuite/gcc.target/arm/neon-vrev.c(revision 0) +++ gcc/testsuite/gcc.target/arm/neon-vrev.c(revision 187999) @@ -0,0 +1,105 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_neon_ok } */ +/* { dg-options -O2 } */ +/* { dg-add-options arm_neon } */ + +#include arm_neon.h + +uint16x4_t +tst_vrev642_u16 (uint16x4_t __a) +{ + uint16x4_t __rv; + uint16x4_t __mask1 = { 3, 2, 1, 0}; + return __builtin_shuffle ( __a, __mask1) ; +} + +uint16x8_t +tst_vrev64q2_u16 (uint16x8_t __a) +{ + uint16x8_t __rv; + uint16x8_t __mask1 = {3, 2, 1, 0, 7, 6, 5, 4 }; + return __builtin_shuffle ( __a, __mask1) ; +} + +uint8x8_t +tst_vrev642_u8 (uint8x8_t __a) +{ + uint8x8_t __rv; + uint8x8_t __mask1 = { 7, 6, 5, 4, 3, 2, 1, 0}; + return __builtin_shuffle ( __a, __mask1) ; +} + +uint8x16_t +tst_vrev64q2_u8 (uint8x16_t __a) +{ + uint8x16_t __rv; + uint8x16_t __mask1 = {7, 6, 5, 4, 3, 2, 1, 0, 15, 14, 13, 12, 11, 10, 9, 8}; + return __builtin_shuffle ( __a, __mask1) ; + +} + +uint32x2_t +tst_vrev642_u32 (uint32x2_t __a) +{ + uint32x2_t __rv; + uint32x2_t __mask1 = {1, 0}; + return __builtin_shuffle ( __a, __mask1) ; + +} + +uint32x4_t +tst_vrev64q2_u32 (uint32x4_t __a) +{ + uint32x4_t __rv; + uint32x4_t __mask1 = {1, 0, 3, 2}; + return __builtin_shuffle ( __a, __mask1) ; +} + +uint16x4_t +tst_vrev322_u16 (uint16x4_t __a) +{ + uint16x4_t __mask1 = { 1, 0, 3, 2 }; + return __builtin_shuffle (__a, __mask1); +} + +uint16x8_t +tst_vrev32q2_u16 (uint16x8_t __a) +{ + uint16x8_t __mask1 = { 1, 0, 3, 2, 5, 4, 7, 6 }; + return __builtin_shuffle (__a, __mask1); +} + +uint8x8_t +tst_vrev322_u8 (uint8x8_t __a) +{ + uint8x8_t __mask1 = { 3, 2, 1, 0, 7, 6, 5, 4}; + return __builtin_shuffle (__a, __mask1); +} + +uint8x16_t +tst_vrev32q2_u8 (uint8x16_t __a) +{ + uint8x16_t __mask1 = { 3, 2, 1, 0, 7, 6, 5, 4, 11, 10, 9, 8, 15, 14, 13, 12}; + return __builtin_shuffle (__a, __mask1); +} + +uint8x8_t +tst_vrev162_u8 (uint8x8_t __a) +{ + uint8x8_t __mask = { 1, 0, 3, 2, 5, 4, 7, 6}; + return __builtin_shuffle (__a, __mask); +} + +uint8x16_t +tst_vrev16q2_u8 (uint8x16_t __a) +{ + uint8x16_t __mask = { 1, 0, 3, 2, 5, 4, 7, 6, 9, 8, 11, 10, 13, 12, 15, 14}; + return __builtin_shuffle (__a, __mask); +} + +/* { dg-final {scan-assembler-times vrev32\.16\\t 2} } */ +/* { dg-final {scan-assembler-times vrev32\.8\\t 2} } */ +/* { dg-final {scan-assembler-times vrev16\.8\\t 2} } */ +/* { dg-final {scan-assembler-times vrev64\.8\\t 2} } */ +/* { dg-final {scan-assembler-times vrev64\.32\\t 2} } */ +/* { dg-final {scan-assembler-times vrev64\.16\\t 2} } */ Index: gcc/config/arm/arm.c === --- gcc/config/arm/arm.c(revision 187998) +++ gcc/config/arm/arm.c(revision 187999) @@ -25637,10 +25637,18 @@ return false; } - for (i = 0; i nelt; i += diff) + for (i = 0; i nelt ; i += diff + 1) for (j = 0; j = diff; j += 1) - if (d-perm[i + j] != i + diff - j) - return false; + { + /* This is guaranteed to be true as the value of diff + is 7, 3, 1 and we should have enough elements in the + queue to generate this. Getting a vector mask with a + value of diff other than these values implies that + something is wrong by the time we get here. */ + gcc_assert (i + j nelt); + if (d-perm[i + j] != i + diff - j) + return
ping*3: Fix gcc.dg/lower-subreg-1.c failure (was: [C Patch]: pr52543)
From: Hans-Peter Nilsson h...@axis.com Date: Wed, 23 May 2012 06:41:58 +0200 From: Hans-Peter Nilsson h...@axis.com Date: Wed, 16 May 2012 08:24:41 +0200 From: Hans-Peter Nilsson h...@axis.com Date: Wed, 9 May 2012 08:02:25 +0200 Ping. I missed the PR number decoration on the ChangeLog entry: PR rtl-optimization/53176 * rtlanal.c (rtx_cost): Adjust default cost for X with a UNITS_PER_WORD factor for all X according to the size of its mode, not just for SUBREGs with untieable modes. http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00609.html Ping again... Yet another ping. brgds, H-P
[PATCH] Fix combiner to create canonical CONST_INTs (PR rtl-optimization/53519)
Hi! GEN_INT (GET_MODE_MASK (mode)) isn't canonical CONST_INT for mode, because it is zero extended rather than sign-extended for GET_MODE_BITSIZE (mode) HOST_BITS_PER_WIDE_INT modes. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2012-05-30 Jakub Jelinek ja...@redhat.com PR rtl-optimization/53519 * combine.c (simplify_shift_const_1) case NOT: Use constm1_rtx instead of GEN_INT (GET_MODE_MASK (mode)) as second operand of XOR. * gcc.c-torture/compile/pr53519.c: New test. --- gcc/combine.c.jj2012-05-12 10:21:06.0 +0200 +++ gcc/combine.c 2012-05-29 21:53:23.448801930 +0200 @@ -10284,8 +10284,7 @@ simplify_shift_const_1 (enum rtx_code co break; /* Make this fit the case below. */ - varop = gen_rtx_XOR (mode, XEXP (varop, 0), - GEN_INT (GET_MODE_MASK (mode))); + varop = gen_rtx_XOR (mode, XEXP (varop, 0), constm1_rtx); continue; case IOR: --- gcc/testsuite/gcc.c-torture/compile/pr53519.c.jj2012-05-29 22:02:16.593763702 +0200 +++ gcc/testsuite/gcc.c-torture/compile/pr53519.c 2012-05-29 22:01:10.0 +0200 @@ -0,0 +1,26 @@ +/* PR rtl-optimization/53519 */ + +int a, b, c, d, e; + +short int +foo (short int x) +{ + return a == 0 ? x : 0; +} + +short int +bar (int x, int y) +{ + return x + y; +} + +void +baz (void) +{ + if (!e) +{ + int f = foo (65535 ^ b); + if (bar (!6L = ~f, ~e) == c) + d = 0; +} +} Jakub