[PATCH, committed] PR 78976 Testcase changes

2017-01-04 Thread Janne Blomqvist
Hi,

As r244011 had to be reverted, this change adds back the testcase
changes that are needed due to r244003.

Committed as obvious.

2017-01-04  Janne Blomqvist  

PR fortran/78534
PR fortran/78976
* gfortran.dg/dependency_49.f90: Change scan-tree-dump-times
due to gfc_trans_string_copy change to avoid -Wstringop-overflow.
* gfortran.dg/transfer_intrinsic_1.f90: Change
scan-tree-dump-times due to gfc_trans_string_copy change to
avoid -Wstringop-overflow.

Index: gfortran.dg/dependency_49.f90
===
--- gfortran.dg/dependency_49.f90   (revision 244047)
+++ gfortran.dg/dependency_49.f90   (working copy)
@@ -11,4 +11,4 @@ program main
   a%x = a%x(2:3)
   print *,a%x
 end program main
-! { dg-final { scan-tree-dump-times "__var_1" 4 "original" } }
+! { dg-final { scan-tree-dump-times "__var_1" 3 "original" } }
Index: gfortran.dg/transfer_intrinsic_1.f90
===
--- gfortran.dg/transfer_intrinsic_1.f90(revision 244047)
+++ gfortran.dg/transfer_intrinsic_1.f90(working copy)
@@ -14,4 +14,4 @@ subroutine BytesToString(bytes, string)
 character(len=*) :: string
 string = transfer(bytes, string)
   end subroutine
-! { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "original" } }
+! { dg-final { scan-tree-dump-times "MIN_EXPR" 2 "original" } }


-- 
Janne Blomqvist


Update configure deps, remove stray \xA0 in picflag.m4, regen

2017-01-04 Thread Alan Modra
Also fix a stray changelog entry.  Some of the regen here is due to
previous changes not being regenerated properly, in part due to the
missing configure dependencies.

I noticed the picflag.m4 high bit set char and after fixing that found
that some configure files didn't automatically update in maintainer
mode.  Which meant they lacked the proper dependencies.  I haven't
updated every Makefile.in/configure/config.h etc. file in the
sources.  There were a few more with lines moved, copyright dates
changed and other trivial changes.

Committed as obvious after bootstrapping on x86_64-linux.

* configure: Regenerate.
config/
* picflag.m4: Remove stray \xA0 in comment.
gcc/
* Makefile.in (aclocal_deps): Update and order as per aclocal.m4.
* configure: Regenerate.
* config.in: Regenerate.
libada/
* Makefile.in (configure_deps): Update and order as per
configure.ac sinclude.
* configure: Regenerate.
libgcc/
* Makefile.in (configure_deps): Update.
* configure: Regenerate.
libiberty/
* Makefile.in (configure_deps): Update.
* configure: Regenerate.
libitm/
* Makefile.in: Regenerate.
* testsuite/Makefile.in: Regenerate.

diff --git a/ChangeLog b/ChangeLog
index a6241ae..db4125a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-01-04  Alan Modra  
+
+   * configure: Regenerate.
+
 2016-12-29  Ben Elliston  
 
* config.sub: Import latest version.
@@ -18,8 +22,7 @@
 
 2016-12-01  Ma Jiang  
 
-   * config/acx.m4: Change "tail +16c" to "tail -c +17".
-   * configure: Regenerated.
+   * configure: Regenerate.
 
 2016-12-01  Matthias Klose  
 
diff --git a/config/ChangeLog b/config/ChangeLog
index c48f6ba..9623063 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,7 @@
+2017-01-04  Alan Modra  
+
+   * picflag.m4: Remove stray \xA0 in comment.
+
 2016-12-12  Rainer Orth  
 
* hwcaps.m4: New file.
@@ -14,6 +18,10 @@
* bootstrap-asan.mk: Replace LSAN_OPTIONS=detect_leaks=0 with
ASAN_OPTIONS=detect_leaks=0:use_odr_indicator=1.
 
+2016-12-01  Ma Jiang  
+
+   * acx.m4: Change "tail +16c" to "tail -c +17".
+
 2016-12-01  Matthias Klose  
 
* pkg.m4: Remove.
diff --git a/config/picflag.m4 b/config/picflag.m4
index 2f5b972..8b106f9 100644
--- a/config/picflag.m4
+++ b/config/picflag.m4
@@ -12,7 +12,7 @@ case "${$2}" in
  # If we are using a compiler supporting mdynamic-no-pic
  # and the option has been tested as safe to add, then cancel
  # it here, since the code generated is incompatible with shared
- # libs.
+ # libs.
  *-mdynamic-no-pic*) $1='-fno-common -mno-dynamic-no-pic' ;;
  *) $1=-fno-common ;;
esac
diff --git a/configure b/configure
index b8d6096..ba5f8bf 100755
--- a/configure
+++ b/configure
@@ -6033,7 +6033,7 @@ target_elf=no
 case $target in
   *-darwin* | *-aix* | *-cygwin* | *-mingw* | *-aout* | *-*coff* | \
   *-msdosdjgpp* | *-vms* | *-wince* | *-*-pe* | \
-  alpha*-dec-osf* | hppa[12]*-*-hpux* | \
+  alpha*-dec-osf* | *-interix* | hppa[12]*-*-hpux* | \
   nvptx-*-none)
 target_elf=no
 ;;
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 6b2888f..1feceea 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-01-04  Alan Modra  
+
+   * Makefile.in (aclocal_deps): Update and order as per aclocal.m4.
+   * configure: Regenerate.
+   * config.in: Regenerate.
+
 2017-01-03  Jeff Law  
 
PR tree-optimizatin/78856
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 5c29edb..b9773f4 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1799,21 +1799,23 @@ aclocal_deps = \
$(srcdir)/../lt~obsolete.m4 \
$(srcdir)/../config/acx.m4 \
$(srcdir)/../config/codeset.m4 \
-   $(srcdir)/../config/extensions.m4 \
-   $(srcdir)/../config/gettext-sister.m4 \
+   $(srcdir)/../config/depstand.m4 \
+   $(srcdir)/../config/dfp.m4 \
$(srcdir)/../config/gcc-plugin.m4 \
+   $(srcdir)/../config/gettext-sister.m4 \
$(srcdir)/../config/iconv.m4 \
$(srcdir)/../config/lcmessage.m4 \
+   $(srcdir)/../config/lead-dot.m4 \
$(srcdir)/../config/lib-ld.m4 \
$(srcdir)/../config/lib-link.m4 \
$(srcdir)/../config/lib-prefix.m4 \
+   $(srcdir)/../config/mmap.m4 \
$(srcdir)/../config/override.m4 \
+   $(srcdir)/../config/picflag.m4 \
$(srcdir)/../config/progtest.m4 \
 $(srcdir)/../config/stdint.m4 \
-   $(srcdir)/../config/unwind_ipinfo.m4 \
$(srcdir)/../config/warnings.m4 \
-   $(srcdir)/../config/dfp.m4 \
-   $(srcdir)/../config/mmap.m4 \
+   $(srcdir)/../config/zlib.m4 \
$(srcdir)/acinclude.m4
 
 $(srcdir)/configure: @MAINT@ $(srcdir)/configure.ac $(srcdir)/aclocal.m4
diff --git a/gcc/config.in b/gcc/config.in
index e02d33e..1959dd7 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -37,13 +

Re: [PATCH] Optimize X << Y with low bits of Y known to be 0 (PR tree-optimization/71563, take 2)

2017-01-04 Thread Richard Biener
On Thu, 29 Dec 2016, Jakub Jelinek wrote:

> Hi!
> 
> On Tue, Dec 20, 2016 at 09:45:03PM +0100, Jakub Jelinek wrote:
> > > Note that you can write (shift @0 SSA_NAME@1) in the pattern instead of a
> > > separate test.
> > 
> > That is what I tried first, but there is some bug in genmatch.c that
> > prevents it.  The:
> >  (for vec (VECTOR_CST CONSTRUCTOR)
> >   (simplify
> >(shiftrotate @0 vec@1)
> > results in case SSA_NAME: being added to a switch:
> > case SSA_NAME:
> >   if (do_valueize (valueize, op1) != NULL_TREE)
> > {
> >   gimple *def_stmt = SSA_NAME_DEF_STMT (op1);
> >   if (gassign *def = dyn_cast  (def_stmt))
> > switch (gimple_assign_rhs_code (def))
> >   {
> >   case CONSTRUCTOR:
> > and the SSA_NAME@1 in another simplification resulted in another
> > case SSA_NAME:
> > into the same switch (rather than appending to the case SSA_NAME).
> 
> And here is the corresponding updated version of the patch:

Ok.

Thanks,
Richard.

> 2016-12-29  Jakub Jelinek  
> 
>   PR tree-optimization/71563
>   * match.pd: Simplify X << Y into X if Y is known to be 0 or
>   out of range value - has low bits known to be zero.
> 
>   * gcc.dg/tree-ssa/pr71563.c: New test.
> 
> --- gcc/match.pd.jj   2016-12-21 10:00:10.809244456 +0100
> +++ gcc/match.pd  2016-12-29 21:56:56.891858831 +0100
> @@ -1515,6 +1515,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (if (tem)
>   (shiftrotate @0 { tem; }))
>  
> +/* Simplify X << Y where Y's low width bits are 0 to X, as only valid
> +   Y is 0.  Similarly for X >> Y.  */
> +#if GIMPLE
> +(for shift (lshift rshift)
> + (simplify
> +  (shift @0 SSA_NAME@1)
> +   (if (INTEGRAL_TYPE_P (TREE_TYPE (@1)))
> +(with {
> +  int width = ceil_log2 (element_precision (TREE_TYPE (@0)));
> +  int prec = TYPE_PRECISION (TREE_TYPE (@1));
> + }
> + (if ((get_nonzero_bits (@1) & wi::mask (width, false, prec)) == 0)
> +  @0)
> +#endif
> +
>  /* Rewrite an LROTATE_EXPR by a constant into an
> RROTATE_EXPR by a new constant.  */
>  (simplify
> --- gcc/testsuite/gcc.dg/tree-ssa/pr71563.c.jj2016-12-29 
> 21:56:12.668414342 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr71563.c   2016-12-29 21:56:12.668414342 
> +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/71563 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +void link_error (void);
> +
> +void
> +foo (int k)
> +{
> +  int t = 1 << ((1 / k) << 8);
> +  if (t != 1)
> +link_error ();
> +}
> +
> +void
> +bar (int k, int l)
> +{
> +  int t = l << (k << 8);
> +  if (t != l)
> +link_error ();
> +}
> +
> +/* { dg-final { scan-tree-dump-not "link_error" "optimized" } } */
> 
> 
>   Jakub
> 
> 

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


Re: [PATCH] Do not sanitize in lower_omp_target context (PR, sanitizer/78815).

2017-01-04 Thread Martin Liška
PING^1

On 12/16/2016 01:04 PM, Martin Liška wrote:
> Currently, use-after-scope relies on fact that entry point of 
> gimplify_decl_expr
> is gimplify_function_tree. Fixed by checking if asan_poisoned_variables is 
> non-null.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 


Re: [PATCH] Fix late dwarf generated early from optimized out globals

2017-01-04 Thread Richard Biener
On Wed, 28 Dec 2016, Andreas Tobler wrote:

> On 28.12.16 19:24, Richard Biener wrote:
> > On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
> >  wrote:
> > > On 16.09.16 13:30, Richard Biener wrote:
> > > > On Thu, 15 Sep 2016, Richard Biener wrote:
> > > > 
> > > > > 
> > > > > This addresses sth I needed to address with the early LTO debug
> > > patches
> > > > > (you might now figure I'm piecemail merging stuff from that patch).
> > > > > 
> > > > > When the cgraph code optimizes out a global we call the
> > > late_global_decl
> > > > > debug hook to eventually add a DW_AT_const_value to its DIE (we
> > > don't
> > > > > really expect a location as that will be invalid after optimizing
> > > out
> > > > > and will be pruned).
> > > > > 
> > > > > With the early LTO debug patches I have introduced a
> > > early_dwarf_finished
> > > > > flag (mainly for consistency checking) and I figured I can use that
> > > to
> > > > > detect the call to the late hook during the early phase and provide
> > > > > the following cleaned up variant of avoiding to create locations
> > > that
> > > > > require later pruning (which doesn't work with emitting the early
> > > DIEs).
> > > > > 
> > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > 
> > > > > I verified it does the correct thing for a unit like
> > > > > 
> > > > > static const int i = 2;
> > > > > 
> > > > > (but ISTR we do have at least one testcase in the testsuite as
> > > well).
> > > > > 
> > > > > Will commit if testing finishes successfully.
> > > > 
> > > > Ok, so it showed issues when merging that back to early-LTO-debug.
> > > > Turns out in LTO we never call early_finish and thus
> > > early_dwarf_finished
> > > > was never set.  Also dwarf2out_late_global_decl itself is a better
> > > > place to constrain generating locations.
> > > > 
> > > > The following variant is in very late stage of testing.
> > > > 
> > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
> > > > with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
> > > > testing in progress.
> > > 
> > > Any chance to backport this commit (r240228) to 6.x?
> > > It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
> > > The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
> > > the bootstrap comparison failure I faced.
> > 
> > Did you analyze the bootstrap miscompare?  I suspect the patch merely papers
> > over the problem.
> 
> gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
> prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841, line
> 253
> 
> 
> The objdump -dSx diff on the non stripped object looked always more or less
> the same, a rodata offset which was different.
> 
> -   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
> +   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410

Hmm, sounds like a constant pool entry was created by -g at a different
time (and thus offset) from regular compilation.  So yes, the patch
in question should have the affect to "fix" this.

Note that I later changed the fix with

2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.
* ipa.c (symbol_table::remove_unreachable_nodes): When not in
WPA signal symbol removal to the debuginfo machinery.
* dwarf2out.c (dwarf2out_late_global_decl): Instead of
using early_finised to guard the we're called for symbol
removal case look at the symtabs definition flag.
(gen_variable_die): Remove redundant check.

(the dwarf2out_late_global_decl and analyze_functions part are
relevant).

That should be the fix to backport (avoiding the early_dwarf_finished
part).

Richard.

> > Was that regular bootstrap (with or without compare-debug)?
> 
> Hm, regular bootstrap, --with-build-config=bootstrap-debug?
> 
> Or do you have something else in mind?
> 
> I rerun on 6.x w/o 'patch' now. Hopefully in 12h I have something.
> 
> Btw, I bisected on trunk, r240227 was nok, the next, r240228 was ok. Then I
> took the part from r240228 to the 6.x branch and my comparison issue was gone.
> 
> If I miss something you need to know pls let me know.
>
> TIA,
> Andreas
> 
> 

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


Re: [PATCH] Do not sanitize in lower_omp_target context (PR, sanitizer/78815).

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 10:19:28AM +0100, Martin Liška wrote:
> PING^1
> 
> On 12/16/2016 01:04 PM, Martin Liška wrote:
> > Currently, use-after-scope relies on fact that entry point of 
> > gimplify_decl_expr
> > is gimplify_function_tree. Fixed by checking if asan_poisoned_variables is 
> > non-null.
> > 
> > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> > 
> > Ready to be installed?

Looking at asan_poisoned_variables, my preference would be to guard:
  asan_poisoned_variables = new hash_set ();
with
if (asan_sanitize_use_after_scope ()
&& !asan_no_sanitize_address_p ())
the delete asan_poisoned_variables; with if (asan_poisoned_variables)
and all the poisoning stuff in the gimplifier also with if
(asan_poisoned_variables) and no need to repeat there the 
asan_sanitize_use_after_scope
() and !asan_no_sanitize_address_p () tests.
  if (asan_poisoned_variables != NULL
  && asan_poisoned_variables->contains (t))
is already fine,
  if (asan_sanitize_use_after_scope ()
  && !asan_no_sanitize_address_p ()
  && !is_vla
  && TREE_ADDRESSABLE (decl)
  && !TREE_STATIC (decl)
  && !DECL_HAS_VALUE_EXPR_P (decl)
  && dbg_cnt (asan_use_after_scope))
should replace the first 2 conditions with asan_poisoned_variables,
  if (asan_sanitize_use_after_scope ()
  && asan_used_labels != NULL
  && asan_used_labels->contains (label))
asan_poison_variables (asan_poisoned_variables, false, pre_p);
should replace asan_sanitize_use_after_scope () with
asan_poisoned_variables.  IMHO no need to add comments, especially not one
mentioning omp lowering - the gimplifier is called from lots of various
places.

Jakub


Re: [PATCH, GCC/testsuite/ARM, ping] Fix empty_fiq_handler target selector

2017-01-04 Thread Kyrill Tkachov


On 03/01/17 17:22, Thomas Preudhomme wrote:

Happy new year!

Ping?

Best regards,

Thomas

On 09/12/16 15:28, Thomas Preudhomme wrote:

Hi,

The current target selector for empty_fiq_handler.c testcase skips the test when
targeting Thumb mode on a device with ARM execution state. Because it checks
Thumb mode by looking for an -mthumb option it fails to work when GCC was
configured with --with-mode=thumb. It is also too restrictive because interrupt
handler can be compiled in Thumb-2. This patch checks the arm_thumb1 effective
target instead of the -mthumb flag to fix both issues.

ChangeLog entry is as follows:


*** gcc/testsuite/ChangeLog ***

2016-12-09  Thomas Preud'homme 

* gcc.target/arm/empty_fiq_handler: Skip instead if targeting Thumb-1
on a non Thumb-only target.


Tested with GCC built for ARMv5T and ARMv7-A with --with-mode=thumb and
--with-mode=arm and for ARMv6S-M with --with-mode=thumb:

* test pass in all cases for ARMv5T and ARMv7-A with -marm
* test pass in all cases for ARMv6S-M and ARMv7-A with -mthumb
* test pass without option when defaulting to ARM for ARMv5T and ARMv7-A
* test pass without option when defaulting to Thumb for ARMv6S-M and ARMv7-A
* test is unsupported with -marm for ARMv5T
* test is unsupported without option when defaulting to Thumb for ARMv5T

Is this ok for stage3?

Best regards,

Thomas


fix_empty_fiq_handler_testcase_selector.patch


diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c 
b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
index 
8313f2199122be153a737946e817a5e3bee60372..69bb0669dd416e1fcb015c278d62961d071fc42f
 100644
--- a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
+++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
@@ -1,5 +1,4 @@
-/* { dg-do compile } */
-/* { dg-skip-if "" { ! arm_cortex_m } { "-mthumb" } } */
+/* { dg-do compile { target { {! arm_thumb1 } || arm_cortex_m } } } */
  


I think you want to add a space between the '{' and '!'.
Otherwise this is ok.
Thanks,
Kyrill


  /* Below code used to trigger an ICE due to missing constraints for
 sp = fp + cst pattern.  */




Re: [PATCH] Use the middle-end boolean_type_node

2017-01-04 Thread Janne Blomqvist
On Tue, Jan 3, 2017 at 9:20 PM, FX  wrote:
>> The regression with 8 bit boolean types surfaced with the z10 machine. The 
>> ABI is much older. Since
>> we cannot change it anymore we should rather talk to the hardware guys to 
>> add the instruction we
>> need. So for now we probably have to live with the regression in the Fortran 
>> cases since as I
>> understand it your change fixes an actual problem.
>
> As far as I understand (and Jane will correct me if I am wrong),

Sure, allow me to correct you: It's Janne, with two n in the middle. :)

> the patch does not fix anything in particular. The idea was that, by 
> transitioning from having all boolean expressions from “int” to “bool” (in C 
> terms), and thus from 32-bit to 8-bit on “typical” targets, the optimizer 
> might be able to emit more compact code.

My motivations were:

- As you say, hopefully more compact code.

- Fixing the co-array intrinsics which pass arguments as
boolean_type_node and on the library side the corresponding arguments
are C _Bool's. I think this has worked previously accidentally, as
arguments are passed in registers anyway, and stack slots are 8
bytes(?) on x86-64. But of course, we shouldn't let this excuse us
from actually fixing it.

- Redefining an ABI type specified by the middle end is bad form, IMHO.

> I am not sure this was tested.
>
> So: maybe it is a case of "Profile. Don't speculate.”

I don't have access to spec2k6, but I just checked the polyhedron 2011
benchmark suite. Target x86_64-pc-linux-gnu, compile options -O3
-funroll-loops -ffast-math -ftree-loop-linear -march=native (native ==
amdfam10 here). The line counts of the .s files (~number of
instructions) are (working directory is current trunk,
gf_trunk_boolean_type_node is trunk with my patch reverted):

  6856 ac.s
  6856 ../gf_trunk_boolean_type_node/ac.s
 235206 aermod.s
 235337 ../gf_trunk_boolean_type_node/aermod.s
 22310 air.s
 22477 ../gf_trunk_boolean_type_node/air.s
 15221 capacita.s
 15252 ../gf_trunk_boolean_type_node/capacita.s
  4905 channel2.s
  4859 ../gf_trunk_boolean_type_node/channel2.s
  34269 doduc.s
  33610 ../gf_trunk_boolean_type_node/doduc.s
 11963 fatigue2.s
 11959 ../gf_trunk_boolean_type_node/fatigue2.s
 17369 gas_dyn2.s
 17304 ../gf_trunk_boolean_type_node/gas_dyn2.s
  31098 induct2.s
  31098 ../gf_trunk_boolean_type_node/induct2.s
  8366 linpk.s
  8367 ../gf_trunk_boolean_type_node/linpk.s
 15139 mdbx.s
 15155 ../gf_trunk_boolean_type_node/mdbx.s
  5302 mp_prop_design.s
  5314 ../gf_trunk_boolean_type_node/mp_prop_design.s
 17420 nf.s
 17457 ../gf_trunk_boolean_type_node/nf.s
 20804 protein.s
 20713 ../gf_trunk_boolean_type_node/protein.s
  52476 rnflow.s
  52449 ../gf_trunk_boolean_type_node/rnflow.s
  37339 test_fpu2.s
  37377 ../gf_trunk_boolean_type_node/test_fpu2.s
  8466 tfft2.s
  8448 ../gf_trunk_boolean_type_node/tfft2.s


So 7 of 17 benchmarks are slightly bigger after the patch (at least
for channel2 it seems due to trunk unrolling a loop which didn't
happen with the patch reverted, haven't checked others), 8 benchmarks
are slightly smaller after the patch, and 2 are equally long. Runtime
differences seem to be a wash, though I didn't test that very
carefully.


-- 
Janne Blomqvist


Re: [PATCH] Special case clzl like builtins in factor_out_conditional_conversion (PR tree-optimization/71016)

2017-01-04 Thread Richard Biener
On Mon, 2 Jan 2017, Jakub Jelinek wrote:

> Hi!
> 
> The clzl-like integral unary builtins at least from my short testing
> on x86_64-linux and aarch64-linux usually benefit from
> factor_out_conditional_conversion not being performed, i.e. the
> sign-extension (or zero-extension) being performed closer to the actual
> builtin, because both library calls and inline code usually computes
> the result in the same mode as the single argument has, so the artificial
> cast to int for the return value and back can be optimized away if it is
> adjacent, but not if it happens in some other bb.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hmm, I don't like special-casing like this.

> Or shall I also check that gimple_bb (SSA_NAME_DEF_STMT (new_arg0))
> is equal to gimple_bb (arg0_def_stmt)?

Looking at the original patch and its testcases it seems like
it wants to deal with the case of the conversion source being
used in the controlling predicate (I've seen other reports of
this transform being harmful).  That is, it's supposed to be
an enablement transform for further phiopt cases (for the
gcc.dg/tree-ssa/pr66726.c case min-max replacement).

Your patch misses a testcase so I can't really see whether such condition
would fix it.

Richard.

> 2017-01-02  Jakub Jelinek  
> 
>   PR tree-optimization/71016
>   * tree-ssa-phiopt.c: Include case-cfn-macros.h.
>   (factor_out_conditional_conversion): Don't optimize if new_arg0 is
>   result of unary integral builtin, new_arg1 is a constant and
>   arg0 has the same precision as builtin's argument.  Formatting fix.
> 
> --- gcc/tree-ssa-phiopt.c.jj  2017-01-01 12:45:37.0 +0100
> +++ gcc/tree-ssa-phiopt.c 2017-01-02 17:01:52.885447496 +0100
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.
>  #include "tree-scalar-evolution.h"
>  #include "tree-inline.h"
>  #include "params.h"
> +#include "case-cfn-macros.h"
>  
>  static unsigned int tree_ssa_phiopt_worker (bool, bool);
>  static bool conditional_replacement (basic_block, basic_block,
> @@ -490,6 +491,36 @@ factor_out_conditional_conversion (edge
>if (!types_compatible_p (TREE_TYPE (new_arg0), TREE_TYPE (new_arg1)))
>  return NULL;
>  
> +  /* Don't factor out cases where new_arg0 is result of an unary builtin
> + like clzl and arg0's type has the same precision as the builtin's
> + argument.  The reason for that is that the inline expansion of
> + those builtins typically computes the result in the same mode as
> + the argument and then just artificially casts it to int, so such
> + conversions undo that artificial casts and are for free.  See PR71016.  
> */
> +  if (TREE_CODE (new_arg0) == SSA_NAME
> +  && is_gimple_call (SSA_NAME_DEF_STMT (new_arg0))
> +  && TREE_CODE (new_arg1) == INTEGER_CST)
> +switch (gimple_call_combined_fn (SSA_NAME_DEF_STMT (new_arg0)))
> +  {
> + tree barg;
> +  CASE_CFN_CLZ:
> +  CASE_CFN_CTZ:
> +  CASE_CFN_CLRSB:
> +  CASE_CFN_FFS:
> +  CASE_CFN_PARITY:
> +  CASE_CFN_POPCOUNT:
> + barg = gimple_call_arg (SSA_NAME_DEF_STMT (new_arg0), 0);
> + if (INTEGRAL_TYPE_P (TREE_TYPE (arg0))
> + && TREE_CODE (barg) == SSA_NAME
> + && INTEGRAL_TYPE_P (TREE_TYPE (barg))
> + && (TYPE_PRECISION (TREE_TYPE (arg0))
> + == TYPE_PRECISION (TREE_TYPE (barg
> +   return NULL;
> + break;
> +  default:
> + break;
> +  }
> +
>/* Create a new PHI stmt.  */
>result = PHI_RESULT (phi);
>temp = make_ssa_name (TREE_TYPE (new_arg0), NULL);
> @@ -524,7 +555,7 @@ factor_out_conditional_conversion (edge
>/* Create the conversion stmt and insert it.  */
>if (convert_code == VIEW_CONVERT_EXPR)
>  temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);
> -  new_stmt = gimple_build_assign (result,  convert_code, temp);
> +  new_stmt = gimple_build_assign (result, convert_code, temp);
>gsi = gsi_after_labels (gimple_bb (phi));
>gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
>  
> 
>   Jakub
> 
> 

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


Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.

2017-01-04 Thread Dominik Vogt
On Fri, Dec 23, 2016 at 05:54:01PM +0100, Georg-Johann Lay wrote:
> Segher Boessenkool schrieb:
> >On Thu, Dec 22, 2016 at 04:18:34PM +0100, Georg-Johann Lay wrote:
> >If you don't have instruction scheduling subregs of mem are allowed (and
> >are counted as registers).  Combine asks recog, and it
> >think this is fine.
> >
> >Why does reload use r31 here?  Why does it think that is
> >valid for HImode?
> I can't tell you, all I'm seeing bunch of new FAILs after r243578.
> >
> >The avr port does not define CANNOT_CHANGE_MODE_CLASS.  It probably
> >should.
> 
> Thanks for that pointer, I'll try it as soon as I can.
> 
> i.e. the paradoxical subreg could be resolved somehow and R25 is
> uninitialized.  It this the purpose of the combine change to come
> up with uninitialized regs because it is known that just the
> initialized parts are used by the code?
> >
> >The purpose of the combine change is to write widening extracts in a
> >more general form, so that backends for processors that can do such
> >more general things do not have to write hundreds (literally) extra
> >patterns for all the cases that could be written as zero_extract.
> 
> One problem is that not all expressions are canonicalized and combine
> might come up with many different kinds of representations for the
> same action.  One common example is inserting one bit.  This might
> be represented as (set (zero_extract)) or as set with masking and
> shifting around or as (set (if_then_else)), with different
> representations if the sign bit is involved or if the source
> bit position is the same or lower or higher than the destination's
> bit position.

I'm working on patches to get more sensible simplify results in
some of these cases, like extracting the sign bit.  Not really
canonical, but rather that dealing with every odd combination it's
better to suppress them.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: [PATCH] Special case clzl like builtins in factor_out_conditional_conversion (PR tree-optimization/71016)

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 10:51:28AM +0100, Richard Biener wrote:
> > The clzl-like integral unary builtins at least from my short testing
> > on x86_64-linux and aarch64-linux usually benefit from
> > factor_out_conditional_conversion not being performed, i.e. the
> > sign-extension (or zero-extension) being performed closer to the actual
> > builtin, because both library calls and inline code usually computes
> > the result in the same mode as the single argument has, so the artificial
> > cast to int for the return value and back can be optimized away if it is
> > adjacent, but not if it happens in some other bb.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Hmm, I don't like special-casing like this.
> 
> > Or shall I also check that gimple_bb (SSA_NAME_DEF_STMT (new_arg0))
> > is equal to gimple_bb (arg0_def_stmt)?
> 
> Looking at the original patch and its testcases it seems like
> it wants to deal with the case of the conversion source being
> used in the controlling predicate (I've seen other reports of
> this transform being harmful).  That is, it's supposed to be
> an enablement transform for further phiopt cases (for the
> gcc.dg/tree-ssa/pr66726.c case min-max replacement).

factor_out_conditional_conversion handles 2 IMHO quite different cases.
One is the case where there are 2 SSA_NAMEs and 2 conversions and the
optimization turns those 2 conversions before the PHI into a single one
after the PHI.  That is likely beneficial in most cases.
The other case is when there is a single conversion, one SSA_NAME and one
INTEGER_CST.  This one might be in rare cases beneficial, in many cases a
wash, in many cases pessimizing.  But perhaps it enables some phiopt
optimization in some cases.
So, unless we want to throw away the SSA_NAME + INTEGER_CST handling
of factor_out_conditional_conversion altogether, we need some heuristics
on whether it is beneficial or not.  If ignoring the cases where we can get
better code by keeping the conversion closer to the operation that produced
the value (e.g. this clzl case where the operation computes it in wider mode
and then just downcasts it and extension after it can be optimized away
because of it), I guess it is a matter of whether the target is whole
word operations or not, what the modes of the converted type and conversion
result type are etc.
It is not clear what you are proposing.

> Your patch misses a testcase so I can't really see whether such condition
> would fix it.

A testcase is in the PR, #c8 and #c9.  Haven't turned those into a
testsuite/ testcase because it looks very fragile to me (the only thing
I can imagine with that would be gcc.target/i386/ and gcc.target/aarch64/
tests with a single function per testcase and doing scan-assembler-not
or something similar there).

Jakub


Re: [PATCH] Special case clzl like builtins in factor_out_conditional_conversion (PR tree-optimization/71016)

2017-01-04 Thread Richard Biener
On Wed, 4 Jan 2017, Jakub Jelinek wrote:

> On Wed, Jan 04, 2017 at 10:51:28AM +0100, Richard Biener wrote:
> > > The clzl-like integral unary builtins at least from my short testing
> > > on x86_64-linux and aarch64-linux usually benefit from
> > > factor_out_conditional_conversion not being performed, i.e. the
> > > sign-extension (or zero-extension) being performed closer to the actual
> > > builtin, because both library calls and inline code usually computes
> > > the result in the same mode as the single argument has, so the artificial
> > > cast to int for the return value and back can be optimized away if it is
> > > adjacent, but not if it happens in some other bb.
> > > 
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > Hmm, I don't like special-casing like this.
> > 
> > > Or shall I also check that gimple_bb (SSA_NAME_DEF_STMT (new_arg0))
> > > is equal to gimple_bb (arg0_def_stmt)?
> > 
> > Looking at the original patch and its testcases it seems like
> > it wants to deal with the case of the conversion source being
> > used in the controlling predicate (I've seen other reports of
> > this transform being harmful).  That is, it's supposed to be
> > an enablement transform for further phiopt cases (for the
> > gcc.dg/tree-ssa/pr66726.c case min-max replacement).
> 
> factor_out_conditional_conversion handles 2 IMHO quite different cases.
> One is the case where there are 2 SSA_NAMEs and 2 conversions and the
> optimization turns those 2 conversions before the PHI into a single one
> after the PHI.  That is likely beneficial in most cases.
> The other case is when there is a single conversion, one SSA_NAME and one
> INTEGER_CST.  This one might be in rare cases beneficial, in many cases a
> wash, in many cases pessimizing.  But perhaps it enables some phiopt
> optimization in some cases.
> So, unless we want to throw away the SSA_NAME + INTEGER_CST handling
> of factor_out_conditional_conversion altogether, we need some heuristics
> on whether it is beneficial or not.  If ignoring the cases where we can get
> better code by keeping the conversion closer to the operation that produced
> the value (e.g. this clzl case where the operation computes it in wider mode
> and then just downcasts it and extension after it can be optimized away
> because of it), I guess it is a matter of whether the target is whole
> word operations or not, what the modes of the converted type and conversion
> result type are etc.
> It is not clear what you are proposing.

For the SSA_NAME + INTEGER_CST case restrict it to the case

  if (x_1 > 5)
tem_2 = (char) x_1;
  # tem_3 = PHI 

that is, (char) x_1 uses x_1 and that also appears in the controlling
GIMPLE_COND.  That's what enables followup minmax replacement
(gcc.dg/tree-ssa/pr66726.c).  Another case where it might be
profitable is if the BB is empty after the conversion is sunk
(may enable other value-replacement cases).
 
> > Your patch misses a testcase so I can't really see whether such condition
> > would fix it.
> 
> A testcase is in the PR, #c8 and #c9.  Haven't turned those into a
> testsuite/ testcase because it looks very fragile to me (the only thing
> I can imagine with that would be gcc.target/i386/ and gcc.target/aarch64/
> tests with a single function per testcase and doing scan-assembler-not
> or something similar there).
> 
>   Jakub
> 
> 

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


Re: Pointer Bounds Checker and trailing arrays (PR68270)

2017-01-04 Thread Richard Biener
On Thu, Dec 22, 2016 at 12:47 AM, Ilya Enkovich  wrote:
> 2016-12-21 22:18 GMT+03:00 Alexander Ivchenko :
>> Right.. here is this updated chunk (otherwise no difference in the patch)
>>
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..6c7862c 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3272,6 +3272,9 @@ chkp_may_narrow_to_field (tree field)
>>  {
>>return DECL_SIZE (field) && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST
>>  && tree_to_uhwi (DECL_SIZE (field)) != 0
>> +&& !(flag_chkp_flexible_struct_trailing_arrays
>> + && TREE_CODE(TREE_TYPE(field)) == ARRAY_TYPE
>> + && !DECL_CHAIN (field))
>>  && (!DECL_FIELD_OFFSET (field)
>>   || TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST)
>>  && (!DECL_FIELD_BIT_OFFSET (field)
>
> OK.

There is array_at_struct_end_p which has a better implementation for
this (the above
considers a TYPE_DECL after the array a field).

Richard.

>>
>> 2016-12-21 21:00 GMT+03:00 Ilya Enkovich :
>>> 2016-12-20 17:44 GMT+03:00 Alexander Ivchenko :
 2016-11-26 0:28 GMT+03:00 Ilya Enkovich :
> 2016-11-25 15:47 GMT+03:00 Alexander Ivchenko :
>> Hi,
>>
>> The patch below addresses PR68270. could you please take a look?
>>
>> 2016-11-25  Alexander Ivchenko  
>>
>>* c-family/c.opt (flag_chkp_flexible_struct_trailing_arrays):
>>Add new option.
>>* tree-chkp.c (chkp_parse_array_and_component_ref): Forbid
>>narrowing when chkp_parse_array_and_component_ref is used and
>>the ARRAY_REF points to an array in the end of the struct.
>>
>>
>>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 7d8a726..e45d6a2 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -1166,6 +1166,11 @@ C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_narrow_to_innermost_ar
>>  Forces Pointer Bounds Checker to use bounds of the innermost arrays in 
>> case of
>>  nested static arryas access.  By default outermost array is used.
>>
>> +fchkp-flexible-struct-trailing-arrays
>> +C ObjC C++ ObjC++ LTO RejectNegative Report
>> Var(flag_chkp_flexible_struct_trailing_arrays)
>> +Allow Pointer Bounds Checker to treat all trailing arrays in structures 
>> as
>> +possibly flexible.
>
> Words 'allow' and 'possibly' are confusing here. This option is about to 
> force
> checker to do something, not to give him a choice.

 Fixed

> New option has to be documented in invoke.texi. It would also be nice to 
> reflect
> changes on GCC MPX wiki page.

 Done
> We have an attribute to change compiler behavior when this option is not 
> set.
> But we have no way to make exceptions when this option is used. Should we
> add one?
 Something like "bnd_fixed_size" ? Could work. Although the customer
 request did not mention the need for that.
 Can I add it in a separate patch?

>>>
>>> Yes.
>>>

>> +
>>  fchkp-optimize
>>  C ObjC C++ ObjC++ LTO Report Var(flag_chkp_optimize) Init(-1)
>>  Allow Pointer Bounds Checker optimizations.  By default allowed
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 2769682..40f99c3 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -3425,7 +3425,9 @@ chkp_parse_array_and_component_ref (tree node, 
>> tree *ptr,
>>if (flag_chkp_narrow_bounds
>>&& !flag_chkp_narrow_to_innermost_arrray
>>&& (!last_comp
>> -  || chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1
>> +  || (chkp_may_narrow_to_field (TREE_OPERAND (last_comp, 1))
>> +  && !(flag_chkp_flexible_struct_trailing_arrays
>> +   && array_at_struct_end_p (var)
>
> This is incorrect place for fix. Consider code
>
> struct S {
>   int a;
>   int b[10];
> };
>
> struct S s;
> int *p = s.b;
>
> Here you need to compute bounds for p and you want your option to take 
> effect
> but in this case you won't event reach your new check because there is no
> ARRAY_REF. And even if we change it to
>
> int *p = s.b[5];
>
> then it still would be narrowed because s.b would still be written
> into 'comp_to_narrow'
> variable. Correct place for fix is in chkp_may_narrow_to_field.

 Done

> Also you should consider fchkp-narrow-to-innermost-arrray option. Should 
> it
> be more powerfull or not? I think fchkp-narrow-to-innermost-arrray 
> shouldn't
> narrow to variable sized fields. BTW looks like right now 
> bnd_variable_size
> attribute is ignored by fchkp-narrow-to-innermost-arrray. This is another
> problem and may be fixed in another patch though.
 The way code works in chkp_parse_array_and_component_ref seems to be
 exactly like you say:  fchkp-narrow-to-innermost-arr

Re: [PATCH][GIMPLEFE] Fix for ICE due to undeclared variable

2017-01-04 Thread Richard Biener
On Wed, Dec 28, 2016 at 7:27 PM, Prasad Ghangal
 wrote:
> Hi,
> The attached patch tries fix ICE due to undeclared variable(s) in the input.
> Successfully bootstrapped on x86_64-pc-linux-gnu, testing is in progress

Ok.

Richard.

>
> Thanks,
> Prasad


Re: [PATCH] varasm: Propagate litpool decl alignment to generated RTX.

2017-01-04 Thread Richard Biener
On Thu, Dec 22, 2016 at 1:58 PM, Andreas Krebbel
 wrote:
> On 12/20/2016 11:38 AM, Richard Biener wrote:
>> On Fri, Dec 16, 2016 at 9:29 PM, Andreas Krebbel
>>  wrote:
>>> When pushing a value into the literal pool the resulting decl might
>>> get a higher alignment than the original expression depending on how a
>>> target defines CONSTANT_ALIGNMENT.  Generating an RTX for the constant
>>> pool access we currently use the alignment from the original
>>> expression.  Changed with the attached patch.
>>
>> And it might be even smaller alignment...  or do we not allow that?
> I did assume that this is not supposed to happen. Adding an assertion 
> triggering in that case
> survived bootstrap and testsuite. s390x only. It basically boils down to 
> whether align_variable and
> set_mem_attributes/get_object_alignment come to different conclusions about 
> the alignment starting
> at either the var decl or the original expression.
>
> ...
>>> Bootstrapped and regtested on x86_64 and s390x.
>>>
>>> No regressions.
>>>
>>> Ok?
>>
>> Ok.
>>
>> Richard.
>
> Ok for GCC 6 branch as well?

Yes.

Richard.

> -Andreas-
>
>


Re: [PATCH] libstdc++: Allow using without lock free atomic int

2017-01-04 Thread Jonathan Wakely

On 03/01/17 15:32 +, Jonathan Wakely wrote:

Here's what I plan to commit to trunk tomorrow.


Committed to trunk.




[PATCH][ARM] ARMv8-M Security Extensions: Warn for unused result for some intrinsics

2017-01-04 Thread Andre Vieira (lists)
Hello,

This patch adds the attribute "warn_unused_result" to the following
intrinsics:
__cmse_TT{,A,AT,T}_fptr
cmse_TT{,A,AT,T}
cmse_nonsecure_caller
cmse_check_address_range

If the result of these intrinsics is not used it means the result of the
checks they perform are never used and that could become the source of a
security vulnerability in the user's code.  We hope this will limit these.

Due to the current limitations of "warn_unused_result", adding them to
the __cmse_TT*_fptr intrinsics is pointless since the user will most
likely use the macro 'cmse_TT*_fptr' instead, which casts the result of
__cmse_TT*_fptr and that seems to be enough to count as a "use". I
decided to leave them in there anyway in case the warning becomes a bit
smarter in the future. Warnings for cmse_check_pointed_object will never
be issued for the same reason. Also if you assign the result of any of
these intrinsics to a variable you never use, you will only get a
warning about an unused variable, though this warning is not turned on
by default.

Ran cmse regression tests for arm-none-eabi both ARMv8-M Baseline and
Mainline.

Is this OK for stage 3?

Cheers,
Andre

gcc/ChangeLog:
2017-01-04  Andre Vieira  

* config/gcc/arm_cmse.h (__cmse_TT_fptr,__cmse_TTA_fptr,
__cmse_TTAT_fptr,__cmse_TTT_fptr,cmse_TT, cmse_TTA, cmse_TTAT,
cmse_TTT, cmse_nonsecure_caller, cmse_check_address_range):
Add warn_unused_result attribute to function declaration.

gcc/testsuite/ChangeLog:
2017-01-04  Andre Vieira  

* gcc.target/arm/cmse/cmse-3.c: Add warning tests for the
warn_unused_result warning.
diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h
index 
82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..d37f4e2b446c3c80d56af8b633323837f327973f
 100644
--- a/gcc/config/arm/arm_cmse.h
+++ b/gcc/config/arm/arm_cmse.h
@@ -116,11 +116,13 @@ typedef void (*__cmse_fptr)(void);
 }
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 __cmse_TT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM ()
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 cmse_TT (void *__p)
 __CMSE_TT_ASM ()
@@ -128,11 +130,13 @@ __CMSE_TT_ASM ()
 #define cmse_TTT_fptr(p) (__cmse_TTT_fptr ((__cmse_fptr)(p)))
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 __cmse_TTT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (t)
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 cmse_TTT (void *__p)
 __CMSE_TT_ASM (t)
@@ -142,11 +146,13 @@ __CMSE_TT_ASM (t)
 #define cmse_TTA_fptr(p) (__cmse_TTA_fptr ((__cmse_fptr)(p)))
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 __cmse_TTA_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (a)
 
 __extension__ static __inline __attribute__ ((__always_inline__))
+__attribute__ ((__warn_unused_result__))
 cmse_address_info_t
 cmse_TTA (void *__p)
 __CMSE_TT_ASM (a)
@@ -154,17 +160,18 @@ __CMSE_TT_ASM (a)
 #define cmse_TTAT_fptr(p) (__cmse_TTAT_fptr ((__cmse_fptr)(p)))
 
 __extension__ static __inline cmse_address_info_t
-__attribute__ ((__always_inline__))
+__attribute__ ((__always_inline__, __warn_unused_result__))
 __cmse_TTAT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (at)
 
 __extension__ static __inline cmse_address_info_t
-__attribute__ ((__always_inline__))
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_TTAT (void *__p)
 __CMSE_TT_ASM (at)
 
 /* FIXME: diagnose use outside cmse_nonsecure_entry functions.  */
-__extension__ static __inline int __attribute__ ((__always_inline__))
+__extension__ static __inline int
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_nonsecure_caller (void)
 {
   return __builtin_arm_cmse_nonsecure_caller ();
@@ -184,7 +191,7 @@ cmse_nonsecure_caller (void)
 #define CMSE_MPU_READWRITE 1
 #define CMSE_MPU_READ  8
 
-__extension__ void *
+__extension__ void * __attribute__ ((__warn_unused_result__))
 cmse_check_address_range (void *, size_t, int);
 
 #define cmse_check_pointed_object(p, f) \
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c 
b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
index 
7f92a4c28be4c8fdc256211f3ed74a383cd4..fd3cd282546b9eee10b7d5730f9096084502c492
 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
@@ -43,3 +43,12 @@ typedef void __attribute__ ((cmse_nonsecure_call)) baz2 
(long long a, int b, str
 typedef struct span __attribute__ ((cmse_nonsecure_call)) qux2 (void); /* { 
dg-error "not available to functions that return value on the stack" } */
 
 typedef void __attribute__ ((cmse_nonsecure_call)) norf2 (int a, ...); /* { 
dg-error "not available to functions with v

Use a specfile that actually allows building programs on NetBSD

2017-01-04 Thread coypu
Like most operating systems, NetBSD has a libc which contains
stuff it needs for most programs to work, and people expect
it to be linked without explicitly specifying -lc.

This patch is needed for just about any program to work.

---
 gcc/config/netbsd.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/config/netbsd.h b/gcc/config/netbsd.h
index f2d6cc6..65ce943 100644
--- a/gcc/config/netbsd.h
+++ b/gcc/config/netbsd.h
@@ -96,6 +96,7 @@ along with GCC; see the file COPYING3.  If not see
%{!pg:-lposix}} \
  %{p:-lposix_p}\
  %{pg:-lposix_p}}  \
+   %{shared:-lc}   \
%{!shared:  \
  %{!symbolic:  \
%{!p:   \
@@ -109,6 +110,7 @@ along with GCC; see the file COPYING3.  If not see
%{!pg:-lposix}} \
  %{p:-lposix_p}\
  %{pg:-lposix_p}}  \
+   %{shared:-lc}   \
%{!shared:  \
  %{!symbolic:  \
%{!p:   \
-- 
2.9.0



RE: Make MIPS soft-fp preserve NaN payloads for NAN2008

2017-01-04 Thread Matthew Fortune
Joseph Myers  writes:
> The MIPS sfp-machine.h has an _FP_CHOOSENAN implementation which
> emulates hardware semantics of not preserving signaling NaN payloads for
> an operation with two NaN arguments (although that doesn't suffice to
> avoid sNaN payload preservation in any case with just one NaN argument).
> 
> However, those are only hardware semantics in the legacy NaN case; in
> the NAN2008 case, the architecture documentation says hardware preserves
> payloads in such cases.  Furthermore, this implementation assumes legacy
> NaN semantics, so in the NAN2008 case the implementation actually has
> the effect of preserving sNaN payloads but not preserving qNaN payloads,
> when both should be preserved.
> 
> This patch fixes the code just to copy from the first argument (at the
> level of libgcc, it's not meaningful which argument is the first and
> which is the second).
> 
> Tested for mips64-linux-gnu (soft float, NAN2008) with the glibc math/
> tests.  OK to commit?
> 
> 2017-01-02  Joseph Myers  
> 
>   * config/mips/sfp-machine.h (_FP_CHOOSENAN): Always preserve NaN
>   payload if [__mips_nan2008].

Thanks for finding and fixing this.

OK to commit.

Matthew


Re: [PATCH] Fix ICE with -fno-sso-struct=none (PR driver/78957)

2017-01-04 Thread Richard Biener
On Mon, Jan 2, 2017 at 8:27 PM, Jakub Jelinek  wrote:
> Hi!
>
> Enum options should not allow negative form, otherwise the option handling
> ICEs on it.  -fsso-struct= allows only big-endian or little-endian,
> -fno-sso-struct=big-endian or -fno-sso-struct=whatever makes no sense.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok, but maybe .opt processing can add RejectNegative on its own for
Enum switches then?  For the vectorizer we have (for backward compatibility):

fvect-cost-model
Common RejectNegative Alias(fvect-cost-model=,dynamic)
Enables the dynamic vectorizer cost model.  Preserved for backward
compatibility.

fno-vect-cost-model
Common RejectNegative Alias(fvect-cost-model=,unlimited)
Enables the unlimited vectorizer cost model.  Preserved for backward
compatibility.


Richard.

> 2017-01-02  Jakub Jelinek  
>
> PR driver/78957
> * c.opt (fsso-struct=): Add RejectNegative.
>
> * gcc.dg/pr78957.c: New test.
>
> --- gcc/c-family/c.opt.jj   2016-12-29 13:56:36.0 +0100
> +++ gcc/c-family/c.opt  2017-01-01 12:10:16.744757723 +0100
> @@ -1626,7 +1626,7 @@ fsquangle
>  C++ ObjC++ Ignore Warn(switch %qs is no longer supported)
>
>  fsso-struct=
> -C ObjC Joined Enum(sso_struct) Var(default_sso) Init(SSO_NATIVE)
> +C ObjC Joined RejectNegative Enum(sso_struct) Var(default_sso) 
> Init(SSO_NATIVE)
>  -fsso-struct=[big-endian|little-endian]Set the default scalar 
> storage order.
>
>  Enum
> --- gcc/testsuite/gcc.dg/pr78957.c.jj   2017-01-01 12:02:41.859809492 +0100
> +++ gcc/testsuite/gcc.dg/pr78957.c  2017-01-01 12:06:05.951094270 +0100
> @@ -0,0 +1,6 @@
> +/* PR driver/78957 */
> +/* { dg-do compile } */
> +/* { dg-options "-fno-sso-struct=none" } */
> +/* { dg-error "unrecognized command line option" "" { target *-*-* } 0 } */
> +
> +int i;
>
> Jakub


Re: [PATCH][ARM] ARMv8-M Security Extensions: Warn for unused result for some intrinsics

2017-01-04 Thread Kyrill Tkachov

Hi Andre,

On 04/01/17 11:21, Andre Vieira (lists) wrote:

Hello,

This patch adds the attribute "warn_unused_result" to the following
intrinsics:
__cmse_TT{,A,AT,T}_fptr
cmse_TT{,A,AT,T}
cmse_nonsecure_caller
cmse_check_address_range

If the result of these intrinsics is not used it means the result of the
checks they perform are never used and that could become the source of a
security vulnerability in the user's code.  We hope this will limit these.

Due to the current limitations of "warn_unused_result", adding them to
the __cmse_TT*_fptr intrinsics is pointless since the user will most
likely use the macro 'cmse_TT*_fptr' instead, which casts the result of
__cmse_TT*_fptr and that seems to be enough to count as a "use". I
decided to leave them in there anyway in case the warning becomes a bit
smarter in the future. Warnings for cmse_check_pointed_object will never
be issued for the same reason. Also if you assign the result of any of
these intrinsics to a variable you never use, you will only get a
warning about an unused variable, though this warning is not turned on
by default.

Ran cmse regression tests for arm-none-eabi both ARMv8-M Baseline and
Mainline.

Is this OK for stage 3?

Cheers,
Andre

gcc/ChangeLog:
2017-01-04  Andre Vieira  

 * config/gcc/arm_cmse.h (__cmse_TT_fptr,__cmse_TTA_fptr,
 __cmse_TTAT_fptr,__cmse_TTT_fptr,cmse_TT, cmse_TTA, cmse_TTAT,
 cmse_TTT, cmse_nonsecure_caller, cmse_check_address_range):
 Add warn_unused_result attribute to function declaration.

gcc/testsuite/ChangeLog:
2017-01-04  Andre Vieira  

 * gcc.target/arm/cmse/cmse-3.c: Add warning tests for the
 warn_unused_result warning.



diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h
index 
82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..d37f4e2b446c3c80d56af8b633323837f327973f
 100644
--- a/gcc/config/arm/arm_cmse.h
+++ b/gcc/config/arm/arm_cmse.h
@@ -116,11 +116,13 @@ typedef void (*__cmse_fptr)(void);
 }
 
 __extension__ static __inline __attribute__ ((__always_inline__))

+__attribute__ ((__warn_unused_result__))

Don't add a second __attribute__ annotation, change the first one to be:
 __attribute__ ((__always_inline__, __warn_unused_result__))


Ok with that change.
Thanks,
Kyrill



Re: [-fcompare-debug] skip more debug stmts in cleanup_empty_eh

2017-01-04 Thread Richard Biener
On Tue, Jan 3, 2017 at 6:28 AM, Alexandre Oliva  wrote:
> Various Ada RTS files failed -fcompare-debug compilation because debug
> stmts prevented EH cleanups from taking place.  Adjusting
> cleanup_empty_eh to skip them fixes it.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Ok.

Richard.

> for  gcc/ChangeLog
>
> * gimple-iterator.h (gsi_one_nondebug_before_end_p): New.
> * tree-eh.c (cleanup_empty_eh): Skip more debug stmts.
> ---
>  gcc/gimple-iterator.h |   14 ++
>  gcc/tree-eh.c |7 ---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
> index a58b0a4b5d..d7cd87e 100644
> --- a/gcc/gimple-iterator.h
> +++ b/gcc/gimple-iterator.h
> @@ -305,6 +305,20 @@ gsi_last_nondebug_bb (basic_block bb)
>return i;
>  }
>
> +/* Return true if I is followed only by debug statements in its
> +   sequence.  */
> +
> +static inline bool
> +gsi_one_nondebug_before_end_p (gimple_stmt_iterator i)
> +{
> +  if (gsi_one_before_end_p (i))
> +return true;
> +  if (gsi_end_p (i))
> +return false;
> +  gsi_next_nondebug (&i);
> +  return gsi_end_p (i);
> +}
> +
>  /* Iterates I statement iterator to the next non-virtual statement.  */
>
>  static inline void
> diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
> index db72156..e9edd97 100644
> --- a/gcc/tree-eh.c
> +++ b/gcc/tree-eh.c
> @@ -4382,7 +4382,8 @@ cleanup_empty_eh (eh_landing_pad lp)
>return false;
>  }
>
> -  resx = last_stmt (bb);
> +  gsi = gsi_last_nondebug_bb (bb);
> +  resx = gsi_stmt (gsi);
>if (resx && is_gimple_resx (resx))
>  {
>if (stmt_can_throw_external (resx))
> @@ -4416,12 +4417,12 @@ cleanup_empty_eh (eh_landing_pad lp)
>resx = gsi_stmt (gsi);
>if (!e_out && gimple_call_builtin_p (resx, BUILT_IN_STACK_RESTORE))
>  {
> -  gsi_next (&gsi);
> +  gsi_next_nondebug (&gsi);
>resx = gsi_stmt (gsi);
>  }
>if (!is_gimple_resx (resx))
>  return ret;
> -  gcc_assert (gsi_one_before_end_p (gsi));
> +  gcc_assert (gsi_one_nondebug_before_end_p (gsi));
>
>/* Determine if there are non-EH edges, or resx edges into the handler.  */
>has_non_eh_pred = false;
>
> --
> Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Richard Biener
On Tue, Jan 3, 2017 at 6:29 AM, Alexandre Oliva  wrote:
> If we include them in the ICF hash, they may cause congruence_groups to
> be processed in a different order due to different hashes, which in turn
> causes different funcdef_nos to be assigned to functions.  Since these
> numbers are included in -fcompare-debug dumps, they cause failures.

Is this because ICF simply compares all optimize/target options?

So it boils down to -g implying -fvar-tracking but -g0 not?  Maybe we need a
third state then ("default")?

> Since these options are not optimization options, in that they do not
> (or should not, save for bugs like this) affect the executable code
> output by the compiler, they should not be marked as such.
>
> This patch removes the Optimization marker from the var-tracking
> options, and adjusts the code that uses these flags to match.
>
> This fixes -fcompare-debug failures in numerous LTO testcases.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?

I don't think this is ok -- 'Optimization' doesn't really mean optimization
but sth we can control per-function.  You essentially remove the ability
to disable var-tracking just for a single (bad) function.

Richard.

> for  gcc/ChangeLog
>
> * common.opt (fvar-tracking): Drop Optimization flag.
> (fvar-tracking-assignments): Likewise.
> (fvar-tracking-assignments-toggle): Likewise.
> (fvar-tracking-uninit): Likewise.
> * tree-inline.c (insert_debug_decl_map): Adjust uses of
> flag_var_tracking_assignments.
> (remap_gimple_stmt): Likewise.
> (insert_init_debug_bind): Likewise.
> (reset_debug_bindings): Likewise.
> ---
>  gcc/common.opt|8 
>  gcc/tree-inline.c |9 -
>  2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 6ebaf9c..c4aad6f 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2644,7 +2644,7 @@ Common Undocumented Var(flag_use_linker_plugin)
>  ; will be set according to optimize, debug_info_level and debug_hooks
>  ; in process_options ().
>  fvar-tracking
> -Common Report Var(flag_var_tracking) Init(2) Optimization
> +Common Report Var(flag_var_tracking) Init(2)
>  Perform variable tracking.
>
>  ; Positive if we should track variables at assignments, negative if
> @@ -2652,13 +2652,13 @@ Perform variable tracking.
>  ; annotations.  When flag_var_tracking_assignments ==
>  ; AUTODETECT_VALUE it will be set according to flag_var_tracking.
>  fvar-tracking-assignments
> -Common Report Var(flag_var_tracking_assignments) Init(2) Optimization
> +Common Report Var(flag_var_tracking_assignments) Init(2)
>  Perform variable tracking by annotating assignments.
>
>  ; Nonzero if we should toggle flag_var_tracking_assignments after
>  ; processing options and computing its default.  */
>  fvar-tracking-assignments-toggle
> -Common Report Var(flag_var_tracking_assignments_toggle) Optimization
> +Common Report Var(flag_var_tracking_assignments_toggle)
>  Toggle -fvar-tracking-assignments.
>
>  ; Positive if we should track uninitialized variables, negative if
> @@ -2666,7 +2666,7 @@ Toggle -fvar-tracking-assignments.
>  ; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
>  ; will be set according to flag_var_tracking.
>  fvar-tracking-uninit
> -Common Report Var(flag_var_tracking_uninit) Optimization
> +Common Report Var(flag_var_tracking_uninit)
>  Perform variable tracking and also tag variables that are uninitialized.
>
>  ftree-vectorize
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 0de0b89..19ef5ee 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -156,7 +156,7 @@ insert_debug_decl_map (copy_body_data *id, tree key, tree 
> value)
>if (!gimple_in_ssa_p (id->src_cfun))
>  return;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (!flag_var_tracking_assignments)
>  return;
>
>if (!target_for_debug_bind (key))
> @@ -1376,8 +1376,7 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
>bool skip_first = false;
>gimple_seq stmts = NULL;
>
> -  if (is_gimple_debug (stmt)
> -  && !opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (is_gimple_debug (stmt) && !flag_var_tracking_assignments)
>  return stmts;
>
>/* Begin by recognizing trees that we'll completely rewrite for the
> @@ -3046,7 +3045,7 @@ insert_init_debug_bind (copy_body_data *id,
>if (!gimple_in_ssa_p (id->src_cfun))
>  return NULL;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (!flag_var_tracking_assignments)
>  return NULL;
>
>tracked_var = target_for_debug_bind (var);
> @@ -4379,7 +4378,7 @@ reset_debug_bindings (copy_body_data *id, 
> gimple_stmt_iterator gsi)
>if (!gimple_in_ssa_p (id->src_cfun))
>  return;
>
> -  if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments))
> +  if (!flag_var_tracking_assignments)
>  return;
>

Re: [PATCH] Fix ICE with -fno-sso-struct=none (PR driver/78957)

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 12:27:09PM +0100, Richard Biener wrote:
> On Mon, Jan 2, 2017 at 8:27 PM, Jakub Jelinek  wrote:
> > Enum options should not allow negative form, otherwise the option handling
> > ICEs on it.  -fsso-struct= allows only big-endian or little-endian,
> > -fno-sso-struct=big-endian or -fno-sso-struct=whatever makes no sense.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Ok, but maybe .opt processing can add RejectNegative on its own for
> Enum switches then?

Rather than implicit RejectNegative it might be better to just diagnose
such options as invalid.  If you agree, I can implement that as follow-up.
Also note that RejectNegative is only needed on the Enum switches that have
the default negatives (that is [Wfm] prefixed I think).

>  For the vectorizer we have (for backward compatibility):
> 
> fvect-cost-model
> Common RejectNegative Alias(fvect-cost-model=,dynamic)
> Enables the dynamic vectorizer cost model.  Preserved for backward
> compatibility.
> 
> fno-vect-cost-model
> Common RejectNegative Alias(fvect-cost-model=,unlimited)
> Enables the unlimited vectorizer cost model.  Preserved for backward
> compatibility.

That is a non-= non-Enum option though.  And not sure why this actually
is RejectNegative, wouldn't
Common Alias(fvect-cost-model=,dynamic,unlimited)
work just on fvect-cost-model (can test that)?
In this case there is no -fsso-struct option (what would it mean), so
-fno-sso-struct isn't there either (again, what would that mean).
The only thing that would make sense IMHO would be to allow
not just big-endian and little-endian, but also native, so one can
cancel earlier -fsso-struct= like
gcc ... -fsso-struct=little-endian ... -fsso-struct=native ...
and have it act as if neither of those options appeared.
CCing Eric who has added this option.

Jakub


Re: [PR tree-optimization/71691] Fix unswitching in presence of maybe-undef SSA_NAMEs (take 2)

2017-01-04 Thread Richard Biener
On Wed, Dec 21, 2016 at 7:43 PM, Aldy Hernandez  wrote:
> On 12/20/2016 09:16 AM, Richard Biener wrote:
>
>> You do not handle memory or calls conservatively which means the existing
>> testcase only needs some obfuscation to become a problem again.  To fix
>> that before /* Check that any SSA names used to define NAME is also fully
>> defined.  */ bail out conservatively, like
>>
>>if (! is_gimple_assign (def)
>>   || gimple_assign_single_p (def))
>> return true;
>
>
> I understand the !is_gimple_assign, which will skip over GIMPLE_CALLs
> setting a value, but the "gimple_assign_single_p(def) == true"??
>
> Won't this last one bail on things like e.3_7 = e, or x_77 = y_88? Don't we
> want to follow up the def chain precisely on these?

For the first (= e) we can't, for the second, yes, the predicate is too strict.
But it's conservative ;)

Likewise for e_2 = VIEW_CONVERT_EXPR  btw, or REAL/IMAGPART_EXPR.

Note both VIEW_CONVERT_EXPR and REAL/IMAGPART_EXPR will also appear
in RHSs we can't handle.

Richard.

>
> Thanks.
> Aldy


Re: [committed] contrib/update-copyright.py --this-year

2017-01-04 Thread Jakub Jelinek
On Sun, Jan 01, 2017 at 01:19:49PM +0100, Jakub Jelinek wrote:
>   Update copyright years.
> 
> (result of contrib/update-copyright.py --this-year).

Apparently a bunch of directories I think we want to handle is not included
by default.  I've updated them by running
contrib/update-copyright.py --this-year include libcc1 libiberty libssp libvtv 
lto-plugin
and will update the script to do this by default next time.

2017-01-04  Jakub Jelinek  

Update copyright years.

--- include/ansidecl.h  (revision 244050)
+++ include/ansidecl.h  (working copy)
@@ -1,5 +1,5 @@
 /* ANSI and traditional C compatability macros
-   Copyright (C) 1991-2015 Free Software Foundation, Inc.
+   Copyright (C) 1991-2017 Free Software Foundation, Inc.
This file is part of the GNU C Library.
 
 This program is free software; you can redistribute it and/or modify
--- include/demangle.h  (revision 244050)
+++ include/demangle.h  (working copy)
@@ -1,5 +1,5 @@
 /* Defs for interface to demanglers.
-   Copyright (C) 1992-2015 Free Software Foundation, Inc.
+   Copyright (C) 1992-2017 Free Software Foundation, Inc.
 
This program is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public License
--- include/dwarf2.def  (revision 244050)
+++ include/dwarf2.def  (working copy)
@@ -1,7 +1,7 @@
 /* -*- c -*-
Declarations and definitions of codes relating to the DWARF2 and
DWARF3 symbolic debugging information formats.
-   Copyright (C) 1992-2015 Free Software Foundation, Inc.
+   Copyright (C) 1992-2017 Free Software Foundation, Inc.
 
Written by Gary Funck (g...@intrepid.com) The Ada Joint Program
Office (AJPO), Florida State University and Silicon Graphics Inc.
--- include/dwarf2.h(revision 244050)
+++ include/dwarf2.h(working copy)
@@ -1,6 +1,6 @@
 /* Declarations and definitions of codes relating to the DWARF2 and
DWARF3 symbolic debugging information formats.
-   Copyright (C) 1992-2016 Free Software Foundation, Inc.
+   Copyright (C) 1992-2017 Free Software Foundation, Inc.
 
Written by Gary Funck (g...@intrepid.com) The Ada Joint Program
Office (AJPO), Florida State University and Silicon Graphics Inc.
--- include/dyn-string.h(revision 244050)
+++ include/dyn-string.h(working copy)
@@ -1,5 +1,5 @@
 /* An abstract string datatype.
-   Copyright (C) 1998-2015 Free Software Foundation, Inc.
+   Copyright (C) 1998-2017 Free Software Foundation, Inc.
Contributed by Mark Mitchell (m...@markmitchell.com).
 
 This file is part of GCC.
--- include/environ.h   (revision 244050)
+++ include/environ.h   (working copy)
@@ -1,5 +1,5 @@
 /* Declare the environ system variable.
-   Copyright (C) 2015 Free Software Foundation, Inc.
+   Copyright (C) 2015-2017 Free Software Foundation, Inc.
 
 This file is part of the libiberty library.
 Libiberty is free software; you can redistribute it and/or
--- include/fibheap.h   (revision 244050)
+++ include/fibheap.h   (working copy)
@@ -1,5 +1,5 @@
 /* A Fibonacci heap datatype.
-   Copyright (C) 1998-2015 Free Software Foundation, Inc.
+   Copyright (C) 1998-2017 Free Software Foundation, Inc.
Contributed by Daniel Berlin (d...@cgsoftware.com).
 
 This file is part of GCC.
--- include/filenames.h (revision 244050)
+++ include/filenames.h (working copy)
@@ -5,7 +5,7 @@
use forward- and back-slash in path names interchangeably, and
some of them have case-insensitive file names.
 
-   Copyright (C) 2000-2015 Free Software Foundation, Inc.
+   Copyright (C) 2000-2017 Free Software Foundation, Inc.
 
 This file is part of BFD, the Binary File Descriptor library.
 
--- include/floatformat.h   (revision 244050)
+++ include/floatformat.h   (working copy)
@@ -1,5 +1,5 @@
 /* IEEE floating point support declarations, for GDB, the GNU Debugger.
-   Copyright (C) 1991-2015 Free Software Foundation, Inc.
+   Copyright (C) 1991-2017 Free Software Foundation, Inc.
 
 This file is part of GDB.
 
--- include/fnmatch.h   (revision 244050)
+++ include/fnmatch.h   (working copy)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991-2015 Free Software Foundation, Inc.
+/* Copyright (C) 1991-2017 Free Software Foundation, Inc.
 
 NOTE: The canonical source of this file is maintained with the GNU C Library.
 Bugs can be reported to bug-gl...@prep.ai.mit.edu.
--- include/gcc-c-fe.def(revision 244050)
+++ include/gcc-c-fe.def(working copy)
@@ -1,6 +1,6 @@
 /* Interface between GCC C FE and GDB  -*- c -*-
 
-   Copyright (C) 2014-2015 Free Software Foundation, Inc.
+   Copyright (C) 2014-2017 Free Software Foundation, Inc.
 
This file is part of GCC.
 
--- include/gcc-c-interface.h   (revision 244050)
+++ include/gcc-c-interface.h   (working copy)
@@ -1,6 +1,6 @@
 /* Interface between GCC C FE and GDB
 
-   Copyright (C) 2014-2015 Free Software Foundation, Inc.
+   Copyright (C) 2014-2017 Free Software Foundation, Inc.
 
This file is part of GCC.
 
--- include/gcc-interface.h  

Re: [committed] contrib/update-copyright.py --this-year

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 12:48:47PM +0100, Jakub Jelinek wrote:
> On Sun, Jan 01, 2017 at 01:19:49PM +0100, Jakub Jelinek wrote:
> > Update copyright years.
> > 
> > (result of contrib/update-copyright.py --this-year).
> 
> Apparently a bunch of directories I think we want to handle is not included
> by default.  I've updated them by running
> contrib/update-copyright.py --this-year include libcc1 libiberty libssp 
> libvtv lto-plugin
> and will update the script to do this by default next time.

And here is corresponding update-copyright.py change I've checked in as
obvious.

2017-01-04  Jakub Jelinek  

* update-copyright.py (GCCCmdLine): Add include, libcc1, libiberty,
libssp, libvtv and lto-plugin to default_dirs.

--- contrib/update-copyright.py (revision 244050)
+++ contrib/update-copyright.py (working copy)
@@ -748,17 +748,23 @@ class GCCCmdLine (CmdLine):
 
 self.default_dirs = [
 'gcc',
+'include',
 'libada',
 'libatomic',
 'libbacktrace',
+'libcc1',
 'libcpp',
 'libdecnumber',
 'libgcc',
 'libgfortran',
 'libgomp',
+'libiberty',
 'libitm',
 'libobjc',
+'libssp',
 'libstdc++-v3',
+'libvtv',
+'lto-plugin',
 ]
 
 GCCCmdLine().main()


Jakub


Re: [PR tree-optimization/71691] Fix unswitching in presence of maybe-undef SSA_NAMEs (take 2)

2017-01-04 Thread Richard Biener
On Tue, Jan 3, 2017 at 6:36 PM, Aldy Hernandez  wrote:
> On 12/20/2016 09:16 AM, Richard Biener wrote:
>>
>> On Fri, Dec 16, 2016 at 3:41 PM, Aldy Hernandez  wrote:
>>>
>>> Hi folks.
>>>
>>> This is a follow-up on Jeff and Richi's interaction on the aforementioned
>>> PR
>>> here:
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01397.html
>>>
>>> I decided to explore the idea of analyzing may-undefness on-demand, which
>>> actually looks rather cheap.
>>>
>>> BTW, I don't understand why we don't have auto_bitmap's, as we already
>>> have
>>> auto_sbitmap's.  I've implemented the former based on auto_sbitmap's code
>>> we
>>> already have.
>>>
>>> The attached patch fixes the bug without introducing any regressions.
>>>
>>> I also tested the patch by compiling 242 .ii files with -O3.  These were
>>> gathered from a stage1 build with -save-temps.  There is a slight time
>>> degradation of 4 seconds within 27 minutes of user time:
>>>
>>> tainted:26:52
>>> orig:   26:48
>>>
>>> This was the average aggregate time of two runs compiling all 242 .ii
>>> files.
>>> IMO, this looks reasonable.  It is after all, -O3.Is it acceptable?
>>
>>
>> +  while (!worklist.is_empty ())
>> +{
>> +  name = worklist.pop ();
>> +  gcc_assert (TREE_CODE (name) == SSA_NAME);
>> +
>> +  if (ssa_undefined_value_p (name, true))
>> +   return true;
>> +
>> +  bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name));
>>
>> it should be already set as we use visited_ssa as "was it ever on the
>> worklist",
>> so maybe renaming it would be a good thing as well.
>
>
> I don't understand what you would prefer here.

Set the bit when you put name on the worklist (and do not do that if the
bit is set).  Thus simply remove the above and add a bitmap_set_bit
for the initial name you put on the worklist.

>>
>> + if (TREE_CODE (name) == SSA_NAME)
>> +   {
>> + /* If an SSA has already been seen, this may be a loop.
>> +Fail conservatively.  */
>> + if (bitmap_bit_p (visited_ssa, SSA_NAME_VERSION (name)))
>> +   return false;
>>
>> so to me "conservative" is returning true, not false.
>
>
> OK
>
>>
>> + bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name));
>> + worklist.safe_push (name);
>>
>> but for loops we can just continue and ignore this use.  And
>> bitmap_set_bit
>> returns whether it set a bit, thus
>>
>> if (bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name)))
>>   worklist.safe_push (name);
>>
>> should work?
>
>
> Fixed.
>
>>
>> +  /* Check that any SSA names used to define NAME is also fully
>> +defined.  */
>> +  use_operand_p use_p;
>> +  ssa_op_iter iter;
>> +  FOR_EACH_SSA_USE_OPERAND (use_p, def, iter, SSA_OP_USE)
>> +   {
>> + name = USE_FROM_PTR (use_p);
>> + if (TREE_CODE (name) == SSA_NAME)
>>
>> always true.
>>
>> +   {
>> + /* If an SSA has already been seen, this may be a loop.
>> +Fail conservatively.  */
>> + if (bitmap_bit_p (visited_ssa, SSA_NAME_VERSION (name)))
>> +   return false;
>> + bitmap_set_bit (visited_ssa, SSA_NAME_VERSION (name));
>> + worklist.safe_push (name);
>>
>> See above.
>>
>> In principle the thing is sound but I'd like to be able to pass in a
>> bitmap of
>> known maybe-undefined/must-defined SSA names to have a cache for
>> multiple invocations from within a pass (like this unswitching case).
>
>
> Done, though bitmaps are now calculated as part of the instantiation.
>
>>
>> Also once you hit defs that are in a post-dominated region of the loop
>> entry
>> you can treat them as not undefined (as their use invokes undefined
>> behavior).  This is also how you treat function parameters (well,
>> ssa_undefined_value_p does), where the call site invokes undefined
>> behavior
>> when passing in undefined values.  So we need an extra parameter
>> specifying
>> the post-dominance region.
>
>
> Done.
>
>>
>> You do not handle memory or calls conservatively which means the existing
>> testcase only needs some obfuscation to become a problem again.  To fix
>> that before /* Check that any SSA names used to define NAME is also fully
>> defined.  */ bail out conservatively, like
>>
>>if (! is_gimple_assign (def)
>>   || gimple_assign_single_p (def))
>> return true;
>
>
> As I asked previously, I understand the !is_gimple_assign, which will skip
> over GIMPLE_CALLs setting a value, but the "gimple_assign_single_p(def) ==
> true"??
>
> Won't this last one bail on things like e.3_7 = e, or x_77 = y_88? Don't we
> want to follow up the def chain precisely on these?
>
> The attached implementation uses a cache, and a pre-computed post-dominance
> region.  A previous incantation of this patch (sans the post-dominance
> stuff) had performance within t

Re: [bootstrap-O3,fortran] silence warning in simplify_transformation_to_array

2017-01-04 Thread Richard Biener
On Tue, Jan 3, 2017 at 7:19 PM, Jeff Law  wrote:
> On 01/02/2017 10:29 PM, Alexandre Oliva wrote:
>>
>> simplify_transformation_to_array had the nested loop unrolled 7 times,
>> which is reasonable given that it iterates over arrays of size
>> GFC_MAX_DIMENSIONS == 7.
>>
>> The problem is that the last iteration increments the index, tests
>> that it's less than result->rank, and then accesses the arrays with
>> the incremented index.
>>
>> We did not optimize out that part in the 7th iteration, so VRP flagged
>> the unreachable code as accessing arrays past the end.
>>
>> It couldn't possibly know that we'd never reach that part, since the
>> test was on result->rank, and it's not obvious (for the compiler) that
>> result->rank <= GFC_MAX_DIMENSIONS.
>>
>> Even an assert to that effect before the enclosing loop didn't avoid
>> the warning turned to error, though; I suppose there might be some
>> aliasing at play, because moving the assert into the loop does, but
>> then, it's not as efficient as testing the index itself against the
>> limit.
>>
>> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?
>>
>> for  gcc/fortran/ChangeLog
>>
>> * simplify.c (simplify_transformation_to_array): Assert the
>> array access is in range.  Fix whitespace.

But once we default to release checking it will warn again?  That said, I think
this kind of workaround is bad :/

Richard.

> OK.
> jeff
>


Re: [-fcompare-debug] find jump before debug insns in expand

2017-01-04 Thread Richard Biener
On Tue, Jan 3, 2017 at 6:28 AM, Alexandre Oliva  wrote:
> A debug insn after the final jump of a basic block may cause the
> expander to emit a dummy move where the non-debug compile won't
> because it finds the jump insn at the end of the insn stream.
>
> Fix the condition so that, instead of requiring the jump as the last
> insn, it also matches a jump followed by debug insns.
>
> This fixes the compilation of libgcc/libgcov-profiler.c with
> -fcompare-debug on i686-linux-gnu.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Ok.

RIchard.

> for  gcc/ChangeLog
>
> * cfgexpand.c (expand_gimple_basic_block): Disregard debug
> insns after final jump in test to emit dummy move.
> ---
>  gcc/cfgexpand.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 97dc648..76bb614 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -5767,7 +5767,9 @@ expand_gimple_basic_block (basic_block bb, bool 
> disable_tail_calls)
>if (single_succ_p (bb)
>&& (single_succ_edge (bb)->flags & EDGE_FALLTHRU)
>&& (last = get_last_insn ())
> -  && JUMP_P (last))
> +  && (JUMP_P (last)
> + || (DEBUG_INSN_P (last)
> + && JUMP_P (prev_nondebug_insn (last)
>  {
>rtx dummy = gen_reg_rtx (SImode);
>emit_insn_after_noloc (gen_move_insn (dummy, dummy), last, NULL);
>
>
> --
> Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [PATCH][PR tree-optimization/78856] Invalidate cached iteration information when threading across multiple loop headers

2017-01-04 Thread Richard Biener
On Wed, Jan 4, 2017 at 6:31 AM, Jeff Law  wrote:
>
> So as noted in the BZ comments the jump threading code has code that detects
> when a jump threading path wants to cross multiple loop headers and
> truncates the jump threading path in that case.
>
> What we should have done instead is invalidate the cached loop information.
>
> Additionally, this BZ shows that just looking at loop headers is not
> sufficient -- we might cross from a reducible to an irreducible region which
> is equivalent to crossing into another loop in that we need to invalidate
> the cached loop iteration information.
>
> What's so damn funny here is that eventually we take nested loops and
> irreducible regions, thread various edges and end up with a nice natural
> loop and no irreducible regions in the end :-)  But the cached iteration
> information is still bogus.
>
> Anyway, this patch corrects both issues.  It treats moving between an
> reducible and irreducible region as crossing a loop header and it
> invalidates the cached iteration information rather than truncating the jump
> thread path.
>
> Bootstrapped and regression tested on x86_64-linux-gnu.  That compiler was
> also used to build all the configurations in config-list.mk.
>
> Installing on the trunk.  I could be convinced to install on the gcc-6
> branch as well since it's affected by the same problem.
>
> Jeff
>
>
> commit 93e3964a4664350446eefe786e3b73eb41d99036
> Author: law 
> Date:   Wed Jan 4 05:31:23 2017 +
>
> PR tree-optimizatin/78856
> * tree-ssa-threadupdate.c: Include tree-vectorizer.h.
> (mark_threaded_blocks): Remove code to truncate thread paths that
> cross multiple loop headers.  Instead invalidate the cached loop
> iteration information and handle case of a thread path walking
> into an irreducible region.
>
> PR tree-optimization/78856
> * gcc.c-torture/execute/pr78856.c: New test.
>
> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@244045
> 138bc75d-0d04-0410-961f-82ee72b054a4
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 3114e02..6b2888f 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-01-03  Jeff Law  
> +
> +   PR tree-optimizatin/78856
> +   * tree-ssa-threadupdate.c: Include tree-vectorizer.h.
> +   (mark_threaded_blocks): Remove code to truncate thread paths that
> +   cross multiple loop headers.  Instead invalidate the cached loop
> +   iteration information and handle case of a thread path walking
> +   into an irreducible region.
> +
>  2016-12-30  Michael Meissner  
>
> PR target/78900
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index cd2a065..cadfbc9 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-01-03  Jeff Law  
> +
> +   PR tree-optimization/78856
> +   * gcc.c-torture/execute/pr78856.c: New test.
> +
>  2017-01-03  Michael Meissner  
>
> PR target/78953
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr78856.c
> b/gcc/testsuite/gcc.c-torture/execute/pr78856.c
> new file mode 100644
> index 000..80f2317
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr78856.c
> @@ -0,0 +1,25 @@
> +extern void exit (int);
> +
> +int a, b, c, d, e, f[3];
> +
> +int main()
> +{
> +  while (d)
> +while (1)
> +  ;
> +  int g = 0, h, i = 0;
> +  for (; g < 21; g += 9)
> +{
> +  int j = 1;
> +  for (h = 0; h < 3; h++)
> +   f[h] = 1;
> +  for (; j < 10; j++) {
> +   d = i && (b ? 0 : c);
> +   i = 1;
> +   if (g)
> + a = e;
> +  }
> +  }
> +  exit (0);
> +}
> +
> diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
> index adbb6e0..2da93a8 100644
> --- a/gcc/tree-ssa-threadupdate.c
> +++ b/gcc/tree-ssa-threadupdate.c
> @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cfgloop.h"
>  #include "dbgcnt.h"
>  #include "tree-cfg.h"
> +#include "tree-vectorizer.h"
>
>  /* Given a block B, update the CFG and SSA graph to reflect redirecting
> one or more in-edges to B to instead reach the destination of an
> @@ -2084,10 +2085,8 @@ mark_threaded_blocks (bitmap threaded_blocks)
>/* Look for jump threading paths which cross multiple loop headers.
>
>   The code to thread through loop headers will change the CFG in ways
> - that break assumptions made by the loop optimization code.
> -
> - We don't want to blindly cancel the requests.  We can instead do
> better
> - by trimming off the end of the jump thread path.  */
> + that invalidate the cached loop iteration information.  So we must
> + detect that case and wipe the cached information.  */
>EXECUTE_IF_SET_IN_BITMAP (tmp, 0, i, bi)
>  {
>basic_block bb = BASIC_BLOCK_FOR_FN (cfun, i);
> @@ -2102,26 +2101,16 @@ mark_threaded_blocks (bitmap threaded_blocks)
>i++)
> {
>   b

Re: [PATCH] Fix ICE with -fno-sso-struct=none (PR driver/78957)

2017-01-04 Thread Richard Biener
On Wed, Jan 4, 2017 at 12:40 PM, Jakub Jelinek  wrote:
> On Wed, Jan 04, 2017 at 12:27:09PM +0100, Richard Biener wrote:
>> On Mon, Jan 2, 2017 at 8:27 PM, Jakub Jelinek  wrote:
>> > Enum options should not allow negative form, otherwise the option handling
>> > ICEs on it.  -fsso-struct= allows only big-endian or little-endian,
>> > -fno-sso-struct=big-endian or -fno-sso-struct=whatever makes no sense.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> Ok, but maybe .opt processing can add RejectNegative on its own for
>> Enum switches then?
>
> Rather than implicit RejectNegative it might be better to just diagnose
> such options as invalid.  If you agree, I can implement that as follow-up.
> Also note that RejectNegative is only needed on the Enum switches that have
> the default negatives (that is [Wfm] prefixed I think).

That would be nice.

>>  For the vectorizer we have (for backward compatibility):
>>
>> fvect-cost-model
>> Common RejectNegative Alias(fvect-cost-model=,dynamic)
>> Enables the dynamic vectorizer cost model.  Preserved for backward
>> compatibility.
>>
>> fno-vect-cost-model
>> Common RejectNegative Alias(fvect-cost-model=,unlimited)
>> Enables the unlimited vectorizer cost model.  Preserved for backward
>> compatibility.
>
> That is a non-= non-Enum option though.

Yes, we do have the Enum variant (with RejectNegative) as well.

>  And not sure why this actually
> is RejectNegative, wouldn't
> Common Alias(fvect-cost-model=,dynamic,unlimited)
> work just on fvect-cost-model (can test that)?

Good question ;)  If it works, ok.

> In this case there is no -fsso-struct option (what would it mean), so
> -fno-sso-struct isn't there either (again, what would that mean).
> The only thing that would make sense IMHO would be to allow
> not just big-endian and little-endian, but also native, so one can
> cancel earlier -fsso-struct= like
> gcc ... -fsso-struct=little-endian ... -fsso-struct=native ...
> and have it act as if neither of those options appeared.
> CCing Eric who has added this option.

Yeah, I agree for -fsso-struct it doesn't make sense to have a -fno-sso-struct.

For the vectorizer we transitioned from -f[no-]vect-cost-model to a
tristate using an Enum and keeping the old form working was requied.

Richard.

>
> Jakub


Re: Use a specfile that actually allows building programs on NetBSD

2017-01-04 Thread coypu
Identical patch was committed to NetBSD in April 28, 2008.
https://github.com/jsonn/src/commit/7105def538f68e0a0034f5c93ec7fc384ca849b2


Re: Pretty printers for versioned namespace

2017-01-04 Thread Jonathan Wakely

On 24/12/16 14:47 +0100, François Dumont wrote:

On 15/12/2016 15:57, Jonathan Wakely wrote:


And we could avoid three re.match expressions with complicated regular
expressions by creating a helper function to do the "startswith"
checks:

def is_specialization_of(type, template_name):
  return re.match('^std::(%s)?%s<.*>$' % (vers_nsp, template_name), 
type) is not None


Then replace impl_type.startswith('std::__uniq_ptr_impl<') with
is_specialization_of(impl_type, '__uniq_ptr_impl')

And replace impl_type.startswith('std::tuple<') with
is_specialization_of(impl_type, 'tuple')

And replace nodetype.name.startswith('std::_Rb_tree_node') with
is_specialization_of(nodetype.name, '_Rb_tree_node')

That makes the code much easier to read.


I agree that hiding the version namespace will be nicer. I just hope 
it is possible. I don't think we can really dictate gdb to hide this 
namespace during the rendering. In the attached path you will see what 
I tried to do. Problem is that I am not sure that printers.py is 
intercepting rendering of all types so version namespace will stay 
visible in those cases.


I think the 2 failures that I have with this patch are reflecting this 
problem:


type = holderstd::__7::char_traits, std::__7::allocatorchar> > >
got: type = holderstd::__7::char_traits, std::__7::allocatorchar> > >

FAIL: libstdc++-prettyprinters/whatis.cc whatis ustring_holder
type = holderstd::__7::char_traits, std::__7::allocator > 


got: type = holderstd::__7::char_traits, std::__7::allocator > 



FAIL: libstdc++-prettyprinters/whatis.cc whatis sstring_holder

Shall I simply use regex in those cases and adopt this approach ?


I'd prefer not to have to use the regex matches in libstdc++.exp as
they complicate things.

For the two examples above, the whatis results are bad even for the
non-versioned namespace. For specializations of basic_string we only
have type printers that recognize the standard typedefs like
std::u16string, but not other specializations. We really want it to
show std::basic_string not the full name. That would
require a TemplateTypePrinter for basic_string. The attached patch
works, and should be easy to incorporate into your changes for the
versioned namespace.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 7690a6b..fd63a5c 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -1372,6 +1372,7 @@ def register_type_printers(obj):
 # Note that we can't have a printer for std::wstreampos, because
 # it shares the same underlying type as std::streampos.
 add_one_type_printer(obj, 'fpos', 'streampos')
+
 add_one_type_printer(obj, 'basic_string', 'u16string')
 add_one_type_printer(obj, 'basic_string', 'u32string')
 
@@ -1397,6 +1398,10 @@ def register_type_printers(obj):
 'unique_ptr<(.*), std::default_delete<\\1 ?> >',
 'unique_ptr<{1}>')
 
+add_one_template_type_printer(obj, 'basic_string',
+'basic_string<((un)?signed char), std::char_traits<\\1 ?>, std::allocator<\\1 ?> >',
+'basic_string<{1}>')
+
 add_one_template_type_printer(obj, 'deque',
 'deque<(.*), std::allocator<\\1 ?> >',
 'deque<{1}>')
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc
index 31ded8b..7a55bb7 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/whatis.cc
@@ -166,11 +166,11 @@ holder knuth_b_holder;
 
 ustring *ustring_ptr;
 holder ustring_holder;
-// { dg-final { whatis-test ustring_holder "holder, std::allocator > >" } }
+// { dg-final { whatis-test ustring_holder "holder >" } }
 
 std::basic_string *sstring_ptr;
 holder< std::basic_string > sstring_holder;
-// { dg-final { whatis-test sstring_holder "holder, std::allocator > >" } }
+// { dg-final { whatis-test sstring_holder "holder >" } }
 
 std::vector>> *seq1_ptr;
 holder< std::vector>> > seq1_holder;


Re: [v3 PATCH] Implement 2801, Default-constructibility of unique_ptr.

2017-01-04 Thread Jonathan Wakely

On 29/12/16 22:10 +0200, Ville Voutilainen wrote:

On 29 December 2016 at 21:57, Ville Voutilainen
 wrote:

Instead of repeating this condition half a dozen times, we could put
it in the __uniq_ptr_impl class template and reuse it, as in the
attached patch (and similarly for the unique_ptr specialization).
What do you think?


It needs to be a bit more dependent than in that patch, I think. I
adjusted the idea a bit,
a new patch attached. It cleans up the code a bit, so it's better, but
not a huge improvement.



Gets a bit better by keeping the __uniq_ptr_impl as just an enable_if,
aliasing its ::type locally
and then using the result in the constraints. Also gets rid of a bunch
of dg-errors in
ptr_deleter_neg.cc. This makes me quite happy. The attached _3.diff
shows the cleanup,
the _4.diff shows the full patch with this cleanup applied.


Looks good. OK for trunk, thanks.



RE: Make MIPS soft-fp preserve NaN payloads for NAN2008

2017-01-04 Thread Maciej W. Rozycki
On Wed, 4 Jan 2017, Matthew Fortune wrote:

> > The MIPS sfp-machine.h has an _FP_CHOOSENAN implementation which
> > emulates hardware semantics of not preserving signaling NaN payloads for
> > an operation with two NaN arguments (although that doesn't suffice to
> > avoid sNaN payload preservation in any case with just one NaN argument).
> > 
> > However, those are only hardware semantics in the legacy NaN case; in
> > the NAN2008 case, the architecture documentation says hardware preserves
> > payloads in such cases.  Furthermore, this implementation assumes legacy
> > NaN semantics, so in the NAN2008 case the implementation actually has
> > the effect of preserving sNaN payloads but not preserving qNaN payloads,
> > when both should be preserved.
> > 
> > This patch fixes the code just to copy from the first argument (at the
> > level of libgcc, it's not meaningful which argument is the first and
> > which is the second).
> > 
> > Tested for mips64-linux-gnu (soft float, NAN2008) with the glibc math/
> > tests.  OK to commit?
> > 
> > 2017-01-02  Joseph Myers  
> > 
> > * config/mips/sfp-machine.h (_FP_CHOOSENAN): Always preserve NaN
> > payload if [__mips_nan2008].
> 
> Thanks for finding and fixing this.
> 
> OK to commit.

 AFAIR we deliberately decided not to define a 2008-NaN soft-float ABI, 
and chose to require all soft-float binaries to use the legacy encoding.

 The implementation of all toolchain pieces reflects this, although 
perhaps the case of passing `-mnan=2008 -msoft-float' to GCC (also keeping 
in mind a `--with-nan=2008' default) has been missed and is not handled 
correctly, either by refusing compilation or by staying with the legacy 
NaN encoding.  GAS might need some tweaks here too.

 FWIW,

  Maciej


Re: [PATCH] Remove padding from DWARF5 headers

2017-01-04 Thread Mark Wielaard
On Wed, Jan 04, 2017 at 12:09:43AM +0100, Jakub Jelinek wrote:
> http://dwarfstd.org/ShowIssue.php?issue=161031.2
> got approved today, so DWARF5 is changing and the various DW_UT_* kinds
> will no longer have the same size of the headers.  So,
> DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
> is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
> DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
> removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
> (length, version and ut kind) are required to be present and the only
> sensible action is to skip the whole unit (using length field).

OK, so I assume my alternative proposal to encode which fields are
used in the unit type field got rejected?
http://dwarfstd.org/ShowIssue.php?issue=161130.5
(Any word on other proposals? They don't seem to have been updated
on the website yet.)

Was anything said about what consumers should do when the version is
unknown? Is the length field still valid and can the unit be skipped
or is it game over?

Is any of the range of unit type values reserved for vendor extensions?

> Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?

Of course. I was already anticipating this would change.

Thanks,

Mark


Re: [PATCH][ARM] ARMv8-M Security Extensions: Warn for unused result for some intrinsics

2017-01-04 Thread Andre Vieira (lists)
On 04/01/17 11:30, Kyrill Tkachov wrote:
> Hi Andre,
> 
> On 04/01/17 11:21, Andre Vieira (lists) wrote:
>> Hello,
>>
>> This patch adds the attribute "warn_unused_result" to the following
>> intrinsics:
>> __cmse_TT{,A,AT,T}_fptr
>> cmse_TT{,A,AT,T}
>> cmse_nonsecure_caller
>> cmse_check_address_range
>>
>> If the result of these intrinsics is not used it means the result of the
>> checks they perform are never used and that could become the source of a
>> security vulnerability in the user's code.  We hope this will limit
>> these.
>>
>> Due to the current limitations of "warn_unused_result", adding them to
>> the __cmse_TT*_fptr intrinsics is pointless since the user will most
>> likely use the macro 'cmse_TT*_fptr' instead, which casts the result of
>> __cmse_TT*_fptr and that seems to be enough to count as a "use". I
>> decided to leave them in there anyway in case the warning becomes a bit
>> smarter in the future. Warnings for cmse_check_pointed_object will never
>> be issued for the same reason. Also if you assign the result of any of
>> these intrinsics to a variable you never use, you will only get a
>> warning about an unused variable, though this warning is not turned on
>> by default.
>>
>> Ran cmse regression tests for arm-none-eabi both ARMv8-M Baseline and
>> Mainline.
>>
>> Is this OK for stage 3?
>>
>> Cheers,
>> Andre
>>
>> gcc/ChangeLog:
>> 2017-01-04  Andre Vieira  
>>
>>  * config/gcc/arm_cmse.h (__cmse_TT_fptr,__cmse_TTA_fptr,
>>  __cmse_TTAT_fptr,__cmse_TTT_fptr,cmse_TT, cmse_TTA, cmse_TTAT,
>>  cmse_TTT, cmse_nonsecure_caller, cmse_check_address_range):
>>  Add warn_unused_result attribute to function declaration.
>>
>> gcc/testsuite/ChangeLog:
>> 2017-01-04  Andre Vieira  
>>
>>  * gcc.target/arm/cmse/cmse-3.c: Add warning tests for the
>>  warn_unused_result warning.
> 
> 
> diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h
> index
> 82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..d37f4e2b446c3c80d56af8b633323837f327973f
> 100644
> --- a/gcc/config/arm/arm_cmse.h
> +++ b/gcc/config/arm/arm_cmse.h
> @@ -116,11 +116,13 @@ typedef void (*__cmse_fptr)(void);
>  }
>  
>  __extension__ static __inline __attribute__ ((__always_inline__))
> +__attribute__ ((__warn_unused_result__))
> 
> Don't add a second __attribute__ annotation, change the first one to be:
>  __attribute__ ((__always_inline__, __warn_unused_result__))
> 
> 
> Ok with that change.
> Thanks,
> Kyrill
> 
Hi Kyrill,

Missed that for some reason...

Changes don't affect ChangeLogs.

Cheers,
Andre
diff --git a/gcc/config/arm/arm_cmse.h b/gcc/config/arm/arm_cmse.h
index 
82b58b1c4f4a12ba6062e2cc2632653788d0eeb7..04983582dac633b649d08ce673e0678c059acf05
 100644
--- a/gcc/config/arm/arm_cmse.h
+++ b/gcc/config/arm/arm_cmse.h
@@ -115,24 +115,28 @@ typedef void (*__cmse_fptr)(void);
   return __result; \
 }
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 __cmse_TT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM ()
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 cmse_TT (void *__p)
 __CMSE_TT_ASM ()
 
 #define cmse_TTT_fptr(p) (__cmse_TTT_fptr ((__cmse_fptr)(p)))
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 __cmse_TTT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (t)
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 cmse_TTT (void *__p)
 __CMSE_TT_ASM (t)
@@ -141,12 +145,14 @@ __CMSE_TT_ASM (t)
 
 #define cmse_TTA_fptr(p) (__cmse_TTA_fptr ((__cmse_fptr)(p)))
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 __cmse_TTA_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (a)
 
-__extension__ static __inline __attribute__ ((__always_inline__))
+__extension__ static __inline
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_address_info_t
 cmse_TTA (void *__p)
 __CMSE_TT_ASM (a)
@@ -154,17 +160,18 @@ __CMSE_TT_ASM (a)
 #define cmse_TTAT_fptr(p) (__cmse_TTAT_fptr ((__cmse_fptr)(p)))
 
 __extension__ static __inline cmse_address_info_t
-__attribute__ ((__always_inline__))
+__attribute__ ((__always_inline__, __warn_unused_result__))
 __cmse_TTAT_fptr (__cmse_fptr __p)
 __CMSE_TT_ASM (at)
 
 __extension__ static __inline cmse_address_info_t
-__attribute__ ((__always_inline__))
+__attribute__ ((__always_inline__, __warn_unused_result__))
 cmse_TTAT (void *__p)
 __CMSE_TT_ASM (at)
 
 /* FIXME: diagnose use outside cmse_nonsecure_entry f

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Alexandre Oliva
On Jan  4, 2017, Richard Biener  wrote:

> On Tue, Jan 3, 2017 at 6:29 AM, Alexandre Oliva  wrote:
>> If we include them in the ICF hash, they may cause congruence_groups to
>> be processed in a different order due to different hashes, which in turn
>> causes different funcdef_nos to be assigned to functions.  Since these
>> numbers are included in -fcompare-debug dumps, they cause failures.

> Is this because ICF simply compares all optimize/target options?

> So it boils down to -g implying -fvar-tracking but -g0 not?

Not quite.  It would be the same if -g or any other option that
shouldn't affect the executable code output were used to compute that
hash.  -g is not among those flags, why should flags that adds detail to
it be?  (Really, -fvar-tracking should have been a -g subflag rather
than an -f one)

> I don't think this is ok -- 'Optimization' doesn't really mean optimization
> but sth we can control per-function.  You essentially remove the ability
> to disable var-tracking just for a single (bad) function.

I see the proposed patch does that indeed.  There are other ways to
accomplish that (disabling var-tracking for a single function), but I
suppose a direct way is important.  So I guess we need some alternate
PerFunction option flag that makes it per-function, but not part of the
ICF hash?

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [bootstrap-O3] add a default initializer to avoid a warning at -O3

2017-01-04 Thread Alexandre Oliva
On Jan  3, 2017, Jason Merrill  wrote:

> Are there bugzillas for these false positive warnings?

I don't think so.

Did you mean in the sense that the patch silences them and thus "fixes"
them, or in the sense that the underlying cause for the warnings is not
fixed and someone might want to look into them at a later time?

FWIW, I hadn't considered the latter (it doesn't seem too hard to find
such warnings anyway; every time I try bootstrap-O3, which is not very
often, I get a few of those), and the former didn't seem enough of a
reason to have bug reports created.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [bootstrap-O3] use unsigned type for regno in df-scan

2017-01-04 Thread Alexandre Oliva
On Jan  3, 2017, Jeff Law  wrote:

> What if REGNO is 2147483648 (assume 32 bit host).  That will get us
> into the else block in df_ref_record as it's >= FIRST_PSEUDO_REGISTER.

> In df_ref_create_structure, we use the same expression to compute
> REGNO, but this time it's interpreted as a signed integer, so
> -2147483648. That gets us into the path where we call
> TEST_HARD_REG_BIT and thus the oob array index.

> Right?

Yup, that's exactly how VRP goes about in concluding that there is
something to warn about.

If we get a pseudo count overflow, I guess we'll have bigger problems
than this one, but VRP doesn't know it ;-)

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [RFA] [PR tree-optimization/33562] [PATCH 1/4] Byte tracking in DSE - v3

2017-01-04 Thread Richard Biener
On Thu, Dec 22, 2016 at 7:26 AM, Jeff Law  wrote:
> This is the first of the 4 part patchkit to address deficiencies in our DSE
> implementation.
>
> This patch addresses the P2 regression 33562 which has been a low priority
> regression since gcc-4.3.  To summarize, DSE no longer has the ability to
> detect an aggregate store as dead if subsequent stores are done in a
> piecemeal fashion.
>
> I originally tackled this by changing how we lower complex objects. That was
> sufficient to address 33562, but was reasonably rejected.
>
> This version attacks the problem by improving DSE to track stores to memory
> at a byte level.  That allows us to determine if a series of stores
> completely covers an earlier store (thus making the earlier store dead).
>
> A useful side effect of this is we can detect when parts of a store are dead
> and potentially rewrite the store.  This patch implements that for complex
> object initializations.  While not strictly part of 33562, it's so closely
> related that I felt it belongs as part of this patch.
>
> This originally limited the size of the tracked memory space to 64 bytes.  I
> bumped the limit after working through the CONSTRUCTOR and mem* trimming
> patches.  The 256 byte limit is still fairly arbitrary and I wouldn't lose
> sleep if we throttled back to 64 or 128 bytes.
>
> Later patches in the kit will build upon this patch.  So if pieces look like
> skeleton code, that's because it is.
>
> The changes since the V2 patch are:
>
> 1. Using sbitmaps rather than bitmaps.
> 2. Returning a tri-state from dse_classify_store (renamed from
> dse_possible_dead_store_p)
> 3. More efficient trim computation
> 4. Moving trimming code out of dse_classify_store
> 5. Refactoring code to delete dead calls/assignments
> 6. dse_optimize_stmt moves into the dse_dom_walker class
>
> Not surprisingly, this patch has most of the changes based on prior feedback
> as it includes the raw infrastructure.
>
> Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?

New functions in sbitmap.c lack function comments.

bitmap_count_bits fails to guard against GCC_VERSION >= 3400 (the version
is not important, but non-GCC host compilers are).  See bitmap.c for a
fallback.

Both bitmap_clear_range and bitmap_set_range look rather inefficient...
(it's not likely anybody will clean this up after you)

I'd say split out the sbitmap.[ch] changes.

+DEFPARAM(PARAM_DSE_MAX_OBJECT_SIZE,
+"dse-max-object-size",
+"Maximum size (in bytes) of objects tracked by dead store
elimination.",
+256, 0, 0)

the docs suggest that DSE doesn't handle larger stores but it does (just in
the original limited way).  Maybe "tracked bytewise" is better.

+static bool
+valid_ao_ref_for_dse (ao_ref *ref)
+{
+  return (ao_ref_base (ref)
+ && ref->max_size != -1
+ && (ref->offset % BITS_PER_UNIT) == 0
+ && (ref->size % BITS_PER_UNIT) == 0
+ && (ref->size / BITS_PER_UNIT) > 0);

I think the last test is better written as ref->size != -1.

Btw, seeing you discount non-byte size/offset stores this somehow asks
for store-merging being done before the last DSE (it currently runs after).
Sth to keep in mind for GCC 8.

+/* Delete a dead store STMT, which is mem* call of some kind.  */

call STMT

+static void
+delete_dead_call (gimple *stmt)
+{
+
excess vertical space
..
+  if (lhs)
+{
+  tree ptr = gimple_call_arg (stmt, 0);
+  gimple *new_stmt = gimple_build_assign (lhs, ptr);
+  unlink_stmt_vdef (stmt);
+  if (gsi_replace (&gsi, new_stmt, true))
+bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index);

  release_ssa_name (gimple_vdef (stmt));


+  { m_live_bytes = sbitmap_alloc (PARAM_VALUE
(PARAM_DSE_MAX_OBJECT_SIZE));m_byte_tracking_enabled = false; }

formatting.

The DSE parts look good to me with the nits above fixed.  Just re-spin
the sbitmap.[ch] part please.

Thanks,
Richard.

>
> PR tree-optimization/33562
> * params.def (PARM_DSE_MAX_OBJECT_SIZE): New PARAM.
> * sbitmap.h (bitmap_clear_range, bitmap_set_range): Prototype new
> functions.
> (bitmap_count_bits): Likewise.
> * sbitmap.c (bitmap_clear_range, bitmap_set_range): New functions.
> (bitmap_count_bits): Likewise.
> * tree-ssa-dse.c: Include params.h.
> (dse_store_status): New enum.
> (initialize_ao_ref_for_dse): New, partially extracted from
> dse_optimize_stmt.
> (valid_ao_ref_for_dse, normalize_ref): New.
> (setup_live_bytes_from_ref, compute_trims): Likewise.
> (clear_bytes_written_by, trim_complex_store): Likewise.
> (maybe_trim_partially_dead_store): Likewise.
> (maybe_trim_complex_store): Likewise.
> (dse_classify_store): Renamed from dse_possibly_dead_store_p.
> Track what bytes live from the original store.  Return tri-state
> for dead, partially dead or live.
> (dse_dom_walker): Add constru

Re: [bootstrap-O3,fortran] silence warning in simplify_transformation_to_array

2017-01-04 Thread Alexandre Oliva
On Jan  4, 2017, Richard Biener  wrote:

> On Tue, Jan 3, 2017 at 7:19 PM, Jeff Law  wrote:
>> On 01/02/2017 10:29 PM, Alexandre Oliva wrote:
>>> * simplify.c (simplify_transformation_to_array): Assert the
>>> array access is in range.  Fix whitespace.

> But once we default to release checking it will warn again?

I guess it will, if one builds with -O3 in that setting.

> That said, I think this kind of workaround is bad :/

I can't say I'm proud of it ;-)

The assert is probably stands on its own: it's reasonable, for
documentation purposes, to state we're not accessing the arrays past
their end, because other parts of the code ensure this is the case.

Now, it's a bit fortunate and unfortunate that adding the assert *also*
silences the warning, because then we add to the patch analysis not just
whether the assert is desirable on its own, but whether the consequence
of silencing the warning in this particular way is desirable.

I figured the assert was still desirable, but if that's not the
consensus, I won't mind dropping the proposed change.  Unfortunately I
don't have another that silences the warning without (or even with)
impact on codegen.

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [RFA] [PR tree-optimization/33562] [PATCH 1/4] Byte tracking in DSE - v3

2017-01-04 Thread Richard Biener
On Wed, Jan 4, 2017 at 2:22 PM, Richard Biener
 wrote:
> On Thu, Dec 22, 2016 at 7:26 AM, Jeff Law  wrote:
>> This is the first of the 4 part patchkit to address deficiencies in our DSE
>> implementation.
>>
>> This patch addresses the P2 regression 33562 which has been a low priority
>> regression since gcc-4.3.  To summarize, DSE no longer has the ability to
>> detect an aggregate store as dead if subsequent stores are done in a
>> piecemeal fashion.
>>
>> I originally tackled this by changing how we lower complex objects. That was
>> sufficient to address 33562, but was reasonably rejected.
>>
>> This version attacks the problem by improving DSE to track stores to memory
>> at a byte level.  That allows us to determine if a series of stores
>> completely covers an earlier store (thus making the earlier store dead).
>>
>> A useful side effect of this is we can detect when parts of a store are dead
>> and potentially rewrite the store.  This patch implements that for complex
>> object initializations.  While not strictly part of 33562, it's so closely
>> related that I felt it belongs as part of this patch.
>>
>> This originally limited the size of the tracked memory space to 64 bytes.  I
>> bumped the limit after working through the CONSTRUCTOR and mem* trimming
>> patches.  The 256 byte limit is still fairly arbitrary and I wouldn't lose
>> sleep if we throttled back to 64 or 128 bytes.
>>
>> Later patches in the kit will build upon this patch.  So if pieces look like
>> skeleton code, that's because it is.
>>
>> The changes since the V2 patch are:
>>
>> 1. Using sbitmaps rather than bitmaps.
>> 2. Returning a tri-state from dse_classify_store (renamed from
>> dse_possible_dead_store_p)
>> 3. More efficient trim computation
>> 4. Moving trimming code out of dse_classify_store
>> 5. Refactoring code to delete dead calls/assignments
>> 6. dse_optimize_stmt moves into the dse_dom_walker class
>>
>> Not surprisingly, this patch has most of the changes based on prior feedback
>> as it includes the raw infrastructure.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?
>
> New functions in sbitmap.c lack function comments.
>
> bitmap_count_bits fails to guard against GCC_VERSION >= 3400 (the version
> is not important, but non-GCC host compilers are).  See bitmap.c for a
> fallback.
>
> Both bitmap_clear_range and bitmap_set_range look rather inefficient...
> (it's not likely anybody will clean this up after you)
>
> I'd say split out the sbitmap.[ch] changes.
>
> +DEFPARAM(PARAM_DSE_MAX_OBJECT_SIZE,
> +"dse-max-object-size",
> +"Maximum size (in bytes) of objects tracked by dead store
> elimination.",
> +256, 0, 0)
>
> the docs suggest that DSE doesn't handle larger stores but it does (just in
> the original limited way).  Maybe "tracked bytewise" is better.

Oh, and new --params need documeting in invoke.texi.

Richard.

> +static bool
> +valid_ao_ref_for_dse (ao_ref *ref)
> +{
> +  return (ao_ref_base (ref)
> + && ref->max_size != -1
> + && (ref->offset % BITS_PER_UNIT) == 0
> + && (ref->size % BITS_PER_UNIT) == 0
> + && (ref->size / BITS_PER_UNIT) > 0);
>
> I think the last test is better written as ref->size != -1.
>
> Btw, seeing you discount non-byte size/offset stores this somehow asks
> for store-merging being done before the last DSE (it currently runs after).
> Sth to keep in mind for GCC 8.
>
> +/* Delete a dead store STMT, which is mem* call of some kind.  */
>
> call STMT
>
> +static void
> +delete_dead_call (gimple *stmt)
> +{
> +
> excess vertical space
> ..
> +  if (lhs)
> +{
> +  tree ptr = gimple_call_arg (stmt, 0);
> +  gimple *new_stmt = gimple_build_assign (lhs, ptr);
> +  unlink_stmt_vdef (stmt);
> +  if (gsi_replace (&gsi, new_stmt, true))
> +bitmap_set_bit (need_eh_cleanup, gimple_bb (stmt)->index);
>
>   release_ssa_name (gimple_vdef (stmt));
>
>
> +  { m_live_bytes = sbitmap_alloc (PARAM_VALUE
> (PARAM_DSE_MAX_OBJECT_SIZE));m_byte_tracking_enabled = false; }
>
> formatting.
>
> The DSE parts look good to me with the nits above fixed.  Just re-spin
> the sbitmap.[ch] part please.
>
> Thanks,
> Richard.
>
>>
>> PR tree-optimization/33562
>> * params.def (PARM_DSE_MAX_OBJECT_SIZE): New PARAM.
>> * sbitmap.h (bitmap_clear_range, bitmap_set_range): Prototype new
>> functions.
>> (bitmap_count_bits): Likewise.
>> * sbitmap.c (bitmap_clear_range, bitmap_set_range): New functions.
>> (bitmap_count_bits): Likewise.
>> * tree-ssa-dse.c: Include params.h.
>> (dse_store_status): New enum.
>> (initialize_ao_ref_for_dse): New, partially extracted from
>> dse_optimize_stmt.
>> (valid_ao_ref_for_dse, normalize_ref): New.
>> (setup_live_bytes_from_ref, compute_trims): Likewise.
>> (clear_bytes_written_by, trim_complex_store): Likewise.
>> (maybe_trim_partially_dead_s

Re: [RFA][PR tree-optimization/61912] [PATCH 2/4] Trimming CONSTRUCTOR stores in DSE - V3

2017-01-04 Thread Richard Biener
On Thu, Dec 22, 2016 at 7:26 AM, Jeff Law  wrote:
> This is the second patch in the kit to improve our DSE implementation.
>
> This patch recognizes when a CONSTRUCTOR assignment could be trimmed at the
> head or tail because those bytes are dead.
>
> The first implementation of this turned the CONSTRUCTOR into a memset. This
> version actually rewrites the RHS and LHS of the CONSTRUCTOR assignment.
>
> You'll note that the implementation computes head and tail trim counts, then
> masks them to an even byte count.  We might even consider masking off the
> two low bits in the counts.  This masking keeps higher alignments on the
> CONSTRUCTOR remnant which helps keep things efficient when the CONSTRUCTOR
> results in a memset call.
>
> This patch hits a lot statically in GCC and the testsuite.  There were
> hundreds of hits in each.
>
> There may be some room for tuning.  Trimming shouldn't ever result in poorer
> performance, but it may also not result in any measurable gain (it depends
> on how much gets trimmed relative to the size of the CONSTRUCTOR node and
> how the CONSTRUCTOR node gets expanded, the processor's capabilities for
> merging stores internally, etc etc).  I suspect the main benefit comes when
> the CONSTRUCTOR collapses down to some thing small that gets expanded
> inline, thus exposing the internals to the rest of the optimization
> pipeline.
>
> We could, in theory, split the CONSTRUCTOR to pick up dead bytes in the
> middle of the CONSTRUCTOR.  I haven't looked to see how applicable that is
> in real code and what the cost/benefit analysis might look like.
>
>
> Bootstrapped and regression tested on x86_64-linux-gnu.  OK for the trunk?

+  categorize_ctor_elements (ctor, &nz_elts, &init_elts, &complete);
+
+  /* This is the only case we currently handle.  It actually seems to
+ catch most cases of actual interest.  */
+  if (init_elts == 0 && !CONSTRUCTOR_NO_CLEARING (ctor))

actually the _only_ case allowed as RHS of stores is an empty CONSTRUCTOR
and thus clearing of the object.  Thus

  gcc_assert (CONSTRUCTOR_NELTS (ctor) == 0);

should work here.

+ /* And the new type for the CONSTRUCTOR.  Essentially it's just
+a char array large enough to cover the non-trimmed parts of
+the original CONSTRUCTOR.  Note we want explicit bounds here
+so that we know how many bytes to clear when expanding the
+CONSTRUCTOR.  */
+ tree type = build_range_type (sizetype, size_zero_node,
+   build_int_cst (sizetype, count));
+ type = build_array_type (char_type_node, type);

there is build_array_type_nelts combining both (note nelts rather than
upper bound).

+ /* Build a MEM_REF representing the whole accessed area, starting
+at the first byte not trimmed.  */
+ tree exp = fold_build2 (MEM_REF, type, lhs_addr,
+ build_int_cst (build_pointer_type (type),
+head_trim));

this doesn't preserve alias info but pessimistically uses alias set
zero.  I believe
you can use

tree alias_type = reference_alias_ptr_type (gimple_assign_lhs (stmt));
..., build_int_cst (alias_type, head_trim)

to do better.

Note that you end up generating invalid gimple (not sure what you are doing wiht
the tree-sra stuff in this patch - it seems to be unused).  Consider

a[i] = {};

where trimming generates

   MEM[&a[i], head_trim] = {};

that is invalid GIMPLE.  I guess we never run into this case because for the LHS
max_size != size during analysis?  To be on the safe side please add a

  if (! is_gimple_min_invariant (lhs_addr))
return;

after computing lhs_addr.

Sth to keep in mind is that nothing on GIMPLE changes the store from a
CONSTRUCTOR
to a store from literal zero.  So even if you end up with a int-size
and int-aligned

  MEM  [&x, 12] = {};

nothing (but RTL expansion) rewrites it to

  MEM  [&x, 12] = 0;

that's something missing from gimple-fold.c (some code can be shared with
MEMSET folding I guess).

Otherwise the patch looks ok to me.

Thanks,
Richard.

> PR tree-optimization/61912
> PR tree-optimization/77485
> * tree-sra.h: New file.
> * ipa-cp.c: Include tree-sra.h
> * ipa-prop.h (build_ref_for_offset): Remove prototype.
> * tree-ssa-dse.c: Include expr.h and tree-sra.h.
> (compute_trims, trim_constructor_store): New functions.
> (maybe_trim_partially_dead_store): Call trim_constructor_store.
>
>
> * g++.dg/tree-ssa/ssa-dse-1.C: New test.
> * gcc.dg/tree-ssa/pr30375: Adjust expected output.
> * gcc.dg/tree-ssa/ssa-dse-24.c: New test.
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index d3b5052..bc5ea87 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -122,6 +122,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-inline.h"
>  #include "ipa-utils.h"
>  #include "tree-

Re: [bootstrap-O3] add a default initializer to avoid a warning at -O3

2017-01-04 Thread Jason Merrill
On Wed, Jan 4, 2017 at 8:13 AM, Alexandre Oliva  wrote:
> On Jan  3, 2017, Jason Merrill  wrote:
>
>> Are there bugzillas for these false positive warnings?
>
> I don't think so.
>
> Did you mean in the sense that the patch silences them and thus "fixes"
> them, or in the sense that the underlying cause for the warnings is not
> fixed and someone might want to look into them at a later time?
>
> FWIW, I hadn't considered the latter (it doesn't seem too hard to find
> such warnings anyway; every time I try bootstrap-O3, which is not very
> often, I get a few of those), and the former didn't seem enough of a
> reason to have bug reports created.

The latter; false positive warnings like this are bugs that should be fixed.

Jason


Re: [PATCH] Remove padding from DWARF5 headers

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 01:58:33PM +0100, Mark Wielaard wrote:
> On Wed, Jan 04, 2017 at 12:09:43AM +0100, Jakub Jelinek wrote:
> > http://dwarfstd.org/ShowIssue.php?issue=161031.2
> > got approved today, so DWARF5 is changing and the various DW_UT_* kinds
> > will no longer have the same size of the headers.  So,
> > DW_UT_compile/DW_UT_partial shrinks by 12/16 bytes (padding 1 and padding 2
> > is removed; 16 bytes for 64-bit DWARF), DW_UT_type remains the same,
> > DW_UT_skeleton/DW_UT_split_compile shrink by 4/8 bytes (padding 2 is
> > removed).  For DW_UT_* kinds consumers don't understand, the first 3 fields
> > (length, version and ut kind) are required to be present and the only
> > sensible action is to skip the whole unit (using length field).
> 
> OK, so I assume my alternative proposal to encode which fields are
> used in the unit type field got rejected?
> http://dwarfstd.org/ShowIssue.php?issue=161130.5
> (Any word on other proposals? They don't seem to have been updated
> on the website yet.)

It will take a while before the web is updated and the changes actually
materialize in the document.

Yes, 161130.5 has been rejected.

As for other proposals, not sure if I remember all the details, I think
160808.1 is in, 161027.1 will say that the hashing of identifiers in
.debug_names (no matter if case sensitive or not) roughly starts by
replacing U+0131 characters with U+0069 (i.e. use i instead of small
dotless i), then applying C + S case folding from Unicode 9.0's
CaseFolding.txt and then hashing the resulting string as described in the
public draft.  At least that is my understanding.  161031.2 accepted,
161031.4 deferred, 161102.1 accepted, 161230.1 accepted, 161122.1
I think we are going to have DW_FORM_strx{1,2,3,4} or something similar
in addition to DW_FORM_strx (i.e. fixes size 1, 2, 3, 4 byte indexes
into .debug_addr), 161130.2 rejected (DW_AT_type should be used instead),
161130.3 rejected, 161206.[123] need more thinking, 161215.[12] rejected,
don't remember others.

> Was anything said about what consumers should do when the version is
> unknown? Is the length field still valid and can the unit be skipped
> or is it game over?

See above, I tried to explain it.  All the units have to have the
unit_length (initial length)
version (uhalf)
unit_type (ubyte)
fields at the beginning of their headers in that order (otherwise you
couldn't even find out what the unit type is), the rest is well defined
only for the unit types documented in the spec.
So, for other unit types (unless they know how to handle them), consumers
should just skip over it (i.e. advance by unit_length from the start of
the version field.

> Is any of the range of unit type values reserved for vendor extensions?

Yes, DW_UT_lo_user ... DW_UT_hi_user.

Jakub


Re: [RFA] [PATCH 4/4] Ignore reads of "dead" memory locations in DSE

2017-01-04 Thread Richard Biener
On Thu, Dec 22, 2016 at 7:26 AM, Jeff Law  wrote:
> This is the final patch in the kit to improve our DSE implementation.
>
> It's based on a observation by Richi.  Namely that a read from bytes of
> memory that are dead can be ignored.  By ignoring such reads we can
> sometimes find additional stores that allow us to either eliminate or trim
> an earlier store more aggressively.
>
> This only hit (by hit I mean the ability to ignore resulted in finding a
> full or partially dead store that we didn't otherwise find) once during a
> bootstrap, but does hit often in the libstdc++ testsuite.  I've added a test
> derived from the conversation between myself and Richi last year.
>
> There's nothing in the BZ database on this issue and I can't reasonably call
> it a bugfix.  I wouldn't lose sleep if this deferred to gcc-8.
>
> Bootstrapped and regression tested on x86-64-linux-gnu.  OK for the trunk or
> defer to gcc-8?
>
>
>
> * tree-ssa-dse.c (live_bytes_read): New function.
> (dse_classify_store): Ignore reads of dead bytes.
>
> * testsuite/gcc.dg/tree-ssa/ssa-dse-26.c: New test.
> * testsuite/gcc.dg/tree-ssa/ssa-dse-26.c: Likewise.
>
>
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
> new file mode 100644
> index 000..6605dfe
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-dse1-details" } */
> +
> +enum constraint_expr_type
> +{
> +  SCALAR, DEREF, ADDRESSOF
> +};
> +typedef struct constraint_expr
> +{
> +  enum constraint_expr_type type;
> +  unsigned int var;
> +  long offset;
> +} constraint_expr ;
> +typedef struct constraint
> +{
> +  struct constraint_expr lhs;
> +  struct constraint_expr rhs;
> +} constraint;
> +static _Bool
> +constraint_expr_equal (struct constraint_expr x, struct constraint_expr y)
> +{
> +  return x.type == y.type && x.var == y.var && x.offset == y.offset;
> +}
> +
> +_Bool
> +constraint_equal (struct constraint a, struct constraint b)
> +{
> +  return constraint_expr_equal (a.lhs, b.lhs)
> +&& constraint_expr_equal (a.rhs, b.rhs);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Deleted dead store" 2 "dse1" } } */
> +
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-27.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-27.c
> new file mode 100644
> index 000..48dc92e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-27.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-dse1-details -fno-tree-fre -fno-tree-sra"
> } */
> +
> +struct S { struct R { int x; int y; } r; int z; } s;
> +
> +extern void blah (struct S);
> +
> +void
> +foo ()
> +{
> +  struct S s = { {1, 2}, 3 };
> +  s.r.x = 1;
> +   s.r.y = 2;
> +   struct R r = s.r;
> +  s.z = 3;
> +  blah (s);
> +}
> +
> +
> +/* { dg-final { scan-tree-dump-times "Deleted dead store" 4 "dse1" } } */
> +
> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
> index a807d6d..f5b53fc 100644
> --- a/gcc/tree-ssa-dse.c
> +++ b/gcc/tree-ssa-dse.c
> @@ -475,6 +475,41 @@ maybe_trim_partially_dead_store (ao_ref *ref, sbitmap
> live, gimple *stmt)
>  }
>  }
>
> +/* Return TRUE if USE_REF reads bytes from LIVE where live is
> +   derived from REF, a write reference.
> +
> +   While this routine may modify USE_REF, it's passed by value, not
> +   location.  So callers do not see those modifications.  */
> +
> +static bool
> +live_bytes_read (ao_ref use_ref, ao_ref *ref, sbitmap live)
> +{
> +  /* We have already verified that USE_REF and REF hit the same object.
> + Now verify that there's actually an overlap between USE_REF and REF.
> */
> +  if ((use_ref.offset < ref->offset
> +   && use_ref.offset + use_ref.size > ref->offset)
> +  || (use_ref.offset >= ref->offset
> + && use_ref.offset < ref->offset + ref->size))

can you use ranges_overlap_p? (tree-ssa-alias.h)

> +{
> +  normalize_ref (&use_ref, ref);
> +
> +  /* If USE_REF covers all of REF, then it will hit one or more
> +live bytes.   This avoids useless iteration over the bitmap
> +below.  */
> +  if (use_ref.offset == ref->offset && use_ref.size == ref->size)
> +   return true;
> +
> +  /* Now iterate over what's left in USE_REF and see if any of
> +those bits are i LIVE.  */
> +  for (int i = (use_ref.offset - ref->offset) / BITS_PER_UNIT;
> +  i < (use_ref.offset + use_ref.size) / BITS_PER_UNIT; i++)
> +   if (bitmap_bit_p (live, i))

a bitmap_bit_in_range_p () would be nice to have.  And it can be more
efficient than this loop...

> + return true;
> +  return false;
> +}
> +  return true;
> +}
> +
>  /* A helper of dse_optimize_stmt.
> Given a GIMPLE_ASSIGN in STMT that writes to REF, find a candidate
> statement *USE_STMT that may prove STMT to be dead.
> @@ -554,6 +589,41 @@ dse_classify_store (ao_ref *ref, gimple *stmt, gimple
>

Re: [PATCH] Remove padding from DWARF5 headers

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 02:46:51PM +0100, Jakub Jelinek wrote:
> As for other proposals, not sure if I remember all the details, I think
> 160808.1 is in, 161027.1 will say that the hashing of identifiers in
> .debug_names (no matter if case sensitive or not) roughly starts by
> replacing U+0131 characters with U+0069 (i.e. use i instead of small

U+0130 as well with U+0069.  Basically the hashing is non-Turkish as well
as Turkish locale compatible, as all of U+0049, U+0069, U+0130 and U+0131
are hashed the same.

Jakub


Re: [PATCH] PR c++/66735 lambda capture by reference

2017-01-04 Thread Nathan Sidwell

On 01/04/2017 12:36 AM, Jason Merrill wrote:

On 01/03/2017 08:57 AM, Nathan Sidwell wrote:



+  type = auto_node;
+  if (by_reference_p)



+  type = build_reference_type (type);
+  by_reference_p = false;

^^^ here

+  if (!is_this && by_reference_p)
+type = build_reference_type (type);


This looks like it will call build_reference_type twice in the explicit
init case, producing a reference to reference.


Note the clearing of by_reference_p just above in this case, but I admit 
that's confusing.  Anyway, it's rendered moot by this patch which addresses:



 if (DECLTYPE_FOR_LAMBDA_CAPTURE (t))
   type = lambda_capture_field_type (type,
-DECLTYPE_FOR_INIT_CAPTURE (t));
+DECLTYPE_FOR_INIT_CAPTURE (t),
+/*by_reference_p=*/false);


Always passing false seems unlikely to be correct.


I wondered about that, but obviously failed to come up with a testcase 
to expose it.  This patch adds such a case, and exposes the need to add 
another flag to DECLTYPE to indicate reference capture -- rather than 
wrap the dependent capture in a reference type itself, as was previously 
the case.


ok?

nathan

--
Nathan Sidwell
2017-01-04  Nathan Sidwell  

	cp/
	PR c++/66735
	* cp-tree.h (DECLTYPE_FOR_REF_CAPTURE): New.
	(lambda_capture_field_type): Update prototype.
	* lambda.c (lambda_capture_field_type): Add is_reference parm.
	Add referenceness here.
	(add_capture): Adjust lambda_capture_field_type call, refactor
	error checking.
	* pt.c (tsubst): Adjust lambda_capture_field_type call.

	testsuite/
	PR c++/66735
	* g++.dg/cpp1y/pr66735.C: New.

Index: cp/cp-tree.h
===
--- cp/cp-tree.h	(revision 244054)
+++ cp/cp-tree.h	(working copy)
@@ -181,6 +181,7 @@ operator == (const cp_expr &lhs, tree rh
   BIND_EXPR_BODY_BLOCK (in BIND_EXPR)
   DECL_NON_TRIVIALLY_INITIALIZED_P (in VAR_DECL)
   CALL_EXPR_ORDERED_ARGS (in CALL_EXPR, AGGR_INIT_EXPR)
+  DECLTYPE_FOR_REF_CAPTURE (in DECLTYPE_TYPE)
4: TREE_HAS_CONSTRUCTOR (in INDIRECT_REF, SAVE_EXPR, CONSTRUCTOR,
 	  CALL_EXPR, or FIELD_DECL).
   IDENTIFIER_TYPENAME_P (in IDENTIFIER_NODE)
@@ -4103,6 +4104,8 @@ more_aggr_init_expr_args_p (const aggr_i
   TREE_LANG_FLAG_1 (DECLTYPE_TYPE_CHECK (NODE))
 #define DECLTYPE_FOR_LAMBDA_PROXY(NODE) \
   TREE_LANG_FLAG_2 (DECLTYPE_TYPE_CHECK (NODE))
+#define DECLTYPE_FOR_REF_CAPTURE(NODE) \
+  TREE_LANG_FLAG_3 (DECLTYPE_TYPE_CHECK (NODE))
 
 /* Nonzero for VAR_DECL and FUNCTION_DECL node means that `extern' was
specified in its declaration.  This can also be set for an
@@ -6528,7 +6531,7 @@ extern tree finish_trait_expr			(enum cp
 extern tree build_lambda_expr   (void);
 extern tree build_lambda_object			(tree);
 extern tree begin_lambda_type   (tree);
-extern tree lambda_capture_field_type		(tree, bool);
+extern tree lambda_capture_field_type		(tree, bool, bool);
 extern tree lambda_return_type			(tree);
 extern tree lambda_proxy_type			(tree);
 extern tree lambda_function			(tree);
Index: cp/lambda.c
===
--- cp/lambda.c	(revision 244054)
+++ cp/lambda.c	(working copy)
@@ -211,29 +211,45 @@ lambda_function (tree lambda)
 }
 
 /* Returns the type to use for the FIELD_DECL corresponding to the
-   capture of EXPR.
-   The caller should add REFERENCE_TYPE for capture by reference.  */
+   capture of EXPR.  EXPLICIT_INIT_P indicates whether this is a
+   C++14 init capture, and BY_REFERENCE_P indicates whether we're
+   capturing by reference.  */
 
 tree
-lambda_capture_field_type (tree expr, bool explicit_init_p)
+lambda_capture_field_type (tree expr, bool explicit_init_p,
+			   bool by_reference_p)
 {
   tree type;
   bool is_this = is_this_parameter (tree_strip_nop_conversions (expr));
+
   if (!is_this && type_dependent_expression_p (expr))
 {
   type = cxx_make_type (DECLTYPE_TYPE);
   DECLTYPE_TYPE_EXPR (type) = expr;
   DECLTYPE_FOR_LAMBDA_CAPTURE (type) = true;
   DECLTYPE_FOR_INIT_CAPTURE (type) = explicit_init_p;
+  DECLTYPE_FOR_REF_CAPTURE (type) = by_reference_p;
   SET_TYPE_STRUCTURAL_EQUALITY (type);
 }
   else if (!is_this && explicit_init_p)
 {
-  type = make_auto ();
-  type = do_auto_deduction (type, expr, type);
+  tree auto_node = make_auto ();
+  
+  type = auto_node;
+  if (by_reference_p)
+	/* Add the reference now, so deduction doesn't lose
+	   outermost CV qualifiers of EXPR.  */
+	type = build_reference_type (type);
+  type = do_auto_deduction (type, expr, auto_node);
 }
   else
-type = non_reference (unlowered_expr_type (expr));
+{
+  type = non_reference (unlowered_expr_type (expr));
+
+  if (!is_this && by_reference_p)
+	type = build_reference_type (type);
+}
+
   return type;
 }
 
@@ -504,9 +520,11 @@ add_capture (tree lambda, tree id, tree
  

Re: [PATCH] Remove padding from DWARF5 headers

2017-01-04 Thread Jan Kratochvil
On Wed, 04 Jan 2017 00:09:43 +0100, Jakub Jelinek wrote:
> Jan/Mark, are you going to adjust GDB/elfutils etc. correspondingly?

My GDB DWARF-5 patchset is still off-trunk so that is sure OK.


Jan


Re: [RFA][PATCH 3/4] Trim mem* calls in DSE

2017-01-04 Thread Richard Biener
On Fri, Dec 16, 2016 at 2:54 AM, Jeff Law  wrote:
> This is the 3rd patch in the kit to improve our DSE implementation.
>
> This patch supports trimming of the head or tail of a memset, memcpy or
> memmove call.  It's conceptually similar to trimming CONSTRUCTORS (and was
> in fact developed first).
>
> Try as I might, I couldn't find a BZ in our database that would be resolved
> by this patch.  There's BZs in the LLVM database in this space, but I didn't
> actually test those.  With that in mind, I don't think I can strictly call
> this a bugfix.  It does represent closing a deficiency when compared to
> LLVM.So while I'd like to see it go onto the trunk, I won't lose sleep
> if we defer to gcc-8.
>
> Note this patch relies on the alignment tweak I mentioned in the discussion
> of patch #2 to avoid creating code that the strlen folding optimization
> can't optimize.  The code is still correct/valid, it's just in a form that
> the strlen folders don't grok.
>
> This includes a trivial test that I used for development purposes.  It hits
> fairly often building GCC itself.  If we wanted more coverage i the
> testsuite, I could extract some tests from GCC and reduce them.
>
> This patch has (of course) been bootstrapped and regression tested on
> x86_64-linux-gnu.  OK for the trunk or defer to gcc-8?

Didn't see a re-post of this one so reviewing the old.

>
> * tree-ssa-dse.c (need_ssa_update): New file scoped boolean.
> (decrement_count): New function.
> (increment_start_addr, trim_memstar_call): Likewise.
> (trim_partially_dead_store): Call trim_memstar_call.
> (pass_dse::execute): Initialize need_ssa_update.  If set, then
> return TODO_ssa_update.
>
> * gcc.dg/tree-ssa/ssa-dse-25.c: New test.
>
> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
> index 1482c7f..b21b9b5 100644
> --- a/gcc/tree-ssa-dse.c
> +++ b/gcc/tree-ssa-dse.c
> @@ -79,6 +80,10 @@ static bitmap need_eh_cleanup;
> It is always safe to return FALSE.  But typically better optimziation
> can be achieved by analyzing more statements.  */
>
> +/* If trimming stores requires insertion of new statements, then we
> +   will need an SSA update.  */
> +static bool need_ssa_update;
> +

huh?  You set this to true after inserting a POINTER_PLUS_EXPR, I don't see
how you need an SSA update for this.

>  static bool
>  initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write)
>  {
> @@ -309,6 +314,113 @@ trim_constructor_store (bitmap orig, bitmap live,
> gimple *stmt)
>  }
>  }
>
> +/* STMT is a memcpy, memmove or memset.  Decrement the number of bytes
> +   copied/set by DECREMENT.  */
> +static void
> +decrement_count (gimple *stmt, int decrement)
> +{
> +  tree *countp = gimple_call_arg_ptr (stmt, 2);
> +  gcc_assert (TREE_CODE (*countp) == INTEGER_CST);
> +  tree x = fold_build2 (MINUS_EXPR, TREE_TYPE (*countp), *countp,
> +   build_int_cst (TREE_TYPE (*countp), decrement));
> +  *countp = x;

thanks to wide-int the following should work

   *countp = wide_int_to_tree (TREE_TYPE (*countp), *countp - decrement);

(if not please use int_const_binop rather than fold_build2 here and
below as well)

> +}
> +
> +static void
> +increment_start_addr (gimple *stmt ATTRIBUTE_UNUSED, tree *where, int
> increment)
> +{
> +  /* If the address wasn't initially a MEM_REF, make it a MEM_REF.  */
> +  if (TREE_CODE (*where) == ADDR_EXPR
> +  && TREE_CODE (TREE_OPERAND (*where, 0)) != MEM_REF)
> +{
> +  tree t = TREE_OPERAND (*where, 0);
> +  t = build_ref_for_offset (EXPR_LOCATION (t), t,
> +   increment * BITS_PER_UNIT, false,
> +   ptr_type_node, NULL, false);

please don't use build_ref_for_offset for this.  Simply only handle the SSA_NAME
case here and below ...

> +  *where = build_fold_addr_expr (t);
> +  return;
> +}
> +  else if (TREE_CODE (*where) == SSA_NAME)
> +{
> +  tree tem = make_ssa_name (TREE_TYPE (*where));
> +  gassign *newop
> += gimple_build_assign (tem, POINTER_PLUS_EXPR, *where,
> +  build_int_cst (sizetype, increment));
> +  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> +  gsi_insert_before (&gsi, newop, GSI_SAME_STMT);
> +  need_ssa_update = true;
> +  *where = tem;
> +  update_stmt (gsi_stmt (gsi));
> +  return;
> +}
> +
> +  /* We can just adjust the offset in the MEM_REF expression.  */
> +  tree x1 = TREE_OPERAND (TREE_OPERAND (*where, 0), 1);
> +  tree x = fold_build2 (PLUS_EXPR, TREE_TYPE (x1), x1,
> +   build_int_cst (TREE_TYPE (x1), increment));
> +  TREE_OPERAND (TREE_OPERAND (*where, 0), 1) = x;
...

re-fold the thing as MEM_REF which will do all the magic for you:

  *where = build_fold_addr_expr (fold_build2 (MEM_REF, char_type_node,
*where, build_int_cst (ptr_type_node, increment)));

that handles &MEM[] and &foo.bar just fine and avoids adding magic he

Re: [-fcompare-debug] var tracking options are not optimization options

2017-01-04 Thread Richard Biener
On Wed, Jan 4, 2017 at 2:08 PM, Alexandre Oliva  wrote:
> On Jan  4, 2017, Richard Biener  wrote:
>
>> On Tue, Jan 3, 2017 at 6:29 AM, Alexandre Oliva  wrote:
>>> If we include them in the ICF hash, they may cause congruence_groups to
>>> be processed in a different order due to different hashes, which in turn
>>> causes different funcdef_nos to be assigned to functions.  Since these
>>> numbers are included in -fcompare-debug dumps, they cause failures.
>
>> Is this because ICF simply compares all optimize/target options?
>
>> So it boils down to -g implying -fvar-tracking but -g0 not?
>
> Not quite.  It would be the same if -g or any other option that
> shouldn't affect the executable code output were used to compute that
> hash.  -g is not among those flags, why should flags that adds detail to
> it be?  (Really, -fvar-tracking should have been a -g subflag rather
> than an -f one)
>
>> I don't think this is ok -- 'Optimization' doesn't really mean optimization
>> but sth we can control per-function.  You essentially remove the ability
>> to disable var-tracking just for a single (bad) function.
>
> I see the proposed patch does that indeed.  There are other ways to
> accomplish that (disabling var-tracking for a single function), but I
> suppose a direct way is important.  So I guess we need some alternate
> PerFunction option flag that makes it per-function, but not part of the
> ICF hash?

Yeah, I suppose ICF cares about those flags for the same reason
the inliner does -- just the inliner does it more fine-grained.

I suppose "[un]setting" opt[12]->x_flag_var_tracking before/after
hash compute and comparison does the trick (with a big fat comment).

That said, the inliner has all the explicit check_maybe_up/down checks
looking for options that make semantic differences and honors
explicit attribute "optimized" 1:1.  But I guess that's nothing we can
change ICF to for GCC 7.

Martin?

Thanks,
Richard.

>
> --
> Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [bootstrap-O3,fortran] silence warning in simplify_transformation_to_array

2017-01-04 Thread Richard Biener
On Wed, Jan 4, 2017 at 2:23 PM, Alexandre Oliva  wrote:
> On Jan  4, 2017, Richard Biener  wrote:
>
>> On Tue, Jan 3, 2017 at 7:19 PM, Jeff Law  wrote:
>>> On 01/02/2017 10:29 PM, Alexandre Oliva wrote:
 * simplify.c (simplify_transformation_to_array): Assert the
 array access is in range.  Fix whitespace.
>
>> But once we default to release checking it will warn again?
>
> I guess it will, if one builds with -O3 in that setting.
>
>> That said, I think this kind of workaround is bad :/
>
> I can't say I'm proud of it ;-)
>
> The assert is probably stands on its own: it's reasonable, for
> documentation purposes, to state we're not accessing the arrays past
> their end, because other parts of the code ensure this is the case.
>
> Now, it's a bit fortunate and unfortunate that adding the assert *also*
> silences the warning, because then we add to the patch analysis not just
> whether the assert is desirable on its own, but whether the consequence
> of silencing the warning in this particular way is desirable.
>
> I figured the assert was still desirable, but if that's not the
> consensus, I won't mind dropping the proposed change.  Unfortunately I
> don't have another that silences the warning without (or even with)
> impact on codegen.

#pragma GCC diagnostic push ("Wno-array-bounds")

#pragma GCC diagnostic pop

? (well, fix the syntax for me please ;))

Richard.

>
> --
> Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [PATCH][tree-ssa-address] Use simplify_gen_binary in gen_addr_rtx

2017-01-04 Thread Richard Biener
On Wed, Dec 21, 2016 at 10:40 AM, Kyrill Tkachov
 wrote:
>
> On 20/12/16 17:30, Richard Biener wrote:
>>
>> On December 20, 2016 5:01:19 PM GMT+01:00, Kyrill Tkachov
>>  wrote:
>>>
>>> Hi all,
>>>
>>> The testcase in this patch generates bogus assembly for arm with -O1
>>> -mfloat-abi=soft:
>>> strdr4, [#0, r3]
>>>
>>> This is due to non-canonical RTL being generated during expansion:
>>> (set (mem:DI (plus:SI (const_int 0 [0])
>>>(reg/f:SI 153)) [0 MEM[symbol: a, index: _26, offset: 0B]+0 S8 A64])
>>>  (reg:DI 154))
>>>
>>> Note the (plus (const_int 0) (reg)). This is being generated in
>>> gen_addr_rtx in tree-ssa-address.c
>>> where it creates an explicit PLUS rtx through gen_rtx_PLUS, which
>>> doesn't try to canonicalise its arguments
>>> or simplify. The correct thing to do is to use simplify_gen_binary that
>>> will handle all this properly.
>>
>> But it has to match up the validity check which passes down exactly the
>> same RTL(?)  Or does this stem from propagation simplifying a MEM after
>> IVOPTs?
>
>
> You mean TARGET_LEGITIMATE_ADDRESS_P? Yes, it gets passed on to that, but
> the arm implementation of that
> doesn't try to handle non-canonical RTL (plus (const0_rtx) (reg) is not
> canonical).
> Or do you mean some other check?

Ok, now looking at the actual patch and the code in question.  For your testcase
it happens that symbol == const0_rtx?  In this case please amend the
if (symbol) check like we do for the base, thus

   if (symbol && symbol != const0_rtx)

Richard.

> Thanks,
> Kyrill
>
>
>>> I didn't change the other gen_rtx_PLUS calls in this function as their
>>> results is later used in XEXP operations
>>> that seem to rely on a PLUS expression being explicitly produced, but
>>> this particular call doesn't, so it's okay
>>> to change it. With this patch the sane assembly is generated:
>>>  strdr4, [r3]
>>>
>>> Bootstrapped and tested on arm-none-linux-gnueabihf, x86_64,
>>> aarch64-none-linux-gnu.
>>>
>>> Ok for trunk?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2016-12-20  Kyrylo Tkachov  
>>>
>>> * tree-ssa-address.c (gen_addr_rtx): Use simplify_gen_binary to add
>>>  *addr to act_elem.
>>>
>>> 2016-12-20  Kyrylo Tkachov  
>>>
>>>  * gcc.dg/20161219.c: New test.
>>
>>
>


RE: Make MIPS soft-fp preserve NaN payloads for NAN2008

2017-01-04 Thread Joseph Myers
On Wed, 4 Jan 2017, Maciej W. Rozycki wrote:

>  AFAIR we deliberately decided not to define a 2008-NaN soft-float ABI, 
> and chose to require all soft-float binaries to use the legacy encoding.

Soft-float and 2008-NaN are naturally completely orthogonal and the 
combination works fine (of course, it doesn't need any special kernel or 
hardware support).  There's no need to disallow the combination.

In any case, the soft-fp change is relevant in the hard-float case as 
well, to make software TFmode behave consistently with hardware SFmode and 
DFmode regarding NaN payload preservation.

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


Re: [PATCH] c++/78765

2017-01-04 Thread Nathan Sidwell

On 01/04/2017 12:40 AM, Jason Merrill wrote:

On 12/16/2016 07:23 AM, Nathan Sidwell wrote:

when cxx_eval_constant_expression finds a nonconstant expression it
returns a TREE without TREE_CONSTANT set.
  else if (non_constant_p && TREE_CONSTANT (r))
  {
  /* This isn't actually constant, so unset TREE_CONSTANT.  */



Hmm, we shouldn't get here for an expression we're going to use as an
lvalue.


I don't think we know we're going to use it as an lvalue early enough. 
We get here from convert_nontype_argument:

 else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
expr = maybe_constant_value (expr);
and fail to have 'var' as a constexpr (because it had a bogus 
initializer, for which we emitted a diagnostic).


We then get to:
  if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
{
  tree t = build_integral_nontype_arg_conv (type, expr, complain);
and that finds the conversion operator, which needs var as the object 
expression.


I suppose at that point, if expr is not constant we've already lost. 
Perhaps build_integral_nontype_arg_conv should bail out earlier?


Or is the problem in the error recovery of:
static constexpr ValueType var = 0;

should we unmark the constexprness of var?

WDYT?

nathan
--
Nathan Sidwell


Re: [PATCH] libstdc++: Allow using without lock free atomic int

2017-01-04 Thread Christophe Lyon
Hi Jonathan,

On 4 January 2017 at 12:02, Jonathan Wakely  wrote:
> On 03/01/17 15:32 +, Jonathan Wakely wrote:
>>
>> Here's what I plan to commit to trunk tomorrow.
>
>
> Committed to trunk.
>
>
After this commit (r244051), I do see improvements, but also a few new failures.
The big picture is at
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/report-build-info.html

Where the expected improvements for arm-none-eabi, with default cpu&fpu are:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/arm-none-eabi/diff-libstdc++-rh60-arm-none-eabi-default-default-default.txt

New failures appear when forcing -march=armv5t in runtestflags (the 3
red items in the 1st report):

  - FAIL appears  [ => FAIL]:

  18_support/exception_ptr/60612-terminate.cc execution test
  18_support/exception_ptr/60612-unexpected.cc execution test
  30_threads/packaged_task/members/at_thread_exit.cc execution test
  30_threads/promise/members/at_thread_exit.cc execution test

Note that in these cases we are compiling the test cases with
-march=armv5t, but link
with libraries built --with-cpu=cortex-a9, so there might be a mismatch?

Christophe


Re: [PATCH][tree-ssa-address] Use simplify_gen_binary in gen_addr_rtx

2017-01-04 Thread Kyrill Tkachov


On 04/01/17 14:19, Richard Biener wrote:

On Wed, Dec 21, 2016 at 10:40 AM, Kyrill Tkachov
 wrote:

On 20/12/16 17:30, Richard Biener wrote:

On December 20, 2016 5:01:19 PM GMT+01:00, Kyrill Tkachov
 wrote:

Hi all,

The testcase in this patch generates bogus assembly for arm with -O1
-mfloat-abi=soft:
 strdr4, [#0, r3]

This is due to non-canonical RTL being generated during expansion:
(set (mem:DI (plus:SI (const_int 0 [0])
(reg/f:SI 153)) [0 MEM[symbol: a, index: _26, offset: 0B]+0 S8 A64])
  (reg:DI 154))

Note the (plus (const_int 0) (reg)). This is being generated in
gen_addr_rtx in tree-ssa-address.c
where it creates an explicit PLUS rtx through gen_rtx_PLUS, which
doesn't try to canonicalise its arguments
or simplify. The correct thing to do is to use simplify_gen_binary that
will handle all this properly.

But it has to match up the validity check which passes down exactly the
same RTL(?)  Or does this stem from propagation simplifying a MEM after
IVOPTs?


You mean TARGET_LEGITIMATE_ADDRESS_P? Yes, it gets passed on to that, but
the arm implementation of that
doesn't try to handle non-canonical RTL (plus (const0_rtx) (reg) is not
canonical).
Or do you mean some other check?

Ok, now looking at the actual patch and the code in question.  For your testcase
it happens that symbol == const0_rtx?  In this case please amend the
if (symbol) check like we do for the base, thus

if (symbol && symbol != const0_rtx)


No, symbol is not const0_rtx (it's just a symbol_ref).
index is const0_rtx and so gets assigned to *addr at the beginning of the 
function.
base and step are NULL_RTX.
So at the time of the check:
   if (*addr)
 *addr = gen_rtx_PLUS (address_mode, *addr, act_elem);
   else
 *addr = act_elem;

*addr is const0_rtx. Do you want to amend that check to:
if (*addr && *addr != const0_rtx) ?

I haven't looked where the const0_rtx index comes from, so I don't know if it
could have other CONST_INT values that may cause trouble.

Kyrill


Richard.


Thanks,
Kyrill



I didn't change the other gen_rtx_PLUS calls in this function as their
results is later used in XEXP operations
that seem to rely on a PLUS expression being explicitly produced, but
this particular call doesn't, so it's okay
to change it. With this patch the sane assembly is generated:
  strdr4, [r3]

Bootstrapped and tested on arm-none-linux-gnueabihf, x86_64,
aarch64-none-linux-gnu.

Ok for trunk?

Thanks,
Kyrill

2016-12-20  Kyrylo Tkachov  

 * tree-ssa-address.c (gen_addr_rtx): Use simplify_gen_binary to add
  *addr to act_elem.

2016-12-20  Kyrylo Tkachov  

  * gcc.dg/20161219.c: New test.






Re: [PATCH] libstdc++: Allow using without lock free atomic int

2017-01-04 Thread Jonathan Wakely

On 04/01/17 16:00 +0100, Christophe Lyon wrote:

Hi Jonathan,

On 4 January 2017 at 12:02, Jonathan Wakely  wrote:

On 03/01/17 15:32 +, Jonathan Wakely wrote:


Here's what I plan to commit to trunk tomorrow.



Committed to trunk.



After this commit (r244051), I do see improvements, but also a few new failures.
The big picture is at
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/report-build-info.html

Where the expected improvements for arm-none-eabi, with default cpu&fpu are:
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/arm-none-eabi/diff-libstdc++-rh60-arm-none-eabi-default-default-default.txt

New failures appear when forcing -march=armv5t in runtestflags (the 3
red items in the 1st report):

 - FAIL appears  [ => FAIL]:

 18_support/exception_ptr/60612-terminate.cc execution test
 18_support/exception_ptr/60612-unexpected.cc execution test
 30_threads/packaged_task/members/at_thread_exit.cc execution test
 30_threads/promise/members/at_thread_exit.cc execution test

Note that in these cases we are compiling the test cases with
-march=armv5t, but link
with libraries built --with-cpu=cortex-a9, so there might be a mismatch?


Yes, this is probably the same issue as PR63829

Richard's comment on the bug report says that arm should always use
atomics, via kernel helpers if necessary, so that there is no ABI
change when using -march for older CPUs.

I don't know how to do that, but I hope we can make some progress on
it for gcc 8.



Re: [PATCH] c++/78765

2017-01-04 Thread Jason Merrill
On Wed, Jan 4, 2017 at 9:33 AM, Nathan Sidwell  wrote:
> On 01/04/2017 12:40 AM, Jason Merrill wrote:
>> On 12/16/2016 07:23 AM, Nathan Sidwell wrote:
>>>
>>> when cxx_eval_constant_expression finds a nonconstant expression it
>>> returns a TREE without TREE_CONSTANT set.
>>>   else if (non_constant_p && TREE_CONSTANT (r))
>>>   {
>>>   /* This isn't actually constant, so unset TREE_CONSTANT.  */
>
>> Hmm, we shouldn't get here for an expression we're going to use as an
>> lvalue.
>
> I don't think we know we're going to use it as an lvalue early enough. We
> get here from convert_nontype_argument:
>  else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
> expr = maybe_constant_value (expr);

OK, that looks like the problem; we shouldn't be calling
maybe_constant_value before we perform the conversion.

Jason


Re: [PATCH] PR c++/66735 lambda capture by reference

2017-01-04 Thread Jason Merrill
OK.

On Wed, Jan 4, 2017 at 8:52 AM, Nathan Sidwell  wrote:
> On 01/04/2017 12:36 AM, Jason Merrill wrote:
>>
>> On 01/03/2017 08:57 AM, Nathan Sidwell wrote:
>
>
>>> +  type = auto_node;
>>> +  if (by_reference_p)
>
>
>>> +  type = build_reference_type (type);
>>> +  by_reference_p = false;
>
> ^^^ here
>>>
>>> +  if (!is_this && by_reference_p)
>>> +type = build_reference_type (type);
>>
>>
>> This looks like it will call build_reference_type twice in the explicit
>> init case, producing a reference to reference.
>
>
> Note the clearing of by_reference_p just above in this case, but I admit
> that's confusing.  Anyway, it's rendered moot by this patch which addresses:
>
>>>  if (DECLTYPE_FOR_LAMBDA_CAPTURE (t))
>>>type = lambda_capture_field_type (type,
>>> -DECLTYPE_FOR_INIT_CAPTURE (t));
>>> +DECLTYPE_FOR_INIT_CAPTURE (t),
>>> +/*by_reference_p=*/false);
>>
>>
>> Always passing false seems unlikely to be correct.
>
>
> I wondered about that, but obviously failed to come up with a testcase to
> expose it.  This patch adds such a case, and exposes the need to add another
> flag to DECLTYPE to indicate reference capture -- rather than wrap the
> dependent capture in a reference type itself, as was previously the case.
>
>
> ok?
>
> nathan
>
> --
> Nathan Sidwell


Re: C++ PATCH for c++/77545 and c++/77284 (ICE with CLEANUP_STMT)

2017-01-04 Thread Jason Merrill
OK.

On Tue, Jan 3, 2017 at 8:37 AM, Marek Polacek  wrote:
> The problem here is that we've gotten to potential_constant_expression_1 with 
> a
> CLEANUP_STMT, but it doesn't know how to handle that so we ICE.  I thought 
> it'd
> be possible to look into CLEANUP_{BODY,EXPR} to determine whether the
> CLEANUP_STMT can be potentially const, but cxx_eval_constant_expression can't
> handle CLEANUP_STMTs so it couldn't evaluate it anyway.  So it seems that it's
> safe to consider CLEANUP_STMTs non-constant.
>
> This happens when initializing __for_range, where finish_eh_cleanup creates
> a CLEANUP_STMT that would run ~A() in case of an exception.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-01-03  Marek Polacek  
>
> PR c++/77545
> PR c++/77284
> * constexpr.c (potential_constant_expression_1): Handle CLEANUP_STMT.
>
> * g++.dg/cpp0x/range-for32.C: New test.
> * g++.dg/cpp0x/range-for33.C: New test.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 1e83b0b..a3dec68 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5661,6 +5661,7 @@ potential_constant_expression_1 (tree t, bool 
> want_rval, bool strict,
>/* We can see these in statement-expressions.  */
>return true;
>
> +case CLEANUP_STMT:
>  case EMPTY_CLASS_EXPR:
>return false;
>
> diff --git gcc/testsuite/g++.dg/cpp0x/range-for32.C 
> gcc/testsuite/g++.dg/cpp0x/range-for32.C
> index e69de29..375a707 100644
> --- gcc/testsuite/g++.dg/cpp0x/range-for32.C
> +++ gcc/testsuite/g++.dg/cpp0x/range-for32.C
> @@ -0,0 +1,16 @@
> +// PR c++/77545
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wno-pedantic" }
> +
> +template < typename T > struct A
> +{
> +  A ();
> +  ~A ();
> +  T t;
> +};
> +
> +void f (A < int > a)
> +{
> +  for (auto x : (A[]) { a })
> +;
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/range-for33.C 
> gcc/testsuite/g++.dg/cpp0x/range-for33.C
> index e69de29..206f36e 100644
> --- gcc/testsuite/g++.dg/cpp0x/range-for33.C
> +++ gcc/testsuite/g++.dg/cpp0x/range-for33.C
> @@ -0,0 +1,14 @@
> +// PR c++/77284
> +// { dg-do compile { target c++11 } }
> +
> +#include 
> +
> +struct A
> +{
> +  ~A () {}
> +};
> +
> +void foo (A & v)
> +{
> +  for (A a : { v }) {};
> +}
>
> Marek


Re: [PATCH] libstdc++: Allow using without lock free atomic int

2017-01-04 Thread Christophe Lyon
On 4 January 2017 at 16:10, Jonathan Wakely  wrote:
> On 04/01/17 16:00 +0100, Christophe Lyon wrote:
>>
>> Hi Jonathan,
>>
>> On 4 January 2017 at 12:02, Jonathan Wakely  wrote:
>>>
>>> On 03/01/17 15:32 +, Jonathan Wakely wrote:


 Here's what I plan to commit to trunk tomorrow.
>>>
>>>
>>>
>>> Committed to trunk.
>>>
>>>
>> After this commit (r244051), I do see improvements, but also a few new
>> failures.
>> The big picture is at
>>
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/report-build-info.html
>>
>> Where the expected improvements for arm-none-eabi, with default cpu&fpu
>> are:
>>
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244051/arm-none-eabi/diff-libstdc++-rh60-arm-none-eabi-default-default-default.txt
>>
>> New failures appear when forcing -march=armv5t in runtestflags (the 3
>> red items in the 1st report):
>>
>>  - FAIL appears  [ => FAIL]:
>>
>>  18_support/exception_ptr/60612-terminate.cc execution test
>>  18_support/exception_ptr/60612-unexpected.cc execution test
>>  30_threads/packaged_task/members/at_thread_exit.cc execution test
>>  30_threads/promise/members/at_thread_exit.cc execution test
>>
>> Note that in these cases we are compiling the test cases with
>> -march=armv5t, but link
>> with libraries built --with-cpu=cortex-a9, so there might be a mismatch?
>
>
> Yes, this is probably the same issue as PR63829

Indeed.

>
> Richard's comment on the bug report says that arm should always use
> atomics, via kernel helpers if necessary, so that there is no ABI
> change when using -march for older CPUs.
>
> I don't know how to do that, but I hope we can make some progress on
> it for gcc 8.
>
That would be good, maybe Richard can post an example in
bugzilla?

Thanks,

Christophe


[patch,avr] PR78883: Implement a dummy scheduler

2017-01-04 Thread Georg-Johann Lay

On 03.01.2017 14:43, Richard Sandiford wrote:

Georg-Johann Lay  writes:

On 02.01.2017 15:54, Dominik Vogt wrote:

On Mon, Jan 02, 2017 at 03:47:43PM +0100, Georg-Johann Lay wrote:

This fixes PR78883 which is a problem in reload revealed by a
change to combine.c.  The fix is as proposed by Segher: implement
CANNOT_CHANGE_MODE_CLASS.

Ok for trunk?

Johann


gcc/
PR target/78883
* config/avr/avr.h (CANNOT_CHANGE_MODE_CLASS): New define.
* config/avr/avr-protos.h (avr_cannot_change_mode_class): New proto.
* config/avr/avr.c (avr_cannot_change_mode_class): New function.

gcc/testsuite/
PR target/78883
* gcc.c-torture/compile/pr78883.c: New test.



Index: config/avr/avr-protos.h
===
--- config/avr/avr-protos.h (revision 244001)
+++ config/avr/avr-protos.h (working copy)
@@ -111,7 +111,7 @@ extern int _reg_unused_after (rtx_insn *
 extern int avr_jump_mode (rtx x, rtx_insn *insn);
 extern int test_hard_reg_class (enum reg_class rclass, rtx x);
 extern int jump_over_one_insn_p (rtx_insn *insn, rtx dest);
-
+extern int avr_cannot_change_mode_class (machine_mode, machine_mode, enum 
reg_class);
 extern int avr_hard_regno_mode_ok (int regno, machine_mode mode);
 extern void avr_final_prescan_insn (rtx_insn *insn, rtx *operand,
int num_operands);
Index: config/avr/avr.c
===
--- config/avr/avr.c(revision 244001)
+++ config/avr/avr.c(working copy)
@@ -11833,6 +11833,21 @@ jump_over_one_insn_p (rtx_insn *insn, rt
 }


+/* Worker function for `CANNOT_CHANGE_MODE_CLASS'.  */
+
+int
+avr_cannot_change_mode_class (machine_mode from, machine_mode to,
+  enum reg_class /* rclass */)
+{
+  /* We cannot access a hard register in a wider mode, for example we
+ must not access (reg:QI 31) as (reg:HI 31).  HARD_REGNO_MODE_OK
+ would avoid such hard regs, but reload would generate it anyway
+ from paradoxical subregs of mem, cf. PR78883.  */
+
+  return GET_MODE_SIZE (to) > GET_MODE_SIZE (from);


I understand how this fixes the ICE, but is it really necessary to
suppress conversions to a wider mode for lower numbered registers?


If there is a better hook, I'll propose an according patch.

My expectation was that HARD_REGNO_MODE_OK would be enough to keep
reload from putting HI into odd registers (and in particular into R31).
But this is obviously not the case...


It should be enough in principle.  It's just a case of whether you want
to fix reload, hack around it, or take the plunge and switch to LRA.


Well, if it can be done in the back-end, then that's generally my strong
preference.  And the blocker for LRA is that it doesn't support cc0,
hence LRA will require an almost complete rewrite of the avr back-end...


Having a (subreg (mem)) is probably key here.  If it had been
(subreg (reg:HI X)) for some pseudo X then init_subregs_of_mode should
have realised that 31 isn't a valid choice for X.

I think the reload fix would be to honour simplifiable_subregs when
reloading the (subreg (mem)).


And internals are not very helpful here.  It only mentions modifying
ordinary subregs of pseudos, but not paradoxical subreg of memory.

What's also astonishing me is that this problem never popped up
during the last > 15 years of avr back-end.


FWIW, the current init_subregs_of_mode/simplifiable_subregs behaviour
is fairly recent (2014) and CANNOT_CHANGE_MODE_CLASS had been used in
the past to avoid errors like this.  Using it that way was always a
hack though.

An alternative would be to add a new macro to control this block in
general_operand:

#ifdef INSN_SCHEDULING
  /* On machines that have insn scheduling, we want all memory
 reference to be explicit, so outlaw paradoxical SUBREGs.
 However, we must allow them after reload so that they can
 get cleaned up by cleanup_subreg_operands.  */
  if (!reload_completed && MEM_P (sub)
  && GET_MODE_SIZE (mode) > GET_MODE_SIZE (GET_MODE (sub)))
return 0;
#endif

The default would still be INSN_SCHEDULING, but AVR could override it
to 1 and reject the same subregs.

That would still be a hack, but at least it would be taking things in
a good direction.  Having different rules depending on whether targets
define a scheduler is just a horrible wart that no-one's ever had chance
to fix.  If using the code above works well on AVR then that'd be a big
step towards making the code unconditional.

It'd definitely be worth checking how it affects code quality though.
(Although the same goes for the current patch, since C_C_M_C is a pretty
big hammer.)

Thanks,
Richard


For now, here is the 2nd approach implemented: Add a dummy scheduler.
At least the test case passes with this change.

Johann


gcc/
PR78883
* config/avr/avr-dfa.md: New file.
* config/avr/avr.md

[PATCH] PR78968 add configure check for __cxa_thread_atexit in libc

2017-01-04 Thread Jonathan Wakely

FreeBSD 11 adds __cxa_thread_atexit to libc, so we should use that
instead of defining our own inferior version. This also avoids
multiple definitions of the symbol.

PR libstdc++/78968
* config.h.in: Regenerate.
* configure: Likewise.
* configure.ac: Check for __cxa_thread_atexit.
* libsupc++/atexit_thread.cc [_GLIBCXX_HAVE___CXA_THREAD_ATEXIT]:
Don't define __cxa_thread_atexit if libc provides it.

Tested powerpc64le-linux, committed to trunk.

commit 82ba7b061acc7afac92646a239991e8a84e0c6dc
Author: Jonathan Wakely 
Date:   Tue Jan 3 13:31:54 2017 +

PR78968 add configure check for __cxa_thread_atexit in libc

PR libstdc++/78968
* config.h.in: Regenerate.
* configure: Likewise.
* configure.ac: Check for __cxa_thread_atexit.
* libsupc++/atexit_thread.cc [_GLIBCXX_HAVE___CXA_THREAD_ATEXIT]:
Don't define __cxa_thread_atexit if libc provides it.

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 3d61771..f2f6e0a 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -255,7 +255,7 @@ if $GLIBCXX_IS_NATIVE; then
   # For TLS support.
   GCC_CHECK_TLS
 
-  AC_CHECK_FUNCS(__cxa_thread_atexit_impl)
+  AC_CHECK_FUNCS(__cxa_thread_atexit_impl __cxa_thread_atexit)
   AC_CHECK_FUNCS(aligned_alloc posix_memalign memalign _aligned_malloc)
 
   # For iconv support.
diff --git a/libstdc++-v3/libsupc++/atexit_thread.cc 
b/libstdc++-v3/libsupc++/atexit_thread.cc
index da63368..923a070 100644
--- a/libstdc++-v3/libsupc++/atexit_thread.cc
+++ b/libstdc++-v3/libsupc++/atexit_thread.cc
@@ -30,7 +30,11 @@
 #include 
 #endif
 
-#if _GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL
+#if _GLIBCXX_HAVE___CXA_THREAD_ATEXIT
+
+// Libc provides __cxa_thread_atexit definition.
+
+#elif _GLIBCXX_HAVE___CXA_THREAD_ATEXIT_IMPL
 
 extern "C" int __cxa_thread_atexit_impl (void (*func) (void *),
 void *arg, void *d);


Re: [PATCH, GCC/testsuite/ARM, ping] Fix empty_fiq_handler target selector

2017-01-04 Thread Thomas Preudhomme

On 04/01/17 09:42, Kyrill Tkachov wrote:


On 03/01/17 17:22, Thomas Preudhomme wrote:

Happy new year!

Ping?

Best regards,

Thomas

On 09/12/16 15:28, Thomas Preudhomme wrote:

Hi,

The current target selector for empty_fiq_handler.c testcase skips the test when
targeting Thumb mode on a device with ARM execution state. Because it checks
Thumb mode by looking for an -mthumb option it fails to work when GCC was
configured with --with-mode=thumb. It is also too restrictive because interrupt
handler can be compiled in Thumb-2. This patch checks the arm_thumb1 effective
target instead of the -mthumb flag to fix both issues.

ChangeLog entry is as follows:


*** gcc/testsuite/ChangeLog ***

2016-12-09  Thomas Preud'homme 

* gcc.target/arm/empty_fiq_handler: Skip instead if targeting Thumb-1
on a non Thumb-only target.


Tested with GCC built for ARMv5T and ARMv7-A with --with-mode=thumb and
--with-mode=arm and for ARMv6S-M with --with-mode=thumb:

* test pass in all cases for ARMv5T and ARMv7-A with -marm
* test pass in all cases for ARMv6S-M and ARMv7-A with -mthumb
* test pass without option when defaulting to ARM for ARMv5T and ARMv7-A
* test pass without option when defaulting to Thumb for ARMv6S-M and ARMv7-A
* test is unsupported with -marm for ARMv5T
* test is unsupported without option when defaulting to Thumb for ARMv5T

Is this ok for stage3?

Best regards,

Thomas


fix_empty_fiq_handler_testcase_selector.patch


diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
index
8313f2199122be153a737946e817a5e3bee60372..69bb0669dd416e1fcb015c278d62961d071fc42f
100644
--- a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
+++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
@@ -1,5 +1,4 @@
-/* { dg-do compile } */
-/* { dg-skip-if "" { ! arm_cortex_m } { "-mthumb" } } */
+/* { dg-do compile { target { {! arm_thumb1 } || arm_cortex_m } } } */



I think you want to add a space between the '{' and '!'.
Otherwise this is ok.


Right.

Did testing with ARMv4T defaulting to Thumb and ARMv7-A defaulting to Thumb and 
ARM to make sure it didn't seem to affect the behavior. Committed as r244057 
with suggested change (attached for reference).


Best regards,

Thomas
diff --git a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
index 8313f2199122be153a737946e817a5e3bee60372..7b8296bfc903780d35059e2e63ed7ef35daf51d1 100644
--- a/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
+++ b/gcc/testsuite/gcc.target/arm/empty_fiq_handler.c
@@ -1,5 +1,4 @@
-/* { dg-do compile } */
-/* { dg-skip-if "" { ! arm_cortex_m } { "-mthumb" } } */
+/* { dg-do compile { target { { ! arm_thumb1 } || arm_cortex_m } } } */
 
 /* Below code used to trigger an ICE due to missing constraints for
sp = fp + cst pattern.  */


Re: [bootstrap-O3] add a default initializer to avoid a warning at -O3

2017-01-04 Thread Jeff Law

On 01/04/2017 06:42 AM, Jason Merrill wrote:

On Wed, Jan 4, 2017 at 8:13 AM, Alexandre Oliva  wrote:

On Jan  3, 2017, Jason Merrill  wrote:


Are there bugzillas for these false positive warnings?


I don't think so.

Did you mean in the sense that the patch silences them and thus "fixes"
them, or in the sense that the underlying cause for the warnings is not
fixed and someone might want to look into them at a later time?

FWIW, I hadn't considered the latter (it doesn't seem too hard to find
such warnings anyway; every time I try bootstrap-O3, which is not very
often, I get a few of those), and the former didn't seem enough of a
reason to have bug reports created.


The latter; false positive warnings like this are bugs that should be fixed.
For the -O1 stuff the lower optimization level disables the optimizers 
that would identify and optimize away the infeasible path.


For -O3 cases with unrolling, the unroller can create oob array 
references with a guard.  Our ability to exploit the guard isn't as 
strong as it should be -- we've got BZs for this already.


For the -O3 these are cases where the inliner exposes the infeasible 
path, but we're not currently able to analyze the infeasible path.


One could argue that any initializer we add ought to avoid a false 
positive for -O2 or -O3 ought to be marked and a bug opened.  But I 
didn't feel it was worth pushing for that level of rigor since at most 
these are missed optimizations.  But it would certainly be nice.


jeff




Re: [PATCH] c++/78765

2017-01-04 Thread Nathan Sidwell

On 01/04/2017 10:13 AM, Jason Merrill wrote:


OK, that looks like the problem; we shouldn't be calling
maybe_constant_value before we perform the conversion.


Yup, that worked.

ok?

nathan


--
Nathan Sidwell
2017-01-04  Nathan Sidwell  

	cp/
	PR c++/78765
	* pt.c (convert_nontype_argument): Don't try and see if integral
	or enum expressions are constants prematurely.

	testsuite/
	PR c++/78765
	* g++.dg/cpp0x/pr78765.C: New.

Index: cp/pt.c
===
--- cp/pt.c	(revision 244056)
+++ cp/pt.c	(working copy)
@@ -6386,7 +6386,7 @@ convert_nontype_argument (tree type, tre
 	   to leave it in that form rather than lower it to a
 	   CONSTRUCTOR.  */;
   else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
-	expr = maybe_constant_value (expr);
+	/* Constant value checking is done later with type conversion.  */;
   else if (cxx_dialect >= cxx1z)
 	{
 	  if (TREE_CODE (type) != REFERENCE_TYPE)
Index: testsuite/g++.dg/cpp0x/pr78765.C
===
--- testsuite/g++.dg/cpp0x/pr78765.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr78765.C	(working copy)
@@ -0,0 +1,15 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+// ICE with failed constexpr object and member fn call
+
+struct ValueType {
+  constexpr operator int() const {return field;}
+  int field;
+};
+
+static constexpr ValueType var = 0; // { dg-error "conversion" }
+
+template  class ValueTypeInfo;
+
+ValueTypeInfo x;


Re: [patch,avr] PR78883: Implement a dummy scheduler

2017-01-04 Thread Segher Boessenkool
On Wed, Jan 04, 2017 at 04:39:36PM +0100, Georg-Johann Lay wrote:
> Well, if it can be done in the back-end, then that's generally my strong
> preference.  And the blocker for LRA is that it doesn't support cc0,
> hence LRA will require an almost complete rewrite of the avr back-end...

Heh, getting rid of cc0 isn't *that* dramatic a change.  It does require
knowing the target really well (or some serious time with arch manuals).

One day all cc0 support will be removed, and it's advantageous to use the
newer code instead anyway...

> +// Currently, the only purpose of the insn scheduler is to work
> +// around PR78883, i.e. we only need the side effect of defining
> +// INSN_SCHEDULING.  Even with a dummy scheduler we will see reodering
> +// of instructions, which is not wanted if not actually needed.
> +{ OPT_LEVELS_ALL, OPT_fschedule_insns, NULL, 0 },
> +{ OPT_LEVELS_ALL, OPT_fschedule_insns2, NULL, 0 },

> +;; Notice that we disable -fschedule-insns and -fschedule-insns2 in
> +;; avr-common.c::TARGET_OPTION_OPTIMIZATION_TABLE because some "random"
> +;; reordering of instructions is not wanted.
> +
> +(define_automaton "avr_dfa")
> +
> +(define_cpu_unit "avr_cpu" "avr_dfa")
> +
> +(define_insn_reservation "avr_cpu_reserve" 1
> +  (const_int 1)
> +  "avr_cpu")

I think you won't see this "random" reordering if you use (const_int 0)
for the condition instead, i.e. the reservation will never match (instead
of always as you have now).


Segher


Re: [bootstrap-O1] enlarge sprintf output buffer to avoid warning

2017-01-04 Thread Martin Sebor

On 01/03/2017 10:39 AM, Jeff Law wrote:

On 01/02/2017 10:28 PM, Alexandre Oliva wrote:

In stage2 of bootstrap-O1, the code that warns if sprintf might
overflow its output buffer cannot tell that an unsigned value narrowed
to 16 bits will fit in 4 bytes with %4x.

I couldn't find a better way to avoid the warning at -O1 than growing
the buffer so that there's no doubt the output will fit.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

for  gcc/c-family/ChangeLog

* c-pretty-print.c (pp_c_tree_decl_identifier): Grow static
buffer to avoid false-positive warning.

Presumably this is an artifact of not running VRP at -O1 and thus we
don't have a narrowed range from the masking operation.

This isn't performance critical code so we *could* avoid the statically
sized array.  But I doubt it's worth the effort.


The manual recommends to use a length modifier to constrain the length
of output to that of a narrower type:

  sprintf (xname, "", ((unsigned short)((uintptr_t)(t) & 0x)));

This should work even without optimization.

Martin


Re: [PATCH] Special case clzl like builtins in factor_out_conditional_conversion (PR tree-optimization/71016)

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 11:19:46AM +0100, Richard Biener wrote:
> For the SSA_NAME + INTEGER_CST case restrict it to the case
> 
>   if (x_1 > 5)
> tem_2 = (char) x_1;
>   # tem_3 = PHI 
> 
> that is, (char) x_1 uses x_1 and that also appears in the controlling
> GIMPLE_COND.  That's what enables followup minmax replacement
> (gcc.dg/tree-ssa/pr66726.c).  Another case where it might be
> profitable is if the BB is empty after the conversion is sunk
> (may enable other value-replacement cases).

Do we actually have any testcases where we need the SSA_NAME + INTEGER_CST
case?
The only testcase that has been added that actually relied on the
factor_out_* is since r242750 handled during gimple folding (already in
*.gimple dump).

If I modify the testcase so that it isn't recognized by the minmax folding,
extern unsigned short y[];

int
foo (int x)
{
  return 64 < y[x] ? 68 : y[x];
}

then vanilla gcc vs. gcc with factor_out* INTEGER_CST case removed gives:
-   cmpw$65, %ax
-   cmovnb  %edx, %eax
+   cmpw$64, %ax
+   cmova   %edx, %eax
so not worse or better.  And, even when the conversion is the only stmt
in the basic block, that still doesn't allow the bb to be removed, we need
the empty bb for the PHIs.

Jakub


C/C++ PATCH to implement -Wpointer-compare warning (PR c++/64767) (version 2)

2017-01-04 Thread Marek Polacek
This is another version of the patch I posted a while ago.  To recap:

Spurred by the recent  findings, I decided to
implement a warning that warns when a pointer is compared with a zero character
literal (constant), because this isn't likely to be intended.  So e.g.

  ptr == L'\0'

is probably wrong and should've been written as

  ptr[0] == L'\0'

This patch adds the warning for the C FE as well as the C++ FE, but since this
code is actually invalid in C++11 (and we reject it), it's only active in C++03.

One issue: in the C FE wchar_type_node == integer_type_node, so we won't warn
for "p == (wchar_t) 0" in the C FE, but we will in the C++ FE.  I'm hoping this
isn't that important an issue.

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

2017-01-04  Marek Polacek  

PR c++/64767
* c.opt (Wpointer-compare): New option.

* c-parser.c (c_parser_postfix_expression): Mark zero character
constants by setting original_type in c_expr.
* c-typeck.c (parser_build_binary_op): Warn when a pointer is compared
with a zero character constant.
(char_type_p): New function.

* typeck.c (cp_build_binary_op): Warn when a pointer is compared with
a zero character literal.

* doc/invoke.texi: Document -Wpointer-compare.

* c-c++-common/Wpointer-compare-1.c: New test.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 3c06aec..eb70647 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -870,6 +870,10 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Warning LangEnabledBy(C ObjC,Wall || Wpedantic)
 Warn when a pointer differs in signedness in an assignment.
 
+Wpointer-compare
+C ObjC C++ ObjC++ Var(warn_pointer_compare) Init(1) Warning
+Warn when a pointer is compared with a zero character constant.
+
 Wpointer-to-int-cast
 C ObjC Var(warn_pointer_to_int_cast) Init(1) Warning
 Warn when a pointer is cast to an integer of a different size.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 6d443da..fb1fd56 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -7530,6 +7530,9 @@ c_parser_postfix_expression (c_parser *parser)
 case CPP_CHAR32:
 case CPP_WCHAR:
   expr.value = c_parser_peek_token (parser)->value;
+  /* For the purpose of warning when a pointer is compared with
+a zero character constant.  */
+  expr.original_type = char_type_node;
   set_c_expr_source_range (&expr, tok_range);
   c_parser_consume_token (parser);
   break;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 54ae7d8..bd6b8eb 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3595,6 +3595,18 @@ parser_build_unary_op (location_t loc, enum tree_code 
code, struct c_expr arg)
   return result;
 }
 
+/* Returns true if TYPE is a character type, *not* including wchar_t.  */
+
+static bool
+char_type_p (tree type)
+{
+  return (type == char_type_node
+ || type == unsigned_char_type_node
+ || type == signed_char_type_node
+ || type == char16_type_node
+ || type == char32_type_node);
+}
+
 /* This is the entry point used by the parser to build binary operators
in the input.  CODE, a tree_code, specifies the binary operator, and
ARG1 and ARG2 are the operands.  In addition to constructing the
@@ -3714,6 +3726,21 @@ parser_build_binary_op (location_t location, enum 
tree_code code,
  && !integer_zerop (tree_strip_nop_conversions (arg1.value
warning_at (location, OPT_Waddress,
"comparison with string literal results in unspecified 
behavior");
+  /* Warn for ptr == '\0', it's likely that it should've been ptr[0].  */
+  if (POINTER_TYPE_P (type1)
+  && null_pointer_constant_p (arg2.value)
+  && char_type_p (type2)
+  && warning_at (location, OPT_Wpointer_compare,
+ "comparison between pointer and zero character "
+ "constant"))
+   inform (arg1.get_start (), "did you mean to dereference the pointer?");
+  else if (POINTER_TYPE_P (type2)
+  && null_pointer_constant_p (arg1.value)
+  && char_type_p (type1)
+  && warning_at (location, OPT_Wpointer_compare,
+ "comparison between pointer and zero character "
+ "constant"))
+   inform (arg2.get_start (), "did you mean to dereference the pointer?");
 }
   else if (TREE_CODE_CLASS (code) == tcc_comparison
   && (code1 == STRING_CST || code2 == STRING_CST))
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 57a69ef..4eff223 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4604,6 +4604,12 @@ cp_build_binary_op (location_t location,
  else
result_type = type0;
 
+ if (char_type_p (TREE_TYPE (orig_op1))
+ && warning (OPT_Wpointer_compare,
+ "comparison between pointer and zero character "
+  

Re: [PATCH] Fix late dwarf generated early from optimized out globals

2017-01-04 Thread Andreas Tobler

On 04.01.17 10:21, Richard Biener wrote:

On Wed, 28 Dec 2016, Andreas Tobler wrote:


On 28.12.16 19:24, Richard Biener wrote:

On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
 wrote:

On 16.09.16 13:30, Richard Biener wrote:

On Thu, 15 Sep 2016, Richard Biener wrote:



This addresses sth I needed to address with the early LTO debug

patches

(you might now figure I'm piecemail merging stuff from that patch).

When the cgraph code optimizes out a global we call the

late_global_decl

debug hook to eventually add a DW_AT_const_value to its DIE (we

don't

really expect a location as that will be invalid after optimizing

out

and will be pruned).

With the early LTO debug patches I have introduced a

early_dwarf_finished

flag (mainly for consistency checking) and I figured I can use that

to

detect the call to the late hook during the early phase and provide
the following cleaned up variant of avoiding to create locations

that

require later pruning (which doesn't work with emitting the early

DIEs).


Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as

well).


Will commit if testing finishes successfully.


Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus

early_dwarf_finished

was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.


Any chance to backport this commit (r240228) to 6.x?
It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
the bootstrap comparison failure I faced.


Did you analyze the bootstrap miscompare?  I suspect the patch merely papers
over the problem.


gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841, line
253


The objdump -dSx diff on the non stripped object looked always more or less
the same, a rodata offset which was different.

-   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
+   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410


Hmm, sounds like a constant pool entry was created by -g at a different
time (and thus offset) from regular compilation.  So yes, the patch
in question should have the affect to "fix" this.

Note that I later changed the fix with

2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.
* ipa.c (symbol_table::remove_unreachable_nodes): When not in
WPA signal symbol removal to the debuginfo machinery.
* dwarf2out.c (dwarf2out_late_global_decl): Instead of
using early_finised to guard the we're called for symbol
removal case look at the symtabs definition flag.
(gen_variable_die): Remove redundant check.

(the dwarf2out_late_global_decl and analyze_functions part are
relevant).

That should be the fix to backport (avoiding the early_dwarf_finished
part).


Ok, I bootstrapped with the attached snippet (had to tweak a bit to 
apply cleanly). w/o the previous mentioned fix (r240228). Is this what 
you had in mind or do you think some parts of the other fix (r240228) is 
also needed?


Thanks for the help, really appreciated.

Andreas


Index: cgraphunit.c
===
--- cgraphunit.c(revision 244050)
+++ cgraphunit.c(working copy)
@@ -1193,8 +1193,16 @@
 at looking at optimized away DECLs, since
 late_global_decl will subsequently be called from the
 contents of the now pruned symbol table.  */
- if (!decl_function_context (node->decl))
-   (*debug_hooks->late_global_decl) (node->decl);
+ if (VAR_P (node->decl)
+   && !decl_function_context (node->decl))
+{
+   /* We are reclaiming totally unreachable code and variables
+   so they effectively appear as readonly.  Show that to
+   the debug machinery.  */
+   TREE_READONLY (node->decl) = 1;
+   node->definition = false;
+   (*debug_hooks->late_global_decl) (node->decl);
+}
 
  node->remove ();
  continue;
Index: dwarf2out.c
===
--- dwarf2out.c (revision 244050)
+++ dwarf2out.c (working copy)
@@ -21088,7 +21088,6 @@
   tree ultimate_origin;
   dw_die_ref var_die;
   dw_die_r

Re: [patch,avr] PR78883: Implement a dummy scheduler

2017-01-04 Thread Richard Sandiford
Segher Boessenkool  writes:
> On Wed, Jan 04, 2017 at 04:39:36PM +0100, Georg-Johann Lay wrote:
>> Well, if it can be done in the back-end, then that's generally my strong
>> preference.  And the blocker for LRA is that it doesn't support cc0,
>> hence LRA will require an almost complete rewrite of the avr back-end...
>
> Heh, getting rid of cc0 isn't *that* dramatic a change.  It does require
> knowing the target really well (or some serious time with arch manuals).
>
> One day all cc0 support will be removed, and it's advantageous to use the
> newer code instead anyway...
>
>> +// Currently, the only purpose of the insn scheduler is to work
>> +// around PR78883, i.e. we only need the side effect of defining
>> +// INSN_SCHEDULING.  Even with a dummy scheduler we will see reodering
>> +// of instructions, which is not wanted if not actually needed.
>> +{ OPT_LEVELS_ALL, OPT_fschedule_insns, NULL, 0 },
>> +{ OPT_LEVELS_ALL, OPT_fschedule_insns2, NULL, 0 },
>
>> +;; Notice that we disable -fschedule-insns and -fschedule-insns2 in
>> +;; avr-common.c::TARGET_OPTION_OPTIMIZATION_TABLE because some "random"
>> +;; reordering of instructions is not wanted.
>> +
>> +(define_automaton "avr_dfa")
>> +
>> +(define_cpu_unit "avr_cpu" "avr_dfa")
>> +
>> +(define_insn_reservation "avr_cpu_reserve" 1
>> +  (const_int 1)
>> +  "avr_cpu")
>
> I think you won't see this "random" reordering if you use (const_int 0)
> for the condition instead, i.e. the reservation will never match (instead
> of always as you have now).

Both ways feel wrong to me TBH.  The sequence that led to this patch is:

1. reload has a bug that no-one really wants to fix (understandable)
2. the bug is triggered by paradoxical subregs of mems
3. those subregs are normally disabled on targets that support insn
   scheduling
4. therefore, define an insn scheduler
5. we don't actually want insn scheduling, so either
   (a) make sure the insn scheduler is never actually used for insn
   scheduling, or
   (b) allow the insn scheduler to run anyway but encourage it to do nothing
   (other than take compile time)

(4) and (5) feel like too much of a hack to me.  They're going to have
other consequences, e.g. we'll no longer give the warning:

  instruction scheduling not supported on this target machine

if users try to use -fschedule-insns.  And since we don't support
meaningful insn scheduling even after this patch, giving the warning
seems more user-friendly than dropping it.

I think the consensus is that we don't want these subregs for AVR
regardless of whether scheduling is used, and probably wouldn't want
them even without this bug.  So why not instead change the condition
used by general_operand, like we were talking about yesterday?
It seems simpler and more direct.

Thanks,
Richard


Re: [PR tree-optimization/67955] Exploit PTA in DSE

2017-01-04 Thread Jeff Law

On 12/09/2016 01:28 AM, Richard Biener wrote:

On Wed, Dec 7, 2016 at 12:18 AM, Jeff Law  wrote:



So I was going through the various DSE related bugs as stumbled across
67955.

The basic issue in 67955 is that DSE is too simplistic when trying to
determine if two writes hit the same memory address.  There are cases were
PTA analysis can get us that information.

The unfortunate problem is that PTA can generate a single points-to set for
pointers to different elements in the same array.  So we can only exploit a
special case.  Namely when we get a PTA singleton and the size of the store
is the same size of the pointed-to object.

That doesn't happen in a bootstrap, but it does happen for the testcase in
the BZ as well as a handful of tests in the testsuite -- Tom reported 6
unique tests where it triggered, I ran my own tests where two more were spit
out.  Not huge.  But the cost is low.

All that I've done with Tom's patch is update the call into the PTA system.
It's very clearly Tom's work.

Bootstrapped and regression tested on x86_64-linux-gnu.  Installing on the
trunk.


Just double-checked it doesn't break

int foo (int *p, int b)
{
  int *q;
  int i = 1;
  if (b)
q = &i;
  else
q = (void *)0;
  *q = 2;
  return i;
}
Right.  ISTM this shouldn't have a singleton points-to set and thus 
shouldn't change behavior.But looking at things more closely, it 
looks like points-to-null is distinct from the points-to variables.  ie, 
we want to know if its a singleton and ! null.  Thus we have to check

pt_solution_singleton_or_null_p (pi->pt, &pt_uid) && !pi->pt->null

I think it's working for you because the path isolation code splits off 
the NULL dereference path.  Adding 
-fno-isolate-erroneous-paths-dereference to the switches shows DSE2, 
incorrectly IMHO, removing the i = 1 store.


Thoughts?




with -fexceptions -fnon-call-exceptions.  More comments below.


jeff

commit cfc96cf6c8ea2c6e638123a93663964f6d78fb10
Author: law 
Date:   Tue Dec 6 23:18:17 2016 +

PR tree-optimization/67955
* tree-ssa-alias.c (same_addr_size_stores_p): New function.
(stmt_kills_ref_p): Use it.

PR tree-optimization/67955
* gcc.dg/tree-ssa/dse-points-to.c: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@243325
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 61eeea3..797b711 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2016-12-06  Tom de Vries  
+
+   PR tree-optimization/67955
+   * tree-ssa-alias.c (same_addr_size_stores_p): New function.
+   (stmt_kills_ref_p): Use it.
+
 2016-12-06  Eric Botcazou  

PR middle-end/78700
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 5adcdd2..6090a96 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-12-06  Tom de Vries  
+
+   PR tree-optimization/67955
+   * gcc.dg/tree-ssa/dse-points-to.c: New test.
+
 2016-12-06  Michael Meissner  

PR target/78658
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/dse-points-to.c
b/gcc/testsuite/gcc.dg/tree-ssa/dse-points-to.c
new file mode 100644
index 000..8003556
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/dse-points-to.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-ccp -fno-tree-forwprop -fno-tree-fre
-fno-tree-vrp" } */
+/* { dg-additional-options "-fdump-tree-dse1-details" } */
+
+int
+f ()
+{
+  int a;
+  int *p = &a;
+  *p = 1;
+  a = 2;
+  return a;
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted dead store.*p_1" 1 "dse1"} }
*/
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 10f1677..37b581d 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2316,6 +2316,78 @@ stmt_may_clobber_ref_p (gimple *stmt, tree ref)
   return stmt_may_clobber_ref_p_1 (stmt, &r);
 }

+/* Return true if store1 and store2 described by corresponding tuples
+have the same size and store to the same
+   address.  */
+
+static bool
+same_addr_size_stores_p (tree base1, HOST_WIDE_INT offset1, HOST_WIDE_INT
size1,
+HOST_WIDE_INT max_size1,
+tree base2, HOST_WIDE_INT offset2, HOST_WIDE_INT
size2,
+HOST_WIDE_INT max_size2)
+{
+  /* For now, just handle VAR_DECL.  */


PARAM and RESULT_DECL (aka SSA_VAR_P) should be safe.

Agreed & fixed.




+  bool base1_obj_p = VAR_P (base1);
+  bool base2_obj_p = VAR_P (base2);
+
+  /* We need one object.  */
+  if (base1_obj_p == base2_obj_p)
+return false;
+  tree obj = base1_obj_p ? base1 : base2;
+
+  /* And we need one MEM_REF.  */
+  bool base1_memref_p = TREE_CODE (base1) == MEM_REF;
+  bool base2_memref_p = TREE_CODE (base2) == MEM_REF;
+  if (base1_memref_p == base2_memref_p)
+return false;
+  tree memref = base1_memref_p ? base1 : base2;
+
+  /* Sizes need to be valid.  */
+  if (max_size1 == -1 || max_size2 == -1
+  || size1 == -1 || size2 == -1)
+return fals

Re: [PATCH], PR 71977/70568/78823: Improve PowerPC code that uses SFmode in unions

2017-01-04 Thread David Edelsohn
On Thu, Dec 29, 2016 at 7:24 PM, Michael Meissner
 wrote:

Mike,

Thanks for the tremendous effort on this patch.  A few comments.

The ChangeLog contains a lot of extraneous commentary that should be
in the documentation comments.  And a few typos in comments.

> * config/rs6000/predicates.md (sf_subreg_operand): New predicate
> to return true if the operand contains a SUBREG mixing SImode and
> SFmode on 64-bit VSX systems with direct move.  This can be a
> problem in that we have to know whether the SFmode value should be
> represented in the 32-bit memory format or the 64-bit scalar
> format used within the floating point and vector registers.

The ChangeLog entry should just say "New predicate."  The description
is in the documentation of the function.

> * config/rs6000/vsx.md (SFBOOL_*): Add peephole2 to recognize when
> we are converting a SFmode to a SImode, moving the result to a GPR
> register, doing a single AND/IOR/XOR operation, and then moving it
> back to a vector register.  Change the insns recognized to move
> the integer value to the vector register and do the operation
> there.  This code occurs quite a bit in the GLIBC math library in
> float math functions.

SFBOOL constants are not a new peephole2.  Please move the explanation
into a documentation comment in front of the peephole2.

> (peephole2 to speed up GLIB math functions): Likewise.
   ^^^ GLIBC?
New peephole2.

> (movsi_from_sf): New insn to handle where we are moving SFmode
> values to SImode registers and we need to convert the value to the
> memory format from the format used within the register.

New define_insn_and_split.

>(movdi_from_sf_zero_ext): Optimize zero extending movsi_from_sf.

New define_insn_and_split.

> (movsf_from_si): New insn to handle where we are moving SImode
> values to SFmode registers and we need to convert the value to the
> the format used within the floating point and vector registers to
> the 32-bit memory format.

New define_insn_and_split.

+;; Return 1 if op is a SUBREG that is used to look at a SFmode value as
+;; and integer or vice versa.
 ^^ an integer?

+  /*.  Allow (set (SUBREG:SI (REG:SF)) (SUBREG:SI (REG:SF))).  */
  ^ No period, one space.

The change to rs6000_emit_move() really should have been in a helper
function. We have to stop adding to the complexity of the function.  I
won't insist that you split it out, but any addition of more than a
few lines in rs6000_emit_move(), prologue or epilogue should be placed
into a separate function.

Okay with those changes.  This change is too invasive for GCC 6
backport at this late phase of GCC 6 release.

We'll see if Segher catches anything additional when he wants some
light reading. :-)

Thanks, David


Re: [patch,avr] PR78883: Implement a dummy scheduler

2017-01-04 Thread Segher Boessenkool
On Wed, Jan 04, 2017 at 06:42:23PM +, Richard Sandiford wrote:
> 1. reload has a bug that no-one really wants to fix (understandable)
> 2. the bug is triggered by paradoxical subregs of mems
> 3. those subregs are normally disabled on targets that support insn
>scheduling
> 4. therefore, define an insn scheduler
> 5. we don't actually want insn scheduling, so either
>(a) make sure the insn scheduler is never actually used for insn
>scheduling, or
>(b) allow the insn scheduler to run anyway but encourage it to do nothing
>(other than take compile time)
> 
> (4) and (5) feel like too much of a hack to me.  They're going to have
> other consequences, e.g. we'll no longer give the warning:
> 
>   instruction scheduling not supported on this target machine
> 
> if users try to use -fschedule-insns.  And since we don't support
> meaningful insn scheduling even after this patch, giving the warning
> seems more user-friendly than dropping it.
> 
> I think the consensus is that we don't want these subregs for AVR
> regardless of whether scheduling is used, and probably wouldn't want
> them even without this bug.

Right, and the same is true for most targets.  Subregs of memory are not
something you want.  As rtl.texi says:


@item mem
@code{subreg}s of @code{mem} were common in earlier versions of GCC and
are still supported.  During the reload pass these are replaced by plain
@code{mem}s.  On machines that do not do instruction scheduling, use of
@code{subreg}s of @code{mem} are still used, but this is no longer
recommended.  Such @code{subreg}s are considered to be
@code{register_operand}s rather than @code{memory_operand}s before and
during reload.  Because of this, the scheduling passes cannot properly
schedule instructions with @code{subreg}s of @code{mem}, so for machines
that do scheduling, @code{subreg}s of @code{mem} should never be used.
To support this, the combine and recog passes have explicit code to
inhibit the creation of @code{subreg}s of @code{mem} when
@code{INSN_SCHEDULING} is defined.


> So why not instead change the condition
> used by general_operand, like we were talking about yesterday?
> It seems simpler and more direct.

We should split off a new "SUBREGS_OF_MEM_ALLOWED" from !INSN_SCHEDULING,
and then probably even default it to false.


Segher


Re: [PR tree-optimization/67955] Exploit PTA in DSE

2017-01-04 Thread Jeff Law

On 01/04/2017 11:55 AM, Jeff Law wrote:

On 12/09/2016 01:28 AM, Richard Biener wrote:

On Wed, Dec 7, 2016 at 12:18 AM, Jeff Law  wrote:



So I was going through the various DSE related bugs as stumbled across
67955.

The basic issue in 67955 is that DSE is too simplistic when trying to
determine if two writes hit the same memory address.  There are cases
were
PTA analysis can get us that information.

The unfortunate problem is that PTA can generate a single points-to
set for
pointers to different elements in the same array.  So we can only
exploit a
special case.  Namely when we get a PTA singleton and the size of the
store
is the same size of the pointed-to object.

That doesn't happen in a bootstrap, but it does happen for the
testcase in
the BZ as well as a handful of tests in the testsuite -- Tom reported 6
unique tests where it triggered, I ran my own tests where two more
were spit
out.  Not huge.  But the cost is low.

All that I've done with Tom's patch is update the call into the PTA
system.
It's very clearly Tom's work.

Bootstrapped and regression tested on x86_64-linux-gnu.  Installing
on the
trunk.


Just double-checked it doesn't break

int foo (int *p, int b)
{
  int *q;
  int i = 1;
  if (b)
q = &i;
  else
q = (void *)0;
  *q = 2;
  return i;
}

Right.  ISTM this shouldn't have a singleton points-to set and thus
shouldn't change behavior.But looking at things more closely, it
looks like points-to-null is distinct from the points-to variables.  ie,
we want to know if its a singleton and ! null.  Thus we have to check
pt_solution_singleton_or_null_p (pi->pt, &pt_uid) && !pi->pt->null

I think it's working for you because the path isolation code splits off
the NULL dereference path.  Adding
-fno-isolate-erroneous-paths-dereference to the switches shows DSE2,
incorrectly IMHO, removing the i = 1 store.

Thoughts?
The more I think about this the more I'm sure we need to verify pt.null 
is not in the points-to set.I've taken the above testcase and added 
it as a negative test.  Bootstrapped, regression tested and committed to 
the trunk along with the other minor cleanups you pointed out.


Thanks,

Jeff

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d43c9bc..3a7ad9d 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2017-01-04  Jeff Law  
+
+   PR tree-optimizatin/67955
+   * tree-ssa-alias.c (same_addr_size_stores_p): Check offsets first.
+   Allow any SSA_VAR_P as the base objects.  Use integer_zerop.  Verify
+   the points-to solution does not include pt_null.  Use DECL_PT_UID
+   unconditionally.
+
 2017-01-04  Uros Bizjak  
 
* config/i386/i386.md (HI/SImode test with imm to QImode splitters):
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d8ff32f..0cbc8cc 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-01-03  Jeff Law  
+
+   PR tree-optimization/67955
+   * gcc.dg/tree-ssa/ssa-dse-28.c: New test.
+
 2017-01-04  Marek Polacek  
 
PR c++/77545
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-28.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-28.c
new file mode 100644
index 000..d35377b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-28.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details -fexceptions 
-fnon-call-exceptions -fno-isolate-erroneous-paths-dereference" } */
+
+
+int foo (int *p, int b)
+{
+  int *q;
+  int i = 1;
+  if (b)
+q = &i;
+  else
+q = (void *)0;
+  *q = 2;
+  return i;
+}
+
+/* { dg-final { scan-tree-dump-not "Deleted dead store" "dse1"} } */
+/* { dg-final { scan-tree-dump-not "Deleted dead store" "dse2"} } */
+/* { dg-final { scan-tree-dump-not "Deleted dead store" "dse3"} } */
+
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 0a9fc74..871fa12 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -2326,9 +2326,13 @@ same_addr_size_stores_p (tree base1, HOST_WIDE_INT 
offset1, HOST_WIDE_INT size1,
 tree base2, HOST_WIDE_INT offset2, HOST_WIDE_INT size2,
 HOST_WIDE_INT max_size2)
 {
-  /* For now, just handle VAR_DECL.  */
-  bool base1_obj_p = VAR_P (base1);
-  bool base2_obj_p = VAR_P (base2);
+  /* Offsets need to be 0.  */
+  if (offset1 != 0
+  || offset2 != 0)
+return false;
+
+  bool base1_obj_p = SSA_VAR_P (base1);
+  bool base2_obj_p = SSA_VAR_P (base2);
 
   /* We need one object.  */
   if (base1_obj_p == base2_obj_p)
@@ -2356,13 +2360,9 @@ same_addr_size_stores_p (tree base1, HOST_WIDE_INT 
offset1, HOST_WIDE_INT size1,
   if (size1 != size2)
 return false;
 
-  /* Offsets need to be 0.  */
-  if (offset1 != 0
-  || offset2 != 0)
-return false;
 
   /* Check that memref is a store to pointer with singleton points-to info.  */
-  if (!tree_int_cst_equal (TREE_OPERAND (memref, 1), integer_zero_node))
+  if (!integer_zerop (TREE_OPERAND (memref, 1)))
 return false;
   

Re: [PATCH] Special case clzl like builtins in factor_out_conditional_conversion (PR tree-optimization/71016)

2017-01-04 Thread Jeff Law

On 01/04/2017 11:01 AM, Jakub Jelinek wrote:

On Wed, Jan 04, 2017 at 11:19:46AM +0100, Richard Biener wrote:

For the SSA_NAME + INTEGER_CST case restrict it to the case

  if (x_1 > 5)
tem_2 = (char) x_1;
  # tem_3 = PHI 

that is, (char) x_1 uses x_1 and that also appears in the controlling
GIMPLE_COND.  That's what enables followup minmax replacement
(gcc.dg/tree-ssa/pr66726.c).  Another case where it might be
profitable is if the BB is empty after the conversion is sunk
(may enable other value-replacement cases).


Do we actually have any testcases where we need the SSA_NAME + INTEGER_CST
case?
The only testcase that has been added that actually relied on the
factor_out_* is since r242750 handled during gimple folding (already in
*.gimple dump).

If I modify the testcase so that it isn't recognized by the minmax folding,
extern unsigned short y[];

int
foo (int x)
{
  return 64 < y[x] ? 68 : y[x];
}

then vanilla gcc vs. gcc with factor_out* INTEGER_CST case removed gives:
-   cmpw$65, %ax
-   cmovnb  %edx, %eax
+   cmpw$64, %ax
+   cmova   %edx, %eax
so not worse or better.  And, even when the conversion is the only stmt
in the basic block, that still doesn't allow the bb to be removed, we need
the empty bb for the PHIs.
My recollection is this was a piece of the puzzle towards solving 45397. 
   Given that we haven't solved 45397 yet, I won't object if we want to 
reject SSA_NAME + INTEGER_CST at this time and come back to it when we 
dig into 45397 again.


jeff


Re: [patch,avr] PR78883: Implement a dummy scheduler

2017-01-04 Thread Jeff Law

On 01/04/2017 12:18 PM, Segher Boessenkool wrote:

On Wed, Jan 04, 2017 at 06:42:23PM +, Richard Sandiford wrote:

1. reload has a bug that no-one really wants to fix (understandable)
2. the bug is triggered by paradoxical subregs of mems
3. those subregs are normally disabled on targets that support insn
   scheduling
4. therefore, define an insn scheduler
5. we don't actually want insn scheduling, so either
   (a) make sure the insn scheduler is never actually used for insn
   scheduling, or
   (b) allow the insn scheduler to run anyway but encourage it to do nothing
   (other than take compile time)

(4) and (5) feel like too much of a hack to me.  They're going to have
other consequences, e.g. we'll no longer give the warning:

  instruction scheduling not supported on this target machine

if users try to use -fschedule-insns.  And since we don't support
meaningful insn scheduling even after this patch, giving the warning
seems more user-friendly than dropping it.

I think the consensus is that we don't want these subregs for AVR
regardless of whether scheduling is used, and probably wouldn't want
them even without this bug.


Right, and the same is true for most targets.  Subregs of memory are not
something you want.  As rtl.texi says:


@item mem
@code{subreg}s of @code{mem} were common in earlier versions of GCC and
are still supported.  During the reload pass these are replaced by plain
@code{mem}s.  On machines that do not do instruction scheduling, use of
@code{subreg}s of @code{mem} are still used, but this is no longer
recommended.  Such @code{subreg}s are considered to be
@code{register_operand}s rather than @code{memory_operand}s before and
during reload.  Because of this, the scheduling passes cannot properly
schedule instructions with @code{subreg}s of @code{mem}, so for machines
that do scheduling, @code{subreg}s of @code{mem} should never be used.
To support this, the combine and recog passes have explicit code to
inhibit the creation of @code{subreg}s of @code{mem} when
@code{INSN_SCHEDULING} is defined.



So why not instead change the condition
used by general_operand, like we were talking about yesterday?
It seems simpler and more direct.


We should split off a new "SUBREGS_OF_MEM_ALLOWED" from !INSN_SCHEDULING,
and then probably even default it to false.
That would work for me :-)  The question in my mind would be unexpected 
fallout at this point in the release process.  Maybe default it to 
!INSN_SCHEDULING to minimize such fallout now, then to false for gcc-8?



jeff


Re: [C++ PATCH] Reject invalid auto foo (), a = 5;

2017-01-04 Thread Jason Merrill
On Tue, Jan 3, 2017 at 5:39 PM, Jakub Jelinek  wrote:
> + if (auto_function_declaration
> + && (TREE_CODE (decl) == FUNCTION_DECL
> + || auto_function_declaration != error_mark_node))
> +   {
> + error_at (decl_specifiers.locations[ds_type_spec],
> +   "non-variable %qD in declaration with more than one "
> +   "declarator with placeholder type",
> +   TREE_CODE (decl) == FUNCTION_DECL
> +   ? decl : auto_function_declaration);
> + auto_function_declaration = error_mark_node;
> +   }
> + else if (auto_function_declaration == NULL_TREE)
> +   auto_function_declaration
> + = TREE_CODE (decl) == FUNCTION_DECL ? decl : error_mark_node;

I might reorder the then/else clauses so you don't need to duplicate
the test whether auto_function_declaration is null, but OK either way.

Jason


Re: [C++ PATCH] Avoid UB in cp_lexer_previous_token (PR c++/71182)

2017-01-04 Thread Jason Merrill
OK.

On Tue, Jan 3, 2017 at 5:42 PM, Jakub Jelinek  wrote:
> Hi!
>
> cp_lexer_new_from_tokens creates lexer that has NULL lexer->buffer,
> calling lexer->buffer->address () therefore is UB (diagnosed by
> --with-boot-config=bootstrap-ubsan).
>
> The following patch fixes this, or Markus offered
>   gcc_assert (!lexer->buffer || tp != lexer->buffer->address ());
> instead.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk (or do you prefer Markus' version)?
>
> 2017-01-03  Jakub Jelinek  
>
> PR c++/71182
> * parser.c (cp_lexer_previous_token): Use vec_safe_address in the
> assertion, as lexer->buffer may be NULL.
>
> * g++.dg/cpp0x/pr71182.C: New test.
>
> --- gcc/cp/parser.c.jj  2017-01-03 09:58:13.0 +0100
> +++ gcc/cp/parser.c 2017-01-03 11:39:56.940595981 +0100
> @@ -766,7 +766,7 @@ cp_lexer_previous_token (cp_lexer *lexer
>/* Skip past purged tokens.  */
>while (tp->purged_p)
>  {
> -  gcc_assert (tp != lexer->buffer->address ());
> +  gcc_assert (tp != vec_safe_address (lexer->buffer));
>tp--;
>  }
>
> --- gcc/testsuite/g++.dg/cpp0x/pr71182.C.jj 2017-01-03 11:44:19.400246340 
> +0100
> +++ gcc/testsuite/g++.dg/cpp0x/pr71182.C2017-01-03 11:44:04.795432815 
> +0100
> @@ -0,0 +1,12 @@
> +// PR c++/71182
> +// { dg-do compile { target c++11 } }
> +
> +class A {
> +  template  void As();
> +};
> +template  class B : A {
> +  void f() {
> +A *g ;
> +g ? g->As() : nullptr;
> +  }
> +};
>
> Jakub


Re: [PATCH] Change DWARF5 .debug_loclists location description sizes from 2-byte length to uleb128 lengths

2017-01-04 Thread Jason Merrill
OK.

On Tue, Jan 3, 2017 at 6:15 PM, Jakub Jelinek  wrote:
> Hi!
>
> http://dwarfstd.org/ShowIssue.php?issue=161102.1
> got accepted today, so DWARF5 is going to use uleb128 sizes instead of
> 2-byte sizes in .debug_loclists section.
> On a randomly chosen *.i file I had around, this results in shrinking
> of .debug_loclists section size from 0xef7df to 0xddd65, so around 7.5%
> saving, not too bad.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Jan/Mark, are you going to adjust the consumers accordingly?  Thanks.
>
> 2017-01-03  Jakub Jelinek  
>
> * dwarf2out.c (output_loc_list): Don't throw away 64K+ location
> descriptions for -gdwarf-5 and emit them as uleb128 instead of
> 2-byte data.
>
> --- gcc/dwarf2out.c.jj  2017-01-03 19:41:45.0 +0100
> +++ gcc/dwarf2out.c 2017-01-03 20:58:21.304628767 +0100
> @@ -9590,7 +9590,7 @@ output_loc_list (dw_loc_list_ref list_he
>  perhaps put it into DW_TAG_dwarf_procedure and refer to that
>  in the expression, but >= 64KB expressions for a single value
>  in a single range are unlikely very useful.  */
> -  if (size > 0x)
> +  if (dwarf_version < 5 && size > 0x)
> continue;
>if (dwarf_version >= 5)
> {
> @@ -9642,8 +9642,6 @@ output_loc_list (dw_loc_list_ref list_he
>   if (strcmp (curr2->begin, curr2->end) == 0
>   && !curr2->force)
> continue;
> - if ((unsigned long) size_of_locs (curr2->expr) > 0x)
> -   continue;
>   break;
> }
>   if (curr2 == NULL || curr->section != curr2->section)
> @@ -9744,8 +9742,13 @@ output_loc_list (dw_loc_list_ref list_he
> }
>
>/* Output the block length for this list of location operations.  */
> -  gcc_assert (size <= 0x);
> -  dw2_asm_output_data (2, size, "%s", "Location expression size");
> +  if (dwarf_version >= 5)
> +   dw2_asm_output_data_uleb128 (size, "Location expression size");
> +  else
> +   {
> + gcc_assert (size <= 0x);
> + dw2_asm_output_data (2, size, "Location expression size");
> +   }
>
>output_loc_sequence (curr->expr, -1);
>  }
>
> Jakub


Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

2017-01-04 Thread Jason Merrill
On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm  wrote:
> PR c++/77829 and PR c++/78656 identify an issue within the C++ frontend
> where it issues nonsensical fix-it hints for misspelled name lookups
> within an explicitly given namespace: it finds the closest name within
> all namespaces, and uses the location of the namespace for the replacement,
> rather than the name.
>
> For example, for this error:
>
>   #include 
>   void* allocate(std::size_t n)
>   {
> return std::alocator().allocate(n);
>   }
>
> we currently emit an erroneous fix-it hint that would generate this
> nonsensical patch:
>
>{
>   -  return std::alocator().allocate(n);
>   +  return allocate::alocator().allocate(n);
>}
>
> whereas we ought to emit a fix-it hint that would generate this patch:
>
>{
>   -  return std::alocator().allocate(n);
>   +  return std::allocator().allocate(n);
>}
>
> This patch fixes the suggestions, in two parts:
>
> The incorrect name in the suggestion is fixed by introducing a
> new function "suggest_alternative_in_explicit_scope"
> for use by qualified_name_lookup_error when handling failures
> in explicitly-given namespaces, looking for hint candidates within
> just that namespace.  The function suggest_alternatives_for gains a
> "suggest_misspellings" bool param, so that we can disable fuzzy name
> lookup for the case where we've ruled out hint candidates in the
> explicitly-given namespace.
>
> This lets us suggest "allocator" (found in "std") rather "allocate"
> (found in the global ns).

This looks good.

> The patch fixes the location for the replacement by updating
> local "unqualified_id" in cp_parser_id_expression from tree to
> cp_expr to avoid implicitly dropping location information, and
> passing this location to a new param of finish_id_expression.
> There are multiple users of finish_id_expression, and it wasn't
> possible to provide location information for the id for all of them
> so the new location information is assumed to be optional there.

> This fixes the underlined location, and ensures that the fix-it hint
> replaces "alocator", rather than "std".

I'm dubious about this approach, as I think this will fix some cases
and not others.  The general problem is that we aren't sure what to do
with the location of a qualified-id: sometimes we use the location of
the unqualified-id, sometimes we use the beginning of the first
nested-name-specifier.  I think the right answer is probably to use a
rich location with the caret on the unqualified-id.  Then the fix-it
hint can use the caret location for the fix-it.  Does that make sense?

Jason


Re: [PATCH], PR 71977/70568/78823: Improve PowerPC code that uses SFmode in unions

2017-01-04 Thread Michael Meissner
On Wed, Jan 04, 2017 at 02:17:16PM -0500, David Edelsohn wrote:
> On Thu, Dec 29, 2016 at 7:24 PM, Michael Meissner
>  wrote:
> 
> Mike,
> 
> Thanks for the tremendous effort on this patch.  A few comments.
> 
> The ChangeLog contains a lot of extraneous commentary that should be
> in the documentation comments.  And a few typos in comments.
> 
> > * config/rs6000/predicates.md (sf_subreg_operand): New predicate
> > to return true if the operand contains a SUBREG mixing SImode and
> > SFmode on 64-bit VSX systems with direct move.  This can be a
> > problem in that we have to know whether the SFmode value should be
> > represented in the 32-bit memory format or the 64-bit scalar
> > format used within the floating point and vector registers.
> 
> The ChangeLog entry should just say "New predicate."  The description
> is in the documentation of the function.

Ok.

> > * config/rs6000/vsx.md (SFBOOL_*): Add peephole2 to recognize when
> > we are converting a SFmode to a SImode, moving the result to a GPR
> > register, doing a single AND/IOR/XOR operation, and then moving it
> > back to a vector register.  Change the insns recognized to move
> > the integer value to the vector register and do the operation
> > there.  This code occurs quite a bit in the GLIBC math library in
> > float math functions.
> 
> SFBOOL constants are not a new peephole2.  Please move the explanation
> into a documentation comment in front of the peephole2.

Ok.

> > (peephole2 to speed up GLIB math functions): Likewise.
>^^^ GLIBC?
> New peephole2.
> 
> > (movsi_from_sf): New insn to handle where we are moving SFmode
> > values to SImode registers and we need to convert the value to the
> > memory format from the format used within the register.
> 
> New define_insn_and_split.
> 
> >(movdi_from_sf_zero_ext): Optimize zero extending movsi_from_sf.
> 
> New define_insn_and_split.
> 
> > (movsf_from_si): New insn to handle where we are moving SImode
> > values to SFmode registers and we need to convert the value to the
> > the format used within the floating point and vector registers to
> > the 32-bit memory format.
> 
> New define_insn_and_split.
> 
> +;; Return 1 if op is a SUBREG that is used to look at a SFmode value as
> +;; and integer or vice versa.
>  ^^ an integer?
> 
> +  /*.  Allow (set (SUBREG:SI (REG:SF)) (SUBREG:SI (REG:SF))).  */
>   ^ No period, one space.

Whoops.  How did that get there?

> The change to rs6000_emit_move() really should have been in a helper
> function. We have to stop adding to the complexity of the function.  I
> won't insist that you split it out, but any addition of more than a
> few lines in rs6000_emit_move(), prologue or epilogue should be placed
> into a separate function.

I will see about moving it out to a helper function.

> Okay with those changes.  This change is too invasive for GCC 6
> backport at this late phase of GCC 6 release.

Yes, I figured it was too invasive for GCC 6, but I know some people want this
patch, ASAP.  In any case, I know it wouldn't just go in, since some of the
constraints are related to VSX small integer support, and that did not make GCC
6.

> We'll see if Segher catches anything additional when he wants some
> light reading. :-)
> 
> Thanks, David
> 

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



[PATCH] PR sanitizer/78992: Fix sigaction definition on 32-bit sparc

2017-01-04 Thread James Clarke
libsanitizer:
PR sanitizer/78992
* sanitizer_common/sanitizer_platform_limits_posix.h: sigaction
should only have __glibc_reserved0 as a member on 64-bit sparc.
---
 libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h 
b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
index 066bf41ffef..c139322839a 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -633,9 +633,12 @@ namespace __sanitizer {
 #ifndef __mips__
 #if defined(__sparc__)
 #if __GLIBC_PREREQ (2, 20)
-// On sparc glibc 2.19 and earlier sa_flags was unsigned long, and
-// __glibc_reserved0 didn't exist.
+// On sparc glibc 2.19 and earlier sa_flags was unsigned long.
+#if defined(__arch64__)
+// To maintain ABI compatibility on sparc64 when switching to an int,
+// __glibc_reserved0 was added.
 int __glibc_reserved0;
+#endif
 int sa_flags;
 #else
 unsigned long sa_flags;
-- 
2.11.0



Re: [PATCH] c++/78765

2017-01-04 Thread Jason Merrill
OK.

Jason

On Wed, Jan 4, 2017 at 11:44 AM, Nathan Sidwell  wrote:
> On 01/04/2017 10:13 AM, Jason Merrill wrote:
>
>> OK, that looks like the problem; we shouldn't be calling
>> maybe_constant_value before we perform the conversion.
>
>
> Yup, that worked.
>
>
> ok?
>
> nathan
>
>
> --
> Nathan Sidwell


Re: [PATCH], PR 71977/70568/78823: Improve PowerPC code that uses SFmode in unions

2017-01-04 Thread Michael Meissner
On Wed, Jan 04, 2017 at 02:17:16PM -0500, David Edelsohn wrote:
> The change to rs6000_emit_move() really should have been in a helper
> function. We have to stop adding to the complexity of the function.  I
> won't insist that you split it out, but any addition of more than a
> few lines in rs6000_emit_move(), prologue or epilogue should be placed
> into a separate function.

I can certainly move it into a static helper function (the code in
rs6000_emit_move is only 20 lines once you eliminate the comments), but did you
want me to move some of the other special case parts that are in the same
region to their own helper functions?  I suspect this should be a GCC 8 work
item, but since I'm already moving stuff around, it would be simple to do it
now while I'm in the code.

These include support for block moves, thread local moves, splitting up 128-bit
IBM double double constants, and all of the decimal reload gunk.

> Okay with those changes.  This change is too invasive for GCC 6
> backport at this late phase of GCC 6 release.
> 
> We'll see if Segher catches anything additional when he wants some
> light reading. :-)
> 
> Thanks, David
> 

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797



Re: C/C++ PATCH to implement -Wpointer-compare warning (PR c++/64767) (version 2)

2017-01-04 Thread Jason Merrill
On Wed, Jan 4, 2017 at 1:19 PM, Marek Polacek  wrote:
> +Note that the code above is illegal in C++11.

"invalid".  The C++ changes are OK.

Jason


Re: [PATCH] PR sanitizer/78992: Fix sigaction definition on 32-bit sparc

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 08:56:07PM +, James Clarke wrote:
> libsanitizer:
>   PR sanitizer/78992
>   * sanitizer_common/sanitizer_platform_limits_posix.h: sigaction
>   should only have __glibc_reserved0 as a member on 64-bit sparc.

It should be committed upstream first and then the ChangeLog entry
should read just:
PR sanitizer/78992
* sanitizer_common/sanitizer_platform_limits_posix.h
(struct __sanitizer_sigaction): Cherry-pick upstream r29.
Otherwise LGTM.

Jakub


[PATCH] Error on Enum option without RejectNegative

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 01:31:28PM +0100, Richard Biener wrote:
> > Rather than implicit RejectNegative it might be better to just diagnose
> > such options as invalid.  If you agree, I can implement that as follow-up.
> > Also note that RejectNegative is only needed on the Enum switches that have
> > the default negatives (that is [Wfm] prefixed I think).
> 
> That would be nice.

This works (and r244071 fails to build with it, r244072 succeeds).
The error is emitted above the option, so it is not hard to find out what
option it is (and it is similar to other similar errors diagnosed by
optc-gen.awk).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-01-04  Jakub Jelinek  

* optc-gen.awk: Emit #error for -W*/-f*/-m* Enum without
RejectNegative.

--- gcc/optc-gen.awk.jj 2017-01-01 12:45:39.0 +0100
+++ gcc/optc-gen.awk2017-01-04 18:24:58.926573992 +0100
@@ -326,6 +326,11 @@ for (i = 0; i < n_opts; i++) {
alias_data = "NULL, NULL, OPT_SPECIAL_ignore"
else
alias_data = "NULL, NULL, N_OPTS"
+   if (flag_set_p("Enum.*", flags[i])) {
+   if (!flag_set_p("RejectNegative", flags[i]) \
+   && opts[i] ~ "^[Wfm]")
+   print "#error Enum allowing negative form"
+   }
} else {
alias_opt = nth_arg(0, alias_arg)
alias_posarg = nth_arg(1, alias_arg)


Jakub


[PATCH] Improve -f{,no}-vect-cost-model

2017-01-04 Thread Jakub Jelinek
On Wed, Jan 04, 2017 at 01:31:28PM +0100, Richard Biener wrote:
> >  And not sure why this actually
> > is RejectNegative, wouldn't
> > Common Alias(fvect-cost-model=,dynamic,unlimited)
> > work just on fvect-cost-model (can test that)?
> 
> Good question ;)  If it works, ok.

And here is a patch for that, in addition to bootstrap/regtests I've
tried in the debugger the effect of both -fvect-cost-model and
-fno-vect-cost-model and they set global_options.x_flag_vect_cost_model
to the expected enum values in each case.

Ok for trunk?

2017-01-04  Jakub Jelinek  

* common.opt (fvect-cost-model): Remove RejectNegative flag, use
3 argument Alias with unlimited for the negative form.
(fno-vect-cost-model): Removed.

--- gcc/common.opt.jj   2017-01-01 12:45:37.0 +0100
+++ gcc/common.opt  2017-01-04 18:30:32.239318711 +0100
@@ -2706,13 +2706,9 @@ EnumValue
 Enum(vect_cost_model) String(cheap) Value(VECT_COST_MODEL_CHEAP)
 
 fvect-cost-model
-Common RejectNegative Alias(fvect-cost-model=,dynamic)
+Common Alias(fvect-cost-model=,dynamic,unlimited)
 Enables the dynamic vectorizer cost model.  Preserved for backward 
compatibility.
 
-fno-vect-cost-model
-Common RejectNegative Alias(fvect-cost-model=,unlimited)
-Enables the unlimited vectorizer cost model.  Preserved for backward 
compatibility.
-
 ftree-vect-loop-version
 Common Ignore
 Does nothing. Preserved for backward compatibility.


Jakub


Re: [PATCH] vimrc: fix TAB settings

2017-01-04 Thread Gerald Pfeifer
On Mon, 14 Nov 2016, Martin Liška wrote:
> Following patch adds TAB settings to contrib/vimrc file.
> Hope it looks reasonable?

This does not appear applied, are you waiting for approval?  If
so, for something like this in contrib we don't need to get overly
strict and I'd say go ahead.

Gerald>From 84eb32c4e84b87d690033f7505b8570427ab8468 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Sun, 13 Nov 2016 12:05:48 +0100
Subject: [PATCH] vimrc: fix TAB settings

---
 contrib/vimrc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/vimrc b/contrib/vimrc
index 34e8f35..7c0c587 100644
--- a/contrib/vimrc
+++ b/contrib/vimrc
@@ -35,7 +35,10 @@ function! SetStyle()
   let l:c_exts = ['c', 'h', 'cpp', 'cc', 'C', 'H', 'def', 'java']
   if index(l:c_exts, l:ext) != -1
 setlocal cindent
+setlocal tabstop=8
 setlocal softtabstop=2
+setlocal shiftwidth=2
+setlocal noexpandtab
 setlocal cinoptions=>4,n-2,{2,^-2,:2,=2,g0,f0,h2,p4,t0,+2,(0,u0,w1,m0
 setlocal textwidth=80
 setlocal formatoptions-=ro formatoptions+=cqlt
-- 
2.10.1



Re: [PATCH] Error on Enum option without RejectNegative

2017-01-04 Thread Joseph Myers
On Wed, 4 Jan 2017, Jakub Jelinek wrote:

> On Wed, Jan 04, 2017 at 01:31:28PM +0100, Richard Biener wrote:
> > > Rather than implicit RejectNegative it might be better to just diagnose
> > > such options as invalid.  If you agree, I can implement that as follow-up.
> > > Also note that RejectNegative is only needed on the Enum switches that 
> > > have
> > > the default negatives (that is [Wfm] prefixed I think).
> > 
> > That would be nice.
> 
> This works (and r244071 fails to build with it, r244072 succeeds).
> The error is emitted above the option, so it is not hard to find out what
> option it is (and it is similar to other similar errors diagnosed by
> optc-gen.awk).
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2017-01-04  Jakub Jelinek  
> 
>   * optc-gen.awk: Emit #error for -W*/-f*/-m* Enum without
>   RejectNegative.

OK.

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


Re: C/C++ PATCH to implement -Wpointer-compare warning (PR c++/64767) (version 2)

2017-01-04 Thread Joseph Myers
On Wed, 4 Jan 2017, Marek Polacek wrote:

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

The C changes are OK.

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


Re: [bootstrap-O3,fortran] silence warning in simplify_transformation_to_array

2017-01-04 Thread Alexandre Oliva
On Jan  4, 2017, Richard Biener  wrote:

>> Unfortunately I don't have another that silences the warning without
>> (or even with) impact on codegen.

> #pragma GCC diagnostic push ("Wno-array-bounds")

> #pragma GCC diagnostic pop

Wwwhaaat?  #pragma GCC diagnostic to silence warnings?  Who'd have
thunk? :-D

*blush*


> ? (well, fix the syntax for me please ;))

Sure can do, but then...  Unlike the assert, that might legitimately
have been put there, this will pollute the source code for the sake of a
non-default build configuration that doesn't usually deserve.  I'll
prepare and post a patch anyway, but do we want to make it standard
practice?

-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer


Re: [C++ PATCH] Don't reject auto deduction of multiple variables if some initializers were type dependent (PR c++/78693)

2017-01-04 Thread Jason Merrill
OK.

On Mon, Jan 2, 2017 at 3:11 PM, Jakub Jelinek  wrote:
> Hi!
>
> As the testcase shows, if some initializers are type dependent, auto_result
> is still using auto, is not yet deduced and the deduction will happen during
> instantiation.  So rejecting it due to inconsistent deduction when one
> type is int and another type auto (or vice versa) is wrong, during
> instantiation it could be still instantiated with initializers that are
> valid.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Though, the PR also shows that we should do similar diagnostics about
> inconsistent auto deduction at instantiation time, but we don't have
> infrastructure for that (at tsubst time we don't know which variables
> were declared together in a single declaration, nor the auto_result
> for those vars where the auto deduction happened already during parsing).
> So I think further work is needed on this PR.
>
> 2017-01-02  Jakub Jelinek  
>
> PR c++/78693
> * parser.c (cp_parser_simple_declaration): Only complain about
> inconsistent auto deduction if auto_result doesn't use auto.
>
> * g++.dg/cpp0x/pr78693.C: New test.
>
> --- gcc/cp/parser.c.jj  2017-01-01 12:45:44.0 +0100
> +++ gcc/cp/parser.c 2017-01-02 14:02:49.690859759 +0100
> @@ -12770,9 +12770,11 @@ cp_parser_simple_declaration (cp_parser*
>if (cp_parser_error_occurred (parser))
> goto done;
>
> -  if (auto_result)
> +  if (auto_result
> + && (!processing_template_decl || !type_uses_auto (auto_result)))
> {
> - if (last_type && last_type != error_mark_node
> + if (last_type
> + && last_type != error_mark_node
>   && !same_type_p (auto_result, last_type))
> {
>   /* If the list of declarators contains more than one declarator,
> --- gcc/testsuite/g++.dg/cpp0x/pr78693.C.jj 2017-01-02 14:05:45.494582539 
> +0100
> +++ gcc/testsuite/g++.dg/cpp0x/pr78693.C2017-01-02 14:05:07.0 
> +0100
> @@ -0,0 +1,31 @@
> +// PR c++/78693
> +// { dg-do compile { target c++11 } }
> +
> +template 
> +void
> +foo (T t)
> +{
> +  auto i = t, j = 1;   // { dg-bogus "inconsistent deduction" }
> +}
> +
> +template 
> +void
> +bar (T t)
> +{
> +  auto i = 1, j = t, k = 2;// { dg-bogus "inconsistent deduction" }
> +}
> +
> +template 
> +void
> +foo (T t, U u)
> +{
> +  auto i = t, j = u;   // { dg-bogus "inconsistent deduction" }
> +}
> +
> +void
> +foo ()
> +{
> +  foo (0);
> +  bar (0);
> +  foo (1, 2);
> +}
>
> Jakub


Re: [C++ PATCH] Reject invalid reference type non-static data members in unions (PR c++/78890)

2017-01-04 Thread Jason Merrill
On Mon, Jan 2, 2017 at 2:32 PM, Jakub Jelinek  wrote:
>   if (TREE_CODE (type) == REFERENCE_TYPE)
> {
> - error ("in C++98 %q+D may not have reference type %qT "
> -"because it is a member of a union", x, type);
> - continue;
> + if (cxx_dialect < cxx11)
> +   {
> + error ("in C++98 %q+D may not have reference type %qT "
> +"because it is a member of a union", x, type);
> + continue;
> +   }

I don't think we need the old error anymore; in C++98 mode we will
have already complained about a static data member reference, so we
can drop the above and just keep the below.

> + else if (TREE_CODE (x) == FIELD_DECL)
> +   {
> + error ("non-static data member %q+D in a union may not "
> +"have reference type %qT", x, type);
> + continue;
> +   }

Jason


Re: [C++ PATCH] Fix -Wunused-but-set-* false positive with ~ of vector type (PR c++/78949)

2017-01-04 Thread Jason Merrill
OK.

On Thu, Dec 29, 2016 at 10:19 AM, Jakub Jelinek  wrote:
> Hi!
>
> For integral arg, mark_exp_read is called during
> cp_perform_integral_promotions, for complex type it is called during
> cp_default_conversion, but for vector types nothing actually calls it.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2016-12-29  Jakub Jelinek  
>
> PR c++/78949
> * typeck.c (cp_build_unary_op): Call mark_rvalue_use on arg if it has
> vector type.
>
> * c-c++-common/Wunused-var-16.c: New test.
>
> --- gcc/cp/typeck.c.jj  2016-12-21 23:12:01.0 +0100
> +++ gcc/cp/typeck.c 2016-12-29 13:48:58.363469890 +0100
> @@ -5907,6 +5907,8 @@ cp_build_unary_op (enum tree_code code,
> inform (location, "did you mean to use logical not (%)?");
>   arg = cp_perform_integral_promotions (arg, complain);
> }
> +  else if (!noconvert && VECTOR_TYPE_P (TREE_TYPE (arg)))
> +   arg = mark_rvalue_use (arg);
>break;
>
>  case ABS_EXPR:
> --- gcc/testsuite/c-c++-common/Wunused-var-16.c.jj  2016-12-29 
> 13:51:07.143825569 +0100
> +++ gcc/testsuite/c-c++-common/Wunused-var-16.c 2016-12-29 13:50:42.0 
> +0100
> @@ -0,0 +1,15 @@
> +/* PR c++/78949 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wunused" } */
> +
> +typedef unsigned char V __attribute__((vector_size(16)));
> +V v;
> +
> +void
> +foo ()
> +{
> +  V y = {};
> +  V x = {};// { dg-bogus "set but not used" }
> +  y &= ~x;
> +  v = y;
> +}
>
> Jakub


  1   2   >