Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Jakub Jelinek]

On Tue, Jul 25, 2017 at 03:52:56PM -0500, Segher Boessenkool wrote:
> On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote:
> > This patch only adds new vector from vector extract and init patterns to
> > the i386 backend, but I had to change many other targets too, because
> > it needs to have the element mode in the vec_extract/vec_init expander
> > names.  Seems most of the backends didn't really have a mode attribute
> > usable for this or had it only in uppercase, while for the names we need
> > lowercase.  Some backends had a convention on how to name lower case
> > vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> > of affected backends to seek advice on what mode attributes they want to
> > use.
> 
> Would it be possible (and useful) to _also_ keep the old names?  Or do
> you expect all targets will want to support all combinations?

Richi's preference was to use only a single conversion optab instead of
old + new when we've discussed it on IRC.  Of course it would be far less
work for me to support say:
OPTAB_CD(vec_extract2_optab, "vec_extract$a$b")
OPTAB_CD(vec_init2_optab, "vec_init$a$b")
OPTAB_D (vec_extract_optab, "vec_extract$a")
OPTAB_D (vec_init_optab, "vec_init$a")
where the single mode vec_extract/vec_init would be
extraction/initialization from element mode and the two mode one would be
used for 2 vector modes.  If there is agreement on that, most of the
config/*/* changes could go away.

> > --- gcc/config/rs6000/vector.md.jj  2017-06-08 20:50:49.0 +0200
> > +++ gcc/config/rs6000/vector.md 2017-07-24 17:44:44.699580927 +0200
> > @@ -74,6 +74,16 @@ (define_mode_attr VEC_base [(V16QI "QI")
> > (V1TI  "TI")
> > (TI"TI")])
> >  
> > +;; As above, but in lower case
> > +(define_mode_attr VEC_base_l [(V16QI "qi")
> > + (V8HI  "hi")
> > + (V4SI  "si")
> > + (V2DI  "di")
> > + (V4SF  "sf")
> > + (V2DF  "df")
> > + (V1TI  "ti")
> > + (TI"ti")])
> > +
> >  ;; Same size integer type for floating point data
> >  (define_mode_attr VEC_int [(V4SF  "v4si")
> >(V2DF  "v2di")])
> 
> > @@ -520,6 +520,17 @@ (define_mode_attr VEL [(V8QI "QI") (V16Q
> > (SI   "SI") (HI   "HI")
> > (QI   "QI")])
> >  
> > +;; Define element mode for each vector mode (lower case).
> > +(define_mode_attr Vel [(V8QI "qi") (V16QI "qi")
> > +   (V4HI "hi") (V8HI "hi")
> > +   (V2SI "si") (V4SI "si")
> > +   (DI "di")   (V2DI "di")
> > +   (V4HF "hf") (V8HF "hf")
> > +   (V2SF "sf") (V4SF "sf")
> > +   (V2DF "df") (DF "df")
> > +   (SI   "si") (HI   "hi")
> > +   (QI   "qi")])
> 
> (Inconsistent spacing, please fix).

It is the same spacing as in VEL right above it, I've tried to follow
whatever weirdo formatting each backend had.

> ("vel" instead of "Vel" for this name?)

That is to follow aarch64 iterator naming convention, where they have
already preexisting e.g. VDBL for
;; Double modes of vector modes.
and Vdbl for:
;; Double modes of vector modes (lower case).
or similarly VHALF vs Vhalf.

> How is this different from VEC_base_l?  They can just be merged it seems.

They can't be merged, each backend has its own iterators and iterator naming
conventions, we don't really have any gcc/iterators.md that would be used
for all backends.  VEC_base_l is just rs6000 (and powerpcspe), using
rs6000 iterator naming conventions, Vel is aarch64 using aarch64 naming
conventions, V_elem_l is arm using arm iterator naming conventions etc.

> (And for that matter, VEC_base and VEL as well).  We'll handle that I
> suppose, don't let it hold up this patch :-)

Jakub

RE: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Richard Biener]

On Tue, 25 Jul 2017, Matthew Fortune wrote:

> Jakub Jelinek  writes:
> > Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves
> > e.g. the code generation for slp-43.c and slp-45.c testcases.
> > make cc1 tested in cross-compilers to the remaining targets.
> 
> No objections for the MIPS part. I've pointed out this change to Sameera to
> see how/if it will affect her autovectorization branch and whether MIPS MSA
> should define more forms of vec_init/vec_expand in general.

Note the vectorizer will try both variants (punning via integer mode
and vector element) so the change is mostly to get around having very
large integer modes like OImode of AVX512 halves or even larger ones
for ARM SVE.

You should only need to have matching component mode vector part
inits/extracts (and all vector modes are power-of-two size).  Thus
for V8SI have V4SI, V2SI and SI components for init/extracts.
If those are not available the vectorizer will try to pun V8SI
to V2TI and to TImode init/extract or V4DI with DImode, etc.
(that's what the bulk conversion of targets besides x86 should
provide).

Richard.

Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Richard Biener]

On Wed, 26 Jul 2017, Jakub Jelinek wrote:

> On Tue, Jul 25, 2017 at 03:52:56PM -0500, Segher Boessenkool wrote:
> > On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote:
> > > This patch only adds new vector from vector extract and init patterns to
> > > the i386 backend, but I had to change many other targets too, because
> > > it needs to have the element mode in the vec_extract/vec_init expander
> > > names.  Seems most of the backends didn't really have a mode attribute
> > > usable for this or had it only in uppercase, while for the names we need
> > > lowercase.  Some backends had a convention on how to name lower case
> > > vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> > > of affected backends to seek advice on what mode attributes they want to
> > > use.
> > 
> > Would it be possible (and useful) to _also_ keep the old names?  Or do
> > you expect all targets will want to support all combinations?
> 
> Richi's preference was to use only a single conversion optab instead of
> old + new when we've discussed it on IRC.  Of course it would be far less

Yep.  To me there's no advantage to having two variants besides avoiding
the mechanical change to existing backends adding the matching
component mode (plus a convenient iterator for that -- or maybe
gen* support for "component of mode").

> work for me to support say:
> OPTAB_CD(vec_extract2_optab, "vec_extract$a$b")
> OPTAB_CD(vec_init2_optab, "vec_init$a$b")
> OPTAB_D (vec_extract_optab, "vec_extract$a")
> OPTAB_D (vec_init_optab, "vec_init$a")
> where the single mode vec_extract/vec_init would be
> extraction/initialization from element mode and the two mode one would be
> used for 2 vector modes.  If there is agreement on that, most of the
> config/*/* changes could go away.
> 
> > > --- gcc/config/rs6000/vector.md.jj2017-06-08 20:50:49.0 
> > > +0200
> > > +++ gcc/config/rs6000/vector.md   2017-07-24 17:44:44.699580927 +0200
> > > @@ -74,6 +74,16 @@ (define_mode_attr VEC_base [(V16QI "QI")
> > >   (V1TI  "TI")
> > >   (TI"TI")])
> > >  
> > > +;; As above, but in lower case
> > > +(define_mode_attr VEC_base_l [(V16QI "qi")
> > > +   (V8HI  "hi")
> > > +   (V4SI  "si")
> > > +   (V2DI  "di")
> > > +   (V4SF  "sf")
> > > +   (V2DF  "df")
> > > +   (V1TI  "ti")
> > > +   (TI"ti")])
> > > +
> > >  ;; Same size integer type for floating point data
> > >  (define_mode_attr VEC_int [(V4SF  "v4si")
> > >  (V2DF  "v2di")])
> > 
> > > @@ -520,6 +520,17 @@ (define_mode_attr VEL [(V8QI "QI") (V16Q
> > >   (SI   "SI") (HI   "HI")
> > >   (QI   "QI")])
> > >  
> > > +;; Define element mode for each vector mode (lower case).
> > > +(define_mode_attr Vel [(V8QI "qi") (V16QI "qi")
> > > + (V4HI "hi") (V8HI "hi")
> > > + (V2SI "si") (V4SI "si")
> > > + (DI "di")   (V2DI "di")
> > > + (V4HF "hf") (V8HF "hf")
> > > + (V2SF "sf") (V4SF "sf")
> > > + (V2DF "df") (DF "df")
> > > + (SI   "si") (HI   "hi")
> > > + (QI   "qi")])
> > 
> > (Inconsistent spacing, please fix).
> 
> It is the same spacing as in VEL right above it, I've tried to follow
> whatever weirdo formatting each backend had.
> 
> > ("vel" instead of "Vel" for this name?)
> 
> That is to follow aarch64 iterator naming convention, where they have
> already preexisting e.g. VDBL for
> ;; Double modes of vector modes.
> and Vdbl for:
> ;; Double modes of vector modes (lower case).
> or similarly VHALF vs Vhalf.
> 
> > How is this different from VEC_base_l?  They can just be merged it seems.
> 
> They can't be merged, each backend has its own iterators and iterator naming
> conventions, we don't really have any gcc/iterators.md that would be used
> for all backends.  VEC_base_l is just rs6000 (and powerpcspe), using
> rs6000 iterator naming conventions, Vel is aarch64 using aarch64 naming
> conventions, V_elem_l is arm using arm iterator naming conventions etc.
> 
> > (And for that matter, VEC_base and VEL as well).  We'll handle that I
> > suppose, don't let it hold up this patch :-)
> 
>   Jakub
> 
> 

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

Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Eric Botcazou]

> Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves
> e.g. the code generation for slp-43.c and slp-45.c testcases.
> make cc1 tested in cross-compilers to the remaining targets.

The SPARC bits are OK by me.

-- 
Eric Botcazou

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Richard Biener]

On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:
> On Tue, 25 Jul 2017, Richard Biener wrote:
>
>>> I think we need Richard to say what the intent is for the valueization
>>> function. It is used both to stop looking at defining stmt if the return
>>> is
>>> NULL, and to replace/optimize one SSA_NAME with another, but currently it
>>> seems hard to prevent looking at the defining statement without
>>> preventing
>>> from looking at the SSA_NAME at all.
>>
>>
>> Yeah, this semantic overloading is an issue.  For gimple_build we have
>> nothing
>> to "valueize" but we only use it to tell genmatch that it may not look at
>> the
>> SSA_NAME_DEF_STMT.
>>
>>> I guess we'll need a fix in genmatch...
>>
>>
>> I'll have a look tomorrow.
>
>
> My impression yesterday was that we could replace the current do_valueize
> wrapper by 2 wrappers (without touching the valueize callbacks):
> - may_check_def_stmt, which returns a bool corresponding to the current
> do_valueize != NULL_TREE
> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
> returns its argument unchanged.
>
> Not very confident about it though.

Note I've been there in the past (twice I think) but always ran into existing
latent issues.  So hopefully we've resolved those, I'm testing the following
simplified variant of what I had back in time.

It'll produce

  switch (TREE_CODE (op0))
{
case SSA_NAME:
  if (gimple *def_stmt = get_def (valueize, op0))
{
  if (gassign *def = dyn_cast  (def_stmt))
switch (gimple_assign_rhs_code (def))
  {
  case MINUS_EXPR:
{
  tree o20 = gimple_assign_rhs1 (def);
  o20 = do_valueize (valueize, o20);
  tree o21 = gimple_assign_rhs2 (def);
  o21 = do_valueize (valueize, o21);
  if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
types_match (op1, o21)))
{

which also indents less which is nice.

Bootstrap/regtest running on x86_64-unknown-linux-gnu.

Richard.

2017-07-26  Richard Biener  

* gimple-match-head.c (do_valueize): Return OP if valueize
returns NULL_TREE.
(get_def): New helper to get at the def stmt of a SSA name
if valueize allows.
* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
do_valueize to get at the def stmt.
(dt_operand::gen_gimple_expr): Simplify do_valueize calls.


> --
> Marc Glisse


p3
Description: Binary data

Re: [PATCH 1/2] [SPARC] Drop superfluous MASK_FPU enable

[Eric Botcazou]

> All TARGET_DEFAULT defines set MASK_FPU.  There is no need to set it in
> some CPU target flags enable.
> 
> gcc/
>   config/sparc/sparc.c (sparc_option_override): Remove MASK_FPU
>   from all CPU target flags enable members.

OK for mainline and 7 branch, thanks.

-- 
Eric Botcazou

Re: Fix thinko in gimple_assign_set_rhs_with_ops

[Richard Biener]

On Tue, Jul 25, 2017 at 3:59 PM, Eric Botcazou  wrote:
> The attached fix to the Ada front-end introduces a regression in the ACATS
> testsuite for cb4009a.  The backtrace is:
>
> #0  operation_could_trap_helper_p(tree_code, bool, bool, bool, bool,
> tree_node*, bool*) () at /home/eric/gnat/gnat-head/src/gcc/tree-eh.c:2439
> #1  0x012946d7 in stmt_could_throw_1_p (stmt=)
> at /home/eric/gnat/gnat-head/src/gcc/tree-eh.c:2759
> #2  stmt_could_throw_p(gimple*) [clone .part.186] ()
> at /home/eric/gnat/gnat-head/src/gcc/tree-eh.c:2809
> #3  0x01295f28 in stmt_could_throw_p (stmt=0x76c5d420)
> at /home/eric/gnat/gnat-head/src/gcc/tree-eh.c:2924
> #4  maybe_clean_or_replace_eh_stmt (old_stmt=old_stmt@entry=0x76c565d8,
> new_stmt=new_stmt@entry=0x76c5d420)
> at /home/eric/gnat/gnat-head/src/gcc/tree-eh.c:2908
> #5  0x00fd8a0b in gsi_replace(gimple_stmt_iterator*, gimple*, bool) ()
> at /home/eric/gnat/gnat-head/src/gcc/gimple-iterator.c:447
> #6  0x00fd14af in
> gimple_assign_set_rhs_with_ops(gimple_stmt_iterator*, tree_code, tree_node*,
> tree_node*, tree_node*) ()
> at /home/eric/gnat/gnat-head/src/gcc/gimple.c:1616
> #7  0x00fe7af7 in replace_stmt_with_simplification (inplace=false,
> seq=0x7fffd818, ops=0x7fffd820, rcode=..., gsi=0x7fffd8e0)
> at /home/eric/gnat/gnat-head/src/gcc/gimple-fold.c:4151
> #8  fold_stmt_1(gimple_stmt_iterator*, bool, tree_node* (*)(tree_node*)) ()
> at /home/eric/gnat/gnat-head/src/gcc/gimple-fold.c:4462
> #9  0x00fe961a in fold_stmt (gsi=gsi@entry=0x7fffd8e0,
> valueize=valueize@entry=0x1331170 )
> at /home/eric/gnat/gnat-head/src/gcc/gimple-fold.c:4689
>
> The folding is turning a TRUNC_DIV_EXPR into a COND_EXPR and the code does:
>
>   /* If the new CODE needs more operands, allocate a new statement.  */
>   if (gimple_num_ops (stmt) < new_rhs_ops + 1)
> {
>   tree lhs = gimple_assign_lhs (stmt);
>   gimple *new_stmt = gimple_alloc (gimple_code (stmt), new_rhs_ops + 1);
>   memcpy (new_stmt, stmt, gimple_size (gimple_code (stmt)));
>   gimple_init_singleton (new_stmt);
>   gsi_replace (gsi, new_stmt, true);
>   stmt = new_stmt;
>
>   /* The LHS needs to be reset as this also changes the SSA name
>  on the LHS.  */
>   gimple_assign_set_lhs (stmt, lhs);
> }
>
> i.e it asks gsi_replace to update EH info, which doesn't work since the new
> statement is dummy at this point.  Fixed by passing false instead of true.
>
> Bootstrapped/regtested on x86_64-suse-linux, applied on mainline as obvious.

I think we should get rid of that update-EH-info flag, always assuming
false.  This
is IMHO a too low-level interface to do this (and we lack a
gsi_replace_without_update)

Not sure if you're willing to do this kind of cleanup ;)

Richard.

>
> 2017-07-25  Eric Botcazou  
>
> * gimple.c (gimple_assign_set_rhs_with_ops): Do not ask gsi_replace
> to update EH info here.
>
>
> 2017-07-25  Javier Miranda  
>
> ada/
> * checks.adb (Apply_Divide_Checks): Ensure that operands are not
> evaluated twice.
>
>
> 2017-07-25  Eric Botcazou  
>
> testsuite/
> * gnat.dg/opt66.adb: New test.
>
>
> --
> Eric Botcazou

[PATCH,AIX] Fully enable XCOFF in libbacktrace on AIX

[REIX, Tony]

Description:
 * This patch fully enables XCOFF in libbacktrace on AIX.

Tests:
 * Fedora25/x86_64 + GCC v7.1.0 : Configure/Build: SUCCESS
   - build made by means of gmake.

ChangeLog:
  * configure.ac, filetype.awk: Separate AIX XCOFF32 and XCOFF64.
  * xcoff.c: Add support for AIX XCOFF32 and XCOFF64 formats.
  * configure, config.h.in: Regenerate.

Cordialement,

Tony Reix

Bull - ATOS
IBM Coop Architect & Technical Leader
Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.net









Index: libbacktrace/ChangeLog
===
--- libbacktrace/ChangeLog	(revision 250514)
+++ libbacktrace/ChangeLog	(working copy)
@@ -1,3 +1,9 @@
+2017-07-25  Tony Reix  
+
+	* configure.ac, filetype.awk: Separate AIX XCOFF32 and XCOFF64.
+	* xcoff.c: Add support for AIX XCOFF32 and XCOFF64 formats.
+	* configure, config.h.in: Regenerate.
+
 2017-07-21  Tony Reix  
 
 	* filetype.awk: Add AIX XCOFF type detection.
Index: libbacktrace/config.h.in
===
--- libbacktrace/config.h.in	(revision 250514)
+++ libbacktrace/config.h.in	(working copy)
@@ -3,6 +3,9 @@
 /* ELF size: 32 or 64 */
 #undef BACKTRACE_ELF_SIZE
 
+/* XCOFF size: 32 or 64 */
+#undef BACKTRACE_XCOFF_SIZE
+
 /* Define to 1 if you have the __atomic functions */
 #undef HAVE_ATOMIC_FUNCTIONS
 
Index: libbacktrace/configure
===
--- libbacktrace/configure	(revision 250514)
+++ libbacktrace/configure	(working copy)
@@ -12048,9 +12048,9 @@ elf*) FORMAT_FILE="elf.lo" ;;
 pecoff) FORMAT_FILE="pecoff.lo"
 backtrace_supports_data=no
 	;;
-xcoff) FORMAT_FILE="xcoff.lo"
-   backtrace_supports_data=no
-   ;;
+xcoff*) FORMAT_FILE="xcoff.lo"
+backtrace_supports_data=no
+;;
 *) { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: could not determine output file type" >&5
 $as_echo "$as_me: WARNING: could not determine output file type" >&2;}
FORMAT_FILE="unknown.lo"
@@ -12072,6 +12072,19 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
+# XCOFF defines.
+xcoffsize=
+case "$libbacktrace_cv_sys_filetype" in
+xcoff32) xcoffsize=32 ;;
+xcoff64) xcoffsize=64 ;;
+*)   xcoffsize=unused
+esac
+
+cat >>confdefs.h <<_ACEOF
+#define BACKTRACE_XCOFF_SIZE $xcoffsize
+_ACEOF
+
+
 BACKTRACE_SUPPORTED=0
 if test "$backtrace_supported" = "yes"; then
   BACKTRACE_SUPPORTED=1
Index: libbacktrace/configure.ac
===
--- libbacktrace/configure.ac	(revision 250514)
+++ libbacktrace/configure.ac	(working copy)
@@ -233,9 +233,9 @@ elf*) FORMAT_FILE="elf.lo" ;;
 pecoff) FORMAT_FILE="pecoff.lo"
 backtrace_supports_data=no
 	;;
-xcoff) FORMAT_FILE="xcoff.lo"
-   backtrace_supports_data=no
-   ;;
+xcoff*) FORMAT_FILE="xcoff.lo"
+backtrace_supports_data=no
+;;
 *) AC_MSG_WARN([could not determine output file type])
FORMAT_FILE="unknown.lo"
backtrace_supported=no
@@ -252,6 +252,15 @@ elf64) elfsize=64 ;;
 esac
 AC_DEFINE_UNQUOTED([BACKTRACE_ELF_SIZE], [$elfsize], [ELF size: 32 or 64])
 
+# XCOFF defines.
+xcoffsize=
+case "$libbacktrace_cv_sys_filetype" in
+xcoff32) xcoffsize=32 ;;
+xcoff64) xcoffsize=64 ;;
+*)   xcoffsize=unused
+esac
+AC_DEFINE_UNQUOTED([BACKTRACE_XCOFF_SIZE], [$xcoffsize], [XCOFF size: 32 or 64])
+
 BACKTRACE_SUPPORTED=0
 if test "$backtrace_supported" = "yes"; then
   BACKTRACE_SUPPORTED=1
Index: libbacktrace/filetype.awk
===
--- libbacktrace/filetype.awk	(revision 250514)
+++ libbacktrace/filetype.awk	(working copy)
@@ -3,6 +3,6 @@
 /\177ELF\002/ { if (NR == 1) { print "elf64"; exit } }
 /\114\001/{ if (NR == 1) { print "pecoff"; exit } }
 /\144\206/{ if (NR == 1) { print "pecoff"; exit } }
-/\001\337/{ if (NR == 1) { print "xcoff"; exit } }
-/\001\367/{ if (NR == 1) { print "xcoff"; exit } }
+/\001\337/{ if (NR == 1) { print "xcoff32"; exit } }
+/\001\367/{ if (NR == 1) { print "xcoff64"; exit } }
 
Index: libbacktrace/xcoff.c
===
--- libbacktrace/xcoff.c	(revision 250514)
+++ libbacktrace/xcoff.c	(working copy)
@@ -1,5 +1,6 @@
-/* xcoff.c -- Get debug data from a XCOFFF file for backtraces.
-   Copyright (C) 2017 Free Software Foundation, Inc.
+/* xcoff.c -- Get debug data from an XCOFF file for backtraces.
+   Copyright (C) 2012-2017 Free Software Foundation, Inc.
+   Adapted from elf.c.
 
 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions are
@@ -31,46 +32,1460 @@ POSSIBILITY OF SUCH DAMAGE.  */
 
 #include "config.h"
 
+#include 
+#include 
 #include 
+#ifdef _AIX
+/* AIX loadquery.  */
+#include 
+#include 
+#endif
 
 #include "backtrace.h"
 #include "internal.h"
 
-/* A trivial routine that always fail

Re: [patch] Ad PR81487: More asprintf -> xasprintf replacements

[Richard Biener]

On Tue, Jul 25, 2017 at 10:01 PM, Georg-Johann Lay  wrote:
> Richard Biener schrieb:
>>
>> On Mon, Jul 24, 2017 at 10:19 AM, Georg-Johann Lay  wrote:
>>>
>>> Hi, as proposed in
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01294.html
>>>
>>> this patch does more asprintf -> xasprintf replacements.
>>>
>>> Bootstrapped + reg-tested on x86_64-linux.
>>>
>>> Ok for trunk?
>>
>>
>> Ok.
>>
>> Thanks,
>> Richard.
>
>
> Is this something we want to backport to v7 ?

Yes.

Richard.

>
>>> Johann
>>>
>>> gcc/
>>> PR 81487
>>> * hsa-brig.c (brig_init): Use xasprintf instead of asprintf.
>>> * gimple-pretty-print.c (dump_profile, dump_probability): Same.
>>> * tree-ssa-structalias.c (alias_get_name): Same.
>>>
>

Re: [PATCH][AArch64] Add addr_type attribute

[Hurugalawadi, Naveen]

Hi James,

Thanks for the review and comments on the patch.

>> What am I missing - you add a new function which is never called?
>> Should this have been in series with a scheduling model change?

Sorry. You are right. This patch is one in series for scheduling and
addition of attributes to improve the performance. 
The function is part of the other patch which will be posted after testing.

>> Note you need to include the POST ones for AARCH64 but
>> it should be similar enough.

Modified the patch as per your suggestion as in PowerPC.

Please review the patch and let me know your comments on it.
Bootstrapped and Regression tested on aarch64-thunder-linux.

Thanks,
Naveendiff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index f876a2b..0fb62fc 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -212,6 +212,30 @@
 ;; no predicated insns.
 (define_attr "predicated" "yes,no" (const_string "no"))
 
+;; Does this instruction use indexed (that is, reg+reg) addressing?
+;; This is used for load and store insns.  If operand 0 or 1 is a MEM
+;; it is automatically set based on that.  If a load or store instruction
+;; has fewer than two operands it needs to set this attribute manually
+;; or the compiler will crash.
+(define_attr "index" "no,yes"
+  (if_then_else (ior (match_operand 0 "index_address_mem")
+ (match_operand 1 "index_address_mem"))
+(const_string "yes")
+(const_string "no")))
+
+;; Does this instruction use update addressing?
+;; This is used for load and store insns.  See the comments for "indexed".
+(define_attr "update" "no,yes"
+  (if_then_else (ior (match_operand 0 "update_address_mem")
+ (match_operand 1 "update_address_mem"))
+(const_string "yes")
+(const_string "no")))
+
+(define_attr "index_shift" "no,yes"
+  (if_then_else (ior (match_operand 0 "index_shift_address_mem")
+ (match_operand 1 "index_shift_address_mem"))
+(const_string "yes")
+(const_string "no")))
 ;; ---
 ;; Pipeline descriptions and scheduling
 ;; ---
@@ -546,7 +570,19 @@
 operands[0] = gen_rtx_MEM (DImode, operands[0]);
 return pftype[INTVAL(operands[1])][locality];
   }
-  [(set_attr "type" "load1")]
+  [(set_attr "type" "load1")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))]
 )
 
 (define_insn "trap"
@@ -1192,7 +1228,19 @@
ldp\\t%w0, %w2, %1
ldp\\t%s0, %s2, %1"
   [(set_attr "type" "load2,neon_load1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))]
 )
 
 (define_insn "load_pairdi"
@@ -1208,7 +1256,19 @@
ldp\\t%x0, %x2, %1
ldp\\t%d0, %d2, %1"
   [(set_attr "type" "load2,neon_load1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 1 "update_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 1 "index_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 1 "index_shift_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))]
 )
 
 
@@ -1227,7 +1287,19 @@
stp\\t%w1, %w3, %0
stp\\t%s1, %s3, %0"
   [(set_attr "type" "store2,neon_store1_2reg")
-   (set_attr "fp" "*,yes")]
+   (set_attr "fp" "*,yes")
+   (set (attr "update")
+	(if_then_else (match_operand 0 "update_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index")
+	(if_then_else (match_operand 0 "index_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))
+   (set (attr "index_shift")
+	(if_then_else (match_operand 0 "index_shift_address_mem")
+		  (const_string "yes")
+		  (const_string "no")))]
 )
 
 (define_insn "store_pairdi"
@@ -1243,7 +1315,19 @@
stp\\t%x1, %x3, %0
stp\\t%d1, %d3, %0"
   [(set_attr "type" "store2,neon_store1_2reg")
-   (set_attr "fp" "*,yes")]
+   (s

Re: [PATCH 2/2] [SPARC] Add -mfsmuld option

[Eric Botcazou]

> Add the -mfsmuld option to control the generation of the FsMULd
> instruction.  In general, this instruction is available in architecture
> version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
> support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
> CPUs of this category do not implement it in hardware, e.g. LEON3/4 with
> GRFPU-lite.

OK on principle, but could we avoid all this shuffling with MASK_FSMULD?  
Having to write MASK_V8|MASK_FSMULD and MASK_V9|MASK_FSMULD is awkward.

What about automatically setting MASK_FSMULD if MASK_FPU is set and disabling 
it by default for v7, cypress and leon (i.e MASK_ISA|MASK_FSMULD for them)?

-- 
Eric Botcazou

Re: [PATCH 2/2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

On 26/07/17 10:09, Eric Botcazou wrote:


Add the -mfsmuld option to control the generation of the FsMULd
instruction.  In general, this instruction is available in architecture
version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
CPUs of this category do not implement it in hardware, e.g. LEON3/4 with
GRFPU-lite.

OK on principle, but could we avoid all this shuffling with MASK_FSMULD?
Having to write MASK_V8|MASK_FSMULD and MASK_V9|MASK_FSMULD is awkward.

What about automatically setting MASK_FSMULD if MASK_FPU is set and disabling
it by default for v7, cypress and leon (i.e MASK_ISA|MASK_FSMULD for them)?


That was my first approach, however, this didn't work well. The 
-fno-fsmuld option didn't work here and was ignored. I guess this is due 
to the asymmetric handling of the enable/disable CPU target flags 
handling with respect to the target_flags_explicit:


  target_flags &= ~cpu->disable;
  target_flags |= (cpu->enable
#ifndef HAVE_AS_FMAF_HPC_VIS3
   & ~(MASK_FMAF | MASK_VIS3)
#endif
#ifndef HAVE_AS_SPARC4
   & ~MASK_CBCOND
#endif
#ifndef HAVE_AS_SPARC5_VIS4
   & ~(MASK_VIS4 | MASK_SUBXC)
#endif
#ifndef HAVE_AS_SPARC6
   & ~(MASK_VIS4B)
#endif
#ifndef HAVE_AS_LEON
   & ~(MASK_LEON | MASK_LEON3)
#endif
   & ~(target_flags_explicit & MASK_FEATURES)
   );

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH 2/2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

On 26/07/17 10:13, Sebastian Huber wrote:


On 26/07/17 10:09, Eric Botcazou wrote:


Add the -mfsmuld option to control the generation of the FsMULd
instruction.  In general, this instruction is available in architecture
version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
CPUs of this category do not implement it in hardware, e.g. LEON3/4 
with

GRFPU-lite.

OK on principle, but could we avoid all this shuffling with MASK_FSMULD?
Having to write MASK_V8|MASK_FSMULD and MASK_V9|MASK_FSMULD is awkward.

What about automatically setting MASK_FSMULD if MASK_FPU is set and 
disabling
it by default for v7, cypress and leon (i.e MASK_ISA|MASK_FSMULD for 
them)?


That was my first approach, however, this didn't work well. The 
-fno-fsmuld option didn't work here and was ignored. I guess this is 
due to the asymmetric handling of the enable/disable CPU target flags 
handling with respect to the target_flags_explicit:


  target_flags &= ~cpu->disable;
  target_flags |= (cpu->enable
#ifndef HAVE_AS_FMAF_HPC_VIS3
   & ~(MASK_FMAF | MASK_VIS3)
#endif
#ifndef HAVE_AS_SPARC4
   & ~MASK_CBCOND
#endif
#ifndef HAVE_AS_SPARC5_VIS4
   & ~(MASK_VIS4 | MASK_SUBXC)
#endif
#ifndef HAVE_AS_SPARC6
   & ~(MASK_VIS4B)
#endif
#ifndef HAVE_AS_LEON
   & ~(MASK_LEON | MASK_LEON3)
#endif
   & ~(target_flags_explicit & MASK_FEATURES)
   );



I will try it again using

target_flags |= MASK_FSMULD & ~target_flags_explicit;

before this block above.

If !TARGET_FPU, this flag is cleared later.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH 2/2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

On 26/07/17 10:09, Eric Botcazou wrote:


What about automatically setting MASK_FSMULD if MASK_FPU is set and disabling
it by default for v7, cypress and leon (i.e MASK_ISA|MASK_FSMULD for them)?


What about the SPARCLITE and SPARCLET processors? I guess they 
previously didn't use the FSMULD due to the (TARGET_V8 || TARGET_9) && 
TARGET_FPU. I have now:


{ "sparclite",MASK_ISA|MASK_FSMULD, MASK_SPARCLITE },
/* The Fujitsu MB86930 is the original sparclite chip, with no FPU.  */
{ "f930",MASK_ISA|MASK_FPU, MASK_SPARCLITE },
/* The Fujitsu MB86934 is the recent sparclite chip, with an FPU.  */
{ "f934",MASK_ISA|MASK_FSMULD, MASK_SPARCLITE },
{ "sparclite86x",MASK_ISA|MASK_FPU, MASK_SPARCLITE },
{ "sparclet",MASK_ISA|MASK_FSMULD, MASK_SPARCLET },
/* TEMIC sparclet */
{ "tsc701",MASK_ISA|MASK_FSMULD, MASK_SPARCLET },

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [RFC] If conversion min/max search, costs and problems

[Robin Dapp]

> Do you have an example where wrong code is generated through the
> noce_convert_multiple_sets_p path (with or without bodged costs)?
> 
> Both AArch64 and x86-64 reject your testcase along this codepath because
> of the constant set of 1. If we work around that by setting bla = n rather
> than bla = 1 , I see this code generation for AArch64:
[..]
> i.e. we have fresh registers. As you note, we do expect a later pass to
> clean up the redundant compare of (reg:SI 73) (reg:SI 77), which can
> throw the cost calculation off.
> 
> If I hack up ix86_noce_conversion_profitable_p to alwyas return true, then
> for your testcase I also see:
[..]
> Again, no overlap.

mhm, right, at first I used the cond_move_process_if_block before
realizing noce_convert_multiple_sets is better suited to do the work.
Seeing that the additional compare is not cleaned up on s390 (no overlap
though), I implicitly assumed it has the same problems as the other path
since it also uses noce_emit_cmove.  Yet, apparently no wrong code is
produced here.  (Additionally, my local branch has a hack for the
const_int case which I missed to include)

On s390 the resulting assembly reads
  cr  %r5,%r1
  locrnhe %r4,%r0
  cr  %r5,%r1
  locrnhe %r1,%r5

I'll have to check why we don't manage to get rid of the compare.

Regarding the cond_move.. case I agree that it is of lesser importance
but I'd still be interested why the overlap condition is needed (if not
for implementation reasons that could be overcome like in ..multiple_sets).

Nevertheless, the cost situation seems strange - I'm not sure if it's
worth to cache the compare like in my patch just to get the costs
"correct" though.  Your reply suggests that the current cost model works
for aarch64 and my example, or did you include my hack?

Regards
 Robin

[PATCH v2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

Add the -mfsmuld option to control the generation of the FsMULd
instruction.  In general, this instruction is available in architecture
version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
CPUs of this category do not implement it in hardware, e.g. LEON3/4 with
GRFPU-lite.

gcc/
* config/sparc/sparc.c (dump_target_flag_bits): Dump MASK_FSMULD.
(sparc_option_override): Honour MASK_FSMULD.
* config/sparc/sparc.h (MASK_FEATURES): Add MASK_FSMULD.
* config/sparc/sparc.md (muldf3_extend): Use TARGET_FSMULD.
* config/sparc/sparc.opt (mfsmuld): New option.
* doc/invoke.texi (mfsmuld): Document option.
---
 gcc/config/sparc/sparc.c   | 30 --
 gcc/config/sparc/sparc.h   |  3 ++-
 gcc/config/sparc/sparc.md  |  2 +-
 gcc/config/sparc/sparc.opt |  4 
 gcc/doc/invoke.texi| 11 ++-
 5 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index 674a3823cb9..8eed2fc5621 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -1304,6 +1304,8 @@ dump_target_flag_bits (const int flags)
 fprintf (stderr, "FLAT ");
   if (flags & MASK_FMAF)
 fprintf (stderr, "FMAF ");
+  if (flags & MASK_FSMULD)
+fprintf (stderr, "FSMULD ");
   if (flags & MASK_FPU)
 fprintf (stderr, "FPU ");
   if (flags & MASK_HARD_QUAD)
@@ -1403,24 +1405,24 @@ sparc_option_override (void)
 const int disable;
 const int enable;
   } const cpu_table[] = {
-{ "v7",MASK_ISA, 0 },
-{ "cypress",   MASK_ISA, 0 },
+{ "v7",MASK_ISA|MASK_FSMULD, 0 },
+{ "cypress",   MASK_ISA|MASK_FSMULD, 0 },
 { "v8",MASK_ISA, MASK_V8 },
 /* TI TMS390Z55 supersparc */
 { "supersparc",MASK_ISA, MASK_V8 },
 { "hypersparc",MASK_ISA, MASK_V8 },
-{ "leon",  MASK_ISA, MASK_V8|MASK_LEON },
+{ "leon",  MASK_ISA|MASK_FSMULD, MASK_V8|MASK_LEON },
 { "leon3", MASK_ISA, MASK_V8|MASK_LEON3 },
-{ "leon3v7",   MASK_ISA, MASK_LEON3 },
-{ "sparclite", MASK_ISA, MASK_SPARCLITE },
+{ "leon3v7",   MASK_ISA|MASK_FSMULD, MASK_LEON3 },
+{ "sparclite", MASK_ISA|MASK_FSMULD, MASK_SPARCLITE },
 /* The Fujitsu MB86930 is the original sparclite chip, with no FPU.  */
 { "f930",  MASK_ISA|MASK_FPU, MASK_SPARCLITE },
 /* The Fujitsu MB86934 is the recent sparclite chip, with an FPU.  */
-{ "f934",  MASK_ISA, MASK_SPARCLITE },
+{ "f934",  MASK_ISA|MASK_FSMULD, MASK_SPARCLITE },
 { "sparclite86x",  MASK_ISA|MASK_FPU, MASK_SPARCLITE },
-{ "sparclet",  MASK_ISA, MASK_SPARCLET },
+{ "sparclet",  MASK_ISA|MASK_FSMULD, MASK_SPARCLET },
 /* TEMIC sparclet */
-{ "tsc701",MASK_ISA, MASK_SPARCLET },
+{ "tsc701",MASK_ISA|MASK_FSMULD, MASK_SPARCLET },
 { "v9",MASK_ISA, MASK_V9 },
 /* UltraSPARC I, II, IIi */
 { "ultrasparc",MASK_ISA,
@@ -1511,6 +1513,11 @@ sparc_option_override (void)
   target_flags |= MASK_LONG_DOUBLE_128;
 }
 
+  /* Enable the FSMULD instruction by default if not explicitly configured by
+ the user.  It may be later disabled by the CPU target flags or if
+ !TARGET_FPU.  */
+  target_flags |= MASK_FSMULD & ~target_flags_explicit;
+
   /* Code model selection.  */
   sparc_cmodel = SPARC_DEFAULT_CMODEL;
 
@@ -1603,11 +1610,11 @@ sparc_option_override (void)
   if (TARGET_VIS4B)
 target_flags |= MASK_VIS4 | MASK_VIS3 | MASK_VIS2 | MASK_VIS;
 
-  /* Don't allow -mvis, -mvis2, -mvis3, -mvis4, -mvis4b and -mfmaf if
+  /* Don't allow -mvis, -mvis2, -mvis3, -mvis4, -mvis4b, -mfmaf and -mfsmuld if
  FPU is disabled.  */
   if (! TARGET_FPU)
 target_flags &= ~(MASK_VIS | MASK_VIS2 | MASK_VIS3 | MASK_VIS4
- | MASK_VIS4B | MASK_FMAF);
+ | MASK_VIS4B | MASK_FMAF | MASK_FSMULD);
 
   /* -mvis assumes UltraSPARC+, so we are sure v9 instructions
  are available; -m64 also implies v9.  */
@@ -1641,6 +1648,9 @@ sparc_option_override (void)
   if (sparc_fix_ut699 || sparc_fix_ut700 || sparc_fix_gr712rc)
 sparc_fix_b2bst = 1;
 
+  if (sparc_fix_ut699)
+target_flags &= ~MASK_FSMULD;
+
   /* Supply a default value for align_functions.  */
   if (align_functions == 0)
 {
diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h
index d7c617e06c3..15a62179af5 100644
--- a/gcc/config/sparc/sparc.h
+++ b/gcc/config/sparc/sparc.h
@@ -438,7 +438,8 @@ extern enum cmodel sparc_cmodel;
 /* Mask of all CPU feature flags.  */
 #define MASK_FEATURES  \
   (MASK_FPU + MASK_HARD_QUAD + MASK_VIS + MASK_VIS2 + MASK_VIS3\
-   + MASK_VIS4 + MASK_CBCOND + MASK_FMAF + MASK_POPC + MASK_SUBXC)
+   + MASK_VIS4 + MASK_CBCOND + MASK_FMAF + MASK_FSMULD 

Re: [PATCH] Add macro DISABLE_COPY_AND_ASSIGN

[Yao Qi]

On 17-07-19 10:30:45, Yao Qi wrote:
> We have many classes that copy cotr and assignment operator are deleted
> in different projects, gcc, gdb and gold.  So this patch adds a macro
> to do this, and replace these existing mechanical code with macro
> DISABLE_COPY_AND_ASSIGN.
> 
> The patch was posted in gdb-patches,
> https://sourceware.org/ml/gdb-patches/2017-07/msg00254.html but we
> think it is better to put this macro in include/ansidecl.h so that
> other projects can use it too.
> 
> Boostrapped on x86_64-linux-gnu.  Is it OK?
> 
> include:
> 
> 2017-07-19  Yao Qi  
>   Pedro Alves  
> 
>   * ansidecl.h (DISABLE_COPY_AND_ASSIGN): New macro.
> 
> gcc/cp:
> 
> 2017-07-19  Yao Qi  
> 
>   * cp-tree.h (class ovl_iterator): Use DISABLE_COPY_AND_ASSIGN.
>   * name-lookup.c (struct name_loopup): Likewise.
> 
> gcc:
> 
> 2017-07-19  Yao Qi  
> 
>   * tree-ssa-scopedtables.h (class avail_exprs_stack): Use
>   DISABLE_COPY_AND_ASSIGN.
>   (class const_and_copies): Likewise.

Ping.
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01134.html

-- 
Yao (齐尧)

[patch,v7,committed] Backport PR81487: asprintf -> xasprintf

[Georg-Johann Lay]

Applied as https://gcc.gnu.org/r250562

Johann

lto-plugin/
Backport from 2017-07-21 trunk r250428.
PR lto/81487
* lto-plugin.c (claim_file_handler): Use xasprintf instead of
asprintf.
[hi!=0]: Swap hi and lo arguments supplied to xasprintf.
gcc/
Backport from 2017-07-25 trunk r250499.
PR 81487
* hsa-brig.c (brig_init): Use xasprintf instead of asprintf.
* gimple-pretty-print.c (dump_probability): Same.
* tree-ssa-structalias.c (alias_get_name): Same.


On 26.07.2017 09:54, Richard Biener wrote:

On Tue, Jul 25, 2017 at 10:01 PM, Georg-Johann Lay  wrote:

Richard Biener schrieb:


On Mon, Jul 24, 2017 at 10:19 AM, Georg-Johann Lay  wrote:


Hi, as proposed in

https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01294.html

this patch does more asprintf -> xasprintf replacements.

Bootstrapped + reg-tested on x86_64-linux.

Ok for trunk?


Ok.

Thanks,
Richard.



Is this something we want to backport to v7 ?


Yes.

Richard.


Index: lto-plugin/lto-plugin.c
===
--- lto-plugin/lto-plugin.c	(revision 250560)
+++ lto-plugin/lto-plugin.c	(working copy)
@@ -975,17 +975,16 @@ claim_file_handler (const struct ld_plug
 
   if (file->offset != 0)
 {
-  char *objname;
   /* We pass the offset of the actual file, not the archive header.
  Can't use PRIx64, because that's C99, so we have to print the
-	 64-bit hex int as two 32-bit ones. */
-  int lo, hi, t;
+	 64-bit hex int as two 32-bit ones.  Use xasprintf instead of
+	 asprintf because asprintf doesn't work as expected on some older
+	 mingw32 hosts.  */
+  int lo, hi;
   lo = file->offset & 0x;
   hi = ((int64_t)file->offset >> 32) & 0x;
-  t = hi ? asprintf (&objname, "%s@0x%x%08x", file->name, lo, hi)
-	 : asprintf (&objname, "%s@0x%x", file->name, lo);
-  check (t >= 0, LDPL_FATAL, "asprintf failed");
-  lto_file.name = objname;
+  lto_file.name = hi ? xasprintf ("%s@0x%x%08x", file->name, hi, lo)
+  			 : xasprintf ("%s@0x%x", file->name, lo);
 }
   else
 {
Index: gcc/gimple-pretty-print.c
===
--- gcc/gimple-pretty-print.c	(revision 250560)
+++ gcc/gimple-pretty-print.c	(working copy)
@@ -89,7 +89,7 @@ dump_probability (int value)
 return "[0.01%]";
 
   char *buf;
-  asprintf (&buf, "[%.2f%%]", fvalue);
+  buf = xasprintf ("[%.2f%%]", fvalue);
   const char *ret = xstrdup_for_dump (buf);
   free (buf);
 
Index: gcc/tree-ssa-structalias.c
===
--- gcc/tree-ssa-structalias.c	(revision 250560)
+++ gcc/tree-ssa-structalias.c	(working copy)
@@ -2827,7 +2827,6 @@ alias_get_name (tree decl)
 {
   const char *res = NULL;
   char *temp;
-  int num_printed = 0;
 
   if (!dump_file)
 return "NULL";
@@ -2836,14 +2835,11 @@ alias_get_name (tree decl)
 {
   res = get_name (decl);
   if (res)
-	num_printed = asprintf (&temp, "%s_%u", res, SSA_NAME_VERSION (decl));
+	temp = xasprintf ("%s_%u", res, SSA_NAME_VERSION (decl));
   else
-	num_printed = asprintf (&temp, "_%u", SSA_NAME_VERSION (decl));
-  if (num_printed > 0)
-	{
-	  res = ggc_strdup (temp);
-	  free (temp);
-	}
+	temp = xasprintf ("_%u", SSA_NAME_VERSION (decl));
+  res = ggc_strdup (temp);
+  free (temp);
 }
   else if (DECL_P (decl))
 {
@@ -2854,12 +2850,9 @@ alias_get_name (tree decl)
 	  res = get_name (decl);
 	  if (!res)
 	{
-	  num_printed = asprintf (&temp, "D.%u", DECL_UID (decl));
-	  if (num_printed > 0)
-		{
-		  res = ggc_strdup (temp);
-		  free (temp);
-		}
+	  temp = xasprintf ("D.%u", DECL_UID (decl));
+	  res = ggc_strdup (temp);
+	  free (temp);
 	}
 	}
 }
Index: gcc/hsa-brig.c
===
--- gcc/hsa-brig.c	(revision 250560)
+++ gcc/hsa-brig.c	(working copy)
@@ -499,7 +499,7 @@ brig_init (void)
 	  else
 	part++;
 	  char *modname2;
-	  asprintf (&modname2, "%s_%s", modname, part);
+	  modname2 = xasprintf ("%s_%s", modname, part);
 	  free (modname);
 	  modname = modname2;
 	}

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Richard Sandiford]

Richard Biener  writes:
> On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:
>> On Tue, 25 Jul 2017, Richard Biener wrote:
>>
 I think we need Richard to say what the intent is for the valueization
 function. It is used both to stop looking at defining stmt if the return
 is
 NULL, and to replace/optimize one SSA_NAME with another, but currently it
 seems hard to prevent looking at the defining statement without
 preventing
 from looking at the SSA_NAME at all.
>>>
>>>
>>> Yeah, this semantic overloading is an issue.  For gimple_build we have
>>> nothing
>>> to "valueize" but we only use it to tell genmatch that it may not look at
>>> the
>>> SSA_NAME_DEF_STMT.
>>>
 I guess we'll need a fix in genmatch...
>>>
>>>
>>> I'll have a look tomorrow.
>>
>>
>> My impression yesterday was that we could replace the current do_valueize
>> wrapper by 2 wrappers (without touching the valueize callbacks):
>> - may_check_def_stmt, which returns a bool corresponding to the current
>> do_valueize != NULL_TREE
>> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
>> returns its argument unchanged.
>>
>> Not very confident about it though.
>
> Note I've been there in the past (twice I think) but always ran into existing
> latent issues.  So hopefully we've resolved those, I'm testing the following
> simplified variant of what I had back in time.
>
> It'll produce
>
>   switch (TREE_CODE (op0))
> {
> case SSA_NAME:
>   if (gimple *def_stmt = get_def (valueize, op0))
> {
>   if (gassign *def = dyn_cast  (def_stmt))
> switch (gimple_assign_rhs_code (def))
>   {
>   case MINUS_EXPR:
> {
>   tree o20 = gimple_assign_rhs1 (def);
>   o20 = do_valueize (valueize, o20);
>   tree o21 = gimple_assign_rhs2 (def);
>   o21 = do_valueize (valueize, o21);
>   if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
> types_match (op1, o21)))
> {
>
> which also indents less which is nice.
>
> Bootstrap/regtest running on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2017-07-26  Richard Biener  
>
> * gimple-match-head.c (do_valueize): Return OP if valueize
> returns NULL_TREE.
> (get_def): New helper to get at the def stmt of a SSA name
> if valueize allows.
> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
> do_valueize to get at the def stmt.
> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>
>
>> --
>> Marc Glisse
>
> 2017-07-26  Richard Biener  
>
>   * gimple-match-head.c (do_valueize): Return OP if valueize
>   returns NULL_TREE.
>   (get_def): New helper to get at the def stmt of a SSA name
>   if valueize allows.
>   * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
>   do_valueize to get at the def stmt.
>   (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>
> Index: gcc/gimple-match-head.c
> ===
> --- gcc/gimple-match-head.c   (revision 250518)
> +++ gcc/gimple-match-head.c   (working copy)
> @@ -779,10 +779,25 @@ inline tree
>  do_valueize (tree (*valueize)(tree), tree op)
>  {
>if (valueize && TREE_CODE (op) == SSA_NAME)
> -return valueize (op);
> +{
> +  tree tem = valueize (op);
> +  if (tem)
> + return tem;
> +}
>return op;
>  }
>  
> +/* Helper for the autogenerated code, get at the definition of NAME when
> +   VALUEIZE allows that.  */
> +
> +inline gimple *
> +get_def (tree (*valueize)(tree), tree name)
> +{
> +  if (valueize && ! valueize (name))
> +return NULL;
> +  return SSA_NAME_DEF_STMT (name);
> +}

I realise this is preexisting, but why do we ignore the value returned
by valueize, even if it's different from NAME?  I struggled over that
with the old code when I hit the same problem with some SVE patches
(for which, thanks for the fix).

A comment might be good.

Thanks,
Richard

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Marc Glisse]

On Wed, 26 Jul 2017, Richard Sandiford wrote:


Richard Biener  writes:

On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:

On Tue, 25 Jul 2017, Richard Biener wrote:


I think we need Richard to say what the intent is for the valueization
function. It is used both to stop looking at defining stmt if the return
is
NULL, and to replace/optimize one SSA_NAME with another, but currently it
seems hard to prevent looking at the defining statement without
preventing
from looking at the SSA_NAME at all.



Yeah, this semantic overloading is an issue.  For gimple_build we have
nothing
to "valueize" but we only use it to tell genmatch that it may not look at
the
SSA_NAME_DEF_STMT.


I guess we'll need a fix in genmatch...



I'll have a look tomorrow.



My impression yesterday was that we could replace the current do_valueize
wrapper by 2 wrappers (without touching the valueize callbacks):
- may_check_def_stmt, which returns a bool corresponding to the current
do_valueize != NULL_TREE
- maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
returns its argument unchanged.

Not very confident about it though.


Note I've been there in the past (twice I think) but always ran into existing
latent issues.  So hopefully we've resolved those, I'm testing the following
simplified variant of what I had back in time.

It'll produce

  switch (TREE_CODE (op0))
{
case SSA_NAME:
  if (gimple *def_stmt = get_def (valueize, op0))
{
  if (gassign *def = dyn_cast  (def_stmt))
switch (gimple_assign_rhs_code (def))
  {
  case MINUS_EXPR:
{
  tree o20 = gimple_assign_rhs1 (def);
  o20 = do_valueize (valueize, o20);
  tree o21 = gimple_assign_rhs2 (def);
  o21 = do_valueize (valueize, o21);
  if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
types_match (op1, o21)))
{

which also indents less which is nice.

Bootstrap/regtest running on x86_64-unknown-linux-gnu.

Richard.

2017-07-26  Richard Biener  

* gimple-match-head.c (do_valueize): Return OP if valueize
returns NULL_TREE.
(get_def): New helper to get at the def stmt of a SSA name
if valueize allows.
* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
do_valueize to get at the def stmt.
(dt_operand::gen_gimple_expr): Simplify do_valueize calls.



--
Marc Glisse


2017-07-26  Richard Biener  

* gimple-match-head.c (do_valueize): Return OP if valueize
returns NULL_TREE.
(get_def): New helper to get at the def stmt of a SSA name
if valueize allows.
* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
do_valueize to get at the def stmt.
(dt_operand::gen_gimple_expr): Simplify do_valueize calls.

Index: gcc/gimple-match-head.c
===
--- gcc/gimple-match-head.c (revision 250518)
+++ gcc/gimple-match-head.c (working copy)
@@ -779,10 +779,25 @@ inline tree
 do_valueize (tree (*valueize)(tree), tree op)
 {
   if (valueize && TREE_CODE (op) == SSA_NAME)
-return valueize (op);
+{
+  tree tem = valueize (op);
+  if (tem)
+   return tem;
+}
   return op;
 }

+/* Helper for the autogenerated code, get at the definition of NAME when
+   VALUEIZE allows that.  */
+
+inline gimple *
+get_def (tree (*valueize)(tree), tree name)
+{
+  if (valueize && ! valueize (name))
+return NULL;
+  return SSA_NAME_DEF_STMT (name);
+}


I realise this is preexisting, but why do we ignore the value returned
by valueize, even if it's different from NAME?


My impression is that we expect it has already been valueized.

--
Marc Glisse

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Richard Sandiford]

Marc Glisse  writes:
> On Wed, 26 Jul 2017, Richard Sandiford wrote:
>> Richard Biener  writes:
>>> On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:
 On Tue, 25 Jul 2017, Richard Biener wrote:

>> I think we need Richard to say what the intent is for the valueization
>> function. It is used both to stop looking at defining stmt if the return
>> is
>> NULL, and to replace/optimize one SSA_NAME with another, but currently it
>> seems hard to prevent looking at the defining statement without
>> preventing
>> from looking at the SSA_NAME at all.
>
>
> Yeah, this semantic overloading is an issue.  For gimple_build we have
> nothing
> to "valueize" but we only use it to tell genmatch that it may not look at
> the
> SSA_NAME_DEF_STMT.
>
>> I guess we'll need a fix in genmatch...
>
>
> I'll have a look tomorrow.


 My impression yesterday was that we could replace the current do_valueize
 wrapper by 2 wrappers (without touching the valueize callbacks):
 - may_check_def_stmt, which returns a bool corresponding to the current
 do_valueize != NULL_TREE
 - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
 returns its argument unchanged.

 Not very confident about it though.
>>>
>>> Note I've been there in the past (twice I think) but always ran into 
>>> existing
>>> latent issues.  So hopefully we've resolved those, I'm testing the following
>>> simplified variant of what I had back in time.
>>>
>>> It'll produce
>>>
>>>   switch (TREE_CODE (op0))
>>> {
>>> case SSA_NAME:
>>>   if (gimple *def_stmt = get_def (valueize, op0))
>>> {
>>>   if (gassign *def = dyn_cast  (def_stmt))
>>> switch (gimple_assign_rhs_code (def))
>>>   {
>>>   case MINUS_EXPR:
>>> {
>>>   tree o20 = gimple_assign_rhs1 (def);
>>>   o20 = do_valueize (valueize, o20);
>>>   tree o21 = gimple_assign_rhs2 (def);
>>>   o21 = do_valueize (valueize, o21);
>>>   if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
>>> types_match (op1, o21)))
>>> {
>>>
>>> which also indents less which is nice.
>>>
>>> Bootstrap/regtest running on x86_64-unknown-linux-gnu.
>>>
>>> Richard.
>>>
>>> 2017-07-26  Richard Biener  
>>>
>>> * gimple-match-head.c (do_valueize): Return OP if valueize
>>> returns NULL_TREE.
>>> (get_def): New helper to get at the def stmt of a SSA name
>>> if valueize allows.
>>> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
>>> do_valueize to get at the def stmt.
>>> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>>>
>>>
 --
 Marc Glisse
>>>
>>> 2017-07-26  Richard Biener  
>>>
>>> * gimple-match-head.c (do_valueize): Return OP if valueize
>>> returns NULL_TREE.
>>> (get_def): New helper to get at the def stmt of a SSA name
>>> if valueize allows.
>>> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
>>> do_valueize to get at the def stmt.
>>> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>>>
>>> Index: gcc/gimple-match-head.c
>>> ===
>>> --- gcc/gimple-match-head.c (revision 250518)
>>> +++ gcc/gimple-match-head.c (working copy)
>>> @@ -779,10 +779,25 @@ inline tree
>>>  do_valueize (tree (*valueize)(tree), tree op)
>>>  {
>>>if (valueize && TREE_CODE (op) == SSA_NAME)
>>> -return valueize (op);
>>> +{
>>> +  tree tem = valueize (op);
>>> +  if (tem)
>>> +   return tem;
>>> +}
>>>return op;
>>>  }
>>>
>>> +/* Helper for the autogenerated code, get at the definition of NAME when
>>> +   VALUEIZE allows that.  */
>>> +
>>> +inline gimple *
>>> +get_def (tree (*valueize)(tree), tree name)
>>> +{
>>> +  if (valueize && ! valueize (name))
>>> +return NULL;
>>> +  return SSA_NAME_DEF_STMT (name);
>>> +}
>>
>> I realise this is preexisting, but why do we ignore the value returned
>> by valueize, even if it's different from NAME?
>
> My impression is that we expect it has already been valueized.

But in that case, why do we try to valueize it again?  It feels like
we're conflating two things.

Richard

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Marc Glisse]

On Wed, 26 Jul 2017, Richard Sandiford wrote:


Marc Glisse  writes:

On Wed, 26 Jul 2017, Richard Sandiford wrote:

Richard Biener  writes:

On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:

On Tue, 25 Jul 2017, Richard Biener wrote:


I think we need Richard to say what the intent is for the valueization
function. It is used both to stop looking at defining stmt if the return
is
NULL, and to replace/optimize one SSA_NAME with another, but currently it
seems hard to prevent looking at the defining statement without
preventing
from looking at the SSA_NAME at all.



Yeah, this semantic overloading is an issue.  For gimple_build we have
nothing
to "valueize" but we only use it to tell genmatch that it may not look at
the
SSA_NAME_DEF_STMT.


I guess we'll need a fix in genmatch...



I'll have a look tomorrow.



My impression yesterday was that we could replace the current do_valueize
wrapper by 2 wrappers (without touching the valueize callbacks):
- may_check_def_stmt, which returns a bool corresponding to the current
do_valueize != NULL_TREE
- maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
returns its argument unchanged.

Not very confident about it though.


Note I've been there in the past (twice I think) but always ran into existing
latent issues.  So hopefully we've resolved those, I'm testing the following
simplified variant of what I had back in time.

It'll produce

  switch (TREE_CODE (op0))
{
case SSA_NAME:
  if (gimple *def_stmt = get_def (valueize, op0))
{
  if (gassign *def = dyn_cast  (def_stmt))
switch (gimple_assign_rhs_code (def))
  {
  case MINUS_EXPR:
{
  tree o20 = gimple_assign_rhs1 (def);
  o20 = do_valueize (valueize, o20);
  tree o21 = gimple_assign_rhs2 (def);
  o21 = do_valueize (valueize, o21);
  if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
types_match (op1, o21)))
{

which also indents less which is nice.

Bootstrap/regtest running on x86_64-unknown-linux-gnu.

Richard.

2017-07-26  Richard Biener  

* gimple-match-head.c (do_valueize): Return OP if valueize
returns NULL_TREE.
(get_def): New helper to get at the def stmt of a SSA name
if valueize allows.
* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
do_valueize to get at the def stmt.
(dt_operand::gen_gimple_expr): Simplify do_valueize calls.



--
Marc Glisse


2017-07-26  Richard Biener  

* gimple-match-head.c (do_valueize): Return OP if valueize
returns NULL_TREE.
(get_def): New helper to get at the def stmt of a SSA name
if valueize allows.
* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
do_valueize to get at the def stmt.
(dt_operand::gen_gimple_expr): Simplify do_valueize calls.

Index: gcc/gimple-match-head.c
===
--- gcc/gimple-match-head.c (revision 250518)
+++ gcc/gimple-match-head.c (working copy)
@@ -779,10 +779,25 @@ inline tree
 do_valueize (tree (*valueize)(tree), tree op)
 {
   if (valueize && TREE_CODE (op) == SSA_NAME)
-return valueize (op);
+{
+  tree tem = valueize (op);
+  if (tem)
+   return tem;
+}
   return op;
 }

+/* Helper for the autogenerated code, get at the definition of NAME when
+   VALUEIZE allows that.  */
+
+inline gimple *
+get_def (tree (*valueize)(tree), tree name)
+{
+  if (valueize && ! valueize (name))
+return NULL;
+  return SSA_NAME_DEF_STMT (name);
+}


I realise this is preexisting, but why do we ignore the value returned
by valueize, even if it's different from NAME?


My impression is that we expect it has already been valueized.


But in that case, why do we try to valueize it again?


We don't know if valueize returned NULL and we kept the argument as is 
(should not look at def stmt) or if valueize actually returned something. 
This way we can also have valueize replace a value with another, and still 
forbid looking at the def stmt of the replacement value.



It feels like we're conflating two things.


We are. The question is how much trouble that causes, compared to the 
complication of for instance having 2 callbacks.


--
Marc Glisse

[PATCH,AIX] Manage .go_export section for AIX

[REIX, Tony]

Description:
 * This patch manages the .go_export section as an EXCLUDE section on AIX.

Tests:
 * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
   - build made by means of gmake.

ChangeLog:
 * go-backend.c (go_write_export_data): Use EXCLUDE section for AIX.


Cordialement,

Tony Reix
Bull - ATOS
IBM Coop Architect & Technical Leader
Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.netIndex: gcc/go/ChangeLog
===
--- gcc/go/ChangeLog	(révision 250528)
+++ gcc/go/ChangeLog	(copie de travail)
@@ -1,3 +1,7 @@
+2017-07-26  Tony Reix  
+
+	* go-backend.c (go_write_export_data): Use EXCLUDE section for AIX.
+
 2017-06-09  Ian Lance Taylor  
 
 	* go-lang.c (go_langhook_post_options): If -fsplit-stack is turned
Index: gcc/go/go-backend.c
===
--- gcc/go/go-backend.c	(révision 250528)
+++ gcc/go/go-backend.c	(copie de travail)
@@ -101,7 +101,11 @@ go_write_export_data (const char *bytes, unsigned
   if (sec == NULL)
 {
   gcc_assert (targetm_common.have_named_sections);
+#ifndef _AIX
   sec = get_section (GO_EXPORT_SECTION_NAME, SECTION_DEBUG, NULL);
+#else
+  sec = get_section (GO_EXPORT_SECTION_NAME, SECTION_EXCLUDE, NULL);
+#endif
 }
 
   switch_to_section (sec);

Re: Patch ping

[Richard Biener]

On Tue, 25 Jul 2017, Jakub Jelinek wrote:

> Hi!
> 
> I'd like to ping 2 patches:
> 
> - UBSAN -fsanitize=pointer-overflow support
>   - http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01365.html

The probablility stuff might need updating?

Can you put the TYPE_PRECISION (sizetype) != POINTER_SIZE check
in option processing and inform people that pointer overflow sanitizing
is not done instead?

Where you handle DECL_BIT_FIELD_REPRESENTATIVE in 
maybe_instrument_pointer_overflow you could instead of building
a new COMPONENT_REF strip the bitfield ref and just remember
DECL_FIELD_OFFSET/BIT_OFFSET to be added to the get_inner_reference
result?  You don't seem to use 'size' anywhere.  You fail to allow
other handled components -- for no good reason?  You fail to handle
&MEM[ptr + CST] a canonical gimple invariant way of ptr +p CST,
the early out bitpos == 0 will cause non-instrumentation here.
(I'd just round down in the case of bitpos % BITS_PER_UNIT != 0)

Otherwise looks good.

> - noipa attribute addition
>  
>   http://gcc.gnu.org/ml/gcc-patches/2016-12/msg01501.html 
>  

Ok.

Thanks,
Richard.

Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Richard Biener]

On Tue, 25 Jul 2017, Jakub Jelinek wrote:

> Hi!
> 
> The following patch adjusts the vec_init and vec_extract optabs, so that
> they don't have in the expander names just the vector mode, but also another
> mode, for vec_extract the mode of the result and for vec_init the mode of
> the elts of the vector passed as second operand.
> 
> Without this patch, the second mode has been implicit, GET_MODE_INNER of
> the vector mode, so one could just extract a single element from a vector
> or construct vector from elements.  While that is most common, we allow
> in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc.
> and the vectorizer uses them.  By having the second mode in the name
> it allows the generic code (vectorizer, expansion) to query whether the
> backend supports such vector from vector expansions or inits from vector
> elts and use them if available.
> 
> For vec_extract, if we say want to extract high V2SImode from V4SImode
> the fallback is try to expand it as DImode extraction from V2DImode.
> This works well in many cases, but doesn't really work for very large
> vectors, say if we want to extract high V8SImode from V16SImode on x86,
> we'd need OImode extraction from V2OImode, which is something the backend
> doesn't have any support for.
> For vec_init, the fallback is usually to go through memory, which is slow in
> many cases.
> 
> This patch only adds new vector from vector extract and init patterns to
> the i386 backend, but I had to change many other targets too, because
> it needs to have the element mode in the vec_extract/vec_init expander
> names.  Seems most of the backends didn't really have a mode attribute
> usable for this or had it only in uppercase, while for the names we need
> lowercase.  Some backends had a convention on how to name lower case
> vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> of affected backends to seek advice on what mode attributes they want to
> use.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves
> e.g. the code generation for slp-43.c and slp-45.c testcases.
> make cc1 tested in cross-compilers to the remaining targets.
> 
> Ok for trunk?

The non-target specific bits are ok.

Thanks,
Richard.

> 2017-07-25  Jakub Jelinek  
> 
>   PR target/80846
>   * optabs.def (vec_extract_optab, vec_init_optab): Change from
>   a direct optab to conversion optab.
>   * optabs.c (expand_vector_broadcast): Use convert_optab_handler
>   with GET_MODE_INNER as last argument instead of optab_handler.
>   * expmed.c (extract_bit_field_1): Likewise.  Use vector from
>   vector extraction if possible and optab is available.
>   * expr.c (store_constructor): Use convert_optab_handler instead
>   of optab_handler.  Use vector initialization from smaller
>   vectors if possible and optab is available.
>   * tree-vect-stmts.c (vectorizable_load): Likewise.
>   * doc/md.texi (vec_extract, vec_init): Document that the optabs
>   now have two modes.
>   * config/i386/i386.c (ix86_expand_vector_init): Handle expansion
>   of vec_init from half-sized vectors with the same element mode.
>   * config/i386/sse.md (ssehalfvecmode): Add V4TI case.
>   (ssehalfvecmodelower, ssescalarmodelower): New mode attributes.
>   (reduc_plus_scal_v8df, reduc_plus_scal_v4df, reduc_plus_scal_v2df,
>   reduc_plus_scal_v16sf, reduc_plus_scal_v8sf, reduc_plus_scal_v4sf,
>   reduc__scal_, reduc_umin_scal_v8hi): Add element mode
>   after mode in gen_vec_extract* calls.
>   (vec_extract): Renamed to ...
>   (vec_extract): ... this.
>   (vec_extract): New expander.
>   (rotl3, rotr3, 3, ashrv2di3): Add
>   element mode after mode in gen_vec_init* calls.
>   (VEC_INIT_HALF_MODE): New mode iterator.
>   (vec_init): Renamed to ...
>   (vec_init): ... this.
>   (vec_init): New expander.
>   * config/i386/mmx.md (vec_extractv2sf): Renamed to ...
>   (vec_extractv2sfsf): ... this.
>   (vec_initv2sf): Renamed to ...
>   (vec_initv2sfsf): ... this.
>   (vec_extractv2si): Renamed to ...
>   (vec_extractv2sisi): ... this.
>   (vec_initv2si): Renamed to ...
>   (vec_initv2sisi): ... this.
>   (vec_extractv4hi): Renamed to ...
>   (vec_extractv4hihi): ... this.
>   (vec_initv4hi): Renamed to ...
>   (vec_initv4hihi): ... this.
>   (vec_extractv8qi): Renamed to ...
>   (vec_extractv8qiqi): ... this.
>   (vec_initv8qi): Renamed to ...
>   (vec_initv8qiqi): ... this.
>   * config/rs6000/vector.md (VEC_base_l): New mode attribute.
>   (vec_init): Renamed to ...
>   (vec_init): ... this.
>   (vec_extract): Renamed to ...
>   (vec_extract): ... this.
>   * config/rs6000/paired.md (vec_initv2sf): Renamed to ...
>   (vec_initv2sfsf): ... this.
>   * config/rs6000/altivec.md (splitter, altivec_copysign_v4sf3,
>   vec_unpacku_hi_v16qi, vec_

Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Uros Bizjak]

On Tue, Jul 25, 2017 at 11:14 AM, Jakub Jelinek  wrote:
> Hi!
>
> The following patch adjusts the vec_init and vec_extract optabs, so that
> they don't have in the expander names just the vector mode, but also another
> mode, for vec_extract the mode of the result and for vec_init the mode of
> the elts of the vector passed as second operand.
>
> Without this patch, the second mode has been implicit, GET_MODE_INNER of
> the vector mode, so one could just extract a single element from a vector
> or construct vector from elements.  While that is most common, we allow
> in GIMPLE e.g. construction of V8DImode from 4 V2DImode elements etc.
> and the vectorizer uses them.  By having the second mode in the name
> it allows the generic code (vectorizer, expansion) to query whether the
> backend supports such vector from vector expansions or inits from vector
> elts and use them if available.
>
> For vec_extract, if we say want to extract high V2SImode from V4SImode
> the fallback is try to expand it as DImode extraction from V2DImode.
> This works well in many cases, but doesn't really work for very large
> vectors, say if we want to extract high V8SImode from V16SImode on x86,
> we'd need OImode extraction from V2OImode, which is something the backend
> doesn't have any support for.
> For vec_init, the fallback is usually to go through memory, which is slow in
> many cases.
>
> This patch only adds new vector from vector extract and init patterns to
> the i386 backend, but I had to change many other targets too, because
> it needs to have the element mode in the vec_extract/vec_init expander
> names.  Seems most of the backends didn't really have a mode attribute
> usable for this or had it only in uppercase, while for the names we need
> lowercase.  Some backends had a convention on how to name lower case
> vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> of affected backends to seek advice on what mode attributes they want to
> use.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, where it improves
> e.g. the code generation for slp-43.c and slp-45.c testcases.
> make cc1 tested in cross-compilers to the remaining targets.
>
> Ok for trunk?
>
> 2017-07-25  Jakub Jelinek  
>
> PR target/80846
> * optabs.def (vec_extract_optab, vec_init_optab): Change from
> a direct optab to conversion optab.
> * optabs.c (expand_vector_broadcast): Use convert_optab_handler
> with GET_MODE_INNER as last argument instead of optab_handler.
> * expmed.c (extract_bit_field_1): Likewise.  Use vector from
> vector extraction if possible and optab is available.
> * expr.c (store_constructor): Use convert_optab_handler instead
> of optab_handler.  Use vector initialization from smaller
> vectors if possible and optab is available.
> * tree-vect-stmts.c (vectorizable_load): Likewise.
> * doc/md.texi (vec_extract, vec_init): Document that the optabs
> now have two modes.
> * config/i386/i386.c (ix86_expand_vector_init): Handle expansion
> of vec_init from half-sized vectors with the same element mode.
> * config/i386/sse.md (ssehalfvecmode): Add V4TI case.
> (ssehalfvecmodelower, ssescalarmodelower): New mode attributes.
> (reduc_plus_scal_v8df, reduc_plus_scal_v4df, reduc_plus_scal_v2df,
> reduc_plus_scal_v16sf, reduc_plus_scal_v8sf, reduc_plus_scal_v4sf,
> reduc__scal_, reduc_umin_scal_v8hi): Add element mode
> after mode in gen_vec_extract* calls.
> (vec_extract): Renamed to ...
> (vec_extract): ... this.
> (vec_extract): New expander.
> (rotl3, rotr3, 3, ashrv2di3): Add
> element mode after mode in gen_vec_init* calls.
> (VEC_INIT_HALF_MODE): New mode iterator.
> (vec_init): Renamed to ...
> (vec_init): ... this.
> (vec_init): New expander.
> * config/i386/mmx.md (vec_extractv2sf): Renamed to ...
> (vec_extractv2sfsf): ... this.
> (vec_initv2sf): Renamed to ...
> (vec_initv2sfsf): ... this.
> (vec_extractv2si): Renamed to ...
> (vec_extractv2sisi): ... this.
> (vec_initv2si): Renamed to ...
> (vec_initv2sisi): ... this.
> (vec_extractv4hi): Renamed to ...
> (vec_extractv4hihi): ... this.
> (vec_initv4hi): Renamed to ...
> (vec_initv4hihi): ... this.
> (vec_extractv8qi): Renamed to ...
> (vec_extractv8qiqi): ... this.
> (vec_initv8qi): Renamed to ...
> (vec_initv8qiqi): ... this.
> * config/rs6000/vector.md (VEC_base_l): New mode attribute.
> (vec_init): Renamed to ...
> (vec_init): ... this.
> (vec_extract): Renamed to ...
> (vec_extract): ... this.
> * config/rs6000/paired.md (vec_initv2sf): Renamed to ...
> (vec_initv2sfsf): ... this.
> * config/rs6000/altivec.md (splitter,

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Richard Biener]

On Wed, Jul 26, 2017 at 11:57 AM, Marc Glisse  wrote:
> On Wed, 26 Jul 2017, Richard Sandiford wrote:
>
>> Marc Glisse  writes:
>>>
>>> On Wed, 26 Jul 2017, Richard Sandiford wrote:

 Richard Biener  writes:
>
> On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse 
> wrote:
>>
>> On Tue, 25 Jul 2017, Richard Biener wrote:
>>
 I think we need Richard to say what the intent is for the
 valueization
 function. It is used both to stop looking at defining stmt if the
 return
 is
 NULL, and to replace/optimize one SSA_NAME with another, but
 currently it
 seems hard to prevent looking at the defining statement without
 preventing
 from looking at the SSA_NAME at all.
>>>
>>>
>>>
>>> Yeah, this semantic overloading is an issue.  For gimple_build we
>>> have
>>> nothing
>>> to "valueize" but we only use it to tell genmatch that it may not
>>> look at
>>> the
>>> SSA_NAME_DEF_STMT.
>>>
 I guess we'll need a fix in genmatch...
>>>
>>>
>>>
>>> I'll have a look tomorrow.
>>
>>
>>
>> My impression yesterday was that we could replace the current
>> do_valueize
>> wrapper by 2 wrappers (without touching the valueize callbacks):
>> - may_check_def_stmt, which returns a bool corresponding to the
>> current
>> do_valueize != NULL_TREE
>> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE,
>> it
>> returns its argument unchanged.
>>
>> Not very confident about it though.
>
>
> Note I've been there in the past (twice I think) but always ran into
> existing
> latent issues.  So hopefully we've resolved those, I'm testing the
> following
> simplified variant of what I had back in time.
>
> It'll produce
>
>   switch (TREE_CODE (op0))
> {
> case SSA_NAME:
>   if (gimple *def_stmt = get_def (valueize, op0))
> {
>   if (gassign *def = dyn_cast  (def_stmt))
> switch (gimple_assign_rhs_code (def))
>   {
>   case MINUS_EXPR:
> {
>   tree o20 = gimple_assign_rhs1 (def);
>   o20 = do_valueize (valueize, o20);
>   tree o21 = gimple_assign_rhs2 (def);
>   o21 = do_valueize (valueize, o21);
>   if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
> types_match (op1, o21)))
> {
>
> which also indents less which is nice.
>
> Bootstrap/regtest running on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2017-07-26  Richard Biener  
>
> * gimple-match-head.c (do_valueize): Return OP if valueize
> returns NULL_TREE.
> (get_def): New helper to get at the def stmt of a SSA name
> if valueize allows.
> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
> do_valueize to get at the def stmt.
> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>
>
>> --
>> Marc Glisse
>
>
> 2017-07-26  Richard Biener  
>
> * gimple-match-head.c (do_valueize): Return OP if valueize
> returns NULL_TREE.
> (get_def): New helper to get at the def stmt of a SSA name
> if valueize allows.
> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
> do_valueize to get at the def stmt.
> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>
> Index: gcc/gimple-match-head.c
> ===
> --- gcc/gimple-match-head.c (revision 250518)
> +++ gcc/gimple-match-head.c (working copy)
> @@ -779,10 +779,25 @@ inline tree
>  do_valueize (tree (*valueize)(tree), tree op)
>  {
>if (valueize && TREE_CODE (op) == SSA_NAME)
> -return valueize (op);
> +{
> +  tree tem = valueize (op);
> +  if (tem)
> +   return tem;
> +}
>return op;
>  }
>
> +/* Helper for the autogenerated code, get at the definition of NAME
> when
> +   VALUEIZE allows that.  */
> +
> +inline gimple *
> +get_def (tree (*valueize)(tree), tree name)
> +{
> +  if (valueize && ! valueize (name))
> +return NULL;
> +  return SSA_NAME_DEF_STMT (name);
> +}


 I realise this is preexisting, but why do we ignore the value returned
 by valueize, even if it's different from NAME?
>>>
>>>
>>> My impression is that we expect it has already been valueized.
>>
>>
>> But in that case, why do we try to valueize it again?
>
>
> We don't know if valueize returned NULL and we kept the argument as is
> (should not look at def stmt) or if v

Re: PING^2: Fwd: SSA range class and removal of VR_ANTI_RANGEs

[Richard Biener]

On Tue, Jul 25, 2017 at 4:50 PM, Andrew MacLeod  wrote:
> On 07/25/2017 03:12 AM, Richard Biener wrote:
>>
>> On Fri, Jul 21, 2017 at 9:30 PM, Aldy Hernandez  wrote:
>>>
>>> On Mon, Jul 17, 2017 at 6:23 AM, Richard Biener
>>>  wrote:

 On Mon, Jul 17, 2017 at 8:51 AM, Aldy Hernandez 
 wrote:
>
> How does this look?

 It's a change that on its own doesn't look worthwhile to me.

 So please post the changes that will build ontop of this.  Like removing
 anti-ranges from VRP or your on-demand value-range stuff.

 Thanks,
 Richard.
>>>
>>>  From the looks of it, we can have a variety of VRP ranges that are not
>>> representable at all with the an integer range class.  For instance, I
>>> see the following ranges built in set_value_range():
>>>
>>> [INT, (nop_expr SSA)]
>>>
>>> [INT, (plus_expr SSA INT)]
>>>
>>> [(negate_expr SSA), (negate_expr SSA)]
>>>
>>> [(plus_expr (negate_expr SSA INT)),
>>>   (plus_expr (negate_expr SSA) INT)]
>>>
>>> [SSA, SSA]
>>>
>>> So...I guess the first suggestion is out of the question ;-).
>>
>> Well.  We do not want range operations implemented twice (really!)
>> so range operations need to work on both representations.  So we
>> should think of a way to get rid of anti-ranges in VRP which, frankly,
>> means that for the sake symbolic ranges we have to trade them
>> with handling open/closed ranges which I'm not sure will be less of
>> a hassle to handle?
>
>
> I originally looked at ranges with symbolic expressions, but as soon as
> there are ranges comprised of more than 1 range, symbolics became a
> nightmare.  At best you can do endpoints, and even then you have to start
> adding complexity..  [0, 100] Union  [5, x_4]  now has to become  [0,
> max(100, x_4)] , and chained operations were making the expressions more and
> more complicated.  Trying to maintain these expression across optimizations
> was also very problematic as the IL changes and these expressions are not in
> the IL and don't suffer updates easily.
>
> At which point one asks themselves whether it actually makes sense to
> integrate that into the range representation, or try a different tactic.
>
> Seems to me its cleaner to have an integral range, and when appropriate
> track symbols separately to determine if their values can be refined.If
> at some point you can resolve those symbols to an integral value,  then you
> simply update the integral range with the new range you've determined.
>
> VRP chooses to do this by creating a completely different structure for
> ranges, and representing endpoints as expression trees. It then updates the
> integral ranges at the end of the pass. It uses anti-ranges as a shorthand
> to handle a special case of a range comprised of 2 ranges.  As it stands
> right now, VRP's version of union and intersection can never have much in
> common with a general integral range implementation. They are 2 very
> different beasts.
>
> So we cant do much about VRP internally yet without some significant
> surgery.
>
> This issue seems orthogonal to me though.  irange is not intended to ever do
> anything with symbols, thus can never replace VRPs internal value_range
> class.We're simply replacing "struct range_info_def" with a new range
> structure and API which can support more precise ranges than a pair of
> integral endpoints.  We'll then build on that by providing routines which
> can generate more precise range information as well.
>
> For the moment we provide an implementation with the same precision to
>   a) ensure code generation remains the same
>   b) allow the compiler to use it for a while to make sure no bugs have been
> introduced
>   c) and there is nothing that would utilize the additional precision yet
> anyway.
>
> I just think its important to get it in the code base so its being used and
> we can be sure its correct.

But nothing uses all the methods.

And we're ending up with two variants of range + range, etc.

irange is a nearly subset of what VRP can handle so it should be able to re-use
VRP workers.  The 'nearly' part is that VRP currently doesn't handle multiple
ranges.  For an unknown reason you didn't start with teaching VRP that bit.

Yes, symbolic ranges complicate that conversion a bit (getting rid of
anti-ranges
in VRP) but you could have either kept anti-ranges for symbolic ranges only
or have open/closed ranges.  The issue with symbolic ranges here is that
from

  if (a != b)

we derive a symbolic range for a that is ~[b, b].  If you want to make that
a union of two ranges you have [MIN, b - 1] U [b + 1, MAX] _but_ both
b - 1 or b + 1 might actually overflow!  So you need to use [MIN, b[ U ]b, MAX]
here (which means both can be empty if b == MIN or b == MAX).

As said I don't like introducing new stuff without fixing existing
stuff.  Handling
open/closed ranges shouldn't be too difficult if you assert that only a symbolic
end can be open.  Then you can re-use the workers from VRP in irange.

D

Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)

[Marek Polacek]

On Tue, Jul 25, 2017 at 03:47:31PM +0200, Richard Biener wrote:
> On Tue, Jul 25, 2017 at 3:30 PM, Eric Botcazou  wrote:
> >> Eric, any comments?
> >
> > No objection for the build2_stat hunk, I think it's in keeping with the Ada
> > semantics.  But the tree_could_trap_p hunk is certainly an abomination...
> >
> >> We could also avoid tree_could_trap_p by special-casing only
> >> *_DIV_EXPR and *_MOD_EXPR
> >> with integer zero 2nd operand explicitely in build2 given there's no
> >> "constant" value for this.  That is,
> >> for FP 1./0. is NaN (a "constant" value) even if the operation might trap.
> >
> > Yes, that would be faster & simpler and avoids the abomination.
> 
> Ok, then let's go with that slightly uglier but less abominal variant ;)

Like this then?

Bootstrapped/regtested on x86_64-linux (including Ada) and 
powerpc64le-unknown-linux-gnu,
ok for trunk?

2017-07-26  Marek Polacek  

PR middle-end/70992
* tree.c (build2_stat): Don't set TREE_CONSTANT on divisions by zero.

* gcc.dg/overflow-warn-1.c: Adjust dg-error.
* gcc.dg/overflow-warn-2.c: Likewise.
* gcc.dg/overflow-warn-3.c: Likewise.
* gcc.dg/overflow-warn-4.c: Likewise.
* gcc.dg/torture/pr70992-2.c: New test.
* gcc.dg/torture/pr70992.c: New test.

diff --git gcc/testsuite/gcc.dg/overflow-warn-1.c 
gcc/testsuite/gcc.dg/overflow-warn-1.c
index 8eb322579cf..a5cd5738636 100644
--- gcc/testsuite/gcc.dg/overflow-warn-1.c
+++ gcc/testsuite/gcc.dg/overflow-warn-1.c
@@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "25:integer 
overflow in expression"
 void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" 
} */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-1 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not computable at load time" "constant" 
{ target *-*-* } .-1 } */
+/* { dg-error "initializer element is not constant" "constant" { target *-*-* 
} .-1 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
diff --git gcc/testsuite/gcc.dg/overflow-warn-2.c 
gcc/testsuite/gcc.dg/overflow-warn-2.c
index f048d6dae2a..05ab104fa4a 100644
--- gcc/testsuite/gcc.dg/overflow-warn-2.c
+++ gcc/testsuite/gcc.dg/overflow-warn-2.c
@@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "integer 
overflow in expression" }
 void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" 
} */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-1 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not computable at load time" "constant" 
{ target *-*-* } .-1 } */
+/* { dg-error "initializer element is not constant" "constant" { target *-*-* 
} .-1 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
diff --git gcc/testsuite/gcc.dg/overflow-warn-3.c 
gcc/testsuite/gcc.dg/overflow-warn-3.c
index 664011e401d..fd4a34f67e2 100644
--- gcc/testsuite/gcc.dg/overflow-warn-3.c
+++ gcc/testsuite/gcc.dg/overflow-warn-3.c
@@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer 
overflow in expression" }
 /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } 
.-1 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not computable at load time" "constant" 
{ target *-*-* } .-1 } */
+/* { dg-error "initializer element is not constant" "constant" { target *-*-* 
} .-1 } */
 /* { dg-warning "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
diff --git gcc/testsuite/gcc.dg/overflow-warn-4.c 
gcc/testsuite/gcc.dg/overflow-warn-4.c
index 52677ce897a..018e3e1e4cd 100644
--- gcc/testsuite/gcc.dg/overflow-warn-4.c
+++ gcc/testsuite/gcc.dg/overflow-warn-4.c
@@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer 
overflow in expression" }
 /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 
.-1 } */
 /* { dg-error "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
-/* { dg-error "initializer element is not computable at load time" "constant" 
{ target *-*-* } .-1 } */
+/* { dg-error "initializer element is not constant" "constant" { target *-*-* 
} .-1 } */
 /* { dg-error "initialization makes pointer from integer without a cast" 
"null" { target *-*-* } .-2 } */
 void *r = (1 ? 0 : INT_MAX+1);
 
diff --git gcc/testsuite/gcc.dg/torture/pr7

Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)

[Richard Biener]

On Wed, Jul 26, 2017 at 1:35 PM, Marek Polacek  wrote:
> On Tue, Jul 25, 2017 at 03:47:31PM +0200, Richard Biener wrote:
>> On Tue, Jul 25, 2017 at 3:30 PM, Eric Botcazou  wrote:
>> >> Eric, any comments?
>> >
>> > No objection for the build2_stat hunk, I think it's in keeping with the Ada
>> > semantics.  But the tree_could_trap_p hunk is certainly an abomination...
>> >
>> >> We could also avoid tree_could_trap_p by special-casing only
>> >> *_DIV_EXPR and *_MOD_EXPR
>> >> with integer zero 2nd operand explicitely in build2 given there's no
>> >> "constant" value for this.  That is,
>> >> for FP 1./0. is NaN (a "constant" value) even if the operation might trap.
>> >
>> > Yes, that would be faster & simpler and avoids the abomination.
>>
>> Ok, then let's go with that slightly uglier but less abominal variant ;)
>
> Like this then?
>
> Bootstrapped/regtested on x86_64-linux (including Ada) and 
> powerpc64le-unknown-linux-gnu,
> ok for trunk?

Ok.

Thanks,
Richard.

> 2017-07-26  Marek Polacek  
>
> PR middle-end/70992
> * tree.c (build2_stat): Don't set TREE_CONSTANT on divisions by zero.
>
> * gcc.dg/overflow-warn-1.c: Adjust dg-error.
> * gcc.dg/overflow-warn-2.c: Likewise.
> * gcc.dg/overflow-warn-3.c: Likewise.
> * gcc.dg/overflow-warn-4.c: Likewise.
> * gcc.dg/torture/pr70992-2.c: New test.
> * gcc.dg/torture/pr70992.c: New test.
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-1.c 
> gcc/testsuite/gcc.dg/overflow-warn-1.c
> index 8eb322579cf..a5cd5738636 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-1.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-1.c
> @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "25:integer 
> overflow in expression"
>  void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in 
> expression" } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-1 } */
>  void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" 
> "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target 
> *-*-* } .-1 } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-2 } */
>  void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-2.c 
> gcc/testsuite/gcc.dg/overflow-warn-2.c
> index f048d6dae2a..05ab104fa4a 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-2.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-2.c
> @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "integer 
> overflow in expression" }
>  void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in 
> expression" } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-1 } */
>  void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" 
> "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target 
> *-*-* } .-1 } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-2 } */
>  void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-3.c 
> gcc/testsuite/gcc.dg/overflow-warn-3.c
> index 664011e401d..fd4a34f67e2 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-3.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-3.c
> @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer 
> overflow in expression" }
>  /* { dg-warning "overflow in constant expression" "constant" { target *-*-* 
> } .-1 } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-2 } */
>  void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" 
> "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target 
> *-*-* } .-1 } */
>  /* { dg-warning "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-2 } */
>  void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-4.c 
> gcc/testsuite/gcc.dg/overflow-warn-4.c
> index 52677ce897a..018e3e1e4cd 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-4.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-4.c
> @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer 
> overflow in expression" }
>  /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 
> .-1 } */
>  /* { dg-error "initialization makes pointer from integer without a cast" 
> "null" { target *-*-* } .-2 } */
>  void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" 
> "constant" { target *-*-* } .-1 } */
> +/* { d

Re: [PATCH] Switch vec_init and vec_extract optabs to 2 mode optab to allow extraction of vector from vector or initialization of vector from smaller vectors (PR target/80846)

[Segher Boessenkool]

On Wed, Jul 26, 2017 at 09:09:04AM +0200, Jakub Jelinek wrote:
> On Tue, Jul 25, 2017 at 03:52:56PM -0500, Segher Boessenkool wrote:
> > On Tue, Jul 25, 2017 at 11:14:32AM +0200, Jakub Jelinek wrote:
> > > This patch only adds new vector from vector extract and init patterns to
> > > the i386 backend, but I had to change many other targets too, because
> > > it needs to have the element mode in the vec_extract/vec_init expander
> > > names.  Seems most of the backends didn't really have a mode attribute
> > > usable for this or had it only in uppercase, while for the names we need
> > > lowercase.  Some backends had a convention on how to name lower case
> > > vs. upper case modes, others didn't have any.  So I'm CCing maintainers
> > > of affected backends to seek advice on what mode attributes they want to
> > > use.
> > 
> > Would it be possible (and useful) to _also_ keep the old names?  Or do
> > you expect all targets will want to support all combinations?
> 
> Richi's preference was to use only a single conversion optab instead of
> old + new when we've discussed it on IRC.  Of course it would be far less
> work for me to support say:
> OPTAB_CD(vec_extract2_optab, "vec_extract$a$b")
> OPTAB_CD(vec_init2_optab, "vec_init$a$b")
> OPTAB_D (vec_extract_optab, "vec_extract$a")
> OPTAB_D (vec_init_optab, "vec_init$a")
> where the single mode vec_extract/vec_init would be
> extraction/initialization from element mode and the two mode one would be
> used for 2 vector modes.  If there is agreement on that, most of the
> config/*/* changes could go away.

And less work for backends, old as well as new.

> > > @@ -520,6 +520,17 @@ (define_mode_attr VEL [(V8QI "QI") (V16Q
> > >   (SI   "SI") (HI   "HI")
> > >   (QI   "QI")])
> > >  
> > > +;; Define element mode for each vector mode (lower case).
> > > +(define_mode_attr Vel [(V8QI "qi") (V16QI "qi")
> > > + (V4HI "hi") (V8HI "hi")
> > > + (V2SI "si") (V4SI "si")
> > > + (DI "di")   (V2DI "di")
> > > + (V4HF "hf") (V8HF "hf")
> > > + (V2SF "sf") (V4SF "sf")
> > > + (V2DF "df") (DF "df")
> > > + (SI   "si") (HI   "hi")
> > > + (QI   "qi")])
> > 
> > (Inconsistent spacing, please fix).
> 
> It is the same spacing as in VEL right above it, I've tried to follow
> whatever weirdo formatting each backend had.
> 
> > ("vel" instead of "Vel" for this name?)
> 
> That is to follow aarch64 iterator naming convention, where they have

Ugh, for some reason I thought this was in rs6000/ as well.  I have
fresh coffee now.  Sorry.


Segher

Re: [PATCH GCC][1/2]Feed bound computation to folder in loop split

[Richard Biener]

On Wed, Jul 26, 2017 at 9:48 AM, Richard Biener
 wrote:
> On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse  wrote:
>> On Tue, 25 Jul 2017, Richard Biener wrote:
>>
 I think we need Richard to say what the intent is for the valueization
 function. It is used both to stop looking at defining stmt if the return
 is
 NULL, and to replace/optimize one SSA_NAME with another, but currently it
 seems hard to prevent looking at the defining statement without
 preventing
 from looking at the SSA_NAME at all.
>>>
>>>
>>> Yeah, this semantic overloading is an issue.  For gimple_build we have
>>> nothing
>>> to "valueize" but we only use it to tell genmatch that it may not look at
>>> the
>>> SSA_NAME_DEF_STMT.
>>>
 I guess we'll need a fix in genmatch...
>>>
>>>
>>> I'll have a look tomorrow.
>>
>>
>> My impression yesterday was that we could replace the current do_valueize
>> wrapper by 2 wrappers (without touching the valueize callbacks):
>> - may_check_def_stmt, which returns a bool corresponding to the current
>> do_valueize != NULL_TREE
>> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
>> returns its argument unchanged.
>>
>> Not very confident about it though.
>
> Note I've been there in the past (twice I think) but always ran into existing
> latent issues.  So hopefully we've resolved those, I'm testing the following
> simplified variant of what I had back in time.
>
> It'll produce
>
>   switch (TREE_CODE (op0))
> {
> case SSA_NAME:
>   if (gimple *def_stmt = get_def (valueize, op0))
> {
>   if (gassign *def = dyn_cast  (def_stmt))
> switch (gimple_assign_rhs_code (def))
>   {
>   case MINUS_EXPR:
> {
>   tree o20 = gimple_assign_rhs1 (def);
>   o20 = do_valueize (valueize, o20);
>   tree o21 = gimple_assign_rhs2 (def);
>   o21 = do_valueize (valueize, o21);
>   if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
> types_match (op1, o21)))
> {
>
> which also indents less which is nice.
>
> Bootstrap/regtest running on x86_64-unknown-linux-gnu.

Now applied with

* gcc/testsuite/gcc.dg/pr70920-2.c: Adjust for transform already
happening in ccp1.
* gcc/testsuite/gcc.dg/pr70920-4.c: Likewise.

that shows it's working (CCP is one of the passes eventually running
into this issue).

Richard.

> Richard.
>
> 2017-07-26  Richard Biener  
>
> * gimple-match-head.c (do_valueize): Return OP if valueize
> returns NULL_TREE.
> (get_def): New helper to get at the def stmt of a SSA name
> if valueize allows.
> * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
> do_valueize to get at the def stmt.
> (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>
>
>> --
>> Marc Glisse

Re: [COMMITED][AArch64] Fix PR79041

[Wilco Dijkstra]

Andreas Schwab wrote:

> That fails in ILP32 mode.

Well -mabi-ilp32 and -mcmodel=large make no sense at all,
that should really give an error... I've committed a patch to 
trunk and GCC7 to disable this test with ILP32.

Wilco

Re: [PATCH v2] [SPARC] Add -mfsmuld option

[Eric Botcazou]

> Add the -mfsmuld option to control the generation of the FsMULd
> instruction.  In general, this instruction is available in architecture
> version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
> support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
> CPUs of this category do not implement it in hardware, e.g. LEON3/4 with
> GRFPU-lite.
> 
> gcc/
>   * config/sparc/sparc.c (dump_target_flag_bits): Dump MASK_FSMULD.
>   (sparc_option_override): Honour MASK_FSMULD.
>   * config/sparc/sparc.h (MASK_FEATURES): Add MASK_FSMULD.
>   * config/sparc/sparc.md (muldf3_extend): Use TARGET_FSMULD.
>   * config/sparc/sparc.opt (mfsmuld): New option.
>   * doc/invoke.texi (mfsmuld): Document option.

OK for mainline and 7 branch modulo:

> @@ -1511,6 +1513,11 @@ sparc_option_override (void)
>target_flags |= MASK_LONG_DOUBLE_128;
>  }
> 
> +  /* Enable the FSMULD instruction by default if not explicitly configured
> by + the user.  It may be later disabled by the CPU target flags or if
> + !TARGET_FPU.  */
> +  target_flags |= MASK_FSMULD & ~target_flags_explicit;

I think that:

  if (!(target_flags_explicit & MASK_FSMULD))
target_flags |= MASK_FSMULD;

is easier to grasp (and there is a precedent with MASK_LRA a few lines below).

>/* Code model selection.  */
>sparc_cmodel = SPARC_DEFAULT_CMODEL;
> 
> @@ -1603,11 +1610,11 @@ sparc_option_override (void)
>if (TARGET_VIS4B)
>  target_flags |= MASK_VIS4 | MASK_VIS3 | MASK_VIS2 | MASK_VIS;
> 
> -  /* Don't allow -mvis, -mvis2, -mvis3, -mvis4, -mvis4b and -mfmaf if
> +  /* Don't allow -mvis, -mvis2, -mvis3, -mvis4, -mvis4b, -mfmaf and
> -mfsmuld if FPU is disabled.  */
>if (! TARGET_FPU)
>  target_flags &= ~(MASK_VIS | MASK_VIS2 | MASK_VIS3 | MASK_VIS4
> -   | MASK_VIS4B | MASK_FMAF);
> +   | MASK_VIS4B | MASK_FMAF | MASK_FSMULD);
> 
>/* -mvis assumes UltraSPARC+, so we are sure v9 instructions
>   are available; -m64 also implies v9.  */
> @@ -1641,6 +1648,9 @@ sparc_option_override (void)
>if (sparc_fix_ut699 || sparc_fix_ut700 || sparc_fix_gr712rc)
>  sparc_fix_b2bst = 1;
> 
> +  if (sparc_fix_ut699)
> +target_flags &= ~MASK_FSMULD;

Add a stupid comment line here, something like:

"Disable FsMULd for the UT699 since it doesn't work correctly."

-- 
Eric Botcazou

[PATCH 1/2, c-family]: Add 'z' to asm_fprintf_char_table

[Uros Bizjak]

Hello!

This patch is the prerequisite for my next patch. It enables %z
extension for asm_fprintf that will emit 'q' or 'l' suffixes for
instructions with word-sized operands.

2017-07-26  Uros Bizjak  

* c-format.c (asm_fprintf_char_table): Add 'z' to format_chars.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for mainline?

Uros.
Index: c-format.c
===
--- c-format.c  (revision 250563)
+++ c-format.c  (working copy)
@@ -672,6 +672,7 @@ static const format_char_info asm_fprintf_char_tab
   { "L",   0, STD_C89, NOARGUMENTS, "",  "",   NULL },
   { "U",   0, STD_C89, NOARGUMENTS, "",  "",   NULL },
   { "r",   0, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",  "", NULL },
+  { "z",   0, STD_C89, NOARGUMENTS, "",  "",   NULL },
   { "@",   0, STD_C89, NOARGUMENTS, "",  "",   NULL },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };

[PATCH 2/2, i386]: Introduce ASM_FPRINTF_EXTENSIONS and simplify ASM_OUTPUT_REG_{PUSH|POP}

[Uros Bizjak]

Hello!

Attached patch introduces a couple of asm_fprintf extensions to
simplify handling of word-mode operands.

2017-07-26  Uros Bizjak  

* config/i386/i386.h (ASM_PRINTF_EXTENSIONS): New macro.
(ASM_OUTPUT_REG_PUSH): Rewrite with new operand modifiers.
(ASM_OUTPUT_REG_POP): Ditto.
* config/i386/i386.c (ix86_asm_output_function_label): Use fputs
instead of asm_fprintf to output pure string.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 250563)
+++ config/i386/i386.c  (working copy)
@@ -8777,8 +8777,8 @@ ix86_asm_output_function_label (FILE *asm_out_file
   if (TARGET_64BIT)
{
  /* leaq [%rsp + 0], %rsp  */
- asm_fprintf (asm_out_file, ASM_BYTE
-  "0x48, 0x8d, 0xa4, 0x24, 0x00, 0x00, 0x00, 0x00\n");
+ fputs (ASM_BYTE "0x48, 0x8d, 0xa4, 0x24, 0x00, 0x00, 0x00, 0x00\n",
+asm_out_file);
}
   else
{
@@ -8785,8 +8785,7 @@ ix86_asm_output_function_label (FILE *asm_out_file
   /* movl.s %edi, %edi
 push   %ebp
 movl.s %esp, %ebp */
- asm_fprintf (asm_out_file, ASM_BYTE
-  "0x8b, 0xff, 0x55, 0x8b, 0xec\n");
+ fputs (ASM_BYTE "0x8b, 0xff, 0x55, 0x8b, 0xec\n", asm_out_file);
}
 }
 }
Index: config/i386/i386.h
===
--- config/i386/i386.h  (revision 250563)
+++ config/i386/i386.h  (working copy)
@@ -2196,29 +2196,33 @@ extern int const svr4_dbx_register_map[FIRST_PSEUD
 #define ASM_PREFERRED_EH_DATA_FORMAT(CODE, GLOBAL) \
   asm_preferred_eh_data_format ((CODE), (GLOBAL))
 
-/* This is how to output an insn to push a register on the stack.
-   It need not be very fast code.  */
+/* These are a couple of extensions to the formats accepted
+   by asm_fprintf:
+ %z prints out opcode suffix for word-mode instruction
+ %r prints out word-mode name for reg_names[arg]  */
+#define ASM_FPRINTF_EXTENSIONS(FILE, ARGS, P)  \
+  case 'z':\
+fputc (TARGET_64BIT ? 'q' : 'l', (FILE));  \
+break; \
+   \
+  case 'r':\
+{  \
+  unsigned int regno = va_arg ((ARGS), int);   \
+  if (LEGACY_INT_REGNO_P (regno))  \
+   fputc (TARGET_64BIT ? 'r' : 'e', (FILE));   \
+  fputs (reg_names[regno], (FILE));\
+  break;   \
+}
 
-#define ASM_OUTPUT_REG_PUSH(FILE, REGNO)  \
-do {   \
-  if (TARGET_64BIT)\
-asm_fprintf ((FILE), "\tpush{q}\t%%r%s\n", \
-reg_names[(REGNO)] + (REX_INT_REGNO_P (REGNO) != 0));  \
-  else \
-asm_fprintf ((FILE), "\tpush{l}\t%%e%s\n", reg_names[(REGNO)]);\
-} while (0)
+/* This is how to output an insn to push a register on the stack.  */
 
-/* This is how to output an insn to pop a register from the stack.
-   It need not be very fast code.  */
+#define ASM_OUTPUT_REG_PUSH(FILE, REGNO)   \
+  asm_fprintf ((FILE), "\tpush%z\t%%%r\n", (REGNO))
 
+/* This is how to output an insn to pop a register from the stack.  */
+
 #define ASM_OUTPUT_REG_POP(FILE, REGNO)  \
-do {   \
-  if (TARGET_64BIT)\
-asm_fprintf ((FILE), "\tpop{q}\t%%r%s\n",  \
-reg_names[(REGNO)] + (REX_INT_REGNO_P (REGNO) != 0));  \
-  else \
-asm_fprintf ((FILE), "\tpop{l}\t%%e%s\n", reg_names[(REGNO)]); \
-} while (0)
+  asm_fprintf ((FILE), "\tpop%z\t%%%r\n", (REGNO))
 
 /* This is how to output an element of a case-vector that is absolute.  */
 

Re: [PATCH v2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

On 26/07/17 14:13, Eric Botcazou wrote:


Add the -mfsmuld option to control the generation of the FsMULd
instruction.  In general, this instruction is available in architecture
version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
CPUs of this category do not implement it in hardware, e.g. LEON3/4 with
GRFPU-lite.

gcc/
* config/sparc/sparc.c (dump_target_flag_bits): Dump MASK_FSMULD.
(sparc_option_override): Honour MASK_FSMULD.
* config/sparc/sparc.h (MASK_FEATURES): Add MASK_FSMULD.
* config/sparc/sparc.md (muldf3_extend): Use TARGET_FSMULD.
* config/sparc/sparc.opt (mfsmuld): New option.
* doc/invoke.texi (mfsmuld): Document option.

OK for mainline and 7 branch modulo:



Thanks for your quick review. I am really glad that we can now use the 
upcoming GCC 7.2 release.


I checked it in as r250570 and r250571.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

[PATCH,AIX] Enable Go for AIX

[REIX, Tony]

Description:
 * This patch enables Go on AIX.

Tests:
 * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
   - build made by means of gmake.

ChangeLog:
 * configure.ac, configure: Enable Go for AIX
 * contrib/config-list.mk: Enable Go for AIX


Cordialement,

Tony Reix

Bull - ATOS
IBM Coop Architect & Technical Leader

Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.net






Index: ChangeLog
===
--- ChangeLog	(révision 250563)
+++ ChangeLog	(copie de travail)
@@ -1,3 +1,7 @@
+2017-07-26  Tony Reix  
+
+	* configure.ac, configure: Enable Go for AIX
+
 2017-07-19  Yury Gribov  
 
 	* MAINTAINERS: Add myself.
Index: configure
===
--- configure	(révision 250563)
+++ configure	(copie de travail)
@@ -3480,7 +3480,7 @@ esac
 # Disable the go frontend on systems where it is known to not work. Please keep
 # this in sync with contrib/config-list.mk.
 case "${target}" in
-*-*-darwin* | *-*-cygwin* | *-*-mingw* | *-*-aix*)
+*-*-darwin* | *-*-cygwin* | *-*-mingw*)
 unsupported_languages="$unsupported_languages go"
 ;;
 esac
@@ -3496,9 +3496,6 @@ if test x$enable_libgo = x; then
 *-*-cygwin* | *-*-mingw*)
 	noconfigdirs="$noconfigdirs target-libgo"
 	;;
-*-*-aix*)
-	noconfigdirs="$noconfigdirs target-libgo"
-	;;
 esac
 fi
 
Index: configure.ac
===
--- configure.ac	(révision 250563)
+++ configure.ac	(copie de travail)
@@ -808,7 +808,7 @@ esac
 # Disable the go frontend on systems where it is known to not work. Please keep
 # this in sync with contrib/config-list.mk.
 case "${target}" in
-*-*-darwin* | *-*-cygwin* | *-*-mingw* | *-*-aix*)
+*-*-darwin* | *-*-cygwin* | *-*-mingw*)
 unsupported_languages="$unsupported_languages go"
 ;;
 esac
@@ -824,9 +824,6 @@ if test x$enable_libgo = x; then
 *-*-cygwin* | *-*-mingw*)
 	noconfigdirs="$noconfigdirs target-libgo"
 	;;
-*-*-aix*)
-	noconfigdirs="$noconfigdirs target-libgo"
-	;;
 esac
 fi
 
Index: contrib/ChangeLog
===
--- contrib/ChangeLog	(révision 250563)
+++ contrib/ChangeLog	(copie de travail)
@@ -1,3 +1,7 @@
+2017-07-26  Tony Reix  
+
+	* config-list.mk: Enable Go for AIX
+
 2017-07-17  Yury Gribov  
 
 	* mklog: Fix extraction of changed file name.
Index: contrib/config-list.mk
===
--- contrib/config-list.mk	(révision 250563)
+++ contrib/config-list.mk	(copie de travail)
@@ -121,7 +121,7 @@ $(LIST): make-log-dir
 		TGT=`echo $@ | awk 'BEGIN { FS = "OPT" }; { print $$1 }'` &&			\
 		TGT=`$(GCC_SRC_DIR)/config.sub $$TGT` &&	\
 		case $$TGT in	\
-			*-*-darwin* | *-*-cygwin* | *-*-mingw* | *-*-aix*)			\
+			*-*-darwin* | *-*-cygwin* | *-*-mingw*)	\
 ADDITIONAL_LANGUAGES="";	\
 ;;\
 			*)	\

Re: [PATCH v2] [SPARC] Add -mfsmuld option

[Sebastian Huber]

On 26/07/17 14:44, Sebastian Huber wrote:


On 26/07/17 14:13, Eric Botcazou wrote:


Add the -mfsmuld option to control the generation of the FsMULd
instruction.  In general, this instruction is available in architecture
version V8 and V9 CPUs with FPU.  Some CPUs of this category do not
support this instruction properly, e.g. AT697E, AT697F and UT699.  Some
CPUs of this category do not implement it in hardware, e.g. LEON3/4 
with

GRFPU-lite.

gcc/
* config/sparc/sparc.c (dump_target_flag_bits): Dump MASK_FSMULD.
(sparc_option_override): Honour MASK_FSMULD.
* config/sparc/sparc.h (MASK_FEATURES): Add MASK_FSMULD.
* config/sparc/sparc.md (muldf3_extend): Use TARGET_FSMULD.
* config/sparc/sparc.opt (mfsmuld): New option.
* doc/invoke.texi (mfsmuld): Document option.

OK for mainline and 7 branch modulo:



Thanks for your quick review. I am really glad that we can now use the 
upcoming GCC 7.2 release.


I checked it in as r250570 and r250571.



I would like to add this to the web site:

Index: htdocs/gcc-7/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/changes.html,v
retrieving revision 1.88
diff -r1.88 changes.html
1248a1249,1251
> Use of the Floating-point Multiply Single to Double (FsMULd)
> instruction can now be controlled by the 
-mfsmuld and

> -fno-fsmuld options.
cvs diff: Diffing htdocs/gcc-8
Index: htdocs/gcc-8/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-8/changes.html,v
retrieving revision 1.7
diff -r1.7 changes.html
148,149c148,153
< 
<
---
> SPARC
> 
>   Use of the Floating-point Multiply Single to Double (FsMULd) 
instruction

>   can now be controlled by the -mfsmuld and
>   -fno-fsmuld options.
> 

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

[PATCH][AArch64] Remove '*' from movsi/di/ti patterns

[Wilco Dijkstra]

Remove the remaining uses of '*' from the movsi/di/ti patterns.
Using '*' in alternatives is typically incorrect at it tells the register
allocator to ignore those alternatives.  So remove these from all the
integer move patterns.  This removes unnecessary int to float moves, for
example gcc.target/aarch64/pr62178.c no longer generates a redundant fmov
since the w = m variant is now allowed.

Passes regress & bootstrap, OK for commit?

ChangeLog:
2017-07-26  Wilco Dijkstra  

* gcc/config/aarch64/aarch64.md (movsi_aarch64): Remove all '*'.
(movdi_aarch64): Likewise.
(movti_aarch64): Likewise.
--

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 
225b64e1daf1663d28bbe8c2d30ba373b4722176..97c5fb08a2fd5d2eee556e1fc20dbf65b089d84b
 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -920,8 +920,8 @@ (define_expand "mov"
 )
 
 (define_insn_and_split "*movsi_aarch64"
-  [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r,*w,m,  m,r,r  
,*w,r,*w")
-   (match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,n,m, 
m,rZ,*w,Usa,Ush,rZ,w,*w"))]
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,r,w,m, m,  r,  
r, w,r,w")
+   (match_operand:SI 1 "aarch64_mov_operand"  " 
r,r,k,M,n,m,m,rZ,w,Usa,Ush,rZ,w,w"))]
   "(register_operand (operands[0], SImode)
 || aarch64_reg_or_zero (operands[1], SImode))"
   "@
@@ -952,8 +952,8 @@ (define_insn_and_split "*movsi_aarch64"
 )
 
 (define_insn_and_split "*movdi_aarch64"
-  [(set (match_operand:DI 0 "nonimmediate_operand" "=r,k,r,r,r,r,*w,m,  m,r,r, 
 *w,r,*w,w")
-   (match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,n,m, 
m,rZ,*w,Usa,Ush,rZ,w,*w,Dd"))]
+  [(set (match_operand:DI 0 "nonimmediate_operand" "=r,k,r,r,r,r,w, m,m,  r,  
r, w,r,w,w")
+   (match_operand:DI 1 "aarch64_mov_operand"  " 
r,r,k,N,n,m,m,rZ,w,Usa,Ush,rZ,w,w,Dd"))]
   "(register_operand (operands[0], DImode)
 || aarch64_reg_or_zero (operands[1], DImode))"
   "@
@@ -1008,9 +1008,9 @@ (define_expand "movti"
 
 (define_insn "*movti_aarch64"
   [(set (match_operand:TI 0
-"nonimmediate_operand"  "=r, *w,r ,*w,r,m,m,*w,m")
+"nonimmediate_operand"  "=r, w,r,w,r,m,m,w,m")
(match_operand:TI 1
-"aarch64_movti_operand" " rn,r ,*w,*w,m,r,Z, m,*w"))]
+"aarch64_movti_operand" " rn,r,w,w,m,r,Z,m,w"))]
   "(register_operand (operands[0], TImode)
 || aarch64_reg_or_zero (operands[1], TImode))"
   "@

Re: Patch ping

[Jakub Jelinek]

On Wed, Jul 26, 2017 at 12:34:10PM +0200, Richard Biener wrote:
> On Tue, 25 Jul 2017, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > I'd like to ping 2 patches:
> > 
> > - UBSAN -fsanitize=pointer-overflow support
> >   - http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01365.html
> 
> The probablility stuff might need updating?

Yes, done in my copy.

> Can you put the TYPE_PRECISION (sizetype) != POINTER_SIZE check
> in option processing and inform people that pointer overflow sanitizing
> is not done instead?

That is problematic, because during the option processing sizetype
is NULL, it is set up only much later.  And that processing depends on
the options being finalized and backend initialized etc.
I guess I could emit a warning in the sanopt pass once or something.
Or do it very late in do_compile, after lang_dependent_init (we don't
handle any other option there though). 
But it is still just a theoretical case, because libubsan is supported
only on selected subset of targets and none of those have such weird
sizetype vs. pointer size setup.  And without libubsan, the only thing
one could do would be -fsanitize=undefined -fsanitize-undefined-trap-on-error
or -fsanitize=pointer-overflow -fsanitize-undefined-trap-on-error,
otherwise one would run into missing libubsan.

> Where you handle DECL_BIT_FIELD_REPRESENTATIVE in 
> maybe_instrument_pointer_overflow you could instead of building
> a new COMPONENT_REF strip the bitfield ref and just remember
> DECL_FIELD_OFFSET/BIT_OFFSET to be added to the get_inner_reference
> result?

That is not enough, the bitfield could be in variable length structure etc.
Furthermore, I need that COMPONENT_REF with the representative later
in the function, so that I can compute the ptr and base_addr.

>  You don't seem to use 'size' anywhere.

size I thought about but then decided not to do anything with it.
There are two cases, one is where there is no ADDR_EXPR and it actually
a memory reference.  
In that case in theory the size could be used, but it would need
to be used only for the positive offsets, so like:
if (off > 0) {
  if (ptr + off + size < ptr)
runtime_fail;
} else if (ptr + off > ptr)
  runtime_fail;
but when it is actually a memory reference, I suppose it will fail
at runtime anyway when performing such an access, so I think it is
unnecessary.  And for the ADDR_EXPR case, the size is irrelevant, we
are just taking address of the start of the object.

> You fail to allow other handled components -- for no good reason?

I was trying to have a quick bail out.  What other handled components might
be relevant?  I guess IMAGPART_EXPR.  For say BIT_FIELD_REF I don't think
I can
  tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);

>  You fail to handle
> &MEM[ptr + CST] a canonical gimple invariant way of ptr +p CST,
> the early out bitpos == 0 will cause non-instrumentation here.

Guess I could use:
  if ((offset == NULL_TREE
   && bitpos == 0
   && (TREE_CODE (inner) != MEM_REF
   || integer_zerop (TREE_OPERAND (inner, 1
The rest of the code will handle it.

> (I'd just round down in the case of bitpos % BITS_PER_UNIT != 0)

But then the
  tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);
won't work again.

> > - noipa attribute addition  
> >
> >   http://gcc.gnu.org/ml/gcc-patches/2016-12/msg01501.html   
> >
> 
> Ok.

Thanks, will retest it now.

Here is the -fsanitize=pointer-overflow patch untested, updated
for the probability and other stuff mentioned above.

Jakub
2017-07-26  Jakub Jelinek  

PR sanitizer/80998
* sanopt.c (pass_sanopt::execute): Handle IFN_UBSAN_PTR.
* tree-ssa-alias.c (call_may_clobber_ref_p_1): Likewise.
* flag-types.h (enum sanitize_code): Add SANITIZER_POINTER_OVERFLOW.
Or it into SANITIZER_UNDEFINED.
* ubsan.c: Include gimple-fold.h and varasm.h.
(ubsan_expand_ptr_ifn): New function.
(instrument_pointer_overflow): New function.
(maybe_instrument_pointer_overflow): New function.
(instrument_object_size): Formatting fix.
(pass_ubsan::execute): Call instrument_pointer_overflow
and maybe_instrument_pointer_overflow.
* internal-fn.c (expand_UBSAN_PTR): New function.
* ubsan.h (ubsan_expand_ptr_ifn): Declare.
* sanitizer.def (__ubsan_handle_pointer_overflow,
__ubsan_handle_pointer_overflow_abort): New builtins.
* tree-ssa-tail-merge.c (merge_stmts_p): Handle IFN_UBSAN_PTR.
* internal-fn.def (UBSAN_PTR): New internal function.
* opts.c (sanitizer_opts): Add pointer-overflow.
* lto-streamer-in.c (input_function): Handle IFN_UBSAN_PTR.
* fold-const.c (build_range_check): Compute pointer range check in

[PATCH] Backport to GCC7

[Martin Liška]

Hi.

I'm going to install following 3 revision in order to fix GCC.x branch:

0001-Backport-r249728.patch
0002-Backport-r249833.patch
0003-Backport-r250561.patch

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Martin
>From ce5f2421462ebf6bc02eec9729502671c3483883 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 26 Jul 2017 08:52:37 +
Subject: [PATCH 3/3] Backport r250561

gcc/ChangeLog:

2017-07-26  Martin Liska  

	PR sanitize/81186
	* function.c (expand_function_start): Make expansion of
	nonlocal_goto_save_area after parm_birth_insn.

gcc/testsuite/ChangeLog:

2017-07-26  Martin Liska  

	PR sanitize/81186
	* gcc.dg/asan/pr81186.c: New test.
---
 gcc/function.c  | 20 ++--
 gcc/testsuite/gcc.dg/asan/pr81186.c | 18 ++
 2 files changed, 28 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/asan/pr81186.c

diff --git a/gcc/function.c b/gcc/function.c
index f625489205b..9cfe58afe90 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5263,6 +5263,16 @@ expand_function_start (tree subr)
 	}
 }
 
+  /* The following was moved from init_function_start.
+ The move is supposed to make sdb output more accurate.  */
+  /* Indicate the beginning of the function body,
+ as opposed to parm setup.  */
+  emit_note (NOTE_INSN_FUNCTION_BEG);
+
+  gcc_assert (NOTE_P (get_last_insn ()));
+
+  parm_birth_insn = get_last_insn ();
+
   /* If the function receives a non-local goto, then store the
  bits we need to restore the frame pointer.  */
   if (cfun->nonlocal_goto_save_area)
@@ -5284,16 +5294,6 @@ expand_function_start (tree subr)
   update_nonlocal_goto_save_area ();
 }
 
-  /* The following was moved from init_function_start.
- The move is supposed to make sdb output more accurate.  */
-  /* Indicate the beginning of the function body,
- as opposed to parm setup.  */
-  emit_note (NOTE_INSN_FUNCTION_BEG);
-
-  gcc_assert (NOTE_P (get_last_insn ()));
-
-  parm_birth_insn = get_last_insn ();
-
   if (crtl->profile)
 {
 #ifdef PROFILE_HOOK
diff --git a/gcc/testsuite/gcc.dg/asan/pr81186.c b/gcc/testsuite/gcc.dg/asan/pr81186.c
new file mode 100644
index 000..7f0f672ca40
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr81186.c
@@ -0,0 +1,18 @@
+/* PR sanitizer/81186 */
+/* { dg-do run } */
+
+int
+main ()
+{
+  __label__ l;
+  void f ()
+  {
+int a[123];
+
+goto l;
+  }
+
+  f ();
+l:
+  return 0;
+}
-- 
2.13.3

>From dfb00296c1467623c3a5355834276309ba5ee3f9 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 30 Jun 2017 08:51:00 +
Subject: [PATCH 2/3] Backport r249833

gcc/ChangeLog:

2017-06-30  Martin Liska  

	PR sanitizer/81021
	* tree-eh.c (lower_resx): Call BUILT_IN_ASAN_HANDLE_NO_RETURN
	before BUILT_IN_UNWIND_RESUME when ASAN is used.

gcc/testsuite/ChangeLog:

2017-06-30  Martin Liska  

	PR sanitizer/81021
	* g++.dg/asan/pr81021.C: New test.
---
 gcc/testsuite/g++.dg/asan/pr81021.C | 33 +
 gcc/tree-eh.c   | 15 +++
 2 files changed, 48 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/asan/pr81021.C

diff --git a/gcc/testsuite/g++.dg/asan/pr81021.C b/gcc/testsuite/g++.dg/asan/pr81021.C
new file mode 100644
index 000..4904f19bf84
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/pr81021.C
@@ -0,0 +1,33 @@
+// { dg-do run }
+
+#include 
+
+struct ConfigFile {
+ConfigFile(std::string filename, std::string delimiter) { throw "error"; }
+ConfigFile(std::string filename) {}
+};
+
+struct Configuration {
+ConfigFile _configFile;
+
+Configuration(const std::string &root, const char *baseName)
+: _configFile(root + baseName, "=") { }
+Configuration(const std::string &root, const char *a, const char *b)
+: _configFile(root + a + b) { }
+};
+
+
+void test() {
+std::string root("etc");
+try {
+Configuration config(root, "notthere");
+}
+catch (...) {
+// exception is thrown, caught here and ignored...
+}
+Configuration config(root, "a", "b"); // ASAN error during constructor here
+}
+
+int main(int argc, const char *argv[]) {
+test();
+}
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index fc016d795b7..ad50b32220e 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "langhooks.h"
 #include "cfgloop.h"
 #include "gimple-low.h"
+#include "asan.h"
 
 /* In some instances a tree and a gimple need to be stored in a same table,
i.e. in hash tables. This is a structure to do this. */
@@ -3304,6 +3305,20 @@ lower_resx (basic_block bb, gresx *stmt,
 	  gimple_call_set_lhs (x, var);
 	  gsi_insert_before (&gsi, x, GSI_SAME_STMT);
 
+	  /* When exception handling is delegated to a caller function, we
+	 have to guarantee that shadow memory variables living on stack
+	 will be cleaner before control is given to a parent function.  */
+	  if ((flag_san

Re: [PATCH v2] [SPARC] Add -mfsmuld option

[Eric Botcazou]

> I would like to add this to the web site:
> 
> Index: htdocs/gcc-7/changes.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-7/changes.html,v
> retrieving revision 1.88
> diff -r1.88 changes.html
> 1248a1249,1251
> 
>  > Use of the Floating-point Multiply Single to Double (FsMULd)
>  > 
>  > instruction can now be controlled by the
> 
> -mfsmuld and
> 
>  > -fno-fsmuld options.
> 

We document changes for only one release so the above is sufficient and OK.

-- 
Eric Botcazou

Re: Patch ping

[Richard Biener]

On Wed, 26 Jul 2017, Jakub Jelinek wrote:

> On Wed, Jul 26, 2017 at 12:34:10PM +0200, Richard Biener wrote:
> > On Tue, 25 Jul 2017, Jakub Jelinek wrote:
> > 
> > > Hi!
> > > 
> > > I'd like to ping 2 patches:
> > > 
> > > - UBSAN -fsanitize=pointer-overflow support
> > >   - http://gcc.gnu.org/ml/gcc-patches/2017-06/msg01365.html
> > 
> > The probablility stuff might need updating?
> 
> Yes, done in my copy.
> 
> > Can you put the TYPE_PRECISION (sizetype) != POINTER_SIZE check
> > in option processing and inform people that pointer overflow sanitizing
> > is not done instead?
> 
> That is problematic, because during the option processing sizetype
> is NULL, it is set up only much later.  And that processing depends on

Ah, ok - fine then as-is.

> the options being finalized and backend initialized etc.
> I guess I could emit a warning in the sanopt pass once or something.
> Or do it very late in do_compile, after lang_dependent_init (we don't
> handle any other option there though). 
> But it is still just a theoretical case, because libubsan is supported
> only on selected subset of targets and none of those have such weird
> sizetype vs. pointer size setup.  And without libubsan, the only thing
> one could do would be -fsanitize=undefined -fsanitize-undefined-trap-on-error
> or -fsanitize=pointer-overflow -fsanitize-undefined-trap-on-error,
> otherwise one would run into missing libubsan.
>
> > Where you handle DECL_BIT_FIELD_REPRESENTATIVE in 
> > maybe_instrument_pointer_overflow you could instead of building
> > a new COMPONENT_REF strip the bitfield ref and just remember
> > DECL_FIELD_OFFSET/BIT_OFFSET to be added to the get_inner_reference
> > result?
> 
> That is not enough, the bitfield could be in variable length structure etc.
> Furthermore, I need that COMPONENT_REF with the representative later
> in the function, so that I can compute the ptr and base_addr.

Ok.
 
> >  You don't seem to use 'size' anywhere.
> 
> size I thought about but then decided not to do anything with it.
> There are two cases, one is where there is no ADDR_EXPR and it actually
> a memory reference.  
> In that case in theory the size could be used, but it would need
> to be used only for the positive offsets, so like:
> if (off > 0) {
>   if (ptr + off + size < ptr)
> runtime_fail;
> } else if (ptr + off > ptr)
>   runtime_fail;
> but when it is actually a memory reference, I suppose it will fail
> at runtime anyway when performing such an access, so I think it is
> unnecessary.  And for the ADDR_EXPR case, the size is irrelevant, we
> are just taking address of the start of the object.
> 
> > You fail to allow other handled components -- for no good reason?
> 
> I was trying to have a quick bail out.  What other handled components might
> be relevant?  I guess IMAGPART_EXPR.  For say BIT_FIELD_REF I don't think
> I can
>   tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);

REALPART/IMAGPART_EXPR, yes.  You can't address BIT_FIELD_REF
apart those on byte boundary (&vector[4] is eventually folded to
a BIT_FIELD_REF).  Similar for VIEW_CONVERT_EXPR, but you are
only building the address on the base?

> >  You fail to handle
> > &MEM[ptr + CST] a canonical gimple invariant way of ptr +p CST,
> > the early out bitpos == 0 will cause non-instrumentation here.
> 
> Guess I could use:
>   if ((offset == NULL_TREE
>&& bitpos == 0
>&& (TREE_CODE (inner) != MEM_REF
>  || integer_zerop (TREE_OPERAND (inner, 1
> The rest of the code will handle it.

Yeah.

> 
> > (I'd just round down in the case of bitpos % BITS_PER_UNIT != 0)
> 
> But then the
>   tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);
> won't work again.

Hmm.  So instead of building the address on the original tree you
could build the difference based on what get_inner_reference returns
in bitpos/offset?

> > > - noipa attribute addition
> > >  
> > >   http://gcc.gnu.org/ml/gcc-patches/2016-12/msg01501.html 
> > >  
> > 
> > Ok.
> 
> Thanks, will retest it now.
> 
> Here is the -fsanitize=pointer-overflow patch untested, updated
> for the probability and other stuff mentioned above.
> 
>   Jakub
> 

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

Re: [PATCH v12] add -fpatchable-function-entry=N,M option

[Andreas Schwab]

On Jul 07 2017, Torsten Duwe  wrote:

> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c 
> b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
> new file mode 100644
> index 000..8514b10e820
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> +/* { dg-final { scan-assembler-times "nop" 2 } } */

This fails on ia64.

> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c 
> b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c
> new file mode 100644
> index 000..0dcf1181dde
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> +/* { dg-final { scan-assembler-times "nop" 3 } } */

Likewise.

> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c 
> b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c
> new file mode 100644
> index 000..a007867dcb0
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> +/* { dg-final { scan-assembler-times "nop" 1 } } */

Likewise.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] PR libstdc++/53984 handle exceptions in basic_istream::sentry

[Andreas Schwab]

On Jul 25 2017, Jonathan Wakely  wrote:

> diff --git a/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc 
> b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
> new file mode 100644
> index 000..e84072e
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
> @@ -0,0 +1,36 @@
> +// Copyright (C) 2017 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3.  If not see
> +// .
> +
> +// { dg-require-file-io "" }

ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: dg-require-file-io 18 
{} for " dg-require-file-io 18 "" "

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v12] add -fpatchable-function-entry=N,M option

[Torsten Duwe]

On Wed, Jul 26, 2017 at 04:16:25PM +0200, Andreas Schwab wrote:
> On Jul 07 2017, Torsten Duwe  wrote:
> 
> > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c 
> > b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
> > new file mode 100644
> > index 000..8514b10e820
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
> > +/* { dg-final { scan-assembler-times "nop" 2 } } */
> 
> This fails on ia64.

The solution is fairly obvious: on architectures where the nop is not called
"nop" provide a custom, cpu-specific test, or document the failure.

Torsten

Re: [PATCH] PR libstdc++/53984 handle exceptions in basic_istream::sentry

[Paolo Carlini]

Hi,

On 26/07/2017 16:21, Andreas Schwab wrote:
ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: 
dg-require-file-io 18 {} for " dg-require-file-io 18 "" " 

Should be already fixed, a trivial typo.

Thanks,
Paolo.

Re: [PATCH v12] add -fpatchable-function-entry=N,M option

[Andreas Schwab]

On Jul 26 2017, Torsten Duwe  wrote:

> On Wed, Jul 26, 2017 at 04:16:25PM +0200, Andreas Schwab wrote:
>> On Jul 07 2017, Torsten Duwe  wrote:
>> 
>> > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c 
>> > b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
>> > new file mode 100644
>> > index 000..8514b10e820
>> > --- /dev/null
>> > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c
>> > @@ -0,0 +1,16 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
>> > +/* { dg-final { scan-assembler-times "nop" 2 } } */
>> 
>> This fails on ia64.
>
> The solution is fairly obvious: on architectures where the nop is not called
> "nop" provide a custom, cpu-specific test, or document the failure.

But on ia64, a nop _is_ called nop.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 12/17] Add server.h and server.c

[Oleg Endo]

On Mon, 2017-07-24 at 16:05 -0400, David Malcolm wrote:
> 
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +.  */
> +
> +#ifndef GCC_SERVER_H
> +#define GCC_SERVER_H
> +
> +/* Wrapper aroung "int" for file descriptors.  */
                ~~~^
              around :)

Re: [PATCH 12/17] Add server.h and server.c

[David Malcolm]

On Wed, 2017-07-26 at 23:35 +0900, Oleg Endo wrote:
> On Mon, 2017-07-24 at 16:05 -0400, David Malcolm wrote:
> >  
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with GCC; see the file COPYING3.  If not see
> > +.  */
> > +
> > +#ifndef GCC_SERVER_H
> > +#define GCC_SERVER_H
> > +
> > +/* Wrapper aroung "int" for file descriptors.  */
> ~~~^
>   around :)

Thanks; fixed in my working copy.

Someone pointed out to me privately that instead/as well as serving on
a port, we could be launched as a subprocess by the IDE, and serve the
RPC over stdin/stdout; this would be simpler for IDEs to cope with.  I
may have a look at supporting that for the next version.

Dave

[PATCH,AIX] Enable libffi for AIX

[REIX, Tony]

Description:
 * This patch enables libffi on AIX.

Tests:
 * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
   - build made by means of gmake.

ChangeLog:
 * configure.ac, configure: Enable libffi for AIX

Cordialement,

Tony Reix

Bull - ATOS
IBM Coop Architect & Technical Leader
Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.netIndex: ChangeLog
===
--- ChangeLog	(révision 250563)
+++ ChangeLog	(copie de travail)
@@ -1,3 +1,8 @@
+2017-07-26  Tony Reix  
+
+	* configure.ac, configure: Enable Go for AIX
+	* configure.ac, configure: Enable libffi for AIX
+
 2017-07-19  Yury Gribov  
 
 	* MAINTAINERS: Add myself.
Index: configure
===
--- configure	(révision 250563)
+++ configure	(copie de travail)
@@ -3463,11 +3463,8 @@ case "${target}" in
 noconfigdirs="$noconfigdirs target-libffi"
 ;;
   powerpc-*-aix*)
-# copied from rs6000-*-* entry
-noconfigdirs="$noconfigdirs target-libffi"
 ;;
   rs6000-*-aix*)
-noconfigdirs="$noconfigdirs target-libffi"
 ;;
   ft32-*-*)
 noconfigdirs="$noconfigdirs target-libffi"
Index: configure.ac
===
--- configure.ac	(révision 250563)
+++ configure.ac	(copie de travail)
@@ -791,11 +791,8 @@ case "${target}" in
 noconfigdirs="$noconfigdirs target-libffi"
 ;;
   powerpc-*-aix*)
-# copied from rs6000-*-* entry
-noconfigdirs="$noconfigdirs target-libffi"
 ;;
   rs6000-*-aix*)
-noconfigdirs="$noconfigdirs target-libffi"
 ;;
   ft32-*-*)
 noconfigdirs="$noconfigdirs target-libffi"

RE: [PATCH][GCC][AArch64] optimize float immediate moves (1 /4) - infrastructure.

[Tamar Christina]

Hi James,

I have updated the patch and have responded to your question blow.

Ok for trunk?

Thanks,
Tamar

> >  static bool
> > @@ -5857,12 +5955,6 @@ aarch64_preferred_reload_class (rtx x,
> reg_class_t regclass)
> >return NO_REGS;
> >  }
> >
> > -  /* If it's an integer immediate that MOVI can't handle, then
> > - FP_REGS is not an option, so we return NO_REGS instead.  */
> > -  if (CONST_INT_P (x) && reg_class_subset_p (regclass, FP_REGS)
> > -  && !aarch64_simd_imm_scalar_p (x, GET_MODE (x)))
> > -return NO_REGS;
> > -
> 
> I don't understand this relaxation could you explain what this achieves and
> why it is safe in this patch?

Because this should be left up to the pattern to decide what should happen and 
not reload.
Leaving the check here also means you'll do a reasonably expensive check twice 
for each constant
you can emit a move for.

Removing extra restriction on the constant classes leaves it up to 
aarch64_legitimate_constant_p to decide if
if the constant can be emitted as a move or should be forced to memory.
aarch64_legitimate_constant_p also calls aarch64_cannot_force_const_mem.

The documentation for TARGET_PREFERRED_RELOAD_CLASS also states: 

"One case where TARGET_PREFERRED_RELOAD_CLASS must not return rclass is if x is 
a legitimate constant which cannot be loaded into some register class. By 
returning NO_REGS you can force x into a memory location. For example, rs6000 
can load immediate values into general-purpose registers, but does not have an 
instruction for loading an immediate value into a floating-point register, so 
TARGET_PREFERRED_RELOAD_CLASS returns NO_REGS when x is a floating-point 
constant. If the constant can't be loaded into any kind of register, code 
generation will be better if TARGET_LEGITIMATE_CONSTANT_P makes the constant 
illegitimate instead of using TARGET_PREFERRED_RELOAD_CLASS. "

So it seems that not only did the original constraint not add anything, we may 
also generate better code now.




float-const-patch-1.patch
Description: float-const-patch-1.patch

[PATCH 0/2] Python testcases to check DWARF output

[Pierre-Marie de Rodat]

Hello,

At the last GNU Cauldron, Richard Biener and I talked about DWARF output
testing. Except for guality tests, which are disabled on several
targets, the only way tests check the DWARF is scanning the annotated
assembly (-dA), making it hard to write reliable tests.

For instance, checking the number of times DW_AT_location is present in
order to check that some specific variable is assigned one is fuzzy.
Depending on the target and on the evolution of the compiler, the number
of output variables, or which one is assigned a location can vary
legitimately but still make the test fail.

On my side, I already had written an out-of-tree testsuite for the DWARF
features I added for Ada. This testsuite uses a DWARF parser in order to
perform checks on a tree:
. I had to update it
a couple of times, for instance when a change created a
DW_TAG_const_type DIE or removed one somewhere in a type tree, but
that’s very rare. I would say that I’m satisfied with the checks I could
express, but I don’t remember I ever caught a regression with them, so I
have no representative experience to share in this area. Maybe DWARF
back-end developpers do a too good job. ;-)

Anyway, Richard and I discussed about doing something similar in-tree,
and here is a candidate set of patches to achieve that:

  * The first patch installs DejaGNU scripts to run a Python interpreter
in testcases.

  * The second one installs other DejaGNU scripts to detect DWARF
dumping tools, plus a small Python library to parse and pattern
match DIEs and their attributes. It also adds several C and Ada
tests as examples; these are inspired by existing homonym tests
based on assembly scanning.

For now, this supports only platforms where objdump is available for the
current target, but extending it to other tools, such as otool on Darwin
should be doable.

I would appreciate feedback about the idea and the implementation I
propose. This is the first time I do more in the testsuite than just
adding new tests, so thank you in advance for you patience in reviewing
these. :-)

I tested these patches on x86_64-linux.

[PATCH 1/2] Introduce testsuite support to run Python tests

[Pierre-Marie de Rodat]

gcc/testsuite/

* lib/gcc-python.exp: New test library.
* python/testutils.py: New Python helper.
---
 gcc/testsuite/lib/gcc-python.exp  | 95 +++
 gcc/testsuite/python/testutils.py | 45 +++
 2 files changed, 140 insertions(+)
 create mode 100644 gcc/testsuite/lib/gcc-python.exp
 create mode 100644 gcc/testsuite/python/testutils.py

diff --git a/gcc/testsuite/lib/gcc-python.exp b/gcc/testsuite/lib/gcc-python.exp
new file mode 100644
index 000..30cf74a87ac
--- /dev/null
+++ b/gcc/testsuite/lib/gcc-python.exp
@@ -0,0 +1,95 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# Helpers to run a Python interpreter
+
+load_lib "remote.exp"
+
+# Return whether a working Python interpreter is available.
+
+proc check-python-available { args } {
+set result [local_exec "python -c print(\"Hello\")" "" "" 300]
+
+set status [lindex $result 0]
+set output [string trim [lindex $result 1]]
+
+if { $status != 0 || $output != "Hello" } {
+   return 0
+} else {
+   return 1
+}
+}
+
+# Run the SCRIPT_PY Python script. Add one PASSing (FAILing) test per output
+# line that starts with "PASS: " ("FAIL: "). Also fail for any other output
+# line and for non-zero exit status code.
+#
+# The Python script can access Python modules and packages in the
+# $srcdir/python directory.
+
+proc python-test { script_py } {
+global srcdir
+
+set testname testname-for-summary
+
+# This assumes that we are three frames down from dg-test, and that
+# it still stores the filename of the testcase in a local variable "name".
+# A cleaner solution would require a new DejaGnu release.
+upvar 2 prog src_file
+
+set asm_file "[file rootname [file tail $src_file]].o"
+set script_py_path "[file dirname $src_file]/$script_py"
+
+set old_pythonpath [getenv "PYTHONPATH"]
+set support_dir "$srcdir/python"
+if { $old_pythonpath == "" } {
+setenv "PYTHONPATH" $support_dir
+} else {
+setenv "PYTHONPATH" "$support_dir:$PYTHONPATH"
+}
+
+set commandline "python $script_py_path $asm_file"
+set timeout 300
+
+verbose -log "Executing: $commandline (timeout = $timeout)" 2
+set result [local_exec $commandline "" "" $timeout]
+
+set status [lindex $result 0]
+set output [lindex $result 1]
+
+if { $status != 0 } {
+   fail [concat "$testname: $script_py stopped with non-zero status" \
+" code ($status)"]
+}
+
+foreach line [split $output "\n"] {
+if { $line == "" } {
+continue
+}
+if { [regexp "^PASS: (.*)" $line dummy message] } {
+pass "$testname/$script_py: $message"
+continue
+}
+if { [regexp "^FAIL: (.*)" $line dummy message] } {
+fail "$testname/$script_py: $message"
+continue
+}
+
+fail "$testname/$script_py: spurious output: $line"
+}
+
+setenv "PYTHONPATH" $old_pythonpath
+}
diff --git a/gcc/testsuite/python/testutils.py 
b/gcc/testsuite/python/testutils.py
new file mode 100644
index 000..503105ad9d0
--- /dev/null
+++ b/gcc/testsuite/python/testutils.py
@@ -0,0 +1,45 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# Helpers to drive a testcase
+
+def print_pass(message):
+"""Emit a PASS message.
+
+:param str message: Message to emit.
+"""
+print('PASS: {}'.format(message))
+
+
+def print_fail(message):
+"""Emit a FAIL message.
+
+:param str message: Message to emit.
+"""
+print('FAIL: {}'.format(message))
+
+
+def check(predicate, message):
+ 

[PATCH 2/2] Introduce Python testcases to check DWARF output

[Pierre-Marie de Rodat]

For now, this supports only platforms that have an objdump available for
the corresponding target. There are several things that would be nico to
have in the future:

  * add support for more DWARF dumping tools, such as otool on Darwin;

  * have a DWARF location expression decoder, to be able to parse and
pattern match expressions that objdump does not decode itself;

  * complete the set of decoders for DIE attributes.

gcc/testsuite/

* lib/gcc-dwarf.exp: New helper files.
* python/dwarfutils/__init__.py,
python/dwarfutils/data.py,
python/dwarfutils/helpers.py,
python/dwarfutils/objdump.py: New Python helpers.
* gcc.dg/debug/dwarf2-py/dwarf2-py.exp,
gnat.dg/dwarf/dwarf.exp: New test drivers.
* gcc.dg/debug/dwarf2-py/sso.c,
gcc.dg/debug/dwarf2-py/sso.py,
gcc.dg/debug/dwarf2-py/var2.c,
gcc.dg/debug/dwarf2-py/var2.py,
gnat.dg/dwarf/debug9.adb,
gnat.dg/dwarf/debug9.py,
gnat.dg/dwarf/debug11.adb,
gnat.dg/dwarf/debug11.py,
gnat.dg/dwarf/debug12.adb,
gnat.dg/dwarf/debug12.ads,
gnat.dg/dwarf/debug12.py: New tests.
---
 gcc/testsuite/gcc.dg/debug/dwarf2-py/dwarf2-py.exp |  52 ++
 gcc/testsuite/gcc.dg/debug/dwarf2-py/sso.c |  19 +
 gcc/testsuite/gcc.dg/debug/dwarf2-py/sso.py|  52 ++
 gcc/testsuite/gcc.dg/debug/dwarf2-py/var2.c|  13 +
 gcc/testsuite/gcc.dg/debug/dwarf2-py/var2.py   |  11 +
 gcc/testsuite/gnat.dg/dg.exp   |   1 +
 gcc/testsuite/gnat.dg/dwarf/debug11.adb|  19 +
 gcc/testsuite/gnat.dg/dwarf/debug11.py |  51 ++
 gcc/testsuite/gnat.dg/dwarf/debug12.adb|  10 +
 gcc/testsuite/gnat.dg/dwarf/debug12.ads|   8 +
 gcc/testsuite/gnat.dg/dwarf/debug12.py |   9 +
 gcc/testsuite/gnat.dg/dwarf/debug9.adb |  45 ++
 gcc/testsuite/gnat.dg/dwarf/debug9.py  |  22 +
 gcc/testsuite/gnat.dg/dwarf/dwarf.exp  |  39 ++
 gcc/testsuite/lib/gcc-dwarf.exp|  41 ++
 gcc/testsuite/python/dwarfutils/__init__.py|  70 +++
 gcc/testsuite/python/dwarfutils/data.py| 597 +
 gcc/testsuite/python/dwarfutils/helpers.py |  11 +
 gcc/testsuite/python/dwarfutils/objdump.py | 338 
 19 files changed, 1408 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2-py/dwarf2-py.exp
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2-py/sso.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2-py/sso.py
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2-py/var2.c
 create mode 100644 gcc/testsuite/gcc.dg/debug/dwarf2-py/var2.py
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug11.adb
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug11.py
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug12.adb
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug12.ads
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug12.py
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug9.adb
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/debug9.py
 create mode 100644 gcc/testsuite/gnat.dg/dwarf/dwarf.exp
 create mode 100644 gcc/testsuite/lib/gcc-dwarf.exp
 create mode 100644 gcc/testsuite/python/dwarfutils/__init__.py
 create mode 100644 gcc/testsuite/python/dwarfutils/data.py
 create mode 100644 gcc/testsuite/python/dwarfutils/helpers.py
 create mode 100644 gcc/testsuite/python/dwarfutils/objdump.py

diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2-py/dwarf2-py.exp 
b/gcc/testsuite/gcc.dg/debug/dwarf2-py/dwarf2-py.exp
new file mode 100644
index 000..5c49bc81a55
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2-py/dwarf2-py.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# Testsuite driver for testcases that check the DWARF output with Python
+# scripts.
+
+load_lib gcc-dg.exp
+load_lib gcc-python.exp
+load_lib gcc-dwarf.exp
+
+# This series of tests require a working Python interpreter and a supported
+# host tool to dump DWARF.
+if { ![check-python-available] || ![detect-dwarf-dump-tool] } {
+return
+}
+
+# If a testcase doesn't have special options, use these.
+global DEFAULT_CFLAGS
+if ![info exists DEFAULT_CFLAGS] then {
+set DEFAULT_CFLAGS " -ansi -pedantic-errors -gdw

[PATCH] x86: Properly check register CFA offset

[H.J. Lu]

X86 epilogue saves register at CFA offset.  Since its location on stack
is computed as CFA - its CFA_OFFSET, CFA_OFFSET points the end of the
saved register location on stack.  This patch updates sp_valid_at and
fp_valid_at to properly check register CFA offset.

Tested on x86-64.  OK for trunk if there are no regressions on i686?

Thanks.

H.J.
--
gcc/

PR target/81563
* config/i386/i386.c (sp_valid_at): Properly check CFA offset.
(fp_valid_at): Likewise.

gcc/testsuite/

PR target/81563
* gcc.target/i386/pr81563.c: New test
---
 gcc/config/i386/i386.c  | 10 ++
 gcc/testsuite/gcc.target/i386/pr81563.c | 14 ++
 2 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81563.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 084b4a6a0db..f1486ff3750 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13102,24 +13102,26 @@ choose_baseaddr_len (unsigned int regno, 
HOST_WIDE_INT offset)
   return len;
 }
 
-/* Determine if the stack pointer is valid for accessing the cfa_offset.  */
+/* Determine if the stack pointer is valid for accessing the cfa_offset.
+   The register is saved at CFA - CFA_OFFSET.  */
 
 static inline bool
 sp_valid_at (HOST_WIDE_INT cfa_offset)
 {
   const struct machine_frame_state &fs = cfun->machine->fs;
   return fs.sp_valid && !(fs.sp_realigned
- && cfa_offset < fs.sp_realigned_offset);
+ && cfa_offset <= fs.sp_realigned_offset);
 }
 
-/* Determine if the frame pointer is valid for accessing the cfa_offset.  */
+/* Determine if the frame pointer is valid for accessing the cfa_offset.
+   The register is saved at CFA - CFA_OFFSET.  */
 
 static inline bool
 fp_valid_at (HOST_WIDE_INT cfa_offset)
 {
   const struct machine_frame_state &fs = cfun->machine->fs;
   return fs.fp_valid && !(fs.sp_valid && fs.sp_realigned
- && cfa_offset >= fs.sp_realigned_offset);
+ && cfa_offset > fs.sp_realigned_offset);
 }
 
 /* Choose a base register based upon alignment requested, speed and/or
diff --git a/gcc/testsuite/gcc.target/i386/pr81563.c 
b/gcc/testsuite/gcc.target/i386/pr81563.c
new file mode 100644
index 000..ebfd583daf5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr81563.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -maccumulate-outgoing-args -mincoming-stack-boundary=2 
-mpreferred-stack-boundary=3 -mregparm=3 -mtune-ctrl=epilogue_using_move" } */
+
+extern void bar (long long int, int);
+
+long long int
+fn1 (long long int x)
+{
+  bar (x, 1);
+  return x;
+}
+
+/* { dg-final { scan-assembler-times "movl\[\\t \]*-8\\(%ebp\\),\[\\t \]*%esi" 
1 } } */
+/* { dg-final { scan-assembler-times "movl\[\\t \]*-4\\(%ebp\\),\[\\t \]*%edi" 
1 } } */
-- 
2.13.3

Re: [PATCH 0/2] Python testcases to check DWARF output

[David Malcolm]

On Wed, 2017-07-26 at 18:00 +0200, Pierre-Marie de Rodat wrote:
> Hello,
> 
> At the last GNU Cauldron, Richard Biener and I talked about DWARF
> output
> testing. Except for guality tests, which are disabled on several
> targets, the only way tests check the DWARF is scanning the annotated
> assembly (-dA), making it hard to write reliable tests.
> 
> For instance, checking the number of times DW_AT_location is present
> in
> order to check that some specific variable is assigned one is fuzzy.
> Depending on the target and on the evolution of the compiler, the
> number
> of output variables, or which one is assigned a location can vary
> legitimately but still make the test fail.
> 
> On my side, I already had written an out-of-tree testsuite for the
> DWARF
> features I added for Ada. This testsuite uses a DWARF parser in order
> to
> perform checks on a tree:
> . I had to update
> it
> a couple of times, for instance when a change created a
> DW_TAG_const_type DIE or removed one somewhere in a type tree, but
> that’s very rare. I would say that I’m satisfied with the checks I
> could
> express, but I don’t remember I ever caught a regression with them,
> so I
> have no representative experience to share in this area. Maybe DWARF
> back-end developpers do a too good job. ;-)
> 
> Anyway, Richard and I discussed about doing something similar in
> -tree,
> and here is a candidate set of patches to achieve that:
> 
>   * The first patch installs DejaGNU scripts to run a Python
> interpreter
> in testcases.
> 
>   * The second one installs other DejaGNU scripts to detect DWARF
> dumping tools, plus a small Python library to parse and pattern
> match DIEs and their attributes. It also adds several C and Ada
> tests as examples; these are inspired by existing homonym tests
> based on assembly scanning.
> 
> For now, this supports only platforms where objdump is available for
> the
> current target, but extending it to other tools, such as otool on
> Darwin
> should be doable.
> 
> I would appreciate feedback about the idea and the implementation I
> propose. This is the first time I do more in the testsuite than just
> adding new tests, so thank you in advance for you patience in
> reviewing
> these. :-)

(FWIW I'm a big fan of Python, so am happy to see this proposal)

> I tested these patches on x86_64-linux.

Which version of Python did you test against?   As far as I can see
you've coded this using the common subset of Python 2 and Python 3;
it's worth spelling out what the assumptions are in this regard (and
what the minimum versions are).

Dave

Re: [PATCH 1/2] Introduce testsuite support to run Python tests

[David Malcolm]

On Wed, 2017-07-26 at 18:00 +0200, Pierre-Marie de Rodat wrote:
[...snip...]

> diff --git a/gcc/testsuite/python/testutils.py
> b/gcc/testsuite/python/testutils.py
> new file mode 100644
> index 000..503105ad9d0
> --- /dev/null
> +++ b/gcc/testsuite/python/testutils.py
> @@ -0,0 +1,45 @@
> +# Copyright (C) 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or
> modify
> +# it under the terms of the GNU General Public License as published
> by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with GCC; see the file COPYING3.  If not see
> +# .
> +
> +# Helpers to drive a testcase
> +
> +def print_pass(message):
> +"""Emit a PASS message.
> +
> +:param str message: Message to emit.
> +"""
> +print('PASS: {}'.format(message))

str.format was introduced in Python 2.6, so presumably the minimum
python 2 version here is at least 2.6+; for Python 3 I believe it was
present in Python 3.0 onwards.

> +
> +def print_fail(message):
> +"""Emit a FAIL message.
> +
> +:param str message: Message to emit.
> +"""
> +print('FAIL: {}'.format(message))
> +
> +
> +def check(predicate, message):
> +"""
> +If `predicate` is True, emit a PASS message, otherwise emit a
> FAIL one.

A very nitpicky nitpick: this comment should be spelled as "is true"
(lowercase), rather than "is True" since the requirement is that
predicate's "truth value" is true, rather than predicate *is* the
boolean "True" singleton; e.g. if someone passes in an int as
predicate, its nonzero-ness would be used, rather than always being
false (since no int *is* the boolean singleton "True").

> +
> +:param bool predicate: Whether the test should pass.
> +:param str message: Message to emit.
> +"""
> +if predicate:
> +print_pass(message)
> +else:
> +print_fail(message)

Re: [PATCH 0/2] Python testcases to check DWARF output

[Pierre-Marie de Rodat]

On 07/26/2017 06:15 PM, David Malcolm wrote:

(FWIW I'm a big fan of Python, so am happy to see this proposal)


Me too. :-)


Which version of Python did you test against?   As far as I can see
you've coded this using the common subset of Python 2 and Python 3;
it's worth spelling out what the assumptions are in this regard (and
what the minimum versions are).


I tested with both Python 2 and Python 3, as “python” can be each one 
depending on the system (it’s Python 3 on my Linux box). I agree I 
should explicitely state that sources must be compatible with both. 
Maybe in lib/gcc-python.exp?


--
Pierre-Marie de Rodat

Re: [PATCH 1/2] Introduce testsuite support to run Python tests

[Pierre-Marie de Rodat]

On 07/26/2017 06:25 PM, David Malcolm wrote:

str.format was introduced in Python 2.6, so presumably the minimum
python 2 version here is at least 2.6+; for Python 3 I believe it was
present in Python 3.0 onwards.


Hm… Python 2.6 is fairly old: last binary release was ages ago, last 
source release was in 2013. Do you think it’s worth supporting it?



+def check(predicate, message):
+"""
+If `predicate` is True, emit a PASS message, otherwise emit a
FAIL one.


A very nitpicky nitpick: this comment should be spelled as "is true"
(lowercase), rather than "is True" since the requirement is that
predicate's "truth value" is true, rather than predicate *is* the
boolean "True" singleton; e.g. if someone passes in an int as
predicate, its nonzero-ness would be used, rather than always being
false (since no int *is* the boolean singleton "True").


I agree with you: I updated the patch on my machine. Thank you!

--
Pierre-Marie de Rodat

[PATCH,AIX] Changes for linking gotools on AIX.

[REIX, Tony]

Description:
 * This patch adds linker options for gotools for AIX.

Tests:
 * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
   - build remade by means of gmake.
   - some test redone in libgo (gmake check)
 * AIX + GCC 7.1.0 :
   - build remade by means of gmake.
   - some test redone in libgo (gmake check)
 
ChangeLog:
 * Makefile.am (AM_LDFLAGS & GOLINK): Changes for linking on AIX.
 * Makefile.in: Rebuild.

Cordialement,

Tony Reix

Bull - ATOS
IBM Coop Architect & Technical Leader
Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.netIndex: gotools/ChangeLog
===
--- gotools/ChangeLog	(révision 250563)
+++ gotools/ChangeLog	(copie de travail)
@@ -1,3 +1,8 @@
+2017-07-26  Tony Reix  
+
+	* Makefile.am (AM_LDFLAGS & GOLINK): Changes for linking on AIX.
+	* Makefile.in: Rebuild.
+
 2017-07-15  Ian Lance Taylor  
 
 	* Makefile.am (CHECK_ENV): Set GOROOT.
Index: gotools/Makefile.am
===
--- gotools/Makefile.am	(révision 250563)
+++ gotools/Makefile.am	(copie de travail)
@@ -40,7 +40,11 @@ GOCOMPILE = $(GOCOMPILER) $(GOCFLAGS)
 
 AM_GOCFLAGS = -I $(libgodir)
 AM_LDFLAGS = -L $(libgodir) -L $(libgodir)/.libs
-GOLINK = $(GOCOMPILER) $(GOCFLAGS) $(AM_GOCFLAGS) $(LDFLAGS) $(AM_LDFLAGS) -o $@
+ifeq ($(shell uname), AIX)
+AM_LDFLAGS += -Wl,-blibpath:$(libdir):/usr/lib:/lib
+GOLINK = LIBRARY_PATH=$(libgodir)/.libs
+endif
+GOLINK += $(GOCOMPILER) $(GOCFLAGS) $(AM_GOCFLAGS) $(LDFLAGS) $(AM_LDFLAGS) -o $@
 
 libgosrcdir = $(srcdir)/../libgo/go
 cmdsrcdir = $(libgosrcdir)/cmd
Index: gotools/Makefile.in
===
--- gotools/Makefile.in	(révision 250563)
+++ gotools/Makefile.in	(copie de travail)
@@ -260,7 +260,11 @@ LIBGODEP = $(libgodir)/libgo.la
 GOCOMPILE = $(GOCOMPILER) $(GOCFLAGS)
 AM_GOCFLAGS = -I $(libgodir)
 AM_LDFLAGS = -L $(libgodir) -L $(libgodir)/.libs
-GOLINK = $(GOCOMPILER) $(GOCFLAGS) $(AM_GOCFLAGS) $(LDFLAGS) $(AM_LDFLAGS) -o $@
+ifeq ($(shell uname), AIX)
+AM_LDFLAGS += -Wl,-blibpath:$(libdir):/usr/lib:/lib
+GOLINK = LIBRARY_PATH=$(libgodir)/.libs
+endif
+GOLINK += $(GOCOMPILER) $(GOCFLAGS) $(AM_GOCFLAGS) $(LDFLAGS) $(AM_LDFLAGS) -o $@
 libgosrcdir = $(srcdir)/../libgo/go
 cmdsrcdir = $(libgosrcdir)/cmd
 libgomiscdir = $(srcdir)/../libgo/misc

[PATCH] PR c++/67054 Allow inheriting constructor with non-default-constructible members

[Leonid Koppel]

This patch addresses PR 67054 (duplicates 62310, 80851). An implicitly-defined 
inheriting constructor was wrongly considered deleted when it would initialize 
a non-default-constructible member, even when a brace-or-equal-initializer was 
present.

The bug only affects deduction of the constructor's deletedness, not the actual 
generation of the constructor, which is why (I hope!) the fix might be so 
simple.

Tested on x86_64-pc-linux-gnu native.

Thanks,
Leo

2017-07-26  Leonid Koppel  

PR c++/67054 - Inheriting constructor with non-default-constructible 
members
* method.c (walk_field_subobs) Consider member initializers (NSDMIs) 
when 
deducing an inheriting constructor.

diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index cca1b146917..8b07f526473 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1342,7 +1342,7 @@ walk_field_subobs (tree fields, tree fnname, 
special_function_kind sfk,
  if (bad && deleted_p)
*deleted_p = true;
}
-  else if (sfk == sfk_constructor)
+  else if (sfk == sfk_constructor || sfk == sfk_inheriting_constructor)
{
  bool bad;
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C 
b/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C
new file mode 100644
index 000..8e31f739d74
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C
@@ -0,0 +1,23 @@
+// PR c++/67054
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+  A(int) {}
+};
+
+struct C
+{
+  C(int) {}
+};
+
+struct B : A
+{
+  using A::A;
+  C c = 42;
+};
+
+int main()
+{
+  B b = 24;
+}

Re: [PATCH 1/2] Introduce testsuite support to run Python tests

[David Malcolm]

On Wed, 2017-07-26 at 18:35 +0200, Pierre-Marie de Rodat wrote:
> On 07/26/2017 06:25 PM, David Malcolm wrote:
> > str.format was introduced in Python 2.6, so presumably the minimum
> > python 2 version here is at least 2.6+; for Python 3 I believe it
> > was
> > present in Python 3.0 onwards.
> 
> Hm… Python 2.6 is fairly old: last binary release was ages ago, last 
> source release was in 2013. Do you think it’s worth supporting it?

IIRC RHEL 6 has Python 2.6 as its /usr/bin/python (but Python 2.7 is
available as a "software collection" add-on).

I don't know if gcc as a project would want to support 2.6+ or simply
2.7 for Python 2.

> > > +def check(predicate, message):
> > > +"""
> > > +If `predicate` is True, emit a PASS message, otherwise emit
> > > a
> > > FAIL one.
> > 
> > A very nitpicky nitpick: this comment should be spelled as "is
> > true"
> > (lowercase), rather than "is True" since the requirement is that
> > predicate's "truth value" is true, rather than predicate *is* the
> > boolean "True" singleton; e.g. if someone passes in an int as
> > predicate, its nonzero-ness would be used, rather than always being
> > false (since no int *is* the boolean singleton "True").
> 
> I agree with you: I updated the patch on my machine. Thank you!

Thanks.  I saw at least one other instance of this in the other patch;
I'll look over it again.

Dave

Re: [PATCH] PR c++/67054 Allow inheriting constructor with non-default-constructible members

[Jason Merrill]

OK, thanks.

On Wed, Jul 26, 2017 at 12:45 PM, Leonid Koppel  wrote:
> This patch addresses PR 67054 (duplicates 62310, 80851). An implicitly-defined 
> inheriting constructor was wrongly considered deleted when it would 
> initialize a non-default-constructible member, even when a 
> brace-or-equal-initializer was present.
>
> The bug only affects deduction of the constructor's deletedness, not the 
> actual generation of the constructor, which is why (I hope!) the fix might be 
> so simple.
>
> Tested on x86_64-pc-linux-gnu native.
>
> Thanks,
> Leo
>
> 2017-07-26  Leonid Koppel  
>
> PR c++/67054 - Inheriting constructor with non-default-constructible 
> members
> * method.c (walk_field_subobs) Consider member initializers (NSDMIs) 
> when
> deducing an inheriting constructor.
>
> diff --git a/gcc/cp/method.c b/gcc/cp/method.c
> index cca1b146917..8b07f526473 100644
> --- a/gcc/cp/method.c
> +++ b/gcc/cp/method.c
> @@ -1342,7 +1342,7 @@ walk_field_subobs (tree fields, tree fnname, 
> special_function_kind sfk,
>   if (bad && deleted_p)
> *deleted_p = true;
> }
> -  else if (sfk == sfk_constructor)
> +  else if (sfk == sfk_constructor || sfk == sfk_inheriting_constructor)
> {
>   bool bad;
>
> diff --git a/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C 
> b/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C
> new file mode 100644
> index 000..8e31f739d74
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/inh-ctor29.C
> @@ -0,0 +1,23 @@
> +// PR c++/67054
> +// { dg-do compile { target c++11 } }
> +
> +struct A
> +{
> +  A(int) {}
> +};
> +
> +struct C
> +{
> +  C(int) {}
> +};
> +
> +struct B : A
> +{
> +  using A::A;
> +  C c = 42;
> +};
> +
> +int main()
> +{
> +  B b = 24;
> +}

Re: ping [PATCH] [MSP430] Fix PR78849: ICE on initialization of global struct containing __int20 array

[Jeff Law]

On 05/19/2017 07:35 AM, Jozef Lawrynowicz wrote:
> Original post: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01030.html
> 
> The attached patch fixes an issue for the msp430 target where the TYPE_SIZE of
> the __int20 type was set using the precision (20 bits) instead of the 
> in-memory
> size (32 bits) of the type. This bug caused an ICE as reported in PR78849:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78849
> 
> The patch passed bootstrap and regression testing with no regressions
> on recent trunk (r247020) for x86_64-pc-linux-gnu.
> 
> The patch passed regression testing with "-mcpu=msp430x/-mlarge" for
> msp430-elf on the gcc-6-branch (r247086). Trunk doesn't build with C++
> support for msp430-elf which is why gcc-6-branch was used.
> 
> If the patch is acceptable I would appreciate if someone could commit it for 
> me
> as I don't have write access.
> 
> 
> 0001-Use-GET_MODE_BITSIZE-when-setting-TYPE_SIZE.patch
> 
> 
> From 81ee936dcdde4f4a7d4036479dbbff77da1e72bb Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz 
> Date: Wed, 12 Apr 2017 14:45:45 +
> Subject: [PATCH] Use GET_MODE_BITSIZE when setting TYPE_SIZE
> 
> 2017-05-XXJozef Lawrynowicz   
> 
>   gcc/
>   PR target/78849
>   * stor-layout.c (initialize_sizetypes): Use GET_MODE_BITSIZE when 
> setting TYPE_SIZE.
>   * tree.c (build_common_tree_nodes): Likewise.
> 
>   gcc/testsuite
>   PR target/78849
>   * gcc.target/msp430/pr78849.c: New test.
>   * gcc.target/msp430/msp430.exp: Remove -pedantic-errors option from 
> DEFAULT_CFLAGS.
TYPE_SIZE, according to my understanding, should be a tree for the size
of the expression in bits.

The problem is for msp430 that size varies depending on where it's used.
 ie, in a register an object might have a bitsize of 20 bits, but in
memory its size is 32 bits.

I don't think that TYPE_SIZE has any concept that the use context is
relevant to selecting the proper size.

I wonder if we would be better off keeping TYPE_SIZE as-is and instead
looking at the end use points where we might have that contextual
information.

Thoughts?
jeff

Re: [PATCH,AIX] Changes for linking gotools on AIX.

[David Edelsohn]

On Wed, Jul 26, 2017 at 12:41 PM, REIX, Tony  wrote:
> Description:
>  * This patch adds linker options for gotools for AIX.
>
> Tests:
>  * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
>- build remade by means of gmake.
>- some test redone in libgo (gmake check)
>  * AIX + GCC 7.1.0 :
>- build remade by means of gmake.
>- some test redone in libgo (gmake check)
>
> ChangeLog:
>  * Makefile.am (AM_LDFLAGS & GOLINK): Changes for linking on AIX.
>  * Makefile.in: Rebuild.

If this is trying to fix AIX search paths, a better solution would
seem to be the equivalent of -static-libstdc++ -static-libgcc.  The Go
tools should be linked statically and not depend on Go shared
libraries.

Thanks, David

Re: [PATCH 1/2] x86,s390: add compiler memory barriers when expanding atomic_thread_fence (PR 80640)

[Jeff Law]

On 05/26/2017 04:58 AM, Alexander Monakov wrote:
> On Wed, 17 May 2017, Alexander Monakov wrote:
> 
>> Ping.
> 
> Ping^2?
> 
>> (to be clear, patch 2/2 is my previous followup in this thread, I forgot to
>> adjust the subject line; it should have said:
>> "[PATCH 2/2] x86: add compiler memory barriers when expanding atomic_load").
>>
>> On Wed, 10 May 2017, Alexander Monakov wrote:
>>
>>> Hi,
>>>
>>> When expanding __atomic_thread_fence(x) to RTL, the i386 backend doesn't 
>>> emit
>>> any instruction except for x==__ATOMIC_SEQ_CST (which emits 'mfence').  
>>> This 
>>> is incorrect: although no machine barrier is needed, the compiler still must
>>> emit a compiler barrier into the IR to prevent propagation and code motion
>>> across the fence.  The testcase added with the patch shows how it can lead
>>> to a miscompilation.
>>>
>>> The proposed patch fixes it by handling non-seq-cst fences exactly like
>>> __atomic_signal_fence is expanded, by emitting asm volatile("":::"memory").
>>>
>>> The s390 backend uses the a similar mem_thread_fence expansion, so the patch
>>> fixes both backends in the same manner.
>>>
>>> Bootstrapped and regtested on x86_64; also checked that s390-linux cc1
>>> successfully builds after the change.  OK for trunk?
>>>
>>> (the original source code in the PR was misusing atomic fences by doing
>>> something like
>>>
>>>   void f(int *p)
>>>   {
>>> while (*p)
>>>   __atomic_thread_fence(__ATOMIC_ACQUIRE);
>>>   }
>>>
>>> but since *p is not atomic, a concurrent write to *p would cause a data 
>>> race and
>>> thus invoke undefined behavior; also, if *p is false prior to entering the 
>>> loop,
>>> execution does not encounter the fence; new test here has code usable 
>>> without UB)
>>>
>>> Alexander
>>>
>>> * config/i386/sync.md (mem_thread_fence): Emit a compiler barrier for
>>> non-seq-cst fences.  Adjust comment.
>>> * config/s390/s390.md (mem_thread_fence): Likewise.
>>> * optabs.c (expand_asm_memory_barrier): Export.
>>> * optabs.h (expand_asm_memory_barrier): Declare.
>>> testsuite/
>>> * gcc.target/i386/pr80640-1.c: New testcase.
So I think this is up to the target maintainers.  I have no concerns
with enabling use of expand_asm_memory_barrier to be used outside of
optabs.  So if the s390/x86 maintainers want to go forward, the optabs
changes are pre-approved.

The reasoning seems sound to me -- we may not need fencing at the
hardware level because it gives a certain set of guarantees, but we may
still need them at the compiler level to prevent undesirable code
motions across the fence point in the compiler.

Jeff

Re: [PATCH 2/2] Introduce Python testcases to check DWARF output

[David Malcolm]

On Wed, 2017-07-26 at 18:00 +0200, Pierre-Marie de Rodat wrote:
[...]
> diff --git a/gcc/testsuite/python/dwarfutils/__init__.py
> b/gcc/testsuite/python/dwarfutils/__init__.py
> new file mode 100644
> index 000..246fbbd15be
> --- /dev/null
> +++ b/gcc/testsuite/python/dwarfutils/__init__.py
[...]
> +def parse_dwarf(object_file=None, single_cu=True):
> +"""
> +Fetch and decode DWARF compilation units in `object_file`.
> +
> +If `single_cu` is True, make sure there is exactly one
> compilation unit and

"is True" -> "is true"

[...]

> --- /dev/null
> +++ b/gcc/testsuite/python/dwarfutils/data.py

> +
> +def get_attr(self, name, single=True, or_error=True):
> +"""Look for an attribute in this DIE.
> +
> +:param str|int name: Attribute name, or number if name is
> unknown.
> +:param bool single: If true, this will raise a KeyError for
> +zero/multiple matches and return an Attribute instance
> when found.
> +Otherwise, return a potentially empty list of
> attributes.
> +:param bool or_error: When True, if `single` is True and no
> attribute

"True" -> "true" in two places

[...]

> +def find(self, predicate=None, tag=None, name=None,
> recursive=True,
> + single=True):
> +"""Look for a DIE that satisfies the given expectations.
> +
> +:param None|(DIE) -> bool predicate: If provided, function
> that filters
> +out DIEs when it returns False.
> +:param str|int|None tag: If provided, filter out DIEs whose
> tag does
> +not match.
> +:param str|None name: If provided, filter out DIEs whose
> name (see
> +the `name` property) does not match.
> +:param bool recursive: If True, perform the search
> recursively in
> +self's children.
> +:param bool single: If True, look for a single DIE and raise
> a

"True" -> "true", I suppose

[...]

> +class MatchResult(object):
> +"""Holder for the result of a DIE tree pattern match."""
> +
> +def __init__(self):
> +self.dict = {}
> +
> +self.mismatch_reason = None
> +"""
> +If left to None, the match succeded. Otherwise, must be set


"succeded" -> "succeeded"

> +
> +def capture(self, name):
> +"""Return what has been captured by the `name` capture.
> +
> +This is valid iff the match succeded.

here again.

[...]


> diff --git a/gcc/testsuite/python/dwarfutils/helpers.py
> b/gcc/testsuite/python/dwarfutils/helpers.py
> new file mode 100644
> index 000..f5e77896ae6
> --- /dev/null
> +++ b/gcc/testsuite/python/dwarfutils/helpers.py
> @@ -0,0 +1,11 @@
> +import sys
> +
> +
> +def as_ascii(str_or_byte):
> +"""
> +Python 2/3 compatibility helper.
> +
> +In Python 2, just return the input. In Python 3, decode the
> input as ASCII.
> +"""
> +return (str_or_byte if sys.version_info.major < 3 else
> +str_or_byte.decode('ascii'))

Aha!  Python 2 and Python 3.


Presumably this all runs with LANG=C so that there's no danger of any
non-ASCII bytes?  (bytes.decode('ascii' will raise a UnicodeDecodeError
if any byte >=128).


> diff --git a/gcc/testsuite/python/dwarfutils/objdump.py
> b/gcc/testsuite/python/dwarfutils/objdump.py
> new file mode 100644
> index 000..52cfc06c03b
> --- /dev/null
> +++ b/gcc/testsuite/python/dwarfutils/objdump.py

[...]

There's a fair amount of non-trivial parsing going on here.
I wonder if it would be helpful to add a "unittest" suite for the
parsing?
(e.g. to have some precanned fragments of objdump output as strings,
and to verify that they're parsed as expected).

Note that I'm not a reviewer for the testsuite, so this is just a
suggestion.

Hope this is constructive
Dave

Re: [PATCH 00/17] RFC: New source-location representation; Language Server Protocol

[Jim Wilson]

On 07/24/2017 01:04 PM, David Malcolm wrote:

* The LSP implementation is a just a proof-of-concept, to further
   motivate capturing the extra data.  Turning it into a "proper" LSP
   server implementation would be a *lot* more work, and I'm unlikely to
   actually do that (but maybe someone on the list wants to take this on?)


Apparently Alexandre Oliva has ideas on how to implement LSP by using 
gdb.  You two may want to compare notes.


Jim

Re: [PATCH] Improve extraction of changed file in contrib/mklog

[Jeff Law]

On 07/09/2017 01:03 PM, Yuri Gribov wrote:
> Hi,
> 
> Currently mklog will fail to analyze lines like this in patches:
> diff -rupN gcc/gcc/testsuite/lib/profopt.exp
> gcc-compare-checks/gcc/testsuite/lib/profopt.exp
> (it fails with "Error: failed to parse diff for ... and ...").
> 
> This patch fixes it. Ok for trunk?
> 
> -Y
> 
> 
> mklog-filename-fix-1.patch
> 
> 
> 2017-07-09  Yury Gribov  
> 
> contrib/
> * mklog: Fix extraction of changed file name.
One could argue that given general directions python would be a better
match than perl, but I don't think it's reasonable to require a rewrite
to move forward.

OK.

jeff

Re: [PATCH 1/2] x86,s390: add compiler memory barriers when expanding atomic_thread_fence (PR 80640)

[Alexander Monakov]

On Wed, 26 Jul 2017, Jeff Law wrote:
> So I think this is up to the target maintainers.  I have no concerns
> with enabling use of expand_asm_memory_barrier to be used outside of
> optabs.  So if the s390/x86 maintainers want to go forward, the optabs
> changes are pre-approved.

Please see the alternative middle-end patch:
 https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00549.html
and the recently reported related bug:
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81316

Since we got it wrong for both fences and loads/stores on two different targets,
perhaps fixing it once and for all in the middle-end is more appropriate.  I
hope extraneous compiler barriers can be tolerated.

> The reasoning seems sound to me -- we may not need fencing at the
> hardware level because it gives a certain set of guarantees, but we may
> still need them at the compiler level to prevent undesirable code
> motions across the fence point in the compiler.

Yep - the same applies to atomic loads/stores too.  They can be expanded
as volatile accesses, but we need compiler barrier(s) anyway (except for
those with relaxed memory model).

Thanks.
Alexander

Re: [PATCH] Fix indirect call optimization done by autoFDO.

[Jeff Law]

On 07/11/2017 04:37 AM, Martin Liška wrote:
> Hello.
> 
> Following is a typo fix which nobody has noticed during testing of
> e.g. gcc/testsuite/gcc.dg/tree-prof/indir-call-prof.c.
> 
> 
> Patch can bootstrap and survives regression tests.
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> 
> gcc/ChangeLog:
> 
> 2017-07-11  Martin Liska  
> 
> * auto-profile.c (autofdo_source_profile::update_inlined_ind_target):
> Fix wrong condition.
The preceeding comment says

"If it is no less than half of the callsite count (stored in INFO), the
original promoted target is considered not hot anymore."

"it" presumably refers to TOTAL  and INFO->count holds the callsite count.

A direct translation would result in

! (total < info->count / 2)

Which is equivalent to

(total >= info->count / 2)

Which seems to match the code.

So is the comment wrong?  Or is my interpretation wrong?

jeff

Re: [PATCH v2] New C++ warning option '-Waccess-specifiers'

[Eric Gallager]

On 7/24/17, Franz Sirl  wrote:
> Am 2017-07-24 um 00:19 schrieb Volker Reichelt:
>> On 23 Jul, Eric Gallager wrote:
>>> On 7/23/17, Volker Reichelt  wrote:
 Hi again,

 here is an updated patch for a new warning about redundant
 access-specifiers. It takes Dave's various comments into account.

 The main changes w.r.t. to the previous versions are:

 * The warning is now a two-level warning with a slightly shorter name:
-Waccess-specifiers=1, -Waccess-specifiers=2
with -Waccess-specifiers defaulting to -Waccess-specifiers=1.
>>>
>>> Just a more generalized comment as a user, but I don't really like
>>> this trend that new warning options are so often given numeric levels
>>> these days. A warning option with different levels requires special
>>> handling in configure scripts or Makefiles, which is harder than just
>>> toggling different names (i.e. how things work without numeric
>>> levels).
>>
>> Fair point.
>
> Another point is the handling of -Werror=. AFAIK it would be impossible
> right now to have "-Werror=access-specifiers=1 -Waccess-specifiers=2",
> with a combined meaning of "error for level 1 + warning for level 2".
>
> Actually, are the intended semantics for the existing cases (eg.
> -Warray-bounds=) vs. -Werror= even documented somewhere?
>
> Franz
>

Not exactly documentation, but there are bugs open about it:
56048: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56048
68845: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68845 (which I see
is yours, Franz)
Might be worth having someone take a look at them.

Re: [PATCH] toplev: avoid recursive emergency_dump_function

[Alexander Monakov]

On Sat, 22 Jul 2017, Segher Boessenkool wrote:
> On Thu, Jul 20, 2017 at 05:40:28PM +0300, Alexander Monakov wrote:
> > Segher pointed out on IRC that ICE reporting with dumps enabled got worse:
> > if emergency_dump_function itself leads to an ICE (e.g. by segfaulting),
> > nested ICE reporting will invoke emergency_dump_function in exactly the
> > same context, but not only would we uselessly announce current pass again,
> > this time around the second SIGSEGV will just terminate cc1 because the
> > signal handler is unregistered (so we won't print the backtrace).  Sorry
> > for not really considering the implications when submitting that patch.
> > 
> > Solve this by substituting the callback for global_dc->internal_error;
> > this avoids invoking emergency_dump_function, and also gives a
> > convenient point to flush the dump file.  OK to apply?
> 
> Extra thanks for that last point, it's been driving me nuts :-)
> 
> > * topvel.c (dumpfile.h): New include.
> 
> s/topvel/toplev/

I assume this is not an approval to apply - can somebody give an explicit OK?

Thanks!
Alexander

Re: Patch ping

[Jakub Jelinek]

On Wed, Jul 26, 2017 at 04:13:30PM +0200, Richard Biener wrote:
> > >  You don't seem to use 'size' anywhere.
> > 
> > size I thought about but then decided not to do anything with it.
> > There are two cases, one is where there is no ADDR_EXPR and it actually
> > a memory reference.  
> > In that case in theory the size could be used, but it would need
> > to be used only for the positive offsets, so like:
> > if (off > 0) {
> >   if (ptr + off + size < ptr)
> > runtime_fail;
> > } else if (ptr + off > ptr)
> >   runtime_fail;
> > but when it is actually a memory reference, I suppose it will fail
> > at runtime anyway when performing such an access, so I think it is
> > unnecessary.  And for the ADDR_EXPR case, the size is irrelevant, we
> > are just taking address of the start of the object.
> > 
> > > You fail to allow other handled components -- for no good reason?
> > 
> > I was trying to have a quick bail out.  What other handled components might
> > be relevant?  I guess IMAGPART_EXPR.  For say BIT_FIELD_REF I don't think
> > I can
> >   tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);
> 
> REALPART/IMAGPART_EXPR, yes.  You can't address BIT_FIELD_REF
> apart those on byte boundary (&vector[4] is eventually folded to
> a BIT_FIELD_REF).  Similar for VIEW_CONVERT_EXPR, but you are
> only building the address on the base?
> 
> > >  You fail to handle
> > > &MEM[ptr + CST] a canonical gimple invariant way of ptr +p CST,
> > > the early out bitpos == 0 will cause non-instrumentation here.
> > 
> > Guess I could use:
> >   if ((offset == NULL_TREE
> >&& bitpos == 0
> >&& (TREE_CODE (inner) != MEM_REF
> >|| integer_zerop (TREE_OPERAND (inner, 1
> > The rest of the code will handle it.
> 
> Yeah.
> 
> > 
> > > (I'd just round down in the case of bitpos % BITS_PER_UNIT != 0)
> > 
> > But then the
> >   tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);
> > won't work again.
> 
> Hmm.  So instead of building the address on the original tree you
> could build the difference based on what get_inner_reference returns
> in bitpos/offset?

I'm building both addresses and subtracting them to get the offset.
I guess the other option is to compute just the address of the base
(i.e. base_addr), and add offset (if non-NULL) plus bitpos / BITS_PER_UNIT
plus offset from the MEM_REF (if any).  In that case it would probably
handle any handled_component_p and bitfields too.

Jakub

Re: [PATCH 1/2] x86,s390: add compiler memory barriers when expanding atomic_thread_fence (PR 80640)

[Jeff Law]

On 07/26/2017 11:19 AM, Alexander Monakov wrote:
> On Wed, 26 Jul 2017, Jeff Law wrote:
>> So I think this is up to the target maintainers.  I have no concerns
>> with enabling use of expand_asm_memory_barrier to be used outside of
>> optabs.  So if the s390/x86 maintainers want to go forward, the optabs
>> changes are pre-approved.
> 
> Please see the alternative middle-end patch:
>  https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00549.html
I almost made the comment that we should look if we could do this at a
higher level.  But then figured that the expander could be called from
multiple locations, thus requiring that all those points be fixed or at
least routed to a common function.

But it looks like all the fence stuff is routed into
expand_mem_thread_fence, so indeed that's a better place.

[ And if there were only a good way to enforce that we can't
  directly call the backend  expander from anywhere other than
  the bless location.  ]



> and the recently reported related bug:
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81316>
> Since we got it wrong for both fences and loads/stores on two different 
> targets,
> perhaps fixing it once and for all in the middle-end is more appropriate.  I
> hope extraneous compiler barriers can be tolerated.
Agreed on middle-end being more appropriate.

I'm not sure what you mean by extraneous compiler barriers -- isn't the
worst case scenario here that the target emits them as well?  So there
would be an extraneous one in that case, but that ought to be a "don't
care".

In the middle end patch, do we need a barrier before the fence as well?
The post-fence barrier prevents reordering the fence with anything which
follows the fence.  But do we have to also prevent reordering the fence
with prior instructions with any of the memory models?  This isn't my
area of expertise, so if it's dumb question, don't hesitate to let me
know :-)


> 
> Yep - the same applies to atomic loads/stores too.  They can be expanded
> as volatile accesses, but we need compiler barrier(s) anyway (except for
> those with relaxed memory model).
Right.
jeff

C++ PATCH to implement P0702R1, list-deduction of vector

[Jason Merrill]

At the Toronto meeting we adjusted the semantics of class template
argument deduction for a class such as std::vector that has an
initializer-list constructor: previously, we would get

vector v1 { 42 }; // vector
vector v2 { v1 }; // vector>

but now v2 is also deduced to vector.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit d2725cd7aad7bec11c2d2ced6f5a02de83fe0427
Author: Jason Merrill 
Date:   Wed Jul 12 13:47:16 2017 -0400

P0702R1 - List deduction of vector.

* pt.c (do_class_deduction): Special-case deduction from a single
element of related type.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index bb32353..173ec3f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25331,6 +25331,24 @@ do_class_deduction (tree ptype, tree tmpl, tree init, 
int flags,
   else if (BRACE_ENCLOSED_INITIALIZER_P (init))
 {
   try_list_ctor = TYPE_HAS_LIST_CTOR (type);
+  if (try_list_ctor && CONSTRUCTOR_NELTS (init) == 1)
+   {
+ /* As an exception, the first phase in 16.3.1.7 (considering the
+initializer list as a single argument) is omitted if the
+initializer list consists of a single expression of type cv U,
+where U is a specialization of C or a class derived from a
+specialization of C.  */
+ tree elt = CONSTRUCTOR_ELT (init, 0)->value;
+ tree etype = TREE_TYPE (elt);
+
+ tree tparms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (tmpl));
+ tree targs = make_tree_vec (TREE_VEC_LENGTH (tparms));
+ int err = unify (tparms, targs, type, etype,
+  UNIFY_ALLOW_DERIVED, /*explain*/false);
+ if (err == 0)
+   try_list_ctor = false;
+ ggc_free (targs);
+   }
   if (try_list_ctor || is_std_init_list (type))
args = make_tree_vector_single (init);
   else
diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction42.C 
b/gcc/testsuite/g++.dg/cpp1z/class-deduction42.C
new file mode 100644
index 000..8217fd4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction42.C
@@ -0,0 +1,19 @@
+// { dg-options -std=c++1z }
+
+#include 
+
+template  struct same;
+template  struct same { };
+
+template 
+struct A
+{
+  A(const A&);
+  A(std::initializer_list);
+};
+
+A a { 1 };
+A b { a };
+
+same s;
+

[PATCH] Improve alloca alignment

[Wilco Dijkstra]

This patch improves alloca alignment.  Currently alloca reserves
too much space as it aligns twice, and generates unnecessary stack
alignment code.  For example alloca (16) generates:

sub sp, sp, #32   ???
mov x1, sp

Similarly alloca (x) generates:

add x0, x0, 30???
and x0, x0, -16
sub sp, sp, x0
mov x0, sp

__builtin_alloca_with_align (x, 256):
add x0, x0, 78???
and x0, x0, -16
sub sp, sp, x0
add x0, sp, 63
and x0, x0, -64

As can be seen the alignment adjustment value is incorrect.
When the requested alignment is lower than the stack alignment, no
extra alignment is needed.  If the requested alignment is higher,
we need to increase the size by the difference of the requested 
alignment and the stack alignment.  As a result, the alloca alignment
is exactly as expected:

alloca (16):
sub sp, sp, #16
mov x1, sp

alloca (x):
add x0, x0, 15
and x0, x0, -16
sub sp, sp, x0
mov x0, sp

__builtin_alloca_with_align (x, 256):
add x0, x0, 63
and x0, x0, -16
sub sp, sp, x0
add x0, sp, 63
and x0, x0, -64

ChangeLog:
2017-07-26  Wilco Dijkstra  

* explow.c (get_dynamic_stack_size): Improve dynamic alignment.

--
diff --git a/gcc/explow.c b/gcc/explow.c
index 
50074e281edd5270c76d29feac6b7a92f598d11d..fbdda5fa1e303664e346f975270415b40aed252d
 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -1234,15 +1234,22 @@ get_dynamic_stack_size (rtx *psize, unsigned size_align,
  example), so we must preventively align the value.  We leave space
  in SIZE for the hole that might result from the alignment operation.  */
 
-  extra = (required_align - BITS_PER_UNIT) / BITS_PER_UNIT;
-  size = plus_constant (Pmode, size, extra);
-  size = force_operand (size, NULL_RTX);
-
-  if (flag_stack_usage_info && pstack_usage_size)
-*pstack_usage_size += extra;
+  /* Since the stack is presumed to be aligned before this allocation,
+ we only need to increase the size of the allocation if the required
+ alignment is more than the stack alignment.
+ Note size_align doesn't need to be updated - if it is larger than the
+ stack alignment, size remains a multiple of the stack alignment, so
+ we can skip rounding up to the stack alignment.  */
+  if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT)
+{
+  extra = (required_align - MAX_SUPPORTED_STACK_ALIGNMENT)
+   / BITS_PER_UNIT;
+  size = plus_constant (Pmode, size, extra);
+  size = force_operand (size, NULL_RTX);
 
-  if (extra && size_align > BITS_PER_UNIT)
-size_align = BITS_PER_UNIT;
+  if (flag_stack_usage_info && pstack_usage_size)
+   *pstack_usage_size += extra;
+}
 
   /* Round the size to a multiple of the required stack alignment.
  Since the stack is presumed to be rounded before this allocation,

[Patch AArch64 Obvious] Unify branch costs to generic_branch_cost

[James Greenhalgh]

Hi,

All the cores in AArch64 use the pair {1, 3} for their branch costs. As
that is covered by generic_branch_cost, we can just use that directly and
save the tiny amount of redundant code. If in future any core wants to
modify this, they can always add a special-case branch-cost back.

Comitted as obvious as revision 250582 after a build-and-test.

Thanks,
James

---
2017-07-26  James Greenhalgh  

* config/aarch64/aarch64.c (cortexa57_branch_cost): Remove.
(thunderx2t99_branch_cost): Likewise.
(cortexa35_tunings): Update to use generic_branch_cost.
(cortexa53_tunings): Likewise.
(cortexa57_tunings): Likewise.
(cortexa72_tunings): Likewise.
(cortexa73_tunings): Likewise.
(thunderx2t99_tunings): Likewise.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d753666..31c0d8b 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -488,20 +488,6 @@ static const struct cpu_branch_cost generic_branch_cost =
   3   /* Unpredictable.  */
 };
 
-/* Branch costs for Cortex-A57.  */
-static const struct cpu_branch_cost cortexa57_branch_cost =
-{
-  1,  /* Predictable.  */
-  3   /* Unpredictable.  */
-};
-
-/* Branch costs for Vulcan.  */
-static const struct cpu_branch_cost thunderx2t99_branch_cost =
-{
-  1,  /* Predictable.  */
-  3   /* Unpredictable.  */
-};
-
 /* Generic approximation modes.  */
 static const cpu_approx_modes generic_approx_modes =
 {
@@ -612,7 +598,7 @@ static const struct tune_params cortexa35_tunings =
   &generic_addrcost_table,
   &cortexa53_regmove_cost,
   &generic_vector_cost,
-  &cortexa57_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost  */
   1, /* issue_rate  */
@@ -638,7 +624,7 @@ static const struct tune_params cortexa53_tunings =
   &generic_addrcost_table,
   &cortexa53_regmove_cost,
   &generic_vector_cost,
-  &cortexa57_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost  */
   2, /* issue_rate  */
@@ -664,7 +650,7 @@ static const struct tune_params cortexa57_tunings =
   &cortexa57_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
-  &cortexa57_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost  */
   3, /* issue_rate  */
@@ -690,7 +676,7 @@ static const struct tune_params cortexa72_tunings =
   &cortexa57_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
-  &cortexa57_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost  */
   3, /* issue_rate  */
@@ -716,7 +702,7 @@ static const struct tune_params cortexa73_tunings =
   &cortexa57_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
-  &cortexa57_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost.  */
   2, /* issue_rate.  */
@@ -871,7 +857,7 @@ static const struct tune_params thunderx2t99_tunings =
   &thunderx2t99_addrcost_table,
   &thunderx2t99_regmove_cost,
   &thunderx2t99_vector_cost,
-  &thunderx2t99_branch_cost,
+  &generic_branch_cost,
   &generic_approx_modes,
   4, /* memmov_cost.  */
   4, /* issue_rate.  */

[Patch AArch64 obvious] Unify address costs to generic_addrcost_table

[James Greenhalgh]

Hi,

The special case address cost tables for Cortex-A57 and qdf24xx are no
different from the generic address cost table. We should just use the
generic address cost table directly. If this changes in future, a core
is welcome to add new address cost tables.

Comitted as obvious as revision 250585 after a build-and-test.

Thanks,
James

---
2017-07-26  James Greenhalgh  

* config/aarch64/aarch64.c (cortexa57_addrcost_table): Remove.
(qdf24xx_addrcost_table): Likewise.
(cortexa57_tunings): Update to use generic_branch_cost.
(cortexa72_tunings): Likewise.
(cortexa73_tunings): Likewise.
(qdf24xx_tunings): Likewise.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 31c0d8b..9aa59e7 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -206,22 +206,6 @@ static const struct cpu_addrcost_table generic_addrcost_table =
   0 /* imm_offset  */
 };
 
-static const struct cpu_addrcost_table cortexa57_addrcost_table =
-{
-{
-  1, /* hi  */
-  0, /* si  */
-  0, /* di  */
-  1, /* ti  */
-},
-  0, /* pre_modify  */
-  0, /* post_modify  */
-  0, /* register_offset  */
-  0, /* register_sextend  */
-  0, /* register_zextend  */
-  0, /* imm_offset  */
-};
-
 static const struct cpu_addrcost_table exynosm1_addrcost_table =
 {
 {
@@ -254,22 +238,6 @@ static const struct cpu_addrcost_table xgene1_addrcost_table =
   0, /* imm_offset  */
 };
 
-static const struct cpu_addrcost_table qdf24xx_addrcost_table =
-{
-{
-  1, /* hi  */
-  0, /* si  */
-  0, /* di  */
-  1, /* ti  */
-},
-  0, /* pre_modify  */
-  0, /* post_modify  */
-  0, /* register_offset  */
-  0, /* register_sextend  */
-  0, /* register_zextend  */
-  0 /* imm_offset  */
-};
-
 static const struct cpu_addrcost_table thunderx2t99_addrcost_table =
 {
 {
@@ -647,7 +615,7 @@ static const struct tune_params cortexa53_tunings =
 static const struct tune_params cortexa57_tunings =
 {
   &cortexa57_extra_costs,
-  &cortexa57_addrcost_table,
+  &generic_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
   &generic_branch_cost,
@@ -673,7 +641,7 @@ static const struct tune_params cortexa57_tunings =
 static const struct tune_params cortexa72_tunings =
 {
   &cortexa57_extra_costs,
-  &cortexa57_addrcost_table,
+  &generic_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
   &generic_branch_cost,
@@ -699,7 +667,7 @@ static const struct tune_params cortexa72_tunings =
 static const struct tune_params cortexa73_tunings =
 {
   &cortexa57_extra_costs,
-  &cortexa57_addrcost_table,
+  &generic_addrcost_table,
   &cortexa57_regmove_cost,
   &cortexa57_vector_cost,
   &generic_branch_cost,
@@ -828,7 +796,7 @@ static const struct tune_params xgene1_tunings =
 static const struct tune_params qdf24xx_tunings =
 {
   &qdf24xx_extra_costs,
-  &qdf24xx_addrcost_table,
+  &generic_addrcost_table,
   &qdf24xx_regmove_cost,
   &generic_vector_cost,
   &generic_branch_cost,

Re: [PATCH] Initialize counters in autoFDO to zero, not to uninitialized.

[Jeff Law]

On 07/11/2017 04:35 AM, Martin Liška wrote:
> Hello.
> 
> This fixes majority of autoFDO test-cases.
> 
> Patch can boostrap and survives regression tests.
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2017-07-11  Martin Liska  
> 
> * auto-profile.c (afdo_annotate_cfg): Assign zero counts to
> BBs and edges seen by autoFDO.
I went back and forth on this a couple times.  I could argue that if we
don't have data for the edge from auto-fdo, then the proper value is
uninitialized().  But I think the response to that argument is that if
the edge didn't show up in the afdo run, then it's counters should be
zero'd as they weren't triggered during the afdo run.

I think a comment immediately prior to the initialization seems wise. OK
with that change.

jeff

Re: [PATCH] Fix when -lssp is added by driver (PR middle-end/81400).

[Jeff Law]

On 07/12/2017 07:38 AM, Martin Liška wrote:
> Hi.
> 
> Following patch adds -lspp when one uses -mstack-protector-guard=global.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> 
> Martin
> 
> gcc/ChangeLog:
> 
> 2017-07-12  Martin Liska  
> 
> PR middle-end/81400
> * gcc.c: Add -lssp when one uses -mstack-protector-guard=global.
Isn't the -m option target specific?  And doesn't that imply this change
should somehow be done in the target's specs?

jeff

Re: [PATCH] Validate that Init value is within range defined by IntegerRange.

[Jeff Law]

On 07/13/2017 02:03 AM, Martin Liška wrote:
> Hello.
> 
> I'm sending the patch that does validation of values of Init.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> 
> 0001-Validate-that-Init-value-is-within-range-defined-by-.patch
> 
> 
> From 46d8c6b9030d677629a4f7c647efe73f4c2ddf0c Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Mon, 10 Jul 2017 10:33:49 +0200
> Subject: [PATCH] Validate that Init value is within range defined by
>  IntegerRange.
> 
> gcc/ChangeLog:
> 
> 2017-07-13  Martin Liska  
> 
>   * opt-functions.awk: Add validation of value of Init.
>   * optc-gen.awk: Pass new argument.
I'm going to assume you have a good reason to special case "-1" for init
and not warn on that.

OK.

jeff

Re: [PATCH] [PowerPC/RTEMS] Add 64-bit support using ELFv2 ABI

[Segher Boessenkool]

Hi!

On Tue, Jul 25, 2017 at 02:25:21PM +0200, Sebastian Huber wrote:
> Add 64-bit support for RTEMS using the ELFv2 ABI with 64-bit long
> double.
> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 2ae0218b5fc..aab7f65c1df 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -2424,7 +2424,7 @@ powerpc-*-rtems*spe*)
>   tmake_file="${tmake_file} powerpcspe/t-fprules powerpcspe/t-rtems 
> powerpcspe/t-ppccomm"
>   ;;
>  powerpc-*-rtems*)
> - tm_file="${tm_file} dbxelf.h elfos.h freebsd-spec.h newlib-stdint.h 
> rs6000/sysv4.h rs6000/eabi.h rs6000/rtems.h rtems.h"
> + tm_file="rs6000/biarch64.h ${tm_file} dbxelf.h elfos.h freebsd-spec.h 
> newlib-stdint.h rs6000/sysv4.h rs6000/rtems.h rtems.h"

This deletes eabi.h and I don't see you add all its definitions to
rtems.h directly (NAME__MAIN etc.)  Is this on purpose?


Segher

Re: [rs6000] Avoid rotates of floating-point modes

[Segher Boessenkool]

Hi!

On Tue, Jul 25, 2017 at 04:10:19PM +0100, Richard Sandiford wrote:
> Segher Boessenkool  writes:
> --- gcc/config/rs6000/rs6000.c2017-07-13 09:25:13.909213921 +0100
> +++ gcc/config/rs6000/rs6000.c2017-07-25 11:14:27.692739547 +0100
> @@ -10503,17 +10503,28 @@ rs6000_const_vec (machine_mode mode)
>  
>  /* Generate a permute rtx that represents an lxvd2x, stxvd2x, or xxpermdi
> for a VSX load or store operation.  */
> -rtx
> -rs6000_gen_le_vsx_permute (rtx source, machine_mode mode)
> +void
> +rs6000_emit_le_vsx_permute (rtx dest, rtx source, machine_mode mode)

Please update the comment.  With that, okay for trunk.  Thanks!


Segher

Re: [PATCH,AIX] Changes for linking gotools on AIX.

[Ian Lance Taylor]

On Wed, Jul 26, 2017 at 9:58 AM, David Edelsohn  wrote:
> On Wed, Jul 26, 2017 at 12:41 PM, REIX, Tony  wrote:
>> Description:
>>  * This patch adds linker options for gotools for AIX.
>>
>> Tests:
>>  * Fedora25/x86_64 + GCC trunk : Configure/Build: SUCCESS
>>- build remade by means of gmake.
>>- some test redone in libgo (gmake check)
>>  * AIX + GCC 7.1.0 :
>>- build remade by means of gmake.
>>- some test redone in libgo (gmake check)
>>
>> ChangeLog:
>>  * Makefile.am (AM_LDFLAGS & GOLINK): Changes for linking on AIX.
>>  * Makefile.in: Rebuild.
>
> If this is trying to fix AIX search paths, a better solution would
> seem to be the equivalent of -static-libstdc++ -static-libgcc.  The Go
> tools should be linked statically and not depend on Go shared
> libraries.

On GNU/Linux I used to use -static-libgo, but I changed it because of
https://gcc.gnu.org/PR64738.  Of course on AIX we can do as you
prefer.

Ian

Re: [PATCH 1/2] x86,s390: add compiler memory barriers when expanding atomic_thread_fence (PR 80640)

[Alexander Monakov]

On Wed, 26 Jul 2017, Jeff Law wrote:
> I'm not sure what you mean by extraneous compiler barriers -- isn't the
> worst case scenario here that the target emits them as well?  So there
> would be an extraneous one in that case, but that ought to be a "don't
> care".

Yes, exactly this.

> In the middle end patch, do we need a barrier before the fence as well?
> The post-fence barrier prevents reordering the fence with anything which
> follows the fence.  But do we have to also prevent reordering the fence
> with prior instructions with any of the memory models?  This isn't my
> area of expertise, so if it's dumb question, don't hesitate to let me
> know :-)

That depends on how pessimistic we want to be with respect to backend
getting it wrong.  My expectation here is that if a backend emits non-empty
RTL, the produced sequence for the fence itself acts as a compiler memory
barrier.

Thanks!
Alexander

Re: [PATCH] PR libstdc++/53984 handle exceptions in basic_istream::sentry

[Paolo Carlini]

Hi again,

On 26/07/2017 16:27, Paolo Carlini wrote:

Hi,

On 26/07/2017 16:21, Andreas Schwab wrote:
ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: 
dg-require-file-io 18 {} for " dg-require-file-io 18 "" " 

Should be already fixed, a trivial typo.
... but now the new test simply fails for me. If I don't spot something 
else trivial over the next few hours I guess better waiting for Jon to 
look into that.


Paolo.

[C++ Patch] PR 71570 ("[6/7/8 regression] ICE on invalid variable capture in cxx_incomplete_type_diagnostic...")

[Paolo Carlini]

Hi,

avoiding the error recovery issues noticed in the bug seems just matter 
of early returning error_mark_node from add_capture, like we already do 
a few lines below in a similar case of ill-formed capture. Tested 
x86_64-linux. If we are going to fix this in a simply way, maybe we 
could also backport to 7...


Thanks, Paolo.



/cp
2017-07-26  Paolo Carlini  

PR c++/71570
* lambda.c (add_capture): Early return if we cannot capture by
reference.

/testsuite
2017-07-26  Paolo Carlini  

PR c++/71570
* g++.dg/cpp0x/lambda/lambda-ice17.C: New.
Index: cp/lambda.c
===
--- cp/lambda.c (revision 250586)
+++ cp/lambda.c (working copy)
@@ -529,7 +529,10 @@ add_capture (tree lambda, tree id, tree orig_init,
   else if (id != this_identifier && by_reference_p)
{
  if (!lvalue_p (initializer))
-   error ("cannot capture %qE by reference", initializer);
+   {
+ error ("cannot capture %qE by reference", initializer);
+ return error_mark_node;
+   }
}
   else
{
Index: testsuite/g++.dg/cpp0x/lambda/lambda-ice17.C
===
--- testsuite/g++.dg/cpp0x/lambda/lambda-ice17.C(revision 0)
+++ testsuite/g++.dg/cpp0x/lambda/lambda-ice17.C(working copy)
@@ -0,0 +1,12 @@
+// PR c++/71570
+// { dg-do compile { target c++11 } }
+
+void foo (int);
+
+void foo (void)
+{
+  [&foo] // { dg-error "cannot capture" }
+  {
+foo (0);
+  };
+}

Re: [PATCH] x86: Properly check register CFA offset

[Uros Bizjak]

On Wed, Jul 26, 2017 at 6:14 PM, H.J. Lu  wrote:
>
> X86 epilogue saves register at CFA offset.  Since its location on stack
> is computed as CFA - its CFA_OFFSET, CFA_OFFSET points the end of the
> saved register location on stack.  This patch updates sp_valid_at and
> fp_valid_at to properly check register CFA offset.
>
> Tested on x86-64.  OK for trunk if there are no regressions on i686?
>
> Thanks.
>
> H.J.
> --
> gcc/
>
> PR target/81563
> * config/i386/i386.c (sp_valid_at): Properly check CFA offset.
> (fp_valid_at): Likewise.
>
> gcc/testsuite/
>
> PR target/81563
> * gcc.target/i386/pr81563.c: New test

OK.

Thanks,
Uros.

> ---
>  gcc/config/i386/i386.c  | 10 ++
>  gcc/testsuite/gcc.target/i386/pr81563.c | 14 ++
>  2 files changed, 20 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr81563.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 084b4a6a0db..f1486ff3750 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -13102,24 +13102,26 @@ choose_baseaddr_len (unsigned int regno, 
> HOST_WIDE_INT offset)
>return len;
>  }
>
> -/* Determine if the stack pointer is valid for accessing the cfa_offset.  */
> +/* Determine if the stack pointer is valid for accessing the cfa_offset.
> +   The register is saved at CFA - CFA_OFFSET.  */
>
>  static inline bool
>  sp_valid_at (HOST_WIDE_INT cfa_offset)
>  {
>const struct machine_frame_state &fs = cfun->machine->fs;
>return fs.sp_valid && !(fs.sp_realigned
> - && cfa_offset < fs.sp_realigned_offset);
> + && cfa_offset <= fs.sp_realigned_offset);
>  }
>
> -/* Determine if the frame pointer is valid for accessing the cfa_offset.  */
> +/* Determine if the frame pointer is valid for accessing the cfa_offset.
> +   The register is saved at CFA - CFA_OFFSET.  */
>
>  static inline bool
>  fp_valid_at (HOST_WIDE_INT cfa_offset)
>  {
>const struct machine_frame_state &fs = cfun->machine->fs;
>return fs.fp_valid && !(fs.sp_valid && fs.sp_realigned
> - && cfa_offset >= fs.sp_realigned_offset);
> + && cfa_offset > fs.sp_realigned_offset);
>  }
>
>  /* Choose a base register based upon alignment requested, speed and/or
> diff --git a/gcc/testsuite/gcc.target/i386/pr81563.c 
> b/gcc/testsuite/gcc.target/i386/pr81563.c
> new file mode 100644
> index 000..ebfd583daf5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr81563.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-O2 -maccumulate-outgoing-args -mincoming-stack-boundary=2 
> -mpreferred-stack-boundary=3 -mregparm=3 -mtune-ctrl=epilogue_using_move" } */
> +
> +extern void bar (long long int, int);
> +
> +long long int
> +fn1 (long long int x)
> +{
> +  bar (x, 1);
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler-times "movl\[\\t \]*-8\\(%ebp\\),\[\\t 
> \]*%esi" 1 } } */
> +/* { dg-final { scan-assembler-times "movl\[\\t \]*-4\\(%ebp\\),\[\\t 
> \]*%edi" 1 } } */
> --
> 2.13.3
>

Re: [PING] [PATCH v4 0/12] [i386] Improve 64-bit Microsoft to System V ABI pro/epilogues

[H.J. Lu]

On Sun, May 14, 2017 at 3:23 AM, Uros Bizjak  wrote:
> On Sun, May 14, 2017 at 12:34 AM, Daniel Santos  
> wrote:
>> On 05/13/2017 11:52 AM, Uros Bizjak wrote:
>>>
>>> On Sat, May 13, 2017 at 1:01 AM, Daniel Santos 
>>> wrote:

 Ping?  I have posted revisions of the following in patch set:

 05/12 - https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01442.html
 09/12 - https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00348.html
 11/12 - https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00350.html

 I have retested them on Linux x86-64 in addition a Wine testsuite
 comparison
 resulting in fewer failed tests (31) than when using unpatched 7.1.0 (78)
 and 5.4.0 (78).  A cursory examination of the now working failures with
 7.1.0 seemed to be to be due to race conditions in Wine that are
 incidentally hidden after the patches.

 Is there anything else needed before we can commit these?  They still
 rebase
 cleanly onto the HEAD, but I can repost as "v5" if you prefer.
>>>
>>> Please go ahead and commit the patches.
>>>
>>> However, please stay around to fix possible fallout. As said - you are
>>> touching quite complex part of the compiler ...
>>>
>>> Thanks,
>>> Uros.
>>
>>
>> Thanks!  I'll definitely be around, I have a lot more that I'm working on
>> with C generics/pseudo-templates (all middle-end stuff). I also want to
>> examine more ways that SSE saves/restores can be omitted in these ms to sysv
>> calls through static analysis and such.
>>
>> Anyway, I don't yet have SVN write access, will you sponsor my request?
>
> The patchset was committed to mainline SVN as r248029.
>
> Uros.

This patch caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81563


-- 
H.J.

[PATCH v5] aarch64: Add split-stack initial support

[Adhemerval Zanella]

This is an update patch based on my previous submission [1].  The changes
from previous version are:

  - Update aarch64_supports_split_stack to return true an let the loader
to actually emit an error if it does not provide the required TCB
field.  The TCB field support is planed for GLIBC 2.27.

  - Some cleanup on morestack-c.c to avoid code duplication (ss_pointer
function).

I am still some sporadic failures, but it due missing linker support
for split calls due not enough stack size (on pprof and other go tests
that call backtrace on signal handling).  This could be mitigate by
increasing the BACKOFF to a higher value, but to not deviate from other
ports with split-stack support this patch is using the default values
(initial stack size of 16k and backoff of 4k).  This should be correctly
handled with proper gold suppor (as for other ports).

--

This patch adds the split-stack support on aarch64 (PR #67877).  As for
other ports this patch should be used along with glibc and gold support.

The support is done similar to other architectures: a split-stack field
is allocated before TCB by glibc, a target-specific __morestack implementation
and helper functions are added in libgcc and compiler supported in adjusted
(split-stack prologue, va_start for argument handling).  I also plan to
send the gold support to adjust stack allocation acrosss split-stack
and default code calls.

Current approach is to set the final stack adjustments using a 2 instructions
at most (mov/movk) which limits stack allocation to upper limit of 4GB.
The morestack call is non standard with x10 hollding the requested stack
pointer, x11 the argument pointer (if required), and x12 to return
continuation address.  Unwinding is handled by a personality routine that
knows how to find stack segments.

Split-stack prologue on function entry is as follow (this goes before the
usual function prologue):

function:
mrsx9, tpidr_el0
movx10, 
movk   x10, #0x0, lsl #16
subx10, sp, x10
movx11, sp  # if function has stacked arguments
adrp   x12, main_fn_entry
addx12, x12, :lo12:.L2
cmpx9, x10
b.lt   
b  __morestack
main_fn_entry:
[function prologue]

Notes:

1. Even if a function does not allocate a stack frame, a split-stack prologue
   is created.  It is to avoid issues with tail call for external symbols
   which might require linker adjustment (libgo/runtime/go-varargs.c).

2. Basic-block reordering (enabled with -O2) will move split-stack TCB ldr
   to after the required stack calculation.

3. Similar to powerpc, When the linker detects a call from split-stack to
   non-split-stack code, it adds 16k (or more) to the value found in "allocate"
   instructions (so non-split-stack code gets a larger stack).  The amount is
   tunable by a linker option.  The edit means aarch64 does not need to
   implement __morestack_non_split, necessary on x86 because insufficient
   space is available there to edit the stack comparison code.  This feature
   is only implemented in the GNU gold linker.

4. AArch64 does not handle >4G stack initially and although it is possible
   to implement it, limiting to 4G allows to materize the allocation with
   only 2 instructions (mov + movk) and thus simplifying the linker
   adjustments required.  Supporting multiple threads each requiring more
   than 4G of stack is probably not that important, and likely to OOM at
   run time.

5. The TCB support on GLIBC is meant to be included in version 2.26.

6. The continuation address materialized on x12 is done using 'adrp'
   plus add and a static relocation.  Current code uses the
   aarch64_expand_mov_immediate function and since a better alternative
   would be 'adp', it could be a future optimization (not implemented
   in this patch).

libgcc/ChangeLog:

* libgcc/config.host: Use t-stack and t-statck-aarch64 for
aarch64*-*-linux.
* libgcc/config/aarch64/morestack-c.c: New file.
* libgcc/config/aarch64/morestack.S: Likewise.
* libgcc/config/aarch64/t-stack-aarch64: Likewise.
* libgcc/generic-morestack.c (__splitstack_find): Add aarch64-specific
code.

gcc/ChangeLog:

* common/config/aarch64/aarch64-common.c
(aarch64_supports_split_stack): New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* gcc/config/aarch64/aarch64-linux.h (TARGET_ASM_FILE_END): Remove
macro.
* gcc/config/aarch64/aarch64-protos.h: Add
aarch64_expand_split_stack_prologue and
aarch64_split_stack_space_check.
* gcc/config/aarch64/aarch64.c (aarch64_gen_far_branch): Add suport
to emit 'b' instruction to rtx different than LABEL_REF.
(aarch64_expand_builtin_va_start): Use internal argument pointer
instead of virtual_incoming_args_rtx.
(morestack_ref): New symbol.
(aarch64_load_split_stack_value): New fu

Re: [PATCH] Improve alloca alignment

[Jeff Law]

On 07/26/2017 11:39 AM, Wilco Dijkstra wrote:
> This patch improves alloca alignment.  Currently alloca reserves
> too much space as it aligns twice, and generates unnecessary stack
> alignment code.  For example alloca (16) generates:
> 
>   sub sp, sp, #32   ???
>   mov x1, sp
> 
> Similarly alloca (x) generates:
> 
>   add x0, x0, 30???
>   and x0, x0, -16
>   sub sp, sp, x0
>   mov x0, sp
> 
> __builtin_alloca_with_align (x, 256):
>   add x0, x0, 78???
>   and x0, x0, -16
>   sub sp, sp, x0
>   add x0, sp, 63
>   and x0, x0, -64
> 
> As can be seen the alignment adjustment value is incorrect.
> When the requested alignment is lower than the stack alignment, no
> extra alignment is needed.  If the requested alignment is higher,
> we need to increase the size by the difference of the requested 
> alignment and the stack alignment.  As a result, the alloca alignment
> is exactly as expected:
> 
> alloca (16):
>   sub sp, sp, #16
>   mov x1, sp
> 
> alloca (x):
>   add x0, x0, 15
>   and x0, x0, -16
>   sub sp, sp, x0
>   mov x0, sp
> 
> __builtin_alloca_with_align (x, 256):
>   add x0, x0, 63
>   and x0, x0, -16
>   sub sp, sp, x0
>   add x0, sp, 63
>   and x0, x0, -64
> 
> ChangeLog:
> 2017-07-26  Wilco Dijkstra  
> 
>   * explow.c (get_dynamic_stack_size): Improve dynamic alignment.
Hehe.  This stuff is a mess.  Dominik went round and round in this code
about a year ago, I'm not sure we ever got it right for the cases he
wanted to improve.

This is something I wanted to look at but had deferred (it makes writing
some stack-clash tests of this code more painful than it really needs to
be).


> 
> --
> diff --git a/gcc/explow.c b/gcc/explow.c
> index 
> 50074e281edd5270c76d29feac6b7a92f598d11d..fbdda5fa1e303664e346f975270415b40aed252d
>  100644
> --- a/gcc/explow.c
> +++ b/gcc/explow.c
> @@ -1234,15 +1234,22 @@ get_dynamic_stack_size (rtx *psize, unsigned 
> size_align,
>   example), so we must preventively align the value.  We leave space
>   in SIZE for the hole that might result from the alignment operation.  */
>  
> -  extra = (required_align - BITS_PER_UNIT) / BITS_PER_UNIT;
> -  size = plus_constant (Pmode, size, extra);
> -  size = force_operand (size, NULL_RTX);
> -
> -  if (flag_stack_usage_info && pstack_usage_size)
> -*pstack_usage_size += extra;
> +  /* Since the stack is presumed to be aligned before this allocation,
> + we only need to increase the size of the allocation if the required
> + alignment is more than the stack alignment.
> + Note size_align doesn't need to be updated - if it is larger than the
> + stack alignment, size remains a multiple of the stack alignment, so
> + we can skip rounding up to the stack alignment.  */
> +  if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT)
> +{
> +  extra = (required_align - MAX_SUPPORTED_STACK_ALIGNMENT)
> + / BITS_PER_UNIT;
> +  size = plus_constant (Pmode, size, extra);
> +  size = force_operand (size, NULL_RTX);
>  
> -  if (extra && size_align > BITS_PER_UNIT)
> -size_align = BITS_PER_UNIT;
> +  if (flag_stack_usage_info && pstack_usage_size)
> + *pstack_usage_size += extra;
> +}
So it's unclear to me why you increase the size iff REQUIRED_ALIGN is
greater than MAX_SUPPORTED_STACK_ALIGNMENT.

ISTM the safe assumption we can make in this code is that the stack is
aligned to STACK_BOUNDARY.  If so, isn't the right test

(required_align > STACK_BOUNDARY)?

And once we decide to make an adjustment, isn't the right adjustment to
make (required_align - STACK_BOUNDARY) / BITS_PER_UNIT?

I feel like I must be missing something really important here.

jeff

Re: [PATCH] x86: Properly check register CFA offset

[H.J. Lu]

On Wed, Jul 26, 2017 at 12:04 PM, Uros Bizjak  wrote:
> On Wed, Jul 26, 2017 at 6:14 PM, H.J. Lu  wrote:
>>
>> X86 epilogue saves register at CFA offset.  Since its location on stack
>> is computed as CFA - its CFA_OFFSET, CFA_OFFSET points the end of the
>> saved register location on stack.  This patch updates sp_valid_at and
>> fp_valid_at to properly check register CFA offset.
>>
>> Tested on x86-64.  OK for trunk if there are no regressions on i686?
>>
>> Thanks.
>>
>> H.J.
>> --
>> gcc/
>>
>> PR target/81563
>> * config/i386/i386.c (sp_valid_at): Properly check CFA offset.
>> (fp_valid_at): Likewise.
>>
>> gcc/testsuite/
>>
>> PR target/81563
>> * gcc.target/i386/pr81563.c: New test
>
> OK.
>

This is what I checked in with the updated commit log.

Thanks.

-- 
H.J.
From f7ccc9fc8419d08d8dbf9bd3ed3eec244196a6db Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Wed, 26 Jul 2017 09:06:22 -0700
Subject: [PATCH] x86: Properly check saved register CFA offset

X86 prologue saves register at CFA offset.  Since its location on stack
is computed as CFA - its CFA_OFFSET, CFA_OFFSET points the end of the
saved register area on stack.  This patch updates sp_valid_at and
fp_valid_at to properly check saved register CFA offset.

gcc/

	PR target/81563
	* config/i386/i386.c (sp_valid_at): Properly check CFA offset.
	(fp_valid_at): Likewise.

gcc/testsuite/

	PR target/81563
	* gcc.target/i386/pr81563.c: New test
---
 gcc/config/i386/i386.c  | 10 ++
 gcc/testsuite/gcc.target/i386/pr81563.c | 14 ++
 2 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr81563.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 084b4a6a0db..f1486ff3750 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13102,24 +13102,26 @@ choose_baseaddr_len (unsigned int regno, HOST_WIDE_INT offset)
   return len;
 }
 
-/* Determine if the stack pointer is valid for accessing the cfa_offset.  */
+/* Determine if the stack pointer is valid for accessing the cfa_offset.
+   The register is saved at CFA - CFA_OFFSET.  */
 
 static inline bool
 sp_valid_at (HOST_WIDE_INT cfa_offset)
 {
   const struct machine_frame_state &fs = cfun->machine->fs;
   return fs.sp_valid && !(fs.sp_realigned
-			  && cfa_offset < fs.sp_realigned_offset);
+			  && cfa_offset <= fs.sp_realigned_offset);
 }
 
-/* Determine if the frame pointer is valid for accessing the cfa_offset.  */
+/* Determine if the frame pointer is valid for accessing the cfa_offset.
+   The register is saved at CFA - CFA_OFFSET.  */
 
 static inline bool
 fp_valid_at (HOST_WIDE_INT cfa_offset)
 {
   const struct machine_frame_state &fs = cfun->machine->fs;
   return fs.fp_valid && !(fs.sp_valid && fs.sp_realigned
-			  && cfa_offset >= fs.sp_realigned_offset);
+			  && cfa_offset > fs.sp_realigned_offset);
 }
 
 /* Choose a base register based upon alignment requested, speed and/or
diff --git a/gcc/testsuite/gcc.target/i386/pr81563.c b/gcc/testsuite/gcc.target/i386/pr81563.c
new file mode 100644
index 000..ebfd583daf5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr81563.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -maccumulate-outgoing-args -mincoming-stack-boundary=2 -mpreferred-stack-boundary=3 -mregparm=3 -mtune-ctrl=epilogue_using_move" } */
+
+extern void bar (long long int, int);
+
+long long int
+fn1 (long long int x)
+{
+  bar (x, 1);
+  return x;
+}
+
+/* { dg-final { scan-assembler-times "movl\[\\t \]*-8\\(%ebp\\),\[\\t \]*%esi" 1 } } */
+/* { dg-final { scan-assembler-times "movl\[\\t \]*-4\\(%ebp\\),\[\\t \]*%edi" 1 } } */
-- 
2.13.3

Re: [PATCH][AArch64] Simplify frame layout for stack probing

[Jeff Law]

On 07/25/2017 07:58 AM, Wilco Dijkstra wrote:
> This patch makes some changes to the frame layout in order to simplify
> stack probing.  We want to use the save of LR as a probe in any non-leaf
> function.  With shrinkwrapping we may only save LR before a call, so it
> is useful to define a fixed location in the callee-saves. So force LR at
> the bottom of the callee-saves even with -fomit-frame-pointer.
> 
> Also remove a rarely used frame layout that saves the callee-saves first
> with -fomit-frame-pointer.
> 
> OK for commit (and backport to GCC7)?
> 
> ChangeLog:
> 2017-07-25  Wilco Dijkstra  
> 
>   * config/aarch64/aarch64.c (aarch64_layout_frame):
>   Ensure LR is always stored at the bottom of the callee-saves.
>   Remove frame option which saves callee-saves at top of frame.
I'll let the appropriate aarch64 maintainers comment on correctness.
But I wanted to give an explicit thanks for simplifying this so that we
can rely on it.

Jeff

Re: [PATCH] enhance -Wrestrict for sprintf %s arguments

[Jeff Law]

On 07/19/2017 10:10 AM, Martin Sebor wrote:
> On 07/19/2017 12:42 AM, Jeff Law wrote:
>> On 07/02/2017 02:00 PM, Martin Sebor wrote:
>>> The attached patch enhances the -Wrestrict warning to detect more
>>> than just trivial instances of overlapping copying by sprintf and
>>> related functions.
>>>
>>> The meat of the patch is relatively simple but because it introduces
>>> dependencies between existing classes in the sprintf pass I had to
>>> move the class definitions around.  That makes the changes look more
>>> extensive than they really are.
>>>
>>> The enhancement works by first determining the base object (or
>>> pointer) from the destination of the sprintf call, the constant
>>> offset into the object, and its size.  For each %s argument, it
>>> then computes same information.  If it determines that overlap
>>> between the two is possible it stores the information for the
>>> directive argument (including the size of the argument) for later
>>> processing.  After the whole call/format string has been processed,
>>> the patch then iterates over the stored directives and their
>>> arguments and compares the size/length of the argument against
>>> the function's overall output.  If they overlap it issues
>>> a warning.
>>>
>>> Tested on x86_64-linux.
>>>
>>> -Wrestrict is not currently included in either -Wextra or -Wall
>>> and this patch doesn't change it even though there have been
>>> requests to add it to one of these two options.  I'd like to do
>>> that as a separate step.
>> Yea, I think separate step is wise.
>>
>>>
>>> As the next step I'd also like to extend a limited form of the
>>> -Wrestrict enhancement to other restrict-qualified built-ins (like
>>> strcpy), and ultimately also to user-defined functions that make
>>> use of restrict.  I think this might perhaps best be done in
>>> a separate pass where the computed pointer information can be
>>> cached to avoid recomputing it for each call, but I don't expect
>>> to be able to have the new pass (or whatever form the enhancement
>>> might end up taking) to also handle sprintf (at least not with
>>> the same accuracy it does now) because the sprintf data for each
>>> format directive are not available outside the sprintf pass.
>> Seems reasonable.  Actual implementation will tell us for sure :-)
>>
>>
>>>
>>> Martin
>>>
>>> gcc-35503.diff
>>>
>>>
>>> PR tree-optimization/35503 - Warning about restricted pointers?
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>> PR tree-optimization/35503
>>> * gcc/c-family/c-common.c (check_function_restrict): Avoid
>>> diagnosing
>>> sprintf et al. unless both -Wformat-overflow and -Wformat-truncation
>>> are disabled.
>>>
>>> gcc/ChangeLog:
>>>
>>> PR tree-optimization/35503
>>> * gimple-ssa-sprintf.c (format_result::alias_info): New struct.
>>> (directive::argno): New member.
>>> (format_result::aliases, format_result::alias_count): New data
>>> members.
>>> (format_result::append_alias): New member function.
>>> (fmtresult::dst_offset): New data member.
>>> (pass_sprintf_length::call_info::dst_origin): New data member.
>>> (pass_sprintf_length::call_info::dst_field, dst_offset): Same.
>>> (char_type_p, array_elt_at_offset, field_at_offset): New functions.
>>> (get_origin_and_offset): Same.
>>> (format_string): Call it.
>>> (format_directive): Call append_alias and set directive argument
>>> number.
>>> (pass_sprintf_length::compute_format_length): Diagnose arguments
>>> that overlap the destination buffer.
>>> (pass_sprintf_length::handle_gimple_call): Initialize new members.
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR tree-optimization/35503
>>> * gcc.dg/tree-ssa/builtin-sprintf-warn-19.c: New test.
>> I'm OK with the general concept of enhancing the warning.  The big
>> question I have is whether or not we'd be better off using the alias
>> oracle here rather than what appears to be rolling our own data
>> structures and analysis routines to describe memory objects and their
>> potential alias relationship.
>>
>> See tree-ssa-alias.h.  In particular you're looking for ao_ref.  You may
>> also be intersted in the points-to solutions.  Would using that
>> infrastructure make sense?
> 
> This patch make only limited use of the alias analysis which
> is in the current form overly broad (i.e., it answers the "may
> alias" question, not the "must alias" one).  That makes it
> suitable for throttling optimization but not so much to trigger
> warnings.  The other challenge is that the information the
> sprintf pass needs to detect overlaps is being computed in
> stages as each directive is being processed, and then again
> when the whole format string has been processed and the size
> of the full output is known.
I thought we had a must-alias query as well.  Though it may not be
properly named :-)Hmm, perhaps not, DSE uses stmt_kills_ref_p which
seems to do most of the necessary checks inline.


> 
> That said, it certainly makes

[PATCH] Fix PR middle-end/81564: ICE in group_case_labels_stmt()

[Peter Bergner]

The test case for PR81564 exposes an issue where the case labels for a
switch statement point to blocks that have already been removed by an
earlier call to cleanup_tree_cfg().  In that case, the code in
group_case_labels_stmt() that does:

  base_bb = label_to_block (CASE_LABEL (base_case));

...returns a NULL base_bb and we SEGV later on when we dereference it:

  if (EDGE_COUNT (base_bb->succs) == 0
  ...

The fix here is to just treat case labels that point to blocks that have
already been deleted similarly to case labels that point to the default
case statement, by removing them.

This passed bootstrap and regtesting on powerpc64le-linux with no regressions.
Ok for trunk?

Peter

gcc/
PR middle-end/81564
* tree-cfg.c (group_case_labels_stmt): Handle already deleted blocks.

gcc/testsuite/
PR middle-end/81564
* gcc.dg/pr81564.c: New test.

Index: gcc/tree-cfg.c
===
--- gcc/tree-cfg.c  (revision 250581)
+++ gcc/tree-cfg.c  (working copy)
@@ -1701,8 +1701,9 @@ group_case_labels_stmt (gswitch *stmt)
   gcc_assert (base_case);
   base_bb = label_to_block (CASE_LABEL (base_case));
 
-  /* Discard cases that have the same destination as the default case.  */
-  if (base_bb == default_bb)
+  /* Discard cases that have the same destination as the default case or
+whose destination blocks have already been removed as unreachable.  */
+  if (base_bb == NULL || base_bb == default_bb)
{
  i++;
  continue;
Index: gcc/testsuite/gcc.dg/pr81564.c
===
--- gcc/testsuite/gcc.dg/pr81564.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/pr81564.c  (working copy)
@@ -0,0 +1,21 @@
+/* PR middle-end/81564 ICE in group_case_labels_stmt().  */
+/* { dg-do compile }  */
+/* { dg-options "-O2" }  */
+
+struct a {
+int b;
+int c;
+};
+
+void
+foo (void)
+{
+  struct a *e;
+  switch (e->c)
+  {
+case 7:
+case 3:
+  if (__builtin_expect(!0, 0))
+   __builtin_unreachable();
+  }
+}

Re: [C++ Patch] PR 71570 ("[6/7/8 regression] ICE on invalid variable capture in cxx_incomplete_type_diagnostic...")

[Jason Merrill]

OK; this is trivial enough for 6/7/8.

On Wed, Jul 26, 2017 at 2:49 PM, Paolo Carlini  wrote:
> Hi,
>
> avoiding the error recovery issues noticed in the bug seems just matter of
> early returning error_mark_node from add_capture, like we already do a few
> lines below in a similar case of ill-formed capture. Tested x86_64-linux. If
> we are going to fix this in a simply way, maybe we could also backport to
> 7...
>
> Thanks, Paolo.
>
> 
>

Re: [PATCH #4, cleanup] Remove PowerPC -mvsx-small-integer

[Segher Boessenkool]

Hi Mike,

On Wed, Jul 26, 2017 at 12:24:17AM -0400, Michael Meissner wrote:
>   * config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER): Delete
>   -mvsx-small-integer option.
>   (ISA_3_0_MASKS_IEEE): Likewise.
>   (POWERPC_MASKS): Likewise.

(OTHER_VSX_VECTOR_MASKS): Likewise.

> @@ -2099,20 +2099,8 @@ rs6000_hard_regno_mode_ok (int regno, ma

> -   if (TARGET_VSX_SMALL_INTEGER)
> - {
> -   if (mode == SImode)
> - return 1;

I don't see why deleting this is correct?

> -   if (TARGET_P9_VECTOR && (mode == HImode || mode == QImode))
> - return 1;

And this.

The rest looks fine I think.


Segher