[PATCH,Testsuite] Check split_stack is ok for target in tree-prof/split-1.c

2017-06-11 Thread Paul Hua
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

2017-06-11 Thread Jan Hubicka
> >
> > 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

2017-06-11 Thread Segher Boessenkool
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

2017-06-11 Thread Rainer Orth
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

2017-06-11 Thread Gerald Pfeifer
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

2017-06-11 Thread Gerald Pfeifer
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.

2017-06-11 Thread Daniel Santos
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

2017-06-11 Thread Tom de Vries

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

2017-06-11 Thread Bin.Cheng
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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)

2017-06-11 Thread Xi Ruoyao
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

2017-06-11 Thread Ian Lance Taylor
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

2017-06-11 Thread Ian Lance Taylor
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

2017-06-11 Thread Ian Lance Taylor
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.

2017-06-11 Thread Pitchumani Sivanupandi

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

2017-06-11 Thread Richard Sandiford
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

2017-06-11 Thread Richard Sandiford
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

2017-06-11 Thread Richard Sandiford
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