Re: [PATCH GCC][v2]Simplify alias check code generation in vectorizer

[Markus Trippelsdorf]

On 2016.09.27 at 15:22 +0200, Richard Biener wrote:
> On Tue, 27 Sep 2016, Robin Dapp wrote:
> 
> > > Also the '=' in the split line goes to the next line according to
> > > coding conventions.
> > 
> > fixed, I had only looked at an instance one function above which had it
> > wrong as well. Also changed comment grammar slightly.
> 
> Ok.

Since you are not listed in MAINTAINERS I've committed your patch.

-- 
Markus

[patch, libgfortran] PR77707 formatted direct access: nextrec off by one

[Jerry DeLisle]

I plan to commit the attached patch in the next few days. Fairly simple.

Regression tested on x86-64.

Regards,

Jerry

2016-09-27  Jerry DeLisle  

PR libgfortran/77707
io/transfer.c (next_record): Flush before calculating next_record.
Correctly calculate.
diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c
index 6009c123..902c0201 100644
--- a/libgfortran/io/transfer.c
+++ b/libgfortran/io/transfer.c
@@ -3720,6 +3720,8 @@ next_record (st_parameter_dt *dtp, int done)
   else
 next_record_w (dtp, done);
 
+  fbuf_flush (dtp->u.p.current_unit, dtp->u.p.mode);
+
   if (!is_stream_io (dtp))
 {
   /* Since we have changed the position, set it to unspecified so
@@ -3733,8 +3735,8 @@ next_record (st_parameter_dt *dtp, int done)
 	  fp = stell (dtp->u.p.current_unit->s);
 	  /* Calculate next record, rounding up partial records.  */
 	  dtp->u.p.current_unit->last_record =
-	(fp + dtp->u.p.current_unit->recl - 1) /
-	  dtp->u.p.current_unit->recl;
+	(fp + dtp->u.p.current_unit->recl) /
+	  dtp->u.p.current_unit->recl - 1;
 	}
   else
 	dtp->u.p.current_unit->last_record++;
@@ -3743,7 +3745,6 @@ next_record (st_parameter_dt *dtp, int done)
   if (!done)
 pre_position (dtp);
 
-  fbuf_flush (dtp->u.p.current_unit, dtp->u.p.mode);
   smarkeor (dtp->u.p.current_unit->s);
 }
 
! { dg-do run }
! PR77707 formatted direct access: nextrec off by one
program directaccess_formatted
  integer nextrec
  open(10, status='scratch', form='formatted', access='direct', recl=10*4)
  write(10,'(10i4)',rec=9) 1,2,3,4,5,6,7,8,9,10
  inquire(unit=10,nextrec=nextrec)
  if (nextrec.ne.10) call abort
  close(10)
end

Re: [PATCH] Fix gcc.dg/tree-ssa/pr68198.c

[Jeff Law]

On 09/27/2016 10:47 PM, Bernd Edlinger wrote:

Hi,

this test does fail because a non-existent dump file is referenced in
the dg-final.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
OK for trunk?

OK.

FWIW, the key here is that the dg-options explicitly disable early jump 
threading, so looking for the early threading dump file is clearly wrong.


Jeff

Re: [PATCH] Fixed up missing semicolons.

[Jeff Law]

On 09/26/2016 07:20 PM, lhmouse wrote:

My name is Liu Hao. :>

Thanks.  I've updated the ChangeLog entry.


Thanks for your work.
And thanks for the mingw patch -- we're always grateful for any help we 
can get on the lesser used platforms.


Jeff

ps.  Yes, I am law on IRC, but I was away when you tried to reach me.  I 
typically don't ever log off...

[PATCH] Fix gcc.dg/tree-ssa/pr68198.c

[Bernd Edlinger]

Hi,

this test does fail because a non-existent dump file is referenced in
the dg-final.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
OK for trunk?



Thanks
Bernd.
2016-09-28  Bernd Edlinger  

	* gcc.dg/tree-ssa/pr68198.c: Fix dg-final.

Index: gcc/testsuite/gcc.dg/tree-ssa/pr68198.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr68198.c	(revision 240540)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr68198.c	(working copy)
@@ -40,4 +40,4 @@
 /* There are 3 FSM jump threading opportunities, two of which will
   get filtered out.  */
 /* { dg-final { scan-tree-dump-times "Registering FSM" 1 "thread1"} } */
-/* { dg-final { scan-tree-dump-times "FSM Thread through multiway branch without threading a multiway branch" 2 "ethread"} } */
+/* { dg-final { scan-tree-dump-times "FSM Thread through multiway branch without threading a multiway branch" 2 "thread1"} } */

Re: [PATCH] fix typos behind incorrect destination buffer length in -Wformat-length warning (77762)

[Jeff Law]

On 09/27/2016 04:50 PM, Martin Sebor wrote:

The attached patch corrects a couple of typos in argument numbers
in the handling of __builtin__vsnprintf_chk calls in the gimple-
ssa-sprintf pass, and another couple of typos in the test for
this that were masking this failure.

As an aside, the patch also fixes the off-by-one line test failures
introduced in r240503.  If there is a way to make the line numbers
relative (as suggested in
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg02070.html) I'm happy
to update the -Wformat-length tests to make use of them (and document
it on the Wiki) if someone can point me at an example (or
documentation).  I couldn't find examples of dg-warning directives
that use the feature.

Thanks
Martin

gcc-77762.diff


PR c/77762 - Incorrect destination buffer length in -Wformat-length warning

gcc/testsuite/ChangeLog:
2016-09-27  Martin Sebor  

PR c/77762
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test_vsnprintf_chk_s):
Call __builtin___vsnprintf_chk, not __builtin___snprintf_chk.
(test_sprintf_p_const): Adjust line numbers to avoid failures
introduced in r240503.

gcc/ChangeLog:
2016-09-27  Martin Sebor  

PR c/77762
* gimple-ssa-sprintf.c (pass_sprintf_length::handle_gimple_call):
Fix typos.

OK.

The relative line number capability was just added:

https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01617.html

Jeff

Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c

[Jeff Law]

On 09/27/2016 03:41 PM, Mike Stump wrote:

On Sep 27, 2016, at 11:38 AM, Jeff Law  wrote:


On 09/27/2016 10:39 AM, James Greenhalgh wrote:

On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:

Hi,

 This patch requires int32plus for
 gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
 failures for a 16 bit int target like the avr. The "%u" format
 specifier tests, for example, use int literals big enough to only fit
 in a long int, and this causes unexpected warnings.

 Comitted to trunk.


This change is obviously incomplete as it does not update the expected
line numbers for warnings generated by this testcase.

Right.

It does make me wonder if these directives could go at the bottom of the file 
so that adding/removing a directive doesn't require updating line #s in the 
file.


We support relative numbers in some of the places now, right?  :-)  absolute 
line numbers should be recoded to relative numbers as people hit them.


Yea, that's probably an even better solution.
jeff

Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Jeff Law]

On 09/27/2016 11:29 AM, Martin Sebor wrote:

On 09/26/2016 02:30 AM, Gerald Pfeifer wrote:

Hi Martin,

I'm afraid there may be further fallout from your patch.  GCC
now bootstraps fine on my tester, but building Wine I started
to see

  internal compiler error: in format_floating, at
gimple-ssa-sprintf.c:1165

four or so days ago.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77740 has the invocation
and a preprocessed input file.


Thanks.  (As I just commented in pr77740, so just to close to loop
here) a patch for the problem awaits code review:

  https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01470.html

I didn't see this in my folder of things to review, so sorry it lingered.

It's OK for the trunk.

THanks,
Jeff

Re: [PATCH] fix profiledbootstrap with -Werror=format-length (77753)

[Jeff Law]

On 09/27/2016 01:30 PM, Martin Sebor wrote:

The attached one line patch increases a local buffer size to
avoid an apparently justified (though in reality likely a false
positive) -Wformat-length warning in varasm.c.  The warning has
been reported to break profiledbootstrap on powerp64le (though
not ordinary bootstrap).

An arguably better solution would be to explicitly constrain
the range of the integer variable to that of valid priority values.
Unfortunately, as pointed out in bug 77721, due to the limitation
of the current VRP implementation, this isn't always reliable.

Martin

gcc-77753.diff


PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to
-Werror=format-length in varasm.c:1573

gcc/ChangeLog:
2016-09-27  Martin Sebor  

PR bootstrap/77753
* varasm.c (assemble_addr_to_section): Increase local buffer size.

OK.

WRT 77721, could you add that test to the testsuite, but xfailed?

Jeff

Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o

[Alan Modra]

On Wed, Sep 28, 2016 at 01:26:12AM +, Joseph Myers wrote:
> On Wed, 28 Sep 2016, Alan Modra wrote:
> 
> > compatibility-ldbl.o is compiled with -mlong-double-64.  When
> > long double .gnu_attribute tags are checked by the linker, it
> > complains about the mismatch between this file and others in
> > libstdc++.
> 
> Is that the only file in libstdc++ that involves long double in its 
> interface at all, and so that gets such attributes?

No, some 12 object files do, all marked with 128-bit IBM long double.
eg. c++locale.o, complex_io.o, hash_tr1.o.

> I'd expect libraries such as libstdc++ and libgcc (generally, all compiler 
> and libc libraries) to be set up in such a way that they will work with 
> all long double choices in user code (via mangling and headers mapping 
> access to long double library functions to the right versions for the 
> chosen type) - and so need to be compiled without these attribute tags to 
> avoid the linker complaining when someone links them with user code built 
> with a non-default choice of long double.  Certainly for glibc I'd think 
> using the option globally to build everything is the right choice (well, 
> except for libnldbl.a, where -mlong-double-64 attributes are logically 
> correct).

Yes, and this is why the linker only warns rather than errors on
mismatching .gnu.attributes tags.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

[Alan Modra]

On Wed, Sep 28, 2016 at 01:20:22AM +, Joseph Myers wrote:
> On Wed, 28 Sep 2016, Alan Modra wrote:
> 
> > I've also added a new option, default on, that disables output of
> > .gnu_attribute tags.  That's needed because this patch alone
> > introduces libstdc++ testsuite regressions, fixed by the next patch.
> 
> This option is missing documentation in invoke.texi.

Oops, I'll add that.

> > +mgnu-attr
> > +Target Report Var(rs6000_gnu_attr) Init(-1) Save
> > +Emit .gnu_attribute tags.
> 
> Why Init(-1)?  That's normally for cases where there is code that checks 
> if it's still -1 (i.e. no option passed explicitly) and does something 
> different in that case, but I don't see any such code in this patch.

Yes, it can be Init(1).  I copied from other options in sysv4.opt.

Incidentally, in playing with #pragma GCC target "-mno-gnu-attr" I
find that it affects all functions in a file, rather than just
functions defined later in the file.  I'll look into that too.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o

[Joseph Myers]

On Wed, 28 Sep 2016, Alan Modra wrote:

> compatibility-ldbl.o is compiled with -mlong-double-64.  When
> long double .gnu_attribute tags are checked by the linker, it
> complains about the mismatch between this file and others in
> libstdc++.

Is that the only file in libstdc++ that involves long double in its 
interface at all, and so that gets such attributes?

I'd expect libraries such as libstdc++ and libgcc (generally, all compiler 
and libc libraries) to be set up in such a way that they will work with 
all long double choices in user code (via mangling and headers mapping 
access to long double library functions to the right versions for the 
chosen type) - and so need to be compiled without these attribute tags to 
avoid the linker complaining when someone links them with user code built 
with a non-default choice of long double.  Certainly for glibc I'd think 
using the option globally to build everything is the right choice (well, 
except for libnldbl.a, where -mlong-double-64 attributes are logically 
correct).

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

Re: [PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

[Joseph Myers]

On Wed, 28 Sep 2016, Alan Modra wrote:

> I've also added a new option, default on, that disables output of
> .gnu_attribute tags.  That's needed because this patch alone
> introduces libstdc++ testsuite regressions, fixed by the next patch.

This option is missing documentation in invoke.texi.

> +mgnu-attr
> +Target Report Var(rs6000_gnu_attr) Init(-1) Save
> +Emit .gnu_attribute tags.

Why Init(-1)?  That's normally for cases where there is code that checks 
if it's still -1 (i.e. no option passed explicitly) and does something 
different in that case, but I don't see any such code in this patch.

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

[PATCH 2/2] Disable .gnu_attribute tags in compatibility-ldbl.o

[Alan Modra]

compatibility-ldbl.o is compiled with -mlong-double-64.  When
long double .gnu_attribute tags are checked by the linker, it
complains about the mismatch between this file and others in
libstdc++.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux biarch.  OK to apply?

* configure.ac (LONG_DOUBLE_COMPAT_FLAGS): New ACSUBST.
* src/Makefile.am (compatibility-ldbl.o, compatibility-ldbl.lo):
Use LONG_DOUBLE_COMPAT_FLAGS.
* Makefile.in: Regenerate.
* configure: Regenerate.
* doc/Makefile.in: Regenerate.
* include/Makefile.in: Regenerate.
* libsupc++/Makefile.in: Regenerate.
* po/Makefile.in: Regenerate.
* python/Makefile.in: Regenerate.
* src/Makefile.in: Regenerate.
* src/c++11/Makefile.in: Regenerate.
* src/c++98/Makefile.in: Regenerate.
* src/filesystem/Makefile.in: Regenerate.
* testsuite/Makefile.in: Regenerate.

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index baf605c..4a5de8c 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -375,6 +375,7 @@ GLIBCXX_ENABLE_LIBSTDCXX_DUAL_ABI([yes])
 GLIBCXX_DEFAULT_ABI
 
 ac_ldbl_compat=no
+LONG_DOUBLE_COMPAT_FLAGS="-mlong-double-64"
 case "$target" in
   powerpc*-*-linux* | \
   sparc*-*-linux* | \
@@ -389,8 +390,13 @@ case "$target" in
 AC_DEFINE([_GLIBCXX_LONG_DOUBLE_COMPAT],1,
  [Define if compatibility should be provided for 
-mlong-double-64.])
 
port_specific_symbol_files="\$(top_srcdir)/config/os/gnu-linux/ldbl-extra.ver"
+case "$target" in
+  powerpc*-*-linux*)
+   LONG_DOUBLE_COMPAT_FLAGS="$LONG_DOUBLE_COMPAT_FLAGS -mno-gnu-attr" ;;
+esac
   fi
 esac
+AC_SUBST(LONG_DOUBLE_COMPAT_FLAGS)
 GLIBCXX_CONDITIONAL(GLIBCXX_LDBL_COMPAT, test $ac_ldbl_compat = yes)
 
 # Check if assembler supports disabling hardware capability support.
diff --git a/libstdc++-v3/src/Makefile.am b/libstdc++-v3/src/Makefile.am
index 00a6168..d9830c2 100644
--- a/libstdc++-v3/src/Makefile.am
+++ b/libstdc++-v3/src/Makefile.am
@@ -109,9 +109,9 @@ libstdc___la_LINK = $(CXXLINK) $(libstdc___la_LDFLAGS)
 # pass -mlong-double-64.
 if GLIBCXX_LDBL_COMPAT
 compatibility-ldbl.lo: compatibility-ldbl.cc
-   $(LTCXXCOMPILE) -mlong-double-64 -c $<
+   $(LTCXXCOMPILE) $(LONG_DOUBLE_COMPAT_FLAGS) -c $<
 compatibility-ldbl.o: compatibility-ldbl.cc
-   $(CXXCOMPILE) -mlong-double-64 -c $<
+   $(CXXCOMPILE) $(LONG_DOUBLE_COMPAT_FLAGS) -c $<
 endif
 
 # Use special rules for C++11 files/objects.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH 1/2][RS6000] .gnu.attribute Tag_GNU_Power_ABI_FP

[Alan Modra]

Extend this attribute to cover long double ABIs, for 64-bit too.  The
idea is that the linker should warn if you are linking object files
with incompatible ABIs, for example IEEE128 long double with IBM long
double.  It's only a warning, and doesn't catch everything.  In
particular, global data mismatches.  ie. initialised global variables
in one format can be used by code that expects a different format
without warning.  Also, a function returning "sizeof (long double)"
or similar won't cause an object file to be tagged.  Oh, and you need
a new linker to see long double warnings.

The patch also corrects an error that crept in to code setting
rs6000_passes_float.  See the added comment.  Passing IEEE128 values
in vsx regs ought to set both Tag_GNU_Power_ABI_FP and
Tag_GNU_Power_ABI_Vector.

I've also added a new option, default on, that disables output of
.gnu_attribute tags.  That's needed because this patch alone
introduces libstdc++ testsuite regressions, fixed by the next patch.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux biarch.  OK to apply?

* config/rs6000/sysv4.opt (mgnu-attr): New option.
* config/rs6000/rs6000.c (HAVE_LD_PPC_GNU_ATTR): Define.
(rs6000_passes_float): Comment.
(rs6000_passes_long_double): New static var.
(call_ABI_of_interest): Return false unless rs6000_gnu_attr is set.
(init_cumulative_args): Set up to emit fp .gnu.attribute for
ELF 64-bit ABIs as well as 32-bit ELF.  Correct rs6000_passes_float
to include fp values returned in vectors.
Set rs6000_passes_long_double.
(rs6000_function_arg_advance_1): Likewise for function args.
(rs6000_elf_file_end): Emit fp .gnu.attribute for ELF 64-bit ABIs,
and SPE.  Emit long double tag value too.
(rs6000_opt_vars): Add gnu-attr.
* configure.ac (HAVE_LD_PPC_GNU_ATTR): New ppc32 test.
* configure: Regenerate.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 4d3706c..49f662a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -183,8 +183,16 @@ unsigned rs6000_pmode;
 unsigned rs6000_pointer_size;
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-/* Flag whether floating point values have been passed/returned.  */
+# ifndef HAVE_LD_PPC_GNU_ATTR
+# define HAVE_LD_PPC_GNU_ATTR 0
+# endif
+/* Flag whether floating point values have been passed/returned.
+   Note that this doesn't say whether fprs are used, since the
+   Tag_GNU_Power_ABI_FP .gnu.attributes value this flag controls
+   should be set for soft-float values passed in gprs and ieee128
+   values passed in vsx registers.  */
 static bool rs6000_passes_float;
+static bool rs6000_passes_long_double;
 /* Flag whether vector values have been passed/returned.  */
 static bool rs6000_passes_vector;
 /* Flag whether small (<= 8 byte) structures have been returned.  */
@@ -10920,7 +10928,7 @@ rs6000_return_in_msb (const_tree valtype)
 static bool
 call_ABI_of_interest (tree fndecl)
 {
-  if (symtab->state == EXPANSION)
+  if (rs6000_gnu_attr && symtab->state == EXPANSION)
 {
   struct cgraph_node *c_node;
 
@@ -10997,7 +11005,7 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree fntype,
 }
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-  if (DEFAULT_ABI == ABI_V4)
+  if (TARGET_ELF && (TARGET_64BIT || DEFAULT_ABI == ABI_V4))
 {
   cum->escapes = call_ABI_of_interest (fndecl);
   if (cum->escapes)
@@ -11025,10 +11033,19 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree 
fntype,
  <= 8))
rs6000_returns_struct = true;
}
- if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (return_mode))
-   rs6000_passes_float = true;
- else if (ALTIVEC_OR_VSX_VECTOR_MODE (return_mode)
-  || SPE_VECTOR_MODE (return_mode))
+ if (SCALAR_FLOAT_MODE_P (return_mode))
+   {
+ rs6000_passes_float = true;
+ if ((HAVE_LD_PPC_GNU_ATTR || TARGET_64BIT)
+ && (FLOAT128_IBM_P (return_mode)
+ || FLOAT128_IEEE_P (return_mode)
+ || (return_type != NULL
+ && (TYPE_MAIN_VARIANT (return_type)
+ == long_double_type_node
+   rs6000_passes_long_double = true;
+   }
+ if (ALTIVEC_OR_VSX_VECTOR_MODE (return_mode)
+ || SPE_VECTOR_MODE (return_mode))
rs6000_passes_vector = true;
}
 }
@@ -11475,16 +11492,23 @@ rs6000_function_arg_advance_1 (CUMULATIVE_ARGS *cum, 
machine_mode mode,
 cum->nargs_prototype--;
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
-  if (DEFAULT_ABI == ABI_V4
+  if (TARGET_ELF && (TARGET_64BIT || DEFAULT_ABI == ABI_V4)
   && cum->escapes)
 {
-  if (SCALAR_FLOAT_MODE_NOT_VECTOR_P (mode))
-   rs6000_passes_float = true;
-  else if (named && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
-   rs6000_passes_vector = true;
-  else if 

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Joseph Myers]

On Tue, 27 Sep 2016, Jonathan Wakely wrote:

> This adds the new 3D std::hypot() functions. This implementation seems
> to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
> hypot(hypot(x, y), z), and should be a bit more accurate at very large
> or very small values due to reducing the arguments by the largest one.
> Improvements welcome though, as this is not my forte.

Should I take it from this implementation that C++ is not concerned by 
certain considerations that would arise for C: spurious underflow 
exceptions from the scaling when some arguments much larger than others; 
spurious "invalid" exceptions from the comparisons when any argument is 
(quiet) NaN; handling of mixed (quiet) NaN and Inf arguments (where ISO C 
Annex F has Inf returned, not NaN)?

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

Re: [PATCH, Fortran] Extension: COTAN and degree-valued trig intrinsics with -fdec-math

[Joseph Myers]

On Mon, 26 Sep 2016, Tobias Burnus wrote:

> Regarding the decimal trigonometric functions, the internet suggests to
> use
>sin (fmod ((x), 360) * M_PI / 180)
> instead of
>sin (x * M_PI / 180)
> to yield better results for angles which are larger than +/-360 degrees.

Actually for good results you should reduce to the interval +/-45 degrees 
(which may require swapping which of sin and cos is used, or between 
tangent and cotangent, and then adjusting the sign of the result, 
depending on the quadrant the original argument was in).  Consider e.g. 
cos of an argument close to 90 degrees; subtracting 90 and taking -sin of 
the result converted to radians gives a good result, converting 90 
directly to radians and taking cos of that yields large errors (in ulps) 
for such input.

(Unfortunately MPFR doesn't have sinpi etc., or the variants sinu etc. 
suggested in its TODO file, which would be helpful for implementing these 
functions for constant arguments.)

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

[PATCH] fix typos behind incorrect destination buffer length in -Wformat-length warning (77762)

[Martin Sebor]

The attached patch corrects a couple of typos in argument numbers
in the handling of __builtin__vsnprintf_chk calls in the gimple-
ssa-sprintf pass, and another couple of typos in the test for
this that were masking this failure.

As an aside, the patch also fixes the off-by-one line test failures
introduced in r240503.  If there is a way to make the line numbers
relative (as suggested in
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg02070.html) I'm happy
to update the -Wformat-length tests to make use of them (and document
it on the Wiki) if someone can point me at an example (or
documentation).  I couldn't find examples of dg-warning directives
that use the feature.

Thanks
Martin
PR c/77762 - Incorrect destination buffer length in -Wformat-length warning

gcc/testsuite/ChangeLog:
2016-09-27  Martin Sebor  

	PR c/77762
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test_vsnprintf_chk_s):
	Call __builtin___vsnprintf_chk, not __builtin___snprintf_chk.
	(test_sprintf_p_const): Adjust line numbers to avoid failures
	introduced in r240503.

gcc/ChangeLog:
2016-09-27  Martin Sebor  

	PR c/77762
	* gimple-ssa-sprintf.c (pass_sprintf_length::handle_gimple_call):
	Fix typos.

Index: gcc/gimple-ssa-sprintf.c
===
--- gcc/gimple-ssa-sprintf.c	(revision 240556)
+++ gcc/gimple-ssa-sprintf.c	(working copy)
@@ -2536,8 +2536,8 @@ pass_sprintf_length::handle_gimple_call (gimple_st
   // Signature:
   //   __builtin___vsnprintf_chk (dst, size, ost, objsize, format, va)
   idx_dstsize = 1;
-  idx_objsize = 2;
-  idx_format = 3;
+  idx_objsize = 3;
+  idx_format = 4;
   info.argidx = -1;
   info.bounded = true;
   break;
Index: gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c	(revision 240556)
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c	(working copy)
@@ -95,7 +95,7 @@ void test_sprintf_p_const (void)
  format null pointers as 0 or 0x0 and so the following will only be
  diagnosed on the former targets.  */
   T (5, "%p", (void*)0);
-  /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux } 96 } */
+  /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux } 97 } */
 
   /* The exact output for %p is unspecified by C.  Two formats are known:
  same as %tx (for example AIX) and same as %#tx (for example Solaris).  */
@@ -107,8 +107,8 @@ void test_sprintf_p_const (void)
  as with signed integer conversions (i.e., it prepends a space).  Other
  known implementations ignore it.  */
   T (6, "% p",(void*)0x234);  /* { dg-warning ". . flag used with .%p." } */
-  /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 108 } */
-  /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } 108 } */
+  /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 109 } */
+  /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } 109 } */
 }
 
 /* Verify that no warning is issued for calls that write into a flexible
@@ -1404,9 +1404,9 @@ void test_vsnprintf_chk_s (__builtin_va_list va)
   /* Verify that specifying a size of the destination buffer that's
  bigger than its actual size (normally determined and passed to
  the function by __builtin_object_size) is diagnosed.  */
-  __builtin___snprintf_chk (buffer, 123, 0, 122, " ");   /* { dg-warning "always overflow|specified size 123 exceeds the size 122 of the destination object" } */
+  __builtin___vsnprintf_chk (buffer, 123, 0, 122, "%-s", va);   /* { dg-warning "always overflow|specified size 123 exceeds the size 122 of the destination object" } */
 
-  __builtin___snprintf_chk (buffer, __SIZE_MAX__, 0, 2, " ");   /* { dg-warning "always overflow|destination size .\[0-9\]+. too large" } */
+  __builtin___vsnprintf_chk (buffer, __SIZE_MAX__, 0, 2, "%-s", va);   /* { dg-warning "always overflow|destination size .\[0-9\]+. too large" } */
 
   T (0, "%s");
   T (1, "%s");

Re: Split c-common.c?

[Joseph Myers]

On Mon, 26 Sep 2016, Marek Polacek wrote:

> Before I spend time on this, I wanted to check if you consider this a good
> idea.  Since c-common.c has grown a lot and is quite large now, I think we
> might split it into c-warn.c, where various warning routines would go.  What 
> do
> you think?

If you have a logical division of c-common.c into different areas of 
functionality, splitting them into separate files makes sense.

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

Re: [PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Marc Glisse]

On Tue, 27 Sep 2016, Jonathan Wakely wrote:


This adds the new 3D std::hypot() functions. This implementation seems
to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
hypot(hypot(x, y), z), and should be a bit more accurate at very large
or very small values due to reducing the arguments by the largest one.
Improvements welcome though, as this is not my forte.


I understand the claims about accuracy, but the one about speed seems very 
surprising to me. Was your test specifically with denormals on 
not-so-recent hardware, or specifically when sqrt does a library call for 
errno? Otherwise, I have a hard time believing that 3 multiplications and 
2 additions can be slower than 4 multiplications, 2 additions, plus a 
bunch of tests and divisions.


--
Marc Glisse

Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c

[Mike Stump]

On Sep 27, 2016, at 11:38 AM, Jeff Law  wrote:
> 
> On 09/27/2016 10:39 AM, James Greenhalgh wrote:
>> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:
>>> Hi,
>>> 
>>>  This patch requires int32plus for
>>>  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
>>>  failures for a 16 bit int target like the avr. The "%u" format
>>>  specifier tests, for example, use int literals big enough to only fit
>>>  in a long int, and this causes unexpected warnings.
>>> 
>>>  Comitted to trunk.
>> 
>> This change is obviously incomplete as it does not update the expected
>> line numbers for warnings generated by this testcase.
> Right.
> 
> It does make me wonder if these directives could go at the bottom of the file 
> so that adding/removing a directive doesn't require updating line #s in the 
> file.

We support relative numbers in some of the places now, right?  :-)  absolute 
line numbers should be recoded to relative numbers as people hit them.

libgo patch committed: Separate mksysinfo inputs into separate Makefile targets

[Ian Lance Taylor]

This patch to libgo separates the inputs to mksysinfo.sh into separate
Makefile targets.  This is a step toward a smaller version of
mksysinfo.sh that will generate system-specific information for the
runtime package.  The runtime package can not import the syscall
package, as the syscall package imports the runtime package.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240559)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-8aca265d317059ae6d9721a4a231895d80d0a82c
+4046a883070c1f5f58de336f7378f3bca69ea2b6
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/Makefile.am
===
--- libgo/Makefile.am   (revision 240334)
+++ libgo/Makefile.am   (working copy)
@@ -691,9 +691,26 @@ s-syscall_arch: Makefile
$(SHELL) $(srcdir)/mvifdiff.sh syscall_arch.go.tmp syscall_arch.go
$(STAMP) $@
 
+SYSINFO_FLAGS = \
+   $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) \
+   $(CPPFLAGS) $(OSCFLAGS) -O
+
+gen-sysinfo.go: s-gen-sysinfo; @true
+s-gen-sysinfo: $(srcdir)/sysinfo.c config.h
+   $(CC) $(SYSINFO_FLAGS) -fdump-go-spec=tmp-gen-sysinfo.go -std=gnu99 -S 
-o sysinfo.s $(srcdir)/sysinfo.c
+   rm -f sysinfo.s
+   $(SHELL) $(srcdir)/mvifdiff.sh tmp-gen-sysinfo.go gen-sysinfo.go
+   $(STAMP) $@
+
+errno.i: s-errno; @true
+s-errno:
+   echo '#include ' | $(CC) $(SYSINFO_FLAGS) -x c - -E -dM > 
tmp-errno.i
+   $(SHELL) $(srcdir)/mvifdiff.sh tmp-errno.i errno.i
+   $(STAMP) $@
+
 sysinfo.go: s-sysinfo; @true
-s-sysinfo: $(srcdir)/mksysinfo.sh config.h
-   CC="$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) 
$(CPPFLAGS) $(OSCFLAGS) -O" $(SHELL) $(srcdir)/mksysinfo.sh
+s-sysinfo: $(srcdir)/mksysinfo.sh gen-sysinfo.go errno.i
+   $(SHELL) $(srcdir)/mksysinfo.sh
$(SHELL) $(srcdir)/mvifdiff.sh tmp-sysinfo.go sysinfo.go
$(STAMP) $@
 
Index: libgo/mksysinfo.sh
===
--- libgo/mksysinfo.sh  (revision 240053)
+++ libgo/mksysinfo.sh  (working copy)
@@ -4,280 +4,21 @@
 # Use of this source code is governed by a BSD-style
 # license that can be found in the LICENSE file.
 
-# Create sysinfo.go.
+# Create sysinfo.go from gen-sysinfo.go and errno.i.
 
 # This shell script creates the sysinfo.go file which holds types and
-# constants extracted from the system header files.  This relies on a
-# hook in gcc: the -fdump-go-spec option will generate debugging
-# information in Go syntax.
+# constants extracted from the system header files.  This reads the
+# raw data from gen-sysinfo.go which is generated using the
+# -fdump-go-spec option.
+
+# This currently exposes some names that ideally should not be
+# exposed, as they match grep patterns.  E.g., WCHAR_MIN gets exposed
+# because it starts with W, like the wait flags.
 
-# We currently #include all the files at once, which works, but leads
-# to exposing some names which ideally should not be exposed, as they
-# match grep patterns.  E.g., WCHAR_MIN gets exposed because it starts
-# with W, like the wait flags.
-
-CC=${CC:-gcc}
 OUT=tmp-sysinfo.go
 
 set -e
 
-rm -f sysinfo.c
-cat > sysinfo.c <
-#include 
-#include 
-#include 
-#include 
-#include 
-/*  needs u_char/u_short, but  is only
-   included by  if _SGIAPI (i.e. _SGI_SOURCE
-   && !_XOPEN_SOURCE.
-only defines TIOCNOTTY if !_XOPEN_SOURCE, while
-does so unconditionally.  */
-#ifdef __sgi__
-#include 
-#include 
-#endif
-#include 
-#if defined(HAVE_NETINET_IN_SYSTM_H)
-#include 
-#endif
-#if defined(HAVE_NETINET_IP_H)
-#include 
-#endif
-#if defined(HAVE_NETINET_IP_MROUTE_H)
-#include 
-#endif
-#if defined(HAVE_NETINET_IF_ETHER_H)
-#include 
-#endif
-#include 
-#include 
-#include 
-#if defined(HAVE_SYSCALL_H)
-#include 
-#endif
-#if defined(HAVE_SYS_SYSCALL_H)
-#include 
-#endif
-#if defined(HAVE_SYS_EPOLL_H)
-#include 
-#endif
-#if defined(HAVE_SYS_FILE_H)
-#include 
-#endif
-#if defined(HAVE_SYS_MMAN_H)
-#include 
-#endif
-#if defined(HAVE_SYS_PRCTL_H)
-#include 
-#endif
-#if defined(HAVE_SYS_PTRACE_H)
-#include 
-#endif
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#if defined(HAVE_SYS_USER_H)
-#include 
-#endif
-#if defined(HAVE_SYS_UTSNAME_H)
-#include 
-#endif
-#if defined(HAVE_SYS_SELECT_H)
-#include 
-#endif
-#include 
-#include 
-#include 
-#include 
-#include 
-#if defined(HAVE_LINUX_FILTER_H)
-#include 
-#endif
-#if defined(HAVE_LINUX_IF_ADDR_H)
-#include 
-#endif
-#if defined(HAVE_LINUX_IF_ETHER_H)
-#include 
-#endif
-#if defined(HAVE_LINUX_IF_TUN_H)
-#include 
-#endif
-#if defined(HAVE_LINUX_NETLINK_H)
-#include 
-#endif
-#if defined(HAVE_LINUX_RTNETLINK_H)
-#include 
-#endif
-#if 

Go patch committed: Don't duplicate calls with multiple results

[Ian Lance Taylor]

While moving expressions around to preserve the specified order of
evaluation, the Go frontend was duplicating call expressions with
multiple results.  This had no bad effect as the second instance
generated no code for the backend.  But, it's pointless and wasteful.
This patch by Than McIntosh fixes the problem.  This fixes
https://golang.org/issue/17237.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240558)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-1d8d834b5eb9f683cc06529145b353bb5b08e7ea
+8aca265d317059ae6d9721a4a231895d80d0a82c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc   (revision 240453)
+++ gcc/go/gofrontend/gogo.cc   (working copy)
@@ -3616,11 +3616,21 @@ Order_eval::statement(Block* block, size
  // be handled specially.  We can't create a temporary
  // because there is no type to give it.  Any actual uses of
  // the values will be done via Call_result_expressions.
- s = Statement::make_statement(*pexpr, true);
-   }
+  //
+  // Since a given call expression can be shared by multiple
+  // Call_result_expressions, avoid hoisting the call the
+  // second time we see it here.
+  if (this->remember_expression(*pexpr))
+s = NULL;
+  else
+s = Statement::make_statement(*pexpr, true);
+}
 
-  block->insert_statement_before(*pindex, s);
-  ++*pindex;
+  if (s != NULL)
+{
+  block->insert_statement_before(*pindex, s);
+  ++*pindex;
+}
 }
 
   if (init != orig_init)
@@ -7949,13 +7959,14 @@ Traverse::remember_type(const Type* type
 }
 
 // Record that we are looking at an expression, and return true if we
-// have already seen it.
+// have already seen it. NB: this routine used to assert if the traverse
+// mask did not include expressions/types -- this is no longer the case,
+// since it can be useful to remember specific expressions during
+// walks that only cover statements.
 
 bool
 Traverse::remember_expression(const Expression* expression)
 {
-  go_assert((this->traverse_mask() & traverse_types) != 0
-|| (this->traverse_mask() & traverse_expressions) != 0);
   if (this->expressions_seen_ == NULL)
 this->expressions_seen_ = new Expressions_seen();
   std::pair ins =

Patch ping

[Jakub Jelinek]

Hi!

On Tue, Sep 20, 2016 at 06:12:26PM +0200, Jakub Jelinek wrote:
> 2016-09-20  Jakub Jelinek  
> 
>   PR c++/77467
>   * constexpr.c (enum constexpr_switch_state): New.
>   (struct constexpr_ctx): Add css_state field.
>   (label_matches): Add CTX and STMT arguments, remove I and
>   DEFAULT_LABEL.  For CASE_LABEL_EXPR assert ctx->css_state != NULL,
>   handle default labels according to css_state.
>   (cxx_eval_statement_list): Remove statement skipping, label_matches
>   and default_label handling code.
>   (cxx_eval_loop_expr): Exit after first iteration even if
>   switches (jump_target).
>   (cxx_eval_switch_expr): Set up css_state field in ctx, if default
>   label has been seen in the body, but no cases matched, evaluate
>   the body second time.
>   (cxx_eval_constant_expression): Handle stmt skipping and label_matches
>   here.  Handle PREDICT_EXPR.  For MODIFY_EXPR or INIT_EXPR, assert
>   statement is not skipped.  For COND_EXPR during skipping, don't
>   evaluate condition, just the then block and if still skipping at the
>   end also the else block.
>   (cxx_eval_outermost_constant_expr): Adjust constexpr_ctx initializer.
>   (is_sub_constant_expr): Likewise.
> 
>   * g++.dg/cpp1y/constexpr-77467.C: New test.

I'd like to ping this patch.  Ok for trunk?

Jakub

Re: [PATCH 4/5] shrink-wrap: Shrink-wrapping for separate components

[Jeff Law]

On 09/23/2016 02:21 AM, Segher Boessenkool wrote:

Deciding what blocks should run with a certain component active so that
the total cost of executing the prologues (and epilogues) is optimal, is
not a computationally feasible problem.  Instead, for each basic block,
we estimate the cost of putting a prologue right before the block, and
if that is cheaper than the total cost of putting prologues optimally
(according to the estimated cost) in the dominator subtrees strictly
dominated by this first block, place it at the first block instead.
This simple procedure places the components optimally for any dominator
sub tree where the root node's cost does not depend on anything outside
its subtree.

The cost is the execution frequency of all edges into the block coming
from blocks that do not have this component active.  The estimated cost
is the execution frequency of the block, minus the execution frequency
of any backedges (which by definition are coming from subtrees, so if
the "head" block gets a prologue, the source block of any backedge has
that component active as well).

Currently, the epilogues are placed as late as possible, given the
constraints.  This does not matter for execution cost, but we could
save a little bit of code size by placing the epilogues in a smarter
way.  This is a possible future optimisation.

Now all that is left is inserting prologues and epilogues on all edges
that jump into resp. out of the "active" set of blocks.  Often we need
to insert some components' prologues (or epilogues) on all edges into
(or out of) a block.  In theory cross-jumping can unify all such, but
in practice that often fails; besides, that is a lot of work.  So in
this case we insert the prologue and epilogue components at the "head"
or "tail" of a block, instead.

As a final optimisation, if a block needs a prologue and its immediate
dominator has the block as a post-dominator, that immediate dominator
gets the prologue as well.


2016-09-23  Segher Boessenkool  

* function.c (thread_prologue_and_epilogue_insns): Recompute the
live info.  Call try_shrink_wrapping_separate.  Compute the
prologue_seq afterwards, if it has possibly changed.  Compute the
split_prologue_seq and epilogue_seq later, too.
* shrink-wrap.c: #include cfgbuild.h.
(dump_components): New function.
(struct sw): New struct.
(SW): New function.
(init_separate_shrink_wrap): New function.
(fini_separate_shrink_wrap): New function.
(place_prologue_for_one_component): New function.
(spread_components): New function.
(disqualify_problematic_components): New function.
(emit_common_heads_for_components): New function.
(emit_common_tails_for_components): New function.
(insert_prologue_epilogue_for_components): New function.
(try_shrink_wrapping_separate): New function.
* shrink-wrap.h: Declare try_shrink_wrapping_separate.

---
 gcc/function.c|   9 +-
 gcc/shrink-wrap.c | 729 ++
 gcc/shrink-wrap.h |   1 +
 3 files changed, 737 insertions(+), 2 deletions(-)

diff --git a/gcc/function.c b/gcc/function.c
index 53bad87..79e7b4f 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5920,9 +5920,7 @@ thread_prologue_and_epilogue_insns (void)
   edge entry_edge = single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun));
   edge orig_entry_edge = entry_edge;

-  rtx_insn *split_prologue_seq = make_split_prologue_seq ();
   rtx_insn *prologue_seq = make_prologue_seq ();
-  rtx_insn *epilogue_seq = make_epilogue_seq ();

   /* Try to perform a kind of shrink-wrapping, making sure the
  prologue/epilogue is emitted only around those parts of the
@@ -5930,6 +5928,13 @@ thread_prologue_and_epilogue_insns (void)

   try_shrink_wrapping (_edge, prologue_seq);

+  try_shrink_wrapping_separate (entry_edge->dest);
+
+  if (crtl->shrink_wrapped_separate)
+prologue_seq = make_prologue_seq ();
Note this implies that make_prologue_seq (and consequently the target 
specific bits for prologue generation) can be safely called more than 
once.  That's probably OK, but might be worth a comment -- your decision 
whether or not to add the comment.




diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
index b85b1c3..0dced22 100644
--- a/gcc/shrink-wrap.c
+++ b/gcc/shrink-wrap.c



+/* Place the prologue for component WHICH, in the basic blocks dominated
+   by HEAD.  Do a DFS over the dominator tree, and set bit WHICH in the
+   HAS_COMPONENTS of a block if either the block has that bit set in
+   NEEDS_COMPONENTS, or it is cheaper to place the prologue here than in all
+   dominator subtrees separately.  */
+static void
+place_prologue_for_one_component (unsigned int which, basic_block head)
+{
+  /* The block we are currently dealing with.  */
+  basic_block bb = head;
+  /* Is this the first time we visit this block, i.e. have we just gone
+ 

Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c

[Christophe Lyon]

On 27 September 2016 at 20:38, Jeff Law  wrote:
> On 09/27/2016 10:39 AM, James Greenhalgh wrote:
>>
>> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:
>>>
>>> Hi,
>>>
>>>   This patch requires int32plus for
>>>   gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
>>>   failures for a 16 bit int target like the avr. The "%u" format
>>>   specifier tests, for example, use int literals big enough to only fit
>>>   in a long int, and this causes unexpected warnings.
>>>
>>>   Comitted to trunk.
>>
>>
>> This change is obviously incomplete as it does not update the expected
>> line numbers for warnings generated by this testcase.
>
> Right.
>
> It does make me wonder if these directives could go at the bottom of the
> file so that adding/removing a directive doesn't require updating line #s in
> the file.
>
> jeff
>

I did observe regressions too, on aarch64:
  - PASS now FAIL [PASS => FAIL]:
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings, line 96)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for
warnings, line 108)

Christophe

Go patch committed: add src information to AST dumps

[Ian Lance Taylor]

This patch by Than McIntosh adds some file/line information to the AST
dumps generated by the Go frontend.  Bootstrapped and ran Go testsuite
on x86_64-pc-linux-gnu.  Committed to mainline.

Ian

2016-09-27  Than McIntosh  

* go-linemap.cc (Gcc_linemap::to_string): New method.
Index: gcc/go/ChangeLog
===
--- gcc/go/ChangeLog(revision 240453)
+++ gcc/go/ChangeLog(working copy)
@@ -1,3 +1,7 @@
+2016-09-27  Ian Lance Taylor  
+
+   * go-linemap.cc (Gcc_linemap::to_string): New method.
+
 2016-09-23  Than McIntosh  
 
* go-gcc-diagnostics.cc: New file.
Index: gcc/go/go-linemap.cc
===
--- gcc/go/go-linemap.cc(revision 240053)
+++ gcc/go/go-linemap.cc(working copy)
@@ -29,6 +29,9 @@ class Gcc_linemap : public Linemap
   void
   stop();
 
+  std::string
+  to_string(Location);
+
  protected:
   Location
   get_predeclared_location();
@@ -60,6 +63,31 @@ Gcc_linemap::start_file(const char *file
   this->in_file_ = true;
 }
 
+// Stringify a location
+
+std::string
+Gcc_linemap::to_string(Location location)
+{
+  const line_map_ordinary *lmo;
+  source_location resolved_location;
+
+  // Screen out unknown and predeclared locations; produce output
+  // only for simple file:line locations.
+  resolved_location =
+  linemap_resolve_location (line_table, location.gcc_location(),
+LRK_SPELLING_LOCATION, );
+  if (lmo == NULL || resolved_location < RESERVED_LOCATION_COUNT)
+return "";
+  const char *path = LINEMAP_FILE (lmo);
+  if (!path)
+return "";
+
+  // Strip the source file down to the base file, to reduce clutter.
+  std::stringstream ss;
+  ss << lbasename(path) << ":" << SOURCE_LINE (lmo, location.gcc_location());
+  return ss.str();
+}
+
 // Stop getting locations.
 
 void
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240457)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-28b79f1d5a3a8924329128999a21d0693e08a603
+1d8d834b5eb9f683cc06529145b353bb5b08e7ea
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/go-linemap.h
===
--- gcc/go/gofrontend/go-linemap.h  (revision 240053)
+++ gcc/go/gofrontend/go-linemap.h  (working copy)
@@ -17,7 +17,6 @@
 // The type is normally passed by value rather than by reference, and
 // it should support that efficiently.  The type should be defined in
 // "go-location.h".
-
 #include "go-location.h"
 
 // The Linemap class is a pure abstract interface, plus some static
@@ -58,6 +57,12 @@ class Linemap
   virtual void
   stop() = 0;
 
+  // Produce a human-readable description of a Location, e.g.
+  // "foo.go:10". Returns an empty string for predeclared, builtin or
+  // unknown locations.
+  virtual std::string
+  to_string(Location) = 0;
+
  protected:
   // Return a special Location used for predeclared identifiers.  This
   // Location should be different from that for any actual source
@@ -122,6 +127,14 @@ class Linemap
 go_assert(Linemap::instance_ != NULL);
 return Linemap::instance_->is_unknown(loc);
   }
+
+  // Produce a human-readable description of a Location.
+  static std::string
+  location_to_string(Location loc)
+  {
+go_assert(Linemap::instance_ != NULL);
+return Linemap::instance_->to_string(loc);
+  }
 };
 
 // The backend interface must define this function.  It should return
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 240453)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -191,6 +191,21 @@ class Error_statement : public Statement
   do_dump_statement(Ast_dump_context*) const;
 };
 
+//
+// Helper to tack on available source position information
+// at the end of a statement.
+//
+static std::string
+dsuffix(Location location)
+{
+  std::string lstr = Linemap::location_to_string(location);
+  if (lstr == "")
+return lstr;
+  std::string rval(" // ");
+  rval += lstr;
+  return rval;
+}
+
 // Dump the AST representation for an error statement.
 
 void
@@ -338,7 +353,7 @@ Variable_declaration_statement::do_dump_
   ast_dump_context->ostream() <<  "= ";
   ast_dump_context->dump_expression(var->init());
 }
-  ast_dump_context->ostream() << std::endl;
+  ast_dump_context->ostream() << dsuffix(location()) << std::endl;
 }
 
 // Make a variable declaration.
@@ -533,7 +548,7 @@ Temporary_statement::do_dump_statement(A
   ast_dump_context->ostream() << " = ";
   ast_dump_context->dump_expression(this->init_);
 }
-  ast_dump_context->ostream() << std::endl;
+  

Re: debug container mutex association

[François Dumont]

On 27/09/2016 17:29, Jonathan Wakely wrote:

On 20/09/16 09:53 +0100, Jonathan Wakely wrote:

On 19/09/16 21:56 +0200, François Dumont wrote:

Hi

  Following our conversation here is a much simpler patch. I just 
consider that all debug containers will have the same alignment.


  Even if I submit this patch as a whole I will commit into pieces, 
at least one for the pure cleanup parts and one for the debug.cc 
change.


  Among those changes there is:
-   __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+   __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());

  I would appreciate if you could tell me what was happening with 
the initial expression. I don't understand why it is compiling. I 
even tried the same in debug.cc and started having compilation errors.


It creates a temporary __scoped_lock, which immediately goes out of
scope and unlocks the mutex again. This should be fixed on the gcc-5
and gcc-6 branches too.


I'm committing this to the gcc-5 and gcc-6 branches.


I still had plan to do so but if you have those branches under your 
hands I appreciate.


Thanks,

François

Re: [PATCH RESEND 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]

[Denys Vlasenko]

On 09/27/2016 01:53 PM, Bernd Schmidt wrote:

On 09/26/2016 09:08 PM, Denys Vlasenko wrote:

+@gccoptlist{-faggressive-loop-optimizations @gol
+-falign-functions[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-jumps[=@var{n}[,@var{m}]] @gol
+-falign-labels[=@var{n}[,@var{m}]] -falign-loops[=@var{n}[,@var{m}]] @gol



 @itemx -falign-functions=@var{n}
+@itemx -falign-functions=@var{n},@var{m}
 @opindex falign-functions
+If @var{m} is not specified, it defaults to @var{n}.


There are inconsistencies here about how many arguments these take.
 There's no documentation of what it would mean to have more than two.
 The first paragraph seems to imply it's only allowed for -falign-functions,
 but the same implementation is used for all three.


Noted. I will improve this part.


+#if defined (__i386__) || defined (__x86_64__)
+  /* Before -falign-foo=N,M,N2,M2 was introduced, x86 had a tweak.
+ -falign-functions=N with N > 8 was adding secondary alignment.
+ -falign-functions=10 was emitting this before every function:
+.p2align 4,,9
+.p2align 3
+ Now this behavior (and more) can be explicitly requested:
+ -falign-functions=16,10,8
+ Retain old behavior if N2 is missing: */
+  else if (a[0].log > 3)
+a[1].log = 3;
+#endif


Can't have such #ifdef blocks in generic code. To start with, this
changesbehaviour based on the host, when you want it to change
 depending on the target. If there's no way to detect such a
 situation from the x86 backend, such as in the option_override
 function, then you'll need a hook.


Ok, will rework this part.


IIUC the intention for the whole patch is that behaviour is unchanged
 by default, but there are additional options for users to choose?


Exactly.


Since it seems this is mostly for x86, maybe Uros should have a say
 in whether this patch is a good idea or not.


Not really. I imagine the same considerations apply to any CPU:
cachelines are getting wider irrespective of architecture.

Re: PATCH to add more FALLTHRU markers

[Jakub Jelinek]

On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote:
> Not sure if I read this code correctly, but if we fall through from
> V32HImode, and we have TARGET_SSE2 set, we execute this code:
> 
>   tmp = "p";
>   ssesuffix = TARGET_AVX512VL ? "q" : "";
> 
> And not gcc_unreachable (), as is probably intended.

It really doesn't matter.
The instruction uses
(define_mode_iterator VI12_AVX_AVX512F
  [ (V64QI "TARGET_AVX512F") (V32QI "TARGET_AVX") V16QI
(V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX") V8HI])
iterator (and, after all, ix86_hard_regno_mode_ok should ensure the same),
which means V64QI/V32HI will only show up for TARGET_AVX512F, V32QI/V16HI
only for TARGET_AVX (which implies TARGET_SSE2), and the slightly
nonsensical
gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
before the switch (the  || TARGET_AVX512VL is pointless, because
TARGET_AVX512VL implies TARGET_SSE2 as well as TARGET_AVX2).
So, I'd go perhaps for (untested) following patch, first diff -up, followed
by diff -upb:

Jakub
--- gcc/config/i386/sse.md  2016-08-30 08:42:09.169067639 +0200
+++ gcc/config/i386/sse.md  2016-09-27 21:56:29.093582896 +0200
@@ -11393,28 +11393,27 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
-  gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_AVX2);
+  /* FALLTHRU */
 case MODE_TI:
-  gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_SSE2);
   switch (mode)
-  {
-case V16SImode:
-case V8DImode:
-  if (TARGET_AVX512F)
-  {
-tmp = "p";
-break;
-  }
-case V8SImode:
-case V4DImode:
-case V4SImode:
-case V2DImode:
-  tmp = TARGET_AVX512VL ? "p" : "p";
-  break;
-default:
-  gcc_unreachable ();
-  }
+   {
+   case V16SImode:
+   case V8DImode:
+ tmp = "p";
+ break;
+   case V8SImode:
+   case V4DImode:
+   case V4SImode:
+   case V2DImode:
+ tmp = TARGET_AVX512VL ? "p" : "p";
+ break;
+   default:
+ gcc_unreachable ();
+   }
   break;
 
case MODE_V8SF:
@@ -11489,45 +11488,41 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
-  gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_AVX2);
+  /* FALLTHRU */
 case MODE_TI:
-  gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_SSE2);
   switch (mode)
-{
-case V64QImode:
-case V32HImode:
-  if (TARGET_AVX512F)
-  {
-tmp = "p";
-ssesuffix = "q";
-break;
-  }
-case V32QImode:
-case V16HImode:
-case V16QImode:
-case V8HImode:
-  if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
-  {
-tmp = "p";
-ssesuffix = TARGET_AVX512VL ? "q" : "";
-break;
-  }
-default:
-  gcc_unreachable ();
-  }
+   {
+   case V64QImode:
+   case V32HImode:
+ tmp = "p";
+ ssesuffix = "q";
+ break;
+   case V32QImode:
+   case V16HImode:
+   case V16QImode:
+   case V8HImode:
+ tmp = "p";
+ ssesuffix = TARGET_AVX512VL ? "q" : "";
+ break;
+   default:
+ gcc_unreachable ();
+   }
   break;
 
case MODE_V8SF:
-  gcc_assert (TARGET_AVX);
+ gcc_assert (TARGET_AVX);
case MODE_V4SF:
-  gcc_assert (TARGET_SSE);
-  tmp = "ps";
-  ssesuffix = "";
-  break;
+ gcc_assert (TARGET_SSE);
+ tmp = "ps";
+ ssesuffix = "";
+ break;
 
default:
-  gcc_unreachable ();
+ gcc_unreachable ();
}
 
   switch (which_alternative)
--- gcc/config/i386/sse.md  2016-08-30 08:42:09.169067639 +0200
+++ gcc/config/i386/sse.md  2016-09-27 21:56:29.093582896 +0200
@@ -11393,19 +11393,18 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
-  gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_AVX2);
+  /* FALLTHRU */
 case MODE_TI:
-  gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_SSE2);
   switch (mode)
   {
 case V16SImode:
 case V8DImode:
-  if (TARGET_AVX512F)
-  {
 tmp = "p";
 break;
-  }
 case V8SImode:
 case V4DImode:
 case V4SImode:
@@ -11489,30 +11488,26 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
-  gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_AVX2);
+  /* FALLTHRU */
 case MODE_TI:
-  gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
+  gcc_assert (TARGET_SSE2);
   switch (mode)
 

Re: Change license of filenames.h to LGPL

[DJ Delorie]

Ozkan Sezer  writes:
> I am not using filename_cmp.c, nor do I include hashtab.h.  The version
> I took was an old one with macros only and I added some more macros and
> inlines to it. (I replied to these, but I forgot including the CC list,
> here: http://gcc.gnu.org/ml/gcc-patches/2016-09/msg02048.html )

I wonder if us relicensing our modified copy would apply to your old
copy.  I mean, are we sure RMS knows you're also relicensing an old
copy, and that the current copy is being relicensed only to avoid future
issues.  If we're only doing it to document the decision, the fact that
hashtab.h and filename_cmp.c are still GPL mostly negates the
effectiveness of our change anyway.

(i.e. it seems like you can get what you need whether we relicense ours
or not, and relicensing ours doesn't have much actual effect).

(again, not opposed to the change, just clarifying everything)

Re: [C++ PATCH] P0018R3, C++17 lambda capture of *this by value as [=,*this]

[Jason Merrill]

OK, thanks.

On Tue, Sep 27, 2016 at 3:22 PM, Jakub Jelinek  wrote:
> On Tue, Sep 27, 2016 at 03:08:07PM -0400, Jason Merrill wrote:
>> On Tue, Sep 27, 2016 at 2:27 PM, Jakub Jelinek  wrote:
>> > This patch implements P0018R3.  Bootstrapped/regtested on x86_64-linux and
>> > i686-linux, ok for trunk?
>>
>> OK, thanks.
>
> Apparently I forgot to add feature macro for this.
> Tested on x86_64-linux, ok for trunk?
>
> 2016-09-27  Jakub Jelinek  
>
> * c-cppbuiltin.c (c_cpp_builtins): Define __cpp_capture_star_this for
> -std=c++1z.
>
> * g++.dg/cpp1z/feat-cxx1z.C: Add __cpp_capture_star_this test.
>
> --- gcc/c-family/c-cppbuiltin.c.jj  2016-09-21 17:12:59.0 +0200
> +++ gcc/c-family/c-cppbuiltin.c 2016-09-27 21:15:39.518454348 +0200
> @@ -933,6 +933,7 @@ c_cpp_builtins (cpp_reader *pfile)
>   cpp_define (pfile, "__cpp_range_based_for=201603");
>   cpp_define (pfile, "__cpp_constexpr=201603");
>   cpp_define (pfile, "__cpp_if_constexpr=201606");
> + cpp_define (pfile, "__cpp_capture_star_this=201603");
> }
>if (flag_concepts)
> /* Use a value smaller than the 201507 specified in
> --- gcc/testsuite/g++.dg/cpp1z/feat-cxx1z.C.jj  2016-09-27 09:46:09.0 
> +0200
> +++ gcc/testsuite/g++.dg/cpp1z/feat-cxx1z.C 2016-09-27 21:16:50.447580459 
> +0200
> @@ -356,6 +356,12 @@
>  #  error "__cpp_aligned_new != 201606"
>  #endif
>
> +#ifndef __cpp_capture_star_this
> +#  error "__cpp_capture_star_this"
> +#elif __cpp_capture_star_this != 201603
> +#  error "__cpp_capture_star_this != 201603"
> +#endif
> +
>  #ifdef __has_cpp_attribute
>
>  #  if ! __has_cpp_attribute(maybe_unused)
>
>
> Jakub

Re: Change license of filenames.h to LGPL

[Ozkan Sezer]

On 9/27/16, Eli Zaretskii  wrote:
>> From: DJ Delorie 
>> Cc: f...@deneb.enyo.de, gcc-patches@gcc.gnu.org, seze...@gmail.com
>> Date: Tue, 27 Sep 2016 15:23:46 -0400
>>
>> Eli Zaretskii  writes:
>> >> But then the implementation would need relicensing as well, wouldn't
>> >> it?
>> >
>> > Which implementation? of Ozkan's library?
>>
>> libiberty's filename_cmp.c is GPL and implements two of the functions in
>> filenames.h; if those are why he's using it, then it's still GPL unless
>> filename_cmp.c is changed also.
>
> I'm guessing he only wants the macros and will delete the rest.  (The
> original file as written by me years ago had nothing but those few
> macros.)  But I will leave it to Ozkan to give the definitive answer.
>

I am not using filename_cmp.c, nor do I include hashtab.h.  The version
I took was an old one with macros only and I added some more macros and
inlines to it. (I replied to these, but I forgot including the CC list,
here: http://gcc.gnu.org/ml/gcc-patches/2016-09/msg02048.html )

--
O.S.

Re: Change license of filenames.h to LGPL

[Eli Zaretskii]

> From: DJ Delorie 
> Cc: f...@deneb.enyo.de, gcc-patches@gcc.gnu.org, seze...@gmail.com
> Date: Tue, 27 Sep 2016 15:23:46 -0400
> 
> Eli Zaretskii  writes:
> >> But then the implementation would need relicensing as well, wouldn't
> >> it?
> >
> > Which implementation? of Ozkan's library?
> 
> libiberty's filename_cmp.c is GPL and implements two of the functions in
> filenames.h; if those are why he's using it, then it's still GPL unless
> filename_cmp.c is changed also.

I'm guessing he only wants the macros and will delete the rest.  (The
original file as written by me years ago had nothing but those few
macros.)  But I will leave it to Ozkan to give the definitive answer.

Re: PATCH to add more FALLTHRU markers

[Marek Polacek]

On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote:
> * Marek Polacek:
> 
> > On Tue, Sep 27, 2016 at 08:58:59PM +0200, Florian Weimer wrote:
> >> * Marek Polacek:
> >> 
> >> > @@ -11498,21 +11508,23 @@
> >> >  case V64QImode:
> >> >  case V32HImode:
> >> >if (TARGET_AVX512F)
> >> > -  {
> >> > -tmp = "p";
> >> > -ssesuffix = "q";
> >> > -break;
> >> > -  }
> >> > +{
> >> > +  tmp = "p";
> >> > +  ssesuffix = "q";
> >> > +  break;
> >> > +}
> >> > +  /* FALLTHRU */
> >> >  case V32QImode:
> >> >  case V16HImode:
> >> >  case V16QImode:
> >> >  case V8HImode:
> >> >if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
> >> > -  {
> >> > -tmp = "p";
> >> > -ssesuffix = TARGET_AVX512VL ? "q" : "";
> >> > -break;
> >> > -  }
> >> > +{
> >> > +  tmp = "p";
> >> > +  ssesuffix = TARGET_AVX512VL ? "q" : "";
> >> > +  break;
> >> > +}
> >> > +  /* FALLTHRU */
> >> >  default:
> >> >gcc_unreachable ();
> >> >}
> >> 
> >> Why isn't this a bug?  Wouldn't we want to reach gcc_unreachable ()
> >> if, for example !TARGET_AVX512F and TARGET_SSE2?
> >
> > ?? In that case the FALLTHRU should be there if it's intentional that
> > we may fall through to default.  Otherwise not sure what you mean.
> 
> Not sure if I read this code correctly, but if we fall through from
> V32HImode, and we have TARGET_SSE2 set, we execute this code:
> 
>   tmp = "p";
>   ssesuffix = TARGET_AVX512VL ? "q" : "";
> 
> And not gcc_unreachable (), as is probably intended.

Kyrill, can you please decide?  Thanks.

Marek

[PATCH] fix profiledbootstrap with -Werror=format-length (77753)

[Martin Sebor]

The attached one line patch increases a local buffer size to
avoid an apparently justified (though in reality likely a false
positive) -Wformat-length warning in varasm.c.  The warning has
been reported to break profiledbootstrap on powerp64le (though
not ordinary bootstrap).

An arguably better solution would be to explicitly constrain
the range of the integer variable to that of valid priority values.
Unfortunately, as pointed out in bug 77721, due to the limitation
of the current VRP implementation, this isn't always reliable.

Martin
PR bootstrap/77753 - [7 Regression] broken profiledbootstrap due to
	-Werror=format-length in varasm.c:1573

gcc/ChangeLog:
2016-09-27  Martin Sebor  

	PR bootstrap/77753
	* varasm.c (assemble_addr_to_section): Increase local buffer size.

Index: gcc/varasm.c
===
--- gcc/varasm.c	(revision 240556)
+++ gcc/varasm.c	(working copy)
@@ -1556,7 +1556,9 @@ assemble_addr_to_section (rtx symbol, se
 section *
 get_cdtor_priority_section (int priority, bool constructor_p)
 {
-  char buf[16];
+  /* Buffer conservatively large enough for the full range of a 32-bit
+ int plus the text below.  */
+  char buf[18];
 
   /* ??? This only works reliably with the GNU linker.  */
   sprintf (buf, "%s.%.5u",

Re: PATCH to add more FALLTHRU markers

[Florian Weimer]

* Marek Polacek:

> On Tue, Sep 27, 2016 at 08:58:59PM +0200, Florian Weimer wrote:
>> * Marek Polacek:
>> 
>> > @@ -11498,21 +11508,23 @@
>> >  case V64QImode:
>> >  case V32HImode:
>> >if (TARGET_AVX512F)
>> > -  {
>> > -tmp = "p";
>> > -ssesuffix = "q";
>> > -break;
>> > -  }
>> > +  {
>> > +tmp = "p";
>> > +ssesuffix = "q";
>> > +break;
>> > +  }
>> > +/* FALLTHRU */
>> >  case V32QImode:
>> >  case V16HImode:
>> >  case V16QImode:
>> >  case V8HImode:
>> >if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
>> > -  {
>> > -tmp = "p";
>> > -ssesuffix = TARGET_AVX512VL ? "q" : "";
>> > -break;
>> > -  }
>> > +  {
>> > +tmp = "p";
>> > +ssesuffix = TARGET_AVX512VL ? "q" : "";
>> > +break;
>> > +  }
>> > +/* FALLTHRU */
>> >  default:
>> >gcc_unreachable ();
>> >}
>> 
>> Why isn't this a bug?  Wouldn't we want to reach gcc_unreachable ()
>> if, for example !TARGET_AVX512F and TARGET_SSE2?
>
> ?? In that case the FALLTHRU should be there if it's intentional that
> we may fall through to default.  Otherwise not sure what you mean.

Not sure if I read this code correctly, but if we fall through from
V32HImode, and we have TARGET_SSE2 set, we execute this code:

  tmp = "p";
  ssesuffix = TARGET_AVX512VL ? "q" : "";

And not gcc_unreachable (), as is probably intended.

Re: Change license of filenames.h to LGPL

[DJ Delorie]

Eli Zaretskii  writes:
>> But then the implementation would need relicensing as well, wouldn't
>> it?
>
> Which implementation? of Ozkan's library?

libiberty's filename_cmp.c is GPL and implements two of the functions in
filenames.h; if those are why he's using it, then it's still GPL unless
filename_cmp.c is changed also.

> Why would it need to
> change?  It's perfectly okay to link GPL code with LGPL code, we do
> this all the time with libgcc, no?  Or am I missing something?

libgcc has an exception that covers most of those cases; be careful when
comparing those to your (his) use case.

Re: [C++ PATCH] P0018R3, C++17 lambda capture of *this by value as [=,*this]

[Jakub Jelinek]

On Tue, Sep 27, 2016 at 03:08:07PM -0400, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 2:27 PM, Jakub Jelinek  wrote:
> > This patch implements P0018R3.  Bootstrapped/regtested on x86_64-linux and
> > i686-linux, ok for trunk?
> 
> OK, thanks.

Apparently I forgot to add feature macro for this.
Tested on x86_64-linux, ok for trunk?

2016-09-27  Jakub Jelinek  

* c-cppbuiltin.c (c_cpp_builtins): Define __cpp_capture_star_this for
-std=c++1z.

* g++.dg/cpp1z/feat-cxx1z.C: Add __cpp_capture_star_this test.

--- gcc/c-family/c-cppbuiltin.c.jj  2016-09-21 17:12:59.0 +0200
+++ gcc/c-family/c-cppbuiltin.c 2016-09-27 21:15:39.518454348 +0200
@@ -933,6 +933,7 @@ c_cpp_builtins (cpp_reader *pfile)
  cpp_define (pfile, "__cpp_range_based_for=201603");
  cpp_define (pfile, "__cpp_constexpr=201603");
  cpp_define (pfile, "__cpp_if_constexpr=201606");
+ cpp_define (pfile, "__cpp_capture_star_this=201603");
}
   if (flag_concepts)
/* Use a value smaller than the 201507 specified in
--- gcc/testsuite/g++.dg/cpp1z/feat-cxx1z.C.jj  2016-09-27 09:46:09.0 
+0200
+++ gcc/testsuite/g++.dg/cpp1z/feat-cxx1z.C 2016-09-27 21:16:50.447580459 
+0200
@@ -356,6 +356,12 @@
 #  error "__cpp_aligned_new != 201606"
 #endif
 
+#ifndef __cpp_capture_star_this
+#  error "__cpp_capture_star_this"
+#elif __cpp_capture_star_this != 201603
+#  error "__cpp_capture_star_this != 201603"
+#endif
+
 #ifdef __has_cpp_attribute
 
 #  if ! __has_cpp_attribute(maybe_unused)


Jakub

Re: Change license of filenames.h to LGPL

[Eli Zaretskii]

> From: DJ Delorie 
> Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com
> Date: Tue, 27 Sep 2016 15:00:46 -0400
> 
> 
> Eli Zaretskii  writes:
> > Because Ozkan wants to use it in an otherwise LGPL package.
> 
> Ok, but that doesn't say why it's different.  That reason could apply to
> any header in there.  Do we need to convert all headers there to LGPL?

I understand that Ozkan only needs that one.  Ozkan?

> Is this "otherwise LGPL package" in one of our repos, or elsewhere?  Is
> he aware that filenames.h includes hashtab.h, which is GPL?
> 
> (I'm not opposed to the change, just trying to make sure I understand
> it's scope)

I'll let Ozkan answer these questions.  But if someone wants to know
which libraries need that, one of them is this:

  http://libtimidity.sourceforge.net/

Re: Change license of filenames.h to LGPL

[Eli Zaretskii]

> From: Florian Weimer 
> Cc: DJ Delorie ,  gcc-patches@gcc.gnu.org,  seze...@gmail.com
> Date: Tue, 27 Sep 2016 20:54:59 +0200
> 
> >> Most of the files in include/ are GPL, not LGPL.  Why is this one
> >> different?
> >
> > Because Ozkan wants to use it in an otherwise LGPL package.
> 
> But then the implementation would need relicensing as well, wouldn't
> it?

Which implementation? of Ozkan's library?  Why would it need to
change?  It's perfectly okay to link GPL code with LGPL code, we do
this all the time with libgcc, no?  Or am I missing something?

> Having both under different licenses is just confusing.

AFAIK, that ship sailed a long time ago, in several projects,
including Binutils and GDB.

Re: PATCH to add more FALLTHRU markers

[Marek Polacek]

On Tue, Sep 27, 2016 at 08:58:59PM +0200, Florian Weimer wrote:
> * Marek Polacek:
> 
> > @@ -11498,21 +11508,23 @@
> >  case V64QImode:
> >  case V32HImode:
> >if (TARGET_AVX512F)
> > -  {
> > -tmp = "p";
> > -ssesuffix = "q";
> > -break;
> > -  }
> > +   {
> > + tmp = "p";
> > + ssesuffix = "q";
> > + break;
> > +   }
> > + /* FALLTHRU */
> >  case V32QImode:
> >  case V16HImode:
> >  case V16QImode:
> >  case V8HImode:
> >if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
> > -  {
> > -tmp = "p";
> > -ssesuffix = TARGET_AVX512VL ? "q" : "";
> > -break;
> > -  }
> > +   {
> > + tmp = "p";
> > + ssesuffix = TARGET_AVX512VL ? "q" : "";
> > + break;
> > +   }
> > + /* FALLTHRU */
> >  default:
> >gcc_unreachable ();
> >}
> 
> Why isn't this a bug?  Wouldn't we want to reach gcc_unreachable ()
> if, for example !TARGET_AVX512F and TARGET_SSE2?

?? In that case the FALLTHRU should be there if it's intentional that
we may fall through to default.  Otherwise not sure what you mean.

Marek

Re: Change license of filenames.h to LGPL

[Ozkan Sezer]

On 9/27/16, DJ Delorie  wrote:
>
> Eli Zaretskii  writes:
>> Because Ozkan wants to use it in an otherwise LGPL package.
>
> Ok, but that doesn't say why it's different.  That reason could apply to
> any header in there.  Do we need to convert all headers there to LGPL?
> Is this "otherwise LGPL package" in one of our repos, or elsewhere?  Is
> he aware that filenames.h includes hashtab.h, which is GPL?
>
> (I'm not opposed to the change, just trying to make sure I understand
> it's scope)
>

FYI: What I originally wanted was an authorization _for me_ to use
filenames.h in LGPL projects with LGPL license notice; the version
I use is modified (not refer to any external code other than libc,
i.e. only macros and inlines) and doesn't include hashtab.h either;
therefore I believe that my request is fulfilled and is not subject
to the concerns raised by you guys.

As I understand it, Eli wanted to reflect it in the mainline copy,
which is a matter to be decided by gcc and binutils maintainers.

--
O.S.

Re: [C++ PATCH] P0018R3, C++17 lambda capture of *this by value as [=,*this]

[Jason Merrill]

On Tue, Sep 27, 2016 at 2:27 PM, Jakub Jelinek  wrote:
> This patch implements P0018R3.  Bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk?

OK, thanks.

> BTW, not sure if/what we should do about say [this...] {} or [*this...] {},
> the grammar seems to allow it (the former already in earlier standards),
> but this or *this is never a parameter pack, so probably should be rejected,
> while we don't even try to parse it.  In any case that looks like
> preexisting pedantry.

Yes, it should be rejected.

Jason

Re: Change license of filenames.h to LGPL

[DJ Delorie]

Eli Zaretskii  writes:
> Because Ozkan wants to use it in an otherwise LGPL package.

Ok, but that doesn't say why it's different.  That reason could apply to
any header in there.  Do we need to convert all headers there to LGPL?
Is this "otherwise LGPL package" in one of our repos, or elsewhere?  Is
he aware that filenames.h includes hashtab.h, which is GPL?

(I'm not opposed to the change, just trying to make sure I understand
it's scope)

Re: PATCH to add more FALLTHRU markers

[Florian Weimer]

* Marek Polacek:

> @@ -11498,21 +11508,23 @@
>  case V64QImode:
>  case V32HImode:
>if (TARGET_AVX512F)
> -  {
> -tmp = "p";
> -ssesuffix = "q";
> -break;
> -  }
> + {
> +   tmp = "p";
> +   ssesuffix = "q";
> +   break;
> + }
> +   /* FALLTHRU */
>  case V32QImode:
>  case V16HImode:
>  case V16QImode:
>  case V8HImode:
>if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
> -  {
> -tmp = "p";
> -ssesuffix = TARGET_AVX512VL ? "q" : "";
> -break;
> -  }
> + {
> +   tmp = "p";
> +   ssesuffix = TARGET_AVX512VL ? "q" : "";
> +   break;
> + }
> +   /* FALLTHRU */
>  default:
>gcc_unreachable ();
>}

Why isn't this a bug?  Wouldn't we want to reach gcc_unreachable ()
if, for example !TARGET_AVX512F and TARGET_SSE2?

Re: PATCH to add more FALLTHRU markers

[Richard Sandiford]

Marek Polacek  writes:
> Currently Makefile.in contains -Wno-error for several of the insn-* files, but
> after further investigation I think with this patch we won't need them 
> anymore.
> I'm not removing it until I bootstrap gcc on more arches, though.  Meanwhile,
> this patch at least makes the code more robust.
>
> Richard S., can you look at the genattrtab.c bit?  Thanks,

genattrab.c part is OK, thanks.

Richard

Re: Change license of filenames.h to LGPL

[Eli Zaretskii]

> Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com
> From: Jeff Law 
> Date: Tue, 27 Sep 2016 12:36:11 -0600
> 
> On 09/27/2016 11:52 AM, DJ Delorie wrote:
> >
> > Most of the files in include/ are GPL, not LGPL.  Why is this one
> > different?
> Right.  ANd it's not like this file inserts anything of significance 
> into the resulting object code.  I'd really like to see more rationale 
> behind the request for a license change.

See my other message.

In the original request I reported that this change was already
approved by Richard Stallman, who asked these same questions.

Re: Change license of filenames.h to LGPL

[Florian Weimer]

* Eli Zaretskii:

>> From: DJ Delorie 
>> Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com
>> Date: Tue, 27 Sep 2016 13:52:10 -0400
>> 
>> 
>> Most of the files in include/ are GPL, not LGPL.  Why is this one
>> different?
>
> Because Ozkan wants to use it in an otherwise LGPL package.

But then the implementation would need relicensing as well, wouldn't
it?

Having both under different licenses is just confusing.

Re: Change license of filenames.h to LGPL

[Eli Zaretskii]

> From: DJ Delorie 
> Cc: gcc-patches@gcc.gnu.org, seze...@gmail.com
> Date: Tue, 27 Sep 2016 13:52:10 -0400
> 
> 
> Most of the files in include/ are GPL, not LGPL.  Why is this one
> different?

Because Ozkan wants to use it in an otherwise LGPL package.

[PATCH] PR fortran/58991 and fortran/58992

[Steve Kargl]

The attached patch fixes both PR fortran/58991 and fortran/58992.
The issue was that for a CHARACTER expression or variable as a
selector in an ASSOCIATE statement, the type-spec was incomplete.
That is, the actual length of the string was not set.  The patch
rectifies the problem (at least in my simple tests).

This patch also fixes the ICE for PR fortran/58618, but gfortran
then generates a wrong-code.  I don't have time to look into this
PR.  PR fortran/58618 may be considered fairly low hanging fruit
for the *lurkers* on the fortran@ list looking to get involved in
gfortran development.

Regression tested on x86_64-*-freebsd.  OK to commit?

PS:  There are still a number of issues with ASSOCIATE.  Help 
 fixing those issues would be welcomed.

PPS: Overloading resolve_assoc_var() for SELECT TYPE may have
 caused more issues than warranted.

2016-09-27  Steven G. Kargl  

PR fortran/58991
PR fortran/58992
* resolve.c (resolve_assoc_var):  Fix CHARACTER type-spec for a
selector in ASSOCIATE.
(resolve_fl_variable): Skip checks for an ASSOCIATE variable.

2016-09-27  Steven G. Kargl  

PR fortran/58991
PR fortran/58992
* gfortran.dg/associate_22.f90: New test.

-- 
Steve
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 240506)
+++ gcc/fortran/resolve.c	(working copy)
@@ -8304,6 +8304,18 @@ resolve_assoc_var (gfc_symbol* sym, bool
   /* Mark this as an associate variable.  */
   sym->attr.associate_var = 1;
 
+  /* Fix up the type-spec for CHARACTER types.  */
+  if (sym->ts.type == BT_CHARACTER && !sym->attr.select_type_temporary)
+{
+  if (!sym->ts.u.cl)
+	sym->ts.u.cl = target->ts.u.cl;
+
+  if (!sym->ts.u.cl->length)
+	sym->ts.u.cl->length
+	  = gfc_get_int_expr (gfc_default_integer_kind,
+			  NULL, target->value.character.length);
+}
+
   /* If the target is a good class object, so is the associate variable.  */
   if (sym->ts.type == BT_CLASS && gfc_expr_attr (target).class_ok)
 sym->attr.class_ok = 1;
@@ -11577,7 +11589,7 @@ resolve_fl_variable (gfc_symbol *sym, in
   if (!deferred_requirements (sym))
 return false;
 
-  if (sym->ts.type == BT_CHARACTER)
+  if (sym->ts.type == BT_CHARACTER && !sym->attr.associate_var)
 {
   /* Make sure that character string variables with assumed length are
 	 dummy arguments.  */
Index: gcc/testsuite/gfortran.dg/associate_22.f90
===
--- gcc/testsuite/gfortran.dg/associate_22.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/associate_22.f90	(working copy)
@@ -0,0 +1,37 @@
+! { dg-do run }
+program foo
+
+   implicit none
+
+   character(len=4) :: s
+   character(len=10) :: a
+
+   ! This works.
+   s = 'abc'
+   associate(t => s)
+  if (trim(t) /= 'abc') call abort
+   end associate
+
+   ! This failed.
+   associate(u => 'abc')
+  if (trim(u) /= 'abc') call abort
+   end associate
+
+   ! This failed.
+   a = s // 'abc'
+   associate(v => s // 'abc')
+  if (trim(v) /= trim(a)) call abort
+   end associate
+
+   ! This failed.
+   a = trim(s) // 'abc'
+   associate(w => trim(s) // 'abc')
+  if (trim(w) /= trim(a)) call abort
+   end associate
+
+   ! This failed.
+   associate(x => trim('abc'))
+  if (trim(x) /= 'abc') call abort
+   end associate
+
+end program foo

Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c

[Jeff Law]

On 09/27/2016 10:39 AM, James Greenhalgh wrote:

On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:

Hi,

  This patch requires int32plus for
  gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
  failures for a 16 bit int target like the avr. The "%u" format
  specifier tests, for example, use int literals big enough to only fit
  in a long int, and this causes unexpected warnings.

  Comitted to trunk.


This change is obviously incomplete as it does not update the expected
line numbers for warnings generated by this testcase.

Right.

It does make me wonder if these directives could go at the bottom of the 
file so that adding/removing a directive doesn't require updating line 
#s in the file.


jeff

Re: [gomp4] Fix tile parsing

[Nathan Sidwell]

On 09/27/16 14:17, Nathan Sidwell wrote:

On 09/27/16 14:07, Jakub Jelinek wrote:


+case OMP_CLAUSE_TILE:
+  /* These clauses really need a positive integral constant
+ expression, but we don't have a predicate for that
+ (yet).  */
+  needs_ice = true;
+  /* FALLTHRU */


But why do you need this (at least for OMP_CLAUSE_COLLAPSE)?
The OMP_CLAUSE_COLLAPSE (and OMP_CLAUSE_ORDERED if not NULL) argument
is required to be INTEGER_CST already during parsing, it can't be even
template argument etc. (because we need that number already during parsing
to find out how many nested loops to parse specially).
So I don't see how would the last argument to tsubst_expr make any
difference there.


Oh, I'd not realized it couldn't be a template argument.  (the same is not true
of tile parameters).


Er, why is tsubst_expr even trying to feed them to tsubst_expr in that case? 
(it's harmless, but extra work)


nathan

Re: Change license of filenames.h to LGPL

[Jeff Law]

On 09/27/2016 11:52 AM, DJ Delorie wrote:


Most of the files in include/ are GPL, not LGPL.  Why is this one
different?
Right.  ANd it's not like this file inserts anything of significance 
into the resulting object code.  I'd really like to see more rationale 
behind the request for a license change.


jeff

[C++ PATCH] P0018R3, C++17 lambda capture of *this by value as [=,*this]

[Jakub Jelinek]

Hi!

This patch implements P0018R3.  Bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

BTW, not sure if/what we should do about say [this...] {} or [*this...] {},
the grammar seems to allow it (the former already in earlier standards),
but this or *this is never a parameter pack, so probably should be rejected,
while we don't even try to parse it.  In any case that looks like
preexisting pedantry.

2016-09-27  Jakub Jelinek  

Implement P0018R3, C++17 lambda capture of *this by value as [=,*this]
* parser.c (cp_parser_lambda_introducer): Formatting fix.  Pass
true instead of false as by_reference_p to add_capture for 'this'.
Parse '*this' simple-capture.
* lambda.c (build_capture_proxy): Handle '*this' capture by value.
(add_capture): Adjust function comment.  For id == this_identifier,
treat by_reference_p as capturing '*this' by reference, i.e. 'this'
by value, and !by_reference_p as capturing '*this' by value.
(add_default_capture): For implicit 'this' capture, always pass
by_reference_p true rather than false.

* g++.dg/cpp1z/lambda-this1.C: New test.
* g++.dg/cpp1z/lambda-this2.C: New test.

--- gcc/cp/parser.c.jj  2016-09-27 09:45:59.0 +0200
+++ gcc/cp/parser.c 2016-09-27 14:46:18.398184654 +0200
@@ -9899,7 +9899,25 @@ cp_parser_lambda_introducer (cp_parser*
  cp_lexer_consume_token (parser->lexer);
  add_capture (lambda_expr,
   /*id=*/this_identifier,
-  /*initializer=*/finish_this_expr(),
+  /*initializer=*/finish_this_expr (),
+  /*by_reference_p=*/true,
+  explicit_init_p);
+ continue;
+   }
+
+  /* Possibly capture `*this'.  */
+  if (cp_lexer_next_token_is (parser->lexer, CPP_MULT)
+ && cp_lexer_nth_token_is_keyword (parser->lexer, 2, RID_THIS))
+   {
+ location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+ if (cxx_dialect < cxx1z)
+   pedwarn (loc, 0, "%<*this%> capture only available with "
+"-std=c++1z or -std=gnu++1z");
+ cp_lexer_consume_token (parser->lexer);
+ cp_lexer_consume_token (parser->lexer);
+ add_capture (lambda_expr,
+  /*id=*/this_identifier,
+  /*initializer=*/finish_this_expr (),
   /*by_reference_p=*/false,
   explicit_init_p);
  continue;
--- gcc/cp/lambda.c.jj  2016-08-09 09:22:23.0 +0200
+++ gcc/cp/lambda.c 2016-09-27 15:46:23.829401937 +0200
@@ -380,6 +380,13 @@ build_capture_proxy (tree member)
 
   type = lambda_proxy_type (object);
 
+  if (name == this_identifier && !POINTER_TYPE_P (type))
+{
+  type = build_pointer_type (type);
+  type = cp_build_qualified_type (type, TYPE_QUAL_CONST);
+  object = build_fold_addr_expr_with_type (object, type);
+}
+
   if (DECL_VLA_CAPTURE_P (member))
 {
   /* Rebuild the VLA type from the pointer and maxindex.  */
@@ -440,7 +447,8 @@ vla_capture_type (tree array_type)
 
 /* From an ID and INITIALIZER, create a capture (by reference if
BY_REFERENCE_P is true), add it to the capture-list for LAMBDA,
-   and return it.  */
+   and return it.  If ID is `this', BY_REFERENCE_P says whether
+   `*this' is captured by reference.  */
 
 tree
 add_capture (tree lambda, tree id, tree orig_init, bool by_reference_p,
@@ -499,7 +507,14 @@ add_capture (tree lambda, tree id, tree
   type = lambda_capture_field_type (initializer, explicit_init_p);
   if (type == error_mark_node)
return error_mark_node;
-  if (by_reference_p)
+  if (id == this_identifier && !by_reference_p)
+   {
+ gcc_assert (POINTER_TYPE_P (type));
+ type = TREE_TYPE (type);
+ initializer = cp_build_indirect_ref (initializer, RO_NULL,
+  tf_warning_or_error);
+   }
+  if (id != this_identifier && by_reference_p)
{
  type = build_reference_type (type);
  if (!dependent_type_p (type) && !lvalue_p (initializer))
@@ -628,8 +643,8 @@ add_default_capture (tree lambda_stack,
 id,
 initializer,
 /*by_reference_p=*/
-   (!this_capture_p
-&& (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
+   (this_capture_p
+|| (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda)
 == CPLD_REFERENCE)),
/*explicit_init_p=*/false);
   initializer = convert_from_reference (var);
--- gcc/testsuite/g++.dg/cpp1z/lambda-this1.C.jj2016-09-27 
16:44:57.302404906 +0200
+++ gcc/testsuite/g++.dg/cpp1z/lambda-this1.C   2016-09-27 17:54:38.595919907 

Re: [ARM] Fix invalid instructions generated for data movement.

[Christophe Lyon]

Hi,


On 27 September 2016 at 15:55, Kyrill Tkachov
 wrote:
> Hi Matthew,
>
>
> On 27/09/16 14:21, Matthew Wahab wrote:
>>
>> Recently added support for ARMv8.2-A
>> (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html) included a
>> number of changes to improve data movement, particularly for HI and HF
>> mode values. These included the use of the Thumb-2 instruction MOVW and
>> of the new VMOV.F16 instruction. There are problems with both: the use
>> of MOVW isn't properly guarded so that it can be generated for targets
>> that don't support it and the VMOV.F16 instruction is wrongly marked as
>> being predicable.
>>
>> This patch adds guards to the use of the MOVW so that it is only
>> generated when the target supports Thumb-2 and fixes the predication on
>> the VMOV.F16 instruction.
>>
>> Tested for arm-none-linux-gnueabihf with native bootstrap and make check
>> on ARMv8-A and for arm-none-eabi with cross compiled check-gcc on an
>> ARMv8.2-A emulator.
>>
>> There is one failure on arm-none-linux-gnueabihf,
>> gcc.dg/guality/pr36728-1.c which is due to MOVW not being generated,
>> even for ARMv7-A. The generated code is otherwise correct. I think I
>> understand why MOVW isn't being emitted but it'll take time to test
>> properly.
>>
>> Since this patch is to fix a broken build is it OK to commit it and to fix
>> the poor code-gen in a follow-up patch?
>> Matthew
>>
>> gcc/
>> 2016-09-27  Matthew Wahab  
>>
>> * config/arm/arm.md (*arm_movsi_insn): Add "arch" attribute.
>> * config/arm/vfp.md (*arm_movhi_vfp): Likewise.
>> (*thumb2_movhi_vfp): Likewise.
>> (*arm_movhi_fp16): Remove predication operand from VMOV.F16
>> template.  Expand predicable attribute to mark VMOV.F16 as not
>> predicable.  Add "arch" attribute.
>> (*thumb2_movhi_fp16): Likewise.
>> (*arm_movsi_vfp): Break a long line.  Add "arch" attribute.
>> (*thumb2_movsi_vfp): Add "arch" attribute.
>
>
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 411754f..999292b 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -6065,6 +6065,7 @@
>[(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1")
> (set_attr "predicable" "yes")
> (set_attr "pool_range" "*,*,*,*,4096,*")
> +   (set_attr "arch" "*,*,*,t2,*,*")
> (set_attr "neg_pool_range" "*,*,*,*,4084,*")]
>  )
>
> The "t2" attribute means that we're currently compiling for Thumb2. What you
> want is to check that the architecture
> we're compiling for supports Thumb2, so in this case you want the v6t2
> value.
> In the interest of fixing the build I'll approve this patch as is since it's
> still correct (just not as general as it could be),
> but it'd be good to have a follow-up patch to fix this.
>gcc.target/arm/constant-pool.c execution test
> Thanks for fixing this,
> Kyrill
>
>

This patch fixes the regression I reported on
gcc.target/arm/constant-pool.c
gfortran.fortran-torture/execute/nan_inf_fmt.f90

as well as the compiler build failure I reported
against patch 6/17 of your series.

Thanks!

Re: [gomp4] Fix tile parsing

[Nathan Sidwell]

On 09/27/16 14:07, Jakub Jelinek wrote:


+   case OMP_CLAUSE_TILE:
+ /* These clauses really need a positive integral constant
+expression, but we don't have a predicate for that
+(yet).  */
+ needs_ice = true;
+ /* FALLTHRU */


But why do you need this (at least for OMP_CLAUSE_COLLAPSE)?
The OMP_CLAUSE_COLLAPSE (and OMP_CLAUSE_ORDERED if not NULL) argument
is required to be INTEGER_CST already during parsing, it can't be even
template argument etc. (because we need that number already during parsing
to find out how many nested loops to parse specially).
So I don't see how would the last argument to tsubst_expr make any
difference there.


Oh, I'd not realized it couldn't be a template argument.  (the same is not true 
of tile parameters).


nathan

Re: [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)

[Jakub Jelinek]

On Tue, Sep 27, 2016 at 10:56:51AM -0400, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 3:57 AM, Jakub Jelinek  wrote:
> > @@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
> > +  if (TREE_CODE (*p) == STATEMENT_LIST)
> >  {
> > +  tree_stmt_iterator i = tsi_last (*p);
> >tsi_link_after (, t, TSI_NEW_STMT);
> >  }
> > +  else
> > +{
> > +  tree list = NULL_TREE;
> > +  append_to_statement_list_force (*p, );
> > +  append_to_statement_list (t, );
> > +  *p = list;
> > +}
> 
> Can't you replace all of this with
> 
> append_to_statement_list (t, p);
> 
> ?
> 
> OK either way.

That works too and is cleaner, thanks.  Bootstrapped/regtested on
x86_64-linux and i686-linux, committed to trunk.

2016-09-27  Jakub Jelinek  

PR c++/77722
* cp-gimplify.c (cp_ubsan_maybe_instrument_return): Instrument also
functions that have just a STATEMENT_LIST instead of BIND_EXPR, or
BIND_EXPR with some statement rather than STATEMENT_LIST as body.

* g++.dg/ubsan/return-4.C: New test.
* g++.dg/ubsan/return-5.C: New test.
* g++.dg/ubsan/return-6.C: New test.

--- gcc/cp/cp-gimplify.c.jj 2016-09-26 12:06:36.060910355 +0200
+++ gcc/cp/cp-gimplify.c2016-09-27 18:06:50.78068 +0200
@@ -1570,14 +1570,11 @@ cp_ubsan_maybe_instrument_return (tree f
 }
   if (t == NULL_TREE)
 return;
-  t = DECL_SAVED_TREE (fndecl);
-  if (TREE_CODE (t) == BIND_EXPR
-  && TREE_CODE (BIND_EXPR_BODY (t)) == STATEMENT_LIST)
-{
-  tree_stmt_iterator i = tsi_last (BIND_EXPR_BODY (t));
-  t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
-  tsi_link_after (, t, TSI_NEW_STMT);
-}
+  tree *p = _SAVED_TREE (fndecl);
+  if (TREE_CODE (*p) == BIND_EXPR)
+p = _EXPR_BODY (*p);
+  t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+  append_to_statement_list (t, p);
 }
 
 void
--- gcc/testsuite/g++.dg/ubsan/return-5.C.jj2016-09-27 18:05:37.138615332 
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-5.C   2016-09-27 18:05:37.138615332 
+0200
@@ -0,0 +1,19 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function 
without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-4.C.jj2016-09-27 18:05:37.137615345 
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-4.C   2016-09-27 18:05:37.137615345 
+0200
@@ -0,0 +1,18 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function 
without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-6.C.jj2016-09-27 18:05:37.138615332 
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-6.C   2016-09-27 18:05:37.138615332 
+0200
@@ -0,0 +1,20 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+  int a = 5;
+  int b = 5;
+}
+
+int
+main ()
+{
+  foo ();
+  return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function 
without returning a value" }

Jakub

Re: [gomp4] Fix tile parsing

[Jakub Jelinek]

On Fri, Sep 23, 2016 at 07:14:00AM -0400, Nathan Sidwell wrote:
> --- c/c-parser.c  (revision 240420)
> +++ c/c-parser.c  (working copy)
> @@ -10882,6 +10882,7 @@ c_parser_omp_clause_collapse (c_parser *
>location_t loc;
>  
>check_no_duplicate_clause (list, OMP_CLAUSE_COLLAPSE, "collapse");
> +  check_no_duplicate_clause (list, OMP_CLAUSE_TILE, "tile");

Not sure I'm very happy about the addition of OpenACC specific tests in
clause parsing that is used also for OpenMP and Cilk+, but I guess I can
live with that.

> --- cp/pt.c   (revision 240420)
> +++ cp/pt.c   (working copy)
> @@ -14543,6 +14543,7 @@ tsubst_omp_clauses (tree clauses, enum c
>nc = copy_node (oc);
>OMP_CLAUSE_CHAIN (nc) = new_clauses;
>new_clauses = nc;
> +  bool needs_ice = false;
>  
>switch (OMP_CLAUSE_CODE (nc))
>   {
> @@ -14572,10 +14573,16 @@ tsubst_omp_clauses (tree clauses, enum c
>   = tsubst_omp_clause_decl (OMP_CLAUSE_DECL (oc), args, complain,
> in_decl);
> break;
> + case OMP_CLAUSE_COLLAPSE:
> + case OMP_CLAUSE_TILE:
> +   /* These clauses really need a positive integral constant
> +  expression, but we don't have a predicate for that
> +  (yet).  */
> +   needs_ice = true;
> +   /* FALLTHRU */

But why do you need this (at least for OMP_CLAUSE_COLLAPSE)?
The OMP_CLAUSE_COLLAPSE (and OMP_CLAUSE_ORDERED if not NULL) argument
is required to be INTEGER_CST already during parsing, it can't be even
template argument etc. (because we need that number already during parsing
to find out how many nested loops to parse specially).
So I don't see how would the last argument to tsubst_expr make any
difference there.

If OMP_CLAUSE_TILE needs something different, then perhaps it should be
handled just on its own, and just call tsubst_expr on the argument with the
other options, the needs_ice is just confusing.

>   case OMP_CLAUSE_IF:
>   case OMP_CLAUSE_NUM_THREADS:
>   case OMP_CLAUSE_SCHEDULE:
> - case OMP_CLAUSE_COLLAPSE:
>   case OMP_CLAUSE_FINAL:
>   case OMP_CLAUSE_DEVICE:
>   case OMP_CLAUSE_DIST_SCHEDULE:
> @@ -14596,8 +14603,8 @@ tsubst_omp_clauses (tree clauses, enum c
>   case OMP_CLAUSE_ASYNC:
>   case OMP_CLAUSE_WAIT:
> OMP_CLAUSE_OPERAND (nc, 0)
> - = tsubst_expr (OMP_CLAUSE_OPERAND (oc, 0), args, complain, 
> -in_decl, /*integral_constant_expression_p=*/false);
> + = tsubst_expr (OMP_CLAUSE_OPERAND (oc, 0), args, complain, in_decl,
> +/*integral_constant_expression_p=*/needs_ice);
> break;
>   case OMP_CLAUSE_REDUCTION:
> if (OMP_CLAUSE_REDUCTION_PLACEHOLDER (oc))

Jakub

[gomp4] loop abstraction fns

[Nathan Sidwell]

In implementing support for tile, the loop structure becomes a little more 
complex, such that oacc_loop_xform_loop's simple scanning approach fails.  I 
realized, that wasn't necessary, if rather than count the abstraction functions 
when discovering the loops, we record the gimple calls themselves. Then we can 
simply iterate over that vector to fill in the partitioning information.  That's 
a better solution on its own merits.


applied to gomp4

nathan
2016-09-27  Nathan Sidwell  

	* omp-low.c (struct oacc_loop): Change 'ifns' to vector of call
	stmts.
	(new_oacc_loop_raw, finish_oacc_loop): Adjust.
	(oacc_loop_discover_walk): Append loop abstraction sites to list.
	(oacc_loop_xform_loop): Delete.
	(oacc_loop_process): Iterate over call list directly.
	(execute_oacc_device_lower): Ignore unknown unspecs.

Index: omp-low.c
===
--- omp-low.c	(revision 240524)
+++ omp-low.c	(working copy)
@@ -253,7 +253,7 @@ struct oacc_loop
   unsigned mask;   /* Partitioning mask.  */
   unsigned inner;  /* Partitioning of inner loops.  */
   unsigned flags;  /* Partitioning flags.  */
-  unsigned ifns;   /* Contained loop abstraction functions.  */
+  vec ifns;  /* Contained loop abstraction functions.  */
   tree chunk_size; /* Chunk size.  */
   gcall *head_end; /* Final marker of head sequence.  */
 };
@@ -19341,7 +19341,6 @@ new_oacc_loop_raw (oacc_loop *parent, lo
   loop->routine = NULL_TREE;
 
   loop->mask = loop->flags = loop->inner = 0;
-  loop->ifns = 0;
   loop->chunk_size = 0;
   loop->head_end = NULL;
 
@@ -19404,7 +19403,7 @@ static oacc_loop *
 finish_oacc_loop (oacc_loop *loop)
 {
   /* If the loop has been collapsed, don't partition it.  */
-  if (!loop->ifns)
+  if (loop->ifns.is_empty ())
 loop->mask = loop->flags = 0;
   return loop->parent;
 }
@@ -19542,9 +19541,9 @@ oacc_loop_discover_walk (oacc_loop *loop
 	  break;
 
 	case IFN_GOACC_LOOP:
-	  /* Count the goacc loop abstraction fns, to determine if the
-	 loop was collapsed already.  */
-	  loop->ifns++;
+	  /* Record the abstraction function, so we can manipulate it
+	 later.  */
+	  loop->ifns.safe_push (call);
 	  break;
 
 	case IFN_UNIQUE:
@@ -19685,51 +19684,6 @@ oacc_loop_xform_head_tail (gcall *from,
 }
 }
 
-/* Transform the IFN_GOACC_LOOP internal functions by providing the
-   determined partitioning mask and chunking argument.  END_MARKER
-   points at the end IFN_HEAD_TAIL call intgroducing the loop.  IFNS
-   is the number of IFN_GOACC_LOOP calls for the loop.  MASK_ARG is
-   the replacement partitioning mask and CHUNK_ARG is the replacement
-   chunking arg.  */
-
-static void
-oacc_loop_xform_loop (gcall *end_marker, unsigned ifns,
-		  tree mask_arg, tree chunk_arg)
-{
-  gimple_stmt_iterator gsi = gsi_for_stmt (end_marker);
-  
-  gcc_checking_assert (ifns);
-  for (;;)
-{
-  for (; !gsi_end_p (gsi); gsi_next ())
-	{
-	  gimple *stmt = gsi_stmt (gsi);
-
-	  if (!is_gimple_call (stmt))
-	continue;
-
-	  gcall *call = as_a  (stmt);
-  
-	  if (!gimple_call_internal_p (call))
-	continue;
-
-	  if (gimple_call_internal_fn (call) != IFN_GOACC_LOOP)
-	continue;
-
-	  *gimple_call_arg_ptr (call, 5) = mask_arg;
-	  *gimple_call_arg_ptr (call, 4) = chunk_arg;
-	  ifns--;
-	  if (!ifns)
-	return;
-	}
-
-  /* The LOOP_BOUND ifn could be in the single successor
-	 block.  */
-  basic_block bb = single_succ (gsi_bb (gsi));
-  gsi = gsi_start_bb (bb);
-}
-}
-
 /* Process the discovered OpenACC loops, setting the correct
partitioning level etc.  */
 
@@ -19742,13 +19696,25 @@ oacc_loop_process (oacc_loop *loop)
   if (loop->mask && !loop->routine)
 {
   int ix;
-  unsigned mask = loop->mask;
-  unsigned dim = GOMP_DIM_GANG;
-  tree mask_arg = build_int_cst (unsigned_type_node, mask);
+  tree mask_arg = build_int_cst (unsigned_type_node, loop->mask);
   tree chunk_arg = loop->chunk_size;
+  gcall *call;
 
-  oacc_loop_xform_loop (loop->head_end, loop->ifns, mask_arg, chunk_arg);
+  for (ix = 0; loop->ifns.iterate (ix, ); ix++)
+	switch (gimple_call_internal_fn (call))
+	  {
+	  case IFN_GOACC_LOOP:
+	gcc_assert (gimple_call_arg (call, 5) == integer_zero_node);
+	*gimple_call_arg_ptr (call, 5) = mask_arg;
+	*gimple_call_arg_ptr (call, 4) = chunk_arg;
+	break;
 
+	  default:
+	gcc_unreachable ();
+	  }
+
+  unsigned dim = GOMP_DIM_GANG;
+  unsigned mask = loop->mask;
   for (ix = 0; ix != GOMP_DIM_MAX && mask; ix++)
 	{
 	  while (!(GOMP_DIM_MASK (dim) & mask))
@@ -20176,7 +20142,7 @@ execute_oacc_device_lower ()
 	  switch (kind)
 		{
 		default:
-		  gcc_unreachable ();
+		  break;
 
 		case IFN_UNIQUE_OACC_FORK:
 		case IFN_UNIQUE_OACC_JOIN:

Re: Change license of filenames.h to LGPL

[DJ Delorie]

Most of the files in include/ are GPL, not LGPL.  Why is this one
different?

Re: [BUILDROBOT] tic6x-uclinux: undefined reference to `gnu_libc_printf_pointer_format(tree_node*, char const**)'

[Martin Sebor]

On 09/26/2016 04:08 PM, Jan-Benedict Glaw wrote:

Hi Martin,

On Thu, 2016-09-08 13:03:12 -0600, Martin Sebor  wrote:

Attached is another update to the patch to address the last round
of comments and suggestions, most notably to:

[...]

with the currently committed version, the tic6x-uclinux target fails,
see ie.
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=630308 :

g++-g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
-DHAVE_CONFIG_H -static-libstdc++ -static-libgcc  -o cc1 c/c-lang.o 
c-family/stub-objc.o attribs.o c/c-errors.o c/c-decl.o c/c-typeck.o 
c/c-convert.o c/c-aux-info.o c/c-objc-common.o c/c-parser.o 
c/c-array-notation.o c/c-fold.o c-family/c-common.o c-family/c-cppbuiltin.o 
c-family/c-dump.o c-family/c-format.o c-family/c-gimplify.o 
c-family/c-indentation.o c-family/c-lex.o c-family/c-omp.o c-family/c-opts.o 
c-family/c-pch.o c-family/c-ppoutput.o c-family/c-pragma.o 
c-family/c-pretty-print.o c-family/c-semantics.o c-family/c-ada-spec.o 
c-family/c-cilkplus.o c-family/array-notation-common.o c-family/cilk.o 
c-family/c-ubsan.o default-c.o \
  cc1-checksum.o libbackend.a main.o libcommon-target.a libcommon.a 
../libcpp/libcpp.a ../libdecnumber/libdecnumber.a libcommon.a 
../libcpp/libcpp.a   ../libbacktrace/.libs/libbacktrace.a 
../libiberty/libiberty.a ../libdecnumber/libdecnumber.a   -lmpc -lmpfr -lgmp 
-rdynamic -ldl  -L./../zlib -lz
c6x.o:(.data+0x778): undefined reference to 
`gnu_libc_printf_pointer_format(tree_node*, char const**)'
collect2: error: ld returned 1 exit status
/home/jbglaw/repos/gcc/gcc/c/Make-lang.in:84: recipe for target 'cc1' failed
make[1]: *** [cc1] Error 1
make[1]: Leaving directory '/home/jbglaw/build/tic6x-uclinux/build-gcc/gcc'
Makefile:4252: recipe for target 'all-gcc' failed
make: *** [all-gcc] Error 2


Add another one for uClibc?


Thanks.  There must be something wrong with the target hook I added.
This is a new area for me so please bear with me while I debug and
fix it.

Martin

Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Martin Sebor]

On 09/27/2016 11:29 AM, Martin Sebor wrote:

On 09/26/2016 02:30 AM, Gerald Pfeifer wrote:

Hi Martin,

I'm afraid there may be further fallout from your patch.  GCC
now bootstraps fine on my tester, but building Wine I started
to see

  internal compiler error: in format_floating, at
gimple-ssa-sprintf.c:1165

four or so days ago.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77740 has the invocation
and a preprocessed input file.


Thanks.  (As I just commented in pr77740, so just to close to loop
here) a patch for the problem awaits code review:

  https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01470.html


I should perhaps mention that since the patch is straightforward
I'll go ahead and commit it as obvious unless there are objections.

Martin

Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Martin Sebor]

On 09/26/2016 02:30 AM, Gerald Pfeifer wrote:

Hi Martin,

I'm afraid there may be further fallout from your patch.  GCC
now bootstraps fine on my tester, but building Wine I started
to see

  internal compiler error: in format_floating, at gimple-ssa-sprintf.c:1165

four or so days ago.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77740 has the invocation
and a preprocessed input file.


Thanks.  (As I just commented in pr77740, so just to close to loop
here) a patch for the problem awaits code review:

  https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01470.html

Martin

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Tue, Sep 27, 2016 at 01:31:15PM +0200, Jakub Jelinek wrote:
> On Tue, Sep 27, 2016 at 12:56:29PM +0200, Marek Polacek wrote:
> > On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote:
> > > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote:
> > > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote:
> > > > > > The intent has been that we catch the most common forms, but still 
> > > > > > require
> > > > > > it not to be complete free form.  Because, as experience shows, 
> > > > > > people are
> > > > > > extremely creative in these comments, and it is not very good idea 
> > > > > > to
> > > > > > support everything.  For ... fall through ... , what is the purpose 
> > > > > > of
> > > > > > those ...s?
> > > > > 
> > > > > No idea, but it has been there for a while and seems perfectly 
> > > > > reasonable.
> > > > > IMO any sentence containing "fall" and "through/thru/etc" on the same 
> > > > > line 
> > > > > should be accepted, otherwise it's just misplaced pickiness.
> > > > 
> > > > +1. Folks will just disable the warning if gcc is not very permissive
> > > > when paring existing comments. You cannot expect anyone to change
> > > > perfectly fine fall-through comments just to accommodate an arbitrary
> > > > gcc style.
> > > 
> > > The accepted style is already very permissive, we don't allow just one
> > > spelling as various lint tools.  I'm afraid looking for various cases of
> > > fall and through/thru possibly separated by anything and surrounded by
> > > anything is IMHO already too much, the compiler shouldn't try to try to
> > > grammar analyze the comments on what they actually talk about and whether 
> > > it
> > > might be related to the switch fall through or something completely
> > > different.  Users should start using [[fallthrough]]; anyway.
> > 
> > I'm thinking perhaps we should also accept /* ... fall through ... */
> > and /* else fall through */, but accepting any sentence containing "fall" 
> > and
> > "through/thru/etc" on the same line would mean that we also accept
> > /* Don't fall through here.  */ and that is clearly not desirable.
> 
> I think it is important to think in terms of what regexps we still want to
> match, even when the matching is actually implemented in C, not using
> regexps.  And yes, you list one reason why arbitrary text with fall and
> through somewhere in it is not a good idea.  Another:
> /* XXX Really fallthru?  */
> (what we have in pch.c).
> So, if you want to allow ... fall through ... and else fall through, and
> perhaps // fall through - some explanation
> then it might be e.g.
> //-fallthrough$
> //@fallthrough@$
> /\*-fallthrough\*/
> /\*@fallthrough@\*/
> //[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?$
> //[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?$
> //[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?$
> /\*[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?\*/
> /\*[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?\*/
> /\*[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?\*/
> where . would match even newlines in the last 3,
> but $ would always match just end of line?

This looks like a step in the right direction.  Apparently, it's hard
to come up with something that will make everyone happy; this might be
partly because GCC is probably the only compiler that attempts to
parse comments like this.  While clang has the implicit fallthrough
warning, they aren't even trying to parse the comments.

Jakub, do you want me to look into this (make GCC accept more), or do
you want to adjust that by yourself?

Marek

Re: Add configure check for -Wimplicit-fallthrough

[Marek Polacek]

On Tue, Sep 27, 2016 at 03:23:52PM +0200, Richard Biener wrote:
> On Tue, Sep 27, 2016 at 3:13 PM, Jakub Jelinek  wrote:
> > On Tue, Sep 27, 2016 at 03:09:36PM +0200, Richard Biener wrote:
> >> On Tue, Sep 27, 2016 at 3:05 PM, Marek Polacek  wrote:
> >> > Using -Wno-error where only -Wno-implicit-fallthrough was meant was 
> >> > deemed
> >> > to coarse, so this patch attempts to add a configure check for this 
> >> > warnign
> >> > and only use -Wno-implicit-fallthrough when appropriate.
> >> >
> >> > Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk?
> >>
> >> It looks to me this would hide eventual bugs in .md files by not
> >> issueing the warning?
> >
> > Guess it depends on what kind of warnings we want to suppress here, if it is
> > something user can control in their *.md files, or something that perhaps
> > changes to the generators could handle (add /* FALLTHRU */ comments or
> > gcc_fallthrough (); in some cases)?
> 
> I just looked at one random one and it was sse.md (repeatedly doing)
> 
> case MODE_V16SF:
>   gcc_assert (TARGET_AVX512F);
> case MODE_V8SF:
>   gcc_assert (TARGET_AVX);
> case MODE_V4SF:
>   gcc_assert (TARGET_SSE);
> ...

These were the easy cases.  Now that I investigated more, I think the rest
can be fixed by making genattrtab.c emit the /* FALLTHRU */ comments.  See
 -- so I think we
might not need this configure check at all in the end.

Marek

PATCH to add more FALLTHRU markers

[Marek Polacek]

Currently Makefile.in contains -Wno-error for several of the insn-* files, but
after further investigation I think with this patch we won't need them anymore.
I'm not removing it until I bootstrap gcc on more arches, though.  Meanwhile,
this patch at least makes the code more robust.

Richard S., can you look at the genattrtab.c bit?  Thanks,

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

2016-09-27  Marek Polacek  

* config/i386/sse.md: Add FALLTHRU markers.
* genattrtab.c (write_attr_case): Also emit FALLTHRU marker.

diff --git gcc/config/i386/sse.md gcc/config/i386/sse.md
index 7e718a0..a503b19 100644
--- gcc/config/i386/sse.md
+++ gcc/config/i386/sse.md
@@ -11268,8 +11268,10 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
   gcc_assert (TARGET_AVX2);
+  /* FALLTHRU */
 case MODE_TI:
   gcc_assert (TARGET_SSE2);
   switch (mode)
@@ -11298,8 +11300,10 @@
 
 case MODE_V16SF:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_V8SF:
   gcc_assert (TARGET_AVX);
+  /* FALLTHRU */
 case MODE_V4SF:
   gcc_assert (TARGET_SSE);
 
@@ -11393,8 +11397,10 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
   gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  /* FALLTHRU */
 case MODE_TI:
   gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
   switch (mode)
@@ -11402,10 +11408,11 @@
 case V16SImode:
 case V8DImode:
   if (TARGET_AVX512F)
-  {
-tmp = "p";
-break;
-  }
+   {
+ tmp = "p";
+ break;
+   }
+ /* FALLTHRU */
 case V8SImode:
 case V4DImode:
 case V4SImode:
@@ -11419,6 +11426,7 @@
 
case MODE_V8SF:
   gcc_assert (TARGET_AVX);
+  /* FALLTHRU */
case MODE_V4SF:
   gcc_assert (TARGET_SSE);
   gcc_assert (!);
@@ -11489,8 +11497,10 @@
 {
 case MODE_XI:
   gcc_assert (TARGET_AVX512F);
+  /* FALLTHRU */
 case MODE_OI:
   gcc_assert (TARGET_AVX2 || TARGET_AVX512VL);
+  /* FALLTHRU */
 case MODE_TI:
   gcc_assert (TARGET_SSE2 || TARGET_AVX512VL);
   switch (mode)
@@ -11498,21 +11508,23 @@
 case V64QImode:
 case V32HImode:
   if (TARGET_AVX512F)
-  {
-tmp = "p";
-ssesuffix = "q";
-break;
-  }
+   {
+ tmp = "p";
+ ssesuffix = "q";
+ break;
+   }
+ /* FALLTHRU */
 case V32QImode:
 case V16HImode:
 case V16QImode:
 case V8HImode:
   if (TARGET_AVX512VL || TARGET_AVX2 || TARGET_SSE2)
-  {
-tmp = "p";
-ssesuffix = TARGET_AVX512VL ? "q" : "";
-break;
-  }
+   {
+ tmp = "p";
+ ssesuffix = TARGET_AVX512VL ? "q" : "";
+ break;
+   }
+ /* FALLTHRU */
 default:
   gcc_unreachable ();
   }
@@ -11520,6 +11532,7 @@
 
case MODE_V8SF:
   gcc_assert (TARGET_AVX);
+  /* FALLTHRU */
case MODE_V4SF:
   gcc_assert (TARGET_SSE);
   tmp = "ps";
diff --git gcc/genattrtab.c gcc/genattrtab.c
index c8e166e..3b47f34 100644
--- gcc/genattrtab.c
+++ gcc/genattrtab.c
@@ -4283,6 +4283,8 @@ write_attr_case (FILE *outf, struct attr_desc *attr, 
struct attr_value *av,
   fprintf (outf, "&& asm_noperands (PATTERN (insn)) < 0)\n");
   write_indent (outf, indent + 2);
   fprintf (outf, "  fatal_insn_not_found (insn);\n");
+  write_indent (outf, indent + 2);
+  fprintf (outf, "/* FALLTHRU */\n");
 }
 
   if (write_case_lines)

Marek

Re: [Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c

[James Greenhalgh]

On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote:
> Hi,
> 
>   This patch requires int32plus for
>   gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
>   failures for a 16 bit int target like the avr. The "%u" format
>   specifier tests, for example, use int literals big enough to only fit
>   in a long int, and this causes unexpected warnings.
> 
>   Comitted to trunk.

This change is obviously incomplete as it does not update the expected
line numbers for warnings generated by this testcase.

Found with my bisect robot:

  Failures:
gcc.dg/tree-ssa/builtin-sprintf-warn-1.c 

  Bisected to: 

  Author: saaadhu 
  Date:   Tue Sep 27 11:05:25 2016 +

Fix bogus test failure for avr

The test has a bunch of hardcoded integer literals that would fit only in a
32 bits+ int, causing overflow warnings for a 16 bit int target like avr.

gcc/testsuite/ChangeLog

2016-09-27  Senthil Kumar Selvaraj  

* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240528 


  FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings, line 
96)
/* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux 
} 96 } */

  FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for warnings, 
line 108)
/* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 108 } 
*/
/* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } 108 
} */

  FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors)

The line numbers here need bumped to match the change you've made.

Thanks,
James


> 2016-09-27  Senthil Kumar Selvaraj  
> 
>   * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.
>  
>   PR fortran/77666
> Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
> ===
> --- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (revision 240524)
> +++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c  (working copy)
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
>  /* { dg-options "-std=c99 -Wformat -Wformat-length=1 
> -ftrack-macro-expansion=0" } */
> +/* { dg-require-effective-target int32plus } */
>  
>  /* When debugging, define LINE to the line number of the test case to 
> exercise
> and avoid exercising any of the others.  The buffer and objsize macros
> 

Re: Implement -Wimplicit-fallthrough (version 9)

[Tom Tromey]

> "Michael" == Michael Matz  writes:

Michael> All those bugs would also have been found as well when it had simply 
Michael> accepted
Michael>   /fall.*thr/i
Michael> anywhere in the preceding comment on one line.  But all the recent 
Michael> spelling changes of comments to cater for the strictness exactly shows 
how 
Michael> misguided that is.  The above would accept "Don't fall through" as 
well.  
Michael> I say: so what?

The point of the warning is to make code more robust.  But accepting any
comment like "Don't fall through" is not more robust, but rather an
error waiting to happen; as IIUC the user has no way to detect this
case.

I think it's better for the comment-scanning feature to be very picky
(or even just not exist at all) -- that way you know exactly what you
are getting.  Lint was traditionally picky IIRC.  And, this is a warning
that isn't default and can also be disabled, so it's not as if users
have no recourse.

Tom

Re: [AArch64][0/14] ARMv8.2-A FP16 extension support

[Jiong Wang]

On 25/07/16 12:26, James Greenhalgh wrote:

On Thu, Jul 07, 2016 at 05:12:48PM +0100, Jiong Wang wrote:

Hello,

As a follow up of

https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html,

This patch set adds ARMv8.2-A FP16 scalar and vector intrinsics support,
gcc middle-end will also be aware of some standard operations that some
instructions can be auto-generated.

According to ACLE, ARMv8.2-A FP16 intrinsics for AArch64 is superset of
intrinsics for AArch32, so all those intrinsic related testcases,
particularly those under the directory advsimd-intrinsics, are also
appliable to AArch64.  This patch set has only included those testcases
that are exclusive for AArch64.

Jiong Wang (14)
   ARMv8.2-A FP16 data processing intrinsics
   ARMv8.2-A FP16 one operand vector intrinsics
   ARMv8.2-A FP16 two operands vector intrinsics
   ARMv8.2-A FP16 three operands vector intrinsics
   ARMv8.2-A FP16 lane vector intrinsics
   ARMv8.2-A FP16 reduction vector intrinsics
   ARMv8.2-A FP16 one operand scalar intrinsics
   ARMv8.2-A FP16 two operands scalar intrinsics
   ARMv8.2-A FP16 three operands scalar intrinsics
   ARMv8.2-A FP16 lane scalar intrinsics

At this point, I've OKed the first 10 patches in the series, these represent
the functional changes to the compiler. I'm leaving the testsuite patches
for now, as they depend on testsuite changes that have yet to be approved
for the ARM port.

To save you from having to continue to rebase the functional parts of this
patch while you wait for review of the ARM changes, I would be OK with you
committing them now, on the understanding that you'll continue to check
the testsuite in the time between now and the testsuite changes are approved,
and that you'll fix any issues that you find.


   ARMv8.2-A FP16 testsuite selector
   ARMv8.2-A testsuite for new data movement intrinsics
   ARMv8.2-A testsuite for new vector intrinsics
   ARMv8.2-A testsuite for new scalar intrinsics
  
I've taken a brief look through these testsuite changes and they look OK

to me. I'll revisit them properly once I've seen the ARM patches go in.


Now as ARM patches have gone in around r240427, I have done a quick 
confirmation

on the status of these four pending testsuite patches:

  https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00337.html
  https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00338.html
  https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00339.html
  https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00340.html

The result is they applies cleanly on gcc trunk, and there is no 
regression on
AArch64 native regression test.  Testcases enabled without requirement 
of FP16

all passed.

I will give a final run on ARM native board and AArch64 emulation 
environment

with ARMv8.2-A FP16 enabled. (Have done this before, just in case something
changed during these days)

OK for trunk if there is no regression?

Thanks

[COMMITTED, aarch64] Delete one redundant word in target-supports.exp comment

[Jiong Wang]

This patch deletes one redundant word in target-supports.exp function comment
for "check_effective_target_arm_v8_2a_fp16_scalar_hw".

   s/instructions floating point instructions/floating point instructions/

The comment is re-indented.  No other changes.

Committed as obivious as r240551.

gcc/testsuite/
2016-09-27  Jiong Wang

 * lib/target-supports.exp
 (check_effective_target_arm_v8_2a_fp16_scalar_hw): Delete redundant 
word
 in function comment.

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 3d11e28..50723de 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -4015,9 +4015,8 @@ proc check_effective_target_arm_v8_1a_neon_hw { } {
 } [add_options_for_arm_v8_1a_neon ""]]
 }
 
-# Return 1 if the target supports executing instructions floating point
-# instructions from ARMv8.2 with the FP16 extension, 0 otherwise.  The
-# test is valid for ARM.
+# Return 1 if the target supports executing floating point instructions from
+# ARMv8.2 with the FP16 extension, 0 otherwise.  The test is valid for ARM.
 
 proc check_effective_target_arm_v8_2a_fp16_scalar_hw { } {
 if { ![check_effective_target_arm_v8_2a_fp16_scalar_ok] } {

Re: Implement -Wimplicit-fallthrough (version 9)

[Michael Matz]

Hi,

On Tue, 27 Sep 2016, Marek Polacek wrote:

> > Perhaps we want -Wimplicit-fallthrough{,=1,=2,=3,=4}, where =1 would 
> > match indeed /fall.*thr/i (note, it will be really costly in this 
> > case, one will have to parse all comments in detail in the 
> > preprocessor, so I'd be against making it the default),
> 
> Perhaps we could use POSIX regcomp/regex functions; do you (or anyone 
> else) have an idea how expensive they are and if it's feasible to use 
> them in the preprocessor?

Why?  The regexp I gave was for demonstration.  Matching /fall.*thr/i 
would be done by something similar to:

  a = strcasestr(comment, "fall");
  if (!a) return;  // no fall
  b = strcasestr(a+4, "thr");
  if (!b) return;  // no thr
  if (memchr(a+4, '\n', b-a-4)) return; // on different lines
  foundit();

(With appropriate massaging that the comment to parse ends with 0.  
strcasestr would need addition to libiberty for where it's not available 
(or falling back to strstr there); obviously the above can be sped up by 
various tricks for ASCII and UTF-8 because of the relation of upper and 
lower case characters.  During tokenizing the comment (i.e. while 
searching for the end) one could already search for "fall" for instance to 
quickly early-out.)


Ciao,
Michael.

[PATCH] Define feature macro and update C++17 library status

[Jonathan Wakely]

This adds the missing feature macro for the searchers and updates the
docs.

* doc/xml/manual/status_cxx2017.xml: Update status.
* doc/html/*: Regenerate.
* include/std/functional (__cpp_lib_boyer_moore_searcher): Define.
* testsuite/20_util/function_objects/searchers.cc: Test feature macro.

Tested powerpc64le-linux, committed to trunk.

commit 5344a11cabf9231dec18db2882034e55e5658b8f
Author: Jonathan Wakely 
Date:   Tue Sep 27 16:25:22 2016 +0100

Define feature macro and update C++17 library status

* doc/xml/manual/status_cxx2017.xml: Update status.
* doc/html/*: Regenerate.
* include/std/functional (__cpp_lib_boyer_moore_searcher): Define.
* testsuite/20_util/function_objects/searchers.cc: Test feature macro.

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index 4ead6b9..feed085 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -171,14 +171,13 @@ Feature-testing recommendations for C++.
 
 
 
-  
Library Fundamentals V1 TS Components: Searchers 
   
http://www.w3.org/1999/xlink; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0220r1.html;>
P0220R1

   
-   No 
+   7 
__cpp_lib_boyer_moore_searcher >= 201603 
 
 
@@ -332,38 +331,35 @@ Feature-testing recommendations for C++.
 
 
 
-  
Fixes for not_fn 
   
http://www.w3.org/1999/xlink; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0358r1.html;>
P0358R1

   
-   No 
+   7 
   
 
 
 
-  
Fixing a design mistake in the searchers interface in Library 
Fundamentals 
   
http://www.w3.org/1999/xlink; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0253r1.pdf;>
P0253R1

   
-   No 
+   7 
   
 
 
 
-  
Extending memory management tools 
   
http://www.w3.org/1999/xlink; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0040r3.html;>
P0040R3

   
-   No 
+   7 

 
 
diff --git a/libstdc++-v3/include/std/functional 
b/libstdc++-v3/include/std/functional
index c750a83..8b2389c 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -2206,6 +2206,7 @@ _GLIBCXX_MEM_FN_TRAITS(&&, false_type, true_type)
 }
 
   // Searchers
+#define __cpp_lib_boyer_moore_searcher 201603
 
   template>
 class default_searcher
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
index 1c72762..5310359 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/searchers.cc
@@ -25,6 +25,12 @@
 #include 
 #include 
 
+#ifndef __cpp_lib_boyer_moore_searcher
+# error "Feature-test macro for searchers missing"
+#elif __cpp_lib_boyer_moore_searcher < 201603
+# error "Feature-test macro for searchers has wrong value"
+#endif
+
 using std::make_default_searcher;
 using std::make_boyer_moore_searcher;
 using std::make_boyer_moore_horspool_searcher;

Re: [PATCH] Last bit from Early LTO debug merge -- move break_out_includes

[Jason Merrill]

OK.

On Tue, Sep 27, 2016 at 9:47 AM, Richard Biener  wrote:
>
> This is moving break_out_includes (aka -feliminate-dwarf2-dups handling)
> to early finish.  It requires some massaging with the way we pick
> up its results which previously "conveniently" ended up in the
> limbo list but after moving to early will be scrapped off by
> late finish doing a limbo list flush (where we just drop all CU DIEs
> thinking it can only be comp_unit_die () itself).  Thus this patch
> adds a cu_die_list list for it (much similar to the comdat one we already
> have).
>
> Eventually -feliminate-dwarf2-dups might die anyway(?), its testing
> coverage is very low and it's declared broken for C++.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Ok for trunk?
>
> Thanks,
> Richard.
>
> 2016-09-27  Richard Biener  
>
> * dwarf2out.c (cu_die_list): New global.
> (dwarf2out_finish): Walk cu_die_list instead of limbo DIEs.  Add
> main_comp_unit_die to cu_die_list if we created it.
> Move break_out_includes ...
> (dwarf2out_early_finish): ... here.  Push created CU DIEs onto
> the cu_die_list.
>
> diff -u gcc/dwarf2out.c gcc/dwarf2out.c
> --- gcc/dwarf2out.c (working copy)
> +++ gcc/dwarf2out.c (working copy)
> @@ -2866,6 +2866,9 @@
>  /* A list of type DIEs that have been separated into comdat sections.  */
>  static GTY(()) comdat_type_node *comdat_type_list;
>
> +/* A list of CU DIEs that have been separated.  */
> +static GTY(()) limbo_die_node *cu_die_list;
> +
>  /* A list of DIEs with a NULL parent waiting to be relocated.  */
>  static GTY(()) limbo_die_node *limbo_die_list;
>
> @@ -27855,11 +27858,6 @@
>resolve_addr (comp_unit_die ());
>move_marked_base_types ();
>
> -  /* Generate separate CUs for each of the include files we've seen.
> - They will go into limbo_die_list.  */
> -  if (flag_eliminate_dwarf2_dups)
> -break_out_includes (comp_unit_die ());
> -
>/* Initialize sections and labels used for actual assembler output.  */
>init_sections_and_labels ();
>
> @@ -27867,7 +27865,7 @@
>   have children.  */
>add_sibling_attributes (comp_unit_die ());
>limbo_die_node *node;
> -  for (node = limbo_die_list; node; node = node->next)
> +  for (node = cu_die_list; node; node = node->next)
>  add_sibling_attributes (node->die);
>for (ctnode = comdat_type_list; ctnode != NULL; ctnode = ctnode->next)
>  add_sibling_attributes (ctnode->root_die);
> @@ -27876,7 +27874,15 @@
>   skeleton compile_unit DIE that remains in the .o, while
>   most attributes go in the DWO compile_unit_die.  */
>if (dwarf_split_debug_info)
> -main_comp_unit_die = gen_compile_unit_die (NULL);
> +{
> +  limbo_die_node *cu;
> +  main_comp_unit_die = gen_compile_unit_die (NULL);
> +  cu = limbo_die_list;
> +  gcc_assert (cu->die == main_comp_unit_die);
> +  limbo_die_list = limbo_die_list->next;
> +  cu->next = cu_die_list;
> +  cu_die_list = cu;
> +}
>else
>  main_comp_unit_die = comp_unit_die ();
>
> @@ -27988,7 +27994,7 @@
>
>/* Output all of the compilation units.  We put the main one last so that
>   the offsets are available to output_pubnames.  */
> -  for (node = limbo_die_list; node; node = node->next)
> +  for (node = cu_die_list; node; node = node->next)
>  output_comp_unit (node->die, 0);
>
>hash_table comdat_type_table (100);
> @@ -28217,6 +28223,21 @@
>prune_unused_types ();
>  }
>
> +  /* Generate separate CUs for each of the include files we've seen.
> + They will go into limbo_die_list and from there to cu_die_list.  */
> +  if (flag_eliminate_dwarf2_dups)
> +{
> +  gcc_assert (limbo_die_list == NULL);
> +  break_out_includes (comp_unit_die ());
> +  limbo_die_node *cu;
> +  while ((cu = limbo_die_list))
> +   {
> + limbo_die_list = cu->next;
> + cu->next = cu_die_list;
> + cu_die_list = cu;
> +   }
> +}
> +
>/* The early debug phase is now finished.  */
>early_dwarf_finished = true;
>  }

Re: debug container mutex association

[Jonathan Wakely]

On 20/09/16 09:53 +0100, Jonathan Wakely wrote:

On 19/09/16 21:56 +0200, François Dumont wrote:

Hi

  Following our conversation here is a much simpler patch. I just 
consider that all debug containers will have the same alignment.


  Even if I submit this patch as a whole I will commit into pieces, 
at least one for the pure cleanup parts and one for the debug.cc 
change.


  Among those changes there is:
-   __gnu_cxx::__scoped_lock(this->_M_get_mutex());
+   __gnu_cxx::__scoped_lock __l(this->_M_get_mutex());

  I would appreciate if you could tell me what was happening with 
the initial expression. I don't understand why it is compiling. I 
even tried the same in debug.cc and started having compilation 
errors.


It creates a temporary __scoped_lock, which immediately goes out of
scope and unlocks the mutex again. This should be fixed on the gcc-5
and gcc-6 branches too.


I'm committing this to the gcc-5 and gcc-6 branches.


commit 5218b515fdb54881d8a2b87c28eb5fd84c52db01
Author: Jonathan Wakely 
Date:   Tue Sep 27 16:22:28 2016 +0100

Fix lifetime of mutex lock in debug iterator

	* include/debug/safe_iterator.h (_Safe_iterator::operator++()): Fix
	lifetime of lock.

diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5368f3b..3f5a7f8 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -296,7 +296,7 @@ namespace __gnu_debug
 	_GLIBCXX_DEBUG_VERIFY(this->_M_incrementable(),
 			  _M_message(__msg_bad_inc)
 			  ._M_iterator(*this, "this"));
-	__gnu_cxx::__scoped_lock(this->_M_get_mutex());
+	__gnu_cxx::__scoped_lock __l(this->_M_get_mutex());
 	++base();
 	return *this;
   }

Re: Implement -Wimplicit-fallthrough (version 9)

[Jakub Jelinek]

On Tue, Sep 27, 2016 at 05:19:10PM +0200, Bernd Schmidt wrote:
> On 09/27/2016 05:04 PM, Jakub Jelinek wrote:
> >>>On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:
> All those bugs would also have been found as well when it had simply
> accepted
>   /fall.*thr/i
> anywhere in the preceding comment on one line.  But all the recent
> spelling changes of comments to cater for the strictness exactly shows how
> misguided that is.  The above would accept "Don't fall through" as well.
> I say: so what?
> 
> >Perhaps we want -Wimplicit-fallthrough{,=1,=2,=3,=4}, where
> >=1 would match indeed /fall.*thr/i (note, it will be really costly in this
> >case, one will have to parse all comments in detail in the preprocessor,
> >so I'd be against making it the default), =2 would allow
> >what we do right now, perhaps with the optional else and dots (perhaps
> >selected other interpunction chars), =3 would only allow the standardized
> >lint comments and =4 would not allow any comments, just the attributes?
> >Then each project can choose what they want.
> 
> I feel that's overthinking it. I believe Michael has identified the correct
> way to think about the issue.

See above, it is very expensive at preprocessing time (look at how the
preprocessor optimizes skipping over comments, with that it is all gone),
and not everybody will want /* Don't fall through here.  */ or
/* This is fallible.  Threats are high.  */ (pick any of the hundreds+
english words with fall in them and thousands+ of words with thr in them)
to disable the warning.

Jakub

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Tue, Sep 27, 2016 at 05:04:23PM +0200, Jakub Jelinek wrote:
> On Tue, Sep 27, 2016 at 04:54:28PM +0200, Marek Polacek wrote:
> > On Tue, Sep 27, 2016 at 10:48:50AM -0400, Jason Merrill wrote:
> > > On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:
> > > > On Tue, 27 Sep 2016, Jakub Jelinek wrote:
> > > >
> > > >> Just compare that to the number of real bugs the warning found in gcc
> > > >> codebase.  It is really worth it for -Wextra.
> > > >
> > > > All those bugs would also have been found as well when it had simply
> > > > accepted
> > > >   /fall.*thr/i
> > > > anywhere in the preceding comment on one line.  But all the recent
> > > > spelling changes of comments to cater for the strictness exactly shows 
> > > > how
> > > > misguided that is.  The above would accept "Don't fall through" as well.
> > > > I say: so what?
> > > 
> > > I agree.
> > 
> > All right, I'm not opposed to making the comment parsing more benevolent.
> > We still should have enough time to fine-tune it.
> 
> Perhaps we want -Wimplicit-fallthrough{,=1,=2,=3,=4}, where
> =1 would match indeed /fall.*thr/i (note, it will be really costly in this
> case, one will have to parse all comments in detail in the preprocessor,
> so I'd be against making it the default),

Perhaps we could use POSIX regcomp/regex functions; do you (or anyone else)
have an idea how expensive they are and if it's feasible to use them in the
preprocessor?

Marek

Re: Implement -Wimplicit-fallthrough (version 9)

[Bernd Schmidt]

On 09/27/2016 05:04 PM, Jakub Jelinek wrote:

On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:

All those bugs would also have been found as well when it had simply
accepted
  /fall.*thr/i
anywhere in the preceding comment on one line.  But all the recent
spelling changes of comments to cater for the strictness exactly shows how
misguided that is.  The above would accept "Don't fall through" as well.
I say: so what?



Perhaps we want -Wimplicit-fallthrough{,=1,=2,=3,=4}, where
=1 would match indeed /fall.*thr/i (note, it will be really costly in this
case, one will have to parse all comments in detail in the preprocessor,
so I'd be against making it the default), =2 would allow
what we do right now, perhaps with the optional else and dots (perhaps
selected other interpunction chars), =3 would only allow the standardized
lint comments and =4 would not allow any comments, just the attributes?
Then each project can choose what they want.


I feel that's overthinking it. I believe Michael has identified the 
correct way to think about the issue.



Bernd

[PATCH] Define 3-argument overloads of std::hypot for C++17 (P0030R1)

[Jonathan Wakely]

This adds the new 3D std::hypot() functions. This implementation seems
to be faster than the naïve sqrt(x*x + y*y + z*z) implementation, or
hypot(hypot(x, y), z), and should be a bit more accurate at very large
or very small values due to reducing the arguments by the largest one.
Improvements welcome though, as this is not my forte.

The test might not be very good, but tests some small integer values
and some other values where accuracy is lost for one or other of the
alternative implementations mentioned above. If this FAILs for some
32-bit targets we might need to adjust the tolerances or the
dg-options.

* doc/xml/manual/status_cxx2017.xml: Update status.
* include/c_global/cmath (hypot): Add three-dimensional overloads.
* testsuite/26_numerics/headers/cmath/hypot.cc: New.

Tested powerpc64le-linux and x86_64-linux, committed to trunk.

commit 33fb40d1445fbc711d64a1ff8b9a0a8f092a2e7e
Author: Jonathan Wakely 
Date:   Tue Sep 27 15:44:45 2016 +0100

Define 3-argument overloads of std::hypot for C++17 (P0030R1)

* doc/xml/manual/status_cxx2017.xml: Update status.
* include/c_global/cmath (hypot): Add three-dimensional overloads.
* testsuite/26_numerics/headers/cmath/hypot.cc: New.

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index 76eaaa0..4ead6b9 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -633,14 +633,13 @@ Feature-testing recommendations for C++.
 
 
 
-  
Proposal to Introduce a 3-Argument Overload to 
std::hypot 
   
http://www.w3.org/1999/xlink; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0030r1.pdf;>
P0030R1

   
-   No 
+   7 
__cpp_lib_hypot >= 201603 
 
 
diff --git a/libstdc++-v3/include/c_global/cmath 
b/libstdc++-v3/include/c_global/cmath
index 6db9dee..fffa0e7 100644
--- a/libstdc++-v3/include/c_global/cmath
+++ b/libstdc++-v3/include/c_global/cmath
@@ -1455,6 +1455,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return hypot(__type(__x), __type(__y));
 }
 
+#if __cplusplus > 201402L
+#define __cpp_lib_hypot 201603
+  // [c.math.hypot3], three-dimensional hypotenuse
+
+  template
+inline _Tp
+__hypot3(_Tp __x, _Tp __y, _Tp __z)
+{
+  __x = std::abs(__x);
+  __y = std::abs(__y);
+  __z = std::abs(__z);
+  if (_Tp __a = __x < __y ? __y < __z ? __z : __y : __x < __z ? __z : __x)
+   return __a * std::sqrt((__x / __a) * (__x / __a)
+  + (__y / __a) * (__y / __a)
+  + (__z / __a) * (__z / __a));
+  else
+   return {};
+}
+
+  inline float
+  hypot(float __x, float __y, float __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline double
+  hypot(double __x, double __y, double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  inline long double
+  hypot(long double __x, long double __y, long double __z)
+  { return std::__hypot3(__x, __y, __z); }
+
+  template
+typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type
+hypot(_Tp __x, _Up __y, _Vp __z)
+{
+  using __type = typename __gnu_cxx::__promote_3<_Tp, _Up, _Vp>::__type;
+  return std::__hypot3<__type>(__x, __y, __z);
+}
+#endif // C++17
+
 #ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO
   constexpr int
   ilogb(float __x)
diff --git a/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc 
b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
new file mode 100644
index 000..4a6841c
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/headers/cmath/hypot.cc
@@ -0,0 +1,138 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target c++1z } }
+
+#include 
+#include 
+#if defined(__TEST_DEBUG)
+#include 
+#define VERIFY(A) \
+if (!(A)) \
+  { \
+std::cout << "line " << __LINE__ \
+  << "  max_abs_frac = " << max_abs_frac \
+  << "  tolerance = " << toler \
+  << std::endl; \
+  }
+#else
+#include 
+#endif
+
+using std::is_same_v;
+static_assert(is_same_v);

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/27/16 16:42, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger
>  wrote:
>> On 09/27/16 16:10, Florian Weimer wrote:
>>> * Bernd Edlinger:
>>>
> “0 << 0” is used in a similar context, to create a zero constant for a
> multi-bit subfield of an integer.
>
> This example comes from GDB, in bfd/elf64-alpha.c:
>
> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>

 Of course that is not a boolean context, and will not get a warning.

 Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".

 Maybe 1 and 0 come from macro expansion
>>>
>>> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
>>> patch, then?
>>
>> I am not sure if it was a good idea.
>>
>> I saw, we had code of the form
>> bool flag = 1 << 2;
>>
>> another value LOOKUP_PROTECT is  1 << 0, and
>> bool flag = 1 << 0;
>>
>> would at least not overflow the allowed value range of a boolean.
>
> Assigning a bit mask to a bool variable is still probably not what was
> intended, even if it doesn't change the value.
>

That works for me too.
I can simply remove that exception.


Bernd.

RE: [PATCH] MIPS/doc: Fix `d' constraint description

[Maciej W. Rozycki]

On Mon, 26 Sep 2016, Matthew Fortune wrote:

> > * config/mips/constraints.md (d): Fix documentation.
> > * doc/md.texi (Machine Constraints): Update accordingly.
> > 
> >  OK to apply?
> 
> OK, thanks for the patch.

 Committed, thanks.

  Maciej

Re: Split c-common.c?

[Jason Merrill]

Sure.

On Mon, Sep 26, 2016 at 10:57 AM, Jeff Law  wrote:
> On 09/26/2016 07:38 AM, Marek Polacek wrote:
>>
>> On Mon, Sep 26, 2016 at 03:26:28PM +0200, Jakub Jelinek wrote:
>>>
>>> On Mon, Sep 26, 2016 at 03:22:08PM +0200, Marek Polacek wrote:

 Before I spend time on this, I wanted to check if you consider this a
 good
 idea.  Since c-common.c has grown a lot and is quite large now, I think
 we
 might split it into c-warn.c, where various warning routines would go.
 What do
 you think?
>>>
>>>
>>> Perhaps c-attribs.c for the attribute handling stuff too?
>>
>>
>> Yeah.  The way I envision c-common.c is to contain shared c-family
>> routines
>> such as c_common_type_for_*, c_save_expr, max_align_t_align, and similar,
>> but
>> the warnings/diagnostics might use their own file.
>
> Works for me.
>
> jeff

Re: Implement -Wimplicit-fallthrough (version 9)

[Jakub Jelinek]

On Tue, Sep 27, 2016 at 04:54:28PM +0200, Marek Polacek wrote:
> On Tue, Sep 27, 2016 at 10:48:50AM -0400, Jason Merrill wrote:
> > On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:
> > > On Tue, 27 Sep 2016, Jakub Jelinek wrote:
> > >
> > >> Just compare that to the number of real bugs the warning found in gcc
> > >> codebase.  It is really worth it for -Wextra.
> > >
> > > All those bugs would also have been found as well when it had simply
> > > accepted
> > >   /fall.*thr/i
> > > anywhere in the preceding comment on one line.  But all the recent
> > > spelling changes of comments to cater for the strictness exactly shows how
> > > misguided that is.  The above would accept "Don't fall through" as well.
> > > I say: so what?
> > 
> > I agree.
> 
> All right, I'm not opposed to making the comment parsing more benevolent.
> We still should have enough time to fine-tune it.

Perhaps we want -Wimplicit-fallthrough{,=1,=2,=3,=4}, where
=1 would match indeed /fall.*thr/i (note, it will be really costly in this
case, one will have to parse all comments in detail in the preprocessor,
so I'd be against making it the default), =2 would allow
what we do right now, perhaps with the optional else and dots (perhaps
selected other interpunction chars), =3 would only allow the standardized
lint comments and =4 would not allow any comments, just the attributes?
Then each project can choose what they want.

Jakub

Re: [wwwdocs] Add some missing C++17 features

[Jason Merrill]

On Mon, Sep 26, 2016 at 12:35 PM, Jakub Jelinek  wrote:
>  
> +  New Rules for auto deduction from braced-init-list
> +   href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3922.html;>N3922
>  
> +  No
> +  
> +

This is in GCC 5.

OK with that change.

Jason

Re: [PATCH] [ARC] New CPU C-define handler.

[Andrew Burgess]

* Claudiu Zissulescu  [2016-05-19 13:57:44 
+0200]:

> This patch refactors how we handle the built-in preprocessor macros and 
> assertions for ARC.
> 
> OK to apply?

This looks like a good improvement to me.

Thanks,
Andrew

> Claudiu
> 
> gcc/
> 2016-05-02  Claudiu Zissulescu  
> 
>   * config/arc/arc-c.c: New file.
>   * config/arc/arc-c.def: Likewise.
>   * config/arc/t-arc: Likewise.
>   * config.gcc: Include arc-c.o as c and cpp object.
>   * config/arc/arc-protos.h (arc_cpu_cpp_builtins): Add prototype.
>   * config/arc/arc.h (TARGET_CPU_CPP_BUILTINS): Use
>   arc_cpu_cpp_builtins.
> ---
>  gcc/config.gcc  |  2 ++
>  gcc/config/arc/arc-c.c  | 69 
> +
>  gcc/config/arc/arc-c.def| 68 
>  gcc/config/arc/arc-protos.h |  1 +
>  gcc/config/arc/arc.h| 56 +---
>  gcc/config/arc/t-arc| 29 +++
>  6 files changed, 170 insertions(+), 55 deletions(-)
>  create mode 100644 gcc/config/arc/arc-c.c
>  create mode 100644 gcc/config/arc/arc-c.def
>  create mode 100644 gcc/config/arc/t-arc
> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 4e98df7..148e020 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -323,6 +323,8 @@ am33_2.0-*-linux*)
>   ;;
>  arc*-*-*)
>   cpu_type=arc
> + c_target_objs="arc-c.o"
> + cxx_target_objs="arc-c.o"
>   ;;
>  arm*-*-*)
>   cpu_type=arm
> diff --git a/gcc/config/arc/arc-c.c b/gcc/config/arc/arc-c.c
> new file mode 100644
> index 000..3bf3fd2
> --- /dev/null
> +++ b/gcc/config/arc/arc-c.c
> @@ -0,0 +1,69 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify it
> +   under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GCC is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +   License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   .
> +*/
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "tm.h"
> +#include "tree.h"
> +#include "tm_p.h"
> +#include "cpplib.h"
> +#include "c-family/c-common.h"
> +#include "target.h"
> +
> +#define builtin_define(TXT) cpp_define (pfile, TXT)
> +#define builtin_assert(TXT) cpp_assert (pfile, TXT)
> +
> +/* Define or undefine macros based on the current target.  */
> +
> +static void
> +def_or_undef_macro (cpp_reader* pfile, const char *name, bool def_p)
> +{
> +  if (def_p)
> +cpp_define (pfile, name);
> +  else
> +cpp_undef (pfile, name);
> +}
> +
> +/* Helper for TARGET_CPU_CPP_BUILTINS hook.  */
> +
> +void
> +arc_cpu_cpp_builtins (cpp_reader * pfile)
> +{
> +  builtin_assert ("cpu=arc");
> +  builtin_assert ("machine=arc");
> +
> +  builtin_define ("__arc__");
> +
> +#undef ARC_C_DEF
> +#define ARC_C_DEF(NAME, CONDITION)   \
> +  def_or_undef_macro (pfile, NAME, CONDITION);
> +
> +#include "arc-c.def"
> +#undef ARC_C_DEF
> +
> +  builtin_define_with_int_value ("__ARC_TLS_REGNO__",
> +  arc_tp_regno);
> +
> +  builtin_define (TARGET_BIG_ENDIAN
> +   ? "__BIG_ENDIAN__" : "__LITTLE_ENDIAN__");
> +  if (TARGET_BIG_ENDIAN)
> +builtin_define ("__big_endian__");
> +
> +}
> diff --git a/gcc/config/arc/arc-c.def b/gcc/config/arc/arc-c.def
> new file mode 100644
> index 000..88c65ac
> --- /dev/null
> +++ b/gcc/config/arc/arc-c.def
> @@ -0,0 +1,68 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify it
> +   under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GCC is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +   License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GCC; see the file COPYING3.  If not see
> +   .
> +*/
> +
> +ARC_C_DEF ("__ARC600__", TARGET_ARC600)
> +ARC_C_DEF ("__ARC601__", TARGET_ARC601)
> +ARC_C_DEF ("__ARC700__", TARGET_ARC700)
> +ARC_C_DEF ("__ARCEM__",  TARGET_EM)
> +ARC_C_DEF ("__ARCHS__",  TARGET_HS)
> 

Change license of filenames.h to LGPL

[Eli Zaretskii]

Hi,

I was asked by Ozkan Sezer (CC'ed) whether I'd agree to relicense
include/filenames.h under LGPL2+ instead of GPL2+.

I talked to Richard Stallman (in private email), and he authorized the
change.  So now I'm proposing the corresponding change to you.

Thanks.

P.S.  Please CC me on any responses, as I'm not subscribed to this
list.


==
Relicense include/filenames.h as LGPL2+.

include/ChangeLog:

2016-09-24  Eli Zaretskii  

* filenames.h: Relicense as LGPL 2.1 or later.

diff --git a/include/filenames.h b/include/filenames.h
index 44553e4..b933bcf 100644
--- a/include/filenames.h
+++ b/include/filenames.h
@@ -10,18 +10,19 @@
 This file is part of BFD, the Binary File Descriptor library.
 
 This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2 of the License, or
-(at your option) any later version.
+it under the terms of the GNU Lesser General Public License as
+published by the Free Software Foundation; either version 2.1 of the
+License, or (at your option) any later version.
 
 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
+GNU Lesser General Public License for more details.
 
-You should have received a copy of the GNU General Public License
-along with this program; if not, write to the Free Software
-Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, 
USA.  */
+You should have received a copy of the GNU Lesser General Public
+License along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+02110-1301, USA.  */
 
 #ifndef FILENAMES_H
 #define FILENAMES_H

Re: [C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)

[Jason Merrill]

On Tue, Sep 27, 2016 at 3:57 AM, Jakub Jelinek  wrote:
> @@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
> +  if (TREE_CODE (*p) == STATEMENT_LIST)
>  {
> +  tree_stmt_iterator i = tsi_last (*p);
>tsi_link_after (, t, TSI_NEW_STMT);
>  }
> +  else
> +{
> +  tree list = NULL_TREE;
> +  append_to_statement_list_force (*p, );
> +  append_to_statement_list (t, );
> +  *p = list;
> +}

Can't you replace all of this with

append_to_statement_list (t, p);

?

OK either way.

Jason

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Tue, Sep 27, 2016 at 10:48:50AM -0400, Jason Merrill wrote:
> On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:
> > On Tue, 27 Sep 2016, Jakub Jelinek wrote:
> >
> >> Just compare that to the number of real bugs the warning found in gcc
> >> codebase.  It is really worth it for -Wextra.
> >
> > All those bugs would also have been found as well when it had simply
> > accepted
> >   /fall.*thr/i
> > anywhere in the preceding comment on one line.  But all the recent
> > spelling changes of comments to cater for the strictness exactly shows how
> > misguided that is.  The above would accept "Don't fall through" as well.
> > I say: so what?
> 
> I agree.

All right, I'm not opposed to making the comment parsing more benevolent.
We still should have enough time to fine-tune it.

Marek

Re: [PATCH] [ARC] Fix emitting jump tables for ARCv2

[Andrew Burgess]

* Claudiu Zissulescu  [2016-04-26 13:28:58 
+0200]:

> The compact casesi option only make sens for ARCv1 cores. For ARCv2 cores we
> use the regular expansion.
> 
> OK to apply?

Claudiu,

Could you rebase this onto the current head please.  I couldn't get
this to merge cleanly.

Thanks,
Andrew


> Claudiu
> 
> gcc/
> 2016-04-26  Claudiu Zissulescu  
> 
>   * common/config/arc/arc-common.c (arc_option_optimization_table):
>   Disable compact casesi as default option.
>   * config/arc/arc.c (arc_override_options): Enable compact casesi
>   option for non-ARCv2 cores.
>   * config/arc/arc.md (movsi_insn): Use @pcl relocation.
>   (movsi_ne): Update assembly printing pattern.
>   (casesi_load): Use short ld instruction.
> ---
>  gcc/common/config/arc/arc-common.c |  1 -
>  gcc/config/arc/arc.c   |  7 +++
>  gcc/config/arc/arc.md  | 11 +++
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/common/config/arc/arc-common.c 
> b/gcc/common/config/arc/arc-common.c
> index 64fb053..17cc1bd 100644
> --- a/gcc/common/config/arc/arc-common.c
> +++ b/gcc/common/config/arc/arc-common.c
> @@ -56,7 +56,6 @@ static const struct default_options 
> arc_option_optimization_table[] =
>  { OPT_LEVELS_ALL, OPT_mbbit_peephole, NULL, 1 },
>  { OPT_LEVELS_SIZE, OPT_mq_class, NULL, 1 },
>  { OPT_LEVELS_SIZE, OPT_mcase_vector_pcrel, NULL, 1 },
> -{ OPT_LEVELS_SIZE, OPT_mcompact_casesi, NULL, 1 },
>  { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>  
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 6f2136e..be55c99 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -812,6 +812,13 @@ arc_override_options (void)
>if (arc_size_opt_level == 3)
>  optimize_size = 1;
>  
> +  /* Compact casesi is not a valid option for ARCv2 family, disable
> + it.  */
> +  if (TARGET_V2)
> +TARGET_COMPACT_CASESI = 0;
> +  else if (optimize_size == 1)
> +TARGET_COMPACT_CASESI = 1;
> +
>if (flag_pic)
>  target_flags |= MASK_NO_SDATA_SET;
>  
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index 718443b..aec4b37 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -713,7 +713,7 @@
> ror %0,((%1*2+1) & 0x3f) ;6
> mov%? %0,%1   ;7
> add %0,%S1;8
> -   * return arc_get_unalign () ? \"add %0,pcl,%1-.+2\" : \"add %0,pcl,%1-.\";
> +   add %0,pcl,%1@pcl
> mov%? %0,%S1%&;10
> mov%? %0,%S1  ;11
> ld%?%U1 %0,%1%&   ;12
> @@ -3467,8 +3467,8 @@
>""
>"@
>   * current_insn_predicate = 0; return \"sub%?.ne %0,%0,%0%&\";
> -mov_s.ne %0,%1
> -mov_s.ne %0,%1
> +* current_insn_predicate = 0; return \"mov%?.ne %0,%1\";
> +* current_insn_predicate = 0; return \"mov%?.ne %0,%1\";
>   mov.ne %0,%1
>   mov.ne %0,%S1"
>[(set_attr "type" "cmove")
> @@ -3777,7 +3777,10 @@
>switch (GET_MODE (diff_vec))
>  {
>  case SImode:
> -  return \"ld.as %0,[%1,%2]%&\";
> +  if ((which_alternative == 0) && TARGET_CODE_DENSITY)
> + return \"ld_s.as %0,[%1,%2]%&\";
> +  else
> + return \"ld.as %0,[%1,%2]%&\";
>  case HImode:
>if (ADDR_DIFF_VEC_FLAGS (diff_vec).offset_unsigned)
>   return \"ld%_.as %0,[%1,%2]\";
> -- 
> 1.9.1
> 

Re: Implement -Wimplicit-fallthrough (version 9)

[Jason Merrill]

On Tue, Sep 27, 2016 at 9:56 AM, Michael Matz  wrote:
> On Tue, 27 Sep 2016, Jakub Jelinek wrote:
>
>> Just compare that to the number of real bugs the warning found in gcc
>> codebase.  It is really worth it for -Wextra.
>
> All those bugs would also have been found as well when it had simply
> accepted
>   /fall.*thr/i
> anywhere in the preceding comment on one line.  But all the recent
> spelling changes of comments to cater for the strictness exactly shows how
> misguided that is.  The above would accept "Don't fall through" as well.
> I say: so what?

I agree.

Jason

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Jason Merrill]

On Tue, Sep 27, 2016 at 10:28 AM, Bernd Edlinger
 wrote:
> On 09/27/16 16:10, Florian Weimer wrote:
>> * Bernd Edlinger:
>>
 “0 << 0” is used in a similar context, to create a zero constant for a
 multi-bit subfield of an integer.

 This example comes from GDB, in bfd/elf64-alpha.c:

 |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);

>>>
>>> Of course that is not a boolean context, and will not get a warning.
>>>
>>> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
>>>
>>> Maybe 1 and 0 come from macro expansion
>>
>> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
>> patch, then?
>
> I am not sure if it was a good idea.
>
> I saw, we had code of the form
> bool flag = 1 << 2;
>
> another value LOOKUP_PROTECT is  1 << 0, and
> bool flag = 1 << 0;
>
> would at least not overflow the allowed value range of a boolean.

Assigning a bit mask to a bool variable is still probably not what was
intended, even if it doesn't change the value.

Jason

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/27/16 16:10, Florian Weimer wrote:
> * Bernd Edlinger:
>
>>> “0 << 0” is used in a similar context, to create a zero constant for a
>>> multi-bit subfield of an integer.
>>>
>>> This example comes from GDB, in bfd/elf64-alpha.c:
>>>
>>> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>>>
>>
>> Of course that is not a boolean context, and will not get a warning.
>>
>> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
>>
>> Maybe 1 and 0 come from macro expansion
>
> But what's the intent of treating 1 << 0 and 0 << 0 differently in the
> patch, then?
>

I am not sure if it was a good idea.

I saw, we had code of the form
bool flag = 1 << 2;

another value LOOKUP_PROTECT is  1 << 0, and
bool flag = 1 << 0;

would at least not overflow the allowed value range of a boolean.


Bernd.

Re: Implement -Wimplicit-fallthrough (version 9)

[Marek Polacek]

On Tue, Sep 27, 2016 at 03:53:25PM +0200, Bernd Schmidt wrote:
> What's the ratio of comments "fixed" to actual bugs found? IMO this is not
> something we should inflict on users unasked.

One might argue that users actually *asked* for this by turning on -Wextra.

Marek

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Florian Weimer]

* Bernd Edlinger:

>> “0 << 0” is used in a similar context, to create a zero constant for a
>> multi-bit subfield of an integer.
>>
>> This example comes from GDB, in bfd/elf64-alpha.c:
>>
>> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>>
>
> Of course that is not a boolean context, and will not get a warning.
>
> Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
>
> Maybe 1 and 0 come from macro expansion

But what's the intent of treating 1 << 0 and 0 << 0 differently in the
patch, then?

Re: [PATCH] [ARC] Add simple shift/rotate ops.

[Andrew Burgess]

* Claudiu Zissulescu  [2016-06-17 11:13:08 
+0200]:

> Basic ARC cpus are having only simple shift operations. Here they are.
> 
> OK to apply?

This looks good to me.

Thanks,
Andrew



> 
> gcc/
> 2016-06-09  Claudiu Zissulescu  
> 
>   * config/arc/arc.md (*rotrsi3_cnt1): New pattern,
>   (*ashlsi2_cnt1, *lshrsi3_cnt1, *ashrsi3_cnt1): Likewise.
> ---
>  gcc/config/arc/arc.md | 40 
>  1 file changed, 40 insertions(+)
> 
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index 852f0e0..a095ba1 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -6219,6 +6219,46 @@
>  (zero_extract:SI (match_dup 1) (match_dup 5) (match_dup 
> 7)))])
> (match_dup 1)])
>  
> +(define_insn "*rotrsi3_cnt1"
> +  [(set (match_operand:SI 0 "dest_reg_operand" "=w")
> + (rotatert:SI (match_operand:SI 1 "register_operand" "c")
> +  (const_int 1)))]
> +  ""
> +  "ror %0,%1%&"
> +  [(set_attr "type" "shift")
> +   (set_attr "predicable" "no")
> +   (set_attr "length" "4")])
> +
> +(define_insn "*ashlsi2_cnt1"
> +  [(set (match_operand:SI 0 "dest_reg_operand"   "=Rcqq,w")
> + (ashift:SI (match_operand:SI 1 "register_operand" "Rcqq,c")
> +(const_int 1)))]
> +  ""
> +  "asl%? %0,%1%&"
> +  [(set_attr "type" "shift")
> +   (set_attr "iscompact" "maybe,false")
> +   (set_attr "predicable" "no,no")])
> +
> +(define_insn "*lshrsi3_cnt1"
> +  [(set (match_operand:SI 0 "dest_reg_operand" "=Rcqq,w")
> + (lshiftrt:SI (match_operand:SI 1 "register_operand" "Rcqq,c")
> +  (const_int 1)))]
> +  ""
> +  "lsr%? %0,%1%&"
> +  [(set_attr "type" "shift")
> +   (set_attr "iscompact" "maybe,false")
> +   (set_attr "predicable" "no,no")])
> +
> +(define_insn "*ashrsi3_cnt1"
> +  [(set (match_operand:SI 0 "dest_reg_operand" "=Rcqq,w")
> + (ashiftrt:SI (match_operand:SI 1 "register_operand" "Rcqq,c")
> +  (const_int 1)))]
> +  ""
> +  "asr%? %0,%1%&"
> +  [(set_attr "type" "shift")
> +   (set_attr "iscompact" "maybe,false")
> +   (set_attr "predicable" "no,no")])
> +
>  ;; include the arc-FPX instructions
>  (include "fpx.md")
>  
> -- 
> 1.9.1
> 

[PATCH, PING] DWARF: remove pessimistic DWARF version checks for imported entities

[Pierre-Marie de Rodat]

Hello,

On 08/18/2016 10:12 PM, Jeff Law wrote:

Jakub -- you want to take a looksie at this one?  Jason is on PTO for
the next couple weeks.  I'm not well versed enough to verify that
d_i_m_o_d_1 has all the right checks or not.


Ping for the patch submitted at 
. Thank you in 
advance!


--
Pierre-Marie de Rodat

[PATCH, PING*2] DWARF: space-optimize loc. descr. for integer literals on 32-bit targets

[Pierre-Marie de Rodat]

Hello,

Ping for the patch submitted at 
. Thanks!


--
Pierre-Marie de Rodat

Re: [PATCH, PING] DWARF: process all TYPE_DECL nodes when iterating on scopes

[Pierre-Marie de Rodat]

On 09/07/2016 11:30 AM, Richard Biener wrote:

That said, with the idea of early debug in place and thus giving
more responsibility to the frontends I wonder in what order the Ada
FE calls debug_hooks.early_global_decl ()? Maybe it's the middle-end
and it should arrange for a more natural order on function nests.  So
I'd appreciate if you can investigate this side of the issue a bit,
that is, simply avoid the bad ordering.


I investigated this track: it’s not the Ada front-end’s calls to the 
debug_hooks.early_global_decl that matter, but actually the ones in 
cgraphunit.c (symbol_table::finalize_compilation_unit).


The new patch attached tries to fix the call order. Bootstrapping and 
regtesting on x86_64-linux were clean except one testcase where the 
debug output changed legitimally (matcher updated). Ok to commit? Thank 
you in advance!


--
Pierre-Marie de Rodat
>From 2508b2be4d58ae29cc0093688c50fefea15f6934 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat 
Date: Fri, 23 Sep 2016 18:16:07 +0200
Subject: [PATCH] DWARF: fix scoping for descriptions of local types

In Ada, it is possible to have nested subprograms in the following
configuration:

procedure Parent is
   type T;
   [...]
   procedure Child (Value : T) is
   begin
  [...]
   end Child;
begin
   [...]
end Parent;

As we currently generate debugging information for Child first before
Parent, the debug info for T appears in global scope since the DIE for
Parent does not exist yet.

First, this patch makes sure nested function cgraph_node's are created
after their parents'.  Then, it reverses the iteration on the symbol
table that calls the early_global_decl debug hook so that debug info for
nested functions is generated after their parents'.

gcc/ChangeLog:

	* cgraph.h (symbol_table::last_function_with_gimple_body,
	symbol_table::previous_function_with_gimple_body): New methods.
	(REVERSE_FOR_EACH_FUNCTION_WITH_GIMPLE_BODY): New iteration macro.
	* cgraph.c (symbol_table::call_cgraph_duplication_hooks): Adjust so
	that nested functions have their cgraph_node created after their
	parents'.
	* cgraphunit.c (symbol_table::finalize_compilation_unit): Call the
	early_global_decl debug hook on last functions in the symtab list (i.e.
	older ones first).

gcc/testsuite/

	* gcc/testsuite/g++.dg/debug/dwarf2/auto1.C: Adjust pattern to
	accomodate new DIEs output order.
---
 gcc/cgraph.c  | 12 --
 gcc/cgraph.h  | 40 +++
 gcc/cgraphunit.c  |  5 ++--
 gcc/testsuite/g++.dg/debug/dwarf2/auto1.C |  2 +-
 4 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index b702a7c..78d1a85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -492,6 +492,14 @@ symbol_table::call_cgraph_duplication_hooks (cgraph_node *node,
 cgraph_node *
 cgraph_node::create (tree decl)
 {
+  cgraph_node *origin = NULL;
+
+  /* If DECL is local to a function, make sure we have a node for this function
+ first so that the symbol table has parent functions created before their
+ children.  This helps debug information generation.  */
+  if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+
   cgraph_node *node = symtab->create_empty ();
   gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
 
@@ -507,9 +515,9 @@ cgraph_node::create (tree decl)
 
   node->register_symbol ();
 
-  if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+  if (origin != NULL)
 {
-  node->origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+  node->origin = origin;
   node->next_nested = node->origin->nested;
   node->origin->nested = node;
 }
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index ecafe63..6b1181c 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -2097,6 +2097,13 @@ public:
   /* Return next reachable static variable with initializer after NODE.  */
   inline cgraph_node *next_function_with_gimple_body (cgraph_node *node);
 
+  /* Return last function with body defined.  */
+  cgraph_node *last_function_with_gimple_body (void);
+
+  /* Return previous reachable static variable with initializer before
+ NODE.  */
+  inline cgraph_node *previous_function_with_gimple_body (cgraph_node *node);
+
   /* Register HOOK to be called with DATA on each removed edge.  */
   cgraph_edge_hook_list *add_edge_removal_hook (cgraph_edge_hook hook,
 		void *data);
@@ -2775,6 +2782,36 @@ symbol_table::next_function_with_gimple_body (cgraph_node *node)
   return NULL;
 }
 
+/* Return last function with body defined.  */
+inline cgraph_node *
+symbol_table::last_function_with_gimple_body (void)
+{
+  cgraph_node *res = NULL;
+  symtab_node *node;
+
+  for (node = nodes; node; node = node->next)
+{
+  cgraph_node *cn = dyn_cast  (node);
+  if (cn && 

Re: Implement -Wimplicit-fallthrough (version 9)

[Bernd Schmidt]

On 09/27/2016 03:49 PM, Jakub Jelinek wrote:

On Tue, Sep 27, 2016 at 03:48:07PM +0200, Bernd Schmidt wrote:

On 09/27/2016 02:01 PM, Marek Polacek wrote:

On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote:

On 09/27/2016 01:51 PM, Marek Polacek wrote:

But the C/C++ keywords are all English, too; lint tools only accept English,
and so it wouldn't seem unreasonable to only accept English keywords in the
comments.  And in any case, I don't see how a compiler can be expected to
be able to parse non-English languages.


It isn't. But it can also be reasonably by expected not to warn about things
that are valid according to the language specification and are frequently
used.


Ok, but note that the warning is in -Wextra, not enabled by default/-Wall.


I think it's problematic enough that it needs to be removed from -Wextra as
well. The latest ia64 backend patch shows that clearly IMO.


Just compare that to the number of real bugs the warning found in gcc
codebase.  It is really worth it for -Wextra.


What's the ratio of comments "fixed" to actual bugs found? IMO this is 
not something we should inflict on users unasked.



Bernd

Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops

[Bernd Edlinger]

On 09/27/16 14:49, Florian Weimer wrote:
> * Jason Merrill:
>
>> On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger
>>  wrote:
>>> This patch makes -Wint-in-bool-context warn on suspicious integer left
>>> shifts, when the integer is signed, which is most likely some kind of
>>> programming error, for instance using "<<" instead of "<".
>>>
>>> The warning is motivated by the fact, that an overflow on integer shift
>>> left is undefined behavior, even if gcc won't optimize the shift based
>>> on the undefined behavior.
>>>
>>> So in absence of undefined behavior the boolean result does not depend
>>> on the shift value, thus the whole shifting is pointless.
>>
>> It's pointless for unsigned integers, too; why not warn for them as
>> well?  And why not warn for 0 << 0 and 1 << 0, which are just as
>> pointless?
>
> “1 << 0“ is often used in a sequence of flag mask definitions.  This
> example is from :
>
> | /* Terminal control structure.  */
> | struct termios
> | {
> |   /* Input modes.  */
> |   tcflag_t c_iflag;
> | #define IGNBRK  (1 << 0)/* Ignore break condition.  */
> | #define BRKINT  (1 << 1)/* Signal interrupt on break.  */
> | #define IGNPAR  (1 << 2)/* Ignore characters with parity errors.  */
> | #define PARMRK  (1 << 3)/* Mark parity and framing errors.  */
>
> “0 << 0” is used in a similar context, to create a zero constant for a
> multi-bit subfield of an integer.
>
> This example comes from GDB, in bfd/elf64-alpha.c:
>
> |   insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>

Of course that is not a boolean context, and will not get a warning.

Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".

Maybe 1 and 0 come from macro expansion



Bernd.