Re: [PR c/52952] More precise locations within format strings
On 7 November 2014 22:39, Joseph Myers jos...@codesourcery.com wrote: Neither Per nor Tom are active in GCC anymore. If the FE maintainers do not feel comfortable reviewing line-map changes, could you nominate Dodji as line-map maintainer if he is willing to accept it? I think he is currently the person that understands that code best. I think Dodji as diagnostics maintainer is better placed than I am to review line-map patches. Then, do you mean that line-map falls under the reach of the diagnostics maintainer? I agree, but Dodji himself does not seem to be sure about this: https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02444.html It would be helpful if one of the FE maintainers clarified this once and for all. Cheers, Manuel.
Re: [GRAPHITE, PATCH] Loop unroll and jam optimization
On Sat, Nov 08, 2014 at 12:32:05AM +0100, Mircea Namolaru wrote: Hello, This is the patch for unroll and jam optimizations. It is based on the code written by Tobias from graphite-optimize-isl.c (the code was unreachable till now) extended and enabled it via a new option -floop-unroll-jam. The patch takes advantage of the new isl based code generator introduced recently in GCC (in fact of the possible options for building the AST). The code generated for this optimization in the case of non-constant loop bounds initially looks as below. This is not very useful because the standard GCC unrolling don't succeed to unroll the most inner loop. ISL AST generated by ISL: for (int c0 = 0; c0 HEIGHT; c0 += 4) for (int c1 = 0; c1 LENGTH - 3; c1 += 1) for (int c2 = c0; c2 = min(HEIGHT - 1, c0 + 3); c2 += 1) S_4(c2, c1); Now, the separating class option (set for unroll and jam) produces this nice loop structure: I'm not sure if Tobi or Albert have told you, but the separation_class option is going to be phased out since its design is fundamentally flawed. If you can't wait until isl-0.15, then I guess you have no choice but to use this option, but you should realize that it will remain frozen in its current broken state (until it is removed at some point). + +/* Set the unroll AST built option. */ + +static __isl_give isl_union_map * +generate_isl_options_2 (scop_p scop, __isl_take isl_union_set *domain, + int dim, int cl) Not a very descriptive function name. +{ + isl_map *map; + isl_space *space, *space_sep; + isl_ctx *ctx; + isl_union_map *mapu; + int nsched = get_max_schedule_dimensions (scop); + + ctx = scop-ctx; + space_sep = isl_space_alloc(ctx, 0, 1, 1); + space_sep = isl_space_wrap(space_sep); + space_sep = isl_space_set_tuple_name(space_sep, isl_dim_set, +separation_class); + space = isl_set_get_space (scop-context); + space_sep = isl_space_align_params(space_sep, isl_space_copy(space)); + space = isl_space_map_from_domain_and_range(space, space_sep); + space = isl_space_add_dims (space,isl_dim_in, nsched); Inconsistent spacing, sometimes with a space before ( and sometimes without. I also noticed some tab/spacing inconsistencies later in the patch, but I already removed that part in my reply. + /* Extract the original and auxiliar maps from pbb-transformed. + Set pbb-transformed to the original map. */ + psmap = smap; + psmap-n = 0; + res = isl_map_foreach_basic_map (pbb-transformed, separate_map, (void *)psmap); + gcc_assert (res == 0); + + isl_map_free(pbb-transformed); + pbb-transformed = isl_map_copy(psmap-map_arr[0]); + I have no idea what this pbb-transformed is supposed to represent, but you appear to be assuming that it has exactly two disjuncts and that they appear in some order. Now, perhaps you have explicitly checked that this map has two disjuncts, but then you should probably bring the check closer since any operation on sets that you perform could change the internal representation (even of other sets). However, in no way can you assume that isl_map_foreach_basic_map will iterate over these disjuncts in any specific order. + bb_domain_s = isl_set_apply (isl_set_copy (bb_domain), +psmap-map_arr[1]); + bb_domain_r = isl_set_apply (bb_domain, psmap-map_arr[0]); + + bb_domain_s = isl_set_complement (bb_domain_s); + bb_domain_r = isl_set_subtract(bb_domain_r,bb_domain_s); Why are you subtracting the complement of a set instead of just intersecting with that set? + /* Create an identity map for everything except DimToVectorize and the + point loop. */ + for (i = 0; i ScheduleDimensions; i++) +{ + if (i == DimToVectorize) +continue; + + c = isl_equality_alloc (isl_local_space_copy (LocalSpace)); + + isl_constraint_set_coefficient_si (c, isl_dim_in, i, -1); + isl_constraint_set_coefficient_si (c, isl_dim_out, i, 1); + + TilingMap = isl_map_add_constraint (TilingMap, c); You can use isl_map_equate instead. @@ -350,17 +425,31 @@ SuffixSchedule); isl_band_list_free (Children); Gaack! gcc is using band forests too... I guess I'll have to keep them around for a while then skimo
Re: [PARCH 1/2, x86, PR63534] Fix darwin bootstrap
So that code creates a set of conflicts which, if I'm reading correctly, will prevent the PIC value from living in a register at all. Which ought to result in it being dumped into the stack and being reloaded for each use. Which ought to be safe (modulo the liveness bug Vlad is working on right now). Does that sound right to either of you? Yes, that's also my understanding of the code and the behavior required by builtin setjmp/longjmp. I can add that the Ada compiler would fall apart if this didn't work correctly in the compiler, and especially in the RA. -- Eric Botcazou
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Fri, Nov 07, 2014 at 10:01:45PM +0100, Richard Biener wrote: Just a comment as these patches flow by - I think this is a huge step backwards from enforcing s1/s2 being a gimple_assign inside gimple_assign_rhs1 to this as_a gassign * boilerplate at _each_ callsite! FWIW, I feel the same way. More to type, worse readability, a lot more of line-wrapping. Sorry, Marek
[PATCH driver/36312] should refuse to overwrite input file with output file
This patch is a minor variant of the one approved here: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00246.html fixing the problem with linker parameters (which are stored in infiles). https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00391.html The only part that has changed, and thus requires approval, is in gcc/gcc.c, in particular the condition: +if ((!infiles[i].language || infiles[i].language[0] != '*') + canonical_filename_eq (infiles[i].name, output_file)) is new. Boottested on x86_64-linux-gnu with --enable-languages=c,c++,objc,fortran,ada,obj-c++ OK? gcc/testsuite/ChangeLog: 2014-11-08 Anthony Brandon anthony.bran...@gmail.com Manuel López-Ibáñez m...@gcc.gnu.org PR driver/36312 * gcc.misc-tests/output.exp: New test case for identical input and output files. include/ChangeLog: 2014-11-08 Anthony Brandon anthony.bran...@gmail.com Manuel López-Ibáñez m...@gcc.gnu.org PR driver/36312 * filenames.h: Add prototype for canonical_filename_eq. gcc/ChangeLog: 2014-11-08 Anthony Brandon anthony.bran...@gmail.com Manuel López-Ibáñez m...@gcc.gnu.org PR driver/36312 * diagnostic-core.h: Add prototype for fatal_error. * diagnostic.c (fatal_error): New function fatal_error. * gcc.c (store_arg): Remove have_o_argbuf_index. (process_command): Check if input and output files are the same. * toplev.c (init_asm_output): Check if input and output files are the same. libiberty/ChangeLog: 2014-11-08 Anthony Brandon anthony.bran...@gmail.com Manuel López-Ibáñez m...@gcc.gnu.org PR driver/36312 * filename_cmp.c (canonical_filename_eq): New function to check if file names are the same. * functions.texi: Updated with documentation for new function. Index: include/filenames.h === --- include/filenames.h (revision 217234) +++ include/filenames.h (working copy) @@ -88,10 +88,12 @@ extern int filename_ncmp (const char *s1 extern hashval_t filename_hash (const void *s); extern int filename_eq (const void *s1, const void *s2); +extern int canonical_filename_eq (const char *a, const char *b); + #ifdef __cplusplus } #endif #endif /* FILENAMES_H */ Index: libiberty/functions.texi === --- libiberty/functions.texi(revision 217234) +++ libiberty/functions.texi(working copy) @@ -123,10 +123,20 @@ is deprecated in favor of @code{memset}. Uses @code{malloc} to allocate storage for @var{nelem} objects of @var{elsize} bytes each, then zeros the memory. @end deftypefn +@c filename_cmp.c:201 +@deftypefn Extension int canonical_filename_eq (const char *@var{a}, const char *@var{b}) + +Return non-zero if file names @var{a} and @var{b} are equivalent. +This function compares the canonical versions of the filenames as returned by +@code{lrealpath()}, so that so that different file names pointing to the same +underlying file are treated as being identical. + +@end deftypefn + @c choose-temp.c:45 @deftypefn Extension char* choose_temp_base (void) Return a prefix for temporary file names or @code{NULL} if unable to find one. The current directory is chosen if all else fails so the @@ -284,11 +294,11 @@ Find the first (least significant) bit s numbered from right to left, starting with bit 1 (corresponding to the value 1). If @var{valu} is zero, zero is returned. @end deftypefn -@c filename_cmp.c:32 +@c filename_cmp.c:37 @deftypefn Extension int filename_cmp (const char *@var{s1}, const char *@var{s2}) Return zero if the two file names @var{s1} and @var{s2} are equivalent. If not equivalent, the returned value is similar to what @code{strcmp} would return. In other words, it returns a negative value if @var{s1} @@ -301,28 +311,28 @@ the case when the two filenames point to However, it does handle the fact that on DOS-like file systems, forward and backward slashes are equal. @end deftypefn -@c filename_cmp.c:178 +@c filename_cmp.c:183 @deftypefn Extension int filename_eq (const void *@var{s1}, const void *@var{s2}) Return non-zero if file names @var{s1} and @var{s2} are equivalent. This function is for use with hashtab.c hash tables. @end deftypefn -@c filename_cmp.c:147 +@c filename_cmp.c:152 @deftypefn Extension hashval_t filename_hash (const void *@var{s}) Return the hash value for file name @var{s} that will be compared using filename_cmp. This function is for use with hashtab.c hash tables. @end deftypefn -@c filename_cmp.c:89 +@c filename_cmp.c:94 @deftypefn Extension int filename_ncmp (const char *@var{s1}, const char *@var{s2}, size_t @var{n}) Return zero if the two file names @var{s1} and @var{s2} are equivalent in range @var{n}. If not equivalent, the returned value is similar to what @code{strncmp} Index: libiberty/filename_cmp.c
Re: [patch sdbout]: Fix ICE on -debug testsuite test const2.C for coff
Hi Jeff, 2014-11-07 21:03 GMT+01:00 Jeff Law l...@redhat.com: On 11/06/14 12:37, Kai Tietz wrote: Hi, This fixes recent fallout of debug-tests on Windows target for sdbout (coff) caused by an ICE. ChangeLog 2014-11-06 Kai Tietz kti...@redhat.com * sdbout.c (sdbout_symbol): Eliminate register only if decl isn't a global variable. Is there a testcase in the suite that triggers this problem? If not, can you try to add one?Out of curiosity, what was DECL_RTL here? Sure, as (partial) mentioned in subject-line the testcase g++.dg/debug/const2.C: Triggers this issue. So there is no need to add a separate testcase for it. Testcase fails for any use of -gcoff1 option. This is probably OK, but I'd really like to know what kind of goofy RTL we passed to eliminate_regs that caused it to fail. I assume it is ok, as I just mimic dwarf2-behavior, which checks before call for eliminate_regs, if rtl isn't a global-var itself. As assumed by checking in debugger I see that eliminate_regs fails on rtx: (mem/u/c:SI (symbol_ref:DI (_ZN1b1dE) [flags 0x2] var_decl 0xffbf d) [0 d+0 S4 A32]) Jeff So a more general question about sdbout. Most prior dbg-coff targets using nowadays dwarf2 too. It seems to me that sdbout didn't got any substantial maintenance the last years anymore. Just fallout was fixed. AFAIK is coff (or was) mainly used nowadays for embedded stuff, and in combination with some tools just supporting coff-dbg information. Do we actually need to support it anymore? Kai
Ping: FR-V rtx iterator patches
Ping for these FR-V patches: https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02645.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02646.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02647.html https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02648.html which convert callers of for_each_rtx to the new rtx iterators in rtl-iter.h. (These are the only uses of for_each_rtx left -- thanks for all the reviews to get this far.) Thanks, Richard
[PATCH] Fix PR56480 aka DR374. Allow explicit specialization in enclosing namespace.
DR374: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#374 allows explicit specialization of templates in the enclosing namespace. This is implemented by consolidating check_specialization_namespace and check_explicit_instantiation_namespace into a new function and pedwarning only for C++98. Only explicit namespace qualifications are accepted, so g++.dg/template/spec17.C is rejected. To check for this condition have_def and in_namespace are passed through to register_specialization() by using a bitmask. Tested on powerpc64-unknown-linux-gnu. (Also tested by running the Boost testsuite and building Firefox.) OK for trunk? Thanks again. 2014-11-08 Markus Trippelsdorf mar...@trippelsdorf.de gcc/cp/ChangeLog: * decl.c (grokfndecl): Also pass through in_namespace. * pt.c (check_instant_or_special_namespace): Consolidate check_specialization_namespace and check_explicit_instantiation_namespace. pedwarn only for C++98. (maybe_process_partial_specialization): Call consolidated function. (register_specialization): Add flag. Only accept explicit namespace qualification. (check_template_variable): Call consolidated function. (check_explicit_specialization): Likewise (push_template_decl_real): Likewise (tsubst_friend_function): Likewise. (tsubst_decl): Likewise. (do_decl_instantiation): Likewise. (do_type_instantiation): Likewise. gcc/testsuite/ChangeLog: * g++.dg/template/spec17.C: Don't dg-error for C++11 and up. * g++.dg/template/spec25.C: Likewise. Adjust regex. * g++.dg/template/spec36.C: Don't dg-error for C++11 and up. * g++.old-deja/g++.ns/template13.C: Likewise. * g++.old-deja/g++.pt/explicit73.C: Likewise. * g++.old-deja/g++.pt/lookup10.C: Likewise. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 4abc1011e61e..931bf2c2aafc 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7855,7 +7855,8 @@ grokfndecl (tree ctype, decl = check_explicit_specialization (orig_declarator, decl, template_count, 2 * funcdef_flag + - 4 * (friendp != 0)); + 4 * (friendp != 0) + + 8 * (in_namespace != 0)); if (decl == error_mark_node) return NULL_TREE; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index fa9652f748c2..64f72e50880d 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -755,57 +755,58 @@ end_explicit_instantiation (void) processing_explicit_instantiation = false; } -/* An explicit specialization or partial specialization of TMPL is being - declared. Check that the namespace in which the specialization is - occurring is permissible. Returns false iff it is invalid to - specialize TMPL in the current namespace. */ +/* If specialization is true, an explicit specialization or partial + specialization of TMPL is being declared. Check that the namespace + in which the specialization is occurring is permissible. Returns + false iff it is invalid to specialize TMPL in the current namespace. + If specialization is false, TMPL is an explicit instantiation. + Check that it is valid to perform this explicit instantiation in + the current namespace. */ static bool -check_specialization_namespace (tree tmpl) +check_instant_or_special_namespace (tree tmpl, bool specialization) { tree tpl_ns = decl_namespace_context (tmpl); - /* [tmpl.expl.spec] - - An explicit specialization shall be declared in the namespace of - which the template is a member, or, for member templates, in the - namespace of which the enclosing class or enclosing class - template is a member. An explicit specialization of a member - function, member class or static data member of a class template - shall be declared in the namespace of which the class template is - a member. */ - if (current_scope() != DECL_CONTEXT (tmpl) - !at_namespace_scope_p ()) -{ - error (specialization of %qD must appear at namespace scope, tmpl); - return false; -} - if (is_associated_namespace (current_namespace, tpl_ns)) -/* Same or super-using namespace. */ -return true; - else + if (specialization) { - permerror (input_location, specialization of %qD in different namespace, tmpl); - permerror (input_location, from definition of %q+#D, tmpl); - return false; -} -} + /* [tmpl.expl.spec] -/* SPEC is an explicit instantiation. Check that it is valid to - perform this explicit instantiation in the current namespace. */ - -static void -check_explicit_instantiation_namespace (tree spec) -{ - tree ns; +An explicit specialization shall be declared in the namespace of +which the template is a member, or, for member templates, in the +namespace of
Re: [PR tree-optimization/61515] Fix poor compile-time behaviour in tree-ssa-threadedge.c
On Sat, Nov 8, 2014 at 12:00 AM, Jeff Law l...@redhat.com wrote: When we thread across a backedge, we have to do invalidations of equivalences. This changes the method by which we identify which objects need invalidation. Previously we'd walk all the SSA_NAMEs and see if any had a current value that needed invalidation. That is obviously expensive if we have a lot of SSA_NAMEs. This patch implements an idea from Richi, basically walk the equivalency unwinder stack, which has equivalences from both DOM and the threader. Check the current value of each object in that stack and see if it needs invalidation. Even pathlogical cases this ought to be considerably faster than walking all the SSA_NAMEs. For the reduced testcase, compilation time in the relevant parts of GCC go from 40 seconds to less than a second. Bootstrapped and regression tested on x86_64-unknown-linux-gnu. Installed on the trunk. Thanks - can we still avoid walking the stack for each LHS by first computing which SSA names to invalidate using a bitmap? For some reason this didn't work with walking SSA names (see my proposed patch in the PR), maybe you can figure out why. But yes, the stack should always be smaller than the number of SSA names (not sure if we can actually prove that, but ...). I hope we can backport this to 4.9 after a while. Thanks, Richard. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 966c0b4..0c8eb79 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2014-11-07 Jeff Law l...@redhat.com + + PR tree-optimization/61515 + * tree-ssa-threadedge.c (invalidate_equivalences): Walk the unwinding + stack rather than looking at every SSA_NAME's value. + 2014-11-07 Richard Biener rguent...@suse.de PR tree-optimization/63605 diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c index de3c819..d5b9696 100644 --- a/gcc/tree-ssa-threadedge.c +++ b/gcc/tree-ssa-threadedge.c @@ -290,15 +290,40 @@ fold_assignment_stmt (gimple stmt) } /* A new value has been assigned to LHS. If necessary, invalidate any - equivalences that are no longer valid. */ + equivalences that are no longer valid. This includes invaliding + LHS and any objects that are currently equivalent to LHS. + + Finding the objects that are currently marked as equivalent to LHS + is a bit tricky. We could walk the ssa names and see if any have + SSA_NAME_VALUE that is the same as LHS. That's expensive. + + However, it's far more efficient to look at the unwinding stack as + that will have all context sensitive equivalences which are the only + ones that we really have to worry about here. */ static void invalidate_equivalences (tree lhs, vectree *stack) { - for (unsigned int i = 1; i num_ssa_names; i++) -if (ssa_name (i) SSA_NAME_VALUE (ssa_name (i)) == lhs) - record_temporary_equivalence (ssa_name (i), NULL_TREE, stack); + /* The stack is an unwinding stack. If the current element is NULL + then it's a stop unwinding marker. Else the current marker is + the SSA_NAME with an equivalence and the prior entry in the stack + is what the current element is equivalent to. */ + for (int i = stack-length() - 1; i = 0; i--) +{ + /* Ignore the stop unwinding markers. */ + if ((*stack)[i] == NULL) + continue; + + /* We want to check the current value of stack[i] to see if +it matches LHS. If so, then invalidate. */ + if (SSA_NAME_VALUE ((*stack)[i]) == lhs) + record_temporary_equivalence ((*stack)[i], NULL_TREE, stack); + + /* Remember, we're dealing with two elements in this case. */ + i--; +} + /* And invalidate any known value for LHS itself. */ if (SSA_NAME_VALUE (lhs)) record_temporary_equivalence (lhs, NULL_TREE, stack); }
Re: [GRAPHITE, PATCH] Loop unroll and jam optimization
On Sat, Nov 8, 2014 at 12:32 AM, Mircea Namolaru mircea.namol...@inria.fr wrote: Hello, This is the patch for unroll and jam optimizations. It is based on the code written by Tobias from graphite-optimize-isl.c (the code was unreachable till now) extended and enabled it via a new option -floop-unroll-jam. Why not -floop-unroll-and-jam? The patch takes advantage of the new isl based code generator introduced recently in GCC (in fact of the possible options for building the AST). The code generated for this optimization in the case of non-constant loop bounds initially looks as below. This is not very useful because the standard GCC unrolling don't succeed to unroll the most inner loop. ISL AST generated by ISL: for (int c0 = 0; c0 HEIGHT; c0 += 4) for (int c1 = 0; c1 LENGTH - 3; c1 += 1) for (int c2 = c0; c2 = min(HEIGHT - 1, c0 + 3); c2 += 1) Hmm, so this iterates at most 4 times, right? Eventually the body is considered too large by GCC or it fails to compute an upper bound for the number of iterations. Is that (an upper bound for the number of iterations) available readily from ISL at code-generation time? If so you can transfer this knowledge to the GCC loop information. I'm curious to see a testcase (and a way to generate the above form) to see what is actually the problem. Thanks, Richard. S_4(c2, c1); Now, the separating class option (set for unroll and jam) produces this nice loop structure: ISL AST generated by ISL: for (int c0 = 0; c0 HEIGHT; c0 += 4) for (int c1 = 0; c1 LENGTH - 3; c1 += 1) if (HEIGHT = c0 + 4) { for (int c2 = c0; c2 = c0 + 3; c2 += 1) S_4(c2, c1); } else for (int c2 = c0; c2 HEIGHT; c2 += 1) S_4(c2, c1); The unroll option (set for unroll and jam) produces: ISL AST generated by ISL: for (int c0 = 0; c0 HEIGHT; c0 += 4) for (int c1 = 0; c1 LENGTH - 3; c1 += 1) if (HEIGHT = c0 + 4) { S_4(c0, c1); S_4(c0 + 1, c1); S_4(c0 + 2, c1); S_4(c0 + 3, c1); } else { S_4(c0, c1); if (HEIGHT = c0 + 2) { S_4(c0 + 1, c1); if (4 * floord(HEIGHT - 3, 4) + 3 == HEIGHT c0 + 3 == HEIGHT) S_4(HEIGHT - 1, c1); } } The separate option (set by default for all dimensions for the new isl based code generator) don't succeed to remove the ifs from the loops and generate two loop structures (this would have been highly desirable). As the stage 1 is going to close soon, quick feedback to this patch is greatly appreciated. Many thanks, Mircea Namolaru
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Fri, Nov 7, 2014 at 10:01 PM, Richard Biener richard.guent...@gmail.com wrote: On Fri, Nov 7, 2014 at 4:21 PM, David Malcolm dmalc...@redhat.com wrote: gcc/ChangeLog.gimple-classes: * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast. (gimple_equal_p): Add checked casts. --- gcc/ChangeLog.gimple-classes | 5 + gcc/tree-ssa-tail-merge.c| 8 +--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/gcc/ChangeLog.gimple-classes b/gcc/ChangeLog.gimple-classes index f43df63..0bd0421 100644 --- a/gcc/ChangeLog.gimple-classes +++ b/gcc/ChangeLog.gimple-classes @@ -1,5 +1,10 @@ 2014-11-06 David Malcolm dmalc...@redhat.com + * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast. + (gimple_equal_p): Add checked casts. + +2014-11-06 David Malcolm dmalc...@redhat.com + * tree-ssa-structalias.c (find_func_aliases): Replace is_gimple_assign with a dyn_cast, introducing local gassign * t_assign, using it in place of t for typesafety. diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c index 5678657..b822214 100644 --- a/gcc/tree-ssa-tail-merge.c +++ b/gcc/tree-ssa-tail-merge.c @@ -484,7 +484,7 @@ same_succ_hash (const_same_succ e) hstate.add_int (gimple_code (stmt)); if (is_gimple_assign (stmt)) - hstate.add_int (gimple_assign_rhs_code (stmt)); + hstate.add_int (gimple_assign_rhs_code (as_a gassign * (stmt))); if (!is_gimple_call (stmt)) continue; if (gimple_call_internal_p (stmt)) @@ -1172,8 +1172,10 @@ gimple_equal_p (same_succ same_succ, gimple s1, gimple s2) if (TREE_CODE (lhs1) != SSA_NAME TREE_CODE (lhs2) != SSA_NAME) return (operand_equal_p (lhs1, lhs2, 0) -gimple_operand_equal_value_p (gimple_assign_rhs1 (s1), -gimple_assign_rhs1 (s2))); +gimple_operand_equal_value_p (gimple_assign_rhs1 ( + as_a gassign * (s1)), +gimple_assign_rhs1 ( + as_a gassign * (s2; Just a comment as these patches flow by - I think this is a huge step backwards from enforcing s1/s2 being a gimple_assign inside gimple_assign_rhs1 to this as_a gassign * boilerplate at _each_ callsite! Which means this step of the refactoring is totally broken and probably requires much more manual work to avoid this kind of uglyness. I definitely won't approve of this kind of changes. To be constructive here - the above case is from within a GIMPLE_ASSIGN case label and thus I'd have expected case GIMPLE_ASSIGN: { gassign *a1 = as_a gassign * (s1); gassign *a2 = as_a gassign * (s2); lhs1 = gimple_assign_lhs (a1); lhs2 = gimple_assign_lhs (a2); if (TREE_CODE (lhs1) != SSA_NAME TREE_CODE (lhs2) != SSA_NAME) return (operand_equal_p (lhs1, lhs2, 0) gimple_operand_equal_value_p (gimple_assign_rhs1 (a1), gimple_assign_rhs1 (a2))); else if (TREE_CODE (lhs1) == SSA_NAME TREE_CODE (lhs2) == SSA_NAME) return vn_valueize (lhs1) == vn_valueize (lhs2); return false; } instead. That's the kind of changes I have expected and have approved of. Thanks, Richard. Thanks, Richard. else if (TREE_CODE (lhs1) == SSA_NAME TREE_CODE (lhs2) == SSA_NAME) return vn_valueize (lhs1) == vn_valueize (lhs2); -- 1.7.11.7
[x86, 3/n] Replace builtins with vector extensions
Hello, this patch mechanically extends +-* for integer vectors of size 256 and 512 (the previous patch only handled 128). Regtested together with the next patch. 2014-11-10 Marc Glisse marc.gli...@inria.fr * config/i386/avxintrin.h (__v4du, __v8su, __v16hu, __v32qu): New typedefs. * config/i386/avx512fintrin.h (__v8du, __v16su, __v32hu, __v64qu): Likewise. (_mm512_mullo_epi32, _mm512_add_epi64, _mm512_sub_epi64, _mm512_add_epi32, _mm512_sub_epi32): Use vector extensions instead of builtins. * config/i386/avx2intrin.h (_mm256_add_epi8, _mm256_add_epi16, _mm256_add_epi32, _mm256_add_epi64, _mm256_mullo_epi16, _mm256_mullo_epi32, _mm256_sub_epi8, _mm256_sub_epi16, _mm256_sub_epi32, _mm256_sub_epi64): Likewise. * config/i386/avx512bwintrin.h (_mm512_mullo_epi16, _mm512_add_epi8, _mm512_sub_epi8, _mm512_sub_epi16, _mm512_add_epi16): Likewise. * config/i386/avx512dqintrin.h (_mm512_mullo_epi64): Likewise. * config/i386/avx512vldqintrin.h (_mm256_mullo_epi64, _mm_mullo_epi64): Likewise. -- Marc GlisseIndex: config/i386/avx2intrin.h === --- config/i386/avx2intrin.h(revision 217249) +++ config/i386/avx2intrin.h(working copy) @@ -97,42 +97,42 @@ extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_packus_epi16 (__m256i __A, __m256i __B) { return (__m256i)__builtin_ia32_packuswb256 ((__v16hi)__A, (__v16hi)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_add_epi8 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_paddb256 ((__v32qi)__A, (__v32qi)__B); + return (__m256i) ((__v32qu)__A + (__v32qu)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_add_epi16 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_paddw256 ((__v16hi)__A, (__v16hi)__B); + return (__m256i) ((__v16hu)__A + (__v16hu)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_add_epi32 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_paddd256 ((__v8si)__A, (__v8si)__B); + return (__m256i) ((__v8su)__A + (__v8su)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_add_epi64 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_paddq256 ((__v4di)__A, (__v4di)__B); + return (__m256i) ((__v4du)__A + (__v4du)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_adds_epi8 (__m256i __A, __m256i __B) { return (__m256i)__builtin_ia32_paddsb256 ((__v32qi)__A, (__v32qi)__B); } extern __inline __m256i @@ -548,28 +548,28 @@ extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_mulhi_epi16 (__m256i __A, __m256i __B) { return (__m256i)__builtin_ia32_pmulhw256 ((__v16hi)__A, (__v16hi)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_mullo_epi16 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_pmullw256 ((__v16hi)__A, (__v16hi)__B); + return (__m256i) ((__v16hu)__A * (__v16hu)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_mullo_epi32 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_pmulld256 ((__v8si)__A, (__v8si)__B); + return (__m256i) ((__v8su)__A * (__v8su)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_mul_epu32 (__m256i __A, __m256i __B) { return (__m256i)__builtin_ia32_pmuludq256 ((__v8si)__A, (__v8si)__B); } extern __inline __m256i @@ -778,42 +778,42 @@ extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_srl_epi64 (__m256i __A, __m128i __B) { return (__m256i)__builtin_ia32_psrlq256((__v4di)__A, (__v2di)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_sub_epi8 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_psubb256 ((__v32qi)__A, (__v32qi)__B); + return (__m256i) ((__v32qu)__A - (__v32qu)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_sub_epi16 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_psubw256 ((__v16hi)__A, (__v16hi)__B); + return (__m256i) ((__v16hu)__A - (__v16hu)__B); } extern __inline __m256i __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) _mm256_sub_epi32 (__m256i __A, __m256i __B) { - return (__m256i)__builtin_ia32_psubd256 ((__v8si)__A, (__v8si)__B); + return (__m256i) ((__v8su)__A - (__v8su)__B); } extern
[x86, 4/n] Replace builtins with vector extensions
Hello, this patch uses |^ for 128 bit integer vectors. I am doing the operations in type __v2du because __builtin_ia32_pand128 was apparently taking __v2di arguments, but using __v4su or any other should be equivalent. Even __int128 would in principle be ok, but since it is not usually stored in a vector register, it seems more likely to generate unexpected code (and we don't have __int256 so it would be inconsistent with other sizes). Regtested with patch 3/n. Ok for the branch? After that, I will post a last patch to generalize |^ to sizes 256 and 512, and I think that will be enough for gcc-5, we should discuss merging. == abs min max can wait until gcc-6, possibly after getting some feedback about +-*/|^. 2014-11-10 Marc Glisse marc.gli...@inria.fr * config/i386/emmintrin.h (_mm_and_si128, _mm_or_si128, _mm_xor_si128): Use vector extensions instead of builtins. -- Marc GlisseIndex: config/i386/emmintrin.h === --- config/i386/emmintrin.h (revision 217249) +++ config/i386/emmintrin.h (working copy) @@ -1244,39 +1244,39 @@ _mm_srl_epi32 (__m128i __A, __m128i __B) extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_srl_epi64 (__m128i __A, __m128i __B) { return (__m128i)__builtin_ia32_psrlq128 ((__v2di)__A, (__v2di)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_and_si128 (__m128i __A, __m128i __B) { - return (__m128i)__builtin_ia32_pand128 ((__v2di)__A, (__v2di)__B); + return (__m128i) ((__v2du)__A (__v2du)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_andnot_si128 (__m128i __A, __m128i __B) { return (__m128i)__builtin_ia32_pandn128 ((__v2di)__A, (__v2di)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_or_si128 (__m128i __A, __m128i __B) { - return (__m128i)__builtin_ia32_por128 ((__v2di)__A, (__v2di)__B); + return (__m128i) ((__v2du)__A | (__v2du)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_xor_si128 (__m128i __A, __m128i __B) { - return (__m128i)__builtin_ia32_pxor128 ((__v2di)__A, (__v2di)__B); + return (__m128i) ((__v2du)__A ^ (__v2du)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_cmpeq_epi8 (__m128i __A, __m128i __B) { return (__m128i)__builtin_ia32_pcmpeqb128 ((__v16qi)__A, (__v16qi)__B); } extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) _mm_cmpeq_epi16 (__m128i __A, __m128i __B)
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Sat, Nov 08, 2014 at 01:07:28PM +0100, Richard Biener wrote: To be constructive here - the above case is from within a GIMPLE_ASSIGN case label and thus I'd have expected case GIMPLE_ASSIGN: { gassign *a1 = as_a gassign * (s1); gassign *a2 = as_a gassign * (s2); lhs1 = gimple_assign_lhs (a1); lhs2 = gimple_assign_lhs (a2); if (TREE_CODE (lhs1) != SSA_NAME TREE_CODE (lhs2) != SSA_NAME) return (operand_equal_p (lhs1, lhs2, 0) gimple_operand_equal_value_p (gimple_assign_rhs1 (a1), gimple_assign_rhs1 (a2))); else if (TREE_CODE (lhs1) == SSA_NAME TREE_CODE (lhs2) == SSA_NAME) return vn_valueize (lhs1) == vn_valueize (lhs2); return false; } instead. That's the kind of changes I have expected and have approved of. But even that looks like just adding extra work for all developers, with no gain. You only have to add extra code and extra temporaries, in switches typically also have to add {} because of the temporaries and thus extra indentation level, and it doesn't simplify anything in the code. Jakub
Re: [gimple-classes, committed 4/6] tree-ssa-tail-merge.c: Use gassign
On Sat, 2014-11-08 at 13:07 +0100, Richard Biener wrote: On Fri, Nov 7, 2014 at 10:01 PM, Richard Biener richard.guent...@gmail.com wrote: On Fri, Nov 7, 2014 at 4:21 PM, David Malcolm dmalc...@redhat.com wrote: gcc/ChangeLog.gimple-classes: * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast. (gimple_equal_p): Add checked casts. --- gcc/ChangeLog.gimple-classes | 5 + gcc/tree-ssa-tail-merge.c| 8 +--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/gcc/ChangeLog.gimple-classes b/gcc/ChangeLog.gimple-classes index f43df63..0bd0421 100644 --- a/gcc/ChangeLog.gimple-classes +++ b/gcc/ChangeLog.gimple-classes @@ -1,5 +1,10 @@ 2014-11-06 David Malcolm dmalc...@redhat.com + * tree-ssa-tail-merge.c (same_succ_hash): Add checked cast. + (gimple_equal_p): Add checked casts. + +2014-11-06 David Malcolm dmalc...@redhat.com + * tree-ssa-structalias.c (find_func_aliases): Replace is_gimple_assign with a dyn_cast, introducing local gassign * t_assign, using it in place of t for typesafety. diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c index 5678657..b822214 100644 --- a/gcc/tree-ssa-tail-merge.c +++ b/gcc/tree-ssa-tail-merge.c @@ -484,7 +484,7 @@ same_succ_hash (const_same_succ e) hstate.add_int (gimple_code (stmt)); if (is_gimple_assign (stmt)) - hstate.add_int (gimple_assign_rhs_code (stmt)); + hstate.add_int (gimple_assign_rhs_code (as_a gassign * (stmt))); if (!is_gimple_call (stmt)) continue; if (gimple_call_internal_p (stmt)) @@ -1172,8 +1172,10 @@ gimple_equal_p (same_succ same_succ, gimple s1, gimple s2) if (TREE_CODE (lhs1) != SSA_NAME TREE_CODE (lhs2) != SSA_NAME) return (operand_equal_p (lhs1, lhs2, 0) -gimple_operand_equal_value_p (gimple_assign_rhs1 (s1), -gimple_assign_rhs1 (s2))); +gimple_operand_equal_value_p (gimple_assign_rhs1 ( + as_a gassign * (s1)), +gimple_assign_rhs1 ( + as_a gassign * (s2; Just a comment as these patches flow by - I think this is a huge step backwards from enforcing s1/s2 being a gimple_assign inside gimple_assign_rhs1 to this as_a gassign * boilerplate at _each_ callsite! Which means this step of the refactoring is totally broken and probably requires much more manual work to avoid this kind of uglyness. I definitely won't approve of this kind of changes. To be constructive here - the above case is from within a GIMPLE_ASSIGN case label and thus I'd have expected case GIMPLE_ASSIGN: { gassign *a1 = as_a gassign * (s1); gassign *a2 = as_a gassign * (s2); lhs1 = gimple_assign_lhs (a1); lhs2 = gimple_assign_lhs (a2); if (TREE_CODE (lhs1) != SSA_NAME TREE_CODE (lhs2) != SSA_NAME) return (operand_equal_p (lhs1, lhs2, 0) gimple_operand_equal_value_p (gimple_assign_rhs1 (a1), gimple_assign_rhs1 (a2))); else if (TREE_CODE (lhs1) == SSA_NAME TREE_CODE (lhs2) == SSA_NAME) return vn_valueize (lhs1) == vn_valueize (lhs2); return false; } instead. That's the kind of changes I have expected and have approved of. I do make the above kind of change in some places within the gimple-classes branch. I think I didn't do it in this case because the body of the case GIMPLE_ASSIGN doesn't yet have braces, so adding locals requires adding them and re-indenting the case body. I didn't spot the opportunity to speed up the code as you do above by converting the two gimple_get_lhs to gimple_assign_lhs. Without that, I guess I decided to simply add the two as_a directly in-place to avoid the reindent. With your speedup it's clearly better to reindent the code. (Got to go now, sorry; I hope to write a better reply on Monday) Thanks Dave
Re: [PATCH] Don't bootstrap libcc1
I am still unable to bootstrap darwin14 without revision r216964 reverted. Executing the simplified command /opt/gcc/build_w/gcc/xg++ -B/opt/gcc/build_w/gcc/ -L/opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs -o .libs/libcc1.0.so .libs/findcomp.o -static-libstdc++ -static-libgcc I get ld: file not found: libstdc++.a collect2: error: ld returned 1 exit status while I see [Book15] build_w/libcc1% ls -l /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a -rw-r--r-- 1 dominiq staff 9118792 Nov 8 15:30 /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a What am I missing? TIA Dominique
Re: [PATCH] Don't bootstrap libcc1
On Sat, Nov 08, 2014 at 04:31:28PM +0100, Dominique d'Humières wrote: I am still unable to bootstrap darwin14 without revision r216964 reverted. Executing the simplified command /opt/gcc/build_w/gcc/xg++ -B/opt/gcc/build_w/gcc/ -L/opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs -o .libs/libcc1.0.so .libs/findcomp.o -static-libstdc++ -static-libgcc I get ld: file not found: libstdc++.a collect2: error: ld returned 1 exit status while I see [Book15] build_w/libcc1% ls -l /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a -rw-r--r-- 1 dominiq staff 9118792 Nov 8 15:30 /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a What am I missing? That is for somebody familiar with all the Mach-O weirdnesses to look at, I don't see anything wrong with the above g++ invocation. Rerun with -v to see how ld is invoked, strace (if Darwin has anything like that) it to see why it doesn't find that libstdc++.a ? Jakub
[patch, ARM] Add support for crtfastmath.o
This patch adds support to the ARM backend to avoid denormals by setting the VFP flush-to-zero bit when linking with -ffast-math and similar options. The mechanism used to accomplish this is a constructor in libgcc that is conditionally linked in via STARTFILE_SPEC; it's basically identical to what a number of other backends are already doing here (Aarch64, i386, MIPS, etc). I tested this by verifying that linking with -ffast-math on the GCC command line causes crtfastmath.o to be passed on the ld command line. I built libgcc with a variety of configuration options to verify that the constructor is suppressed for soft-float targets, that it builds successfully for Thumb-2 with VFP, etc. I also regression-tested the gcc testsuite on an arm-none-linux-gnueabi build configured for -march=armv8-a with VFP floating point enabled. OK to commit? -Sandra 2014-11-08 Sandra Loosemore san...@codesourcery.com Chris Jones chr...@nvidia.com Joshua Conner jcon...@nvidia.com gcc/ * config/arm/unknown-elf.h (STARTFILE_SPEC): Add conditional linking of crtfastmath.o. * config/arm/linux-eabi.h (STARTFILE_SPEC): Likewise. libgcc/ * config.host (arm*-*-linux*): Add support for crtfastmath.o. (arm*-*-uclinux*): Likewise. (arm*-*-eabi* | arm*-*-rtems*): Likewise. * config/arm/crtfastmath.c: New file. Index: gcc/config/arm/unknown-elf.h === --- gcc/config/arm/unknown-elf.h (revision 216500) +++ gcc/config/arm/unknown-elf.h (working copy) @@ -32,7 +32,9 @@ #define UNKNOWN_ELF_STARTFILE_SPEC crti%O%s crtbegin%O%s crt0%O%s #undef STARTFILE_SPEC -#define STARTFILE_SPEC UNKNOWN_ELF_STARTFILE_SPEC +#define STARTFILE_SPEC \ + %{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \ + UNKNOWN_ELF_STARTFILE_SPEC #define UNKNOWN_ELF_ENDFILE_SPEC crtend%O%s crtn%O%s Index: gcc/config/arm/linux-eabi.h === --- gcc/config/arm/linux-eabi.h (revision 216500) +++ gcc/config/arm/linux-eabi.h (working copy) @@ -107,6 +107,7 @@ #undef ENDFILE_SPEC #define ENDFILE_SPEC \ + %{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s} \ LINUX_OR_ANDROID_LD (GNU_USER_TARGET_ENDFILE_SPEC, ANDROID_ENDFILE_SPEC) /* Use the default LIBGCC_SPEC, not the version in linux-elf.h, as we Index: libgcc/config.host === --- libgcc/config.host (revision 216500) +++ libgcc/config.host (working copy) @@ -370,14 +370,15 @@ arm*-*-netbsdelf*) tmake_file=$tmake_file arm/t-arm arm/t-netbsd t-slibgcc-gld-nover ;; arm*-*-linux*) # ARM GNU/Linux with ELF - tmake_file=${tmake_file} arm/t-arm t-fixedpoint-gnu-prefix + tmake_file=${tmake_file} arm/t-arm t-fixedpoint-gnu-prefix t-crtfm tmake_file=${tmake_file} arm/t-elf arm/t-bpabi arm/t-linux-eabi t-slibgcc-libgcc tm_file=$tm_file arm/bpabi-lib.h unwind_header=config/arm/unwind-arm.h tmake_file=$tmake_file t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp + extra_parts=$extra_parts crtfastmath.o ;; arm*-*-uclinux*) # ARM ucLinux - tmake_file=${tmake_file} t-fixedpoint-gnu-prefix + tmake_file=${tmake_file} t-fixedpoint-gnu-prefix t-crtfm tmake_file=$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp tmake_file=${tmake_file} arm/t-bpabi tm_file=$tm_file arm/bpabi-lib.h @@ -389,7 +390,7 @@ arm*-*-eabi* | arm*-*-symbianelf* | arm* tm_file=$tm_file arm/bpabi-lib.h case ${host} in arm*-*-eabi* | arm*-*-rtems*) - tmake_file=${tmake_file} arm/t-bpabi + tmake_file=${tmake_file} arm/t-bpabi t-crtfm extra_parts=crtbegin.o crtend.o crti.o crtn.o ;; arm*-*-symbianelf*) Index: libgcc/config/arm/crtfastmath.c === --- libgcc/config/arm/crtfastmath.c (revision 0) +++ libgcc/config/arm/crtfastmath.c (revision 0) @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2014 Free Software Foundation, Inc. + * + * This file is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 3, or (at your option) any + * later version. + * + * This file is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * Under Section 7 of GPL version 3, you are granted additional + * permissions described in the GCC Runtime Library Exception, version + * 3.1, as published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License and + * a copy of the GCC Runtime Library Exception along with this program; + * see the files COPYING3 and COPYING.RUNTIME
Re: [PARCH 1/2, x86, PR63534] Fix darwin bootstrap
Hi Jeff, On 7 Nov 2014, at 20:28, Jeff Law wrote: On 11/06/14 06:01, Evgeny Stupachenko wrote: Now I see that equiv reload could be special for PIC register. Let's apply more conservative patch. Darwin bootstrap passed with the patch applied on r216304 (along with already committed to trunk patches from PR63618 and PR63620). 2014-11-06 Evgeny Stupachenko evstu...@gmail.com PR target/63534 * config/i386/i386.c (builtin_setjmp_receiver): Use pic_offset_table_rtx for PIC register. (nonlocal_goto_receiver): Delete. OK for the trunk. One more Darwin bug gets squashed :-) FOAD, are you referring to this bug (63534) - or is there an implication that the nonlocal_goto_reciever was buggy (*before* the trunk changes) - I ask because the same implementation is present on the open branches, and if it's broken would like to sort that out. cheers Iain
Re: [PATCH] Don't bootstrap libcc1
Dominique, I thought from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63773#c1 that you were having clean bootstraps on darwin. I have tested x86_64-apple-darwin11/12/13/14 without issues in the creation of libcc1.0 % otool -L libcc1.0.so libcc1.0.so: /sw/lib/gcc5.0/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.21.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0) /sw/lib/gcc5.0/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0) except for the oddity that one isn't being created in the -m32 multilib. Is this a hard bootstrap failure? Jack ps From build log I see... libtool: link: /sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/./gcc/xg++ -B/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/./gcc/ -nostdinc++ -nostdinc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/include/x86_64-apple-darwin13.4.0 -I/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/include -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5.0-20141107/libstdc++-v3/libsupc++ -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5.0-20141107/libstdc++-v3/include/backward -I/sw/src/fink.build/gcc50-5.0.0-1000/gcc-5.0-20141107/libstdc++-v3/testsuite/util -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/libsupc++/.libs -B/sw/lib/gcc5.0/x86_64-apple-darwin13.4.0/bin/ -B/sw/lib/gcc5.0/x86_64-apple-darwin13.4.0/lib/ -isystem /sw/lib/gcc5.0/x86_64-apple-darwin13.4.0/include -isystem /sw/lib/gcc5.0/x86_64-apple-darwin13.4.0/sys-include-Wl,-undefined -Wl,dynamic_lookup -o .libs/libcc1.0.so -bundle .libs/findcomp.o .libs/libcc1.o .libs/names.o .libs/callbacks.o .libs/connection.o .libs/marshall.o -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/libsupc++/.libs -Wl,-no_pie ../libiberty/pic/libiberty.a -Wl,-exported_symbols_list,.libs/libcc1-symbols.expsym On Sat, Nov 8, 2014 at 10:31 AM, Dominique d'Humières domi...@lps.ens.fr wrote: I am still unable to bootstrap darwin14 without revision r216964 reverted. Executing the simplified command /opt/gcc/build_w/gcc/xg++ -B/opt/gcc/build_w/gcc/ -L/opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs -o .libs/libcc1.0.so .libs/findcomp.o -static-libstdc++ -static-libgcc I get ld: file not found: libstdc++.a collect2: error: ld returned 1 exit status while I see [Book15] build_w/libcc1% ls -l /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a -rw-r--r-- 1 dominiq staff 9118792 Nov 8 15:30 /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a What am I missing? TIA Dominique
Re: [PARCH 1/2, x86, PR63534] Fix darwin bootstrap
On 11/08/14 09:16, Iain Sandoe wrote: Hi Jeff, On 7 Nov 2014, at 20:28, Jeff Law wrote: On 11/06/14 06:01, Evgeny Stupachenko wrote: Now I see that equiv reload could be special for PIC register. Let's apply more conservative patch. Darwin bootstrap passed with the patch applied on r216304 (along with already committed to trunk patches from PR63618 and PR63620). 2014-11-06 Evgeny Stupachenko evstu...@gmail.com PR target/63534 * config/i386/i386.c (builtin_setjmp_receiver): Use pic_offset_table_rtx for PIC register. (nonlocal_goto_receiver): Delete. OK for the trunk. One more Darwin bug gets squashed :-) FOAD, are you referring to this bug (63534) - or is there an implication that the nonlocal_goto_reciever was buggy (*before* the trunk changes) - I ask because the same implementation is present on the open branches, and if it's broken would like to sort that out. cheers Iain No need to do anything on the branches as the issue with the {setjmp,nonlocal_goto}_receiver is specific to the change to expose the PIC register to register allocation. jeff
Re: [PATCH] Don't bootstrap libcc1
On 8 Nov 2014, at 15:41, Jakub Jelinek wrote: On Sat, Nov 08, 2014 at 04:31:28PM +0100, Dominique d'Humières wrote: I am still unable to bootstrap darwin14 without revision r216964 reverted. Executing the simplified command /opt/gcc/build_w/gcc/xg++ -B/opt/gcc/build_w/gcc/ -L/opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs -o .libs/libcc1.0.so .libs/findcomp.o -static-libstdc++ -static-libgcc I get ld: file not found: libstdc++.a collect2: error: ld returned 1 exit status while I see [Book15] build_w/libcc1% ls -l /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a -rw-r--r-- 1 dominiq staff 9118792 Nov 8 15:30 /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a What am I missing? That is for somebody familiar with all the Mach-O weirdnesses to look at, I don't see anything wrong with the above g++ invocation. Rerun with -v to see how ld is invoked, strace (if Darwin has anything like that) it to see why it doesn't find that libstdc++.a ? This is not really mach-o related, but a consequence of the way in which GCC specs substitution works. Unless the libstdc++-v3/.libs and libsupc++/.libs are visible as -Bx, spec substitution will not work (it's not enough to provide -L). This is done elsewhere (e.g. gnattools), it just needs an appropriate addition to the libcc1/Makefile.in (will try and take a look later in the week, if no-one else gets to it first). Iain
Re: [PATCH] Don't bootstrap libcc1
Iain, Any idea why this isn't failing universally? On all of the machines tested here with 'make bootstrap', the linkage of libcc1.so finds the necessary libstdc++ from the set of flags... -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/libsupc++/.libs as I posted earlier in this thread. Why wouldn't Dominique be getting those emitted in his build and shouldn't they suffice? Jack On Sat, Nov 8, 2014 at 4:03 PM, Iain Sandoe i...@codesourcery.com wrote: On 8 Nov 2014, at 15:41, Jakub Jelinek wrote: On Sat, Nov 08, 2014 at 04:31:28PM +0100, Dominique d'Humières wrote: I am still unable to bootstrap darwin14 without revision r216964 reverted. Executing the simplified command /opt/gcc/build_w/gcc/xg++ -B/opt/gcc/build_w/gcc/ -L/opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs -o .libs/libcc1.0.so .libs/findcomp.o -static-libstdc++ -static-libgcc I get ld: file not found: libstdc++.a collect2: error: ld returned 1 exit status while I see [Book15] build_w/libcc1% ls -l /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a -rw-r--r-- 1 dominiq staff 9118792 Nov 8 15:30 /opt/gcc/build_w/x86_64-apple-darwin14.0.0/libstdc++-v3/src/.libs/libstdc++.a What am I missing? That is for somebody familiar with all the Mach-O weirdnesses to look at, I don't see anything wrong with the above g++ invocation. Rerun with -v to see how ld is invoked, strace (if Darwin has anything like that) it to see why it doesn't find that libstdc++.a ? This is not really mach-o related, but a consequence of the way in which GCC specs substitution works. Unless the libstdc++-v3/.libs and libsupc++/.libs are visible as -Bx, spec substitution will not work (it's not enough to provide -L). This is done elsewhere (e.g. gnattools), it just needs an appropriate addition to the libcc1/Makefile.in (will try and take a look later in the week, if no-one else gets to it first). Iain
Re: [PATCH] Don't bootstrap libcc1
Le 8 nov. 2014 à 22:55, Jack Howarth howarth.at@gmail.com a écrit : Iain, Any idea why this isn't failing universally? On all of the machines tested here with 'make bootstrap', the linkage of libcc1.so finds the necessary libstdc++ from the set of flags... -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/libsupc++/.libs as I posted earlier in this thread. Why wouldn't Dominique be getting those emitted in his build and shouldn't they suffice? Jack Because you bootstrap with clang (I confirm it) while I am bootstrapping with gcc 4.9 for Ada. Dominique
[PATCH] Reset contexts in possible_polymorphic_call_targets properly
Hi in a patch I work on I store ipa_polymorphic_call_contexts in a vector and thus they do not get properly constructed, merely memset to zero. This means that I happen to be using know nothing contexts which have their outer_type set to NULL but the various flags are also false, unlike in the properly constructed ones. When I pass such context to possible_polymorphic_call_targets, I get wrong complete results of size one because it sets the outer_type to otr_type but leaves the maybe_derived_type flag cleared. So I changed the function to reset the context using clear_outer_type(otr_type) instead, which I believe is the proper way of doing it. However, I had to make that method public to do so. Bootstrapped and tested on x86_64-linux. OK for trunk? Thanks, Martin 2014-11-08 Martin Jambor mjam...@suse.cz * cgraph.h (clear_outer_type): Make public. Fix comment. * ipa-devirt.c (possible_polymorphic_call_targets): Use clear_outer_type when resetting the context. Index: src/gcc/cgraph.h === --- src.orig/gcc/cgraph.h 2014-11-05 17:12:51.284464567 +0100 +++ src/gcc/cgraph.h2014-11-08 19:53:22.726610625 +0100 @@ -1334,6 +1334,10 @@ public: /* Make context non-speculative. */ void clear_speculation (); + /* Produce context specifying all derrived types of OTR_TYPE. If OTR_TYPE is + NULL, the context is set to dummy I know nothing setting. */ + void clear_outer_type (tree otr_type = NULL); + /* Walk container types and modify context to point to actual class containing OTR_TYPE (if non-NULL) as base class. Return true if resulting context is valid. @@ -1374,7 +1378,6 @@ private: bool combine_speculation_with (tree, HOST_WIDE_INT, bool, tree); void set_by_decl (tree, HOST_WIDE_INT); bool set_by_invariant (tree, tree, HOST_WIDE_INT); - void clear_outer_type (tree otr_type = NULL); bool speculation_consistent_p (tree, HOST_WIDE_INT, bool, tree); void make_speculative (tree otr_type = NULL); }; @@ -2724,9 +2727,8 @@ ipa_polymorphic_call_context::clear_spec speculative_maybe_derived_type = false; } -/* Produce context specifying all derrived types of OTR_TYPE. - If OTR_TYPE is NULL or type of the OBJ_TYPE_REF, the context is set - to dummy I know nothing setting. */ +/* Produce context specifying all derrived types of OTR_TYPE. If OTR_TYPE is + NULL, the context is set to dummy I know nothing setting. */ inline void ipa_polymorphic_call_context::clear_outer_type (tree otr_type) Index: src/gcc/ipa-devirt.c === --- src.orig/gcc/ipa-devirt.c 2014-11-03 20:17:30.200011833 +0100 +++ src/gcc/ipa-devirt.c2014-11-08 19:48:47.882600150 +0100 @@ -2281,10 +2281,7 @@ possible_polymorphic_call_targets (tree /* Without outer type, we have no use for offset. Just do the basic search from innter type */ if (!context.outer_type) -{ - context.outer_type = otr_type; - context.offset = 0; -} +context.clear_outer_type (otr_type); /* We need to update our hiearchy if the type does not exist. */ outer_type = get_odr_type (context.outer_type, true); /* If the type is complete, there are no derivations. */
Re: [PATCH] Don't bootstrap libcc1
Dominique, That is curious. I wouldn't have thought that the compiler selection would have had such a radical effect on the linkage flags emitted for the build directories. Jack On Sat, Nov 8, 2014 at 4:59 PM, Dominique d'Humières domi...@lps.ens.fr wrote: Le 8 nov. 2014 à 22:55, Jack Howarth howarth.at@gmail.com a écrit : Iain, Any idea why this isn't failing universally? On all of the machines tested here with 'make bootstrap', the linkage of libcc1.so finds the necessary libstdc++ from the set of flags... -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/src/.libs -L/sw/src/fink.build/gcc50-5.0.0-1000/darwin_objdir/x86_64-apple-darwin13.4.0/libstdc++-v3/libsupc++/.libs as I posted earlier in this thread. Why wouldn't Dominique be getting those emitted in his build and shouldn't they suffice? Jack Because you bootstrap with clang (I confirm it) while I am bootstrapping with gcc 4.9 for Ada. Dominique
Re: [ping] libatomic: Fix sub-word CAS synthesis on LP64 targets
Thank you for approving the patch. I neglected to mention that I do not have write access. Would you or someone else be so kind as to commit this? On Fri, Nov 7, 2014 at 5:20 AM, Richard Henderson r...@redhat.com wrote: On 11/06/2014 09:24 PM, Andrew Waterman wrote: 2014-10-23 Andrew Waterman water...@cs.berkeley.edu * cas_n.c (libat_compare_exchange): Add missing cast. Ok. r~
__float128 typeinfo
Hello, I am digging out https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00637.html It isn't completely clear if the libstdc++ part was accepted or not. I won't commit immediately (waiting on another patch), but I'd like to be ready. The front-end part is included for reference, both versions were approved, I'll use the first one if arm vectors are reworked in the same way as aarch64 before the end of stage 1, and the other one if for some reason it gets delayed. 2014-11-10 Marc Glisse marc.gli...@inria.fr PR libstdc++/43622 gcc/cp/ * rtti.c (emit_support_tinfos): Handle __float128. libstdc++-v3/ * config/abi/pre/float128.ver: New file. * configure.ac: Use float128.ver when relevant. * configure: Regenerate. * testsuite/util/testsuite_abi.cc (check_version): Accept new CXXABI_FLOAT128 version. -- Marc GlisseIndex: gcc/cp/rtti.c === --- gcc/cp/rtti.c (revision 217249) +++ gcc/cp/rtti.c (working copy) @@ -1540,20 +1540,35 @@ emit_support_tinfos (void) return; doing_runtime = 1; for (ix = 0; fundamentals[ix]; ix++) emit_support_tinfo_1 (*fundamentals[ix]); for (ix = 0; ix NUM_INT_N_ENTS; ix ++) if (int_n_enabled_p[ix]) { emit_support_tinfo_1 (int_n_trees[ix].signed_type); emit_support_tinfo_1 (int_n_trees[ix].unsigned_type); } +#if 1 + for (tree t = registered_builtin_types; t; t = TREE_CHAIN (t)) +emit_support_tinfo_1 (TREE_VALUE (t)); +#else + /* Search for an extra floating point type like __float128. */ + for (enum machine_mode mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT); + mode != VOIDmode; + mode = GET_MODE_WIDER_MODE (mode)) +{ + tree type = c_common_type_for_mode (mode, false); + if (type type != float_type_node type != double_type_node + type != long_double_type_node) + emit_support_tinfo_1 (type); +} +#endif } /* Finish a type info decl. DECL_PTR is a pointer to an unemitted tinfo decl. Determine whether it needs emitting, and if so generate the initializer. */ bool emit_tinfo_decl (tree decl) { tree type = TREE_TYPE (DECL_NAME (decl)); Index: libstdc++-v3/config/abi/pre/float128.ver === --- libstdc++-v3/config/abi/pre/float128.ver(revision 0) +++ libstdc++-v3/config/abi/pre/float128.ver(working copy) @@ -0,0 +1,10 @@ +# Appended to version file. + +CXXABI_FLOAT128 { + +# typeinfo and typeinfo name for __float128 +_ZT[IS]g; +_ZT[IS]Pg; +_ZT[IS]PKg; + +}; Index: libstdc++-v3/configure === --- libstdc++-v3/configure (revision 217249) +++ libstdc++-v3/configure (working copy) @@ -15703,20 +15703,23 @@ $as_echo #define _GLIBCXX_USE_FLOAT128 $as_echo $enable_float128 6; } rm -f conftest* ac_ext=c ac_cpp='$CPP $CPPFLAGS' ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext 5' ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS 5' ac_compiler_gnu=$ac_cv_c_compiler_gnu +if test $enable_float128 = yes; then + port_specific_symbol_files=$port_specific_symbol_files \$(top_srcdir)/config/abi/pre/float128.ver +fi # Checks for compiler support that doesn't require linking. # All these tests are for C++; save the language and the compiler flags. # The CXXFLAGS thing is suspicious, but based on similar bits previously # found in GLIBCXX_CONFIGURE. ac_ext=cpp ac_cpp='$CXXCPP $CPPFLAGS' ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext 5' Index: libstdc++-v3/configure.ac === --- libstdc++-v3/configure.ac (revision 217249) +++ libstdc++-v3/configure.ac (working copy) @@ -146,20 +146,23 @@ GLIBCXX_ENABLE_HOSTED # Enable descriptive messages to standard output on termination. GLIBCXX_ENABLE_VERBOSE # Enable compiler support that doesn't require linking. GLIBCXX_ENABLE_SJLJ_EXCEPTIONS GLIBCXX_ENABLE_PCH($is_hosted) GLIBCXX_ENABLE_THREADS GLIBCXX_ENABLE_ATOMIC_BUILTINS GLIBCXX_ENABLE_DECIMAL_FLOAT GLIBCXX_ENABLE_INT128_FLOAT128 +if test $enable_float128 = yes; then + port_specific_symbol_files=$port_specific_symbol_files \$(top_srcdir)/config/abi/pre/float128.ver +fi # Checks for compiler support that doesn't require linking. GLIBCXX_CHECK_COMPILER_FEATURES # Enable all the variable C++ runtime options that don't require linking. GLIBCXX_ENABLE_CSTDIO GLIBCXX_ENABLE_CLOCALE GLIBCXX_ENABLE_ALLOCATOR GLIBCXX_ENABLE_CHEADERS($c_model) dnl c_model from configure.host GLIBCXX_ENABLE_LONG_LONG([yes]) Index: libstdc++-v3/testsuite/util/testsuite_abi.cc === --- libstdc++-v3/testsuite/util/testsuite_abi.cc(revision 217249) +++
Re: [PATCH][11/n] Merge from match-and-simplify, bit patterns from forwprop
On Thu, Nov 6, 2014 at 12:55 AM, Richard Biener rguent...@suse.de wrote: This merges patterns implementing the bitwise patterns from tree-ssa-forwprop.c. I've removed duplicate functionality from fold-const.c as I found them, some may be still lurking in the depths. This also fixes a bug in genmatch which made user-defined predicates matching anything, thus (match foo @0 (if ... not work (that is: ignored). Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2014-11-06 Richard Biener rguent...@suse.de * match.pd: Implement bitwise binary and unary simplifications from tree-ssa-forwprop.c. * fold-const.c (fold_unary_loc): Remove them here. (fold_binary_loc): Likewise. * tree-ssa-forwprop.c (simplify_not_neg_expr): Remove. (truth_valued_ssa_name): Likewise. (lookup_logical_inverted_value): Likewise. (simplify_bitwise_binary_1): Likewise. (hoist_conversion_for_bitop_p): Likewise. (simplify_bitwise_binary_boolean): Likewise. (simplify_bitwise_binary): Likewise. (pass_forwprop::execute): Remove calls to simplify_not_neg_expr and simplify_bitwise_binary. * genmatch.c (dt_node::append_true_op): Use safe_as_a for parent. (decision_tree::insert): Also insert non-expressions. * gcc.dg/tree-ssa/forwprop-28.c: Adjust scanning for the desired transform. This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63787 H.J.
[RFC, C] add warning for unpromoted bit-field uses
We had a request from a customer to add a warning to the C front end to diagnose cases where bit-fields larger than an int are used in shift expressions; confusingly, the operation is done in the precision of the bit-field size rather than its declared type. This is a symptom of a larger problem, of course, as it affects bit-field uses in other expressions besides shifts, too. The C standard specifies that bit-fields have an integral type of the specified number of bits. On the other hand, in C++, bit-fields do have their declared type, so it seems appropriate to tie the new proposed warning to -Wc++-compat. I thought that the point at which integral promotions are applied would be a good place to catch this, as it excludes places where bit-fields are already being converted by assignment or explicit cast. I think we also want to exclude warning about bit-fields whose promoted type is identical to the type that would result from promoting the field's declared type, so it makes sense to emit the warning at the place where we're dealing with such promotions. I've put together the attached patch as a first cut. I haven't done the requisite bootstrap and full testing on it yet, but I thought I'd put it out there for comment first to see if there is consensus that this is a reasonable thing to do. Maybe details like the warning option name and message could be improved, etc, too. Any other feedback? -Sandra 2014-11-08 Sandra Loosemore san...@codesourcery.com gcc/c-family/ * c.opt (-Wbitfield-conversion): New option. gcc/ * doc/invoke.texi (Option Summary): Add -Wbitfield-conversion. (Warning Options): Document it. gcc/c/ * c-typeck.c (perform_integral_promotions): Handle -Wbitfield-conversion. gcc/testsuite/ * gcc.dg/Wbitfield-conversion.c: New test case. Index: gcc/c-family/c.opt === --- gcc/c-family/c.opt (revision 217259) +++ gcc/c-family/c.opt (working copy) @@ -287,6 +287,10 @@ Wbad-function-cast C ObjC Var(warn_bad_function_cast) Warning Warn about casting functions to incompatible types +Wbitfield-conversion +C ObjC Var(warn_bitfield_conversion) Warning LangEnabledBy(C ObjC,Wc++-compat) +Warn about bit-fields not promoted to their declared types + Wbool-compare C ObjC C++ ObjC++ Var(warn_bool_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about boolean expression compared with an integer value different from true/false Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 217259) +++ gcc/doc/invoke.texi (working copy) @@ -291,7 +291,7 @@ Objective-C and Objective-C++ Dialects}. -Wmissing-parameter-type -Wmissing-prototypes -Wnested-externs @gol -Wold-style-declaration -Wold-style-definition @gol -Wstrict-prototypes -Wtraditional -Wtraditional-conversion @gol --Wdeclaration-after-statement -Wpointer-sign} +-Wdeclaration-after-statement -Wpointer-sign -Wbitfield-conversion} @item Debugging Options @xref{Debugging Options,,Options for Debugging Your Program or GCC}. @@ -5265,6 +5265,18 @@ Suppress warnings when a positional init a structure that has been marked with the @code{designated_init} attribute. +@item -Wbitfield-conversion @r{(C and Objective-C only)} +@opindex Wbitfield-conversion +In C, bit-fields have an integral type of the specified bit size, +rather than the declared type of the bit-field as they do in C++. +This may lead to unexpected results when uncasted accesses to bit-fields +of types or sizes larger than @code{int} are used in other expressions. +This option causes GCC to emit a warning suggesting +an explicit cast in situations where the integral promotions do not give +a bit-field value its promoted declared type. + +@option{-Wbitfield-conversion} is enabled by @option{-Wc++-compat}. + @end table @node Debugging Options Index: gcc/c/c-typeck.c === --- gcc/c/c-typeck.c (revision 217259) +++ gcc/c/c-typeck.c (working copy) @@ -2057,6 +2057,27 @@ perform_integral_promotions (tree exp) return convert (type, exp); } + /* In C, bit-fields have an integral type of the specified number of + bits, while in C++ they have their declared types. If both the + declared type and bit-field size are int-sized or smaller, the + integral promotions will smooth over the difference by promoting + to (possibly unsigned) int. Otherwise, we may emit a warning + suggesting an explicit cast. */ + if (warn_bitfield_conversion + TREE_CODE (exp) == COMPONENT_REF + DECL_C_BIT_FIELD (TREE_OPERAND (exp, 1))) +{ + tree field = TREE_OPERAND (exp, 1); + tree fieldtype = DECL_BIT_FIELD_TYPE (field); + + if (0 compare_tree_int (TYPE_SIZE (fieldtype), +TYPE_PRECISION (integer_type_node)) +