Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code
Dear Janus, Of course you can commit to 4.6. Be quick, though; 4.6.2 was due for release now-ish - GCC 4.6 branch remains open under normal release branch rules, accepting regression and documentation fixes. GCC 4.6.2 is tentatively planned for late September or early October. Thanks Paul On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi Paul, This is OK for trunk. Thanks fo rthe patch. Thanks. Committed to trunk as r180062. What about 4.6? Cheers, Janus On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi all, here is a patch which fixes the regression in comment #2 of the PR in the subject line. What it does is setting the 'ts.is_c_interop' flag correctly for constants with kind-parameter specification (such as '0.0_c_double'), as is already being done for variables. Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6? Cheers, Janus 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * primary.c (match_kind_param): Detect ISO_C_BINDING kinds. (get_kind): Pass on 'is_iso_c' flag. (match_integer_constant,match_real_constant,match_logical_constant): Set 'ts.is_c_interop'. 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * gfortran.dg/c_kind_tests_3.f03: New. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
[pph] Function Merging (issue5278047)
Add function merging. First, let function mangled names match. Second, overwrite existing struct functions for merged functions. Third, add filtering for already emitted functions. The result is ICEs in the call graph code. Add tests for overload sets with more than one member. Bootstrapped on x64. 2011-10-16 Lawrence Crowl cr...@google.com Index: gcc/testsuite/ChangeLog.pph * g++.dg/pph/c4inline.cc: Change to ICE in cgraph. * g++.dg/pph/x1keyed.cc: Likewise. * g++.dg/pph/x1keyno.cc: Likewise. * g++.dg/pph/x4keyed.cc: Likewise. * g++.dg/pph/x4keyex.cc: Likewise. * g++.dg/pph/x4keyno.cc: Likewise. * g++.dg/pph/x4tmplclass1.cc: Likewise. * g++.dg/pph/x4tmplclass2.cc: Likewise. * g++.dg/pph/x4tmplfuncinln.cc: Likewise. * g++.dg/pph/x4tmplfuncninl.cc: Likewise. * g++.dg/pph/x6rtti.cc: Likewise. * g++.dg/pph/x7rtti.cc: Likewise. * g++.dg/pph/z4tmplclass1.cc: Likewise. * g++.dg/pph/z4tmplclass2.cc: Likewise. * g++.dg/pph/z4tmplfuncinln.cc: Likewise. * g++.dg/pph/z4tmplfuncninl.cc: Likewise. * g++.dg/pph/x1tmplclass2.cc: Change to missing function. Index: gcc/cp/ChangeLog.pph 2011-10-16 Lawrence Crowl cr...@google.com * pph-streamer-in.c (pph_match_to_overload): Comment on overloading. (pph_match_to_function): Allow functions to match for merging. Comment on overloading. (pph_match_to_link): Comment on overloading. (pph_in_struct_function): Implement overwriting a struct function when merging. (pph_in_symtab): Add filtering for already emitted functions. * pph-streamer.h (pph_trace_tree): Add a boolean parameter specifying whether or not the tree was actually merged. * pph-streamer.c (pph_trace_tree): Add a boolean parameter specifying whether or not the tree was actually merged. Change output to correspond. Update callers. Index: gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc === --- gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (revision 180072) +++ gcc/testsuite/g++.dg/pph/x4tmplfuncninl.cc (working copy) @@ -1,6 +1,5 @@ -// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } } -// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in instantiate_decl { xfail *-*-* } 0 } -// { dg-excess-errors Template list problems } +// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } } +// { dg-bogus x4tmplfuncninl.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502 { xfail *-*-* } 0 } #include x0tmplfuncninl1.h #include x0tmplfuncninl2.h Index: gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc === --- gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (revision 180072) +++ gcc/testsuite/g++.dg/pph/z4tmplfuncninl.cc (working copy) @@ -1,6 +1,5 @@ -// { dg-xfail-if ICE { *-*-* } { -fpph-map=pph.map } } -// { dg-bogus a0tmplfuncninl_g.h:12:23: internal compiler error: in instantiate_decl { xfail *-*-* } 0 } -// { dg-excess-errors Template list problems } +// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } } +// { dg-bogus z4tmplfuncninl.cc:1:0: internal compiler error: in cgraph_create_node, at cgraph.c:502 { xfail *-*-* } 0 } #include x0tmplfuncninl3.h #include x0tmplfuncninl4.h Index: gcc/testsuite/g++.dg/pph/x4keyno.cc === --- gcc/testsuite/g++.dg/pph/x4keyno.cc (revision 180071) +++ gcc/testsuite/g++.dg/pph/x4keyno.cc (working copy) @@ -1,5 +1,8 @@ -// { dg-xfail-if BOGUS MERGE AUXVAR { *-*-* } { -fpph-map=pph.map } } -// { dg-bogus x4keyno.cc:12:1: error: redefinition of 'const char _ZTS5keyno { xfail *-*-* } 0 } +// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } } +// { dg-bogus x4keyno.cc:15:1: internal compiler error: in cgraph_analyze_functions, at cgraphunit.c:1193 { xfail *-*-* } 0 } +// { dg-excess-errors typeinfo name duplicatd } + +// Previously. // The variable for the typeinfo name for 'keyno' is duplicated. #include x0keyno1.h Index: gcc/testsuite/g++.dg/pph/x1keyed.cc === --- gcc/testsuite/g++.dg/pph/x1keyed.cc (revision 180071) +++ gcc/testsuite/g++.dg/pph/x1keyed.cc (working copy) @@ -1,3 +1,6 @@ +// { dg-xfail-if ICE CGRAPH { *-*-* } { -fpph-map=pph.map } } +// { dg-bogus x1keyed.cc:12:1: internal compiler error: in cgraph_analyze_functions, at cgraphunit.c:1193 { xfail *-*-* } 0 } + #include x0keyed1.h int keyed::key( int arg ) { return mix( field arg ); } Index: gcc/testsuite/g++.dg/pph/x7rtti.cc === --- gcc/testsuite/g++.dg/pph/x7rtti.cc (revision 180071) +++ gcc/testsuite/g++.dg/pph/x7rtti.cc (working copy) @@ -1,18 +1,7 @@ // FIXME pph: This should be a { dg=do run
[patch] Fix PR tree-optimization/49960 ,Fix self data dependence
This patch fixes the failures described in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49960 It also fixes bzips when run with autopar enabled. In both cases the self dependences are not handled correctly. In the first case, a non affine access is analyzed: in the second, the distance vector is not calculated correctly (the distance vector considered for for self dependences is always (0,0,...)) As a result, the loops get wrongfully parallelized. The patch avoids the special handling of self dependences, and analyzes all dependences in the same way. Specific adjustments and support for the self dependence cases were made. Bootstrap and testsuite pass successfully for ppc64-redhat-linux. OK for trunk? Thank you, Razya ChangeLog: PR tree-optimization/49960 * tree-data-ref.c (compute_self_dependence): Remove. (initialize_data_dependence_relation): Add intializations. Remove compute_self_dependence. (add_other_self_distances): Add support for two dimensions if the second is zero. (compute_affine_dependence): Remove the !DDR_SELF_REFERENCE condition. (compute_all_dependences): Remove call to compute_self_dependence. Add call to compute_affine_dependence testsuite/ChangeLog: PR tree-optimization/49660 * gcc.dg/autopar/pr49660.c: New test. * gcc.dg/autopar/pr49660-1.c: New test. Index: tree-data-ref.c === --- tree-data-ref.c (revision 179799) +++ tree-data-ref.c (working copy) @@ -1346,7 +1346,6 @@ dr_may_alias_p (const struct data_reference *a, co return refs_may_alias_p (addr_a, addr_b); } -static void compute_self_dependence (struct data_dependence_relation *); /* Initialize a data dependence relation between data accesses A and B. NB_LOOPS is the number of loops surrounding the references: the @@ -1386,13 +1385,30 @@ initialize_data_dependence_relation (struct data_r the data dependence tests, just initialize the ddr and return. */ if (operand_equal_p (DR_REF (a), DR_REF (b), 0)) { + if (loop_nest + !object_address_invariant_in_loop_p (VEC_index (loop_p, loop_nest, 0), + DR_BASE_OBJECT (a))) + { + DDR_ARE_DEPENDENT (res) = chrec_dont_know; + return res; + } DDR_AFFINE_P (res) = true; DDR_ARE_DEPENDENT (res) = NULL_TREE; DDR_SUBSCRIPTS (res) = VEC_alloc (subscript_p, heap, DR_NUM_DIMENSIONS (a)); DDR_LOOP_NEST (res) = loop_nest; DDR_INNER_LOOP (res) = 0; DDR_SELF_REFERENCE (res) = true; - compute_self_dependence (res); + for (i = 0; i DR_NUM_DIMENSIONS (a); i++) + { + struct subscript *subscript; + + subscript = XNEW (struct subscript); + SUB_CONFLICTS_IN_A (subscript) = conflict_fn_not_known (); + SUB_CONFLICTS_IN_B (subscript) = conflict_fn_not_known (); + SUB_LAST_CONFLICT (subscript) = chrec_dont_know; + SUB_DISTANCE (subscript) = chrec_dont_know; + VEC_safe_push (subscript_p, heap, DDR_SUBSCRIPTS (res), subscript); + } return res; } @@ -3119,8 +3135,11 @@ add_other_self_distances (struct data_dependence_r { if (DDR_NUM_SUBSCRIPTS (ddr) != 1) { - DDR_ARE_DEPENDENT (ddr) = chrec_dont_know; - return; + if (DDR_NUM_SUBSCRIPTS (ddr) != 2 || !integer_zerop (DR_ACCESS_FN (DDR_A (ddr), 1))) + { + DDR_ARE_DEPENDENT (ddr) = chrec_dont_know; + return; + } } access_fun = DR_ACCESS_FN (DDR_A (ddr), 0); @@ -4037,8 +4056,7 @@ compute_affine_dependence (struct data_dependence_ } /* Analyze only when the dependence relation is not yet known. */ - if (DDR_ARE_DEPENDENT (ddr) == NULL_TREE - !DDR_SELF_REFERENCE (ddr)) + if (DDR_ARE_DEPENDENT (ddr) == NULL_TREE) { dependence_stats.num_dependence_tests++; @@ -4113,39 +4131,6 @@ compute_affine_dependence (struct data_dependence_ fprintf (dump_file, )\n); } -/* This computes the dependence relation for the same data - reference into DDR. */ - -static void -compute_self_dependence (struct data_dependence_relation *ddr) -{ - unsigned int i; - struct subscript *subscript; - - if (DDR_ARE_DEPENDENT (ddr) != NULL_TREE) -return; - - for (i = 0; VEC_iterate (subscript_p, DDR_SUBSCRIPTS (ddr), i, subscript); - i++) -{ - if (SUB_CONFLICTS_IN_A (subscript)) - free_conflict_function (SUB_CONFLICTS_IN_A (subscript)); - if (SUB_CONFLICTS_IN_B (subscript)) - free_conflict_function (SUB_CONFLICTS_IN_B (subscript)); - - /* The accessed index overlaps for each iteration. */ - SUB_CONFLICTS_IN_A (subscript) - = conflict_fn (1, affine_fn_cst
Re: [C++ Patch] PR 32614
On Sun, Oct 16, 2011 at 1:03 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Sun, Oct 16, 2011 at 5:42 AM, Richard Guenther richard.guent...@gmail.com wrote: On Sun, Oct 16, 2011 at 12:31 PM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/16/2011 12:28 PM, Gabriel Dos Reis wrote: On Sun, Oct 16, 2011 at 5:03 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: Hi, in this simple documentation PR, Tom noticed that we have a (very long standing) inconsistency between the default value of -fmessage-length for C++ as documented and as implemented: in fact it's 0 in cxx-pretty-print.c, like all the other front-ends. At the time of the PR people briefly envisage changing the default but the discussion didn't go anywhere, I think we can as well do the below, for the time being at least, remove the inconsistency. Ok? I still think the default for g++ should be 72. Notice that other front-ends have set it to zero because they feared something, unlike the C++ frontend. To be clear, I have no strong opinion. But last time Richard Gunther strongly disagreed (and now he is a Global Reviewer ;) Thus, just let me know guys... 0 is just so much more convenient for consumers and all consumers that care for line lengths can properly wrap around. So I don't see a good reason to have -fmessage-length at all. This is an argument that should have been made more than a decade ago and -fmessage-length was *requested* by users who did care about line length, and implemented for g++. I wasn't around at that time, so sorry for not raising my opinion earlier ;) Richard.
Re: [PATCH] Clear DECL_GIMPLE_REG_P when making parameter copy addressable (PR tree-optimization/50735)
On Sun, Oct 16, 2011 at 5:47 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! gimplify_parameters uses create_tmp_reg, but sometimes it decides to make it addressable (if the PARM_DECL is addressable). If so, it must not be DECL_GIMPLE_REG_P. Alternatively we could call create_tmp_reg only if !TREE_ADDRESSABLE and call create_tmp_var instead for TREE_ADDRESSABLE (+ set TREE_ADDRESSABLE). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I think this should be exactly the other way around, using create_tmp_var, copying TREE_ADDRESSABLE and setting DECL_GIMPLE_REG_P if it is not addressable. Ok with that change. Thanks, Richard. 2011-10-16 Jakub Jelinek ja...@redhat.com PR tree-optimization/50735 * function.c (gimplify_parameters): When making local TREE_ADDRESSABLE, also clear DECL_GIMPLE_REG_P. --- gcc/function.c.jj 2011-10-14 08:21:56.0 +0200 +++ gcc/function.c 2011-10-15 12:43:23.0 +0200 @@ -3624,7 +3624,10 @@ gimplify_parameters (void) not the PARMs. Keep the parms address taken as we'll query that flag during gimplification. */ if (TREE_ADDRESSABLE (parm)) - TREE_ADDRESSABLE (local) = 1; + { + TREE_ADDRESSABLE (local) = 1; + DECL_GIMPLE_REG_P (local) = 0; + } } else { Jakub
Re: [PATCH 1/5] Use MADV_DONTNEED for freeing in garbage collector
On Sun, Oct 16, 2011 at 8:33 PM, Andi Kleen a...@firstfloor.org wrote: On Sun, Oct 16, 2011 at 12:38:16PM +0200, Richard Guenther wrote: On Sun, Oct 16, 2011 at 7:30 AM, Andi Kleen a...@firstfloor.org wrote: Andi Kleen a...@firstfloor.org writes: From: Andi Kleen a...@linux.intel.com Use the Linux MADV_DONTNEED call to unmap free pages in the garbage collector.Then keep the unmapped pages in the free list. This avoid excessive memory fragmentation on large LTO bulds, which can lead to gcc bumping into the Linux vm_max_map limit per process. Could I have a decision on this patch please? The problem in PR50636 is still there and this is the minimum fix to fix it on Linux as far as I know. If this patch is not the right way to go I would appreciate some guidance on an alternative (but low cost) implementation. Note I don't have capacity for any overly complicated solutions. The patch looks generally ok, but you are never giving back pages to the It gives back pages, just not virtual address space. But I guess that is what you meant. On the other hand this patch can actually give you more virtual address space when you need large regions (2 pages or so). The reason is that the old allocation pattern fragments the whole address space badly and only leaves these small holes. With the madvise patch that does not happen, ggc is all in a compacted chunk. Sure, but we do compete with the glibc heap with virtual memory usage (I wonder if GGC should simply use malloc/free ...). So I am worried that we run out of address space earlier this way. But I guess we can revisit this when we run into actual problems ... system, and as we have other memory allocations that do not use the ggc pools you drain virtual memory on 32bit hosts. Is any other patch in this series compensating for it? If not I'd say we should munmap the pages when a full mapped range (2MB) is free. Can you rename I wrote such a patch initially, but ran into various problems, so I dropped it from the series. I can revisit it. Yes, please revisit it. It should be as simple as scanning for a large chunk in free_pages I suppose. 'unmapped' to 'discarded' please? That would be less confusing. Ok I can do that. Was that an approval? Ok with the rename. Thanks, Richard. -Andi
Re: [patch] Fix PR tree-optimization/49960 ,Fix self data dependence
On Mon, Oct 17, 2011 at 8:23 AM, Razya Ladelsky ra...@il.ibm.com wrote: This patch fixes the failures described in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49960 It also fixes bzips when run with autopar enabled. In both cases the self dependences are not handled correctly. In the first case, a non affine access is analyzed: in the second, the distance vector is not calculated correctly (the distance vector considered for for self dependences is always (0,0,...)) As a result, the loops get wrongfully parallelized. The patch avoids the special handling of self dependences, and analyzes all dependences in the same way. Specific adjustments and support for the self dependence cases were made. Can you elaborate on @@ -3119,8 +3135,11 @@ add_other_self_distances (struct data_dependence_r { if (DDR_NUM_SUBSCRIPTS (ddr) != 1) { - DDR_ARE_DEPENDENT (ddr) = chrec_dont_know; - return; + if (DDR_NUM_SUBSCRIPTS (ddr) != 2 || !integer_zerop (DR_ACCESS_FN (DDR_A (ddr), 1))) + { + DDR_ARE_DEPENDENT (ddr) = chrec_dont_know; + return; + } } access_fun = DR_ACCESS_FN (DDR_A (ddr), 0); ? It needed a comment before, and now so even more. The rest of the patch is ok, I suppose the above hunk is to enhance something, not to fix the bug? Thanks, Richard. Bootstrap and testsuite pass successfully for ppc64-redhat-linux. OK for trunk? Thank you, Razya ChangeLog: PR tree-optimization/49960 * tree-data-ref.c (compute_self_dependence): Remove. (initialize_data_dependence_relation): Add intializations. Remove compute_self_dependence. (add_other_self_distances): Add support for two dimensions if the second is zero. (compute_affine_dependence): Remove the !DDR_SELF_REFERENCE condition. (compute_all_dependences): Remove call to compute_self_dependence. Add call to compute_affine_dependence testsuite/ChangeLog: PR tree-optimization/49660 * gcc.dg/autopar/pr49660.c: New test. * gcc.dg/autopar/pr49660-1.c: New test.
Re: [patch, testsuite, ARM] Skip architecture option in pr42575.c
On 28 September 2011 09:48, Joey Ye joey...@arm.com wrote: 2011-09-28 Joey Ye joey...@arm.com * gcc.target/arm/pr42575.c: Remove architecture option. OK. FTR - Joey had checked that it ran ok with an optional march=armv5te in RUNTESTFLAGS . Ramana
Re: [PATCH 3/7] Emit macro expansion related diagnostics
Jason Merrill ja...@redhat.com writes: On 10/13/2011 01:12 PM, Dodji Seketeli wrote: + while (true) +{ + if (!linemap_macro_expansion_map_p (map0) + || !linemap_macro_expansion_map_p (map1) + || map0 == map1) + break; I'd put the test in the condition, but if you find it clearer this way, I guess that's fine. I've done the change. Let's go ahead and check all this in. Thanks. I'll send the final set of patches I'll commit, to ease future references. It will not contain the patch which subject was Kill pedantic warnings on system headers macros as I'll need to change the C (and probably the C++) FE to make each declspec have its own location first. -- Dodji
[patch] Update gcc.dg/vect/vect-21.c
Hi, With Jakub's patch for bool types the 3 loops in gcc.dg/vect/vect-21.c are now vectorizable on targets that support vector conditions. Tested on powerpc64-suse-linux. Committed. Ira testsuite/ChangeLog: * gcc.dg/vect/vect-21.c: Expect the loops to get vectorized on targets that support vector condition. Index: testsuite/gcc.dg/vect/vect-21.c === --- testsuite/gcc.dg/vect/vect-21.c (revision 180075) +++ testsuite/gcc.dg/vect/vect-21.c (working copy) @@ -123,7 +123,7 @@ int main (void) return main1 (); } -/* { dg-final { scan-tree-dump-times vectorized 3 loops 1 vect { xfail *-*-* } } } */ +/* { dg-final { scan-tree-dump-times vectorized 3 loops 1 vect { target vect_condition } } } */ /* { dg-final { scan-tree-dump-times Vectorizing an unaligned access 0 vect } } */ /* { dg-final { cleanup-tree-dump vect } } */
Re: [PATCH] fix -Wsign-compare error in objc-act.c (PR objc/50743)
(I don't have svn write access so I'll need someone else to commit it if it's approved.) I can apply it for you. But ... do you have a copyright assignment in place for contributions to GCC ? The patch looks small and trivial enough that I think I can apply it without a signed copyright assignment form, but if you plan on contributing more, it would make sense to sign one. :-) Thanks
[PATCH 0/6] Tracking locations of tokens resulting from macro expansion
This set of patches to track locations of tokens access macro expansion was reviewed and accepted at http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01346.html. I have bootstrapped and tested it on x86_64-unknown-linux-gnu against trunk and I am checking it in now. Thanks.
[PATCH 4/6] Support -fdebug-cpp option
This patch adds -fdebug-cpp option. When used with -E this dumps the relevant macro map before every single token. This clutters the output a lot but has proved to be invaluable in tracking some bugs during the development of the virtual location support. gcc/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * doc/cppopts.texi: Document -fdebug-cpp. * doc/invoke.texi: Add -fdebug-cpp to the list of preprocessor options. gcc/c-family/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * c.opt (fdebug-cpp): New option. * c-opts.c (c_common_handle_option): Handle the option. * c-ppoutput.c (maybe_print_line_1): New static function. Takes an output stream in parameter. Factorized from ... (maybe_print_line): ... this. Dump location debug information when -fdebug-cpp is in effect. (print_line_1): New static function. Takes an output stream in parameter. Factorized from ... (print_line): ... here. Dump location information when -fdebug-cpp is in effect. (scan_translation_unit): Dump location information when -fdebug-cpp is in effect. libcpp/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * include/cpplib.h (struct cpp_options)debug: New struct member. * include/line-map.h (linemap_dump_location): Declare ... * line-map.c (linemap_dump_location): ... new function. --- gcc/ChangeLog |7 + gcc/c-family/ChangeLog| 16 gcc/c-family/c-opts.c |4 +++ gcc/c-family/c-ppoutput.c | 57 gcc/c-family/c.opt|4 +++ gcc/doc/cppopts.texi | 13 ++ gcc/doc/invoke.texi |2 +- libcpp/ChangeLog |7 + libcpp/include/cpplib.h |4 +++ libcpp/include/line-map.h |4 +++ libcpp/line-map.c | 38 ++ 11 files changed, 144 insertions(+), 12 deletions(-) diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c index 3184539..6869d5c 100644 --- a/gcc/c-family/c-opts.c +++ b/gcc/c-family/c-opts.c @@ -628,6 +628,10 @@ c_common_handle_option (size_t scode, const char *arg, int value, cpp_opts-preprocessed = value; break; +case OPT_fdebug_cpp: + cpp_opts-debug = 1; + break; + case OPT_ftrack_macro_expansion: if (value) value = 2; diff --git a/gcc/c-family/c-ppoutput.c b/gcc/c-family/c-ppoutput.c index 892f1ea..df46ce4 100644 --- a/gcc/c-family/c-ppoutput.c +++ b/gcc/c-family/c-ppoutput.c @@ -59,7 +59,9 @@ static void account_for_newlines (const unsigned char *, size_t); static int dump_macro (cpp_reader *, cpp_hashnode *, void *); static void dump_queued_macros (cpp_reader *); +static void print_line_1 (source_location, const char*, FILE *); static void print_line (source_location, const char *); +static void maybe_print_line_1 (source_location, FILE *); static void maybe_print_line (source_location); static void do_line_change (cpp_reader *, const cpp_token *, source_location, int); @@ -243,7 +245,12 @@ scan_translation_unit (cpp_reader *pfile) in_pragma = false; } else - cpp_output_token (token, print.outf); + { + if (cpp_get_options (parse_in)-debug) + linemap_dump_location (line_table, token-src_loc, +print.outf); + cpp_output_token (token, print.outf); + } if (token-type == CPP_COMMENT) account_for_newlines (token-val.str.text, token-val.str.len); @@ -297,8 +304,9 @@ scan_translation_unit_trad (cpp_reader *pfile) /* If the token read on logical line LINE needs to be output on a different line to the current one, output the required newlines or a line marker, and return 1. Otherwise return 0. */ + static void -maybe_print_line (source_location src_loc) +maybe_print_line_1 (source_location src_loc, FILE *stream) { int src_line = LOCATION_LINE (src_loc); const char *src_file = LOCATION_FILE (src_loc); @@ -306,7 +314,7 @@ maybe_print_line (source_location src_loc) /* End the previous line of text. */ if (print.printed) { - putc ('\n', print.outf); + putc ('\n', stream); print.src_line++; print.printed = 0; } @@ -318,22 +326,37 @@ maybe_print_line (source_location src_loc) { while (src_line print.src_line) { - putc ('\n', print.outf); + putc ('\n', stream); print.src_line++; } } else -print_line (src_loc, ); +print_line_1 (src_loc, , stream); + +} + +/* If the token read on logical line LINE needs to be output on a + different line to the current one, output the required newlines or + a line marker, and return 1. Otherwise
[PATCH 6/6] Reduce memory waste due to non-power-of-2 allocs
This patch basically arranges for the allocation size of line_map buffers to be as close as possible to a power of two. This *significantly* decreases peak memory consumption as (macro) maps are numerous and stay live during all the compilation. The patch adds a new ggc_round_alloc_size interface to the ggc allocator. In each of the two main allocator implementations ('page' and 'zone') the function has been extracted from the main allocation function code and returns the actual size of the allocated memory region, thus giving a chance to the caller to maximize the amount of memory it actually uses from the allocated memory region. In the 'none' allocator implementation (that uses xmalloc) the ggc_round_alloc_size just returns the requested allocation size. gcc/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * ggc.h (ggc_round_alloc_size): Declare new public entry point. * ggc-none.c (ggc_round_alloc_size): New public stub function. * ggc-page.c (ggc_alloced_size_order_for_request): New static function. Factorized from ggc_internal_alloc_stat. (ggc_round_alloc_size): New public function. Uses ggc_alloced_size_order_for_request. (ggc_internal_alloc_stat): Use ggc_alloced_size_order_for_request. * ggc-zone.c (ggc_round_alloc_size): New public function extracted from ggc_internal_alloc_zone_stat. (ggc_internal_alloc_zone_stat): Use ggc_round_alloc_size. * toplev.c (general_init): Initialize line_table-alloced_size_for_request. libcpp/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * include/line-map.h (struct line_maps::alloced_size_for_request): New member. * line-map.c (new_linemap): Use set-alloced_size_for_request to get the actual allocated size of line maps. --- gcc/ChangeLog | 16 + gcc/ggc-none.c|9 +++ gcc/ggc-page.c| 53 +++- gcc/ggc-zone.c| 27 -- gcc/ggc.h |2 + gcc/toplev.c |1 + libcpp/ChangeLog |8 ++ libcpp/include/line-map.h |8 ++ libcpp/line-map.c | 39 - 9 files changed, 138 insertions(+), 25 deletions(-) diff --git a/gcc/ggc-none.c b/gcc/ggc-none.c index 97d25b9..e57d617 100644 --- a/gcc/ggc-none.c +++ b/gcc/ggc-none.c @@ -39,6 +39,15 @@ ggc_alloc_typed_stat (enum gt_types_enum ARG_UNUSED (gte), size_t size return xmalloc (size); } +/* For a given size of memory requested for allocation, return the + actual size that is going to be allocated. */ + +size_t +ggc_round_alloc_size (size_t requested_size) +{ + return requested_size; +} + void * ggc_internal_alloc_stat (size_t size MEM_STAT_DECL) { diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c index 624f029..f919a6b 100644 --- a/gcc/ggc-page.c +++ b/gcc/ggc-page.c @@ -1054,6 +1054,47 @@ static unsigned char size_lookup[NUM_SIZE_LOOKUP] = 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9 }; +/* For a given size of memory requested for allocation, return the + actual size that is going to be allocated, as well as the size + order. */ + +static void +ggc_round_alloc_size_1 (size_t requested_size, + size_t *size_order, + size_t *alloced_size) +{ + size_t order, object_size; + + if (requested_size NUM_SIZE_LOOKUP) +{ + order = size_lookup[requested_size]; + object_size = OBJECT_SIZE (order); +} + else +{ + order = 10; + while (requested_size (object_size = OBJECT_SIZE (order))) +order++; +} + + if (size_order) +*size_order = order; + if (alloced_size) +*alloced_size = object_size; +} + +/* For a given size of memory requested for allocation, return the + actual size that is going to be allocated. */ + +size_t +ggc_round_alloc_size (size_t requested_size) +{ + size_t size = 0; + + ggc_round_alloc_size_1 (requested_size, NULL, size); + return size; +} + /* Typed allocation function. Does nothing special in this collector. */ void * @@ -1072,17 +1113,7 @@ ggc_internal_alloc_stat (size_t size MEM_STAT_DECL) struct page_entry *entry; void *result; - if (size NUM_SIZE_LOOKUP) -{ - order = size_lookup[size]; - object_size = OBJECT_SIZE (order); -} - else -{ - order = 10; - while (size (object_size = OBJECT_SIZE (order))) - order++; -} + ggc_round_alloc_size_1 (size, order, object_size); /* If there are non-full pages for this size allocation, they are at the head of the list. */ diff --git a/gcc/ggc-zone.c b/gcc/ggc-zone.c index d0c1d79..79c8c03 100644 --- a/gcc/ggc-zone.c +++ b/gcc/ggc-zone.c @@ -1073,6 +1073,24 @@ free_chunk (char *ptr, size_t size, struct alloc_zone *zone)
[PATCH 5/6] Add line map statistics to -fmem-report output
This patch adds statistics about line maps' memory consumption and macro expansion to the output of -fmem-report. It has been useful in trying to reduce the memory consumption of the macro maps support. gcc/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * input.c (ONE_K, ONE_M, SCALE, STAT_LABEL, FORMAT_AMOUNT): New macros. (num_expanded_macros_counter, num_macro_tokens_counter): Declare new counters. (dump_line_table_statistics): Define new function. * input.h (dump_line_table_statistics): Declare new function. * toplev.c (dump_memory_report): Call dump_line_table_statistics. libcpp/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * line-map.h (struct linemap_stats): Declare new struct. (linemap_get_statistics): Declare ... * line-map.c (linemap_get_statistics): ... new function. * macro.c (num_expanded_macros_counter, num_macro_tokens_counter): Declare new counters. (enter_macro_context, replace_args): Update num_macro_tokens_counter. (cpp_get_token_1): Update num_expanded_macros_counter. --- gcc/ChangeLog | 11 + gcc/input.c | 96 + gcc/input.h |2 + gcc/toplev.c |1 + libcpp/ChangeLog | 12 ++ libcpp/include/line-map.h | 21 ++ libcpp/line-map.c | 63 + libcpp/macro.c| 29 -- 8 files changed, 231 insertions(+), 4 deletions(-) diff --git a/gcc/input.c b/gcc/input.c index 89af274..41842b7 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -46,3 +46,99 @@ expand_location (source_location loc) LRK_SPELLING_LOCATION); return xloc; } + +#define ONE_K 1024 +#define ONE_M (ONE_K * ONE_K) + +/* Display a number as an integer multiple of either: + - 1024, if said integer is = to 10 K (in base 2) + - 1024 * 1024, if said integer is = 10 M in (base 2) + */ +#define SCALE(x) ((unsigned long) ((x) 10 * ONE_K \ + ? (x) \ + : ((x) 10 * ONE_M \ +? (x) / ONE_K \ +: (x) / ONE_M))) + +/* For a given integer, display either: + - the character 'k', if the number is higher than 10 K (in base 2) + but strictly lower than 10 M (in base 2) + - the character 'M' if the number is higher than 10 M (in base2) + - the charcter ' ' if the number is strictly lower than 10 K */ +#define STAT_LABEL(x) ((x) 10 * ONE_K ? ' ' : ((x) 10 * ONE_M ? 'k' : 'M')) + +/* Display an integer amount as multiple of 1K or 1M (in base 2). + Display the correct unit (either k, M, or ' ') after the amout, as + well. */ +#define FORMAT_AMOUNT(size) SCALE (size), STAT_LABEL (size) + +/* Dump statistics to stderr about the memory usage of the line_table + set of line maps. This also displays some statistics about macro + expansion. */ + +void +dump_line_table_statistics (void) +{ + struct linemap_stats s; + size_t total_used_map_size, +macro_maps_size, +total_allocated_map_size; + + memset (s, 0, sizeof (s)); + + linemap_get_statistics (line_table, s); + + macro_maps_size = s.macro_maps_used_size ++ s.macro_maps_locations_size; + + total_allocated_map_size = s.ordinary_maps_allocated_size ++ s.macro_maps_allocated_size ++ s.macro_maps_locations_size; + + total_used_map_size = s.ordinary_maps_used_size ++ s.macro_maps_used_size ++ s.macro_maps_locations_size; + + fprintf (stderr, Number of expanded macros: %5lu\n, + s.num_expanded_macros); + if (s.num_expanded_macros != 0) +fprintf (stderr, Average number of tokens per macro expansion: %5lu\n, + s.num_macro_tokens / s.num_expanded_macros); + fprintf (stderr, + \nLine Table allocations during the + compilation process\n); + fprintf (stderr, Number of ordinary maps used:%5lu%c\n, + SCALE (s.num_ordinary_maps_used), + STAT_LABEL (s.num_ordinary_maps_used)); + fprintf (stderr, Ordinary map used size: %5lu%c\n, + SCALE (s.ordinary_maps_used_size), + STAT_LABEL (s.ordinary_maps_used_size)); + fprintf (stderr, Number of ordinary maps allocated: %5lu%c\n, + SCALE (s.num_ordinary_maps_allocated), + STAT_LABEL (s.num_ordinary_maps_allocated)); + fprintf (stderr, Ordinary maps allocated size:%5lu%c\n, + SCALE (s.ordinary_maps_allocated_size), + STAT_LABEL (s.ordinary_maps_allocated_size)); + fprintf (stderr, Number of macro maps used: %5lu%c\n, + SCALE (s.num_macro_maps_used), + STAT_LABEL (s.num_macro_maps_used)); + fprintf (stderr, Macro maps used size:%5lu%c\n, + SCALE
Re: [PATCH] fortran/50407 -- Format strings from user-defined operator or kind type string
On 10/16/2011 09:38 PM, Steve Kargl wrote: The attach patch fixes the construction of a format string from a user-defined operator or from a string with a kind type prefix. OK. Thanks for the patch. Tobias 2011-10-16 Steven G. Karglka...@gcc.gnu.org * io.c (match_dt_format): Match a user-defined operator or a kind type prefixed string. 2011-10-16 Steven G. Karglka...@gcc.gnu.org * gfortran.dg/format_string.f: New test.
Re: resent2 [PATCH] Fix ICE in redirect_jump, at jump.c:1497 PR50496
On 10/15/11 16:21, Eric Botcazou wrote: so the correct fix is very likely something like: Index: cfgrtl.c === --- cfgrtl.c(revision 179844) +++ cfgrtl.c(working copy) @@ -1024,13 +1024,20 @@ patch_jump_insn (rtx insn, rtx old_label if (!currently_expanding_to_rtl || JUMP_LABEL (insn) == old_label) { + rtx new_label; + /* If the insn doesn't go where we think, we're confused. */ gcc_assert (JUMP_LABEL (insn) == old_label); + if (new_bb == EXIT_BLOCK_PTR) + new_label = ret_rtx; + else + new_label = block_label (new_bb); + /* If the substitution doesn't succeed, die. This can happen if the back end emitted unrecognizable instructions or if target is exit block on some arches. */ - if (!redirect_jump (insn, block_label (new_bb), 0)) + if (!redirect_jump (insn, new_label, 0)) { gcc_assert (new_bb == EXIT_BLOCK_PTR); return false; Bernd, should all the callers of redirect_jump/redirect_jump_1 be audited for this pattern (there are 3 of them in cfgrtl.c for example)? I think first we'll need to find the caller and make sure it really wants a return and not a simple_return. Bernd
Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
On 10/15/2011 11:21 PM, Steve Kargl wrote: OK for trunk? 2011-10-15 Steven G. Karglka...@gcc.gnu.org * gfortran.dg/ishft_3.f90: Update test. I am not so happy with complete test replacements. How about adding it as new test case? 2011-10-15 Steven G. Karglka...@gcc.gnu.org * check.c (less_than_bitsize1): Check|shift| = bit_size(i). (gfc_check_ishftc): Check|shift| = bit_size(i) and check that size is positive. I somehow find less_than_bitsize1's + if (strncmp (arg2, ISHFT, 5) == 0) not that elegant and would prefer another argument, which tells the function that it should take the absolute value of the argument; however, given that ISHFT seems to be the only function which allows negative arguments, one could also leave it. OK with considering the two remarks. Thanks for the patch! Tobias PS: As you have probably seen, I found a related issue regarding DSHIFTL/DSHIFTR: PR 50753. Index: testsuite/gfortran.dg/ishft_3.f90 === --- testsuite/gfortran.dg/ishft_3.f90 (revision 179208) +++ testsuite/gfortran.dg/ishft_3.f90 (working copy) @@ -1,11 +1,38 @@ ! { dg-do compile } +! PR fortran/50514 program ishft_3 - integer i, j - write(*,*) ishftc( 3, 2, 3 ) - write(*,*) ishftc( 3, 2, i ) - write(*,*) ishftc( 3, i, j ) - write(*,*) ishftc( 3, 128 ) ! { dg-error exceeds BIT_SIZE of first } - write(*,*) ishftc( 3, 0, 128 ) ! { dg-error exceeds BIT_SIZE of first } - write(*,*) ishftc( 3, 0, 0 )! { dg-error Invalid third argument } - write(*,*) ishftc( 3, 3, 2 )! { dg-error exceeds third argument } + + implicit none + + integer j, m + + m = 42 + ! + ! These should compile. + ! + j = ishft(m, 16) + j = ishft(m, -16) + j = ishftc(m, 16) + j = ishftc(m, -16) + ! + ! These should issue an error. + ! + j = ishft(m, 640)! { dg-error absolute value of SHIFT } + j = ishftc(m, 640) ! { dg-error absolute value of SHIFT } + j = ishft(m, -640) ! { dg-error absolute value of SHIFT } + j = ishftc(m, -640) ! { dg-error absolute value of SHIFT } + + ! abs(SHIFT) must be= SIZE + + j = ishftc(m, 1, 2) + j = ishftc(m, 1, 2) + j = ishftc(m, -1, 2) + j = ishftc(m, -1, 2) + + j = ishftc(m, 10, 2)! { dg-error absolute value of SHIFT } + j = ishftc(m, 10, 2)! { dg-error absolute value of SHIFT } + j = ishftc(m, -10, 2)! { dg-error absolute value of SHIFT } + j = ishftc(m, -10, 2)! { dg-error absolute value of SHIFT } + + j = ishftc(m, 1, -2) ! { dg-error must be positive } end program Index: fortran/check.c === --- fortran/check.c (revision 179208) +++ fortran/check.c (working copy) @@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf { gfc_extract_int (expr2,i2); i3 = gfc_validate_kind (BT_INTEGER, expr1-ts.kind, false); + + /* For ISHFT[C],|shift| = bit_size(i). */ + if (strncmp (arg2, ISHFT, 5) == 0) + { + if (i2 0) + i2 = -i2; + + if (i2 gfc_integer_kinds[i3].bit_size) + { + gfc_error (The absolute value of SHIFT at %L must be less +than or equal to BIT_SIZE('%s'), + expr2-where, arg1); + return FAILURE; + } + } + if (or_equal) { if (i2 gfc_integer_kinds[i3].bit_size) @@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr * || type_check (shift, 1, BT_INTEGER) == FAILURE) return FAILURE; + if (less_than_bitsize1 (I, i, ISHFT, shift, true) == FAILURE) +return FAILURE; + return SUCCESS; } @@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr || type_check (shift, 1, BT_INTEGER) == FAILURE) return FAILURE; - if (size != NULL type_check (size, 2, BT_INTEGER) == FAILURE) + if (size != NULL) +{ + int i2, i3; + + if (type_check (size, 2, BT_INTEGER) == FAILURE) + return FAILURE; + + if (less_than_bitsize1 (I, i, SIZE, size, true) == FAILURE) + return FAILURE; + + gfc_extract_int (size,i3); + if (i3= 0) + { + gfc_error (SIZE at %L must be positive,size-where); + return FAILURE; + } + + gfc_extract_int (shift,i2); + if (i2 0) + i2 = -i2; + + if (i2 i3) + { + gfc_error (The absolute value of SHIFT at %L must be less than +or equal to SIZE at %L,shift-where,size-where); + return FAILURE; + } +} + else if (less_than_bitsize1 (I, i, ISHFTC, shift, true) == FAILURE) return FAILURE; return SUCCESS;
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 11:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. The original patch is ok. Thanks, Richard. Paolo.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 5:25 AM, Richard Guenther richard.guent...@gmail.com wrote: On Mon, Oct 17, 2011 at 11:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. The original patch is ok. Richard, I don't think it is.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 12:26 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. I disagree. Well, both would be an improvement. Restoring the documented default would be a move in the wrong direction though. Why have different defaults for different languages anyway? How long has the new default be in effect? Richard.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 5:28 AM, Richard Guenther richard.guent...@gmail.com wrote: On Mon, Oct 17, 2011 at 12:26 PM, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlini paolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. I disagree. Well, both would be an improvement. Restoring the documented default would be a move in the wrong direction though. Why have different defaults for different languages anyway? That is a question for the other front-ends which are not obliged to pick just about anything implemented for g++. How long has the new default be in effect? Richard.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote: Hello, So I committed the gimplify patch separate. And here is the remaining fold-const patch. The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which cover the one special-case for branching. Also tree-ssa/20040204-1.c covers tests for branching code (on targets having high-engough BRANCH_COST and no special-casing - like MIPS, S/390, and AVR. ChangeLog 2011-10-14 Kai Tietz kti...@redhat.com * fold-const.c (simple_operand_p_2): New function. (fold_truthop): Rename to (fold_truth_andor_1): function name. Additionally remove branching creation for logical and/or. (fold_truth_andor): Handle branching creation for logical and/or here. Bootstrapped and regression-tested for all languages plus Ada and Obj-C++ on x86_64-pc-linux-gnu. Ok for apply? Ok with ... Regards, Kai Index: gcc/gcc/fold-const.c === --- gcc.orig/gcc/fold-const.c +++ gcc/gcc/fold-const.c @@ -112,13 +112,13 @@ static tree decode_field_reference (loca static int all_ones_mask_p (const_tree, int); static tree sign_bit_p (tree, const_tree); static int simple_operand_p (const_tree); +static bool simple_operand_p_2 (tree); static tree range_binop (enum tree_code, tree, tree, int, tree, int); static tree range_predecessor (tree); static tree range_successor (tree); static tree fold_range_test (location_t, enum tree_code, tree, tree, tree); static tree fold_cond_expr_with_comparison (location_t, tree, tree, tree, tree); static tree unextend (tree, int, int, tree); -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); static tree optimize_minmax_comparison (location_t, enum tree_code, tree, tree, tree); static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l return lhs; } -/* Subroutine for fold_truthop: decode a field reference. +/* Subroutine for fold_truth_andor_1: decode a field reference. If EXP is a comparison reference, we return the innermost reference. @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val) return NULL_TREE; } -/* Subroutine for fold_truthop: determine if an operand is simple enough +/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough to be evaluated unconditionally. */ static int @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp) STRIP_NOPS (exp); return (CONSTANT_CLASS_P (exp) - || TREE_CODE (exp) == SSA_NAME + || TREE_CODE (exp) == SSA_NAME || (DECL_P (exp) ! TREE_ADDRESSABLE (exp) ! TREE_THIS_VOLATILE (exp) @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp) registers aren't expensive. */ (! TREE_STATIC (exp) || DECL_REGISTER (exp; } + +/* Subroutine for fold_truth_andor: determine if an operand is simple enough + to be evaluated unconditionally. + I addition to simple_operand_p, we assume that comparisons and logic-not + operations are simple, if their operands are simple, too. */ + +static bool +simple_operand_p_2 (tree exp) +{ + enum tree_code code; + + /* Strip any conversions that don't change the machine mode. */ + STRIP_NOPS (exp); + + code = TREE_CODE (exp); + + if (TREE_CODE_CLASS (code) == tcc_comparison) + return (!tree_could_trap_p (exp) + simple_operand_p_2 (TREE_OPERAND (exp, 0)) + simple_operand_p_2 (TREE_OPERAND (exp, 1))); recurse with simple_operand_p. + + if (TREE_SIDE_EFFECTS (exp) + || tree_could_trap_p (exp)) Move this check before the tcc_comparison check and remove the then redundant tree_could_trap_p check there. + return false; + + switch (code) + { + case SSA_NAME: + return true; Do not handle here, it's handled in simple_operand_p. + case TRUTH_NOT_EXPR: + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + case BIT_NOT_EXPR: + if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE) + return false; Remove the BIT_NOT_EXPR handling. Thus, simply change this switch to if (code == TRUTH_NOT_EXPR) return simple_operand_p_2 (TREE_OPERAND (exp, 0)); return simple_operand_p (exp); + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + default: + return simple_operand_p (exp); + } +} + /* The following functions are subroutines to fold_range_test and allow it to try to change a logical combination of comparisons into a range test. @@ -4888,7 +4928,7 @@ fold_range_test (location_t loc, enum tr return 0; } -/* Subroutine for fold_truthop: C is an INTEGER_CST interpreted as a P +/* Subroutine for fold_truth_andor_1: C is an INTEGER_CST interpreted as a P bit value. Arrange things
Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code
Of course you can commit to 4.6. Ok. Btw, my patch had a small regression (PR50752), which was very quickly reported by Joost. I have just committed the obvious fix as r180079. Be quick, though; 4.6.2 was due for release now-ish - I know. I'll try to do it today. Cheers, Janus On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi Paul, This is OK for trunk. Thanks fo rthe patch. Thanks. Committed to trunk as r180062. What about 4.6? Cheers, Janus On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi all, here is a patch which fixes the regression in comment #2 of the PR in the subject line. What it does is setting the 'ts.is_c_interop' flag correctly for constants with kind-parameter specification (such as '0.0_c_double'), as is already being done for variables. Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6? Cheers, Janus 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * primary.c (match_kind_param): Detect ISO_C_BINDING kinds. (get_kind): Pass on 'is_iso_c' flag. (match_integer_constant,match_real_constant,match_logical_constant): Set 'ts.is_c_interop'. 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * gfortran.dg/c_kind_tests_3.f03: New. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [C++ Patch] PR 32614
On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. Well either my English is even weaker than I thought, or restoring doesn't apply here: the line of code at issue, pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, and nothing similar with 72 as second argument existed before. Paolo.
Re: [PATCH 3/6] Emit macro expansion related diagnostics
On Mon, Oct 17, 2011 at 11:57 AM, Dodji Seketeli do...@redhat.com wrote: In this third instalment the diagnostic machinery -- when faced with the virtual location of a token resulting from macro expansion -- uses the new linemap APIs to unwind the stack of macro expansions that led to that token and emits a [hopefully] more useful message than what we have today. diagnostic_report_current_module has been slightly changed to use the location given by client code instead of the global input_location variable. This results in more precise diagnostic locations in general but then the patch adjusts some C++ tests which output changed as a result of this. Three new regression tests have been added. The mandatory screenshot goes like this: [dodji@adjoa gcc]$ cat -n test.c 1 #define OPERATE(OPRD1, OPRT, OPRD2) \ 2 OPRD1 OPRT OPRD2; 3 4 #define SHIFTL(A,B) \ 5 OPERATE (A,,B) 6 7 #define MULT(A) \ 8 SHIFTL (A,1) 9 10 void 11 g () 12 { 13 MULT (1.0);/* 1.0 1; -- so this is an error. */ 14 } [dodji@adjoa gcc]$ ./cc1 -quiet -ftrack-macro-expansion test.c test.c: In function 'g': test.c:5:14: erreur: invalid operands to binary (have 'double' and 'int') test.c:2:9: note: in expansion of macro 'OPERATE' test.c:5:3: note: expanded from here test.c:5:14: note: in expansion of macro 'SHIFTL' test.c:8:3: note: expanded from here test.c:8:3: note: in expansion of macro 'MULT2' test.c:13:3: note: expanded from here This broke bootstrap on x86_64-linux. /space/rguenther/src/svn/trunk/libcpp/line-map.c: In function 'source_location linemap_macro_map_loc_to_exp_point(const line_map*, source_location)': /space/rguenther/src/svn/trunk/libcpp/line-map.c:628:12: error: variable 'token_no' set but not used [-Werror=unused-but-set-variable] cc1plus: all warnings being treated as errors make[3]: *** [line-map.o] Error 1 gcc/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * gcc/diagnostic.h (diagnostic_report_current_module): Add a location parameter. * diagnostic.c (diagnostic_report_current_module): Add a location parameter to the function definition. Use it instead of input_location. Resolve the virtual location rather than just looking up its map and risking to touch a resulting macro map. (default_diagnostic_starter): Pass the relevant diagnostic location to diagnostic_report_current_module. * tree-diagnostic.c (maybe_unwind_expanded_macro_loc): New. (virt_loc_aware_diagnostic_finalizer): Likewise. (diagnostic_report_current_function): Pass the relevant location to diagnostic_report_current_module. * tree-diagnostic.h (virt_loc_aware_diagnostic_finalizer): Declare new function. * toplev.c (general_init): By default, use the new virt_loc_aware_diagnostic_finalizer as diagnostic finalizer. * Makefile.in: Add vec.h dependency to tree-diagnostic.c. gcc/cp/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * error.c (cp_diagnostic_starter): Pass the relevant location to diagnostic_report_current_module. (cp_diagnostic_finalizer): Call virt_loc_aware_diagnostic_finalizer. gcc/testsuite/ChangeLog 2011-10-15 Tom Tromey tro...@redhat.com Dodji Seketeli do...@redhat.com * lib/prune.exp (prune_gcc_output): Prune output referring to included files. * gcc.dg/cpp/macro-exp-tracking-1.c: New test. * gcc.dg/cpp/macro-exp-tracking-2.c: Likewise. * gcc.dg/cpp/macro-exp-tracking-3.c: Likewise. * gcc.dg/cpp/pragma-diagnostic-2.c: Likewise. --- gcc/ChangeLog | 21 +++ gcc/Makefile.in | 3 +- gcc/cp/ChangeLog | 7 + gcc/cp/error.c | 5 +- gcc/diagnostic.c | 13 +- gcc/diagnostic.h | 2 +- gcc/testsuite/ChangeLog | 10 ++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c | 21 +++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-2.c | 21 +++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-3.c | 14 ++ gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-4.c | 14 ++ gcc/testsuite/gcc.dg/cpp/pragma-diagnostic-2.c | 34 + gcc/testsuite/lib/prune.exp | 1 + gcc/toplev.c | 3 + gcc/tree-diagnostic.c | 182 ++- gcc/tree-diagnostic.h | 3 +- 16 files changed, 343 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/macro-exp-tracking-1.c create mode 100644
Re: [PATCH] Fix pr50717: widening multiply bad code
On Sat, Oct 15, 2011 at 11:27 PM, Andrew Stubbs a...@codesourcery.com wrote: This patch fixes a bug in which the widening multiply-and-accumulate optimization failed to take the intermediate types into account. The effect of this is that the compiler would do what the programmer expected to happen, rather than what the C standard requires to happen (in many cases), so obviously this needed fixing. :) It still needs to optimize the cases where the optimization doesn't actually change anything (because it's known that overflow cannot occur), so I don't want to completely disallow extends between multiply and plus, and I believe this patch achieves this. OK? Ok. THanks, Richard. Andrew
Re: resent2 [PATCH] Fix ICE in redirect_jump, at jump.c:1497 PR50496
On 10/15/11 16:21, Eric Botcazou wrote: PR middle-end/50496 * cfgrtl.c (try_redirect_by_replacing_jump): Treat EXIT_BLOCK_PTR case separately before call to redirect_jump(). Add assertion. (patch_jump_insn): Same. This will definitely disable redirections to the exit block. Yes. However, in the case that caused the PR, this was attempted with reload_completed == 0, so we cannot generate return patterns anyway and must fail. I think the original patch is OK. Bernd
Re: [PR50672, PATCH] Fix ice triggered by -ftree-tail-merge: verify_ssa failed: no immediate_use list
On Sun, Oct 16, 2011 at 12:05 PM, Tom de Vries tom_devr...@mentor.com wrote: On 10/14/2011 12:00 PM, Richard Guenther wrote: On Fri, Oct 14, 2011 at 1:12 AM, Tom de Vries tom_devr...@mentor.com wrote: On 10/12/2011 02:19 PM, Richard Guenther wrote: On Wed, Oct 12, 2011 at 8:35 AM, Tom de Vries vr...@codesourcery.com wrote: Richard, I have a patch for PR50672. When compiling the testcase from the PR with -ftree-tail-merge, the scenario is as follows: We start out tail_merge_optimize with blocks 14 and 20, which are alike, but not equal, since they have different successors: ... # BLOCK 14 freq:690 # PRED: 25 [61.0%] (false,exec) if (wD.2197_57(D) != 0B) goto bb 15; else goto bb 16; # SUCC: 15 [78.4%] (true,exec) 16 [21.6%] (false,exec) # BLOCK 20 freq:2900 # PRED: 29 [100.0%] (fallthru) 31 [100.0%] (fallthru) # .MEMD.2447_209 = PHI .MEMD.2447_125(29), .MEMD.2447_129(31) if (wD.2197_57(D) != 0B) goto bb 5; else goto bb 6; # SUCC: 5 [85.0%] (true,exec) 6 [15.0%] (false,exec) ... In the first iteration, we merge block 5 with block 15 and block 6 with block 16. After that, the blocks 14 and 20 are equal. In the second iteration, the blocks 14 and 20 are merged, by redirecting the incoming edges of block 20 to block 14, and removing block 20. Block 20 also contains the definition of .MEMD.2447_209. Removing the definition delinks the vuse of .MEMD.2447_209 in block 5: ... # BLOCK 5 freq:6036 # PRED: 20 [85.0%] (true,exec) # PT = nonlocal escaped D.2306_58 = thisD.2200_10(D)-D.2156; # .MEMD.2447_132 = VDEF .MEMD.2447_209 # USE = anything # CLB = anything drawLineD.2135 (D.2306_58, wD.2197_57(D), gcD.2198_59(D)); goto bb 17; # SUCC: 17 [100.0%] (fallthru,exec) ... And block 5 is retained and block 15 is discarded? Indeed. After the pass, when executing the TODO_update_ssa_only_virtuals, we update the drawLine call in block 5 using rewrite_update_stmt, which calls maybe_replace_use for the vuse operand. However, maybe_replace_use doesn't have an effect since the old vuse and the new vuse happen to be the same (rdef == use), so SET_USE is not called and the vuse remains delinked: ... if (rdef rdef != use) SET_USE (use_p, rdef); ... The patch fixes this by forcing SET_USE for delinked uses. That isn't the correct fix. Whoever unlinks the vuse (by removing its definition) has to replace it with something valid, which is either the bare symbol .MEM, or the VUSE associated with the removed VDEF (thus, as unlink_stmt_vdef does). Another try. For each deleted bb, we call unlink_stmt_vdef for the statements, and replace the .MEM phi uses with the bare .MEM symbol. Bootstrapped and reg-tested on x86_64. Ok for trunk? Better. For + + FOR_EACH_IMM_USE_STMT (use_stmt, iter, res) + { + FOR_EACH_IMM_USE_ON_STMT (use_p, iter) + SET_USE (use_p, SSA_NAME_VAR (res)); + } you can use mark_virtual_phi_result_for_renaming (phi) instead. + for (i = gsi_last_bb (bb); !gsi_end_p (i); gsi_prev_nondebug (i)) + unlink_stmt_vdef (gsi_stmt (i)); is that actually necessary? That is, isn't the block that follows a deleted block always starting with a virtual PHI? Block 20 is deleted. Block 5 follows the deleted block 20. Block 5 does not start with a virtual PHI. If not it should be enough to walk to the first stmt that uses a virtual operand and similar to the PHI case replace all its uses with the bare symbol. I think we need to handle the exposed uses (meaning outside the block) of vdefs in each deleted block. The only vdef that can have exposed uses is the last vdef in the block. So we should find the last vdef (gimple with gimple_vdef or virtual PHI) and handle the related uses. Bootstrapped and regtested on x86_64. OK for trunk? Hmmm. I can see that this will fail when the block has a store but not a virtual PHI node, thus, when renaming does not reach a bare .MEM symbol walking the use-def chain from the VUSE of the store. What release_last_vdef should do is trigger a rename of subsequent VUSEs in successors of the deleted block. Which requires you to do something like static void rename_last_vdef (basic_block bb) { + gimple_stmt_iterator i; + + for (i = gsi_last_bb (bb); !gsi_end_p (i); gsi_prev_nondebug (i)) +{ + gimple stmt = gsi_stmt (i); + if (gimple_vdef (stmt) == NULL_TREE) + continue; + + mark_virtual_operand_for_renaming (gimple_vdef (stmt)); return; } + for (i = gsi_start_phis (bb); !gsi_end_p (i); gsi_next (i)) +{ + gimple phi = gsi_stmt (i); + tree res = gimple_phi_result (phi); + + if (is_gimple_reg (res)) + continue; + + mark_virtual_phi_result_for_renaming (phi); + return; +} } And split out the result_var = SSA_NAME_VAR (result_ssa); FOR_EACH_IMM_USE_STMT (stmt, iter, result_ssa) {
Re: [C++ Patch] PR 32614
On Mon, 17 Oct 2011, Paolo Carlini wrote: On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. Well either my English is even weaker than I thought, or restoring doesn't apply here: the line of code at issue, pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, and nothing similar with 72 as second argument existed before. Thus clearly the documentation is wrong ;) So, I'll approve the patch for the release branches (where we don't want to change the default). We can bikeshed a bit more for trunk. Richard.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 5:53 AM, Richard Guenther rguent...@suse.de wrote: On Mon, 17 Oct 2011, Paolo Carlini wrote: On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. Well either my English is even weaker than I thought, or restoring doesn't apply here: the line of code at issue, pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, and nothing similar with 72 as second argument existed before. Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. So, I'll approve the patch for the release branches (where we don't want to change the default). We can bikeshed a bit more for trunk. Richard.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
2011/10/17 Richard Guenther richard.guent...@gmail.com: On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote: Hello, So I committed the gimplify patch separate. And here is the remaining fold-const patch. The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which cover the one special-case for branching. Also tree-ssa/20040204-1.c covers tests for branching code (on targets having high-engough BRANCH_COST and no special-casing - like MIPS, S/390, and AVR. ChangeLog 2011-10-14 Kai Tietz kti...@redhat.com * fold-const.c (simple_operand_p_2): New function. (fold_truthop): Rename to (fold_truth_andor_1): function name. Additionally remove branching creation for logical and/or. (fold_truth_andor): Handle branching creation for logical and/or here. Bootstrapped and regression-tested for all languages plus Ada and Obj-C++ on x86_64-pc-linux-gnu. Ok for apply? Ok with ... Regards, Kai Index: gcc/gcc/fold-const.c === --- gcc.orig/gcc/fold-const.c +++ gcc/gcc/fold-const.c @@ -112,13 +112,13 @@ static tree decode_field_reference (loca static int all_ones_mask_p (const_tree, int); static tree sign_bit_p (tree, const_tree); static int simple_operand_p (const_tree); +static bool simple_operand_p_2 (tree); static tree range_binop (enum tree_code, tree, tree, int, tree, int); static tree range_predecessor (tree); static tree range_successor (tree); static tree fold_range_test (location_t, enum tree_code, tree, tree, tree); static tree fold_cond_expr_with_comparison (location_t, tree, tree, tree, tree); static tree unextend (tree, int, int, tree); -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); static tree optimize_minmax_comparison (location_t, enum tree_code, tree, tree, tree); static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l return lhs; } -/* Subroutine for fold_truthop: decode a field reference. +/* Subroutine for fold_truth_andor_1: decode a field reference. If EXP is a comparison reference, we return the innermost reference. @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val) return NULL_TREE; } -/* Subroutine for fold_truthop: determine if an operand is simple enough +/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough to be evaluated unconditionally. */ static int @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp) STRIP_NOPS (exp); return (CONSTANT_CLASS_P (exp) - || TREE_CODE (exp) == SSA_NAME + || TREE_CODE (exp) == SSA_NAME || (DECL_P (exp) ! TREE_ADDRESSABLE (exp) ! TREE_THIS_VOLATILE (exp) @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp) registers aren't expensive. */ (! TREE_STATIC (exp) || DECL_REGISTER (exp; } + +/* Subroutine for fold_truth_andor: determine if an operand is simple enough + to be evaluated unconditionally. + I addition to simple_operand_p, we assume that comparisons and logic-not + operations are simple, if their operands are simple, too. */ + +static bool +simple_operand_p_2 (tree exp) +{ + enum tree_code code; + + /* Strip any conversions that don't change the machine mode. */ + STRIP_NOPS (exp); + + code = TREE_CODE (exp); + + if (TREE_CODE_CLASS (code) == tcc_comparison) + return (!tree_could_trap_p (exp) + simple_operand_p_2 (TREE_OPERAND (exp, 0)) + simple_operand_p_2 (TREE_OPERAND (exp, 1))); recurse with simple_operand_p. No, as this again would reject simple operations and additionally wouldn't check for trapping. + + if (TREE_SIDE_EFFECTS (exp) + || tree_could_trap_p (exp)) Move this check before the tcc_comparison check and remove the then redundant tree_could_trap_p check there. Ok + return false; + + switch (code) + { + case SSA_NAME: + return true; Do not handle here, it's handled in simple_operand_p. Well, was more a short-cut here. + case TRUTH_NOT_EXPR: + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + case BIT_NOT_EXPR: + if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE) + return false; Remove the BIT_NOT_EXPR handling. Thus, simply change this switch to Why should we reject simple ~X operations from gimplified code here? I admit that from FE-code we won't see that, as always an integer-cast is done for foo (_Bool x) { ... if (~x) ... }, but from gimplified-code this is the general description of an boolean-typed != 0? if (code == TRUTH_NOT_EXPR) return simple_operand_p_2 (TREE_OPERAND (exp, 0)); return simple_operand_p (exp); + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + default: +
Re: [patch] C6X unwinding/exception handling
On 10/10/2011 02:23 PM, Paul Brook wrote: Index: libjava/exception.cc === --- libjava/exception.cc (revision 179739) +++ libjava/exception.cc (working copy) @@ -135,6 +135,7 @@ { _Unwind_Ptr Start; _Unwind_Ptr LPStart; + _Unwind_Ptr ttype_base; const unsigned char *TType; const unsigned char *action_table; unsigned char ttype_encoding; @@ -184,7 +185,7 @@ _Unwind_Ptr ptr; ptr = (_Unwind_Ptr) (info-TType - (i * 4)); - ptr = _Unwind_decode_target2(ptr); + ptr = _Unwind_decode_typeinfo_ptr (info-ttype_base, (_Unwind_Word) ptr); return reinterpret_castvoid **(ptr); } @@ -325,6 +326,7 @@ // Parse the LSDA header. p = parse_lsda_header (context, language_specific_data, info); + info.ttype_base = base_of_encoded_value (info.ttype_encoding, context); #ifdef HAVE_GETIPINFO ip = _Unwind_GetIPInfo (context, ip_before_insn); #else No. The purpose of my patch was to remove the arm specific code. The only difference I can see in this bit of code is that libstdc++ uses base_of_encoded_value/read_encoded_value_with_base whereas libjava uses context/read_encoded_value. I expect you want something like the patch below (completely untested). I checked the attached patch, test results at http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html which are the same as with my suggested patch. Ok for the trunk? Matthias libjava/ 2011-10-17 Paul Brook p...@codesourcery.com * exception.cc (parse_lsda_header): hardcode ttype_encoding for older ARM EABI toolchains. (get_ttype_entry) Remove __ARM_EABI_UNWINDER__ variant. libobjc/ 2011-10-17 Paul Brook p...@codesourcery.com Matthias Klose d...@ubuntu.com * exception.c (parse_lsda_header): hardcode ttype_encoding for older ARM EABI toolchains. (get_ttype_entry) Remove __ARM_EABI_UNWINDER__ variant. Index: libjava/exception.cc === --- libjava/exception.cc(revision 180086) +++ libjava/exception.cc(working copy) @@ -161,6 +161,11 @@ info-ttype_encoding = *p++; if (info-ttype_encoding != DW_EH_PE_omit) { +#if _GLIBCXX_OVERRIDE_TTYPE_ENCODING + /* Older ARM EABI toolchains set this value incorrectly, so use a +hardcoded OS-specific format. */ + info-ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING; +#endif p = read_uleb128 (p, tmp); info-TType = p + tmp; } @@ -176,22 +181,7 @@ return p; } -#ifdef __ARM_EABI_UNWINDER__ - static void ** -get_ttype_entry(_Unwind_Context *, lsda_header_info* info, _uleb128_t i) -{ - _Unwind_Ptr ptr; - - ptr = (_Unwind_Ptr) (info-TType - (i * 4)); - ptr = _Unwind_decode_target2(ptr); - - return reinterpret_castvoid **(ptr); -} - -#else - -static void ** get_ttype_entry (_Unwind_Context *context, lsda_header_info *info, long i) { _Unwind_Ptr ptr; @@ -202,8 +192,6 @@ return reinterpret_castvoid **(ptr); } -#endif - // Using a different personality function name causes link failures // when trying to mix code using different exception handling models. #ifdef SJLJ_EXCEPTIONS Index: libobjc/exception.c === --- libobjc/exception.c (revision 180086) +++ libobjc/exception.c (working copy) @@ -159,6 +159,11 @@ info-ttype_encoding = *p++; if (info-ttype_encoding != DW_EH_PE_omit) { +#if _GLIBCXX_OVERRIDE_TTYPE_ENCODING + /* Older ARM EABI toolchains set this value incorrectly, so use a +hardcoded OS-specific format. */ + info-ttype_encoding = _GLIBCXX_OVERRIDE_TTYPE_ENCODING; +#endif p = read_uleb128 (p, tmp); info-TType = p + tmp; } @@ -174,27 +179,7 @@ return p; } -#ifdef __ARM_EABI_UNWINDER__ - static Class -get_ttype_entry (struct lsda_header_info *info, _uleb128_t i) -{ - _Unwind_Ptr ptr; - - ptr = (_Unwind_Ptr) (info-TType - (i * 4)); - ptr = _Unwind_decode_target2 (ptr); - - /* NULL ptr means catch-all. Note that if the class is not found, - this will abort the program. */ - if (ptr) -return objc_getRequiredClass ((const char *) ptr); - else -return 0; -} - -#else - -static Class get_ttype_entry (struct lsda_header_info *info, _Unwind_Word i) { _Unwind_Ptr ptr; @@ -211,8 +196,6 @@ return 0; } -#endif - /* Using a different personality function name causes link failures when trying to mix code using different exception handling models. */
Re: [patch RFC,PR50038]
Ping. Could please someone check if my approach is OK and it is worth to continue work on patch for PR50038? Thanks Ilya 2011/10/11 Ilya Enkovich enkovich@gmail.com: 2011/10/4 Richard Henderson r...@redhat.com: On 10/04/2011 08:42 AM, Joseph S. Myers wrote: On Tue, 4 Oct 2011, Ilya Tocar wrote: Hi everyone, This patch fixes PR 50038 (redundant zero extensions) by modifying implicit-zee pass to also remove unneeded zero extensions from QImode to SImode. Hardcoding particular modes like this in the target-independent parts of the compiler is fundamentally ill-conceived. Right now it hardcodes the (SImode, DImode) pair. You're adding hardcoding of (QImode, SImode) as well. But really it should consider all pairs of (integer mode, wider integer mode), with the machine description (or target hooks) determining which pairs are relevant on a particular target. Changing it not to hardcode particular modes would be better than adding a second pair. That along with not hard-coding ZERO_EXTEND. Both MIPS and Alpha have much the same free operations, but with SIGN_EXTEND. I remember rejecting one iteration of this pass with this hard-coded, but the pass was apparently approved by someone else without that being corrected. r~ Hello guys, Could you please look at my patch version? I tried to remove all unnecessary mode restrictions and cover SIGN_EXTEND case. I did not test this patch yet, just checked it worked on reproducer from PR50038. Thanks Ilya --- gcc/ * implicit-zee.c (ext_cand): New. (ext_cand_pool): Likewise. (add_ext_candidate): Likewise. (zee_init): Likewise. (zee_cleanup): Likewise. (combine_set_zero_extend): Get extend candidate as new parameter. Now handle sign extend cases and all modes. (transform_ifelse): Likewise. (merge_def_and_ze): Likewise. (combine_reaching_defs): Change parameter type. (zero_extend_info): Changed insn_list type. (add_removable_zero_extend): Relaxed mode and code filter. (find_removable_zero_extends): Changed return type. (find_and_remove_ze): Var type changes. (rest_of_handle_zee): Initialization and cleanup added.
Re: [C++ Patch] PR 32614
On Mon, 17 Oct 2011, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 5:53 AM, Richard Guenther rguent...@suse.de wrote: On Mon, 17 Oct 2011, Paolo Carlini wrote: On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. Well either my English is even weaker than I thought, or restoring doesn't apply here: the line of code at issue, pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, and nothing similar with 72 as second argument existed before. Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. The initial patch, split between rev. 31343 and 31999 indeed added + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. I still think automatic wrapping (at 72 columns!? A terminal is 80x24!) should not be done by default. You probably will break a lot of existing scripts that assume the default of zero. Richard.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 5:38 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/17/2011 12:26 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 4:42 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: FWIW, I still believe that tweaking the documentation to match the long standing reality, would be a small improvement. I'm not going to further insist, anyway. It isn't improvement. The improvement would be to restore the documented default. Well either my English is even weaker than I thought, or restoring doesn't apply here: the line of code at issue, pp_set_line_maximum_length (pp, 0), has been added by Gaby in Rev 70777, and nothing similar with 72 as second argument existed before. Looking at the changset, now I remember: That line was part of a change set that was improving the *C* pretty-printer I added earlier and to maximize sharing more cose between the C and C++ pretty printers. The zero length was added as an attempt to respect the *C* front-end desire not have line wrapping. Richard talked about other front-ends, but at the time, there was only two front-ends who were using the pretty printers: C and C++. The C front-end was adopting bits of the C++ front-end. Every other front-end were doing whatever they wanted. It wasn't like there was a bit debate with other front-ends to decide what the default should be for all.
Re: [C++ Patch] PR 32614
On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote: Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. Indeed, as far as I can see, you added that line while *preserving* the existing behavior and preparing the C++ variant of the pretty_print machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking, there is nothing to *restore*. But I may be wrong, I don't own viewcvs, I just quickly queried it. Paolo.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote: The initial patch, split between rev. 31343 and 31999 indeed added Thanks for helping tracking history. + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. Yes, most likely. I still think automatic wrapping (at 72 columns!? A terminal is 80x24!) should not be done by default. The choice of 72 at the time was based on the 80 of the terminal and Emacs guidelines. The requests I have heared most vocally was to increase the length of line, not to suppress the wrapping by default. You probably will break a lot of existing scripts that assume the default of zero. Richard.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote: Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. Indeed, as far as I can see, you added that line while *preserving* the existing behavior and preparing the C++ variant of the pretty_print machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking, there is nothing to *restore*. I do not know what you mean by there is nothing to restore. Look at the other mail by Richard. The C pretty-printer *post*-dated the C++ pretty printer. But I may be wrong, I don't own viewcvs, I just quickly queried it. Paolo.
Re: [C++ Patch] PR 32614
On 10/17/2011 01:08 PM, Richard Guenther wrote: The initial patch, split between rev. 31343 and 31999 indeed added + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. Wow, 31343, I didn't look back so much. Now the issue is whether, after *so* many years with 0, people would *really* consider seeing the default changed to 72, at variance with C and all the other front-ends. I seriously doubt it. Do we have strong evidence of that? For example, is there something similar in other C++ front-end? Paolo.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote: 2011/10/17 Richard Guenther richard.guent...@gmail.com: On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote: Hello, So I committed the gimplify patch separate. And here is the remaining fold-const patch. The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which cover the one special-case for branching. Also tree-ssa/20040204-1.c covers tests for branching code (on targets having high-engough BRANCH_COST and no special-casing - like MIPS, S/390, and AVR. ChangeLog 2011-10-14 Kai Tietz kti...@redhat.com * fold-const.c (simple_operand_p_2): New function. (fold_truthop): Rename to (fold_truth_andor_1): function name. Additionally remove branching creation for logical and/or. (fold_truth_andor): Handle branching creation for logical and/or here. Bootstrapped and regression-tested for all languages plus Ada and Obj-C++ on x86_64-pc-linux-gnu. Ok for apply? Ok with ... Regards, Kai Index: gcc/gcc/fold-const.c === --- gcc.orig/gcc/fold-const.c +++ gcc/gcc/fold-const.c @@ -112,13 +112,13 @@ static tree decode_field_reference (loca static int all_ones_mask_p (const_tree, int); static tree sign_bit_p (tree, const_tree); static int simple_operand_p (const_tree); +static bool simple_operand_p_2 (tree); static tree range_binop (enum tree_code, tree, tree, int, tree, int); static tree range_predecessor (tree); static tree range_successor (tree); static tree fold_range_test (location_t, enum tree_code, tree, tree, tree); static tree fold_cond_expr_with_comparison (location_t, tree, tree, tree, tree); static tree unextend (tree, int, int, tree); -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); static tree optimize_minmax_comparison (location_t, enum tree_code, tree, tree, tree); static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l return lhs; } -/* Subroutine for fold_truthop: decode a field reference. +/* Subroutine for fold_truth_andor_1: decode a field reference. If EXP is a comparison reference, we return the innermost reference. @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val) return NULL_TREE; } -/* Subroutine for fold_truthop: determine if an operand is simple enough +/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough to be evaluated unconditionally. */ static int @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp) STRIP_NOPS (exp); return (CONSTANT_CLASS_P (exp) - || TREE_CODE (exp) == SSA_NAME + || TREE_CODE (exp) == SSA_NAME || (DECL_P (exp) ! TREE_ADDRESSABLE (exp) ! TREE_THIS_VOLATILE (exp) @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp) registers aren't expensive. */ (! TREE_STATIC (exp) || DECL_REGISTER (exp; } + +/* Subroutine for fold_truth_andor: determine if an operand is simple enough + to be evaluated unconditionally. + I addition to simple_operand_p, we assume that comparisons and logic-not + operations are simple, if their operands are simple, too. */ + +static bool +simple_operand_p_2 (tree exp) +{ + enum tree_code code; + + /* Strip any conversions that don't change the machine mode. */ + STRIP_NOPS (exp); + + code = TREE_CODE (exp); + + if (TREE_CODE_CLASS (code) == tcc_comparison) + return (!tree_could_trap_p (exp) + simple_operand_p_2 (TREE_OPERAND (exp, 0)) + simple_operand_p_2 (TREE_OPERAND (exp, 1))); recurse with simple_operand_p. No, as this again would reject simple operations and additionally wouldn't check for trapping. ? Your code allows arbitrarily complex expressions. Also tree_could_trap_p obviously extents to operands. + + if (TREE_SIDE_EFFECTS (exp) + || tree_could_trap_p (exp)) Move this check before the tcc_comparison check and remove the then redundant tree_could_trap_p check there. Ok + return false; + + switch (code) + { + case SSA_NAME: + return true; Do not handle here, it's handled in simple_operand_p. Well, was more a short-cut here. + case TRUTH_NOT_EXPR: + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + case BIT_NOT_EXPR: + if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE) + return false; Remove the BIT_NOT_EXPR handling. Thus, simply change this switch to Why should we reject simple ~X operations from gimplified code here? Because this is FE triggered code. From gimple you won't ever see such complex expressions (never even the TRUTH_AND*_EXPR variants). I admit that from FE-code we won't see that, as always an
Re: [C++ Patch] PR 32614
On 10/17/2011 01:16 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote: Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. Indeed, as far as I can see, you added that line while *preserving* the existing behavior and preparing the C++ variant of the pretty_print machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking, there is nothing to *restore*. I do not know what you mean by there is nothing to restore. Look at the other mail by Richard. The C pretty-printer *post*-dated the C++ pretty printer. Hey, I don't own viewcvs, of svn, for that matter, you could also dare to help a bit with this crazy archeological task, can't you?!? I looked back only untile 70777, and that point and a bit earlier there where already no 72s around, thus, right *nothing to restore*. Now we are learning that *even earlier* we had a 72. Fine. Now, after so many years, are we really sure that our users would consider an *improvement* a 72? I'm honestly not sure at all. Again, what the best C++ front-ends around do, by default? I'm sincerely curious. Paolo.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:14 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/17/2011 01:08 PM, Richard Guenther wrote: The initial patch, split between rev. 31343 and 31999 indeed added + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. Wow, 31343, I didn't look back so much. I never understood why you doubted that the default was 72 way before I added the C pretty printer. I can understand you don't like it; but that should not involve doubting the facts. Now the issue is whether, after *so* many years with 0, people would *really* consider seeing the default changed to 72, at variance with C and all the other front-ends. Again this argument is making a sort of revisionism. The 72 default was added to g++, and other front-ends (in reality at the time, only C could be affected) decided not to. Over the years, we have moved to share more and more codes with other front-ends. The fact that other front-ends have been using more and more of the code that was designed for g++ should not be argument to change the default for g++. It should be other reasons. I seriously doubt it. Do we have strong evidence of that? For example, is there something similar in other C++ front-end? Paolo.
Re: [C++ Patch] PR 32614
On 10/17/2011 01:24 PM, Gabriel Dos Reis wrote: Again this argument is making a sort of revisionism. The 72 default was added to g++, and other front-ends (in reality at the time, only C could be affected) decided not to. Over the years, we have moved to share more and more codes with other front-ends. The fact that other front-ends have been using more and more of the code that was designed for g++ should not be argument to change the default for g++. It should be other reasons. At this point, after something like 10 years, I think we badly need other reasons to change the C++ default. You are arguing as if we just inadvertently changed the C++ default yesterday. If I were a C++ front-end maintainer today, I would **strongly** oppose any change to 72 not strongly motivated at least by a comparison with other high quality front-ends and some feedback from the users asking a different default. Do we have any of the latter? Paolo.
Re: [C++ Patch] PR 32614
On Mon, 17 Oct 2011, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote: The initial patch, split between rev. 31343 and 31999 indeed added Thanks for helping tracking history. + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. Yes, most likely. I still think automatic wrapping (at 72 columns!? A terminal is 80x24!) should not be done by default. The choice of 72 at the time was based on the 80 of the terminal and Emacs guidelines. I see (and yes, editors probably do not break lines by default - but my terminals do, and I usually enlarge them to be able to decipher C++ diagnostics). I still think that not breaking existing consumers is more important than to restore something that hasn't been in effect for years. Oh, and yes, making documentation match reality is an improvement. Let's wait for Jason to break the tie. Richard.
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:19 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/17/2011 01:16 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:09 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: On 10/17/2011 12:56 PM, Gabriel Dos Reis wrote: Thus clearly the documentation is wrong ;) ;-) Not necessarily. Paolo does not say why that line was added. I don't remember adding that line to change the default. Indeed, as far as I can see, you added that line while *preserving* the existing behavior and preparing the C++ variant of the pretty_print machinery. Thus, AFAICS, 72 never existed anywhere and, strictly speaking, there is nothing to *restore*. I do not know what you mean by there is nothing to restore. Look at the other mail by Richard. The C pretty-printer *post*-dated the C++ pretty printer. Hey, I don't own viewcvs, of svn, for that matter, you could also dare to help a bit with this crazy archeological task, can't you?!? Let's not be quick to judgment and throw more rocks before we get all the facts. Please understand that I have been helping and looking at past changesets and present history. I appreciate that Richard did not think I was just be delusional and helped going back further. I can help by presenting history. It is not my fault when you choose to doubt or ignore. That isn't under my control.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
2011/10/17 Richard Guenther richard.guent...@gmail.com: On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote: 2011/10/17 Richard Guenther richard.guent...@gmail.com: On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote: Hello, So I committed the gimplify patch separate. And here is the remaining fold-const patch. The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which cover the one special-case for branching. Also tree-ssa/20040204-1.c covers tests for branching code (on targets having high-engough BRANCH_COST and no special-casing - like MIPS, S/390, and AVR. ChangeLog 2011-10-14 Kai Tietz kti...@redhat.com * fold-const.c (simple_operand_p_2): New function. (fold_truthop): Rename to (fold_truth_andor_1): function name. Additionally remove branching creation for logical and/or. (fold_truth_andor): Handle branching creation for logical and/or here. Bootstrapped and regression-tested for all languages plus Ada and Obj-C++ on x86_64-pc-linux-gnu. Ok for apply? Ok with ... Regards, Kai Index: gcc/gcc/fold-const.c === --- gcc.orig/gcc/fold-const.c +++ gcc/gcc/fold-const.c @@ -112,13 +112,13 @@ static tree decode_field_reference (loca static int all_ones_mask_p (const_tree, int); static tree sign_bit_p (tree, const_tree); static int simple_operand_p (const_tree); +static bool simple_operand_p_2 (tree); static tree range_binop (enum tree_code, tree, tree, int, tree, int); static tree range_predecessor (tree); static tree range_successor (tree); static tree fold_range_test (location_t, enum tree_code, tree, tree, tree); static tree fold_cond_expr_with_comparison (location_t, tree, tree, tree, tree); static tree unextend (tree, int, int, tree); -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); static tree optimize_minmax_comparison (location_t, enum tree_code, tree, tree, tree); static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l return lhs; } -/* Subroutine for fold_truthop: decode a field reference. +/* Subroutine for fold_truth_andor_1: decode a field reference. If EXP is a comparison reference, we return the innermost reference. @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val) return NULL_TREE; } -/* Subroutine for fold_truthop: determine if an operand is simple enough +/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough to be evaluated unconditionally. */ static int @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp) STRIP_NOPS (exp); return (CONSTANT_CLASS_P (exp) - || TREE_CODE (exp) == SSA_NAME + || TREE_CODE (exp) == SSA_NAME || (DECL_P (exp) ! TREE_ADDRESSABLE (exp) ! TREE_THIS_VOLATILE (exp) @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp) registers aren't expensive. */ (! TREE_STATIC (exp) || DECL_REGISTER (exp; } + +/* Subroutine for fold_truth_andor: determine if an operand is simple enough + to be evaluated unconditionally. + I addition to simple_operand_p, we assume that comparisons and logic-not + operations are simple, if their operands are simple, too. */ + +static bool +simple_operand_p_2 (tree exp) +{ + enum tree_code code; + + /* Strip any conversions that don't change the machine mode. */ + STRIP_NOPS (exp); + + code = TREE_CODE (exp); + + if (TREE_CODE_CLASS (code) == tcc_comparison) + return (!tree_could_trap_p (exp) + simple_operand_p_2 (TREE_OPERAND (exp, 0)) + simple_operand_p_2 (TREE_OPERAND (exp, 1))); recurse with simple_operand_p. No, as this again would reject simple operations and additionally wouldn't check for trapping. ? Your code allows arbitrarily complex expressions. Also tree_could_trap_p obviously extents to operands. Ah, ok. I wasn't aware that it walks into tree. + + if (TREE_SIDE_EFFECTS (exp) + || tree_could_trap_p (exp)) Move this check before the tcc_comparison check and remove the then redundant tree_could_trap_p check there. Ok + return false; + + switch (code) + { + case SSA_NAME: + return true; Do not handle here, it's handled in simple_operand_p. Well, was more a short-cut here. + case TRUTH_NOT_EXPR: + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + case BIT_NOT_EXPR: + if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE) + return false; Remove the BIT_NOT_EXPR handling. Thus, simply change this switch to Why should we reject simple ~X operations from gimplified code here? Because this is FE triggered code. From gimple you won't ever see such complex
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:29 AM, Richard Guenther rguent...@suse.de wrote: On Mon, 17 Oct 2011, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:08 AM, Richard Guenther rguent...@suse.de wrote: The initial patch, split between rev. 31343 and 31999 indeed added Thanks for helping tracking history. + /* Enable automatic line wrapping by default */ + set_message_length (72); to C++ lang_decode_option. Later it got appearantly lost somehow, probably during some of the Great Option Reorgs. Yes, most likely. I still think automatic wrapping (at 72 columns!? A terminal is 80x24!) should not be done by default. The choice of 72 at the time was based on the 80 of the terminal and Emacs guidelines. I see (and yes, editors probably do not break lines by default - but my terminals do, and I usually enlarge them to be able to decipher C++ diagnostics). These days, it is common for terminals to have line wrap. At the time, it wasn't common. Another suggestion I have heard when the line break was introduced, was to do it based on the characteristics of the output stream -- for example, adding colors (hello Benjamin!) which I believe SuSE added, and which demands testing the output stream. Another suggestion was people wanted to look at COLUMNS to automatically set the line length -- this comes from people using more than 80. -fmessage-length=0 had an internal necessity: the testsuites. I modified the testsuite framework to automatically set the length to 0. I still think that not breaking existing consumers is more important than to restore something that hasn't been in effect for years. Oh, and yes, making documentation match reality is an improvement. Let's wait for Jason to break the tie. Richard.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
On Mon, Oct 17, 2011 at 1:31 PM, Kai Tietz ktiet...@googlemail.com wrote: 2011/10/17 Richard Guenther richard.guent...@gmail.com: On Mon, Oct 17, 2011 at 12:59 PM, Kai Tietz ktiet...@googlemail.com wrote: 2011/10/17 Richard Guenther richard.guent...@gmail.com: On Fri, Oct 14, 2011 at 9:43 PM, Kai Tietz ktiet...@googlemail.com wrote: Hello, So I committed the gimplify patch separate. And here is the remaining fold-const patch. The important tests here are in gcc.dg/tree-ssa/builtin-expect[1-4].c, which cover the one special-case for branching. Also tree-ssa/20040204-1.c covers tests for branching code (on targets having high-engough BRANCH_COST and no special-casing - like MIPS, S/390, and AVR. ChangeLog 2011-10-14 Kai Tietz kti...@redhat.com * fold-const.c (simple_operand_p_2): New function. (fold_truthop): Rename to (fold_truth_andor_1): function name. Additionally remove branching creation for logical and/or. (fold_truth_andor): Handle branching creation for logical and/or here. Bootstrapped and regression-tested for all languages plus Ada and Obj-C++ on x86_64-pc-linux-gnu. Ok for apply? Ok with ... Regards, Kai Index: gcc/gcc/fold-const.c === --- gcc.orig/gcc/fold-const.c +++ gcc/gcc/fold-const.c @@ -112,13 +112,13 @@ static tree decode_field_reference (loca static int all_ones_mask_p (const_tree, int); static tree sign_bit_p (tree, const_tree); static int simple_operand_p (const_tree); +static bool simple_operand_p_2 (tree); static tree range_binop (enum tree_code, tree, tree, int, tree, int); static tree range_predecessor (tree); static tree range_successor (tree); static tree fold_range_test (location_t, enum tree_code, tree, tree, tree); static tree fold_cond_expr_with_comparison (location_t, tree, tree, tree, tree); static tree unextend (tree, int, int, tree); -static tree fold_truthop (location_t, enum tree_code, tree, tree, tree); static tree optimize_minmax_comparison (location_t, enum tree_code, tree, tree, tree); static tree extract_muldiv (tree, tree, enum tree_code, tree, bool *); @@ -3500,7 +3500,7 @@ optimize_bit_field_compare (location_t l return lhs; } -/* Subroutine for fold_truthop: decode a field reference. +/* Subroutine for fold_truth_andor_1: decode a field reference. If EXP is a comparison reference, we return the innermost reference. @@ -3668,7 +3668,7 @@ sign_bit_p (tree exp, const_tree val) return NULL_TREE; } -/* Subroutine for fold_truthop: determine if an operand is simple enough +/* Subroutine for fold_truth_andor_1: determine if an operand is simple enough to be evaluated unconditionally. */ static int @@ -3678,7 +3678,7 @@ simple_operand_p (const_tree exp) STRIP_NOPS (exp); return (CONSTANT_CLASS_P (exp) - || TREE_CODE (exp) == SSA_NAME + || TREE_CODE (exp) == SSA_NAME || (DECL_P (exp) ! TREE_ADDRESSABLE (exp) ! TREE_THIS_VOLATILE (exp) @@ -3692,6 +3692,46 @@ simple_operand_p (const_tree exp) registers aren't expensive. */ (! TREE_STATIC (exp) || DECL_REGISTER (exp; } + +/* Subroutine for fold_truth_andor: determine if an operand is simple enough + to be evaluated unconditionally. + I addition to simple_operand_p, we assume that comparisons and logic-not + operations are simple, if their operands are simple, too. */ + +static bool +simple_operand_p_2 (tree exp) +{ + enum tree_code code; + + /* Strip any conversions that don't change the machine mode. */ + STRIP_NOPS (exp); + + code = TREE_CODE (exp); + + if (TREE_CODE_CLASS (code) == tcc_comparison) + return (!tree_could_trap_p (exp) + simple_operand_p_2 (TREE_OPERAND (exp, 0)) + simple_operand_p_2 (TREE_OPERAND (exp, 1))); recurse with simple_operand_p. No, as this again would reject simple operations and additionally wouldn't check for trapping. ? Your code allows arbitrarily complex expressions. Also tree_could_trap_p obviously extents to operands. Ah, ok. I wasn't aware that it walks into tree. + + if (TREE_SIDE_EFFECTS (exp) + || tree_could_trap_p (exp)) Move this check before the tcc_comparison check and remove the then redundant tree_could_trap_p check there. Ok + return false; + + switch (code) + { + case SSA_NAME: + return true; Do not handle here, it's handled in simple_operand_p. Well, was more a short-cut here. + case TRUTH_NOT_EXPR: + return simple_operand_p_2 (TREE_OPERAND (exp, 0)); + case BIT_NOT_EXPR: + if (TREE_CODE (TREE_TYPE (exp)) != BOOLEAN_TYPE) + return false; Remove the BIT_NOT_EXPR handling. Thus, simply change this switch to Why should we reject simple ~X operations from gimplified code here?
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlini paolo.carl...@oracle.com wrote: I would **strongly** oppose any change to 72 not strongly motivated at least by a comparison with other high quality front-ends I love it when you make arguments like this. It makes me smile, and I like smiling when I just get off bed :-) Just another fact: the decision of line wrapping was based in comparison to what another high quality front-end was doing. It wasn't a design decision made out of tin air. I suspect you do not even agree with the fact that the change was accidental, not intended. Yet, I bet you will shortly tell me that I am not helping with giving history behind the changes.
Re: [C++ Patch] PR 32614
On 10/17/2011 01:44 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: I would **strongly** oppose any change to 72 not strongly motivated at least by a comparison with other high quality front-ends I love it when you make arguments like this. It makes me smile, and I like smiling when I just get off bed :-) I don't see why. In any case please consider also the second half of that phrase, the users, like Richard for example, or me. And please help re-assessing the situation wrt the other front-ends *today* not in the stone age. Paolo.
Re: [PATCH 3/6] Emit macro expansion related diagnostics
Richard Guenther richard.guent...@gmail.com writes: This broke bootstrap on x86_64-linux. /space/rguenther/src/svn/trunk/libcpp/line-map.c: In function 'source_location linemap_macro_map_loc_to_exp_point(const line_map*, source_location)': /space/rguenther/src/svn/trunk/libcpp/line-map.c:628:12: error: variable 'token_no' set but not used [-Werror=unused-but-set-variable] cc1plus: all warnings being treated as errors Sigh. I guess the reason why my testing hasn't caught this is that I am bootstrapping with --enable-checking and so on my system ENABLE_CHECKING is defined and my GCC_VERSION = 2007. I am bootstrapping and testing the obvious patch below with --disable-bootstrap and I am planning to check it in if it passes, under the obvious rule. Sorry for the inconvenience. commit e957242a9a8ec8f297e05ca0dae1d63bf543fad8 Author: Dodji Seketeli do...@redhat.com Date: Mon Oct 17 13:33:41 2011 +0200 Fix bootstrapping with --disable-checking libcpp/ChangeLog * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a variable without using it if ENABLE_CHECKING is not defined. diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 87b8bfe..a1d0fbb 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -625,14 +625,12 @@ source_location linemap_macro_map_loc_to_exp_point (const struct line_map *map, source_location location) { - unsigned token_no; - linemap_assert (linemap_macro_expansion_map_p (map) location = MAP_START_LOCATION (map)); /* Make sure LOCATION is correct. */ - token_no = location - MAP_START_LOCATION (map); - linemap_assert (token_no MACRO_MAP_NUM_MACRO_TOKENS (map)); + linemap_assert ((location - MAP_START_LOCATION (map)) + MACRO_MAP_NUM_MACRO_TOKENS (map)); return MACRO_MAP_EXPANSION_POINT_LOCATION (map); } -- Dodji
Re: [C++ Patch] PR 32614
On Mon, Oct 17, 2011 at 6:48 AM, Paolo Carlini paolo.carl...@oracle.com wrote: On 10/17/2011 01:44 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 6:26 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: I would **strongly** oppose any change to 72 not strongly motivated at least by a comparison with other high quality front-ends I love it when you make arguments like this. It makes me smile, and I like smiling when I just get off bed :-) I don't see why. If I reveal that then I may very well lose a valuable source of smile inducer :-) In any case please consider also the second half of that phrase, the users, like Richard for example, or me. you should consider I did -- just like I give you the benefits of doubt (for example, I do not assume you want to be rude for entertainment purposes). And please help re-assessing the situation wrt the other front-ends *today* not in the stone age. Paolo.
Re: [VTA, PR49310] O(n+m)-ish emit_notes
On Tue, Oct 11, 2011 at 04:19:52PM -0300, Alexandre Oliva wrote: Here's what I've got so far. Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? I see +FAIL: gcc.c-torture/compile/pr19080.c -O3 -g (internal compiler error) +FAIL: gcc.c-torture/compile/pr19080.c -O3 -g (test for excess errors) regression with this patch on x86_64-linux (--enable-checking=yes,rtl), ICE in var-tracking. Can you please look at that? +/* Enumeration type used to discriminate various types of one-part + variables. */ +typedef enum onepart_enum { Just a style nit, isn't { supposed to go on the next line? + /* Not a one-part variable. */ + NOT_ONEPART = 0, + /* A one-part DECL that is not a DEBUG_EXPR_DECL. */ + ONEPART_VDECL = 1, + /* A DEBUG_EXPR_DECL. */ + ONEPART_DEXPR = 2, + /* A VALUE. */ + ONEPART_VALUE = 3 +} onepart_enum_t; + /* Structure describing where the variable is located. */ typedef struct variable_def { Otherwise looks ok. Jakub
[PATCH][RFC] Fix PR50716, override type alignment knowledge
This changes alignment computation of a MEM during expansion from MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp)) to something weaker (with possibly less alignment) when we were able to compute an explicit misaligned value (thus, get_object_alignment_1 (exp, misalign) would return an alignment of 8, but misaligned by 4, for example - in which case get_object_alignment () would return 4). This is to less often fall into the trap that the frontends and the middle-end are not careful about providing a type with properly alignment for misaligned memory references (classical example is struct __attribute__((packed)) { int i; } a; a, where a has int * type, not int __attribute__((aligned(1))) * type). The patch unbreaks the testcase in the PR (which is strictly invalid) by using precise misalign information we are able to compute at -O1 and up: typedef int vec __attribute__((vector_size(16))); int main () { int * arr = __builtin_malloc (1024); vec *p = (vec *) arr[1]; *p = (vec){1, 2, 3, 4}; return *(char *)p; } on x86_64 without the patch we emit movaps while with the patch we use a movups instruction (which is being nice to our users). I didn't bother to try thinking of a heuristic when to use the misaligned info and when not - always using it makes the most sense consistency-wise. I had considered only using TYPE_ALIGN () when get_object_alignment_1 returns BITS_PER_UNIT for the alignment (which sort-of means don't know). Bootstrap and regtest scheduled on x86_64-unknown-linux-gnu. Any comments? Thanks, Richard. 2011-10-17 Richard Guenther rguent...@suse.de PR middle-end/50716 * expr.c (get_object_or_type_alignment): New function. (expand_assignment): Use it. (expand_expr_real_1): Likewise. Index: gcc/expr.c === *** gcc/expr.c (revision 180077) --- gcc/expr.c (working copy) *** get_bit_range (unsigned HOST_WIDE_INT *b *** 4544,4549 --- 4544,4568 } } + /* Return the alignment of the object EXP, also considering its type +when we do not know of explicit misalignment. +??? Note that generally the type of an expression is not kept +consistent with misalignment information by the frontend, for +example when taking the address of a member of a packed structure. +So taking into account type information for alignment is generally +wrong, but is done here as a compromise. */ + + static unsigned int + get_object_or_type_alignment (tree exp) + { + unsigned HOST_WIDE_INT misalign; + unsigned int align = get_object_alignment_1 (exp, misalign); + align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align); + if (misalign != 0) + align = (misalign -misalign); + return align; + } + /* Expand an assignment that stores the value of FROM into TO. If NONTEMPORAL is true, try generating a nontemporal store. */ *** expand_assignment (tree to, tree from, b *** 4553,4559 rtx to_rtx = 0; rtx result; enum machine_mode mode; ! int align; enum insn_code icode; /* Don't crash if the lhs of the assignment was erroneous. */ --- 4572,4578 rtx to_rtx = 0; rtx result; enum machine_mode mode; ! unsigned int align; enum insn_code icode; /* Don't crash if the lhs of the assignment was erroneous. */ *** expand_assignment (tree to, tree from, b *** 4571,4578 if ((TREE_CODE (to) == MEM_REF || TREE_CODE (to) == TARGET_MEM_REF) mode != BLKmode !((align = MAX (TYPE_ALIGN (TREE_TYPE (to)), get_object_alignment (to))) ! (signed) GET_MODE_ALIGNMENT (mode)) ((icode = optab_handler (movmisalign_optab, mode)) != CODE_FOR_nothing)) { --- 4590,4597 if ((TREE_CODE (to) == MEM_REF || TREE_CODE (to) == TARGET_MEM_REF) mode != BLKmode !((align = get_object_or_type_alignment (to)) ! GET_MODE_ALIGNMENT (mode)) ((icode = optab_handler (movmisalign_optab, mode)) != CODE_FOR_nothing)) { *** expand_expr_real_1 (tree exp, rtx target *** 9241,9247 addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); struct mem_address addr; enum insn_code icode; ! int align; get_address_description (exp, addr); op0 = addr_for_mem_ref (addr, as, true); --- 9260,9266 addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); struct mem_address addr; enum insn_code icode; ! unsigned int align; get_address_description (exp, addr); op0 = addr_for_mem_ref (addr, as, true); *** expand_expr_real_1 (tree exp, rtx target *** 9249,9257 temp = gen_rtx_MEM (mode, op0); set_mem_attributes (temp, exp, 0); set_mem_addr_space (temp, as); ! align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp));
[PATCH] Fix PR50729
This fixes PR50729, so much for not implementing it by using value-ranges ... opens up the door for more bugs ;) Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2011-10-17 Richard Guenther rguent...@suse.de PR tree-optimization/50729 * tree-vrp.c (extract_range_from_unary_expr_1): Remove redundant test. (simplify_conversion_using_ranges): Properly test the intermediate result. * gcc.dg/torture/pr50729.c: New testcase. Index: gcc/tree-vrp.c === *** gcc/tree-vrp.c (revision 180077) --- gcc/tree-vrp.c (working copy) *** extract_range_from_unary_expr_1 (value_r *** 2913,2927 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]). */ if (POINTER_TYPE_P (type)) { ! if (CONVERT_EXPR_CODE_P (code)) ! { ! if (range_is_nonnull (vr0)) ! set_value_range_to_nonnull (vr, type); ! else if (range_is_null (vr0)) ! set_value_range_to_null (vr, type); ! else ! set_value_range_to_varying (vr); ! } else set_value_range_to_varying (vr); return; --- 2913,2922 determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]). */ if (POINTER_TYPE_P (type)) { ! if (range_is_nonnull (vr0)) ! set_value_range_to_nonnull (vr, type); ! else if (range_is_null (vr0)) ! set_value_range_to_null (vr, type); else set_value_range_to_varying (vr); return; *** simplify_conversion_using_ranges (gimple *** 7288,7297 TYPE_UNSIGNED (TREE_TYPE (middleop))); middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)), TYPE_UNSIGNED (TREE_TYPE (middleop))); ! /* If the middle values do not represent a proper range fail. */ ! if (double_int_cmp (middlemin, middlemax, ! TYPE_UNSIGNED (TREE_TYPE (middleop))) 0) return false; if (!double_int_equal_p (double_int_ext (middlemin, TYPE_PRECISION (finaltype), TYPE_UNSIGNED (finaltype)), --- 7283,7299 TYPE_UNSIGNED (TREE_TYPE (middleop))); middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)), TYPE_UNSIGNED (TREE_TYPE (middleop))); ! /* If the middle values are not equal to the original values fail. ! But only if the inner cast truncates (thus we ignore differences ! in extension to handle the case going from a range to an anti-range ! and back). */ ! if ((TYPE_PRECISION (TREE_TYPE (innerop)) ! TYPE_PRECISION (TREE_TYPE (middleop))) !(!double_int_equal_p (innermin, middlemin) ! || !double_int_equal_p (innermax, middlemax))) return false; + /* Require that the final conversion applied to both the original + and the intermediate range produces the same result. */ if (!double_int_equal_p (double_int_ext (middlemin, TYPE_PRECISION (finaltype), TYPE_UNSIGNED (finaltype)), Index: gcc/testsuite/gcc.dg/torture/pr50729.c === *** gcc/testsuite/gcc.dg/torture/pr50729.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr50729.c (revision 0) *** *** 0 --- 1,20 + /* { dg-do run } */ + /* { dg-require-effective-target int32plus } */ + + extern void abort (void); + unsigned short __attribute__((noinline)) + foo (int i) + { + if (i = 0 +i = 0x40) + return (unsigned short)(signed char)i; + return i; + } + int main() + { + int i; + for (i = 0; i 0x; ++i) + if (foo(i) != (unsigned short)(signed char) i) + abort (); + return 0; + }
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
Ok, I see. This might be profitable to do that. So fold_truth_op hunk looks like this @@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_ build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg), ll_arg, rl_arg), build_int_cst (TREE_TYPE (ll_arg), 0)); - - if (LOGICAL_OP_NON_SHORT_CIRCUIT) - { - if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs) - return build2_loc (loc, code, truth_type, lhs, rhs); - return NULL_TREE; - } } /* See if the comparisons can be merged. Then get all the parameters for @@ -8380,13 +8400,77 @@ fold_truth_andor (location_t loc, enum t lhs is another similar operation, try to merge its rhs with our rhs. Then try to merge our lhs and rhs. */ if (TREE_CODE (arg0) == code - 0 != (tem = fold_truthop (loc, code, type, - TREE_OPERAND (arg0, 1), arg1))) + 0 != (tem = fold_truth_andor_1 (loc, code, type, +TREE_OPERAND (arg0, 1), arg1))) return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem); - if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0) + if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0) return tem; + if ((BRANCH_COST (optimize_function_for_speed_p (cfun), + false) = 2) + LOGICAL_OP_NON_SHORT_CIRCUIT + simple_operand_p_2 (arg1)) +{ + enum tree_code ncode; + + if (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR) +{ + ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR : TRUTH_OR_EXPR); + + /* Transform ((A AND-IF B) AND-IF C) into (A AND-IF (B AND C)), +or ((A OR-IF B) OR-IF C) into (A OR-IF (B OR C)) +We don't want to pack more than two leafs to a non-IF AND/OR +expression. +If tree-code of left-hand operand isn't an AND/OR-IF code and not +equal to CODE, then we don't want to add right-hand operand. +If the inner right-hand side of left-hand operand has +side-effects, or isn't simple, then we can't add to it, +as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == code + /* Needed for sequence points to handle trappings, and +side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1), +arg1); + return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), + tem); + } + /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B) +into (A OR B). +For sequence point consistancy, we need to check for trapping, +and side-effects. */ + else if (simple_operand_p_2 (arg0)) + return fold_build2_loc (loc, ncode, type, arg0, arg1); + } + else +{ + ncode = (code == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR + : TRUTH_ORIF_EXPR); + /* Transform ((A AND-IF B) AND C) into (A AND-IF (B AND C)), +or ((A OR-IF B) OR C) into (A OR-IF (B OR C)) +We don't want to pack more than two leafs to a non-IF AND/OR +expression. +If tree-code of left-hand operand isn't an AND/OR-IF code and not +equal to NCODE, then we don't want to add right-hand operand. +If the inner right-hand side of left-hand operand has +side-effects, or isn't simple, then we can't add to it, +as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == ncode + /* Needed for sequence points to handle trappings, and +side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 1), +arg1); + return fold_build2_loc (loc, ncode, type, + TREE_OPERAND (arg0, 0), tem); + } + } + +} + return NULL_TREE; } Ok, with other changes you mentioned? Regards, Kai
Re: [pph] Refactor Vectors and Chains (issue5263051)
On Sun, Oct 16, 2011 at 22:09, Lawrence Crowl cr...@google.com wrote: Factor the vector and chain output routines to remove boolean control parameters. The functions pph_out_tree_vec_1 and pph_out_chain_1 split their conditional parts of their implementation into their use cases, calling each other as needed. pph_out_tree_vec - nothing special pph_out_tree_vec_unchain - unchaining pph_out_mergeable_tree_vec - merging, unchaining, reversing pph_out_tree_vec_filtered - filtering pph_out_chain - nothing special pph_out_chain_filtered - filtering pph_out_mergeable_chain_filtered - merging, unchaining, reversing, filtering But, you are duplicating code that the previous patch had explicitly commonized. Why? It's easier to keep the core streaming logic in one function, to have a single point of debugging when dealing with sync problems. This change fixes an ordering bug, but now causes an ICE to surface, which will be addressed later. What ordering bug? Diego.
Re: [PATCH, testsuite, i386] FMA3 testcases + typo fix in MD
Thanks! K On Sat, Oct 15, 2011 at 3:08 PM, Uros Bizjak ubiz...@gmail.com wrote: On Sat, Oct 15, 2011 at 10:32 AM, Uros Bizjak ubiz...@gmail.com wrote: --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/fma_double_1.c @@ -0,0 +1,19 @@ +/* { dg-do compile } */ +/* { dg-prune-output .*warning: 'sseregparm' attribute ignored.* } */ That prunes too much. gcc-dg-prune brackets the regexp so that it only matches within a single line, but your use of .* (which also matches newlines) overrides that and will cause the *whole* output to be thrown away, including any unrelated error messages. Perhaps e can add -Wno-attributes to dg-options and remove dg-prune-output. I will look at this possibility and create a patch. It works. 2011-10-15 Uros Bizjak ubiz...@gmail.com * gcc.target/i386/fma_float_?.c (dg-prune_output): Remove. (dg-options): Add -Wno-attributes. * gcc.target/i386/fma_double_?.c: Ditto. * gcc.target/i386/fma_run_float_?.c: Ditto. * gcc.target/i386/fma_run_double_?.c: Ditto. * gcc.target/i386/l_fma_float_?.c: Dtto. * gcc.target/i386/l_fma_double_?.c: Ditto. * gcc.target/i386/l_fma_run_float_?.c: Ditto. * gcc.target/i386/l_fma_run_double_?.c: Ditto. Attached patch was tested on x86_64-pc-linux-gnu {,-m32} and committed to SVN. Uros.
[Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit
This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine and the FLUSH statement. It removes the _commit from gfortran's buf_flush. Background: * gfortran internally buffers the I/O, but it calls the nonbuffering open/write/read functions (and not, e.g., fopen/fwrite/fread). On Unix/Darwin/Linux system, the changes become immediately visible to all processes after a write(). * On Windows, there seems to be a file-descriptor specific system buffer such that the data only becomes available to other processes after either a _commit or after closing the file. * The Windows _commit() is a combination of a (system) buffer flush - as a write() already implies on POSIX systems, but it also ensures that the data ends up on the disk, which on POSIX systems is done via fsync(). Currently, _commit() is also called for the internal buf_flush, which can happen very often and thus delays the I/O a lot (cf. PR 50016). With this patch, it is only called when the user explicitly called FLUSH(). Contrary to non-_WIN32 systems, this also flushes to the hard disk, which causes some slow down. However, as the user (should) only rarely call the function, it should not pose a real performance issue. The patch should ensure that the values can be read via other file-descriptors within the same program or from other programs; on the same time, it prevents excessive _commit() calling as it is done with the current code. An alternative would be not to call _commit() at all, leaving it to the user. That would match the fsync() result on POSIX systems, but I think having a file not available in the current status for other processes causes more confusion that it helps. Thus, I think for FLUSH, one should really make sure other processes get the right data. The patch was build and regtested on x86-64-linux (where it should be a noop). OK for the trunk? Can someone test it on MinGW/MinGW-64? Tobias PS: For the discussion, see http://gcc.gnu.org/ml/fortran/2011-10/msg00079.html PPS: I regard this patch as rather independent of INQUIRE even if the issue first occurred with INQUIRE (PR 44698). Thus, it is independent from whether one calls stat() for an open file or whether one uses a different method like seek(SEEK_END) + tell() to obtain the file size. 2011-10-17 Tobias Burnus bur...@net-b.de PR fortran/50016 * intrinsic.texi (FLUSH): Document that it calls _commit on MinGW(-w64). 2011-10-17 Tobias Burnus bur...@net-b.de PR fortran/50016 * io/file_pos.c (st_flush): Call _commit on MinGW(-w64). * io/intrinsics.c (flush_i4, flush_i8): Ditto. * io/unix.c (flush_all_units_1, flush_all_units): Ditto. (buf_flush): Remove _commit call. diff --git a/gcc/fortran/intrinsic.texi b/gcc/fortran/intrinsic.texi index 11f87a5..1a0e930 100644 --- a/gcc/fortran/intrinsic.texi +++ b/gcc/fortran/intrinsic.texi @@ -4775,7 +4775,10 @@ that the data is committed to disk. On POSIX systems, you can request that all data is transferred to the storage device by calling the @code{fsync} function, with the POSIX file descriptor of the I/O unit as argument (retrieved with GNU intrinsic -@code{FNUM}). The following example shows how: +@code{FNUM}). The following example shows how. On Windows (MinGW and +MinGW-w64), the @code{FLUSH} subroutine and @code{FLUSH} statement always +call @code{_commit} to ensure that other file descriptors see the current +file status; @code{_commit} additionally flushes the data to the disk. @smallexample ! Declare the interface for POSIX fsync function diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c index b034f38..4efdc1f 100644 --- a/libgfortran/io/file_pos.c +++ b/libgfortran/io/file_pos.c @@ -452,6 +452,10 @@ st_flush (st_parameter_filepos *fpp) fbuf_flush (u, u-mode); sflush (u-s); +#ifdef _WIN32 + /* Without _commit, changes are not visible to other file descriptors. */ + _commit (u-s-fd); +#endif unlock_unit (u); } else diff --git a/libgfortran/io/intrinsics.c b/libgfortran/io/intrinsics.c index f48bd77..eaf2e17 100644 --- a/libgfortran/io/intrinsics.c +++ b/libgfortran/io/intrinsics.c @@ -207,6 +207,11 @@ flush_i4 (GFC_INTEGER_4 *unit) if (us != NULL) { sflush (us-s); +#ifdef _WIN32 + /* Without _commit, changes are not visible + to other file descriptors. */ + _commit (u-s-fd); +#endif unlock_unit (us); } } @@ -230,6 +235,11 @@ flush_i8 (GFC_INTEGER_8 *unit) if (us != NULL) { sflush (us-s); +#ifdef _WIN32 + /* Without _commit, changes are not visible + to other file descriptors. */ + _commit (u-s-fd); +#endif unlock_unit (us); } } diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c index 25cb559..a88e83b 100644 --- a/libgfortran/io/unix.c +++ b/libgfortran/io/unix.c @@ -443,10 +443,6 @@ buf_flush (unix_stream * s) if (s-ndirty != 0) return -1; -#ifdef _WIN32 - _commit (s-fd); -#endif - return 0; }
Re: [ARM] Fix PR49641
On 14/10/11 14:31, Bernd Schmidt wrote: On 07/13/11 16:03, Richard Earnshaw wrote: * config/arm/arm.c (store_multiple_sequence): Avoid cases where the base reg is stored iff compiling for Thumb1. * gcc.target/arm/pr49641.c: New test. Ping. Richard, you replied to the mail but didn't comment on the patch. Bernd Sorry, I thought I'd made it clear that I don't think the compiler should ever use STM with write-back if the base register is in the stored list. We must certainly never do it if the base register is not the first register in the list as this has always been unpredictable. BTW, this is not Thumb1 specific, it applies at all times. So, no the patch is not OK as it stands. R.
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
On Mon, Oct 17, 2011 at 2:22 PM, Kai Tietz ktiet...@googlemail.com wrote: Ok, I see. This might be profitable to do that. So fold_truth_op hunk looks like this @@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_ build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg), ll_arg, rl_arg), build_int_cst (TREE_TYPE (ll_arg), 0)); - - if (LOGICAL_OP_NON_SHORT_CIRCUIT) - { - if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs) - return build2_loc (loc, code, truth_type, lhs, rhs); - return NULL_TREE; - } } /* See if the comparisons can be merged. Then get all the parameters for @@ -8380,13 +8400,77 @@ fold_truth_andor (location_t loc, enum t lhs is another similar operation, try to merge its rhs with our rhs. Then try to merge our lhs and rhs. */ if (TREE_CODE (arg0) == code - 0 != (tem = fold_truthop (loc, code, type, - TREE_OPERAND (arg0, 1), arg1))) + 0 != (tem = fold_truth_andor_1 (loc, code, type, + TREE_OPERAND (arg0, 1), arg1))) return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem); - if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0) + if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0) return tem; + if ((BRANCH_COST (optimize_function_for_speed_p (cfun), + false) = 2) + LOGICAL_OP_NON_SHORT_CIRCUIT + simple_operand_p_2 (arg1)) + { + enum tree_code ncode; + + if (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR) + { + ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR : TRUTH_OR_EXPR); + + /* Transform ((A AND-IF B) AND-IF C) into (A AND-IF (B AND C)), + or ((A OR-IF B) OR-IF C) into (A OR-IF (B OR C)) + We don't want to pack more than two leafs to a non-IF AND/OR + expression. + If tree-code of left-hand operand isn't an AND/OR-IF code and not + equal to CODE, then we don't want to add right-hand operand. + If the inner right-hand side of left-hand operand has + side-effects, or isn't simple, then we can't add to it, + as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == code + /* Needed for sequence points to handle trappings, and + side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1), + arg1); + return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), + tem); + } + /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B) + into (A OR B). + For sequence point consistancy, we need to check for trapping, + and side-effects. */ + else if (simple_operand_p_2 (arg0)) + return fold_build2_loc (loc, ncode, type, arg0, arg1); + } + else + { + ncode = (code == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR + : TRUTH_ORIF_EXPR); + /* Transform ((A AND-IF B) AND C) into (A AND-IF (B AND C)), + or ((A OR-IF B) OR C) into (A OR-IF (B OR C)) + We don't want to pack more than two leafs to a non-IF AND/OR + expression. + If tree-code of left-hand operand isn't an AND/OR-IF code and not + equal to NCODE, then we don't want to add right-hand operand. + If the inner right-hand side of left-hand operand has + side-effects, or isn't simple, then we can't add to it, + as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == ncode + /* Needed for sequence points to handle trappings, and + side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 1), + arg1); + return fold_build2_loc (loc, ncode, type, + TREE_OPERAND (arg0, 0), tem); + } + } + + } + return NULL_TREE; } Ok, with other changes you mentioned? This can be done without so much code duplication. Regards, Kai
Re: [pph] Make libcpp symbol validation a warning (issue5235061)
On Fri, Oct 14, 2011 at 20:27, Gabriel Charette gcharet...@gmail.com wrote: Yes, I understand that. But when the second 2.pph is skipped when reading foo.pph, the reading of its line_table is also skipped (as foo.pph doesn't contain the line_table information for 2.h, 2.pph does and adds it when its included as a child, but if it's skipped, the line_table info for 2.h should never make it in the line_table), so I don't see why this is an issue for the line_table (other than the assert about the number of line table entries read). What I was suggesting is that as far as the assert is concerned it would be stronger to count the number of skipped child headers on read and assert num_read+num_skipped == num_expected_childs basically (it is still only an assert so no big deal I guess). Ah, I see what you are saying. I didn't really bother too much with that assert. Since I was not reading the line table again, I figured both asserts were triggering because of the different values coming from the skipped file, so I left them out. Essentially this patch fixes the last bug we had in the line_table merging (i.e. that guarded out headers in the non-pph version weren't guarded out in the pph version) and this is a good thing. I'm just being picky about weakening asserts! I still think it would be nice to have a way to test constructs like the line_table at the end of parsing (maybe a new flag, as I was suggesting in my previous email, as gcc doesn't allow for modular testing) and compare pph and non-pph versions. Testing at this level would potentially be much better than trying to understand tricky test failures from the ground up. This is beyond the scope of this patch of course, but something to keep in mind I think. Yeah. I'll come back to it at a later point. Diego.
Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling
Tristan == Tristan Gingold ging...@adacore.com writes: Tom Another way to look at it is that there have been many changes to GCC's Tom DWARF output in the last few years. Surely these have broken these Tom DWARF consumers more than this change possibly could. Tristan Yes, but there is -gstrict-dwarf to stay compatible with old behavior. Yes, but this change is strictly compliant. What makes it different from any other change that was made to make GCC more compliant? Hypothetical consumers could also break on those changes. Tom
Re: [PATCH][ARM] -m{cpu,tune,arch}=native
On 20/09/11 11:51, Andrew Stubbs wrote: On 09/09/11 12:55, Richard Earnshaw wrote: The part number field is meaningless outside of the context of a a specific vendor -- only taken as a pair can they refer to a specific part. So why is the vendor field hard-coded rather than factored into the table of parts. Maybe it would be better to have a table of tables, with the top-level table being indexed by vendor id. Something like Yes, but since I only have part numbers for one vendor, I left that sort of thing out on the principle that it's best not to add complexity until you need it. Anyway, I have done it now, so here it is. :) I've also fixed the problem that if it didn't recognise the CPU, it defaulted to the hard default, ignoring the --with-cpu configured default. OK? Andrew tune-native.patch 2011-09-20 Andrew Stubbs a...@codesourcery.com gcc/ * config.host (arm*-*-linux*): Add driver-arm.o and x-arm. * config/arm/arm.opt: Add 'native' processor_type and arm_arch enum values. * config/arm/arm.h (host_detect_local_cpu): New prototype. (EXTRA_SPEC_FUNCTIONS): New define. (MCPU_MTUNE_NATIVE_SPECS): New define. (DRIVER_SELF_SPECS): New define. * config/arm/driver-arm.c: New file. * config/arm/x-arm: New file. * doc/invoke.texi (ARM Options): Document -mcpu=native, -mtune=native and -march=native. There's a presumption in host_detect_local_cpu() that CPU implementer will appear before CPU part in the output of /proc/cpuinfo. That's probably a pretty safe assumption (and it appears that it will handle that case relatively safely -- ie not crash). I'll let you decide whether that's important enough to fix. However another problem I've just spotted is that you never close the file descriptor if you fail to parse the output properly (ie jump to not_found). That should be fixed before this is committed. OK with the second issue resolved. R.
[C++ Patch] PR 44524
Hi, here submitter requests a more accurate error message for X.Y where X is a pointer to class type. Thus the below, tested x86_64-linux. Ok for mainline? Thanks, Paolo. // /cp 2011-10-17 Paolo Carlini paolo.carl...@oracle.com PR c++/44524 * typeck.c (build_class_member_access_expr): Provide a better error message for X.Y where X is a pointer to class type. (finish_class_member_access_expr): Likewise. /testsuite 2011-10-17 Paolo Carlini paolo.carl...@oracle.com PR c++/44524 * g++.dg/parse/error41.C: New. * g++.dg/parse/error20.C: Adjust. Index: testsuite/g++.dg/parse/error20.C === --- testsuite/g++.dg/parse/error20.C(revision 180087) +++ testsuite/g++.dg/parse/error20.C(working copy) @@ -12,7 +12,7 @@ struct C { }; int main() { C c; - A(c.p.i); // { dg-error 9:request for member 'i' in 'c.C::p', which is of non-class type 'B } + A(c.p.i); // { dg-error 9:request for member 'i' in 'c.C::p', which has pointer type 'B } return 0; } Index: testsuite/g++.dg/parse/error41.C === --- testsuite/g++.dg/parse/error41.C(revision 0) +++ testsuite/g++.dg/parse/error41.C(revision 0) @@ -0,0 +1,11 @@ +// PR c++/44524 + +templatetypename, typename +struct map +{ + bool empty(); +}; + +int bar(mapint, float *X) { + return X.empty(); // { dg-error which has pointer type 'map } +} Index: cp/typeck.c === --- cp/typeck.c (revision 180087) +++ cp/typeck.c (working copy) @@ -2128,8 +2128,16 @@ build_class_member_access_expr (tree object, tree if (!CLASS_TYPE_P (object_type)) { if (complain tf_error) - error (request for member %qD in %qE, which is of non-class type %qT, - member, object, object_type); + { + if (POINTER_TYPE_P (object_type) + CLASS_TYPE_P (TREE_TYPE (object_type))) + error (request for member %qD in %qE, which has pointer + type %qT (maybe you meant to use %-% ?), + member, object, object_type); + else + error (request for member %qD in %qE, which is of non-class + type %qT, member, object, object_type); + } return error_mark_node; } @@ -2508,8 +2516,16 @@ finish_class_member_access_expr (tree object, tree if (!CLASS_TYPE_P (object_type)) { if (complain tf_error) - error (request for member %qD in %qE, which is of non-class type %qT, - name, object, object_type); + { + if (POINTER_TYPE_P (object_type) + CLASS_TYPE_P (TREE_TYPE (object_type))) + error (request for member %qD in %qE, which has pointer + type %qT (maybe you meant to use %-% ?), + name, object, object_type); + else + error (request for member %qD in %qE, which is of non-class + type %qT, name, object, object_type); + } return error_mark_node; }
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
Sure, Is simplier and also handles (A T[-IF] (B T-IF C) - (A T B) T-IF C case, which can happen by framing in conditions. @@ -8380,13 +8400,65 @@ fold_truth_andor (location_t loc, enum t lhs is another similar operation, try to merge its rhs with our rhs. Then try to merge our lhs and rhs. */ if (TREE_CODE (arg0) == code - 0 != (tem = fold_truthop (loc, code, type, - TREE_OPERAND (arg0, 1), arg1))) + 0 != (tem = fold_truth_andor_1 (loc, code, type, +TREE_OPERAND (arg0, 1), arg1))) return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem); - if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0) + if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0) return tem; + if ((BRANCH_COST (optimize_function_for_speed_p (cfun), + false) = 2) + LOGICAL_OP_NON_SHORT_CIRCUIT) +{ + enum tree_code ncode, icode; + + ncode = (code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR) + ? TRUTH_AND_EXPR : TRUTH_OR_EXPR; + icode = ncode == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR : TRUTH_ORIF_EXPR; + + /* Transform ((A AND-IF B) AND[-IF] C) into (A AND-IF (B AND C)), +or ((A OR-IF B) OR[-IF] C) into (A OR-IF (B OR C)) +We don't want to pack more than two leafs to a non-IF AND/OR +expression. +If tree-code of left-hand operand isn't an AND/OR-IF code and not +equal to IF-CODE, then we don't want to add right-hand operand. +If the inner right-hand side of left-hand operand has +side-effects, or isn't simple, then we can't add to it, +as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == icode + simple_operand_p_2 (arg1) + /* Needed for sequence points to handle trappings, and +side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1), +arg1); + return fold_build2_loc (loc, icode, type, TREE_OPERAND (arg0, 0), + tem); + } + /* Same as abouve but for (A AND[-IF] (B AND-IF C)) - ((A AND B) AND-IF C), + or (A OR[-IF] (B OR-IF C) - ((A OR B) OR-IF C). */ + else if (TREE_CODE (arg1) == icode + simple_operand_p_2 (arg0) + /* Needed for sequence points to handle trappings, and +side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg1, 0))) + { + tem = fold_build2_loc (loc, ncode, type, +arg0, TREE_OPERAND (arg1, 0)); + return fold_build2_loc (loc, icode, type, tem, + TREE_OPERAND (arg1, 1)); + } + /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B) +into (A OR B). +For sequence point consistancy, we need to check for trapping, +and side-effects. */ + else if (code == icode simple_operand_p_2 (arg0) +simple_operand_p_2 (arg1)) + return fold_build2_loc (loc, ncode, type, arg0, arg1); +} + return NULL_TREE; } Regards, Kai
Re: [C++ Patch] PR 44524
On Mon, Oct 17, 2011 at 8:32 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi, here submitter requests a more accurate error message for X.Y where X is a pointer to class type. Thus the below, tested x86_64-linux. Ok for mainline? s/is of pointer type/has pointer type/g Thanks, Paolo. //
Re: [PATCH 3/6] Emit macro expansion related diagnostics
Finally here is what I am checking in, which passes bootstrap with --disable-checking --enable-languages=all,ada -- modulo this other bug that breaks bootstrap as well http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709. It's been OKed off-line by Tom Tromey. commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3 Author: Dodji Seketeli do...@redhat.com Date: Mon Oct 17 13:33:41 2011 +0200 Fix bootstrapping with --disable-checking libcpp/ChangeLog * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a variable without using it if ENABLE_CHECKING is not defined. Mark the LOCATION parameter as being unused. diff --git a/libcpp/line-map.c b/libcpp/line-map.c index 87b8bfe..43e2856 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -621,18 +621,16 @@ linemap_macro_expansion_map_p (const struct line_map *map) Read the comments of struct line_map and struct line_map_macro in line-map.h to understand what a macro expansion point is. */ -source_location +static source_location linemap_macro_map_loc_to_exp_point (const struct line_map *map, - source_location location) + source_location location ATTRIBUTE_UNUSED) { - unsigned token_no; - linemap_assert (linemap_macro_expansion_map_p (map) location = MAP_START_LOCATION (map)); /* Make sure LOCATION is correct. */ - token_no = location - MAP_START_LOCATION (map); - linemap_assert (token_no MACRO_MAP_NUM_MACRO_TOKENS (map)); + linemap_assert ((location - MAP_START_LOCATION (map)) + MACRO_MAP_NUM_MACRO_TOKENS (map)); return MACRO_MAP_EXPANSION_POINT_LOCATION (map); } -- Dodji
Re: [C++ Patch] PR 44524
On 10/17/2011 03:39 PM, Gabriel Dos Reis wrote: On Mon, Oct 17, 2011 at 8:32 AM, Paolo Carlinipaolo.carl...@oracle.com wrote: Hi, here submitter requests a more accurate error message for X.Y where X is a pointer to class type. Thus the below, tested x86_64-linux. Ok for mainline? s/is of pointer type/has pointer type/g Thanks, changed in my local tree. By the way, I wondered that, then found a couple of instances of 'has pointer' in the tree, but only in comments, if I remember correctly. Paolo.
Re: [patch] C6X unwinding/exception handling
I checked the attached patch, test results at http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html which are the same as with my suggested patch. Ok for the trunk? I probably don't have authority to approve this, but looks OK to me. Paul
Re: Out-of-order update of new_spill_reg_store[]
gcc/ * reload1.c (reload_regs_reach_end_p): Replace with... (reload_reg_rtx_reaches_end_p): ...this function. (new_spill_reg_store): Update commentary. (emit_input_reload_insns): Don't clear new_spill_reg_store here. (emit_output_reload_insns): Check reload_reg_rtx_reaches_end_p before setting new_spill_reg_store. (emit_reload_insns): Use a separate loop to clear new_spill_reg_store. Use reload_reg_rtx_reaches_end_p instead of reload_regs_reach_end_p. Also use reload_reg_rtx_reaches_end_p when recording inheritance information for non-spill reload registers. Just an update to say that based on our discussion I think the general approach is OK, but I'm still trying to figure out what exactly this piece of code is doing, and whether the changes to it make sense: @@ -8329,30 +8329,33 @@ emit_reload_insns (struct insn_chain *ch the storing insn so that we may delete this insn with delete_output_reload. */ src_reg = reload_reg_rtx_for_output[r]; - - /* If this is an optional reload, try to find the source reg - from an input reload. */ - if (! src_reg) + if (src_reg +reload_reg_rtx_reaches_end_p (src_reg, r)) + store_insn = new_spill_reg_store[REGNO (src_reg)]; + else { + /* If this is an optional reload, try to find the + source reg from an input reload. */ rtx set = single_set (insn); if (set SET_DEST (set) == rld[r].out) { int k; + rtx cand; src_reg = SET_SRC (set); store_insn = insn; for (k = 0; k n_reloads; k++) - { - if (rld[k].in == src_reg) - { - src_reg = reload_reg_rtx_for_input[k]; - break; - } - } + if (rld[k].in == src_reg) + { + cand = reload_reg_rtx_for_input[k]; + if (reload_reg_rtx_reaches_end_p (cand, k)) + { + src_reg = cand; + break; + } + } } } - else - store_insn = new_spill_reg_store[REGNO (src_reg)]; if (src_reg REG_P (src_reg) REGNO (src_reg) FIRST_PSEUDO_REGISTER) { Bernd
Re: [PATCH 0/9] [RFC] Expand SMS functionality
Roman Zhuykov zhr...@ispras.ru writes: [PATCH 4/9] Move the SMS pass earlier http://gcc.gnu.org/ml/gcc-patches/2011-07/msg01811.html I don't think this is a good idea. To get good results, SMS really needs to run as close to the register allocator as possible, otherwise later passes might disrupt the schedule. Richard
Re: [patch] C6X unwinding/exception handling
I checked the attached patch, test results at http://gcc.gnu.org/ml/gcc-testresults/2011-10/msg01377.html which are the same as with my suggested patch. Ok for the trunk? I probably don't have authority to approve this, but looks OK to me. The libobjc bits are Ok for trunk. Thanks
Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.
Thanks for inputs, Jakub! I am attaching updated patch. Updated testsuite/ChangeLog entry: 2011-10-17 Kirill Yukhin kirill.yuk...@intel.com * gcc.target/i386/avx2-vpop-check.h: New header. * gcc.target/i386/avx2-vpaddd-3.c: New test. * gcc.target/i386/avx2-vpaddw-3.c: Ditto. * gcc.target/i386/avx2-vpaddb-3.c: Ditto. * gcc.target/i386/avx2-vpaddq-3.c: Ditto. * gcc.target/i386/avx2-vpand-3.c: Ditto. * gcc.target/i386/avx2-vpmulld-3.c: Ditto. * gcc.target/i386/avx2-vpmullw-3.c: Ditto. * gcc.target/i386/avx2-vpsrad-3.c: Ditto. * gcc.target/i386/avx2-vpsraw-3.c: Ditto. * gcc.target/i386/avx2-vpsrld-3.c: Ditto. * gcc.target/i386/avx2-vpsrlw-3.c: Ditto. * gcc.target/i386/avx2-vpsubb-3.c: Ditto. * gcc.target/i386/avx2-vpsubd-3.c: Ditto. * gcc.target/i386/avx2-vpsubq-3.c: Ditto. * gcc.target/i386/avx2-vpsubw-3.c: Ditto. Could you please guys have a look? K avx2.vect-3.tests.gcc.patch Description: Binary data
Re: [PATCH] Simplify and fix restrict handling
On Fri, 14 Oct 2011, Richard Guenther wrote: This follows up Michas testcase where we fail to handle the conservatively propagated restrict tags properly. The following patch simplifies handling of restrict in the oracle and thus only excludes NONLOCAL (as designed), but not ESCAPED from conflict checking. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. So, after some regressions caused by this patch and some more thinking (about more possible issues) I concluded that we can simplify things even more by not making restrict vars point to NONLOCAL, but only to their tag (but marking that as global and able to have a points-to set). This way the special-casing of NONLOCAL vs. restrict can go away, and with it all its possible problems. Restrict is now similar to malloc () memory that escapes. Hopefully this one is without regressions ;) Bootstrap and regtest running on x86_64-unknown-linux-gnu. It seems we can remove DECL_IS_RESTRICTED_P again, as well as the internal is_restrict_var flag. I'll do that if the patch tests ok (the is_restrict_var flag removal immediately, the DECL_IS_RESTRICTED_P removal as followup as it touches Fortran). Thanks, Richard. 2011-10-17 Richard Guenther rguent...@suse.de * tree-ssa-alias.h (struct pt_solution): Remove vars_contains_restrict member. (pt_solutions_same_restrict_base): Remove. (pt_solution_set): Adjust. * tree-ssa-alias.c (ptr_deref_may_alias_decl_p): Remove vars_contains_restrict handling. (dump_points_to_solution): Likewise. (ptr_derefs_may_alias_p): Do not call pt_solutions_same_restrict_base. * tree-ssa-structalias.c (make_constraint_from_restrict): Make the tag global. (create_variable_info_for): Do not make restrict vars point to NONLOCAL. (intra_create_variable_infos): Likewise. (find_what_var_points_to): Remove vars_contains_restrict handling. (pt_solution_set): Adjust. (pt_solution_ior_into): Likewise. (pt_solutions_same_restrict_base): Remove. * cfgexpand.c (update_alias_info_with_stack_vars): Adjust. * gimple-pretty-print.c (pp_points_to_solution): Likewise. Index: gcc/tree-ssa-alias.c === --- gcc/tree-ssa-alias.c(revision 180077) +++ gcc/tree-ssa-alias.c(working copy) @@ -219,13 +219,6 @@ ptr_deref_may_alias_decl_p (tree ptr, tr if (!pi) return true; - /* If the decl can be used as a restrict tag and we have a restrict - pointer and that pointers points-to set doesn't contain this decl - then they can't alias. */ - if (DECL_RESTRICTED_P (decl) - pi-pt.vars_contains_restrict) -return bitmap_bit_p (pi-pt.vars, DECL_PT_UID (decl)); - return pt_solution_includes (pi-pt, decl); } @@ -316,11 +309,6 @@ ptr_derefs_may_alias_p (tree ptr1, tree if (!pi1 || !pi2) return true; - /* If both pointers are restrict-qualified try to disambiguate - with restrict information. */ - if (!pt_solutions_same_restrict_base (pi1-pt, pi2-pt)) -return false; - /* ??? This does not use TBAA to prune decls from the intersection that not both pointers may access. */ return pt_solutions_intersect (pi1-pt, pi2-pt); @@ -426,8 +414,6 @@ dump_points_to_solution (FILE *file, str dump_decl_set (file, pt-vars); if (pt-vars_contains_global) fprintf (file, (includes global vars)); - if (pt-vars_contains_restrict) - fprintf (file, (includes restrict tags)); } } Index: gcc/tree-ssa-alias.h === --- gcc/tree-ssa-alias.h(revision 180077) +++ gcc/tree-ssa-alias.h(working copy) @@ -54,8 +54,6 @@ struct GTY(()) pt_solution /* Nonzero if the pt_vars bitmap includes a global variable. */ unsigned int vars_contains_global : 1; - /* Nonzero if the pt_vars bitmap includes a restrict tag variable. */ - unsigned int vars_contains_restrict : 1; /* Set of variables that this pointer may point to. */ bitmap vars; @@ -130,10 +128,8 @@ extern bool pt_solution_singleton_p (str extern bool pt_solution_includes_global (struct pt_solution *); extern bool pt_solution_includes (struct pt_solution *, const_tree); extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *); -extern bool pt_solutions_same_restrict_base (struct pt_solution *, -struct pt_solution *); extern void pt_solution_reset (struct pt_solution *); -extern void pt_solution_set (struct pt_solution *, bitmap, bool, bool); +extern void pt_solution_set (struct pt_solution *, bitmap, bool); extern void pt_solution_set_var (struct pt_solution *, tree); extern void dump_pta_stats (FILE *); Index: gcc/tree-ssa-structalias.c === ---
[Patch,AVR] Housekeeping insn attributes remove assembler dialect
This is more code clean-up for insn attributes. It removes mcu_have_movw, mcu_mega and defines enabled and isa attributes instead. The isa attribute which triggers enabled is a replacement for AVR_HAVE_MOVW assembler dialect. We don't actually support assembler dialects but different ISAs so that an attribute that reflects ISA capabilities seems more appropriate and straight forward here. Moreover, it's easier to write down different instruction lengths (which wouldn't occur if it was just an assembler dialect). The only notable change is to epilogue_restores: (set (reg:HI REG_Y) (plus:HI (reg:HI REG_Y) (match_operand:HI 0 immediate_operand i,i))) - (set (reg:HI REG_SP) -(reg:HI REG_Y)) + (set (reg:HI REG_SP) +(plus:HI (reg:HI REG_Y) + (match_dup 0))) The original code does not quite represent what the insn does: A PARALLEL's actions all happen simultaneously, not in sequence. Thus, an instruction sequence like Y = Y + const SP = Y has wo be written in RTL as Y = Y + const SP = Y + const and *not* as Y = Y + const SP = Y The patch passes without regressions. The patch passes with 2 more regressions if the test suite is run with -mcall-prologues, but the two additional run-fails ./gcc.dg/sibcall-3.c ./gcc.dg/sibcall-4.c are because tail call optimization is turned off at -mcall-prologues, see avr.c:avr_function_ok_for_sibcall(). Ok for trunk? Johann * config/avr/avr.h (ASSEMBLER_DIALECT): Remove. * config/avr/avr.md (mcu_have_movw, mcu_mega): Remove attributes. (adjust_len): Add alternative call. (isa, enabled): New insn attributes. (length): Use match_test with AVR_HAVE_JMP_CALL instead of mcu_mega attribute. (*sbrx_branchmode): Ditto. (*sbrx_and_branchmode): Ditto. (*sbix_branch): Ditto. (*sbix_branch_bit7): Ditto. (*sbix_branch_tmp): Ditto. (*sbix_branch_tmp_bit7): Ditto. (jump): Ditto. (negsi2): Use attribute isa instead of assembler dialect. (extendhisi2): Ditto. (call_insn, call_value_insn): Set adjust_len attribute. (indirect_jump): Indent to coding rules. (call_prologue_saves): Use isa attribute instead of mcu_mega. (epilogue_restores): Ditto. Fix setting of SP as described in the RTX pattern. (*indirect_jump): Fusion of *jcindirect_jump, *njcindirect_jump and *indirect_jump_avr6. (*tablejump): Fusion of *tablejump_rjmp and *tablejump_lib. (*jcindirect_jump, *njcindirect_jump, *indirect_jump_avr6): Remove. (*tablejump_rjmp, *tablejump_lib): Remove. * config/avr/avr.c (adjust_insn_length): Handle ADJUST_LEN_CALL. Index: config/avr/avr.md === --- config/avr/avr.md (revision 180076) +++ config/avr/avr.md (working copy) @@ -84,17 +84,6 @@ (define_attr cc none,set_czn,set_zn,s (define_attr type branch,branch1,arith,xcall (const_string arith)) -(define_attr mcu_have_movw yes,no - (const (if_then_else (symbol_ref AVR_HAVE_MOVW) - (const_string yes) - (const_string no - -(define_attr mcu_mega yes,no - (const (if_then_else (symbol_ref AVR_HAVE_JMP_CALL) - (const_string yes) - (const_string no - - ;; The size of instructions in bytes. ;; XXX may depend from cc @@ -124,7 +113,7 @@ (define_attr length (const_int 3) (const_int 4))) (eq_attr type xcall) - (if_then_else (eq_attr mcu_mega no) + (if_then_else (match_test !AVR_HAVE_JMP_CALL) (const_int 1) (const_int 2))] (const_int 2))) @@ -133,11 +122,10 @@ (define_attr length ;; Following insn attribute tells if and how the adjustment has to be ;; done: ;; no No adjustment needed; attribute length is fine. -;; yesAnalyse pattern in adjust_insn_length by hand. ;; Otherwise do special processing depending on the attribute. (define_attr adjust_len - out_bitop, out_plus, addto_sp, tsthi, tstsi, compare, + out_bitop, out_plus, addto_sp, tsthi, tstsi, compare, call, mov8, mov16, mov32, reload_in16, reload_in32, ashlqi, ashrqi, lshrqi, ashlhi, ashrhi, lshrhi, @@ -145,6 +133,50 @@ (define_attr adjust_len no (const_string no)) +;; Flavours of instruction set architecture (ISA), used in enabled attribute + +;; mov: ISA has no MOVW +;; movw: ISA has MOVW +;; rjmp: ISA has no CALL/JMP +;; jmp: ISA has CALL/JMP +;; ijmp: ISA has no EICALL/EIJMP +;; eijmp: ISA has EICALL/EIJMP + +(define_attr isa + mov,movw, rjmp,jmp, ijmp,eijmp, + standard + (const_string standard)) + +(define_attr enabled + (cond [(eq_attr isa standard) + (const_int 1) + + (and (eq_attr isa mov) + (match_test !AVR_HAVE_MOVW)) + (const_int 1) + +
Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.
On Mon, Oct 17, 2011 at 06:27:04PM +0400, Kirill Yukhin wrote: Thanks for inputs, Jakub! I am attaching updated patch. Updated testsuite/ChangeLog entry: 2011-10-17 Kirill Yukhin kirill.yuk...@intel.com * gcc.target/i386/avx2-vpop-check.h: New header. * gcc.target/i386/avx2-vpaddd-3.c: New test. * gcc.target/i386/avx2-vpaddw-3.c: Ditto. * gcc.target/i386/avx2-vpaddb-3.c: Ditto. * gcc.target/i386/avx2-vpaddq-3.c: Ditto. * gcc.target/i386/avx2-vpand-3.c: Ditto. * gcc.target/i386/avx2-vpmulld-3.c: Ditto. * gcc.target/i386/avx2-vpmullw-3.c: Ditto. * gcc.target/i386/avx2-vpsrad-3.c: Ditto. * gcc.target/i386/avx2-vpsraw-3.c: Ditto. * gcc.target/i386/avx2-vpsrld-3.c: Ditto. * gcc.target/i386/avx2-vpsrlw-3.c: Ditto. * gcc.target/i386/avx2-vpsubb-3.c: Ditto. * gcc.target/i386/avx2-vpsubd-3.c: Ditto. * gcc.target/i386/avx2-vpsubq-3.c: Ditto. * gcc.target/i386/avx2-vpsubw-3.c: Ditto. Could you please guys have a look? This is ok for the trunk. Jakub
RE: [Patch,AVR] Print no-return functions as JMP
There is no real post morten debugging on AVR as there is nothing like core dump. But if there is any kind of debugging at all, you might use the debugger to put a breakpoint in abort(), and if so, having that invoked via jmp means you don't know who called it. So you'd want a way to suppress that optimization. paul
Re: [patch tree-optimization]: Improve handling of conditional-branches on targets with high branch costs
On Mon, Oct 17, 2011 at 3:36 PM, Kai Tietz ktiet...@googlemail.com wrote: Sure, Is simplier and also handles (A T[-IF] (B T-IF C) - (A T B) T-IF C case, which can happen by framing in conditions. @@ -8380,13 +8400,65 @@ fold_truth_andor (location_t loc, enum t lhs is another similar operation, try to merge its rhs with our rhs. Then try to merge our lhs and rhs. */ if (TREE_CODE (arg0) == code - 0 != (tem = fold_truthop (loc, code, type, - TREE_OPERAND (arg0, 1), arg1))) + 0 != (tem = fold_truth_andor_1 (loc, code, type, + TREE_OPERAND (arg0, 1), arg1))) return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), tem); - if ((tem = fold_truthop (loc, code, type, arg0, arg1)) != 0) + if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0) return tem; + if ((BRANCH_COST (optimize_function_for_speed_p (cfun), + false) = 2) + LOGICAL_OP_NON_SHORT_CIRCUIT) + { + enum tree_code ncode, icode; + + ncode = (code == TRUTH_ANDIF_EXPR || code == TRUTH_AND_EXPR) + ? TRUTH_AND_EXPR : TRUTH_OR_EXPR; + icode = ncode == TRUTH_AND_EXPR ? TRUTH_ANDIF_EXPR : TRUTH_ORIF_EXPR; + + /* Transform ((A AND-IF B) AND[-IF] C) into (A AND-IF (B AND C)), + or ((A OR-IF B) OR[-IF] C) into (A OR-IF (B OR C)) + We don't want to pack more than two leafs to a non-IF AND/OR + expression. + If tree-code of left-hand operand isn't an AND/OR-IF code and not + equal to IF-CODE, then we don't want to add right-hand operand. + If the inner right-hand side of left-hand operand has + side-effects, or isn't simple, then we can't add to it, + as otherwise we might destroy if-sequence. */ + if (TREE_CODE (arg0) == icode + simple_operand_p_2 (arg1) + /* Needed for sequence points to handle trappings, and + side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg0, 1))) + { + tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1), + arg1); + return fold_build2_loc (loc, icode, type, TREE_OPERAND (arg0, 0), + tem); + } + /* Same as abouve but for (A AND[-IF] (B AND-IF C)) - ((A AND B) AND-IF C), + or (A OR[-IF] (B OR-IF C) - ((A OR B) OR-IF C). */ + else if (TREE_CODE (arg1) == icode + simple_operand_p_2 (arg0) + /* Needed for sequence points to handle trappings, and + side-effects. */ + simple_operand_p_2 (TREE_OPERAND (arg1, 0))) + { + tem = fold_build2_loc (loc, ncode, type, + arg0, TREE_OPERAND (arg1, 0)); + return fold_build2_loc (loc, icode, type, tem, + TREE_OPERAND (arg1, 1)); + } + /* Transform (A AND-IF B) into (A AND B), or (A OR-IF B) + into (A OR B). + For sequence point consistancy, we need to check for trapping, + and side-effects. */ + else if (code == icode simple_operand_p_2 (arg0) + simple_operand_p_2 (arg1)) + return fold_build2_loc (loc, ncode, type, arg0, arg1); + } + return NULL_TREE; } Ok with the rest of the changes I requested. Richard. Regards, Kai
FYI: minor fix in gcc/configure
I'm checking this in as obvious. Sergio pointed out, via this patch, that gcc/configure didn't properly emit whether sys/std.h was discovered. Tested by re-running configure and examining the output. Tom 2011-10-17 Sergio Durigan Junior sergi...@redhat.com * configure.ac: Display `yes' if the SystemTap header has been found. * configure: Regenerate. Index: configure.ac === --- configure.ac(revision 180092) +++ configure.ac(working copy) @@ -4578,6 +4578,7 @@ AC_MSG_CHECKING(sys/sdt.h in the target C library) have_sys_sdt_h=no if test -f $target_header_dir/sys/sdt.h; then + have_sys_sdt_h=yes AC_DEFINE(HAVE_SYS_SDT_H, 1, [Define if your target C library provides sys/sdt.h]) fi
Re: [Patch,AVR] Print no-return functions as JMP
paul_kon...@dell.com schrieb: There is no real post morten debugging on AVR as there is nothing like core dump. But if there is any kind of debugging at all, you might use the debugger to put a breakpoint in abort(), and if so, having that invoked via jmp means you don't know who called it. So you'd want a way to suppress that optimization. paul Regarding the number of objections, I think it's best to drop this patch. The clean-up to the call insns is contained in http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01530.html Thanks for all of your feedback! Johann
Re: [PATCH, i386 tests] New tests to check vectorization for AVX2 insns.
Thanks, guys, could anybody please commit that? K On Mon, Oct 17, 2011 at 6:33 PM, Jakub Jelinek ja...@redhat.com wrote: On Mon, Oct 17, 2011 at 06:27:04PM +0400, Kirill Yukhin wrote: Thanks for inputs, Jakub! I am attaching updated patch. Updated testsuite/ChangeLog entry: 2011-10-17 Kirill Yukhin kirill.yuk...@intel.com * gcc.target/i386/avx2-vpop-check.h: New header. * gcc.target/i386/avx2-vpaddd-3.c: New test. * gcc.target/i386/avx2-vpaddw-3.c: Ditto. * gcc.target/i386/avx2-vpaddb-3.c: Ditto. * gcc.target/i386/avx2-vpaddq-3.c: Ditto. * gcc.target/i386/avx2-vpand-3.c: Ditto. * gcc.target/i386/avx2-vpmulld-3.c: Ditto. * gcc.target/i386/avx2-vpmullw-3.c: Ditto. * gcc.target/i386/avx2-vpsrad-3.c: Ditto. * gcc.target/i386/avx2-vpsraw-3.c: Ditto. * gcc.target/i386/avx2-vpsrld-3.c: Ditto. * gcc.target/i386/avx2-vpsrlw-3.c: Ditto. * gcc.target/i386/avx2-vpsubb-3.c: Ditto. * gcc.target/i386/avx2-vpsubd-3.c: Ditto. * gcc.target/i386/avx2-vpsubq-3.c: Ditto. * gcc.target/i386/avx2-vpsubw-3.c: Ditto. Could you please guys have a look? This is ok for the trunk. Jakub
[v3] Remove duplicate symbol in gnu.ver (PR bootstrap/50715)
As described in the PR, after Benjamin's patch 2011-10-10 Benjamin Kosnik b...@redhat.com PR libstdc++/49818 * config/abi/pre/gnu.ver (CXXABI_1.3.6): Add symbols. __cxa_get_exception_ptr is now present in both CXXABI_1.3.1 and CXXABI_1.3.6, which breaks Solaris bootstrap with Sun ld: ld: fatal: libstdc++-symbols.ver-sun: 5210: symbol '__cxa_get_exception_ptr' is already defined in file: libstdc++-symbols.ver-sun: symbol version conflict The duplicate serves no purpose I can see, and even with both entries, __cxa_get_exception_ptr is bound to version CXXABI_1.3.1 only on x86_64-unknown-linux-gnu, so it seems safe to remove. This restores Solaris bootstrap, tested on i386-pc-solaris2.11. Ok for mainline? Rainer 2011-10-14 Rainer Orth r...@cebitec.uni-bielefeld.de PR bootstrap/50715 * config/abi/pre/gnu.ver (CXXABI_1.3.6): Remove duplicate __cxa_get_exception_ptr. # HG changeset patch # Parent fbadbe8d19738b0abbefa6cabc32fa04d969b09f Remove duplicate symbol in gnu.ver (PR bootstrap/50715) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1496,7 +1496,6 @@ CXXABI_1.3.6 { __cxa_allocate_dependent_exception; __cxa_free_dependent_exception; -__cxa_get_exception_ptr; __cxa_deleted_virtual; -} CXXABI_1.3.5; \ No newline at end of file +} CXXABI_1.3.5; -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [v3] Remove duplicate symbol in gnu.ver (PR bootstrap/50715)
On 10/17/2011 05:00 PM, Rainer Orth wrote: As described in the PR, after Benjamin's patch 2011-10-10 Benjamin Kosnikb...@redhat.com PR libstdc++/49818 * config/abi/pre/gnu.ver (CXXABI_1.3.6): Add symbols. __cxa_get_exception_ptr is now present in both CXXABI_1.3.1 and CXXABI_1.3.6, which breaks Solaris bootstrap with Sun ld: ld: fatal: libstdc++-symbols.ver-sun: 5210: symbol '__cxa_get_exception_ptr' is already defined in file: libstdc++-symbols.ver-sun: symbol version conflict The duplicate serves no purpose I can see, and even with both entries, __cxa_get_exception_ptr is bound to version CXXABI_1.3.1 only on x86_64-unknown-linux-gnu, so it seems safe to remove. This restores Solaris bootstrap, tested on i386-pc-solaris2.11. Ok for mainline? Ok, thanks. Paolo.
Re: [PATCH, alpha, v2]: Fix PR target/50737, FAIL: Throw_3 -O3 execution
On 10/16/2011 01:45 PM, Uros Bizjak wrote: libgcc/ChangeLog: 2011-10-16 Uros Bizjak ubiz...@gmail.com Eric Botcazou ebotca...@adacore.com PR target/50737 * config/alpha/linux-unwind.h (alpha_fallback_frame_state): Set fs-signal_frame to 1. libjava/ChangeLog: 2011-10-16 Uros Bizjak ubiz...@gmail.com Eric Botcazou ebotca...@adacore.com PR target/50737 * include/dwarf2-signal.h [__alpha__]: Remove MAKE_THROW_FRAME definition. Ok. r~
Re: [Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit
On Mon, Oct 17, 2011 at 15:49, Tobias Burnus bur...@net-b.de wrote: This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine and the FLUSH statement. It removes the _commit from gfortran's buf_flush. Like I argued in this message http://gcc.gnu.org/ml/fortran/2011-10/msg00094.html , I think this is a gross mistake. libgfortran should not require _commit nor fsync in any situation. Those calls are useful for writing databases and other applications which must make data integrity guarantees, and are prepared to pay the performance cost associated with it. It's absolutely not something a language support library should do unless the language spec explicitly requires such data integrity guarantees. Background: * gfortran internally buffers the I/O, but it calls the nonbuffering open/write/read functions (and not, e.g., fopen/fwrite/fread). On Unix/Darwin/Linux system, the changes become immediately visible to all processes after a write(). * On Windows, there seems to be a file-descriptor specific system buffer such that the data only becomes available to other processes after either a _commit or after closing the file. * The Windows _commit() is a combination of a (system) buffer flush - as a write() already implies on POSIX systems, but it also ensures that the data ends up on the disk, which on POSIX systems is done via fsync(). I admit I'm somewhat confused by this issue, and despite extensive googling I haven't been able to come up with a clear explanation for the behavior seen in PR 44698. That write() would be buffered on windows makes no sense to me, and I have found no documentation supporting this view. The closest what I found was in the documentation for WriteFile and WriteFileEX (which are the win32 native calls, write() is a wrapper around one of these (the EX version if available, presumably)): http://msdn.microsoft.com/en-us/library/windows/desktop/aa365748%28v=vs.85%29.aspx When writing to a file, the last write time is not fully updated until all handles used for writing have been closed. Therefore, to ensure an accurate last write time, close the file handle immediately after writing to the file. This suggests that metadata updates are not immediately visible to other processes. Or at least it makes sense that it would update the size at the same time it updates the last write time. Which would explain why stat(/path/to/file) would show an old size, as the size hasn't necessarily yet been updated to the directory, although the file data itself is already transferred to the OS. And, while I haven't checked it, it would make sense that fstat(fd,...) would return the up to date info, as that would just need to check the data via the process local file descriptor table rather than looking up the metadata via the directory. That is, I guess that the implementation is something along the lines of these calls not being buffered (thus no data lost if the process crashes after WriteFile(EX)), but the kernel maintains a per-handle metadata cache which is flushed to the filesystem during batched metadata updates (and close(), _commit(), or process ending), at which point it also becomes visible to other processes. Or something like that. That being said, this is just me speculating. If someone knows better, please feel free to share. And, while I'm at it, this kind of relaxed consistency is not unheard of in the unix world either. Consider NFS, where data and metadata may not be flushed to the server until fsync() or close() is called, or the attribute cache timeout forces the writeout(?), and thus it's possible for clients to have an inconsistent view of a file. In both cases the remedy is the same; if this kind of consistency matters, the user should close the file or fsync()/_commit() before expecting that the OS metadata is consistent. I think that's a better option than sprinkling _commit() all over the library. So I would rather prefer my own patch from the URL above. Also, I think it would be nice if we could get this fix into 4.6.2.. -- Janne Blomqvist
Re: [Patch, Fortran ] PR 50016: Slow Fortran I/O on Windows and flushing/_commit
Hi Janne, On 10/17/2011 05:30 PM, Janne Blomqvist wrote: On Mon, Oct 17, 2011 at 15:49, Tobias Burnusbur...@net-b.de wrote: This patch adds a call to _commit() on _WIN32 for the FLUSH subroutine and the FLUSH statement. It removes the _commit from gfortran's buf_flush. Like I argued in this message http://gcc.gnu.org/ml/fortran/2011-10/msg00094.html, I think this is a gross mistake. [...] And I think it is a mistake to not make the data available to other processes as it is indicated by the Fortran 2008 standard: Execution of a FLUSH statement causes data written to an external le to be available to other processes, or causes data placed in an external file by means other than Fortran to be available to a READ statement. These actions are processor dependent. Thus, I think it makes sense for FLUSH to call _commit on Windows. If you don't want to have a slow down: Simply do not call FLUSH. libgfortran should not require _commit nor fsync in any situation. Those calls are useful for writing databases and other applications which must make data integrity guarantees, and are prepared to pay the performance cost associated with it. It's absolutely not something a language support library should do unless the language spec explicitly requires such data integrity guarantees. Well, Fortran does not need to write the data to the file, however, the purpose of FLUSH is that I can, e.g., run execute_command_line with the file the program just has written. It will work on Unix/Linux but not on MinGW/MinGW-w64 without a _commit (or without closing the file). That write() would be buffered on windows makes no sense to me Why shouldn't it be buffed? Typical Windows programs open files with an exclusive lock and as Windows never had the pipes and many small programs as Unix did, having a per-file-descriptor buffer is easier to implement, avoids multi-thread issues and is potentially faster. If a program wants to make the data available, it can just _commit it or close the file handle - that way one also has a perfect data integrity. And, while I'm at it, this kind of relaxed consistency is not unheard of in the unix world either. Consider NFS, where data and metadata may not be flushed to the server until fsync() or close() is called, or the attribute cache timeout forces the writeout(?), and thus it's possible for clients to have an inconsistent view of a file. Well, most of the time it works well on the same system: If I call execute_command_line, the data is up to date. The issue with NFS only occurs if I want to access the data remotely, which is another issue. If one wants to do that, one can use a parallel access with, e.g., HDF5 or MPIv2 or the Coarray TS (to be written and implemented). In both cases the remedy is the same; if this kind of consistency matters, the user should close the file or fsync()/_commit() before expecting that the OS metadata is consistent. I think that's a better option than sprinkling _commit() all over the library. No, for the required consistency, FLUSH is enough (including calling _commit on MinGW/MinGW-w64). It makes sure that if the program crashes, the data is still there, it makes the data available for other processes. Only if one wants to have complete integrity, one can call fsync. However, with NFS, Lustre et al., I am not 100% sure that the data is immediately available on all other clients after fsync returned. So I would rather prefer my own patch from the URL above. Also, I think it would be nice if we could get this fix into 4.6.2.. I also would like to see this fixed for 4.6.2. However, a prerequisite is that we agree on how to implement it. Regarding your patch: I think it does not solve the FLUSH issue. For the file size itself, I think the patch is okay, but frankly, I think for the performance it does not really matter which approach is taken. And I do not like the test-suite part of your patch. Tobias
Re: [Patch, Fortran] PR 47023: [4.6/4.7 regression] C_Sizeof: Rejects valid code
Be quick, though; 4.6.2 was due for release now-ish - I know. I'll try to do it today. Alrighty. Just committed the patch to the 4.6 branch as r180099. Seems we're down to three open Fortran front-end regressions for the upcoming 4.6.2 release: * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42954 (TARGET_*_CPP_BUILDINS issues with gfortran) * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50410 (ICE in record_reference) * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50684 (Incorrect error for move_alloc on element inside intent(in) pointer) I had started on the third one, but I'm not sure if I can finish it in time for 4.6.2. Anyone up to looking at the second one? Cheers, Janus On Sun, Oct 16, 2011 at 9:45 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi Paul, This is OK for trunk. Thanks fo rthe patch. Thanks. Committed to trunk as r180062. What about 4.6? Cheers, Janus On Sun, Oct 16, 2011 at 2:58 PM, Janus Weil ja...@gcc.gnu.org wrote: Hi all, here is a patch which fixes the regression in comment #2 of the PR in the subject line. What it does is setting the 'ts.is_c_interop' flag correctly for constants with kind-parameter specification (such as '0.0_c_double'), as is already being done for variables. Regtested on x86_64-unknown-linux-gnu. Ok for trunk and 4.6? Cheers, Janus 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * primary.c (match_kind_param): Detect ISO_C_BINDING kinds. (get_kind): Pass on 'is_iso_c' flag. (match_integer_constant,match_real_constant,match_logical_constant): Set 'ts.is_c_interop'. 2011-10-16 Janus Weil ja...@gcc.gnu.org PR fortran/47023 * gfortran.dg/c_kind_tests_3.f03: New. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH, ARM] Unaligned accesses for builtin memcpy [2/2]
Hi Julian, There are a couple of minor formatting nits. +static int +arm_movmemqi_unaligned (rtx *operands) + /* Inlined memcpy using ldr/str/ldrh/strh can be quite big: try to limit + size of code if optimizing for size. We'll use ldm/stm if src_aligned + or dst_aligned though: allow more interleaving in those cases since the + resulting code can be smaller. */ Watch out the or being misaligned compared to the other text. + /* Note that the loop created by arm_block_move_unaligned_loop may be + subject to loop unrolling, which makes tuning this condition a little + redundant. */ Same with `redundant' On 13 October 2011 18:53, Julian Brown jul...@codesourcery.com wrote: On Wed, 28 Sep 2011 14:33:17 +0100 No, sorry, I don't have any benchmark results available at present. I think we'd have to have terrifically bad luck for it to be a performance degradation, though... Hmmm OK - but please watch out for any bug reports or any test-suite fallout this week with multilibs other than what you might have tested. I re-tested the patch for good measure, in case of bitrot (and the new tests pass with the patch applied, of course). OK to apply now? OK by me with those changes. cheers Ramana
Re: [Patch,AVR] Housekeeping insn attributes remove assembler dialect
2011/10/17 Georg-Johann Lay a...@gjlay.de: This is more code clean-up for insn attributes. It removes mcu_have_movw, mcu_mega and defines enabled and isa attributes instead. The isa attribute which triggers enabled is a replacement for AVR_HAVE_MOVW assembler dialect. We don't actually support assembler dialects but different ISAs so that an attribute that reflects ISA capabilities seems more appropriate and straight forward here. Moreover, it's easier to write down different instruction lengths (which wouldn't occur if it was just an assembler dialect). The only notable change is to epilogue_restores: (set (reg:HI REG_Y) (plus:HI (reg:HI REG_Y) (match_operand:HI 0 immediate_operand i,i))) - (set (reg:HI REG_SP) - (reg:HI REG_Y)) + (set (reg:HI REG_SP) + (plus:HI (reg:HI REG_Y) + (match_dup 0))) The original code does not quite represent what the insn does: A PARALLEL's actions all happen simultaneously, not in sequence. Thus, an instruction sequence like Y = Y + const SP = Y has wo be written in RTL as Y = Y + const SP = Y + const and *not* as Y = Y + const SP = Y The patch passes without regressions. The patch passes with 2 more regressions if the test suite is run with -mcall-prologues, but the two additional run-fails ./gcc.dg/sibcall-3.c ./gcc.dg/sibcall-4.c are because tail call optimization is turned off at -mcall-prologues, see avr.c:avr_function_ok_for_sibcall(). Ok for trunk? Approved. Denis.
Re: [PATCH] fortran/50514 -- Fix static chekcing of ISHFT[C] arguments.
On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote: On 10/15/2011 11:21 PM, Steve Kargl wrote: OK for trunk? 2011-10-15 Steven G. Karglka...@gcc.gnu.org * gfortran.dg/ishft_3.f90: Update test. I am not so happy with complete test replacements. How about adding it as new test case? Well, the old testcase is %cat ishft_3.f90 ! { dg-do compile } program ishft_3 integer i, j write(*,*) ishftc( 3, 2, 3 ) write(*,*) ishftc( 3, 2, i ) write(*,*) ishftc( 3, i, j ) write(*,*) ishftc( 3, 128 ) ! { dg-error exceeds BIT_SIZE of first } write(*,*) ishftc( 3, 0, 128 ) ! { dg-error exceeds BIT_SIZE of first } write(*,*) ishftc( 3, 0, 0 )! { dg-error Invalid third argument } write(*,*) ishftc( 3, 3, 2 )! { dg-error exceeds third argument } end program 1) It's misnamed. There is no testing of ishft. 2) i and j are undefined, so lines 4 and 5 are invalid. Even if i and j were defined, there is nothing special about those lines in that gfortran generates a function call to a runtime routine. Note, this a dg-do compile test. 3) The four dg-error strings would need to be updated to the new error messages. The only line that would survive is the first line, which is covered in the updated testcase. 2011-10-15 Steven G. Karglka...@gcc.gnu.org * check.c (less_than_bitsize1): Check|shift| = bit_size(i). (gfc_check_ishftc): Check|shift| = bit_size(i) and check that size is positive. I somehow find less_than_bitsize1's + if (strncmp (arg2, ISHFT, 5) == 0) not that elegant and would prefer another argument, which tells the function that it should take the absolute value of the argument; however, given that ISHFT seems to be the only function which allows negative arguments, one could also leave it. In looking at the other uses of less_than_bitsize1() I could pass arg2=NULL for ISHFT[C], and then the code would become if (arg2 == NULL) { /* Special case for ISHFT[C]. */ Would that be better? -- Steve
Re: [PATCH 3/6] Emit macro expansion related diagnostics
On Mon, Oct 17, 2011 at 6:41 AM, Dodji Seketeli do...@redhat.com wrote: Finally here is what I am checking in, which passes bootstrap with --disable-checking --enable-languages=all,ada -- modulo this other bug that breaks bootstrap as well http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50709. It's been OKed off-line by Tom Tromey. commit c1cd2be336ceec75cf40ac5f32cc4f72b8fc5da3 Author: Dodji Seketeli do...@redhat.com Date: Mon Oct 17 13:33:41 2011 +0200 Fix bootstrapping with --disable-checking libcpp/ChangeLog * line-map.c (linemap_macro_map_loc_to_exp_point): Avoid setting a variable without using it if ENABLE_CHECKING is not defined. Mark the LOCATION parameter as being unused. There are at least 2 bootstrap problems: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50760 -- H.J.
[PATCH] Handle USING_MADVISE without USING_MMAP
I plan to check in the following patch as obvious after it passed testing. cygwin has a MADV_DONTNEED, but does not use mmap. The ifdefs for madvise assumed this wouldn't happen and it broke the cygwin build. Just don't set USING_MADVISE when USING_MMAP is not set. Thanks to Kai Titz for testing. -Andi 2011-10-17 Andi Kleen a...@linux.intel.com * ggc-page.c (USING_MADVISE): Adjust ifdef to check for USING_MMAP. diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c index 9b35291..2da99db 100644 --- a/gcc/ggc-page.c +++ b/gcc/ggc-page.c @@ -50,7 +50,7 @@ along with GCC; see the file COPYING3. If not see #define USING_MALLOC_PAGE_GROUPS #endif -#if defined(HAVE_MADVISE) defined(MADV_DONTNEED) +#if defined(HAVE_MADVISE) defined(MADV_DONTNEED) defined(USING_MMAP) # define USING_MADVISE #endif -- 1.7.5.4 -- a...@linux.intel.com -- Speaking for myself only.
Re: [C++ Patch] PR 32614
On 10/17/2011 07:48 AM, Paolo Carlini wrote: And please help re-assessing the situation wrt the other front-ends *today* not in the stone age. EDG always wraps diagnostics at ~80 columns. clang wraps diagnostics at $COLUMNS when stderr is going to a terminal, and doesn't wrap otherwise. The clang behavior seems like the right way to go. Jason
[C++ Patch] PR 50757
Hi, exactly like the recently fixed c++/17212. Tested x86_64-linux. Ok for mainline? Thanks, Paolo. / /gcc 2011-10-17 Paolo Carlini paolo.carl...@oracle.com PR c++/50757 * c-family/c.opt ([Wnonnull]): Add C++ and Objective-C++. * doc/invoke.texi: Update. /testsuite 2011-10-17 Paolo Carlini paolo.carl...@oracle.com PR c++/50757 * g++.dg/warn/format7.C: New. * obj-c++.dg/warn7.mm: Likewise. Index: doc/invoke.texi === --- doc/invoke.texi (revision 180100) +++ doc/invoke.texi (working copy) @@ -3223,7 +3222,7 @@ Enable @option{-Wformat} plus format checks not in @option{-Wformat}. Currently equivalent to @samp{-Wformat -Wformat-nonliteral -Wformat-security -Wformat-y2k}. -@item -Wnonnull @r{(C and Objective-C only)} +@item -Wnonnull @opindex Wnonnull @opindex Wno-nonnull Warn about passing a null pointer for arguments marked as Index: c-family/c.opt === --- c-family/c.opt (revision 180100) +++ c-family/c.opt (working copy) @@ -510,7 +510,7 @@ C++ ObjC++ Var(warn_nonvdtor) Warning Warn about non-virtual destructors Wnonnull -C ObjC Var(warn_nonnull) Warning +C ObjC C++ ObjC++ Var(warn_nonnull) Warning Warn about NULL being passed to argument slots marked as requiring non-NULL Wnormalized= Index: testsuite/g++.dg/warn/format7.C === --- testsuite/g++.dg/warn/format7.C (revision 0) +++ testsuite/g++.dg/warn/format7.C (revision 0) @@ -0,0 +1,10 @@ +// PR c++/50757 +// { dg-options -Wformat -Wno-nonnull } + +extern void *f (void *__s) __attribute__ ((__nonnull__ (1))); + +int main() +{ + void* const s = 0; + f(s); +} Index: testsuite/obj-c++.dg/warn7.mm === --- testsuite/obj-c++.dg/warn7.mm (revision 0) +++ testsuite/obj-c++.dg/warn7.mm (revision 0) @@ -0,0 +1,10 @@ +// PR c++/50757 +// { dg-options -Wformat -Wno-nonnull } + +extern void *f (void *__s) __attribute__ ((__nonnull__ (1))); + +int main() +{ + void* const s = 0; + f(s); +}
Re: [C++ Patch] PR 32614
On 10/17/2011 07:31 PM, Jason Merrill wrote: clang wraps diagnostics at $COLUMNS when stderr is going to a terminal, and doesn't wrap otherwise. The clang behavior seems like the right way to go. Thanks Jason. I'll see how to implement this. Paolo.
Re: [C++ Patch] PR 50757
OK. Jason
Re: [C++ Patch] PR 44524
OK. Jason