Re: [PATCH] gdbinit: add a new command and fix one

[Martin Liška]

On 5/29/19 8:18 PM, Segher Boessenkool wrote:
> On Wed, May 29, 2019 at 10:14:58AM -0600, Jeff Law wrote:
>> On 5/29/19 3:46 AM, Martin Liška wrote:
>>> Hi.
>>>
>>> The patch is about a small change in .gdbinit file.
>>>
>>> Ready for trunk?
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2019-05-29  Martin Liska  
>>>
>>> * gdbinit.in: Fix 'ptc' command.  Add tt
>>> that prints TREE_TYPE($).
>>> ---
>>>  gcc/gdbinit.in | 10 +-
>>>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> There already *is* a "tt" command. 

Oops. I'm renaming that to trt and I'm going to install that.

Martin

> Not that that one is likely very
> useful for debugging GCC, but still...  Please check before overriding
> commands.
> 
> 
> Segher
> 

>From 5e49f23ecedcc5278f14278a6bccd6d003b5cc60 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Wed, 29 May 2019 11:45:07 +0200
Subject: [PATCH] gdbinit: add a new command and fix one

gcc/ChangeLog:

2019-05-29  Martin Liska  

	* gdbinit.in: Fix 'ptc' command.  Add trt
	that prints TREE_TYPE($).
---
 gcc/gdbinit.in | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in
index e16c3c8ef87..440fd2586f1 100644
--- a/gcc/gdbinit.in
+++ b/gcc/gdbinit.in
@@ -113,7 +113,7 @@ Works only when an inferior is executing.
 end
 
 define ptc
-output (enum tree_code) $.common.code
+output (enum tree_code) $.base.code
 echo \n
 end
 
@@ -201,6 +201,14 @@ document pcfun
 Print current function.
 end
 
+define trt
+print ($.typed.type)
+end
+
+document trt
+Print TREE_TYPE of the tree node that is $
+end
+
 define break-on-diagnostic
 break diagnostic_show_locus
 end
-- 
2.21.0

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Alex Henrie]

On Tue, May 28, 2019 at 1:05 PM Martin Sebor  wrote:
>
> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > Then is it preferable to simply silence Wattributes in this case?
>
> This case being PR86407?  I'd say yes.

> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.
>
> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?
>
> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?

Those are very good points. The more information the optimizer has
about how the function works, the better it can optimize the code near
the function call. This would even apply to ms_hook_prologue because
the optimizer should definitely not inline indirect calls to hookable
functions even if it would inline a similar non-hookable function.

At this point I think I'm convinced that any attribute that applies to
a function should also be allowed on a function pointer without any
warnings. We can start by turning off the warnings for the "fndecl"
attributes and then clean up the other attributes as time goes on.
Sound good?

-Alex

On Tue, May 28, 2019 at 1:05 PM Martin Sebor  wrote:
>
> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:
> >>
> >> I'm not sure we really need a new warning for this.
> >
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> >>
> >> I don't think GCC makes a formal distinction between function
> >> attributes that affect only function definitions vs those that
> >> affect its users, or both.  It might be a useful distinction
> >> to introduce, perhaps even for functions as well as variables,
> >> but as it is, users (as well as GCC developers) are on our own
> >> to figure it out.
> >
> > Then is it preferable to simply silence Wattributes in this case?
>
> This case being PR86407?  I'd say yes.  I think one general problem
> to solve is the missing suppression for typedefs.  This could be
> useful for other warnings as well.  Another is providing a knob to
> control the warning when one kind of an attribute is used with
> an entity of a different kind (function vs type, or variable vs
> type).  Yet another might be to control warnings for individual
> attributes.
>
> >
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:
> >>
> >>> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> >>> +
> >>
> >> But this is a declaration?
> >
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> >>
> >> My first question is: what is the meaning of "function definition
> >> attributes?"  Presumably those that affect only the definition of
> >> a function and not its callers or other users?
> >
> > As far as I can tell, "fndecl" is a misnomer: these attributes are
> > more accurately called "function definition attributes", i.e.
> > attributes that affect the assembly code of the function but do not
> > affect its calling convention.
>
> That's one possible definition but there are examples that don't
> fit it (at least not very neatly).
>
> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.
>
> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?
>
> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?
>
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> >>
> >> Finally, with this as a prerequisite, if we decided that a warning
> >> like this would be useful, tests to verify that it works for all
> >> the definition attributes and not for the rest would need to be
> >> added (i.e., in addition to ms_hook_prologue).
> >
> > Okay, I will add tests for the other function attributes that should
> > behave in the same way, commenting out the tests that will require
> > more work to pass.
> >
> > The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
> > Wine without causing spurious warnings. This will go a long way
> > towards making Wine compatible with cu

Re: [PATCH, x86, testsuite] Two tests that need to be native TLS?

[Uros Bizjak]

On Fri, May 24, 2019 at 9:43 AM Iain Sandoe  wrote:
>
> Hi Uros,
>
> The following two tests fail on Darwin, which is an emulated TLS target.
> ISTM that the tests really required native support, so that the right fix
> is to require that. (I can skip them for Darwin, if this isn’t the right 
> solution).

The test looks at the output of tls_global_dynamic_64_ pattern,
which is only used with native tls.

> OK?

OK.

Thanks,
Uros.

>
> thanks
> Iain
>
> gcc/testsuite/
>
> * gcc.target/i386/pr86257.c: Require native TLS support.
> * gcc.target/i386/stack-prot-sym.c: Likewise.
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr86257.c 
> b/gcc/testsuite/gcc.target/i386/pr86257.c
> index 07fbba9..bc758c2 100644
> --- a/gcc/testsuite/gcc.target/i386/pr86257.c
> +++ b/gcc/testsuite/gcc.target/i386/pr86257.c
> @@ -1,6 +1,6 @@
>  /* { dg-require-effective-target lp64 } */
>  /* { dg-require-effective-target fpic } */
> -/* { dg-require-effective-target tls } */
> +/* { dg-require-effective-target tls_native } */
>  /* { dg-options "-g -fPIC -mtls-dialect=gnu" } */
>
>  __thread int i;
> diff --git a/gcc/testsuite/gcc.target/i386/stack-prot-sym.c 
> b/gcc/testsuite/gcc.target/i386/stack-prot-sym.c
> index 7f63424..dcd7cbd 100644
> --- a/gcc/testsuite/gcc.target/i386/stack-prot-sym.c
> +++ b/gcc/testsuite/gcc.target/i386/stack-prot-sym.c
> @@ -1,4 +1,5 @@
>  /* { dg-do compile } */
> +/* { dg-require-effective-target tls_native } */
>  /* { dg-options "-O2 -fstack-protector-all -mstack-protector-guard=tls 
> -mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=my_guard" } */
>
>  void f(void) { }
>

Re: [Patch] Fix ix86_expand_sse_comi_round (PR Target/89750, PR Target/86444)

[Hongtao Liu]

On Thu, May 30, 2019 at 3:23 AM Jeff Law  wrote:
>
> On 5/9/19 10:54 PM, Hongtao Liu wrote:
> > On Fri, May 10, 2019 at 3:55 AM Jeff Law  wrote:
> >>
> >> On 5/6/19 11:38 PM, Hongtao Liu wrote:
> >>> Hi Uros and GCC:
> >>>   This patch is to fix ix86_expand_sse_comi_round whose implementation
> >>> was not correct.
> >>>   New implentation aligns with _mm_cmp_round_s[sd]_mask.
> >>>
> >>> Bootstrap and regression tests for x86 is fine.
> >>> Ok for trunk?
> >>>
> >>>
> >>> ChangeLog:
> >>> gcc/
> >>>* config/i386/i386-expand.c (ix86_expand_sse_comi_round):
> >>>Modified, original implementation isn't correct.
> >>>
> >>> gcc/testsuite
> >>>* gcc.target/i386/avx512f-vcomisd-2.c: New runtime tests.
> >>>* gcc.target/i386/avx512f-vcomisd-2.c: Likewise.
> >> So you'll have to bear with me, I'm not really familiar with this code,
> >> but in the absence of a maintainer I'll try to work through it.
> >>
> >>
> >>>
> >>> -- BR, Hongtao
> >>>
> >>>
> >>> 0001-Fix-ix86_expand_sse_comi_round.patch
> >>>
> >>> Index: gcc/ChangeLog
> >>> ===
> >>> --- gcc/ChangeLog (revision 270933)
> >>> +++ gcc/ChangeLog (working copy)
> >>> @@ -1,3 +1,11 @@
> >>> +2019-05-06  H.J. Lu  
> >>> + Hongtao Liu  
> >>> +
> >>> + PR Target/89750
> >>> + PR Target/86444
> >>> + * config/i386/i386-expand.c (ix86_expand_sse_comi_round):
> >>> + Modified, original implementation isn't correct.
> >>> +
> >>>  2019-05-06  Segher Boessenkool  
> >>>
> >>>   * config/rs6000/rs6000.md (FIRST_ALTIVEC_REGNO, LAST_ALTIVEC_REGNO)
> >>> Index: gcc/config/i386/i386-expand.c
> >>> ===
> >>> --- gcc/config/i386/i386-expand.c (revision 270933)
> >>> +++ gcc/config/i386/i386-expand.c (working copy)
> >>> @@ -9853,18 +9853,24 @@
> >>>const struct insn_data_d *insn_p = &insn_data[icode];
> >>>machine_mode mode0 = insn_p->operand[0].mode;
> >>>machine_mode mode1 = insn_p->operand[1].mode;
> >>> -  enum rtx_code comparison = UNEQ;
> >>> -  bool need_ucomi = false;
> >>>
> >>>/* See avxintrin.h for values.  */
> >>> -  enum rtx_code comi_comparisons[32] =
> >>> +  static const enum rtx_code comparisons[32] =
> >> So I assume the comment refers to the _CMP_* #defines in avxintrin.h?
> >>
> >   Yes.
> >>
> >>>  {
> >>> -  UNEQ, GT, GE, UNORDERED, LTGT, UNLE, UNLT, ORDERED, UNEQ, UNLT,
> >>> -  UNLE, LT, LTGT, GE, GT, LT, UNEQ, GT, GE, UNORDERED, LTGT, UNLE,
> >>> -  UNLT, ORDERED, UNEQ, UNLT, UNLE, LT, LTGT, GE, GT, LT
> >>> +  EQ, LT, LE, UNORDERED, NE, UNGE, UNGT, ORDERED,
> >>> +  EQ, UNLT, UNLE, UNORDERED, LTGT, GE, GT, ORDERED,
> >>> +  EQ, LT, LE, UNORDERED, NE, UNGE, UNGT, ORDERED,
> >>> +  EQ, UNLT, UNLE, UNORDERED, LTGT, GE, GT, ORDERED
> >>>  };
> >>
> >> For CMP_EQ_UQ aren't we looking for an unordered comparison, so UNEQ
> >> seems right, but you're using EQ.  Can you double-check this?  If it's
> >> wrong, then please make sure we cover this case with a test.
> >>
> > Avx512f-vcomis[sd]-2.c covers all 32 compare predicates.
> > UNEQ and EQ behave differently when either operand is NAN, besides
> > they're the same.
> > Since NAN operands are handled separtely, so EQ/UNEQ makes no
> > difference, That why this passes cover tests.
> > I'll correct it.
> Ah.  Thanks.  FWIW my approach was to walk through the _CMP_* defines in
> avxintrin.h and map that to what I thought the comparison ought to be.
> Then I reviewed my result against your patch.  I got a couple wrong, but
> could easily see my mistake.  The only one I couldn't reconcile was the
> CMP_EQ_UQ.  Knowing the NaNs are handled separately makes it clear.
> Thanks gain.
>
>
>
> >>
> >>
> >>
> >>> @@ -9932,11 +10021,37 @@
> >>>  }
> >>>
> >>>emit_insn (pat);
> >>> +
> >>> +  /* XXX: Set CCFPmode and check a different CCmode.  Does it work
> >>> + correctly?  */
> >>> +  if (GET_MODE (set_dst) != mode)
> >>> +set_dst = gen_rtx_REG (mode, REGNO (set_dst));
> >> This looks worrisome, even without the cryptic comment.  I don't think
> >> you can just blindly change the mode like that.  Unless you happen to
> >> know that the only things you test in the new mode were set in precisely
> >> the same way as the old mode.
> >>
> > Modified as:
> > +  /* NB: Set CCFPmode and check a different CCmode.  */
> > +  if (GET_MODE (set_dst) != mode)
> > +set_dst = gen_rtx_REG (mode, FLAGS_REG);
> That might actually be worse.  The mode carries semantic information
> about where to find the various condition codes within the flags
> register and which condition codes are valid.  The register number
> determines which (of possibly many) flags registers we are querying.
>
> Thus if the mode of SET_DEST is not the same as MODE, then there is a
> mismatch between the point where the condition codes were set and where
> we want to use them.
>
> That can only be safe

Re: [libgomp, testsuite] Generalize getconf _NPROCESSORS_ONLN

[Rainer Orth]

Hi Jakub,

>>> > So, something similar to what your patch does, but don't substitute the
>>> > actual
>>> > CPU count, but a command to determine the number of CPUs?
>>> 
>>> I'm not convinced that would work reliably: you can easily have one set
>>> of commands on one machine, but a slightly different one on another.  If
>>> we really want to go this route, that means extracting the autoconf
>>> macro's logic into a separate script and use that at make check time.
>>> Not sure if that's worth the effort, especially since we're not really
>>> interested in the exact number of cores, just small (<= 8) vs. large (> 8).
>>
>> Ok, so can we do a middle-ground, instead of the current change to
>> Makefile.am just replace the two spots that do num_cpus=1 with
>> num_cpus=@CPU_COUNT@
>> and keep the getconf invocation in there?  On Linux it will do what it used
>> to do, and on targets that don't support getconf _NPROCESSORS_ONLN it will
>> use a configure time determined value?
>
> fine with me.  I'll send an updated patch after testing.

this version does this and passed the same testing as the previous one.
Ok for mainline now?

Thanks.
Rainer

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


2019-05-28  Rainer Orth  

libgomp:
* configure.ac: Call AX_COUNT_CPUS.
Substitute CPU_COUNT.
* testsuite/Makefile.am (check-am): Use CPU_COUNT as processor
count fallback.
* aclocal.m4: Regenerate.
* configure: Regenerate.
* Makefile.in, testsuite/Makefile.in: Regenerate.

config:
* ax_count_cpus.m4: New file.

# HG changeset patch
# Parent  af986e73ea19567ed6d9553b898d7e3d475a528f
Generalize getconf _NPROCESSORS_ONLN

diff --git a/config/ax_count_cpus.m4 b/config/ax_count_cpus.m4
new file mode 100644
--- /dev/null
+++ b/config/ax_count_cpus.m4
@@ -0,0 +1,101 @@
+# ===
+#  https://www.gnu.org/software/autoconf-archive/ax_count_cpus.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_COUNT_CPUS([ACTION-IF-DETECTED],[ACTION-IF-NOT-DETECTED])
+#
+# DESCRIPTION
+#
+#   Attempt to count the number of logical processor cores (including
+#   virtual and HT cores) currently available to use on the machine and
+#   place detected value in CPU_COUNT variable.
+#
+#   On successful detection, ACTION-IF-DETECTED is executed if present. If
+#   the detection fails, then ACTION-IF-NOT-DETECTED is triggered. The
+#   default ACTION-IF-NOT-DETECTED is to set CPU_COUNT to 1.
+#
+# LICENSE
+#
+#   Copyright (c) 2014,2016 Karlson2k (Evgeny Grin) 
+#   Copyright (c) 2012 Brian Aker 
+#   Copyright (c) 2008 Michael Paul Bailey 
+#   Copyright (c) 2008 Christophe Tournayre 
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 22
+
+  AC_DEFUN([AX_COUNT_CPUS],[dnl
+  AC_REQUIRE([AC_CANONICAL_HOST])dnl
+  AC_REQUIRE([AC_PROG_EGREP])dnl
+  AC_MSG_CHECKING([the number of available CPUs])
+  CPU_COUNT="0"
+
+  # Try generic methods
+
+  # 'getconf' is POSIX utility, but '_NPROCESSORS_ONLN' and
+  # 'NPROCESSORS_ONLN' are platform-specific
+  command -v getconf >/dev/null 2>&1 && \
+CPU_COUNT=`getconf _NPROCESSORS_ONLN 2>/dev/null || getconf NPROCESSORS_ONLN 2>/dev/null` || CPU_COUNT="0"
+  AS_IF([[test "$CPU_COUNT" -gt "0" 2>/dev/null || ! command -v nproc >/dev/null 2>&1]],[[: # empty]],[dnl
+# 'nproc' is part of GNU Coreutils and is widely available
+CPU_COUNT=`OMP_NUM_THREADS='' nproc 2>/dev/null` || CPU_COUNT=`nproc 2>/dev/null` || CPU_COUNT="0"
+  ])dnl
+
+  AS_IF([[test "$CPU_COUNT" -gt "0" 2>/dev/null]],[[: # empty]],[dnl
+# Try platform-specific preferred methods
+AS_CASE([[$host_os]],dnl
+  [[*linux*]],[[CPU_COUNT=`lscpu -p 2>/dev/null | $EGREP -e '^@<:@0-9@:>@+,' -c` || CPU_COUNT="0"]],dnl
+  [[*darwin*]],[[CPU_COUNT=`sysctl -n hw.logicalcpu 2>/dev/null` || CPU_COUNT="0"]],dnl
+  [[freebsd*]],[[command -v sysctl >/dev/null 2>&1 && CPU_COUNT=`sysctl -n kern.smp.cpus 2>/dev/null` || CPU_COUNT="0"]],dnl
+  [[netbsd*]], [[command -v sysctl >/dev/null 2>&1 && CPU_COUNT=`sysctl -n hw.ncpuonline 2>/dev/null` || CPU_COUNT="0"]],dnl
+  [[solaris*]],[[command -v psrinfo >/dev/null 2>&1 && CPU_COUNT=`psrinfo 2>/dev/null | $EGREP -e '^@<:@0-9@:>@.*on-line' -c 2>/dev/null` || CPU_COUNT="0"]],dnl
+  [[mingw*]],[[CPU_COUNT=`ls -qpU1 /proc/registry/HKEY_LOCAL_MACHINE/HARDWARE/DESCRIPTION/System/CentralProcessor/ 2>/dev/null | $EGREP -e '^@<:@0-9@:>@+/' -c` || CPU_COUNT="0"]],dnl
+  [[msys*]],[[CPU_COUNT=

Re: [libgomp, testsuite] Generalize getconf _NPROCESSORS_ONLN

[Jakub Jelinek]

On Thu, May 30, 2019 at 10:59:59AM +0200, Rainer Orth wrote:
> this version does this and passed the same testing as the previous one.
> Ok for mainline now?
> 
> Thanks.
> Rainer
> 
> -- 
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University
> 
> 
> 2019-05-28  Rainer Orth  
> 
>   libgomp:
>   * configure.ac: Call AX_COUNT_CPUS.
>   Substitute CPU_COUNT.
>   * testsuite/Makefile.am (check-am): Use CPU_COUNT as processor
>   count fallback.
>   * aclocal.m4: Regenerate.
>   * configure: Regenerate.
>   * Makefile.in, testsuite/Makefile.in: Regenerate.
> 
>   config:
>   * ax_count_cpus.m4: New file.

Ok, thanks.

Jakub

Re: [AArch64] [SVE] PR88837 - Poor vector construction code in VL-specific mode

[Prathamesh Kulkarni]

On Wed, 29 May 2019 at 18:10, Richard Sandiford
 wrote:
>
> Prathamesh Kulkarni  writes:
> > Hi,
> > The attached patch tries to improve initialization for fixed-length
> > SVE vector and it's algorithm is described in comments for
> > aarch64_sve_expand_vector_init() in the patch, with help from Richard
> > Sandiford. I verified tests added in the patch pass with qemu and am
> > trying to run bootstrap+test on patch in qemu.
> > Does the patch look OK ?
> >
> > Thanks,
> > Prathamesh
> >
> > 2019-05-27  Prathamesh Kulkarni  
> >   Richard Sandiford  
>
> Although we iterated on ideas for the patch a bit, I didn't write
> any of it, so the changelog should just have your name.
>
> > [...]
> > @@ -3207,3 +3207,15 @@
> >  DONE;
> >}
> >  )
> > +
> > +;; Standard pattern name vec_init.
> > +
> > +(define_expand "vec_init"
>
> The rest of the file doesn't have blank lines after the comment.
>
> > +/* Subroutine of aarch64_sve_expand_vector_init for handling
> > +   trailing constants.
> > +   This function works as follows:
> > +   (a) Create a new vector consisting of trailing constants.
> > +   (b) Initialize TARGET with the constant vector using emit_move_insn.
> > +   (c) Insert remaining elements in TARGET using insr.
> > +   NELTS is the total number of elements in original vector while
> > +
>
> truncated sentence, guess the rest would have been:
>
>NELTS_REQD is the number of elements that are actually significant.
>
> or something.
>
> > +   ??? The heuristic used is to do above only if number of constants
> > +   is at least half the total number of elements.  May need fine tuning.  
> > */
> > +
> > +static bool
> > +aarch64_sve_expand_vector_init_handle_trailing_constants
> > + (rtx target, const rtx_vector_builder &builder, int nelts, int nelts_reqd)
> > +{
> > +  machine_mode mode = GET_MODE (target);
> > +  scalar_mode elem_mode = GET_MODE_INNER (mode);
> > +  int n_trailing_constants = 0;
> > +
> > +  for (int i = nelts_reqd - 1;
> > +   i >= 0 && aarch64_legitimate_constant_p (elem_mode, builder.elt 
> > (i));
> > +   i--)
> > +n_trailing_constants++;
> > +
> > +  if (n_trailing_constants >= nelts_reqd / 2)
> > +{
> > +  rtx_vector_builder v (mode, 1, nelts);
> > +  for (int i = 0; i < nelts; i++)
> > + v.quick_push (builder.elt (i + nelts_reqd - n_trailing_constants));
> > +  rtx const_vec = v.build ();
> > +  emit_move_insn (target, const_vec);
> > +
> > +  for (int i = nelts_reqd - n_trailing_constants - 1; i >= 0; i--)
> > + emit_insr (target, builder.elt (i));
> > +
> > +  return true;
> > +}
> > +
> > +  return false;
> > +}
> > +
> > +/* Subroutine of aarch64_sve_expand_vector_init.
> > +   Works as follows:
> > +   (a) Initialize TARGET by broadcasting element NELTS_REQD - 1 of BUILDER.
> > +   (b) Skip trailing elements from BUILDER, which are same as
>
> s/are same/are the same/
>
> > +   element NELTS_REQD - 1.
> > +   (c) Insert earlier elements in reverse order in TARGET using insr.  */
> > +
> > +static void
> > +aarch64_sve_expand_vector_init_insert_elems (rtx target,
> > +  const rtx_vector_builder 
> > &builder,
> > +  int nelts_reqd)
> > +{
> > +  machine_mode mode = GET_MODE (target);
> > +  scalar_mode elem_mode = GET_MODE_INNER (mode);
> > +
> > +  struct expand_operand ops[2];
> > +  enum insn_code icode = optab_handler (vec_duplicate_optab, mode);
> > +  gcc_assert (icode != CODE_FOR_nothing);
> > +
> > +  create_output_operand (&ops[0], target, mode);
> > +  create_input_operand (&ops[1], builder.elt (nelts_reqd - 1), elem_mode);
> > +  expand_insn (icode, 2, ops);
> > +
> > +  int ndups = builder.count_dups (nelts_reqd - 1, -1, -1);
> > +  for (int i = nelts_reqd - ndups - 1; i >= 0; i--)
> > +emit_insr (target, builder.elt (i));
> > +}
> > +
> > +/* Subroutine of aarch64_sve_expand_vector_init to handle case
> > +   when all trailing elements of builder are same.
> > +   This works as follows:
> > +   (a) Using expand_insn interface to broadcast last vector element in 
> > TARGET.
>
> s/Using/Use/
>
> > +   (b) Insert remaining elements in TARGET using insr.
> > +
> > +   ??? The heuristic used is to do above if number of same trailing 
> > elements
> > +   is at least 3/4 of total number of elements, loosely based on
> > +   heuristic from mostly_zeros_p. May need fine-tuning.  */
>
> Should be two spaces before "May".
>
> > [...]
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/init_1.c 
> > b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> > new file mode 100644
> > index 000..c51876947fb
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> > @@ -0,0 +1,27 @@
> > +/* { dg-do compile { target aarch64_asm_sve_ok } } */
> > +/* { dg-options "-O2 -ftree-vectorize -fno-schedule-insns 
> > -msve-vector-bits=256 --save-temps" } */
>
> These tests shouldn't require -ftree-vectoriz

Re: [PATCH] Further C lapack workaround tweaks

[Jakub Jelinek]

Hi!

On Wed, May 29, 2019 at 06:41:57PM -0700, Jerry DeLisle wrote:
> Just a typo:
> 
> > +Disallow tail call optimization when a calling routine may have omitted
> character lenghts.
> 
> s/lenghts/lengths

Thanks.  Found similar typo in tree-predcom.c (and in old ChangeLog entries
that I'm not going to change).  Fixed thusly, committed as obvious
to trunk and 9/8 branches.

2019-05-30  Jakub Jelinek  

* tree-predcom.c (is_inv_store_elimination_chain): Fix a typo - lenght
to length.

* lang.opt (ftail-call-workaround=): Fix a typo - lenghts to lengths.

--- gcc/tree-predcom.c.jj   2019-02-01 09:34:50.173636370 +0100
+++ gcc/tree-predcom.c  2019-05-30 11:28:17.949976960 +0200
@@ -1713,7 +1713,7 @@ is_inv_store_elimination_chain (struct l
 
   gcc_assert (!chain->has_max_use_after);
 
-  /* If loop iterates for unknown times or fewer times than chain->lenght,
+  /* If loop iterates for unknown times or fewer times than chain->length,
  we still need to setup root variable and propagate it with PHI node.  */
   tree niters = number_of_latch_executions (loop);
   if (TREE_CODE (niters) != INTEGER_CST
--- gcc/fortran/lang.opt.jj 2019-05-29 16:08:33.840751724 +0200
+++ gcc/fortran/lang.opt2019-05-30 11:28:41.221594732 +0200
@@ -767,7 +767,7 @@ Frotran Alias(ftail-call-workaround=,1,0
 
 ftail-call-workaround=
 Fortran RejectNegative Joined UInteger IntegerRange(0, 2) 
Var(flag_tail_call_workaround) Init(1)
-Disallow tail call optimization when a calling routine may have omitted 
character lenghts.
+Disallow tail call optimization when a calling routine may have omitted 
character lengths.
 
 funderscoring
 Fortran Var(flag_underscoring) Init(1)


Jakub

Re: [AArch64] [SVE] PR88837 - Poor vector construction code in VL-specific mode

[Richard Sandiford]

Prathamesh Kulkarni  writes:
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/init_1.c 
> b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> new file mode 100644
> index 000..cbfeff4a59c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile { target aarch64_asm_sve_ok } } */
> +/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" 
> } */

Sorry for not noticing last time, but the combination of aarch64_asm_sve_ok
and --save-temps only makes sense for assemble tests, not compile tests.
So these should either be:

/* { dg-do assemble { target aarch64_asm_sve_ok } } */
/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" } 
*/

or:

/* { dg-do compile } */
/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256" } */

Might as well as go for the first I guess.  Same for the other
non-run tests.

OK with that change, thanks.

Richard

[PATCH] Update documentation of implementation-defined library features

[Jonathan Wakely]

* doc/xml/manual/status_cxx2011.xml: Use  for
documentation of implementation-defined types for [thread.req.native].
* doc/xml/manual/status_cxx2017.xml: Update documentation of
implementation-defined strings for [variant.bad.access]. Fix typo in
documentation of implementation-defined support for [fs.conform.9945].
* doc/html/*: Regenerate.

Committed to trunk.

commit 041ae901cccdb321d584a644724480bd076ddcf5
Author: Jonathan Wakely 
Date:   Thu May 30 11:13:00 2019 +0100

Update documentation of implementation-defined library features

* doc/xml/manual/status_cxx2011.xml: Use  for
documentation of implementation-defined types for 
[thread.req.native].
* doc/xml/manual/status_cxx2017.xml: Update documentation of
implementation-defined strings for [variant.bad.access]. Fix typo in
documentation of implementation-defined support for 
[fs.conform.9945].
* doc/html/*: Regenerate.

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2011.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
index 9c25b8fd81f..6f3551ff65d 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2011.xml
@@ -2793,31 +2793,44 @@ particular release.
   is subject to change at any time. Any use of
   native_handle is inherently non-portable and
   not guaranteed to work between major releases of GCC.
-  
- thread: The native handle type 
is
-a typedef for __gthread_t i.e. pthread_t
-when GCC is configured with the posix thread
-model. The value of the native handle is undefined for a thread
+  
+  
+ thread
+ 
+The native handle type is a typedef for __gthread_t
+i.e. pthread_t when GCC is configured with the
+posix thread model.
+The value of the native handle is undefined for a thread
 which is not joinable.
- 
- mutex and
-timed_mutex:
+ 
+  
+  
+ mutex
+ timed_mutex
+ 
 The native handle type is __gthread_mutex_t* i.e.
 pthread_mutex_t* for the posix
 thread model.
- 
- recursive_mutex and
- recursive_timed_mutex:
+ 
+  
+  
+ recursive_mutex
+ recursive_timed_mutex
+ 
 The native handle type is __gthread_recursive_mutex_t*
 i.e. pthread_mutex_t* for the posix
 thread model.
- 
- condition_variable: The native
-handle type is __gthread_cond_t* i.e.
+ 
+  
+  
+ condition_variable
+ 
+The native handle type is __gthread_cond_t* i.e.
 pthread_cond_t* for the posix
 thread model.
- 
-  
+ 
+  
+  

 

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
index a11e93cda90..9aba079c251 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2017.xml
@@ -1043,7 +1043,11 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.
 

   23.7.10 [variant.bad.access]
-  what() returns "Unexpected index".
+  what() returns one of the strings
+  "std::get: variant is valueless",
+  "std::get: wrong index for variant",
+  "std::visit: variant is valueless",
+  or "std::visit: variant is valueless".

 

@@ -1117,7 +1121,7 @@ and test for __STDCPP_MATH_SPEC_FUNCS__ >= 
201003L.

   30.10.2.1 [fs.conform.9945]
   The behavior of the filesystem library implementation will depend on
-  the target operating system. Some features will not be not supported
+  the target operating system. Some features will not be supported
   on some targets.

 

[PATCH] Update C++20 status table

[Jonathan Wakely]

The status of P1353R0 was "Partial" because we don't define the
__cpp_lib_three_way_comparison macro, but that's because we don't
support the feature. So the paper can be marked as done.

* doc/xml/manual/status_cxx2020.xml: Add feature-test macro for
P0811R3. Change status of P1353R0.
* doc/html/*: Regenerate.

Committed to trunk.


commit 6ca929538e3a6b166fa3d88f05811cfec1b3ad50
Author: Jonathan Wakely 
Date:   Thu May 30 11:29:58 2019 +0100

Update C++20 status table

The status of P1353R0 was "Partial" because we don't define the
__cpp_lib_three_way_comparison macro, but that's because we don't
support the feature. So the paper can be marked as done.

* doc/xml/manual/status_cxx2020.xml: Add feature-test macro for
P0811R3. Change status of P1353R0.
* doc/html/*: Regenerate.

diff --git a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml 
b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
index c7a543f85d9..8a17747e3bd 100644
--- a/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
+++ b/libstdc++-v3/doc/xml/manual/status_cxx2020.xml
@@ -938,18 +938,17 @@ Feature-testing recommendations for C++.

   
9.1 
-  
+   __cpp_lib_interpolate >= 201902L 
 
 
 
-  
 Missing feature test macros 
   
 http://www.w3.org/1999/xlink"; 
xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1353r0.html";>
P1353R0

   
-   Partial 
+   9.1 
   
 
   

Re: [PATCH] Further C lapack workaround tweaks

[Marek Polacek]

On Thu, May 30, 2019 at 11:37:35AM +0200, Jakub Jelinek wrote:
> Hi!
> 
> On Wed, May 29, 2019 at 06:41:57PM -0700, Jerry DeLisle wrote:
> > Just a typo:
> > 
> > > +Disallow tail call optimization when a calling routine may have omitted
> > character lenghts.
> > 
> > s/lenghts/lengths

There's also another typo:

--- gcc/fortran/lang.opt(revision 271375)
+++ gcc/fortran/lang.opt(revision 271738)
@@ -758,6 +758,13 @@ fsign-zero
 Fortran Var(flag_sign_zero) Init(1)
 Apply negative sign to zero values.

+ftail-call-workaround
+Frotran Alias(ftail-call-workaround=,1,0)
+

Frotran -> Fortran

Marek

Re: [PATCH] Further C lapack workaround tweaks

[Marek Polacek]

On Thu, May 30, 2019 at 07:26:28AM -0400, Marek Polacek wrote:
> On Thu, May 30, 2019 at 11:37:35AM +0200, Jakub Jelinek wrote:
> > Hi!
> > 
> > On Wed, May 29, 2019 at 06:41:57PM -0700, Jerry DeLisle wrote:
> > > Just a typo:
> > > 
> > > > +Disallow tail call optimization when a calling routine may have omitted
> > > character lenghts.
> > > 
> > > s/lenghts/lengths
> 
> There's also another typo:
> 
> --- gcc/fortran/lang.opt(revision 271375)
> +++ gcc/fortran/lang.opt(revision 271738)
> @@ -758,6 +758,13 @@ fsign-zero
>  Fortran Var(flag_sign_zero) Init(1)
>  Apply negative sign to zero values.
> 
> +ftail-call-workaround
> +Frotran Alias(ftail-call-workaround=,1,0)
> +
> 
> Frotran -> Fortran

Thus fixed, applying to trunk and branches.

2019-05-30  Marek Polacek  

* lang.opt (ftail-call-workaround): Fix a typo.

diff --git gcc/fortran/lang.opt gcc/fortran/lang.opt
index b0d31ce20bb..93ea3d3977b 100644
--- gcc/fortran/lang.opt
+++ gcc/fortran/lang.opt
@@ -763,7 +763,7 @@ Fortran Var(flag_sign_zero) Init(1)
 Apply negative sign to zero values.
 
 ftail-call-workaround
-Frotran Alias(ftail-call-workaround=,1,0)
+Fortran Alias(ftail-call-workaround=,1,0)
 
 ftail-call-workaround=
 Fortran RejectNegative Joined UInteger IntegerRange(0, 2) 
Var(flag_tail_call_workaround) Init(1)

Re: [PATCH 3/3][GCC][AARCH64] Add support for pointer authentication B key

[Sam Tebbs]

The fix has been committed as r271780. Apologies for the failure everyone.

Sam

On 29/05/2019 15:22, Sam Tebbs wrote:
> Thanks for finding this Christoph, I had this failure a while ago but it
> stopped happening so I thought all was good. I have a fix ready.
>
> Sam
>
> On 29/05/2019 12:22, Christophe Lyon wrote:
>> On Wed, 29 May 2019 at 11:23, Sam Tebbs  wrote:
>>> The libgcc changes have been acknowledged off-list. Committed as r271735.
>>>
>> After this commit, I'm seeing errors while building libstdc++:
>> 0x11c29f3 aarch64_return_address_signing_enabled()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/aarch64.c:4865
>> 0x11c2a08 aarch64_post_cfi_startproc(_IO_FILE*, tree_node*)
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/aarch64.c:15373
>> 0xa27098 dwarf2out_do_cfi_startproc
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/dwarf2out.c:972
>> 0xa43d6e dwarf2out_begin_prologue(unsigned int, unsigned int, char const*)
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/dwarf2out.c:1106
>> 0xae05d5 final_start_function_1
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:1735
>> 0xae0c2f final_start_function(rtx_insn*, _IO_FILE*, int)
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/final.c:1818
>> 0x11c4442 aarch64_output_mi_thunk
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/aarch64.c:6085
>> 0x9cfa4f cgraph_node::expand_thunk(bool, bool)
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:1831
>> 0x9d0dba cgraph_node::assemble_thunks_and_aliases()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2122
>> 0x9d0d89 cgraph_node::assemble_thunks_and_aliases()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2140
>> 0x9d1068 cgraph_node::expand()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2259
>> 0x9d23ec expand_all_functions
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2332
>> 0x9d23ec symbol_table::compile()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2683
>> 0x9d5020 symbol_table::compile()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2595
>> 0x9d5020 symbol_table::finalize_compilation_unit()
>>   
>> /tmp/8467855_6.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/cgraphunit.c:2861
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See  for instructions.
>> make[5]: *** [Makefile:900: strstream.lo] Error 1
>>
>> in aarch64-none-linux-gnu/libstdc++-v3/src/c++98
>>
>> (same for aarch64[_be]-elf)
>>
>> Christophe
>>
>>> On 01/03/2019 14:12, Sam Tebbs wrote:
 On 31/01/2019 14:54, Sam Tebbs wrote:
> 
>> ping 3. The preceding two patches were committed a while ago but require
>> the minor libgcc changes in this patch, which are the only parts left to
>> be reviewed.
> ping 4
 Attached is a rebased patch made to work on top of Sudi Das' BTI patch
 (by renaming UNSPEC_PACISP to UNSPEC_PACIASP and UNSPEC_PACIBSP in
 aarch64-bti-insert.c). The updated changelog is below.

 Are the libgcc changes OK for trunk?

 gcc/
 2019-03-01  Sam Tebbs

 * config/aarch64/aarch64-builtins.c (aarch64_builtins): Add
 AARCH64_PAUTH_BUILTIN_AUTIB1716 and 
 AARCH64_PAUTH_BUILTIN_PACIB1716.
 * config/aarch64/aarch64-builtins.c 
 (aarch64_init_pauth_hint_builtins):
 Add autib1716 and pacib1716 initialisation.
 * config/aarch64/aarch64-builtins.c (aarch64_expand_builtin): Add 
 checks
 for autib1716 and pacib1716.
 * config/aarch64/aarch64-protos.h (aarch64_key_type,
 aarch64_post_cfi_startproc): Define.
 * config/aarch64/aarch64-protos.h (aarch64_ra_sign_key): Define 
 extern.
 * config/aarch64/aarch64.c 
 (aarch64_handle_standard_branch_protection,
 aarch64_handle_pac_ret_protection): Set default sign key to A.
 * config/aarch64/aarch64.c (aarch64_expand_epilogue,
 aarch64_expand_prologue): Add check for b-key.
 * config/aarch64/aarch64.c (aarch64_ra_sign_key,
 aarch64_post_cfi_startproc, aarch64_handle_pac_ret_b_key): Define.
 * config/aarch64/aarch64.h (TARGET_ASM_POST_CFI_STARTPROC): Define.
 * config/aarch64/aarch64.c (aarch64_pac_ret_subtypes): Add "b-key".
 * config/aarch64/aarch64.md (unspec): Add UNSPEC_AUTIA1716,
 UNSPEC_AUTIB1716, UNSPEC

[PATCH][GCC][AARCH64] Fix libstdc++ build failure after r271735

[Sam Tebbs]

Hi all,

Libstdc++ has been failing to build after I committed r271735. This 
patch fixes
the build failure by checking if the current function is a thunk
before emitting the .cfi_b_key_frame directive.

Committed as obvious in r271780. Apologies for the failure.

2019-05-30  Sam Tebbs  

     * aarch64/aarch64.c (aarch64_post_cfi_startproc): Add
     cfun->is_thunk check.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9e4b335244acea03d974521a2dc6a914317e7727..757a6210ab7d155fa0a122a669d755bb0332fdf0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -15370,7 +15370,7 @@ aarch64_declare_function_name (FILE *stream, const char* name,
 void
 aarch64_post_cfi_startproc (FILE *f, tree ignored ATTRIBUTE_UNUSED)
 {
-  if (aarch64_return_address_signing_enabled ()
+  if (!cfun->is_thunk && aarch64_return_address_signing_enabled ()
   && aarch64_ra_sign_key == AARCH64_KEY_B)
 	asm_fprintf (f, "\t.cfi_b_key_frame\n");
 }

Re: [PATCH][AArch64] Make use of FADDP in simple reductions

[Sudakshina Das]

Hi Elen

Thank you for doing this. You will need a maintainer's approval but I 
would like to add a couple of comments. Please find them inline.

On 08/05/2019 14:36, Elen Kalda wrote:
> Hi,
> 
> This patch adds a pattern to support the FADDP (scalar) instruction.
> 
> Before the patch, the C code
> 
> typedef double v2df __attribute__((vector_size (16)));
> 
> double
> foo (v2df x)
> {
>return x[1] + x[0];
> }
> 
> generated:
> foo:
>  dup d1, v0.d[0]
>  dup d0, v0.d[1]
>  faddd0, d1, d0
>  ret
> 
> After patch:
> foo:
>   faddp   d0, v0.2d
>   ret
> 
> 
> Bootstrapped and done regression tests on aarch64-none-linux-gnu -
> no issues found.
> 
> Best wishes,
> Elen
> 
> 
> gcc/ChangeLog:
> 
> 2019-04-24  Elen Kalda  
> 
>   * config/aarch64/aarch64-simd.md (*aarch64_faddp): New.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-04-24  Elen Kalda  
> 
>   * gcc.target/aarch64/simd/scalar_faddp.c: New test.
> 

 > diff --git a/gcc/config/aarch64/aarch64-simd.md 
b/gcc/config/aarch64/aarch64-simd.md
 > index 
e3852c5d182b70978d7603225fce55c0b8ee2894..89fedc6cb3f0c6eb74c6f8d0b21cedb5ae20a095
 
100644
 > --- a/gcc/config/aarch64/aarch64-simd.md
 > +++ b/gcc/config/aarch64/aarch64-simd.md
 > @@ -2372,6 +2372,21 @@
 >[(set_attr "type" "neon_fp_reduc_add_")]
 >  )
 >
 > +(define_insn "*aarch64_faddp"
 > +  [(set (match_operand: 0 "register_operand" "=w")
 > +(plus:
 > +  (vec_select: (match_operand:VHSDF 1 "register_operand" "w")

I do not think the VHSDF mode should be used here. I believe you may 
have taken this from the vector form of this instruction but that seems 
to be different than the scalar one. Someone with more floating point 
instruction experience can chime in here.

 > +(parallel[(match_operand 2 "const_int_operand" "n")]))
 > +  (vec_select: (match_dup:VHSDF 1)
 > +(parallel[(match_operand 3 "const_int_operand" "n")]]
 > +  "TARGET_SIMD
 > +  && ((INTVAL (operands[2]) == 0 && INTVAL (operands[3]) == 1)

Just some minor indentation issue. The && should be below T

 > +|| (INTVAL (operands[2]) == 1 && INTVAL (operands[3]) == 0))"

Likewise this should be below the second opening brace '('

...

 > --- /dev/null
 > +++ b/gcc/testsuite/gcc.target/aarch64/simd/scalar_faddp.c
 > @@ -0,0 +1,31 @@
 > +/* { dg-do assemble } */

This can be dg-do compile since you only want an assembly file

 > +/* { dg-require-effective-target arm_v8_2a_fp16_scalar_ok } */
 > +/* { dg-add-options arm_v8_2a_fp16_scalar } */
 > +/* { dg-additional-options "-save-temps -O1" } */

The --save-temps can then be removed as the dg-do compile will produce 
the .s file for you

 > +/* { dg-final { scan-assembler-not "dup" } } */
...


Thanks
Sudi

[patch][aarch64]: add support for fabd in sve

[Sylvia Taylor]

Greetings,

This patch adds support in SVE to combine:
- fsub and fabs into fabd

fsubz0.s, z0.s, z1.s
fabsz0.s, p1/m, z0.s
---
fabdz0.s, p1/m, z0.s, z1.s

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk? If yes, I don't have commit rights,
so if someone can please commit it on my behalf.

Cheers,
Syl

gcc/ChangeLog:

2019-05-30  Sylvia Taylor  

* config/aarch64/aarch64-sve.md
(*fabd3): New.

gcc/testsuite/ChangeLog:

2019-05-30  Sylvia Taylor  

* gcc.target/aarch64/sve/fabd.c: New.
diff --git a/gcc/config/aarch64/aarch64-sve.md 
b/gcc/config/aarch64/aarch64-sve.md
index 
3f39c4c5b63798515ed4c109836b036573de4aad..4c46aa55dfc174424ff47447f26c44b038d768ea
 100644
--- a/gcc/config/aarch64/aarch64-sve.md
+++ b/gcc/config/aarch64/aarch64-sve.md
@@ -2528,6 +2528,19 @@
   "\t%0., %1/m, %2."
 )
 
+(define_insn "*fabd3"
+  [(set (match_operand:SVE_F 0 "register_operand" "=w")
+   (unspec:SVE_F
+ [(match_operand: 1 "register_operand" "Upl")
+  (abs:SVE_F
+   (minus:SVE_F
+   (match_operand:SVE_F 2 "register_operand" "0")
+   (match_operand:SVE_F 3 "register_operand" "w")))]
+   UNSPEC_MERGE_PTRUE))]
+  "TARGET_SVE"
+  "fabd\t%0., %1/m, %2., %3."
+)
+
 ;; Unpredicated FRINTy.
 (define_expand "2"
   [(set (match_operand:SVE_F 0 "register_operand")
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/fabd.c 
b/gcc/testsuite/gcc.target/aarch64/sve/fabd.c
new file mode 100644
index 
..13ad83be24ceb0d3319cb3bcfdbd6372b4d1a48e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/fabd.c
@@ -0,0 +1,35 @@
+/* { dg-do assemble { target aarch64_asm_sve_ok } } */
+/* { dg-options "-O3 --save-temps" } */
+
+#define N 16
+
+typedef float *__restrict__ vnx4sf;
+typedef double *__restrict__ vnx2df;
+typedef _Float16 *__restrict__ vnx8hf_a;
+typedef __fp16 *__restrict__ vnx8hf_b;
+
+extern float fabsf (float);
+extern double fabs (double);
+
+#define FABD(type, abs, n) \
+   void fabd_##type (type res, type a, type b) \
+   {   \
+   int i;  \
+   for (i = 0; i < n; i++) \
+   res[i] = abs (a[i] - b[i]); \
+   }
+
+#define TEST_SVE_F_MODES(FUNC) \
+  FUNC (vnx2df, fabs, N)   \
+  FUNC (vnx4sf, fabsf, N)  \
+  FUNC (vnx8hf_a, fabsf, N)\
+  FUNC (vnx8hf_b, fabsf, N)\
+
+TEST_SVE_F_MODES (FABD)
+
+/* { dg-final { scan-assembler "fabd" } } */
+/* { dg-final { scan-assembler-not "fsub" } } */
+/* { dg-final { scan-assembler-not "fabs" } } */
+/* { dg-final { scan-assembler-times {\tfabd\tz[0-9]+\.d, p[0-7]/m, 
z[0-9]+\.d, z[0-9]+\.d\n} 1 } } */
+/* { dg-final { scan-assembler-times {\tfabd\tz[0-9]+\.s, p[0-7]/m, 
z[0-9]+\.s, z[0-9]+\.s\n} 1 } } */
+/* { dg-final { scan-assembler-times {\tfabd\tz[0-9]+\.h, p[0-7]/m, 
z[0-9]+\.h, z[0-9]+\.h\n} 4 } } */

[patch][aarch64]: add usra and ssra combine patterns

[Sylvia Taylor]

Greetings,

This patch adds support to combine:

1) ushr and add into usra, example:

ushrv0.16b, v0.16b, 2
add v0.16b, v0.16b, v2.16b
---
usrav2.16b, v0.16b, 2

2) sshr and add into ssra, example:

sshrv1.16b, v1.16b, 2
add v1.16b, v1.16b, v3.16b
---
ssrav3.16b, v1.16b, 2

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk? If yes, I don't have any commit rights,
so can someone please commit it on my behalf.

Cheers,
Syl

gcc/ChangeLog:

2019-05-30  Sylvia Taylor  

* config/aarch64/aarch64-simd.md
(*aarch64_simd_sra): New.
* config/aarch64/iterators.md
(SHIFTRT): New iterator.
(sra_op): New attribute.

gcc/testsuite/ChangeLog:

2019-05-30  Sylvia Taylor  

* gcc.target/aarch64/simd/ssra.c: New test.
* gcc.target/aarch64/simd/usra.c: New test.
diff --git a/gcc/config/aarch64/aarch64-simd.md 
b/gcc/config/aarch64/aarch64-simd.md
index 
e3852c5d182b70978d7603225fce55c0b8ee2894..502ac5f3b45a1da059bb07701150a531091378ed
 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -953,6 +953,18 @@
   [(set_attr "type" "neon_shift_imm")]
 )
 
+(define_insn "*aarch64_simd_sra"
+ [(set (match_operand:VDQ_I 0 "register_operand" "=w")
+   (plus:VDQ_I
+  (SHIFTRT:VDQ_I
+   (match_operand:VDQ_I 1 "register_operand" "w")
+   (match_operand:VDQ_I 2 "aarch64_simd_rshift_imm" "Dr"))
+  (match_operand:VDQ_I 3 "register_operand" "0")))]
+  "TARGET_SIMD"
+  "sra\t%0., %1., %2"
+  [(set_attr "type" "neon_shift_acc")]
+)
+
 (define_insn "aarch64_simd_imm_shl"
  [(set (match_operand:VDQ_I 0 "register_operand" "=w")
(ashift:VDQ_I (match_operand:VDQ_I 1 "register_operand" "w")
@@ -3110,22 +3122,22 @@
 operands[2] = aarch64_endian_lane_rtx (mode, INTVAL (operands[2]));
 return "smov\\t%0, %1.[%2]";
   }
-  [(set_attr "type" "neon_to_gp")]
-)
-
-(define_insn "*aarch64_get_lane_zero_extend"
-  [(set (match_operand:GPI 0 "register_operand" "=r")
-   (zero_extend:GPI
- (vec_select:
-   (match_operand:VDQQH 1 "register_operand" "w")
-   (parallel [(match_operand:SI 2 "immediate_operand" "i")]]
-  "TARGET_SIMD"
-  {
-operands[2] = aarch64_endian_lane_rtx (mode,
-  INTVAL (operands[2]));
-return "umov\\t%w0, %1.[%2]";
-  }
-  [(set_attr "type" "neon_to_gp")]
+  [(set_attr "type" "neon_to_gp")]
+)
+
+(define_insn "*aarch64_get_lane_zero_extend"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+   (zero_extend:GPI
+ (vec_select:
+   (match_operand:VDQQH 1 "register_operand" "w")
+   (parallel [(match_operand:SI 2 "immediate_operand" "i")]]
+  "TARGET_SIMD"
+  {
+operands[2] = aarch64_endian_lane_rtx (mode,
+  INTVAL (operands[2]));
+return "umov\\t%w0, %1.[%2]";
+  }
+  [(set_attr "type" "neon_to_gp")]
 )
 
 ;; Lane extraction of a value, neither sign nor zero extension
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index 
6caeeac80867edda29b5438efdcee475ed609ff6..6273b7be5932aef695d12e9f723a43cb6c50abe8
 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -1160,6 +1160,8 @@
 ;; This code iterator allows the shifts supported in arithmetic instructions
 (define_code_iterator ASHIFT [ashift ashiftrt lshiftrt])
 
+(define_code_iterator SHIFTRT [ashiftrt lshiftrt])
+
 ;; Code iterator for logical operations
 (define_code_iterator LOGICAL [and ior xor])
 
@@ -1342,6 +1344,9 @@
 (define_code_attr shift [(ashift "lsl") (ashiftrt "asr")
 (lshiftrt "lsr") (rotatert "ror")])
 
+;; Op prefix for shift right and accumulate.
+(define_code_attr sra_op [(ashiftrt "s") (lshiftrt "u")])
+
 ;; Map shift operators onto underlying bit-field instructions
 (define_code_attr bfshift [(ashift "ubfiz") (ashiftrt "sbfx")
   (lshiftrt "ubfx") (rotatert "extr")])
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/ssra.c 
b/gcc/testsuite/gcc.target/aarch64/simd/ssra.c
new file mode 100644
index 
..e9c2e04c0b88ac18be81f4ee8a872e6829af9db2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/ssra.c
@@ -0,0 +1,36 @@
+/* { dg-do compile { target aarch64*-*-* } } */
+/* { dg-options "-O3" } */
+/* { dg-skip-if "" { *-*-* } {"*sve*"} {""} } */
+
+#include 
+
+#define SSRA(func, vtype, n)   \
+   void func ()\
+   {   \
+   int i;  \
+   for (i = 0; i < n; i++) \
+   {   \
+   s1##vtype[i] += s2##vtype[i] >> 2;  \
+   }   \
+   }
+
+#define TEST_VDQ_I_MODES(FUNC)   

Re: [AArch64] [SVE] PR88837 - Poor vector construction code in VL-specific mode

[Prathamesh Kulkarni]

On Thu, 30 May 2019 at 15:10, Richard Sandiford
 wrote:
>
> Prathamesh Kulkarni  writes:
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/init_1.c 
> > b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> > new file mode 100644
> > index 000..cbfeff4a59c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
> > @@ -0,0 +1,27 @@
> > +/* { dg-do compile { target aarch64_asm_sve_ok } } */
> > +/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 
> > --save-temps" } */
>
> Sorry for not noticing last time, but the combination of aarch64_asm_sve_ok
> and --save-temps only makes sense for assemble tests, not compile tests.
> So these should either be:
>
> /* { dg-do assemble { target aarch64_asm_sve_ok } } */
> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" 
> } */
>
> or:
>
> /* { dg-do compile } */
> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256" } */
>
> Might as well as go for the first I guess.  Same for the other
> non-run tests.
>
> OK with that change, thanks.
Thanks for pointing out, updated the patch with dg-do assemble.
Sorry for silly ques - What configure option should be passed to gcc
to generate code with -msve-vector-bits=256 by default ?
I suppose that'd be necessary for correctness testing, to test patch
with run tests that contain initializers and don't explicitly pass
-msve-vector-bits=256 ?

Thanks,
Prathamesh
>
> Richard

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Martin Sebor]

On 5/29/19 11:45 AM, Jeff Law wrote:

On 5/22/19 3:34 PM, Martin Sebor wrote:

-Wreturn-local-addr detects a subset of instances of returning
the address of a local object from a function but the warning
doesn't try to handle alloca or VLAs, or some non-trivial cases
of ordinary automatic variables[1].

The attached patch extends the implementation of the warning to
detect those.  It still doesn't detect instances where the address
is the result of a built-in such strcpy[2].

Tested on x86_64-linux.

Martin

[1] For example, this is only diagnosed with the patch:

   void* f (int i)
   {
 struct S { int a[2]; } s[2];
 return &s->a[i];
   }

[2] The following is not diagnosed even with the patch:

   void sink (void*);

   void* f (int i)
   {
 char a[6];
 char *p = __builtin_strcpy (a, "123");
 sink (p);
 return p;
   }

I would expect detecting to be possible and useful.  Maybe as
a follow-up.

gcc-71924.diff

PR middle-end/71924 - missing -Wreturn-local-addr returning alloca result
PR middle-end/90549 - missing -Wreturn-local-addr maybe returning an address of 
a local array plus offset

gcc/ChangeLog:

PR c/71924
* gimple-ssa-isolate-paths.c (is_addr_local): New function.
(warn_return_addr_local_phi_arg, warn_return_addr_local): Same.
(find_implicit_erroneous_behavior): Call warn_return_addr_local_phi_arg.
(find_explicit_erroneous_behavior): Call warn_return_addr_local.

gcc/testsuite/ChangeLog:

PR c/71924
* gcc.dg/Wreturn-local-addr-2.c: New test.
* gcc.dg/Walloca-4.c: Prune expected warnings.
* gcc.dg/pr41551.c: Same.
* gcc.dg/pr59523.c: Same.
* gcc.dg/tree-ssa/pr88775-2.c: Same.
* gcc.dg/winline-7.c: Same.

diff --git a/gcc/gimple-ssa-isolate-paths.c b/gcc/gimple-ssa-isolate-paths.c
index 33fe352bb23..2933ecf502e 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -341,6 +341,135 @@ stmt_uses_0_or_null_in_undefined_way (gimple *stmt)
return false;
  }
  
+/* Return true if EXPR is a expression of pointer type that refers

+   to the address of a variable with automatic storage duration.
+   If so, set *PLOC to the location of the object or the call that
+   allocated it (for alloca and VLAs).  When PMAYBE is non-null,
+   also consider PHI statements and set *PMAYBE when some but not
+   all arguments of such statements refer to local variables, and
+   to clear it otherwise.  */
+
+static bool
+is_addr_local (tree exp, location_t *ploc, bool *pmaybe = NULL,
+  hash_set *visited = NULL)
+{
+  if (TREE_CODE (exp) == ADDR_EXPR)
+{
+  tree baseaddr = get_base_address (TREE_OPERAND (exp, 0));
+  if (TREE_CODE (baseaddr) == MEM_REF)
+   return is_addr_local (TREE_OPERAND (baseaddr, 0), ploc, pmaybe, 
visited);
+
+  if ((!VAR_P (baseaddr)
+  || is_global_var (baseaddr))
+ && TREE_CODE (baseaddr) != PARM_DECL)
+   return false;
+
+  *ploc = DECL_SOURCE_LOCATION (baseaddr);
+  return true;
+}
+
+  if (TREE_CODE (exp) == SSA_NAME)
+{
+  gimple *def_stmt = SSA_NAME_DEF_STMT (exp);
+  enum gimple_code code = gimple_code (def_stmt);
+
+  if (is_gimple_assign (def_stmt))
+   {
+ tree type = TREE_TYPE (gimple_assign_lhs (def_stmt));
+ if (POINTER_TYPE_P (type))
+   {
+ tree ptr = gimple_assign_rhs1 (def_stmt);
+ return is_addr_local (ptr, ploc, pmaybe, visited);
+   }
+ return false;
+   }
+
+  if (code == GIMPLE_CALL
+ && gimple_call_builtin_p (def_stmt))
+   {
+ tree fn = gimple_call_fndecl (def_stmt);
+ int code = DECL_FUNCTION_CODE (fn);
+ if (code != BUILT_IN_ALLOCA
+ && code != BUILT_IN_ALLOCA_WITH_ALIGN)
+   return false;
+
+ *ploc = gimple_location (def_stmt);
+ return true;
+   }
+
+  if (code == GIMPLE_PHI && pmaybe)
+   {
+ unsigned count = 0;
+ gphi *phi_stmt = as_a  (def_stmt);
+
+ unsigned nargs = gimple_phi_num_args (phi_stmt);
+ for (unsigned i = 0; i < nargs; ++i)
+   {
+ if (!visited->add (phi_stmt))
+   {
+ tree arg = gimple_phi_arg_def (phi_stmt, i);
+ if (is_addr_local (arg, ploc, pmaybe, visited))
+   ++count;
+   }
+   }
+
+ *pmaybe = count && count < nargs;
+ return count != 0;
+   }
+}
+
+  return false;
+}

Is there some reason you didn't query the alias oracle here?  It would
seem a fairly natural fit.  Ultimately given a pointer (which will be an
SSA_NAME) you want to ask whether or not it conclusively points into the
stack.

That would seem to dramatically simplify is_addr_local.


I did think about it but decided against changing the existing
design (iterating over PHI arguments), for a couple of reasons:

1) It feels like a bigger change th

Re: [PATCH] A jump threading opportunity for condition branch

[Jeff Law]

On 5/30/19 12:57 AM, Jiufu Guo wrote:
> Richard Biener  writes:
> 
>> On May 29, 2019 10:21:46 PM GMT+02:00, Jeff Law  wrote:
>>> On 5/24/19 6:45 AM, Richard Biener wrote:
>>> [ Aggressive snipping ]
>>>
 As said in my first review I'd just check whether for the
 edge we want to thread through the definition comes from a CMP.
 Suppose you have

  # val_1 = PHI 
  if (val_1 != 0)

 and only one edge has a b_3 = d_5 != 0 condition it's still
 worth tail-duplicating the if block.
>>> Agreed.  The cost of tail duplicating here is so small we should be
>>> doing it highly aggressively.  About the only case where we might not
>>> want to would be if we're optimizing for size rather than speed.  That
>>> case isn't clearly a win either way.
>>
>> Even there the PHI likely causes edge copies to be inserted. So I
>> wouldn't care for the moment. The proper check would be !
>> Optimize_edge_for_size_p (e).
> For most of this kind of case where the bb contains just one conditional
> jump stmt, it may not increase the size especially for there are
> combinings in follow passes -- it may save size ;)
My point was it's not as clear cut.  Regardless I think we've gone
pretty deep into the weeds.  I think we could easily handle that case as
a follow-up.

jeff

Re: [PATCH] A jump threading opportunity for condition branch

[Jeff Law]

On 5/30/19 12:44 AM, Richard Biener wrote:
> On May 29, 2019 10:21:46 PM GMT+02:00, Jeff Law  wrote:
>> On 5/24/19 6:45 AM, Richard Biener wrote:
>> [ Aggressive snipping ]
>>
>>> As said in my first review I'd just check whether for the
>>> edge we want to thread through the definition comes from a CMP.
>>> Suppose you have
>>>
>>>  # val_1 = PHI 
>>>  if (val_1 != 0)
>>>
>>> and only one edge has a b_3 = d_5 != 0 condition it's still
>>> worth tail-duplicating the if block.
>> Agreed.  The cost of tail duplicating here is so small we should be
>> doing it highly aggressively.  About the only case where we might not
>> want to would be if we're optimizing for size rather than speed.  That
>> case isn't clearly a win either way.
> 
> Even there the PHI likely causes edge copies to be inserted. So I wouldn't 
> care for the moment. The proper check would be ! Optimize_edge_for_size_p 
> (e). 
Agreed, with capitalization fixed :-)
jeff

[wwwdocs] Add sized-deallocation functions to discussion of new/delete

[Jonathan Wakely]

Committed to CVS.

Index: htdocs/gcc-9/porting_to.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/porting_to.html,v
retrieving revision 1.5
diff -u -r1.5 porting_to.html
--- htdocs/gcc-9/porting_to.html	13 May 2019 11:57:03 -	1.5
+++ htdocs/gcc-9/porting_to.html	30 May 2019 15:01:55 -
@@ -144,15 +144,17 @@
 
 The simplest solution is to only replace the ordinary
 operator new(size_t) and
-operator delete(void*)
+operator delete(void*) and
+operator delete(void*, size_t)
 functions, and the replaced versions will be used by all of
 operator new(size_t, nothrow_t),
 operator new[](size_t) and
 operator new[](size_t, nothrow_t)
 and the corresponding operator delete functions.
 To support types with extended alignment you may also need to replace
-operator new(size_t, std::align_val_t) and
-operator delete(void*, std::align_val_t)
+operator new(size_t, align_val_t) and
+operator delete(void*, align_val_t) and
+operator delete(void*, align_val_t)
 (which will then be used by the nothrow and array forms for
 extended alignments).
 

Re: [PATCH V2] A jump threading opportunity for condition branch

[Jiufu Guo]

Jeff Law  writes:

> On 5/29/19 6:36 AM, Richard Biener wrote:
>> On Tue, 28 May 2019, Jiufu Guo wrote:
>> 
>>> Hi,
>>>
>>> This patch implements a new opportunity of jump threading for PR77820.
>>> In this optimization, conditional jumps are merged with unconditional
>>> jump. And then moving CMP result to GPR is eliminated.
>>>
>>> This version is based on the proposal of Richard, Jeff and Andrew, and
>>> refined to incorporate comments.  Thanks for the reviews!
>>>
>>> Bootstrapped and tested on powerpc64le and powerpc64be with no
>>> regressions (one case is improved) and new testcases are added. Is this
>>> ok for trunk?
>>>
>>> Example of this opportunity looks like below:
>>>
>>>   
>>>   p0 = a CMP b
>>>   goto ;
>>>
>>>   
>>>   p1 = c CMP d
>>>   goto ;
>>>
>>>   
>>>   # phi = PHI 
>>>   if (phi != 0) goto ; else goto ;
>>>
>>> Could be transformed to:
>>>
>>>   
>>>   p0 = a CMP b
>>>   if (p0 != 0) goto ; else goto ;
>>>
>>>   
>>>   p1 = c CMP d
>>>   if (p1 != 0) goto ; else goto ;
>>>
>>>
>>> This optimization eliminates:
>>> 1. saving CMP result: p0 = a CMP b.
>>> 2. additional CMP on branch: if (phi != 0).
>>> 3. converting CMP result if there is phi = (INT_CONV) p0 if there is.
>>>
>>> Thanks!
>>> Jiufu Guo
>>>
>>>
>>> [gcc]
>>> 2019-05-28  Jiufu Guo  
>>> Lijia He  
>>>
>>> PR tree-optimization/77820
>>> * tree-ssa-threadedge.c
>>> (edge_forwards_cmp_to_conditional_jump_through_empty_bb_p): New
>>> function.
>>> (thread_across_edge): Add call to
>>> edge_forwards_cmp_to_conditional_jump_through_empty_bb_p.
>>>
>>> [gcc/testsuite]
>>> 2019-05-28  Jiufu Guo  
>>> Lijia He  
>>>
>>> PR tree-optimization/77820
>>> * gcc.dg/tree-ssa/phi_on_compare-1.c: New testcase.
>>> * gcc.dg/tree-ssa/phi_on_compare-2.c: New testcase.
>>> * gcc.dg/tree-ssa/phi_on_compare-3.c: New testcase.
>>> * gcc.dg/tree-ssa/phi_on_compare-4.c: New testcase.
>>> * gcc.dg/tree-ssa/split-path-6.c: Update testcase.
>>>
>>> ---
>>>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c | 30 ++
>>>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c | 23 +++
>>>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c | 25 
>>>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c | 40 +
>>>  gcc/testsuite/gcc.dg/tree-ssa/split-path-6.c |  2 +-
>>>  gcc/tree-ssa-threadedge.c| 76 
>>> +++-
>>>  6 files changed, 192 insertions(+), 4 deletions(-)
>>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
>>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
>>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
>>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c
>>>
>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c 
>>> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
>>> new file mode 100644
>>> index 000..5227c87
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
>>> @@ -0,0 +1,30 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
>>> +
>>> +void g (int);
>>> +void g1 (int);
>>> +
>>> +void
>>> +f (long a, long b, long c, long d, long x)
>>> +{
>>> +  _Bool t;
>>> +  if (x)
>>> +{
>>> +  g (a + 1);
>>> +  t = a < b;
>>> +  c = d + x;
>>> +}
>>> +  else
>>> +{
>>> +  g (b + 1);
>>> +  a = c + d;
>>> +  t = c > d;
>>> +}
>>> +
>>> +  if (t)
>>> +g1 (c);
>>> +
>>> +  g (a);
>>> +}
>>> +
>>> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c 
>>> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
>>> new file mode 100644
>>> index 000..eaf89bb
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
>>> @@ -0,0 +1,23 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
>>> +
>>> +void g (void);
>>> +void g1 (void);
>>> +
>>> +void
>>> +f (long a, long b, long c, long d, int x)
>>> +{
>>> +  _Bool t;
>>> +  if (x)
>>> +t = c < d;
>>> +  else
>>> +t = a < b;
>>> +
>>> +  if (t)
>>> +{
>>> +  g1 ();
>>> +  g ();
>>> +}
>>> +}
>>> +
>>> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c 
>>> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
>>> new file mode 100644
>>> index 000..d5a1e0b
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
>>> @@ -0,0 +1,25 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
>>> +
>>> +void g (void);
>>> +void g1 (void);
>>> +
>>> +void
>>> +f (long a, long b, long c, long d, int x)
>>> +{
>>> +  int t;
>>> +  if (x)
>>> +t = a < b;
>>> +  else if (d == x)
>>> +t = c < b;
>>> +  else
>>> +t = d > c;
>>> +
>>> +  if (t)
>>> +{
>>> +  g1 ();
>>> +

Re: [PATCH v2] rs6000: Add undocumented switch -mprefixed-addr

[Segher Boessenkool]

On Wed, May 29, 2019 at 07:09:47PM -0500, Bill Schmidt wrote:
> Here's another version of the -mprefixed-addr patch.  I've attempted to 
> address
> Segher's comments, other than the suggestion to merge 
> OPTION_MASK_P8_FUSION_SIGN
> into OPTION_MASK_P8_FUSION, which I think is too big a rat's nest for this 
> patch
> series.

Yeah, probably a wise idea.  But we'll need to untangle it some day :-(

>   * rs6000.c (rs6000_option_override_internal): Error if -mpcrel is
>   specified without -mprefixed-addr or -mcpu=future.  Error if
>   -mprefied-addr is specified without -mcpu=future.

Typo.  (Aspell thinks this should be "purified", heh.  It also suggests
"putrefied".  But not the obvious "prefried").

Okay for trunk with that fixed.  Thanks!


Segher

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jeff Law]

On 5/30/19 8:52 AM, Martin Sebor wrote:
> On 5/29/19 11:45 AM, Jeff Law wrote:
>> On 5/22/19 3:34 PM, Martin Sebor wrote:
>>> -Wreturn-local-addr detects a subset of instances of returning
>>> the address of a local object from a function but the warning
>>> doesn't try to handle alloca or VLAs, or some non-trivial cases
>>> of ordinary automatic variables[1].
>>>
>>> The attached patch extends the implementation of the warning to
>>> detect those.  It still doesn't detect instances where the address
>>> is the result of a built-in such strcpy[2].
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> [1] For example, this is only diagnosed with the patch:
>>>
>>>    void* f (int i)
>>>    {
>>>  struct S { int a[2]; } s[2];
>>>  return &s->a[i];
>>>    }
>>>
>>> [2] The following is not diagnosed even with the patch:
>>>
>>>    void sink (void*);
>>>
>>>    void* f (int i)
>>>    {
>>>  char a[6];
>>>  char *p = __builtin_strcpy (a, "123");
>>>  sink (p);
>>>  return p;
>>>    }
>>>
>>> I would expect detecting to be possible and useful.  Maybe as
>>> a follow-up.
>>>
>>> gcc-71924.diff
>>>
>>> PR middle-end/71924 - missing -Wreturn-local-addr returning alloca
>>> result
>>> PR middle-end/90549 - missing -Wreturn-local-addr maybe returning an
>>> address of a local array plus offset
>>>
>>> gcc/ChangeLog:
>>>
>>> PR c/71924
>>> * gimple-ssa-isolate-paths.c (is_addr_local): New function.
>>> (warn_return_addr_local_phi_arg, warn_return_addr_local): Same.
>>> (find_implicit_erroneous_behavior): Call
>>> warn_return_addr_local_phi_arg.
>>> (find_explicit_erroneous_behavior): Call warn_return_addr_local.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR c/71924
>>> * gcc.dg/Wreturn-local-addr-2.c: New test.
>>> * gcc.dg/Walloca-4.c: Prune expected warnings.
>>> * gcc.dg/pr41551.c: Same.
>>> * gcc.dg/pr59523.c: Same.
>>> * gcc.dg/tree-ssa/pr88775-2.c: Same.
>>> * gcc.dg/winline-7.c: Same.
>>>
>>> diff --git a/gcc/gimple-ssa-isolate-paths.c
>>> b/gcc/gimple-ssa-isolate-paths.c
>>> index 33fe352bb23..2933ecf502e 100644
>>> --- a/gcc/gimple-ssa-isolate-paths.c
>>> +++ b/gcc/gimple-ssa-isolate-paths.c
>>> @@ -341,6 +341,135 @@ stmt_uses_0_or_null_in_undefined_way (gimple
>>> *stmt)
>>>     return false;
>>>   }
>>>   +/* Return true if EXPR is a expression of pointer type that refers
>>> +   to the address of a variable with automatic storage duration.
>>> +   If so, set *PLOC to the location of the object or the call that
>>> +   allocated it (for alloca and VLAs).  When PMAYBE is non-null,
>>> +   also consider PHI statements and set *PMAYBE when some but not
>>> +   all arguments of such statements refer to local variables, and
>>> +   to clear it otherwise.  */
>>> +
>>> +static bool
>>> +is_addr_local (tree exp, location_t *ploc, bool *pmaybe = NULL,
>>> +   hash_set *visited = NULL)
>>> +{
>>> +  if (TREE_CODE (exp) == ADDR_EXPR)
>>> +    {
>>> +  tree baseaddr = get_base_address (TREE_OPERAND (exp, 0));
>>> +  if (TREE_CODE (baseaddr) == MEM_REF)
>>> +    return is_addr_local (TREE_OPERAND (baseaddr, 0), ploc, pmaybe,
>>> visited);
>>> +
>>> +  if ((!VAR_P (baseaddr)
>>> +   || is_global_var (baseaddr))
>>> +  && TREE_CODE (baseaddr) != PARM_DECL)
>>> +    return false;
>>> +
>>> +  *ploc = DECL_SOURCE_LOCATION (baseaddr);
>>> +  return true;
>>> +    }
>>> +
>>> +  if (TREE_CODE (exp) == SSA_NAME)
>>> +    {
>>> +  gimple *def_stmt = SSA_NAME_DEF_STMT (exp);
>>> +  enum gimple_code code = gimple_code (def_stmt);
>>> +
>>> +  if (is_gimple_assign (def_stmt))
>>> +    {
>>> +  tree type = TREE_TYPE (gimple_assign_lhs (def_stmt));
>>> +  if (POINTER_TYPE_P (type))
>>> +    {
>>> +  tree ptr = gimple_assign_rhs1 (def_stmt);
>>> +  return is_addr_local (ptr, ploc, pmaybe, visited);
>>> +    }
>>> +  return false;
>>> +    }
>>> +
>>> +  if (code == GIMPLE_CALL
>>> +  && gimple_call_builtin_p (def_stmt))
>>> +    {
>>> +  tree fn = gimple_call_fndecl (def_stmt);
>>> +  int code = DECL_FUNCTION_CODE (fn);
>>> +  if (code != BUILT_IN_ALLOCA
>>> +  && code != BUILT_IN_ALLOCA_WITH_ALIGN)
>>> +    return false;
>>> +
>>> +  *ploc = gimple_location (def_stmt);
>>> +  return true;
>>> +    }
>>> +
>>> +  if (code == GIMPLE_PHI && pmaybe)
>>> +    {
>>> +  unsigned count = 0;
>>> +  gphi *phi_stmt = as_a  (def_stmt);
>>> +
>>> +  unsigned nargs = gimple_phi_num_args (phi_stmt);
>>> +  for (unsigned i = 0; i < nargs; ++i)
>>> +    {
>>> +  if (!visited->add (phi_stmt))
>>> +    {
>>> +  tree arg = gimple_phi_arg_def (phi_stmt, i);
>>> +  if (is_addr_local (arg, ploc, pmaybe, visited))
>>> +    ++count;
>>> +    }
>>> +    }
>>> +
>>> +  *pmaybe = count && count < nargs;
>>> +  return count != 0;
>>> +    }
>>> +    }
>>> +
>>> +  return false;
>>> +}
>> Is the

Make aliasing_component_refs_p to work harder when same_type_for_tbaa returns -1

[Jan Hubicka]

Hi,
this patch makes aliasing_component_refs_p to not give up at the first
-1 returned by types_same_for_tbaa_p and continue searching for pairs of types
we know to be the same.  This affects disambiguations as follows:

With patch:
  refs_may_alias_p: 3013678 disambiguations, 3314059 queries
  ref_maybe_used_by_call_p: 7112 disambiguations, 3039278 queries
  call_may_clobber_ref_p: 817 disambiguations, 817 queries
  aliasing_component_ref_p: 636 disambiguations, 15844 queries
  TBAA oracle: 1417999 disambiguations 2915696 queries
   552182 are in alias set 0
   569795 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   259437 are dependent in the DAG
   116283 are aritificially in conflict with void *

Without:

Alias oracle query stats:
  refs_may_alias_p: 3013194 disambiguations, 3313539 queries
  ref_maybe_used_by_call_p: 7112 disambiguations, 3038794 queries
  call_may_clobber_ref_p: 817 disambiguations, 817 queries
  aliasing_component_ref_p: 152 disambiguations, 14285 queries
  TBAA oracle: 1417999 disambiguations 2914656 queries
   552182 are in alias set 0
   569275 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   258917 are dependent in the DAG
   116283 are aritificially in conflict with void *

Basically all comming from disambiguating
  MEM[(const Element_t[3] &)_116][1];
  usedGuards.upper_m[2];
There are number of similar matches in testsuite.  
More disambiguations would be possible if we did not allow partial
overlaps of arrays.

I had to however plug a problem with alias-2.c testcase (the one about
overlapping arrays):

/* We do not want to treat int[3] as an object that cannot overlap
   itself but treat it as arbitrary sub-array of a larger array object.  */
int ar1(int (*p)[3], int (*q)[3])
{
  (*p)[0] = 1;
  (*q)[1] = 2;
  return (*p)[0];
}
int main()
{
  int a[4];
  if (ar1 ((int (*)[3])&a[1], (int (*)[3])&a[0]) != 2)
__builtin_abort ();
  return 0;
}

Previously indirect_refs_may_alias_p bypased the offset+range test because
it explicitly tests for array types:

  /* But avoid treating arrays as "objects", instead assume they
 can overlap by an exact multiple of their element size.  */
  && TREE_CODE (TREE_TYPE (ptrtype1)) != ARRAY_TYPE)
return ranges_maybe_overlap_p (offset1, max_size1, offset2, max_size2);

Later the code continues to aliasing_component_refs_p which used to give up
comparing int[3] and int with -1 because of:

  /* ??? In Ada, an lvalue of an unconstrained type can be used to access an
 object of one of its constrained subtypes, e.g. when a function with an
 unconstrained parameter passed by reference is called on an object and
 inlined.  But, even in the case of a fixed size, type and subtypes are
 not equivalent enough as to share the same TYPE_CANONICAL, since this
 would mean that conversions between them are useless, whereas they are
 not (e.g. type and subtypes can have different modes).  So, in the end,
 they are only guaranteed to have the same alias set.  */
  if (get_alias_set (type1) == get_alias_set (type2))
return -1;

This is more of an accident and there are cases where we do not trip across
this -1 and we disambiguate array accesses that seems unsafe to me.

With my change aliasing_component_refs_p finds the match of the
array types (type_same_for_tbaa_p returns 1 with non-LTO becuase they
have same canonical type) and disambiguates based on disjoint access ranges.

I have thus went ahead and updated all uses of type_same_for_tbaa_p to special
case arrays and reffer to this testcase (which seems odd and is only one in
testsuite): We can still do useful disambiguation if the array is not toplevel
reference or we know that the memory object is not bigger. This is tested by a
testcase I added and is quite frequent in real world code.

I also added check to give up on VLAs since I can not convicne myself that
this is safe: I think early inlining VLAs and streaming them may lead to
same VLA type have two different sizes at a time enabling it to partially
overlap.

* gcc.dg/lto/alias-access-path-2.0.c: New testcase.

* tree-ssa-alias.c (aliasing_component_refs_p): Do not give up
immediately after same_types_for_tbaa_p returns -1 and continue
looking for possible exact match; if matching types are arrays
watch for partial overlaps.
(indirect_ref_may_alias_decl_p): Watch for partial array overlaps.
(indirect_refs_may_alias_p): Do type based disambiguation first;
update comment.
Index: testsuite/g++.dg/lto/pr88130_0.C
===
--- testsuite/g++.dg/lto/pr88130_0.C(nonexistent)
+++ testsuite/g++.dg/lto/pr88130_0.C(working copy)
@

Re: [PATCH V2] A jump threading opportunity for condition branch

[Jeff Law]

On 5/28/19 7:53 AM, Jiufu Guo wrote:
> Hi,
> 
> This patch implements a new opportunity of jump threading for PR77820.
> In this optimization, conditional jumps are merged with unconditional
> jump. And then moving CMP result to GPR is eliminated.
> 
> This version is based on the proposal of Richard, Jeff and Andrew, and
> refined to incorporate comments.  Thanks for the reviews!
> 
> Bootstrapped and tested on powerpc64le and powerpc64be with no
> regressions (one case is improved) and new testcases are added. Is this
> ok for trunk?
> 
> Example of this opportunity looks like below:
> 
>   
>   p0 = a CMP b
>   goto ;
> 
>   
>   p1 = c CMP d
>   goto ;
> 
>   
>   # phi = PHI 
>   if (phi != 0) goto ; else goto ;
> 
> Could be transformed to:
> 
>   
>   p0 = a CMP b
>   if (p0 != 0) goto ; else goto ;
> 
>   
>   p1 = c CMP d
>   if (p1 != 0) goto ; else goto ;
> 
> 
> This optimization eliminates:
> 1. saving CMP result: p0 = a CMP b.
> 2. additional CMP on branch: if (phi != 0).
> 3. converting CMP result if there is phi = (INT_CONV) p0 if there is.
Just an FYI, I threw the V2 patch into my tester overnight.  THere's
still several targets to build, but so far no regressions.  In my spot
checks of the embedded targets, they're passing the new tests -- odds
are the tests are simple enough to not run into BRANCH_COST issues so
we're not going to have the insane xfail lists or alternate expected
outputs we've had for other tests in this space.

jeff
> 

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Martin Sebor]

On 5/30/19 9:13 AM, Jeff Law wrote:

On 5/30/19 8:52 AM, Martin Sebor wrote:

On 5/29/19 11:45 AM, Jeff Law wrote:

On 5/22/19 3:34 PM, Martin Sebor wrote:

-Wreturn-local-addr detects a subset of instances of returning
the address of a local object from a function but the warning
doesn't try to handle alloca or VLAs, or some non-trivial cases
of ordinary automatic variables[1].

The attached patch extends the implementation of the warning to
detect those.  It still doesn't detect instances where the address
is the result of a built-in such strcpy[2].

Tested on x86_64-linux.

Martin

[1] For example, this is only diagnosed with the patch:

    void* f (int i)
    {
  struct S { int a[2]; } s[2];
  return &s->a[i];
    }

[2] The following is not diagnosed even with the patch:

    void sink (void*);

    void* f (int i)
    {
  char a[6];
  char *p = __builtin_strcpy (a, "123");
  sink (p);
  return p;
    }

I would expect detecting to be possible and useful.  Maybe as
a follow-up.

gcc-71924.diff

PR middle-end/71924 - missing -Wreturn-local-addr returning alloca
result
PR middle-end/90549 - missing -Wreturn-local-addr maybe returning an
address of a local array plus offset

gcc/ChangeLog:

 PR c/71924
 * gimple-ssa-isolate-paths.c (is_addr_local): New function.
 (warn_return_addr_local_phi_arg, warn_return_addr_local): Same.
 (find_implicit_erroneous_behavior): Call
warn_return_addr_local_phi_arg.
 (find_explicit_erroneous_behavior): Call warn_return_addr_local.

gcc/testsuite/ChangeLog:

 PR c/71924
 * gcc.dg/Wreturn-local-addr-2.c: New test.
 * gcc.dg/Walloca-4.c: Prune expected warnings.
 * gcc.dg/pr41551.c: Same.
 * gcc.dg/pr59523.c: Same.
 * gcc.dg/tree-ssa/pr88775-2.c: Same.
 * gcc.dg/winline-7.c: Same.

diff --git a/gcc/gimple-ssa-isolate-paths.c
b/gcc/gimple-ssa-isolate-paths.c
index 33fe352bb23..2933ecf502e 100644
--- a/gcc/gimple-ssa-isolate-paths.c
+++ b/gcc/gimple-ssa-isolate-paths.c
@@ -341,6 +341,135 @@ stmt_uses_0_or_null_in_undefined_way (gimple
*stmt)
     return false;
   }
   +/* Return true if EXPR is a expression of pointer type that refers
+   to the address of a variable with automatic storage duration.
+   If so, set *PLOC to the location of the object or the call that
+   allocated it (for alloca and VLAs).  When PMAYBE is non-null,
+   also consider PHI statements and set *PMAYBE when some but not
+   all arguments of such statements refer to local variables, and
+   to clear it otherwise.  */
+
+static bool
+is_addr_local (tree exp, location_t *ploc, bool *pmaybe = NULL,
+   hash_set *visited = NULL)
+{
+  if (TREE_CODE (exp) == ADDR_EXPR)
+    {
+  tree baseaddr = get_base_address (TREE_OPERAND (exp, 0));
+  if (TREE_CODE (baseaddr) == MEM_REF)
+    return is_addr_local (TREE_OPERAND (baseaddr, 0), ploc, pmaybe,
visited);
+
+  if ((!VAR_P (baseaddr)
+   || is_global_var (baseaddr))
+  && TREE_CODE (baseaddr) != PARM_DECL)
+    return false;
+
+  *ploc = DECL_SOURCE_LOCATION (baseaddr);
+  return true;
+    }
+
+  if (TREE_CODE (exp) == SSA_NAME)
+    {
+  gimple *def_stmt = SSA_NAME_DEF_STMT (exp);
+  enum gimple_code code = gimple_code (def_stmt);
+
+  if (is_gimple_assign (def_stmt))
+    {
+  tree type = TREE_TYPE (gimple_assign_lhs (def_stmt));
+  if (POINTER_TYPE_P (type))
+    {
+  tree ptr = gimple_assign_rhs1 (def_stmt);
+  return is_addr_local (ptr, ploc, pmaybe, visited);
+    }
+  return false;
+    }
+
+  if (code == GIMPLE_CALL
+  && gimple_call_builtin_p (def_stmt))
+    {
+  tree fn = gimple_call_fndecl (def_stmt);
+  int code = DECL_FUNCTION_CODE (fn);
+  if (code != BUILT_IN_ALLOCA
+  && code != BUILT_IN_ALLOCA_WITH_ALIGN)
+    return false;
+
+  *ploc = gimple_location (def_stmt);
+  return true;
+    }
+
+  if (code == GIMPLE_PHI && pmaybe)
+    {
+  unsigned count = 0;
+  gphi *phi_stmt = as_a  (def_stmt);
+
+  unsigned nargs = gimple_phi_num_args (phi_stmt);
+  for (unsigned i = 0; i < nargs; ++i)
+    {
+  if (!visited->add (phi_stmt))
+    {
+  tree arg = gimple_phi_arg_def (phi_stmt, i);
+  if (is_addr_local (arg, ploc, pmaybe, visited))
+    ++count;
+    }
+    }
+
+  *pmaybe = count && count < nargs;
+  return count != 0;
+    }
+    }
+
+  return false;
+}

Is there some reason you didn't query the alias oracle here?  It would
seem a fairly natural fit.  Ultimately given a pointer (which will be an
SSA_NAME) you want to ask whether or not it conclusively points into the
stack.

That would seem to dramatically simplify is_addr_local.


I did think about it but decided against changing the existing
design (iterating over PHI arguments), for a couple of reasons:

1) It feels like a bigger change than my simple "bug fix" calls
    for.

I suspect the net result will be simpler though

Re: [libstdc++,doc] doc/xml/manual/support.xml - link adjustment and simplification

[Jonathan Wakely]

On 28/05/19 17:24 +0100, Jonathan Wakely wrote:

On 26/05/19 19:46 +0200, Gerald Pfeifer wrote:

The links adjustment I would just have committed right away, but
I'd also like to suggest swe simplify the section: the following
paragraph doesn't really add much, but duplicates the external
link.

Thoughts?


It's the same link, but not the same reference. One is pointing to the
advice in the book, and one is pointing to the accompanying example
code that is available in the CDROM version of the book.

That whole file needs updates, it's all out of date (C++11 adds
nullptr as a better alternative to NULL, and in the next section the
list of six flavours is waaay out of date, as there are several
new overloads in recent standards).


I decided to make a few small changes, and ended up rewriting several
sections. Committed to trunk.


commit f9aa870eaf882d779fc30141ffaabce84f959a99
Author: Jonathan Wakely 
Date:   Thu May 30 16:43:20 2019 +0100

Update libstdc++ documentation for Support and Diagnostics clauses

* doc/xml/manual/diagnostics.xml: Update list of headers that define
exception classes.
* doc/xml/manual/support.xml: Rewrite advice around NULL. Rewrite
section about new/delete overloads. Improve section on verbose
terminate handler.
* doc/html/*: Regenerate.

diff --git a/libstdc++-v3/doc/xml/manual/diagnostics.xml b/libstdc++-v3/doc/xml/manual/diagnostics.xml
index 67620e2cb40..08f576965d2 100644
--- a/libstdc++-v3/doc/xml/manual/diagnostics.xml
+++ b/libstdc++-v3/doc/xml/manual/diagnostics.xml
@@ -21,28 +21,38 @@
   API Reference
 
 
-  All exception objects are defined in one of the standard header
-  files: exception,
-  stdexcept, new, and
-  typeinfo.
+  Most exception classes are defined in one of the standard headers
+  ,
+  ,
+  , and
+  .
+  The C++ 2011 revision of the standard added more exception types
+  in the headers
+  ,
+  ,
+  , and
+  .
+  The C++ 2017 revision of the standard added more exception types
+  in the headers
+  ,
+  ,
+  , and
+  .
 
 
 
-  The base exception object is exception,
-  located in exception. This object has no
-  string member.
+  All exceptions thrown by the library have a base class of type
+  std::exception,
+  defined in .
+  This type has no std::string member.
 
 
 
   Derived from this are several classes that may have a
-  string member: a full hierarchy can be
+  std::string member. A full hierarchy can be
   found in the source documentation.
 
 
-
-  Full API details.
-
-
 
   
   
diff --git a/libstdc++-v3/doc/xml/manual/support.xml b/libstdc++-v3/doc/xml/manual/support.xml
index 53f4fbc5225..da8fed0e015 100644
--- a/libstdc++-v3/doc/xml/manual/support.xml
+++ b/libstdc++-v3/doc/xml/manual/support.xml
@@ -26,9 +26,9 @@
 
 Types
   
-  
+
   Fundamental Types
-
+
 
   C++ has the following builtin types:
 
@@ -90,11 +90,9 @@
 
   
   Numeric Properties
-
-
 
 
-The header limits defines
+The header  defines
 traits classes to give access to various implementation
 defined-aspects of the fundamental types. The traits classes --
 fourteen in total -- are all specializations of the class template
@@ -145,42 +143,50 @@
   
 
   NULL
-
+
 
  The only change that might affect people is the type of
  NULL: while it is required to be a macro,
  the definition of that macro is not allowed
- to be (void*)0, which is often used in C.
+ to be an expression with pointer type such as
+ (void*)0, which is often used in C.
 
 
 
  For g++, NULL is
  #define'd to be
  __null, a magic keyword extension of
- g++.
+ g++ that is slightly safer than a plain integer.
 
 
 
  The biggest problem of #defining NULL to be
  something like 0L is that the compiler will view
  that as a long integer before it views it as a pointer, so
- overloading won't do what you expect. (This is why
- g++ has a magic extension, so that
- NULL is always a pointer.)
+ overloading won't do what you expect. It might not even have the
+ same size as a pointer, so passing NULL to a
+ varargs function where a pointer is expected might not even work
+ correctly if sizeof(NULL) < sizeof(void*).
+ The G++ __null extension is defined so that
+ sizeof(__null) == sizeof(void*) to avoid this problem.
 
 
-In his book http://www.w3.org/1999/xlink";
-  xlink:href="http://www.aristeia.com/books.html";>Effective
-  C++, Scott Meyers points out that the best way
-to solve this problem is to not overload on pointer-vs-integer
-typ

Re: [AArch64] [SVE] PR88837 - Poor vector construction code in VL-specific mode

[Richard Sandiford]

Prathamesh Kulkarni  writes:
> On Thu, 30 May 2019 at 15:10, Richard Sandiford
>  wrote:
>>
>> Prathamesh Kulkarni  writes:
>> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/init_1.c 
>> > b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
>> > new file mode 100644
>> > index 000..cbfeff4a59c
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/init_1.c
>> > @@ -0,0 +1,27 @@
>> > +/* { dg-do compile { target aarch64_asm_sve_ok } } */
>> > +/* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 
>> > --save-temps" } */
>>
>> Sorry for not noticing last time, but the combination of aarch64_asm_sve_ok
>> and --save-temps only makes sense for assemble tests, not compile tests.
>> So these should either be:
>>
>> /* { dg-do assemble { target aarch64_asm_sve_ok } } */
>> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256 --save-temps" 
>> } */
>>
>> or:
>>
>> /* { dg-do compile } */
>> /* { dg-options "-O2 -fno-schedule-insns -msve-vector-bits=256" } */
>>
>> Might as well as go for the first I guess.  Same for the other
>> non-run tests.
>>
>> OK with that change, thanks.
> Thanks for pointing out, updated the patch with dg-do assemble.
> Sorry for silly ques - What configure option should be passed to gcc
> to generate code with -msve-vector-bits=256 by default ?
> I suppose that'd be necessary for correctness testing, to test patch
> with run tests that contain initializers and don't explicitly pass
> -msve-vector-bits=256 ?

There's no configure option, but you can test with things like
--target_board unix/-msve-vector-bits=256 or
--target_board unix{,/-msve-vector-bits=256} (to test both with
and without -msve-vector-bits=256).

Richard

Re: [PATCH] rs6000: Add undocumented switch -mprefixed-addr

[Michael Meissner]

On Wed, May 29, 2019 at 03:26:27PM -0500, Bill Schmidt wrote:
> On 5/29/19 8:16 AM, Segher Boessenkool wrote:
> > Hi!
> >
> > On Wed, May 29, 2019 at 07:42:38AM -0500, Bill Schmidt wrote:
> >>* rs6000-cpus.def (OTHER_FUSION_MASKS): New #define.
> >>(ISA_FUTURE_MASKS_SERVER): Add OPTION_MASK_PREFIXED_ADDR. Mask off
> >>OTHER_FUSION_MASKS.
> > Two spaces after a full stop (here and later again).
> 
> Oops, yep.
> >
> >> +/* ISA masks setting fusion options.  */
> >> +#define OTHER_FUSION_MASKS(OPTION_MASK_P8_FUSION  
> >> \
> >> +   | OPTION_MASK_P8_FUSION_SIGN)
> > Or merge the two masks into one?
> 
> I'll ask Mike to explain this, as I don't know why there are two masks.

The intention is to allow for using the numeric prefixed instructions without
pc-relative.  I.e.

long c = 0x12345;

would generate:

PLI r,0x12345

Instead of ADDI/ADDIS, but still not generate the pc-relative instructions.
The intention was when we get to timing stuff, there are fewer differences.  I
could live with dropping prefixed-addr as a separate switch.

However, since there are other prefixed instructions that we will do someday
that aren't necessarily using pc-relative, if we drop it, we need to make sure
all of the prefixed stuff is now targetted on -mfuture instead of
-mprefixed-addr.

> >
> >>  /* Support for a future processor's features.  */
> >> -#define ISA_FUTURE_MASKS_SERVER   (ISA_3_0_MASKS_SERVER   
> >> \
> >> -   | OPTION_MASK_FUTURE   \
> >> -   | OPTION_MASK_PCREL)
> >> +#define ISA_FUTURE_MASKS_SERVER   ((ISA_3_0_MASKS_SERVER  
> >> \
> >> +| OPTION_MASK_FUTURE  \
> >> +| OPTION_MASK_PCREL   \
> >> +| OPTION_MASK_PREFIXED_ADDR)  \
> >> +   & ~OTHER_FUSION_MASKS)
> > OTHER_FUSION_MASKS shouldn't be part of ISA_3_0_MASKS_SERVER.  Fix that
> > instead?  Fusion is a property of specific CPUs, not of ISA versions.
> 
> Agreed, I think this should be masked out of ISA_3_0_MASKS_SERVER, but
> would like Mike to agree before I change it in case I'm missing
> something obvious.

Mostly it was me being cautious that I wasn't sure all of the p8 fusion stuff
would play well with prefixed addressing (since the p8 fusion stuff does use
peephole2 and it might not be prepared for prefixed instructions).

> >
> >> -  /* -mpcrel requires the prefixed load/store support on FUTURE systems.  
> >> */
> >> -  if (!TARGET_FUTURE && TARGET_PCREL)
> >> +  /* -mprefixed-addr and -mpcrel require the prefixed load/store support 
> >> on
> >> + FUTURE systems.  */
> >> +  if (!TARGET_FUTURE && (TARGET_PCREL || TARGET_PREFIXED_ADDR))
> >>  {
> >>if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> >>error ("%qs requires %qs", "-mpcrel", "-mcpu=future");
> > PCREL requires PREFIXED_ADDR, please simplify.
> >
> >> +  if (TARGET_PCREL && !TARGET_PREFIXED_ADDR)
> >> +{
> >> +  if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> >> +  error ("%qs requires %qs", "-mpcrel", "-mprefixed-addr");
> >> +
> >>rs6000_isa_flags &= ~OPTION_MASK_PCREL;
> >>  }
> > Maybe put this test first, if that makes things easier or more logical?
> ok

Yes, I was working on this with my no-pcrel default patch I was about to submit.

> >
> >> @@ -36379,6 +36391,7 @@ static struct rs6000_opt_mask const 
> >> rs6000_opt_masks[] =
> >>{ "power9-vector",  OPTION_MASK_P9_VECTOR,  false, 
> >> true  },
> >>{ "powerpc-gfxopt", OPTION_MASK_PPC_GFXOPT, false, 
> >> true  },
> >>{ "powerpc-gpopt",  OPTION_MASK_PPC_GPOPT,  false, 
> >> true  },
> >> +  { "prefixed-addr",  OPTION_MASK_PREFIXED_ADDR,  false, 
> >> true  },
> > Do we want this?  Why?
> 
> Performance folks are using it for testing purposes.  Eventually this
> will probably drop out, but for now I think it's best to have the
> undocumented switch.

I use that table with -mdebug=reg so I can make sure exactly what options are
on or off.  Please add any undocumented switch to the table.

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

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jeff Law]

On 5/30/19 9:34 AM, Martin Sebor wrote:

>>> If the alias oracle can be used to give the same results without
>>> excessive false positives then I think it would be fine to make
>>> use of it.  Is that something you consider a prerequisite for
>>> this change or should I look into it as a followup?
>> I think we should explore it a bit before making a final decision.  It
>> may guide us for other work in this space (like detecting escaping
>> locals).   I think a dirty prototype to see if it's even in the right
>> ballpark would make sense.
> 
> Okay, let me look into it.
Sounds good.  Again, go with a quick prototype to see if it's likely
feasible.  The tests you've added should dramatically help evaluating if
the oracle is up to the task.

> 
>>> FWIW, I'm working on enhancing this to detect returning freed
>>> pointers (under a different option).  That seems like a good
>>> opportunity to also look into making use of the alias oracle.
>> Yes.  I think there's two interesting cases here to ponder.  If we free
>> a pointer that must point to a local, then we can warn & trap.  If we
>> free a pointer that may point to a local, then we can only warn (unless
>> we can isolate the path).
> 
> I wasn't actually thinking of freeing locals but it sounds like
> a useful enhancement as well.  Thanks for the suggestion! :)
> 
> To be clear, what I'm working on is detecting:
> 
>   void* f (void *p)
>   {
>     free (p);   // note: pointer was freed here
>     // ...
>     return p;   // warning: returning a freed pointer
>   }
Ah, we were talking about different things. Though what you're doing
might be better modeled in a true global static analyzer as a
use-after-free problem.  My sense is that translation-unit local version
of that problem really isn't that useful in practice.  THough I guess
there isn't anything bad with having a TU local version.


> 
>>> Besides these enhancements, there's also a request to diagnose
>>> dereferencing pointers to compound literals whose lifetime has
>>> ended (PR 89990), or more generally, those to any such local
>>> object.  It's about preventing essentially the same problem
>>> (accessing bad data or corrupting others) but it seems that
>>> both the analysis and the handling will be sufficiently
>>> different to consider implementing it somewhere else.  What
>>> are your thoughts on that?
>> I think the tough problem here is we lose binding scopes as we lower
>> into gimple, so solving it in the general case may be tough.  We've
>> started adding some clobbers into the IL to denote object death points,
>> but I'm not sure if they're sufficient to implement this kind of warning.
> 
> I was afraid that might be a problem.
Way back in the early days of tree-ssa we kept the binding scopes.  But
that proved problematical in various ways.  Mostly they just got in the
way of analysis an optimization and we spent far too much time in the
optimizers working around them or removing those which were empty.

They'd be helpful in this kind of analysis, stack slot sharing vs the
inliner and a couple other problems.  I don't know if the pendulum has
moved far enough to revisit :-)

jeff

Re: Teach same_types_for_tbaa to structurally compare arrays, pointers and vectors

[Martin Jambor]

Hi,

On Wed, May 29 2019, Jan Hubicka wrote:
>> > but we do not optimize it. I.e. optimized dump has:
>> >
>> > test ()
>> > {
>> >   struct bar * barptr.0_1;
>> >   struct foo * fooptr.1_2;
>> >   int _6;
>> >
>> >[local count: 1073741824]:
>> >   barptr.0_1 = barptr;
>> >   barptr.0_1->val2 = 1;
>> >   fooptr.1_2 = fooptr;
>> >   MEM[(struct foo *)fooptr.1_2] = 0;
>> >   _6 = barptr.0_1->val2;
>> >   return _6;
>> > }
>> >
>> > I see no reason why we should not constant propagate the return value.
>> 
>> Indeed a good example.  Make it work and add it to the testsuite ;)
>
> I think Martin Jambor is working on it.  One needs -fno-tree-sra to
> get this optimized :)
> Othewise we punt on the check that both types in MEM_REF are the same.
>

I'd like to fix this with the following patch which passes bootstrap but
I have just found that it makes the following three tests fail:

 gcc.dg/tree-ssa/ssa-dse-26.c scan-tree-dump-times dse1 "Deleted dead store: x 
= " 1
 gcc.dg/tree-ssa/ssa-dse-26.c scan-tree-dump-times dse1 "Deleted dead store: y 
= " 1
 gnat.dg/opt39.adb scan-tree-dump-times optimized "MEM" 1

Hopefully all require only adjusting the testcase somehow, but it's
still something I need to look at tomorrow.  The patch can then be
incrementally improved to also work with one-element arrays which are
however indexed with an SSA_NAME.

As far as alias oracle statistics are concerned, the patch (on top of
r271763) improves from:

Alias oracle query stats:
  refs_may_alias_p: 3792598 disambiguations, 4181030 queries
  ref_maybe_used_by_call_p: 8291 disambiguations, 3829922 queries
  call_may_clobber_ref_p: 1092 disambiguations, 1092 queries
  aliasing_component_ref_p: 192 disambiguations, 18684 queries
  TBAA oracle: 1820750 disambiguations 3584527 queries
   778806 are in alias set 0
   723538 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   261267 are dependent in the DAG
   166 are aritificially in conflict with void *

to:

Alias oracle query stats:
  refs_may_alias_p: 3786679 disambiguations, 4174429 queries
  ref_maybe_used_by_call_p: 8285 disambiguations, 3824058 queries
  call_may_clobber_ref_p: 1092 disambiguations, 1092 queries
  aliasing_component_ref_p: 362 disambiguations, 19215 queries
  TBAA oracle: 1822975 disambiguations 3579314 queries
   775069 are in alias set 0
   727061 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   254043 are dependent in the DAG
   166 are aritificially in conflict with void *


Martin


Subject: [PATCH] Make SRA re-construct orginal memory accesses when easy

2019-05-30  Martin Jambor  

* tree-sra.c (struct access): New field grp_same_access_path.
(dump_access): Dump it.
(build_reconstructed_reference): New function.
(build_ref_for_model): Use it if possible.
(path_comparable_for_same_access): New function.
(same_access_path_p): Likewise.
(sort_and_splice_var_accesses): Set the new flag.
(analyze_access_subtree): Likewise.
(propagate_subaccesses_across_link): Propagate zero value of the new
flag down the access tree.

testsuite/
* gcc.dg/tree-ssa/alias-access-path-1.c: Remove -fno-tree-sra option.
---
 .../gcc.dg/tree-ssa/alias-access-path-1.c |   2 +-
 gcc/tree-sra.c| 197 +-
 2 files changed, 190 insertions(+), 9 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c 
b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c
index 264f72aff0a..ba90b56fe5c 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-fre3 -fno-tree-sra" } */
+/* { dg-options "-O2 -fdump-tree-fre3" } */
 struct foo
 {
   int val;
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index fd51a3d0323..dc2a776d66c 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -242,6 +242,10 @@ struct access
  access tree.  */
   unsigned grp_unscalarized_data : 1;
 
+  /* Set if all accesses in the group consist of the same chain of
+ COMPONENT_REFs and ARRAY_REFs.  */
+  unsigned grp_same_access_path : 1;
+
   /* Does this access and/or group contain a write access through a
  BIT_FIELD_REF?  */
   unsigned grp_partial_lhs : 1;
@@ -443,16 +447,18 @@ dump_access (FILE *f, struct access *access, bool grp)
 "grp_scalar_write = %d, grp_total_scalarization = %d, "
 "grp_hint = %d, grp_covered = %d, "
 "grp_unscalarizable_region = %d, grp_unscalarized_data = %d, "
-"grp_partial_lhs = %d, grp_to_be_replaced = %d, "
-"grp_to_be_debug_replaced = %d, grp_maybe_modified = %d, "
+

[PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

Hi,

PR 90581 (provide an option to adjust the maximum depth of nested #include)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581

is to add a new cpp option -fmax-inlcude-depth to set the maximum depth of 
nested #include.

'-fmax-include-depth=DEPTH'
 Set the maximum depth of the nested include.  The default value is
 200.

Please check the attached patch.
I have done bootstrap and regression test on X86, no any issue.

thanks a lot.

Qing.



gcc/ChangeLog:

2019-05-30  qing zhao  

* doc/cppopts.texi: Add document for -fmax-include-depth.
* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.

libcpp/ChangeLog:

2019-05-30  qing zhao  

* directives.c (do_include_common): Replace CPP_STACK_MAX with
CPP_OPTION (pfile, max_include_depth).
* include/cpplib.h (struct cpp_options): Add new field
max_include_depth.
* init.c (cpp_create_reader): Initiate new field max_include_depth.
* internal.h: Delete CPP_STACK_MAX.

gcc/c-family/ChangeLog:

2019-05-30  qing zhao  

* c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
* c.opt: Add new option -fmax-include-depth.

gcc/testsuite/ChangeLog:

2019-05-30  qing zhao  

* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
* c-c++-common/cpp/fmax-include-depth.c: New test.



0001-PR-preprocessor-90581.patch
Description: Binary data

Re: Simplify more EXACT_DIV_EXPR comparisons

[Jeff Law]

On 5/29/19 3:27 PM, Marc Glisse wrote:
> On Mon, 20 May 2019, Richard Biener wrote:
> 
>> On Mon, May 20, 2019 at 10:16 AM Marc Glisse 
>> wrote:
>>>
>>> On Mon, 20 May 2019, Richard Biener wrote:
>>>
 On Sun, May 19, 2019 at 6:16 PM Marc Glisse 
 wrote:
>
> Hello,
>
> 2 pieces:
>
> - the first one handles the case where the denominator is negative. It
> doesn't happen often with exact_div, so I don't handle it
> everywhere, but
> this one looked trivial
>
> - handle the case where a pointer difference is cast to an unsigned
> type
> before being compared to a constant (I hit this in std::vector).
> With some
> range info we could probably handle some non-constant cases as well...
>
> The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)
>
> void f (void*);
> void g (int *p, int *q)
> {
>    __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
>    if (n < 100)
>  f (__builtin_alloca (n));
> }
>
> At the time of walloca2, we have
>
>    _1 = p_5(D) - q_6(D);
>    # RANGE [-2305843009213693952, 2305843009213693951]
>    _2 = _1 /[ex] 4;
>    # RANGE ~[2305843009213693952, 16140901064495857663]
>    n_7 = (long unsigned intD.10) _2;
>    _11 = (long unsigned intD.10) _1;
>    if (_11 <= 396)
> [...]
>    _3 = allocaD.1059 (n_7);
>
> and warn.

 That's indeed to complicated relation of _11 to n_7 for
 VRP predicate discovery.

> However, DOM3 later produces
>
>    _1 = p_5(D) - q_6(D);
>    _11 = (long unsigned intD.10) _1;
>    if (_11 <= 396)

 while _11 vs. _1 works fine.

> [...]
>    # RANGE [0, 99] NONZERO 127
>    _2 = _1 /[ex] 4;
>    # RANGE [0, 99] NONZERO 127
>    n_7 = (long unsigned intD.10) _2;
>    _3 = allocaD.1059 (n_7);
>
> so I am tempted to say that the walloca2 pass is too early, xfail the
> testcase and file an issue...

 Hmm, there's a DOM pass before walloca2 already and moving
 walloca2 after loop opts doesn't look like the best thing to do?
 I suppose it's not DOM but sinking that does the important transform
 here?  That is,

 Index: gcc/passes.def
 ===
 --- gcc/passes.def  (revision 271395)
 +++ gcc/passes.def  (working copy)
 @@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
   NEXT_PASS (pass_optimize_bswap);
   NEXT_PASS (pass_laddress);
   NEXT_PASS (pass_lim);
 -  NEXT_PASS (pass_walloca, false);
   NEXT_PASS (pass_pre);
   NEXT_PASS (pass_sink_code);
 +  NEXT_PASS (pass_walloca, false);
   NEXT_PASS (pass_sancov);
   NEXT_PASS (pass_asan);
   NEXT_PASS (pass_tsan);

 fixes it?
>>>
>>> I will check, but I don't think walloca uses any kind of on-demand
>>> VRP, so
>>> we still need some pass to update the ranges after sinking, which
>>> doesn't
>>> seem to happen until the next DOM pass.
>>
>> Oh, ok...  Aldy, why's this a separate pass anyways?  I think similar
>> other warnigns are emitted from RTL expansion?  So maybe we can
>> indeed move the pass towards warn_restrict or late_warn_uninit.
> 
> I tried moving it after 'sink' and that didn't help. Moving it next to
> warn_restrict works for this test but breaks 2 others that currently
> "work" by accident (+ one where the message changes between "unbounded"
> and "too large", it isn't clear what the difference is between those
> messages).
So "unbounded" means there was no controlling predicate that would allow
an upper bound on the size of the alloca to be determined.

"too large" breaks down into two cases.  One where we know it's too
large, the other when it may be too large.  The latter can happen due to
casts and perhaps other things (controlling predicate with a dynamic
upper bound maybe?)


> 
> My suggestion, in addition to the original patch, is
> 
> * gcc.dg/Walloca-13.c: Xfail.
> 
> --- Walloca-13.c    (revision 271742)
> +++ Walloca-13.c    (working copy)
> @@ -1,12 +1,12 @@
>  /* { dg-do compile } */
>  /* { dg-require-effective-target alloca } */
>  /* { dg-options "-Walloca-larger-than=100 -O2" } */
> 
>  void f (void*);
> 
>  void g (int *p, int *q)
>  {
>    __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
>    if (n < 100)
> -    f (__builtin_alloca (n));
> +    f (__builtin_alloca (n)); // { dg-bogus "may be too large due to
> conversion" "" { xfail { *-*-* } } }
>  }
> 
> Is that ok?
Aldy would need to chime in -- at first glance it looks like we've lost
precision here.

jeff

Re: [PATCH] PR libstdc++/71579 assert that type traits are not misused with an incomplete type

[Jonathan Wakely]

On 30/05/19 09:06 +0300, Antony Polukhin wrote:

чт, 9 мая 2019 г. в 00:10, Jonathan Wakely :


On 06/05/19 14:19 +0300, Antony Polukhin wrote:
>@@ -924,14 +984,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   template
> struct is_default_constructible
> : public __is_default_constructible_safe<_Tp>::type
>-{ };
>+{
>+  static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
>+  "template argument must be a complete class or an unbounded array");
>+};
>
>   /// is_constructible

This "///" comment is for Doxygen, and should remain with the
is_constructible trait, not __is_constructible_impl.


Fixed that issue along with some other misplaced Doxygen comments.
Rebased the patch and removed some trailing white-spaces.


Thanks! Rebasing this was on my TODO list for today.


--
Best regards,
Antony Polukhin



diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index b209460..c5f0672 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,27 @@
+2019-05-29  Antony Polukhin  
+
+   PR libstdc++/71579
+   * include/std/type_traits: Add static_asserts to make sure that
+   type traits are not misused with an incomplete type.


This changelog could be more explicit. It doesn't have to list every
changed trait, but maybe something like:

* include/std/type_traits: Add static_asserts to make sure that
type traits are not misused with an incomplete type. Create
   new base characteristics without assertions that can be reused
   in other traits.


+   * testsuite/20_util/__is_complete_or_unbounded/memoization.cc:
+   New test.
+   * testsuite/20_util/__is_complete_or_unbounded/memoization_neg.cc:
+   Likewise.
+   * testsuite/20_util/__is_complete_or_unbounded/value.cc: Likewise.


I think I'd prefer not to have the double underscores in the directory
name here.


@@ -876,19 +935,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  template
struct is_nothrow_destructible
: public __is_nt_destructible_safe<_Tp>::type
+{
+  static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
+   "template argument must be a complete class or an unbounded array");
+};
+
+template


N.B. This 'template' keyword should only be indented two spaces,
however ...


+struct __is_constructible_impl
+  : public __bool_constant<__is_constructible(_Tp, _Args...)>
{ };


I thought this could be an alias template instead of a class template:

 template
   using __is_constructible_impl
 = __bool_constant<__is_constructible(_Tp, _Args...)>;

But it causes failures in the 20_util/variable_templates_for_traits.cc
test. It looks like it instantiates some things too eagerly if it's an
alias template.


  /// is_constructible
  template
struct is_constructible
-  : public __bool_constant<__is_constructible(_Tp, _Args...)>
+  : public __is_constructible_impl<_Tp, _Args...>
+{
+  static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
+   "template argument must be a complete class or an unbounded array");
+};
+
+  template
+struct __is_default_constructible_impl
+: public __is_constructible_impl<_Tp>::type
{ };


Do we need this new __is_default_constructible_impl type?
Could we just use __is_constructible_impl<_Tp> instead, to avoid an
extra step (and extra template instantiations)?


@@ -946,12 +1030,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
: public __is_nt_default_constructible_atom<_Tp>
{ };

+template


Indentation again :-)


+struct __is_nothrow_default_constructible_impl
+: public __and_<__is_default_constructible_impl<_Tp>,
+__is_nt_default_constructible_impl<_Tp>>
+{ };


This one can be an alias template:

 template
   using __is_nothrow_default_constructible_impl
 = typename __and_<__is_constructible_impl<_Tp>,
__is_nt_default_constructible_impl<_Tp>>::type;


+template
+struct __is_nothrow_assignable_impl
+: public __and_<__bool_constant<__is_assignable(_Tp, _Up)>,
+   __is_nt_assignable_impl<_Tp, _Up>>
+{ };


I wanted this one to be an alias template:

 template
   using __is_nothrow_assignable_impl
 = __and_<__bool_constant<__is_assignable(_Tp, _Up)>,
  __is_nt_assignable_impl<_Tp, _Up>>;

But that also causes the same test to fail. I think it would work if
we moved the __is_assignable intrinsic into a new __is_assignable_impl
class template, but then we'd be adding a new class template just to
get rid of another one. That doesn't seem sensible.

I've attached a relative diff, to be applied on top of yours, with my
suggested tweaks. Do you see any issues with it?

(If you're happy with those tweaks I can go ahead and apply this,
there's no need for an updated patch from you).

commit 732d9c1c9634900437f560538305a6ffde5d6e8d
Author: Jonathan Wakely 
Date:   Thu May 30 17:30:45 2019 +0100

simplify

[PATCH] Fix alignment option parser (PR90684)

[Wilco Dijkstra]

Fix the alignment option parser to always allow up to 4 alignments.
Now -falign-functions=16:8:8:8 no longer reports an error.

OK for commit (and backport to GCC9)?

ChangeLog:
2019-05-30  Wilco Dijkstra  


PR driver/90684
* gcc/opts.c (parse_and_check_align_values): Allow 4 alignment values.
--

diff --git a/gcc/opts.c b/gcc/opts.c
index 
a1ccd97746890b8259d000cbdeeaddc02df0b74a..d3501421f7879113ab4422063cedfabd23cadce7
 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2022,14 +2022,7 @@ parse_and_check_align_values (const char *flag,
   free (str);
 
   /* Check that we have a correct number of values.  */
-#ifdef SUBALIGN_LOG
-  unsigned max_valid_values = 4;
-#else
-  unsigned max_valid_values = 2;
-#endif
-
-  if (result_values.is_empty ()
-  || result_values.length () > max_valid_values)
+  if (result_values.is_empty () || result_values.length () > 4)
 {
   if (report_error)
error_at (loc, "invalid number of arguments for %<-falign-%s%> "

[patch, doc, fortran] Document some trans* stuff

[Thomas Koenig]

Hello world,

I thought I would add a little bit of documentation about
what trans-* is doing, in the hope that it may be useful.
The scalarizer could also use some documentation, but at
least we have the wiki article there.

Tested with "make", "make dvi" and "make pdf".

OK for trunk?

Regards

Thomas

2019-05-30  Thomas Koenig  

* gfc-internals.texi (Translating to GENERIC): New chapter.
Index: gfc-internals.texi
===
--- gfc-internals.texi	(Revision 271629)
+++ gfc-internals.texi	(Arbeitskopie)
@@ -119,6 +119,8 @@ not accurately reflect the status of the most rece
 * Frontend Data Structures::
Data structures used by the frontend
 * Object Orientation:: Internals of Fortran 2003 OOP features.
+* Translating to GENERIC::
+   Generating the intermediate language for later stages.
 * LibGFortran::The LibGFortran Runtime Library.
 * GNU Free Documentation License::
How you can copy and share this manual.
@@ -724,7 +726,145 @@ operator call is replaced with an internally gener
 type-bound procedure call to the respective definition and that call is
 further processed.
 
+@c -
+@c - Translating to TREE
+@c -
 
+@node Translating to GENERIC
+@chapter Generating the intermediate language for later stages.
+
+This chapter deals with the transformation of gfortran's frontend data
+structures to the intermediate language used by the later stages of
+the compiler, the so-called middle end.
+
+Data structures relating to this are found in the source files
+@file{trans*.h} and @file{trans-*.c}.
+
+@menu
+* Basic Data Structures::   Basic data structures.
+* Converting Expressions::  Converting expressions to tree.
+* Translating Statements::  Translating statements.
+* Accessing Declarations::  Accessing declarations.
+@end menu
+
+@node Basic Data Structures
+@section Basic data structures
+
+Gfortran creates GENERIC as an intermediate language for the
+middle-end. Details about GENERIC can be found in the GCC manual.
+
+The basic data structure of GENERIC is a @code{tree}. Everything in
+GENERIC is a @code{tree}, including types and statements.  Fortunately
+for the gfortran programmer, @code{tree} variables are
+garbage-collected, so doing memory management for them is not
+necessary.
+
+@code{tree} expressions are built using functions such as, for
+example, @code{fold_build2_loc}.  For two tree variables @code{a} and
+@code{b}, both of which have the type @code{gfc_arry_index_type},
+calculation @code{c = a * b} would be done by
+
+@smallexample
+c = fold_build2_loc (input_location, MULT_EXPR,
+ gfc_array_index_type, a, b);
+@end smallexample
+
+The types have to agree, otherwise internal compiler errors will occur
+at a later stage.  Expressions can be converted to a different type
+using @code{fold_convert}.
+
+Accessing individual members in the @code{tree} structures should not
+be done. Rather, access should be done via macros.
+
+One basic data structure is the @code{stmtblock_t} struct. This is
+used for holding a list of statements, expressed as @code{tree}
+expressions.  If a block is created using @code{gfc_start_block}, it
+has its own scope for variables; if it is created using
+@code{gfc_init_block}, it does not have its own scope.
+
+It is possible to
+@itemize @bullet
+@item Add an expression to the end of a block using
+  @code{gfc_add_expr_to_block}
+@item Add an expression to the bebinning of a block using
+  @code{void gfc_prepend_expr_to_block}
+@item Make a block into a single @code{tree} using
+  @code{gfc_finish_block}.  This is needed to 
+@end itemize
+
+Variables are also @code{tree} expressions, they can be created using
+@code{gfc_create_var}. Assigning to a variable can be done with
+@code{gfc_add_modify}.
+
+An example: Creating a default integer type variable in the current
+scope with the prefix ``everything'' in the @code{stmt_block}
+@code{block} and assigning the value 42 would be
+
+@smallexample
+tree var, *block;
+/* Initialize block somewhere here.  */
+var = gfc_create_var (integer_type_node, "everything");
+gfc_add_modify (block, var, build_int_cst (integer_type_node, 42));
+@end smallexample
+
+@node Converting Expressions
+@section Converting Expressons to tree
+
+Converting expressions to @code{tree} is done by functions called
+@code{gfc_conv_*}.
+
+The central data structure for a GENERIC expression is the
+@code{gfc_se} structure.  Its @code{expr} member is a @code{tree} that
+holds the value of the expression.  A @code{gfc_se} structure is
+initialized using @code{gfc_init_se}; it needs to be embedded in an
+outer @code{gfc_se}.
+
+Evaluating Fortran expressions often require things to be done before
+and afte

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jason Merrill]

On Thu, May 30, 2019 at 12:16 PM Jeff Law  wrote:
>
> On 5/30/19 9:34 AM, Martin Sebor wrote:
>
> >>> If the alias oracle can be used to give the same results without
> >>> excessive false positives then I think it would be fine to make
> >>> use of it.  Is that something you consider a prerequisite for
> >>> this change or should I look into it as a followup?
> >> I think we should explore it a bit before making a final decision.  It
> >> may guide us for other work in this space (like detecting escaping
> >> locals).   I think a dirty prototype to see if it's even in the right
> >> ballpark would make sense.
> >
> > Okay, let me look into it.
> Sounds good.  Again, go with a quick prototype to see if it's likely
> feasible.  The tests you've added should dramatically help evaluating if
> the oracle is up to the task.
>
> >
> >>> FWIW, I'm working on enhancing this to detect returning freed
> >>> pointers (under a different option).  That seems like a good
> >>> opportunity to also look into making use of the alias oracle.
> >> Yes.  I think there's two interesting cases here to ponder.  If we free
> >> a pointer that must point to a local, then we can warn & trap.  If we
> >> free a pointer that may point to a local, then we can only warn (unless
> >> we can isolate the path).
> >
> > I wasn't actually thinking of freeing locals but it sounds like
> > a useful enhancement as well.  Thanks for the suggestion! :)
> >
> > To be clear, what I'm working on is detecting:
> >
> >   void* f (void *p)
> >   {
> > free (p);   // note: pointer was freed here
> > // ...
> > return p;   // warning: returning a freed pointer
> >   }
> Ah, we were talking about different things. Though what you're doing
> might be better modeled in a true global static analyzer as a
> use-after-free problem.  My sense is that translation-unit local version
> of that problem really isn't that useful in practice.  THough I guess
> there isn't anything bad with having a TU local version.
>
>
> >
> >>> Besides these enhancements, there's also a request to diagnose
> >>> dereferencing pointers to compound literals whose lifetime has
> >>> ended (PR 89990), or more generally, those to any such local
> >>> object.  It's about preventing essentially the same problem
> >>> (accessing bad data or corrupting others) but it seems that
> >>> both the analysis and the handling will be sufficiently
> >>> different to consider implementing it somewhere else.  What
> >>> are your thoughts on that?
> >> I think the tough problem here is we lose binding scopes as we lower
> >> into gimple, so solving it in the general case may be tough.  We've
> >> started adding some clobbers into the IL to denote object death points,
> >> but I'm not sure if they're sufficient to implement this kind of warning.
> >
> > I was afraid that might be a problem.
> Way back in the early days of tree-ssa we kept the binding scopes.  But
> that proved problematical in various ways.  Mostly they just got in the
> way of analysis an optimization and we spent far too much time in the
> optimizers working around them or removing those which were empty.
>
> They'd be helpful in this kind of analysis, stack slot sharing vs the
> inliner and a couple other problems.  I don't know if the pendulum has
> moved far enough to revisit :-)

Why wouldn't clobbers be sufficient?

Jaason

Go patch committed: intrinsify sync/atomic functions

[Ian Lance Taylor]

This Go frontend patch by Cherry Zhang intrinsifies the sync/atomic
functions.  It also makes sure not to intrinsify calls in go or defer
statements.  Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 271761)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-84b8000c32f671c6cc89df1290ed6e0170308644
+4dc60d989293d070702024e7dea52b9849f74775
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 271669)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -10432,7 +10432,8 @@ Call_expression::do_flatten(Gogo* gogo,
 
   // Lower to compiler intrinsic if possible.
   Func_expression* fe = this->fn_->func_expression();
-  if (fe != NULL
+  if (!this->is_concurrent_ && !this->is_deferred_
+  && fe != NULL
   && (fe->named_object()->is_function_declaration()
   || fe->named_object()->is_function()))
 {
@@ -10471,6 +10472,73 @@ Call_expression::intrinsify(Gogo* gogo,
   int int_size = int_type->named_type()->real_type()->integer_type()->bits() / 
8;
   int ptr_size = 
uintptr_type->named_type()->real_type()->integer_type()->bits() / 8;
 
+  if (package == "sync/atomic")
+{
+  // sync/atomic functions and runtime/internal/atomic functions
+  // are very similar. In order not to duplicate code, we just
+  // redirect to the latter and let the code below to handle them.
+  // In case there is no equivalent functions (slight variance
+  // in types), we just make an artificial name (begin with '$').
+  // Note: no StorePointer, SwapPointer, and CompareAndSwapPointer,
+  // as they need write barriers.
+  if (name == "LoadInt32")
+name = "$Loadint32";
+  else if (name == "LoadInt64")
+name = "Loadint64";
+  else if (name == "LoadUint32")
+name = "Load";
+  else if (name == "LoadUint64")
+name = "Load64";
+  else if (name == "LoadUintptr")
+name = "Loaduintptr";
+  else if (name == "LoadPointer")
+name = "Loadp";
+  else if (name == "StoreInt32")
+name = "$Storeint32";
+  else if (name == "StoreInt64")
+name = "$Storeint64";
+  else if (name == "StoreUint32")
+name = "Store";
+  else if (name == "StoreUint64")
+name = "Store64";
+  else if (name == "StoreUintptr")
+name = "Storeuintptr";
+  else if (name == "AddInt32")
+name = "$Xaddint32";
+  else if (name == "AddInt64")
+name = "Xaddint64";
+  else if (name == "AddUint32")
+name = "Xadd";
+  else if (name == "AddUint64")
+name = "Xadd64";
+  else if (name == "AddUintptr")
+name = "Xadduintptr";
+  else if (name == "SwapInt32")
+name = "$Xchgint32";
+  else if (name == "SwapInt64")
+name = "$Xchgint64";
+  else if (name == "SwapUint32")
+name = "Xchg";
+  else if (name == "SwapUint64")
+name = "Xchg64";
+  else if (name == "SwapUintptr")
+name = "Xchguintptr";
+  else if (name == "CompareAndSwapInt32")
+name = "$Casint32";
+  else if (name == "CompareAndSwapInt64")
+name = "$Casint64";
+  else if (name == "CompareAndSwapUint32")
+name = "Cas";
+  else if (name == "CompareAndSwapUint64")
+name = "Cas64";
+  else if (name == "CompareAndSwapUintptr")
+name = "Casuintptr";
+  else
+return NULL;
+
+  package = "runtime/internal/atomic";
+}
+
   if (package == "runtime")
 {
   // Handle a couple of special runtime functions.  In the runtime
@@ -10557,7 +10625,8 @@ Call_expression::intrinsify(Gogo* gogo,
   int memorder = __ATOMIC_SEQ_CST;
 
   if ((name == "Load" || name == "Load64" || name == "Loadint64" || name 
== "Loadp"
-   || name == "Loaduint" || name == "Loaduintptr" || name == "LoadAcq")
+   || name == "Loaduint" || name == "Loaduintptr" || name == "LoadAcq"
+   || name == "$Loadint32")
   && this->args_ != NULL && this->args_->size() == 1)
 {
   if (int_size < 8 && (name == "Load64" || name == "Loadint64"))
@@ -10577,6 +10646,11 @@ Call_expression::intrinsify(Gogo* gogo,
   code = Runtime::ATOMIC_LOAD_8;
   res_type = uint64_type;
 }
+  else if (name == "$Loadint32")
+{
+  code = Runtime::ATOMIC_LOAD_4;
+  res_type = int32_type;
+}
   else if (name == "Loadint64")
 {
   code = Runtime::ATOMIC_LOAD_8;
@@ -10618,10 +10692,11 @@ Call_expression::intrinsify(Gogo* gogo,
 }
 
   if ((name 

Re: [patch, doc, fortran] Document some trans* stuff

[Steve Kargl]

On Thu, May 30, 2019 at 06:47:16PM +0200, Thomas Koenig wrote:
> +
> +It is possible to
> +@itemize @bullet
> +@item Add an expression to the end of a block using
> +  @code{gfc_add_expr_to_block}
> +@item Add an expression to the bebinning of a block using

s/bebinning/beginning


> +  @code{void gfc_prepend_expr_to_block}
> +@item Make a block into a single @code{tree} using
> +  @code{gfc_finish_block}.  This is needed to 

Dangling sentence?

> +@end itemize
> +
> +Evaluating Fortran expressions often require things to be done before
> +and after evaulation of the expression, for example code for the

s/evaulation/evaluation

> +allocation of a temporary variable and its subsequent deallocation.
> +Therefore, @code{gfc_se} contains the members @code{pre} and a
> +@code{post}, which point to @code{stmt_block} blocks for code that
> +needs to be executed before and after evaluation of the expression.


Thomas, with the above fixes, I think this a great
addition to manual.  OK to commit.

-- 
Steve

Re: [patch][aarch64]: add support for fabd in sve

[Richard Sandiford]

Sylvia Taylor  writes:
> Greetings,
>
> This patch adds support in SVE to combine:
> - fsub and fabs into fabd
>
> fsubz0.s, z0.s, z1.s
> fabsz0.s, p1/m, z0.s
> ---
> fabdz0.s, p1/m, z0.s, z1.s
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> Ok for trunk? If yes, I don't have commit rights,
> so if someone can please commit it on my behalf.
>
> Cheers,
> Syl
>
> gcc/ChangeLog:
>
> 2019-05-30  Sylvia Taylor  
>
>   * config/aarch64/aarch64-sve.md
>   (*fabd3): New.
>
> gcc/testsuite/ChangeLog:
>
> 2019-05-30  Sylvia Taylor  
>
>   * gcc.target/aarch64/sve/fabd.c: New.

Thanks, applied as r271785.  (I renamed the test to fabd_1.c while
committing, to make it more consistent with the others in that directory.)

Richard

Fwd: CFV: "C++ programming in GNU" interest group (possibly including coding standard development)

[James Youngman]

Dear GCC developers,

I'm looking into creating a C++ coding standard for GNU programs.
See below for the CFV (which you therefore might have already read by
being on the other list; if so, sorry for the duplication).

I'd be particularly interested in the question of whether the coding
standards for GCC (i.e.
https://www.gnu.org/software/gcc/codingconventions.html) would be
usefully adopted by GNU projects (written in C++) generally.  If we
can re-use the existing standard for GCC, that might reduce the
overall size of the effort and reduce the likelihood of making choices
with unanticipated consequences.

Supposing you support the idea, I think you would likely be able to
provide valuable context on what has worked (or, perhaps, not) in the
GCC coding standard and to what extent you think this might be useful
elsewhere in GNU.   If you can volunteer to help this effort, please
let us know (by replying on this thread).

Thanks,
James.


-- Forwarded message -
From: James Youngman 
Date: Thu, May 30, 2019 at 4:04 PM
Subject: CFV: "C++ programming in GNU" interest group (possibly
including coding standard development)
To: Antonio Diaz Diaz , Bruno Haible
, Niels Möller 


This email is BCC'ed to  so that the members
of that list know what's happening.

If you are on the To: line, my understanding is that you have
volunteered to participate in discussions regarding the use of C++ in
GNU projects.

If you are not on the To: line, my understanding is that you have not
volunteered to participate.  I understand that despite this you might
be interested in hearing about how things are going.   I propose to
provide updates to  but I'm not asking
people who simply want to get updates to do anything at this point.

Hence, if you don't want to work on this, you can stop reading now.


For those still reading, first, thanks for volunteering to help.  I
would like to start by asking you to suggest other GNU developers who
might also be interested in this area.   I'd like to ensure that we
consult widely across GNU so that we can get input (and ideally, help)
from any GNU developers wishing to be involved.   Getting input from a
wide group will also help to ensure that we don't end up adopting a
narrow view and
perhaps proposing a coding standard that's simply not broadly useful
to GNU projects in general.

If you want to invite someone who you believe might be interested, CC
them on this thread (feel free to ask them about it first if you're
not sure) or invite them to join the mailing list once it has been
created.   If you are not after all interested in participating
yourself, please say so on this thread so that the other volunteers
know to stop CCing you.

I propose to discuss our next steps on a separate mailing list and
invite additional volunteers to join it, so that we don't generate too
much traffic on .  The mailing list will
need a name.   Please send suggestions to me directly.  After a short
time I will mail you all with the list of suggestions and we can find
a consensus.

In our discussions, we might also touch on some related matters (e.g.
whether there might be resources other than a coding standard that
might be generally useful to C++ projects in GNU, or tools already in
GNU that would be of particular use in C++ development).

Any suggestions would be welcome, simply go ahead.


TL;DR:
1. Thanks for volunteering to help.  Let us know if you don't actually want to.
2. Please invite others to participate.  CC them, or invite them to
join the list.

Thanks,
James Youngman.

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Andreas Schwab]

On Mai 21 2019, Jim Wilson  wrote:

> On Sun, May 19, 2019 at 5:22 AM Andreas Schwab  wrote:
>> ../../../libgo/go/runtime/mbitmap.go: In function 
>> ‘runtime.setMarked.runtime.markBits’:
>> ../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error: 
>> Segmentation fault
>>   291 |  atomic.Or8(m.bytep, m.mask)
>>   | ^
>
> This is failing for RISC-V because __atomic_or_fetch_1 isn't a
> built-in function that can be expanded inline.  You have to call the
> library function in libatomic.  The C front-end is registering all of
> the built-in functions, but it looks like the go front-end is only
> registering functions it thinks it needs and this list is incomplete.
> In expand_builtin, case BUILT_IN_ATOMIC_OR_FETCH_1, the external
> library call for this gets set to BUILT_IN_ATOMIC_FETCH_OR_1.  Then in
> expand_builtin_atomic_fetch_op when we call builtin_decl_explicit
> (ext_call) it returns NULL.  This is because the go front end
> registered BUILT_IN_ATOMIC_OR_FETCH_1 as a built-in, but did not
> register BUILT_IN_ATOMIC_FETCH_OR_1 as a built-in.  The NULL return
> from builtin_decl_explicit gives us an ADDR_EXPR with a NULL operand
> which eventually causes the internal compiler error.  It looks like
> the same thing is done with all of the op_fetch built-ins, so use of
> any of them means that the fetch_op built-in also has to be
> registered.  I verified with a quick hack that I need both
> BUILT_IN_ATOMIC_FETCH_OR_1 and BUILT_IN_ATOMIC_FETCH_AND_1 defined as
> built-ins to make a RISC-V go build work.  I haven't done any testing
> yet.

Here are the test results:
http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [patch, doc, fortran] Document some trans* stuff

[Thomas Koenig]

Hi Steve,


Thomas, with the above fixes, I think this a great
addition to manual.  OK to commit.


Committed with your fixes (sometimes I think my fingers
are going blind :-) as r271786.

Best regards

Thomas

C++ PATCH to fix typo in cp-tree.h

[Marek Polacek]

Noticed this is -> if typo.  Applying to trunk as obvious.

2019-05-30  Marek Polacek  

* cp-tree.h (TYPE_HAS_NONTRIVIAL_DESTRUCTOR): Fix a typo.

diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 7a74fd4fac5..edd59d5f000 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -4295,7 +4295,7 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
 /* Nonzero for _TYPE node means that this type does not have a trivial
destructor.  Therefore, destroying an object of this type will
involve a call to a destructor.  This can apply to objects of
-   ARRAY_TYPE is the type of the elements needs a destructor.  */
+   ARRAY_TYPE if the type of the elements needs a destructor.  */
 #define TYPE_HAS_NONTRIVIAL_DESTRUCTOR(NODE) \
   (TYPE_LANG_FLAG_4 (NODE))
 

Re: CFV: "C++ programming in GNU" interest group (possibly including coding standard development)

[Mike Stump]

On May 30, 2019, at 10:55 AM, James Youngman  wrote:
> I'm looking into creating a C++ coding standard for GNU programs.

So, parts of how we do C++ isn't really relevant to a larger community.  We at 
times cater to a C legacy and in a pure C++ project, there may be no need to do 
this.

ChangeLogs are debatable, and a project might want to option out of them.  
We've talk about this from time to time, but haven't decided to do that.

We sometimes talk about significantly older language standards.  I'd like to 
think most project would update which language standard they use just a wee bit 
faster than we do.

But, all in all, sounds like a reasonable thing to do.  One skilled it the art 
of C++ and coding standards should be able to tease out the things that makes 
less sense I'd like to think.

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Bernhard Reutner-Fischer]

On 30 May 2019 18:23:43 CEST, Qing Zhao  wrote:
>Hi,
>
>PR 90581 (provide an option to adjust the maximum depth of nested
>#include)
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>
>is to add a new cpp option -fmax-inlcude-depth

Typo inl vs inc.

Why isn't this a param?
Wouldn't a param ease range checking not to overflow the uint max and maybe 
automagically provide diagnostics for out of range input?

In the docs you mix "number" and "depth".

thanks,

> to set the maximum depth
>of nested #include.
>
>'-fmax-include-depth=DEPTH'
> Set the maximum depth of the nested include.  The default value is
> 200.
>
>Please check the attached patch.
>I have done bootstrap and regression test on X86, no any issue.
>
>thanks a lot.
>
>Qing.
>
>
>
>gcc/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* doc/cppopts.texi: Add document for -fmax-include-depth.
>* doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.
>
>libcpp/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* directives.c (do_include_common): Replace CPP_STACK_MAX with
>CPP_OPTION (pfile, max_include_depth).
>* include/cpplib.h (struct cpp_options): Add new field
>max_include_depth.
>* init.c (cpp_create_reader): Initiate new field max_include_depth.
>* internal.h: Delete CPP_STACK_MAX.
>
>gcc/c-family/ChangeLog:
>
>2019-05-30  qing zhao  
>
>   * c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
>* c.opt: Add new option -fmax-include-depth.
>
>gcc/testsuite/ChangeLog:
>
>2019-05-30  qing zhao  
>
>* c-c++-common/cpp/fmax-include-depth-1a.h: New test.
>* c-c++-common/cpp/fmax-include-depth-1b.h: New test.
>* c-c++-common/cpp/fmax-include-depth.c: New test.

Re: [PATCH] rs6000: Add basic support for prefixed and PC-relative addresses

[Segher Boessenkool]

On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
>   * config/rs6000/predicates.md (pcrel_address): New
>   define_predicate.

Please put that on one line?

> +  if (LABEL_REF_P (x))
> + output_asm_label (x);
> +  else
> + output_addr_const (file, x);

Why does LABEL_REF need separate handling here?

> +  if (offset)
> + fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",
> +  offset);

Maybe

  if (offset)
fprintf (file, "%+" PRId64, offset);

(but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for
that then.  Oh well).

> +rs6000_prefixed_address (rtx addr, machine_mode mode)

> +  if (GET_CODE (addr) == PLUS)
> +{
> +  HOST_WIDE_INT value, mask;

Please declare these where they are defined?

> +  /* DS instruction (bottom 2 bits must be 0).  For 32-bit integers, we
> +  need to use DS instructions if we are sign-extending the value with
> +  LWA.  For 32-bit floating point, we need DS instructions to load and
> +  store values to the traditional Altivec registers.  */
> +  else if (GET_MODE_SIZE (mode) >= 4)
> + mask = 3;

I don't much like penalising scalar single precision float like this.
But, this also hurts unaligned lwz...  Do we have statistics on that?
Offline, I guess :-)

> +  /* Return true if we must use a prefixed instruction.  */
> +  return ((value & ~mask) != value);

(value & mask) != 0


Okay with those things frobbed a little, or looked at.  Thanks!


Segher


Segher

Re: [PATCH] rs6000: Add target supports for "future" system

[Segher Boessenkool]

On Wed, May 29, 2019 at 10:52:38PM -0500, Bill Schmidt wrote:
>   * lib/target-supports.exp (check_powerpc_future_hw_available):
>   New.

That fits on one line just fine?

> +# Return 1 if this is a PowerPC target supporting -mfuture.
> +# Limit this to 64-bit linux systems for now until other
> +# targets support FUTURE.
> +
> +proc check_effective_target_powerpc_future_ok { } {
> +if { ([istarget powerpc64*-*-linux*]) } {

It should be limited to powerpc64le-*-linux, or at least to ELFv2, right?

Not that that should matter at all: you pass -mfuture, and the compiler
will not allow that on unsupported configurations anyway (right? :-) )

But, okay for trunk.  Thanks!


Segher

Re: [PATCH] rs6000: Update test cases to use powerpc_future_ok

[Segher Boessenkool]

Hi!

On Wed, May 29, 2019 at 10:54:58PM -0500, Bill Schmidt wrote:
> 2019-05-29  Bill Schmidt  
>   Michael Meissner  
> 
>   * gcc.target/powerpc/cpu-future.c: Require powerpc_future_ok.
>   * gcc.target/powerpc/localentry-1.c: Likewise.
>   * gcc.target/powerpc/localentry-direct-1.c: Likewise.
>   * gcc.target/powerpc/notoc-direct-1.c: Likewise.
>   * gcc.target/powerpc/pcrel-sibcall-1.c: Likewise.

Okay for trunk.  Thanks!


Segher

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Marek Polacek]

On Tue, May 28, 2019 at 09:30:56PM +, Sean Gillespie wrote:
> This adds a new warning, -Wglobal-constructors, that warns whenever a
> decl requires a global constructor or destructor. This new warning fires
> whenever a thread_local or global variable is declared whose type has a
> non-trivial constructor or destructor. When faced with both a constructor
> and a destructor, the error message mentions the destructor and is only
> fired once.
> 
> This warning mirrors the Clang option -Wglobal-constructors, which warns
> on the same thing. -Wglobal-constructors was present in Apple's GCC and
> later made its way into Clang.
> 
> Bootstrapped and regression-tested on x86-64 linux, new tests passing.

Thanks for the patch!  Do you have a copyright assignment on file?  Unsure
if this patch is small enough not to need it.

> gcc/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  
> 
>   * doc/invoke.texi: Add new flag -Wglobal-constructors.
> 
> gcc/c-family/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  
> 
>   * c.opt: Add new flag -Wglobal-constructors.
> 
> gcc/cp/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  
> 
>   * decl.c (expand_static_init): Warn if a thread local or static decl
>   requires a non-trivial constructor or destructor.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-05-28  Sean Gillespie  
> 
>   * g++.dg/warn/global-constructors-1.C: New test.
>   * g++.dg/warn/global-constructors-2.C: New test.

These are fine but please also mention "PR c++/71482" in them.

> ---
>  gcc/c-family/c.opt|  4 +++
>  gcc/cp/decl.c | 17 ++---
>  gcc/doc/invoke.texi   |  7 ++
>  .../g++.dg/warn/global-constructors-1.C   | 25 +++
>  .../g++.dg/warn/global-constructors-2.C   | 23 +
>  5 files changed, 73 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
>  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 046d489f7eb..cf62625ca42 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -613,6 +613,10 @@ Wformat-truncation=
>  C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) 
> Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) 
> IntegerRange(0, 2)
>  Warn about calls to snprintf and similar functions that truncate output.
>  
> +Wglobal-constructors
> +C++ ObjC++ Var(warn_global_constructors) Warning
> +Warn when a global declaration requires a constructor to initialize.
> +

Ok, I agree this shouldn't be turned on by -Wall or -Wextra.

> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 19d14a6a5e9..c1d66195bd7 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
>finish_then_clause (if_stmt);
>finish_if_stmt (if_stmt);
>  }
> -  else if (CP_DECL_THREAD_LOCAL_P (decl))
> -tls_aggregates = tree_cons (init, decl, tls_aggregates);
>else
> -static_aggregates = tree_cons (init, decl, static_aggregates);
> +   {
> +if (CP_DECL_THREAD_LOCAL_P (decl))
> +  tls_aggregates = tree_cons (init, decl, tls_aggregates);
> +else
> +  static_aggregates = tree_cons (init, decl, static_aggregates);
> +
> +if (warn_global_constructors)

This check is not wrong but you can probably drop it.  We use it if the warning
is expensive, but this doesn't seem to be the case.

> + {
> +  if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
> +warning(OPT_Wglobal_constructors, "declaration requires a global 
> destructor");
> +  else
> +warning(OPT_Wglobal_constructors, "declaration requires a global 
> constructor");
> + }
> +   }

The formatting a bit wrong, please use two spaces to indent.  There should be
a space before a (.  And the lines should not exceed 80 characters.

I think it would be better to use warning_at instead of warning.  As the
location, you can use 'dloc' defined earlier in the function if you move
it out of the block.


It seems this patch will also warn for

struct A {
  A() = default;
};

A a;

which I think we don't want?

I also think you want to add a test using a constexpr constructor.

Marek

Re: [PATCH] rs6000: Add basic support for prefixed and PC-relative addresses

[Bill Schmidt]

On 5/30/19 2:20 PM, Segher Boessenkool wrote:
> On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
>>  * config/rs6000/predicates.md (pcrel_address): New
>>  define_predicate.
> Please put that on one line?

OK.  Emacs in ChangeLog and Fill modes seems to set a line length
somewhat less than 79.  I generally follow what it tells me, but I can
fix this.
>
>> +  if (LABEL_REF_P (x))
>> +output_asm_label (x);
>> +  else
>> +output_addr_const (file, x);
> Why does LABEL_REF need separate handling here?

Mike, please respond?
>
>> +  if (offset)
>> +fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",
>> + offset);
> Maybe
>
>   if (offset)
>   fprintf (file, "%+" PRId64, offset);
>
> (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for
> that then.  Oh well).

Will have a look.
>
>> +rs6000_prefixed_address (rtx addr, machine_mode mode)
>> +  if (GET_CODE (addr) == PLUS)
>> +{
>> +  HOST_WIDE_INT value, mask;
> Please declare these where they are defined?

ok.
>
>> +  /* DS instruction (bottom 2 bits must be 0).  For 32-bit integers, we
>> + need to use DS instructions if we are sign-extending the value with
>> + LWA.  For 32-bit floating point, we need DS instructions to load and
>> + store values to the traditional Altivec registers.  */
>> +  else if (GET_MODE_SIZE (mode) >= 4)
>> +mask = 3;
> I don't much like penalising scalar single precision float like this.
> But, this also hurts unaligned lwz...  Do we have statistics on that?
> Offline, I guess :-)

Again, I'll defer to Mike on those details (online or offline). ;-)
>
>> +  /* Return true if we must use a prefixed instruction.  */
>> +  return ((value & ~mask) != value);
> (value & mask) != 0

OK.
>
>
> Okay with those things frobbed a little, or looked at.  Thanks!

Thanks for the review on your day off!
Bill
>
>
> Segher
>
>
> Segher

Re: [v3 PATCH] basic_string spurious use of a default constructible allocator - LWG2788

[Jonathan Wakely]

On 27/05/19 11:53 +0100, Nina Dinka Ranns wrote:

Tested on Linux x86_64
basic_string spurious use of a default constructible allocator - LWG2788

2019-05-27  Nina Dinka Ranns  
   basic_string spurious use of a default constructible allocator - LWG2788
   * bits/basic_string.tcc:
   (_M_replace_dispatch()): string temporary now constructed with
the current allocator
   * testsuite/21_strings/basic_string/allocator/char/lwg2788.cc: New
   * testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc: New


Thanks!



Index: libstdc++-v3/include/bits/basic_string.tcc
===
--- libstdc++-v3/include/bits/basic_string.tcc  (revision 271509)
+++ libstdc++-v3/include/bits/basic_string.tcc  (working copy)
@@ -381,7 +381,9 @@
  _InputIterator __k1, _InputIterator __k2,
  std::__false_type)
  {
-   const basic_string __s(__k1, __k2);
+// _GLIBCXX_RESOLVE_LIB_DEFECTS
+// 2788. unintentionally require a default constructible allocator


I've fixed the indentation of this comment.


+   const basic_string __s(__k1, __k2, this->get_allocator());
const size_type __n1 = __i2 - __i1;
return _M_replace(__i1 - begin(), __n1, __s._M_data(),
  __s.size());
Index: libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc
===
--- libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc (revision 271509)
+++ libstdc++-v3/testsuite/20_util/nonesuch/nonesuch.cc (nonexistent)


And dropped this part of the patch which spuriously removed an
unrelated file.

Tested powerpc64le-linux with both string ABIs.

The version I've committed is attached.


commit a0ee1818604eb6998e9cc26005ba754021a3f067
Author: Jonathan Wakely 
Date:   Thu May 30 17:45:38 2019 +0100

LWG2788 basic_string spurious use of a default constructible allocator

This only change the cxx11 basic_string, because COW strings don't
correctly propagate allocators anyway.

2019-05-30  Nina Dinka Ranns  

LWG2788 basic_string spurious use of a default constructible allocator
* include/bits/basic_string.tcc [_GLIBCXX_USE_CXX11_ABI]
(basic_string::_M_replace_dispatch): Construct temporary string with
the current allocator.
* testsuite/21_strings/basic_string/allocator/char/lwg2788.cc: New.
* testsuite/21_strings/basic_string/allocator/wchar_t/lwg2788.cc: New.

diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index e2315cb1467..ab986a6c827 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -381,7 +381,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			  _InputIterator __k1, _InputIterator __k2,
 			  std::__false_type)
   {
-	const basic_string __s(__k1, __k2);
+	// _GLIBCXX_RESOLVE_LIB_DEFECTS
+	// 2788. unintentionally require a default constructible allocator
+	const basic_string __s(__k1, __k2, this->get_allocator());
 	const size_type __n1 = __i2 - __i1;
 	return _M_replace(__i1 - begin(), __n1, __s._M_data(),
 			  __s.size());
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc
new file mode 100644
index 000..36dd414e887
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/allocator/char/lwg2788.cc
@@ -0,0 +1,85 @@
+// { dg-do run { target c++11 } }
+// { dg-require-effective-target cxx11-abi }
+
+// 2019-05-27  Nina Dinka Ranns  
+//
+// Copyright (C) 2015-2019 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
+// .
+
+#include 
+#include 
+
+using C = char;
+using traits = std::char_traits;
+int constructCount = 0;
+
+static void resetCounter()
+{
+ constructCount = 0;
+}
+
+template 
+struct TestAllocator
+{
+  typedef Tp value_type;
+  using size_type = unsigned;
+
+  TestAllocator() noexcept { constructCount++; }
+
+  template 
+  TestAllocator(const TestAllocator&) {}
+
+  Tp *allocate(std::size_t n)
+  { return std::allocator().allocate(n); }
+
+  void deallocate(Tp *p, std::size_t n)
+  { std::allocator().deal

Re: [PATCH] PR libstdc++/85494 use rdseed and rand_s in std::random_device

[Jonathan Wakely]

On 29/05/19 23:03 +0100, Jonathan Wakely wrote:

On 29/05/19 15:45 +0100, Jonathan Wakely wrote:

Add support for additional sources of randomness to std::random_device,
to allow using RDSEED for Intel CPUs and rand_s for Windows. When
supported these can be selected using the tokens "rdseed" and "rand_s".
For *-w64-mingw32 targets the "default" token will now use rand_s, and
for other i?86-*-* and x86_64-*-* targets it will try to use "rdseed"
first, then "rdrand", and finally "/dev/urandom".

To simplify the declaration of std::random_device in  the
constructors now unconditionally call _M_init instead of _M_init_pretr1,
and the function call operator now unconditionally calls _M_getval. The
library code now decides whether _M_init and _M_getval should use a real
source of randomness or the mt19937 engine.

Existing code compiled against old libstdc++ headers will still call
_M_init_pretr1 and _M_getval_pretr1, but those functions now forward to
_M_init and _M_getval if a real source of randomness is available. This
means existing code compiled for mingw-w64 will start to use rand_s just
by linking to a new libstdc++.dll.

* acinclude.m4 (GLIBCXX_CHECK_X86_RDSEED): Define macro to check if
the assembler supports rdseed.
* config.h.in: Regenerate.
* configure: Regenerate.
* configure.ac: Use GLIBCXX_CHECK_X86_RDSEED.
* config/os/mingw32-w64/os_defines.h (_GLIBCXX_USE_CRT_RAND_S): Define.
* doc/html/*: Regenerate.
* doc/xml/manual/status_cxx2011.xml: Document new tokens.
* include/bits/random.h (random_device::random_device()): Always call
_M_init rather than _M_init_pretr1.
(random_device::random_device(const string&)): Likewise.
(random_device::operator()()): Always call _M_getval().
(random_device::_M_file): Replace first member of union with an
anonymous struct, with _M_file as its first member.
* src/c++11/random.cc [_GLIBCXX_X86_RDRAND] (USE_RDRAND): Define.
[_GLIBCXX_X86_RDSEED] (USE_RDSEED): Define.
(USE_MT19937): Define if none of the above are defined.
(USE_POSIX_FILE_IO): Define.
(_M_strtoul): Remove.
[USE_RDSEED] (__x86_rdseed): Define new function.
[_GLIBCXX_USE_CRT_RAND_S] (__winxp_rand_s): Define new function.
(random_device::_M_init(const string&)): Initialize new union members.
Add support for "rdseed" and "rand_s" tokens. Decide what the
"default" token does according to which USE_* macros are defined.
[USE_POSIX_FILE_IO]: Store a file descriptor.
[USE_MT19937]: Forward to _M_init_pretr1 instead.
(random_device::_M_init_pretr1(const string&)) [USE_MT19937]: Inline
code from _M_strtoul.
[!USE_MT19937]: Call _M_init, transforming the old default token or
numeric tokens to "default".
(random_device::_M_fini()) [USE_POSIX_FILE_IO]: Use close not fclose.
(random_device::_M_getval()): Use new union members to obtain a
random number from the stored function pointer or file descriptor.
[USE_MT19937]: Obtain a value from the mt19937 engine.
(random_device::_M_getval_pretr1()): Call _M_getval().
(random_device::_M_getentropy()) [USE_POSIX_FILE_IO]: Use _M_fd
instead of fileno.
[!USE_MT19937] (mersenne_twister): Do not instantiate when not needed.
* testsuite/26_numerics/random/random_device/85494.cc: New test.


This fixes a test that started to fail in mingw.


I forgot that the random_device::_M_init member is duplicated in
src/c++11/cow-string-inst.cc for the old strings, so tests using the
old string now fail.

I'll try to find a better way to define those member functions for old
strings.

Re: [PATCH] A jump threading opportunity for condition branch

[Jeff Law]

On 5/30/19 12:41 AM, Richard Biener wrote:
> On May 29, 2019 10:18:01 PM GMT+02:00, Jeff Law  wrote:
>> On 5/23/19 6:11 AM, Richard Biener wrote:
>>> On Thu, 23 May 2019, Jiufu Guo wrote:
>>>
 Hi,

 Richard Biener  writes:

> On Tue, 21 May 2019, Jiufu Guo wrote:
>>
>> +}
>> +
>> +  if (TREE_CODE_CLASS (gimple_assign_rhs_code (def)) !=
>> tcc_comparison)
>> +return false;
>> +
>> +  /* Check if phi's incoming value is defined in the incoming
>> basic_block.  */
>> +  edge e = gimple_phi_arg_edge (phi, index);
>> +  if (def->bb != e->src)
>> +return false;
> why does this matter?
>
 Through preparing pathes and duplicating block, this transform can
>> also
 help to combine a cmp in previous block and a gcond in current
>> block.
 "if (def->bb != e->src)" make sure the cmp is define in the incoming
 block of the current; and then combining "cmp with gcond" is safe. 
>> If
 the cmp is defined far from the incoming block, it would be hard to
 achieve the combining, and the transform may not needed.
>>> We're in SSA form so the "combining" doesn't really care where the
>>> definition comes from.
>> Combining doesn't care, but we need to make sure the copy of the
>> conditional ends up in the right block since it wouldn't necessarily be
>> associated with def->bb anymore.  But I'd expect the sinking pass to
>> make this a non-issue in practice anyway.
>>
>>>
>> +
>> +  if (!single_succ_p (def->bb))
>> +return false;
> Or this?  The actual threading will ensure this will hold true.
>
 Yes, other thread code check this and ensure it to be true, like
 function thread_through_normal_block. Since this new function is
>> invoked
 outside thread_through_normal_block, so, checking single_succ_p is
>> also
 needed for this case.
>>> I mean threading will isolate the path making this trivially true.
>>> It's also no requirement for combining, in fact due to the single-use
>>> check the definition can be sinked across the edge already (if
>>> the edges dest didn't have multiple predecessors which this threading
>>> will fix as well).
>> I don't think so.  The CMP source block could end with a call and have
>> an abnormal edge (for example).  We can't put the copied conditional
>> before the call and putting it after the call essentially means
>> creating
>> a new block.
>>
>> The CMP source block could also end with a conditional.  Where do we
>> put
>> the one we want to copy into the CMP source block in that case? :-)
>>
>> This is something else we'd want to check if we ever allowed the the
>> CMP
>> defining block to not be the immediate predecessor of the conditional
>> jump block.  If we did that we'd need to validate that the block where
>> we're going to insert the copy of the jump has a single successor.
> 
> But were just isolating a path here. The actual combine job is left to 
> followup cleanups. 
Absolutely agreed.  My point was that there's some additional stuff we'd
have to verify does the right thing if we wanted to allow the CMP to be
somewhere other than in the immediate predecessor of the conditional
jump block.

Jeff

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer  
> wrote:
> 
> On 30 May 2019 18:23:43 CEST, Qing Zhao  wrote:
>> Hi,
>> 
>> PR 90581 (provide an option to adjust the maximum depth of nested
>> #include)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>> 
>> is to add a new cpp option -fmax-inlcude-depth
> 
> Typo inl vs inc.

Okay, will fix this.

> 
> Why isn't this a param?

do you mean to change “-fmax-include-depth=” to “-param max-include-depth=“?

> Wouldn't a param ease range checking not to overflow the uint max and maybe 
> automagically provide diagnostics for out of range input?
> 
> In the docs you mix "number" and "depth”.

will fix this.

thanks for the feedback.

Qing
> 
> thanks,
> 
>> to set the maximum depth
>> of nested #include.
>> 
>> '-fmax-include-depth=DEPTH'
>>Set the maximum depth of the nested include.  The default value is
>>200.
>> 
>> Please check the attached patch.
>> I have done bootstrap and regression test on X86, no any issue.
>> 
>> thanks a lot.
>> 
>> Qing.
>> 
>> 
>> 
>> gcc/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * doc/cppopts.texi: Add document for -fmax-include-depth.
>>   * doc/invoke.texi (Preprocessor Options): List -fmax-include-depth.
>> 
>> libcpp/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * directives.c (do_include_common): Replace CPP_STACK_MAX with
>>   CPP_OPTION (pfile, max_include_depth).
>>   * include/cpplib.h (struct cpp_options): Add new field
>>   max_include_depth.
>>   * init.c (cpp_create_reader): Initiate new field max_include_depth.
>>   * internal.h: Delete CPP_STACK_MAX.
>> 
>> gcc/c-family/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>  * c-opts.c (c_common_handle_option): Handle -fmax-include-depth.
>>   * c.opt: Add new option -fmax-include-depth.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 2019-05-30  qing zhao  
>> 
>>   * c-c++-common/cpp/fmax-include-depth-1a.h: New test.
>>   * c-c++-common/cpp/fmax-include-depth-1b.h: New test.
>>   * c-c++-common/cpp/fmax-include-depth.c: New test.
> 

Re: [PATCH] rs6000: Add basic support for prefixed and PC-relative addresses

[Segher Boessenkool]

On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote:
> On 5/30/19 2:20 PM, Segher Boessenkool wrote:
> > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
> >>* config/rs6000/predicates.md (pcrel_address): New
> >>define_predicate.
> > Please put that on one line?
> 
> OK.  Emacs in ChangeLog and Fill modes seems to set a line length
> somewhat less than 79.  I generally follow what it tells me, but I can
> fix this.

This would be just 76 ;-)

> >> +  if (offset)
> >> +  fprintf (file, "%s" HOST_WIDE_INT_PRINT_DEC, (offset > 0) ? "+" : "",
> >> +   offset);
> > Maybe
> >
> >   if (offset)
> > fprintf (file, "%+" PRId64, offset);
> >
> > (but there really should be a HOST_WIDE_INT_PRINT_DEC_PLUS or similar for
> > that then.  Oh well).
> 
> Will have a look.

Very many things in GCC do this, but it's so inelegant :-)  It's fine to
commit it like this, as I said.  But it annoys me every time I see it :-)


Segher

Re: [PATCH][Preprocessor]patch to fix PR 90581

[David Malcolm]

On Thu, 2019-05-30 at 15:17 -0500, Qing Zhao wrote:
> > On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer  > gmail.com> wrote:
> > 
> > On 30 May 2019 18:23:43 CEST, Qing Zhao 
> > wrote:
> > > Hi,
> > > 
> > > PR 90581 (provide an option to adjust the maximum depth of nested
> > > #include)
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> > > 
> > > is to add a new cpp option -fmax-inlcude-depth
> > 
> > Typo inl vs inc.
> 
> Okay, will fix this.
> 
> > 
> > Why isn't this a param?
> 
> do you mean to change “-fmax-include-depth=” to “-param max-include-
> depth=“?
> 

That sounds like a good idea to me.

> > Wouldn't a param ease range checking not to overflow the uint max
> > and maybe automagically provide diagnostics for out of range input?

(indeed)

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 3:38 PM, David Malcolm  wrote:
> 
> On Thu, 2019-05-30 at 15:17 -0500, Qing Zhao wrote:
>>> On May 30, 2019, at 2:13 PM, Bernhard Reutner-Fischer >> gmail.com> wrote:
>>> 
>>> On 30 May 2019 18:23:43 CEST, Qing Zhao 
>>> wrote:
 Hi,
 
 PR 90581 (provide an option to adjust the maximum depth of nested
 #include)
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
 
 is to add a new cpp option -fmax-inlcude-depth
>>> 
>>> Typo inl vs inc.
>> 
>> Okay, will fix this.
>> 
>>> 
>>> Why isn't this a param?
>> 
>> do you mean to change “-fmax-include-depth=” to “-param max-include-
>> depth=“?
>> 
> 
> That sounds like a good idea to me.

can -param be accepted by preprocessor? 

I didn’t see any -param in gcc/c-family/c.opt.

where should I define a -param for preprocessor?

thanks.

Qing
> 
>>> Wouldn't a param ease range checking not to overflow the uint max
>>> and maybe automagically provide diagnostics for out of range input?
> 
> (indeed)
> 
> 

Re: [PATCH][Preprocessor]patch to fix PR 90581

[David Malcolm]

On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
> Hi,
> 
> PR 90581 (provide an option to adjust the maximum depth of nested
> #include)
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
> 
> is to add a new cpp option -fmax-inlcude-depth to set the maximum
> depth of nested #include.
> 
> '-fmax-include-depth=DEPTH'
>  Set the maximum depth of the nested include.  The default value
> is
>  200.
> 
> Please check the attached patch.
> I have done bootstrap and regression test on X86, no any issue.
> 
> thanks a lot.
> 
> Qing.
> 
Thanks for working on this.  It's looking promising, but I agree that a
param would be better than an option.

One idea that occurred to me looking at the patch...

> index 3ee8bc4..480c282 100644
> --- a/libcpp/directives.c
> +++ b/libcpp/directives.c
> @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
> type)
>  }
>  
>/* Prevent #include recursion.  */
> -  if (pfile->line_table->depth >= CPP_STACK_MAX)
> +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
>  cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");

...a nice usability tweak here would be to give a hint about the new
param, to give the user an idea on how to increase the limit.

Maybe something like:

cpp_error (pfile, CPP_DL_ERROR,
  "%<#include%> nested too deeply (depth %i);"
  " use %<--param max-include-depth=LIMIT%> to support deeper nesting",
  pfile->line_table->depth);

(though probably that would be better as a followup "note")

Dave

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Wed, May 22, 2019 at 6:41 PM Ian Lance Taylor  wrote:
> Jim, you can go ahead and commit that patch with a ChangeLog entry.
> (The files in go/gcc but not in go/gcc/gofrontend) are under normal
> GCC rules.)  Thanks.

OK.  Committed.  I did an x86_64-linux go build and check to make sure
I didn't break that.

Jim

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Wed, May 22, 2019 at 6:36 PM Cherry Zhang  wrote:
> Jim, thank you for the fix! The patch looks good to me. Maybe we should also 
> apply this to __atomic_add_fetch_4 and __atomic_add_fetch_8?

Sorry about the delay, I caught a virus last week and am behind on everything.

For 64-bit RISC-V those are open coded and hence not a problems.  All
of the major targets open code everything except RISC-V which needs a
library call for the 1 and 2 byte atomics, though this is on our list
of things to fix.  We just haven't gotten around to it yet.  For
32-bit RISC-V, the 8-byte atomic would require a library call, but
32-bit riscv-linux isn't formally supported, as the glibc patches are
not upstream yet, so this isn't a practical problem yet.

Anyways, yes, in theory, we should be handling these two also, but
there probably aren't any well supported targets that will fail if
they are missing.

Jim

[PATCH] libiberty: Don't use VLAs if __STDC_NO_VLA__ is non-zero

[Michael Forney]

VLAs are optional in C11, and an implementation that does not support
them will define __STDC_NO_VLA__ to 1.

2019-05-30  Michael Forney  

* cp-demangle.c: Don't define CP_DYNAMIC_ARRAYS if __STDC_NO_VLA__
is non-zero.
---
 libiberty/cp-demangle.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 4e5b733b548..aa78c86dd44 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -192,9 +192,9 @@ static void d_init_info (const char *, int, size_t, struct 
d_info *);
 #else
 #ifdef __STDC__
 #ifdef __STDC_VERSION__
-#if __STDC_VERSION__ >= 199901L
+#if __STDC_VERSION__ >= 199901L && !__STDC_NO_VLA__
 #define CP_DYNAMIC_ARRAYS
-#endif /* __STDC__VERSION >= 199901L */
+#endif /* __STDC_VERSION__ >= 199901L && !__STDC_NO_VLA__ */
 #endif /* defined (__STDC_VERSION__) */
 #endif /* defined (__STDC__) */
 #endif /* ! defined (__GNUC__) */
-- 
2.20.1

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Thu, May 30, 2019 at 11:11 AM Andreas Schwab  wrote:
> Here are the test results:
> http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html

I tried running RISC-V go checks on my system.  I see 7 unexpected
failures for gcc check-go, 6 unexpected failures for check-gotools,
and 1 unexpected failure for check-target-libgo.  I haven't tried
building and testing the go support before so I don't have any old
results to compare against.  It seems reasonable for a first attempt
though.

Jim

Re: [PATCH][Preprocessor]patch to fix PR 90581

[Qing Zhao]

> On May 30, 2019, at 3:46 PM, David Malcolm  wrote:
> 
> On Thu, 2019-05-30 at 11:23 -0500, Qing Zhao wrote:
>> Hi,
>> 
>> PR 90581 (provide an option to adjust the maximum depth of nested
>> #include)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581
>> 
>> is to add a new cpp option -fmax-inlcude-depth to set the maximum
>> depth of nested #include.
>> 
>> '-fmax-include-depth=DEPTH'
>> Set the maximum depth of the nested include.  The default value
>> is
>> 200.
>> 
>> Please check the attached patch.
>> I have done bootstrap and regression test on X86, no any issue.
>> 
>> thanks a lot.
>> 
>> Qing.
>> 
> Thanks for working on this.  It's looking promising, but I agree that a
> param would be better than an option.

I just checked both gcc source code and gcc documentation. 
looks like that 

—param name=value 

is an option for “Options that Control Optimization”.

Since this new option is for preprocessor, I think that it might not be fit in?

let me know if I miss anything here.

> 
> One idea that occurred to me looking at the patch...
> 
>> index 3ee8bc4..480c282 100644
>> --- a/libcpp/directives.c
>> +++ b/libcpp/directives.c
>> @@ -831,7 +831,7 @@ do_include_common (cpp_reader *pfile, enum include_type 
>> type)
>> }
>> 
>>   /* Prevent #include recursion.  */
>> -  if (pfile->line_table->depth >= CPP_STACK_MAX)
>> +  if (pfile->line_table->depth >= CPP_OPTION (pfile, max_include_depth))
>> cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply");
> 
> ...a nice usability tweak here would be to give a hint about the new
> param, to give the user an idea on how to increase the limit.

Yes, I agree.
will fix this.

Thanks.

Qing
> 
> Maybe something like:
> 
> cpp_error (pfile, CPP_DL_ERROR,
> "%<#include%> nested too deeply (depth %i);"
> " use %<--param max-include-depth=LIMIT%> to support deeper nesting",
> pfile->line_table->depth);
> 
> (though probably that would be better as a followup "note")
> 
> Dave

[PATCH,libstdc++] C++-20 costexpr and

[Ed Smith-Rowland via gcc-patches]

Greetings,

I was not quite able to finish this in for gcc9 but here is the patch for:

?? Implement C++20 p0202 - Add Constexpr Modifiers to Functions
 ??in  and  Headers.
 ??Implement C++20 p1023 - constexpr comparison operators for std::array.

I believe I have answered peoples concerns with the last patch attempts 
[https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].


The patch is large because of test cases but really just boils down to 
adding constexpr for c++2a.


The patch passes for gnu++2a and pre-gnu++2a on x86_64-linux.

OK for trunk?

Ed Smith-Rowland


2019-05-31  Edward Smith-Rowland  <3dw...@verizon.net>

Implement C++20 p0202 - Add Constexpr Modifiers to Functions
in  and  Headers.
Implement C++20 p1023 - constexpr comparison operators for std::array.
* include/bits/algorithmfwd.h (all_of, any_of, binary_search, copy,
copy_backward, copy_if, copy_n, equal_range, fill, fill_n, find_end,
find_if_not, includes, is_heap, is_heap_until, is_partitioned,
is_permutation, is_sorted, is_sorted_until, lower_bound, none_of,
partition_copy, partition_point, remove, remove_if, remove_copy,
remove_copy_if, replace_copy, replace_copy_if, reverse_copy,
rotate_copy, unique, upper_bound, adjacent_find, count, count_if, equal,
find, find_first_of, find_if, for_each, generate, generate_n,
lexicographical_compare, merge, mismatch, replace, replace_if, search,
search_n, set_difference, set_intersection, set_symmetric_difference,
set_union, transform, unique_copy): Mark constexpr.
Make versions of operator() const.
* include/bits/cpp_type_traits.h (__miter_base): Make constexpr.
* include/bits/predefined_ops.h (_Iter_less_val, __iter_comp_val,
_Val_less_iter, __val_less_iter, __val_comp_iter, _Iter_equal_to_iter,
__iter_equal_to_iter, _Iter_equal_to_val, __iter_equal_to_val,
_Iter_comp_val, __iter_comp_val, _Val_comp_iter, __val_comp_iter,
_Iter_equals_val, __iter_equals_val, _Iter_equals_iter,
__iter_comp_iter, __pred_iter, __iter_comp_val, __negate): Constexpr
ctors. Add const operator().
* include/bits/stl_algo.h (__find_if, __find_if_not, __find_if_not_n,
__search, __search_n_aux, __search_n, __find_end, find_end, all_of
none_of, any_of, find_if_not, is_partitioned, partition_point,
__remove_copy_if, remove_copy, remove_copy_if, copy_if, __copy_n,
copy_n, partition_copy, __remove_if, remove, remove_if, __adjacent_find,
__unique, unique, __unique_copy, reverse_copy, rotate_copy,
__unguarded_linear_insert, __insertion_sort, __unguarded_insertion_sort,
__final_insertion_sort, lower_bound, __upper_bound, upper_bound,
__equal_range, equal_range, binary_search, __includes, includes,
__next_permutation, __prev_permutation, __replace_copy_if, replace_copy,
replace_copy_if, __count_if, is_sorted, __is_sorted_until,
is_sorted_until, __is_permutation, is_permutation, for_each, find,
find_if, find_first_of, adjacent_find, count, count_if, search,
search_n, transform, replace, replace_if, generate, generate_n,
unique_copy, __merge, merge, __set_union, set_union, __set_intersection,
set_intersection, __set_difference, set_difference,
__set_symmetric_difference, set_symmetric_difference): Constexpr.
* include/bits/stl_algobase.h (__niter_base, __niter_wrap, __copy_m,
__copy_move_a, copy, move, __copy_move_backward::__copy_move_b,
__copy_move_backward_a, __copy_move_backward_a2, copy_backward,
move_backward, fill, __fill_a, __fill_n_a, fill_n, __equal::equal,
__equal_aux, __newlast1, __cnd2, __newlast1, __cnd2,
__lexicographical_compare_impl, __lexicographical_compare::__lc,
__lexicographical_compare, __lexicographical_compare_aux,
__lower_bound, lower_bound, equal, __equal4, lexicographical_compare,
__mismatch, mismatch): Constexpr.
(__memmove, __memcmp): New constexpr wrappers.
* include/bits/stl_heap.h (__is_heap_until, __is_heap, is_heap_until,
is_heap): Constexpr.
* include/bits/stl_iterator.h (__niter_base, __miter_base): Constexpr.
* include/std/array: Make comparison ops constexpr.
* include/std/utility: Make exchange constexpr.
* testsuite/23_containers/array/tuple_interface/get_neg.cc: Adjust.
* testsuite/23_containers/array/tuple_interface/
tuple_element_neg.cc: Adjust.
* testsuite/25_algorithms/copy_n/58982.cc: Adjust error scan.
Prune "deleted".
* testsuite/25_algorithms/copy/58982.cc: Adjust error scan.
Prune "deleted".
* testsuite/25_algorithms/copy/move_iterators/69478.cc: Prune "deleted".
* testsuite/25_algorithms/copy_backward/move_iterators/69478.cc:
Prune "deleted".
*

[committed] Optimize lastprivate conditional in worksharing constructs nested in parallel constructs

[Jakub Jelinek]

Hi!

The following patch implements an optimization.  For orphaned worksharing
constructs we need to ask the runtime library to allocate some memory
in the first thread that encounters the construct, but if it is nested
inside of a parallel construct, we can allocate the memory on the stack of
the initial thread at the start of the parallel construct.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2019-05-30  Jakub Jelinek  

* gimplify.c (enum gimplify_omp_var_data): Add GOVD_CONDTEMP.
(gimplify_adjust_omp_clauses_1): Handle GOVD_CONDTEMP.
(gimplify_omp_for): If worksharing loop with lastprivate conditional
is nested inside of parallel region, add _condtemp_ clause to both.
* tree-nested.c (convert_nonlocal_omp_clauses,
convert_local_omp_clauses): Ignore OMP_CLAUSE__CONDTEMP_ instead of
assertion failure.
* omp-general.h (struct omp_for_data): Add have_pointer_condtemp
member.
* omp-general.c (omp_extract_for_data): Compute it.
* omp-low.c (scan_sharing_clauses): Handle OMP_CLAUSE__CONDTEMP_.
(lower_rec_input_clauses): Likewise.
(lower_lastprivate_conditional_clauses): If OMP_CLAUSE__CONDTEMP_
clause is already present, just add one further one after it.
(lower_lastprivate_clauses): Handle cond_ptr with array type.
(lower_send_shared_vars): Clear _condtemp_ vars.
(lower_omp_1) : Handle target data like critical
or section or taskgroup.
* omp-expand.c (determine_parallel_type): Disallow combining only if
first OMP_CLAUSE__CONDTEMP_ has pointer type.  Disallow combining
of parallel sections if OMP_CLAUSE__CONDTEMP_ is present.
(expand_omp_for_generic, expand_omp_for_static_nochunk,
expand_omp_for_static_chunk, expand_omp_for): Use
fd->have_pointer_condtemp instead of fd->lastprivate_conditional to
determine if a special set of API routines are needed and if condtemp
needs to be initialized, while always initialize cond_var if
fd->lastprivate_conditional is non-zero.

--- gcc/gimplify.c.jj   2019-05-29 09:49:20.449598524 +0200
+++ gcc/gimplify.c  2019-05-30 13:34:54.139334279 +0200
@@ -116,6 +116,8 @@ enum gimplify_omp_var_data
   /* Flag for GOVD_LASTPRIVATE: conditional modifier.  */
   GOVD_LASTPRIVATE_CONDITIONAL = 0x80,
 
+  GOVD_CONDTEMP = 0x100,
+
   GOVD_DATA_SHARE_CLASS = (GOVD_SHARED | GOVD_PRIVATE | GOVD_FIRSTPRIVATE
   | GOVD_LASTPRIVATE | GOVD_REDUCTION | GOVD_LINEAR
   | GOVD_LOCAL)
@@ -9527,6 +9529,11 @@ gimplify_adjust_omp_clauses_1 (splay_tre
 code = OMP_CLAUSE_LASTPRIVATE;
   else if (flags & (GOVD_ALIGNED | GOVD_NONTEMPORAL))
 return 0;
+  else if (flags & GOVD_CONDTEMP)
+{
+  code = OMP_CLAUSE__CONDTEMP_;
+  gimple_add_tmp_var (decl);
+}
   else
 gcc_unreachable ();
 
@@ -11523,6 +11530,36 @@ gimplify_omp_for (tree *expr_p, gimple_s
 }
   else
 gimplify_seq_add_stmt (pre_p, gfor);
+
+  if (TREE_CODE (orig_for_stmt) == OMP_FOR)
+{
+  struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp;
+  unsigned lastprivate_conditional = 0;
+  while (ctx
+&& (ctx->region_type == ORT_TARGET_DATA
+|| ctx->region_type == ORT_TASKGROUP))
+   ctx = ctx->outer_context;
+  if (ctx && (ctx->region_type & ORT_PARALLEL) != 0)
+   for (tree c = gimple_omp_for_clauses (gfor);
+c; c = OMP_CLAUSE_CHAIN (c))
+ if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE
+ && OMP_CLAUSE_LASTPRIVATE_CONDITIONAL (c))
+   ++lastprivate_conditional;
+  if (lastprivate_conditional)
+   {
+ struct omp_for_data fd;
+ omp_extract_for_data (gfor, &fd, NULL);
+ tree type = build_array_type_nelts (unsigned_type_for (fd.iter_type),
+ lastprivate_conditional);
+ tree var = create_tmp_var_raw (type);
+ tree c = build_omp_clause (UNKNOWN_LOCATION, OMP_CLAUSE__CONDTEMP_);
+ OMP_CLAUSE_DECL (c) = var;
+ OMP_CLAUSE_CHAIN (c) = gimple_omp_for_clauses (gfor);
+ gimple_omp_for_set_clauses (gfor, c);
+ omp_add_variable (ctx, var, GOVD_CONDTEMP | GOVD_SEEN);
+   }
+}
+
   if (ret != GS_ALL_DONE)
 return GS_ERROR;
   *expr_p = NULL_TREE;
--- gcc/tree-nested.c.jj2019-05-24 23:30:35.923880583 +0200
+++ gcc/tree-nested.c   2019-05-30 14:25:44.542164529 +0200
@@ -1348,6 +1348,7 @@ convert_nonlocal_omp_clauses (tree *pcla
case OMP_CLAUSE_AUTO:
case OMP_CLAUSE_IF_PRESENT:
case OMP_CLAUSE_FINALIZE:
+   case OMP_CLAUSE__CONDTEMP_:
  break;
 
  /* The following clause belongs to the OpenACC cache directive, which
@@ -1369,7 +1370,6 @@ convert_nonlocal_omp_clauses (tree *pcla
 function decomposition happens before that.  */
case OMP_

[PATCH] Fix ICE in ssa_create_duplicates (PR tree-optimization/90671)

[Jakub Jelinek]

Hi!

The following testcase ICEs on the trunk, because both gsi_split_seq_after
and gsi_insert_seq_after have assertions that the split is not after
gsi_end_p iterator or insertion is not after such an iterator.

My understanding of Alex' change is that it wants to hide any added
debug stmts temporarily from create_block_for_threading duplication, so that
it remains just in one of the blocks.  Now, template_last_to_copy is
initialized using last_bb (template_block), if that block is initially
completely empty, template_last_to_copy will be gsi_end_p, gsi_stmt on it
NULL, so I think in that case we can't split any sequence anywhere, we
simply want to hide the whole sequence from the block duplication and put it
in afterwards.

The following patch does that.  Bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

2019-05-30  Jakub Jelinek  

PR tree-optimization/90671
* tree-ssa-threadupdate.c (ssa_create_duplicates): If
template_block used to be empty on the first call, don't use
gsi_split_seq_after and gsi_insert_seq_after, but remember whole
seq with bb_seq and set it with set_bb_seq.

* gcc.dg/torture/pr90671.c: New test.

--- gcc/tree-ssa-threadupdate.c.jj  2019-05-23 12:57:15.522512319 +0200
+++ gcc/tree-ssa-threadupdate.c 2019-05-30 10:15:44.718468219 +0200
@@ -1142,12 +1142,25 @@ ssa_create_duplicates (struct redirectio
   gimple_seq seq = NULL;
   if (gsi_stmt (local_info->template_last_to_copy)
  != gsi_stmt (gsi_last_bb (local_info->template_block)))
-   seq = gsi_split_seq_after (local_info->template_last_to_copy);
+   {
+ if (gsi_end_p (local_info->template_last_to_copy))
+   {
+ seq = bb_seq (local_info->template_block);
+ set_bb_seq (local_info->template_block, NULL);
+   }
+ else
+   seq = gsi_split_seq_after (local_info->template_last_to_copy);
+   }
   create_block_for_threading (local_info->template_block, rd, 0,
  &local_info->duplicate_blocks);
   if (seq)
-   gsi_insert_seq_after (&local_info->template_last_to_copy,
- seq, GSI_SAME_STMT);
+   {
+ if (gsi_end_p (local_info->template_last_to_copy))
+   set_bb_seq (local_info->template_block, seq);
+ else
+   gsi_insert_seq_after (&local_info->template_last_to_copy,
+ seq, GSI_SAME_STMT);
+   }
 
   /* Go ahead and wire up outgoing edges and update PHIs for the duplicate
 block.   */
--- gcc/testsuite/gcc.dg/torture/pr90671.c.jj   2019-05-30 10:20:13.686068207 
+0200
+++ gcc/testsuite/gcc.dg/torture/pr90671.c  2019-05-30 10:19:50.815442342 
+0200
@@ -0,0 +1,16 @@
+/* PR tree-optimization/90671 */
+/* { dg-do compile } */
+/* { dg-additional-options "-w -g" } */
+
+int a;
+
+int
+main ()
+{
+  int b, c;
+  for (c = 0; c < 2; c++)
+while (a)
+  if (b)
+   break;
+  return 0;
+}

Jakub

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Sean Gillespie]

Thank you for the review!

On Thu, May 30, 2019 at 12:38 PM Marek Polacek  wrote:
>
> Thanks for the patch!  Do you have a copyright assignment on file?  Unsure
> if this patch is small enough not to need it.
>

Also unsure, though I assumed that this patch was small enough to not need one.
I'll go ahead and do it regardless.

> > gcc/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  
> >
> >   * doc/invoke.texi: Add new flag -Wglobal-constructors.
> >
> > gcc/c-family/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  
> >
> >   * c.opt: Add new flag -Wglobal-constructors.
> >
> > gcc/cp/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  
> >
> >   * decl.c (expand_static_init): Warn if a thread local or static decl
> >   requires a non-trivial constructor or destructor.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-05-28  Sean Gillespie  
> >
> >   * g++.dg/warn/global-constructors-1.C: New test.
> >   * g++.dg/warn/global-constructors-2.C: New test.
>
> These are fine but please also mention "PR c++/71482" in them.
>

Will fix.

> > ---
> >  gcc/c-family/c.opt|  4 +++
> >  gcc/cp/decl.c | 17 ++---
> >  gcc/doc/invoke.texi   |  7 ++
> >  .../g++.dg/warn/global-constructors-1.C   | 25 +++
> >  .../g++.dg/warn/global-constructors-2.C   | 23 +
> >  5 files changed, 73 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
> >  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C
> >
> > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > index 046d489f7eb..cf62625ca42 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -613,6 +613,10 @@ Wformat-truncation=
> >  C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger 
> > Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO 
> > ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
> >  Warn about calls to snprintf and similar functions that truncate output.
> >
> > +Wglobal-constructors
> > +C++ ObjC++ Var(warn_global_constructors) Warning
> > +Warn when a global declaration requires a constructor to initialize.
> > +
>
> Ok, I agree this shouldn't be turned on by -Wall or -Wextra.

Yeah - the way I use this with clang is to forbid global constructors
with -Werror=global-constructors and -Wglobal-constructors. Since
this is a legitimate language feature I don't think it should be
included in either of those catch-all warning sets.

>
> > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> > index 19d14a6a5e9..c1d66195bd7 100644
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
> >finish_then_clause (if_stmt);
> >finish_if_stmt (if_stmt);
> >  }
> > -  else if (CP_DECL_THREAD_LOCAL_P (decl))
> > -tls_aggregates = tree_cons (init, decl, tls_aggregates);
> >else
> > -static_aggregates = tree_cons (init, decl, static_aggregates);
> > +   {
> > +if (CP_DECL_THREAD_LOCAL_P (decl))
> > +  tls_aggregates = tree_cons (init, decl, tls_aggregates);
> > +else
> > +  static_aggregates = tree_cons (init, decl, static_aggregates);
> > +
> > +if (warn_global_constructors)
>
> This check is not wrong but you can probably drop it.  We use it if the 
> warning
> is expensive, but this doesn't seem to be the case.
>

Will fix.

> > + {
> > +  if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
> > +warning(OPT_Wglobal_constructors, "declaration requires a global 
> > destructor");
> > +  else
> > +warning(OPT_Wglobal_constructors, "declaration requires a global 
> > constructor");
> > + }
> > +   }
>
> The formatting a bit wrong, please use two spaces to indent.  There should be
> a space before a (.  And the lines should not exceed 80 characters.
>
> I think it would be better to use warning_at instead of warning.  As the
> location, you can use 'dloc' defined earlier in the function if you move
> it out of the block.
>

Will fix.

>
> It seems this patch will also warn for
>
> struct A {
>   A() = default;
> };
>
> A a;
>
> which I think we don't want?
>
> I also think you want to add a test using a constexpr constructor.
>
> Marek

You're completely right. I have a modified version of this patch that
expands the test coverage to include these cases while also
handling the general problem of constant and non-constant
initializers. I'll submit it shortly.


Sean Gillespie

Re: [PATCH] rs6000: Add basic support for prefixed and PC-relative addresses

[Michael Meissner]

On Thu, May 30, 2019 at 02:43:49PM -0500, Bill Schmidt wrote:
> On 5/30/19 2:20 PM, Segher Boessenkool wrote:
> > On Wed, May 29, 2019 at 10:49:35PM -0500, Bill Schmidt wrote:
> >>* config/rs6000/predicates.md (pcrel_address): New
> >>define_predicate.
> > Please put that on one line?
> 
> OK.  Emacs in ChangeLog and Fill modes seems to set a line length
> somewhat less than 79.  I generally follow what it tells me, but I can
> fix this.
> >
> >> +  if (LABEL_REF_P (x))
> >> +  output_asm_label (x);
> >> +  else
> >> +  output_addr_const (file, x);
> > Why does LABEL_REF need separate handling here?
> 
> Mike, please respond?

The current code doesn't need special handling.  I don't recall if earlier code
did, or I just assumed output_addr_const did not handle LABEL_REF's.

> > I don't much like penalising scalar single precision float like this.
> > But, this also hurts unaligned lwz...  Do we have statistics on that?
> > Offline, I guess :-)
> 
> Again, I'll defer to Mike on those details (online or offline). ;-)

I have rewritten this section.  In particular rather than basing everything off
of the mode, I have a now have a more targeted approach that knows which
register, mode, and with sign extension requires D/DS/DQ addresses.  Of course
this only works after register allocation.  Before register allocation, we
still have to make a best guess based only on the mode.

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

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Martin Sebor]

On 5/30/19 10:15 AM, Jeff Law wrote:

On 5/30/19 9:34 AM, Martin Sebor wrote:


If the alias oracle can be used to give the same results without
excessive false positives then I think it would be fine to make
use of it.  Is that something you consider a prerequisite for
this change or should I look into it as a followup?

I think we should explore it a bit before making a final decision.  It
may guide us for other work in this space (like detecting escaping
locals).   I think a dirty prototype to see if it's even in the right
ballpark would make sense.


Okay, let me look into it.

Sounds good.  Again, go with a quick prototype to see if it's likely
feasible.  The tests you've added should dramatically help evaluating if
the oracle is up to the task.


So to expand on what I said on the phone when we spoke: the problem
I quickly ran into with the prototype is that I wasn't able to find
a way to identify pointers to alloca/VLA storage.

In the the points-to solution for the pointer being returned they
both have the vars_contains_escaped_heap flag set.  That seems like
an omission that shouldn't be hard to fix, but on its own, I don't
think it would be sufficient.

In the IL a VLA is represented as a pointer to an array, but when
returning a pointer into a VLA (at some offset so it's an SSA_NAME),
the pointer's point-to solution doesn't include the VLA pointer or
(AFAICS) make it possible to tell even that it is a VLA.  For example
here:

  f (int n)
  {
int * p;
int[0:D.1912] * a.1;
sizetype _1;
void * saved_stack.3_3;
sizetype _6;

 [local count: 1073741824]:
saved_stack.3_3 = __builtin_stack_save ();
_1 = (sizetype) n_2(D);
_6 = _1 * 4;
a.1_8 = __builtin_alloca_with_align (_6, 32);
p_9 = a.1_8 + _6;
__builtin_stack_restore (saved_stack.3_3);
return p_9;
  }

p_9's solution's is:

  p_9, points-to vars: { D.1925 } (escaped, escaped heap)

I couldn't find out how to determine that D.1925 is a VLA (or even
what it is).  It's not among the function's local variables that
FOR_EACH_LOCAL_DECL iterates over.

By replacing the VLA in the test case with a call to malloc I get
this for the returned p_7:

  p_7, points-to NULL, points-to vars: { D.1916 } (escaped, escaped heap)

Again, I see no way to quickly tell that this pointer points into
the block returned from malloc.

I'm sure Richard will correct me if there is a simple way to do it
but I couldn't find one.

If there is a way to identify that the returned pointer is for
a VLA (or alloca), for parity with the current patch we also
need to find the location of the VLA declaration or the alloca
call so that the warning could point to it.  Unless there's
a straight path from the mysterious D.1925 above to that
decl/call statement, finding it would likely require some
sor of traversal.  At that point I wouldn't be surprised if
the complexity of the solution didn't approach or even exceed
the current approach.

Martin


FWIW, I'm working on enhancing this to detect returning freed
pointers (under a different option).  That seems like a good
opportunity to also look into making use of the alias oracle.

Yes.  I think there's two interesting cases here to ponder.  If we free
a pointer that must point to a local, then we can warn & trap.  If we
free a pointer that may point to a local, then we can only warn (unless
we can isolate the path).


I wasn't actually thinking of freeing locals but it sounds like
a useful enhancement as well.  Thanks for the suggestion! :)

To be clear, what I'm working on is detecting:

   void* f (void *p)
   {
     free (p);   // note: pointer was freed here
     // ...
     return p;   // warning: returning a freed pointer
   }

Ah, we were talking about different things. Though what you're doing
might be better modeled in a true global static analyzer as a
use-after-free problem.  My sense is that translation-unit local version
of that problem really isn't that useful in practice.  THough I guess
there isn't anything bad with having a TU local version.





Besides these enhancements, there's also a request to diagnose
dereferencing pointers to compound literals whose lifetime has
ended (PR 89990), or more generally, those to any such local
object.  It's about preventing essentially the same problem
(accessing bad data or corrupting others) but it seems that
both the analysis and the handling will be sufficiently
different to consider implementing it somewhere else.  What
are your thoughts on that?

I think the tough problem here is we lose binding scopes as we lower
into gimple, so solving it in the general case may be tough.  We've
started adding some clobbers into the IL to denote object death points,
but I'm not sure if they're sufficient to implement this kind of warning.


I was afraid that might be a problem.

Way back in the early days of tree-ssa we kept the binding scopes.  But
that proved problematical in various ways.  Mostly they just got in the
way of analysis an optimi

Re: [PATCH] PR71482: Add -Wglobal-constructors

[Martin Sebor]

On 5/28/19 3:30 PM, Sean Gillespie wrote:

This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. This new warning fires
whenever a thread_local or global variable is declared whose type has a
non-trivial constructor or destructor. When faced with both a constructor
and a destructor, the error message mentions the destructor and is only
fired once.

This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.

Bootstrapped and regression-tested on x86-64 linux, new tests passing.


I can't tell from the Clang online manual:

Is the warning meant to trigger just for globals, or for all
objects with static storage duration regardless of scope (i.e.,
including namespace-scope objects and static locals and static
class members)?

"Requires a constructor to initialize" doesn't seem very clear
to me.  Is the warning intended to trigger for objects that
require dynamic initialization?  If so, then what about dynamic
intialization of objects of trivial types, such as this:

  static int i = std::string ("foo").length ();

or even

  static int j = strlen (getenv ("TMP"));

If these aren't meant to be diagnosed then the description should
make it clear (the first one involves a ctor, the second one does
not).  But I would think that if the goal is to find sources of
dynamic initialization then diagnosing the above would be useful.
If so, the description should make it clear and tests verifying
that it works should be added.

Martin

PS Dynamic initialization can be optimized into static
initialization, even when it involves a user-defined constructor.
If the purpose of the warning is to find objects that are
dynamically initialized in the sense of the C++ language then
implementing it in the front-end is sufficient.  But if the goal
is to detect constructors that will actually run at runtime (i.e.,
have a startup cost and may have dependencies on one another) then
it needs to be implemented much later.



gcc/ChangeLog:

2019-05-28  Sean Gillespie  

* doc/invoke.texi: Add new flag -Wglobal-constructors.

gcc/c-family/ChangeLog:

2019-05-28  Sean Gillespie  

* c.opt: Add new flag -Wglobal-constructors.

gcc/cp/ChangeLog:

2019-05-28  Sean Gillespie  

* decl.c (expand_static_init): Warn if a thread local or static decl
requires a non-trivial constructor or destructor.

gcc/testsuite/ChangeLog:

2019-05-28  Sean Gillespie  

* g++.dg/warn/global-constructors-1.C: New test.
* g++.dg/warn/global-constructors-2.C: New test.
---
  gcc/c-family/c.opt|  4 +++
  gcc/cp/decl.c | 17 ++---
  gcc/doc/invoke.texi   |  7 ++
  .../g++.dg/warn/global-constructors-1.C   | 25 +++
  .../g++.dg/warn/global-constructors-2.C   | 23 +
  5 files changed, 73 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
  create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..cf62625ca42 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -613,6 +613,10 @@ Wformat-truncation=
  C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) 
Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) 
IntegerRange(0, 2)
  Warn about calls to snprintf and similar functions that truncate output.
  
+Wglobal-constructors

+C++ ObjC++ Var(warn_global_constructors) Warning
+Warn when a global declaration requires a constructor to initialize.
+
  Wif-not-aligned
  C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
  Warn when the field in a struct is not aligned.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 19d14a6a5e9..c1d66195bd7 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
finish_then_clause (if_stmt);
finish_if_stmt (if_stmt);
  }
-  else if (CP_DECL_THREAD_LOCAL_P (decl))
-tls_aggregates = tree_cons (init, decl, tls_aggregates);
else
-static_aggregates = tree_cons (init, decl, static_aggregates);
+   {
+if (CP_DECL_THREAD_LOCAL_P (decl))
+  tls_aggregates = tree_cons (init, decl, tls_aggregates);
+else
+  static_aggregates = tree_cons (init, decl, static_aggregates);
+
+if (warn_global_constructors)
+ {
+  if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
+warning(OPT_Wglobal_constructors, "declaration requires a global 
destructor");
+  else
+warning(OPT_Wglobal_constructors, "declaration requires a global 
constructor");
+ }
+   }
  }
  
  

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4964cc41ba3..a9b2e47a302 100644
--- a/gcc/doc/invoke.te

Re: [PATCH V2] A jump threading opportunity for condition branch

[Jeff Law]

On 5/30/19 9:03 AM, Jiufu Guo wrote:
> Jeff Law  writes:
> 
>> On 5/29/19 6:36 AM, Richard Biener wrote:
>>> On Tue, 28 May 2019, Jiufu Guo wrote:
>>>
 Hi,

 This patch implements a new opportunity of jump threading for PR77820.
 In this optimization, conditional jumps are merged with unconditional
 jump. And then moving CMP result to GPR is eliminated.

 This version is based on the proposal of Richard, Jeff and Andrew, and
 refined to incorporate comments.  Thanks for the reviews!

 Bootstrapped and tested on powerpc64le and powerpc64be with no
 regressions (one case is improved) and new testcases are added. Is this
 ok for trunk?

 Example of this opportunity looks like below:

   
   p0 = a CMP b
   goto ;

   
   p1 = c CMP d
   goto ;

   
   # phi = PHI 
   if (phi != 0) goto ; else goto ;

 Could be transformed to:

   
   p0 = a CMP b
   if (p0 != 0) goto ; else goto ;

   
   p1 = c CMP d
   if (p1 != 0) goto ; else goto ;


 This optimization eliminates:
 1. saving CMP result: p0 = a CMP b.
 2. additional CMP on branch: if (phi != 0).
 3. converting CMP result if there is phi = (INT_CONV) p0 if there is.

 Thanks!
 Jiufu Guo


 [gcc]
 2019-05-28  Jiufu Guo  
Lijia He  

PR tree-optimization/77820
* tree-ssa-threadedge.c
(edge_forwards_cmp_to_conditional_jump_through_empty_bb_p): New
function.
(thread_across_edge): Add call to
edge_forwards_cmp_to_conditional_jump_through_empty_bb_p.

 [gcc/testsuite]
 2019-05-28  Jiufu Guo  
Lijia He  

PR tree-optimization/77820
* gcc.dg/tree-ssa/phi_on_compare-1.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-2.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-3.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-4.c: New testcase.
* gcc.dg/tree-ssa/split-path-6.c: Update testcase.

 ---
  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c | 30 ++
  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c | 23 +++
  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c | 25 
  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c | 40 +
  gcc/testsuite/gcc.dg/tree-ssa/split-path-6.c |  2 +-
  gcc/tree-ssa-threadedge.c| 76 
 +++-
  6 files changed, 192 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c

 diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c 
 b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
 new file mode 100644
 index 000..5227c87
 --- /dev/null
 +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
 @@ -0,0 +1,30 @@
 +/* { dg-do compile } */
 +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
 +
 +void g (int);
 +void g1 (int);
 +
 +void
 +f (long a, long b, long c, long d, long x)
 +{
 +  _Bool t;
 +  if (x)
 +{
 +  g (a + 1);
 +  t = a < b;
 +  c = d + x;
 +}
 +  else
 +{
 +  g (b + 1);
 +  a = c + d;
 +  t = c > d;
 +}
 +
 +  if (t)
 +g1 (c);
 +
 +  g (a);
 +}
 +
 +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } 
 */
 diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c 
 b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
 new file mode 100644
 index 000..eaf89bb
 --- /dev/null
 +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
 @@ -0,0 +1,23 @@
 +/* { dg-do compile } */
 +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
 +
 +void g (void);
 +void g1 (void);
 +
 +void
 +f (long a, long b, long c, long d, int x)
 +{
 +  _Bool t;
 +  if (x)
 +t = c < d;
 +  else
 +t = a < b;
 +
 +  if (t)
 +{
 +  g1 ();
 +  g ();
 +}
 +}
 +
 +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } 
 */
 diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c 
 b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
 new file mode 100644
 index 000..d5a1e0b
 --- /dev/null
 +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
 @@ -0,0 +1,25 @@
 +/* { dg-do compile } */
 +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
 +
 +void g (void);
 +void g1 (void);
 +
 +void
 +f (long a, long b, lo

Re: [PATCH] fix more -Wformat-diag issues

[Jeff Law]

On 5/22/19 10:34 AM, Martin Sebor wrote:
> Incorporating the feedback I got on the -Wformat-diag checker
> provided an opportunity to tighten up existing and implement
> a small number of few additional rules based on GCC Coding
> Conventions (https://gcc.gnu.org/codingconventions.html).
> The checker now also warns for incorrect uses of the following:
> 
>  *  bitfield (rather than bit-field)
>  *  builtin (rather than built-in)
>  *  command line (rather than command-line)
>  *  enumeral (rather than enumerated)
>  *  floating point (rather than floating-point)
>  *  non-zero (rather than nonzero)
> 
> In addition, it has become better at finding unquoted qualifiers
> (like const in const-qualified or "const %qT" rather than % %T%>"), and detects some additional abbreviations (e.g., "stmt"
> instead of "statement").
> 
> These improvements exposed a number of additional issues in our
> sources that the attached patch corrects.
> 
> As before, I have tested the patch on x86_64-linux and adjusted
> the fallout in the test suite.  More cleanup will likely be needed
> on other targets but based on the prior changes I don't expect it
> to be extensive.
> 
> I will post the patch with the checker implementation separately.
> 
> Martin
> 
> PS As discussed in the thread below, some of the instances of
> added hyphenation in "floating-point" aren't strictly necessary
> and the  wording might need to be tweaked a bit to make it so:
>   https://gcc.gnu.org/ml/gcc/2019-05/msg00168.html
> I'll handle it in a subsequent commit if it's viewed important.
> 
> gcc-wformat-diag.diff
> 
> gcc/c/ChangeLog:
> 
>   * c-decl.c (start_decl): Adjust quoting and hyphenation
>   in diagnostics.
>   (finish_decl): Same.
>   (finish_enum): Same.
>   (start_function): Same.
>   (declspecs_add_type): Same.
>   * c-parser.c (warn_for_abs): Same.
>   * c-typeck.c (build_binary_op): Same.
> 
> gcc/c-family/ChangeLog:
> 
>   * c-attribs.c (handle_mode_attribute): Adjust quoting and hyphenation.
>   (handle_alias_ifunc_attribute): Same.
>   (handle_copy_attribute): Same.
>   (handle_weakref_attribute): Same.
>   (handle_nonnull_attribute): Same.
>   * c-warn.c (warn_for_sign_compare): Same.
>   (warn_for_restrict): Same.
>   * c.opt: Same.
> 
>   * c-common.h (GCC_DIAG_STYLE): Adjust.
>(GCC_DIAG_RAW_STYLE): New macro.
> 
> gcc/cp/ChangeLog:
> 
>   * call.c (build_conditional_expr_1): Adjust quoting and hyphenation.
>   (convert_like_real): Same.
>   (convert_arg_to_ellipsis): Same.
>   * constexpr.c (diag_array_subscript): Same.
>   * constraint.cc (diagnose_trait_expression): Same.
>   * cvt.c (ocp_convert): Same.
>   * decl.c (start_decl): Same.
>   (check_for_uninitialized_const_var): Same.
>   (grokfndecl): Same.
>   (check_special_function_return_type): Same.
>   (finish_enum_value_list): Same.
>   (start_preparsed_function): Same.
>   * parser.c (cp_parser_decl_specifier_seq): Same.
>   * typeck.c (cp_build_binary_op): Same.
>   (build_static_cast_1): Same.
> 
>   * cp-tree.h (GCC_DIAG_STYLE): Adjust.
>(GCC_DIAG_RAW_STYLE): New macro.
> 
> gcc/lto/ChangeLog:
> 
>   * lto-common.c (lto_file_finalize): Adjust quoting and hyphenation.
> 
> gcc/objc/ChangeLog:
> 
>   * objc-act.c (objc_build_setter_call): Adjust quoting and hyphenation.
>   * objc-encoding.c (encode_gnu_bitfield): Same.
> 
> gcc/ChangeLog:
> 
>   * config/i386/i386-features.c (ix86_get_function_versions_dispatcher):
>   Adjust quoting and hyphenation.
>   * convert.c (convert_to_real_1): Same.
>   * gcc.c (driver_wrong_lang_callback): Same.
>   (driver::handle_unrecognized_options): Same.
>   * gimple-ssa-nonnull-compare.c (do_warn_nonnull_compare): Same.
>   * opts-common.c (cmdline_handle_error): Same.
>   (read_cmdline_option): Same.
>   * opts-global.c (complain_wrong_lang): Same.
>   (print_ignored_options): Same.
>   (handle_common_deferred_options): Same.
>   * pretty-print.h: Same.
>   * print-rtl.c (debug_bb_n_slim): Same.
>   * sched-rgn.c (make_pass_sched_fusion): Same.
>   * tree-cfg.c (verify_gimple_assign_unary): Same.
>   (verify_gimple_label): Same.
>   * tree-ssa-operands.c (verify_ssa_operands): Same.
>   * varasm.c (do_assemble_alias): Same.
>   (assemble_alias): Same.
> 
>   * diagnostic-core.h (GCC_DIAG_STYLE): Adjust.
>(GCC_DIAG_RAW_STYLE): New macro.
> 
>   * cfghooks.c: Disable -Wformat-diags.
>   * cfgloop.c: Same.
>   * cfgrtl.c: Same.
>   * cgraph.c: Same.
>   * diagnostic-show-locus.c: Same.
>   * diagnostic.c: Same.
>   * gimple-pretty-print.c: Same.
>   * graph.c: Same.
>   * symtab.c: Same.
>   * tree-eh.c Same.
>   * tree-pretty-print.c: Same.
>   * tree-ssa.c: Same.
> 
>   * configure: Regenerate.
>   * configure.ac (ACX_PROG_CXX_WARNING_O

Re: value_range_base::{non_zero_p, set_zero, set_non_zero}

[Jeff Law]

On 5/30/19 12:58 AM, Aldy Hernandez wrote:
> Hi.
> 
> We have zero_p in the API, but we don't have non_zero_p.  Instead we use
> a non-API function range_is_nonnull.  I've fixed this.
> 
> I have also gotten rid of the duplicity of using the non-API
> range_is_null in favor of value_range_base::zero_p().
> 
> Furthermore, there's value_range*::set_null and
> value_range*::set_nonnull().  It's inconsistent to use null/nonnull as
> well as zero/non_zero throughout.  I've moved everything to *zero.
> 
> Finally, it seems to me that the derived value_range versions of
> set_*zero/null are a bit confusing in that they clear equivalences
> behind the scenes.  There's no intuitive reason why setting a range of
> [0,0] versus [5,10] should clear equivalences.  I've made the
> equivalence nuking explicit in the handful of places where we do this,
> and thus reduced the need for separate value_range versions.
> 
> I believe with these changes, as well as the pending intersect patch,
> we've cleaned up the remaining value_range uses where we actually wanted
> to use value_range_base.  Or at least the remaining "value_range tem"
> business.
> 
> OK?
> 
> Aldy
> 
> curr.patch
> 
> commit 55294d340a0727dbe985ee4bf3c1969a19bcbe6d
> Author: Aldy Hernandez 
> Date:   Tue May 28 19:30:31 2019 +0200
> 
> * tree-vrp.h (value_range_base::non_zero_p): New.
> (value_range_base::set_nonnull): Rename to...
> (value_range_base::set_non_zero): ...this.
> (value_range_base::set_null): Rename to...
> (value_range_base::set_zero): ...this.
> (value_range::set_nonnull): Remove.
> (value_range::set_null): Remove.
> * tree-vrp.c (range_is_null): Remove.
> (range_is_nonnull): Remove.
> (extract_range_from_binary_expr): Use value_range_base::*zero_p
> instead of range_is_*null.
> (extract_range_from_unary_expr): Same.
> (value_range_base::set_nonnull): Rename to...
> (value_range_base::set_non_zero): ...this.
> (value_range::set_nonnull): Remove.
> (value_range_base::set_null): Rename to...
> (value_range_base::set_zero): ...this.
> (value_range::set_null): Remove.
> (extract_range_from_binary_expr): Rename set_*null uses to
> set_*zero.
> (extract_range_from_unary_expr): Same.
> (union_helper): Same.
> * vr-values.c (get_value_range): Use set_*zero instead of
> set_*null.
> (vr_values::extract_range_from_binary_expr): Same.
> (vr_values::extract_range_basic): Same.
> 
OK
jeff

Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git

[Joseph Myers]

On Wed, 29 May 2019, Segher Boessenkool wrote:

> On Wed, May 29, 2019 at 12:53:30AM +, Joseph Myers wrote:
> > On Fri, 24 May 2019, Segher Boessenkool wrote:
> > 
> > > IMO the best we can do is use what we already have: what CVS or SVN used
> > > as the committer identity.  *That* info is *correct* at least.
> > 
> > CVS and SVN have a local identity.  git has a global identity.  I consider 
> 
> Git has an identity (well, two) _per commit_, and there is no way you can
> reconstruct people's prefered name and email address (at any point in time,
> for every commit separately) correctly.  IMO it is much better to not even
> try.  We already *have* enough info for anyone to trivially look up who wrote
> what, and what might be that person's email address at the time.  But
> pretending that is more than a guess is just wrong.

I think not doing a best-effort identification (name+email) is just as 
wrong as converting a CVS repository to a changeset-based system without 
doing a best-effort unification of commits to different files around the 
same time with the same log message into changesets.  Both are the same 
sort of heuristic conversion of data to the form idiomatic for a different 
version control system based around different concepts.  Neither is 
perfect, but the most useful conversion tries to combine CVS commits to 
different files into changesets, and the most useful conversion tries to 
identify authors in the way idiomatic for git using the information we 
have about what person (globally) a given username on a given system 
corresponds to.

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

Re: [PATCH] warn on returning alloca and VLA (PR 71924, 90549)

[Jeff Law]

On 5/30/19 11:23 AM, Jason Merrill wrote:
> On Thu, May 30, 2019 at 12:16 PM Jeff Law  wrote:
>>
>> On 5/30/19 9:34 AM, Martin Sebor wrote:
>>
> If the alias oracle can be used to give the same results without
> excessive false positives then I think it would be fine to make
> use of it.  Is that something you consider a prerequisite for
> this change or should I look into it as a followup?
 I think we should explore it a bit before making a final decision.  It
 may guide us for other work in this space (like detecting escaping
 locals).   I think a dirty prototype to see if it's even in the right
 ballpark would make sense.
>>>
>>> Okay, let me look into it.
>> Sounds good.  Again, go with a quick prototype to see if it's likely
>> feasible.  The tests you've added should dramatically help evaluating if
>> the oracle is up to the task.
>>
>>>
> FWIW, I'm working on enhancing this to detect returning freed
> pointers (under a different option).  That seems like a good
> opportunity to also look into making use of the alias oracle.
 Yes.  I think there's two interesting cases here to ponder.  If we free
 a pointer that must point to a local, then we can warn & trap.  If we
 free a pointer that may point to a local, then we can only warn (unless
 we can isolate the path).
>>>
>>> I wasn't actually thinking of freeing locals but it sounds like
>>> a useful enhancement as well.  Thanks for the suggestion! :)
>>>
>>> To be clear, what I'm working on is detecting:
>>>
>>>   void* f (void *p)
>>>   {
>>> free (p);   // note: pointer was freed here
>>> // ...
>>> return p;   // warning: returning a freed pointer
>>>   }
>> Ah, we were talking about different things. Though what you're doing
>> might be better modeled in a true global static analyzer as a
>> use-after-free problem.  My sense is that translation-unit local version
>> of that problem really isn't that useful in practice.  THough I guess
>> there isn't anything bad with having a TU local version.
>>
>>
>>>
> Besides these enhancements, there's also a request to diagnose
> dereferencing pointers to compound literals whose lifetime has
> ended (PR 89990), or more generally, those to any such local
> object.  It's about preventing essentially the same problem
> (accessing bad data or corrupting others) but it seems that
> both the analysis and the handling will be sufficiently
> different to consider implementing it somewhere else.  What
> are your thoughts on that?
 I think the tough problem here is we lose binding scopes as we lower
 into gimple, so solving it in the general case may be tough.  We've
 started adding some clobbers into the IL to denote object death points,
 but I'm not sure if they're sufficient to implement this kind of warning.
>>>
>>> I was afraid that might be a problem.
>> Way back in the early days of tree-ssa we kept the binding scopes.  But
>> that proved problematical in various ways.  Mostly they just got in the
>> way of analysis an optimization and we spent far too much time in the
>> optimizers working around them or removing those which were empty.
>>
>> They'd be helpful in this kind of analysis, stack slot sharing vs the
>> inliner and a couple other problems.  I don't know if the pendulum has
>> moved far enough to revisit :-)
> 
> Why wouldn't clobbers be sufficient?
I haven't looked into the clobbers in any detail.  If we're aggressively
emitting them at the end of object life, then they may be sufficient to
start tackling these issues.

jeff

Re: [PATCH] RX: Add rx-*-linux target

[Jeff Law]

On 5/29/19 12:27 PM, Jeff Law wrote:
> On 5/23/19 6:05 AM, Yoshinori Sato wrote:
>> I ported linux kernel to Renesas RX.
>>
>> rx-*-elf target output a binary different from the standard ELF.
>> It has the same format as the Renesas compiler.
>>
>> But the linux kernel requires the standard ELF format.
>> I want to define a rx-*-linux target so that I can generate
>> a standard ELF binary.
> Presumably you're resubmitting after your assignment got recorded (I
> think I saw that fly by recently).
> 
> I'll construct a ChangeLog and install this on the trunk.
So this is causing libgcc to fail to build for rx-elf.  The problem is
the DF=SF #define.  I think you need so split that out so that it's only
used for rx-linux.

Jeff

Re: value_range_base::{non_zero_p, set_zero, set_non_zero}

[Martin Sebor]

On 5/30/19 12:58 AM, Aldy Hernandez wrote:

Hi.

We have zero_p in the API, but we don't have non_zero_p.  Instead we use 
a non-API function range_is_nonnull.  I've fixed this.


I have also gotten rid of the duplicity of using the non-API 
range_is_null in favor of value_range_base::zero_p().


Furthermore, there's value_range*::set_null and 
value_range*::set_nonnull().  It's inconsistent to use null/nonnull as 
well as zero/non_zero throughout.  I've moved everything to *zero.


With the -Wformat-diag cleanup still fresh in my memory, I can't
help but point out that the GCC spelling convention calls for
"nonzero" vs "non-zero" or "non zero".

Naming the function set_nonzero() would be in line with both
the convention and established practice (over 2000 instances)
and set_non_zero would not be (only 22 instances of non_zero
in GCC sources).

This, of course, is in contrast to things like bit-field and
built-in where the convention calls for the hyphen but where
in code we seem to prefer "bitfield" nonetheless ;-) (Names
like get_bit_field_ref_def and bit_field_size being
the exceptions).

Martin



Finally, it seems to me that the derived value_range versions of 
set_*zero/null are a bit confusing in that they clear equivalences 
behind the scenes.  There's no intuitive reason why setting a range of 
[0,0] versus [5,10] should clear equivalences.  I've made the 
equivalence nuking explicit in the handful of places where we do this, 
and thus reduced the need for separate value_range versions.


I believe with these changes, as well as the pending intersect patch, 
we've cleaned up the remaining value_range uses where we actually wanted 
to use value_range_base.  Or at least the remaining "value_range tem" 
business.


OK?

Aldy

Re: undefined behavior in value_range::equiv_add()?

[Jeff Law]

On 5/29/19 10:20 AM, Aldy Hernandez wrote:
> On 5/29/19 12:12 PM, Jeff Law wrote:
>> On 5/29/19 9:58 AM, Aldy Hernandez wrote:
>>> On 5/29/19 9:24 AM, Richard Biener wrote:
 On Wed, May 29, 2019 at 2:18 PM Aldy Hernandez 
 wrote:
>
> As per the API, and the original documentation to value_range,
> VR_UNDEFINED and VR_VARYING should never have equivalences.  However,
> equiv_add is tacking on equivalences blindly, and there are various
> regressions that happen if I fix this oversight.
>
> void
> value_range::equiv_add (const_tree var,
>   const value_range *var_vr,
>   bitmap_obstack *obstack)
> {
>  if (!m_equiv)
>    m_equiv = BITMAP_ALLOC (obstack);
>  unsigned ver = SSA_NAME_VERSION (var);
>  bitmap_set_bit (m_equiv, ver);
>  if (var_vr && var_vr->m_equiv)
>    bitmap_ior_into (m_equiv, var_vr->m_equiv);
> }
>
> Is this a bug in the documentation / API, or is equiv_add incorrect
> and
> we should fix the fall-out elsewhere?

 I think this must have been crept in during the classification.  If you
 go back to say GCC 7 you shouldn't see value-ranges with
 UNDEFINED/VARYING state in the lattice that have equivalences.

 It may not be easy to avoid with the new classy interface but we're
 certainly not tacking on them "blindly".  At least we're not supposed
 to.  As usual the intermediate state might be "broken" but
 intermediateness is not sth the new class "likes".
>>>
>>> It looks like extract_range_from_stmt (by virtue of
>>> vrp_visit_assignment_or_call and then extract_range_from_ssa_name)
>>> returns one of these intermediate ranges.  It would seem to me that an
>>> outward looking API method like vr_values::extract_range_from_stmt
>>> shouldn't be returning inconsistent ranges.  Or are there no guarantees
>>> for value_ranges from within all of vr_values?
>> ISTM that if we have an implementation constraint that says a VR_VARYING
>> or VR_UNDEFINED range can't have equivalences, then we need to honor
>> that at the minimum for anything returned by an external API.  Returning
>> an inconsistent state is bad.  I'd even state that we should try damn
>> hard to avoid it in internal APIs as well.
> 
> Agreed * 2.
> 
>>
>>>
>>> Perhaps I should give a little background.  As part of your
>>> value_range_base re-factoring last year, you mentioned that you didn't
>>> split out intersect like you did union because of time or oversight.  I
>>> have code to implement intersect (attached), for which I've noticed that
>>> I must leave equivalences intact, even when transitioning to
>>> VR_UNDEFINED:
>>>
>>> [from the attached patch]
>>> +  /* If THIS is varying we want to pick up equivalences from OTHER.
>>> + Just special-case this here rather than trying to fixup after the
>>> + fact.  */
>>> +  if (this->varying_p ())
>>> +    this->deep_copy (other);
>>> +  else if (this->undefined_p ())
>>> +    /* ?? Leave any equivalences already present in an undefined.
>>> +   This is technically not allowed, but we may get an in-flight
>>> +   value_range in an intermediate state.  */
>> Where/when does this happen?
> 
> The above snippet is not currently in mainline.  It's in the patch I'm
> proposing to clean up intersect.  It's just that while cleaning up
> intersect I noticed that if we keep to the value_range API, we end up
> clobbering an equivalence to a VR_UNDEFINED that we depend up further up
> the call chain.
> 
> The reason it doesn't happen in mainline is because intersect_helper
> bails early on an undefined, thus leaving the problematic equivalence
> intact.
> 
> You can see it in mainline though, with the following testcase:
> 
> int f(int x)
> {
>   if (x != 0 && x != 1)
>     return -2;
> 
>   return !x;
> }
> 
> Break in evrp_range_analyzer::record_ranges_from_stmt() and see that the
> call to extract_range_from_stmt() returns a VR of undefined *WITH*
> equivalences:
> 
>   vr_values->extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
> 
> This VR is later fed to update_value_range() and ultimately intersect(),
> which in mainline, leaves the equivalences alone, but IMO should respect
> that API and nuke them.
So I think this is coming from extract_range_from_ssa_name:

> void
> vr_values::extract_range_from_ssa_name (value_range *vr, tree var)
> {
>   value_range *var_vr = get_value_range (var);
> 
>   if (!var_vr->varying_p ())
> vr->deep_copy (var_vr);
>   else
> vr->set (var);
> 
>   vr->equiv_add (var, get_value_range (var), &vrp_equiv_obstack);
> }

Where we blindly add VAR to the equiv bitmap in the range.

AFAICT gcc-7 has equivalent code, it's just not inside the class.

I don't know what the fallout might be, but we could conditionalize the
call to add_equivalence to avoid the inconsistent state.

Jeff

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Joseph Myers]

On Thu, 30 May 2019, Alex Henrie wrote:

> At this point I think I'm convinced that any attribute that applies to
> a function should also be allowed on a function pointer without any
> warnings. We can start by turning off the warnings for the "fndecl"
> attributes and then clean up the other attributes as time goes on.

This is inherently a property of the attribute in question.  The issue is 
not whether it applies to function pointers; it's whether it applies to 
function types.

For example, the "section" or "alias" attributes are attributes that apply 
to a declaration, but not a type.  Because they apply to variables as well 
as functions, they are meaningful on function pointers - but the meaning 
is *not* the same as applying them to the pointed-to function.

The "flatten" attribute, however, seems only meaningful for functions, not 
variables, not function types and not function pointers.

We should try to work out for each attribute exactly what construct it 
appertains to - which for many but not all function attributes is indeed 
the type of the function rather than the function itself.  Then move to 
making such attributes work on types.  But for attributes such as 
"flatten" that logically appertain to the declaration not its type, we 
should continue to diagnose them on function pointers or types.

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

Re: CFV: "C++ programming in GNU" interest group (possibly including coding standard development)

[Joseph Myers]

On Thu, 30 May 2019, Mike Stump wrote:

> On May 30, 2019, at 10:55 AM, James Youngman  wrote:
> > I'm looking into creating a C++ coding standard for GNU programs.
> 
> So, parts of how we do C++ isn't really relevant to a larger community.  
> We at times cater to a C legacy and in a pure C++ project, there may be 
> no need to do this.

And on the other side of things, libstdc++ surely does plenty of things 
that would not be appropriate for most C++ projects that are not the 
standard library implementation - so it should be clear that many coding 
standards are not expected to apply in that context.

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

Re: value_range_base::{non_zero_p, set_zero, set_non_zero}

[Joseph Myers]

On Thu, 30 May 2019, Martin Sebor wrote:

> This, of course, is in contrast to things like bit-field and
> built-in where the convention calls for the hyphen but where

For both bit-field and nonzero what we do in documentation is consistent 
with the C standard, even if code is less consistent.  (grep on the C 
standard sources shows 148 lines matching for nonzero and only 2 for 
non-zero.)

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

Re: [PATCH,libstdc++] C++-20 costexpr and

[Ed Smith-Rowland via gcc-patches]

On 5/30/19 5:05 PM, Ed Smith-Rowland via libstdc++ wrote:

Greetings,

I was not quite able to finish this in for gcc9 but here is the patch 
for:


?? Implement C++20 p0202 - Add Constexpr Modifiers to Functions
 ??in  and  Headers.
 ??Implement C++20 p1023 - constexpr comparison operators for 
std::array.


I believe I have answered peoples concerns with the last patch 
attempts [https://gcc.gnu.org/ml/libstdc++/2019-03/msg00132.html].


The patch is large because of test cases but really just boils down to 
adding constexpr for c++2a.


The patch passes for gnu++2a and pre-gnu++2a on x86_64-linux.

OK for trunk?

Ed Smith-Rowland


Actually, I got 6 excess error fails in C++20 (I keep forgetting to 
actually test that option, sorry):


FAIL: 25_algorithms/copy/58982.cc (test for excess errors)
FAIL: 25_algorithms/copy/move_iterators/69478.cc (test for excess errors)
FAIL: 25_algorithms/copy_backward/move_iterators/69478.cc (test for 
excess errors)

FAIL: 25_algorithms/copy_n/58982.cc (test for excess errors)
FAIL: 25_algorithms/move/69478.cc (test for excess errors)
FAIL: 25_algorithms/move_backward/69478.cc (test for excess errors)

These excess errors are all 'error: use of deleted function' for copy 
assignments - which is true.?? I just had the wrong prune string. My 
deja-gnu is wobbly.


The guts of the previous patch are still unchanged but here is a new 
patch with the fixed testcases.


This passes C++20 and earlier.

Sorry for the noise.

OK?

Ed


2019-05-31  Edward Smith-Rowland  <3dw...@verizon.net>

Implement C++20 p0202 - Add Constexpr Modifiers to Functions
in  and  Headers.
Implement C++20 p1023 - constexpr comparison operators for std::array.
* include/bits/algorithmfwd.h (all_of, any_of, binary_search, copy,
copy_backward, copy_if, copy_n, equal_range, fill, fill_n, find_end,
find_if_not, includes, is_heap, is_heap_until, is_partitioned,
is_permutation, is_sorted, is_sorted_until, lower_bound, none_of,
partition_copy, partition_point, remove, remove_if, remove_copy,
remove_copy_if, replace_copy, replace_copy_if, reverse_copy,
rotate_copy, unique, upper_bound, adjacent_find, count, count_if, equal,
find, find_first_of, find_if, for_each, generate, generate_n,
lexicographical_compare, merge, mismatch, replace, replace_if, search,
search_n, set_difference, set_intersection, set_symmetric_difference,
set_union, transform, unique_copy): Mark constexpr.
Make versions of operator() const.
* include/bits/cpp_type_traits.h (__miter_base): Make constexpr.
* include/bits/predefined_ops.h (_Iter_less_val, __iter_comp_val,
_Val_less_iter, __val_less_iter, __val_comp_iter, _Iter_equal_to_iter,
__iter_equal_to_iter, _Iter_equal_to_val, __iter_equal_to_val,
_Iter_comp_val, __iter_comp_val, _Val_comp_iter, __val_comp_iter,
_Iter_equals_val, __iter_equals_val, _Iter_equals_iter,
__iter_comp_iter, __pred_iter, __iter_comp_val, __negate): Constexpr
ctors. Add const operator().
* include/bits/stl_algo.h (__find_if, __find_if_not, __find_if_not_n,
__search, __search_n_aux, __search_n, __find_end, find_end, all_of
none_of, any_of, find_if_not, is_partitioned, partition_point,
__remove_copy_if, remove_copy, remove_copy_if, copy_if, __copy_n,
copy_n, partition_copy, __remove_if, remove, remove_if, __adjacent_find,
__unique, unique, __unique_copy, reverse_copy, rotate_copy,
__unguarded_linear_insert, __insertion_sort, __unguarded_insertion_sort,
__final_insertion_sort, lower_bound, __upper_bound, upper_bound,
__equal_range, equal_range, binary_search, __includes, includes,
__next_permutation, __prev_permutation, __replace_copy_if, replace_copy,
replace_copy_if, __count_if, is_sorted, __is_sorted_until,
is_sorted_until, __is_permutation, is_permutation, for_each, find,
find_if, find_first_of, adjacent_find, count, count_if, search,
search_n, transform, replace, replace_if, generate, generate_n,
unique_copy, __merge, merge, __set_union, set_union, __set_intersection,
set_intersection, __set_difference, set_difference,
__set_symmetric_difference, set_symmetric_difference): Constexpr.
* include/bits/stl_algobase.h (__niter_base, __niter_wrap, __copy_m,
__copy_move_a, copy, move, __copy_move_backward::__copy_move_b,
__copy_move_backward_a, __copy_move_backward_a2, copy_backward,
move_backward, fill, __fill_a, __fill_n_a, fill_n, __equal::equal,
__equal_aux, __newlast1, __cnd2, __newlast1, __cnd2,
__lexicographical_compare_impl, __lexicographical_compare::__lc,
__lexicographical_compare, __lexicographical_compare_aux,
__lower_bound, lower_bound, equal, __equal4, lexicographical_compare,
__mismatch, mismatch): Constexp

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Alex Henrie]

On Thu, May 30, 2019 at 6:59 PM Joseph Myers  wrote:
>
> On Thu, 30 May 2019, Alex Henrie wrote:
>
> > At this point I think I'm convinced that any attribute that applies to
> > a function should also be allowed on a function pointer without any
> > warnings. We can start by turning off the warnings for the "fndecl"
> > attributes and then clean up the other attributes as time goes on.
>
> This is inherently a property of the attribute in question.  The issue is
> not whether it applies to function pointers; it's whether it applies to
> function types.
>
> For example, the "section" or "alias" attributes are attributes that apply
> to a declaration, but not a type.  Because they apply to variables as well
> as functions, they are meaningful on function pointers - but the meaning
> is *not* the same as applying them to the pointed-to function.
>
> The "flatten" attribute, however, seems only meaningful for functions, not
> variables, not function types and not function pointers.
>
> We should try to work out for each attribute exactly what construct it
> appertains to - which for many but not all function attributes is indeed
> the type of the function rather than the function itself.  Then move to
> making such attributes work on types.  But for attributes such as
> "flatten" that logically appertain to the declaration not its type, we
> should continue to diagnose them on function pointers or types.

In Wine we need a way to (without warnings) put ms_hook_prologue into
a macro that is applied to functions, function pointers, and function
pointer typedefs. It sounds like you're saying that you will not
accept a patch that silences or splits off warnings about using
ms_hook_prologue with function pointers and function pointer typedefs.
So how do you think Wine's problem should be solved?

It seems to me that any information about the target of a function
pointer, even the flatten attribute or the ms_hook_prologue attribute,
provides information that could be useful for optimizing the code
around the indirect function call. That sounds like a compelling
argument for allowing these attributes in more places without
warnings.

-Alex

Re: [RFC][PR88838][SVE] Use 32-bit WHILELO in LP64 mode

[Kugan Vivekanandarajah]

Hi Richard,

Thanks for the review.

On Tue, 28 May 2019 at 20:44, Richard Sandiford
 wrote:
>
> Kugan Vivekanandarajah  writes:
> > [...]
> > diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
> > index b3fae5b..c15b8a2 100644
> > --- a/gcc/tree-vect-loop-manip.c
> > +++ b/gcc/tree-vect-loop-manip.c
> > @@ -415,10 +415,16 @@ vect_set_loop_masks_directly (struct loop *loop, 
> > loop_vec_info loop_vinfo,
> > bool might_wrap_p)
> >  {
> >tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo);
> > +  tree iv_type = LOOP_VINFO_MASK_IV_TYPE (loop_vinfo);
> >tree mask_type = rgm->mask_type;
> >unsigned int nscalars_per_iter = rgm->max_nscalars_per_iter;
> >poly_uint64 nscalars_per_mask = TYPE_VECTOR_SUBPARTS (mask_type);
> > +  bool convert = false;
> >
> > +  /* If the compare_type is not iv_type, we will create an IV with
> > + iv_type with truncated use (i.e. converted to the correct type).  */
> > +  if (compare_type != iv_type)
> > +convert = true;
> >/* Calculate the maximum number of scalar values that the rgroup
> >   handles in total, the number that it handles for each iteration
> >   of the vector loop, and the number that it should skip during the
> > @@ -444,12 +450,43 @@ vect_set_loop_masks_directly (struct loop *loop, 
> > loop_vec_info loop_vinfo,
> >   processed.  */
> >tree index_before_incr, index_after_incr;
> >gimple_stmt_iterator incr_gsi;
> > +  gimple_stmt_iterator incr_gsi2;
> >bool insert_after;
> > -  tree zero_index = build_int_cst (compare_type, 0);
> > +  tree zero_index;
> >standard_iv_increment_position (loop, &incr_gsi, &insert_after);
> > -  create_iv (zero_index, nscalars_step, NULL_TREE, loop, &incr_gsi,
> > -  insert_after, &index_before_incr, &index_after_incr);
> >
> > +  if (convert)
> > +{
> > +  /* If we are creating IV of iv_type and then converting.  */
> > +  zero_index = build_int_cst (iv_type, 0);
> > +  tree step = build_int_cst (iv_type,
> > +  LOOP_VINFO_VECT_FACTOR (loop_vinfo));
> > +  /* Creating IV of iv_type.  */
> > +  create_iv (zero_index, step, NULL_TREE, loop, &incr_gsi,
> > +  insert_after, &index_before_incr, &index_after_incr);
> > +  /* Create truncated index_before and after increament.  */
> > +  tree index_before_incr_trunc = make_ssa_name (compare_type);
> > +  tree index_after_incr_trunc = make_ssa_name (compare_type);
> > +  gimple *incr_before_stmt = gimple_build_assign 
> > (index_before_incr_trunc,
> > +   NOP_EXPR,
> > +   index_before_incr);
> > +  gimple *incr_after_stmt = gimple_build_assign 
> > (index_after_incr_trunc,
> > +  NOP_EXPR,
> > +  index_after_incr);
> > +  incr_gsi2 = incr_gsi;
> > +  gsi_insert_before (&incr_gsi2, incr_before_stmt, GSI_NEW_STMT);
> > +  gsi_insert_after (&incr_gsi, incr_after_stmt, GSI_NEW_STMT);
> > +  index_before_incr = index_before_incr_trunc;
> > +  index_after_incr = index_after_incr_trunc;
> > +  zero_index = build_int_cst (compare_type, 0);
> > +}
> > +  else
> > +{
> > +  /* If the IV is of compare_type, no convertion needed.  */
> > +  zero_index = build_int_cst (compare_type, 0);
> > +  create_iv (zero_index, nscalars_step, NULL_TREE, loop, &incr_gsi,
> > +  insert_after, &index_before_incr, &index_after_incr);
> > +}
> >tree test_index, test_limit, first_limit;
> >gimple_stmt_iterator *test_gsi;
> >if (might_wrap_p)
>
> Now that we have an explicit iv_type, there shouldn't be any need to
> treat this as two special cases.  I think we should just convert the
> IV to the comparison type before passing it to the WHILE.

Changed it.
>
> > @@ -617,6 +654,41 @@ vect_set_loop_masks_directly (struct loop *loop, 
> > loop_vec_info loop_vinfo,
> >return next_mask;
> >  }
> >
> > +/* Return the iv_limit for fully masked loop LOOP with LOOP_VINFO.
> > +   If it is not possible to calcilate iv_limit, return -1.  */
>
> Maybe:
>
> /* Decide whether it is possible to use a zero-based induction variable
>when vectorizing LOOP_VINFO with a fully-masked loop.  If it is,
>return the value that the induction variable must be able to hold
>in order to ensure that the loop ends with an all-false mask.
>Return -1 otherwise.  */
>
> I think the function should go on in tree-vect-loop.c instead.

OK.
>
> > +widest_int
> > +vect_get_loop_iv_limit (struct loop *loop, loop_vec_info loop_vinfo)
>
> Maybe: vect_iv_limit_for_full_masking
>
> Probably worth dropping the "loop" parameter and getting it from
> LOOP_VINFO.
OK.

>
> > +
> > +  /* Convert skip_niters to the right type.  */
>
> Comment no longer applies.
>
> > +  tree niters_skip = LOOP_

Re: [PATCH] PR libstdc++/71579 assert that type traits are not misused with an incomplete type

[Antony Polukhin]

On Thu, May 30, 2019, 19:38 Jonathan Wakely  wrote:
<...>

> I've attached a relative diff, to be applied on top of yours, with my
> suggested tweaks. Do you see any issues with it?
>
> (If you're happy with those tweaks I can go ahead and apply this,
> there's no need for an updated patch from you).
>

Looks good. Please apply!

>