[wwwdocs] testing/testing-ftensor.html updates

[Gerald Pfeifer]

This is a change I committed August 23rd last year, and now found
this mail to gcc-patches@ in my postponed folder.  Ahem.

I'm not sure anyone still does any form of testing using this, but
at least the instructions (and links and how to build) are both more 
up-to-date, general, and also shorter now in case.

Gerald

https://gcc.gnu.org/testing/testing-ftensor.html

Index: testing/testing-ftensor.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/testing/testing-ftensor.html,v
retrieving revision 1.4
diff -u -r1.4 testing-ftensor.html
--- testing/testing-ftensor.html9 Dec 2009 18:29:50 -   1.4
+++ testing/testing-ftensor.html23 Aug 2016 12:39:12 -
@@ -8,37 +8,26 @@
 FTensor build and test guide
 
 This page is a guide to running the testing and timing programs for the
-http://www.gps.caltech.edu/~walter/FTensor/index.html";>FTensor
-tensor class library as part of GCC integration testing.  FTensor
-can be built with GCC version 3.1 and later.
+http://www.wlandry.net/Projects/FTensor";>FTensor
+tensor class library as part of GCC integration testing.
 
 Resource usage
 
-The FTensor distribution, available at
-http://www.gps.caltech.edu/~walter/FTensor/FTensor-1.1pre22.tar.gz";>
-FTensor-1.1pre22.tar.gz, is a 169K file.  The uncompressed
-distribution comprises some 2.3 MB of source files.  Building the testing
-and timing programs adds between 20 and 32 MB of object files and executables
-to this.
+The uncompressed distribution comprises some 2.9 MB of source files.
 
+
 
 Prepare
 
 To prepare for a build, perform the following:
 
-Unpack the file:
-
-tar zxf FTensor-1.1pre22.tar.gz, or
-gunzip -c FTensor-1.1pre22.tar.gz | tar xf -
-
-
-Change directory to the repository thus created:
-
-cd FTensor-1.1pre22
-
-
+Unpack the source archive.
+Change directory to the directory thus created.
+Run ./waf configure followed by ./waf
+to configure and build.
 
 
 Testing
@@ -52,13 +41,11 @@
 To build and run the subset of conformance tests meant for compiler
 testing, do:
 
-
+
 cd tests/conformance
-
 make CXX= CXXOPTIMIZE="" test_compiler
-
 ./test_compiler
-
+
 
 The conformance tests print out PASS or FAIL
 followed by a unique test name.  There should be no failures.
@@ -75,19 +62,11 @@
 To build and run the performance tests using the optimizations that
 are normally used for them, simply do:
 
-
+
 cd tests/speed
-./one_over_script
-
-
-To use different optimizations and/or a different compiler, first do:
-
-
 make CXX= CXXOPTIMIZE="" one_over one_over_fast
-
-
-In this case when the the script one_over_script invokes
-make, there is nothing for make to do.
+./one_over_script
+
 
 In addition to printing timing information, one_over_script
 prints out PASS or FAIL for each performance test,
@@ -101,8 +80,8 @@
 
 Cleanup
 
-Cleanup (to prepare the directory for a fresh run, e.g.
-with another version of the compiler), is done as follows:
+Cleanup (to prepare for a fresh run, e.g.  with another version of
+the compiler), is done as follows:
 
 (cd tests/conformance; make clean)
 (cd tests/speed; make clean)

Re: [RFC][PATCH][AArch64] Improve generic branch cost

[Richard Earnshaw (lists)]

On 09/03/17 14:42, Wilco Dijkstra wrote:
> Hi,
> 
> Recently we've put a lot of effort into improving ifcvt to use CSEL on 
> AArch64.
> In  https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01639.html James determined
> the best value for AArch64 code generation.  Although this setting is used 
> when
> explicitly targeting Cortex cores, it is not otherwise used.  This means by
> default GCC will not use (F)CSEL in many common cases.  Most code is built
> without -mcpu= and thus doesn't use CSEL like this example from GLIBC:
> 
> strtok:
> stpx29, x30, [sp, -48]!
> addx29, sp, 0
> stpx21, x22, [sp, 32]
> movx21, x1
> stpx19, x20, [sp, 16]
> adrpx22, .LANCHOR0
> movx19, x0
> cbzx0, .L12
> .L2:ldrbw0, [x19]
> 
> .L12:
> ldrx19, [x22, #:lo12:.LANCHOR0]
> b.L2
> 
> With -mcpu=cortex-a57 GCC generates:
> 
> stpx29, x30, [sp, -48]!
> cmpx0, 0
> addx29, sp, 0
> stpx21, x22, [sp, 32]
> adrpx21, .LANCHOR0
> stpx19, x20, [sp, 16]
> movx19, x0
> ldrx0, [x21, #:lo12:.LANCHOR0]
> cselx19, x0, x19, eq
> ldrbw0, [x19]
> 
> This is generally faster and smaller.  On one benchmark the new setting fixes 
> a 
> regression since GCC6 and improves performance by 49%.  So I propose to change
> generic_branch_cost to be the same as cortexa57_branch_cost so that all 
> supported
> cores benefit equally from CSEL.  Are there any objections to this?
> 
> Wilco
> 
> 
> ChangeLog:
> 2017-03-09  Wilco Dijkstra  
> 
>   * config/aarch64/aarch64.c (generic_branch_cost): Copy 
> cortexa57_branch_cost.

This is OK.  We already have a number of cores using these values so I
don't think this is likely to be a risky change even in stage 4.

R.

> --
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 5870b5e5d7e8e48cf925b3a62030346f041a7fd6..ea16074af86087a6200d9895583e05acf43d90e2
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -377,8 +377,8 @@ static const struct cpu_vector_cost xgene1_vector_cost =
>  /* Generic costs for branch instructions.  */
>  static const struct cpu_branch_cost generic_branch_cost =
>  {
> -  2,  /* Predictable.  */
> -  2   /* Unpredictable.  */
> +  1,  /* Predictable.  */
> +  3   /* Unpredictable.  */
>  };
>  
>  /* Branch costs for Cortex-A57.  */
> 

[build,darwin] Fix toplevel configure test for LTO on darwin

[FX]

When LTO was introduced, on macOS it needed darwin9 to work. Over time, most 
tests for “*-apple-darwin9” in the toplevel configure were changed to also 
include later darwin versions: *-darwin[[912]]*  However, the check for LTO was 
not updated. As far as I know, this is merely an oversight: LTO works fine on 
Darwin, and most vendors (homebrew, macports, etc.) have been shipping with 
--enable-lto for years.

The attached patch makes it build LTO by default on darwin >= 9. I realize it’s 
late but I hope the very restricted nature (and ultra-low risk) of the patch 
make it a candidate for inclusion in trunk nonetheless.

Bootstrapped and regtested on x86_64-apple-darwin16. OK to commit?

FX




lto_darwin.ChangeLog
Description: Binary data


lto_darwin.diff
Description: Binary data

[PATCH] Document -fipa-vrp

[Martin Jambor]

Hi,

I have noticed that -fipa-vrp was not documented in
gcc/doc/invoke.texi so I propose the following.  When at ti, I took
the liberty of replacing "ipa" with "interprocedural" in the
description of -fipa-bit-cp.

Tested with make info, OK for trunk?

Thanks,

Martin


2017-03-17  Martin Jambor  

* doc/invoke.texi (Option Options): Include -fipa-vrp in the list.
(List of -O2 options): Likewise.
(-fipa-bit-cp): Replace "ipa" with "interprocedural."
(-fipa-vrp) New.
---
 gcc/doc/invoke.texi | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 15eb0e0289a..d7854760b8b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -372,7 +372,7 @@ Objective-C and Objective-C++ Dialects}.
 -fif-conversion2  -findirect-inlining @gol
 -finline-functions  -finline-functions-called-once  -finline-limit=@var{n} @gol
 -finline-small-functions  -fipa-cp  -fipa-cp-clone @gol
--fipa-bit-cp @gol
+-fipa-bit-cp -fipa-vrp @gol
 -fipa-pta  -fipa-profile  -fipa-pure-const  -fipa-reference  -fipa-icf @gol
 -fira-algorithm=@var{algorithm} @gol
 -fira-region=@var{region}  -fira-hoist-pressure @gol
@@ -7079,6 +7079,7 @@ also turns on the following optimization flags:
 -findirect-inlining @gol
 -fipa-cp @gol
 -fipa-bit-cp @gol
+-fipa-vrp @gol
 -fipa-sra @gol
 -fipa-icf @gol
 -fisolate-erroneous-paths-dereference @gol
@@ -8089,9 +8090,15 @@ This flag is enabled by default at @option{-O3}.
 
 @item -fipa-bit-cp
 @opindex -fipa-bit-cp
-When enabled, perform ipa bitwise constant propagation. This flag is
-enabled by default at @option{-O2}. It requires that @option{-fipa-cp}
-is enabled.
+When enabled, perform interprocedural bitwise constant
+propagation. This flag is enabled by default at @option{-O2}. It
+requires that @option{-fipa-cp} is enabled.
+
+@item -fipa-vrp
+@opindex -fipa-vrp
+When enabled, perform interprocedural propagation of value
+ranges. This flag is enabled by default at @option{-O2}. It requires
+that @option{-fipa-cp} is enabled.
 
 @item -fipa-icf
 @opindex fipa-icf
-- 
2.12.0

Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[James Greenhalgh]

On Thu, Mar 16, 2017 at 08:26:42PM -0700, Jim Wilson wrote:
> On Thu, Mar 16, 2017 at 11:01 AM, Andrew Pinski  wrote:
> > On Thu, Mar 16, 2017 at 10:22 AM, Wilco Dijkstra  
> > wrote:
> >> Many supported cores implement fusion of AES instructions.  When fusion
> >> happens it can give a significant performance gain.  If not, scheduling
> >> fusion candidates next to each other has almost no effect on performance.
> >> Due to the high benefit/low cost it makes sense to enable AES fusion with
> >> -mcpu=generic so that cores that support it always benefit.  Any 
> >> objections?
> 
> No objection.  I'm not currently tracking performance of -mcpu=generic
> on falkor, so I'm not very concerned about changes to the generic
> tuning structure.

Thanks for the feedback Jim, Andrew.

This patch is OK for trunk. As Richard pointed out on the branch costs
thread, if we had a bug here we'd likely have seen it by now on those
cores which do enable the fusion.

Thanks,
James

[PATCH] Fix PR80048

[Richard Biener]

Bootstrapped / tested on x86_64-unknown-linux-gnu, applied.

Richard.

2017-03-17  Richard Biener  

PR tree-optimization/80048
* sese.c (free_sese_info): Properly release rename_map and
copied_bb_map elements.

Index: gcc/sese.c
===
--- gcc/sese.c  (revision 246188)
+++ gcc/sese.c  (working copy)
@@ -199,11 +199,11 @@ free_sese_info (sese_info_p region)
   region->loop_nest.release ();
 
   for (rename_map_t::iterator it = region->rename_map->begin ();
-   it != region->rename_map->begin (); ++it)
+   it != region->rename_map->end (); ++it)
 (*it).second.release ();
 
   for (bb_map_t::iterator it = region->copied_bb_map->begin ();
-   it != region->copied_bb_map->begin (); ++it)
+   it != region->copied_bb_map->end (); ++it)
 (*it).second.release ();
 
   delete region->rename_map;

[PATCH] Fix PR80050

[Richard Biener]

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2017-03-17  Richard Biener  

PR middle-end/80050
* genmatch.c (parser::next): Remove pointless check for CPP_EOF.
(parser::peek): Likewise.

Index: gcc/genmatch.c
===
--- gcc/genmatch.c  (revision 246188)
+++ gcc/genmatch.c  (working copy)
@@ -3826,8 +3826,7 @@ parser::next ()
 {
   token = cpp_get_token (r);
 }
-  while (token->type == CPP_PADDING
-&& token->type != CPP_EOF);
+  while (token->type == CPP_PADDING);
   return token;
 }
 
@@ -3842,8 +3841,7 @@ parser::peek (unsigned num)
 {
   token = cpp_peek_token (r, i++);
 }
-  while ((token->type == CPP_PADDING
- && token->type != CPP_EOF)
+  while (token->type == CPP_PADDING
 || (--num > 0));
   /* If we peek at EOF this is a fatal error as it leaves the
  cpp_reader in unusable state.  Assume we really wanted a

Re: [PATCH] Document -fipa-vrp

[Richard Biener]

On Fri, Mar 17, 2017 at 11:55 AM, Martin Jambor  wrote:
> Hi,
>
> I have noticed that -fipa-vrp was not documented in
> gcc/doc/invoke.texi so I propose the following.  When at ti, I took
> the liberty of replacing "ipa" with "interprocedural" in the
> description of -fipa-bit-cp.
>
> Tested with make info, OK for trunk?

Ok.

Richard.

> Thanks,
>
> Martin
>
>
> 2017-03-17  Martin Jambor  
>
> * doc/invoke.texi (Option Options): Include -fipa-vrp in the list.
> (List of -O2 options): Likewise.
> (-fipa-bit-cp): Replace "ipa" with "interprocedural."
> (-fipa-vrp) New.
> ---
>  gcc/doc/invoke.texi | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 15eb0e0289a..d7854760b8b 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -372,7 +372,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fif-conversion2  -findirect-inlining @gol
>  -finline-functions  -finline-functions-called-once  -finline-limit=@var{n} 
> @gol
>  -finline-small-functions  -fipa-cp  -fipa-cp-clone @gol
> --fipa-bit-cp @gol
> +-fipa-bit-cp -fipa-vrp @gol
>  -fipa-pta  -fipa-profile  -fipa-pure-const  -fipa-reference  -fipa-icf @gol
>  -fira-algorithm=@var{algorithm} @gol
>  -fira-region=@var{region}  -fira-hoist-pressure @gol
> @@ -7079,6 +7079,7 @@ also turns on the following optimization flags:
>  -findirect-inlining @gol
>  -fipa-cp @gol
>  -fipa-bit-cp @gol
> +-fipa-vrp @gol
>  -fipa-sra @gol
>  -fipa-icf @gol
>  -fisolate-erroneous-paths-dereference @gol
> @@ -8089,9 +8090,15 @@ This flag is enabled by default at @option{-O3}.
>
>  @item -fipa-bit-cp
>  @opindex -fipa-bit-cp
> -When enabled, perform ipa bitwise constant propagation. This flag is
> -enabled by default at @option{-O2}. It requires that @option{-fipa-cp}
> -is enabled.
> +When enabled, perform interprocedural bitwise constant
> +propagation. This flag is enabled by default at @option{-O2}. It
> +requires that @option{-fipa-cp} is enabled.
> +
> +@item -fipa-vrp
> +@opindex -fipa-vrp
> +When enabled, perform interprocedural propagation of value
> +ranges. This flag is enabled by default at @option{-O2}. It requires
> +that @option{-fipa-cp} is enabled.
>
>  @item -fipa-icf
>  @opindex fipa-icf
> --
> 2.12.0
>
>

Re: [PATCH] Fix PR79908

[Richard Biener]

On Tue, Mar 14, 2017 at 10:36 PM, Bill Schmidt
 wrote:
>
>> On Mar 14, 2017, at 11:07 AM, Bill Schmidt  
>> wrote:
>>>
>>> Your suggestion failed bootstrap in libiberty on vprintf-support.c.  
>>> Compilation failed with:
>>>
>>> /home/wschmidt/gcc/build/gcc-mainline-test2-debug/gcc/xgcc 
>>> -B/home/wschmidt/gcc/build/gcc-mainline-test2-debug/gcc/ 
>>> -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/bin/
>>>  
>>> -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/bin/
>>>  
>>> -B/home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/lib/
>>>  -isystem 
>>> /home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/include
>>>  -isystem 
>>> /home/wschmidt/gcc/install/gcc-mainline-test2-debug/powerpc64le-unknown-linux-gnu/sys-include
>>> -c -DHAVE_CONFIG_H -g -O2 -gtoggle  -I. 
>>> -I/home/wschmidt/gcc/gcc-mainline-test2/libiberty/../include  -W -Wall 
>>> -Wwrite-strings -Wc++-compat -Wstrict-prototypes -Wshadow=local -pedantic  
>>> -D_GNU_SOURCE -fPIC 
>>> /home/wschmidt/gcc/gcc-mainline-test2/libiberty/vprintf-support.c -o 
>>> pic/vprintf-support.o
>>>
>>> The initial expression being gimplified is ADDR_EXPR (VAR_DECL (ap)).  
>>> Gimplification
>>> turns this into MEM_REF (VAR_DECL (D.4274), 0), and the is_gimple_val test 
>>> fails on that.
>>
>> Reduced test case:
>>
>> typedef __builtin_va_list __gnuc_va_list;
>> typedef __gnuc_va_list va_list;
>>
>> void
>> foo (va_list args)
>> {
>>  va_list ap;
>>  __builtin_va_copy (ap, args);
>>  (void)__builtin_va_arg (ap, int);
>>  __builtin_va_end(ap);
>> }
>>
>
> Perhaps something like this?  It appears to be doing better on bootstrap, and 
> avoids
> failure on both the original problem and the new test case above:
>
> Index: gcc/tree-stdarg.c
> ===
> --- gcc/tree-stdarg.c   (revision 246109)
> +++ gcc/tree-stdarg.c   (working copy)
> @@ -1058,7 +1058,13 @@ expand_ifn_va_arg_1 (function *fun)
> gimplify_assign (lhs, expr, &pre);
>   }
> else
> - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
> + {
> +   enum gimplify_status status;
> +   status = gimplify_expr (&expr, &pre, &post, is_gimple_lvalue,
> +   fb_lvalue | fb_mayfail);
> +   if (status == GS_ERROR)
> + gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_rvalue);
> + }
>
> input_location = saved_location;
> pop_gimplify_context (NULL);

No, I was confused in thinking gimplify_expr would handle the case
properly.  For
just gimplifying side-effects we should use the middle-end
gimplification machinery:

Index: tree-stdarg.c
===
--- tree-stdarg.c   (revision 246188)
+++ tree-stdarg.c   (working copy)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
 #include "gimple-iterator.h"
 #include "gimple-walk.h"
 #include "gimplify.h"
+#include "gimplify-me.h"
 #include "tree-into-ssa.h"
 #include "tree-cfg.h"
 #include "tree-stdarg.h"
@@ -1058,12 +1059,12 @@ expand_ifn_va_arg_1 (function *fun)
gimplify_assign (lhs, expr, &pre);
  }
else
- gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
+ force_gimple_operand (expr, &pre, false, NULL_TREE);

input_location = saved_location;
pop_gimplify_context (NULL);

-   gimple_seq_add_seq (&pre, post);
+   gimple_seq_add_seq_without_update (&pre, post);
update_modified_stmts (pre);

/* Add the sequence after IFN_VA_ARG.  This splits the bb right
@@ -1072,11 +1073,10 @@ expand_ifn_va_arg_1 (function *fun)
gimple_find_sub_bbs (pre, &i);

/* Remove the IFN_VA_ARG gimple_call.  It's the last stmt in the
-  bb.  */
+  bb if we added any stmts.  */
unlink_stmt_vdef (stmt);
release_ssa_name_fn (fun, gimple_vdef (stmt));
gsi_remove (&i, true);
-   gcc_assert (gsi_end_p (i));

/* We're walking here into the bbs which contain the expansion of
   IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs


> Bill

Re: [PATCH] add calls.c to GTFILES in Makefile.in (PR 79936)

[Richard Biener]

On Tue, Mar 14, 2017 at 4:52 PM, Jeff Law  wrote:
> On 03/14/2017 03:01 AM, Richard Biener wrote:
>>
>> On Tue, Mar 14, 2017 at 1:37 AM, Martin Sebor  wrote:
>>>
>>> Ping: this a P3 regression targeted at 7.0.1.  It was found
>>> on x86_64-apple-darwin10.8.0 but affects all targets (even
>>> if it doesn't happen to manifest on them):
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00342.html
>>
>>
>> I think you need to include gt-calls.h at the bottom of the file.  See
>> other
>> GTFILES
>
> But the patch does include gt-calls.h at the end of calls.c, which is
> consistent with other uses.
>
> Or did you have something else in mind?

Eh, no.  Just didn't spot that part...

Patch is ok.

Richard.

> jeff
>

Re: [PATCH] Install gcov-dump.

[Tom de Vries]

On 14/03/17 13:30, Martin Liška wrote:

Tested on my local machine that's properly installed.

Ready for trunk?


I'm guessing there's an invariant that installed tools mention a bug url 
with --help.


Using attached patch, we get:
...
$ gcov-dump --help
Usage: gcov-dump [OPTION] ... gcovfiles
Print coverage file contents
  -h, --help   Print this help
  -v, --versionPrint version number
  -l, --long   Dump record contents too
  -p, --positions  Dump record positions
  -w, --working-sets   Dump working set computed from summary

For bug reporting instructions, please see:
.
...

OK for trunk, 5 and 6 branch?

Thanks,
- Tom
Print bug url for gcov-dump --help

2017-03-17  Tom de Vries  

	* gcov-dump.c (print_usage): Print bug_report_url.

---
 gcc/gcov-dump.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/gcov-dump.c b/gcc/gcov-dump.c
index 3311772..91c4866 100644
--- a/gcc/gcov-dump.c
+++ b/gcc/gcov-dump.c
@@ -136,6 +136,8 @@ print_usage (void)
   printf ("  -l, --long   Dump record contents too\n");
   printf ("  -p, --positions  Dump record positions\n");
   printf ("  -w, --working-sets   Dump working set computed from summary\n");
+  printf ("\nFor bug reporting instructions, please see:\n%s.\n",
+	   bug_report_url);
 }
 
 static void

Re: [PATCH] Install gcov-dump.

[Richard Biener]

On Fri, Mar 17, 2017 at 12:54 PM, Tom de Vries  wrote:
> On 14/03/17 13:30, Martin Liška wrote:
>>
>> Tested on my local machine that's properly installed.
>>
>> Ready for trunk?
>
>
> I'm guessing there's an invariant that installed tools mention a bug url
> with --help.
>
> Using attached patch, we get:
> ...
> $ gcov-dump --help
> Usage: gcov-dump [OPTION] ... gcovfiles
> Print coverage file contents
>   -h, --help   Print this help
>   -v, --versionPrint version number
>   -l, --long   Dump record contents too
>   -p, --positions  Dump record positions
>   -w, --working-sets   Dump working set computed from summary
>
> For bug reporting instructions, please see:
> .
> ...
>
> OK for trunk, 5 and 6 branch?

Ok.

Bonus point if you also write the missing gcov-dump.texi (so we get a
manpage as well).

Thanks,
Richard.

> Thanks,
> - Tom

Re: [PATCH] MPX: Fix option handling.

[Rainer Orth]

Jakub Jelinek  writes:

> On Fri, Mar 10, 2017 at 02:09:20PM +0100, Martin Liška wrote:
>> Hello.
>> 
>> This is follow-up patch which I agreed on with Jakub.
>> It enables CHKP with LSAN and majority of UBSAN options.
>> 
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>> 
>> Ready to be installed?
>> Martin
>
>> >From a410d5e4e028d34dc00b4aa637cdcd3986b971d8 Mon Sep 17 00:00:00 2001
>> From: marxin 
>> Date: Fri, 10 Mar 2017 11:05:27 +0100
>> Subject: [PATCH] MPX: Fix option handling.
>> 
>> gcc/ChangeLog:
>> 
>> 2017-03-10  Martin Liska  
>> 
>> PR target/65705
>> PR target/69804
>>  * toplev.c (process_options): Enable MPX with LSAN and UBSAN
>>  (except -fsanitize=bounds and -fsanitize=bounds-strict).
>
> Please avoid the ()s, that is confusing with ()s used to surround
> function etc. names.  Just use UBSAN,
>   except ... strict.
>
>>  * tree-chkp.c (chkp_walk_pointer_assignments): Verify that
>>  FIELD != NULL.
>
>> +  error_at (UNKNOWN_LOCATION,
>> +"-fcheck-pointer-bounds is not supported with "
>> +"-fsanitize=bounds-strict");
>> +  flag_check_pointer_bounds = 0;
>
> Given the recent i18n discussions, perhaps this ought to be
> %<-fcheck-pointer-bounds%> and %<-fsanitize=bounds-strict%> and similarly
> elsewhere (of course not for Address/Thread Sanitizer words).
>
> Ok for trunk either way.

Unfortunately, that last change broke gcc.target/i386/pr65044.c:

FAIL: gcc.target/i386/pr65044.c  (test for errors, line )
FAIL: gcc.target/i386/pr65044.c (test for excess errors)

seen e.g. on Linux/x86_64 and Solaris/x86.

We have

Excess errors:
cc1: error: '-fcheck-pointer-bounds' is not supported with Address Sanitizer

while the test expects

/* { dg-error "-fcheck-pointer-bounds is not supported with Address Sanitizer" 
"" { target *-*-* } 0 } */

I guess that, given gcc-dg.exp hardcodes LANG=C, changing the dg-error
to just include the single quotes should be enough.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

[PATCH] Fix stmt_could_throw_1_p for constructor RHS

[Richard Biener]

Ever since operation_could_trap_helper_p claimed that CONSTRUCTOR
(or COMPLEX_EXPR) cannot throw stmt_could_throw_1_p ignored whether
the LHS of an assignment possibly could.  The following fixes this
by refactoring the code a bit and handling the LHS before the
RHS.

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

Richard.

2017-03-17  Richard Biener  

PR middle-end/80075
* tree-eh.c (stmt_could_throw_1_p): Only handle gimple assigns.
Properly verify the LHS before the RHS possibly claims to be
handled.
(stmt_could_throw_p): Hande gimple conds fully here.  Clobbers
do not throw.

* g++.dg/torture/pr80075.C: New testcase.

Index: gcc/tree-eh.c
===
*** gcc/tree-eh.c   (revision 246216)
--- gcc/tree-eh.c   (working copy)
*** tree_could_trap_p (tree expr)
*** 2726,2734 
 an assignment or a conditional) may throw.  */
  
  static bool
! stmt_could_throw_1_p (gimple *stmt)
  {
!   enum tree_code code = gimple_expr_code (stmt);
bool honor_nans = false;
bool honor_snans = false;
bool fp_operation = false;
--- 2726,2734 
 an assignment or a conditional) may throw.  */
  
  static bool
! stmt_could_throw_1_p (gassign *stmt)
  {
!   enum tree_code code = gimple_assign_rhs_code (stmt);
bool honor_nans = false;
bool honor_snans = false;
bool fp_operation = false;
*** stmt_could_throw_1_p (gimple *stmt)
*** 2742,2752 
|| TREE_CODE_CLASS (code) == tcc_binary
|| code == FMA_EXPR)
  {
!   if (is_gimple_assign (stmt)
! && TREE_CODE_CLASS (code) == tcc_comparison)
t = TREE_TYPE (gimple_assign_rhs1 (stmt));
-   else if (gimple_code (stmt) == GIMPLE_COND)
-   t = TREE_TYPE (gimple_cond_lhs (stmt));
else
t = gimple_expr_type (stmt);
fp_operation = FLOAT_TYPE_P (t);
--- 2742,2749 
|| TREE_CODE_CLASS (code) == tcc_binary
|| code == FMA_EXPR)
  {
!   if (TREE_CODE_CLASS (code) == tcc_comparison)
t = TREE_TYPE (gimple_assign_rhs1 (stmt));
else
t = gimple_expr_type (stmt);
fp_operation = FLOAT_TYPE_P (t);
*** stmt_could_throw_1_p (gimple *stmt)
*** 2759,2775 
honor_trapv = true;
  }
  
/* Check if the main expression may trap.  */
-   t = is_gimple_assign (stmt) ? gimple_assign_rhs2 (stmt) : NULL;
ret = operation_could_trap_helper_p (code, fp_operation, honor_trapv,
!  honor_nans, honor_snans, t,
   &handled);
if (handled)
  return ret;
  
/* If the expression does not trap, see if any of the individual operands 
may
   trap.  */
!   for (i = 0; i < gimple_num_ops (stmt); i++)
  if (tree_could_trap_p (gimple_op (stmt, i)))
return true;
  
--- 2756,2776 
honor_trapv = true;
  }
  
+   /* First check the LHS.  */
+   if (tree_could_trap_p (gimple_assign_lhs (stmt)))
+ return true;
+ 
/* Check if the main expression may trap.  */
ret = operation_could_trap_helper_p (code, fp_operation, honor_trapv,
!  honor_nans, honor_snans,
!  gimple_assign_rhs2 (stmt),
   &handled);
if (handled)
  return ret;
  
/* If the expression does not trap, see if any of the individual operands 
may
   trap.  */
!   for (i = 1; i < gimple_num_ops (stmt); i++)
  if (tree_could_trap_p (gimple_op (stmt, i)))
return true;
  
*** stmt_could_throw_p (gimple *stmt)
*** 2795,2805 
  case GIMPLE_CALL:
return !gimple_call_nothrow_p (as_a  (stmt));
  
- case GIMPLE_ASSIGN:
  case GIMPLE_COND:
!   if (!cfun->can_throw_non_call_exceptions)
  return false;
!   return stmt_could_throw_1_p (stmt);
  
  case GIMPLE_ASM:
if (!cfun->can_throw_non_call_exceptions)
--- 2796,2817 
  case GIMPLE_CALL:
return !gimple_call_nothrow_p (as_a  (stmt));
  
  case GIMPLE_COND:
!   {
!   if (!cfun->can_throw_non_call_exceptions)
! return false;
!   gcond *cond = as_a  (stmt);
!   tree lhs = gimple_cond_lhs (cond);
!   return operation_could_trap_p (gimple_cond_code (cond),
!  FLOAT_TYPE_P (TREE_TYPE (lhs)),
!  false, NULL_TREE);
!   }
! 
! case GIMPLE_ASSIGN:
!   if (!cfun->can_throw_non_call_exceptions
! || gimple_clobber_p (stmt))
  return false;
!   return stmt_could_throw_1_p (as_a  (stmt));
  
  case GIMPLE_ASM:
if (!cfun->can_throw_non_call_exceptions)
Index: gcc/testsuite/g++.dg/torture/pr80075.C
===
*** gcc/testsuite/g++.dg/torture/pr80075.C  (nonexistent)
-

Re: [PATCH] [ARM] Cleanup macro TARGET_EITHER

[Kyrill Tkachov]

Hi Sudi,

On 16/03/17 11:11, Sudi Das wrote:


Hi all

This is a cleanup patch to remove the macro TARGET_EITHER. This macro seems to 
have become irrelevant in recent times since its previous definition had been 
commented out and replaced with 1.

Bootstrapped and tested on arm-none-linux-gnueabihf.



I've had my eye on that cleanup for some time now, thanks for doing this.
This patch is ok.
Seeing as we're close to the GCC 7 release and this is not a regression fix
I'd rather hold off applying it until after the release, so I've put this in
my queue of things to commit then.

Thanks,
Kyrill


Sudi

2017-03-10  Sudakshina Das  

* config/arm/arm.h (TARGET_EITHER): Delete.
* config/arm/arm.md: Delete instances of TARGET_EITHER.

Re: [PATCH 4/5 ][P1] [PR tree-optimization/71437] Use a dominator order walk rather than random for VRP threading

[Trevor Saunders]

On Thu, Mar 16, 2017 at 01:17:13PM -0600, Jeff Law wrote:
> +thread_outgoing_edges (basic_block bb, gcond *dummy_cond,
> +class const_and_copies *const_and_copies,
> +class avail_exprs_stack *avail_exprs_stack,

nit picking, but what's the point in the class keyword here?

> +tree (*simplify) (gimple *, gimple *,
> +  class avail_exprs_stack *,
> +  basic_block))

this could just be pfn_simplify right?

> +private:
> +  class const_and_copies *m_const_and_copies;
> +  class avail_exprs_stack *m_avail_exprs_stack;

same nit about class here.

> +  const_and_copies *equiv_stack = new const_and_copies ();
>  
> +  hash_table *avail_exprs
> += new hash_table (1024);
> +  avail_exprs_stack *avail_exprs_stack
> += new class avail_exprs_stack (avail_exprs);

None of these ever live passed the end of the function right? so why not
put the objects on the stack?

>delete equiv_stack;
> +  delete avail_exprs_stack;

this misses cleaning up the hash table right?  Though of course its all
unneeded if these objects are on the stack.

Hoep that's useful.

Trev

Re: [PATCH] [ARM] Cleanup macro TARGET_EITHER

[Richard Earnshaw (lists)]

On 17/03/17 12:22, Kyrill Tkachov wrote:
> Hi Sudi,
> 
> On 16/03/17 11:11, Sudi Das wrote:
>>
>> Hi all
>>
>> This is a cleanup patch to remove the macro TARGET_EITHER. This macro
>> seems to have become irrelevant in recent times since its previous
>> definition had been commented out and replaced with 1.
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
> 
> I've had my eye on that cleanup for some time now, thanks for doing this.
> This patch is ok.
> Seeing as we're close to the GCC 7 release and this is not a regression fix
> I'd rather hold off applying it until after the release, so I've put
> this in
> my queue of things to commit then.
> 
> Thanks,
> Kyrill
> 
>> Sudi
>>
>> 2017-03-10  Sudakshina Das  
>>
>> * config/arm/arm.h (TARGET_EITHER): Delete.
>> * config/arm/arm.md: Delete instances of TARGET_EITHER.
> 

The point of TARGET_EITHER was mostly to document that the operation was
deliberately enabled in all compilation modes and that leaving out a
choice of TARGET32 vs Thumb1 was not accidental.  As such, it's more
documentation than necessary code.

Does this really affect the performance/behaviour of the compiler?  If
it doesn't then I'd be in favour of leaving it as is.

R.

[PATCH] Fix uninitialized data in gimple-ssa-store-merging.c (PR tree-optimization/80079)

[Marek Polacek]

Uninitialized data are never good, especially with MALLOC_PERTURB_ set in
your environment.  This patch from the PR fixes stuff for me.

Applying to trunk.

2017-03-17  Marek Polacek  
Markus Trippelsdorf  

PR tree-optimization/80079
* gimple-ssa-store-merging.c (class pass_store_merging): Initialize
m_stores_head.

diff --git gcc/gimple-ssa-store-merging.c gcc/gimple-ssa-store-merging.c
index 5bdb459..17a95a5 100644
--- gcc/gimple-ssa-store-merging.c
+++ gcc/gimple-ssa-store-merging.c
@@ -725,7 +725,7 @@ class pass_store_merging : public gimple_opt_pass
 {
 public:
   pass_store_merging (gcc::context *ctxt)
-: gimple_opt_pass (pass_data_tree_store_merging, ctxt)
+: gimple_opt_pass (pass_data_tree_store_merging, ctxt), m_stores_head ()
   {
   }
 

Marek

[PATCH] Fix PR80032 - handle CLOBBER gimplification differently

[Richard Biener]

The following addresses PR80032 which reports that stack usage has gone
up since DCE got the ability to remove clobbers.  (Control-dependent-)DCE
basically treats clobbers as not necessary and after eliminating
unnecessary stmts sees if it can retain them and if not, removes them.
This ability was added to keep removing "empty" loops (well, now loops
with clobbers).  With control-dependent DCE this always will remove
code like

 if (cond)
   x = CLOBBER;

because as the clobber itself is not necessary the controlling stmt
isn't either and thus the BB with the clobber gets removed.  We can't
simply promote the CLOBBER to execute unconditionally because that
changes semantics.

But what we can do (I think) and what the patch does is at the time
we add the CLOBBER, add it to a point post-dominating the old insertion
point to avoid the above situation.  This will be a spurious one
in the case the condition was not true but at this point we know
that the variable wasn't live in that case.

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

Ok?

Thanks,
Richard.

2017-03-17  Richard Biener  

PR tree-optimization/80032
* gimplify.c (gimple_push_cleanup): Add force_uncond parameter,
if set force the cleanup to happen unconditionally.
(gimplify_target_expr): Push inserted clobbers with force_uncond
to avoid them being removed by control-dependent DCE.

* g++.dg/opt/pr80032.C: New testcase.

Index: gcc/gimplify.c
===
*** gcc/gimplify.c  (revision 246216)
--- gcc/gimplify.c  (working copy)
*** gimplify_cleanup_point_expr (tree *expr_
*** 6288,6297 
  
  /* Insert a cleanup marker for gimplify_cleanup_point_expr.  CLEANUP
 is the cleanup action required.  EH_ONLY is true if the cleanup should
!only be executed if an exception is thrown, not on normal exit.  */
  
  static void
! gimple_push_cleanup (tree var, tree cleanup, bool eh_only, gimple_seq *pre_p)
  {
gimple *wce;
gimple_seq cleanup_stmts = NULL;
--- 6288,6300 
  
  /* Insert a cleanup marker for gimplify_cleanup_point_expr.  CLEANUP
 is the cleanup action required.  EH_ONLY is true if the cleanup should
!only be executed if an exception is thrown, not on normal exit.
!If FORCE_UNCOND is true perform the cleanup unconditionally;  this is
!only valid for clobbers.  */
  
  static void
! gimple_push_cleanup (tree var, tree cleanup, bool eh_only, gimple_seq *pre_p,
!bool force_uncond = false)
  {
gimple *wce;
gimple_seq cleanup_stmts = NULL;
*** gimple_push_cleanup (tree var, tree clea
*** 6301,6307 
if (seen_error ())
  return;
  
!   if (gimple_conditional_context ())
  {
/* If we're in a conditional context, this is more complex.  We only
 want to run the cleanup if we actually ran the initialization that
--- 6304,6311 
if (seen_error ())
  return;
  
!   if (gimple_conditional_context ()
!   && ! force_uncond)
  {
/* If we're in a conditional context, this is more complex.  We only
 want to run the cleanup if we actually ran the initialization that
*** gimplify_target_expr (tree *expr_p, gimp
*** 6426,6436 
NULL);
  TREE_THIS_VOLATILE (clobber) = true;
  clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
! if (cleanup)
!   cleanup = build2 (COMPOUND_EXPR, void_type_node, cleanup,
! clobber);
! else
!   cleanup = clobber;
}
  if (asan_poisoned_variables && dbg_cnt (asan_use_after_scope))
{
--- 6430,6436 
NULL);
  TREE_THIS_VOLATILE (clobber) = true;
  clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
! gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
  if (asan_poisoned_variables && dbg_cnt (asan_use_after_scope))
{
Index: gcc/testsuite/g++.dg/opt/pr80032.C
===
Index: gcc/testsuite/g++.dg/opt/pr80032.C
===
--- gcc/testsuite/g++.dg/opt/pr80032.C  (nonexistent)
+++ gcc/testsuite/g++.dg/opt/pr80032.C  (working copy)
@@ -0,0 +1,121 @@
+// PR tree-optimization/80032
+/* { dg-do compile } */
+/* { dg-require-effective-target c++11 } */
+/* { dg-options "-O2" } */
+/* If DCE removes too many CLOBBERs then stack usage goes through the
+   roof as stack slots can no longer be shared.  */
+/* { dg-additional-options "-Wstack-usage=200" { target x86_64-*-* i?86-*-* } 
} */
+
+typedef unsigned a;
+namespace test {
+enum b { c };
+class ADataContainer;
+class BitMask;
+namespace api {
+

Re: [PATCH] [ARM] Cleanup macro TARGET_EITHER

[Kyrill Tkachov]

Hi Richard,

On 17/03/17 13:11, Richard Earnshaw (lists) wrote:

On 17/03/17 12:22, Kyrill Tkachov wrote:

Hi Sudi,

On 16/03/17 11:11, Sudi Das wrote:

Hi all

This is a cleanup patch to remove the macro TARGET_EITHER. This macro
seems to have become irrelevant in recent times since its previous
definition had been commented out and replaced with 1.

Bootstrapped and tested on arm-none-linux-gnueabihf.


I've had my eye on that cleanup for some time now, thanks for doing this.
This patch is ok.
Seeing as we're close to the GCC 7 release and this is not a regression fix
I'd rather hold off applying it until after the release, so I've put
this in
my queue of things to commit then.

Thanks,
Kyrill


Sudi

2017-03-10  Sudakshina Das  

 * config/arm/arm.h (TARGET_EITHER): Delete.
 * config/arm/arm.md: Delete instances of TARGET_EITHER.

The point of TARGET_EITHER was mostly to document that the operation was
deliberately enabled in all compilation modes and that leaving out a
choice of TARGET32 vs Thumb1 was not accidental.  As such, it's more
documentation than necessary code.

Does this really affect the performance/behaviour of the compiler?  If
it doesn't then I'd be in favour of leaving it as is.


Looking at the usages of TARGET_EITHER I don't see any pattern where it looks
particularly enlightening. Some patterns gated on TARGET_EITHER have, 
immediately after that,
an
if (TARGET_32BIT)
 ...
else
 ...

path which makes it obvious that it handles all configurations anyway.
Some of the uses are gating fairly straightforward standard names like ashlsi3, 
negsi2
and saying that they are available for all configurations is not very helpful 
IMO as you'd
expect those fundamental patterns to be always available anyway.

Finally, TARGET_EITHER is not a particularly helpful name IMO. For a reader 
that would
in theory need the extra documentation, it doesn't convey what is it an EITHER 
*of*
(in this case 32-bit ARM/Thumb or 16-bit Thumb), they'd still need to look it up
in arm.h to see that this is what it means rather than an either of "VFP vs 
NEON"
or "softfloat vs hardfloat ABI" or some other binary choice in the arm backend.

Is there any particular usage of TARGET_EITHER that looks useful as 
documentation?
If so, I'd suggest we add a comment there clarifying it and still remove the 
TARGET_EITHER.

Thanks,
Kyrill


R.

[PATCH] S/390: PR78857: Don't use load and test if result is live.

[Andreas Krebbel]

The FP load and test instruction should not be used for a comparison if
the target operand is being used afterwards.  It unfortunately turns
SNaNs into QNaNs.

Bootstrapped and regression tested on s390x.

gcc/ChangeLog:

2017-03-17  Andreas Krebbel  

* config/s390/s390.md ("cmp_ccs_0"): Add a clobber of the
target operand.  A new splitter adds the clobber statement in case
the target operand is dead anyway.

gcc/testsuite/ChangeLog:

2017-03-17  Andreas Krebbel  

* gcc.target/s390/load-and-test-fp-1.c: New test.
* gcc.target/s390/load-and-test-fp-2.c: New test.
---
 gcc/config/s390/s390.md| 27 --
 gcc/testsuite/gcc.target/s390/load-and-test-fp-1.c | 17 ++
 gcc/testsuite/gcc.target/s390/load-and-test-fp-2.c | 16 +
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/load-and-test-fp-1.c
 create mode 100644 gcc/testsuite/gcc.target/s390/load-and-test-fp-2.c

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 164a644..19daf31 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -1284,16 +1284,39 @@
 
 ; (TF|DF|SF|TD|DD|SD) instructions
 
+
+; load and test instructions turn SNaN into QNaN what is not
+; acceptable if the target will be used afterwards.  On the other hand
+; they are quite convenient for implementing comparisons with 0.0. So
+; try to enable them via splitter if the value isn't needed anymore.
+
 ; ltxbr, ltdbr, ltebr, ltxtr, ltdtr
 (define_insn "*cmp_ccs_0"
   [(set (reg CC_REGNUM)
-(compare (match_operand:FP 0 "register_operand" "f")
- (match_operand:FP 1 "const0_operand"   "")))]
+   (compare (match_operand:FP 0 "register_operand"  "f")
+(match_operand:FP 1 "const0_operand""")))
+   (clobber (match_operand:FP  2 "register_operand" "=0"))]
   "s390_match_ccmode(insn, CCSmode) && TARGET_HARD_FLOAT"
   "ltr\t%0,%0"
[(set_attr "op_type" "RRE")
 (set_attr "type"  "fsimp")])
 
+(define_split
+  [(set (match_operand 0 "cc_reg_operand")
+   (compare (match_operand:FP 1 "register_operand")
+(match_operand:FP 2 "const0_operand")))]
+  "TARGET_HARD_FLOAT && REG_P (operands[1]) && dead_or_set_p (insn, 
operands[1])"
+  [(parallel
+[(set (match_dup 0) (match_dup 3))
+ (clobber (match_dup 1))])]
+ {
+   /* s390_match_ccmode requires the compare to have the same CC mode
+  as the CC destination register.  */
+   operands[3] = gen_rtx_COMPARE (GET_MODE (operands[0]),
+ operands[1], operands[2]);
+ })
+
+
 ; cxtr, cxbr, cdtr, cdbr, cebr, cdb, ceb
 (define_insn "*cmp_ccs"
   [(set (reg CC_REGNUM)
diff --git a/gcc/testsuite/gcc.target/s390/load-and-test-fp-1.c 
b/gcc/testsuite/gcc.target/s390/load-and-test-fp-1.c
new file mode 100644
index 000..b9d5912
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/load-and-test-fp-1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch" } */
+
+/* a is used after the comparison.  We cannot use load and test here
+   since it would turn SNaNs into QNaNs.  */
+
+double gl;
+
+double
+foo (double dummy, double a)
+{
+  if (a == 0.0)
+gl = 1;
+  return a;
+}
+
+/* { dg-final { scan-assembler "cdbr\t" } } */
diff --git a/gcc/testsuite/gcc.target/s390/load-and-test-fp-2.c 
b/gcc/testsuite/gcc.target/s390/load-and-test-fp-2.c
new file mode 100644
index 000..7646fdd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/load-and-test-fp-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* a is not used after the comparison.  So we should use load and test
+   here.  */
+
+double gl;
+
+void
+bar (double a)
+{
+  if (a == 0.0)
+gl = 1;
+}
+
+/* { dg-final { scan-assembler "ltdbr\t" } } */
-- 
2.9.1

C++ PATCH to fix ICE with noexcept and -fgnu-tm (PR c++/80059)

[Marek Polacek]

Paolo recently added the perform_implicit_conversion_flags call in
build_must_not_throw_expr which means that COND might now be an
IMPLICIT_CONV_EXPR.  That means we need to make sure that COND is instantiated
before we try to fold it, because we can get here while parsing a template. 

Me & Paolo discussed how to fix this, and I think the following is best.

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

2017-03-17  Marek Polacek  
Paolo Carlini  

PR c++/80059 - ICE with noexcept and __transaction_atomic
* except.c (build_must_not_throw_expr): Call
instantiate_non_dependent_expr_sfinae.

* g++.dg/tm/pr80059-2.C: New test.
* g++.dg/tm/pr80059.C: New test.

diff --git gcc/cp/except.c gcc/cp/except.c
index 45d00cc..298094e 100644
--- gcc/cp/except.c
+++ gcc/cp/except.c
@@ -271,6 +271,7 @@ build_must_not_throw_expr (tree body, tree cond)
   cond = perform_implicit_conversion_flags (boolean_type_node, cond,
tf_warning_or_error,
LOOKUP_NORMAL);
+  cond = instantiate_non_dependent_expr_sfinae (cond, tf_none);
   cond = cxx_constant_value (cond);
   if (integer_zerop (cond))
return body;
diff --git gcc/testsuite/g++.dg/tm/pr80059-2.C 
gcc/testsuite/g++.dg/tm/pr80059-2.C
index e69de29..10edb3a 100644
--- gcc/testsuite/g++.dg/tm/pr80059-2.C
+++ gcc/testsuite/g++.dg/tm/pr80059-2.C
@@ -0,0 +1,13 @@
+// PR c++/80059
+// { dg-do compile { target c++11 } }
+// { dg-options "-fgnu-tm" }
+
+template int foo(T b)
+{
+  return __transaction_atomic noexcept(b) (0); // { dg-error "is not a 
constant expression" }
+}
+
+void bar()
+{
+  foo(true);
+}
diff --git gcc/testsuite/g++.dg/tm/pr80059.C gcc/testsuite/g++.dg/tm/pr80059.C
index e69de29..1b705b6 100644
--- gcc/testsuite/g++.dg/tm/pr80059.C
+++ gcc/testsuite/g++.dg/tm/pr80059.C
@@ -0,0 +1,13 @@
+// PR c++/80059
+// { dg-do compile { target c++11 } }
+// { dg-options "-fgnu-tm" }
+
+template int foo(bool b)
+{
+  return __transaction_atomic noexcept(b) (0); // { dg-error "is not a 
constant expression" }
+}
+
+void bar()
+{
+  foo(true);
+}

Marek

[v3 PATCH] Implement LWG 2900, The copy and move constructors of optional are not constexpr.

[Ville Voutilainen]

Tested on Linux-x64.

2017-03-17  Ville Voutilainen  

Implement LWG 2900, The copy and move constructors
of optional are not constexpr.
* include/std/optional (_Optional_payload): New.
(_Optional_base): Remove the bool parameter.
(_Optional_base<_Tp, false>): Remove.
(_Optional_base()): Adjust.
(_Optional_base(nullopt_t)): Likewise.
(_Optional_base(in_place_t, _Args&&...)): Likewise.
(_Optional_base(in_place_t, initializer_list<_Up>, _Args&&...)):
Likewise.
(_Optional_base(const _Optional_base&)): Likewise.
(_Optional_base(_Optional_base&&)): Likewise.
(operator=(const _Optional_base&)): Likewise.
(operator=(_Optional_base&&)): Likewise.
(~_Optional_base()): Remove.
(_M_is_engaged()): Adjust.
(_M_get()): Likewise.
(_M_construct(_Args&&...)): Likewise.
(_M_destruct()): Likewise.
(_M_reset()): Likewise.
(_Optional_base::_Empty_byte): Remove.
(_Optional_base::_M_empty): Remove.
(_Optional_base::_M_payload): Adjust.
* testsuite/20_util/optional/cons/value_neg.cc: Adjust.
* testsuite/20_util/optional/constexpr/cons/value.cc: Add tests.
diff --git a/libstdc++-v3/include/std/optional 
b/libstdc++-v3/include/std/optional
index 3f540ec..e67ba89 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -95,125 +95,127 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __throw_bad_optional_access()
   { _GLIBCXX_THROW_OR_ABORT(bad_optional_access()); }
 
-  /**
-* @brief Class template that holds the necessary state for @ref optional
-* and that has the responsibility for construction and the special members.
-*
-* Such a separate base class template is necessary in order to
-* conditionally enable the special members (e.g. copy/move constructors).
-* Note that this means that @ref _Optional_base implements the
-* functionality for copy and move assignment, but not for converting
-* assignment.
-*
-* @see optional, _Enable_special_members
-*/
-  template::value>
-class _Optional_base
-{
-private:
-  // Remove const to avoid prohibition of reusing object storage for
-  // const-qualified types in [3.8/9]. This is strictly internal
-  // and even optional itself is oblivious to it.
-  using _Stored_type = remove_const_t<_Tp>;
 
-public:
+  // Payload for constexpr optionals.
+  template ::value
+ && is_trivially_move_constructible<_Tp>::value,
+   bool /*_ShouldProvideDestructor*/ =
+ is_trivially_destructible<_Tp>::value>
+struct _Optional_payload
+{
+  constexpr _Optional_payload()
+   : _M_empty() {}
 
-  // Constructors for disengaged optionals.
-  constexpr _Optional_base() noexcept
-  : _M_empty{} { }
+  template
+  constexpr _Optional_payload(in_place_t, _Args&&... __args)
+   : _M_payload(std::forward<_Args>(__args)...),
+ _M_engaged(true)
+  {}
 
-  constexpr _Optional_base(nullopt_t) noexcept
-  : _Optional_base{} { }
+  template
+  constexpr _Optional_payload(std::initializer_list<_Up> __il,
+ _Args&&... __args)
+   : _M_payload(__il, std::forward<_Args>(__args)...),
+ _M_engaged(true) {}
+
+  template  struct __ctor_tag {};
+  constexpr _Optional_payload(__ctor_tag,
+ const _Tp& __other)
+   : _M_payload(__other),
+ _M_engaged(true)
+  {}
+
+  constexpr _Optional_payload(__ctor_tag)
+   : _M_empty()
+  {}
+
+  constexpr _Optional_payload(__ctor_tag, _Tp&& __other)
+   : _M_payload(std::move(__other)),
+ _M_engaged(true)
+  {}
+
+  constexpr _Optional_payload(bool __engaged,
+ const _Optional_payload&
+ __other)
+   : _Optional_payload(__engaged ?
+   _Optional_payload(__ctor_tag{},
+ __other._M_payload) :
+   _Optional_payload(__ctor_tag{}))
+  {}
+
+  constexpr _Optional_payload(bool __engaged,
+ _Optional_payload&&
+ __other)
+   : _Optional_payload(__engaged ?
+   _Optional_payload(__ctor_tag{},
+ std::move(__other._M_payload)) :
+   _Optional_payload(__ctor_tag{}))
+  {}
 
-  // Constructors for engaged optionals.
-  template, bool> = false>
-constexpr explicit _Optional_base(in_place_t, _Args&&... __args)
-: _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { }
+  using _Stored_type = remove_const_t<_Tp>;
+  struct _Empty_byte { };
+  union {
+  _Empty_byte _M_empty;
+  _Stored_type _M_payload;
+  };
+  bool _M_engaged = false;
+};
 
-  template&,
-  

Re: [PATCH] Fix PR79908

[Bill Schmidt]

Hi,

> On Mar 17, 2017, at 6:44 AM, Richard Biener  
> wrote:
> 
> No, I was confused in thinking gimplify_expr would handle the case
> properly.  For
> just gimplifying side-effects we should use the middle-end
> gimplification machinery:
> 
> Index: tree-stdarg.c
> ===
> --- tree-stdarg.c   (revision 246188)
> +++ tree-stdarg.c   (working copy)
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
> #include "gimple-iterator.h"
> #include "gimple-walk.h"
> #include "gimplify.h"
> +#include "gimplify-me.h"
> #include "tree-into-ssa.h"
> #include "tree-cfg.h"
> #include "tree-stdarg.h"
> @@ -1058,12 +1059,12 @@ expand_ifn_va_arg_1 (function *fun)
>gimplify_assign (lhs, expr, &pre);
>  }
>else
> - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
> + force_gimple_operand (expr, &pre, false, NULL_TREE);
> 
>input_location = saved_location;
>pop_gimplify_context (NULL);
> 
> -   gimple_seq_add_seq (&pre, post);
> +   gimple_seq_add_seq_without_update (&pre, post);
>update_modified_stmts (pre);
> 
>/* Add the sequence after IFN_VA_ARG.  This splits the bb right
> @@ -1072,11 +1073,10 @@ expand_ifn_va_arg_1 (function *fun)
>gimple_find_sub_bbs (pre, &i);
> 
>/* Remove the IFN_VA_ARG gimple_call.  It's the last stmt in the
> -  bb.  */
> +  bb if we added any stmts.  */
>unlink_stmt_vdef (stmt);
>release_ssa_name_fn (fun, gimple_vdef (stmt));
>gsi_remove (&i, true);
> -   gcc_assert (gsi_end_p (i));
> 
>/* We're walking here into the bbs which contain the expansion of
>   IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs

Looks good, but for some reason I hit a segfault in the linker building 
gengtype when I tried to bootstrap with this.  I assume it's something
latent and unrelated, but will need to investigate.

Bill

Re: [PATCH 4/5 ][P1] [PR tree-optimization/71437] Use a dominator order walk rather than random for VRP threading

[Jeff Law]

On 03/17/2017 06:38 AM, Trevor Saunders wrote:

On Thu, Mar 16, 2017 at 01:17:13PM -0600, Jeff Law wrote:

+thread_outgoing_edges (basic_block bb, gcond *dummy_cond,
+  class const_and_copies *const_and_copies,
+  class avail_exprs_stack *avail_exprs_stack,


nit picking, but what's the point in the class keyword here?
Habit :-)  There's probably a ton of this kind of thing in my code.  I'd 
like to defer cleaning that up until gcc-8.






+  tree (*simplify) (gimple *, gimple *,
+class avail_exprs_stack *,
+basic_block))


this could just be pfn_simplify right?

Yes.  I fixed one of these, I wouldn't be surprised if there's more.




+  const_and_copies *equiv_stack = new const_and_copies ();

+  hash_table *avail_exprs
+= new hash_table (1024);
+  avail_exprs_stack *avail_exprs_stack
+= new class avail_exprs_stack (avail_exprs);


None of these ever live passed the end of the function right? so why not
put the objects on the stack?
Oversight.  The equivalency stack was original live at the end of the 
function, but that's one of the things I changed.  This should be fixed 
for gcc-7.  I'll take care of it later today.





   delete equiv_stack;
+  delete avail_exprs_stack;


this misses cleaning up the hash table right?  Though of course its all
unneeded if these objects are on the stack.

Right.  I'll address this stuff in a follow-up later today.



Hoep that's useful.

Of course.  Feedback is always appreciated.

jeff

[PATCH 5/5 ][P1] [PR tree-optimization/71437] Record ASSERT_EXPR conditions and test them in VRP threading

[Jeff Law]

This is the final patch which finally solves 71437. With the refactoring 
in place, this patch is quite trivial.


All we do is record conditions implied by ASSERT_EXPRs we encounter 
during the dominator walk.  We also record implied conditions.  So if we 
had a == 12 in the ASSERT_EXPR, we'll record a == 12, a >= 12 and a <= 
12 as true in the available expression hash table.  You get the idea.


Then in the threading simplification callback we first lookup the 
condition we want to simplify to see if it's in our hash table (via the 
ASSERT_EXPRs).  And viola it all just works.


20040305-1.c is compromised  by the improved jump threading in VRP.  We 
thread jumps earlier which in turn allows if-combination to fire. 
Thankfully we can just test earlier (dce2).


ssa-dom-thread-4.c is slightly tweaked as we discover a jump threads 
earlier in VRP rather than in DOM.  I've adjusted the expected outputs.


Bootstrapped and regression tested on top of patches #1-#3 on 
x86_64-linux-gnu and ppc64le-linux-gnu.  Installing on the trunk.


Jeff

Re: Combiner fix for PR79910

[Jeff Law]

On 03/15/2017 11:07 AM, Jeff Law wrote:

On 03/15/2017 09:00 AM, Bernd Schmidt wrote:

On 03/15/2017 12:09 AM, Bernd Schmidt wrote:

 I'll retest with your
suggestion and with the bitmap creation conditional on i1 being nonnull.


Like this (also had to throw in a bitmap_empty_p). Retested as before.
Ok?

Yea, not surprised you needed the bitmap_empty_p.

OK with or without the debug counter -- your decide on whether or not to
include it.
Given your note that you had planned to remove the dbg_cnt stuff, I went 
ahead and remove the dbg_cnt stuff, and installed the patch (and testcase).


Jeff

C PATCH to add C11 references

[Marek Polacek]

This means precisely zilch, but hey, it's Friday.  The references are mostly
the same as C99, but they differ for e.g. declarators.  I wish C had what C++
has, i.e. references like [stmt.switch].

Ok for trunk?

2017-03-17  Marek Polacek  

* c-parser.c: Add C11 references.

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 5bc238b..988369e 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1316,7 +1316,7 @@ static void c_parser_cilk_grainsize (c_parser *, bool *);
 
 static void c_parser_parse_rtl_body (c_parser *parser, char *start_with_pass);
 
-/* Parse a translation unit (C90 6.7, C99 6.9).
+/* Parse a translation unit (C90 6.7, C99 6.9, C11 6.9).
 
translation-unit:
  external-declarations
@@ -1359,7 +1359,7 @@ c_parser_translation_unit (c_parser *parser)
   error ("storage size of %q+D isn%'t known", decl);
 }
 
-/* Parse an external declaration (C90 6.7, C99 6.9).
+/* Parse an external declaration (C90 6.7, C99 6.9, C11 6.9).
 
external-declaration:
  function-definition
@@ -1475,8 +1475,8 @@ static void c_finish_omp_declare_simd (c_parser *, tree, 
tree, vec);
 static void c_finish_oacc_routine (struct oacc_routine_data *, tree, bool);
 
 /* Parse a declaration or function definition (C90 6.5, 6.7.1, C99
-   6.7, 6.9.1).  If FNDEF_OK is true, a function definition is
-   accepted; otherwise (old-style parameter declarations) only other
+   6.7, 6.9.1, C11 6.7, 6.9.1).  If FNDEF_OK is true, a function definition
+   is accepted; otherwise (old-style parameter declarations) only other
declarations are accepted.  If STATIC_ASSERT_OK is true, a static
assertion is accepted; otherwise (old-style parameter declarations)
it is not.  If NESTED is true, we are inside a function or parsing
@@ -2262,7 +2262,7 @@ c_parser_static_assert_declaration_no_semi (c_parser 
*parser)
 }
 
 /* Parse some declaration specifiers (possibly none) (C90 6.5, C99
-   6.7), adding them to SPECS (which may already include some).
+   6.7, C11 6.7), adding them to SPECS (which may already include some).
Storage class specifiers are accepted iff SCSPEC_OK; type
specifiers are accepted iff TYPESPEC_OK; alignment specifiers are
accepted iff ALIGNSPEC_OK; attributes are accepted at the start
@@ -2279,7 +2279,7 @@ c_parser_static_assert_declaration_no_semi (c_parser 
*parser)
handled as storage class specifiers, as is __thread.  Alignment
specifiers are from C11.
 
-   C90 6.5.1, C99 6.7.1:
+   C90 6.5.1, C99 6.7.1, C11 6.7.1:
storage-class-specifier:
  typedef
  extern
@@ -2290,14 +2290,14 @@ c_parser_static_assert_declaration_no_semi (c_parser 
*parser)
 
(_Thread_local is new in C11.)
 
-   C99 6.7.4:
+   C99 6.7.4, C11 6.7.4:
function-specifier:
  inline
  _Noreturn
 
(_Noreturn is new in C11.)
 
-   C90 6.5.2, C99 6.7.2:
+   C90 6.5.2, C99 6.7.2, C11 6.7.2:
type-specifier:
  void
  char
@@ -2319,7 +2319,7 @@ c_parser_static_assert_declaration_no_semi (c_parser 
*parser)
(_Bool and _Complex are new in C99.)
(atomic-type-specifier is new in C11.)
 
-   C90 6.5.3, C99 6.7.3:
+   C90 6.5.3, C99 6.7.3, C11 6.7.3:
 
type-qualifier:
  const
@@ -2656,7 +2656,7 @@ c_parser_declspecs (c_parser *parser, struct c_declspecs 
*specs,
  out: ;
 }
 
-/* Parse an enum specifier (C90 6.5.2.2, C99 6.7.2.2).
+/* Parse an enum specifier (C90 6.5.2.2, C99 6.7.2.2, C11 6.7.2.2).
 
enum-specifier:
  enum attributes[opt] identifier[opt] { enumerator-list } attributes[opt]
@@ -2819,7 +2819,7 @@ c_parser_enum_specifier (c_parser *parser)
   return ret;
 }
 
-/* Parse a struct or union specifier (C90 6.5.2.1, C99 6.7.2.1).
+/* Parse a struct or union specifier (C90 6.5.2.1, C99 6.7.2.1, C11 6.7.2.1).
 
struct-or-union-specifier:
  struct-or-union attributes[opt] identifier[opt]
@@ -3014,8 +3014,8 @@ c_parser_struct_or_union_specifier (c_parser *parser)
   return ret;
 }
 
-/* Parse a struct-declaration (C90 6.5.2.1, C99 6.7.2.1), *without*
-   the trailing semicolon.
+/* Parse a struct-declaration (C90 6.5.2.1, C99 6.7.2.1, C11 6.7.2.1),
+   *without* the trailing semicolon.
 
struct-declaration:
  specifier-qualifier-list struct-declarator-list
@@ -3300,13 +3300,13 @@ c_parser_alignas_specifier (c_parser * parser)
 }
 
 /* Parse a declarator, possibly an abstract declarator (C90 6.5.4,
-   6.5.5, C99 6.7.5, 6.7.6).  If TYPE_SEEN_P then a typedef name may
-   be redeclared; otherwise it may not.  KIND indicates which kind of
-   declarator is wanted.  Returns a valid declarator except in the
-   case of a syntax error in which case NULL is returned.  *SEEN_ID is
-   set to true if an identifier being declared is seen; this is used
-   to diagnose bad forms of abstract array declarators and to
-   determine whether an identifier list is syntactically permitted.
+   6.5.5, C99 6.7.5, 6.7.6, C11 6.7.6, 6.7.7).  If TYPE_SEEN_P then
+   a typedef name may be redeclared; otherwise it may not.  KIND
+   indicates which 

Re: patch, libfortran] [patch, fortran] Fix PR 79956, part two, attempt 3

[Markus Trippelsdorf]

On 2017.03.15 at 00:01 +0100, Thomas Koenig wrote:
> Hello world,
> 
> well, here is the third attempt at fixing the second part of the PR.
> Glancing over the source, I think there are quite a few places where
> we currently issue a runtime error which we could replace by an
> assert, but that's something for 8.0.
> 
> Regression-tested on x86_64-pc-linux-gnu.

> Index: generated/reshape_c10.c
> ===
> --- generated/reshape_c10.c   (Revision 245760)
> +++ generated/reshape_c10.c   (Arbeitskopie)
> @@ -232,6 +232,11 @@ reshape_c10 (gfc_array_c10 * const restrict ret,
>  }
>  
>sdim = GFC_DESCRIPTOR_RANK (source);
> +
> +  /* sdim is always > 0; this lets the compiler optimize more and
> +   avoids a warning.  */
> +  GFC_ASSERT(sdim>0);
> +

You have committed a different patch, which is obviously wrong:

diff --git a/libgfortran/generated/reshape_c10.c 
b/libgfortran/generated/reshape_c10.c
index 00c64aeb746f..af45e960ee7f 100644
--- a/libgfortran/generated/reshape_c10.c
+++ b/libgfortran/generated/reshape_c10.c
@@ -78,6 +78,10 @@ reshape_c10 (gfc_array_c10 * const restrict ret,
   index_type shape_data[GFC_MAX_DIMENSIONS];
 
   rdim = GFC_DESCRIPTOR_EXTENT(shape,0);
+  /* rdim is always > 0; this lets the compiler optimize more and
+   avoids a potential warning.  */
+  GFC_ASSERT(sdim>0);

You should use rdim not sdim in the GFC_ASSERT.

-- 
Markus

[aarch64] PR target/80052 Fix typo in aarch64.opt

[Richard Earnshaw (lists)]

Fixes the reported typo in aarch64.opt (dummping -> dumping).

Committed as obvious.

* aarch64.opt(verbose-cost-dump): Fix typo.
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 
54368848bbb249949921a3018d927c4bd61b1fbd..942a7d558f26f0c8c18b0a94f4f92c575e06f057
 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -187,4 +187,4 @@ single precision and to 32 bits for double precision.
 
 mverbose-cost-dump
 Common Undocumented Var(flag_aarch64_verbose_cost)
-Enables verbose cost model dummping in the debug dump files.
+Enables verbose cost model dumping in the debug dump files.

Re: [PATCH] Fix PR79908

[Bill Schmidt]

> On Mar 17, 2017, at 9:52 AM, Bill Schmidt  wrote:
> 
> Hi,
> 
>> On Mar 17, 2017, at 6:44 AM, Richard Biener  
>> wrote:
>> 
>> No, I was confused in thinking gimplify_expr would handle the case
>> properly.  For
>> just gimplifying side-effects we should use the middle-end
>> gimplification machinery:
>> 
>> Index: tree-stdarg.c
>> ===
>> --- tree-stdarg.c   (revision 246188)
>> +++ tree-stdarg.c   (working copy)
>> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.
>> #include "gimple-iterator.h"
>> #include "gimple-walk.h"
>> #include "gimplify.h"
>> +#include "gimplify-me.h"
>> #include "tree-into-ssa.h"
>> #include "tree-cfg.h"
>> #include "tree-stdarg.h"
>> @@ -1058,12 +1059,12 @@ expand_ifn_va_arg_1 (function *fun)
>>   gimplify_assign (lhs, expr, &pre);
>> }
>>   else
>> - gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
>> + force_gimple_operand (expr, &pre, false, NULL_TREE);
>> 
>>   input_location = saved_location;
>>   pop_gimplify_context (NULL);
>> 
>> -   gimple_seq_add_seq (&pre, post);
>> +   gimple_seq_add_seq_without_update (&pre, post);
>>   update_modified_stmts (pre);
>> 
>>   /* Add the sequence after IFN_VA_ARG.  This splits the bb right
>> @@ -1072,11 +1073,10 @@ expand_ifn_va_arg_1 (function *fun)
>>   gimple_find_sub_bbs (pre, &i);
>> 
>>   /* Remove the IFN_VA_ARG gimple_call.  It's the last stmt in the
>> -  bb.  */
>> +  bb if we added any stmts.  */
>>   unlink_stmt_vdef (stmt);
>>   release_ssa_name_fn (fun, gimple_vdef (stmt));
>>   gsi_remove (&i, true);
>> -   gcc_assert (gsi_end_p (i));
>> 
>>   /* We're walking here into the bbs which contain the expansion of
>>  IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs
> 
> Looks good, but for some reason I hit a segfault in the linker building 
> gengtype when I tried to bootstrap with this.  I assume it's something
> latent and unrelated, but will need to investigate.

Ah, I misread the build log.  The link of gengtype succeeds, but gengtype
has apparently been miscompiled (stage2) as it tries to call strlen() with
an address of zero.  So something wrong with this patch.  Looks like the
miscompilation is of libiberty_vprintf_buffer_size in 
libiberty/vprintf-support.c.

I looked at the before and after dumps and we see that VA_ARGs with
no lhs are just removed from the code, instead of expanding the advance
of the va pointer.

Before:

 [1.39%]:  
  VA_ARG (&ap, 0B, 0B); 
  goto  (); [100.00%]   

 [1.39%]:  
  VA_ARG (&ap, 0B, 0B); 
  total_width_89 = total_width_17 + 337;
  goto  (); [100.00%]   

After:

 [1.39%]:
  goto  (); [100.00%]

 [1.39%]:
  total_width_89 = total_width_17 + 337;
  goto  (); [100.00%]

Thanks,
Bill

Re: [PATCH v2, doc] Revise GCC manual section 6.11, Additional Floating Types

[Bill Schmidt]

Joseph, any further comments, or may I commit this?

Thanks!
Bill

> On Mar 13, 2017, at 6:31 PM, Segher Boessenkool  
> wrote:
> 
> Hi Bill,
> 
> On Mon, Mar 13, 2017 at 01:11:01PM -0500, Bill Schmidt wrote:
>> Index: gcc/doc/extend.texi
>> ===
>> --- gcc/doc/extend.texi  (revision 246014)
>> +++ gcc/doc/extend.texi  (working copy)
>> @@ -948,10 +948,28 @@ names can be used together with @code{_Complex} to
>> types.
>> 
>> As an extension, GNU C and GNU C++ support additional floating
>> -types, @code{__float80} and @code{__float128} to support 80-bit
>> -(@code{XFmode}) and 128-bit (@code{TFmode}) floating types; these are
>> -aliases for the type names @code{_Float64x} and @code{_Float128}.
>> -Support for additional types includes the arithmetic operators:
>> +types, which are not supported by all targets.
>> +@itemize @bullet
>> +@item @code{__float128} is available on i386, x86_64, IA-64, and
>> +hppa HP-UX, as well as on PowerPC 64-bit GNU/Linux targets that enable
>> +the vector scalar (VSX) instruction set.  @code{__float128} supports
>> +the 128-bit floating type.  On i386, x86_64, PowerPC, and IA-64
>> +other than HP-UX, @code{__float128} is an alias for @code{_Float128}.
>> +On hppa and IA-64 HP-UX, @code{__float128} is an alias for @code{long
>> +double}.
> 
> __float128 (-mfloat128) does not require 64-bit PowerPC, only hardware
> support (-mfloat128-hardware) requires it (and only because we haven't
> implemented it for 32-bit).
> 
> 
> Segher
> 

[PATCH] Fix PR80054 (SLSR dominance bug)

[Bill Schmidt]

Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80054 reports an ICE
in SSA validation after SLSR (definition does not dominate use).
The problem occurs within conditional strength reduction candidate
processing.

PRE creates a situation where a conditional SLSR candidate depends
on a PHI that is not dominated by the basis for the candidate.
SLSR currently doesn't notice this and eventually creates a phi
basis that is not dominated by the true basis, leading to the error.

This patch eliminates the problem by detecting the dominance failure
and aborting the optimization when it occurs.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  I'll plan to commit this on Monday if there are no
comments.

Thanks,
Bill


[gcc]

2017-03-17  Bill Schmidt  

PR tree-optimization/80054
* gimple-ssa-strength-reduction.c (all_phi_incrs_profitable): Fail
the optimization if a PHI or any of its arguments is not dominated
by the candidate's basis.  Use gphi* rather than gimple* as
appropriate.
(replace_profitable_candidates): Clean up a gimple* variable that
should be a gphi* variable.

[gcc/testsuite]

2017-03-17  Bill Schmidt  

PR tree-optimization/80054
* g++.dg/torture/pr80054.C: New file.


Index: gcc/gimple-ssa-strength-reduction.c
===
--- gcc/gimple-ssa-strength-reduction.c (revision 246198)
+++ gcc/gimple-ssa-strength-reduction.c (working copy)
@@ -3279,17 +3279,34 @@ insert_initializers (slsr_cand_t c)
 }
 
 /* Return TRUE iff all required increments for candidates feeding PHI
-   are profitable to replace on behalf of candidate C.  */
+   are profitable (and legal!) to replace on behalf of candidate C.  */
 
 static bool
-all_phi_incrs_profitable (slsr_cand_t c, gimple *phi)
+all_phi_incrs_profitable (slsr_cand_t c, gphi *phi)
 {
   unsigned i;
   slsr_cand_t basis = lookup_cand (c->basis);
   slsr_cand_t phi_cand = *stmt_cand_map->get (phi);
 
+  /* If the basis doesn't dominate the PHI (including when the PHI is
+ in the same block as the basis), we won't be able to create a PHI
+ using the basis here.  */
+  basic_block basis_bb = gimple_bb (basis->cand_stmt);
+  basic_block phi_bb = gimple_bb (phi);
+
+  if (phi_bb == basis_bb
+  || !dominated_by_p (CDI_DOMINATORS, phi_bb, basis_bb))
+return false;
+
   for (i = 0; i < gimple_phi_num_args (phi); i++)
 {
+  /* If the PHI arg resides in a block not dominated by the basis,
+we won't be able to create a PHI using the basis here.  */
+  basic_block pred_bb = gimple_phi_arg_edge (phi, i)->src;
+
+  if (!dominated_by_p (CDI_DOMINATORS, pred_bb, basis_bb))
+   return false;
+
   tree arg = gimple_phi_arg_def (phi, i);
 
   if (!operand_equal_p (arg, phi_cand->base_expr, 0))
@@ -3298,7 +3315,7 @@ static bool
 
  if (gimple_code (arg_def) == GIMPLE_PHI)
{
- if (!all_phi_incrs_profitable (c, arg_def))
+ if (!all_phi_incrs_profitable (c, as_a  (arg_def)))
return false;
}
  else
@@ -3565,7 +3582,7 @@ replace_profitable_candidates (slsr_cand_t c)
{
  if (phi_dependent_cand_p (c))
{
- gimple *phi = lookup_cand (c->def_phi)->cand_stmt;
+ gphi *phi = as_a  (lookup_cand (c->def_phi)->cand_stmt);
 
  if (all_phi_incrs_profitable (c, phi))
{
Index: gcc/testsuite/g++.dg/torture/pr80054.C
===
--- gcc/testsuite/g++.dg/torture/pr80054.C  (nonexistent)
+++ gcc/testsuite/g++.dg/torture/pr80054.C  (working copy)
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+
+/* Used to fail in SLSR because of a dominance violation.  PR80054.  */
+
+extern short var_2;
+extern short var_4;
+extern const bool var_32;
+extern short var_36;
+extern const bool var_37;
+extern bool var_46;
+extern unsigned int var_47;
+extern short var_49;
+extern unsigned int var_56;
+extern unsigned int var_62;
+extern unsigned int var_65;
+extern bool var_831;
+extern unsigned int var_843;
+extern short var_846;
+extern short var_889;
+
+void foo() {
+  if (var_36 * var_37)
+var_831 = var_56 = 0;
+  else
+var_65 = 0;
+
+  if (var_46)
+var_843 = 0;
+
+  var_846 = 0;
+
+  if ((var_4 == 0) >> (var_32 | -(var_37 < var_46 || var_36)) + 8)
+var_49 = 2032651381 * bool(var_2 * var_37);
+  else {
+var_62 = 0;
+var_47 = (var_46 || var_36) * (var_2 * var_37);
+  }
+
+  var_889 = bool(var_2 * var_37);
+}

[PATCH,rs6000] Add documentation to describe implicit handling of command-line target options

[Kelvin Nilsen]

This patch adds comments to clarify the automatic setting and clearing
of target attribute flags in order to assure consistency between
configuration settings and between multiple interrelated compilation
target options.  Particular attention is given to the target options
that affect the C preprocessor macros that are automatically defined to
denote support is enabled for particular target options.

This patch consists entirely of new comments.  Nevertheless, it has
been bootstrapped on powerpc64le-unknown-linux with no regressions.

Is this ok for trunk?

gcc/ChangeLog:

2017-03-17  Kelvin Nilsen  

* config/rs6000/rs6000-c.c (rs6000_target_modify_macros): Add
comments.
* config/rs6000/rs6000.c (rs6000_option_override_internal): Add
comments.

Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 246086)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -343,6 +343,71 @@ rs6000_target_modify_macros (bool define_p, HOST_W
 (define_p) ? "define" : "undef",
 flags, bu_mask);
 
+  /* Each of the flags mentioned below controls whether certain
+ preprocessor macros will be automatically defined when
+ preprocessing source files for compilation by this compiler.
+ While most of these flags can be enabled or disabled
+ explicitly by specifying certain command-line options when
+ invoking the compiler, there are also many ways in which these
+ flags are enabled or disabled implicitly, based on compiler
+ defaults, configuration choices, and on the presence of certain
+ related command-line options.  Many, but not all, of these
+ implicit behaviors can be found in file "rs6000.c", the
+ rs6000_option_override_internal() function.
+
+ In general, each of the flags may be automatically enabled in
+ any of the following conditions:
+
+ 1. If no -mcpu target is specified on the command line and no
+   --with-cpu target is specified to the configure command line
+   and the TARGET_DEFAULT macro for this default cpu host
+   includes the flag, and the flag has not been explicitly disabled
+   by command-line options.
+
+ 2. If the target specified with -mcpu=target on the command line, or
+   in the absence of a -mcpu=target command-line option, if the
+   target specified using --with-cpu=target on the configure
+   command line, is disqualified because the associated binary
+   tools (e.g. the assembler) lack support for the requested cpu,
+   and the TARGET_DEFAULT macro for this default cpu host
+   includes the flag, and the flag has not been explicitly disabled
+   by command-line options.
+
+ 3. If either of the above two conditions apply except that the
+   TARGET_DEFAULT macro is defined to equal zero, and
+   TARGET_POWERPC64 and
+   a) BYTES_BIG_ENDIAN and the flag to be enabled is either
+  MASK_PPC_GVXOPT or MASK_POWERPC64 (flags for "powerpc64"
+  target), or
+   b) !BYTES_BIG_ENDIAN and the flag to be enabled is either
+  MASK_POWERPC64 or it is one of the flags included in
+  ISA_2_7_MASKS_SERVER (flags for "powerpc64le" target).
+
+ 4. If a cpu has been requested with a -mcpu=target command-line option
+   and this cpu has not been disqualified due to shortcomings of the
+   binary tools, and the set of flags associated with the requested cpu
+   include the flag to be enabled.  See rs6000-cpus.def for macro
+   definitions that represent various ABI standards
+   (e.g. ISA_2_1_MASKS, ISA_3_0_MASKS_SERVER) and for a list of
+   the specific flags that are associated with each of the cpu
+   choices that can be specified as the target of a -mcpu=target
+   compile option, or as the the target of a --with-cpu=target
+   configure option.  Target flags that are specified in either
+   of these two ways are considered "implicit" since the flags
+   are not mentioned specifically by name.
+
+   Additional documentation describing behavior specific to
+   particular flags is provided below, immediately preceding the
+   use of each relevant flag.
+
+ 5. If there is no -mcpu=target command-line option, and the cpu
+   requested by a --with-cpu=target command-line option has not
+   been disqualified due to shortcomings of the binary tools, and
+   the set of flags associated with the specified target include
+   the flag to be enabled.  See the notes immediately above for a
+   summary of the flags associated with particular cpu
+   definitions.  */
+
   /* rs6000_isa_flags based options.  */
   rs6000_define_or_undefine_macro (define_p, "_ARCH_PPC");
   if ((flags & OPTION_MASK_PPC_GPOPT) != 0)
@@ -363,6 +428,12 @@ rs6000_target_modify_macros (bool define_p, HOST_W
 rs6000_define_or_undefine_macro (define_p,

C++ PATCH for c++/80073 (C++17 ICE with virtual base)

[Jason Merrill]

The code for setting CLASSTYPE_NON_AGGREGATE failed to consider
indirect virtual bases.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 18b4a698996d151d87e5db2084859c83facd798f
Author: Jason Merrill 
Date:   Fri Mar 17 13:14:42 2017 -0400

PR c++/80073 - C++17 ICE with virtual base.

* decl.c (xref_basetypes): Also check for indirect vbases.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 61ecf81..bf24e8b 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13812,6 +13812,9 @@ xref_basetypes (tree ref, tree base_list)
 
   if (max_vbases)
 {
+  /* An aggregate can't have virtual base classes.  */
+  CLASSTYPE_NON_AGGREGATE (ref) = true;
+
   vec_alloc (CLASSTYPE_VBASECLASSES (ref), max_vbases);
 
   if (max_dvbases)
diff --git a/gcc/testsuite/g++.dg/init/vbase2.C 
b/gcc/testsuite/g++.dg/init/vbase2.C
new file mode 100644
index 000..1711ea9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/vbase2.C
@@ -0,0 +1,8 @@
+// PR c++/80073
+// { dg-do compile { target c++11 } }
+
+struct A {};
+
+struct B : virtual A {};
+
+struct C : B {} c {};

Re: [PATCH] RISC-V documentation cleanups

[Palmer Dabbelt]

On Tue, 14 Mar 2017 13:29:22 PDT (-0700), ger...@pfeifer.com wrote:
> On Mon, 13 Mar 2017, Palmer Dabbelt wrote:
>> A recent mailing list post about install.texi cleanup suggested I
>> take a look at ours, and there were a few problems:
>>
>>  * No table of contents entries
>>  * Not alphabetically ordered
>>  * Missing a note about requiring binutils-2.28
>
> This looks fine with a few changes, thank you.
>
> Perhaps a bit repetitive?  I'm wondering, would it make sense to
> have general riscv64-*-* entries that cover the general items?

It is a bit repetitive, but I copied the format from someone else.  I'd be OK
just having a single riscv* entry, as it's really one ISA: it's all described
in one manual and was all designed, implemented, and released at the same time;
and while there are riscv32/riscv64 as variants, there's actually more
(with/without floating point, for example).

>> 2017-03-13  Palmer Dabbelt  >
>> * doc/install.texi (Specific) : Add table of contents
>> link.
>> : Likewise.
>> : Likewise
>> : Likewise.
>
> Here I'd just say
>
>   * doc/install.texi (Specific): Add foo, bar, didl, doo to the
>   table of contents.
>
>> : Add a note about requiring binutils-2.28.
>
> Here and in the text, binutils 2.28 (without the dash).
>
> Approved.

Committed.

> PS: We usually send patches to the gcc-patches@ list.

Sorry, I must have just mis-typed.

C++ PATCH for c++/78345, ICE with lambda as array initializer

[Jason Merrill]

build_aggr_init was blithely passing most any initializer on to
build_vec_init, which saw a CONSTRUCTOR and assumed that it would be
of the right type, leading to an ICE.  Fixed by not taking the
INIT_EXPR shortcut if the CONSTRUCTOR has the wrong type.

That returned us to the GCC 6 state, where we would sometimes silently
accept a single expression used as an array initializer.  I think
that's a remnant of an extension that predates my work on the
compiler, and several places already reject that pattern, so let's
reject it here with a permerror.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 5929f60b57733160099e1d34dde93d01ae2e0319
Author: Jason Merrill 
Date:   Fri Mar 17 07:45:02 2017 -0400

PR c++/78345 - ICE initializing array from lambda.

* init.c (build_aggr_init): Check array initializer.
(build_vec_init): Check the type of a CONSTRUCTOR.

diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 8bfcbde..ebb1245 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1617,6 +1617,10 @@ build_aggr_init (tree exp, tree init, int flags, 
tsubst_flags_t complain)
   if (init == error_mark_node)
 return error_mark_node;
 
+  location_t init_loc = (init
+? EXPR_LOC_OR_LOC (init, input_location)
+: location_of (exp));
+
   TREE_READONLY (exp) = 0;
   TREE_THIS_VOLATILE (exp) = 0;
 
@@ -1656,6 +1660,16 @@ build_aggr_init (tree exp, tree init, int flags, 
tsubst_flags_t complain)
TREE_TYPE (init) = cv_unqualified (itype);
  from_array = (itype && same_type_p (TREE_TYPE (init),
  TREE_TYPE (exp)));
+
+ if (init && !from_array
+ && !BRACE_ENCLOSED_INITIALIZER_P (init))
+   {
+ if (complain & tf_error)
+   permerror (init_loc, "array must be initialized "
+  "with a brace-enclosed initializer");
+ else
+   return error_mark_node;
+   }
}
 
   stmt_expr = build_vec_init (exp, NULL_TREE, init,
@@ -3945,6 +3959,9 @@ build_vec_init (tree base, tree maxindex, tree init,
  ? vec_copy_assign_is_trivial (inner_elt_type, init)
  : !TYPE_NEEDS_CONSTRUCTING (type))
   && ((TREE_CODE (init) == CONSTRUCTOR
+  && (BRACE_ENCLOSED_INITIALIZER_P (init)
+  || (same_type_ignoring_top_level_qualifiers_p
+  (atype, TREE_TYPE (init
   /* Don't do this if the CONSTRUCTOR might contain something
  that might throw and require us to clean up.  */
   && (vec_safe_is_empty (CONSTRUCTOR_ELTS (init))
diff --git a/gcc/testsuite/g++.dg/init/array48.C 
b/gcc/testsuite/g++.dg/init/array48.C
new file mode 100644
index 000..27fec13
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/array48.C
@@ -0,0 +1,7 @@
+// PR c++/78345
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+  const int i;
+} a[1] = []{}; // { dg-error "array.*init" }
diff --git a/gcc/testsuite/g++.dg/torture/pr70499.C 
b/gcc/testsuite/g++.dg/torture/pr70499.C
index 954fea5..e08c26f 100644
--- a/gcc/testsuite/g++.dg/torture/pr70499.C
+++ b/gcc/testsuite/g++.dg/torture/pr70499.C
@@ -1,5 +1,5 @@
 // { dg-do compile }
-// { dg-additional-options "-w -Wno-psabi" }
+// { dg-additional-options "-w -fpermissive -Wno-psabi" }
 // { dg-additional-options "-mavx" { target x86_64-*-* i?86-*-* } }
 
 typedef double __m256d __attribute__ ((__vector_size__ (32), __may_alias__));

Re: [PATCH] Various fixes for facets

[Jonathan Wakely]

On 16/03/17 15:23 +, Jonathan Wakely wrote:

On 14/03/17 18:46 +, Jonathan Wakely wrote:

On 13/03/17 19:35 +, Jonathan Wakely wrote:

This is a series of patches to fix various bugs in the Unicode
character conversion facets.

Ther first patch fixes a silly < versus <= bug that meant that 0x
got written as a surrogate pair instead of as simply 0xff, and an
endianness bug for the internal representation of UTF-16 code units
stored in char32_t or wchar_t values. That's PR 79511.

The second patch fixes some incorrect bitwise operations (because I
confused & and |) and some incorrect limits (because I confused max
and min). That fixes determining the endianness of the external
representation bytes when they start with a Byte OrderMark, and
correctly reports errors on invalid UCS2. It also fixes
wstring_convert so that it reports the number of characters that were
converted prior to an error. That's PR 79980.

The third patch fixes the output of the encoding() and max_length()
member functions on the codecvt facets, because I wasn't correctly
accounting for a BOM or for the differences between UTF-16 and UCS2.

I plan to commit these for all branches, but I'll wait until after GCC
7.1 is released, and fix it for 7.2 instead. These bugs aren't
important enough to rush into trunk now.


One more patch for a problem found by the libc++ testsuite. Now we
pass all the libc++ tests, and we even pass a test that libc++ fails.
With this, I hope our  is 100% conforming. Just in time to be
deprecated for C++17 :-)


I've committed these to trunk, on the basis that they're intended to
be backported to all branches anyway (fixing features that are
currently broken in all branches). There's no point waiting if we plan
to commit them anyway, it would just mean doing an extra backport (5,
6, 7 *and* 8).

Backports will be done soon.


I backported all the recent  fixes to gcc-6-branch and it was
failing one test, due to unaligned reads in std::codecvt_utf16. That
type reads UTF-16 data from a const char* (Why narrow characters when
we have char16_t? Because  likes to be awkward) and I was
doing that by casting the const char* to const char16_t*. That isn't
safe when the first char isn't aligned correctly for a char16_t.

This patch fixes all the unaligned accesses by abstracting the
operations on the pointers to use new overlaoded operators on the
range type. A new partial specialization range
uses memcpy to read/write char16_t values from the char*, avoiding
alignment problems. The primary template (range) just
dereferences the pointers directly.

Tested x86_64-linux, powerpc64le-linux, powerpc64-linux,
powerpc-ibm-aix7.2.0.0 (which has 2-byte wchar_t).

Also tested with ubsan to confirm the unaligned accesses are gone.

Committed to trunk, gcc-6-branch, gcc-5-branch.

commit 96ebc791ce1bd9cbba913d0b25b60ee4a09c41f1
Author: Jonathan Wakely 
Date:   Fri Mar 17 13:00:00 2017 +

Fix alignment bugs in std::codecvt_utf16

	* src/c++11/codecvt.cc (range): Add non-type template parameter and
	define oerloaded operators for reading and writing code units.
	(range): Define partial specialization for accessing
	wide characters in potentially unaligned byte ranges.
	(ucs2_span(const char16_t*, const char16_t*, ...))
	(ucs4_span(const char16_t*, const char16_t*, ...)): Change parameters
	to range in order to avoid unaligned reads.
	(__codecvt_utf16_base::do_out)
	(__codecvt_utf16_base::do_out)
	(__codecvt_utf16_base::do_out): Use range specialization for
	unaligned data to avoid unaligned writes.
	(__codecvt_utf16_base::do_in)
	(__codecvt_utf16_base::do_in)
	(__codecvt_utf16_base::do_in): Likewise for writes. Return
	error if there are unprocessable trailing bytes.
	(__codecvt_utf16_base::do_length)
	(__codecvt_utf16_base::do_length)
	(__codecvt_utf16_base::do_length): Pass arguments of type
	range to span functions.
	* testsuite/22_locale/codecvt/codecvt_utf16/misaligned.cc: New test.

diff --git a/libstdc++-v3/src/c++11/codecvt.cc b/libstdc++-v3/src/c++11/codecvt.cc
index 02866ef..1187339 100644
--- a/libstdc++-v3/src/c++11/codecvt.cc
+++ b/libstdc++-v3/src/c++11/codecvt.cc
@@ -57,17 +57,104 @@ namespace
   const char32_t incomplete_mb_character = char32_t(-2);
   const char32_t invalid_mb_sequence = char32_t(-1);
 
-  template
+  // Utility type for reading and writing code units of type Elem from
+  // a range defined by a pair of pointers.
+  template
 struct range
 {
   Elem* next;
   Elem* end;
 
+  // Write a code unit.
+  range& operator=(Elem e)
+  {
+	*next++ = e;
+	return *this;
+  }
+
+  // Read the next code unit.
   Elem operator*() const { return *next; }
 
-  range& operator++() { ++next; return *this; }
+  // Read the Nth code unit.
+  Elem operator[](size_t n) const { return next[n]; }
 
+  // Move to the next code unit.
+  range& operator++()
+ 

Re: patch, libfortran] [patch, fortran] Fix PR 79956, part two, attempt 3

[Thomas Koenig]

Am 17.03.2017 um 17:48 schrieb Markus Trippelsdorf:

You should use rdim not sdim in the GFC_ASSERT.


Fixed in rev 246248.

Sorry for the breakage.

Thomas

Re: C PATCH to add C11 references

[Joseph Myers]

On Fri, 17 Mar 2017, Marek Polacek wrote:

> This means precisely zilch, but hey, it's Friday.  The references are mostly
> the same as C99, but they differ for e.g. declarators.  I wish C had what C++
> has, i.e. references like [stmt.switch].
> 
> Ok for trunk?

OK.

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

Re: [PATCH v2, doc] Revise GCC manual section 6.11, Additional Floating Types

[Joseph Myers]

On Fri, 17 Mar 2017, Bill Schmidt wrote:

> Joseph, any further comments, or may I commit this?

Is there a current patch version somewhere reflecting all comments so far?

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

Re: Combiner fix for PR79910

[Segher Boessenkool]

Thanks for not cc:ing me on any of this.

On Wed, Mar 15, 2017 at 04:00:21PM +0100, Bernd Schmidt wrote:
> +/* Set up a set of registers used in an insn.  Called through note_uses,
> +   arguments as described for that function.  */
> +
> +static void
> +record_used_regs (rtx *xptr, void *data)
> +{
> +  bitmap set = (bitmap)data;

Space after cast, throughout.

> +  int i, j;
> +  enum rtx_code code;
> +  const char *fmt;
> +  rtx x = *xptr;
> +
> +  /* repeat is used to turn tail-recursion into iteration since GCC
> + can't do it when there's no return value.  */
> + repeat:

This comment is incorrect.

> +   /* If we are about to do the last recursive call
> +  needed at this level, change it into iteration.
> +  This function is called enough to be worth it.  */

This function is called enough that it should not be called at all.

> +  /* For combinations that may result in two insns, we have to gather
> + some extra information about registers used, so that we can
> + update all relevant LOG_LINKS later.  */

Please just refuse to do the combination in such cases instead.

> + /* See comments above where we calculate the bitmap.  */
> + EXECUTE_IF_SET_IN_BITMAP ((bitmap)new_regs_in_i2,
> +   LAST_VIRTUAL_REGISTER, i, iter)

Why do you need a cast here at all?

> +   {
> + rtx reg = regno_reg_rtx[i];
> + rtx_insn *other;
> + for (other = NEXT_INSN (i2); other != i3; other = NEXT_INSN (other))
> +   if (NONDEBUG_INSN_P (other)
> +   && (reg_overlap_mentioned_p (reg, PATTERN (other))
> +   || (CALL_P (other) && find_reg_fusage (other, USE, reg
> + {
> +   if (dump_file)
> + fprintf (dump_file,
> +  "found extra use of reg %d at insn %d\n", i,
> +  INSN_UID (other));
> +   insn_link **plink;
> +   for (plink = &LOG_LINKS (other);
> +*plink;
> +plink = &(*plink)->next)
> + {
> +   insn_link *link = *plink;
> +   if (link->regno == i)
> + {
> +   *plink = link->next;
> +   link->next = i3links;
> +   i3links = link;
> +   break;
> + }
> + }
> +   break;
> + }
> +   }

This should be a separate function.

So, no, I'm not okay with this.  It is very expensive, it is doing open
heart surgery on combine's internal structures (in a way that may or may
not work), and all that to combine some insns in a case that should not
exist anyway.


Segher

Re: Combiner fix for PR79910

[Segher Boessenkool]

On Wed, Mar 15, 2017 at 12:09:18AM +0100, Bernd Schmidt wrote:
> I suppose at the moment we don't do 2->2 combinations, so we could 
> conditionalize this on having an i1.

You suppose wrong.  If one of the resulting insns is set_noop_p then
2->2 is allowed, for example.  Also in the hopefully not so far future
we will allow 2->2 in general.


Segher

Re: Combiner fix for PR79910

[Jeff Law]

On 03/17/2017 04:16 PM, Segher Boessenkool wrote:

On Wed, Mar 15, 2017 at 12:09:18AM +0100, Bernd Schmidt wrote:

I suppose at the moment we don't do 2->2 combinations, so we could
conditionalize this on having an i1.


You suppose wrong.  If one of the resulting insns is set_noop_p then
2->2 is allowed, for example.  Also in the hopefully not so far future
we will allow 2->2 in general.

But in a 2->2 where one is a nop we're not going to run into problems.

jeff

Re: Combiner fix for PR79910

[Jeff Law]

On 03/17/2017 04:14 PM, Segher Boessenkool wrote:

Thanks for not cc:ing me on any of this.
There's really no need for getting upset.  Bernd posted the message to 
the patches list.  That's sufficient.




On Wed, Mar 15, 2017 at 04:00:21PM +0100, Bernd Schmidt wrote:

+/* Set up a set of registers used in an insn.  Called through note_uses,
+   arguments as described for that function.  */
+
+static void
+record_used_regs (rtx *xptr, void *data)
+{
+  bitmap set = (bitmap)data;


Space after cast, throughout.

A nit.  Bernd, can you please fix this.




+  int i, j;
+  enum rtx_code code;
+  const char *fmt;
+  rtx x = *xptr;
+
+  /* repeat is used to turn tail-recursion into iteration since GCC
+ can't do it when there's no return value.  */
+ repeat:


This comment is incorrect.
Depends on the situation.  GCC's ability to turn tail recursion into 
iteration is not good.





+ /* If we are about to do the last recursive call
+needed at this level, change it into iteration.
+This function is called enough to be worth it.  */


This function is called enough that it should not be called at all.
I disagree.  We have a code gen bug. We need a solution.  Feel free to 
come up with something better.





+  /* For combinations that may result in two insns, we have to gather
+ some extra information about registers used, so that we can
+ update all relevant LOG_LINKS later.  */


Please just refuse to do the combination in such cases instead.

?!? That would essentially mean we can't do 3->2 combinations.




+   /* See comments above where we calculate the bitmap.  */
+   EXECUTE_IF_SET_IN_BITMAP ((bitmap)new_regs_in_i2,
+ LAST_VIRTUAL_REGISTER, i, iter)


Why do you need a cast here at all?


+ {
+   rtx reg = regno_reg_rtx[i];
+   rtx_insn *other;
+   for (other = NEXT_INSN (i2); other != i3; other = NEXT_INSN (other))
+ if (NONDEBUG_INSN_P (other)
+ && (reg_overlap_mentioned_p (reg, PATTERN (other))
+ || (CALL_P (other) && find_reg_fusage (other, USE, reg
+   {
+ if (dump_file)
+   fprintf (dump_file,
+"found extra use of reg %d at insn %d\n", i,
+INSN_UID (other));
+ insn_link **plink;
+ for (plink = &LOG_LINKS (other);
+  *plink;
+  plink = &(*plink)->next)
+   {
+ insn_link *link = *plink;
+ if (link->regno == i)
+   {
+ *plink = link->next;
+ link->next = i3links;
+ i3links = link;
+ break;
+   }
+   }
+ break;
+   }
+ }


This should be a separate function.

Bernd, can you pull this out into its own function.



So, no, I'm not okay with this.  It is very expensive, it is doing open
heart surgery on combine's internal structures (in a way that may or may
not work), and all that to combine some insns in a case that should not
exist anyway.
Please don't be dramatic.  If you've got a better suggestion for how to 
fix this, be my guest.  I don't like the compile-time cost either, but I 
don't see a better solution.


Jeff

[PATCH] Disable test inapplicable to RISC-V

[Palmer Dabbelt]

From: Andrew Waterman 

The test is coupled to the branch cost model.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/ssa-thread-14.c: Adjust target selector.
---
 gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
index aa1323a..f12fb07 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
@@ -1,4 +1,4 @@
-/* { dg-do compile { target { ! { logical_op_short_circuit || { m68k*-*-* 
mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* 
mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-* } } } } }  */
+/* { dg-do compile { target { ! { logical_op_short_circuit || { m68k*-*-* 
mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* 
mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-* riscv*-*-* } } } } }  
*/
 /* { dg-additional-options "-O2 -fdump-tree-vrp-details" }  */
 /* { dg-additional-options "-mbranch-cost=2" { target i?86-*-* x86_64-*-* } }  
*/
 /* { dg-final { scan-tree-dump-times "Threaded jump" 8 "vrp1" } }  */
-- 
2.10.2

[PATCH] Use more conservative fences on RISC-V

[Palmer Dabbelt]

The RISC-V memory model is still in the process of being formally
specified, so for now we're going to be safe and add the I/O bits to
userspace fences because there's no way to know if userspace is touching
memory-mapped I/O regions at compile time.

This will have no impact on existing microarchitecutres because they
treat all fences conservatively.

gcc/ChangeLog:

2017-03-17  Palmer Dabbelt  

* config/riscv/riscv.c (riscv_print_operand): Use "fence
iorw,ow".
* config/riscv/sync.mc (mem_thread_fence_1): Use "fence
iorw,iorw".
---
 gcc/ChangeLog| 7 +++
 gcc/config/riscv/riscv.c | 2 +-
 gcc/config/riscv/sync.md | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3e108dd..de32689 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2017-03-17  Palmer Dabbelt  
+
+   * config/riscv/riscv.c (riscv_print_operand): Use "fence
+   iorw,ow".
+   * config/riscv/sync.mc (mem_thread_fence_1): Use "fence
+   iorw,iorw".
+
 2017-03-17  Palmer Dabbelt  : Add riscv32-*-elf,
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 25cc803..fa93c3c 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -2794,7 +2794,7 @@ riscv_print_operand (FILE *file, rtx op, int letter)
 
 case 'F':
   if (riscv_memmodel_needs_release_fence ((enum memmodel) INTVAL (op)))
-   fputs ("fence rw,w; ", file);
+   fputs ("fence iorw,ow; ", file);
   break;
 
 default:
diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 09970b9..cde19e3 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -53,7 +53,7 @@
(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BARRIER))
(match_operand:SI 1 "const_int_operand" "")] ;; model
   ""
-  "fence\trw,rw")
+  "fence\tiorw,iorw")
 
 ;; Atomic memory operations.
 
-- 
2.10.2

[PATCH] RISC-V: Don't prefer FP_REGS for integers

[Palmer Dabbelt]

On RISC-V we can't store integers in floating-point registers as this is
forbidden by the ISA.  We've always disallowed this, but we were
setting the preferred mode to FP_REGS for some integer modes.  This
caused the LRA to blow up with some hard to read error messages.

This patch removes the prefered mode hook, as the right thing to do here
is nothing.

Thanks to Kito for finding the bug, and mpf for the fix.  See also
.

PR target/79912
---
 gcc/ChangeLog|  6 ++
 gcc/config/riscv/riscv.c | 13 -
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index de32689..5e90179 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,11 @@
 2017-03-17  Palmer Dabbelt  
 
+   PR target/79912
+   * config/riscv/riscv.c (riscv_preferred_reload_class): Remove.
+   (TARGET_PREFERRED_RELOAD_CLASS): Likewise.
+
+2017-03-17  Palmer Dabbelt  
+
* config/riscv/riscv.c (riscv_print_operand): Use "fence
iorw,ow".
* config/riscv/sync.mc (mem_thread_fence_1): Use "fence
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index fa93c3c..d5928c3 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -3629,16 +3629,6 @@ riscv_class_max_nregs (reg_class_t rclass, enum 
machine_mode mode)
   return 0;
 }
 
-/* Implement TARGET_PREFERRED_RELOAD_CLASS.  */
-
-static reg_class_t
-riscv_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass)
-{
-  return reg_class_subset_p (FP_REGS, rclass) ? FP_REGS :
-reg_class_subset_p (GR_REGS, rclass) ? GR_REGS :
-rclass;
-}
-
 /* Implement TARGET_MEMORY_MOVE_COST.  */
 
 static int
@@ -4031,9 +4021,6 @@ riscv_cannot_copy_insn_p (rtx_insn *insn)
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST riscv_address_cost
 
-#undef  TARGET_PREFERRED_RELOAD_CLASS
-#define TARGET_PREFERRED_RELOAD_CLASS riscv_preferred_reload_class
-
 #undef TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START riscv_file_start
 #undef TARGET_ASM_FILE_START_FILE_DIRECTIVE
-- 
2.10.2

Re: [PATCH,rs6000] Add documentation to describe implicit handling of command-line target options

[Segher Boessenkool]

Hi Kelvin,

On Fri, Mar 17, 2017 at 12:27:41PM -0600, Kelvin Nilsen wrote:
> This patch adds comments to clarify the automatic setting and clearing
> of target attribute flags in order to assure consistency between
> configuration settings and between multiple interrelated compilation
> target options.  Particular attention is given to the target options
> that affect the C preprocessor macros that are automatically defined to
> denote support is enabled for particular target options.

Thanks for doing this.

> + 3. If either of the above two conditions apply except that the
> + TARGET_DEFAULT macro is defined to equal zero, and
> + TARGET_POWERPC64 and
> + a) BYTES_BIG_ENDIAN and the flag to be enabled is either
> +MASK_PPC_GVXOPT or MASK_POWERPC64 (flags for "powerpc64"
> +target), or

GFXOPT?

> + 4. If a cpu has been requested with a -mcpu=target command-line option
> + and this cpu has not been disqualified due to shortcomings of the
> + binary tools,

[Not directly related to this patch of course.]  Is there any good reason
why we do not require a new enough binutils?  During development there are
issues, sure, but why should we for example not require a binutils that
supports POWER8, these days?

> + 2. If TARGET_P8_VECTOR is off.
> +  */

Comment closing does not go on a separate line.

>/* options from the builtin masks.  */
> +  /* Note that RS6000_BTM_SPE is enabled only if TARGET_SPE
> + (e.g. -mspe).  */
>if ((bu_mask & RS6000_BTM_SPE) != 0)
>  rs6000_define_or_undefine_macro (define_p, "__SPE__");
> +  /* Note that RS6000_BTM_PARIED is enabled only if
> + TARGET_PAIRED_FLOAT is enabled (e.g. -mpaired).  */
>if ((bu_mask & RS6000_BTM_PAIRED) != 0)
>  rs6000_define_or_undefine_macro (define_p, "__PAIRED__");
> +  /* Note that RS6000_BTM_CELL is enabled only if (rs6000_cpu ==
> + PROCESSOR_CELL) (e.g. -mcpu=cell).  */
>if ((bu_mask & RS6000_BTM_CELL) != 0)
>  rs6000_define_or_undefine_macro (define_p, "__PPU__");

I'm not sure these few are useful, they are pretty obvious?  But okay
(if you fix the typo, "PARIED").

Okay with those typoes fixed.

These comments describe what the existing code does.  Did you find anything
that seems wrong or questionable?


Segher

[PATCH] fix aarch64/combine bootstrap miscompare

[Jim Wilson]

We get different code for libcpp/line-map.o in stage2 and stage3,
because stage2 is compiled with -gtoggle and stage3 is not.  Bernd's
recent combine patch is being confused by debug insns.  This is fixed
by changing a prev_nonnote_insn call to a prev_nonnote_nondebug_insn
call.  There is also a redundant test for i1 that I've removed.

This was tested with an x86_64 bootstrap, aarch64 bootstrap, and
aarch64 make check.  I noticed 3 new failures as a result of the
combine patch that need to be looked at
FAIL: gcc.target/aarch64/tst_5.c scan-assembler tst\t(x|w)[0-9]+,[ \t]*255
FAIL: gcc.target/aarch64/tst_5.c scan-assembler tst\t(x|w)[0-9]+,[ \t]*65535
FAIL: gcc.target/aarch64/tst_6.c scan-assembler tst\t(x|w)[0-9]+,[ \t]*65535
The testcases are expecting a tst instruction to be generated, but we
get and/cmp instead.  I think this is because combinations with a hard
reg are now disabled in some cases.  I haven't had a chance to look at
this in detail yet.

The patch was preapproved by Jeff and has been checked in.

Jim
2017-03-17  Jim Wilson  

	* combine.c (try_combine): Delete redundant i1 test.  Call
	prev_nonnote_nondebug_insn instead of prev_nonnote_insn.

Index: gcc/combine.c
===
--- gcc/combine.c	(revision 246226)
+++ gcc/combine.c	(working copy)
@@ -2806,9 +2806,8 @@
 	bitmap_set_bit (links_regset, ll->regno);
   FOR_EACH_LOG_LINK (ll, i2)
 	bitmap_set_bit (links_regset, ll->regno);
-  if (i1)
-	FOR_EACH_LOG_LINK (ll, i1)
-	  bitmap_set_bit (links_regset, ll->regno);
+  FOR_EACH_LOG_LINK (ll, i1)
+	bitmap_set_bit (links_regset, ll->regno);
   if (i0)
 	FOR_EACH_LOG_LINK (ll, i0)
 	  bitmap_set_bit (links_regset, ll->regno);
@@ -4142,7 +4141,7 @@
 	   multi-word registers.  Later, when fixing up LOG_LINKS, we
 	   deal with the case where a pseudo use moved.  */
 	if (!bitmap_empty_p (new_regs_in_i2)
-	&& prev_nonnote_insn (i3) != i2
+	&& prev_nonnote_nondebug_insn (i3) != i2
 	&& bitmap_first_set_bit (new_regs_in_i2) < FIRST_PSEUDO_REGISTER)
 	  {
 	undo_all ();