[PATCH,Testsuite] Check split_stack is ok for target in tree-prof/split-1.c
Hi: tree-prof/split-1.c use -fsplit-stack in dg-options but not check is ok for target. This patch add "dg-require-effective-target split_stack" for it. Ok for commit ? Paul. ChangeLog 2017-06-11 Chenghua Xu * gcc.dg/tree-prof/split-1.c: Require split_stack support. diff --git a/gcc/testsuite/gcc.dg/tree-prof/split-1.c b/gcc/testsuite/gcc.dg/tree-prof/split-1.c index a42fccf..4b90b63 100644 --- a/gcc/testsuite/gcc.dg/tree-prof/split-1.c +++ b/gcc/testsuite/gcc.dg/tree-prof/split-1.c @@ -1,6 +1,7 @@ /* Test case that we don't get a link-time error when using -fsplit-stack with -freorder-blocks-and-partition. */ /* { dg-require-effective-target freorder } */ +/* { dg-require-effective-target split_stack } */ /* { dg-options "-O2 -fsplit-stack" } */ extern unsigned int sleep (unsigned int);
Re: Patch RFC: disable block partitioning with split stack
> > > > Thanks for explanation. Perhaps we could have this documented, because > > otherwise people will think the option is simply broken. I guess even better > > we could have configure autodetection for the broken linker. > > Committed to mainline with docs as follows. Thanks, the patch however disables rerodering unconditionally because flag_split_stack is set to -1 at that time. I have comitted the following 2017-06-10 Jan Hubicka * opts.c (finish_options): Move test for flag_split_stack after it has been initialized. --- trunk/gcc/opts.c2017/06/11 05:29:34 249104 +++ trunk/gcc/opts.c2017/06/11 09:33:22 249105 @@ -863,19 +863,6 @@ opts->x_flag_reorder_blocks = 1; } - /* If stack splitting is turned on, and the user did not explicitly - request function partitioning, turn off partitioning, as it - confuses the linker when trying to handle partitioned split-stack - code that calls a non-split-stack functions. But if partitioning - was turned on explicitly just hope for the best. */ - if (opts->x_flag_split_stack - && opts->x_flag_reorder_blocks_and_partition - && !opts_set->x_flag_reorder_blocks_and_partition) -opts->x_flag_reorder_blocks_and_partition = 0; - - if (opts->x_flag_reorder_blocks_and_partition - && !opts_set->x_flag_reorder_functions) -opts->x_flag_reorder_functions = 1; /* Pipelining of outer loops is only possible when general pipelining capabilities are requested. */ @@ -927,6 +914,20 @@ } } + /* If stack splitting is turned on, and the user did not explicitly + request function partitioning, turn off partitioning, as it + confuses the linker when trying to handle partitioned split-stack + code that calls a non-split-stack functions. But if partitioning + was turned on explicitly just hope for the best. */ + if (opts->x_flag_split_stack + && opts->x_flag_reorder_blocks_and_partition + && !opts_set->x_flag_reorder_blocks_and_partition) +opts->x_flag_reorder_blocks_and_partition = 0; + + if (opts->x_flag_reorder_blocks_and_partition + && !opts_set->x_flag_reorder_functions) +opts->x_flag_reorder_functions = 1; + /* Tune vectorization related parametees according to cost model. */ if (opts->x_flag_vect_cost_model == VECT_COST_MODEL_CHEAP) {
[PATCH] Fix new split-1.c testcase
The new split-1.c testcase fails on targets that do not support split stack (like 32-bit PowerPC Linux). This patch fixes it by only running the testcase if split stack is supported. It also adds the reorder flag to the options, so that the test actually tests what it says it tests. Is this okay for trunk? Segher 2017-06-11 Segher Boessenkool gcc/testsuite/ * gcc.dg/tree-prof/split-1.c: Require effective target split_stack. Add -freorder-blocks-and-partition to options. --- gcc/testsuite/gcc.dg/tree-prof/split-1.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/tree-prof/split-1.c b/gcc/testsuite/gcc.dg/tree-prof/split-1.c index a42fccf..4de1123 100644 --- a/gcc/testsuite/gcc.dg/tree-prof/split-1.c +++ b/gcc/testsuite/gcc.dg/tree-prof/split-1.c @@ -1,7 +1,8 @@ /* Test case that we don't get a link-time error when using -fsplit-stack with -freorder-blocks-and-partition. */ +/* { dg-require-effective-target split_stack } */ /* { dg-require-effective-target freorder } */ -/* { dg-options "-O2 -fsplit-stack" } */ +/* { dg-options "-O2 -fsplit-stack -freorder-blocks-and-partition" } */ extern unsigned int sleep (unsigned int); -- 1.9.3
Re: [PATCH,Testsuite] Check split_stack is ok for target in tree-prof/split-1.c
Hi Paul, > tree-prof/split-1.c use -fsplit-stack in dg-options but not check is > ok for target. > This patch add "dg-require-effective-target split_stack" for it. > > Ok for commit ? ok with the ChangeLog nit below fixed. Thanks. Rainer > ChangeLog > 2017-06-11 Chenghua Xu ^ two spaces here > > * gcc.dg/tree-prof/split-1.c: Require split_stack support. -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: MAINTAINERS update
On Tue, 30 May 2017, Bernd Schmidt wrote: > On 05/30/2017 09:05 AM, Richard Biener wrote: >> This leaves the nvptx and c6x ports without a maintainer. Do >> you have any recommendations for a successor here? > Not really. It would be a shame to lose the C6X port though. If I'm > CC'd on any bug reports I'm prepared to keep it working - if that's > considered sufficient, I can readd myself as maintainer. I think that would be preferrable. Even if practically it may not make a huge difference, people with less background/involvement will know who to contact, and having an entire port without maintainer just doesn't feel right. Gerald
Re: [PATCH doc] update documentation of x86 -mcx16 option
On Sun, 4 Jun 2017, Sandra Loosemore wrote: > This is good, thanks. I think it's fine for GCC 7 branch as well (I guess > it's not a regression fix, but it seems unlikely to break anything). Plus the regression-only rule does not apply for docs. ;-) Gerald
[PATCH try 2 resend] [i386] Remove warnings for ignoring -mcall-ms2sysv-xlogues.
I appear to have forgotten to cc gcc-patches, sorry about that. There are currently three cases where we issue a warning when disabling -mcall-ms2sysv-xlogues for a function, but I never added a proper warning, so there's no mechanism for disabling it. This is something that I meant to address sooner. I'm thinking that it's better to just remove the warning entirely and document these cases, rather than adding a new warning. Any thoughts? These are the conditions: * the use of -fsplit-stack, * the use of static call chains (not sure if we can ever have that), and * if the function calls __buildin_eh_return. Some of these cases can likely be supported, but they are just on the "not yet tested" list. 2017-06-11 Daniel Santos --- gcc/config/i386/i386.c | 26 +++--- gcc/doc/invoke.texi| 25 - 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index d5c2d46bf5e..2dc6e53c765 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -12772,18 +12772,6 @@ ix86_builtin_setjmp_frame_value (void) return stack_realign_fp ? hard_frame_pointer_rtx : virtual_stack_vars_rtx; } -/* Emits a warning for unsupported msabi to sysv pro/epilogues. */ -static void warn_once_call_ms2sysv_xlogues (const char *feature) -{ - static bool warned_once = false; - if (!warned_once) -{ - warning (0, "-mcall-ms2sysv-xlogues is not compatible with %s", - feature); - warned_once = true; -} -} - /* When using -fsplit-stack, the allocation routines set a field in the TCB to the bottom of the stack plus this much space, measured in bytes. */ @@ -12814,18 +12802,10 @@ ix86_compute_frame_layout (void) gcc_assert (TARGET_SSE); gcc_assert (!ix86_using_red_zone ()); - if (crtl->calls_eh_return) + if (crtl->calls_eh_return || ix86_static_chain_on_stack) { gcc_assert (!reload_completed); m->call_ms2sysv = false; - warn_once_call_ms2sysv_xlogues ("__builtin_eh_return"); - } - - else if (ix86_static_chain_on_stack) - { - gcc_assert (!reload_completed); - m->call_ms2sysv = false; - warn_once_call_ms2sysv_xlogues ("static call chains"); } /* Finally, compute which registers the stub will manage. */ @@ -29290,9 +29270,9 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1, else if (ix86_function_ms_hook_prologue (current_function_decl)) ; - /* TODO: Cases not yet examined. */ + /* TODO: Compatibility not yet examined. */ else if (flag_split_stack) - warn_once_call_ms2sysv_xlogues ("-fsplit-stack"); + ; else { diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index c1168823af7..eec02b43a4f 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -25389,11 +25389,26 @@ using the function attributes @code{ms_abi} and @code{sysv_abi}. @opindex mno-call-ms2sysv-xlogues Due to differences in 64-bit ABIs, any Microsoft ABI function that calls a System V ABI function must consider RSI, RDI and XMM6-15 as clobbered. By -default, the code for saving and restoring these registers is emitted inline, -resulting in fairly lengthy prologues and epilogues. Using -@option{-mcall-ms2sysv-xlogues} emits prologues and epilogues that -use stubs in the static portion of libgcc to perform these saves and restores, -thus reducing function size at the cost of a few extra instructions. +default, the instructions for saving and restoring these registers are emitted +inline, resulting in fairly lengthy pro- and epilogues. Using +@option{-mcall-ms2sysv-xlogues} emits pro- and epilogues that use stubs in the +static portion of libgcc to perform these saves and restores, thus reducing +function size at the cost of executing a few extra instructions. This cost is +theoretically mitigated or eliminated by reduced instruction cache utilization, +temporal locality of the stubs, and the stubs' use of MOV instructions over +PUSH and POP. + +This option is not supported with SEH, so it is completely unavailable on +Windows. It is also silently disabled if a function: + +@enumerate +@item is built with @option{-mno-sse2} or @option{-fsplit-stack}, +@item has @code{__attribute__ ((ms_hook_prologue))}, or +@item either throws an exception or explicitly calls @code{__builtin_eh_return}. +@end enumerate + +Support for @option{-fsplit-stack} and @code{__builtin_eh_return} may be +added at some time in the future, but has not yet been tested. @item -mtls-dialect=@var{type} @opindex mtls-dialect -- 2.11.0
[nvptx, committed, PR79939] Disable constant pool for nvptx
Hi, this patch fixes PR79939, a cc1 hang while trying to emit a constant in the constant pool. Fixed by disabling the constant pool for nvptx. Tested on target nvptx. Committed as obvious. Thanks, - Tom Disable constant pool for nvptx 2017-06-11 Tom de Vries PR target/79939 * config/nvptx/nvptx.c (nvptx_cannot_force_const_mem): New function. Return true. (TARGET_CANNOT_FORCE_CONST_MEM): Redefine to nvptx_cannot_force_const_mem. --- gcc/config/nvptx/nvptx.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 2eb5570..daeec27 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -5328,6 +5328,13 @@ nvptx_goacc_reduction (gcall *call) } } +static bool +nvptx_cannot_force_const_mem (machine_mode mode ATTRIBUTE_UNUSED, + rtx x ATTRIBUTE_UNUSED) +{ + return true; +} + #undef TARGET_OPTION_OVERRIDE #define TARGET_OPTION_OVERRIDE nvptx_option_override @@ -5442,6 +5449,9 @@ nvptx_goacc_reduction (gcall *call) #undef TARGET_GOACC_REDUCTION #define TARGET_GOACC_REDUCTION nvptx_goacc_reduction +#undef TARGET_CANNOT_FORCE_CONST_MEM +#define TARGET_CANNOT_FORCE_CONST_MEM nvptx_cannot_force_const_mem + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-nvptx.h"
Re: [PATCH PR78005]Fix miscompare issue by computing correct guard condition for vectorized loop
On Sat, Jun 10, 2017 at 10:40 AM, Richard Sandiford wrote: > Sorry to return this old patch, but: > > Bin Cheng writes: >> -/* Calculate the number of iterations under which scalar loop will be >> - preferred than vectorized loop. NITERS_PROLOG is the number of >> - iterations of prolog loop. If it's integer const, the integer >> - number is also passed by INT_NITERS_PROLOG. VF is vector factor; >> - TH is the threshold for vectorized loop if CHECK_PROFITABILITY is >> - true. This function also store upper bound of the result in BOUND. */ >> +/* Calculate the number of iterations above which vectorized loop will be >> + preferred than scalar loop. NITERS_PROLOG is the number of iterations >> + of prolog loop. If it's integer const, the integer number is also passed >> + in INT_NITERS_PROLOG. BOUND_PROLOG is the upper bound (included) of >> + number of iterations of prolog loop. VFM1 is vector factor minus one. >> + If CHECK_PROFITABILITY is true, TH is the threshold below which scalar >> + (rather than vectorized) loop will be executed. This function stores >> + upper bound (included) of the result in BOUND_SCALAR. */ >> >> static tree >> vect_gen_scalar_loop_niters (tree niters_prolog, int int_niters_prolog, >> - int bound_prolog, int vf, int th, int *bound, >> - bool check_profitability) >> + int bound_prolog, int vfm1, int th, >> + int *bound_scalar, bool check_profitability) >> { >>tree type = TREE_TYPE (niters_prolog); >>tree niters = fold_build2 (PLUS_EXPR, type, niters_prolog, >> - build_int_cst (type, vf)); >> + build_int_cst (type, vfm1)); >> >> - *bound = vf + bound_prolog; >> + *bound_scalar = vfm1 + bound_prolog; >>if (check_profitability) >> { >> - th++; >> + /* TH indicates the minimum niters of vectorized loop, while we >> + compute the maximum niters of scalar loop. */ >> + th--; > > Are you sure about this last change? It looks like it should be dropping Hi Richard, Thanks for spotting this. I vaguely remember I got this from the way how niter/th was checked in previous peeling code, but did't double check it now. I tend to believe there is inconsistence about th, especially with comment like: /* Threshold of number of iterations below which vectorzation will not be performed. It is calculated from MIN_PROFITABLE_ITERS and PARAM_MIN_VECT_LOOP_BOUND. */ unsigned int th; I also tend to believe the inconsistence was introduced partly because niters in vectorizer stands for latch_niters + 1, while latch_niters in rest of the compiler. and..., > the increment rather than replacing it with a decrement. > > It looks like the threshold is already the maximum niters for the scalar > loop. It's set by: > > min_scalar_loop_bound = ((PARAM_VALUE (PARAM_MIN_VECT_LOOP_BOUND) > * vectorization_factor) - 1); > > /* Use the cost model only if it is more conservative than user specified > threshold. */ > th = (unsigned) min_scalar_loop_bound; > if (min_profitable_iters > && (!min_scalar_loop_bound > || min_profitable_iters > min_scalar_loop_bound)) > th = (unsigned) min_profitable_iters; > > LOOP_VINFO_COST_MODEL_THRESHOLD (loop_vinfo) = th; > > (Note the "- 1" in the min_scalar_loop_bound. The multiplication result > is the minimum niters for the vector loop.) To be honest, min_scalar_loop_bound is more likely for something's lower bound which is the niters for the vector loop. If it refers to the niters scalar loop, it is in actuality the "max" value we should use. I am not quite sure here, partly because I am not a native speaker. > > min_profitable_iters sounds like it _ought_ to be the minimum niters for > which the vector loop is used, but vect_estimate_min_profitable_iters > instead returns the largest niters for which the scalar loop should be > preferred: > > /* Cost model disabled. */ > if (unlimited_cost_model (LOOP_VINFO_LOOP (loop_vinfo))) > { > dump_printf_loc (MSG_NOTE, vect_location, "cost model disabled.\n"); > *ret_min_profitable_niters = 0; > *ret_min_profitable_estimate = 0; > return; > } > [...] > /* Because the condition we create is: > if (niters <= min_profitable_iters) >then skip the vectorized loop. */ > min_profitable_iters--; > [...] > min_profitable_estimate --; > > Also, when deciding whether the threshold needs to be used, we have: > > th = LOOP_VINFO_COST_MODEL_THRESHOLD (loop_vinfo); > if (th >= LOOP_VINFO_VECT_FACTOR (loop_vinfo) - 1 > && !LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo)) > { > if (dump_enabled_p ()) > dump_printf_loc (MSG_NOTE, vect_location, > "Profitability threshold is %d loop iterations.\n", > th); > check
[PATCH 0/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
Hi, I've implemented -Wstring-plus-int and -Wstring-plus-char (like their counterpart in Clang) for GCC. This series of patch has been bootstrapped and regtested. OK for trunk? Currently these options are not enabled by default like Clang does. Maybe we could make them enabled by default or by -Wall/-Wextra later. Xi Ruoyao (6): Move char_type_p prototype into c-common.h New warning option -Wstring-plus-int New warning option -Wstring-plus-char New tests for -Wstring-plus-int New tests for -Wstring-plus-char Document new warning options gcc/c-family/c-common.c| 25 gcc/c-family/c-common.h| 2 ++ gcc/c-family/c-warn.c | 22 ++ gcc/c-family/c.opt | 10 gcc/c/c-typeck.c | 17 +- gcc/cp/call.c | 28 ++ gcc/cp/cp-tree.h | 1 - gcc/cp/tree.c | 2 +- gcc/doc/invoke.texi| 22 +- gcc/testsuite/c-c++-common/Wstring-plus-char.c | 26 + gcc/testsuite/c-c++-common/Wstring-plus-int.c | 26 + gcc/testsuite/g++.dg/Wstring-plus-char-1.C | 16 + gcc/testsuite/g++.dg/Wstring-plus-char-2.C | 26 + gcc/testsuite/g++.dg/Wstring-plus-char-3.C | 32 ++ gcc/testsuite/g++.dg/Wstring-plus-int-1.C | 9 gcc/testsuite/g++.dg/Wstring-plus-int-2.C | 10 16 files changed, 270 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-char.c create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-1.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-2.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-3.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University
[PATCH 1/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch moves prototype of char_type_p into c-common.h, so we can use it in c-family/*. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * c-family/c-common.h (char_type_p): New prototype. * c/c-typeck.c (char_type_p): Make the function extern. * cp/cp-tree.h (char_type_p): Remove prototype. * cp/tree.c (char_type_p): Change the return type to bool. --- gcc/c-family/c-common.h | 1 + gcc/c/c-typeck.c| 3 ++- gcc/cp/cp-tree.h| 1 - gcc/cp/tree.c | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 79072e6..82ed4f6 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -962,6 +962,7 @@ extern tree build_real_imag_expr (location_t, enum tree_code, tree); extern tree build_unary_op (location_t, enum tree_code, tree, bool); extern tree build_binary_op (location_t, enum tree_code, tree, tree, int); extern tree perform_integral_promotions (tree); +extern bool char_type_p (tree); /* These functions must be defined by each front-end which implements a variant of the C language. They are used by port files. */ diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index ba44406..8adfa9a 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see #include "gimplify.h" #include "tree-inline.h" #include "omp-general.h" +#include "c-family/c-common.h" #include "c-family/c-objc.h" #include "c-family/c-ubsan.h" #include "cilk.h" @@ -3605,7 +3606,7 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg) /* Returns true if TYPE is a character type, *not* including wchar_t. */ -static bool +bool char_type_p (tree type) { return (type == char_type_node diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 5dd6023..65c1b82 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6928,7 +6928,6 @@ extern bool cv_qualified_p (const_tree); extern tree cv_unqualified (tree); extern special_function_kind special_function_p (const_tree); extern int count_trees(tree); -extern int char_type_p(tree); extern void verify_stmt_tree (tree); extern linkage_kind decl_linkage (tree); extern duration_kind decl_storage_duration (tree); diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index bb17278..d6c1785 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -4855,7 +4855,7 @@ special_function_p (const_tree decl) /* Returns nonzero if TYPE is a character type, including wchar_t. */ -int +bool char_type_p (tree type) { return (same_type_p (type, char_type_node) -- 2.7.1
[PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch adds warning option -Wstring-plus-int for C/C++. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * c-family/c.opt: New option -Wstring-plus-int. * c-family/c-common.c (pointer_int_sum): Checking for -Wstring-plus-int. --- gcc/c-family/c-common.c | 25 + gcc/c-family/c.opt | 5 + 2 files changed, 30 insertions(+) -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 4395e51..1eee48f 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -3100,6 +3100,31 @@ pointer_int_sum (location_t loc, enum tree_code resultcode, else size_exp = size_in_bytes_loc (loc, TREE_TYPE (result_type)); + /* Handle -Wstring-plus-int, warn for adding string literals + and an integer which may result in a wild pointer. */ + if (warn_string_plus_int + && resultcode == PLUS_EXPR + && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (result_type +{ + tree orig_ptrop = tree_strip_nop_conversions(ptrop); + if (TREE_CODE (orig_ptrop) == ADDR_EXPR) +{ + tree obj = TREE_OPERAND (orig_ptrop, 0); + if (TREE_CODE (obj) == STRING_CST) +{ + tree t = TYPE_DOMAIN (TREE_TYPE (obj)); + if (TREE_CODE (intop) != INTEGER_CST + || tree_int_cst_lt (intop, TYPE_MIN_VALUE (t)) + || int_cst_value (intop) + > int_cst_value (TYPE_MAX_VALUE (t)) + 1) +warning_at (loc, OPT_Wstring_plus_int, +"add %qT to string does not append to " +"the string", +TREE_TYPE (intop)); +} +} +} + /* We are manipulating pointer values, so we don't need to warn about relying on undefined signed overflow. We disable the warning here because we use integer types so fold won't know that diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 37bb236..94ba3eb 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -732,6 +732,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini Under the control of Object Size type, warn about buffer overflow in string manipulation functions like memcpy and strcpy. +Wstring-plus-int +C ObjC C++ ObjC++ Var(warn_string_plus_int) Warning +Warn about adding strings and integers, which is likely an ill-formed +attempt to append the string. + Wsuggest-attribute=format C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning Warn about functions which might be candidates for format attributes. -- 2.7.1
[PATCH 3/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch adds warning option -Wstring-plus-char for C/C++. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * c-family/c-common.h (warn_if_string_plus_char): New prototype. * c-family/c-warn.c (warn_if_string_plus_char): New function. * c-family/c.opt: New option -Wstring-plus-char. * cp/call.c (build_new_op_1): Check for -Wstring-plus-char. * c/c-typeck.c (parser_build_binary_op, build_modify_expr): Check for -Wstring-plus-char. --- gcc/c-family/c-common.h | 1 + gcc/c-family/c-warn.c | 22 ++ gcc/c-family/c.opt | 5 + gcc/c/c-typeck.c| 14 ++ gcc/cp/call.c | 28 5 files changed, 70 insertions(+) -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 82ed4f6..82f1c25 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1501,6 +1501,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool, bool); extern void warn_for_omitted_condop (location_t, tree); extern void warn_for_restrict (unsigned, tree *, unsigned); +extern void warn_if_string_plus_char (location_t, tree, tree); /* Places where an lvalue, or modifiable lvalue, may be required. Used to select diagnostic messages in lvalue_error and diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 35321a6..d804500 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2332,6 +2332,28 @@ warn_for_restrict (unsigned param_pos, tree *argarray, unsigned nargs) arg_positions.length ()); } +/* Like char_type_p, but check the main variant and filter out + char16_t (uint_least16_t) and char32_t (uint_least32_t) in C11. */ +static inline bool +type_main_variant_is_char (tree type) +{ + type = TYPE_MAIN_VARIANT (type); + return char_type_p (type) + && type != uint_least16_type_node + && type != uint_least32_type_node; +} + +void +warn_if_string_plus_char (location_t loc, tree ptrtype, tree inttype) +{ + if (POINTER_TYPE_P (ptrtype) + && type_main_variant_is_char (TREE_TYPE (ptrtype)) + && type_main_variant_is_char (inttype)) +warning_at (loc, OPT_Wstring_plus_char, +"add %qT to string pointer %qT does not append " +"to the string", inttype, ptrtype); +} + /* Callback function to determine whether an expression TP or one of its subexpressions comes from macro expansion. Used to suppress bogus warnings. */ diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 94ba3eb..e13cc6c 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -732,6 +732,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini Under the control of Object Size type, warn about buffer overflow in string manipulation functions like memcpy and strcpy. +Wstring-plus-char +C ObjC C++ ObjC++ Var(warn_string_plus_char) Warning +Warn about adding string pointers and characters, which is likely an +ill-formed attempt to append the string. + Wstring-plus-int C ObjC C++ ObjC++ Var(warn_string_plus_int) Warning Warn about adding strings and integers, which is likely an ill-formed diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 8adfa9a..44e7e02 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -3659,6 +3659,12 @@ parser_build_binary_op (location_t location, enum tree_code code, arg1.get_start (), arg2.get_finish ()); + if (warn_string_plus_char && code == PLUS_EXPR) +{ + warn_if_string_plus_char (location, type1, type2); + warn_if_string_plus_char (location, type2, type1); +} + /* Check for cases such as x+y<, [], () must be non-static member functions. */ case MODIFY_EXPR: + if (code2 == PLUS_EXPR) +{ + /* Saved for warn_if_string_plus_char. */ + orig_type1 = TREE_TYPE (arg1); + orig_type2 = TREE_TYPE (arg2); +} if (code2 != NOP_EXPR) break; /* FALLTHRU */ @@ -5649,6 +5657,11 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1, case ARRAY_REF: memonly = true; break; +case PLUS_EXPR: + /* Saved for warn_if_string_plus_char. */ + orig_type1 = TREE_TYPE (arg1); + orig_type2 = TREE_TYPE (arg2); + break; default: break; @@ -5977,6 +5990,13 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1, switch (code) { case MODIFY_EXPR: + if (code2 == PLUS_EXPR + && (complain & tf_warning) + && warn_string_plus_char) +{ + warn_if_string_plus_char (loc, TREE_TYPE (arg1), orig_type2); + warn_if_string_plus_char (loc, TREE_TYPE (arg2), orig_type2); +} return cp_build_modify_expr (loc, arg1, code2, arg2, complain); case INDIRECT_REF: @@ -6016,6 +6036,14 @@ build_new_op_1 (location_t loc, enum
[PATCH 4/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch adds tests for -Wstring-plus-int. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * testsuite/c-c++-common/Wstring-plus-int.c: New test. * testsuite/g++.dg/Wstring-plus-int-1.C: Ditto. * testsuite/g++.dg/Wstring-plus-int-2.C: Ditto. --- gcc/testsuite/c-c++-common/Wstring-plus-int.c | 26 ++ gcc/testsuite/g++.dg/Wstring-plus-int-1.C | 9 + gcc/testsuite/g++.dg/Wstring-plus-int-2.C | 10 ++ 3 files changed, 45 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/testsuite/c-c++-common/Wstring-plus-int.c b/gcc/testsuite/c-c++-common/Wstring-plus-int.c new file mode 100644 index 000..6172bd0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstring-plus-int.c @@ -0,0 +1,26 @@ +/* Test -Wstring-plus-int. */ + +/* { dg-do compile } */ +/* { dg-options "-Wstring-plus-int" } */ + +extern int getchar(); +extern int offset; + +int main(void) +{ + const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */ + const char *b = "aa" + getchar(); /* { dg-warning "does not append" } */ + const char *c = "aa" + 4; /* { dg-warning "does not append" } */ + const char *d = "aa" + -1; /* { dg-warning "does not append" } */ + const char *e = 'x' + "aa"; /* { dg-warning "does not append" } */ + const char *f = "aa" + offset; /* { dg-warning "does not append" } */ + + /* This is legal (at least Clang think it is). */ + const char *g = "aa" + 3; /* { dg-bogus "does not append" } */ + + /* Although they are strange, still shouldn't + be warned by this warning. Maybe -Warray-bounds. */ + const char (*h)[3] = &"aa" + 1; /* { dg-bogus "does not append" } */ + char i = "aa"[4]; /* { dg-bogus "does not append" } */ + const char *j = "aa" - 1; /* { dg-bogus "does not append" } */ +} diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-1.C b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C new file mode 100644 index 000..fc74428 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C @@ -0,0 +1,9 @@ +/* Test -Wstring-plus-int for C++ wide char types. */ + +/* { dg-do compile } */ +/* { dg-options "-Wstring-plus-int" } */ + +int main(void) +{ + const wchar_t *a = L"aa" + L'a'; /* { dg-warning "does not append" } */ +} diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-2.C b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C new file mode 100644 index 000..b69da41 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C @@ -0,0 +1,10 @@ +/* Test -Wstring-plus-int for C++ 2011 unicode char types. */ + +/* { dg-do compile } */ +/* { dg-options "-std=c++11 -Wstring-plus-int" } */ + +int main(void) +{ + const char16_t *a = u"aa" + u'a'; /* { dg-warning "does not append" } */ + const char32_t *b = U"aa" + U'a'; /* { dg-warning "does not append" } */ +} -- 2.7.1
[PATCH 5/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch adds tests for -Wstring-plus-char. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * testsuite/c-c++-common/Wstring-plus-char.c: New test. * testsuite/g++.dg/Wstring-plus-char-1.C: Ditto. * testsuite/g++.dg/Wstring-plus-char-2.C: Ditto. * testsuite/g++.dg/Wstring-plus-char-3.C: Ditto. --- gcc/testsuite/c-c++-common/Wstring-plus-char.c | 26 + gcc/testsuite/g++.dg/Wstring-plus-char-1.C | 16 + gcc/testsuite/g++.dg/Wstring-plus-char-2.C | 26 + gcc/testsuite/g++.dg/Wstring-plus-char-3.C | 32 ++ 4 files changed, 100 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-char.c create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-1.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-2.C create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-3.C -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/testsuite/c-c++-common/Wstring-plus-char.c b/gcc/testsuite/c-c++-common/Wstring-plus-char.c new file mode 100644 index 000..6d07750 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstring-plus-char.c @@ -0,0 +1,26 @@ +/* Test -Wstring-plus-char. */ + +/* { dg-do compile } */ +/* { dg-options "-Wstring-plus-char" } */ + +typedef __UINT_LEAST16_TYPE__ uint_least16_t; + +char *a; +const char *b; +char c; +const char *d; +uint_least16_t x; + +int main(void) +{ + d = a + c; /* { dg-warning "does not append" } */ + d = b + c; /* { dg-warning "does not append" } */ + d = a + 'a'; /* { dg-warning "does not append" } */ + d = b + 'a'; /* { dg-warning "does not append" } */ + d = 'a' + b; /* { dg-warning "does not append" } */ + d += 'a'; /* { dg-warning "does not append" } */ + d = a + x; /* { dg-bogus "does not append" } */ + d = b + x; /* { dg-bogus "does not append" } */ + d = a + 1u; /* { dg-bogus "does not append" } */ + d = a + 1; /* { dg-bogus "does not append" } */ +} diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-1.C b/gcc/testsuite/g++.dg/Wstring-plus-char-1.C new file mode 100644 index 000..98f6b66 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wstring-plus-char-1.C @@ -0,0 +1,16 @@ +/* Test -Wstring-plus-char for C++ wide char types. */ + +/* { dg-do compile } */ +/* { dg-options "-Wstring-plus-char" } */ + +const wchar_t *a, *d; +wchar_t b; +int c; + +int main(void) +{ + d = a + L'a'; /* { dg-warning "does not append" } */ + d = a + b; /* { dg-warning "does not append" } */ + d = a + c; /* { dg-bogus "does not append" } */ + d = a + 1; /* { dg-bogus "does not append" } */ +} diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-2.C b/gcc/testsuite/g++.dg/Wstring-plus-char-2.C new file mode 100644 index 000..4bb2ba3 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wstring-plus-char-2.C @@ -0,0 +1,26 @@ +/* Test -Wstring-plus-char for C++ 2011 unicode char types. */ + +/* { dg-do compile } */ +/* { dg-options "-std=c++11 -Wstring-plus-char" } */ + +typedef __UINT_LEAST16_TYPE__ uint_least16_t; +typedef __UINT_LEAST32_TYPE__ uint_least32_t; + +const char16_t *a, *d; +char16_t b; +uint_least16_t c; +const char32_t *e, *h; +char32_t f; +uint_least32_t g; + +int main(void) +{ + d = a + u'a'; /* { dg-warning "does not append" } */ + d = a + b; /* { dg-warning "does not append" } */ + d = a + c; /* { dg-bogus "does not append" } */ + d = a + 1; /* { dg-bogus "does not append" } */ + h = e + U'a'; /* { dg-warning "does not append" } */ + h = e + f; /* { dg-warning "does not append" } */ + h = e + g; /* { dg-bogus "does not append" } */ + h = e + 1; /* { dg-bogus "does not append" } */ +} diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-3.C b/gcc/testsuite/g++.dg/Wstring-plus-char-3.C new file mode 100644 index 000..a313059 --- /dev/null +++ b/gcc/testsuite/g++.dg/Wstring-plus-char-3.C @@ -0,0 +1,32 @@ +/* Test -Wstring-plus-char. + This is a more realistic test case. */ + +/* { dg-do compile } */ +/* { dg-options "-Wstring-plus-char" } */ + +class good_string +{ +public: + good_string(const char *); + good_string &operator=(const char *); + good_string operator+(char) const; + operator const char *(void) const; +}; + +/* Someone has forgotten operator+ overload... */ +class bad_string +{ +public: + bad_string(const char *); + bad_string &operator=(const char *); + operator const char *(void) const; +}; + +int main() +{ + good_string good = "aa"; + good = good + 'a'; /* { dg-bogus "does not append" } */ + bad_string bad = "bb"; + bad = bad + 'b'; /* { dg-warning "does not append" } */ + return 0; +} -- 2.7.1
[PATCH 6/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)
This patch adds document of -Wstring-plus-int and -Wstring-plus-char. gcc/ChangeLog: 2017-06-12 Xi Ruoyao * doc/invoke.texi (Warning Options): Document -Wstring-plus-int and -Wstring-plus-char. --- gcc/doc/invoke.texi | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5d41649..4859341 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -311,7 +311,7 @@ Objective-C and Objective-C++ Dialects}. -Wsizeof-pointer-memaccess -Wsizeof-array-argument @gol -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol -Wstrict-aliasing=n -Wstrict-overflow -Wstrict-overflow=@var{n} @gol --Wstringop-overflow=@var{n} @gol +-Wstringop-overflow=@var{n} @gol -Wstring-plus-char -Wstring-plus-int -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol -Wsuggest-final-types @gol -Wsuggest-final-methods -Wsuggest-override @gol -Wmissing-format-attribute -Wsubobject-linkage @gol @@ -5125,6 +5125,26 @@ whether to issue a warning. Similarly to @option{-Wstringop-overflow=3} this setting of the option may result in warnings for benign code. @end table +@item -Wstring-plus-char +@opindex Wstring-plus-char +@opindex Wno-string-plus-char +Warn for adding a character to a string pointer, which seems like a failed +attempt to append to the string. For example, this option will issue a +warning for the code below. + +@smallexample +const char *p = "abc", *q; +q = p + 'a'; +@end smallexample + +@item -Wstring-plus-int +@opindex Wstring-plus-int +@opindex Wno-string-plus-int +Warn for adding an integer to a string literal, which may forms a pointer +out of the bound of the string. The typical examples this warns about are +@samp{"abc" + 'd'}, @samp{"abc" + getchar()} and @samp{"abc" + 5}, but +not @samp{"abc" + 1}. + @item -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @opindex Wsuggest-attribute= @opindex Wno-suggest-attribute= -- 2.7.1
Re: [PATCH] Fix new split-1.c testcase
On Sun, Jun 11, 2017 at 4:40 AM, Segher Boessenkool wrote: > > The new split-1.c testcase fails on targets that do not support split > stack (like 32-bit PowerPC Linux). This patch fixes it by only running > the testcase if split stack is supported. It also adds the reorder > flag to the options, so that the test actually tests what it says it > tests. > > Is this okay for trunk? Whoops, sorry about that. Adding dg-require-effective-target split_stack is fine. Adding an explicit -freorder-blocks-and-partition option is not. Adding the explicit option will cause the test to fail when using gold, as the two options are not compatible. The point of the test is to test that using -fsplit-stack disables the default enabling of -freorder-blocks-and-partition. Thanks. Ian > 2017-06-11 Segher Boessenkool > > gcc/testsuite/ > * gcc.dg/tree-prof/split-1.c: Require effective target split_stack. > Add -freorder-blocks-and-partition to options. > > --- > gcc/testsuite/gcc.dg/tree-prof/split-1.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.dg/tree-prof/split-1.c > b/gcc/testsuite/gcc.dg/tree-prof/split-1.c > index a42fccf..4de1123 100644 > --- a/gcc/testsuite/gcc.dg/tree-prof/split-1.c > +++ b/gcc/testsuite/gcc.dg/tree-prof/split-1.c > @@ -1,7 +1,8 @@ > /* Test case that we don't get a link-time error when using > -fsplit-stack with -freorder-blocks-and-partition. */ > +/* { dg-require-effective-target split_stack } */ > /* { dg-require-effective-target freorder } */ > -/* { dg-options "-O2 -fsplit-stack" } */ > +/* { dg-options "-O2 -fsplit-stack -freorder-blocks-and-partition" } */ > > extern unsigned int sleep (unsigned int); > > -- > 1.9.3 >
Re: Patch RFC: disable block partitioning with split stack
On Sun, Jun 11, 2017 at 2:38 AM, Jan Hubicka wrote: >> > >> > Thanks for explanation. Perhaps we could have this documented, because >> > otherwise people will think the option is simply broken. I guess even >> > better >> > we could have configure autodetection for the broken linker. >> >> Committed to mainline with docs as follows. > > Thanks, the patch however disables rerodering unconditionally because > flag_split_stack is set to -1 at that time. I have comitted the following > > 2017-06-10 Jan Hubicka > > * opts.c (finish_options): Move test for flag_split_stack after > it has been initialized. Argh, sorry about that. Thanks for fixing it. Ian
libbacktrace patch committed: Fix race on parallel initialization
The code in libbacktrace had a race when doing parallel initialization: if two threads start to initialize at the same time, and one completes first, the other, while running in backtrace_initialize, may see that the structure is initialized and thus not change *fileline_fn. The caller will then set state->fileline_fn to *fileline_fn, but since backtrace_initialize has not changed it that will be NULL. The effect is that if the timing is right the code can then call a NULL function pointer. This patch fixes the problem by always initializing *fileline_fn in backtrace_initialize. It adds a test in ttest.c that does 10 backtraces in parallel with an new state. To make writing the test easier I copied the test support functions out of btest.c into testlib.c. While doing that I eliminated some of the duplication in edtest.c by making that use testlib.c as well. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: Makefile.am === --- Makefile.am (revision 249070) +++ Makefile.am (working copy) @@ -89,7 +89,7 @@ TESTS = $(check_PROGRAMS) if NATIVE -btest_SOURCES = btest.c +btest_SOURCES = btest.c testlib.c btest_CFLAGS = $(AM_CFLAGS) -g -O btest_LDADD = libbacktrace.la @@ -100,7 +100,7 @@ stest_LDADD = libbacktrace.la check_PROGRAMS += stest -edtest_SOURCES = edtest.c edtest2_build.c +edtest_SOURCES = edtest.c edtest2_build.c testlib.c edtest_LDADD = libbacktrace.la check_PROGRAMS += edtest @@ -111,6 +111,16 @@ gen_edtest2_build: $(srcdir)/edtest2.c $(SHELL) $(srcdir)/../move-if-change tmp-edtest2_build.c edtest2_build.c echo timestamp > $@ +if HAVE_PTHREAD + +check_PROGRAMS += ttest + +ttest_SOURCES = ttest.c testlib.c +ttest_CFLAGS = -pthread +ttest_LDADD = libbacktrace.la + +endif HAVE_PTHREAD + endif NATIVE # We can't use automake's automatic dependency tracking, because it Index: btest.c === --- btest.c (revision 249070) +++ btest.c (working copy) @@ -43,237 +43,7 @@ POSSIBILITY OF SUCH DAMAGE. */ #include "backtrace.h" #include "backtrace-supported.h" -/* Portable attribute syntax. Actually some of these tests probably - won't work if the attributes are not recognized. */ - -#ifndef GCC_VERSION -# define GCC_VERSION (__GNUC__ * 1000 + __GNUC_MINOR__) -#endif - -#if (GCC_VERSION < 2007) -# define __attribute__(x) -#endif - -#ifndef ATTRIBUTE_UNUSED -# define ATTRIBUTE_UNUSED __attribute__ ((__unused__)) -#endif - -/* Used to collect backtrace info. */ - -struct info -{ - char *filename; - int lineno; - char *function; -}; - -/* Passed to backtrace callback function. */ - -struct bdata -{ - struct info *all; - size_t index; - size_t max; - int failed; -}; - -/* Passed to backtrace_simple callback function. */ - -struct sdata -{ - uintptr_t *addrs; - size_t index; - size_t max; - int failed; -}; - -/* Passed to backtrace_syminfo callback function. */ - -struct symdata -{ - const char *name; - uintptr_t val, size; - int failed; -}; - -/* The backtrace state. */ - -static void *state; - -/* The number of failures. */ - -static int failures; - -/* Return the base name in a path. */ - -static const char * -base (const char *p) -{ - const char *last; - const char *s; - - last = NULL; - for (s = p; *s != '\0'; ++s) -{ - if (IS_DIR_SEPARATOR (*s)) - last = s + 1; -} - return last != NULL ? last : p; -} - -/* Check an entry in a struct info array. */ - -static void -check (const char *name, int index, const struct info *all, int want_lineno, - const char *want_function, int *failed) -{ - if (*failed) -return; - if (all[index].filename == NULL || all[index].function == NULL) -{ - fprintf (stderr, "%s: [%d]: missing file name or function name\n", - name, index); - *failed = 1; - return; -} - if (strcmp (base (all[index].filename), "btest.c") != 0) -{ - fprintf (stderr, "%s: [%d]: got %s expected test.c\n", name, index, - all[index].filename); - *failed = 1; -} - if (all[index].lineno != want_lineno) -{ - fprintf (stderr, "%s: [%d]: got %d expected %d\n", name, index, - all[index].lineno, want_lineno); - *failed = 1; -} - if (strcmp (all[index].function, want_function) != 0) -{ - fprintf (stderr, "%s: [%d]: got %s expected %s\n", name, index, - all[index].function, want_function); - *failed = 1; -} -} - -/* The backtrace callback function. */ - -static int -callback_one (void *vdata, uintptr_t pc ATTRIBUTE_UNUSED, - const char *filename, int lineno, const char *function) -{ - struct bdata *data = (struct bdata *) vdata; - struct info *p; - - if (data->index >= data->max) -{ - fprintf (stderr, "callback_one: callback called too many times\n"); - data->failed = 1; - return
Re: [patch,avr] Add support for devices with flash accessible by LD.
On Friday 09 June 2017 03:59 PM, Georg-Johann Lay wrote: Hi, This patch adds support for devices that can access flash memory by LD* instructions, hence there is no need to put .rodata in RAM. The default linker script for the new multilib versions already supports this feature, it's similar to avrtiny, cf. https://sourceware.org/PR21472 This patch does the following: * Add multilib variants avrxmega3 and avrxmega3/short-calls. * Add new option -mshort-calls for multilib selection between devices with <= 8KiB flash and > 8KiB flash. * Add specs handling for -mshort-calls: The compiler knows if this option is needed or not appropriate (similar to -msp8). * Add new ISA feature AVR_ISA_RCALL for multilib selection via -mshort-calls. * Add a new row to architecture description that contains the start address of flash memory in the RAM address range. (The actual value is not needed). * For devices with flash in RAM space, don't let .rodata objects trigger need for __do_copy_data. * Add some devices. * Add configure test for Binutils PR21472. * Adjust documentation etc. Even the smaller devices with flash <= 8KiB support JMP+CALL, but we get better code by assuming RJMP+RCALL: Jump tables are more efficient and insn length computation is more exact (CALL -> RCALL relaxation would need -mrelax and insn size would still be 4 bytes). Moreover, avr-libc uses __AVR_HAVE_JMP_CALL__ to determine vector entry size, and libgcc uses that macro for flash size estimation. Hi Johann, + AVR_ISA_RCALL = 0x10 /* Use RJMP / RCALL even though RJMP / RCALL +are available (-mshort-calls). */ I think you meant "even though JMP/ CALL are avariable". +AVR_MCU ("attiny817",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny817__", 0x3e00, 0x0, 0x2000) +AVR_MCU ("attiny1616", ARCH_AVRXMEGA3, AVR_ISA_NONE, "__AVR_ATtiny1616__", 0x3800, 0x0, 0x4000) Add attiny1614 device. AVR_MCU ("attiny1614", ARCH_AVRXMEGA3, AVR_ISA_NONE, "__AVR_ATtiny1614__", 0x3800, 0x0, 0x4000) Regards, Pitchumani
Re: [2/2] PR 80769: Incorrect strlen optimisation
Ping*2 Richard Sandiford writes: > In this testcase, we (correctly) record after: > > strcpy (p1, "abcde"); > char *p2 = strchr (p1, '\0'); > strcpy (p2, q); > > that the length of p1 and p2 can be calculated by converting the > second strcpy to: > > tmp = stpcpy (p2, q) > > and then doing tmp - p1 for p1 and tmp - p2 for p2. This is delayed > until we know whether we actually need it. Then: > > char *p3 = strchr (p2, '\0'); > > forces us to calculate the length of p2 in this way. At this point > we had three related strinfos: > > p1: delayed length, calculated from tmp = stpcpy (p2, q) > p2: known length, tmp - p2 > p3: known length, 0 > > After: > > memcpy (p3, "x", 2); > > we use adjust_related_strinfos to add 1 to each length. However, > that didn't do anything for delayed lengths because: > > else if (si->stmt != NULL) > /* Delayed length computation is unaffected. */ > ; > > So after the memcpy we had: > > p1: delayed length, calculated from tmp = stpcpy (p2, q) > p2: known length, tmp - p2 + 1 > p3: known length, 1 > > where the length of p1 was no longer correct. > > I thought about three fixes: > > (1) Make adjust_related_strinfos drop strinfos with a delayed length. > > (2) Make adjust_related_strinfos compute the length itself > (via get_string_length). > > (3) Make get_string_length update all related strinfos. We can then > maintain the invariant that all lengths in a chain are delayed or > none are. > > (3) seemed like the best. get_string_length has already made the > invasive step of changing the code and computing the length for one > strinfo. Updating the related strinfos is just a couple of fold_builds, > of the kind that the pass already does in several places. > > The point is that the code above only triggers if one of the delayed > lengths has been computed. That makes (1) unnecessarily pessimistic. > It also wasn't obvious (to me) from first glance, so (2) might look > more intrusive than it is. I think it becomes easier to reason about > if we do (3) and can assume that all lengths are delayed or none are. > It also makes the min-length/maybe-not-terminated patch easier. > > [ I can go into more detail about why this should be enough to > maintain the invariant, and why the asserts should be valid, > but the message is already pretty long. ] > > Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Thanks, > Richard > > > 2017-05-16 Richard Sandiford > > gcc/ > PR tree-optimization/80769 > * tree-ssa-strlen.c (strinfo): Document that "stmt" is also used > for malloc and calloc. Document the new invariant that all related > strinfos have delayed lengths or none do. > (verify_related_strinfos): Move earlier in file. > (set_endptr_and_length): New function, split out from... > (get_string_length): ...here. Also set the lengths of related > strinfos. > (zero_length_string): Assert that chainsi has known (rather than > delayed) lengths. > (adjust_related_strinfos): Likewise. > > gcc/testsuite/ > PR tree-optimization/80769 > * gcc.dg/strlenopt-31.c: New test. > * gcc.dg/strlenopt-31g.c: Likewise. > > Index: gcc/tree-ssa-strlen.c > === > --- gcc/tree-ssa-strlen.c 2017-05-16 08:00:03.808133862 +0100 > +++ gcc/tree-ssa-strlen.c 2017-05-16 08:20:51.408572143 +0100 > @@ -61,7 +61,13 @@ struct strinfo >tree length; >/* Any of the corresponding pointers for querying alias oracle. */ >tree ptr; > - /* Statement for delayed length computation. */ > + /* This is used for two things: > + > + - To record the statement that should be used for delayed length > + computations. We maintain the invariant that all related strinfos > + have delayed lengths or none do. > + > + - To record the malloc or calloc call that produced this result. */ >gimple *stmt; >/* Pointer to '\0' if known, if NULL, it can be computed as > ptr + length. */ > @@ -451,6 +457,45 @@ set_strinfo (int idx, strinfo *si) >(*stridx_to_strinfo)[idx] = si; > } > > +/* Return the first strinfo in the related strinfo chain > + if all strinfos in between belong to the chain, otherwise NULL. */ > + > +static strinfo * > +verify_related_strinfos (strinfo *origsi) > +{ > + strinfo *si = origsi, *psi; > + > + if (origsi->first == 0) > +return NULL; > + for (; si->prev; si = psi) > +{ > + if (si->first != origsi->first) > + return NULL; > + psi = get_strinfo (si->prev); > + if (psi == NULL) > + return NULL; > + if (psi->next != si->idx) > + return NULL; > +} > + if (si->idx != si->first) > +return NULL; > + return si; > +} > + > +/* Set SI's endptr to ENDPTR and compute its length based on SI->ptr. > + Use LOC for folding. */ > + > +static void > +set_endptr_and_le
Re: Fix pessimistic DImode handling in combine.c:make_field_assignment
Ping Richard Sandiford writes: > Richard Sandiford writes: >> The make_field_assignment code: >> >> src = force_to_mode (src, mode, >> GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT >> ? HOST_WIDE_INT_M1U >> : (HOST_WIDE_INT_1U << len) - 1, >> 0); >> >> would ignore the field length len for DImode, even though DImode can be >> handled using HWIs. I think the code should be testing len instead. >> >> The patch was originally part of the SVE machine_mode series. >> Retesting showed that it changed the asm output on powerpc for a few >> tests, so I thought it should go in separately. Each test change >> seemed to be an improvement. >> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. I no longer have >> access to the compile farm to test on the powerpc boxes there. > > Now tested on powerpc64le-linux-gnu too (thanks to Segher for the access). > >> Thanks, >> Richard >> >> >> 2017-05-24 Richard Sandiford >> >> gcc/ >> * combine.c (make_field_assignment): Check len rather than the mode >> precision when calling force_to_mode. >> >> Index: gcc/combine.c >> === >> --- gcc/combine.c2017-05-03 08:46:32.777861592 +0100 >> +++ gcc/combine.c2017-05-24 09:25:25.170351268 +0100 >> @@ -9634,7 +9634,7 @@ make_field_assignment (rtx x) >> other, pos), >> dest); >>src = force_to_mode (src, mode, >> - GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT >> + len >= HOST_BITS_PER_WIDE_INT >> ? HOST_WIDE_INT_M1U >> : (HOST_WIDE_INT_1U << len) - 1, >> 0);
Re: Reorganise machmode.h headers
Ping Richard Sandiford writes: > Jeff Law writes: >> On 11/16/2016 09:32 AM, Richard Sandiford wrote: >>> Later patches will make machmode.h rely on wide-int.h and the >>> new poly-int.h, so it needs to appear later in the coretypes.h >>> include list. >>> >>> Previously machmode.h included insn-modes.h, which as well as >>> the main mode enum contains configuration information like >>> MAX_BITSIZE_MODE_ANY_INT. This still needs to come first, >>> since files like wide-int.h depend on the configuration >>> information. >>> >>> Similarly, later patches will make the auto-generated inline >>> mode size functions use poly-int.h, so the patch splits them >>> out into their own header file and includes it after the >>> integer utilities. >>> >>> The patch also makes the generator files include machmode.h >>> via coretypes.h. Previously they did it by more indirect means. >>> >>> Finally, the patch makes wide-int-print.h available via coretypes.h >>> too. There didn't seem to be any reason to force only the print >>> routines to be included directly, and it would be painful to extend >>> that approach to the new polynomial integer classes. >>> >>> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? >>> >>> Thanks, >>> Richard >>> >>> >>> [ This patch is part of the SVE series posted here: >>> https://gcc.gnu.org/ml/gcc/2016-11/msg00030.html ] >>> >>> gcc/ >>> 2016-11-16 Richard Sandiford >>> Alan Hayward >>> David Sherwood >>> >>> * Makefile.in (MACHMODE_H): Remove insn-modes.h >>> (CORETYPES_H): New define. >>> (MOSTLYCLEANFILES): Add insn-modes-inline.h. >>> (insn-modes-inline.h, s-modes-inline-h): New rules. >>> (generated_files): Add insn-modes-inline.h. >>> (RTL_BASE_H, TREE_CORE_H): Use CORETYPES_H instead of coretypes.h. >>> (build/gensupport.o, build/print-rtl.o, build/read-md.o): Likewise. >>> (build/read-rtl.o, build/rtl.o, build/vec.o, build/hash-table.o) >>> (build/inchash.o, build/gencondmd.o, build/genattr.o): Likewise. >>> (build/genattr-common.o, build/genattrtab.o, build/genautomata.o) >>> (build/gencheck.o, build/gencodes.o, build/genconditions.o): Likewise. >>> (build/genconfig.o, build/genconstants.o, build/genemit.o): Likewise. >>> (build/genenums.o, build/genextract.o, build/genflags.o): Likewise. >>> (build/gentarget-def.o, build/genmddeps.o, build/genopinit.o) >>> (build/genoutput.o, build/genpeep.o, build/genpreds.o): Likewise. >>> (build/genrecog.o, build/genmddump.o, build/genmatch.o): Likewise. >>> (build/gencfn-macros.o, build/gcov-iov.o): Likewise. >>> * coretypes.h: Include everything up to real.h for generators. >>> Include insn-modes.h first. Include wide-int-print.h after >>> wide-int.h. Include insn-modes-inline.h and then machmode.h. >>> * machmode.h: Don't include insn-modes.h here. >>> * function-tests.c: Remove includes of signop.h, machmode.h, >>> double-int.h and wide-int.h. >>> * rtl.h: Likewise. >>> * gcc-rich-location.c: Remove includes of machmode.h, double-int.h >>> and wide-int.h. >>> * optc-save-gen.awk: Likewise. >>> * gencheck.c (BITS_PER_UNIT): Delete dummy definition. >>> * godump.c: Remove include of wide-int-print.h. >>> * pretty-print.h: Likewise. >>> * wide-int-print.cc: Likewise. >>> * wide-int.cc: Likewise. >>> * hash-map-tests.c: Remove include of signop.h. >>> * hash-set-tests.c: Likewise. >>> * rtl-tests.c: Likewise. >>> * mkconfig.sh: Remove include of machmode.h. >>> * genmodes.c (emit_insn_modes_h): Split emission of inline functions >>> into... >>> (emit_insn_modes_inline_h): ...this new function. Emit the code >>> into an insn-modes-inline.h header file, adding appropriate >>> include guards and end comments. >>> (emit_insn_modes_c_header): Remove include of machmode.h. >>> (emit_min_insn_modes_c_header): Include coretypes.h rather than >>> machmode.h. >>> (main): Handle -i flag and call emit_insn_modes_inline_h when >>> it is passed. >> So I don't see anything here particularly problematical. My question is >> whether or not there's anything significant to be gained to moving >> forward with this kit, assuming the 67 piece kit is not likely to move >> forward. >> >> I do think you'll need some tweaks to the contrib/header-tools which >> know about the core headers and dependencies. Hopefully what's in there >> is easy enough to figure out how to twiddle appropriately. > > OK, thanks for the pointer. I think this patch should do that. > > gcc-order-headers seems to have bitrotted a bit, since it gives: > > Traceback (most recent call last): > File "../contrib/header-tools/gcc-order-headers", line 267, in > process_known_dups () > File "../contrib/header-tools/gcc-order-headers", line 101, in > process_known_dups > if dups[i] and "rtl.h" in dups[i]: > KeyError: 'dumpfile.h' > > But the change itself