Re: [RFC][SSA] Iterator to visit SSA

[Kugan Vivekanandarajah]

Hi Richard,

On 5 September 2016 at 17:57, Richard Biener  wrote:
> On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
>  wrote:
>> Hi All,
>>
>> While looking at gcc source, I noticed that we are iterating over SSA
>> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
>> in others. It seems that variable 0 is always NULL TREE.
>
> Yeah, that's artificial because we don't assign SSA name version 0 (for some
> unknown reason).
>
>> But it can
>> confuse people who are looking for the first time. Therefore It might
>> be good to follow some consistent usage here.
>>
>> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
>> other case. Here is attempt to do this based on what is done in other
>> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
>> new regressions. is this OK?
>
> First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
> as SSAVAR might be confused with iterating over SSA_NAME_VAR.
>
> Then, if you add an iterator why leave the name == NULL handling to consumers?
> That looks odd.
>
> Then, SSA names are just in a vector thus why not re-use the vector iterator?
>
> That is,
>
> #define FOR_EACH_SSA_NAME (name) \
>   for (unsigned i = 1; SSANAMES (cfun).iterate (i, ); ++i)
>
> would be equivalent to your patch?
>
> Please also don't add new iterators that implicitely use 'cfun' but always use
> one that passes it as explicit arg.

I think defining FOR_EACH_SSA_NAME with vector iterator is better. But
we will not be able to skill NULL ssa_names with that. I also added
index variable to the macro so that there want be any conflicts if the
index variable "i" (or whatever) is also defined in the loop.

Bootstrapped and regression tested on x86_64-linux-gnu with no new
regressions. Is this OK for trunk?

Thanks,
Kugan


gcc/ChangeLog:

2016-09-06  Kugan Vivekanandarajah  

* tree-ssanames.h (FOR_EACH_SSA_NAME): New.
* cfgexpand.c (update_alias_info_with_stack_vars): Use
FOR_EACH_SSA_NAME to iterate over SSA variables.
(pass_expand::execute): Likewise.
* omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
* tree-cfg.c (dump_function_to_file): Likewise.
* tree-into-ssa.c (pass_build_ssa::execute): Likewise.
(update_ssa): Likewise.
* tree-ssa-alias.c (dump_alias_info): Likewise.
* tree-ssa-ccp.c (ccp_finalize): Likewise.
* tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
(create_outofssa_var_map): Likewise.
(coalesce_ssa_name): Likewise.
* tree-ssa-operands.c (dump_immediate_uses): Likewise.
* tree-ssa-pre.c (compute_avail): Likewise.
* tree-ssa-sccvn.c (init_scc_vn): Likewise.
(scc_vn_restore_ssa_info): Likewise.
(free_scc_vn): Likwise.
(run_scc_vn): Likewise.
* tree-ssa-structalias.c (compute_points_to_sets): Likewise.
* tree-ssa-ter.c (new_temp_expr_table): Likewise.
* tree-ssa-copy.c (fini_copy_prop): Likewise.
* tree-ssa.c (verify_ssa): Likewise.

>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Kugan
>>
>>
>> gcc/ChangeLog:
>>
>> 2016-09-05  Kugan Vivekanandarajah  
>>
>> * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
>> (ssa_iterator::get): Likewise.
>> (ssa_iterator::next): Likewise.
>> (FOR_EACH_SSAVAR): Likewise.
>> * cfgexpand.c (update_alias_info_with_stack_vars): Use
>> FOR_EACH_SSAVAR to iterate over SSA variables.
>> (pass_expand::execute): Likewise.
>> * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
>> * tree-cfg.c (dump_function_to_file): Likewise.
>> * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
>> (update_ssa): Likewise.
>> * tree-ssa-alias.c (dump_alias_info): Likewise.
>> * tree-ssa-ccp.c (ccp_finalize): Likewise.
>> * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
>> (create_outofssa_var_map): Likewise.
>> (coalesce_ssa_name): Likewise.
>> * tree-ssa-operands.c (dump_immediate_uses): Likewise.
>> * tree-ssa-pre.c (compute_avail): Likewise.
>> * tree-ssa-sccvn.c (init_scc_vn): Likewise.
>> (scc_vn_restore_ssa_info): Likewise.
>> (free_scc_vn): Likwise.
>> (run_scc_vn): Likewise.
>> * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
>> * tree-ssa-ter.c (new_temp_expr_table): Likewise.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 130a16b..6f94335 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -815,12 +815,12 @@ update_alias_info_with_stack_vars (void)
   if (decls_to_partitions)
 {
   unsigned i;
+  tree name;
   hash_set visited;
   bitmap temp = BITMAP_ALLOC (_var_bitmap_obstack);
 
-  for (i = 1; i < num_ssa_names; i++)
+  FOR_EACH_SSA_NAME (i, name)
{
- tree name = ssa_name (i);
  struct ptr_info_def *pi;
 
  if (name
@@ -6163,6 +6163,7 @@ pass_expand::execute (function *fun)
   edge e;
   

Re: [PATCH] Delete GCJ

[Eric Gallager]

On 9/5/16, Bernd Edlinger  wrote:
> On 05.09.2016 17:13, Andrew Haley wrote:
>  > As discussed.  I think I should ask a Global reviewer to approve this
>  > one.  For obvious reasons I haven't included the diffs to the deleted
>  > gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
>  > is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
>  > if anyone would like to try it.
>  >
>  > Andrew.
>  >
>  >
>  > 2016-09-05  Andrew Haley  
>  >
>  >* Makefile.def: Remove libjava.
>  >* Makefile.tpl: Likewise.
>  >* Makefile.in: Regenerate.
>  >* configure.ac: Likewise.
>  >* configure: Likewise.
>  >* gcc/java: Remove.
>  >* libjava: Likewise.
>
>
> I think you can remove libffi as well, right?
>
>


I think Go uses libffi as well; for further discussion see bug 11660:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=11660
(people there seem to think ObjC uses libffi, too, but I haven't
noticed it doing that myself)

Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[David Malcolm]

On Mon, 2016-09-05 at 21:00 +0200, Uros Bizjak wrote:
> On Mon, Sep 5, 2016 at 7:25 PM, Jakub Jelinek 
> wrote:
> > Hi!
> > 
> > While most of the i386.opt -m= options have enum args and thus
> > cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy=
> > (and also
> > -mrecip=) don't use that, with the CPU strings being maintained
> > inside of a
> > function rather than in some *.def file that could be also sourced
> > into the
> > *.opt or something (and similarly for the strategies).
> > 
> > This patch adds inform calls that handle those similarly to what
> > cmdline_handle_error does for the options with enum values.
> > In addition, it adds %qs instead of %s in a couple of spaces, and
> > stops reporting incorrect attribute option("march=...") when it is
> > target("march=...") etc.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> > 
> > 2016-09-05  Jakub Jelinek  
> > 
> > PR middle-end/77475
> > * config/i386/i386.c: Include spellcheck.h.
> > (ix86_parse_stringop_strategy_string): Simplify, use %qs
> > instead of %s
> > where desirable, use argument instead of arg in the
> > diagnostic
> > wording, add list of supported strategies and spellcheck
> > hint.
> > (ix86_option_override_internal): Emit target("m...")
> > instead of
> > option("m...") in the diagnostic.  Use %qs instead of %s in
> > invalid
> > -march/-mtune option diagnostic.  Add list of supported
> > arches/tunings
> > and spellcheck hint.
> > 
> > * gcc.target/i386/pr65990.c: Adjust expected diagnostics.
> 
> OK as far as x86 target is concerned, but please allow a day for
> David
> to eventually comment on the implementation.

The calls into spellcheck.h are minimal and look reasonable (I can't
really comment on the x86 aspects).  So I have little to add beyond the
cleanups that Manu already observed.

One thing: shouldn't this have testcases that give test coverage for
emitting hints for -march and -mtune? Something like
  gcc/testsuite/gcc.dg/spellcheck-options-11.c
(though obviously the new ones would be target-specific).


> Thanks,
> Uros.
> 
> > --- gcc/config/i386/i386.c.jj   2016-09-05 12:41:03.0 +0200
> > +++ gcc/config/i386/i386.c  2016-09-05 16:44:45.184981211 +0200
> > @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
> >  #include "case-cfn-macros.h"
> >  #include "regrename.h"
> >  #include "dojump.h"
> > +#include "spellcheck.h"
> > 
> >  /* This file should be included last.  */
> >  #include "target-def.h"
> > @@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
> >next_range_str = strchr (curr_range_str, ',');
> >if (next_range_str)
> >  *next_range_str++ = '\0';
> > +  const char *opt
> > +   = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
> > 
> >if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
> > alg_name, , align))
> >  {
> > -  error ("wrong arg %s to option %s", curr_range_str,
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("wrong argument %qs to option %qs",
> > curr_range_str, opt);
> >return;
> >  }
> > 
> >if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs
> > != -1))
> >  {
> > -  error ("size ranges of option %s should be increasing",
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("size ranges of option %qs should be increasing",
> > opt);
> >return;
> >  }
> > 
> > @@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
> > 
> >if (i == last_alg)
> >  {
> > -  error ("wrong stringop strategy name %s specified for
> > option %s",
> > - alg_name,
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("wrong stringop strategy name %qs specified for
> > option %s",
> > +alg_name, opt);
> > +
> > + size_t len = 0;
> > + for (i = 0; i < last_alg; i++)
> > +   len += strlen (stringop_alg_names[i]) + 1;
> > +
> > + char *s, *p;
> > + auto_vec  candidates;
> > + s = XALLOCAVEC (char, len);
> > + p = s;
> > + for (i = 0; i < last_alg; i++)
> > + if ((stringop_alg) i != rep_prefix_8_byte ||
> > TARGET_64BIT)
> > +   {
> > + size_t arglen = strlen (stringop_alg_names[i]);
> > + memcpy (p, stringop_alg_names[i], arglen);
> > + p[arglen] = ' ';
> > + p += arglen + 1;
> > + candidates.safe_push (stringop_alg_names[i]);
> > +   }
> > + p[-1] = 0;
> > + const char *hint = find_closest_string (alg_name,
> > );
> > + if (hint)
> > +   inform (input_location,

Re: [PATCH] Fix template-params-12f.C on darwin/vxworks (PR debug/77389)

[Mike Stump]

On Sep 4, 2016, at 10:23 AM, Dominique d'Humières  wrote:
> 
> The same should apply to g++.dg/debug/dwarf2/template-params-12g.C:
> 
> --- ../_clean/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C 
> 2016-08-12 09:59:34.0 +0200
> +++ gcc/testsuite/g++.dg/debug/dwarf2/template-params-12g.C   2016-08-30 
> 11:36:48.0 +0200
> @@ -1,4 +1,4 @@
> -// { dg-options "-gdwarf-2 -dA" }
> +// { dg-options "-gdwarf-2 -gno-strict-dwarf -dA" }
> // { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) 
> DW_TAG_template_value_param" 1 } }
> // { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) 
> DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* 
> DW_AT_type\n\[^\n\]*\[^\n\]* DW_AT_location\n\[^\n\]* 
> DW_OP_addr\n\[^\n\]*_ZN1B1gEv\[^\n\]*\n\[^\n\]* DW_OP_stack_value\n\[^\n\]* 
> DW_OP_piece\n\[^\n\]*\n\[^\n\]* DW_OP_lit0\n\[^\n\]* 
> DW_OP_stack_value\n\[^\n\]* DW_OP_piece" 1 } }
> #include "template-params-12.H »
> 
> Unless someone objects, I’ll commit the patch in the coming days.

Ok.

> I also noticed that the same should apply to 
> g++.dg/debug/dwarf2/imported-decl-2.C (pr57519). Is it OK to commit the 
> following patch to trunk and the gcc-5 and 6 branches?

Ok.

Re: [PATCH] Delete GCJ

[Bernd Edlinger]

On 05.09.2016 17:13, Andrew Haley wrote:
 > As discussed.  I think I should ask a Global reviewer to approve this
 > one.  For obvious reasons I haven't included the diffs to the deleted
 > gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
 > is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
 > if anyone would like to try it.
 >
 > Andrew.
 >
 >
 > 2016-09-05  Andrew Haley  
 >
 >  * Makefile.def: Remove libjava.
 >  * Makefile.tpl: Likewise.
 >  * Makefile.in: Regenerate.
 >  * configure.ac: Likewise.
 >  * configure: Likewise.
 >  * gcc/java: Remove.
 >  * libjava: Likewise.


I think you can remove libffi as well, right?


Bernd.

[PATCH] Fix a few simple cases where -Wparentheses does not warn for omitted middle value

[Bernd Edlinger]

Hi,

I've noticed that there is already a -Wparentheses warning for code like

  int y = x == 2 ?: 1

=> warning: the omitted middle operand in ?: will always be 'true',
suggest explicit middle operand [-Wparentheses]

But it is not emitted for code that uses bool, like:

void foo(bool x)
{
   int y = x ?: 1;

and under C it is not emitted for compound expressions
that end with a comparison, like:

   int y = (i++,i==1) ?: 1;

C++ is OK, but does only miss to warn on the bool data type.

The attached patch should fix these warnings.


Bootstrap and reg-testing is still running,
Is it OK for trunk after reg-testing?



Thanks
Bernd.
gcc/c-family:
2016-09-05  Bernd Edlinger  

	* c-common.c (warn_for_omitted_condop): Also warn for boolean data.

gcc/c:
2016-09-05  Bernd Edlinger  

	* c-parser.c (c_parser_conditional_expression): Pass the rightmost
	COMPOUND_EXPR to warn_for_omitted_condop.

testsuite:
2016-09-05  Bernd Edlinger  

	* c-c++-common/warn-ommitted-condop.c: Add more test cases.


Index: gcc/c/c-parser.c
===
--- gcc/c/c-parser.c	(revision 240001)
+++ gcc/c/c-parser.c	(working copy)
@@ -6423,16 +6423,20 @@ c_parser_conditional_expression (c_parser *parser,
   if (c_parser_next_token_is (parser, CPP_COLON))
 {
   tree eptype = NULL_TREE;
+  tree e;
 
   middle_loc = c_parser_peek_token (parser)->location;
   pedwarn (middle_loc, OPT_Wpedantic, 
 	   "ISO C forbids omitting the middle term of a ?: expression");
-  warn_for_omitted_condop (middle_loc, cond.value);
   if (TREE_CODE (cond.value) == EXCESS_PRECISION_EXPR)
 	{
 	  eptype = TREE_TYPE (cond.value);
 	  cond.value = TREE_OPERAND (cond.value, 0);
 	}
+  e = cond.value;
+  while (TREE_CODE (e) == COMPOUND_EXPR)
+	e = TREE_OPERAND (e, 1);
+  warn_for_omitted_condop (middle_loc, e);
   /* Make sure first operand is calculated only once.  */
   exp1.value = c_save_expr (default_conversion (cond.value));
   if (eptype)
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c	(revision 240001)
+++ gcc/c-family/c-common.c	(working copy)
@@ -10619,7 +10619,8 @@ fold_offsetof (tree expr)
 void
 warn_for_omitted_condop (location_t location, tree cond) 
 { 
-  if (truth_value_p (TREE_CODE (cond))) 
+  if (TREE_CODE (TREE_TYPE (cond)) == BOOLEAN_TYPE
+  || truth_value_p (TREE_CODE (cond))) 
   warning_at (location, OPT_Wparentheses, 
 		"the omitted middle operand in ?: will always be %, "
 		"suggest explicit middle operand");
Index: gcc/testsuite/c-c++-common/warn-ommitted-condop.c
===
--- gcc/testsuite/c-c++-common/warn-ommitted-condop.c	(revision 240001)
+++ gcc/testsuite/c-c++-common/warn-ommitted-condop.c	(working copy)
@@ -1,8 +1,12 @@
 /* { dg-options "-Wparentheses -ftrack-macro-expansion=0" } */
 
+#ifndef __cplusplus
+#define bool _Bool
+#endif
+
 extern void f2 (int);
 
-void bar (int x, int y, int z)
+void bar (int x, int y, int z, bool b)
 {
 #define T(op) f2 (x op y ? : 1) 
 #define T2(op) f2 (x op y ? 2 : 1) 
@@ -16,6 +20,8 @@ extern void f2 (int);
   T(||); /* { dg-warning "omitted middle operand" } */
   T(&&); /* { dg-warning "omitted middle operand" } */
   f2 (!x ? : 1);  /* { dg-warning "omitted middle operand" } */
+  f2 ((x,!x) ? : 1);  /* { dg-warning "omitted middle operand" } */
+  f2 ((x,y,!x) ? : 1);  /* { dg-warning "omitted middle operand" } */
   T2(<); /* { dg-bogus "omitted middle operand" } */
   T2(>); /* { dg-bogus "omitted middle operand" } */
   T2(==); /* { dg-bogus "omitted middle operand" } */
@@ -26,4 +32,5 @@ extern void f2 (int);
   T(*); /* { dg-bogus "omitted middle operand" } */
   T(/); /* { dg-bogus "omitted middle operand" } */
   T(^); /* { dg-bogus "omitted middle operand" } */
+  f2 (b ? : 1);  /* { dg-warning "omitted middle operand" } */
 }

Re: [GCC][PATCH] Add __artificial__ attribute to Aarch64 NEON intrinsics

[Andrew Pinski]

On Mon, Sep 5, 2016 at 4:53 AM, Tamar Christina  wrote:
> Hi all,
>
> This patch adds __artificial__ attribute to the intrinsics
> in arm_neon.h so that costs are associated to the user
> function during profiling and during the intrinsics are
> are hidden in trace.
>
> The attribute does not affect code generation.
>
> No new tests for this since it would require a gdb test
> but regression tests on aarch64-none-elf was performed.

I think this is obvious.  Note I would change it one more step to be:
__extension__ extern__inline TYPE
__attribute__  ((__always_inline__, __gnu_inline__, __artificial__))

The main reason is because in C++98 (and I can't remember if C++11),
template are not supposed to consider a static function being a
candidate (though GCC does not get it right is no reason why not to
change it).

Thanks,
Andrew Pinski



>
> The attribute was added with the following bash script:
>
> #!/bin/bash
>
> # first apply to the ones in #define blocks and add extra \ at the end
> sed -i -r 's/(__inline.+)(__attribute__\s*)\(\((.+)\)\)\s*\\/\1\\\n\2 \(\(\3, 
> __artificial__\)\) \\/m' \
>  gcc/config/aarch64/arm_neon.h
>
> # Then write all normal functions
> sed -i -r 's/(__inline.+)(__attribute__\s*)\(\((.+)\)\)/\1\n\2 \(\(\3, 
> __artificial__\)\)/m' \
>  gcc/config/aarch64/arm_neon.h
>
> # Then correct any trailing whitespaces we might have introduced
> sed -i 's/[ \t]*$//' \
>  gcc/config/aarch64/arm_neon.h
>
> # And then finish up by correcting some attribute values which don't fit the 
> patterns above.
> sed -i -r 's/(__attribute__\s+)\(\((__always_inline__)\)\)\s+\\/\1\(\(\2, 
> __artificial__\)\) \\/m' \
>  gcc/config/aarch64/arm_neon.h
>
> It would be easier I think to review/run the script than the changes.
> But just for completeness I have attached the G-zipped patch file.
>
> Ok for trunk?
>
> PS. I don't have commit rights, so if OK can someone apply it for me?
>
> Thanks,
> Tamar
>
> gcc/
> 2016-08-19  Tamar Christina  
>
> * config/aarch64/arm_neon.h: Add __artificial__ attribute
>   to all inlined functions

[committed] Fix check_effective_target_vect_simd_clones comment

[Jakub Jelinek]

Hi!

I've noticed the comment in check_effective_target_vect_simd_clones
doesn't match the reality, fixed thusly, committed to trunk as obvious.

2016-09-05  Jakub Jelinek  

* lib/target-supports.exp (check_effective_target_vect_simd_clones):
Update comment to mention also avx512f.

--- gcc/testsuite/lib/target-supports.exp.jj2016-08-29 12:17:15.0 
+0200
+++ gcc/testsuite/lib/target-supports.exp   2016-09-03 11:24:19.121609356 
+0200
@@ -2909,10 +2909,10 @@ proc check_effective_target_vect_simd_cl
 } else {
set et_vect_simd_clones_saved($et_index) 0
if { [istarget i?86-*-*] || [istarget x86_64-*-*] } {
-   # On i?86/x86_64 #pragma omp declare simd builds a sse2, avx and
-   # avx2 clone.  Only the right clone for the specified arch will be
-   # chosen, but still we need to at least be able to assemble
-   # avx2.
+   # On i?86/x86_64 #pragma omp declare simd builds a sse2, avx, avx2
+   # and avx512f clone.  Only the right clone for the specified arch
+   # will be chosen, but still we need to at least be able to assemble
+   # avx512f.
if { [check_effective_target_avx512f] } {
set et_vect_simd_clones_saved($et_index) 1
}

Jakub

[committed] Cherry-pick asan fix (PR sanitizer/77396)

[Jakub Jelinek]

Hi!

Upstream has accepted my fix, so I've cherry-picked it to trunk and added
another testcase, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.

2016-09-05  Jakub Jelinek  

PR sanitizer/77396
* asan/asan_globals.cc: Cherry-pick upstream r280657.

* g++.dg/asan/pr77396-2.C: New test.

--- libsanitizer/asan/asan_globals.cc.jj
+++ libsanitizer/asan/asan_globals.cc
@@ -368,10 +368,10 @@ void __asan_unregister_globals(__asan_gl
 // initializer can only touch global variables in the same TU.
 void __asan_before_dynamic_init(const char *module_name) {
   if (!flags()->check_initialization_order ||
-  !CanPoisonMemory())
+  !CanPoisonMemory() ||
+  !dynamic_init_globals)
 return;
   bool strict_init_order = flags()->strict_init_order;
-  CHECK(dynamic_init_globals);
   CHECK(module_name);
   CHECK(asan_inited);
   BlockingMutexLock lock(_for_globals);
@@ -394,7 +394,8 @@ void __asan_before_dynamic_init(const ch
 // TU are poisoned.  It simply unpoisons all dynamically initialized globals.
 void __asan_after_dynamic_init() {
   if (!flags()->check_initialization_order ||
-  !CanPoisonMemory())
+  !CanPoisonMemory() ||
+  !dynamic_init_globals)
 return;
   CHECK(asan_inited);
   BlockingMutexLock lock(_for_globals);
--- gcc/testsuite/g++.dg/asan/pr77396-2.C.jj2016-09-05 14:55:06.569653141 
+0200
+++ gcc/testsuite/g++.dg/asan/pr77396-2.C   2016-09-05 14:52:44.0 
+0200
@@ -0,0 +1,12 @@
+// PR sanitizer/77396
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "check_initialization_order=true" }
+
+struct S { S () { asm volatile ("" : : : "memory"); } };
+static S c;
+
+int
+main ()
+{
+  return 0;
+}

Jakub

Re: [MPX] Fix for PR77267

[Ilya Enkovich]

2016-09-05 12:56 GMT+03:00 Alexander Ivchenko :
> Ok, thanks. The full updated patch is below. I also removed the
> '--whole-archive' thing from -static-libmpxwrappers case.

This version is OK for trunk with proper ChangeLog and after proper testing.

> Would that be possible to backport that patch to 6 branch as well?

I suppose it is safe enough for porting but wait for some time before porting
to make sure it doesn't cause troubles on trunk.

>
> And could you please also address Sergey's comment on adding weak
> symbol that he's made in the bugzilla?

I'll have a look.

Thanks,
Ilya

>
> diff --git a/gcc/config.in b/gcc/config.in
> index fc3321c..a736de3 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1538,6 +1538,12 @@
>  #endif
>
>
> +/* Define if your linker supports --push-state/--pop-state */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#endif
> +
> +
>  /* Define if your linker links a mix of read-only and read-write sections 
> into
> a read-write section. */
>  #ifndef USED_FOR_TARGET
> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
> index 4b9910f..2273170 100644
> --- a/gcc/config/i386/linux-common.h
> +++ b/gcc/config/i386/linux-common.h
> @@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
>  #endif
>  #endif
>
> +#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
> +#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
> +#else
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
> +#define MPX_LD_AS_NEEDED_GUARD_POP ""
> +#endif
> +
>  #ifndef LIBMPX_SPEC
>  #if defined(HAVE_LD_STATIC_DYNAMIC)
>  #define LIBMPX_SPEC "\
>  %{mmpx:%{fcheck-pointer-bounds:\
>  %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>  %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
> --lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
> +%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_PUSH "} -lmpx \
> +%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_POP "} \
> +%{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
>  LIBMPX_LIBS ""
>  #else
>  #define LIBMPX_SPEC "\
> @@ -98,8 +108,8 @@ along with GCC; see the file COPYING3.  If not see
>  #define LIBMPXWRAPPERS_SPEC "\
>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>  %{static:-lmpxwrappers}\
> -%{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
> --lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
> +%{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION "}\
> +-lmpxwrappers %{static-libmpxwrappers: "\
>  LD_DYNAMIC_OPTION "}"
>  #else
>  #define LIBMPXWRAPPERS_SPEC "\
> diff --git a/gcc/configure b/gcc/configure
> index 871ed0c..094 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -29609,6 +29609,30 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
>  $as_echo "$ld_bndplt_support" >&6; }
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
> --push-state/--pop-state options" >&5
> +$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +  ld_pushpopstate_support=yes
> +fi
> +  elif test x$gcc_cv_ld != x; then
> +# Check if linker supports --push-state/--pop-state options
> +if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; 
> then
> +  ld_pushpopstate_support=yes
> +fi
> +  fi
> +fi
> +if test x"$ld_pushpopstate_support" = xyes; then
> +
> +$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" 
> >&5
> +$as_echo "$ld_pushpopstate_support" >&6; }
> +
>  # Configure the subdirectories
>  # AC_CONFIG_SUBDIRS($subdirs)
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 241e82d..93af766 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -6237,6 +6237,27 @@ if test x"$ld_bndplt_support" = xyes; then
>  fi
>  AC_MSG_RESULT($ld_bndplt_support)
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +AC_MSG_CHECKING(linker --push-state/--pop-state options)
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +  ld_pushpopstate_support=yes
> +fi
> +  elif test x$gcc_cv_ld != x; then
> +# Check if linker supports --push-state/--pop-state options
> +if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; 
> then
> +  ld_pushpopstate_support=yes
> 

Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[Manuel López-Ibáñez]

On 05/09/16 20:42, Manuel López-Ibáñez wrote:

On 05/09/16 18:25, Jakub Jelinek wrote:

Hi!

While most of the i386.opt -m= options have enum args and thus
cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
-mrecip=) don't use that, with the CPU strings being maintained inside of a
function rather than in some *.def file that could be also sourced into the
*.opt or something (and similarly for the strategies).

This patch adds inform calls that handle those similarly to what
cmdline_handle_error does for the options with enum values.
In addition, it adds %qs instead of %s in a couple of spaces, and
stops reporting incorrect attribute option("march=...") when it is
target("march=...") etc.


Something like the following should avoid a lot of (future) duplication
(untested):


My proposal had an obvious regression if (e->unknown_error). This one might be 
better:


Index: opts-common.c
===
--- opts-common.c   (revision 239995)
+++ opts-common.c   (working copy)
@@ -1069,6 +1069,38 @@
   decoded->errors = 0;
 }

+static void
+candidates_to_string (char *s, const auto_vec  *candidates)
+{
+  int i;
+  const char *candidate;
+  char *p = s;
+  FOR_EACH_VEC_ELT (*candidates, i, candidate)
+{
+  gcc_assert (candidate);
+  size_t arglen = strlen (candidate);
+  memcpy (p, candidate, arglen);
+  p[arglen] = ' ';
+  p += arglen + 1;
+}
+  p[-1] = 0;
+}
+
+void
+unrecognized_argument_hint (location_t loc, const char *opt, const char *arg,
+const auto_vec  ,
+size_t total_len)
+{
+  char *s = XALLOCAVEC (char, total_len);
+  candidates_to_string (s, );
+  const char *hint = find_closest_string (arg, );
+  if (hint)
+inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
+   opt, s, hint);
+  else
+inform (loc, "valid arguments to %qs are: %s", opt, s);
+}
+
 /* Perform diagnostics for read_cmdline_option and control_warning_option
functions.  Returns true if an error has been diagnosed.
LOC and LANG_MASK arguments like in read_cmdline_option.
@@ -1107,40 +1139,22 @@
   if (errors & CL_ERR_ENUM_ARG)
 {
   const struct cl_enum *e = _enums[option->var_enum];
-  unsigned int i;
-  size_t len;
-  char *s, *p;
-
   if (e->unknown_error)
error_at (loc, e->unknown_error, arg);
   else
-   error_at (loc, "unrecognized argument in option %qs", opt);
+   error_at (loc, "argument %qs to %qs not recognized", arg, opt);

-  len = 0;
-  for (i = 0; e->values[i].arg != NULL; i++)
-   len += strlen (e->values[i].arg) + 1;
-
+  size_t len = 0;
+  unsigned int i;
   auto_vec  candidates;
-  s = XALLOCAVEC (char, len);
-  p = s;
   for (i = 0; e->values[i].arg != NULL; i++)
{
  if (!enum_arg_ok_for_language (>values[i], lang_mask))
-   continue;
- size_t arglen = strlen (e->values[i].arg);
- memcpy (p, e->values[i].arg, arglen);
- p[arglen] = ' ';
- p += arglen + 1;
+   continue;
+ len += strlen (e->values[i].arg) + 1;
  candidates.safe_push (e->values[i].arg);
}
-  p[-1] = 0;
-  const char *hint = find_closest_string (arg, );
-  if (hint)
-   inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
-   option->opt_text, s, hint);
-  else
-   inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
-
+  unrecognized_argument_hint (loc, opt, arg, candidates, len);
   return true;
 }

Re: [PATCHv2] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/05/16 21:23, Denis Campredon wrote:
> Hi,
> Your patch does not emit warning for the following case:
> void f(int j){bool i = j ?: 3;}
>
> But for emit one for
> void f(){bool i = 4 ?: 2;}
> Regards
>

Yes, good point.

It is probably not completely unrealistic
that the middle expression may be accidentally
left out?

The first example is a bit too complicated, because
the warning does only work with integer constants
in the moment, but "j ?: 3" is expanded as "j ? j : 3",
while "4 ?: 2" is expanded as "4 ? 4 : 2" which
matches the check.

I'm not sure if detecting the special case j ?: 3
is worth the effort, but it may be doable.

Should I try that as a follow-up patch?


Thanks
Bernd.

Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[Jakub Jelinek]

On Mon, Sep 05, 2016 at 08:42:37PM +0100, Manuel López-Ibáñez wrote:
> Something like the following should avoid a lot of (future) duplication 
> (untested):

You're right.  I've missed that I actually push the candidates into a vector
anyway, so the concatenation can be done in a common code.

> Index: opts-common.c
> ===
> --- opts-common.c (revision 239995)
> +++ opts-common.c (working copy)
> @@ -1069,6 +1069,40 @@
>decoded->errors = 0;
>  }
> 
> +static void
> +candidates_to_string(char *s, const auto_vec  *candidates)

Formatting, missing space before (.

> +{
> +  int i;
> +  const char *candidate;
> +  char *p = s;
> +  FOR_EACH_VEC_ELT (*candidates, i, candidate)
> +{
> +  gcc_assert (candidate);
> +  size_t arglen = strlen (candidate);
> +  memcpy (p, candidate, arglen);
> +  p[arglen] = ' ';
> +  p += arglen + 1;
> +}
> +  p[-1] = 0;
> +}

As I think this isn't performance sensitive code, I think it would be better
to simplify the callers and compute total_len inside of the following
function (i.e. walk the vector 3 times (once in unrecognized_argument_error
to compute total_len, once in candidates_to_string, once in
find_closest_string).

> +
> +void
> +unrecognized_argument_error (location_t loc, const char *opt, const char 
> *arg,
> +  const auto_vec  ,
> +  size_t total_len)
> +{
> +  error_at (loc, "argument %qs to %qs not recognized", arg, opt);
> +
> +  char *s = XALLOCAVEC (char, total_len);
> +  candidates_to_string (s, );
> +  const char *hint = find_closest_string (arg, );
> +  if (hint)
> +inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> + opt, s, hint);
> +  else
> +inform (loc, "valid arguments to %qs are: %s", opt, s);
> +}
> +
>  /* Perform diagnostics for read_cmdline_option and control_warning_option
> functions.  Returns true if an error has been diagnosed.
> LOC and LANG_MASK arguments like in read_cmdline_option.
> @@ -1107,40 +1141,22 @@
>if (errors & CL_ERR_ENUM_ARG)
>  {
>const struct cl_enum *e = _enums[option->var_enum];
> -  unsigned int i;
> -  size_t len;
> -  char *s, *p;
> -
>if (e->unknown_error)
>   error_at (loc, e->unknown_error, arg);
>else
> - error_at (loc, "unrecognized argument in option %qs", opt);
> -
> -  len = 0;
> -  for (i = 0; e->values[i].arg != NULL; i++)
> - len += strlen (e->values[i].arg) + 1;
> -
> -  auto_vec  candidates;
> -  s = XALLOCAVEC (char, len);
> -  p = s;
> -  for (i = 0; e->values[i].arg != NULL; i++)
>   {
> -   if (!enum_arg_ok_for_language (>values[i], lang_mask))
> - continue;
> -   size_t arglen = strlen (e->values[i].arg);
> -   memcpy (p, e->values[i].arg, arglen);
> -   p[arglen] = ' ';
> -   p += arglen + 1;
> -   candidates.safe_push (e->values[i].arg);
> +   size_t len = 0;
> +   unsigned int i;
> +   auto_vec  candidates;
> +   for (i = 0; e->values[i].arg != NULL; i++)
> + {
> +   if (!enum_arg_ok_for_language (>values[i], lang_mask))
> + continue;
> +   len += strlen (e->values[i].arg) + 1;
> +   candidates.safe_push (e->values[i].arg);
> + }
> +   unrecognized_argument_error (loc, opt, arg, candidates, len);
>   }
> -  p[-1] = 0;
> -  const char *hint = find_closest_string (arg, );
> -  if (hint)
> - inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> - option->opt_text, s, hint);
> -  else
> - inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
> -
>return true;
>  }
> 

Plus obviously the unrecognized_argument_error needs to be declared in some
header file.

That said, for x86 -march/-mtune uses this is problematic, as it uses either
%<-march=%> argument or target("march=") attribute wording there depending
on whether it is a command line option or target attribute.  Though, it is
not good this way for translation anyway.  Perhaps use XNEWVEC instead of
XALLOCAVEC for the all options string, and have the helper function just
return that + hint, inform by itself and then free the string?

Jakub

Re: [v3, patch, variant] user-defined operator& and std::variant

[Tim Shen]

On Sat, Sep 3, 2016 at 12:46 PM, Mikhail Strelnikov wrote:
> Hello,
>
> Following code does not compile,
>
> #include 
>
> namespace n
> {
> template
> void operator&(T) {}
> struct s{};
> }
>
> int main()
> {
> std::variant v;
> std::get(v);
> }
>
> error: include/c++/7.0.0/variant:315:4: error: invalid static_cast
>
> diff -r -u a/libstdc++-v3/include/std/variant 
> b/libstdc++-v3/include/std/variant
> --- a/libstdc++-v3/include/std/variant 2016-08-26 13:34:30.823029400 +0300
> +++ b/libstdc++-v3/include/std/variant 2016-09-03 18:01:26.431299300 +0300
> @@ -312,7 +312,7 @@
>_M_storage() const
>{
>   return const_cast(
> -  static_cast(&_M_first._M_storage));
> +  static_cast(std::__addressof(_M_first._M_storage)));
>}
>
>union

Thanks for the patch! Tested on x86_64-linux-gnu and committed as r239996.

I changed std::__addressof to std::addressof since it's standardized
since C++11.


-- 
Regards,
Tim Shen

Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[Manuel López-Ibáñez]

On 05/09/16 18:25, Jakub Jelinek wrote:

Hi!

While most of the i386.opt -m= options have enum args and thus
cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
-mrecip=) don't use that, with the CPU strings being maintained inside of a
function rather than in some *.def file that could be also sourced into the
*.opt or something (and similarly for the strategies).

This patch adds inform calls that handle those similarly to what
cmdline_handle_error does for the options with enum values.
In addition, it adds %qs instead of %s in a couple of spaces, and
stops reporting incorrect attribute option("march=...") when it is
target("march=...") etc.


Something like the following should avoid a lot of (future) duplication 
(untested):

Index: opts-common.c
===
--- opts-common.c   (revision 239995)
+++ opts-common.c   (working copy)
@@ -1069,6 +1069,40 @@
   decoded->errors = 0;
 }

+static void
+candidates_to_string(char *s, const auto_vec  *candidates)
+{
+  int i;
+  const char *candidate;
+  char *p = s;
+  FOR_EACH_VEC_ELT (*candidates, i, candidate)
+{
+  gcc_assert (candidate);
+  size_t arglen = strlen (candidate);
+  memcpy (p, candidate, arglen);
+  p[arglen] = ' ';
+  p += arglen + 1;
+}
+  p[-1] = 0;
+}
+
+void
+unrecognized_argument_error (location_t loc, const char *opt, const char *arg,
+const auto_vec  ,
+size_t total_len)
+{
+  error_at (loc, "argument %qs to %qs not recognized", arg, opt);
+
+  char *s = XALLOCAVEC (char, total_len);
+  candidates_to_string (s, );
+  const char *hint = find_closest_string (arg, );
+  if (hint)
+inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
+   opt, s, hint);
+  else
+inform (loc, "valid arguments to %qs are: %s", opt, s);
+}
+
 /* Perform diagnostics for read_cmdline_option and control_warning_option
functions.  Returns true if an error has been diagnosed.
LOC and LANG_MASK arguments like in read_cmdline_option.
@@ -1107,40 +1141,22 @@
   if (errors & CL_ERR_ENUM_ARG)
 {
   const struct cl_enum *e = _enums[option->var_enum];
-  unsigned int i;
-  size_t len;
-  char *s, *p;
-
   if (e->unknown_error)
error_at (loc, e->unknown_error, arg);
   else
-   error_at (loc, "unrecognized argument in option %qs", opt);
-
-  len = 0;
-  for (i = 0; e->values[i].arg != NULL; i++)
-   len += strlen (e->values[i].arg) + 1;
-
-  auto_vec  candidates;
-  s = XALLOCAVEC (char, len);
-  p = s;
-  for (i = 0; e->values[i].arg != NULL; i++)
{
- if (!enum_arg_ok_for_language (>values[i], lang_mask))
-   continue;
- size_t arglen = strlen (e->values[i].arg);
- memcpy (p, e->values[i].arg, arglen);
- p[arglen] = ' ';
- p += arglen + 1;
- candidates.safe_push (e->values[i].arg);
+ size_t len = 0;
+ unsigned int i;
+ auto_vec  candidates;
+ for (i = 0; e->values[i].arg != NULL; i++)
+   {
+ if (!enum_arg_ok_for_language (>values[i], lang_mask))
+   continue;
+ len += strlen (e->values[i].arg) + 1;
+ candidates.safe_push (e->values[i].arg);
+   }
+ unrecognized_argument_error (loc, opt, arg, candidates, len);
}
-  p[-1] = 0;
-  const char *hint = find_closest_string (arg, );
-  if (hint)
-   inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
-   option->opt_text, s, hint);
-  else
-   inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
-
   return true;
 }

Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[Uros Bizjak]

On Mon, Sep 5, 2016 at 7:25 PM, Jakub Jelinek  wrote:
> Hi!
>
> While most of the i386.opt -m= options have enum args and thus
> cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
> -mrecip=) don't use that, with the CPU strings being maintained inside of a
> function rather than in some *.def file that could be also sourced into the
> *.opt or something (and similarly for the strategies).
>
> This patch adds inform calls that handle those similarly to what
> cmdline_handle_error does for the options with enum values.
> In addition, it adds %qs instead of %s in a couple of spaces, and
> stops reporting incorrect attribute option("march=...") when it is
> target("march=...") etc.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-09-05  Jakub Jelinek  
>
> PR middle-end/77475
> * config/i386/i386.c: Include spellcheck.h.
> (ix86_parse_stringop_strategy_string): Simplify, use %qs instead of %s
> where desirable, use argument instead of arg in the diagnostic
> wording, add list of supported strategies and spellcheck hint.
> (ix86_option_override_internal): Emit target("m...") instead of
> option("m...") in the diagnostic.  Use %qs instead of %s in invalid
> -march/-mtune option diagnostic.  Add list of supported arches/tunings
> and spellcheck hint.
>
> * gcc.target/i386/pr65990.c: Adjust expected diagnostics.

OK as far as x86 target is concerned, but please allow a day for David
to eventually comment on the implementation.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2016-09-05 12:41:03.0 +0200
> +++ gcc/config/i386/i386.c  2016-09-05 16:44:45.184981211 +0200
> @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
>  #include "case-cfn-macros.h"
>  #include "regrename.h"
>  #include "dojump.h"
> +#include "spellcheck.h"
>
>  /* This file should be included last.  */
>  #include "target-def.h"
> @@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
>next_range_str = strchr (curr_range_str, ',');
>if (next_range_str)
>  *next_range_str++ = '\0';
> +  const char *opt
> +   = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
>
>if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
> alg_name, , align))
>  {
> -  error ("wrong arg %s to option %s", curr_range_str,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("wrong argument %qs to option %qs", curr_range_str, opt);
>return;
>  }
>
>if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs != -1))
>  {
> -  error ("size ranges of option %s should be increasing",
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("size ranges of option %qs should be increasing", opt);
>return;
>  }
>
> @@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
>
>if (i == last_alg)
>  {
> -  error ("wrong stringop strategy name %s specified for option %s",
> - alg_name,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("wrong stringop strategy name %qs specified for option %s",
> +alg_name, opt);
> +
> + size_t len = 0;
> + for (i = 0; i < last_alg; i++)
> +   len += strlen (stringop_alg_names[i]) + 1;
> +
> + char *s, *p;
> + auto_vec  candidates;
> + s = XALLOCAVEC (char, len);
> + p = s;
> + for (i = 0; i < last_alg; i++)
> + if ((stringop_alg) i != rep_prefix_8_byte || TARGET_64BIT)
> +   {
> + size_t arglen = strlen (stringop_alg_names[i]);
> + memcpy (p, stringop_alg_names[i], arglen);
> + p[arglen] = ' ';
> + p += arglen + 1;
> + candidates.safe_push (stringop_alg_names[i]);
> +   }
> + p[-1] = 0;
> + const char *hint = find_closest_string (alg_name, );
> + if (hint)
> +   inform (input_location,
> +   "valid arguments to %qs are: %s; did you mean %qs?",
> +   opt, s, hint);
> + else
> +   inform (input_location, "valid arguments to %qs are: %s",
> +   opt, s);
>return;
>  }
>
> @@ -4565,10 +4592,8 @@ ix86_parse_stringop_strategy_string (cha
>   && !TARGET_64BIT)
> {
>   /* rep; movq isn't available in 32-bit code.  */
> - error ("stringop strategy name %s specified for option %s "
> -"not supported for 32-bit code",
> - alg_name,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("stringop strategy name %qs specified for option %qs "
> +

Re: [PATCH, i386] Fix ICE with a md builtin call (PR target/69255)

[Uros Bizjak]

On Mon, Sep 5, 2016 at 7:14 PM, Jakub Jelinek  wrote:
> Hi!
>
> As the testcase shows, if we want to diagnose a md builtin not enabled in
> the current ISA, we call error and then return const0_rtx.  That isn't a
> good choice if the result is BLKmode, which can happen for vector modes
> that aren't enabled in the current ISA.  In that case, returning target
> is better.  In the PR I've also mentioned DECL_MODE issues, but looking at
> expr.c, I see there:
>   /* DECL_MODE might change when TYPE_MODE depends on attribute target
>  settings for VECTOR_TYPE_P that might switch for the function.  */
>   if (currently_expanding_to_rtl
>   && code == VAR_DECL && MEM_P (decl_rtl)
>   && VECTOR_TYPE_P (type) && exp && DECL_MODE (exp) != mode)
> decl_rtl = change_address (decl_rtl, TYPE_MODE (type), 0);
>   else
> decl_rtl = copy_rtx (decl_rtl);
> so we should be fine.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hm...

... perhaps we can emit a warning here and expand the builtin as a
call? This way, we will mirror the case when builtin requires some
ISA, e.g.:

void foo ()
{
  __builtin_ia32_stmxcsr();
}

$ gcc -S -mno-sse dd.c
dd.c: In function ‘foo’:
dd.c:3:3: warning: implicit declaration of function
‘__builtin_ia32_stmxcsr’ [-Wimplicit-function-declaration]
   __builtin_ia32_stmxcsr();
   ^~

Uros.

> 2016-09-05  Jakub Jelinek  
>
> PR target/69255
> * config/i386/i386.c (ix86_expand_builtin): For builtin with
> unsupported or unknown ISA, return target if non-NULL, instead of
> const0_rtx.
>
> * gcc.target/i386/pr69255-1.c: New test.
> * gcc.target/i386/pr69255-2.c: New test.
> * gcc.target/i386/pr69255-3.c: New test.
>
> --- gcc/config/i386/i386.c.jj   2016-09-02 18:18:10.0 +0200
> +++ gcc/config/i386/i386.c  2016-09-05 12:41:03.341382125 +0200
> @@ -36107,7 +36107,7 @@ ix86_expand_builtin (tree exp, rtx targe
>   error ("%qE needs isa option %s", fndecl, opts);
>   free (opts);
> }
> -  return const0_rtx;
> +  return target ? target : const0_rtx;
>  }
>
>switch (fcode)
> --- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj2016-09-05 
> 12:50:29.455307440 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-1.c   2016-09-05 12:50:06.0 
> +0200
> @@ -0,0 +1,16 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target "no-avx512vl"
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p)
> +{
> +  a = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);/* { dg-error "needs 
> isa option -m32 -mavx512vl" } */
> +}
> +
> +/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { 
> target *-*-* } 13 } */
> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj2016-09-05 
> 12:50:32.931264038 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c   2016-09-05 12:49:57.0 
> +0200
> @@ -0,0 +1,14 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target ""
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p)
> +{
> +  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);/* { dg-error 
> "needs isa option -m32 -mavx512vl" } */
> +}
> --- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj2016-09-05 
> 12:50:35.951226330 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-3.c   2016-09-05 12:49:13.0 
> +0200
> @@ -0,0 +1,16 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target ""
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p, __attribute__ ((__vector_size__ (32))) long long *q)
> +{
> +  *q = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);   /* { dg-error "needs 
> isa option -m32 -mavx512vl" } */
> +}
> +
> +/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { 
> target *-*-* } 13 } */
>
> Jakub

Re: [PATCH] Delete GCJ

[Eric Gallager]

On 9/5/16, Gerald Pfeifer  wrote:
> On Mon, 5 Sep 2016, Andrew Haley wrote:
>> As discussed.  I think I should ask a Global reviewer to approve this
>> one.  For obvious reasons I haven't included the diffs to the deleted
>> gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
>> is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
>> if anyone would like to try it.
>
> And here is the patch for the web pages.
>
> Note I did not include all the removed java/* contents.  Is there
> anything particular you'd like to retain there?
>
> Gerald
>
> Index: index.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v
> retrieving revision 1.1026
> diff -u -r1.1026 index.html
> --- index.html25 Aug 2016 10:55:41 -  1.1026
> +++ index.html5 Sep 2016 16:22:07 -
> @@ -15,8 +15,7 @@
>  C,
>  C++,
>  Objective-C, Fortran,
> -Java, Ada, and Go, as well as libraries for these
> -languages (libstdc++, libgcj,...).
> +Ada, and Go, as well as libraries for these languages (libstdc++,...).
>  GCC was originally written as the compiler for the   href="http://www.gnu.org/gnu/thegnuproject.html;>GNU operating system.
>  The GNU system was developed to be 100% free software, free in the sense
> Index: style.mhtml
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/style.mhtml,v
> retrieving revision 1.131
> diff -u -r1.131 style.mhtml
> --- style.mhtml   23 Aug 2016 06:49:17 -  1.131
> +++ style.mhtml   5 Sep 2016 16:22:08 -
> @@ -10,15 +10,6 @@
>
>  >
>
> -;;; For the "java/" pages, we want the navigation bar.
> -
> - "java/[^/]*.html">
> -  -  
> -  
> - >
> ->
> -
>  ;;; Note that the  line really needs to start in the first
> column.
>
>  
> @@ -105,26 +96,6 @@
>
>
>
> -   "java/[^/]*.html">
> --
> -
> -
> -
> -
> -
> -GCJ Home
> -GCC Home
> -FAQ
> -Documentation
> -Contributing
> -Done with GCJ
> -
> -
> -
> -   >
> -  >
> -
>
>About GCC
>
>


I'd think something should go under the "Caveats" section of
https://gcc.gnu.org/gcc-7/changes.html
too, so people aren't surprised.
(There was never a deprecation notice in the equivalent page for GCC
6, by the way)

Re: [PATCH, i386]: Fix zero-extension optimizations from mask registers (PR target/77476)

[Uros Bizjak]

On Mon, Sep 5, 2016 at 7:07 PM, Jakub Jelinek  wrote:
> Hi!
>
> On Fri, Aug 05, 2016 at 02:22:39PM +0200, Uros Bizjak wrote:
>> 2016-08-05  Uros Bizjak  
>>
>> * config/i386/i386.md (*zero_extendsidi2): Add (*r,*k) alternative.
>> (zero_extenddi2): Ditto.
>> (*zero_extendsi2): Ditto.
>> (*zero_extendqihi2): Ditto.
>
> As the PR says, unfortunately not all kmov instructions are supported by all
> AVX512F supporting ISAs, kmovb is AVX512DQ, kmovw is AVX512F and kmovd and
> kmovq are AVX512BW.
>
> Thus, the following patch enables those alternatives only when the
> instructions are available.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested also with sde
> -knl on the avx512f-* testcase.  Ok for trunk?
>
> 2016-09-05  Jakub Jelinek  
>
> PR target/77476
> * config/i386/i386.md (isa): Add x64_avx512bw.
> (*zero_extendsidi2): For alternative 11 use x64_avx512bw isa.
> (kmov_isa): New mode attr.
> (zero_extenddi2): Use  isa for the last alternative.
> (*zero_extendsi2): Likewise.
> (*zero_extendqihi2): Use avx512dq isa for the last alternative.
>
> * gcc.target/i386/avx512f-pr77476.c: New test.
> * gcc.target/i386/avx512bw-pr77476.c: New test.
> * gcc.target/i386/avx512dq-pr77476.c: New test.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386.md.jj  2016-08-29 12:17:41.0 +0200
> +++ gcc/config/i386/i386.md 2016-09-05 10:35:53.404313654 +0200
> @@ -799,7 +799,7 @@ (define_attr "isa" "base,x64,x64_sse4,x6
> sse2,sse2_noavx,sse3,sse4,sse4_noavx,avx,noavx,
> avx2,noavx2,bmi,bmi2,fma4,fma,avx512f,noavx512f,
> fma_avx512f,avx512bw,noavx512bw,avx512dq,noavx512dq,
> -   avx512vl,noavx512vl,x64_avx512dq"
> +   avx512vl,noavx512vl,x64_avx512dq,x64_avx512bw"
>(const_string "base"))
>
>  (define_attr "enabled" ""
> @@ -812,6 +812,8 @@ (define_attr "enabled" ""
>(symbol_ref "TARGET_64BIT && TARGET_AVX")
>  (eq_attr "isa" "x64_avx512dq")
>(symbol_ref "TARGET_64BIT && TARGET_AVX512DQ")
> +(eq_attr "isa" "x64_avx512bw")
> +  (symbol_ref "TARGET_64BIT && TARGET_AVX512BW")
>  (eq_attr "isa" "nox64") (symbol_ref "!TARGET_64BIT")
>  (eq_attr "isa" "sse2") (symbol_ref "TARGET_SSE2")
>  (eq_attr "isa" "sse2_noavx")
> @@ -3735,12 +3737,14 @@ (define_insn "*zero_extendsidi2"
>[(set (attr "isa")
>   (cond [(eq_attr "alternative" "0,1,2")
>   (const_string "nox64")
> -   (eq_attr "alternative" "3,7,11")
> +   (eq_attr "alternative" "3,7")
>   (const_string "x64")
> (eq_attr "alternative" "8")
>   (const_string "x64_sse4")
> (eq_attr "alternative" "10")
>   (const_string "sse2")
> +   (eq_attr "alternative" "11")
> + (const_string "x64_avx512bw")
>]
>(const_string "*")))
> (set (attr "type")
> @@ -3804,6 +3808,9 @@ (define_split
> (set (match_dup 4) (const_int 0))]
>"split_double_mode (DImode, [0], 1, [3], [4]);")
>
> +(define_mode_attr kmov_isa
> +  [(QI "avx512dq") (HI "avx512f") (SI "avx512bw") (DI "avx512bw")])
> +
>  (define_insn "zero_extenddi2"
>[(set (match_operand:DI 0 "register_operand" "=r,*r")
> (zero_extend:DI
> @@ -3812,7 +3819,8 @@ (define_insn "zero_extenddi2"
>"@
> movz{l|x}\t{%1, %k0|%k0, %1}
> kmov\t{%1, %k0|%k0, %1}"
> -  [(set_attr "type" "imovx,mskmov")
> +  [(set_attr "isa" "*,")
> +   (set_attr "type" "imovx,mskmov")
> (set_attr "mode" "SI")])
>
>  (define_expand "zero_extendsi2"
> @@ -3863,7 +3871,8 @@ (define_insn "*zero_extendsi2"
>"@
> movz{l|x}\t{%1, %0|%0, %1}
> kmov\t{%1, %0|%0, %1}"
> -  [(set_attr "type" "imovx,mskmov")
> +  [(set_attr "isa" "*,")
> +   (set_attr "type" "imovx,mskmov")
> (set_attr "mode" "SI,")])
>
>  (define_expand "zero_extendqihi2"
> @@ -3914,6 +3923,7 @@ (define_insn "*zero_extendqihi2"
> movz{bl|x}\t{%1, %k0|%k0, %1}
> kmovb\t{%1, %k0|%k0, %1}"
>[(set_attr "type" "imovx,mskmov")
> +   (set_attr "isa" "*,avx512dq")
> (set_attr "mode" "SI,QI")])
>
>  (define_insn_and_split "*zext_doubleword_and"
> --- gcc/testsuite/gcc.target/i386/avx512f-pr77476.c.jj  2016-09-05 
> 10:23:42.108364379 +0200
> +++ gcc/testsuite/gcc.target/i386/avx512f-pr77476.c 2016-09-05 
> 10:23:26.0 +0200
> @@ -0,0 +1,76 @@
> +/* PR target/77476 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -mavx512f" } */
> +/* { dg-require-effective-target avx512f } */
> +
> +#ifndef PR77476_TEST
> +#include "avx512f-check.h"
> +#define PR77476_TEST avx512f_test
> +#endif
> +
> +unsigned short s;
> +unsigned int i;
> +unsigned long long l;
> +
> +void
> +f1 (void)
> +{
> +  unsigned char a = 0xff;
> +  asm volatile ("" : "+Yk" (a));
> +  s = a;
> +}
> +
> +void
> +f2 

Re: [PATCH] Delete GCJ

[Eric Gallager]

On 9/5/16, Matthias Klose  wrote:
> On 05.09.2016 17:13, Andrew Haley wrote:
>> As discussed.  I think I should ask a Global reviewer to approve this
>> one.  For obvious reasons I haven't included the diffs to the deleted
>> gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
>> is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
>> if anyone would like to try it.
>>
>> Andrew.
>>
>>
>> 2016-09-05  Andrew Haley  
>>
>>  * Makefile.def: Remove libjava.
>>  * Makefile.tpl: Likewise.
>>  * Makefile.in: Regenerate.
>>  * configure.ac: Likewise.
>>  * configure: Likewise.
>>  * gcc/java: Remove.
>>  * libjava: Likewise.
>
> Please consider removing boehm-gc as well.  The only other user is
> --enable-objc-gc, which better should use an external boehm-gc.
>
> Matthias
>
>


How about a compromise to have it be downloaded with the
contrib/download_prerequisites script, instead of entirely keeping it,
or entirely deleting it?

Eric

Ping**2! Re: [PATCH, Fortran] Extension: AUTOMATIC/STATIC symbol attributes with -fdec-static

[Fritz Reese]

https://gcc.gnu.org/ml/fortran/2016-08/msg00173.html
On Mon, Aug 29, 2016 at 8:36 AM, Fritz Reese  wrote:
>
> https://gcc.gnu.org/ml/fortran/2016-08/msg00077.html
> On Wed, Aug 17, 2016 at 7:20 AM, Fritz Reese  wrote:
> > This patch extends the GNU Fortran front-end to add support for
> > DEC-style AUTOMATIC and STATIC symbol attributes with a new flag
> > -fdec-static, allowing explicit control of variable storage. AUTOMATIC
> > local variables are placed on the stack, whereas STATIC variables are
> > placed in static storage.
>
> Patch still pending review.
>
> > Bootstraps and regtests on x86_64-redhat-linux. Questions, comments,
> > and critique welcome. Ok for trunk?
> ...
> >
> > 08-17-2016  Fritz Reese  
> >
> > gcc/fortran/
> > * lang.opt, invoke.texi, gfortran.texi: New flag -fdec-static.
> > * options.c (set_dec_flags): Set -fdec-static with -fdec.
> > * gfortran.h (symbol_attribute): New attribute AUTOMATIC.
> > * gfortran.h (gfc_add_automatic): New prototype.
> > * match.h (gfc_match_automatic, gfc_match_static): New functions.
> > * decl.c (gfc_match_automatic, gfc_match_static): Ditto.
> > * symbol.c (gfc_add_automatic): Ditto.
> > * decl.c (match_attr_spec): Match AUTOMATIC and STATIC decls.
> > * parse.c (decode_specification_statement, decode_statement): Ditto.
> > * resolve.c (apply_default_init_local, resolve_fl_variable_derived,
> > resolve_symbol): Support for automatic attribute.
> > * symbol.c (check_conflict, gfc_copy_attr, gfc_is_var_automatic):
> > Ditto.
> > * trans-decl.c (gfc_finish_var_decl): Ditto.
> >
> > gcc/testsuite/gfortran.dg/
> > * dec_static_1.f90, dec_static_2.f90: New testcases.
>


---
Fritz Reese

Re: [PATCH] Fix location of command line backend reported issues (PR middle-end/77475)

[Richard Biener]

On September 5, 2016 7:20:57 PM GMT+02:00, Jakub Jelinek  
wrote:
>Hi!
>
>While it would be perhaps nice to pass explicit location_t in the
>target
>option handling code, there are hundreds of error/warning/sorry calls
>in lots of backends, and lots of those routines are used not just
>for the process_options time (i.e. command line options), but also for
>pragma GCC target and target option handling, so at least for the time
>being
>I think it is easier to just use UNKNOWN_LOCATION for the command line
>option diagnostics.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>2016-09-05  Jakub Jelinek  
>
>   PR middle-end/77475
>   * toplev.c (process_options): Temporarily set input_location
>   to UNKNOWN_LOCATION around targetm.target_option.override () call.
>
>--- gcc/toplev.c.jj2016-09-03 11:18:55.0 +0200
>+++ gcc/toplev.c   2016-09-05 15:05:19.819995470 +0200
>@@ -1220,7 +1220,10 @@ process_options (void)
>   no_backend = lang_hooks.post_options (_input_filename);
> 
>   /* Some machines may reject certain combinations of options.  */
>+  location_t saved_location = input_location;
>+  input_location = UNKNOWN_LOCATION;
>   targetm.target_option.override ();
>+  input_location = saved_location;
> 
>   if (flag_diagnostics_generate_patch)
>   global_dc->edit_context_ptr = new edit_context ();
>
>   Jakub

[PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)

[Jakub Jelinek]

Hi!

While most of the i386.opt -m= options have enum args and thus
cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
-mrecip=) don't use that, with the CPU strings being maintained inside of a
function rather than in some *.def file that could be also sourced into the
*.opt or something (and similarly for the strategies).

This patch adds inform calls that handle those similarly to what
cmdline_handle_error does for the options with enum values.
In addition, it adds %qs instead of %s in a couple of spaces, and
stops reporting incorrect attribute option("march=...") when it is
target("march=...") etc.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-05  Jakub Jelinek  

PR middle-end/77475
* config/i386/i386.c: Include spellcheck.h.
(ix86_parse_stringop_strategy_string): Simplify, use %qs instead of %s
where desirable, use argument instead of arg in the diagnostic
wording, add list of supported strategies and spellcheck hint.
(ix86_option_override_internal): Emit target("m...") instead of
option("m...") in the diagnostic.  Use %qs instead of %s in invalid
-march/-mtune option diagnostic.  Add list of supported arches/tunings
and spellcheck hint.

* gcc.target/i386/pr65990.c: Adjust expected diagnostics.

--- gcc/config/i386/i386.c.jj   2016-09-05 12:41:03.0 +0200
+++ gcc/config/i386/i386.c  2016-09-05 16:44:45.184981211 +0200
@@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
 #include "case-cfn-macros.h"
 #include "regrename.h"
 #include "dojump.h"
+#include "spellcheck.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
   next_range_str = strchr (curr_range_str, ',');
   if (next_range_str)
 *next_range_str++ = '\0';
+  const char *opt
+   = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
 
   if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
alg_name, , align))
 {
-  error ("wrong arg %s to option %s", curr_range_str,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong argument %qs to option %qs", curr_range_str, opt);
   return;
 }
 
   if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs != -1))
 {
-  error ("size ranges of option %s should be increasing",
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("size ranges of option %qs should be increasing", opt);
   return;
 }
 
@@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
 
   if (i == last_alg)
 {
-  error ("wrong stringop strategy name %s specified for option %s",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong stringop strategy name %qs specified for option %s",
+alg_name, opt);
+
+ size_t len = 0;
+ for (i = 0; i < last_alg; i++)
+   len += strlen (stringop_alg_names[i]) + 1;
+
+ char *s, *p;
+ auto_vec  candidates;
+ s = XALLOCAVEC (char, len);
+ p = s;
+ for (i = 0; i < last_alg; i++)
+ if ((stringop_alg) i != rep_prefix_8_byte || TARGET_64BIT)
+   {
+ size_t arglen = strlen (stringop_alg_names[i]);
+ memcpy (p, stringop_alg_names[i], arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ candidates.safe_push (stringop_alg_names[i]);
+   }
+ p[-1] = 0;
+ const char *hint = find_closest_string (alg_name, );
+ if (hint)
+   inform (input_location,
+   "valid arguments to %qs are: %s; did you mean %qs?",
+   opt, s, hint);
+ else
+   inform (input_location, "valid arguments to %qs are: %s",
+   opt, s);
   return;
 }
 
@@ -4565,10 +4592,8 @@ ix86_parse_stringop_strategy_string (cha
  && !TARGET_64BIT)
{
  /* rep; movq isn't available in 32-bit code.  */
- error ("stringop strategy name %s specified for option %s "
-"not supported for 32-bit code",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("stringop strategy name %qs specified for option %qs "
+"not supported for 32-bit code", alg_name, opt);
  return;
}
 
@@ -4580,8 +4605,7 @@ ix86_parse_stringop_strategy_string (cha
 input_ranges[n].noalign = true;
   else
 {
-  error ("unknown alignment %s specified for option %s",
- align, is_memset ? "-mmemset_strategy=" : 
"-mmemcpy_strategy=");
+ error ("unknown alignment %qs specified for 

[PATCH] Fix location of command line backend reported issues (PR middle-end/77475)

[Jakub Jelinek]

Hi!

While it would be perhaps nice to pass explicit location_t in the target
option handling code, there are hundreds of error/warning/sorry calls
in lots of backends, and lots of those routines are used not just
for the process_options time (i.e. command line options), but also for
pragma GCC target and target option handling, so at least for the time being
I think it is easier to just use UNKNOWN_LOCATION for the command line
option diagnostics.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-05  Jakub Jelinek  

PR middle-end/77475
* toplev.c (process_options): Temporarily set input_location
to UNKNOWN_LOCATION around targetm.target_option.override () call.

--- gcc/toplev.c.jj 2016-09-03 11:18:55.0 +0200
+++ gcc/toplev.c2016-09-05 15:05:19.819995470 +0200
@@ -1220,7 +1220,10 @@ process_options (void)
   no_backend = lang_hooks.post_options (_input_filename);
 
   /* Some machines may reject certain combinations of options.  */
+  location_t saved_location = input_location;
+  input_location = UNKNOWN_LOCATION;
   targetm.target_option.override ();
+  input_location = saved_location;
 
   if (flag_diagnostics_generate_patch)
   global_dc->edit_context_ptr = new edit_context ();

Jakub

[C++ PATCH] Fix ICE in cp/error.c (PR c++/77482)

[Jakub Jelinek]

Hi!

The recent concept changes that were also backported to 6.2 break
diagnostics e.g. on the following testcase, sometimes it ICEs during
reporting of the first error, so isn't just a normal low prio error
recovery.  The thing is that on the testcase the VAR_DECL has no
DECL_LANG_SPECIFIC, DECL_DECLARED_CONSTEXPR_P is a lang flag rather than
lang_specific field.  I believe in various places in cp/error.c we check
for *_LANG_SPECIFIC similarly.  In addition, the hunk had formatting issues.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk/6.3?

2016-09-05  Jakub Jelinek  

PR c++/77482
* error.c (dump_simple_decl): Only check DECL_DECLARED_CONCEPT_P
if DECL_LANG_SPECIFIC is non-NULL.  Fix up formatting.

* g++.dg/cpp0x/constexpr-77482.C: New test.

--- gcc/cp/error.c.jj   2016-09-02 18:17:32.0 +0200
+++ gcc/cp/error.c  2016-09-05 14:01:43.091770870 +0200
@@ -959,14 +959,13 @@ dump_simple_decl (cxx_pretty_printer *pp
 {
   if (flags & TFF_DECL_SPECIFIERS)
 {
-  if (VAR_P (t)
- && DECL_DECLARED_CONSTEXPR_P (t))
-{
-  if (DECL_DECLARED_CONCEPT_P (t))
-pp_cxx_ws_string (pp, "concept");
-  else
-   pp_cxx_ws_string (pp, "constexpr");
-}
+  if (VAR_P (t) && DECL_DECLARED_CONSTEXPR_P (t))
+{
+ if (DECL_LANG_SPECIFIC (t) && DECL_DECLARED_CONCEPT_P (t))
+   pp_cxx_ws_string (pp, "concept");
+ else
+   pp_cxx_ws_string (pp, "constexpr");
+   }
   dump_type_prefix (pp, type, flags & ~TFF_UNQUALIFIED_NAME);
   pp_maybe_space (pp);
 }
--- gcc/testsuite/g++.dg/cpp0x/constexpr-77482.C.jj 2016-09-05 
13:58:59.609821176 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-77482.C2016-09-05 
13:58:18.0 +0200
@@ -0,0 +1,6 @@
+// PR c++/77482
+// { dg-do compile { target c++11 } }
+
+constexpr auto x;  // { dg-error "declaration\[^\n\r]*has no initializer" }
+extern struct S s;
+constexpr auto y = s;  // { dg-error "has incomplete type" }

Jakub

[PATCH, i386] Fix ICE with a md builtin call (PR target/69255)

[Jakub Jelinek]

Hi!

As the testcase shows, if we want to diagnose a md builtin not enabled in
the current ISA, we call error and then return const0_rtx.  That isn't a
good choice if the result is BLKmode, which can happen for vector modes
that aren't enabled in the current ISA.  In that case, returning target
is better.  In the PR I've also mentioned DECL_MODE issues, but looking at
expr.c, I see there:
  /* DECL_MODE might change when TYPE_MODE depends on attribute target
 settings for VECTOR_TYPE_P that might switch for the function.  */
  if (currently_expanding_to_rtl
  && code == VAR_DECL && MEM_P (decl_rtl)
  && VECTOR_TYPE_P (type) && exp && DECL_MODE (exp) != mode)
decl_rtl = change_address (decl_rtl, TYPE_MODE (type), 0);
  else
decl_rtl = copy_rtx (decl_rtl);
so we should be fine.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-05  Jakub Jelinek  

PR target/69255
* config/i386/i386.c (ix86_expand_builtin): For builtin with
unsupported or unknown ISA, return target if non-NULL, instead of
const0_rtx.

* gcc.target/i386/pr69255-1.c: New test.
* gcc.target/i386/pr69255-2.c: New test.
* gcc.target/i386/pr69255-3.c: New test.

--- gcc/config/i386/i386.c.jj   2016-09-02 18:18:10.0 +0200
+++ gcc/config/i386/i386.c  2016-09-05 12:41:03.341382125 +0200
@@ -36107,7 +36107,7 @@ ix86_expand_builtin (tree exp, rtx targe
  error ("%qE needs isa option %s", fndecl, opts);
  free (opts);
}
-  return const0_rtx;
+  return target ? target : const0_rtx;
 }
 
   switch (fcode)
--- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj2016-09-05 
12:50:29.455307440 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-1.c   2016-09-05 12:50:06.0 
+0200
@@ -0,0 +1,16 @@
+/* PR target/69255 */
+/* { dg-do compile } */
+/* { dg-options "-mno-avx512f" } */
+
+#pragma GCC target "avx512vl"
+#pragma GCC target "no-avx512vl"
+__attribute__ ((__vector_size__ (32))) long long a;
+__attribute__ ((__vector_size__ (16))) int b;
+
+void
+foo (const long long *p)
+{
+  a = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);/* { dg-error "needs 
isa option -m32 -mavx512vl" } */
+}
+
+/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { 
target *-*-* } 13 } */
--- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj2016-09-05 
12:50:32.931264038 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-2.c   2016-09-05 12:49:57.0 
+0200
@@ -0,0 +1,14 @@
+/* PR target/69255 */
+/* { dg-do compile } */
+/* { dg-options "-mno-avx512f" } */
+
+#pragma GCC target "avx512vl"
+#pragma GCC target ""
+__attribute__ ((__vector_size__ (32))) long long a;
+__attribute__ ((__vector_size__ (16))) int b;
+
+void
+foo (const long long *p)
+{
+  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);/* { dg-error 
"needs isa option -m32 -mavx512vl" } */
+}
--- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj2016-09-05 
12:50:35.951226330 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-3.c   2016-09-05 12:49:13.0 
+0200
@@ -0,0 +1,16 @@
+/* PR target/69255 */
+/* { dg-do compile } */
+/* { dg-options "-mno-avx512f" } */
+
+#pragma GCC target "avx512vl"
+#pragma GCC target ""
+__attribute__ ((__vector_size__ (32))) long long a;
+__attribute__ ((__vector_size__ (16))) int b;
+
+void
+foo (const long long *p, __attribute__ ((__vector_size__ (32))) long long *q)
+{
+  *q = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);   /* { dg-error "needs 
isa option -m32 -mavx512vl" } */
+}
+
+/* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { 
target *-*-* } 13 } */

Jakub

[C++ PATCH] Fix constexpr switch handling (PR c++/77467)

[Jakub Jelinek]

Hi!

cxx_eval_switch_expr assumes that SWITCH_EXPR's body is always a
STATEMENT_LIST, but that doesn't have to be the case.
As the testcase shows, if there are any variable declarations in the
switch body, it can be also a BIND_EXPR, which cxx_eval_constant_expression
handles properly, and as bar in the testcase shows, it can be also just
a single statement (as try isn't allowed in constexpr functions, I think
we just want to do what cxx_eval_statement_list would do on such a statement
if it was wrapped into a STATEMENT_LIST - ignore it, as the case NNN: or 
default:
is not present.

Bootstrapped/regtested on x86_64-linux and i686-linux?  What about older
branches?

2016-09-05  Jakub Jelinek  

PR c++/77467
* constexpr.c (cxx_eval_switch_expr): Call cxx_eval_constant_expression
instead of cxx_eval_statement_list, for body other than STATEMENT_LIST
or BIND_EXPR don't evaluate the body at all.

* g++.dg/cpp1y/constexpr-77467.C: New test.

--- gcc/cp/constexpr.c.jj   2016-08-30 08:42:06.0 +0200
+++ gcc/cp/constexpr.c  2016-09-05 11:34:30.185518395 +0200
@@ -3572,8 +3572,12 @@ cxx_eval_switch_expr (const constexpr_ct
   *jump_target = cond;
 
   tree body = TREE_OPERAND (t, 1);
-  cxx_eval_statement_list (ctx, body,
-  non_constant_p, overflow_p, jump_target);
+  /* If body is a statement other than STATEMENT_LIST or BIND_EXPR,
+ it should be skipped.  E.g. switch (a) b = a;  */
+  if (TREE_CODE (body) == STATEMENT_LIST
+  || TREE_CODE (body) == BIND_EXPR)
+cxx_eval_constant_expression (ctx, body, false,
+ non_constant_p, overflow_p, jump_target);
   if (breaks (jump_target) || switches (jump_target))
 *jump_target = NULL_TREE;
   return NULL_TREE;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-77467.C.jj 2016-09-05 
11:19:30.593750642 +0200
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-77467.C2016-09-05 
11:37:11.929477518 +0200
@@ -0,0 +1,33 @@
+// PR c++/77467
+// { dg-do compile { target c++14 } }
+
+constexpr int
+foo (const int x, const unsigned n) noexcept
+{
+  switch (n)
+{
+case 0:
+  return 1;
+case 1:
+  return x;
+default:
+  const auto m = (n >> 1);
+  const auto y = foo (x, m);
+  return ((m << 1) == n) ? y * y : x * y * y;
+}
+}
+
+static_assert (foo (3, 2) == 9, "");
+static_assert (foo (2, 3) == 8, "");
+
+constexpr int
+bar (int x)
+{
+  int a = x;
+  switch (x)
+a = x + 1;
+  return a;
+}
+
+static_assert (bar (0) == 0, "");
+static_assert (bar (1) == 1, "");

Jakub

[PATCH, i386]: Fix zero-extension optimizations from mask registers (PR target/77476)

[Jakub Jelinek]

Hi!

On Fri, Aug 05, 2016 at 02:22:39PM +0200, Uros Bizjak wrote:
> 2016-08-05  Uros Bizjak  
> 
> * config/i386/i386.md (*zero_extendsidi2): Add (*r,*k) alternative.
> (zero_extenddi2): Ditto.
> (*zero_extendsi2): Ditto.
> (*zero_extendqihi2): Ditto.

As the PR says, unfortunately not all kmov instructions are supported by all
AVX512F supporting ISAs, kmovb is AVX512DQ, kmovw is AVX512F and kmovd and
kmovq are AVX512BW.

Thus, the following patch enables those alternatives only when the
instructions are available.

Bootstrapped/regtested on x86_64-linux and i686-linux, tested also with sde
-knl on the avx512f-* testcase.  Ok for trunk?

2016-09-05  Jakub Jelinek  

PR target/77476
* config/i386/i386.md (isa): Add x64_avx512bw.
(*zero_extendsidi2): For alternative 11 use x64_avx512bw isa.
(kmov_isa): New mode attr.
(zero_extenddi2): Use  isa for the last alternative.
(*zero_extendsi2): Likewise.
(*zero_extendqihi2): Use avx512dq isa for the last alternative.

* gcc.target/i386/avx512f-pr77476.c: New test.
* gcc.target/i386/avx512bw-pr77476.c: New test.
* gcc.target/i386/avx512dq-pr77476.c: New test.

--- gcc/config/i386/i386.md.jj  2016-08-29 12:17:41.0 +0200
+++ gcc/config/i386/i386.md 2016-09-05 10:35:53.404313654 +0200
@@ -799,7 +799,7 @@ (define_attr "isa" "base,x64,x64_sse4,x6
sse2,sse2_noavx,sse3,sse4,sse4_noavx,avx,noavx,
avx2,noavx2,bmi,bmi2,fma4,fma,avx512f,noavx512f,
fma_avx512f,avx512bw,noavx512bw,avx512dq,noavx512dq,
-   avx512vl,noavx512vl,x64_avx512dq"
+   avx512vl,noavx512vl,x64_avx512dq,x64_avx512bw"
   (const_string "base"))
 
 (define_attr "enabled" ""
@@ -812,6 +812,8 @@ (define_attr "enabled" ""
   (symbol_ref "TARGET_64BIT && TARGET_AVX")
 (eq_attr "isa" "x64_avx512dq")
   (symbol_ref "TARGET_64BIT && TARGET_AVX512DQ")
+(eq_attr "isa" "x64_avx512bw")
+  (symbol_ref "TARGET_64BIT && TARGET_AVX512BW")
 (eq_attr "isa" "nox64") (symbol_ref "!TARGET_64BIT")
 (eq_attr "isa" "sse2") (symbol_ref "TARGET_SSE2")
 (eq_attr "isa" "sse2_noavx")
@@ -3735,12 +3737,14 @@ (define_insn "*zero_extendsidi2"
   [(set (attr "isa")
  (cond [(eq_attr "alternative" "0,1,2")
  (const_string "nox64")
-   (eq_attr "alternative" "3,7,11")
+   (eq_attr "alternative" "3,7")
  (const_string "x64")
(eq_attr "alternative" "8")
  (const_string "x64_sse4")
(eq_attr "alternative" "10")
  (const_string "sse2")
+   (eq_attr "alternative" "11")
+ (const_string "x64_avx512bw")
   ]
   (const_string "*")))
(set (attr "type")
@@ -3804,6 +3808,9 @@ (define_split
(set (match_dup 4) (const_int 0))]
   "split_double_mode (DImode, [0], 1, [3], [4]);")
 
+(define_mode_attr kmov_isa
+  [(QI "avx512dq") (HI "avx512f") (SI "avx512bw") (DI "avx512bw")])
+
 (define_insn "zero_extenddi2"
   [(set (match_operand:DI 0 "register_operand" "=r,*r")
(zero_extend:DI
@@ -3812,7 +3819,8 @@ (define_insn "zero_extenddi2"
   "@
movz{l|x}\t{%1, %k0|%k0, %1}
kmov\t{%1, %k0|%k0, %1}"
-  [(set_attr "type" "imovx,mskmov")
+  [(set_attr "isa" "*,")
+   (set_attr "type" "imovx,mskmov")
(set_attr "mode" "SI")])
 
 (define_expand "zero_extendsi2"
@@ -3863,7 +3871,8 @@ (define_insn "*zero_extendsi2"
   "@
movz{l|x}\t{%1, %0|%0, %1}
kmov\t{%1, %0|%0, %1}"
-  [(set_attr "type" "imovx,mskmov")
+  [(set_attr "isa" "*,")
+   (set_attr "type" "imovx,mskmov")
(set_attr "mode" "SI,")])
 
 (define_expand "zero_extendqihi2"
@@ -3914,6 +3923,7 @@ (define_insn "*zero_extendqihi2"
movz{bl|x}\t{%1, %k0|%k0, %1}
kmovb\t{%1, %k0|%k0, %1}"
   [(set_attr "type" "imovx,mskmov")
+   (set_attr "isa" "*,avx512dq")
(set_attr "mode" "SI,QI")])
 
 (define_insn_and_split "*zext_doubleword_and"
--- gcc/testsuite/gcc.target/i386/avx512f-pr77476.c.jj  2016-09-05 
10:23:42.108364379 +0200
+++ gcc/testsuite/gcc.target/i386/avx512f-pr77476.c 2016-09-05 
10:23:26.0 +0200
@@ -0,0 +1,76 @@
+/* PR target/77476 */
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512f" } */
+/* { dg-require-effective-target avx512f } */
+
+#ifndef PR77476_TEST
+#include "avx512f-check.h"
+#define PR77476_TEST avx512f_test
+#endif
+
+unsigned short s;
+unsigned int i;
+unsigned long long l;
+
+void
+f1 (void)
+{
+  unsigned char a = 0xff;
+  asm volatile ("" : "+Yk" (a));
+  s = a;
+}
+
+void
+f2 (void)
+{
+  unsigned char a = 0xff;
+  asm volatile ("" : "+Yk" (a));
+  i = a;
+}
+
+void
+f3 (void)
+{
+  unsigned char a = 0xff;
+  asm volatile ("" : "+Yk" (a));
+  l = a;
+}
+
+void
+f4 (void)
+{
+  unsigned short a = 0x;
+  asm volatile ("" : "+Yk" (a));
+  i = a;
+}
+
+void
+f5 (void)
+{
+  unsigned short a = 0x;
+  asm 

Make max_align_t respect _Float128 [version 2]

[Joseph Myers]

[Patch version 2 adds an update to cxx_fundamental_alignment_p.]

The _FloatN, _FloatNx, _DecimalN and _DecimalNx types are specified in
such a way that they are basic types, meaning that max_align_t must be
at least as aligned as those types.

On 32-bit x86, max_align_t is currently 8-byte aligned, but
_Decimal128 and _Float128 are 16-byte aligned, so the alignment of
max_align_t needs to increase to meet the standard requirements for
these types.

This patch implements such an increase.  Because max_align_t needs to
be usable for C++ as well as for C,  can't actually refer to
_Float128, but needs to use __float128 (or some other notation for the
type) instead.  And since __float128 is architecture-specific, there
isn't a preprocessor conditional that means "__float128 is available"
(whereas one could test __FLT128_MANT_DIG__ to see if _Float128 is
available; __SIZEOF_FLOAT128__ is available on x86 only).  But I
believe the only case that actually has an alignment problem here is
32-bit x86, and  already has lots of conditional specific to
particular architectures or OSes, so this patch uses a conditional on
__i386__; that also is the minimal change that ensures neither size
nor alignment of max_align_t is changed in any case other than where
it is necessary.  If any other architectures turn out to have such an
issue, it will show up as failures of one of the testcases added by
this patch.

Such an increase is of course an ABI change, but a reasonably safe
one, in that max_align_t doesn't tend to appear in library interfaces
(rather, it's something to use when writing allocators and similar
code; most matches found on codesearch.debian.net look like copies of
the gnulib stddef.h module rather than anything actually using
max_align_t at all).

cxx_fundamental_alignment_p has a corresponding change (adding
_Float128 to the types considered).

(I think glibc malloc alignment should also increase to 16-byte on
32-bit x86 so it works for allocating objects of these types, which is
now straightforward given the fix made for 32-bit powerpc.)

Bootstrapped with no regressions on x86_64-pc-linux-gnu, and
spot-tested with -m32 that the new float128-align.c test now compiles
where previously it didn't.  OK to commit?

gcc:
2016-09-05  Joseph Myers  

* ginclude/stddef.h (max_align_t) [__i386__]: Add __float128
element.

gcc/c-family:
2016-09-05  Joseph Myers  

* c-common.c (cxx_fundamental_alignment_p): Also consider
alignment of float128_type_node.

gcc/testsuite:
2016-09-05  Joseph Myers  

* gcc.dg/float128-align.c, gcc.dg/float128x-align.c,
gcc.dg/float16-align.c, gcc.dg/float32-align.c,
gcc.dg/float32x-align.c, gcc.dg/float64-align.c,
gcc.dg/float64x-align.c, gcc.dg/floatn-align.h: New tests.

Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c (revision 239987)
+++ gcc/c-family/c-common.c (working copy)
@@ -12855,8 +12855,11 @@ scalar_to_vector (location_t loc, enum tree_code c
 bool
 cxx_fundamental_alignment_p  (unsigned align)
 {
-  return (align <=  MAX (TYPE_ALIGN (long_long_integer_type_node),
-TYPE_ALIGN (long_double_type_node)));
+  unsigned int max_align = MAX (TYPE_ALIGN (long_long_integer_type_node),
+   TYPE_ALIGN (long_double_type_node));
+  if (float128_type_node != NULL_TREE)
+max_align = MAX (max_align, TYPE_ALIGN (float128_type_node));
+  return align <= max_align;
 }
 
 /* Return true if T is a pointer to a zero-sized aggregate.  */
Index: gcc/ginclude/stddef.h
===
--- gcc/ginclude/stddef.h   (revision 239987)
+++ gcc/ginclude/stddef.h   (working copy)
@@ -426,6 +426,14 @@ typedef __WINT_TYPE__ wint_t;
 typedef struct {
   long long __max_align_ll __attribute__((__aligned__(__alignof__(long 
long;
   long double __max_align_ld __attribute__((__aligned__(__alignof__(long 
double;
+  /* _Float128 is defined as a basic type, so max_align_t must be
+ sufficiently aligned for it.  This code must work in C++, so we
+ use __float128 here; that is only available on some
+ architectures, but only on i386 is extra alignment needed for
+ __float128.  */
+#ifdef __i386__
+  __float128 __max_align_f128 
__attribute__((__aligned__(__alignof(__float128;
+#endif
 } max_align_t;
 #endif
 #endif /* C11 or C++11.  */
Index: gcc/testsuite/gcc.dg/float128-align.c
===
--- gcc/testsuite/gcc.dg/float128-align.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/float128-align.c   (working copy)
@@ -0,0 +1,9 @@
+/* Test _Float128 alignment.  */
+/* { dg-do compile } */
+/* { dg-options "" } */
+/* { dg-add-options float128 } */
+/* { dg-require-effective-target float128 } */
+

Re: [PATCH, docs] invoke.texi: random copy-editing

[Sandra Loosemore]

On 09/02/2016 12:17 AM, Gerald Pfeifer wrote:

On Wed, 29 Aug 2012, Sandra Loosemore wrote:

* doc/invoke.texi: Fix numerous typos and punctuation/grammatical
errors throughout the file.  Re-word some awkward sentences and
paragraphs.


There are three questions (and to some extent suggestions) on this
patch and the text covered by it that I'm wondering about.  Hope
that's still fine after all the time.

I'm happy to make any changes myself, but am looking at your expertise.


  Item 11:  Define a copy constructor and an assignment operator for classes
-with dynamically allocated memory.
+with dynamically-allocated memory.

Why the dash here?  Is this because it's seens as a technical term?
(Usually it's the Germans with those absolutelylongandnonbreaking words.
;-)


-(C++ only) A base class is not initialized in a derived class' copy
+(C++ only) A base class is not initialized in a derived class's copy
  constructor.

"class's" twists my brain a little.  What do you think about using
"in a copy constructor of a derived class" instead?


  When profile feedback is available (see @option{-fprofile-generate}) the 
actual
-recursion depth can be guessed from probability that function will recurse via
-given call expression.  This parameter limits inlining only to call expression
-whose probability exceeds given threshold (in percents).  The default value is
-10.
+recursion depth can be guessed from probability that function recurses via a
+given call expression.  This parameter limits inlining only to call expressions
+whose probability exceeds the given threshold (in percents).

This predates your patch, but should this be "the probability"?

Gerald

--D6BB43F4FA.1472886072/ainaz.pair.com--
ReSent-Date: Sat, 3 Sep 2016 09:46:53 +0200 (CEST)
ReSent-From: Gerald Pfeifer 
ReSent-To: Sandra Loosemore 
ReSent-Subject: Re: [PATCH, docs] invoke.texi: random copy-editing
ReSent-Message-ID: 

On Wed, 29 Aug 2012, Sandra Loosemore wrote:

* doc/invoke.texi: Fix numerous typos and punctuation/grammatical
errors throughout the file.  Re-word some awkward sentences and
paragraphs.


There are three questions (and to some extent suggestions) on this
patch and the text covered by it that I'm wondering about.  Hope
that's still fine after all the time.

I'm happy to make any changes myself, but am looking at your expertise.


  Item 11:  Define a copy constructor and an assignment operator for classes
-with dynamically allocated memory.
+with dynamically-allocated memory.

Why the dash here?  Is this because it's seens as a technical term?
(Usually it's the Germans with those absolutelylongandnonbreaking words.
;-)


Adjective phrases immediately before the noun they modify are 
hyphenated.  This is the same reason why we write "floating-point 
arithmetic" but "floating point", unhyphenated, as a noun.




-(C++ only) A base class is not initialized in a derived class' copy
+(C++ only) A base class is not initialized in a derived class's copy
  constructor.

"class's" twists my brain a little.  What do you think about using
"in a copy constructor of a derived class" instead?


Yes, that's better.


  When profile feedback is available (see @option{-fprofile-generate}) the 
actual
-recursion depth can be guessed from probability that function will recurse via
-given call expression.  This parameter limits inlining only to call expression
-whose probability exceeds given threshold (in percents).  The default value is
-10.
+recursion depth can be guessed from probability that function recurses via a
+given call expression.  This parameter limits inlining only to call expressions
+whose probability exceeds the given threshold (in percents).

This predates your patch, but should this be "the probability"?


Yes, please.

-Sandra

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Edlinger]

On 09/05/16 18:45, Joseph Myers wrote:
> On Mon, 5 Sep 2016, Marek Polacek wrote:
>
>> Still not sure about other operations.  I guess no one would
>> object to warning on bool1 % bool2, but should we warn for
>> bool1 + bool2?
>
> I think boolean addition (with the result interpreted as an integer, not
> converted back to boolean) is perfectly reasonable - counting the number
> of flags that are true, for example (say if there are several conditions
> and it's an error for more than one of them to hold - of course that would
> be bool1 + bool2 + bool3 + bool4, etc.).
>

Yes, that's what I had in mind too.  I think I even remember having seen
code like this, which is OK as long as the result is stored in an
integer variable.

But "if (bool1 + bool2)" should be written as "if (bool1 | bool2)",
and "if (bool1 * bool2)" should be written as "if (bool1 & bool2)".

I think a warning for boolean + and * suggesting to use | and &
is justified for clarity.


Bernd.

Re: [PATCH, docs] invoke.texi: random copy-editing

[Sandra Loosemore]

On 09/01/2016 01:04 PM, Gerald Pfeifer wrote:

On Wed, 29 Aug 2012, Sandra Loosemore wrote:

* doc/invoke.texi: Fix numerous typos and punctuation/grammatical
errors throughout the file.  Re-word some awkward sentences and
paragraphs.


I noticed you changed return-value and return-type to their
variants without a dash.  Would it make sense to add the
following to https://gcc.gnu.org/codingconventions.html#Spelling ?

Gerald

Index: codingconventions.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
retrieving revision 1.76
diff -u -r1.76 codingconventions.html
--- codingconventions.html  16 Aug 2016 09:24:33 -  1.76
+++ codingconventions.html  1 Sep 2016 19:02:58 -
@@ -457,6 +457,11 @@
  


+"return type", "return value"
+"return-type", "return-value"
+
+  
+  
  "run time" (noun), "run-time" (adjective);
the time at which the program is run
  "runtime"


Perhaps add here that "return type" and "return value" are nouns.  It 
would be correct to hyphenate them if they were used as adjective 
phrases immediately before a noun (although I'm having trouble coming up 
with an example of such usage that would make any sense).


-Sandra

Re: [PATCH, docs] invoke.texi: random copy-editing

[Sandra Loosemore]

On 09/01/2016 06:22 AM, Gerald Pfeifer wrote:

Hi Sandra,

On Wed, 29 Aug 2012, Sandra Loosemore wrote:

I've had this largish pile of random copy-edits to invoke.texi left over
from my previous passes through that file earlier this year.


that was an amazing amount of changes; I admire your patience and
thoroughness!


I'll wait a few days before committing to give folks a chance to object
and/or volunteer to review the whole patch.  ;-)


That was more like a few years, but I did go through the patch. :-o

On thing I noticed is that you converted "nop" to "NOP", is that
a standard you generally suggest to establish?  If so, I've got a
couple more cases.


Yes, I think "NOP" is more readable than "nop" in running text.  (It's 
also how Wikipedia capitalizes the term).



Let me know, and I'll apply this patch.


Looks good to me.

-Sandra

Re: [PATCHv2] Add a warning for suspicious use of conditional expressions in boolean context

[Joseph Myers]

On Mon, 5 Sep 2016, Bernd Edlinger wrote:

> But I think the reasoning is still correct, left shifting
> in a boolean context is suspicious, even if -fwrapv may make
> it defined.  Do you agree?

Well, you can argue that if you want to test whether low bits are all 
zero, you should just use binary & with a suitable mask, or else cast to 
unsigned for the left shift.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Joseph Myers]

On Mon, 5 Sep 2016, Marek Polacek wrote:

> Still not sure about other operations.  I guess no one would
> object to warning on bool1 % bool2, but should we warn for
> bool1 + bool2?

I think boolean addition (with the result interpreted as an integer, not 
converted back to boolean) is perfectly reasonable - counting the number 
of flags that are true, for example (say if there are several conditions 
and it's an error for more than one of them to hold - of course that would 
be bool1 + bool2 + bool3 + bool4, etc.).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH, ARM] Add a new target hook to compute the frame layout

[Bernd Edlinger]

Hi Richard,

what do you think of this patch, is it OK (with the suggested wording)?


Thanks
Bernd.

On 08/05/16 16:06, Richard Earnshaw (lists) wrote:
> On 05/08/16 13:49, Bernd Edlinger wrote:
>> On 08/05/16 11:29, Richard Earnshaw (lists) wrote:
>>> On 04/08/16 22:16, Bernd Edlinger wrote:
 Hi,

 this patch introduces a new target hook that allows the target's
 INITIAL_ELIMINATION_OFFSET function to use cached values instead of
 re-computing the frame layout every time.

 I have updated the documentation a bit and hope it is clearer this time.

 It still needs a review by ARM port maintainers.

 If the ARM port maintainers find this patch useful, that would be fine.

>>>
>>> I need to look into this more, but my first thought was that the
>>> documentation is confusing, or there is a real problem in this patch.
>>>
>>
>> Thanks for your quick response.
>>
>> The documentation is actually the most difficult part for me.
>>
>>> As I understand it the frame has to be re-laid out each time the
>>> contents of the frame changes (an extra register becomes live or another
>>> spill slot is needed).  So saying that it is laid out once can't be
>>> right if (as I read it initially) you mean 'once per function' since I
>>> think it needs to be 'once for each time the frame contents changes'.
>>>
>>> Of course, things might be a bit different with LRA compared with
>>> reload, but I strongly suspect this is never going to be an 'exactly
>>> once per function' operation.
>>>
>>
>> Right.  It will be done 2 or 3 times for each function.
>> LRA and reload behave identical in that respect.
>>
>> But each time reload changes something in the input data the
>> INITIAL_EMIMINATION_OFFSET is called several times, and the results
>> have to be consistent in each iteration.
>>
>> The frame layout function has no way to know if the frame layout
>> is expected to change or not.
>>
>> Many targets use reload_completed as an indication when the frame layout
>> may not change at all, but that is only an approximation.
>>
>>> Can you clarify your meaning in the documentation please?
>>>
>>
>> I meant 'once' in the sense of 'once for each time the frame contents
>> change'.
>>
>> Thus I'd change that sentence to:
>>
>> "This target hook allows the target to compute the frame layout once for
>> each time the frame contents change and make use of the cached frame
>> layout in @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing it
>> on every invocation.  This is particularly useful for targets that have
>> an expensive frame layout function.  Implementing this callback is
>> optional."
>>
>
> Thanks, that's pretty much what I expected would be the case.
>
> Could I suggest:
>
> This target hook is called once each time the frame layout needs to be
> recalculated.  The calculations can be cached by the target and can then
> be used by @code{INITIAL_ELIMINATION_OFFSET} instead of re-computing the
> layout on every invocation of that hook.  This is particularly useful
> for targets that have an expensive frame layout function.  Implementing
> this callback is optional.
>
> R.
>
>>
>> Thanks
>> Bernd.
>>
>>
>>> R.
>>>

 Thanks
 Bernd.

 On 06/21/16 23:29, Jeff Law wrote:
> On 06/16/2016 08:47 AM, Bernd Edlinger wrote:
>> Hi!
>>
>>
>> By the design of the target hook INITIAL_ELIMINATION_OFFSET
>> it is necessary to call this function several times with
>> different register combinations.
>> Most targets use a cached data structure that describes the
>> exact frame layout of the current function.
>>
>> It is safe to skip the computation when reload_completed = true,
>> and most targets do that already.
>>
>> However while reload is doing its work, it is not clear when to
>> do the computation and when not.  This results in unnecessary
>> work.  Computing the frame layout can be a simple function or an
>> arbitrarily complex one, that walks all instructions of the current
>> function for instance, which is more or less the common case.
>>
>>
>> This patch adds a new optional target hook that can be used
>> by the target to factor the INITIAL_ELIMINATION_OFFSET-hook
>> into a O(n) computation part, and a O(1) result function.
>>
>> The patch implements a compute_frame_layout target hook just
>> for ARM in the moment, to show the principle.
>> Other targets may also implement that hook, if it seems appropriate.
>>
>>
>> Boot-strapped and reg-tested on arm-linux-gnueabihf.
>> OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> changelog-frame-layout.txt
>>
>>
>> 2016-06-16  Bernd Edlinger  
>>
>>   * target.def (compute_frame_layout): New optional target hook.
>>   * doc/tm.texi.in (TARGET_COMPUTE_FRAME_LAYOUT): Add hook.
>>   * doc/tm.texi 

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Sandra Loosemore]

On 09/05/2016 09:55 AM, Bernd Schmidt wrote:



On 09/05/2016 12:52 PM, Marek Polacek wrote:

On Mon, Sep 05, 2016 at 12:35:13PM +0200, Bernd Schmidt wrote:

On 09/02/2016 05:13 PM, Marek Polacek wrote:

diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 87da1f1..38d55d4 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5437,8 +5437,8 @@ if (a < 0 && a < 0) @{ @dots{} @}
 @opindex Wlogical-not-parentheses
 @opindex Wno-logical-not-parentheses
 Warn about logical not used on the left hand side operand of a
comparison.
-This option does not warn if the RHS operand is of a boolean type.
Its
-purpose is to detect suspicious code like the following:
+This option does not warn if the right operand is considered to be
a Boolean
+expression.  Its purpose is to detect suspicious code like the
following:


I think "Boolean" shouldn't be capitalized. The patch looks ok to me
otherwise.


No strong opinions, but looking at
https://en.wikipedia.org/wiki/Boolean_expression
I see that it's capitalized there, so I think let's keep "Boolean".


I looked at other occurrences in this file, and they are lower-case.
Across the other texi files upper-case is also the exception. Let's ask
Sandra for a ruling?


U.  There seems to be precedent for both capitalizing it and not.

The C standard doesn't capitalize it (but the only place I could find 
where "boolean" used in a context where it wouldn't be capitalized 
anyway, such as the first word in a section title, is the index).  The 
C++ standard isn't consistent, using both "Boolean" and "boolean" (and 
sometimes even on the same page).  The documents I have handy are older 
drafts, though; maybe that has been cleaned up in current/final versions.


Looking at some other languages, the Python documentation capitalizes:

https://docs.python.org/3/library/stdtypes.html

But the Scheme specification editors deliberately chose not to do so:

http://www.r6rs.org/r6rs-editors/2007-April/002143.html

The Haskell specification also uses lowercase:

https://www.haskell.org/onlinereport/haskell2010/haskellch6.html#x13-1170006.1

Mark-Jason Dominus gave the matter some serious thought:

http://blog.plover.com/lang/Boolean.html

Anyway, I will be happy either way as long as the documentation is 
consistent.


-Sandra

Re: [PATCH] Delete GCJ

[Gerald Pfeifer]

On Mon, 5 Sep 2016, Andrew Haley wrote:
> As discussed.  I think I should ask a Global reviewer to approve this
> one.  For obvious reasons I haven't included the diffs to the deleted
> gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
> is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
> if anyone would like to try it.

And here is the patch for the web pages.

Note I did not include all the removed java/* contents.  Is there
anything particular you'd like to retain there?

Gerald

Index: index.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v
retrieving revision 1.1026
diff -u -r1.1026 index.html
--- index.html  25 Aug 2016 10:55:41 -  1.1026
+++ index.html  5 Sep 2016 16:22:07 -
@@ -15,8 +15,7 @@
 C,
 C++,
 Objective-C, Fortran,
-Java, Ada, and Go, as well as libraries for these
-languages (libstdc++, libgcj,...).
+Ada, and Go, as well as libraries for these languages (libstdc++,...).
 GCC was originally written as the compiler for the http://www.gnu.org/gnu/thegnuproject.html;>GNU operating system.
 The GNU system was developed to be 100% free software, free in the sense
Index: style.mhtml
===
RCS file: /cvs/gcc/wwwdocs/htdocs/style.mhtml,v
retrieving revision 1.131
diff -u -r1.131 style.mhtml
--- style.mhtml 23 Aug 2016 06:49:17 -  1.131
+++ style.mhtml 5 Sep 2016 16:22:08 -
@@ -10,15 +10,6 @@
   
 >
 
-;;; For the "java/" pages, we want the navigation bar.
-
- "java/[^/]*.html">
- 
-  
- >
->
-
 ;;; Note that the  line really needs to start in the first column.
 
 
@@ -105,26 +96,6 @@
   
   
 
-   "java/[^/]*.html">
-   
-
-
-
-
-
-GCJ Home
-GCC Home
-FAQ
-Documentation
-Contributing
-Done with GCJ
-
-
-
-   >
-  >
-
   
   About GCC
   

Re: [PATCH] Delete GCJ

[Andrew Haley]

On 05/09/16 17:15, Richard Biener wrote:
> On September 5, 2016 5:13:06 PM GMT+02:00, Andrew Haley  
> wrote:
>> As discussed.  I think I should ask a Global reviewer to approve this
>> one.  For obvious reasons I haven't included the diffs to the deleted
>> gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
>> is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
>> if anyone would like to try it.
> 
> Isn't there also java specific C++ frontend parts?

There certainly are, but deleting them without breaking anything else
is going to be rather delicate.  I'm trying to do this one step at a
time, rather cautiously.

Andrew.

Re: [PATCH, c++, PR77427 ] Set TYPE_STRUCTURAL_EQUALITY for sysv_abi va_list

[Tom de Vries]

On 05/09/16 09:49, Richard Biener wrote:

On Sun, Sep 4, 2016 at 11:30 PM, Tom de Vries  wrote:

> On 04/09/16 16:08, Richard Biener wrote:

>>
>> On September 4, 2016 12:33:02 PM GMT+02:00, Tom de Vries
>>  wrote:

>>>
>>> On 04/09/16 08:12, Richard Biener wrote:


 On September 3, 2016 5:23:35 PM GMT+02:00, Tom de Vries

>>>
>>>  wrote:

>>
>> Hi,
>>
>> this patch fixes a c++ ICE, a p1 6/7 regression.
>>
>>
>> Consider test.C:
>> ...
>> void bar (__builtin_va_list &);
>>
>> struct c
>> {
>>   operator const __builtin_va_list &();
>>   operator __builtin_va_list &();
>> };
>>
>> void
>> foo (void) {
>>   struct c c1;
>>
>>   bar (c1);
>> }
>> ...
>>
>> The compiler ICEs as follows:
>> ...
>> test.C: In function ‘void foo()’:
>> test.C:13:10: internal compiler error: canonical types differ for
>> identical types __va_list_tag [1] and __va_list_tag [1]
>>bar (c1);
>>   ^
>> comptypes(tree_node*, tree_node*, int)
>> src/gcc/cp/typeck.c:1430
>> reference_related_p(tree_node*, tree_node*)
>> src/gcc/cp/call.c:1415
>> reference_binding
>> src/gcc/cp/call.c:1559
>> implicit_conversion
>> src/gcc/cp/call.c:1805
>> build_user_type_conversion_1
>> src/gcc/cp/call.c:3776
>> reference_binding
>> src/gcc/cp/call.c:1664
>> implicit_conversion
>> src/gcc/cp/call.c:1805
>> add_function_candidate
>> src/gcc/cp/call.c:2141
>> add_candidates
>> src/gcc/cp/call.c:5394
>> perform_overload_resolution
>> src/gcc/cp/call.c:4066
>> build_new_function_call(tree_node*, vec>>
>>> vl_embed>**,

>>
>>   bool, int)
>> src/gcc/cp/call.c:4143
>> finish_call_expr(tree_node*, vec**,

>>>
>>> bool,

>>
>>   bool, int)
>> src/gcc/cp/semantics.c:2440
>> ...
>>
>> The regression is caused by the commit for PR70955, that adds a
>> "sysv_abi va_list" attribute to the struct in the va_list type

>>>
>>> (which

>>
>> is
>> an array of one of struct).
>>
>> The ICE in comptypes happens as follows: we're comparing two

>>>
>>> versions

>>
>> of
>> va_list type (with identical array element type), each with the
>> canonical type set to themselves. Since the types are considered
>> identical, they're supposed to have identical canonical types,

>>>
>>> which is
>>>

 Did you figure out why they are not assigned the same canonical type?

>>>
>>>
>>> When constructing the first type in ix86_build_builtin_va_list_64, it's
>>>
>>> cached.
>>>
>>> When constructing the second type in build_array_type_1 (with call
>>> stack: grokdeclarator -> cp_build_qualified_type_real ->
>>> build_cplus_array_type -> build_cplus_array_type -> build_array_type ->
>>>
>>> build_array_type_1), we call type_hash_canon.
>>>
>>> But the cached type has name __builtin_sysv_va_list, and the new type
>>> has no name, so we hit the clause 'TYPE_NAME (a->type) != TYPE_NAME
>>> (b->type)' in type_cache_hasher::equal.
>>>
>>> Consequently, TYPE_CANONICAL for the new type remain set to self.

>>
>>
>> But how did it then work before the patch causing this?

>
>
> Before the patch that introduced the attribute, rather than assigning the
> result of ix86_build_builtin_va_list_64 directly sysv_va_list_type_node, an
> intermediate build_variant_type_copy was used.
>
> This had as consequence that the copy was named and not added to the cache,
> and that the original in the type cache remained unnamed.
>
> Adding the build_variant_type_copy back fixes the ICE. But I'm not sure if
> that's a correct fix. The copy would have it's canonical type set to the
> original type. But if we'd search for the canonical type of the copy in the
> type cache, we wouldn't find it.



Huh.  Can't see how making a copy would affect the type cache -- AFAIK nothing
adds the record to the type hash.


Correct.


 The array type is there


First the array type is constructed by ix86_build_builtin_va_list_64, 
and entered into the type cache. Then the type is assigned to 
ms_va_list_type_node.


Lateron, the ms_va_list_type_node is returned by ix86_enum_va_list, and 
c_common_nodes_and_builtin calls lang_hooks.decls.pushdecl for the node:

...
   lang_hooks.decls.pushdecl
(build_decl (UNKNOWN_LOCATION,
 TYPE_DECL, get_identifier (pname),
 ptype));
...
In that process it adds a name to the type node (to be precise, in 
set_underlying_type, case DECL_IS_BUILTIN).


If it adds the name to the original type, then we have a named type in 
the type cache. If it adds the name to a copy of the type, then we have 
an unnamed 

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Schmidt]

On 09/05/2016 12:52 PM, Marek Polacek wrote:

On Mon, Sep 05, 2016 at 12:35:13PM +0200, Bernd Schmidt wrote:

On 09/02/2016 05:13 PM, Marek Polacek wrote:

diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 87da1f1..38d55d4 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5437,8 +5437,8 @@ if (a < 0 && a < 0) @{ @dots{} @}
 @opindex Wlogical-not-parentheses
 @opindex Wno-logical-not-parentheses
 Warn about logical not used on the left hand side operand of a comparison.
-This option does not warn if the RHS operand is of a boolean type.  Its
-purpose is to detect suspicious code like the following:
+This option does not warn if the right operand is considered to be a Boolean
+expression.  Its purpose is to detect suspicious code like the following:


I think "Boolean" shouldn't be capitalized. The patch looks ok to me
otherwise.


No strong opinions, but looking at
https://en.wikipedia.org/wiki/Boolean_expression
I see that it's capitalized there, so I think let's keep "Boolean".


I looked at other occurrences in this file, and they are lower-case. 
Across the other texi files upper-case is also the exception. Let's ask 
Sandra for a ruling?



Bernd

RE: [PATCH] Delete GCJ

[Matthew Fortune]

Andrew Haley  writes:
> As discussed.  I think I should ask a Global reviewer to approve this
> one.  For obvious reasons I haven't included the diffs to the deleted
> gcc/java and libjava directories.  The whole tree, post GCJ-deletion, is
> at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
> if anyone would like to try it.

I hadn't realised libjava was earmarked for removal (and I have no
objection) but given I have an outstanding bug in libjava I wonder how
we will deal with bug fix backports when we can't commit to trunk first?

Matthew

Re: [PATCH] Delete GCJ

[Andrew Haley]

On 05/09/16 16:29, Matthias Klose wrote:
> Please consider removing boehm-gc as well.  The only other user is
> --enable-objc-gc, which better should use an external boehm-gc.

I can do that, but I do not want to do so with this patch.

Andrew.

Re: [PATCH] Delete GCJ

[Matthias Klose]

On 05.09.2016 17:13, Andrew Haley wrote:
> As discussed.  I think I should ask a Global reviewer to approve this
> one.  For obvious reasons I haven't included the diffs to the deleted
> gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
> is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
> if anyone would like to try it.
> 
> Andrew.
> 
> 
> 2016-09-05  Andrew Haley  
> 
>   * Makefile.def: Remove libjava.
>   * Makefile.tpl: Likewise.
>   * Makefile.in: Regenerate.
>   * configure.ac: Likewise.
>   * configure: Likewise.
>   * gcc/java: Remove.
>   * libjava: Likewise.

Please consider removing boehm-gc as well.  The only other user is
--enable-objc-gc, which better should use an external boehm-gc.

Matthias

[PATCH] Delete GCJ

[Andrew Haley]

As discussed.  I think I should ask a Global reviewer to approve this
one.  For obvious reasons I haven't included the diffs to the deleted
gcc/java and libjava directories.  The whole tree, post GCJ-deletion,
is at svn+ssh://gcc.gnu.org/svn/gcc/branches/gcj/gcj-deletion-branch
if anyone would like to try it.

Andrew.


2016-09-05  Andrew Haley  

* Makefile.def: Remove libjava.
* Makefile.tpl: Likewise.
* Makefile.in: Regenerate.
* configure.ac: Likewise.
* configure: Likewise.
* gcc/java: Remove.
* libjava: Likewise.

Index: Makefile.in
===
--- Makefile.in (revision 239988)
+++ Makefile.in (working copy)
@@ -322,8 +322,6 @@
 HOST_LIBELFLIBS = @libelflibs@
 HOST_LIBELFINC = @libelfinc@

-EXTRA_CONFIGARGS_LIBJAVA = @EXTRA_CONFIGARGS_LIBJAVA@
-
 # --
 # Programs producing files for the BUILD machine
 # --
@@ -1007,7 +1005,6 @@
 maybe-configure-target-winsup \
 maybe-configure-target-libgloss \
 maybe-configure-target-libffi \
-maybe-configure-target-libjava \
 maybe-configure-target-zlib \
 maybe-configure-target-boehm-gc \
 maybe-configure-target-rda \
@@ -1174,7 +1171,6 @@
 all-target: maybe-all-target-winsup
 all-target: maybe-all-target-libgloss
 all-target: maybe-all-target-libffi
-all-target: maybe-all-target-libjava
 all-target: maybe-all-target-zlib
 all-target: maybe-all-target-boehm-gc
 all-target: maybe-all-target-rda
@@ -1268,7 +1264,6 @@
 info-target: maybe-info-target-winsup
 info-target: maybe-info-target-libgloss
 info-target: maybe-info-target-libffi
-info-target: maybe-info-target-libjava
 info-target: maybe-info-target-zlib
 info-target: maybe-info-target-boehm-gc
 info-target: maybe-info-target-rda
@@ -1355,7 +1350,6 @@
 dvi-target: maybe-dvi-target-winsup
 dvi-target: maybe-dvi-target-libgloss
 dvi-target: maybe-dvi-target-libffi
-dvi-target: maybe-dvi-target-libjava
 dvi-target: maybe-dvi-target-zlib
 dvi-target: maybe-dvi-target-boehm-gc
 dvi-target: maybe-dvi-target-rda
@@ -1442,7 +1436,6 @@
 pdf-target: maybe-pdf-target-winsup
 pdf-target: maybe-pdf-target-libgloss
 pdf-target: maybe-pdf-target-libffi
-pdf-target: maybe-pdf-target-libjava
 pdf-target: maybe-pdf-target-zlib
 pdf-target: maybe-pdf-target-boehm-gc
 pdf-target: maybe-pdf-target-rda
@@ -1529,7 +1522,6 @@
 html-target: maybe-html-target-winsup
 html-target: maybe-html-target-libgloss
 html-target: maybe-html-target-libffi
-html-target: maybe-html-target-libjava
 html-target: maybe-html-target-zlib
 html-target: maybe-html-target-boehm-gc
 html-target: maybe-html-target-rda
@@ -1616,7 +1608,6 @@
 TAGS-target: maybe-TAGS-target-winsup
 TAGS-target: maybe-TAGS-target-libgloss
 TAGS-target: maybe-TAGS-target-libffi
-TAGS-target: maybe-TAGS-target-libjava
 TAGS-target: maybe-TAGS-target-zlib
 TAGS-target: maybe-TAGS-target-boehm-gc
 TAGS-target: maybe-TAGS-target-rda
@@ -1703,7 +1694,6 @@
 install-info-target: maybe-install-info-target-winsup
 install-info-target: maybe-install-info-target-libgloss
 install-info-target: maybe-install-info-target-libffi
-install-info-target: maybe-install-info-target-libjava
 install-info-target: maybe-install-info-target-zlib
 install-info-target: maybe-install-info-target-boehm-gc
 install-info-target: maybe-install-info-target-rda
@@ -1790,7 +1780,6 @@
 install-pdf-target: maybe-install-pdf-target-winsup
 install-pdf-target: maybe-install-pdf-target-libgloss
 install-pdf-target: maybe-install-pdf-target-libffi
-install-pdf-target: maybe-install-pdf-target-libjava
 install-pdf-target: maybe-install-pdf-target-zlib
 install-pdf-target: maybe-install-pdf-target-boehm-gc
 install-pdf-target: maybe-install-pdf-target-rda
@@ -1877,7 +1866,6 @@
 install-html-target: maybe-install-html-target-winsup
 install-html-target: maybe-install-html-target-libgloss
 install-html-target: maybe-install-html-target-libffi
-install-html-target: maybe-install-html-target-libjava
 install-html-target: maybe-install-html-target-zlib
 install-html-target: maybe-install-html-target-boehm-gc
 install-html-target: maybe-install-html-target-rda
@@ -1964,7 +1952,6 @@
 installcheck-target: maybe-installcheck-target-winsup
 installcheck-target: maybe-installcheck-target-libgloss
 installcheck-target: maybe-installcheck-target-libffi
-installcheck-target: maybe-installcheck-target-libjava
 installcheck-target: maybe-installcheck-target-zlib
 installcheck-target: maybe-installcheck-target-boehm-gc
 installcheck-target: maybe-installcheck-target-rda
@@ -2051,7 +2038,6 @@
 mostlyclean-target: maybe-mostlyclean-target-winsup
 mostlyclean-target: maybe-mostlyclean-target-libgloss
 mostlyclean-target: maybe-mostlyclean-target-libffi
-mostlyclean-target: maybe-mostlyclean-target-libjava
 mostlyclean-target: maybe-mostlyclean-target-zlib
 mostlyclean-target: 

C++ patch ping

[Jakub Jelinek]

Hi!

I'd like to ping 3 patches from a week ago:

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg01995.html
  - PR77375 - wrong-code with mutable members in base classes

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg01998.html
  - PR77338 - fix constexpr ICE on PARM_DECL with incomplete type

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg02002.html
  - PR77379 - fix testcase mangling regexps for 32-bit targets

Jakub

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Edlinger]

On 09/05/16 17:00, Eric Gallager wrote:
> On 9/5/16, Bernd Edlinger  wrote:
>> On 09/05/16 14:03, Marek Polacek wrote:
>>> On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
 Hi,

> +  r += !a == ~b;
> +  r += !a == ~(int) b;

 I don't understand why ~b should not be warned at -Wall.
>>>
>>> Yeah, there was an RFE for this but I'm not finding it.
>>>
>>
>> I certainly agree that the warning is compeletely misleading here:
>>
>> test.c: In function 'f':
>> test.c:10:11: warning: logical not is only applied to the left hand side
>> of comparison [-Wlogical-not-parentheses]
>>  r += !a == ~b;
>>  ^~
>> test.c:10:8: note: add parentheses around left hand side expression to
>> silence this warning
>>  r += !a == ~b;
>>   ^~
>>   ( )
>>
>> this will not fix it, but make  it worse.
>> I think a better warning would be
>> warning: ~ on boolean value, did you mean ! ?
>>
>
>
> The exclamation mark followed by question mark looks kind of confusing
> to me; it looks too much like punctuation (instead of an operator).
> Maybe quote it or spell it out?
> i.e.
> warning: '~' on boolean value, did you mean '!' ?
> or
> warning: tilde used to negate boolean value, did you mean to use an
> exclamation mark to negate it instead?
>
>

Yes, thanks good point.

Bernd.

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Edlinger]

On 09/05/16 16:28, Marek Polacek wrote:
> On Mon, Sep 05, 2016 at 02:08:20PM +, Bernd Edlinger wrote:
>> On 09/05/16 14:03, Marek Polacek wrote:
>>> On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
 Hi,

> +  r += !a == ~b;
> +  r += !a == ~(int) b;

 I don't understand why ~b should not be warned at -Wall.
>>>
>>> Yeah, there was an RFE for this but I'm not finding it.
>>>
>>
>> I certainly agree that the warning is compeletely misleading here:
>>
>> test.c: In function 'f':
>> test.c:10:11: warning: logical not is only applied to the left hand side
>> of comparison [-Wlogical-not-parentheses]
>>  r += !a == ~b;
>>  ^~
>> test.c:10:8: note: add parentheses around left hand side expression to
>> silence this warning
>>  r += !a == ~b;
>>   ^~
>>   ( )
>>
>> this will not fix it, but make  it worse.
>> I think a better warning would be
>> warning: ~ on boolean value, did you mean ! ?
>
> Could you please open a PR?  I'll take care of it.
>

Done, thanks: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77490

> Still not sure about other operations.  I guess no one would
> object to warning on bool1 % bool2, but should we warn for
> bool1 + bool2?
>

Yeah, any % bool is just blandly insane.  Please add a warning for that
as well.

Not totally sure what to do about bool + bool: If it is *not* used in 
boolean context it's probably OK, but in boolean context why not use |
instead?

Note: we even optimize unary - in boolean context:
see c-family/c-common.c (c_common_truthvalue_conversion):

 case NEGATE_EXPR:
 case ABS_EXPR:
 case FLOAT_EXPR:
 case EXCESS_PRECISION_EXPR:
   /* These don't change whether an object is nonzero or zero.  */
   return c_common_truthvalue_conversion (location, TREE_OPERAND 
(expr, 0));


I wonder why there is no warning for NEGATE_EXPR and ABS_EXPR.


Bernd.

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Eric Gallager]

On 9/5/16, Bernd Edlinger  wrote:
> On 09/05/16 14:03, Marek Polacek wrote:
>> On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
>>> Hi,
>>>
>>>   > +  r += !a == ~b;
>>>   > +  r += !a == ~(int) b;
>>>
>>> I don't understand why ~b should not be warned at -Wall.
>>
>> Yeah, there was an RFE for this but I'm not finding it.
>>
>
> I certainly agree that the warning is compeletely misleading here:
>
> test.c: In function 'f':
> test.c:10:11: warning: logical not is only applied to the left hand side
> of comparison [-Wlogical-not-parentheses]
> r += !a == ~b;
> ^~
> test.c:10:8: note: add parentheses around left hand side expression to
> silence this warning
> r += !a == ~b;
>  ^~
>  ( )
>
> this will not fix it, but make  it worse.
> I think a better warning would be
> warning: ~ on boolean value, did you mean ! ?
>


The exclamation mark followed by question mark looks kind of confusing
to me; it looks too much like punctuation (instead of an operator).
Maybe quote it or spell it out?
i.e.
warning: '~' on boolean value, did you mean '!' ?
or
warning: tilde used to negate boolean value, did you mean to use an
exclamation mark to negate it instead?


>
> Index: gcc/c/c-typeck.c
> ===
> --- gcc/c/c-typeck.c(revision 239971)
> +++ gcc/c/c-typeck.c(working copy)
> @@ -4192,6 +4192,17 @@ build_unary_op (location_t location,
> break;
>
>   case BIT_NOT_EXPR:
> + {
> +   tree e = arg;
> +
> +   /* Warn if the condition has boolean value.  */
> +   while (TREE_CODE (e) == COMPOUND_EXPR)
> + e = TREE_OPERAND (e, 1);
> +
> +   if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
> +   || truth_value_p (TREE_CODE (e)))
> + warning(0, "~ on boolean value, did you mean ! ?");
> +  }
> /* ~ works on integer types and non float vectors. */
> if (typecode == INTEGER_TYPE
>|| (typecode == VECTOR_TYPE
> Index: gcc/cp/typeck.c
> ===
> --- gcc/cp/typeck.c (revision 239971)
> +++ gcc/cp/typeck.c (working copy)
> @@ -5839,6 +5839,8 @@ cp_build_unary_op (enum tree_code code, tree xarg,
> break;
>
>   case BIT_NOT_EXPR:
> +  if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
> +   warning (0, "~ on boolean value, did you mean ! ?");
> if (TREE_CODE (TREE_TYPE (arg)) == COMPLEX_TYPE)
>  {
>code = CONJ_EXPR;
>
>
>
> Bernd.
>

[PATCH] Allow FP to be used as a call-saved registe

[Tamar Christina]

Hi All,

This patch allows the FP register to be used as a call-saved
register when -fomit-frame-pointer is used.

The change is done in such a way that the defaults do not change.
To use the FP register both -fomit-frame-pointer and
-fcall-saved- need to be used.

Regression ran on aarch64-none-linux-gnu and no regressions.
Bootstrapped and ran regressions on `x86_64` and no regressions.

A new test fp_free_1 was added to test functionality.

Ok for trunk?

Thanks,
Tamar

PS. I don't have commit rights so if OK can someone apply the patch for me.

gcc/
2016-09-01  Tamar Christina  

* gcc/reginfo.c (fix_register): Allow FP to be set if
-fomit-frame-pointer.

gcc/testsuite/
2016-08-17  Tamar Christina  

* gcc.target/aarch64/fp_free_1.c: New.diff --git a/gcc/reginfo.c b/gcc/reginfo.c
index 0cda6aa620098c752522add589b42631b382d9fc..ea96e236a5f76fb7ddc2c9406b7b377aa0eb3087 100644
--- a/gcc/reginfo.c
+++ b/gcc/reginfo.c
@@ -692,12 +692,13 @@ fix_register (const char *name, int fixed, int call_used)
   for (i = reg; i < reg + nregs; i++)
 	{
 	  if ((i == STACK_POINTER_REGNUM
+	   || (
 #ifdef HARD_FRAME_POINTER_REGNUM
-	   || i == HARD_FRAME_POINTER_REGNUM
+		   i == HARD_FRAME_POINTER_REGNUM
 #else
-	   || i == FRAME_POINTER_REGNUM
+		   i == FRAME_POINTER_REGNUM
 #endif
-	   )
+		&& !flag_omit_frame_pointer))
 	  && (fixed == 0 || call_used == 0))
 	{
 	  switch (fixed)
diff --git a/gcc/testsuite/gcc.target/aarch64/fp_free_1.c b/gcc/testsuite/gcc.target/aarch64/fp_free_1.c
new file mode 100644
index ..79e23b13d3fb0f801e201c69286d27bac97aa013
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/fp_free_1.c
@@ -0,0 +1,32 @@
+/* { dg-do run { target aarch64-*-* } } */
+/* { dg-options "-O2 -fno-inline -fomit-frame-pointer -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7 -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19 -ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25 -ffixed-x26 -ffixed-x27 -ffixed-x28 -ffixed-x30 -mgeneral-regs-only -fno-ipa-cp -fno-schedule-fusion -fno-peephole2 -fcall-saved-x29  -fdump-rtl-ira" } */
+
+extern void abort ();
+
+int
+dec (int a, int b)
+{
+  return a + b;
+}
+
+int
+cal (int a, int b)
+{
+  int sum1 = a * b;
+  int sum2 = a / b;
+  int sum = dec (sum1, sum2);
+  return a + b + sum + sum1 + sum2;
+}
+
+int
+main (int argc, char **argv)
+{
+  int ret = cal (2, 1);
+
+  if (ret != 11)
+abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-rtl-dump "assign reg 29" "ira" } } */

Re: [PATCHv2] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/05/16 13:23, Joseph Myers wrote:
> On Sun, 4 Sep 2016, Bernd Edlinger wrote:
>
>> good to have as well.  But this code would still be suspicious
>> even if (x << y) is put in parentheses, because the shift count does
>> not change the result of the condition, as the integer overflow is
>> undefined behavior, and if it does not have side effects or does
>> not throw something, it can even be optimized away.
>
> It's defined in GNU C (when the shift count is nonnegative and less than
> the width of the type) - we document that we do not optimize on the basis
> of it being undefined (although ubsan will still detect it).
>

Oh, good to know, thanks for this info.

Fortunately this will only be a warning, no optimization.

But I think the reasoning is still correct, left shifting
in a boolean context is suspicious, even if -fwrapv may make
it defined.  Do you agree?


Bernd.

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Marek Polacek]

On Mon, Sep 05, 2016 at 02:08:20PM +, Bernd Edlinger wrote:
> On 09/05/16 14:03, Marek Polacek wrote:
> > On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
> >> Hi,
> >>
> >>   > +  r += !a == ~b;
> >>   > +  r += !a == ~(int) b;
> >>
> >> I don't understand why ~b should not be warned at -Wall.
> >
> > Yeah, there was an RFE for this but I'm not finding it.
> >
> 
> I certainly agree that the warning is compeletely misleading here:
> 
> test.c: In function 'f':
> test.c:10:11: warning: logical not is only applied to the left hand side 
> of comparison [-Wlogical-not-parentheses]
> r += !a == ~b;
> ^~
> test.c:10:8: note: add parentheses around left hand side expression to 
> silence this warning
> r += !a == ~b;
>  ^~
>  ( )
> 
> this will not fix it, but make  it worse.
> I think a better warning would be
> warning: ~ on boolean value, did you mean ! ?
 
Could you please open a PR?  I'll take care of it.

Still not sure about other operations.  I guess no one would
object to warning on bool1 % bool2, but should we warn for
bool1 + bool2?

Marek

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Edlinger]

On 09/05/16 14:03, Marek Polacek wrote:
> On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
>> Hi,
>>
>>   > +  r += !a == ~b;
>>   > +  r += !a == ~(int) b;
>>
>> I don't understand why ~b should not be warned at -Wall.
>
> Yeah, there was an RFE for this but I'm not finding it.
>

I certainly agree that the warning is compeletely misleading here:

test.c: In function 'f':
test.c:10:11: warning: logical not is only applied to the left hand side 
of comparison [-Wlogical-not-parentheses]
r += !a == ~b;
^~
test.c:10:8: note: add parentheses around left hand side expression to 
silence this warning
r += !a == ~b;
 ^~
 ( )

this will not fix it, but make  it worse.
I think a better warning would be
warning: ~ on boolean value, did you mean ! ?

>> Frankly I don't even understand why the above statements are
>> completely optimized away.
>>
>> r += !a == ~b;
>> is optimized away, but
>>
>> b = ~b;
>> r += !a == b;
>>
>> Is not.  Why?
>
> Something in ccp I suppose.  But I didn't investigate closely because
> it's not really relevant to this patch, sorry.
>

Yes, I think the problem is that ~ on bool maps [0:1] to [-1:-2]
and thus the result is no longer boolean.  But probaby the assignment
normalizes the result again: b = ~b; has the effect of setting b = 1.

A warning for "~ on boolean" would not be too difficult, but making a
fix-it will probably need more work:

Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c(revision 239971)
+++ gcc/c/c-typeck.c(working copy)
@@ -4192,6 +4192,17 @@ build_unary_op (location_t location,
break;

  case BIT_NOT_EXPR:
+ {
+   tree e = arg;
+
+   /* Warn if the condition has boolean value.  */
+   while (TREE_CODE (e) == COMPOUND_EXPR)
+ e = TREE_OPERAND (e, 1);
+
+   if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE
+   || truth_value_p (TREE_CODE (e)))
+ warning(0, "~ on boolean value, did you mean ! ?");
+  }
/* ~ works on integer types and non float vectors. */
if (typecode == INTEGER_TYPE
   || (typecode == VECTOR_TYPE
Index: gcc/cp/typeck.c
===
--- gcc/cp/typeck.c (revision 239971)
+++ gcc/cp/typeck.c (working copy)
@@ -5839,6 +5839,8 @@ cp_build_unary_op (enum tree_code code, tree xarg,
break;

  case BIT_NOT_EXPR:
+  if (TREE_CODE (TREE_TYPE (arg)) == BOOLEAN_TYPE)
+   warning (0, "~ on boolean value, did you mean ! ?");
if (TREE_CODE (TREE_TYPE (arg)) == COMPLEX_TYPE)
 {
   code = CONJ_EXPR;



Bernd.

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Marek Polacek]

On Fri, Sep 02, 2016 at 03:51:49PM +, Bernd Edlinger wrote:
> Hi,
> 
>  > +  r += !a == ~b;
>  > +  r += !a == ~(int) b;
> 
> I don't understand why ~b should not be warned at -Wall.
 
Yeah, there was an RFE for this but I'm not finding it.

> Frankly I don't even understand why the above statements are
> completely optimized away.
> 
> r += !a == ~b;
> is optimized away, but
> 
> b = ~b;
> r += !a == b;
> 
> Is not.  Why?

Something in ccp I suppose.  But I didn't investigate closely because
it's not really relevant to this patch, sorry.

Marek

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 1 September 2016 at 21:28, Martin Sebor  wrote:
>> The attached version passes bootstrap+test on ppc64le-linux-gnu.
>> Given that it only looks if parameters are restrict qualified and not
>> how they're used inside the callee,
>> this can have false positives as in above test-cases.
>> Should the warning be put in Wextra rather than Wall (I have left it
>> in Wall in the patch)  or only enabled with -Wrestrict ?
>
>
> Awesome!  I've wished for years for a warning like this!
Thanks for experimenting with the patch!
>
> I'm curious if you've tested other examples from 6.7.3.1 of C11
> besides Example 3.  Example 4 seems like something GCC should be
> able to detect but I didn't get a warning with the patch.
Oops, I wasn't aware about example 4, and only implemented the warning
for function call. IIUC, the assignment p = q will result in undefined behavior
if q is qualified with restrict and p is at an outer-scope relative to q ?
>
> I would expect the warning to be especially valuable with string
> manipulation functions like memcpy that have undefined behavior
> for overlapping regions, such as in:
>
>   char a[4];
>
>   void g (void)
>   {
> __builtin_memcpy (a, a + 1, 3);
>   }
>
> But here, too, I didn't get a warning.  I understand that this
> case cannot be handled for arbitrary functions whose semantics
> aren't known but with standard functions for which GCC provides
> intrinsics the effects are known and aliasing violations can in
> common cases be detected.
Indeed, thanks for the suggestions. I will add support for some
standard functions in a follow-up patch.

Thanks,
Prathamesh
>
> Martin

[GCC][PATCH] Add __artificial__ attribute to Aarch64 NEON intrinsics

[Tamar Christina]

Hi all,

This patch adds __artificial__ attribute to the intrinsics
in arm_neon.h so that costs are associated to the user
function during profiling and during the intrinsics are
are hidden in trace.

The attribute does not affect code generation.

No new tests for this since it would require a gdb test
but regression tests on aarch64-none-elf was performed.

The attribute was added with the following bash script:

#!/bin/bash

# first apply to the ones in #define blocks and add extra \ at the end
sed -i -r 's/(__inline.+)(__attribute__\s*)\(\((.+)\)\)\s*\\/\1\\\n\2 \(\(\3, 
__artificial__\)\) \\/m' \
 gcc/config/aarch64/arm_neon.h

# Then write all normal functions
sed -i -r 's/(__inline.+)(__attribute__\s*)\(\((.+)\)\)/\1\n\2 \(\(\3, 
__artificial__\)\)/m' \
 gcc/config/aarch64/arm_neon.h

# Then correct any trailing whitespaces we might have introduced
sed -i 's/[ \t]*$//' \
 gcc/config/aarch64/arm_neon.h

# And then finish up by correcting some attribute values which don't fit the 
patterns above.
sed -i -r 's/(__attribute__\s+)\(\((__always_inline__)\)\)\s+\\/\1\(\(\2, 
__artificial__\)\) \\/m' \
 gcc/config/aarch64/arm_neon.h

It would be easier I think to review/run the script than the changes.
But just for completeness I have attached the G-zipped patch file.

Ok for trunk?

PS. I don't have commit rights, so if OK can someone apply it for me?

Thanks,
Tamar

gcc/
2016-08-19  Tamar Christina  

* config/aarch64/arm_neon.h: Add __artificial__ attribute
  to all inlined functions

attr.gzip
Description: attr.gzip

Re: Testing _Complex varargs passing [was: Alpha, ABI change: pass SFmode and SCmode varargs by reference]

[Joseph Myers]

On Sun, 4 Sep 2016, Uros Bizjak wrote:

> It looks that different handling of _Complex char, _Complex short and
> _Complex float is there on purpose. Is (was?) there a limitation in a
> c language standard that prevents passing of these arguments as
> varargs?

Well, ISO C doesn't define complex integers at all.  But it's deliberate 
(see DR#206) that _Complex float doesn't promote to _Complex double in 
variable arguments.  And there is nothing in ISO C to stop _Complex float 
being passed in variable arguments.

For all these types including the complex integer ones: given that the 
front end doesn't promote them, they should be usable in variable 
arguments.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Implement C _FloatN, _FloatNx types [version 6]

[Joseph Myers]

On Sat, 3 Sep 2016, Andreas Schwab wrote:

> On Aug 17 2016, Joseph Myers  wrote:
> 
> > Index: gcc/testsuite/gcc.dg/torture/float32-basic.c
> > ===
> > --- gcc/testsuite/gcc.dg/torture/float32-basic.c(nonexistent)
> > +++ gcc/testsuite/gcc.dg/torture/float32-basic.c(working copy)
> > @@ -0,0 +1,9 @@
> > +/* Test _Float32.  */
> > +/* { dg-do run } */
> > +/* { dg-options "" } */
> > +/* { dg-add-options float32 } */
> > +/* { dg-require-effective-target float32_runtime } */
> > +
> > +#define WIDTH 32
> > +#define EXT 0
> > +#include "floatn-basic.h"
> 
> This fails on powerpc32, in vafn.

That seems like the alpha issue - an ABI needs to be defined and 
implemented.  Unfortunately the powerabi mailing list, which would have 
been the right place for ABI coordination between implementors, died 
several years ago.  (The 32-bit ABI source code is available at 
.)

If we suppose that _Float32 values should be passed in FPRs in the same 
circumstances in which float values are passed in FPRs, then 
rs6000_gimplify_va_arg would, in the case of an SFmode value coming from 
saved FPRs, need to extract a double value from the saved FPRs and then 
convert to float.  (There would also be the question of later _Float32 
arguments passed on the stack in variable arguments; if those are handled 
like prototyped arguments, they would go in 4-byte stack slots, and 
probably the compiler already does that.)  Of course other ABI choices are 
possible.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCHv2] Add a warning for suspicious use of conditional expressions in boolean context

[Joseph Myers]

On Sun, 4 Sep 2016, Bernd Edlinger wrote:

> good to have as well.  But this code would still be suspicious
> even if (x << y) is put in parentheses, because the shift count does
> not change the result of the condition, as the integer overflow is
> undefined behavior, and if it does not have side effects or does
> not throw something, it can even be optimized away.

It's defined in GNU C (when the shift count is nonnegative and less than 
the width of the type) - we document that we do not optimize on the basis 
of it being undefined (although ubsan will still detect it).

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH, rtl-optimization]: Fix PR77452, ICE: in plus_constant, at explow.c

[Richard Biener]

On September 5, 2016 12:11:47 PM GMT+02:00, Bernd Schmidt  
wrote:
>On 09/04/2016 02:12 PM, Uros Bizjak wrote:
>> As shown in the PR [1], combine is able to simplify lowpart
>> CONST_VECTOR constant pool reference to its inner-mode reference.
>> However, plus_constant was not able to extract the constant from
>> narrowed access.
>>
>> Attached patch teaches plus_constant how to handle this situation.
>
>Ok. It looks like accesses to non-lowpart subregs would be handled by 
>not optimizing them as well rather than crashing.

Are we missing similar handling for complex integer constants?

Richard.

>
>Bernd

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Marek Polacek]

On Mon, Sep 05, 2016 at 12:35:13PM +0200, Bernd Schmidt wrote:
> On 09/02/2016 05:13 PM, Marek Polacek wrote:
> > diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> > index 87da1f1..38d55d4 100644
> > --- gcc/doc/invoke.texi
> > +++ gcc/doc/invoke.texi
> > @@ -5437,8 +5437,8 @@ if (a < 0 && a < 0) @{ @dots{} @}
> >  @opindex Wlogical-not-parentheses
> >  @opindex Wno-logical-not-parentheses
> >  Warn about logical not used on the left hand side operand of a comparison.
> > -This option does not warn if the RHS operand is of a boolean type.  Its
> > -purpose is to detect suspicious code like the following:
> > +This option does not warn if the right operand is considered to be a 
> > Boolean
> > +expression.  Its purpose is to detect suspicious code like the following:
> 
> I think "Boolean" shouldn't be capitalized. The patch looks ok to me
> otherwise.
 
No strong opinions, but looking at
https://en.wikipedia.org/wiki/Boolean_expression
I see that it's capitalized there, so I think let's keep "Boolean".

> > +  r += !a == (b | c);
> 
> I do wonder whether we should give a different warning for this though. I
> personally prefer code involving bools to use || to show it's a logical
> operation. Bit-wise may indicate mistakes where the programmer thought he
> was operating on integers.

Yea.  I'd swear that I've seen an RFE somewhere for this in the GCC BZ,
I mean to warn for bool1 * bool2, bool1 / bool2, etc.  Though I'm not sure
if warning for bool1 | bool2 would be desirable.

Thanks, I think I'll commit this later today.

Marek

Re: [patch v2] Get rid of stack trampolines for nested functions (3/4)

[Segher Boessenkool]

Hi Eric,

On Sun, Sep 04, 2016 at 10:14:22PM +0200, Eric Botcazou wrote:
>   * config/aarch64/aarch64.h(TARGET_CUSTOM_FUNCTION_DESCRIPTORS):Define

Space after ":".  Many spaces are missing in the changelogs for this
series.

> Index: config/rs6000/rs6000.h
> ===
> --- config/rs6000/rs6000.h(revision 239944)
> +++ config/rs6000/rs6000.h(working copy)
> @@ -2914,3 +2914,6 @@ extern GTY(()) tree rs6000_builtin_types[RS6000_BT
>  extern GTY(()) tree rs6000_builtin_decls[RS6000_BUILTIN_COUNT];
>  
>  #define TARGET_SUPPORTS_WIDE_INT 1
> +
> +/* Use custom descriptors instead of trampolines when possible if not AIX.  
> */
> +#define TARGET_CUSTOM_FUNCTION_DESCRIPTORS (DEFAULT_ABI == ABI_AIX ? 0 : 1)

Make this "If not AIX or ELFv1" please?  The ABI_AIX name is a bit
confusing sometimes.


Segher

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Schmidt]

On 09/02/2016 05:13 PM, Marek Polacek wrote:

diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 87da1f1..38d55d4 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5437,8 +5437,8 @@ if (a < 0 && a < 0) @{ @dots{} @}
 @opindex Wlogical-not-parentheses
 @opindex Wno-logical-not-parentheses
 Warn about logical not used on the left hand side operand of a comparison.
-This option does not warn if the RHS operand is of a boolean type.  Its
-purpose is to detect suspicious code like the following:
+This option does not warn if the right operand is considered to be a Boolean
+expression.  Its purpose is to detect suspicious code like the following:


I think "Boolean" shouldn't be capitalized. The patch looks ok to me 
otherwise.



+  r += !a == (b | c);


I do wonder whether we should give a different warning for this though. 
I personally prefer code involving bools to use || to show it's a 
logical operation. Bit-wise may indicate mistakes where the programmer 
thought he was operating on integers.



Bernd

Re: MAINTAINERS update

[Gerald Pfeifer]

On Thu, 19 May 2016, Jeff Law wrote:
Spurred by the lack of response to Sandra's message WRT a cygwin/mingw 
issue, I did a quick pass through the MAINTAINERS file for folks that 
are listed as maintainers, but aren't (to the best of my knowledge) 
acting in those positions anymore.


I removed their names from the maintainers sections, but kept/added them 
to write-after-approval.  Affected individuals:


Geoff Keating
Kazu Hirata
Steve Ellcey
Eric Christopher
Kai Tietz
Dave Korn
Bryce McKinlay
Michael Hayes
Torbjorn Granlund
Jason Eckhardt
Roger Sayle
Daniel Berlin



OK for the trunk?


Yes, thanks.

(I noticed that nobody acked this so far.  This being a form of
documentation, I guess I can approve. ;-)

Gerald* MAINTAINERS: Move several inactive maintainers to the
write-after-approval section.

diff --git a/MAINTAINERS b/MAINTAINERS
index c615168..abda292 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24,7 +24,6 @@ Richard Earnshaw  

 Richard Biener 
 Richard Henderson  
 Jakub Jelinek  
-Geoffrey Keating   
 Richard Kenner 
 Jeff Law   
 Michael Meissner   
@@ -59,14 +58,12 @@ frv portNick Clifton

 frv port   Alexandre Oliva 
 ft32 port  James Bowman
 h8 portJeff Law
-h8 portKazu Hirata 
 hppa port  Jeff Law
 hppa port  John David Anglin   
 i386 port  Jan Hubicka 
 i386 port  Uros Bizjak 
 i386 vector ISA extns  Kirill Yukhin   
 ia64 port  Jim Wilson  
-ia64 port  Steve Ellcey
 iq2000 portNick Clifton
 lm32 port  Sebastien Bourdeauducq  
 m32c port  DJ Delorie  
@@ -77,7 +74,6 @@ m68k-motorola-sysv port   Philippe De Muyter  

 mcore port Nick Clifton
 microblaze Michael Eager   
 mips port  Catherine Moore 
-mips port  Eric Christopher
 mips port  Matthew Fortune 
 mmix port  Hans-Peter Nilsson  
 mn10300 port   Jeff Law
@@ -125,12 +121,10 @@ xtensa port   Sterling Augustine  

 aixDavid Edelsohn  
 Android sub-port   Maxim Kuvyrkov  
 darwin portMike Stump  
-darwin portEric Christopher
 DJGPP  DJ Delorie  
 freebsdAndreas Tobler  
 GNU/Hurd   Thomas Schwinge 
 hpux   John David Anglin   
-hpux   Steve Ellcey
 solarisRainer Orth 

 netbsd Jason Thorpe
 netbsd Krister Walfridsson 
@@ -141,8 +135,6 @@ RTEMS Ports Sebastian Huber 

 VMSDouglas Rupp
 VMSTristan Gingold 
 VxWorks ports  Nathan Sidwell  
-windows, cygwin, mingw Kai Tietz   
-windows, cygwin, mingw Dave Korn   
 
Language Front Ends Maintainers
 
@@ -169,7 +161,6 @@ fp-bit  Ian Lance Taylor

 libdecnumber   Ben Elliston
 libgcc Ian Lance Taylor
 libgcj Tom Tromey  
-libgcj Bryce McKinlay  

Re: [PATCH, rtl-optimization]: Fix PR77452, ICE: in plus_constant, at explow.c

[Bernd Schmidt]

On 09/04/2016 02:12 PM, Uros Bizjak wrote:

As shown in the PR [1], combine is able to simplify lowpart
CONST_VECTOR constant pool reference to its inner-mode reference.
However, plus_constant was not able to extract the constant from
narrowed access.

Attached patch teaches plus_constant how to handle this situation.


Ok. It looks like accesses to non-lowpart subregs would be handled by 
not optimizing them as well rather than crashing.



Bernd

Re: [MPX] Fix for PR77267

[Alexander Ivchenko]

Ok, thanks. The full updated patch is below. I also removed the
'--whole-archive' thing from -static-libmpxwrappers case.
Would that be possible to backport that patch to 6 branch as well?

And could you please also address Sergey's comment on adding weak
symbol that he's made in the bugzilla?

diff --git a/gcc/config.in b/gcc/config.in
index fc3321c..a736de3 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1538,6 +1538,12 @@
 #endif


+/* Define if your linker supports --push-state/--pop-state */
+#ifndef USED_FOR_TARGET
+#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
+#endif
+
+
 /* Define if your linker links a mix of read-only and read-write sections into
a read-write section. */
 #ifndef USED_FOR_TARGET
diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
index 4b9910f..2273170 100644
--- a/gcc/config/i386/linux-common.h
+++ b/gcc/config/i386/linux-common.h
@@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 #endif

+#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
+#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
+#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
+#else
+#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
+#define MPX_LD_AS_NEEDED_GUARD_POP ""
+#endif
+
 #ifndef LIBMPX_SPEC
 #if defined(HAVE_LD_STATIC_DYNAMIC)
 #define LIBMPX_SPEC "\
 %{mmpx:%{fcheck-pointer-bounds:\
 %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
 %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
--lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
+%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_PUSH "} -lmpx \
+%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_POP "} \
+%{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
 LIBMPX_LIBS ""
 #else
 #define LIBMPX_SPEC "\
@@ -98,8 +108,8 @@ along with GCC; see the file COPYING3.  If not see
 #define LIBMPXWRAPPERS_SPEC "\
 %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
 %{static:-lmpxwrappers}\
-%{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
--lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
+%{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION "}\
+-lmpxwrappers %{static-libmpxwrappers: "\
 LD_DYNAMIC_OPTION "}"
 #else
 #define LIBMPXWRAPPERS_SPEC "\
diff --git a/gcc/configure b/gcc/configure
index 871ed0c..094 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -29609,6 +29609,30 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
 $as_echo "$ld_bndplt_support" >&6; }

+# Check linker supports '--push-state'/'--pop-state'
+ld_pushpopstate_support=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
--push-state/--pop-state options" >&5
+$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
+if test x"$ld_is_gold" = xno; then
+  if test $in_tree_ld = yes ; then
+if test "$gcc_cv_gld_major_version" -eq 2 -a
"$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
2; then
+  ld_pushpopstate_support=yes
+fi
+  elif test x$gcc_cv_ld != x; then
+# Check if linker supports --push-state/--pop-state options
+if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
+  ld_pushpopstate_support=yes
+fi
+  fi
+fi
+if test x"$ld_pushpopstate_support" = xyes; then
+
+$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" >&5
+$as_echo "$ld_pushpopstate_support" >&6; }
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)

diff --git a/gcc/configure.ac b/gcc/configure.ac
index 241e82d..93af766 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -6237,6 +6237,27 @@ if test x"$ld_bndplt_support" = xyes; then
 fi
 AC_MSG_RESULT($ld_bndplt_support)

+# Check linker supports '--push-state'/'--pop-state'
+ld_pushpopstate_support=no
+AC_MSG_CHECKING(linker --push-state/--pop-state options)
+if test x"$ld_is_gold" = xno; then
+  if test $in_tree_ld = yes ; then
+if test "$gcc_cv_gld_major_version" -eq 2 -a
"$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
2; then
+  ld_pushpopstate_support=yes
+fi
+  elif test x$gcc_cv_ld != x; then
+# Check if linker supports --push-state/--pop-state options
+if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
+  ld_pushpopstate_support=yes
+fi
+  fi
+fi
+if test x"$ld_pushpopstate_support" = xyes; then
+  AC_DEFINE(HAVE_LD_PUSHPOPSTATE_SUPPORT, 1,
+ [Define if your linker supports --push-state/--pop-state])
+fi
+AC_MSG_RESULT($ld_pushpopstate_support)
+
 # Configure the subdirectories
 # AC_CONFIG_SUBDIRS($subdirs)

Re: [PATCH] Fix alter_output_for_subst_insn (PR other/77421)

[Richard Biener]

On Fri, 2 Sep 2016, Jakub Jelinek wrote:

> Hi!
> 
> The two ports that use define_subst (ix86 and visium) don't do anything in
> this function, other than returning early - return insn_out, so all I could
> do is look at the intent.
> The *insn_out == '*' || *insn_out != '@' got flagged by some tool, the
> "*insn_out == '*' || " part is unnecessary, since '*' != '@'.  I guess the
> reason for it being there is that template starting with * is C code (which
> this function has nothing to do for), template starting with @ is what the
> function wants to do something about and other template strings it wants to
> ignore too.
> 
> But, when I got to this function, I found various weirdo formatting and
> other issues, the most important is that it leaks memory and in my
> understanding allocates that buffer completely uselessly, as it is pretty
> much a strdup of the insn_out after skipping initial spaces (and all @,
> which is weird, IMNSHO the only @ that it should skip is the very first
> one), except that '\0' isn't there and the length is remembered.  But, we
> don't change it at all, so we can as well use the original insn_out.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-09-02  Jakub Jelinek  
> 
>   PR other/77421
>   * gensupport.c (alter_output_for_subst_insn): Remove redundant
>   *insn_out == '*' test.  Don't copy unnecessary to yet another
>   memory buffer, and don't leak it.
> 
> --- gcc/gensupport.c.jj   2016-08-12 17:33:38.0 +0200
> +++ gcc/gensupport.c  2016-09-02 14:55:01.217182312 +0200
> @@ -1632,33 +1632,30 @@ duplicate_each_alternative (const char *
>  static const char *
>  alter_output_for_subst_insn (rtx insn, int alt)
>  {
> -  const char *insn_out, *sp ;
> -  char *old_out, *new_out, *cp;
> -  int i, j, new_len;
> +  const char *insn_out, *old_out;
> +  char *new_out, *cp;
> +  size_t old_len, new_len;
> +  int j;
>  
>insn_out = XTMPL (insn, 3);
>  
> -  if (alt < 2 || *insn_out == '*' || *insn_out != '@')
> +  if (alt < 2 || *insn_out != '@')
>  return insn_out;
>  
> -  old_out = XNEWVEC (char, strlen (insn_out)),
> -  sp = insn_out;
> +  old_out = insn_out + 1;
> +  while (ISSPACE (*old_out))
> +old_out++;
> +  old_len = strlen (old_out);
>  
> -  while (ISSPACE (*sp) || *sp == '@')
> -sp++;
> -
> -  for (i = 0; *sp;)
> -old_out[i++] = *sp++;
> -
> -  new_len = alt * (i + 1) + 1;
> +  new_len = alt * (old_len + 1) + 1;
>  
>new_out = XNEWVEC (char, new_len);
>new_out[0] = '@';
>  
> -  for (j = 0, cp = new_out + 1; j < alt; j++, cp += i + 1)
> +  for (j = 0, cp = new_out + 1; j < alt; j++, cp += old_len + 1)
>  {
> -  memcpy (cp, old_out, i);
> -  *(cp+i) = (j == alt - 1) ? '\0' : '\n';
> +  memcpy (cp, old_out, old_len);
> +  cp[old_len] = (j == alt - 1) ? '\0' : '\n';
>  }
>  
>return new_out;
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH] Remove unnecessary conditional in get_odr_type (PR rtl-optimization/77425)

[Richard Biener]

On Fri, Sep 2, 2016 at 5:07 PM, Jakub Jelinek  wrote:
> Hi!
>
> As mentioned in the PR, we have
> static GTY(()) vec  *odr_types_ptr;
> #define odr_types (*odr_types_ptr)
> and in the else block do
>   odr_types[val->id] = 0;
> first (which is actually
>   (*odr_types_ptr)[val->id] = 0;
> without the obfuscation) and then
>   if (odr_types_ptr)
> ...
> odr_types_ptr is known to be non-NULL in this case, otherwise it couldn't be
> dereferenced first - it can be NULL only when nothing has been added yet,
> but that happens only if the then path is taken.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2016-09-02  Jakub Jelinek  
>
> PR rtl-optimization/77425
> * ipa-devirt.c (get_odr_type): Set val->id unconditionally.
>
> --- gcc/ipa-devirt.c.jj 2016-04-14 21:20:19.0 +0200
> +++ gcc/ipa-devirt.c2016-09-01 12:42:07.077740393 +0200
> @@ -2136,8 +2136,7 @@ get_odr_type (tree type, bool insert)
>/* Be sure we did not recorded any derived types; these may need
>  renumbering too.  */
>gcc_assert (val->derived_types.length() == 0);
> -  if (odr_types_ptr)
> -   val->id = odr_types.length ();
> +  val->id = odr_types.length ();
>vec_safe_push (odr_types_ptr, val);
>  }
>return val;
>
> Jakub

Re: [RFC][SSA] Iterator to visit SSA

[Richard Biener]

On Mon, Sep 5, 2016 at 7:26 AM, Kugan Vivekanandarajah
 wrote:
> Hi All,
>
> While looking at gcc source, I noticed that we are iterating over SSA
> variable from 0 to num_ssa_names in some cases and 1 to num_ssa_names
> in others. It seems that variable 0 is always NULL TREE.

Yeah, that's artificial because we don't assign SSA name version 0 (for some
unknown reason).

> But it can
> confuse people who are looking for the first time. Therefore It might
> be good to follow some consistent usage here.
>
> It might be also good to gave a FOR_EACH_SSAVAR iterator as we do in
> other case. Here is attempt to do this based on what is done in other
> places. Bootstrapped and regression tested on X86_64-linux-gnu with no
> new regressions. is this OK?

First of all some bikeshedding - FOR_EACH_SSA_NAME would be better
as SSAVAR might be confused with iterating over SSA_NAME_VAR.

Then, if you add an iterator why leave the name == NULL handling to consumers?
That looks odd.

Then, SSA names are just in a vector thus why not re-use the vector iterator?

That is,

#define FOR_EACH_SSA_NAME (name) \
  for (unsigned i = 1; SSANAMES (cfun).iterate (i, ); ++i)

would be equivalent to your patch?

Please also don't add new iterators that implicitely use 'cfun' but always use
one that passes it as explicit arg.

Thanks,
Richard.


> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2016-09-05  Kugan Vivekanandarajah  
>
> * tree-ssanames.h (ssa_iterator::ssa_iterator): New.
> (ssa_iterator::get): Likewise.
> (ssa_iterator::next): Likewise.
> (FOR_EACH_SSAVAR): Likewise.
> * cfgexpand.c (update_alias_info_with_stack_vars): Use
> FOR_EACH_SSAVAR to iterate over SSA variables.
> (pass_expand::execute): Likewise.
> * omp-simd-clone.c (ipa_simd_modify_function_body): Likewise.
> * tree-cfg.c (dump_function_to_file): Likewise.
> * tree-into-ssa.c (pass_build_ssa::execute): Likewise.
> (update_ssa): Likewise.
> * tree-ssa-alias.c (dump_alias_info): Likewise.
> * tree-ssa-ccp.c (ccp_finalize): Likewise.
> * tree-ssa-coalesce.c (build_ssa_conflict_graph): Likewise.
> (create_outofssa_var_map): Likewise.
> (coalesce_ssa_name): Likewise.
> * tree-ssa-operands.c (dump_immediate_uses): Likewise.
> * tree-ssa-pre.c (compute_avail): Likewise.
> * tree-ssa-sccvn.c (init_scc_vn): Likewise.
> (scc_vn_restore_ssa_info): Likewise.
> (free_scc_vn): Likwise.
> (run_scc_vn): Likewise.
> * tree-ssa-structalias.c (compute_points_to_sets): Likewise.
> * tree-ssa-ter.c (new_temp_expr_table): Likewise.

Re: [PATCH, c++, PR77427 ] Set TYPE_STRUCTURAL_EQUALITY for sysv_abi va_list

[Richard Biener]

On Sun, Sep 4, 2016 at 11:30 PM, Tom de Vries  wrote:
> On 04/09/16 16:08, Richard Biener wrote:
>>
>> On September 4, 2016 12:33:02 PM GMT+02:00, Tom de Vries
>>  wrote:
>>>
>>> On 04/09/16 08:12, Richard Biener wrote:

 On September 3, 2016 5:23:35 PM GMT+02:00, Tom de Vries
>>>
>>>  wrote:
>>
>> Hi,
>>
>> this patch fixes a c++ ICE, a p1 6/7 regression.
>>
>>
>> Consider test.C:
>> ...
>> void bar (__builtin_va_list &);
>>
>> struct c
>> {
>>   operator const __builtin_va_list &();
>>   operator __builtin_va_list &();
>> };
>>
>> void
>> foo (void) {
>>   struct c c1;
>>
>>   bar (c1);
>> }
>> ...
>>
>> The compiler ICEs as follows:
>> ...
>> test.C: In function ‘void foo()’:
>> test.C:13:10: internal compiler error: canonical types differ for
>> identical types __va_list_tag [1] and __va_list_tag [1]
>>bar (c1);
>>   ^
>> comptypes(tree_node*, tree_node*, int)
>> src/gcc/cp/typeck.c:1430
>> reference_related_p(tree_node*, tree_node*)
>> src/gcc/cp/call.c:1415
>> reference_binding
>> src/gcc/cp/call.c:1559
>> implicit_conversion
>> src/gcc/cp/call.c:1805
>> build_user_type_conversion_1
>> src/gcc/cp/call.c:3776
>> reference_binding
>> src/gcc/cp/call.c:1664
>> implicit_conversion
>> src/gcc/cp/call.c:1805
>> add_function_candidate
>> src/gcc/cp/call.c:2141
>> add_candidates
>> src/gcc/cp/call.c:5394
>> perform_overload_resolution
>> src/gcc/cp/call.c:4066
>> build_new_function_call(tree_node*, vec>>
>>> vl_embed>**,
>>
>>   bool, int)
>> src/gcc/cp/call.c:4143
>> finish_call_expr(tree_node*, vec**,
>>>
>>> bool,
>>
>>   bool, int)
>> src/gcc/cp/semantics.c:2440
>> ...
>>
>> The regression is caused by the commit for PR70955, that adds a
>> "sysv_abi va_list" attribute to the struct in the va_list type
>>>
>>> (which
>>
>> is
>> an array of one of struct).
>>
>> The ICE in comptypes happens as follows: we're comparing two
>>>
>>> versions
>>
>> of
>> va_list type (with identical array element type), each with the
>> canonical type set to themselves. Since the types are considered
>> identical, they're supposed to have identical canonical types,
>>>
>>> which is
>>>
 Did you figure out why they are not assigned the same canonical type?
>>>
>>>
>>> When constructing the first type in ix86_build_builtin_va_list_64, it's
>>>
>>> cached.
>>>
>>> When constructing the second type in build_array_type_1 (with call
>>> stack: grokdeclarator -> cp_build_qualified_type_real ->
>>> build_cplus_array_type -> build_cplus_array_type -> build_array_type ->
>>>
>>> build_array_type_1), we call type_hash_canon.
>>>
>>> But the cached type has name __builtin_sysv_va_list, and the new type
>>> has no name, so we hit the clause 'TYPE_NAME (a->type) != TYPE_NAME
>>> (b->type)' in type_cache_hasher::equal.
>>>
>>> Consequently, TYPE_CANONICAL for the new type remain set to self.
>>
>>
>> But how did it then work before the patch causing this?
>
>
> Before the patch that introduced the attribute, rather than assigning the
> result of ix86_build_builtin_va_list_64 directly sysv_va_list_type_node, an
> intermediate build_variant_type_copy was used.
>
> This had as consequence that the copy was named and not added to the cache,
> and that the original in the type cache remained unnamed.
>
> Adding the build_variant_type_copy back fixes the ICE. But I'm not sure if
> that's a correct fix. The copy would have it's canonical type set to the
> original type. But if we'd search for the canonical type of the copy in the
> type cache, we wouldn't find it.

Huh.  Can't see how making a copy would affect the type cache -- AFAIK nothing
adds the record to the type hash.  The array type is there (and the
FEs have some
"interesting" code regarding qualifiers on arrays).  Btw,

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 4531647..4e00dee 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10550,9 +10550,13 @@ ix86_build_builtin_va_list_64 (void)

   TYPE_ATTRIBUTES (record) = tree_cons (get_identifier ("sysv_abi va_list"),
NULL_TREE, TYPE_ATTRIBUTES (record));
+  SET_TYPE_STRUCTURAL_EQUALITY (record);


This should be enough

   /* The correct type is an array type of one element.  */
-  return build_array_type (record, build_index_type (size_zero_node));
+  tree res = build_array_type (record, build_index_type (size_zero_node));
+  SET_TYPE_STRUCTURAL_EQUALITY (res);
+

and this should be done automagically by build_array_type.

I'm fine 

Re: [PATCH] Tree-level fix for PR 69526

[Robin Dapp]

Ping.
diff --git a/gcc/gimple-match-head.c b/gcc/gimple-match-head.c
index 2beadbc..d66fcb1 100644
--- a/gcc/gimple-match-head.c
+++ b/gcc/gimple-match-head.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
 #include "gimplify.h"
+#include "tree-vrp.h"
 
 
 /* Forward declarations of the private auto-generated matchers.
diff --git a/gcc/match.pd b/gcc/match.pd
index b24bfb4..f54b734 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1119,6 +1119,113 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(minus @0 (minus @0 @1))
@1)
 
+  /* ((T)(A +- CST)) +- CST -> (T)(A) +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+ (for inner_op (plus minus)
+   (simplify
+	 (outer_op (convert (inner_op@3 @0 INTEGER_CST@1)) INTEGER_CST@2)
+	   (with
+	{
+	   /* If the inner operation is wrapped inside a conversion, we have to
+	  check it overflows/underflows and can only then perform the
+	  simplification, i.e. add the second constant to the first
+	  (wrapped) and convert afterwards.  This fixes
+	  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69526 */
+	bool inner_ovf = false;
+
+	bool combine_ovf = true;
+	tree cst = NULL_TREE;
+	bool combine_constants = false;
+	bool types_ok = false;
+
+	tree inner_type = TREE_TYPE (@3);
+	/* Check overflow in wrapped binop? */
+	bool check_inner_ovf = !TYPE_OVERFLOW_UNDEFINED (inner_type);
+	/* Check overflow when combining constants? */
+	bool check_combine_ovf = TYPE_OVERFLOW_UNDEFINED (type);
+
+	signop s1 = TYPE_SIGN (type);
+
+	if (TYPE_PRECISION (type) >= TYPE_PRECISION (inner_type))
+	  {
+		types_ok = true;
+
+		/* Check if the inner binop does not overflow i.e. we have VRP
+		   information and can prove prove it.  */
+		if (check_inner_ovf)
+		  inner_ovf = binop_overflow (inner_op, @0, @1, inner_type);
+		else
+		  inner_ovf = false;
+
+		wide_int w1 = @1;
+		wide_int w2 = @2;
+
+		wide_int combined_cst;
+
+		/* Convert to TYPE keeping possible signedness even when
+		   dealing with unsigned types. */
+		wide_int v1;
+		v1 = v1.from (w1, w2.get_precision(), tree_int_cst_sign_bit
+			  (@1) ? SIGNED : UNSIGNED);
+
+		if (inner_op == MINUS_EXPR)
+		  w1 = wi::neg (w1);
+
+		if (outer_op == MINUS_EXPR)
+		  w2 = wi::neg (w2);
+
+		/* Combine in outer, larger type */
+		combined_cst = wi::add (v1, w2, TYPE_SIGN (type), _ovf);
+
+		/* The combined constant is ok if its type does not exhibit
+		   undefined overflow or there was no overflow when
+		   combining.  */
+		bool combined_cst_ok = !check_combine_ovf || !combine_ovf;
+		if (!inner_ovf && combined_cst_ok)
+		  {
+		/* convert to tree of outer type */
+		cst = wide_int_to_tree (type, combined_cst);
+		combine_constants = true;
+		  }
+	  }
+	  }
+	(if (types_ok && combine_constants && cst)
+	 (outer_op (convert { @0; }) { cst; }))
+	
+#endif
+
+  /* ((T)(A)) +- CST -> (T)(A +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+(simplify
+ (outer_op (convert @0) INTEGER_CST@2)
+ /* Perform binary operation inside the cast if the constant fits
+	and there is no overflow.  */
+   (with
+	{
+	  bool simplify = false;
+
+	  wide_int cst = @2;
+	  tree inner_type = TREE_TYPE (@0);
+	  tree cst_inner = wide_int_to_tree (inner_type, cst);
+	  bool inner_ovf = true;
+
+	  if (TYPE_PRECISION (inner_type) >= TYPE_PRECISION (type)
+	  || TREE_CODE (inner_type) != INTEGER_TYPE)
+	simplify = false;
+	  else
+	  {
+	inner_ovf = binop_overflow (outer_op, @0, cst_inner, inner_type);
+	if (!inner_ovf)
+	  simplify = true;
+	  }
+	}
+	(if (simplify && @0)
+	 (convert (outer_op @0 (convert { @2; }
+	)))
+#endif
+
   /* (A +- CST) +- CST -> A + CST  */
   (for outer_op (plus minus)
(for inner_op (plus minus)
@@ -1132,6 +1239,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (if (cst && !TREE_OVERFLOW (cst))
(inner_op @0 { cst; } ))
 
+
   /* (CST - A) +- CST -> CST - A  */
   (for outer_op (plus minus)
(simplify
diff --git a/gcc/testsuite/gcc.dg/wrapped-binop-simplify-signed-1.c b/gcc/testsuite/gcc.dg/wrapped-binop-simplify-signed-1.c
new file mode 100644
index 000..0afe99a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/wrapped-binop-simplify-signed-1.c
@@ -0,0 +1,76 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-forwprop1-details" } */
+/* { dg-final { scan-tree-dump-times "gimple_simplified to" 11 "forwprop1" } } */
+
+#include 
+
+// should simplify, ignore possible signed overflow in (a - 2) and combine
+// constants
+long foo(int a)
+{
+  return (long)(a - 2) + 1;
+}
+
+// should simplify
+long bar(int a)
+{
+  return (long)(a + 3) - 1;
+}
+
+// should simplify with combined constant in outer type
+long baz(int a)
+{
+  return (long)(a - 1) + 2;
+}
+
+// should simplify
+long baf(int a)
+{
+  return (long)(a + 1) - 2;
+}
+
+// should simplify (same as above)
+long bak(int a)
+{

Re: [PATCH, vec-tails 07/10] Support loop epilogue combining

[Richard Biener]

On Fri, Sep 2, 2016 at 4:46 PM, Yuri Rumyantsev  wrote:
> Hi Jeff,
>
> I am trying to reduce cost of repeated call of if-conversion for
> epilogue vectorization. I'd like to clarify your recommendation -
> should I design additional support for versioning in
> vect_do_peeling_for_loop_bound or lightweight version of if-conversion
> is sufficient? Any help in clarification will be appreciated.

For general infrastructure it would be nice to expose a (post-)dominator
compute for MESE (post-dominators) / SEME (dominators) regions.  I believe
what makes if-conversion expensive is the post-dom compute which happens
for each loop for the whole function.  It shouldn't be very difficult
to write this,
sharing as much as possible code with the current DOM code might need
quite some refactoring though.

If you want to avoid this work then you have to go the versioning route.

Richard.

> Thanks ahead.
> Yuri.
>
> 2016-08-01 19:10 GMT+03:00 Jeff Law :
>> On 08/01/2016 03:09 AM, Ilya Enkovich wrote:
>>>
>>> 2016-07-26 18:38 GMT+03:00 Ilya Enkovich :

 2016-07-26 18:26 GMT+03:00 Jeff Law :
>
> On 07/26/2016 03:57 AM, Ilya Enkovich wrote:
>>>
>>>
>>>
>>> Ilya, what's the fundamental reason why we need to run
>>> if-conversion again? Yes, I know you want to if-convert the
>>> epilogue, but why?
>>>
>>> What are the consequences of not doing if-conversion on the
>>> epilogue? Presumably we miss a vectorization opportunity on the
>>> tail.  But that may be a reasonable limitation to allow the
>>> existing work to move forward while you go back and revamp things a
>>> little.
>>
>>
>>
>> If we have some control-flow in a loop then we have to if-convert it
>> for vectorizer. We need to preserve both versions: if-converted one
>> for vectorizer and the original one to be used if vectorization
>> fails.  For epilogues we have similar situation and need two
>> versions.  I do it by running if-conversion on a copy of original
>> loop. Note that it doesn't run full if-conversion pass. If-conversion
>> is called for epilogue loop only.
>
>
> Right.  So what I think Richi wants you to try is to use the
> if-converted
> loop to construct the if-converted epilogue.  It seems conceptually
> simple
> and low cost -- the question is on the implementation side.  I have no
> clue
> how painful that would be.


 Probably another part of if-conversion may be re-used to build required
 epilogue.  I'll have a look.
>>>
>>>
>>> Hi,
>>>
>>> Yuri will continue my work from this point.
>>
>> Understood.  I'm actually got some comments on #5 and Yuri is already on the
>> CC list for that draft message.
>>
>> Jeff

Re: [PATCH 3/4][Ada,DJGPP] Ada support for DJGPP

[Eric Botcazou]

> Attached output is from last test build (r239639 with DJGPP related patches
> applied, last version of patches for Ada).

Very strange error, line 28 of gtype-ada.h is supposed to have a guard for 
nodes containing the 'common' structure.  Can you post an excerpt of the file?

-- 
Eric Botcazou