Re: [PATCH] PR fortran/87993 -- An array can have a kind type inquiry suffix

[Janne Blomqvist]

On Tue, Aug 13, 2019 at 3:35 AM Steve Kargl
 wrote:
>
> The attached patch ahs been regression tested on x86_64-*-freebsd.
> It probably borders on obvious, but I'll ask away.  OK to commit?
>
> 2019-08-12  Steven G. Kargl  
>
> PR fortran/87993
> * expr.c (gfc_simplify_expr):  Simplifcation of an array with a kind
> type inquiry suffix yields a constant expression.
>
> 2019-08-12  Steven G. Kargl  
>
> PR fortran/87993
> * gfortran.dg/pr87993.f90: New test.

Ok.


-- 
Janne Blomqvist

Re: [PATCH] PR fortran/89647 -- Allow host associated procedure to be a binding target

[Janne Blomqvist]

On Tue, Aug 13, 2019 at 1:32 AM Steve Kargl
 wrote:
>
> The attached patch fixes PR fortran/89647, and has been
> regression tested on x86_64-*-freebsd.
>
> If a procedure is made accessible by host association, the
> that procedure can be used as binding target.  During the
> resolution of a type bound procedure, gfortran needs to
> check the parent namespace for a procedure before it fails.
>
> 2019-08-12  Steven G. Kargl  
>
> PF fortran/89647
> resolve.c (resolve_typebound_procedure): Allow host associated
> procedure to be a binding target.  While here, wrap long line.
>
> 2019-08-12  Steven G. Kargl  
>
> PF fortran/89647
> * gfortran.dg/pr89647.f90: New test.
>
> OK to commit?


Ok, thanks.


-- 
Janne Blomqvist

Re: [Patch v2] Enable math functions linking with static library for LTO

[luoxhu]

On 2019/8/13 10:22, luoxhu wrote:
diff --git a/gcc/testsuite/gcc.dg/pr91287.c 
b/gcc/testsuite/gcc.dg/pr91287.c

new file mode 100644
index 000..c816e0537aa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91287.c
@@ -0,0 +1,40 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */


You don't use -flto here so the testcase doesn't exercise any of the patched
code.  Does it work when you add -flto here?  That is, do
scan-symbol[-not] properly use gcc-nm or the linker plugin?


-flto is needed here to check this patch correctness, my mistake here, 
thanks for catching.  atan2 will exists in pr91287.o even without lto as

pr91287.o has the instruction "bl atan2".
After adding -flto the case also works as symbol is written to pr91287.o.
PS: update other changes in patch attached.

What's more, this test case depends on this patch 
https://www.sourceware.org/ml/binutils/2019-08/msg00113.html.

otherwise nm will report error (use plugin or local gcc-nm is OK):

PASS: gcc.dg/pr91287.c (test for excess errors)
ERROR: gcc.dg/pr91287.c: error executing dg-final: /usr/bin/nm: pr91287.o: 
plugin needed to handle lto object
UNRESOLVED: gcc.dg/pr91287.c: error executing dg-final: /usr/bin/nm: 
pr91287.o: plugin needed to handle lto object


Xionghu

[PATCH, i386]: Add missing *mmx_pinsr{q,d} patterns

[Uros Bizjak]

We can implement these for TARGET_MMX_WITH_SSE and TARGET_SSE4_1.

2019-08-13  Uroš Bizjak  

* config/i386/i386.md (ix86_expand_vector_set) :
Use vec_merge path for TARGET_MMX_WITH_SSE && TARGET_SSE4_1.
: Ditto.
* config/i386/mmx.md (*mmx_pinsrd): New insn pattern.
(*mmx_pinsrb): Ditto.

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

Committed to mainline SVN.

Uros.
Index: config/i386/i386-expand.c
===
--- config/i386/i386-expand.c   (revision 274317)
+++ config/i386/i386-expand.c   (working copy)
@@ -14243,8 +14243,13 @@ ix86_expand_vector_set (bool mmx_ok, rtx target, r
 
   switch (mode)
 {
+case E_V2SImode:
+  use_vec_merge = TARGET_MMX_WITH_SSE && TARGET_SSE4_1;
+  if (use_vec_merge)
+   break;
+  /* FALLTHRU */
+
 case E_V2SFmode:
-case E_V2SImode:
   if (mmx_ok)
{
  tmp = gen_reg_rtx (GET_MODE_INNER (mode));
@@ -14409,6 +14414,7 @@ ix86_expand_vector_set (bool mmx_ok, rtx target, r
   break;
 
 case E_V8QImode:
+  use_vec_merge = TARGET_MMX_WITH_SSE && TARGET_SSE4_1;
   break;
 
 case E_V32QImode:
Index: config/i386/mmx.md
===
--- config/i386/mmx.md  (revision 274317)
+++ config/i386/mmx.md  (working copy)
@@ -1394,6 +1394,36 @@
(set_attr "type" "mmxcvt,sselog,sselog")
(set_attr "mode" "DI,TI,TI")])
 
+(define_insn "*mmx_pinsrd"
+  [(set (match_operand:V2SI 0 "register_operand" "=x,Yv")
+(vec_merge:V2SI
+  (vec_duplicate:V2SI
+(match_operand:SI 2 "nonimmediate_operand" "rm,rm"))
+ (match_operand:V2SI 1 "register_operand" "0,Yv")
+  (match_operand:SI 3 "const_int_operand")))]
+  "TARGET_MMX_WITH_SSE && TARGET_SSE4_1
+   && ((unsigned) exact_log2 (INTVAL (operands[3]))
+   < GET_MODE_NUNITS (V2SImode))"
+{
+  operands[3] = GEN_INT (exact_log2 (INTVAL (operands[3])));
+  switch (which_alternative)
+{
+case 1:
+  return "vpinsrd\t{%3, %2, %1, %0|%0, %1, %2, %3}";
+case 0:
+  return "pinsrd\t{%3, %2, %0|%0, %2, %3}";
+default:
+  gcc_unreachable ();
+}
+}
+  [(set_attr "isa" "noavx,avx")
+   (set_attr "prefix_data16" "1")
+   (set_attr "prefix_extra" "1")
+   (set_attr "type" "sselog")
+   (set_attr "length_immediate" "1")
+   (set_attr "prefix" "orig,vex")
+   (set_attr "mode" "TI")])
+
 (define_expand "mmx_pinsrw"
   [(set (match_operand:V4HI 0 "register_operand")
 (vec_merge:V4HI
@@ -1444,6 +1474,42 @@
(set_attr "length_immediate" "1")
(set_attr "mode" "DI,TI,TI")])
 
+(define_insn "*mmx_pinsrb"
+  [(set (match_operand:V8QI 0 "register_operand" "=x,Yv")
+(vec_merge:V8QI
+  (vec_duplicate:V8QI
+(match_operand:QI 2 "nonimmediate_operand" "rm,rm"))
+ (match_operand:V8QI 1 "register_operand" "0,Yv")
+  (match_operand:SI 3 "const_int_operand")))]
+  "TARGET_MMX_WITH_SSE && TARGET_SSE4_1
+   && ((unsigned) exact_log2 (INTVAL (operands[3]))
+   < GET_MODE_NUNITS (V8QImode))"
+{
+  operands[3] = GEN_INT (exact_log2 (INTVAL (operands[3])));
+  switch (which_alternative)
+{
+case 1:
+  if (MEM_P (operands[2]))
+   return "vpinsrb\t{%3, %2, %1, %0|%0, %1, %2, %3}";
+  else
+   return "vpinsrb\t{%3, %k2, %1, %0|%0, %1, %k2, %3}";
+case 0:
+  if (MEM_P (operands[2]))
+   return "pinsrb\t{%3, %2, %0|%0, %2, %3}";
+  else
+   return "pinsrb\t{%3, %k2, %0|%0, %k2, %3}";
+default:
+  gcc_unreachable ();
+}
+}
+  [(set_attr "isa" "noavx,avx")
+   (set_attr "type" "sselog")
+   (set_attr "prefix_data16" "1")
+   (set_attr "prefix_extra" "1")
+   (set_attr "length_immediate" "1")
+   (set_attr "prefix" "orig,vex")
+   (set_attr "mode" "TI")])
+
 (define_insn "mmx_pextrw"
   [(set (match_operand:SI 0 "register_operand" "=r,r")
 (zero_extend:SI

Re: [Patch v2] Enable math functions linking with static library for LTO

[luoxhu]

Hi Richard,

On 2019/8/12 16:51, Richard Biener wrote:

On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:


Hi Richard,
Thanks for your comments, updated the v2 patch as below:
1. Define and use builtin_with_linkage_p.
2. Add comments.
3. Add a testcase.

In LTO mode, if static library and dynamic library contains same
function and both libraries are passed as arguments, linker will link
the function in dynamic library no matter the sequence.  This patch
will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
is a math function, then the function in static library will be linked
first if its sequence is ahead of the dynamic library.


Comments below


gcc/ChangeLog

 2019-08-12  Xiong Hu Luo  

 PR lto/91287
 * builtins.c (builtin_with_linkage_p): New function.
 * builtins.h (builtin_with_linkage_p): New function.
 * symtab.c (write_symbol): Use builtin_with_linkage_p.
 * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
 Likewise.

gcc/testsuite/ChangeLog

 2019-08-12  Xiong Hu Luo  

 PR lto/91287
 * gcc.dg/pr91287.c: New testcase.
---
  gcc/builtins.c | 89 ++
  gcc/builtins.h |  2 +
  gcc/lto-streamer-out.c |  4 +-
  gcc/symtab.c   | 13 -
  gcc/testsuite/gcc.dg/pr91287.c | 40 +++
  5 files changed, 145 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/gcc.dg/pr91287.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 695a9d191af..f4dea941a27 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
*p = (char)tree_to_uhwi (t);
return true;
  }
+
+/* Return true if DECL is a specified builtin math function.  These functions
+   should have symbol in symbol table to provide linkage with faster version of
+   libraries.  */


The comment should read like

/* Return true if the builtin DECL is implemented in a standard
library.  Otherwise
returns false which doesn't guarantee it is not (thus the list of
handled builtins
below may be incomplete).  */


+bool
+builtin_with_linkage_p (tree decl)
+{
+  if (!decl)
+return false;


Omit this check please.


+  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
+switch (DECL_FUNCTION_CODE (decl))
+{
+  CASE_FLT_FN (BUILT_IN_ACOS):
+  CASE_FLT_FN (BUILT_IN_ACOSH):
+  CASE_FLT_FN (BUILT_IN_ASIN):
+  CASE_FLT_FN (BUILT_IN_ASINH):
+  CASE_FLT_FN (BUILT_IN_ATAN):
+  CASE_FLT_FN (BUILT_IN_ATANH):
+  CASE_FLT_FN (BUILT_IN_ATAN2):
+  CASE_FLT_FN (BUILT_IN_CBRT):
+  CASE_FLT_FN (BUILT_IN_CEIL):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
+  CASE_FLT_FN (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
+  CASE_FLT_FN (BUILT_IN_COS):
+  CASE_FLT_FN (BUILT_IN_COSH):
+  CASE_FLT_FN (BUILT_IN_ERF):
+  CASE_FLT_FN (BUILT_IN_ERFC):
+  CASE_FLT_FN (BUILT_IN_EXP):
+  CASE_FLT_FN (BUILT_IN_EXP2):
+  CASE_FLT_FN (BUILT_IN_EXPM1):
+  CASE_FLT_FN (BUILT_IN_FABS):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
+  CASE_FLT_FN (BUILT_IN_FDIM):
+  CASE_FLT_FN (BUILT_IN_FLOOR):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
+  CASE_FLT_FN (BUILT_IN_FMA):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
+  CASE_FLT_FN (BUILT_IN_FMAX):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
+  CASE_FLT_FN (BUILT_IN_FMIN):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
+  CASE_FLT_FN (BUILT_IN_FMOD):
+  CASE_FLT_FN (BUILT_IN_FREXP):
+  CASE_FLT_FN (BUILT_IN_HYPOT):
+  CASE_FLT_FN (BUILT_IN_ILOGB):
+  CASE_FLT_FN (BUILT_IN_LDEXP):
+  CASE_FLT_FN (BUILT_IN_LGAMMA):
+  CASE_FLT_FN (BUILT_IN_LLRINT):
+  CASE_FLT_FN (BUILT_IN_LLROUND):
+  CASE_FLT_FN (BUILT_IN_LOG):
+  CASE_FLT_FN (BUILT_IN_LOG10):
+  CASE_FLT_FN (BUILT_IN_LOG1P):
+  CASE_FLT_FN (BUILT_IN_LOG2):
+  CASE_FLT_FN (BUILT_IN_LOGB):
+  CASE_FLT_FN (BUILT_IN_LRINT):
+  CASE_FLT_FN (BUILT_IN_LROUND):
+  CASE_FLT_FN (BUILT_IN_MODF):
+  CASE_FLT_FN (BUILT_IN_NAN):
+  CASE_FLT_FN (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
+  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
+  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
+  CASE_FLT_FN (BUILT_IN_POW):
+  CASE_FLT_FN (BUILT_IN_REMAINDER):
+  CASE_FLT_FN (BUILT_IN_REMQUO):
+  CASE_FLT_FN (BUILT_IN_RINT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
+  CASE_FLT_FN (BUILT_IN_ROUND):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
+  CASE_FLT_FN (BUILT_IN_SCALBLN):
+  CASE_FLT_FN (BUILT_IN_SCALBN):
+  CASE_FLT_FN (BUILT_IN_SIN):
+  CASE_FLT_FN (BUILT_IN_SINH):
+  CASE_FLT_FN (BUILT_IN_SINCOS):
+  CASE_FLT_FN (BUILT_IN_SQRT):
+  CASE_FLT_FN_FLOATN_NX (BUILT_IN_SQRT):
+  CASE_FLT_FN (BUILT_IN_TAN):
+  CASE_FLT_FN (BUILT_IN_TANH):
+  CASE_FLT_FN (BUILT_IN_TGA

[PATCH] PR fortran/87993 -- An array can have a kind type inquiry suffix

[Steve Kargl]

The attached patch ahs been regression tested on x86_64-*-freebsd.
It probably borders on obvious, but I'll ask away.  OK to commit?

2019-08-12  Steven G. Kargl  

PR fortran/87993
* expr.c (gfc_simplify_expr):  Simplifcation of an array with a kind
type inquiry suffix yields a constant expression.

2019-08-12  Steven G. Kargl  

PR fortran/87993
* gfortran.dg/pr87993.f90: New test.

-- 
Steve
Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c	(revision 274320)
+++ gcc/fortran/expr.c	(working copy)
@@ -2227,6 +2227,11 @@ gfc_simplify_expr (gfc_expr *p, int type)
   if (!simplify_ref_chain (p->ref, type, &p))
 	return false;
 
+  /* If the following conditions hold, we found something like kind type
+	 inquiry of the form a(2)%kind while simplify the ref chain.  */
+  if (p->expr_type == EXPR_CONSTANT && !p->ref && !p->rank && !p->shape)
+	return true;
+
   if (!simplify_constructor (p->value.constructor, type))
 	return false;
 
Index: gcc/testsuite/gfortran.dg/pr87993.f90
===
--- gcc/testsuite/gfortran.dg/pr87993.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr87993.f90	(working copy)
@@ -0,0 +1,8 @@
+! { dg-do run }
+! Code contributed by Gerhard Steinmetz 
+program p
+   integer, parameter :: a(2) = 1
+   integer, parameter :: b = a%kind
+   if (any(a /= 1)) stop 1
+   if (b /= kind(a)) stop 2
+end

Re: [PATCH] fold more string comparison with known result (PR 90879)

[Jeff Law]

On 8/12/19 4:17 PM, Martin Sebor wrote:
> On 8/12/19 2:04 PM, Jeff Law wrote:
>> On 8/9/19 4:14 PM, Martin Sebor wrote:
>>> On 8/9/19 10:58 AM, Jakub Jelinek wrote:
 On Fri, Aug 09, 2019 at 10:51:09AM -0600, Martin Sebor wrote:
> That said, we should change this code one way or the other.
> There is even less of a guarantee that other compilers support
> writing past the end of arrays that have non-zero size than
> that they recognize the documented zero-length extension.

 We use that everywhere forever, so no.
>>>
>>> Just because some invalid code has been in place "forever" doesn't
>>> mean it cannot be changed.  Relying on undocumented "extensions"
>>> because they just happen to work with the compilers they have been
>>> exposed to is exactly how naive users get in trouble.  Our answer
>>> to reports of "bugs" when the behavior changes is typically: fix
>>> your code.  There's little reason to expect other compiler writers
>>> to be any more accommodating.
>>>
 See e.g. rtx u.fld and u.hwint arrays, tree_exp operands array,
 gimple_statement_with_ops op array just to name a few that are
 everywhere.  Coverity is indeed unhappy about
 that, but it would be with [0] certainly too.  Another option is
 to use maximum possible size where we know it (which is the case of
 rtxes and most tree expressions and gimple stmts, but not e.g.
 CALL_EXPR or GIMPLE_CALL where there is no easy upper bound.
>>>
>>> The solution introduced in C99 is a flexible array.  C++
>>> compilers usually support it as well.  Those that don't are
>>> likely to support the zero-length array (even Visual C++ does).
>>> If there's a chance that some don't support either do you really
>>> think it's safe to assume they will do something sane with
>>> the [1] hack?  If you're concerned that the flexible array syntax
>>> or the zero length array won't compile, add a configure test to
>>> see if it does and use whatever alternative is most appropriate.
>> Given that we require a C++03 compiler to build GCC, I think we can
>> revisit how we represent the trailing array.  But that seems independent
>> of the bulk of this patch.
>>
>> Can we separate this issue from the rest of the patch?
> 
> The updated patch I posted is independent of the trailing
> [1] array hack:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2019-08/msg00643.html
I must have dropped this from my queue by accident.  I'll go find it and
give it a looksie as well.

jeff

Re: types for VR_VARYING

[Jeff Law]

On 8/12/19 12:43 PM, Aldy Hernandez wrote:
> This is a fresh re-post of:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg6.html
> 
> Andrew gave me some feedback a week ago, and I obviously don't remember
> what it was because I was about to leave on PTO.  However, I do remember
> I addressed his concerns before getting drunk on rum in tropical islands.
FWIW found a great coffee infused rum while in Kauai last week.  I'm not
a coffee fan, but it was wonderful.  The one bottle we brought back
isn't going to last until Cauldron and I don't think I can get a special
order filled before I leave :(

Their more traditional rums were good as well.  Koloa Rum.  See if you
can get it locally :-)


> 
> This patch adds MIN/MAX values for VR_VARYING.  As was discussed
> earlier, we are only changing VR_VARYING, not VR_UNDEFINED as was the
> original plan.
> 
> As I mentioned earlier, I am tired of re-doing ChangeLogs, so I'll whip
> up the changelog when the review starts.
ACK.



> @@ -150,12 +166,11 @@ vr_values::set_defs_to_varying (gimple *stmt)
>ssa_op_iter i;
>tree def;
>FOR_EACH_SSA_TREE_OPERAND (def, stmt, i, SSA_OP_DEF)
> -{
> -  value_range *vr = get_value_range (def);
> -  /* Avoid writing to vr_const_varying get_value_range may return.  */
> -  if (!vr->varying_p ())
> - vr->set_varying ();
> -}
> +if (value_range_base::supports_type_p (TREE_TYPE (def)))
> +  {
> + value_range *vr = get_value_range (def);
> + vr->set_varying (TREE_TYPE (def));
> +  }
>  }
Is the supports_type_p stuff there to placate the calls from ipa-cp?  I
can live with it in the short term, but it really feels like there
should be something in the ipa-cp client that avoids this silliness.


>  
>  /* Update the value range and equivalence set for variable VAR to

> @@ -1920,7 +1935,7 @@ vr_values::dump_all_value_ranges (FILE *file)
>  vr_values::vr_values () : vrp_value_range_pool ("Tree VRP value ranges")
>  {
>values_propagated = false;
> -  num_vr_values = num_ssa_names;
> +  num_vr_values = num_ssa_names + num_ssa_names / 10;
>vr_value = XCNEWVEC (value_range *, num_vr_values);
>vr_phi_edge_counts = XCNEWVEC (int, num_ssa_names);
>bitmap_obstack_initialize (&vrp_equiv_obstack);
My recollection was someone (Richi) proposed 2X for the initial
allocation which should ensure that in all but the most pathological
cases that we never trigger a reallocation.  In terms of "wasted" space,
it shouldn't matter in practice.

So if you could check the old thread and verify that Richi recommended
the 2X initial allocation and if so, make that minor update.

Jeff

Re: enforce canonicalization of value_range's

[Jeff Law]

On 8/12/19 12:48 PM, Aldy Hernandez wrote:
> This is a ping of:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg8.html
> 
> I have addressed the comments Jeff mentioned there.
> 
> This patch goes before the VR_VARYING + types patch, but I can adapt
> either one to come first.  I can even munge them together into one
> patch, if it helps review.
> 
> Tested on x86-64 Linux.
> 
> OK (assuming ChangeLog entries)?
> 
> Aldy
> 
> 
> curr.patch
> 
> commit ea908bdbfc8cdb4bb63e7d42630d01203edbac41
> Author: Aldy Hernandez 
> Date:   Mon Jul 15 18:09:27 2019 +0200
> 
> Enforce canonicalization in value_range.
Per your other message, I'll assume you'll write a suitable ChangeLog
entry before committing :-)


> 
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index 594ee9adc17..de2f39d8487 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -69,23 +69,20 @@ along with GCC; see the file COPYING3.  If not see
>  #include "builtins.h"
>  #include "wide-int-range.h"
>  
> +static bool
> +ranges_from_anti_range (const value_range_base *ar,
> + value_range_base *vr0, value_range_base *vr1,
> + bool handle_pointers = false);
> +
Presumably this was better than moving the implementation earlier.


> +/* Normalize symbolics into constants.  */
> +
> +value_range_base
> +value_range_base::normalize_symbolics () const
> +{
> +  if (varying_p () || undefined_p ())
> +return *this;
> +  tree ttype = type ();
> +  bool min_symbolic = !is_gimple_min_invariant (min ());
> +  bool max_symbolic = !is_gimple_min_invariant (max ());
Sadly "symbolic" is a particularly poor term.  When I think symbolics, I
think SYMBOL_REF, LABEL_REF and the tree equivalents.  They're
*symbols*.  Symbolics in this case are really things like SSA_NAMEs.
Confused the hell out of me briefly.

If we weren't on a path to kill VRP I'd probably suggest a distinct
effort to constify this code.  Some of the changes were a bit confusing
when it looked like we'd dropped a call to set the range of an object.
But those were just local copies, so setting the type/min/max directly
was actually fine.  constification would make this a bit clearer.  But
again, I don't think it's worth the effort given the long term
trajectory for tree-vrp.c.


So where does the handle_pointers stuff matter?   I'm a bit surprised we
have to do anything special for them.


OK.  I don't expect the answers to the minor questions above will
ultimately change anything.

jeff

Re: [PATCH] Improve DSE to handle redundant zero initializations.

[Jeff Law]

On 8/12/19 2:14 PM, Matthew Beliveau wrote:
> This patch improves DSE to handle missing optimizations for zero
> initializations.
> 
> Thank you,
> Matthew Beliveau
> 
> 
> DSE.patch
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-08-12  Matthew Beliveau  
> 
>   PR c++/DSE.patch
>   * tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
>   catch more redundant zero initalization cases.
>   (dse_dom_walker::dse_optimize_stmt): Improved check to catch more
>   redundant zero initialization cases.
> 
>   * gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
>   * gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.
> 
> @@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt 
> (gimple_stmt_iterator *gsi)
>  {
>bool by_clobber_p = false;
>  
> -  /* First see if this store is a CONSTRUCTOR and if there
> -  are subsequent CONSTRUCTOR stores which are totally
> -  subsumed by this statement.  If so remove the subsequent
> -  CONSTRUCTOR store.
> +  /* Check if this store initalizes zero, or some aggregate of zeros,
> +  and check if there are subsequent stores which are subsumed by this
> +  statement.  If so, remove the subsequent store.
Perhaps...

/* Check if this statement stores zero to a memory location
   and if there is a subsequent store of zero to the same
   memory location.  If so delete the subsequent store.  */


With the ChangeLog nits noted by Marek and the comment fix this is fine
for the trunk.

jeff

[PATCH] PR fortran/89647 -- Allow host associated procedure to be a binding target

[Steve Kargl]

The attached patch fixes PR fortran/89647, and has been
regression tested on x86_64-*-freebsd.

If a procedure is made accessible by host association, the
that procedure can be used as binding target.  During the
resolution of a type bound procedure, gfortran needs to
check the parent namespace for a procedure before it fails.

2019-08-12  Steven G. Kargl  

PF fortran/89647
resolve.c (resolve_typebound_procedure): Allow host associated 
procedure to be a binding target.  While here, wrap long line.

2019-08-12  Steven G. Kargl  

PF fortran/89647
* gfortran.dg/pr89647.f90: New test.

OK to commit?

-- 
Steve
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 274320)
+++ gcc/fortran/resolve.c	(working copy)
@@ -13583,14 +13583,34 @@ resolve_typebound_procedure (gfc_symtree* stree)
 }
   else
 {
+  /* If proc has not been resolved at this point, proc->name may 
+	 actually be a USE associated entity. See PR fortran/89647. */
+  if (!proc->resolved
+	  && proc->attr.function == 0 && proc->attr.subroutine == 0)
+	{
+	  gfc_symbol *tmp;
+	  gfc_find_symbol (proc->name, gfc_current_ns->parent, 1, &tmp);
+	  if (tmp && tmp->attr.use_assoc)
+	{
+	  proc->module = tmp->module;
+	  proc->attr.proc = tmp->attr.proc;
+	  proc->attr.function = tmp->attr.function;
+	  proc->attr.subroutine = tmp->attr.subroutine;
+	  proc->attr.use_assoc = tmp->attr.use_assoc;
+	  proc->ts = tmp->ts;
+	  proc->result = tmp->result;
+	}
+	}
+
   /* Check for F08:C465.  */
   if ((!proc->attr.subroutine && !proc->attr.function)
 	  || (proc->attr.proc != PROC_MODULE
 	  && proc->attr.if_source != IFSRC_IFBODY)
 	  || proc->attr.abstract)
 	{
-	  gfc_error ("%qs must be a module procedure or an external procedure with"
-		" an explicit interface at %L", proc->name, &where);
+	  gfc_error ("%qs must be a module procedure or an external "
+		 "procedure with an explicit interface at %L",
+		 proc->name, &where);
 	  goto error;
 	}
 }
Index: gcc/testsuite/gfortran.dg/pr89647.f90
===
--- gcc/testsuite/gfortran.dg/pr89647.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr89647.f90	(working copy)
@@ -0,0 +1,33 @@
+! { dg-do compile }
+! Code contributed by Ian Harvey  
+  MODULE m1
+IMPLICIT NONE
+PUBLIC :: False
+PUBLIC :: True
+  CONTAINS
+FUNCTION False() RESULT(b)
+  LOGICAL :: b
+  b = .FALSE.
+END FUNCTION False
+
+FUNCTION True() RESULT(b)
+  LOGICAL :: b
+  b = .TRUE.
+END FUNCTION True
+  END MODULE m1
+
+  MODULE m2
+USE m1
+IMPLICIT NONE
+TYPE, ABSTRACT :: t_parent
+CONTAINS
+  PROCEDURE(False), DEFERRED, NOPASS :: Binding
+END TYPE t_parent
+  CONTAINS
+SUBROUTINE s
+  TYPE, EXTENDS(t_parent) :: t_extension
+  CONTAINS
+PROCEDURE, NOPASS :: Binding => True
+  END TYPE t_extension
+END SUBROUTINE s
+  END MODULE m2

Re: [PATCH] PR other/91396 Fix static link error with -fvtable-verify

[Caroline Tice via gcc-patches]

The bootstrap succeeded.

On Mon, Aug 12, 2019 at 11:51 AM Caroline Tice  wrote:
>
> Hi,
>
> This patch is to fix a bug where linking with -fvtable-verify  and
> -static causes the linker to complain about multiple definitions of
> things in the vtv_end*.o files (once from the .o file and once from
> libvtv.a).   (See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91396).
> The fix is for the GNU_USER_TARGET_ENDFILE_SPEC to only include the
> vtv_end*.o files if we're not doing static linking (and we are using
> -fvtable-verify flags).
>
> Tested this fix by verifying that the test case in the bug compiles
> both with and without -static, and that there were no new regressions
> in the libvtv testsuite.  I'm in the process of doing a full bootstrap
> build with this change.   Assuming the bootstrap passes, is this ok to
> commit?
>
> Index: gcc/config/gnu-user.h
> ===
> --- gcc/config/gnu-user.h (revision 274317)
> +++ gcc/config/gnu-user.h (working copy)
> @@ -73,9 +73,9 @@
> GNU userspace "finalizer" file, `crtn.o'.  */
>
>  #define GNU_USER_TARGET_ENDFILE_SPEC \
> -  "%{fvtable-verify=none:%s; \
> +  "%{!static:%{fvtable-verify=none:%s; \
>   fvtable-verify=preinit:vtv_end_preinit.o%s; \
> - fvtable-verify=std:vtv_end.o%s} \
> + fvtable-verify=std:vtv_end.o%s}} \
> %{static:crtend.o%s; \
>   shared|static-pie|" PIE_SPEC ":crtendS.o%s; \
>   :crtend.o%s} " \
>
> ChangeLog entry:
>
> 2019-08-12  Caroline Tice  
>
> PR other/91396
> * config/gnu-user.h (GNU_USER_TARGET_ENDFILE_SPEC): Only add the
> vtv_end.o or vtv_end_preinit.o files if !static.

Re: [PATCH] fold more string comparison with known result (PR 90879)

[Martin Sebor]

On 8/12/19 2:04 PM, Jeff Law wrote:

On 8/9/19 4:14 PM, Martin Sebor wrote:

On 8/9/19 10:58 AM, Jakub Jelinek wrote:

On Fri, Aug 09, 2019 at 10:51:09AM -0600, Martin Sebor wrote:

That said, we should change this code one way or the other.
There is even less of a guarantee that other compilers support
writing past the end of arrays that have non-zero size than
that they recognize the documented zero-length extension.


We use that everywhere forever, so no.


Just because some invalid code has been in place "forever" doesn't
mean it cannot be changed.  Relying on undocumented "extensions"
because they just happen to work with the compilers they have been
exposed to is exactly how naive users get in trouble.  Our answer
to reports of "bugs" when the behavior changes is typically: fix
your code.  There's little reason to expect other compiler writers
to be any more accommodating.


See e.g. rtx u.fld and u.hwint arrays, tree_exp operands array,
gimple_statement_with_ops op array just to name a few that are
everywhere.  Coverity is indeed unhappy about
that, but it would be with [0] certainly too.  Another option is
to use maximum possible size where we know it (which is the case of
rtxes and most tree expressions and gimple stmts, but not e.g.
CALL_EXPR or GIMPLE_CALL where there is no easy upper bound.


The solution introduced in C99 is a flexible array.  C++
compilers usually support it as well.  Those that don't are
likely to support the zero-length array (even Visual C++ does).
If there's a chance that some don't support either do you really
think it's safe to assume they will do something sane with
the [1] hack?  If you're concerned that the flexible array syntax
or the zero length array won't compile, add a configure test to
see if it does and use whatever alternative is most appropriate.

Given that we require a C++03 compiler to build GCC, I think we can
revisit how we represent the trailing array.  But that seems independent
of the bulk of this patch.

Can we separate this issue from the rest of the patch?


The updated patch I posted is independent of the trailing
[1] array hack:

  https://gcc.gnu.org/ml/gcc-patches/2019-08/msg00643.html

Martin

Re: [PATCH] fold more string comparison with known result (PR 90879)

[Jeff Law]

On 8/9/19 10:17 AM, Martin Sebor wrote:
> GCC 9 optimizes a subset of expression of the form
> (0 == strcmp(a, b)) based on the length and/or size of
> the arguments but it doesn't take advantage of all
> the opportunities there.  For example in the following,
> although it folds the first test to false it doesn't fold
> the second one:
> 
>   char a[4];
> 
>   void f (void)
>   {
> if (strlen (a) > 3)   // folded to false by GCC 8+
>   abort ();
> 
> if (strcmp (a, "1234") == 0)   // folded by patched GCC
>   abort ();
> }
> 
> The attached patch extends the strcmp optimization added in
> GCC 9 to also handle the latter cases (among others).  Testing
> the enhancement with several other sizable code bases besides
> GCC (Binutils/GDB, the Linux kernel, and LLVM) shows that code
> like this is rare.  After thinking about it I decided it's more
> likely a bug than a significant optimization opportunity, so
> I introduced a new warning to point it out: -Wstring-compare
> (enabled in -Wextra).
> 
> Besides this enhancement, the patch also improves the current
> optimization to fold strcmp calls with conditional arguments
> such as in:
> 
>   void f (char *s, int i)
>   {
> strcpy (s, "12");
> if (strcmp (s, i ? "123" : "1234") == 0)   // folded
>   abort ();
>   }
> 
> Martin
> 
> PS The diff looks like the changes are more extensive than they
> actually are.
> 
> gcc-90879.diff
> 
> PR tree-optimization/90879 - fold zero-equality of strcmp between a longer 
> string and a smaller array
> 
> gcc/c-family/ChangeLog:
> 
>   PR tree-optimization/90879
>   * c.opt (-Wstring-compare): New option.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR tree-optimization/90879
>   * gcc.dg/Wstring-compare-2.c: New test.
>   * gcc.dg/Wstring-compare.c: New test.
>   * gcc.dg/strcmpopt_3.c: Scan the optmized dump instead of strlen.
>   * gcc.dg/strcmpopt_6.c: New test.
>   * gcc.dg/strlenopt-65.c: Remove uinnecessary declarations, add
>   test cases.
>   * gcc.dg/strlenopt-66.c: Run it.
>   * gcc.dg/strlenopt-67.c: New test.
>   * gcc.dg/strlenopt-68.c: New test.
> 
> gcc/ChangeLog:
> 
>   PR tree-optimization/90879
>   * builtins.c (check_access): Avoid using maxbound when null.
>   * calls.c (maybe_warn_nonstring_arg): Adjust to get_range_strlen change.
>   * doc/invoke.texi (-Wstring-compare): Document new warning option.
>   * gengtype-state.c (state_ident_st): Use a zero-length array instead.
>   (state_token_st): Same.  Make last.
>   (state_ident_by_name): Allocate enough space for terminating nul.
>   * gimple-fold.c (get_range_strlen_tree): Make setting maxbound
>   conditional.
>   (get_range_strlen): Overwrite initial maxbound when non-null.
>   * gimple-ssa-sprintf.c (get_string_length): Adjust to get_range_strlen
>   change.
>   * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Same.
>   (used_only_for_zero_equality): New function.
>   (handle_builtin_memcmp): Call it.
>   (determine_min_objsize): Return an integer instead of tree.
>   (get_len_or_size, strxcmp_eqz_result): New functions.
>   (maybe_warn_pointless_strcmp): New function.
>   (handle_builtin_string_cmp): Call it.  Fold zero-equality of strcmp
>   between a longer string and a smaller array.
> 
> diff --git a/gcc/gengtype-state.c b/gcc/gengtype-state.c
> index 03f40694ec6..80a8b57e9a2 100644
> --- a/gcc/gengtype-state.c
> +++ b/gcc/gengtype-state.c
> @@ -79,6 +79,14 @@ enum state_token_en
>STOK_NAME /* hash-consed name or identifier.  */
>  };
>  
> +/* Suppress warning: ISO C forbids zero-size array for stok_string
> +   below.  The arrays are treated as flexible array members but in
> +   otherwise an empty struct or as a member of a union cannot be
> +   declared as such.  They must have zero size to keep GCC from
> +   assuming their bound reflect their size.  */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wpedantic"
> +
>  
>  /* Structure and hash-table used to share identifiers or names.  */
>  struct state_ident_st
> @@ -86,11 +94,10 @@ struct state_ident_st
>/* TODO: We could improve the parser by reserving identifiers for
>   state keywords and adding a keyword number for them.  That would
>   mean adding another field in this state_ident_st struct.  */
> -  char stid_name[1]; /* actually bigger & null terminated */
> +  char stid_name[0]; /* actually bigger & null terminated */
>  };
>  static htab_t state_ident_tab;
>  
> -
>  /* The state_token_st structure is for lexical tokens in the read
> state file.  The stok_kind field discriminates the union.  Tokens
> are allocated by peek_state_token which calls read_a_state_token
> @@ -110,14 +117,15 @@ struct state_token_st
>union  /* discriminated by stok_kind! 
> */
>{
>  int stok_num;/* when STOK_IN

Patch to support extended characters in C/C++ identifiers

[Lewis Hyatt]

Hello-

The attached patch for libcpp adds support for extended characters (e.g. UTF-8)
in identifiers. A preliminary version of the patch was posted on PR c/67224 as
Comment 26 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67224#c26) and
discussed with Joseph Myers. Here is an updated patch incorporating all
feedback received so far. I hope it is suitable now; please let me know if I
can do anything else to make it ready for you to apply. I am happy to work on
it further, whatever is needed. I can't easily test on anything other than
x86_64-linux though. I did bootstrap all languages and run all tests on that
platform, everything was good.

The (relatively short) changes to libcpp are included inline here. I attached
the test cases as a gzipped patch to avoid any problems with the encoding (the
test cases contain some invalid UTF-8 and also other encodings such as latin-1
as part of the testing).

Thanks for taking a look at it!

-Lewis

libcpp/ChangeLog

PR c/67224
* charset.c (_cpp_valid_utf8): New function to help lex UTF-8 tokens.
* internal.h (_cpp_valid_utf8): Declare.
* lex.c (forms_identifier_p): Use it to recognize UTF-8 identifiers.
(_cpp_lex_direct): Handle UTF-8 in identifiers and CPP_OTHER tokens.
Do all work in "default" case to avoid slowing down typical code paths.
Also handle $ and UCN in the default case for consistency.

gcc/testsuite/ChangeLog

PR c/67224
* c-c++-common/cpp/ucnid-2011-1-utf8.c: New test.
* g++.dg/cpp/ucnid-1-utf8.C: New test.
* g++.dg/cpp/ucnid-2-utf8.C: New test.
* g++.dg/cpp/ucnid-3-utf8.C: New test.
* g++.dg/cpp/ucnid-4-utf8.C: New test.
* g++.dg/other/ucnid-1-utf8.C: New test.
* gcc.dg/cpp/ucnid-1-utf8.c: New test.
* gcc.dg/cpp/ucnid-10-utf8.c: New test.
* gcc.dg/cpp/ucnid-11-utf8.c: New test.
* gcc.dg/cpp/ucnid-12-utf8.c: New test.
* gcc.dg/cpp/ucnid-13-utf8.c: New test.
* gcc.dg/cpp/ucnid-14-utf8.c: New test.
* gcc.dg/cpp/ucnid-15-utf8.c: New test.
* gcc.dg/cpp/ucnid-2-utf8.c: New test.
* gcc.dg/cpp/ucnid-3-utf8.c: New test.
* gcc.dg/cpp/ucnid-4-utf8.c: New test.
* gcc.dg/cpp/ucnid-6-utf8.c: New test.
* gcc.dg/cpp/ucnid-7-utf8.c: New test.
* gcc.dg/cpp/ucnid-9-utf8.c: New test.
* gcc.dg/ucnid-1-utf8.c: New test.
* gcc.dg/ucnid-10-utf8.c: New test.
* gcc.dg/ucnid-11-utf8.c: New test.
* gcc.dg/ucnid-12-utf8.c: New test.
* gcc.dg/ucnid-13-utf8.c: New test.
* gcc.dg/ucnid-14-utf8.c: New test.
* gcc.dg/ucnid-15-utf8.c: New test.
* gcc.dg/ucnid-16-utf8.c: New test.
* gcc.dg/ucnid-2-utf8.c: New test.
* gcc.dg/ucnid-3-utf8.c: New test.
* gcc.dg/ucnid-4-utf8.c: New test.
* gcc.dg/ucnid-5-utf8.c: New test.
* gcc.dg/ucnid-6-utf8.c: New test.
* gcc.dg/ucnid-7-utf8.c: New test.
* gcc.dg/ucnid-8-utf8.c: New test.
* gcc.dg/ucnid-9-utf8.c: New test.
diff --git a/libcpp/charset.c b/libcpp/charset.c
index 8a0e5cbb29b..4f1bee96cee 100644
--- a/libcpp/charset.c
+++ b/libcpp/charset.c
@@ -1198,6 +1198,84 @@ convert_ucn (cpp_reader *pfile, const uchar *from, const uchar *limit,
   return from;
 }
 
+/*  Performs a similar task as _cpp_valid_ucn, but parses UTF-8-encoded
+extended characters rather than UCNs.  If the return value is TRUE, then a
+character was successfully decoded and stored in *CP; *PSTR has been
+updated to point one past the valid UTF-8 sequence.  Diagnostics may have
+been emitted if the character parsed is not allowed in the current context.
+If the return value is FALSE, then *PSTR has not been modified and *CP may
+equal 0, to indicate that *PSTR does not form a valid UTF-8 sequence, or it
+may, when processing an identifier in C mode, equal a codepoint that was
+validly encoded but is not allowed to appear in an identifier.  In either
+case, no diagnostic is emitted, and the return value of FALSE should cause
+a new token to be formed.
+
+Unlike _cpp_valid_ucn, this will never be called when lexing a string; only
+a potential identifier, or a CPP_OTHER token.  NST is unused in the latter
+case.
+
+As in _cpp_valid_ucn, IDENTIFIER_POS is 0 when not in an identifier, 1 for
+the start of an identifier, or 2 otherwise.  */
+
+extern bool
+_cpp_valid_utf8 (cpp_reader *pfile,
+		 const uchar **pstr,
+		 const uchar *limit,
+		 int identifier_pos,
+		 struct normalize_state *nst,
+		 cppchar_t *cp)
+{
+  const uchar *base = *pstr;
+  size_t inbytesleft = limit - base;
+  if (one_utf8_to_cppchar (pstr, &inbytesleft, cp))
+{
+  /* No diagnostic here as this byte will rather become a
+	 new token.  */
+  *cp = 0;
+  return false;
+}
+
+  if (identifier_pos)
+{
+  switch (ucn_valid_in_identifier (pfile, *cp, nst))
+	{
+
+	case 0:
+	  /* In C++, thi

Re: [PATCH] fixincludes breaks mingw64 build

[Gerald Pfeifer]

On Tue, 11 Jun 2019, Bruce Korb wrote:
> I should probably update the e-address as well. Yes, I still receive
> gnu.org email, but not daily. As infrequent as monthly. Can I do
> something to auto-forward it to an e-address I process daily?

You can log in to fencepost.gnu.org and edit /com/mailer/aliases 
there; that is pretty straightforward once you're logged in.

> Finally, my gcc sources are also years out of date, so if someone is
> willing to tweak the docs for me, it'd save some bother. :) 

Let me know how you'd like to see fixincludes/README updated
(current version attached) and I'll take care.

> I'm actually retired now and spend more time with a camera than 
> with code.

Enjoy!

Gerald
GCC MAINTAINER INFORMATION
==

If you are having some problem with a system header that is either
broken by the manufacturer, or is broken by the fixinclude process,
then you will need to alter or add information to the include fix
definitions file, ``inclhack.def''.  Please also send relevant
information to gcc-b...@gcc.gnu.org, gcc-patches@gcc.gnu.org and,
please, to me:  bk...@gnu.org.

To make your fix, you will need to do several things:

1.  Obtain access to the AutoGen program on some platform.  It does
not have to be your build platform, but it is more convenient.

2.  Edit "inclhack.def" to reflect the changes you need to make.
See below for information on how to make those changes.

3.  Run the "genfixes" shell script to produce a new copy of
the "fixincl.x" file.

4.  Rebuild the compiler and check the header causing the issue.
Make sure it is now properly handled.  Add tests to the
"test_text" entry(ies) that validate your fix.  This will
help ensure that future fixes won't negate your work.
Do *NOT* specify test text for "wrap" or "replacement" fixes.
There is no real possibility that these fixes will fail.
If they do, you will surely know straight away.

NOTE:  "test_text" is interpreted by the shell as it gets
copied into the test header.  THEREFORE you must quote
dollar sign characters and back quotes -- unless you mean
for them to be interpreted by the shell.

e.g. the math_huge_val_from_dbl_max test_text needs to
put text into both float.h and math.h, so it includes a
back-quoted script to add text to float.h.

5.  Go into the fixincludes build directory and type, "make check".
You are guaranteed to have issues printed out as a result.
Look at the diffs produced.  Make sure you have not clobbered
the proper functioning of a different fix.  Make sure your
fix is properly tested and it does what it is supposed to do.

6.  Now that you have the right things happening, synchronize the
$(srcdir)/tests/base directory with the $(builddir)/tests/res
directory.  The output of "make check" will be some diffs that
should give you some hints about what to do.

7.  Rerun "make check" and verify that there are no issues left.


MAKING CHANGES TO INCLHACK.DEF
==

0.  If you are not the fixincludes maintainer, please send that
person email about any changes you may want to make.  Thanks!

1.  Every fix must have a "hackname" that is compatible with C syntax
for variable names and is unique without regard to alphabetic case.
Please keep them alphabetical by this name.  :-)

2.  If the problem is known to exist only in certain files, then
identify the files with "files = " entries.  If you use fnmatch(3C)
wild card characters in a "files" entry, be certain that the first
"files" entry has no such character.  Otherwise, the "make check"
machinery will attempt to create files with those characters in the
name.  That is inconvenient.

3.  It is relatively expensive to fire off a process to fix a source
file, therefore write apply tests to avoid unnecessary fix
processes.  The preferred apply tests are "select", "bypass", "mach"
"sum", and "c-test" because they are performed internally:

* select - Run a regex on the contents of the file being considered.
   All such regex-es must match.  Matching is done with
   extended regular expressions.

* bypass - Run a regex on the contents of the file being considered.
   No such regex may match.

* sum- Select a specific version of a file that has a matching
   check sum.  The BSD version of checksum ["sum(1BSD)"]
   is used.  Each "sum" entry should contain exactly three
   space separated tokens:  the sum, some number and the
   basename of the file.  The "some number" is ignored.
   If there are multiple "sum" entries, only one needs to
   match in order to pass.  For example:

   sum = '1234 3 foobar.h';

   specifies that the "foobar.h" header in any directory
   will match if it has the checksum 1234.

* c_test - ca

Re: [MSP430][PATCH 2/2] Read MCU data from external file

[Jeff Law]

On 8/8/19 6:17 AM, Jozef Lawrynowicz wrote:
> This patch extends the MCU data handling so that MCU data can be provided
> in an external file (devices.csv). This means the compiler doesn't have to be
> updated and rebuilt to support new devices when they are released.
> 
> TI distribute devices.csv with other support files (header files, linker
> scripts) on their website. I have also tested that a program builds 
> successfully
> for every MCU and the correct hwmult library is passed to the linker.
> I also built the msp430 cross compiler using a trunk build of native GCC to
> ensure all the error and warning messages added by my code are correct, as
> checked by -Wformat-diag.
> 
> 
> 0002-MSP430-Devices-2-Read-MCU-data-from-external-devices.patch
> 
> From 6f67cdd282f2351d7450e343314beeaa745f0159 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz 
> Date: Tue, 6 Aug 2019 10:52:54 +0100
> Subject: [PATCH 2/2] MSP430: Devices [2]: Read MCU data from external
>  devices.csv file, if it exists
> 
> gcc/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  
> 
>   * gcc/config/msp430/driver-msp430.c (msp430_set_driver_var): New.
>   * gcc/config/msp430/msp430-devices.c (canonicalize_path_dirsep): New.
>   (msp430_check_path_for_devices): New.
>   (parse_devices_csv_1): New.
>   (parse_devices_csv): New.
>   (msp430_extract_mcu_data): Try to find devices.csv and search for the
>   MCU data in devices.csv before using the hard-coded data.
>   Warn if devices.csv isn't found and the MCU wasn't found in the
>   hard-coded data either.
>   * gcc/config/msp430/msp430.h (DRIVER_SELF_SPECS): Call
>   msp430_set_driver_var for -mno-warn-devices-csv and -mdevices-csv-loc.
>   Search for devices.csv on -I and -L paths.
>   (EXTRA_SPEC_FUNCTIONS): Add msp430_check_path_for_devices and
>   msp430_set_driver_var.
>   * gcc/config/msp430/msp430.opt: Add -mwarn-devices-csv and
>   -mdevices-csv-loc=.
>   * gcc/doc/invoke.texi (-mmcu): Document that -I and -L paths are
>   searched for devices.csv.
>   (mwarn-devices-csv): Document option.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  
> 
>   * gcc.target/msp430/msp430.exp (msp430_device_permutations_runtest):
>   Handle csv-* and bad-devices-* tests.
>   * gcc.target/msp430/devices/README: Document how bad-devices-* tests
>   work.
>   * gcc.target/msp430/devices/bad-devices-1.c: New test.
>   * gcc.target/msp430/devices/bad-devices-2.c: Likewise.
>   * gcc.target/msp430/devices/bad-devices-3.c: Likewise.
>   * gcc.target/msp430/devices/bad-devices-4.c: Likewise.
>   * gcc.target/msp430/devices/bad-devices-5.c: Likewise.
>   * gcc.target/msp430/devices/bad-devices-6.c: Likewise.
>   * gcc.target/msp430/devices/csv-device-order.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_00.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_01.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_02.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_04.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_08.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_10.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_11.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_12.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_14.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_18.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_20.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_21.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_22.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_24.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430_28.c: Likewise.
>   * gcc.target/msp430/devices/csv-msp430fr5969.c: Likewise.
>   * gcc.target/msp430/devices/hard-foo.c: Likewise.
>   * gcc.target/msp430/devices/bad-devices-1.csv: New test support file.
>   * gcc.target/msp430/devices/bad-devices-2.csv: Likewise.
>   * gcc.target/msp430/devices/bad-devices-3.csv: Likewise.
>   * gcc.target/msp430/devices/bad-devices-4.csv: Likewise.
>   * gcc.target/msp430/devices/bad-devices-5.csv: Likewise.
>   * gcc.target/msp430/devices/bad-devices-6.csv: Likewise.
>   * gcc.target/msp430/devices/devices.csv: Likewise.
> ---
So it's good we don't have to do updates when a new devices.csv is
released -- except when we devices require new multilibs, but I don't
offhand see a sensible way to deal with that.

OK.

jeff

Re: [MSP430][PATCH 1/2] Consolidate handling of hard-coded MCU data

[Jeff Law]

On 8/8/19 6:14 AM, Jozef Lawrynowicz wrote:
> This patch improves the handling of MCU data by consolidating multiple
> copies of hard-coded MCU data into a single location, and adds a new function
> to be used as a single entry point for the extraction of MCU data for the
> selected MCU.
> 
> This ensures the data is only extracted once per invocation of the
> driver/compiler, whilst previously, the data for the MCU is extracted each 
> time
> it is needed.
> 
> Some notes:
> - The GNU assembler doesn't do anything with the -mmcu option beyond setting 
> up
>   the CPU ISA, so if the GCC driver passes it the -mcpu option, which it will
>   always do if -mmcu is specified, then it is redundant to also pass it -mmcu.
> - The indenting in some places (e.g. msp430_select_hwmult_lib) looks wrong in
>   the patched file, but to make the diff a lot easier to read I have kept the
>   indenting the same as it was before. I can fix this after the patch is
>   accepted.
FWIW, another way to address the indentation issue is with the diff -b
option which says to ignore whitespace differences.

Regardless, yes, please clean up any indentation issues.


> 
> 
> 0001-MSP430-Devices-1-Consolidate-handling-of-hard-coded-.patch
> 
> From cd131b07e0447d104c99317e7ac37c2420c1bf6e Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz 
> Date: Wed, 31 Jul 2019 22:53:50 +0100
> Subject: [PATCH 1/2] MSP430: Devices [1]: Consolidate handling of hard-coded
>  MCU data
> 
> gcc/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  
> 
>   * gcc/config.gcc (msp430*-*-*): Add msp430-devices.o to extra_objs and
>   extra_gcc_objs.
>   * gcc/config/msp430/driver-msp430.c: Remove msp430_mcu_data.
>   (msp430_select_cpu): New spec function.
>   (msp430_select_hwmult_lib): Use msp430_extract_mcu_data to extract
>   MCU data.
>   * gcc/config/msp430/msp430-devices.c: New file.
>   * gcc/config/msp430/msp430-devices.h: New file.
>   * gcc/config/msp430/msp430.c: Remove msp430_mcu_data.
>   (msp430_option_override): Use msp430_extract_mcu_data to extract
>   MCU data.
>   (msp430_use_f5_series_hwmult): Likewise.
>   (use_32bit_hwmult): Likewise.
>   (msp430_no_hwmult): Likewise.
>   * gcc/config/msp430/msp430.h (ASM_SPEC): Don't pass -mmcu to the
>   assembler.
>   (DRIVER_SELF_SPECS): Call msp430_select_cpu if -mmcu is used without
>   and -mcpu option.
>   (EXTRA_SPEC_FUNCTIONS): Add msp430_select_cpu.
>   * gcc/config/msp430/t-msp430: Add rule to build msp430-devices.o.
>   Remove hard-coded MCU multilib data.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-08-XX  Jozef Lawrynowicz  
> 
>   * gcc.target/msp430/msp430.exp
>   (check_effective_target_msp430_430_selected): New.
>   (check_effective_target_msp430_430x_selected): New.
>   (check_effective_target_msp430_mlarge_selected): New.
>   (check_effective_target_msp430_hwmul_not_none): New.
>   (check_effective_target_msp430_hwmul_not_16bit): New.
>   (check_effective_target_msp430_hwmul_not_32bit): New.
>   (check_effective_target_msp430_hwmul_not_f5): New.
>   (msp430_get_opts): New.
>   (msp430_device_permutations_runtest): New.
>   * gcc.target/msp430/devices/README: New file.
>   * gcc.target/msp430/devices-main.c: New test.
>   * gcc.target/msp430/devices/hard-cc430f5123.c: Likewise.
>   * gcc.target/msp430/devices/hard-foo.c: Likewise.
>   * gcc.target/msp430/devices/hard-msp430afe253.c: Likewise.
>   * gcc.target/msp430/devices/hard-msp430cg4616.c: Likewise.
>   * gcc.target/msp430/devices/hard-msp430f4783.c: Likewise.
>   * gcc.target/msp430/devices/hard-rf430frl154h_rom.c: Likewise.
Presumably we aren't supporting switching the selected mcu via
attributes on a per-function basis?

> +/* Main entry point to load the MCU data for the given -mmcu into
> +   extracted_mcu_data.  hard_msp430_mcu_data (initialized at the bottom of 
> this
> +   file) is searched for the MCU name.
> +   This function only needs to be executed once, but it can be first called
> +   from a number of different locations.  */
> +void
> +msp430_extract_mcu_data (const char * mcu_name)
> +{
> +  static int executed = 0;
> +  int i;
> +  if (mcu_name == NULL || executed == 1)
> +return;
> +  executed = 1;
> +  /* FIXME: This array is alpha sorted - we could use a binary search.  */
> +  for (i = ARRAY_SIZE (hard_msp430_mcu_data); i--;)
> +if (strcasecmp (mcu_name, hard_msp430_mcu_data[i].name) == 0)
> +  {
> + extracted_mcu_data = hard_msp430_mcu_data[i];
> + break;
> +  }
I guess we only run this once, so the linear search isn't a serious
compile-time issue?

Assuming we don't care about switching the selected mcu within a
compilation unit, OK for the trunk.

jeff

Re: [PATCH] PR fortran/91414: Improved PRNG

[Steve Kargl]

On Sun, Aug 11, 2019 at 12:37:49PM +0300, Janne Blomqvist wrote:
> Update the PRNG from xorshift1024* to xoshiro256** by the same
> author. For details see
> 
> http://prng.di.unimi.it/
> 
> and the paper at
> 
> https://arxiv.org/abs/1805.01407
> 
> Also the seeding is slightly improved, by reading only 8 bytes from
> the operating system and using the simple splitmix64 PRNG to fill in
> the rest of the PRNG state (as recommended by the xoshiro author),
> instead of reading the entire state from the OS.
> 
> Regtested on x86_64-pc-linux-gnu, Ok for trunk?
> 

Looks good to me.

-- 
Steve

Re: [PATCH] Improve DSE to handle redundant zero initializations.

[Marek Polacek]

I have no comments on the patch itself, but...

On Mon, Aug 12, 2019 at 04:14:40PM -0400, Matthew Beliveau wrote:
> 2019-08-12  Matthew Beliveau  
> 
>   PR c++/DSE.patch

Please drop this.

>   * tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
>   catch more redundant zero initalization cases.
>   (dse_dom_walker::dse_optimize_stmt): Improved check to catch more
>   redundant zero initialization cases.

The second entry should just say "Likewise."

Also, no need to CC Jason on non-C++ patches.

Marek

Re: [Patch, DWARF] Ignore address spaces for get_nearest_type_subqualifiers

[Jeff Law]

On 8/6/19 1:58 AM, senthilkumar.selva...@microchip.com wrote:
> Hi,
> 
>   r254484 explicitly added previously ignored address space 
>   qualifiers, when
>   emitting DWARF dies for types.
> 
>   Adding address spaces to cv_qual_mask, however, breaks
>   nearest_type_subqualifier, which uses the number of bits set 
>   (i.e
>   popcount_hwi) to find the type variant that has the largest 
>   strict subset of a
>   given type's qualifiers. Address spaces are simply represented 
>   as 16 bits, and
>   therefore the number of set bits there has no relevance when 
>   finding
>   nearest_type_subqualifier. This also causes 
>   gcc.target/avr/pr52472.c to ICE.
> 
>   This patch fixes that by excluding address space quals from 
>   cv_qual_mask.
>   Bootstrap and reg test for x86_64-linux in progress. Ok to 
>   commit to trunk
>   if the tests pass?
> 
> Regards
> Senthil
> 
> gcc/ChangeLog:
> 
> 2019-08-06  Senthil Kumar Selvaraj 
> 
> 
>   * dwarf2out.c (modified_type_die): CLEAR_QUAL_ADDR_SPACE from
> cv_qual_mask before calling get_nearest_type_subqualifiers.
> 
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index aa7fd7eb465..157c970d729 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -13295,8 +13295,9 @@ modified_type_die (tree type, int 
> cv_quals, bool reverse,
>/* Determine a lesser qualified type that most closely 
>matches
>this one.  Then generate DW_TAG_* entries for the remaining
>qualifiers.  */
> +  int nearest_qual_mask = CLEAR_QUAL_ADDR_SPACE 
> (cv_qual_mask);
>sub_quals = get_nearest_type_subqualifiers (type, cv_quals,
> -   cv_qual_mask);
> +   nearest_qual_mask);
>if (sub_quals && use_debug_types)
>   {
> bool needed = false;
Is there some reason we couldn't initialize nearest_qual_mask in the
same place we initialize cv_qual_mask and just not include the address
space qualifiers?  ie something like this:

  const int cv_qual_mask = (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE
| TYPE_QUAL_RESTRICT | TYPE_QUAL_ATOMIC |
ENCODE_QUAL_ADDR_SPACE(~0U));
  const int nearest_qual_mask = (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE
| TYPE_QUAL_RESTRICT | TYPE_QUAL_ATOMIC);


I realize your approach would handle any qualifier outside of the
address space bits.  But it may be better in this code to be explicit
about what we handle and what we don't.

Thoughts?

jeff

[PATCH] Improve DSE to handle redundant zero initializations.

[Matthew Beliveau]

This patch improves DSE to handle missing optimizations for zero
initializations.

Thank you,
Matthew Beliveau
Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-08-12  Matthew Beliveau  

	PR c++/DSE.patch
	* tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
	catch more redundant zero initalization cases.
	(dse_dom_walker::dse_optimize_stmt): Improved check to catch more
	redundant zero initialization cases.

	* gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
	* gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.

diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
new file mode 100644
index 000..b8d01d1644b
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+void blah (char *);
+
+void bar ()
+{
+  char a[256] = "";
+  a[3] = 0; 
+  blah (a);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
new file mode 100644
index 000..8cefa6f0cb7
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+#include 
+
+void blahd (double *);
+
+void fubar ()
+{
+  double d;
+  double *x = &d;
+
+  memset (&d, 0 , sizeof d);
+  *x = 0.0;
+  blahd (x);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
index 5b7c4fc6d1a..dcaeb8edbfe 100644
--- gcc/tree-ssa-dse.c
+++ gcc/tree-ssa-dse.c
@@ -628,11 +628,8 @@ dse_optimize_redundant_stores (gimple *stmt)
   tree fndecl;
   if ((is_gimple_assign (use_stmt)
 	   && gimple_vdef (use_stmt)
-	   && ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
-	&& CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
-	&& !gimple_clobber_p (stmt))
-	   || (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
-		   && integer_zerop (gimple_assign_rhs1 (use_stmt)
+	   && initializer_zerop (gimple_op (use_stmt, 1), NULL)
+	   && !gimple_clobber_p (stmt))
 	  || (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
 	  && (fndecl = gimple_call_fndecl (use_stmt)) != NULL
 	  && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
@@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
 {
   bool by_clobber_p = false;
 
-  /* First see if this store is a CONSTRUCTOR and if there
-	 are subsequent CONSTRUCTOR stores which are totally
-	 subsumed by this statement.  If so remove the subsequent
-	 CONSTRUCTOR store.
+  /* Check if this store initalizes zero, or some aggregate of zeros,
+	 and check if there are subsequent stores which are subsumed by this
+	 statement.  If so, remove the subsequent store.
 
 	 This will tend to make fewer calls into memset with longer
 	 arguments.  */
-  if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
-	  && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
+  if (initializer_zerop (gimple_op (stmt, 1), NULL)
 	  && !gimple_clobber_p (stmt))
 	dse_optimize_redundant_stores (stmt);
 

Re: [C++ PATCH] Adjust -Wsequence-point for C++17 changes (PR c++/91415)

[Jeff Law]

On 8/12/19 8:48 AM, Jakub Jelinek wrote:
> Hi!
> 
> The following patch adds some tweaks for -Wsequence-point warning for C++17
> and later.  In particular, stop warning about no sequence point in between
> <<, >>, ., -> and [] expressions, where E1 is in C++17 sequenced before E2.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> As mentioned in the PR, this is just part of the needed changes, I've tried
> to adjust handling of MODIFY_EXPR, but didn't figure out exactly what needs
> to be done, and .* / ->* aren't handled either, and CALL_EXPR needs probably
> some verification too.
> 
> 2019-08-12  Jakub Jelinek  
> 
>   PR c++/91415
>   * c-common.c (verify_tree): For LSHIFT_EXPR, RSHIFT_EXPR,
>   COMPONENT_REF and ARRAY_REF in cxx_dialect >= cxx17 mode handle it
>   like COMPOUND_EXPR rather than normal expression.
> 
>   * g++.dg/warn/sequence-pt-4.C: New test.
OK
jeff

Re: [PATCH] fold more string comparison with known result (PR 90879)

[Jeff Law]

On 8/9/19 4:14 PM, Martin Sebor wrote:
> On 8/9/19 10:58 AM, Jakub Jelinek wrote:
>> On Fri, Aug 09, 2019 at 10:51:09AM -0600, Martin Sebor wrote:
>>> That said, we should change this code one way or the other.
>>> There is even less of a guarantee that other compilers support
>>> writing past the end of arrays that have non-zero size than
>>> that they recognize the documented zero-length extension.
>>
>> We use that everywhere forever, so no.
> 
> Just because some invalid code has been in place "forever" doesn't
> mean it cannot be changed.  Relying on undocumented "extensions"
> because they just happen to work with the compilers they have been
> exposed to is exactly how naive users get in trouble.  Our answer
> to reports of "bugs" when the behavior changes is typically: fix
> your code.  There's little reason to expect other compiler writers
> to be any more accommodating.
> 
>> See e.g. rtx u.fld and u.hwint arrays, tree_exp operands array,
>> gimple_statement_with_ops op array just to name a few that are
>> everywhere.  Coverity is indeed unhappy about
>> that, but it would be with [0] certainly too.  Another option is
>> to use maximum possible size where we know it (which is the case of
>> rtxes and most tree expressions and gimple stmts, but not e.g.
>> CALL_EXPR or GIMPLE_CALL where there is no easy upper bound.
> 
> The solution introduced in C99 is a flexible array.  C++
> compilers usually support it as well.  Those that don't are
> likely to support the zero-length array (even Visual C++ does).
> If there's a chance that some don't support either do you really
> think it's safe to assume they will do something sane with
> the [1] hack?  If you're concerned that the flexible array syntax
> or the zero length array won't compile, add a configure test to
> see if it does and use whatever alternative is most appropriate.
Given that we require a C++03 compiler to build GCC, I think we can
revisit how we represent the trailing array.  But that seems independent
of the bulk of this patch.

Can we separate this issue from the rest of the patch?

jeff

Re: [patch, fortran] Some corrections for DO loop index warnings

[Steve Kargl]

On Mon, Aug 12, 2019 at 05:18:18PM +0200, Thomas Koenig wrote:
> Hello world,
> 
> the attached patch fixes three problems with DO loop index warnings:
> 
> - DO loops in contained procedures were not checked
> 
> - Zero-trip loops gave a false positive
> 
> - DO loops in blocks gave the same warning twice
> 
> plus it fixes the resulting fallout from the test suite.
> 
> Regression-tested.  OK for trunk?  I also think that this is something
> that could be safely backported to gcc-9.
> 

Ok.  If it regression cleanly on gcc9, go ahead an commit there as well.

-- 
Steve

Re: [PATCH] fix and improve strlen conditional handling of merged stores (PR 91183, 91294, 91315)

[Jeff Law]

On 8/9/19 5:42 PM, Martin Sebor wrote:
>>> @@ -3408,7 +3457,13 @@ static bool
>>>   }
>>>       gimple *stmt = SSA_NAME_DEF_STMT (exp);
>>> -  if (gimple_code (stmt) != GIMPLE_PHI)
>>> +  if (gimple_assign_single_p (stmt))
>>> +    {
>>> +  tree rhs = gimple_assign_rhs1 (stmt);
>>> +  return count_nonzero_bytes (rhs, offset, nbytes, lenrange,
>>> nulterm,
>>> +  allnul, allnonnul, snlim);
>>> +    }
>>> +  else if (gimple_code (stmt) != GIMPLE_PHI)
>>>   return false;
>> What cases are you handling here?  Are there any cases where a single
>> operand expression on the RHS affects the result.  For example, if we've
>> got a NOP_EXPR which zero extends RHS?  Does that change the nonzero
>> bytes in a way that is significant?
>>
>> I'm not opposed to handling single operand objects here, I'm just
>> concerned that we're being very lenient in just stripping away the
>> operator and looking at the underlying object.
> 
> I remember adding the code because of a test failure but not
> the specifics anymore.  No tests fail with it removed so it
> may not be needed.  As you know, I've been juggling a few
> enhancements in this area and copying code between them as
> I need it so it's possible that I copied too much, or that
> some other change has obviated it, or also that the test
> failed somewhere else and I forgot to copy the test along
> with the code  I'll remove it until it's needed.
Let's pull it for now.  If we come across the need again, we can
obviously revisit with a testcase.


> 
>>> @@ -3795,7 +3824,14 @@ handle_store (gimple_stmt_iterator *gsi)
>>>   }
>>>     else
>>>   si->nonzero_chars = build_int_cst (size_type_node, offset);
>>> -  si->full_string_p = full_string_p;
>>> +
>>> +  /* Set FULL_STRING_P only if the length of the strings being
>>> + written is the same, and clear it if the strings have
>>> + different lengths.  In the latter case the length stored
>>> + in si->NONZERO_CHARS becomes the lower bound.
>>> + FIXME: Handle the upper bound of the length if possible.  */
>>> +  si->full_string_p = full_string_p && lenrange[0] == lenrange[1];
>> So there seems to be a disconnect between the comment and the code.
>>
>> The comment indicates you care about the lengths of the two strings
>> being the same.  But what you're really comparing when the lenrange[0]
>> == lenrange[1] test is that the min and max of RHS are the same.
> 
> The comment tries to make clear that all the arrays on the RHS
> of the assignment must have the same length in order to set
> FULL_STRING_P.  Like here where LENRANGE = { 4, 4, 4 }:
> 
>   void f (char *s)
>   {
>     if (__builtin_strlen (s) != 2)
>   return;
> 
>     *(int*)a = i ? 0x : 0x;
>   }
> 
> but not here where LENRANGE = { 1, 4, 4 }:
> 
>     *(int*)a = i < 0 ? 0x : i ? 0x0022 : 0x3300;
> 
> If the bounds of the range of lengths of all the strings on
> the RHS are the same they're all the same length.
> 
> I'm open to phrasing it better.
Oh, I think I see what I was missing.  In the case where RHS is a
conditional (or perhaps a SSA_NAME which was set from a PHI) LENRANGE
will have the min/max/# bytes for the RHS was a whole, not just a single
component of the RHS.


>> It generally looks reasonable, so I think we just need to reach a
>> conclusion on the gimple_assign_single_p cases we're trying to handle
>> and the possible mismatch between the comment and the code.
> 
> Do you want me to post another revision with
> the gimple_assign_single_p test removed?
I think remove that hunk, bootstrap, test, commit and post for archival
purposes.  I do not think another round of review is necessary.

jeff

Re: [PATCH] Teach mklog to reference PRs.

[Pedro Alves]

On 8/12/19 8:34 PM, Pedro Alves wrote:
> Let me share my 2c -- the format GDB uses doesn't affect most GCC forks

I meant most GCC folks.

Thanks,
Pedro Alves

Re: [PATCH] Teach mklog to reference PRs.

[Pedro Alves]

On 8/1/19 4:01 PM, Jakub Jelinek wrote:
> You can easily tweak the script to handle the other way too.
> Looking around, different people have different style, some people don't
> post the date/name/email lines at all, others do, some people post them
> multiple times, once for each ChangeLog, others just once, and for the
> latter case, some people post gcc/, etc. prefixes before the entries
> afterwards, while others don't, and others do it only conditionally
> (e.g. when the number of ChangeLog files is too high or it isn't immediately
> obvious which ones they are for, say one entry for gcc and one for
> gcc/testsuite ChangeLog, or similarly for gcc/cp and gcc/testsuite etc.
> isn't hard to figure out and is just noise.
> So, either we have multiple options for mklog, so that people if they prefer
> some other style don't have to rewrite it all the time, or have one style we
> want to recommend.  If the latter, I think it is better to have a style that
> is perhaps automatically parseable by a script, on the other side for
> readers of the mailing list should minimize unnecessary cruft and
> redundancies.  For that I think the email line just once, then empty line,
> then PR lines and/or line with short summary as some people use, then
> the dirnames of ChangeLog entries (but no ChangeLog, that is always the
> case) and actual entries sounds best to me.

Let me share my 2c -- the format GDB uses doesn't affect most GCC forks
of course, though uniformity across GNU tools is a good thing to have
in my opinion, to ease sharing tooling and ease moving between projects.

"email just once" assumes that you have a single author or set of authors
for the whole patch.  But if you have a patch that includes entries by
different authors, then you have to have multiple ChangeLog "blocks" each
with its author/email line, ending up where you didn't want to to begin
with, anyway, like:

-MM-DD  John Doe  

gcc/
* ...

-MM-DD  Joe Shmoe  

gcc/testsuite/
* ...


With the "email line per ChangeLog file entry" style, there's
no exception to the rule, it all just always looks the same, like:

gcc/
-MM-DD  John Doe  

* ...

gcc/testsuite/
-MM-DD  Joe Shmoe  

* ...

This isn't your everyday patch, of course, but it does
show up here and there.

IMHO, saving a few characters/bytes doesn't justify the simplicity
of just having a full entry per ChangeLog file.

FWIW, this is the style used by GDB, as documented at the end
of section 9, at
.


"In your patch email you should also specify which changelog is being modified. 
Before the line containing the date and your name/email, add a line with the 
path to the changelog. If there are multiple components being updated with 
multiple changelog edits, split these into sections, one for each changelog:

gdb/ChangeLog:
2013-12-12  John Doe  

PR gdb/
* breakpoint.c (handle_some_event): Remove reference to foo.  Call
bar.
* configure.ac (build_warnings): Do not use -Wformat-nonliteral
with -Wno-format.
* configure: Regenerate.
* NEWS: Mention awesome feature.

gdb/testsuite/ChangeLog:
2013-12-12  John Doe  

PR gdb/
* Makefile: Test changes for awesome feature.
"


Pedro Alves

[wwwdocs] Add missing C++20 feature

[Marek Polacek]

I missed this one.  As a sign of repentance, I'll aim to address it this week.

Applying to CVS.

Index: cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.96
diff -u -r1.96 cxx-status.html
--- cxx-status.html 7 Aug 2019 14:34:01 -   1.96
+++ cxx-status.html 12 Aug 2019 18:55:03 -
@@ -422,6 +422,12 @@
   No (https://gcc.gnu.org/PR91360";>PR91360)
   
 
+
+  More implicit moves (merge P0527R1 and P1155R3)
+  http://wg21.link/p1825r0";>P1825R0
+  No (https://gcc.gnu.org/PR91427";>PR91427)
+  
+
   
 
   C++17 Support in GCC

[PATCH] PR other/91396 Fix static link error with -fvtable-verify

[Caroline Tice via gcc-patches]

Hi,

This patch is to fix a bug where linking with -fvtable-verify  and
-static causes the linker to complain about multiple definitions of
things in the vtv_end*.o files (once from the .o file and once from
libvtv.a).   (See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91396).
The fix is for the GNU_USER_TARGET_ENDFILE_SPEC to only include the
vtv_end*.o files if we're not doing static linking (and we are using
-fvtable-verify flags).

Tested this fix by verifying that the test case in the bug compiles
both with and without -static, and that there were no new regressions
in the libvtv testsuite.  I'm in the process of doing a full bootstrap
build with this change.   Assuming the bootstrap passes, is this ok to
commit?

Index: gcc/config/gnu-user.h
===
--- gcc/config/gnu-user.h (revision 274317)
+++ gcc/config/gnu-user.h (working copy)
@@ -73,9 +73,9 @@
GNU userspace "finalizer" file, `crtn.o'.  */

 #define GNU_USER_TARGET_ENDFILE_SPEC \
-  "%{fvtable-verify=none:%s; \
+  "%{!static:%{fvtable-verify=none:%s; \
  fvtable-verify=preinit:vtv_end_preinit.o%s; \
- fvtable-verify=std:vtv_end.o%s} \
+ fvtable-verify=std:vtv_end.o%s}} \
%{static:crtend.o%s; \
  shared|static-pie|" PIE_SPEC ":crtendS.o%s; \
  :crtend.o%s} " \

ChangeLog entry:

2019-08-12  Caroline Tice  

PR other/91396
* config/gnu-user.h (GNU_USER_TARGET_ENDFILE_SPEC): Only add the
vtv_end.o or vtv_end_preinit.o files if !static.

enforce canonicalization of value_range's

[Aldy Hernandez]

This is a ping of:

https://gcc.gnu.org/ml/gcc-patches/2019-07/msg8.html

I have addressed the comments Jeff mentioned there.

This patch goes before the VR_VARYING + types patch, but I can adapt 
either one to come first.  I can even munge them together into one 
patch, if it helps review.


Tested on x86-64 Linux.

OK (assuming ChangeLog entries)?

Aldy

commit ea908bdbfc8cdb4bb63e7d42630d01203edbac41
Author: Aldy Hernandez 
Date:   Mon Jul 15 18:09:27 2019 +0200

Enforce canonicalization in value_range.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 594ee9adc17..de2f39d8487 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -69,23 +69,20 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "wide-int-range.h"
 
+static bool
+ranges_from_anti_range (const value_range_base *ar,
+			value_range_base *vr0, value_range_base *vr1,
+			bool handle_pointers = false);
+
 /* Set of SSA names found live during the RPO traversal of the function
for still active basic-blocks.  */
 static sbitmap *live;
 
-void
-value_range_base::set (enum value_range_kind kind, tree min, tree max)
-{
-  m_kind = kind;
-  m_min = min;
-  m_max = max;
-  if (flag_checking)
-check ();
-}
-
 void
 value_range::set_equiv (bitmap equiv)
 {
+  if (undefined_p () || varying_p ())
+equiv = NULL;
   /* Since updating the equivalence set involves deep copying the
  bitmaps, only do it if absolutely necessary.
 
@@ -261,7 +258,8 @@ value_range_base::constant_p () const
 void
 value_range_base::set_undefined ()
 {
-  set (VR_UNDEFINED, NULL, NULL);
+  m_kind = VR_UNDEFINED;
+  m_min = m_max = NULL;
 }
 
 void
@@ -273,7 +271,8 @@ value_range::set_undefined ()
 void
 value_range_base::set_varying ()
 {
-  set (VR_VARYING, NULL, NULL);
+  m_kind = VR_VARYING;
+  m_min = m_max = NULL;
 }
 
 void
@@ -324,6 +323,24 @@ value_range::equiv_add (const_tree var,
 bool
 value_range_base::singleton_p (tree *result) const
 {
+  if (m_kind == VR_ANTI_RANGE)
+{
+  if (nonzero_p ())
+	{
+	  if (TYPE_PRECISION (type ()) == 1)
+	{
+	  if (result)
+		*result = m_max;
+	  return true;
+	}
+	  return false;
+	}
+
+  value_range_base vr0, vr1;
+  return (ranges_from_anti_range (this, &vr0, &vr1, true)
+	  && vr1.undefined_p ()
+	  && vr0.singleton_p (result));
+}
   if (m_kind == VR_RANGE
   && vrp_operand_equal_p (min (), max ())
   && is_gimple_min_invariant (min ()))
@@ -499,23 +516,28 @@ static assert_locus **asserts_for;
 /* Return the maximum value for TYPE.  */
 
 tree
-vrp_val_max (const_tree type)
+vrp_val_max (const_tree type, bool handle_pointers)
 {
-  if (!INTEGRAL_TYPE_P (type))
-return NULL_TREE;
-
-  return TYPE_MAX_VALUE (type);
+  if (INTEGRAL_TYPE_P (type))
+return TYPE_MAX_VALUE (type);
+  if (POINTER_TYPE_P (type) && handle_pointers)
+{
+  wide_int max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+  return wide_int_to_tree (const_cast (type), max);
+}
+  return NULL_TREE;
 }
 
 /* Return the minimum value for TYPE.  */
 
 tree
-vrp_val_min (const_tree type)
+vrp_val_min (const_tree type, bool handle_pointers)
 {
-  if (!INTEGRAL_TYPE_P (type))
-return NULL_TREE;
-
-  return TYPE_MIN_VALUE (type);
+  if (INTEGRAL_TYPE_P (type))
+return TYPE_MIN_VALUE (type);
+  if (POINTER_TYPE_P (type) && handle_pointers)
+return build_zero_cst (const_cast (type));
+  return NULL_TREE;
 }
 
 /* Return whether VAL is equal to the maximum value of its type.
@@ -626,8 +648,7 @@ intersect_range_with_nonzero_bits (enum value_range_kind vr_type,
extract ranges from var + CST op limit.  */
 
 void
-value_range_base::set_and_canonicalize (enum value_range_kind kind,
-	tree min, tree max)
+value_range_base::set (enum value_range_kind kind, tree min, tree max)
 {
   /* Use the canonical setters for VR_UNDEFINED and VR_VARYING.  */
   if (kind == VR_UNDEFINED)
@@ -645,7 +666,9 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
   if (TREE_CODE (min) != INTEGER_CST
   || TREE_CODE (max) != INTEGER_CST)
 {
-  set (kind, min, max);
+  m_kind = kind;
+  m_min = min;
+  m_max = max;
   return;
 }
 
@@ -681,12 +704,13 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
   kind = kind == VR_RANGE ? VR_ANTI_RANGE : VR_RANGE;
 }
 
+  tree type = TREE_TYPE (min);
+
   /* Anti-ranges that can be represented as ranges should be so.  */
   if (kind == VR_ANTI_RANGE)
 {
   /* For -fstrict-enums we may receive out-of-range ranges so consider
  values < -INF and values > INF as -INF/INF as well.  */
-  tree type = TREE_TYPE (min);
   bool is_min = (INTEGRAL_TYPE_P (type)
 		 && tree_int_cst_compare (min, TYPE_MIN_VALUE (type)) <= 0);
   bool is_max = (INTEGRAL_TYPE_P (type)
@@ -729,22 +753,37 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
 }
 }
 
+  /* Normalize [MIN, MAX] in

types for VR_VARYING

[Aldy Hernandez]

This is a fresh re-post of:

https://gcc.gnu.org/ml/gcc-patches/2019-07/msg6.html

Andrew gave me some feedback a week ago, and I obviously don't remember 
what it was because I was about to leave on PTO.  However, I do remember 
I addressed his concerns before getting drunk on rum in tropical islands.


This patch adds MIN/MAX values for VR_VARYING.  As was discussed 
earlier, we are only changing VR_VARYING, not VR_UNDEFINED as was the 
original plan.


As I mentioned earlier, I am tired of re-doing ChangeLogs, so I'll whip 
up the changelog when the review starts.


Tested on x86-64 Linux.

OK?

Aldy
commit dad943a48ff2fccc203bf11839b7e3016e44dfe1
Author: Aldy Hernandez 
Date:   Mon Jul 22 10:04:43 2019 +0200

Add type to VR_VARYING.

diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index 4c68af847e1..0184703a13c 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -251,13 +251,18 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block bb)
   if (virtual_operand_p (lhs))
 	continue;
 
+  /* Skips floats and other things we can't represent in a
+	 range.  */
+  if (!value_range_base::supports_type_p (TREE_TYPE (lhs)))
+	continue;
+
   value_range vr_result;
   bool interesting = stmt_interesting_for_vrp (phi);
   if (!has_unvisited_preds && interesting)
 	vr_values->extract_range_from_phi_node (phi, &vr_result);
   else
 	{
-	  vr_result.set_varying ();
+	  vr_result.set_varying (TREE_TYPE (lhs));
 	  /* When we have an unvisited executable predecessor we can't
 	 use PHI arg ranges which may be still UNDEFINED but have
 	 to use VARYING for them.  But we can still resort to
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 69c00a9c5a5..859ad3fbaf5 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -977,7 +977,12 @@ ipcp_vr_lattice::set_to_bottom ()
 {
   if (m_vr.varying_p ())
 return false;
-  m_vr.set_varying ();
+  /* ?? We create all sorts of VARYING ranges for floats, structures,
+ and other types which we cannot handle as ranges.  We should
+ probably avoid handling them throughout the pass, but it's easier
+ to create a sensible VARYING here and let the lattice
+ propagate.  */
+  m_vr.set_varying (integer_type_node);
   return true;
 }
 
diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c
index 785227df690..43bca63da09 100644
--- a/gcc/tree-ssa-threadedge.c
+++ b/gcc/tree-ssa-threadedge.c
@@ -182,8 +182,10 @@ record_temporary_equivalences_from_phis (edge e,
 	new_vr->deep_copy (vr_values->get_value_range (src));
 	  else if (TREE_CODE (src) == INTEGER_CST)
 	new_vr->set (src);
+	  else if (value_range_base::supports_type_p (TREE_TYPE (src)))
+	new_vr->set_varying (TREE_TYPE (src));
 	  else
-	new_vr->set_varying ();
+	new_vr->set_undefined ();
 
 	  /* This is a temporary range for DST, so push it.  */
 	  evrp_range_analyzer->push_value_range (dst, new_vr);
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 8b80bce8945..3911db9c26e 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -440,14 +440,16 @@ get_range_info (const_tree name, value_range_base &vr)
   wide_int wmin, wmax;
   enum value_range_kind kind = get_range_info (name, &wmin, &wmax);
 
-  if (kind == VR_VARYING || kind == VR_UNDEFINED)
-min = max = NULL;
+  if (kind == VR_VARYING)
+vr.set_varying (TREE_TYPE (name));
+  else if (kind == VR_UNDEFINED)
+vr.set_undefined ();
   else
 {
   min = wide_int_to_tree (TREE_TYPE (name), wmin);
   max = wide_int_to_tree (TREE_TYPE (name), wmax);
+  vr.set (kind, min, max);
 }
-  vr.set (kind, min, max);
   return kind;
 }
 
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index de2f39d8487..e215b032f78 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -186,9 +186,11 @@ value_range_base::check ()
 	break;
   }
 case VR_UNDEFINED:
-case VR_VARYING:
   gcc_assert (!min () && !max ());
   break;
+case VR_VARYING:
+  gcc_assert (m_min && m_max);
+  break;
 default:
   gcc_unreachable ();
 }
@@ -214,6 +216,10 @@ value_range::check ()
 bool
 value_range_base::equal_p (const value_range_base &other) const
 {
+  /* Ignore types for undefined.  All undefines are equal.  */
+  if (undefined_p ())
+return m_kind == other.m_kind;
+
   return (m_kind == other.m_kind
 	  && vrp_operand_equal_p (m_min, other.m_min)
 	  && vrp_operand_equal_p (m_max, other.m_max));
@@ -269,16 +275,19 @@ value_range::set_undefined ()
 }
 
 void
-value_range_base::set_varying ()
+value_range_base::set_varying (tree type)
 {
+  gcc_assert (supports_type_p (type));
   m_kind = VR_VARYING;
-  m_min = m_max = NULL;
+  m_min = vrp_val_min (type, true);
+  m_max = vrp_val_max (type, true);
 }
 
 void
-value_range::set_varying ()
+value_range::set_varying (tree type)
 {
-  set (VR_VARYING, NULL, NULL, NULL);
+  value_range_base::set_varying (type);
+  equiv_clear ();
 }
 
 /* Return TRUE 

Re: [PATCH][AArch64] Increase default function alignment

[James Greenhalgh]

On Fri, May 31, 2019 at 12:52:32PM +0100, Wilco Dijkstra wrote:
> With -mcpu=generic the function alignment is currently 8, however almost all
> supported cores prefer 16 or higher, so increase the default to 16:12.
> This gives ~0.2% performance increase on SPECINT2017, while codesize is 0.12%
> larger.

OK.

Thanks,
James

> ChangeLog:
> 2019-05-31  Wilco Dijkstra  
> 
>   * config/aarch64/aarch64.c (generic_tunings): Set function alignment to 
> 16.
> 
> --
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 0023cb37bbae5afe9387840c1bb6b43586d4fac2..ed1422af6aab5e3c6eeea37ec57e69b64092a0ab
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -693,7 +693,7 @@ static const struct tune_params generic_tunings =
>4, /* memmov_cost  */
>2, /* issue_rate  */
>(AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
> -  "8",   /* function_align.  */
> +  "16:12",   /* function_align.  */
>"4",   /* jump_align.  */
>"8",   /* loop_align.  */
>2, /* int_reassoc_width.  */
> 

Re: [PATCH][aarch64] Use neoversen1 tuning struct for -mcpu=cortex-a76

[James Greenhalgh]

On Tue, Jul 30, 2019 at 05:59:15PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> The neoversen1 tuning struct gives better performance on the Cortex-A76, 
> so use that.
> The only difference from the current tuning is the function and label 
> alignment settings.
> 
> This gives about 1.3% improvement on SPEC2006 int and 0.3% on SPEC2006 fp.
> 
> Tested on aarch64-none-elf.
> 
> Ok for trunk?

OK.

Thanks,
James

> Thanks,
> Kyrill
> 
> 2019-07-31  Kyrylo Tkachov  
> 
>      * config/aarch64/aarch64-cores.def (cortex-a76): Use neoversen1 tuning
>      struct.
> 

Re: [PATCH][AArch64] Fix PR81800

[James Greenhalgh]

On Tue, May 28, 2019 at 06:11:29PM +0100, Wilco Dijkstra wrote:
> PR81800 is about the lrint inline giving spurious FE_INEXACT exceptions.
> The previous change for PR81800 didn't fix this: when lrint is disabled
> in the backend, the midend will simply use llrint.  This actually makes
> things worse since llrint now also ignores FE_INVALID exceptions!
> The fix is to disable lrint/llrint on double if the size of a long is
> smaller (ie. ilp32).
> 
> Passes regress and bootstrap on AArch64. OK for commit?

OK.

Thanks,
James

> 
> ChangeLog
> 2018-11-13  Wilco Dijkstra
> 
> gcc/
>   PR target/81800
>   * gcc/config/aarch64/aarch64.md (lrint): Disable lrint pattern if GPF
>   operand is larger than a long int.
> 
> testsuite/
>   PR target/81800
>   * gcc.target/aarch64/no-inline-lrint_3.c: New test.
> 
> --
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 
> 5a1894063a1ed2db1cc947c9c449d48808ed96ae..f08cd0930b3fc6527fbca218ad3c464f1ead0103
>  100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -6304,7 +6304,7 @@ (define_expand "lrint2"
>[(match_operand:GPI 0 "register_operand")
> (match_operand:GPF 1 "register_operand")]
>"TARGET_FLOAT
> -   && ((GET_MODE_SIZE (mode) <= GET_MODE_SIZE (mode))
> +   && ((GET_MODE_BITSIZE (mode) <= LONG_TYPE_SIZE)
> || !flag_trapping_math || flag_fp_int_builtin_inexact)"
>  {
>rtx cvt = gen_reg_rtx (mode);
> diff --git a/gcc/testsuite/gcc.target/aarch64/no-inline-lrint_3.c 
> b/gcc/testsuite/gcc.target/aarch64/no-inline-lrint_3.c
> new file mode 100644
> index 
> ..ca772cb999e7b6cfbd3f080111d3eb479d43f47b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/no-inline-lrint_3.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target ilp32 } */
> +/* { dg-options "-O3 -fno-math-errno -fno-fp-int-builtin-inexact" } */
> +
> +#define TEST(name, float_type, int_type, fn) void f_##name (float_type x) \
> +{  \
> +  volatile int_type   b = __builtin_##fn (x);
>   \
> +}
> +
> +TEST (dld, double, long, lrint)
> +TEST (flf, float , long, lrintf)
> +
> +TEST (did, double, int, lrint)
> +TEST (fif, float , int, lrintf)
> +
> +/* { dg-final { scan-assembler-times "fcvtzs\tw\[0-9\]+, \[d,s\]\[0-9\]+" 2 
> } } */
> +/* { dg-final { scan-assembler-times "bl\tlrint" 2 } } */
> 

Re: [PATCH] Add generic support for "noinit" attribute

[Jozef Lawrynowicz]

Hi,

On Tue, 30 Jul 2019 15:35:23 +0200
Christophe Lyon  wrote:

> Hi,
> 
> Thanks for the useful feedback.
> 
> 
> On Tue, 16 Jul 2019 at 11:54, Richard Sandiford
>  wrote:
> >
> > Thanks for doing this in a generic way.
> >
> > Christophe Lyon  writes:  
> > > @@ -2224,6 +2234,50 @@ handle_weak_attribute (tree *node, tree name,
> > >return NULL_TREE;
> > >  }
> > >
> > > +/* Handle a "noinit" attribute; arguments as in struct
> > > +   attribute_spec.handler.  Check whether the attribute is allowed
> > > +   here and add the attribute to the variable decl tree or otherwise
> > > +   issue a diagnostic.  This function checks NODE is of the expected
> > > +   type and issues diagnostics otherwise using NAME.  If it is not of
> > > +   the expected type *NO_ADD_ATTRS will be set to true.  */
> > > +
> > > +static tree
> > > +handle_noinit_attribute (tree * node,
> > > +   tree   name,
> > > +   tree   args,
> > > +   intflags ATTRIBUTE_UNUSED,
> > > +   bool *no_add_attrs)
> > > +{
> > > +  const char *message = NULL;
> > > +
> > > +  gcc_assert (DECL_P (*node));
> > > +  gcc_assert (args == NULL);
> > > +
> > > +  if (TREE_CODE (*node) != VAR_DECL)
> > > +message = G_("%qE attribute only applies to variables");
> > > +
> > > +  /* Check that it's possible for the variable to have a section.  */
> > > +  if ((TREE_STATIC (*node) || DECL_EXTERNAL (*node) || in_lto_p)
> > > +  && DECL_SECTION_NAME (*node))
> > > +message = G_("%qE attribute cannot be applied to variables "
> > > +  "with specific sections");
> > > +
> > > +  /* If this var is thought to be common, then change this.  Common
> > > + variables are assigned to sections before the backend has a
> > > + chance to process them.  */
> > > +  if (DECL_COMMON (*node))
> > > +DECL_COMMON (*node) = 0;
> > > +
> > > +  if (message)
> > > +{
> > > +  warning (OPT_Wattributes, message, name);
> > > +  *no_add_attrs = true;
> > > +}
> > > +
> > > +  return NULL_TREE;
> > > +}  
> >
> > This might cause us to clear DECL_COMMON even when rejecting the
> > attribute.  Also, the first message should win over the others
> > (e.g. for a function in a particular section).
> >
> > So I think the "/* Check that it's possible ..." should be an else-if
> > and the DECL_COMMON stuff should be specific to !message.  
> 
> You are right, thanks.
> 
> Jozef, this is true for msp430_data_attr() too. I'll let you update it.
> 
> >
> > Since this is specific to ELF targets, I think we should also
> > warn if !targetm.have_switchable_bss_sections.
> >  
> OK
> 
> > > @@ -2338,6 +2336,8 @@ msp430_select_section (tree decl, int reloc, 
> > > unsigned HOST_WIDE_INT align)
> > >  {
> > >if (TREE_CODE (decl) == FUNCTION_DECL)
> > >   return text_section;
> > > +  /* FIXME: ATTR_NOINIT is handled generically in
> > > +  default_elf_select_section.  */
> > >else if (has_attr (ATTR_NOINIT, decl))
> > >   return noinit_section;
> > >else if (has_attr (ATTR_PERSIST, decl))  
> >
> > I guess adding a FIXME is OK.  It's very tempting to remove
> > the noinit stuff and use default_elf_select_section instead of
> > default_select_section, but I realise that'd be difficult to test.  
> 
> I added that as suggested by Jozef, it's best if he handles the
> change you propose, I guess he can do proper testing.

BTW, regarding this, I have rearranged msp430_select_section in the attached WIP
patch, so that it will use default_elf_select_section to handle "noinit". Once
your patch is applied, I'll submit some variant of this patch.

Still, better leave the FIXME in, and I'll remove it in my patch.

Likewise for all the other fixes required in msp430.c, once this patch is
applied, I'll fix those up.

Thanks,
Jozef

> 
> 
> > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > > index f2619e1..850153e 100644
> > > --- a/gcc/doc/extend.texi
> > > +++ b/gcc/doc/extend.texi
> > > @@ -7129,6 +7129,12 @@ The @code{visibility} attribute is described in
> > >  The @code{weak} attribute is described in
> > >  @ref{Common Function Attributes}.
> > >
> > > +@item noinit
> > > +@cindex @code{noinit} variable attribute
> > > +Any data with the @code{noinit} attribute will not be initialized by
> > > +the C runtime startup code, or the program loader.  Not initializing
> > > +data in this way can reduce program startup times.
> > > +
> > >  @end table
> > >
> > >  @node ARC Variable Attributes  
> >
> > Would be good to mention that the attribute is specific to ELF targets.
> > (Yeah, we don't seem to do that consistently for other attributes.)
> > It might also be worth saying that it requires specific linker support.  
> OK, done.
> 
> >  
> > > diff --git a/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c 
> > > b/gcc/testsuite/gcc.c-torture/execute/noinit-attribute.c
> > > new file mode 100644
> > > index 000..f33c0d0
> > > --- /dev/null
> 

Re: [AArch64] Add a "y" constraint for V0-V7

[James Greenhalgh]

On Wed, Aug 07, 2019 at 07:19:12PM +0100, Richard Sandiford wrote:
> Some indexed SVE FCMLA operations have a 3-bit register field that
> requires one of Z0-Z7.  This patch adds a public "y" constraint for that.
> 
> The patch also documents "x", which is again intended to be a public
> constraint.
> 
> Tested on aarch64-linux-gnu (with and without SVE) and aarch64_be-elf.
> OK to install?


I had the vague recollection that 'y' already meant something... I'm
guessing you already checked, but just in case, please check.

Otherwise, this is OK.

Thanks,
James


> 
> Richard
> 
> 
> 2019-08-07  Richard Sandiford  
> 
> gcc/
>   * doc/md.texi: Document the x and y constraints for AArch64.
>   * config/aarch64/aarch64.h (FP_LO8_REGNUM_P): New macro.
>   (FP_LO8_REGS): New reg_class.
>   (REG_CLASS_NAMES, REG_CLASS_CONTENTS): Add an entry for FP_LO8_REGS.
>   * config/aarch64/aarch64.c (aarch64_hard_regno_nregs)
>   (aarch64_regno_regclass, aarch64_class_max_nregs): Handle FP_LO8_REGS.
>   * config/aarch64/predicates.md (aarch64_simd_register): Use
>   FP_REGNUM_P instead of checking the classes manually.
>   * config/aarch64/constraints.md (y): New constraint.
> 
> gcc/testsuite/
>   * gcc.target/aarch64/asm-x-constraint-1.c: New test.
>   * gcc.target/aarch64/asm-y-constraint-1.c: Likewise.
> 

Re: [AArch64] Make aarch64_classify_vector_mode use a switch statement

[James Greenhalgh]

On Wed, Aug 07, 2019 at 07:24:18PM +0100, Richard Sandiford wrote:
> aarch64_classify_vector_mode used properties of a mode to test whether
> the mode was a single Advanced SIMD vector, a single SVE vector, or a
> tuple of SVE vectors.  That works well for current trunk and is simpler
> than checking for modes by name.
> 
> However, for the ACLE and for planned autovec improvements, we also
> need partial SVE vector modes that hold:
> 
> - half of the available 32-bit elements
> - a half or quarter of the available 16-bit elements
> - a half, quarter, or eighth of the available 8-bit elements
> 
> These should be packed in memory and unpacked in registers.  E.g.
> VNx2SI has half the number of elements of VNx4SI, and so is half the
> size in memory.  When stored in registers, each VNx2SI element occupies
> the low 32 bits of a VNx2DI element, with the upper bits being undefined.
> 
> The upshot is that:
> 
>   GET_MODE_SIZE (VNx4SImode) == 2 * GET_MODE_SIZE (VNx2SImode)
> 
> since GET_MODE_SIZE must always be the memory size.  This in turn means
> that for fixed-length SVE, some partial modes can have the same size as
> Advanced SIMD modes.  We then need to be specific about which mode we're
> dealing with.
> 
> This patch prepares for that by switching based on the mode instead
> of querying properties.
> 
> A later patch makes sure that Advanced SIMD modes always win over
> partial SVE vector modes in normal queries.
> 
> Tested on aarch64-linux-gnu (with and without SVE) and aarch64_be-elf.
> OK to install?

OK.

Thanks,
James

> 
> Richard
> 
> 
> 2019-08-07  Richard Sandiford  
> 
> gcc/
>   * config/aarch64/aarch64.c (aarch64_classify_vector_mode): Switch
>   based on the mode instead of testing properties of it.

Re: [AArch64] Make the complete mnemonic

[James Greenhalgh]

On Wed, Aug 07, 2019 at 08:23:48PM +0100, Richard Sandiford wrote:
> The Advanced SIMD and SVE permute patterns both split the permute
> operation into a base name and a hilo suffix.  That works well, but it
> means that for "@" patterns, we need to pass the permute code twice,
> once for the base name and once for the suffix.
> 
> Having a unified name avoids that and also makes the definitions
> slightly simpler.
> 
> Tested on aarch64-linux-gnu (with and without SVE) and aarch64_be-elf.
> OK to install?

OK.

Thanks,
James

> 
> 2019-08-07  Richard Sandiford  
> 
> gcc/
>   * config/aarch64/iterators.md (perm_insn): Include the "1"/"2" suffix.
>   (perm_hilo): Remove UNSPEC_ZIP*, UNSEPC_TRN* and UNSPEC_UZP*.
>   * config/aarch64/aarch64-simd.md
>   (aarch64_): Rename to..
>   (aarch64_): ...this and remove perm_hilo
>   from the asm template.
>   * config/aarch64/aarch64-sve.md
>   (aarch64_): Rename to..
>   (aarch64_): ...this and remove perm_hilo
>   from the asm template.
>   (aarch64_): Rename to..
>   (aarch64_): ...this and remove perm_hilo
>   from the asm template.
>   * config/aarch64/aarch64-simd-builtins.def: Update comment.

[PATCH] PR libstdc++/90361 add missing macro definition

[Jonathan Wakely]

The src/c++17/string-inst.cc file needs to override the default string
ABI so that it still contains the expected symbols even when the library
is configured with --with-default-libstdcxx-abi=gcc4-compatible.

PR libstdc++/90361
* src/c++17/string-inst.cc: Use _GLIBCXX_USE_CXX11_ABI=1 by default.

Tested x86_64-linux, committed to trunk.


commit 5225243411b6c5ffb5c30e558b25721450337c01
Author: Jonathan Wakely 
Date:   Mon Aug 12 17:16:55 2019 +0100

PR libstdc++/90361 add missing macro definition

The src/c++17/string-inst.cc file needs to override the default string
ABI so that it still contains the expected symbols even when the library
is configured with --with-default-libstdcxx-abi=gcc4-compatible.

PR libstdc++/90361
* src/c++17/string-inst.cc: Use _GLIBCXX_USE_CXX11_ABI=1 by default.

diff --git a/libstdc++-v3/src/c++17/string-inst.cc 
b/libstdc++-v3/src/c++17/string-inst.cc
index c095a90587e..4dc0a9ca449 100644
--- a/libstdc++-v3/src/c++17/string-inst.cc
+++ b/libstdc++-v3/src/c++17/string-inst.cc
@@ -26,6 +26,12 @@
 // ISO C++ 14882:2017 24  Strings library
 //
 
+#ifndef _GLIBCXX_USE_CXX11_ABI
+// Instantiations in this file use the new SSO std::string ABI unless included
+// by another file which defines _GLIBCXX_USE_CXX11_ABI=0.
+# define _GLIBCXX_USE_CXX11_ABI 1
+#endif
+
 #include 
 
 namespace std _GLIBCXX_VISIBILITY(default)

Re: [PATCH] Teach mklog to reference PRs.

[Marek Polacek]

On Mon, Aug 12, 2019 at 10:27:46AM -0600, Martin Sebor wrote:
> FWIW, another feature I find useful that mklog doesn't seem to have
> is one that strips the context details from testsuite ChangeLogs.
> Hardly anyone bothers to mention the functions, macros, types, or
> variables they changed in tests, so having the script strip it from
> the ChangeLog entry (unless some special option is used for the few
> of us who really want to mention it) would reduce the work one has
> to put in to edit the templates.

Perhaps it could have an option to avoid generating ChangeLog entries for
testsuite/* altogether:
https://gcc.gnu.org/codingconventions.html#ChangeLogs

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [PATCH] Teach mklog to reference PRs.

[Martin Sebor]

On 8/2/19 12:04 AM, Martin Liška wrote:

On 8/2/19 7:45 AM, Martin Liška wrote:

I agree with that approach.


I'm sending an example how it would look like for something
bigger:

$ git diff 
e8a3be407068bfb9c82f0f6656b30d26cc2f484a~15..e8a3be407068bfb9c82f0f6656b30d26cc2f484a > 
patch && ./contrib/mklog patch > changelog.txt

Thoughts?


I didn't know about mklog at first and I wrote my own script
to do the same thing, so I think the enhancement is useful.

My script also inserts the PR string like your patch but without
any special options it does it by a) extracting the PR number
from the patch name (my naming convention for patch files is
-.diff, such as gcc-12345.diff), and b) querying
Bugzilla for the component name of the bug (additional bug ids
can be passed to the script on the command line).  I would
consider switching to mklog if it had this feature plus the one
below.

To answer your question above, I have yet another script to apply
both a patch and insert the ChangeLogs.  The script expects each
ChangeLog entry at the top of the patch to be introduced by
the directory prefix, like so (separated by blank lines):

PR c/12345 - title of the C bug from Bugzilla
PR c/23456 - title of the other C bug from Bugzilla
PR c++/34567 - title of the C++ bug from Bugzilla

gcc/ChangeLog:

PR c/12345
* foo.c (blah): Make blah better.

gcc/c/ChangeLog:

PR c/12345
PR c/23456
* bar.c (blah): Some C change.

gcc/cp/ChangeLog:

PR c/12345
PR c++/34567
* bar.c (blah): Some other change.

Here's an example of the format from one of my recent patches:
  https://gcc.gnu.org/ml/gcc-patches/2019-08/msg0.html

The script extracts each block between the .../ChangeLog: lines,
adds it at the top of the ChangeLog file, and then takes the whole
thing and makes it the SVN description that I then pass as
an argument to svn ci -F.

The format you ask about would let me come pretty close to doing
that.  What it wouldn't let me do is use different bugs for
different components in the same commit.  It doesn't happen often
to me so it wouldn't a big deal but it might be something to think
about in case it happens more commonly to others.  I would have
to massage my applypatch.sh script to adjust to the slightly
different format but that should be easy.

FWIW, another feature I find useful that mklog doesn't seem to have
is one that strips the context details from testsuite ChangeLogs.
Hardly anyone bothers to mention the functions, macros, types, or
variables they changed in tests, so having the script strip it from
the ChangeLog entry (unless some special option is used for the few
of us who really want to mention it) would reduce the work one has
to put in to edit the templates.

Martin

Re: [EXTERNAL]Re: [PATCH 1/2][MIPS] Emit .note.GNU-stack for soft-float linux targets.

[Dragan Mladjenovic]

On 09.08.2019. 23:31, Jeff Law wrote:
> On 8/5/19 4:47 AM, Dragan Mladjenovic wrote:
>> From: "Dragan Mladjenovic" 
>>
>> gcc/ChangeLog:
>>
>> 2019-08-05  Dragan Mladjenovic  
>>
>>  * config/mips/linux.h (NEED_INDICATE_EXEC_STACK): Define to
>>  TARGET_SOFT_FLOAT.
>>  * config/mips/mips.c (TARGET_ASM_FILE_END): Define to ...
>>  (mips_asm_file_end): New function. Delegate to
>>  file_end_indicate_exec_stack if NEED_INDICATE_EXEC_STACK is true.
>>  * config/mips/mips.h (NEED_INDICATE_EXEC_STACK): Define to 0.
>>
>> libgcc/ChangeLog:
>>
>> 2019-08-05  Dragan Mladjenovic  
>>
>>  * config/mips/gnustack.h: New file.
>>  * config/mips/crti.S: Include gnustack.h.
>>  * config/mips/crtn.S: Likewise.
>>  * config/mips/mips16.S: Likewise.
>>  * config/mips/vr4120-div.S: Likewise.
> Seems reasonable.  What testing has been done for this patch?  I don't
> doubt it works for the MIPS linux targets, I'm more interested in making
> sure it doesn't do the wrong thing for the embedded mips targets.

I've built a cross mips-mti-elf toolchain, albeit with reduced multilib, 
but still there were not .note.GNU-stack in sysroot.
Is this enough?

 >
 > Do you have write access to the repository?
 >

I do not have write access to the repository.

Best regards,
Dragan

[patch, fortran] Some corrections for DO loop index warnings

[Thomas Koenig]

Hello world,

the attached patch fixes three problems with DO loop index warnings:

- DO loops in contained procedures were not checked

- Zero-trip loops gave a false positive

- DO loops in blocks gave the same warning twice

plus it fixes the resulting fallout from the test suite.

Regression-tested.  OK for trunk?  I also think that this is something
that could be safely backported to gcc-9.


2019-08-12  Thomas Koenig  

PR fortran/91424
* frontend-passes.c (do_subscript): Do not warn for an
expression a second time.  Do not warn about a zero-trip loop.
(doloop_warn): Also look at contained namespaces.

2019-08-12  Thomas Koenig  

PR fortran/91424
* gfortran.dg/do_subscript_3.f90: New test.
* gfortran.dg/do_subscript_4.f90: New test.
* gfortran.dg/pr70754.f90: Use indices that to not overflow.

2019-08-12  Thomas Koenig  

PR fortran/91422
* testsuite/libgomp.oacc-fortran/routine-7.f90: Correct array
dimension.

! { dg-do compile }
! PR fortran/91424
! Check that only one warning is issued inside blocks, and that
! warnings are also issued for contained subroutines.

program main
  real :: a(5)
  block
integer :: j
do j=0, 5  ! { dg-warning "out of bounds" }
   a(j) = 2. ! { dg-warning "out of bounds" }
end do
  end block
  call x
contains
  subroutine x
integer :: i
do i=1,6 ! { dg-warning "out of bounds" }
   a(i) = 2.  ! { dg-warning "out of bounds" }
end do
  end subroutine x
end program main
Index: gcc/fortran/frontend-passes.c
===
--- gcc/fortran/frontend-passes.c	(revision 273855)
+++ gcc/fortran/frontend-passes.c	(working copy)
@@ -2556,6 +2556,12 @@ do_subscript (gfc_expr **e)
   if (in_assoc_list)
 return 0;
 
+  /* We already warned about this.  */
+  if (v->do_not_warn)
+return 0;
+
+  v->do_not_warn = 1;
+
   for (ref = v->ref; ref; ref = ref->next)
 {
   if (ref->type == REF_ARRAY && ref->u.ar.type == AR_ELEMENT)
@@ -2608,7 +2614,6 @@ do_subscript (gfc_expr **e)
 	  else
 		have_do_start = false;
 
-
 	  if (dl->ext.iterator->end->expr_type == EXPR_CONSTANT)
 		{
 		  have_do_end = true;
@@ -2620,6 +2625,17 @@ do_subscript (gfc_expr **e)
 	  if (!have_do_start && !have_do_end)
 		return 0;
 
+	  /* No warning inside a zero-trip loop.  */
+	  if (have_do_start && have_do_end)
+		{
+		  int sgn, cmp;
+
+		  sgn = mpz_cmp_ui (do_step, 0);
+		  cmp = mpz_cmp (do_end, do_start);
+		  if ((sgn > 0 && cmp < 0) || (sgn < 0 && cmp > 0))
+		break;
+		}
+
 	  /* May have to correct the end value if the step does not equal
 		 one.  */
 	  if (have_do_start && have_do_end && mpz_cmp_ui (do_step, 1) != 0)
@@ -2761,6 +2777,12 @@ static void
 doloop_warn (gfc_namespace *ns)
 {
   gfc_code_walker (&ns->code, doloop_code, do_function, NULL);
+
+  for (ns = ns->contained; ns; ns = ns->sibling)
+{
+  if (ns->code == NULL || ns->code->op != EXEC_BLOCK)
+	doloop_warn (ns);
+}
 }
 
 /* This selction deals with inlining calls to MATMUL.  */
Index: gcc/testsuite/gfortran.dg/pr70754.f90
===
--- gcc/testsuite/gfortran.dg/pr70754.f90	(revision 273855)
+++ gcc/testsuite/gfortran.dg/pr70754.f90	(working copy)
@@ -18,12 +18,13 @@ contains
 integer (ii4), dimension(40,40) :: c
 integer  i, j
 
-do i=1,20
-  b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) &
- + 34 * a(i,j-1) + a(i+1,j+1) &
- + a(i+1,j-1) + a(i-1,j+1) &
- + a(i-1,j-1)
-  c(i,j) = 123
+j = 10
+do i=11,30
+   b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) &
++ 34 * a(i,j-1) + a(i+1,j+1) &
++ a(i+1,j-1) + a(i-1,j+1) &
++ a(i-1,j-1)
+   c(i,j) = 123
 end do
 
 where ((xyz(:,:,2) /= 0) .and. (c /= 0))
Index: libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90
===
--- libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90	(revision 273855)
+++ libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90	(working copy)
@@ -109,7 +109,7 @@ end subroutine gang
 
 subroutine seq (a)
   !$acc routine seq
-  integer, intent (inout) :: a(M)
+  integer, intent (inout) :: a(N)
   integer :: i
 
   do i = 1, N
! { dg-do compile }
! PR 91424 - this used to warn although the DO loop is zero trip.
program main
  implicit none
  integer :: i
  real :: a(2)
  do i=1,3,-1
 a(i) = 2.
  end do
  print *,a
end program main

Re: [PATCH] Add missing _mm{256,512}_zext* intrinsics (PRs target/83250, target/91340)

[Uros Bizjak]

On Mon, Aug 12, 2019 at 4:57 PM Jakub Jelinek  wrote:
>
> Hi!
>
> The following patch adds 9 missing intrinsics, which are like _mm*_cast*,
> but don't leave the upper bits undefined - set them to zero instead.
> The implementation uses code that combine manages to optimize well,
> the only problem is that as the 512-bit intrinsics are supposed to be
> avx512f and some needed intrinsics they'd ideally use are avx512dq, it means
> that for _mm512_zextpd128_pd512/_mm512_zextps256_ps512 we emit
> vmovaps/vmovapd instead of vmovapd/vmovaps.
>
> I've also discovered that for AVX, there is no test coverage of the various
> cast intrinsics, so I've added that too.
>
> The PR has some details on other possible expansions, it would be nice to
> optimize also those definitions into the same code, but it will require some
> extra define_insn_and_split, though I think that can be done incrementally;
> and once done, perhaps we could change the 
> _mm512_zextpd128_pd512/_mm512_zextps256_ps512
> so that they actually generate the right ps vs. pd variant of move.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-08-12  Jakub Jelinek  
>
> PR target/83250
> PR target/91340
> * config/i386/avxintrin.h (_mm256_zextpd128_pd256,
> _mm256_zextps128_ps256, _mm256_zextsi128_si256): New intrinsics.
> * config/i386/avx512fintrin.h (_mm512_zextpd128_pd512,
> _mm512_zextps128_ps512, _mm512_zextsi128_si512, 
> _mm512_zextpd256_pd512,
> _mm512_zextps256_ps512, _mm512_zextsi256_si512): Likewise.
>
> * gcc.target/i386/avx-typecast-1.c: New test.
> * gcc.target/i386/avx-typecast-2.c: New test.
> * gcc.target/i386/avx512f-typecast-2.c: New test.

OK for AVX, LGTM for AVX512F.

Thanks,
Uros.

>
> --- gcc/config/i386/avxintrin.h.jj  2019-08-05 12:25:34.476667673 +0200
> +++ gcc/config/i386/avxintrin.h 2019-08-12 14:33:07.905601186 +0200
> @@ -1484,6 +1484,26 @@ _mm256_castsi128_si256 (__m128i __A)
>return (__m256i) __builtin_ia32_si256_si ((__v4si)__A);
>  }
>
> +/* Similarly, but with zero extension instead of undefined values.  */
> +
> +extern __inline __m256d __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
> +_mm256_zextpd128_pd256 (__m128d __A)
> +{
> +  return _mm256_insertf128_pd (_mm256_setzero_pd (), __A, 0);
> +}
> +
> +extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
> +_mm256_zextps128_ps256 (__m128 __A)
> +{
> +  return _mm256_insertf128_ps (_mm256_setzero_ps (), __A, 0);
> +}
> +
> +extern __inline __m256i __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
> +_mm256_zextsi128_si256 (__m128i __A)
> +{
> +  return _mm256_insertf128_si256 (_mm256_setzero_si256 (), __A, 0);
> +}
> +
>  extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm256_set_m128 ( __m128 __H, __m128 __L)
>  {
> --- gcc/config/i386/avx512fintrin.h.jj  2019-07-12 09:34:49.524385009 +0200
> +++ gcc/config/i386/avx512fintrin.h 2019-08-12 14:36:52.281169281 +0200
> @@ -15437,6 +15437,48 @@ _mm512_castsi256_si512 (__m256i __A)
>return (__m512i)__builtin_ia32_si512_256si ((__v8si)__A);
>  }
>
> +extern __inline __m512d
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextpd128_pd512 (__m128d __A)
> +{
> +  return (__m512d) _mm512_insertf32x4 (_mm512_setzero_ps (), (__m128) __A, 
> 0);
> +}
> +
> +extern __inline __m512
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextps128_ps512 (__m128 __A)
> +{
> +  return _mm512_insertf32x4 (_mm512_setzero_ps (), __A, 0);
> +}
> +
> +extern __inline __m512i
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextsi128_si512 (__m128i __A)
> +{
> +  return _mm512_inserti32x4 (_mm512_setzero_si512 (), __A, 0);
> +}
> +
> +extern __inline __m512d
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextpd256_pd512 (__m256d __A)
> +{
> +  return _mm512_insertf64x4 (_mm512_setzero_pd (), __A, 0);
> +}
> +
> +extern __inline __m512
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextps256_ps512 (__m256 __A)
> +{
> +  return (__m512) _mm512_insertf64x4 (_mm512_setzero_pd (), (__m256d) __A, 
> 0);
> +}
> +
> +extern __inline __m512i
> +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> +_mm512_zextsi256_si512 (__m256i __A)
> +{
> +  return _mm512_inserti64x4 (_mm512_setzero_si512 (), __A, 0);
> +}
> +
>  extern __inline __mmask16
>  __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
>  _mm512_cmpeq_epu32_mask (__m512i __A, __m512i __B)
> --- gcc/testsuite/gcc.target/i386/avx-typecast-1.c.jj   2019-08-12 
> 15:12:51.597209881 +0200
> +++ gcc/testsuite/gcc.target/i386/avx-typecast-1.c  2019-08-12 
> 15:12:47.334274860 +0200
> @@ -0,0 +1,83 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -ma

Re: [PATCH] Properly detect working jobserver in gcc driver.

[Jeff Law]

On 8/9/19 7:05 AM, Martin Liška wrote:
> On 8/9/19 2:38 PM, Martin Liška wrote:
>> On 8/9/19 10:19 AM, Richard Biener wrote:
>>> OK with that.  I still think that making -flto use a jobserver if detected
>>> (but _not_ use the number of CPU cores by default) makes
>>> sense as an independent change.
>> In order to address that, I'm suggesting following patch that I've been
>> testing.
>>
>> Martin
>>
> Hm, I take back the config changes.
> 
> Martin
> 
> 
> 0001-Automatically-detect-GNU-jobserver-with-flto.patch
> 
> From 1f9c9f74a84ec3ca930bbc9525ef2185200e0ce8 Mon Sep 17 00:00:00 2001
> From: Martin Liska 
> Date: Fri, 9 Aug 2019 14:03:11 +0200
> Subject: [PATCH] Automatically detect GNU jobserver with -flto.
> 
> gcc/ChangeLog:
> 
> 2019-08-09  Martin Liska  
> 
>   * doc/invoke.texi: Document automatic detection of jobserver.
>   * lto-wrapper.c (run_gcc): Detect jobserver always.
OK
jeff

Re: [PATCH] Port value profiling to -fopt-info infrastructure.

[Jeff Law]

On 8/9/19 7:13 AM, Martin Liška wrote:
> On 8/9/19 10:13 AM, Richard Biener wrote:
>> On Thu, Aug 8, 2019 at 4:17 PM Jeff Law  wrote:
>>> On 8/8/19 7:04 AM, Martin Liška wrote:
 Hi.

 As requested by Richi, I'm suggesting to use new dump_printf
 optimization info infrastructure.

 Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

 Ready to be installed?
 Thanks,
 Martin

 gcc/ChangeLog:

 2019-08-08  Martin Liska  

   * value-prof.c (gimple_divmod_fixed_value_transform):
   Use dump_printf_loc.
   (gimple_mod_pow2_value_transform): Likewise.
   (gimple_mod_subtract_transform): Likewise.
   (init_node_map): Likewise.
   (gimple_ic_transform): Likewise.
   (gimple_stringops_transform): Likewise.

 gcc/testsuite/ChangeLog:

 2019-08-08  Martin Liska  

   * g++.dg/tree-prof/indir-call-prof.C: Add -optimize
   to -fdump-ipa-profile.
   * g++.dg/tree-prof/morefunc.C: Likewise.
   * g++.dg/tree-prof/reorder.C: Likewise.
   * gcc.dg/tree-prof/ic-misattribution-1.c: Likewise.
   * gcc.dg/tree-prof/indir-call-prof.c: Likewise.
   * gcc.dg/tree-prof/stringop-1.c: Likewise.
   * gcc.dg/tree-prof/stringop-2.c: Likewise.
   * gcc.dg/tree-prof/val-prof-1.c: Likewise.
   * gcc.dg/tree-prof/val-prof-2.c: Likewise.
   * gcc.dg/tree-prof/val-prof-3.c: Likewise.
   * gcc.dg/tree-prof/val-prof-4.c: Likewise.
   * gcc.dg/tree-prof/val-prof-5.c: Likewise.
   * gcc.dg/tree-prof/val-prof-7.c: Likewise.
 ---
  .../g++.dg/tree-prof/indir-call-prof.C|   2 +-
 diff --git a/gcc/value-prof.c b/gcc/value-prof.c
 index 759458868a8..9d9785b179d 100644
 --- a/gcc/value-prof.c
 +++ b/gcc/value-prof.c
 @@ -809,12 +809,9 @@ gimple_divmod_fixed_value_transform 
 (gimple_stmt_iterator *si)
 @@ -1445,41 +1447,36 @@ gimple_ic_transform (gimple_stmt_iterator *gsi)
>>> [ ... ]
 -  if (dump_file)
 +  if (dump_enabled_p ())
  {
 -  fprintf (dump_file, "Indirect call -> direct call ");
 -  print_generic_expr (dump_file, gimple_call_fn (stmt), TDF_SLIM);
 -  fprintf (dump_file, "=> ");
 -  print_generic_expr (dump_file, direct_call->decl, TDF_SLIM);
 -  fprintf (dump_file, " transformation on insn postponned to 
 ipa-profile");
 -  print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
 -  fprintf (dump_file, "hist->count %" PRId64
 -" hist->all %" PRId64"\n", count, all);
 +  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,
 +"Indirect call -> direct call "
 +"%T => %T transformation on insn postponed "
 +"to ipa-profile: %G", gimple_call_fn (stmt),
 +direct_call->decl, stmt);
 +  dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt,
 +"hist->count %" PRId64
 +" hist->all %" PRId64"\n", count, all);
  }
>>> It's not entirely clear if you want MSG_OPTIMIZED_LOCATION vs
>>> MSG_MISSED_OPTIMIZATION here.  Double check and adjust if needed.
>> But we don't want multi-line stuff here but a single message for
>> MSG_OPTIMIZED_LOCATION, eventually detail printed with MSG_NOTE.
>> Can you adjust accordingly, esp. try not dumping GIMPLE stmts for
>> the non-NOTE message give it is directed at users.  So just
>>
>>   Indirect call -> direct call %T -> %T transformation
>>
>> (without the postponed stuff, that's implementation detail not interesting).
> Ok, there's a patch that I've just tested.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
>> Richard.
>>
>>> OK with or without that adjustment.
>>>
>>> Jeff
> 
> 
> 0001-Simplify-dump_printf-in-value-prof.c.patch
> 
> From 4eafa3655a6f557d69c2c41e29634a8c805ea8cc Mon Sep 17 00:00:00 2001
> From: Martin Liska 
> Date: Fri, 9 Aug 2019 14:34:55 +0200
> Subject: [PATCH] Simplify dump_printf in value-prof.c
> 
> gcc/ChangeLog:
> 
> 2019-08-09  Martin Liska  
> 
>   * value-prof.c (gimple_ic_transform): Add new line.
>   Print details with MSG_NOTE.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-08-09  Martin Liska  
> 
>   * gcc.dg/tree-prof/ic-misattribution-1.c: Use -fdump-ipa-profile-node.
OK
jeff

Re: [patch] handle casesi dispatch insns in create_trace_edges

[Olivier Hainque]

> On 12 Aug 2019, at 16:48, Jeff Law  wrote:
>> 
>> Searching for tablejump related prototypes in rtl.h led to
>> declarations for functions in rtlanal.c. Wouldn't that be a
>> better place than rtl.c for the new function ?

> I think part of the reason I settled on rtl.c was because we're not
> really just looking at the form, not doing any real "analysis".  BUt I
> think either location for the implementation is fine.

Ok. Both are fine with me; I was just curious if there
was a particular motivation for one or the other.

I had tested with rtlanal but re-building is quick enough so
I'll move to rtl.c to match your suggestion.

Thanks,

Regards,

Olivier

[PATCH] Add missing _mm{256,512}_zext* intrinsics (PRs target/83250, target/91340)

[Jakub Jelinek]

Hi!

The following patch adds 9 missing intrinsics, which are like _mm*_cast*,
but don't leave the upper bits undefined - set them to zero instead.
The implementation uses code that combine manages to optimize well,
the only problem is that as the 512-bit intrinsics are supposed to be
avx512f and some needed intrinsics they'd ideally use are avx512dq, it means
that for _mm512_zextpd128_pd512/_mm512_zextps256_ps512 we emit
vmovaps/vmovapd instead of vmovapd/vmovaps.

I've also discovered that for AVX, there is no test coverage of the various
cast intrinsics, so I've added that too.

The PR has some details on other possible expansions, it would be nice to
optimize also those definitions into the same code, but it will require some
extra define_insn_and_split, though I think that can be done incrementally;
and once done, perhaps we could change the 
_mm512_zextpd128_pd512/_mm512_zextps256_ps512
so that they actually generate the right ps vs. pd variant of move.

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

2019-08-12  Jakub Jelinek  

PR target/83250
PR target/91340
* config/i386/avxintrin.h (_mm256_zextpd128_pd256,
_mm256_zextps128_ps256, _mm256_zextsi128_si256): New intrinsics.
* config/i386/avx512fintrin.h (_mm512_zextpd128_pd512,
_mm512_zextps128_ps512, _mm512_zextsi128_si512, _mm512_zextpd256_pd512,
_mm512_zextps256_ps512, _mm512_zextsi256_si512): Likewise.

* gcc.target/i386/avx-typecast-1.c: New test.
* gcc.target/i386/avx-typecast-2.c: New test.
* gcc.target/i386/avx512f-typecast-2.c: New test.

--- gcc/config/i386/avxintrin.h.jj  2019-08-05 12:25:34.476667673 +0200
+++ gcc/config/i386/avxintrin.h 2019-08-12 14:33:07.905601186 +0200
@@ -1484,6 +1484,26 @@ _mm256_castsi128_si256 (__m128i __A)
   return (__m256i) __builtin_ia32_si256_si ((__v4si)__A);
 }
 
+/* Similarly, but with zero extension instead of undefined values.  */
+
+extern __inline __m256d __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
+_mm256_zextpd128_pd256 (__m128d __A)
+{
+  return _mm256_insertf128_pd (_mm256_setzero_pd (), __A, 0);
+}
+
+extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
+_mm256_zextps128_ps256 (__m128 __A)
+{
+  return _mm256_insertf128_ps (_mm256_setzero_ps (), __A, 0);
+}
+
+extern __inline __m256i __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
+_mm256_zextsi128_si256 (__m128i __A)
+{
+  return _mm256_insertf128_si256 (_mm256_setzero_si256 (), __A, 0);
+}
+
 extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
 _mm256_set_m128 ( __m128 __H, __m128 __L)
 {
--- gcc/config/i386/avx512fintrin.h.jj  2019-07-12 09:34:49.524385009 +0200
+++ gcc/config/i386/avx512fintrin.h 2019-08-12 14:36:52.281169281 +0200
@@ -15437,6 +15437,48 @@ _mm512_castsi256_si512 (__m256i __A)
   return (__m512i)__builtin_ia32_si512_256si ((__v8si)__A);
 }
 
+extern __inline __m512d
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextpd128_pd512 (__m128d __A)
+{
+  return (__m512d) _mm512_insertf32x4 (_mm512_setzero_ps (), (__m128) __A, 0);
+}
+
+extern __inline __m512
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextps128_ps512 (__m128 __A)
+{
+  return _mm512_insertf32x4 (_mm512_setzero_ps (), __A, 0);
+}
+
+extern __inline __m512i
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextsi128_si512 (__m128i __A)
+{
+  return _mm512_inserti32x4 (_mm512_setzero_si512 (), __A, 0);
+}
+
+extern __inline __m512d
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextpd256_pd512 (__m256d __A)
+{
+  return _mm512_insertf64x4 (_mm512_setzero_pd (), __A, 0);
+}
+
+extern __inline __m512
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextps256_ps512 (__m256 __A)
+{
+  return (__m512) _mm512_insertf64x4 (_mm512_setzero_pd (), (__m256d) __A, 0);
+}
+
+extern __inline __m512i
+__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
+_mm512_zextsi256_si512 (__m256i __A)
+{
+  return _mm512_inserti64x4 (_mm512_setzero_si512 (), __A, 0);
+}
+
 extern __inline __mmask16
 __attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
 _mm512_cmpeq_epu32_mask (__m512i __A, __m512i __B)
--- gcc/testsuite/gcc.target/i386/avx-typecast-1.c.jj   2019-08-12 
15:12:51.597209881 +0200
+++ gcc/testsuite/gcc.target/i386/avx-typecast-1.c  2019-08-12 
15:12:47.334274860 +0200
@@ -0,0 +1,83 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx" } */
+/* { dg-require-effective-target avx } */
+
+#include "avx-check.h"
+
+extern int memcmp (const void *, const void *, __SIZE_TYPE__);
+
+void
+avx_test (void)
+{
+  union256i_d  a, ad;
+  union256  b, bd;
+  union256d  c, cd;
+  union128i_d  d, dd;
+  union128  e, ed;
+  union128d  f, fd;
+  int i;
+
+  for (i = 0; i < 8; i++)
+{
+  a

[PATCH] Add noexcept-specifier to std::apply and std::make_from_tuple

[Jonathan Wakely]

When unpacking a std::tuple we know that the std::get calls are
noexcept, so only the invocation (for std::apply) and construction (for
std::make_from_tuple) can throw.

We also know the std::get calls won't throw for a std::array, but this
patch doesn't specialize the variable template for std::array. For an
arbitrary tuple-like type we don't know if the std::get calls will
throw, and so just use a potentially-throwing noexcept-specifier.

* include/std/tuple (__unpack_std_tuple): New variable template and
partial specializations.
(apply, make_from_tuple): Add noexcept-specifier.
* testsuite/20_util/tuple/apply/2.cc: New test.
* testsuite/20_util/tuple/make_from_tuple/2.cc: New test.

Tested x86_64-linux, committed to trunk.


commit 3426bcb0546ef9b4bbc528204da7b79206f8aa87
Author: Jonathan Wakely 
Date:   Thu Aug 8 21:44:13 2019 +0100

Add noexcept-specifier to std::apply and std::make_from_tuple

When unpacking a std::tuple we know that the std::get calls are
noexcept, so only the invocation (for std::apply) and construction (for
std::make_from_tuple) can throw.

We also know the std::get calls won't throw for a std::array, but this
patch doesn't specialize the variable template for std::array. For an
arbitrary tuple-like type we don't know if the std::get calls will
throw, and so just use a potentially-throwing noexcept-specifier.

* include/std/tuple (__unpack_std_tuple): New variable template and
partial specializations.
(apply, make_from_tuple): Add noexcept-specifier.
* testsuite/20_util/tuple/apply/2.cc: New test.
* testsuite/20_util/tuple/make_from_tuple/2.cc: New test.

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 980dd6d6270..dd966b3a0bc 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -1591,6 +1591,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   { }
 
 #if __cplusplus >= 201703L
+
+  // Unpack a std::tuple into a type trait and use its value.
+  // For cv std::tuple<_Up> the result is _Trait<_Tp, cv _Up...>::value.
+  // For cv std::tuple<_Up>& the result is _Trait<_Tp, cv _Up&...>::value.
+  // Otherwise the result is false (because we don't know if std::get throws).
+  template class _Trait, typename _Tp, typename _Tuple>
+inline constexpr bool __unpack_std_tuple = false;
+
+  template class _Trait, typename _Tp, typename... _Up>
+inline constexpr bool __unpack_std_tuple<_Trait, _Tp, tuple<_Up...>>
+  = _Trait<_Tp, _Up...>::value;
+
+  template class _Trait, typename _Tp, typename... _Up>
+inline constexpr bool __unpack_std_tuple<_Trait, _Tp, tuple<_Up...>&>
+  = _Trait<_Tp, _Up&...>::value;
+
+  template class _Trait, typename _Tp, typename... _Up>
+inline constexpr bool __unpack_std_tuple<_Trait, _Tp, const tuple<_Up...>>
+  = _Trait<_Tp, const _Up...>::value;
+
+  template class _Trait, typename _Tp, typename... _Up>
+inline constexpr bool __unpack_std_tuple<_Trait, _Tp, const tuple<_Up...>&>
+  = _Trait<_Tp, const _Up&...>::value;
+
 # define __cpp_lib_apply 201603
 
   template 
@@ -1604,6 +1628,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template 
 constexpr decltype(auto)
 apply(_Fn&& __f, _Tuple&& __t)
+noexcept(__unpack_std_tuple)
 {
   using _Indices
= make_index_sequence>>;
@@ -1622,6 +1647,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template 
 constexpr _Tp
 make_from_tuple(_Tuple&& __t)
+noexcept(__unpack_std_tuple)
 {
   return __make_from_tuple_impl<_Tp>(
 std::forward<_Tuple>(__t),
diff --git a/libstdc++-v3/testsuite/20_util/tuple/apply/2.cc 
b/libstdc++-v3/testsuite/20_util/tuple/apply/2.cc
new file mode 100644
index 000..aa5968f397f
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/tuple/apply/2.cc
@@ -0,0 +1,62 @@
+// Copyright (C) 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
+// .
+
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+// Test noexcept-specifier on std::apply
+
+#include 
+
+using std::tuple;
+using std::declval;
+
+void f1();
+
+static_assert( !noexcept(apply(f1, declval>())) );
+static_assert( !noexcept(apply(f1, declval&>())) );
+stat

Re: [patch] handle casesi dispatch insns in create_trace_edges

[Jeff Law]

On 8/12/19 2:54 AM, Olivier Hainque wrote:
> Hi Jeff,
> 
> Thanks for your prompt feedback :-)
> 
>>>* rtl.h (tablejump_casesi_pattern): New helper, casesi
>>>recognition logic originating from code in cfgrtl.c.
>>>* cfgrtl.c (patch_jump_insn): Use it.
>>>* dwarf2cfi.c (create_trace_edges): Handle casesi patterns.
> 
>> Is there a reason to think the routine is performance critical enough to
>> be inlined?
> 
> No, no particular reason. Just an implicit thought that having a new
> state as close as possible from the previous one was of interest.
> 
>>  If not it would make more sense to me to put it into rtl.c
>> with just a declaration in rtl.h
> 
>> So if it is performance critical, then the patch is OK as-is.  If not,
>> moving the implementation into rtl.c with a declaration in rtl.h should
>> be considered pre-approved -- just post it here for archival purposes.
> 
> Understood, thanks!
> 
> Searching for tablejump related prototypes in rtl.h led to
> declarations for functions in rtlanal.c. Wouldn't that be a
> better place than rtl.c for the new function ?
I think part of the reason I settled on rtl.c was because we're not
really just looking at the form, not doing any real "analysis".  BUt I
think either location for the implementation is fine.

jeff

[C++ PATCH] Adjust -Wsequence-point for C++17 changes (PR c++/91415)

[Jakub Jelinek]

Hi!

The following patch adds some tweaks for -Wsequence-point warning for C++17
and later.  In particular, stop warning about no sequence point in between
<<, >>, ., -> and [] expressions, where E1 is in C++17 sequenced before E2.

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

As mentioned in the PR, this is just part of the needed changes, I've tried
to adjust handling of MODIFY_EXPR, but didn't figure out exactly what needs
to be done, and .* / ->* aren't handled either, and CALL_EXPR needs probably
some verification too.

2019-08-12  Jakub Jelinek  

PR c++/91415
* c-common.c (verify_tree): For LSHIFT_EXPR, RSHIFT_EXPR,
COMPONENT_REF and ARRAY_REF in cxx_dialect >= cxx17 mode handle it
like COMPOUND_EXPR rather than normal expression.

* g++.dg/warn/sequence-pt-4.C: New test.

--- gcc/c-family/c-common.c.jj  2019-08-12 09:45:54.463491950 +0200
+++ gcc/c-family/c-common.c 2019-08-12 12:01:32.783135654 +0200
@@ -1889,6 +1889,7 @@ verify_tree (tree x, struct tlist **pbef
 case COMPOUND_EXPR:
 case TRUTH_ANDIF_EXPR:
 case TRUTH_ORIF_EXPR:
+sequenced_binary:
   tmp_before = tmp_nosp = tmp_list2 = tmp_list3 = 0;
   verify_tree (TREE_OPERAND (x, 0), &tmp_before, &tmp_nosp, NULL_TREE);
   warn_for_collisions (tmp_nosp);
@@ -2031,8 +2035,18 @@ verify_tree (tree x, struct tlist **pbef
  x = TREE_OPERAND (x, 0);
  goto restart;
}
-  gcc_fallthrough ();
+  goto do_default;
+
+case LSHIFT_EXPR:
+case RSHIFT_EXPR:
+case COMPONENT_REF:
+case ARRAY_REF:
+  if (cxx_dialect >= cxx17)
+   goto sequenced_binary;
+  goto do_default;
+
 default:
+do_default:
   /* For other expressions, simply recurse on their operands.
 Manual tail recursion for unary expressions.
 Other non-expressions need not be processed.  */
--- gcc/testsuite/g++.dg/warn/sequence-pt-4.C.jj2019-08-12 
12:16:27.205660149 +0200
+++ gcc/testsuite/g++.dg/warn/sequence-pt-4.C   2019-08-12 12:22:07.540530959 
+0200
@@ -0,0 +1,21 @@
+/* More sequence point warning tests  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+struct S { int a[10]; };
+void bar (int, int, int, int, int, int, int, int);
+
+int
+foo (int i, int x[10][10], int y[10], struct S z[10], struct S *w[10])
+{
+  int b = x[i++][i++]; /* { dg-warning "undefined" "sequence point warning" { 
target c++14_down } } */
+  int c = i++ << i++;  /* { dg-warning "undefined" "sequence point warning" { 
target c++14_down } } */
+  int d = i++ >> i++;  /* { dg-warning "undefined" "sequence point warning" { 
target c++14_down } } */
+  int e = i++ && i++;
+  int f = i++ ? i++ : i++;
+  int g = (i++, i++);
+  int h = z[i++].a[i++];   /* { dg-warning "undefined" "sequence point 
warning" { target c++14_down } } */
+  int j = w[i++]->a[i++];  /* { dg-warning "undefined" "sequence point 
warning" { target c++14_down } } */
+  bar (b, c, d, e, f,g, h, j);
+  return i;
+}

Jakub

Re: [PATCH][RFC][x86] Fix PR91154, add SImode smax, allow SImode add in SSE regs

[Uros Bizjak]

On Mon, Aug 12, 2019 at 2:27 PM Richard Biener  wrote:
>
> On Fri, 9 Aug 2019, Uros Bizjak wrote:
>
> > On Fri, Aug 9, 2019 at 3:00 PM Richard Biener  wrote:
> >
> > > > > > > > > > > > (define_mode_iterator MAXMIN_IMODE [SI "TARGET_SSE4_1"] 
> > > > > > > > > > > > [DI "TARGET_AVX512F"])
> > > > > > > > > > > >
> > > > > > > > > > > > and then we need to split DImode for 32bits, too.
> > > > > > > > > > >
> > > > > > > > > > > For now, please add "TARGET_64BIT && TARGET_AVX512F" for 
> > > > > > > > > > > DImode
> > > > > > > > > > > condition, I'll provide _doubleword splitter later.
> > > > > > > > > >
> > > > > > > > > > Shouldn't that be TARGET_AVX512VL instead?  Or does the 
> > > > > > > > > > insn use %g0 etc.
> > > > > > > > > > to force use of %zmmN?
> > > > > > > > >
> > > > > > > > > It generates V4SI mode, so - yes, AVX512VL.
> > > > > > > >
> > > > > > > > case SMAX:
> > > > > > > > case SMIN:
> > > > > > > > case UMAX:
> > > > > > > > case UMIN:
> > > > > > > >   if ((mode == DImode && (!TARGET_64BIT || 
> > > > > > > > !TARGET_AVX512VL))
> > > > > > > >   || (mode == SImode && !TARGET_SSE4_1))
> > > > > > > > return false;
> > > > > > > >
> > > > > > > > so there's no way to use AVX512VL for 32bit?
> > > > > > >
> > > > > > > There is a way, but on 32bit targets, we need to split DImode
> > > > > > > operation to a sequence of SImode operations for unconverted 
> > > > > > > pattern.
> > > > > > > This is of course doable, but somehow more complex than simply
> > > > > > > emitting a DImode compare + DImode cmove, which is what current
> > > > > > > splitter does. So, a follow-up task.
> > > > > >
> > > > > > Please find attached the complete .md part that enables SImode for
> > > > > > TARGET_SSE4_1 and DImode for TARGET_AVX512VL for both, 32bit and 
> > > > > > 64bit
> > > > > > targets. The patterns also allows for memory operand 2, so STV has
> > > > > > chance to create the vector pattern with implicit load. In case STV
> > > > > > fails, the memory operand 2 is loaded to the register first;  
> > > > > > operand
> > > > > > 2 is used in compare and cmove instruction, so pre-loading of the
> > > > > > operand should be beneficial.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > > Also note, that splitting should happen rarely. Due to the cost
> > > > > > function, STV should effectively always convert minmax to a vector
> > > > > > insn.
> > > > >
> > > > > I've analyzed the 464.h264ref slowdown on Haswell and it is due to
> > > > > this kind of "simple" conversion:
> > > > >
> > > > >   5.50 │1d0:   test   %esi,%es
> > > > >   0.07 │   mov$0x0,%ex
> > > > >│   cmovs  %eax,%es
> > > > >   5.84 │   imul   %r8d,%es
> > > > >
> > > > > to
> > > > >
> > > > >   0.65 │1e0:   vpxor  %xmm0,%xmm0,%xmm0
> > > > >   0.32 │   vpmaxs -0x10(%rsp),%xmm0,%xmm0
> > > > >  40.45 │   vmovd  %xmm0,%eax
> > > > >   2.45 │   imul   %r8d,%eax
> > > > >
> > > > > which looks like a RA artifact in the end.  We spill %esi only
> > > > > with -mstv here as STV introduces a (subreg:V4SI ...) use
> > > > > of a pseudo ultimatively set from di.  STV creates an additional
> > > > > pseudo for this (copy-in) but it places that copy next to the
> > > > > original def rather than next to the start of the chain it
> > > > > converts which is probably the issue why we spill.  And this
> > > > > is because it inserts those at each definition of the pseudo
> > > > > rather than just at the reaching definition(s) or at the
> > > > > uses of the pseudo in the chain (that because there may be
> > > > > defs of that pseudo in the chain itself).  Note that STV emits
> > > > > such "conversion" copies as simple reg-reg moves:
> > > > >
> > > > > (insn 1094 3 4 2 (set (reg:SI 777)
> > > > > (reg/v:SI 438 [ y ])) "refbuf.i":4:1 -1
> > > > >  (nil))
> > > > >
> > > > > but those do not prevail very long (this one gets removed by CSE2).
> > > > > So IRA just sees the (subreg:V4SI (reg/v:SI 438 [ y ]) 0) use
> > > > > and computes
> > > > >
> > > > > r438: preferred NO_REGS, alternative NO_REGS, allocno NO_REGS
> > > > > a297(r438,l0) costs: SSE_REGS:5628,5628 MEM:3618,3618
> > > > >
> > > > > so I wonder if STV shouldn't instead emit gpr->xmm moves
> > > > > here (but I guess nothing again prevents RTL optimizers from
> > > > > combining that with the single-use in the max instruction...).
> > > > >
> > > > > So this boils down to STV splitting live-ranges but other
> > > > > passes undoing that and then RA not considering splitting
> > > > > live-ranges here, arriving at unoptimal allocation.
> > > > >
> > > > > A testcase showing this issue is (simplified from 464.h264ref
> > > > > UMVLine16Y_11):
> > > > >
> > > > > unsigned short
> > > > > UMVLine16Y_11 (short unsigned int * Pic, int y, int width)
> > > > > {
> > > > >   if (y != width)
> > > > > {
> > > > >   y = y < 0 ? 0 : y;
> > > > >   return Pic[y * width];
> > > > >   

Re: [PATCH 5/9] Come up with an abstraction.

[Martin Liška]

On 8/12/19 2:43 PM, Richard Biener wrote:
> On Mon, Aug 12, 2019 at 1:49 PM Martin Liška  wrote:
>>
>> On 8/12/19 1:40 PM, Richard Biener wrote:
>>> On Mon, Aug 12, 2019 at 1:19 PM Martin Liška  wrote:

 On 8/8/19 5:55 PM, Michael Matz wrote:
> Hi,
>
> On Mon, 10 Jun 2019, Martin Liska wrote:
>
>> 2019-07-24  Martin Liska  
>>
>>  * fold-const.c (operand_equal_p): Rename to ...
>>  (operand_compare::operand_equal_p): ... this.
>>  (add_expr):  Rename to ...
>>  (operand_compare::hash_operand): ... this.
>>  (operand_compare::operand_equal_valueize): Likewise.
>>  (operand_compare::hash_operand_valueize): Likewise.
>>  * fold-const.h (operand_equal_p): Set default
>>  value for last argument.
>>  (class operand_compare): New.
>
> Hmpf.  A class without any data?  That doesn't sound like a good design.

 Yes, the base class (current operand_equal_p) does not have a data.
 But the ICF derive class has a data and e.g. 
 func_checker::operand_equal_valueize
 will use m_label_bb_map.get (t1). Which are member data of class 
 func_checker.

> You seem to need it only to have the possibility of virtual functions,
> i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a
> simple distinction of two cases.  What do you think about encoding the
> additional new (ICF) case in the (existing) 'flags' argument to
> operand_equal_p (and in case the ICF flag is set simply call the
> "callback" directly)?

 That's possible. I can add two more callbacks to the operand_equal_p 
 function
 (hash_operand_valueize and operand_equal_valueize).

 Is Richi also supporting this approach?
>>>
>>> I still see no value in the abstraction since you invoke none of the
>>> (virtual) methods from the base class operand_equal_p.
>>
>> I call operand_equal_valueize (and hash_operand) from operand_equal_p.
>> These are then used in IPA ICF (patch 6/9).
> 
> Ugh.  I see you call that after
> 
>   if (TREE_CODE (arg0) != TREE_CODE (arg1))
> {
> ...
> }
>   else
> return false;
> }
> 
> and also after
> 
>   /* Check equality of integer constants before bailing out due to
>  precision differences.  */
>   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
> 
> which means for arg0 == SSA_NAME and arg1 == INTEGER_CST you return false
> instead of valueizing arg0 to the possibly same or same "lose" value
> and returning true.

Yes. ICF does not allow to have anything where TREE_CODEs do not match.

> 
> Also
> 
> +  int val = operand_equal_valueize (arg0, arg1, flags);
> +  if (val == 1)
> +return 1;
> +  if (val == 0)
> +return 0;
> 
> suggests that you pass in arbirtrary trees for "valueization" but it
> isn't actually
> valueization that is performed but instead it should do an alternate 
> comparison
> of arg0 and arg1 with valueization.  Why's this done this way instead of
> sth like
> 
>   if (TREE_CODE (arg0) == SSA_NAME)
>arg0 = operand_equal_valueize (arg0, flags);
>  if (TREE_CODE (arg1) == SSA_NAME)
>arg1 = operand_equal_valueize (arg1, flags);

Because I want to be given a pair of trees about which the function
operand_equal_valueize returns match/no-match/dunno.

> 
> and why's this done with virtual functions rather than a callback that we can
> cheaply check for NULLness in the default implementation?

I can transform it into a hook. But as mentioned I'll need two hooks.

> 
> So - what does ICF want to make "equal" that isn't equal normally and how's
> that "valueization"?

E.g. for a FUNCTION_DECL, ICF always return true because it can only calls
the operand_equal_p after callgraph is compared. Similarly for LABEL_DECLs,
we have a map (m_label_bb_map). Please take a look at patch 6/9 in this
series.

Thanks,
Martin

> 
> Thanks,
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
 Thanks,
 Martin

> IMHO that would also make the logic within
> operand_equal_p clearer, because you don't have to think about all the
> potential callback functions that might be called.
>
>
> Ciao,
> Michael.
>

>>

Re: [PATCH] Teach mklog to reference PRs.

[Martin Liška]

On 8/2/19 8:04 AM, Martin Liška wrote:
> On 8/2/19 7:45 AM, Martin Liška wrote:
>> I agree with that approach.
> 
> I'm sending an example how it would look like for something
> bigger:
> 
> $ git diff 
> e8a3be407068bfb9c82f0f6656b30d26cc2f484a~15..e8a3be407068bfb9c82f0f6656b30d26cc2f484a
>  > patch && ./contrib/mklog patch > changelog.txt
> 
> Thoughts?
> 
> Martin
> 

Hello.

I've reconsidered the ChangeLog format and I would like to "revert" my decision.
I prefer to stay with the current format because:

1) each changelog entry can directly be copied to a ChangeLog file;
   no need for appending PRxxx and authors from the changelog
2) I have a working script that applies such patches to SVN:
   https://github.com/marxin/gcc-util/blob/master/boilerplate/applypatch.py

  If there's an interest, I can clean it up and suggest to contrib

3) I also have a script that extracts patch and changelog entries back
   to email message:
   
https://github.com/marxin/script-misc/blob/master/gcc-extract-backport-patches.py

  That's handy for backporting where one wants to not have a ChangeLog 
collisions.
  Then I use applypatch.py --backport to apply extracted changes with 'Backport 
from mainline'
  header.

What others do for patch application and back extracting for backport testing?
I'm sending an example of patch backport for 
2a245bc81b9e09cba4930d12d0417da229c37d67.

Martin
>From 2a245bc81b9e09cba4930d12d0417da229c37d67 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 22 Jul 2019 07:34:10 +
Subject: Backport r273660

gcc/ChangeLog:

2019-07-22  Martin Liska  

	PR driver/91172
	* opts-common.c (decode_cmdline_option): Decode
	argument of -Werror and check it for a wrong language.
	* opts-global.c (complain_wrong_lang): Remove such case.

gcc/testsuite/ChangeLog:

2019-07-22  Martin Liska  

	PR driver/91172
	* gcc.dg/pr91172.c: New test.

---
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 660dfe63858..e3f9c549b10 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -537,7 +537,8 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,

   extra_args = 0;

-  opt_index = find_opt (argv[0] + 1, lang_mask);
+  const char *opt_value = argv[0] + 1;
+  opt_index = find_opt (opt_value, lang_mask);
   i = 0;
   while (opt_index == OPT_SPECIAL_unknown
 	 && i < ARRAY_SIZE (option_map))
@@ -745,6 +746,23 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
   /* Check if this is a switch for a different front end.  */
   if (!option_ok_for_language (option, lang_mask))
 errors |= CL_ERR_WRONG_LANG;
+  else if (strcmp (option->opt_text, "-Werror=") == 0
+	   && strchr (opt_value, ',') == NULL)
+{
+  /* Verify that -Werror argument is a valid warning
+	 for a language.  */
+  char *werror_arg = xstrdup (opt_value + 6);
+  werror_arg[0] = 'W';
+
+  size_t warning_index = find_opt (werror_arg, lang_mask);
+  if (warning_index != OPT_SPECIAL_unknown)
+	{
+	  const struct cl_option *warning_option
+	= &cl_options[warning_index];
+	  if (!option_ok_for_language (warning_option, lang_mask))
+	errors |= CL_ERR_WRONG_LANG;
+	}
+}

   /* Convert the argument to lowercase if appropriate.  */
   if (arg && option->cl_tolower)
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index bf4db775928..7c5bd16c7ea 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -103,10 +103,14 @@ complain_wrong_lang (const struct cl_decoded_option *decoded,
 	   text, bad_lang);
   else if (lang_mask == CL_DRIVER)
 gcc_unreachable ();
-  else
+  else if (ok_langs[0] != '\0')
 /* Eventually this should become a hard error IMO.  */
 warning (0, "command-line option %qs is valid for %s but not for %s",
 	 text, ok_langs, bad_lang);
+  else
+/* Happens for -Werror=warning_name.  */
+warning (0, "%<-Werror=%> argument %qs is not valid for %s",
+	 text, bad_lang);

   free (ok_langs);
   free (bad_lang);
diff --git a/gcc/testsuite/gcc.dg/pr91172.c b/gcc/testsuite/gcc.dg/pr91172.c
new file mode 100644
index 000..a38a0580f4a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91172.c
@@ -0,0 +1,3 @@
+/* { dg-do compile } */
+/* { dg-options "-Werror=target-lifetime" } */
+/* { dg-warning "'-Werror\=' argument '-Werror=target-lifetime' is not valid for C" "" { target *-*-* } 0 } */
--
2.22.0

Re: [PATCH] fold more string comparison with known result (PR 90879)

[Michael Matz]

Hi,

On Fri, 9 Aug 2019, Martin Sebor wrote:

> The solution introduced in C99 is a flexible array.  C++
> compilers usually support it as well.  Those that don't are
> likely to support the zero-length array (even Visual C++ does).
> If there's a chance that some don't support either do you really
> think it's safe to assume they will do something sane with
> the [1] hack?

As the [1] "hack" is the traditional pre-C99 (and C++) idiom to 
implement flexible trailing char arrays, yes, I do expect all existing 
(and not any more existing) compilers to do the obvious and sane thing 
with it.  IOW: it's more portable in practice than our documented 
zero-length extension.  And that's what matters for the things compiled by 
the host compiler.

Without requiring C99 (which would be a different discussion) and a 
non-existing C++ standard we can't write this code (in this form) in a 
standard conforming way, no matter what we wish for.  Hence it seems 
prudent to use the most portable variant of all the non-standard ways, the 
trailing [1] array.


Ciao,
Michael.

Re: [Patch, ira] Invalid assert in reload1.c::finish_spills?

[Vladimir Makarov]

On 2019-08-09 5:23 p.m., Jeff Law wrote:

I've committed it after letting it bootstrap/regression test on ppc64le,
aarch64 and others.  It also didn't regress on any of the *-elf targets
I'm testing which are far more likely to exercise this code.

Thank you, Jeff.

Re: [PATCH 5/9] Come up with an abstraction.

[Michael Matz]

Hi,

On Mon, 12 Aug 2019, Martin Liška wrote:

> > You seem to need it only to have the possibility of virtual functions, 
> > i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a 
> > simple distinction of two cases.  What do you think about encoding the 
> > additional new (ICF) case in the (existing) 'flags' argument to 
> > operand_equal_p (and in case the ICF flag is set simply call the 
> > "callback" directly)?
> 
> That's possible. I can add two more callbacks to the operand_equal_p 
> function (hash_operand_valueize and operand_equal_valueize).

That's premature; why provide callbacks when it's always either NULL or 
a single value?  What I meant is put code like this into operand_equal_p:

  if (flags & OE_FOR_ICF)
op0 = oep_icf_valueize (op0);
...

Less indirection, less confusion.


Ciao,
Michael.

Re: [PATCH 5/9] Come up with an abstraction.

[Richard Biener]

On Mon, Aug 12, 2019 at 1:49 PM Martin Liška  wrote:
>
> On 8/12/19 1:40 PM, Richard Biener wrote:
> > On Mon, Aug 12, 2019 at 1:19 PM Martin Liška  wrote:
> >>
> >> On 8/8/19 5:55 PM, Michael Matz wrote:
> >>> Hi,
> >>>
> >>> On Mon, 10 Jun 2019, Martin Liska wrote:
> >>>
>  2019-07-24  Martin Liska  
> 
>   * fold-const.c (operand_equal_p): Rename to ...
>   (operand_compare::operand_equal_p): ... this.
>   (add_expr):  Rename to ...
>   (operand_compare::hash_operand): ... this.
>   (operand_compare::operand_equal_valueize): Likewise.
>   (operand_compare::hash_operand_valueize): Likewise.
>   * fold-const.h (operand_equal_p): Set default
>   value for last argument.
>   (class operand_compare): New.
> >>>
> >>> Hmpf.  A class without any data?  That doesn't sound like a good design.
> >>
> >> Yes, the base class (current operand_equal_p) does not have a data.
> >> But the ICF derive class has a data and e.g. 
> >> func_checker::operand_equal_valueize
> >> will use m_label_bb_map.get (t1). Which are member data of class 
> >> func_checker.
> >>
> >>> You seem to need it only to have the possibility of virtual functions,
> >>> i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a
> >>> simple distinction of two cases.  What do you think about encoding the
> >>> additional new (ICF) case in the (existing) 'flags' argument to
> >>> operand_equal_p (and in case the ICF flag is set simply call the
> >>> "callback" directly)?
> >>
> >> That's possible. I can add two more callbacks to the operand_equal_p 
> >> function
> >> (hash_operand_valueize and operand_equal_valueize).
> >>
> >> Is Richi also supporting this approach?
> >
> > I still see no value in the abstraction since you invoke none of the
> > (virtual) methods from the base class operand_equal_p.
>
> I call operand_equal_valueize (and hash_operand) from operand_equal_p.
> These are then used in IPA ICF (patch 6/9).

Ugh.  I see you call that after

  if (TREE_CODE (arg0) != TREE_CODE (arg1))
{
...
}
  else
return false;
}

and also after

  /* Check equality of integer constants before bailing out due to
 precision differences.  */
  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)

which means for arg0 == SSA_NAME and arg1 == INTEGER_CST you return false
instead of valueizing arg0 to the possibly same or same "lose" value
and returning true.

Also

+  int val = operand_equal_valueize (arg0, arg1, flags);
+  if (val == 1)
+return 1;
+  if (val == 0)
+return 0;

suggests that you pass in arbirtrary trees for "valueization" but it
isn't actually
valueization that is performed but instead it should do an alternate comparison
of arg0 and arg1 with valueization.  Why's this done this way instead of
sth like

  if (TREE_CODE (arg0) == SSA_NAME)
   arg0 = operand_equal_valueize (arg0, flags);
 if (TREE_CODE (arg1) == SSA_NAME)
   arg1 = operand_equal_valueize (arg1, flags);

and why's this done with virtual functions rather than a callback that we can
cheaply check for NULLness in the default implementation?

So - what does ICF want to make "equal" that isn't equal normally and how's
that "valueization"?

Thanks,
Richard.

> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >>> IMHO that would also make the logic within
> >>> operand_equal_p clearer, because you don't have to think about all the
> >>> potential callback functions that might be called.
> >>>
> >>>
> >>> Ciao,
> >>> Michael.
> >>>
> >>
>

Re: [PATCH][RFC][x86] Fix PR91154, add SImode smax, allow SImode add in SSE regs

[Richard Biener]

On Fri, 9 Aug 2019, Uros Bizjak wrote:

> On Fri, Aug 9, 2019 at 3:00 PM Richard Biener  wrote:
> 
> > > > > > > > > > > (define_mode_iterator MAXMIN_IMODE [SI "TARGET_SSE4_1"] 
> > > > > > > > > > > [DI "TARGET_AVX512F"])
> > > > > > > > > > >
> > > > > > > > > > > and then we need to split DImode for 32bits, too.
> > > > > > > > > >
> > > > > > > > > > For now, please add "TARGET_64BIT && TARGET_AVX512F" for 
> > > > > > > > > > DImode
> > > > > > > > > > condition, I'll provide _doubleword splitter later.
> > > > > > > > >
> > > > > > > > > Shouldn't that be TARGET_AVX512VL instead?  Or does the insn 
> > > > > > > > > use %g0 etc.
> > > > > > > > > to force use of %zmmN?
> > > > > > > >
> > > > > > > > It generates V4SI mode, so - yes, AVX512VL.
> > > > > > >
> > > > > > > case SMAX:
> > > > > > > case SMIN:
> > > > > > > case UMAX:
> > > > > > > case UMIN:
> > > > > > >   if ((mode == DImode && (!TARGET_64BIT || !TARGET_AVX512VL))
> > > > > > >   || (mode == SImode && !TARGET_SSE4_1))
> > > > > > > return false;
> > > > > > >
> > > > > > > so there's no way to use AVX512VL for 32bit?
> > > > > >
> > > > > > There is a way, but on 32bit targets, we need to split DImode
> > > > > > operation to a sequence of SImode operations for unconverted 
> > > > > > pattern.
> > > > > > This is of course doable, but somehow more complex than simply
> > > > > > emitting a DImode compare + DImode cmove, which is what current
> > > > > > splitter does. So, a follow-up task.
> > > > >
> > > > > Please find attached the complete .md part that enables SImode for
> > > > > TARGET_SSE4_1 and DImode for TARGET_AVX512VL for both, 32bit and 64bit
> > > > > targets. The patterns also allows for memory operand 2, so STV has
> > > > > chance to create the vector pattern with implicit load. In case STV
> > > > > fails, the memory operand 2 is loaded to the register first;  operand
> > > > > 2 is used in compare and cmove instruction, so pre-loading of the
> > > > > operand should be beneficial.
> > > >
> > > > Thanks.
> > > >
> > > > > Also note, that splitting should happen rarely. Due to the cost
> > > > > function, STV should effectively always convert minmax to a vector
> > > > > insn.
> > > >
> > > > I've analyzed the 464.h264ref slowdown on Haswell and it is due to
> > > > this kind of "simple" conversion:
> > > >
> > > >   5.50 │1d0:   test   %esi,%es
> > > >   0.07 │   mov$0x0,%ex
> > > >│   cmovs  %eax,%es
> > > >   5.84 │   imul   %r8d,%es
> > > >
> > > > to
> > > >
> > > >   0.65 │1e0:   vpxor  %xmm0,%xmm0,%xmm0
> > > >   0.32 │   vpmaxs -0x10(%rsp),%xmm0,%xmm0
> > > >  40.45 │   vmovd  %xmm0,%eax
> > > >   2.45 │   imul   %r8d,%eax
> > > >
> > > > which looks like a RA artifact in the end.  We spill %esi only
> > > > with -mstv here as STV introduces a (subreg:V4SI ...) use
> > > > of a pseudo ultimatively set from di.  STV creates an additional
> > > > pseudo for this (copy-in) but it places that copy next to the
> > > > original def rather than next to the start of the chain it
> > > > converts which is probably the issue why we spill.  And this
> > > > is because it inserts those at each definition of the pseudo
> > > > rather than just at the reaching definition(s) or at the
> > > > uses of the pseudo in the chain (that because there may be
> > > > defs of that pseudo in the chain itself).  Note that STV emits
> > > > such "conversion" copies as simple reg-reg moves:
> > > >
> > > > (insn 1094 3 4 2 (set (reg:SI 777)
> > > > (reg/v:SI 438 [ y ])) "refbuf.i":4:1 -1
> > > >  (nil))
> > > >
> > > > but those do not prevail very long (this one gets removed by CSE2).
> > > > So IRA just sees the (subreg:V4SI (reg/v:SI 438 [ y ]) 0) use
> > > > and computes
> > > >
> > > > r438: preferred NO_REGS, alternative NO_REGS, allocno NO_REGS
> > > > a297(r438,l0) costs: SSE_REGS:5628,5628 MEM:3618,3618
> > > >
> > > > so I wonder if STV shouldn't instead emit gpr->xmm moves
> > > > here (but I guess nothing again prevents RTL optimizers from
> > > > combining that with the single-use in the max instruction...).
> > > >
> > > > So this boils down to STV splitting live-ranges but other
> > > > passes undoing that and then RA not considering splitting
> > > > live-ranges here, arriving at unoptimal allocation.
> > > >
> > > > A testcase showing this issue is (simplified from 464.h264ref
> > > > UMVLine16Y_11):
> > > >
> > > > unsigned short
> > > > UMVLine16Y_11 (short unsigned int * Pic, int y, int width)
> > > > {
> > > >   if (y != width)
> > > > {
> > > >   y = y < 0 ? 0 : y;
> > > >   return Pic[y * width];
> > > > }
> > > >   return Pic[y];
> > > > }
> > > >
> > > > where the condition and the Pic[y] load mimics the other use of y.
> > > > Different, even worse spilling is generated by
> > > >
> > > > unsigned short
> > > > UMVLine16Y_11 (short unsigned int * Pic, int y, int width)
> > > > {
> > > >   y = y < 0 ? 0 : y

Re: [PATCH 5/9] Come up with an abstraction.

[Martin Liška]

On 8/12/19 1:40 PM, Richard Biener wrote:
> On Mon, Aug 12, 2019 at 1:19 PM Martin Liška  wrote:
>>
>> On 8/8/19 5:55 PM, Michael Matz wrote:
>>> Hi,
>>>
>>> On Mon, 10 Jun 2019, Martin Liska wrote:
>>>
 2019-07-24  Martin Liska  

  * fold-const.c (operand_equal_p): Rename to ...
  (operand_compare::operand_equal_p): ... this.
  (add_expr):  Rename to ...
  (operand_compare::hash_operand): ... this.
  (operand_compare::operand_equal_valueize): Likewise.
  (operand_compare::hash_operand_valueize): Likewise.
  * fold-const.h (operand_equal_p): Set default
  value for last argument.
  (class operand_compare): New.
>>>
>>> Hmpf.  A class without any data?  That doesn't sound like a good design.
>>
>> Yes, the base class (current operand_equal_p) does not have a data.
>> But the ICF derive class has a data and e.g. 
>> func_checker::operand_equal_valueize
>> will use m_label_bb_map.get (t1). Which are member data of class 
>> func_checker.
>>
>>> You seem to need it only to have the possibility of virtual functions,
>>> i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a
>>> simple distinction of two cases.  What do you think about encoding the
>>> additional new (ICF) case in the (existing) 'flags' argument to
>>> operand_equal_p (and in case the ICF flag is set simply call the
>>> "callback" directly)?
>>
>> That's possible. I can add two more callbacks to the operand_equal_p function
>> (hash_operand_valueize and operand_equal_valueize).
>>
>> Is Richi also supporting this approach?
> 
> I still see no value in the abstraction since you invoke none of the
> (virtual) methods from the base class operand_equal_p.

I call operand_equal_valueize (and hash_operand) from operand_equal_p.
These are then used in IPA ICF (patch 6/9).

Martin

> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>>> IMHO that would also make the logic within
>>> operand_equal_p clearer, because you don't have to think about all the
>>> potential callback functions that might be called.
>>>
>>>
>>> Ciao,
>>> Michael.
>>>
>>

Re: [PATCH 5/9] Come up with an abstraction.

[Richard Biener]

On Mon, Aug 12, 2019 at 1:19 PM Martin Liška  wrote:
>
> On 8/8/19 5:55 PM, Michael Matz wrote:
> > Hi,
> >
> > On Mon, 10 Jun 2019, Martin Liska wrote:
> >
> >> 2019-07-24  Martin Liska  
> >>
> >>  * fold-const.c (operand_equal_p): Rename to ...
> >>  (operand_compare::operand_equal_p): ... this.
> >>  (add_expr):  Rename to ...
> >>  (operand_compare::hash_operand): ... this.
> >>  (operand_compare::operand_equal_valueize): Likewise.
> >>  (operand_compare::hash_operand_valueize): Likewise.
> >>  * fold-const.h (operand_equal_p): Set default
> >>  value for last argument.
> >>  (class operand_compare): New.
> >
> > Hmpf.  A class without any data?  That doesn't sound like a good design.
>
> Yes, the base class (current operand_equal_p) does not have a data.
> But the ICF derive class has a data and e.g. 
> func_checker::operand_equal_valueize
> will use m_label_bb_map.get (t1). Which are member data of class func_checker.
>
> > You seem to need it only to have the possibility of virtual functions,
> > i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a
> > simple distinction of two cases.  What do you think about encoding the
> > additional new (ICF) case in the (existing) 'flags' argument to
> > operand_equal_p (and in case the ICF flag is set simply call the
> > "callback" directly)?
>
> That's possible. I can add two more callbacks to the operand_equal_p function
> (hash_operand_valueize and operand_equal_valueize).
>
> Is Richi also supporting this approach?

I still see no value in the abstraction since you invoke none of the
(virtual) methods from the base class operand_equal_p.

Richard.

> Thanks,
> Martin
>
> > IMHO that would also make the logic within
> > operand_equal_p clearer, because you don't have to think about all the
> > potential callback functions that might be called.
> >
> >
> > Ciao,
> > Michael.
> >
>

Re: [PATCH 5/9] Come up with an abstraction.

[Martin Liška]

On 8/8/19 5:55 PM, Michael Matz wrote:
> Hi,
> 
> On Mon, 10 Jun 2019, Martin Liska wrote:
> 
>> 2019-07-24  Martin Liska  
>>
>>  * fold-const.c (operand_equal_p): Rename to ...
>>  (operand_compare::operand_equal_p): ... this.
>>  (add_expr):  Rename to ...
>>  (operand_compare::hash_operand): ... this.
>>  (operand_compare::operand_equal_valueize): Likewise.
>>  (operand_compare::hash_operand_valueize): Likewise.
>>  * fold-const.h (operand_equal_p): Set default
>>  value for last argument.
>>  (class operand_compare): New.
> 
> Hmpf.  A class without any data?  That doesn't sound like a good design.

Yes, the base class (current operand_equal_p) does not have a data.
But the ICF derive class has a data and e.g. 
func_checker::operand_equal_valueize
will use m_label_bb_map.get (t1). Which are member data of class func_checker.
  
> You seem to need it only to have the possibility of virtual functions, 
> i.e. fancy callbacks.  AFAICS you only have one derived class, i.e. a 
> simple distinction of two cases.  What do you think about encoding the 
> additional new (ICF) case in the (existing) 'flags' argument to 
> operand_equal_p (and in case the ICF flag is set simply call the 
> "callback" directly)?

That's possible. I can add two more callbacks to the operand_equal_p function
(hash_operand_valueize and operand_equal_valueize).

Is Richi also supporting this approach?
Thanks,
Martin

> IMHO that would also make the logic within 
> operand_equal_p clearer, because you don't have to think about all the 
> potential callback functions that might be called.
> 
> 
> Ciao,
> Michael.
> 

Re: PR driver/91130 Use CL_DRIVER when handling of COLLECT_GCC_OPTIONS in lto-wrapper.c

[Richard Biener]

On Mon, 12 Aug 2019, Richard Biener wrote:

> On Wed, 7 Aug 2019, Richard Earnshaw (lists) wrote:
> 
> > On 07/08/2019 17:15, Jakub Jelinek wrote:
> > > On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote:
> > > > > Though, I'm fine if you commit your patch now as a fix and Richi's 
> > > > > patch
> > > > > with the above incremental change is applied on top of it 
> > > > > incrementally
> > > > > as a cleanup.
> > > > > 
> > > > >   Jakub
> > > > > 
> > > > 
> > > > Ok, I'll do that.
> > > 
> > > Thanks.
> > > 
> > > > Do you want it on the gcc-9 branch as well?  I'm running
> > > > a bootstrap of it right now.
> > > 
> > > Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week?
> > > 
> > >   Jakub
> > > 
> > 
> > I'm OoO next week, but can do it when I get back.
> 
> I installed the following followup after testing on 
> x86_64-unknown-linux-gnu.

And dealing with the backporting now.

Richard.

> Richard.
> 
> 2019-08-12  Richard Biener  
> 
>   PR driver/91130
>   * lto-wrapper.c (get_options_from_collect_gcc_options): Remove
>   lang_mask option, always use CL_DRIVER.
>   (get_options_from_collect_gcc_options): Adjust.
>   (find_and_merge_options): Likewise.
>   (run_gcc): Likewise.
> 
> Index: gcc/lto-wrapper.c
> ===
> --- gcc/lto-wrapper.c (revision 274235)
> +++ gcc/lto-wrapper.c (working copy)
> @@ -128,12 +128,11 @@ maybe_unlink (const char *file)
>  #define DUMPBASE_SUFFIX ".ltrans18446744073709551615"
>  
>  /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS
> -   environment according to LANG_MASK.  */
> +   environment.  */
>  
>  static void
>  get_options_from_collect_gcc_options (const char *collect_gcc,
> const char *collect_gcc_options,
> -   unsigned int lang_mask,
> struct cl_decoded_option 
> **decoded_options,
> unsigned int *decoded_options_count)
>  {
> @@ -176,8 +175,7 @@ get_options_from_collect_gcc_options (co
>argc = obstack_object_size (&argv_obstack) / sizeof (void *) - 1;
>argv = XOBFINISH (&argv_obstack, const char **);
>  
> -  decode_cmdline_options_to_array (argc, (const char **)argv,
> -lang_mask,
> +  decode_cmdline_options_to_array (argc, (const char **)argv, CL_DRIVER,
>  decoded_options, decoded_options_count);
>obstack_free (&argv_obstack, NULL);
>  }
> @@ -1009,8 +1007,7 @@ find_and_merge_options (int fd, off_t fi
>  {
>struct cl_decoded_option *f2decoded_options;
>unsigned int f2decoded_options_count;
> -  get_options_from_collect_gcc_options (collect_gcc,
> - fopts, CL_DRIVER,
> +  get_options_from_collect_gcc_options (collect_gcc, fopts,
>   &f2decoded_options,
>   &f2decoded_options_count);
>if (!fdecoded_options)
> @@ -1282,7 +1279,6 @@ run_gcc (unsigned argc, char *argv[])
>  fatal_error (input_location,
>"environment variable % must be set");
>get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options,
> - CL_DRIVER,
>   &decoded_options,
>   &decoded_options_count);
>  
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

[PATCH] Fix PR91375

[Richard Biener]

Bootstrapped & tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2019-08-12  Richard Biener  

PR lto/91375
* tree.c (free_lang_data_in_type): Do not free TYPE_BINFO dependent on
flag_devirtualize.

Index: gcc/tree.c
===
--- gcc/tree.c  (revision 274308)
+++ gcc/tree.c  (working copy)
@@ -5531,8 +5531,7 @@ free_lang_data_in_type (tree type, class
  free_lang_data_in_binfo (TYPE_BINFO (type));
  /* We need to preserve link to bases and virtual table for all
 polymorphic types to make devirtualization machinery working.  */
- if (!BINFO_VTABLE (TYPE_BINFO (type))
- || !flag_devirtualize)
+ if (!BINFO_VTABLE (TYPE_BINFO (type)))
TYPE_BINFO (type) = NULL;
}
 }

Re: PR driver/91130 Use CL_DRIVER when handling of COLLECT_GCC_OPTIONS in lto-wrapper.c

[Richard Biener]

On Wed, 7 Aug 2019, Richard Earnshaw (lists) wrote:

> On 07/08/2019 17:15, Jakub Jelinek wrote:
> > On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote:
> > > > Though, I'm fine if you commit your patch now as a fix and Richi's patch
> > > > with the above incremental change is applied on top of it incrementally
> > > > as a cleanup.
> > > > 
> > > > Jakub
> > > > 
> > > 
> > > Ok, I'll do that.
> > 
> > Thanks.
> > 
> > > Do you want it on the gcc-9 branch as well?  I'm running
> > > a bootstrap of it right now.
> > 
> > Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week?
> > 
> > Jakub
> > 
> 
> I'm OoO next week, but can do it when I get back.

I installed the following followup after testing on 
x86_64-unknown-linux-gnu.

Richard.

2019-08-12  Richard Biener  

PR driver/91130
* lto-wrapper.c (get_options_from_collect_gcc_options): Remove
lang_mask option, always use CL_DRIVER.
(get_options_from_collect_gcc_options): Adjust.
(find_and_merge_options): Likewise.
(run_gcc): Likewise.

Index: gcc/lto-wrapper.c
===
--- gcc/lto-wrapper.c   (revision 274235)
+++ gcc/lto-wrapper.c   (working copy)
@@ -128,12 +128,11 @@ maybe_unlink (const char *file)
 #define DUMPBASE_SUFFIX ".ltrans18446744073709551615"
 
 /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS
-   environment according to LANG_MASK.  */
+   environment.  */
 
 static void
 get_options_from_collect_gcc_options (const char *collect_gcc,
  const char *collect_gcc_options,
- unsigned int lang_mask,
  struct cl_decoded_option 
**decoded_options,
  unsigned int *decoded_options_count)
 {
@@ -176,8 +175,7 @@ get_options_from_collect_gcc_options (co
   argc = obstack_object_size (&argv_obstack) / sizeof (void *) - 1;
   argv = XOBFINISH (&argv_obstack, const char **);
 
-  decode_cmdline_options_to_array (argc, (const char **)argv,
-  lang_mask,
+  decode_cmdline_options_to_array (argc, (const char **)argv, CL_DRIVER,
   decoded_options, decoded_options_count);
   obstack_free (&argv_obstack, NULL);
 }
@@ -1009,8 +1007,7 @@ find_and_merge_options (int fd, off_t fi
 {
   struct cl_decoded_option *f2decoded_options;
   unsigned int f2decoded_options_count;
-  get_options_from_collect_gcc_options (collect_gcc,
-   fopts, CL_DRIVER,
+  get_options_from_collect_gcc_options (collect_gcc, fopts,
&f2decoded_options,
&f2decoded_options_count);
   if (!fdecoded_options)
@@ -1282,7 +1279,6 @@ run_gcc (unsigned argc, char *argv[])
 fatal_error (input_location,
 "environment variable % must be set");
   get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options,
-   CL_DRIVER,
&decoded_options,
&decoded_options_count);
 

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener]

On Mon, Aug 12, 2019 at 12:49 PM Richard Biener
 wrote:
>
> On Mon, Aug 12, 2019 at 12:43 PM Martin Liška  wrote:
> >
> > On 8/12/19 12:39 PM, Richard Biener wrote:
> > > which is the "other" body for 'open' but it shouldn't really be output
> > > to the symbol table.  Still we want to emit its body for the purpose
> > > of inlining.  So IMHO the fix is not to do magic '0' appending for
> > > the user-asm-name case but instead "mangling" of extern inline
> > > bodies because those are the speciality causing the issue in the end.
> >
> > Ok, so please advise me how can we achieve that?
>
> I'd change lto_get_section_name to take the fndecl instead of name
> and key on DECL_EXTERNAL/DECL_INLINE (or how we identify
> extern gnu-inline functions), pre-pending a zero.

Or even "simpler"(?) by mangling DECL_ASSEMBLER_NAME for
these functions throughout GCC - they should never be emitted.

Honza?

Richard.

> Richard.
>
> > Martin

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener]

On Mon, Aug 12, 2019 at 12:43 PM Martin Liška  wrote:
>
> On 8/12/19 12:39 PM, Richard Biener wrote:
> > which is the "other" body for 'open' but it shouldn't really be output
> > to the symbol table.  Still we want to emit its body for the purpose
> > of inlining.  So IMHO the fix is not to do magic '0' appending for
> > the user-asm-name case but instead "mangling" of extern inline
> > bodies because those are the speciality causing the issue in the end.
>
> Ok, so please advise me how can we achieve that?

I'd change lto_get_section_name to take the fndecl instead of name
and key on DECL_EXTERNAL/DECL_INLINE (or how we identify
extern gnu-inline functions), pre-pending a zero.

Richard.

> Martin

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener]

On Mon, Aug 12, 2019 at 12:39 PM Richard Biener
 wrote:
>
> On Mon, Aug 12, 2019 at 11:57 AM Martin Liška  wrote:
> >
> > On 8/12/19 11:45 AM, Richard Biener wrote:
> > > On Fri, Aug 9, 2019 at 3:57 PM Martin Liška  wrote:
> > >>
> > >> Hi.
> > >>
> > >> The patch is about prevention of LTO section name clashing.
> > >> Now we have a situation where body of 2 functions is streamed
> > >> into the same ELF section. Then we'll end up with smashed data.
> > >>
> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > >>
> > >> Ready to be installed?
> > >
> > > I think the comment should mention why we skip a leading '*'
> > > at all.
> >
> > git blame helps us here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42531
> >
> >
> > > IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME?
> >
> > Yes, it's prepended here:
> > set_user_assembler_name
> >
> > > And section names cannot contain '*'?
> >
> > As seen in the PR, not.
> >
> > > Or do we need to actually
> > > indentify '*fn' and 'fn' as the same?
> >
> > No, they should be identified as different symbols.
> >
> > >  For the testcase what is the clashing
> > > symbol?
> >
> > void __open_alias(int, ...) __asm__("open"); - aka *open
> > and:
> > +extern __inline __attribute__((__gnu_inline__)) int open() {}
>
> Hmm, so for
>
> void __open_alias(int, ...) __asm__("open");
> void __open_alias(int flags, ...) {}
>
> a non-LTO compile will output __open_alias with the symbol "open".
> Then we have
>
> extern __inline __attribute__((__gnu_inline__)) int open() {}
>
> which is the "other" body for 'open' but it shouldn't really be output
> to the symbol table.  Still we want to emit its body for the purpose
> of inlining.  So IMHO the fix is not to do magic '0' appending for
> the user-asm-name case but instead "mangling" of extern inline
> bodies because those are the speciality causing the issue in the end.
>
> >
> > >  Can't we have many so that using "0" always is broken as well?
> >
> > If I'll define 2 symbols that alias to a same asm name, I'll get:
> > $ cat clash.c
> > void __open_alias(int, ...) __asm__("open");
> > void __open_alias2(int, ...) __asm__("open");
> > void __open_alias(int flags, ...) {}
> > void __open_alias2(int flags, ...) {}
> > extern __inline __attribute__((__gnu_inline__)) int open() {}
> > struct {
> >   void *func;
> > } a = {open};
> >
> > int main()
> > {
> >   return 0;
> > }
> >
> > $ gcc clash.c -flto
> > lto1: fatal error: missing resolution data for *open
> > compilation terminated.
> >
> > Which is a reasonable error message to me.
>
> Here I don't agree, earlier diagnostic of such invalid testcase
> would be welcome instead.  If you build w/o LTO you'll get
>
> /tmp/ccjjlMhr.s: Assembler messages:
> /tmp/ccjjlMhr.s:40: Error: symbol `open' is already defined
>
> IMHO this is invalid (undiagnosed) C.

Btw, with -flto we also only get a single function section for
both (not sure if the bytecode ends up sensible).

> Richard.
>
>
>
> > Martin
> >
> > >
> > > Richard.
> > >
> > >> Thanks,
> > >> Martin
> > >>
> > >> gcc/ChangeLog:
> > >>
> > >> 2019-08-09  Martin Liska  
> > >>
> > >> PR lto/91393
> > >> PR lto/88220
> > >> * lto-streamer.c (lto_get_section_name): Replace '*' leading
> > >> character with '0'.
> > >>
> > >> gcc/testsuite/ChangeLog:
> > >>
> > >> 2019-08-09  Martin Liska  
> > >>
> > >> PR lto/91393
> > >> PR lto/88220
> > >> * gcc.dg/lto/pr91393_0.c: New test.
> > >> ---
> > >>  gcc/lto-streamer.c   | 15 ---
> > >>  gcc/testsuite/gcc.dg/lto/pr91393_0.c | 11 +++
> > >>  2 files changed, 23 insertions(+), 3 deletions(-)
> > >>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr91393_0.c
> > >>
> > >>
> >

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Martin Liška]

On 8/12/19 12:39 PM, Richard Biener wrote:
> which is the "other" body for 'open' but it shouldn't really be output
> to the symbol table.  Still we want to emit its body for the purpose
> of inlining.  So IMHO the fix is not to do magic '0' appending for
> the user-asm-name case but instead "mangling" of extern inline
> bodies because those are the speciality causing the issue in the end.

Ok, so please advise me how can we achieve that?

Martin

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener]

On Mon, Aug 12, 2019 at 11:57 AM Martin Liška  wrote:
>
> On 8/12/19 11:45 AM, Richard Biener wrote:
> > On Fri, Aug 9, 2019 at 3:57 PM Martin Liška  wrote:
> >>
> >> Hi.
> >>
> >> The patch is about prevention of LTO section name clashing.
> >> Now we have a situation where body of 2 functions is streamed
> >> into the same ELF section. Then we'll end up with smashed data.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> >
> > I think the comment should mention why we skip a leading '*'
> > at all.
>
> git blame helps us here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42531
>
>
> > IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME?
>
> Yes, it's prepended here:
> set_user_assembler_name
>
> > And section names cannot contain '*'?
>
> As seen in the PR, not.
>
> > Or do we need to actually
> > indentify '*fn' and 'fn' as the same?
>
> No, they should be identified as different symbols.
>
> >  For the testcase what is the clashing
> > symbol?
>
> void __open_alias(int, ...) __asm__("open"); - aka *open
> and:
> +extern __inline __attribute__((__gnu_inline__)) int open() {}

Hmm, so for

void __open_alias(int, ...) __asm__("open");
void __open_alias(int flags, ...) {}

a non-LTO compile will output __open_alias with the symbol "open".
Then we have

extern __inline __attribute__((__gnu_inline__)) int open() {}

which is the "other" body for 'open' but it shouldn't really be output
to the symbol table.  Still we want to emit its body for the purpose
of inlining.  So IMHO the fix is not to do magic '0' appending for
the user-asm-name case but instead "mangling" of extern inline
bodies because those are the speciality causing the issue in the end.

>
> >  Can't we have many so that using "0" always is broken as well?
>
> If I'll define 2 symbols that alias to a same asm name, I'll get:
> $ cat clash.c
> void __open_alias(int, ...) __asm__("open");
> void __open_alias2(int, ...) __asm__("open");
> void __open_alias(int flags, ...) {}
> void __open_alias2(int flags, ...) {}
> extern __inline __attribute__((__gnu_inline__)) int open() {}
> struct {
>   void *func;
> } a = {open};
>
> int main()
> {
>   return 0;
> }
>
> $ gcc clash.c -flto
> lto1: fatal error: missing resolution data for *open
> compilation terminated.
>
> Which is a reasonable error message to me.

Here I don't agree, earlier diagnostic of such invalid testcase
would be welcome instead.  If you build w/o LTO you'll get

/tmp/ccjjlMhr.s: Assembler messages:
/tmp/ccjjlMhr.s:40: Error: symbol `open' is already defined

IMHO this is invalid (undiagnosed) C.

Richard.



> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >> gcc/ChangeLog:
> >>
> >> 2019-08-09  Martin Liska  
> >>
> >> PR lto/91393
> >> PR lto/88220
> >> * lto-streamer.c (lto_get_section_name): Replace '*' leading
> >> character with '0'.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 2019-08-09  Martin Liska  
> >>
> >> PR lto/91393
> >> PR lto/88220
> >> * gcc.dg/lto/pr91393_0.c: New test.
> >> ---
> >>  gcc/lto-streamer.c   | 15 ---
> >>  gcc/testsuite/gcc.dg/lto/pr91393_0.c | 11 +++
> >>  2 files changed, 23 insertions(+), 3 deletions(-)
> >>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr91393_0.c
> >>
> >>
>

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Martin Liška]

On 8/12/19 11:45 AM, Richard Biener wrote:
> On Fri, Aug 9, 2019 at 3:57 PM Martin Liška  wrote:
>>
>> Hi.
>>
>> The patch is about prevention of LTO section name clashing.
>> Now we have a situation where body of 2 functions is streamed
>> into the same ELF section. Then we'll end up with smashed data.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> I think the comment should mention why we skip a leading '*'
> at all.

git blame helps us here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42531


> IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME?

Yes, it's prepended here:
set_user_assembler_name

> And section names cannot contain '*'?

As seen in the PR, not.

> Or do we need to actually
> indentify '*fn' and 'fn' as the same?

No, they should be identified as different symbols.

>  For the testcase what is the clashing
> symbol?

void __open_alias(int, ...) __asm__("open"); - aka *open
and:
+extern __inline __attribute__((__gnu_inline__)) int open() {}


>  Can't we have many so that using "0" always is broken as well?

If I'll define 2 symbols that alias to a same asm name, I'll get:
$ cat clash.c
void __open_alias(int, ...) __asm__("open");
void __open_alias2(int, ...) __asm__("open");
void __open_alias(int flags, ...) {}
void __open_alias2(int flags, ...) {}
extern __inline __attribute__((__gnu_inline__)) int open() {}
struct {
  void *func;
} a = {open};

int main()
{
  return 0;
}

$ gcc clash.c -flto
lto1: fatal error: missing resolution data for *open
compilation terminated.

Which is a reasonable error message to me.

Martin

> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-08-09  Martin Liska  
>>
>> PR lto/91393
>> PR lto/88220
>> * lto-streamer.c (lto_get_section_name): Replace '*' leading
>> character with '0'.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-08-09  Martin Liska  
>>
>> PR lto/91393
>> PR lto/88220
>> * gcc.dg/lto/pr91393_0.c: New test.
>> ---
>>  gcc/lto-streamer.c   | 15 ---
>>  gcc/testsuite/gcc.dg/lto/pr91393_0.c | 11 +++
>>  2 files changed, 23 insertions(+), 3 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr91393_0.c
>>
>>

Re: [PATCH] Prevent LTO section collision for a symbol name starting with '*'.

[Richard Biener]

On Fri, Aug 9, 2019 at 3:57 PM Martin Liška  wrote:
>
> Hi.
>
> The patch is about prevention of LTO section name clashing.
> Now we have a situation where body of 2 functions is streamed
> into the same ELF section. Then we'll end up with smashed data.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I think the comment should mention why we skip a leading '*'
at all.  IIRC this is some target mangling applied to DECL_ASSEMBLER_NAME?
And section names cannot contain '*'?  Or do we need to actually
indentify '*fn' and 'fn' as the same?  For the testcase what is the clashing
symbol?  Can't we have many so that using "0" always is broken as well?

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-08-09  Martin Liska  
>
> PR lto/91393
> PR lto/88220
> * lto-streamer.c (lto_get_section_name): Replace '*' leading
> character with '0'.
>
> gcc/testsuite/ChangeLog:
>
> 2019-08-09  Martin Liska  
>
> PR lto/91393
> PR lto/88220
> * gcc.dg/lto/pr91393_0.c: New test.
> ---
>  gcc/lto-streamer.c   | 15 ---
>  gcc/testsuite/gcc.dg/lto/pr91393_0.c | 11 +++
>  2 files changed, 23 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr91393_0.c
>
>

[Ada] Suppress_Initialization not respected for private subtypes

[Pierre-Marie de Rodat]

The compiler fails to suppress initialization on a variable of a subtype
of a private type (such as System.Address) even though the subtype has
aspect Suppress_Initialization. This can lead to errors on object
declarations specified with Thread_Local_Storage when Initialize_Scalars
is applied (as well as leading to default initialization when it
shouldn't).

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Gary Dismukes  

gcc/ada/

* sem_prag.adb (Analyze_Pragma, Pragma_Suppress_Initialization):
For private types, set the Suppress_Initialization flag on the
Full_View of the entity rather than the entity's base type.

gcc/testsuite/

* gnat.dg/suppress_initialization2.adb,
gnat.dg/suppress_initialization2.ads: New testcase.--- gcc/ada/sem_prag.adb
+++ gcc/ada/sem_prag.adb
@@ -24169,7 +24169,7 @@ package body Sem_Prag is
   Error_Pragma_Arg
 ("argument of pragma% cannot be an incomplete type", Arg1);
else
-  Set_Suppress_Initialization (Full_View (Base_Type (E)));
+  Set_Suppress_Initialization (Full_View (E));
end if;
 
 --  For first subtype, set flag on base type

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/suppress_initialization2.adb
@@ -0,0 +1,5 @@
+package body Suppress_Initialization2 is
+
+   procedure Dummy is null;
+
+end Suppress_Initialization2;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/suppress_initialization2.ads
@@ -0,0 +1,13 @@
+pragma Initialize_Scalars;
+
+with System;
+
+package Suppress_Initialization2 is
+
+   subtype Sub_Addr is System.Address with Suppress_Initialization;
+
+   O : Sub_Addr with Thread_Local_Storage;  -- OK: no error should be reported
+
+   procedure Dummy;
+
+end Suppress_Initialization2;

[Ada] Hang on loop in generic with subtype indication specifying a range

[Pierre-Marie de Rodat]

The compiler may hang when a for loop expanded in a generic
instantiation has a range specified by a subtype indication with an
explicit range that has a bound that is an attribute applied to a
discriminant-dependent array component. The Parent field of the bound
may not be set, which can lead to endless looping when an actual subtype
created for the array component is passed to Insert_Actions. This is
fixed by setting the Parent fields of the copied bounds before
Preanalyze is called on them.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Gary Dismukes  

gcc/ada/

* sem_ch5.adb (Prepare_Param_Spec_Loop): Set the parents of the
copied low and high bounds in the case where the loop range is
given by a discrete_subtype_indication, to prevent hanging (or
Assert_Failure) in Insert_Actions.

gcc/testsuite/

* gnat.dg/generic_inst7.adb, gnat.dg/generic_inst7_pkg.adb,
gnat.dg/generic_inst7_pkg.ads, gnat.dg/generic_inst7_types.ads:
New testcase.--- gcc/ada/sem_ch5.adb
+++ gcc/ada/sem_ch5.adb
@@ -3636,11 +3636,16 @@ package body Sem_Ch5 is
 then
Rng := Range_Expression (Constraint (Rng));
 
-   --  Preanalyze the bounds of the range constraint
+   --  Preanalyze the bounds of the range constraint, setting
+   --  parent fields to associate the copied bounds with the range,
+   --  allowing proper tree climbing during preanalysis.
 
Low  := New_Copy_Tree (Low_Bound  (Rng));
High := New_Copy_Tree (High_Bound (Rng));
 
+   Set_Parent (Low, Rng);
+   Set_Parent (High, Rng);
+
Preanalyze (Low);
Preanalyze (High);
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/generic_inst7.adb
@@ -0,0 +1,11 @@
+--  { dg-do compile }
+
+with Generic_Inst7_Pkg;
+
+procedure Generic_Inst7 is
+
+  package Inst is new Generic_Inst7_Pkg;
+
+begin
+   null;
+end Generic_Inst7;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/generic_inst7_pkg.adb
@@ -0,0 +1,12 @@
+package body Generic_Inst7_Pkg is
+
+   use type Generic_Inst7_Types.Index;
+
+   procedure Process (List : in out Generic_Inst7_Types.List) is
+   begin
+  for I in Generic_Inst7_Types.Index range 1 .. List.Arr'length loop
+ null;
+  end loop;
+   end Process;
+
+end Generic_Inst7_Pkg;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/generic_inst7_pkg.ads
@@ -0,0 +1,8 @@
+with Generic_Inst7_Types;
+
+generic
+package Generic_Inst7_Pkg is
+
+   procedure Process (List : in out Generic_Inst7_Types.List);
+
+end Generic_Inst7_Pkg;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/generic_inst7_types.ads
@@ -0,0 +1,15 @@
+package Generic_Inst7_Types is
+
+   type Index is new Integer range 0 .. 10;
+
+   type Element is record
+  I : Integer;
+   end record;
+
+   type Element_Array is array (Index range <>) of Element;
+
+   type List (Size : Index := 1) is record
+  Arr : Element_Array (1 .. Size);
+   end record;
+
+end Generic_Inst7_Types;

[Ada] Fix leak of Do_Range_Check flag in -gnatVa mode

[Pierre-Marie de Rodat]

This fixes a small glitch in Insert_Valid_Check, which needs to
propagate the Do_Range_Check flag onto the rewritten expression, but
uses its Original_Node as the source of the copy.  Now Original_Node
does not necessarily point to the node that was just rewritten, but to
the ultimately original node, which is not the same node if the
expression was rewritten multiple times.  The end result is that a
stalled Do_Range_Check flag can be wrongly resintated and leak to the
code generator.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* checks.adb (Insert_Valid_Check): Do not retrieve the
Do_Range_Check flag from the Original_Node but from the
Validated_Object.  Remove useless bypass for floating-point
types.

gcc/testsuite/

* gnat.dg/range_check7.adb: New testcase.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -7589,17 +7589,14 @@ package body Checks is
 
 Set_Validated_Object (Var_Id, New_Copy_Tree (Exp));
 
---  Reset the Do_Range_Check flag so it doesn't leak elsewhere
-
-Set_Do_Range_Check (Validated_Object (Var_Id), False);
-
 Rewrite (Exp, New_Occurrence_Of (Var_Id, Loc));
 
---  Copy the Do_Range_Check flag over to the new Exp, so it doesn't
---  get lost. Floating point types are handled elsewhere.
+--  Move the Do_Range_Check flag over to the new Exp so it doesn't
+--  get lost and doesn't leak elsewhere.
 
-if not Is_Floating_Point_Type (Typ) then
-   Set_Do_Range_Check (Exp, Do_Range_Check (Original_Node (Exp)));
+if Do_Range_Check (Validated_Object (Var_Id)) then
+   Set_Do_Range_Check (Exp);
+   Set_Do_Range_Check (Validated_Object (Var_Id), False);
 end if;
 
 PV := New_Occurrence_Of (Var_Id, Loc);

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/range_check7.adb
@@ -0,0 +1,22 @@
+--  { dg-do compile }
+--  { dg-options "-gnatVa" }
+
+procedure Range_Check7 is
+
+  type Short is range -32768 .. 32767;
+
+  type Int is range -2 ** 31 .. 2 ** 31 - 1;
+
+  subtype Nat is Int range 0 .. Int'Last;
+
+  type Ptr is access all Short;
+
+  procedure Proc (P : Ptr) is
+N : constant Nat := Nat (P.all);
+  begin
+null;
+  end;
+
+begin
+  null;
+end;

[Ada] Prevent crash in Is_Reachable

[Pierre-Marie de Rodat]

This patch fixes a bug in Is_Reachable, which causes a crash when checks
are on.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Bob Duff  

gcc/ada/

* libgnat/a-cbmutr.adb (Is_Reachable): Declare Idx to be of the
base subtype.  Clearly it makes no sense to loop "while Idx >=
0", if Idx is of a nonnegative subtype.--- gcc/ada/libgnat/a-cbmutr.adb
+++ gcc/ada/libgnat/a-cbmutr.adb
@@ -1767,10 +1767,8 @@ package body Ada.Containers.Bounded_Multiway_Trees is
  (Container : Tree;
   From, To  : Count_Type) return Boolean
is
-  Idx : Count_Type;
-
+  Idx : Count_Type'Base := From;
begin
-  Idx := From;
   while Idx >= 0 loop
  if Idx = To then
 return True;

[Ada] Fix internal error on comparison of unaligned slices

[Pierre-Marie de Rodat]

This fixes an internal error in the code generator when it is trying to
take the address of a slice which does not start on a byte boundary, in
order to generate a comparison between slices with a dynamic length.

This case is not supported by the code generator and comes from an
explicit representation clause on a record type, so it must be detected
and handled by the front-end by expanding the comparison on an
element-by-element basis.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* exp_ch4.adb (Expand_N_Op_Eq): Expand the array equality if
either operand is a possibly unaligned slice.
* exp_ch6.adb (Expand_Simple_Function_Return): Do not generate a
copy for a possibly unaligned object if it is represented as a
scalar.
* exp_util.adb (Is_Possibly_Unaligned_Slice): Do not always
return false if the target doesn't have strict alignment.

gcc/testsuite/

* gnat.dg/slice10.adb: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -8068,7 +8068,9 @@ package body Exp_Ch4 is
and then not Is_Floating_Point_Type (Component_Type (Typl))
and then not Is_Atomic_Or_VFA (Component_Type (Typl))
and then not Is_Possibly_Unaligned_Object (Lhs)
+   and then not Is_Possibly_Unaligned_Slice (Lhs)
and then not Is_Possibly_Unaligned_Object (Rhs)
+   and then not Is_Possibly_Unaligned_Slice (Rhs)
and then Support_Composite_Compare_On_Target
  then
 null;

--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -203,8 +203,8 @@ package body Exp_Ch6 is
--  For all parameter modes, actuals that denote components and slices of
--  packed arrays are expanded into suitable temporaries.
--
-   --  For non-scalar objects that are possibly unaligned, add call by copy
-   --  code (copy in for IN and IN OUT, copy out for OUT and IN OUT).
+   --  For nonscalar objects that are possibly unaligned, add call by copy code
+   --  (copy in for IN and IN OUT, copy out for OUT and IN OUT).
--
--  For OUT and IN OUT parameters, add predicate checks after the call
--  based on the predicates of the actual type.
@@ -2019,7 +2019,7 @@ package body Exp_Ch6 is
 elsif Is_Ref_To_Bit_Packed_Array (Actual) then
Add_Simple_Call_By_Copy_Code;
 
---  If a non-scalar actual is possibly bit-aligned, we need a copy
+--  If a nonscalar actual is possibly bit-aligned, we need a copy
 --  because the back-end cannot cope with such objects. In other
 --  cases where alignment forces a copy, the back-end generates
 --  it properly. It should not be generated unconditionally in the
@@ -2235,7 +2235,7 @@ package body Exp_Ch6 is
 elsif Is_Ref_To_Bit_Packed_Array (Actual) then
Add_Simple_Call_By_Copy_Code;
 
---  If a non-scalar actual is possibly unaligned, we need a copy
+--  If a nonscalar actual is possibly unaligned, we need a copy
 
 elsif Is_Possibly_Unaligned_Object (Actual)
   and then not Represented_As_Scalar (Etype (Formal))
@@ -7413,12 +7413,13 @@ package body Exp_Ch6 is
  end;
   end if;
 
-  --  If we are returning an object that may not be bit-aligned, then copy
-  --  the value into a temporary first. This copy may need to expand to a
-  --  loop of component operations.
+  --  If we are returning a nonscalar object that is possibly unaligned,
+  --  then copy the value into a temporary first. This copy may need to
+  --  expand to a loop of component operations.
 
   if Is_Possibly_Unaligned_Slice (Exp)
-or else Is_Possibly_Unaligned_Object (Exp)
+or else (Is_Possibly_Unaligned_Object (Exp)
+  and then not Represented_As_Scalar (Etype (Exp)))
   then
  declare
 ExpR : constant Node_Id   := Relocate_Node (Exp);

--- gcc/ada/exp_util.adb
+++ gcc/ada/exp_util.adb
@@ -8485,12 +8485,6 @@ package body Exp_Util is
  return False;
   end if;
 
-  --  We only need to worry if the target has strict alignment
-
-  if not Target_Strict_Alignment then
- return False;
-  end if;
-
   --  If it is a slice, then look at the array type being sliced
 
   declare

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/slice10.adb
@@ -0,0 +1,29 @@
+--  { dg-do run }
+
+procedure Slice10 is
+
+   subtype Str is String (1 .. 3);
+
+   type T is record
+  B : Boolean;
+  S : Str;
+   end record;
+
+   for T use record
+  B at 0 range 0 .. 0;
+  S at 0 range 1 .. 24;
+   end record;
+
+   function Match (X, Y: T; Length : Positive) return Boolean is
+   begin
+  return X.S (1 .. Length) = Y.S (1 .. Length);
+   end;
+
+   X, Y : T := (B => True, S => "123");
+
+begin
+   X.B := False;
+   if not match (X, Y, 3) then
+  r

[Ada] Remove doc for language version switches

[Pierre-Marie de Rodat]

Remove documentation for Ada language version switches, and note that
they are no longer needed.  These tools now silently ignore such
switches, and process the file correctly no matter what version of Ada
is used.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Bob Duff  

gcc/ada/

* doc/gnat_ugn/gnat_utility_programs.rst (gnatmetric, gnatpp,
gnatstub): Remove documentation for Ada language version
switches, and note that they are no longer needed.--- gcc/ada/doc/gnat_ugn/gnat_utility_programs.rst
+++ gcc/ada/doc/gnat_ugn/gnat_utility_programs.rst
@@ -1797,9 +1797,6 @@ Alternatively, you may run the script using the following command line:
   .. index:: ! gnatmetric
   .. index:: Metric tool
 
-  This documentation is for the new libadalang-based version
-  of ``gnatmetric``, which replaces the ASIS-based version.
-
   The ``gnatmetric`` tool is a utility
   for computing various program metrics.
   It takes an Ada source file as input and generates a file containing the
@@ -1822,14 +1819,16 @@ Alternatively, you may run the script using the following command line:
   * ``switches`` specify the metrics to compute and define the destination for
 the output
 
-  * Each ``filename`` is the name (including the extension) of a source
-file to process. 'Wildcards' are allowed, and
-the file name may contain path information.
-If no ``filename`` is supplied, then the ``switches`` list must contain
-at least one
-:switch:`--files` switch (see :ref:`Other_gnatmetric_Switches`).
-Including both a :switch:`--files` switch and one or more
-``filename`` arguments is permitted.
+  * Each ``filename`` is the name of a source file to process. 'Wildcards' are
+allowed, and the file name may contain path information.  If no
+``filename`` is supplied, then the ``switches`` list must contain at least
+one :switch:`--files` switch (see :ref:`Other_gnatmetric_Switches`).
+Including both a :switch:`--files` switch and one or more ``filename``
+arguments is permitted.
+
+Note that it is no longer necessary to specify the Ada language version;
+``gnatmetric`` can process Ada source code written in any version from
+Ada 83 onward without specifying any language version switch.
 
   The following subsections describe the various switches accepted by
   ``gnatmetric``, organized by category.
@@ -1928,6 +1927,16 @@ Alternatively, you may run the script using the following command line:
 to exclude all directory information from the file names that are
 output.)
 
+   .. index:: --wide-character-encoding (gnatmetric)
+
+   :switch:`--wide-character-encoding={e}`
+ Specify the wide character encoding method for the input and output
+ files. ``e`` is one of the following:
+
+ * *8* - UTF-8 encoding
+
+ * *b* - Brackets encoding (default value)
+
 
   .. index:: Disable Metrics For Local Units in gnatmetric
 
@@ -2811,6 +2820,11 @@ Alternatively, you may run the script using the following command line:
   :switch:`-sfn`
 :switch:`--short-file-names`
 
+  .. index:: -W (gnatsmetric)
+
+  :switch:`-W{e}`
+:switch:`--wide-character-encoding={e}`
+
   .. index:: -nolocal (gnatmetric)
 
   :switch:`-nolocal`
@@ -2846,9 +2860,6 @@ Alternatively, you may run the script using the following command line:
.. index:: ! gnatpp
.. index:: pretty printer
 
-   This documentation is for the new libadalang-based version
-   of ``gnatpp``, which replaces the ASIS-based version.
-
The ``gnatpp`` tool is a utility for source reformatting / pretty
printing. It takes an Ada source file as input and generates a
reformatted version as output. You can specify various style
@@ -2880,6 +2891,10 @@ Alternatively, you may run the script using the following command line:
  file name may contain path information; it does not have to follow
  the GNAT file naming rules
 
+ Note that it is no longer necessary to specify the Ada language version;
+ ``gnatpp`` can process Ada source code written in any version from
+ Ada 83 onward without specifying any language version switch.
+
 
.. _Switches_for_gnatpp:
 
@@ -3633,30 +3648,6 @@ Alternatively, you may run the script using the following command line:
all the immediate units of the argument project.
 
 
-   .. index:: --gnat83 (gnatpp)
-
-   :switch:`--gnat83`
- Ada 83 mode
-
-
-   .. index:: --gnat95 (gnatpp)
-
-   :switch:`--gnat95`
- Ada 95 mode
-
-
-   .. index:: --gnat2005 (gnatpp)
-
-   :switch:`--gnat2005`
- Ada 2005 mode
-
-
-   .. index:: --gnat2012 (gnatpp)
-
-   :switch:`--gnat2012`
- Ada 2012 mode
-
-
.. _Formatting_Rules:
 
Formatting Rules
@@ -4243,6 +4234,10 @@ Alternatively, you may run the script using the following command line:
   or creates the name file to generate using the standard GNAT
   naming conventions.
 
+  Note that it is no longer necessary to specify the Ada language vers

[Ada] New parameter Quiet for procedure GNAT.Command_Line.Getopt

[Pierre-Marie de Rodat]

Getopt procedure is parsing the command line or set of strings. If the
command line contains unknown switch than the Getopt prints error
message to the console and raises the exception Invalid_Switch. The
printing can be inappropriate in some cases. The new parameter Quiet
allows avoiding console output.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Dmitriy Anisimkov  

gcc/ada/

* libgnat/g-comlin.ads, libgnat/g-comlin.adb (Getopt): Add
parameter Quiet. Need to do not output error messages to
console. Invalid_Switch exception generation surrounded by an
error message.--- gcc/ada/libgnat/g-comlin.adb
+++ gcc/ada/libgnat/g-comlin.adb
@@ -753,7 +753,8 @@ package body GNAT.Command_Line is
 
 Parser.Current_Index := End_Index + 1;
 
-raise Invalid_Switch;
+raise Invalid_Switch with
+  "Unrecognized option " & Full_Switch (Parser);
  end if;
 
  End_Index := Parser.Current_Index + Max_Length - 1;
@@ -883,7 +884,8 @@ package body GNAT.Command_Line is
  Last=> Arg'Last,
  Extra   => Parser.Switch_Character);
   Parser.Current_Index := Arg'Last + 1;
-  raise Invalid_Switch;
+  raise Invalid_Switch with
+"Unrecognized option " & Full_Switch (Parser);
end if;
  end case;
 
@@ -3365,7 +3367,8 @@ package body GNAT.Command_Line is
  (Config  : Command_Line_Configuration;
   Callback: Switch_Handler := null;
   Parser  : Opt_Parser := Command_Line_Parser;
-  Concatenate : Boolean := True)
+  Concatenate : Boolean := True;
+  Quiet   : Boolean := False)
is
   Local_Config: Command_Line_Configuration := Config;
   Getopt_Switches : String_Access;
@@ -3575,12 +3578,14 @@ package body GNAT.Command_Line is
 
  --  Message inspired by "ls" on Unix
 
- Put_Line (Standard_Error,
-   Base_Name (Ada.Command_Line.Command_Name)
-   & ": unrecognized option '"
-   & Full_Switch (Parser)
-   & "'");
- Try_Help;
+ if not Quiet then
+Put_Line (Standard_Error,
+  Base_Name (Ada.Command_Line.Command_Name)
+  & ": unrecognized option '"
+  & Full_Switch (Parser)
+  & "'");
+Try_Help;
+ end if;
 
  raise;
 

--- gcc/ada/libgnat/g-comlin.ads
+++ gcc/ada/libgnat/g-comlin.ads
@@ -738,7 +738,8 @@ package GNAT.Command_Line is
  (Config  : Command_Line_Configuration;
   Callback: Switch_Handler := null;
   Parser  : Opt_Parser := Command_Line_Parser;
-  Concatenate : Boolean := True);
+  Concatenate : Boolean := True;
+  Quiet   : Boolean := False);
--  Similar to the standard Getopt function. For each switch found on the
--  command line, this calls Callback, if the switch is not handled
--  automatically.
@@ -756,6 +757,7 @@ package GNAT.Command_Line is
--  to display the help message and raises Exit_From_Command_Line.
--  If an invalid switch is specified on the command line, this procedure
--  will display an error message and raises Invalid_Switch again.
+   --  If the Quiet parameter is True then the error message is not displayed.
--
--  This function automatically expands switches:
--

[Ada] New aspect/pragma No_Caching for analysis of volatile data

[Pierre-Marie de Rodat]

A new aspect/pragma can be attached to volatile variables to indicate
that such a variable is not used for interactions with the external
world, but only that accesses to that variable should not be optimized
by the compiler. This is in particular useful for guarding against fault
injection. SPARK Reference manual has been updated to allow this use of
volatile data, see section 7.1.2, so that GNATprove can analyze such
variables as not volatile.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Yannick Moy  

gcc/ada/

* aspects.adb, aspects.ads (Aspect_No_Caching): New aspect.
* contracts.adb, contracts.ads (Add_Contract_Item): Add handling
of No_Caching.
(Analyze_Object_Contract): Add handling of No_Caching.
* einfo.adb, einfo.ads
(Get_Pragma): Add handling of No_Caching.
* doc/gnat_rm/implementation_defined_aspects.rst,
doc/gnat_rm/implementation_defined_pragmas.rst: Document new
aspect/pragma.
* gnat_rm.texi: Regenerate.
* par-prag.adb (Prag): New pragma Pragma_No_Caching.
* sem_ch13.adb (Analyze_Aspect_Specifications,
Check_Aspect_At_Freeze_Point): Add handling of No_Caching.
* sem_prag.adb (Analyze_Pragma): Deal with pragma No_Caching.
* sem_prag.ads (Analyze_External_Property_In_Decl_Part): Now
applies to No_Caching.
* sem_util.adb, sem_util.ads (Is_Effectively_Volatile): Add
handling of No_Caching.
(No_Caching_Enabled): New query function.
* snames.ads-tmpl: New names for pragma.

gcc/testsuite/

* gnat.dg/no_caching.adb, gnat.dg/no_caching.ads: New testcase.

patch.diff.gz
Description: application/gzip

[Ada] Missing check on outbound parameter of a non-null access type

[Pierre-Marie de Rodat]

This patch adds code to generate proper post-call checks when an actual
for an in-out or out parameter has a non-null access type. No
constraints are applied to an inbound access parameter, but on exit a
not-null check must be performed if the type of the actual requires it.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Ed Schonberg  

gcc/ada/

* exp_ch6.adb (Expand_Actuals. Add_Call_By_Copy_Code): Add code
to generate proper checks when an actual for an in-out or out
parameter has a non-null access type.  No constraints are
applied to an inbound access parameter, but on exit a not-null
check must be performed if the type of the actual requires it.

gcc/testsuite/

* gnat.dg/null_check.adb: New testcase.--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -1406,6 +1406,16 @@ package body Exp_Ch6 is
Init := New_Occurrence_Of (Var, Loc);
 end if;
 
+ --  Access types are passed in without checks, but if a copy-back is
+ --  required for a null-excluding check on an in-out or out parameter,
+ --  then the initial value is that of the actual.
+
+ elsif Is_Access_Type (E_Formal)
+   and then Can_Never_Be_Null (Etype (Actual))
+   and then not Can_Never_Be_Null (E_Formal)
+ then
+Init := New_Occurrence_Of (Var, Loc);
+
  else
 Init := Empty;
  end if;
@@ -1544,6 +1554,19 @@ package body Exp_Ch6 is
 Type_Access_Level (E_Formal;
 
else
+  if Is_Access_Type (E_Formal)
+and then Can_Never_Be_Null (Etype (Actual))
+and then not Can_Never_Be_Null (E_Formal)
+  then
+ Append_To (Post_Call,
+   Make_Raise_Constraint_Error (Loc,
+ Condition =>
+   Make_Op_Eq (Loc,
+ Left_Opnd  => New_Occurrence_Of (Temp, Loc),
+ Right_Opnd => Make_Null (Loc)),
+ Reason => CE_Access_Check_Failed));
+  end if;
+
   Append_To (Post_Call,
 Make_Assignment_Statement (Loc,
   Name   => Lhs,
@@ -1942,7 +1965,8 @@ package body Exp_Ch6 is
 Apply_Constraint_Check (Actual, E_Formal);
 
  --  Out parameter case. No constraint checks on access type
- --  RM 6.4.1 (13)
+ --  RM 6.4.1 (13), but on return a null-excluding check may be
+ --  required (see below).
 
  elsif Is_Access_Type (E_Formal) then
 null;
@@ -2049,11 +2073,14 @@ package body Exp_Ch6 is
 --  formal subtype are not the same, requiring a check.
 
 --  It is necessary to exclude tagged types because of "downward
---  conversion" errors.
+--  conversion" errors, but null-excluding checks on return may be
+--  required.
 
 elsif Is_Access_Type (E_Formal)
-  and then not Same_Type (E_Formal, E_Actual)
   and then not Is_Tagged_Type (Designated_Type (E_Formal))
+  and then (not Same_Type (E_Formal, E_Actual)
+or else (Can_Never_Be_Null (E_Actual)
+  and then not Can_Never_Be_Null (E_Formal)))
 then
Add_Call_By_Copy_Code;
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/null_check.adb
@@ -0,0 +1,19 @@
+--  { dg-do run }
+
+procedure Null_Check with SPARK_Mode is
+   type Int_Ptr is access Integer;
+   subtype Not_Null_Int_Ptr is not null Int_Ptr;
+
+   procedure Set_To_Null (X : out Int_Ptr) with Global => null is
+   begin
+  X := null;
+   end Set_To_Null;
+
+   X : Not_Null_Int_Ptr := new Integer'(12);
+begin
+   Set_To_Null (X);
+   raise Program_Error;
+exception
+   when Constraint_Error =>
+  null;
+end Null_Check;

[Ada] More precise handling of Size/Object_Size in GNATprove

[Pierre-Marie de Rodat]

GNATprove does a partial expansion which did not allow getting the
most precise value for attributes Size/Object_Size. Now fixed.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Yannick Moy  

gcc/ada/

* exp_attr.adb, exp_attr.ads (Expand_Size_Attribute): New
procedure to share part of the attribute expansion with
GNATprove mode.
(Expand_N_Attribute_Reference): Extract part of the
Size/Object_Size expansion in the new procedure
Expand_Size_Attribute.
* exp_spark.adb (Expand_SPARK_N_Attribute_Reference): Expand
Size/Object_Size attributes using the new procedure
Expand_Size_Attribute.--- gcc/ada/exp_attr.adb
+++ gcc/ada/exp_attr.adb
@@ -3598,8 +3598,8 @@ package body Exp_Attr is
   --Result_Type (System.Fore (Universal_Real (Type'First)),
   --  Universal_Real (Type'Last))
 
-  --  Note that we know that the type is a non-static subtype, or Fore
-  --  would have itself been computed dynamically in Eval_Attribute.
+  --  Note that we know that the type is a nonstatic subtype, or Fore would
+  --  have itself been computed dynamically in Eval_Attribute.
 
   when Attribute_Fore =>
  Rewrite (N,
@@ -5849,7 +5849,6 @@ package body Exp_Attr is
  | Attribute_VADS_Size
   =>
  Size : declare
-Siz  : Uint;
 New_Node : Node_Id;
 
  begin
@@ -5961,128 +5960,12 @@ package body Exp_Attr is
Rewrite (N, New_Node);
Analyze_And_Resolve (N, Typ);
return;
-
---  Case of known RM_Size of a type
-
-elsif (Id = Attribute_Size or else Id = Attribute_Value_Size)
-  and then Is_Entity_Name (Pref)
-  and then Is_Type (Entity (Pref))
-  and then Known_Static_RM_Size (Entity (Pref))
-then
-   Siz := RM_Size (Entity (Pref));
-
---  Case of known Esize of a type
-
-elsif Id = Attribute_Object_Size
-  and then Is_Entity_Name (Pref)
-  and then Is_Type (Entity (Pref))
-  and then Known_Static_Esize (Entity (Pref))
-then
-   Siz := Esize (Entity (Pref));
-
---  Case of known size of object
-
-elsif Id = Attribute_Size
-  and then Is_Entity_Name (Pref)
-  and then Is_Object (Entity (Pref))
-  and then Known_Esize (Entity (Pref))
-  and then Known_Static_Esize (Entity (Pref))
-then
-   Siz := Esize (Entity (Pref));
-
---  For an array component, we can do Size in the front end if the
---  component_size of the array is set.
-
-elsif Nkind (Pref) = N_Indexed_Component then
-   Siz := Component_Size (Etype (Prefix (Pref)));
-
---  For a record component, we can do Size in the front end if
---  there is a component clause, or if the record is packed and the
---  component's size is known at compile time.
-
-elsif Nkind (Pref) = N_Selected_Component then
-   declare
-  Rec  : constant Entity_Id := Etype (Prefix (Pref));
-  Comp : constant Entity_Id := Entity (Selector_Name (Pref));
-
-   begin
-  if Present (Component_Clause (Comp)) then
- Siz := Esize (Comp);
-
-  elsif Is_Packed (Rec) then
- Siz := RM_Size (Ptyp);
-
-  else
- Apply_Universal_Integer_Attribute_Checks (N);
- return;
-  end if;
-   end;
-
---  All other cases are handled by the back end
-
-else
-   Apply_Universal_Integer_Attribute_Checks (N);
-
-   --  If Size is applied to a formal parameter that is of a packed
-   --  array subtype, then apply Size to the actual subtype.
-
-   if Is_Entity_Name (Pref)
- and then Is_Formal (Entity (Pref))
- and then Is_Array_Type (Ptyp)
- and then Is_Packed (Ptyp)
-   then
-  Rewrite (N,
-Make_Attribute_Reference (Loc,
-  Prefix =>
-New_Occurrence_Of (Get_Actual_Subtype (Pref), Loc),
-  Attribute_Name => Name_Size));
-  Analyze_And_Resolve (N, Typ);
-   end if;
-
-   --  If Size applies to a dereference of an access to
-   --  unconstrained packed array, the back end needs to see its
-   --  unconstrained nominal type, but also a hint to the actual
-   --  constrained type.
-
-   if Nkind (Pref) = N_Explicit_Dereference
- and then Is_Array_Type (Ptyp)
-

[Ada] Crash on illegal left-hand side in assignment of renamed variable

[Pierre-Marie de Rodat]

This patch fixes a crash on an assignment where the left-hand side is a
renaming of a function call that does not involve ceiling priorities.
This avoids a compiler crash in some cases, and prevents a useless
retrieval and compilation of run-time packages.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Ed Schonberg  

gcc/ada/

* sem_util.adb (Is_Expaned_Priority_Attribute): Check whether
call comes from a rewritten attribute before comparing name with
Get_Ceiling run-time subprogram.

gcc/testsuite/

* gnat.dg/renaming15.adb: New testcase.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -14669,6 +14669,7 @@ package body Sem_Util is
   return
 Nkind (E) = N_Function_Call
   and then not Configurable_Run_Time_Mode
+  and then Nkind (Original_Node (E)) = N_Attribute_Reference
   and then (Entity (Name (E)) = RTE (RE_Get_Ceiling)
  or else Entity (Name (E)) = RTE (RO_PE_Get_Ceiling));
end Is_Expanded_Priority_Attribute;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/renaming15.adb
@@ -0,0 +1,32 @@
+--  { dg-do compile }
+
+with Ada.Containers.Hashed_Maps;
+with Ada.Text_IO;
+
+procedure Renaming15 is
+   use Ada.Containers;
+
+   subtype String_T is String (1 .. 3);
+
+   function Hash (Aircraft_Id : Integer) return Hash_Type is
+   (Hash_Type (Aircraft_Id) * (2 ** 31 - 1));
+   function Equal (Left, Right : Integer) return Boolean is (Left = Right);
+   package Radar_Map is new Hashed_Maps (Integer, String_T, Hash, Equal);
+
+   Radars : Radar_Map.Map;
+
+   procedure Change_Elem_Value is
+   begin
+  for C in Radars.Iterate loop
+ declare
+E : String_T renames Radar_Map.Element (C);
+ begin
+E := "Xyz";  --  { dg-error "left hand side of assignment must be a variable" }
+Ada.Text_IO.Put_Line (E);
+ end;
+  end loop;
+   end Change_Elem_Value;
+begin
+   Radars.Include (1, "jjj");
+   Change_Elem_Value;
+end Renaming15;

[Ada] Fix IPv6 numeric address detection

[Pierre-Marie de Rodat]

IPv6 numeric address can't have less than 2 colons. It fixes the error
when Get_Host_By_Name called with hostname composed by only hexadecimal
symbols.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Dmitriy Anisimkov  

gcc/ada/

* libgnat/g-socket.adb (Is_IPv6_Address): Check that no less
then 2 colons in IPv6 numeric address.--- gcc/ada/libgnat/g-socket.adb
+++ gcc/ada/libgnat/g-socket.adb
@@ -1797,7 +1797,7 @@ package body GNAT.Sockets is
  end if;
   end loop;
 
-  return Colons <= 8;
+  return Colons in 2 .. 8;
end Is_IPv6_Address;
 
-

[Ada] Inconsistent compile time Constraint_Error warning

[Pierre-Marie de Rodat]

This patch corrects several bugs within the compiler which led to
inconsistent handling of compile time Constraint_Errors. Notibly,
subtype out of range checks which are only out of range of the subtype
must be warnings while out of range checks where the value is out of
range of the base type must be an error. Also, type conversions and
qualified expressions on literals constitute errors on any out of range
value. The compiler needed many of these cases clarified.


-- Source --


--  main.ads

with System;
package Main is

   type T_Enum is (Enum_1, Enum_2, Unknown)
 with Default_Value => Unknown;

   subtype T_Valid_Enum is T_Enum range Enum_1 .. Enum_2;

   Value : T_Valid_Enum; --  WARNING

   generic
  type T_Element is (<>);
  Init : T_Element;
   package Generic_Test is
  Value : T_Element := Init;
   end;

   package titi is new Generic_Test (T_Valid_Enum, Unknown); --  WARNING

   type My_Float is digits System.Max_Base_Digits;

   My_Float_Last : constant := My_Float'Last;
   Out_Of_Range  : constant := My_Float_Last + 1.0;

   Flt1 : My_Float := Out_Of_Range; --  ERROR

   A : Positive := Positive (16#__#); --  ERROR
   B : Positive := 16#__#;--  ERROR
   C : Positive := 0; --  WARNING
   D : Positive := Positive (0);  --  ERROR
   E : Positive := Positive'(16#__#); --  ERROR
   F : Positive := Positive'(0);  --  ERROR
end;

-
-- Compilation --
-

$ gnatmake -q -gnatw_a main.adb
main.ads:9:12: warning: value not in range of type "T_Valid_Enum" defined at
line 7
main.ads:9:12: warning: "Constraint_Error" will be raised at run time
main.ads:18:52: warning: value not in range of type "T_Element" defined at
line 12, instance at line 18
main.ads:18:52: warning: "Constraint_Error" will be raised at run time
main.ads:25:23: value not in range of type "My_Float" defined at line 20
main.ads:25:23: static expression fails Constraint_Check
main.ads:27:19: value not in range of type "Standard.Positive"
main.ads:27:19: static expression fails Constraint_Check
main.ads:28:19: value not in range of type "Standard.Positive"
main.ads:28:19: static expression fails Constraint_Check
main.ads:29:19: warning: value not in range of type "Standard.Positive"
main.ads:29:19: warning: "Constraint_Error" will be raised at run time
main.ads:30:19: value not in range of type "Standard.Positive"
main.ads:30:19: static expression fails Constraint_Check
main.ads:31:27: value not in range of type "Standard.Positive"
main.ads:31:27: static expression fails Constraint_Check
main.ads:32:27: value not in range of type "Standard.Positive"
main.ads:32:27: static expression fails Constraint_Check
gnatmake: "main.ads" compilation error

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Justin Squirek  

gcc/ada/

* sem_eval.adb (Check_Non_Static_Context): Add a condition to
determine if a range violation constitues a warning or an error.
(Out_Of_Range): Add a condition to determine if a range
violation constitues a warning or an error.--- gcc/ada/sem_eval.adb
+++ gcc/ada/sem_eval.adb
@@ -562,23 +562,31 @@ package body Sem_Eval is
   elsif Is_Out_Of_Range (N, Base_Type (T), Assume_Valid => True) then
  Out_Of_Range (N);
 
-  --  Give warning if outside subtype (where one or both of the bounds of
-  --  the subtype is static). This warning is omitted if the expression
-  --  appears in a range that could be null (warnings are handled elsewhere
-  --  for this case).
+  --  Give a warning or error on the value outside the subtype. A
+  --  warning is omitted if the expression appears in a range that could
+  --  be null (warnings are handled elsewhere for this case).
 
   elsif T /= Base_Type (T) and then Nkind (Parent (N)) /= N_Range then
  if Is_In_Range (N, T, Assume_Valid => True) then
 null;
 
  elsif Is_Out_Of_Range (N, T, Assume_Valid => True) then
-
 --  Ignore out of range values for System.Priority in CodePeer
 --  mode since the actual target compiler may provide a wider
 --  range.
 
 if CodePeer_Mode and then T = RTE (RE_Priority) then
Set_Do_Range_Check (N, False);
+
+--  Determine if the out of range violation constitutes a warning
+--  or an error based on context according to RM 4.9 (34/3).
+
+elsif Nkind_In (Original_Node (N), N_Type_Conversion,
+   N_Qualified_Expression)
+  and then Comes_From_Source (Original_Node (N))
+then
+   Apply_Compile_Time_Constraint_Error
+ (N, "value not in range of}", CE_Range_Check_Failed);
 else
Apply_Compile_Time_Constraint_Error
  (N, "value not in range of}<

[Ada] Improve error message for Object_Size clause on dynamic array

[Pierre-Marie de Rodat]

This makes the compiler issue the same error:

size clause not allowed for variable length type

for an Object_Size clause on a variable-sized type as for a Size clause,
for example on the following procedure:

procedure P (X, Y : Integer) is
   subtype Sub is String (X .. Y) with Object_Size => 64;
begin
   null;
end;

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* freeze.adb (Freeze_Entity): Give the same error for an
Object_Size clause on a variable-sized type as for a Size
clause.--- gcc/ada/freeze.adb
+++ gcc/ada/freeze.adb
@@ -6803,7 +6803,7 @@ package body Freeze is
  --  Do not allow a size clause for a type which does not have a size
  --  that is known at compile time
 
- if Has_Size_Clause (E)
+ if (Has_Size_Clause (E) or else Has_Object_Size_Clause (E))
and then not Size_Known_At_Compile_Time (E)
  then
 --  Suppress this message if errors posted on E, even if we are

[Ada] Do not suppress checks in instances of internal generics

[Pierre-Marie de Rodat]

This patch removes suppression of checks in nested instances of internal
packages. No test.

This was inconsistent: only for packages, not for subprograms. Only for
nested instantiations, not library level ones. Not for GNAT units.

Furthermore, the user should have control via pragma Suppress or
switches.

Furthermore, without this change, there could be missing tampering
checks in Ada.Containers.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Bob Duff  

gcc/ada/

* sem_ch12.adb (Instantiate_Package_Body): Remove suppression of
checks in instances of internal units.
* sem_ch6.adb (Analyze_Function_Return): Do not generate a
constraint check on an extended_return_statement if the subtype
of the return object in the statement is identical to the return
subtype of the function.--- gcc/ada/sem_ch12.adb
+++ gcc/ada/sem_ch12.adb
@@ -11601,25 +11601,7 @@ package body Sem_Ch12 is
 --  indicate that the body instance is to be delayed.
 
 Install_Body (Act_Body, Inst_Node, Gen_Body, Gen_Decl);
-
---  Now analyze the body. We turn off all checks if this is an
---  internal unit, since there is no reason to have checks on for
---  any predefined run-time library code. All such code is designed
---  to be compiled with checks off.
-
---  Note that we do NOT apply this criterion to children of GNAT
---  The latter units must suppress checks explicitly if needed.
-
---  We also do not suppress checks in CodePeer mode where we are
---  interested in finding possible runtime errors.
-
-if not CodePeer_Mode
-  and then In_Predefined_Unit (Gen_Decl)
-then
-   Analyze (Act_Body, Suppress => All_Checks);
-else
-   Analyze (Act_Body);
-end if;
+Analyze (Act_Body);
  end if;
 
  Inherit_Context (Gen_Body, Inst_Node);

--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -1056,9 +1056,17 @@ package body Sem_Ch6 is
  --  Apply constraint check. Note that this is done before the implicit
  --  conversion of the expression done for anonymous access types to
  --  ensure correct generation of the null-excluding check associated
- --  with null-excluding expressions found in return statements.
-
- Apply_Constraint_Check (Expr, R_Type);
+ --  with null-excluding expressions found in return statements. We
+ --  don't need a check if the subtype of the return object is the
+ --  same as the result subtype of the function.
+
+ if Nkind (N) /= N_Extended_Return_Statement
+   or else Nkind (Obj_Decl) /= N_Object_Declaration
+   or else Nkind (Object_Definition (Obj_Decl)) not in N_Has_Entity
+   or else Entity (Object_Definition (Obj_Decl)) /= R_Type
+ then
+Apply_Constraint_Check (Expr, R_Type);
+ end if;
 
  --  The return value is converted to the return type of the function,
  --  which implies a predicate check if the return type is predicated.

[Ada] Prevent crash in Put_Scaled

[Pierre-Marie de Rodat]

This patch fixes a bug in Put_Scaled, which causes a crash when checks
are on.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Bob Duff  

gcc/ada/

* libgnat/a-tifiio.adb (Put_Scaled): Prevent AA from being
negative, since Field is range 0 .. something.--- gcc/ada/libgnat/a-tifiio.adb
+++ gcc/ada/libgnat/a-tifiio.adb
@@ -560,7 +560,7 @@ package body Ada.Text_IO.Fixed_IO is
  E   : Integer)
   is
  pragma Assert (E >= -Max_Digits);
- AA : constant Field := E + A;
+ AA : constant Field := Integer'Max (E + A, 0);
  N  : constant Natural := (AA + Max_Digits - 1) / Max_Digits + 1;
 
  Q  : array (0 .. N - 1) of Int64 := (others => 0);

[Ada] Implement Ada.Directories.Hierarchical_File_Names

[Pierre-Marie de Rodat]

This patch corrects certain behaviors within Ada.Directories to better
conform to conformance tests and implements the package
Ada.Directories.Hierarchical_File_Names outlined in AI05-0049-1.

Only partial test sources are included.


-- Source --


--  main.ads

with Ada.Directories.Hierarchical_File_Names;
use Ada.Directories.Hierarchical_File_Names;

with Ada.Exceptions; use Ada.Exceptions;
with Ada.Text_IO;use Ada.Text_IO;

procedure Main is
   FULL_PATH_A : constant String := "/export/work/user/bug";
   FULL_PATH_B : constant String := "/export/work/user";

   RELATIVE_PATH_A : constant String := "export/work/user/bug/";
   RELATIVE_PATH_B : constant String := "export/work/user/bug";

   SIMPLE_PATH_A : constant String := "bug/";
   SIMPLE_PATH_B : constant String := "bug";

   ROOT_PATH : constant String := "/";

   CURRENT_DIR : constant String := ".";
   PARENT_DIR  : constant String := "..";

   RELATIVE_WITH_CURRENT : constant String := RELATIVE_PATH_A & ".";
   RELATIVE_WITH_PARENT  : constant String := RELATIVE_PATH_A & "..";
begin
   Put_Line ("Simple_Name");
   Put_Line (Is_Simple_Name (FULL_PATH_A)'Image);
   Put_Line (Is_Simple_Name (FULL_PATH_B)'Image);
   Put_Line (Is_Simple_Name (RELATIVE_PATH_A)'Image);
   Put_Line (Is_Simple_Name (RELATIVE_PATH_B)'Image);
   Put_Line (Is_Simple_Name (SIMPLE_PATH_A)'Image);
   Put_Line (Is_Simple_Name (SIMPLE_PATH_B)'Image);
   Put_Line (Is_Simple_Name (ROOT_PATH)'Image);
   Put_Line (Is_Simple_Name (CURRENT_DIR)'Image);
   Put_Line (Is_Simple_Name (PARENT_DIR)'Image);
   Put_Line (Is_Simple_Name (RELATIVE_WITH_CURRENT)'Image);
   Put_Line (Is_Simple_Name (RELATIVE_WITH_PARENT)'Image);
   Put_Line (Simple_Name (FULL_PATH_A));
   Put_Line (Simple_Name (FULL_PATH_B));
   Put_Line (Simple_Name (RELATIVE_PATH_A));
   Put_Line (Simple_Name (RELATIVE_PATH_B));
   Put_Line (Simple_Name (SIMPLE_PATH_A));
   Put_Line (Simple_Name (SIMPLE_PATH_B));
   Put_Line (Simple_Name (ROOT_PATH));
   Put_Line (Simple_Name (CURRENT_DIR));
   Put_Line (Simple_Name (PARENT_DIR));
   Put_Line (Simple_Name (RELATIVE_WITH_CURRENT));
   Put_Line (Simple_Name (RELATIVE_WITH_PARENT));

   Put_Line ("Root_Directory_Name");
   Put_Line (Is_Root_Directory_Name (FULL_PATH_A)'Image);
   Put_Line (Is_Root_Directory_Name (FULL_PATH_B)'Image);
   Put_Line (Is_Root_Directory_Name (RELATIVE_PATH_A)'Image);
   Put_Line (Is_Root_Directory_Name (RELATIVE_PATH_B)'Image);
   Put_Line (Is_Root_Directory_Name (SIMPLE_PATH_A)'Image);
   Put_Line (Is_Root_Directory_Name (SIMPLE_PATH_B)'Image);
   Put_Line (Is_Root_Directory_Name (ROOT_PATH)'Image);
   Put_Line (Is_Root_Directory_Name (CURRENT_DIR)'Image);
   Put_Line (Is_Root_Directory_Name (PARENT_DIR)'Image);
   Put_Line (Is_Root_Directory_Name (RELATIVE_WITH_CURRENT)'Image);
   Put_Line (Is_Root_Directory_Name (RELATIVE_WITH_PARENT)'Image);

   Put_Line ("Is_Parent_Directory_Name");
   Put_Line (Is_Parent_Directory_Name (FULL_PATH_A)'Image);
   Put_Line (Is_Parent_Directory_Name (FULL_PATH_B)'Image);
   Put_Line (Is_Parent_Directory_Name (RELATIVE_PATH_A)'Image);
   Put_Line (Is_Parent_Directory_Name (RELATIVE_PATH_B)'Image);
   Put_Line (Is_Parent_Directory_Name (SIMPLE_PATH_A)'Image);
   Put_Line (Is_Parent_Directory_Name (SIMPLE_PATH_B)'Image);
   Put_Line (Is_Parent_Directory_Name (ROOT_PATH)'Image);
   Put_Line (Is_Parent_Directory_Name (CURRENT_DIR)'Image);
   Put_Line (Is_Parent_Directory_Name (PARENT_DIR)'Image);
   Put_Line (Is_Parent_Directory_Name (RELATIVE_WITH_CURRENT)'Image);
   Put_Line (Is_Parent_Directory_Name (RELATIVE_WITH_PARENT)'Image);

   Put_Line ("Is_Current_Directory_Name");
   Put_Line (Is_Current_Directory_Name (FULL_PATH_A)'Image);
   Put_Line (Is_Current_Directory_Name (FULL_PATH_B)'Image);
   Put_Line (Is_Current_Directory_Name (RELATIVE_PATH_A)'Image);
   Put_Line (Is_Current_Directory_Name (RELATIVE_PATH_B)'Image);
   Put_Line (Is_Current_Directory_Name (SIMPLE_PATH_A)'Image);
   Put_Line (Is_Current_Directory_Name (SIMPLE_PATH_B)'Image);
   Put_Line (Is_Current_Directory_Name (ROOT_PATH)'Image);
   Put_Line (Is_Current_Directory_Name (CURRENT_DIR)'Image);
   Put_Line (Is_Current_Directory_Name (PARENT_DIR)'Image);
   Put_Line (Is_Current_Directory_Name (RELATIVE_WITH_CURRENT)'Image);
   Put_Line (Is_Current_Directory_Name (RELATIVE_WITH_PARENT)'Image);

   Put_Line ("Is_Full_Name");
   Put_Line (Is_Full_Name (FULL_PATH_A)'Image);
   Put_Line (Is_Full_Name (FULL_PATH_B)'Image);
   Put_Line (Is_Full_Name (RELATIVE_PATH_A)'Image);
   Put_Line (Is_Full_Name (RELATIVE_PATH_B)'Image);
   Put_Line (Is_Full_Name (SIMPLE_PATH_A)'Image);
   Put_Line (Is_Full_Name (SIMPLE_PATH_B)'Image);
   Put_Line (Is_Full_Name (ROOT_PATH)'Image);
   Put_Line (Is_Full_Name (CURRENT_DIR)'Image);
   Put_Line (Is_Full_Name (PARENT_DIR)'Image);
   Put_Line (Is_Full_Name (RELATIVE_WITH_CURRENT)'Image);
   Put_Line (Is_Full_Name (RELATIVE_WITH_PARENT)'Image);

   Put_Line ("Relative_Name");
   Put_Line (Is_Relative_Nam

[Ada] Eliminate redundant range checks on conversions

[Pierre-Marie de Rodat]

This gets rid of redundant range checks generated in 5 out of the 9
cases of scalar conversions, i.e. (integer, fixed-point, floating-point)
converted to (integer, fixed-point, floating-point).

The problem is that the Real_Range_Check routine rewrites the conversion
node into a conversion to the base type so, when its parent node is
analyzed, a new conversion to the subtype may be introduced, depending
on the context, giving rise to a second range check against the subtype
bounds.

This change makes Real_Range_Check rewrite the expression of the
conversion node instead of the node, so that the type of the node is
preserved and no new conversion is introduced.  As a matter of fact,
this is exactly what happens in the float-to-float case which goes to
the Generate_Range_Check circuit instead and does not suffer from the
duplication of range checks.

For the following procedure, the compiler must now generate exactly one
range check per nested function:

procedure P is

  type I1 is new Integer range -100 .. 100;

  type I2 is new Integer range -200 .. 200;

  type D1 is delta 0.5 range -100.0 .. 100.0;

  type D2 is delta 0.5 range -200.0 .. 200.0;

  type F1 is new Long_Float range -100.0 .. 100.0;

  type F2 is new Long_Float range -200.0 .. 200.0;

  function Conv (A : I2) return I1 is
  begin
return I1 (A);
  end;

  function Conv (A : D2) return I1 is
  begin
return I1 (A);
  end;

  function Conv (A : F2) return I1 is
  begin
return I1 (A);
  end;

  function Conv (A : I2) return D1 is
  begin
return D1 (A);
  end;

  function Conv (A : D2) return D1 is
  begin
return D1 (A);
  end;

  function Conv (A : F2) return D1 is
  begin
return D1 (A);
  end;

  function Conv (A : I2) return F1 is
  begin
return F1 (A);
  end;

  function Conv (A : D2) return F1 is
  begin
return F1 (A);
  end;

  function Conv (A : F2) return F1 is
  begin
return F1 (A);
  end;

begin
  null;
end;

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* exp_ch4.adb (Real_Range_Check): Do not rewrite the conversion
node but its expression instead, after having fetched its
current value.  Clear the Do_Range_Check flag on entry.  Return
early for a rewritten float-to-float conversion.  Remove
redundant local variable.  Suppress all checks when inserting
the temporary and do not reanalyze the node.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -11229,12 +11229,12 @@ package body Exp_Ch4 is
 
   -- Tnn : typ'Base := typ'Base (x);
   -- [constraint_error when Tnn < typ'First or else Tnn > typ'Last]
-  -- Tnn
+  -- typ (Tnn)
 
   --  This is necessary when there is a conversion of integer to float or
   --  to fixed-point to ensure that the correct checks are made. It is not
-  --  necessary for float to float where it is enough to simply set the
-  --  Do_Range_Check flag.
+  --  necessary for the float-to-float case where it is enough to just set
+  --  the Do_Range_Check flag on the expression.
 
   procedure Real_Range_Check is
  Btyp : constant Entity_Id := Base_Type (Target_Type);
@@ -11246,6 +11246,7 @@ package body Exp_Ch4 is
  Hi_Val : Node_Id;
  Lo_Arg : Node_Id;
  Lo_Val : Node_Id;
+ Expr   : Entity_Id;
  Tnn: Entity_Id;
 
   begin
@@ -11255,6 +11256,12 @@ package body Exp_Ch4 is
 return;
  end if;
 
+ Expr := Expression (N);
+
+ --  Clear the flag once for all
+
+ Set_Do_Range_Check (Expr, False);
+
  --  Nothing to do if range checks suppressed, or target has the same
  --  range as the base type (or is the base type).
 
@@ -11263,22 +11270,24 @@ package body Exp_Ch4 is
   and then
 Hi = Type_High_Bound (Btyp))
  then
---  Unset the range check flag on the current value of
---  Expression (N), since the captured Operand may have
---  been rewritten (such as for the case of a conversion
---  to a fixed-point type).
-
-Set_Do_Range_Check (Expression (N), False);
 return;
  end if;
 
  --  Nothing to do if expression is an entity on which checks have been
  --  suppressed.
 
- if Is_Entity_Name (Operand)
-   and then Range_Checks_Suppressed (Entity (Operand))
+ if Is_Entity_Name (Expr)
+   and then Range_Checks_Suppressed (Entity (Expr))
+ then
+return;
+ end if;
+
+ --  Nothing to do if expression was rewritten into a float-to-float
+ --  conversion, since this kind of conversions is handled elsewhere.
+
+ if Is_Floating_Point_Type (Etype (Expr))
+   and then Is_Floating_Point_Type (Target_Type)
  then
-Set_Do_Range_Check (Expression (N), False);
 return;
 

[Ada] Adapt new extended traversal of AST to have optional part

[Pierre-Marie de Rodat]

The new extended traversal of the AST for GNATprove use now optionally
traverses the ranges under Itypes, based on a formal parameter.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Yannick Moy  

gcc/ada/

* sem_util.adb, sem_util.ads (Traverse_More_Func,
Traverse_More_Proc): Add formal parameter for Itypes traversal.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -25565,11 +25565,13 @@ package body Sem_Util is
null;
  end case;
 
- --  Then process unattached nodes which come from Itypes. This only
- --  concerns currently ranges of scalar (possibly as index) types.
- --  This traversal is protected against looping with Processing_Itype.
+ --  If Process_Itypes is True, process unattached nodes which come
+ --  from Itypes. This only concerns currently ranges of scalar
+ --  (possibly as index) types. This traversal is protected against
+ --  looping with Processing_Itype.
 
- if not Processing_Itype
+ if Process_Itypes
+   and then not Processing_Itype
and then Nkind (Node) in N_Has_Etype
and then Present (Etype (Node))
and then Is_Itype (Etype (Node))
@@ -25628,7 +25630,7 @@ package body Sem_Util is

 
procedure Traverse_More_Proc (Node : Node_Id) is
-  function Traverse is new Traverse_More_Func (Process);
+  function Traverse is new Traverse_More_Func (Process, Process_Itypes);
   Discard : Traverse_Final_Result;
   pragma Warnings (Off, Discard);
begin

--- gcc/ada/sem_util.ads
+++ gcc/ada/sem_util.ads
@@ -2814,14 +2814,17 @@ package Sem_Util is
 
generic
   with function Process (N : Node_Id) return Traverse_Result is <>;
+  Process_Itypes : Boolean := False;
function Traverse_More_Func (Node : Node_Id) return Traverse_Final_Result;
--  This is a version of Atree.Traverse_Func that not only traverses
--  syntactic children of nodes, but also semantic children which are
--  logically children of the node. This concerns currently lists of
--  action nodes and ranges under Itypes, both inserted by the compiler.
+   --  Itypes are only traversed when Process_Itypes is True.
 
generic
   with function Process (N : Node_Id) return Traverse_Result is <>;
+  Process_Itypes : Boolean := False;
procedure Traverse_More_Proc (Node : Node_Id);
pragma Inline (Traverse_More_Proc);
--  This is the same as Traverse_More_Func except that no result is

[Ada] Extended traversal subprograms for GNATprove

[Pierre-Marie de Rodat]

GNATprove needs traversal subprograms that do not simply traverse
syntactic nodes like Atree.Traverse_Func and Atree.Traverse_Proc, but
also traverse semantic nodes which are logically children of the nodes.
Now available through Sem_Util.Traverse_More_Func and
Sem_Util.Traverse_More_Proc.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Yannick Moy  

gcc/ada/

* sem_util.adb, sem_util.ads (Traverse_More_Func,
Traverse_More_Proc): New traversal subprograms.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -26,7 +26,6 @@
 with Treepr; -- ???For debugging code below
 
 with Aspects;  use Aspects;
-with Atree;use Atree;
 with Casing;   use Casing;
 with Checks;   use Checks;
 with Debug;use Debug;
@@ -25437,6 +25436,205 @@ package body Sem_Util is
   end if;
end Transfer_Entities;
 
+   
+   -- Traverse_More_Func --
+   
+
+   function Traverse_More_Func (Node : Node_Id) return Traverse_Final_Result is
+
+  Processing_Itype : Boolean := False;
+  --  Set to True while traversing the nodes under an Itype, to prevent
+  --  looping on Itype handling during that traversal.
+
+  function Process_More (N : Node_Id) return Traverse_Result;
+  --  Wrapper over the Process callback to handle parts of the AST that
+  --  are not normally traversed as syntactic children.
+
+  function Traverse_Rec (N : Node_Id) return Traverse_Final_Result;
+  --  Main recursive traversal implemented as an instantiation of
+  --  Traverse_Func over a modified Process callback.
+
+  --
+  -- Process_More --
+  --
+
+  function Process_More (N : Node_Id) return Traverse_Result is
+
+ procedure Traverse_More (N   : Node_Id;
+  Res : in out Traverse_Result);
+ procedure Traverse_More (L   : List_Id;
+  Res : in out Traverse_Result);
+ --  Traverse a node or list and update the traversal result to value
+ --  Abandon when needed.
+
+ ---
+ -- Traverse_More --
+ ---
+
+ procedure Traverse_More (N   : Node_Id;
+  Res : in out Traverse_Result)
+ is
+ begin
+--  Do not process any more nodes if Abandon was reached
+
+if Res = Abandon then
+   return;
+end if;
+
+if Traverse_Rec (N) = Abandon then
+   Res := Abandon;
+end if;
+ end Traverse_More;
+
+ procedure Traverse_More (L   : List_Id;
+  Res : in out Traverse_Result)
+ is
+N : Node_Id := First (L);
+
+ begin
+--  Do not process any more nodes if Abandon was reached
+
+if Res = Abandon then
+   return;
+end if;
+
+while Present (N) loop
+   Traverse_More (N, Res);
+   Next (N);
+end loop;
+ end Traverse_More;
+
+ --  Local variables
+
+ Node   : Node_Id;
+ Result : Traverse_Result;
+
+  --  Start of processing for Process_More
+
+  begin
+ --  Initial callback to Process. Return immediately on Skip/Abandon.
+ --  Otherwise update the value of Node for further processing of
+ --  non-syntactic children.
+
+ Result := Process (N);
+
+ case Result is
+when OK  => Node := N;
+when OK_Orig => Node := Original_Node (N);
+when Skip=> return Skip;
+when Abandon => return Abandon;
+ end case;
+
+ --  Process the relevant semantic children which are a logical part of
+ --  the AST under this node before returning for the processing of
+ --  syntactic children.
+
+ --  Start with all non-syntactic lists of action nodes
+
+ case Nkind (Node) is
+when N_Component_Association =>
+   Traverse_More (Loop_Actions (Node),  Result);
+
+when N_Elsif_Part =>
+   Traverse_More (Condition_Actions (Node), Result);
+
+when N_Short_Circuit =>
+   Traverse_More (Actions (Node),   Result);
+
+when N_Case_Expression_Alternative =>
+   Traverse_More (Actions (Node),   Result);
+
+when N_Iteration_Scheme =>
+   Traverse_More (Condition_Actions (Node), Result);
+
+when N_If_Expression =>
+   Traverse_More (Then_Actions (Node),  Result);
+   Traverse_More (Else_Actions (Node),  Result);
+
+--  Various nodes have a field Actions as a syntactic node,
+--  so it will be traversed in the regular syntactic traversal.
+
+when N_Compilation_Unit_Aux
+  

[Ada] Fix missing range check for In/Out parameter with -gnatVa

[Pierre-Marie de Rodat]

This plugs another small loophole in the front-end which fails to
generate a range check for a scalar In/Out parameter when -gnatVa is
specified.  This also fixes a few more leaks of the Do_Range_Check flag
on actual parameters, both in regular and -gnatVa modes, as well as a
leak specific to expression function in -gnatp mode.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* checks.adb (Insert_Valid_Check): Reset the Do_Range_Check flag
on the validated object.
* exp_ch6.adb (Add_Call_By_Copy_Code): Reset the Do_Range_Check
flag on the actual here, as well as on the Expression if the
actual is a N_Type_Conversion node.
(Add_Validation_Call_By_Copy_Code): Generate the incoming range
check if needed and reset the Do_Range_Check flag on the
Expression if the actual is a N_Type_Conversion node.
(Expand_Actuals): Do not reset the Do_Range_Check flag here.
Generate the incoming range check for In parameters here instead
of...
(Expand_Call_Helper): ...here.  Remove redudant condition.
* sem_res.adb (Resolve_Actuals): Use local variable A_Typ and
remove obsolete comments.
(Resolve_Type_Conversion): Do not force the Do_Range_Check flag
on the operand if range checks are suppressed.

gcc/testsuite/

* gnat.dg/range_check6.adb: New testcase.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -7588,8 +7588,12 @@ package body Checks is
   Suppress => Validity_Check);
 
 Set_Validated_Object (Var_Id, New_Copy_Tree (Exp));
+
+--  Reset the Do_Range_Check flag so it doesn't leak elsewhere
+
+Set_Do_Range_Check (Validated_Object (Var_Id), False);
+
 Rewrite (Exp, New_Occurrence_Of (Var_Id, Loc));
-PV := New_Occurrence_Of (Var_Id, Loc);
 
 --  Copy the Do_Range_Check flag over to the new Exp, so it doesn't
 --  get lost. Floating point types are handled elsewhere.
@@ -7598,6 +7602,8 @@ package body Checks is
Set_Do_Range_Check (Exp, Do_Range_Check (Original_Node (Exp)));
 end if;
 
+PV := New_Occurrence_Of (Var_Id, Loc);
+
  --  Otherwise the expression does not denote a variable. Force its
  --  evaluation by capturing its value in a constant. Generate:
 

--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -1295,7 +1295,14 @@ package body Exp_Ch6 is
 Indic := New_Occurrence_Of (F_Typ, Loc);
  end if;
 
+ --  The new code will be properly analyzed below and the setting of
+ --  the Do_Range_Check flag recomputed so remove the obsolete one.
+
+ Set_Do_Range_Check (Actual, False);
+
  if Nkind (Actual) = N_Type_Conversion then
+Set_Do_Range_Check (Expression (Actual), False);
+
 V_Typ := Etype (Expression (Actual));
 
 --  If the formal is an (in-)out parameter, capture the name
@@ -1689,6 +1696,20 @@ package body Exp_Ch6 is
  Var_Id  : Entity_Id;
 
   begin
+ --  Generate range check if required
+
+ if Do_Range_Check (Actual) then
+Generate_Range_Check (Actual, E_Formal, CE_Range_Check_Failed);
+ end if;
+
+ --  If there is a type conversion in the actual, it will be reinstated
+ --  below, the new instance will be properly analyzed and the setting
+ --  of the Do_Range_Check flag recomputed so remove the obsolete one.
+
+ if Nkind (Actual) = N_Type_Conversion then
+Set_Do_Range_Check (Expression (Actual), False);
+ end if;
+
  --  Copy the value of the validation variable back into the object
  --  being validated.
 
@@ -2073,14 +2094,6 @@ package body Exp_Ch6 is
 (Ekind (Formal) = E_In_Out_Parameter
   and then not In_Subrange_Of (E_Actual, E_Formal)))
 then
-   --  Perhaps the setting back to False should be done within
-   --  Add_Call_By_Copy_Code, since it could get set on other
-   --  cases occurring above???
-
-   if Do_Range_Check (Actual) then
-  Set_Do_Range_Check (Actual, False);
-   end if;
-
Add_Call_By_Copy_Code;
 end if;
 
@@ -2194,6 +2207,12 @@ package body Exp_Ch6 is
  --  Processing for IN parameters
 
  else
+--  Generate range check if required
+
+if Do_Range_Check (Actual) then
+   Generate_Range_Check (Actual, E_Formal, CE_Range_Check_Failed);
+end if;
+
 --  For IN parameters in the bit-packed array case, we expand an
 --  indexed component (the circuit in Exp_Ch4 deliberately left
 --  indexed components appearing as actuals untouched, so that
@@ -3054,16 +3073,6 @@ package body Exp_Ch6 is
   Actual := First_Actual (Call

[Ada] Fix incorrect Do_Range_Check on type conversion

[Pierre-Marie de Rodat]

This gets rid of another leak of the Do_Range_Check flag to the back-end
which is specific to expression functions.  No functional changes.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* checks.adb (Activate_Range_Check): Remove redundant argument.
(Generate_Range_Check): Likewise.
(Apply_Float_Conversion_Check): Reset the Do_Range_Check flag on
entry and remove redundant condition.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -445,7 +445,7 @@ package body Checks is
 
procedure Activate_Range_Check (N : Node_Id) is
begin
-  Set_Do_Range_Check (N, True);
+  Set_Do_Range_Check (N);
   Possible_Local_Raise (N, Standard_Constraint_Error);
end Activate_Range_Check;
 
@@ -2031,6 +2031,12 @@ package body Checks is
  return;
   end if;
 
+  --  Here we will generate an explicit range check, so we don't want to
+  --  set the Do_Range check flag, since the range check is taken care of
+  --  by the code we will generate.
+
+  Set_Do_Range_Check (Ck_Node, False);
+
   if not Compile_Time_Known_Value (LB)
   or not Compile_Time_Known_Value (HB)
   then
@@ -2079,7 +2085,6 @@ package body Checks is
   if Nkind (Ck_Node) = N_Real_Literal
 and then Etype (Ck_Node) = Universal_Real
 and then Is_Integer_Type (Target_Typ)
-and then Nkind (Parent (Ck_Node)) = N_Type_Conversion
   then
  declare
 Int_Val : constant Uint := UR_To_Uint (Realval (Ck_Node));
@@ -6936,7 +6941,7 @@ package body Checks is
   --  flag set, we do not want to generate the explicit range check code.
 
   if GNATprove_Mode or else not Expander_Active then
- Set_Do_Range_Check (N, True);
+ Set_Do_Range_Check (N);
  return;
   end if;
 

[Ada] Add special bypass for obsolete code pattern

[Pierre-Marie de Rodat]

This change prevents the analysis phase of the front-end from setting
the Do_Range_Check flag in the very peculiar case of the source of a
conversion whose result is passed by reference to a "valued procedure",
because the expansion phase would not be able to generate the check.

This pattern appears in the ancient DEC Starlet package and it doesn't
seem to be useful at this point to change the expander to deal with it,
so instead the analysis phase is adjusted.  Morever the compiler already
issues a warning in this case so this is probably good enough.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-08-12  Eric Botcazou  

gcc/ada/

* sem_res.adb: Add with & use clause for Sem_Mech and
alphabetize.
(Resolve_Actuals): Do not apply a scalar range check for the
source of a conversion whose result is passed by reference to a
valued procedure.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -30,9 +30,9 @@ with Debug_A;  use Debug_A;
 with Einfo;use Einfo;
 with Errout;   use Errout;
 with Expander; use Expander;
-with Exp_Disp; use Exp_Disp;
 with Exp_Ch6;  use Exp_Ch6;
 with Exp_Ch7;  use Exp_Ch7;
+with Exp_Disp; use Exp_Disp;
 with Exp_Tss;  use Exp_Tss;
 with Exp_Util; use Exp_Util;
 with Freeze;   use Freeze;
@@ -51,12 +51,12 @@ with Restrict; use Restrict;
 with Rident;   use Rident;
 with Rtsfind;  use Rtsfind;
 with Sem;  use Sem;
-with Sem_Aux;  use Sem_Aux;
 with Sem_Aggr; use Sem_Aggr;
 with Sem_Attr; use Sem_Attr;
+with Sem_Aux;  use Sem_Aux;
 with Sem_Cat;  use Sem_Cat;
-with Sem_Ch4;  use Sem_Ch4;
 with Sem_Ch3;  use Sem_Ch3;
+with Sem_Ch4;  use Sem_Ch4;
 with Sem_Ch6;  use Sem_Ch6;
 with Sem_Ch8;  use Sem_Ch8;
 with Sem_Ch13; use Sem_Ch13;
@@ -67,9 +67,9 @@ with Sem_Elab; use Sem_Elab;
 with Sem_Elim; use Sem_Elim;
 with Sem_Eval; use Sem_Eval;
 with Sem_Intr; use Sem_Intr;
-with Sem_Util; use Sem_Util;
-with Targparm; use Targparm;
+with Sem_Mech; use Sem_Mech;
 with Sem_Type; use Sem_Type;
+with Sem_Util; use Sem_Util;
 with Sem_Warn; use Sem_Warn;
 with Sinfo;use Sinfo;
 with Sinfo.CN; use Sinfo.CN;
@@ -77,6 +77,7 @@ with Snames;   use Snames;
 with Stand;use Stand;
 with Stringt;  use Stringt;
 with Style;use Style;
+with Targparm; use Targparm;
 with Tbuild;   use Tbuild;
 with Uintp;use Uintp;
 with Urealp;   use Urealp;
@@ -4613,8 +4614,19 @@ package body Sem_Res is
 
if Nkind (A) = N_Type_Conversion then
   if Is_Scalar_Type (A_Typ) then
- Apply_Scalar_Range_Check
-   (Expression (A), Etype (Expression (A)), A_Typ);
+
+ --  Special case here tailored to Exp_Ch6.Is_Legal_Copy,
+ --  which would prevent the check from being generated.
+ --  This is for Starlet only though, so long obsolete.
+
+ if Mechanism (F) = By_Reference
+   and then Is_Valued_Procedure (Nam)
+ then
+null;
+ else
+Apply_Scalar_Range_Check
+  (Expression (A), Etype (Expression (A)), A_Typ);
+ end if;
 
  --  In addition the return value must meet the constraints
  --  of the object type (see the comment below).

Re: [patch] handle casesi dispatch insns in create_trace_edges

[Olivier Hainque]

Hi Jeff,

Thanks for your prompt feedback :-)

>>* rtl.h (tablejump_casesi_pattern): New helper, casesi
>>recognition logic originating from code in cfgrtl.c.
>>* cfgrtl.c (patch_jump_insn): Use it.
>>* dwarf2cfi.c (create_trace_edges): Handle casesi patterns.

> Is there a reason to think the routine is performance critical enough to
> be inlined?

No, no particular reason. Just an implicit thought that having a new
state as close as possible from the previous one was of interest.

>  If not it would make more sense to me to put it into rtl.c
> with just a declaration in rtl.h

> So if it is performance critical, then the patch is OK as-is.  If not,
> moving the implementation into rtl.c with a declaration in rtl.h should
> be considered pre-approved -- just post it here for archival purposes.

Understood, thanks!

Searching for tablejump related prototypes in rtl.h led to
declarations for functions in rtlanal.c. Wouldn't that be a
better place than rtl.c for the new function ?

Thanks in advance,

Olivier

Re: [Patch v2] Enable math functions linking with static library for LTO

[Richard Biener]

On Mon, Aug 12, 2019 at 8:50 AM luoxhu  wrote:
>
> Hi Richard,
> Thanks for your comments, updated the v2 patch as below:
> 1. Define and use builtin_with_linkage_p.
> 2. Add comments.
> 3. Add a testcase.
>
> In LTO mode, if static library and dynamic library contains same
> function and both libraries are passed as arguments, linker will link
> the function in dynamic library no matter the sequence.  This patch
> will output LTO symbol node as UNDEF if BUILT_IN_NORMAL function FNDECL
> is a math function, then the function in static library will be linked
> first if its sequence is ahead of the dynamic library.

Comments below

> gcc/ChangeLog
>
> 2019-08-12  Xiong Hu Luo  
>
> PR lto/91287
> * builtins.c (builtin_with_linkage_p): New function.
> * builtins.h (builtin_with_linkage_p): New function.
> * symtab.c (write_symbol): Use builtin_with_linkage_p.
> * lto-streamer-out.c (symtab_node::output_to_lto_symbol_table_p):
> Likewise.
>
> gcc/testsuite/ChangeLog
>
> 2019-08-12  Xiong Hu Luo  
>
> PR lto/91287
> * gcc.dg/pr91287.c: New testcase.
> ---
>  gcc/builtins.c | 89 ++
>  gcc/builtins.h |  2 +
>  gcc/lto-streamer-out.c |  4 +-
>  gcc/symtab.c   | 13 -
>  gcc/testsuite/gcc.dg/pr91287.c | 40 +++
>  5 files changed, 145 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr91287.c
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 695a9d191af..f4dea941a27 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -11244,3 +11244,92 @@ target_char_cst_p (tree t, char *p)
>*p = (char)tree_to_uhwi (t);
>return true;
>  }
> +
> +/* Return true if DECL is a specified builtin math function.  These functions
> +   should have symbol in symbol table to provide linkage with faster version 
> of
> +   libraries.  */

The comment should read like

/* Return true if the builtin DECL is implemented in a standard
library.  Otherwise
   returns false which doesn't guarantee it is not (thus the list of
handled builtins
   below may be incomplete).  */

> +bool
> +builtin_with_linkage_p (tree decl)
> +{
> +  if (!decl)
> +return false;

Omit this check please.

> +  if (DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL)
> +switch (DECL_FUNCTION_CODE (decl))
> +{
> +  CASE_FLT_FN (BUILT_IN_ACOS):
> +  CASE_FLT_FN (BUILT_IN_ACOSH):
> +  CASE_FLT_FN (BUILT_IN_ASIN):
> +  CASE_FLT_FN (BUILT_IN_ASINH):
> +  CASE_FLT_FN (BUILT_IN_ATAN):
> +  CASE_FLT_FN (BUILT_IN_ATANH):
> +  CASE_FLT_FN (BUILT_IN_ATAN2):
> +  CASE_FLT_FN (BUILT_IN_CBRT):
> +  CASE_FLT_FN (BUILT_IN_CEIL):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_CEIL):
> +  CASE_FLT_FN (BUILT_IN_COPYSIGN):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_COPYSIGN):
> +  CASE_FLT_FN (BUILT_IN_COS):
> +  CASE_FLT_FN (BUILT_IN_COSH):
> +  CASE_FLT_FN (BUILT_IN_ERF):
> +  CASE_FLT_FN (BUILT_IN_ERFC):
> +  CASE_FLT_FN (BUILT_IN_EXP):
> +  CASE_FLT_FN (BUILT_IN_EXP2):
> +  CASE_FLT_FN (BUILT_IN_EXPM1):
> +  CASE_FLT_FN (BUILT_IN_FABS):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FABS):
> +  CASE_FLT_FN (BUILT_IN_FDIM):
> +  CASE_FLT_FN (BUILT_IN_FLOOR):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FLOOR):
> +  CASE_FLT_FN (BUILT_IN_FMA):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMA):
> +  CASE_FLT_FN (BUILT_IN_FMAX):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMAX):
> +  CASE_FLT_FN (BUILT_IN_FMIN):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_FMIN):
> +  CASE_FLT_FN (BUILT_IN_FMOD):
> +  CASE_FLT_FN (BUILT_IN_FREXP):
> +  CASE_FLT_FN (BUILT_IN_HYPOT):
> +  CASE_FLT_FN (BUILT_IN_ILOGB):
> +  CASE_FLT_FN (BUILT_IN_LDEXP):
> +  CASE_FLT_FN (BUILT_IN_LGAMMA):
> +  CASE_FLT_FN (BUILT_IN_LLRINT):
> +  CASE_FLT_FN (BUILT_IN_LLROUND):
> +  CASE_FLT_FN (BUILT_IN_LOG):
> +  CASE_FLT_FN (BUILT_IN_LOG10):
> +  CASE_FLT_FN (BUILT_IN_LOG1P):
> +  CASE_FLT_FN (BUILT_IN_LOG2):
> +  CASE_FLT_FN (BUILT_IN_LOGB):
> +  CASE_FLT_FN (BUILT_IN_LRINT):
> +  CASE_FLT_FN (BUILT_IN_LROUND):
> +  CASE_FLT_FN (BUILT_IN_MODF):
> +  CASE_FLT_FN (BUILT_IN_NAN):
> +  CASE_FLT_FN (BUILT_IN_NEARBYINT):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_NEARBYINT):
> +  CASE_FLT_FN (BUILT_IN_NEXTAFTER):
> +  CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
> +  CASE_FLT_FN (BUILT_IN_POW):
> +  CASE_FLT_FN (BUILT_IN_REMAINDER):
> +  CASE_FLT_FN (BUILT_IN_REMQUO):
> +  CASE_FLT_FN (BUILT_IN_RINT):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_RINT):
> +  CASE_FLT_FN (BUILT_IN_ROUND):
> +  CASE_FLT_FN_FLOATN_NX (BUILT_IN_ROUND):
> +  CASE_FLT_FN (BUILT_IN_SCALBLN):
> +  CASE_FLT_FN (BUILT_IN_SCALBN):
> +  CASE_FLT_FN (BUILT_IN_SIN):
> +  CASE_FLT_FN (BUILT_IN_SINH):
> +  CASE_FLT_FN (BUILT_IN_SINCOS):
> +  CASE_FLT_FN (BUILT_IN_S

Re: C++ PATCH for c++/91416 - GC during late parsing collects live data

[Richard Biener]

On Sun, Aug 11, 2019 at 7:12 PM Marek Polacek  wrote:
>
> This is a crash that points to a GC problem.  Consider this test:
>
>   __attribute__ ((unused)) struct S {
> S() { }
>   } s;
>
> We're parsing a simple-declaration.  While parsing the decl specs, we parse 
> the
> attribute, which means creating a TREE_LIST using ggc_alloc_*.
>
> A function body is a complete-class context so when parsing the
> member-specification of this class-specifier, we parse the bodies of the
> functions we'd queued in cp_parser_late_parsing_for_member.  This then leads 
> to
> this call chain:
> cp_parser_function_definition_after_declarator -> expand_or_defer_fn ->
> expand_or_defer_fn_1 -> maybe_clone_body -> expand_or_defer_fn ->
> cgraph_node::finalize_function -> ggc_collect.
>
> In this test, the ggc_collect call collects the TREE_LIST we had allocated, 
> and
> a crash duly ensues.  We can't avoid late parsing of members in this context,
> so my fix is to bump function_depth, exactly following cp_parser_lambda_body.
> Since we are performing late parsing, we know we have to be nested in a class.
> (We still ggc_collect later, in c_parse_final_cleanups.)

So the struct S itself is properly referenced by a GC root?  If so why not
attach the attribute list to the tentative struct instead?  Or do we
fear we have
other non-rooted data live at the point we collect?  If so shouldn't we instead
bump function_depth when parsing a declaration in general?

>
> But here's the thing.  This goes back to ancient r104500, at least.  How has
> this not broken before?  All you need to trigger it is to enable GC checking
> and have a class with a ctor/member function, that has an attribute.  You'd
> think that since we've got hundreds of those in the testsuite, at least one of
> them would trigger this crash.  Or that there'd be several reports about this 
> in
> the BZ already.  Yet I didn't find any.  Truly, I'm perplexed.
>
> Anyway, bootstrapped/regtested on x86_64-linux, ok for trunk?  How about 9.3?
> I vote yes.
>
> 2019-08-11  Marek Polacek  
>
> PR c++/91416 - GC during late parsing collects live data.
> * parser.c (cp_parser_late_parsing_for_member): Increment 
> function_depth
> around call to cp_parser_function_definition_after_declarator.
>
> * g++.dg/other/gc6.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index b56cc6924f4..0d4d32e9670 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -28934,6 +28934,8 @@ cp_parser_late_parsing_for_member (cp_parser* parser, 
> tree member_function)
>function_scope = current_function_decl;
>if (function_scope)
> push_function_context ();
> +  else
> +   ++function_depth;
>
>/* Push the body of the function onto the lexer stack.  */
>cp_parser_push_lexer_for_tokens (parser, tokens);
> @@ -28966,6 +28968,9 @@ cp_parser_late_parsing_for_member (cp_parser* parser, 
> tree member_function)
>/* Leave the scope of the containing function.  */
>if (function_scope)
> pop_function_context ();
> +  else
> +   --function_depth;
> +
>cp_parser_pop_lexer (parser);
>  }
>
> diff --git gcc/testsuite/g++.dg/other/gc6.C gcc/testsuite/g++.dg/other/gc6.C
> new file mode 100644
> index 000..385be5c945e
> --- /dev/null
> +++ gcc/testsuite/g++.dg/other/gc6.C
> @@ -0,0 +1,7 @@
> +// PR c++/91416 - GC during late parsing collects live data.
> +// { dg-do compile }
> +// { dg-options "--param ggc-min-heapsize=0 --param ggc-min-expand=0" }
> +
> +__attribute__ ((unused)) struct S {
> +  S() { }
> +} s;

Re: [PATCH] Properly register dead cgraph_nodes in passes.c.

[Martin Liška]

On 8/9/19 4:39 PM, Martin Sebor wrote:
> On 8/9/19 6:41 AM, Martin Liška wrote:
>> Hi.
>>
>> The patch prevents crashes caused by fact that do_per_function_toporder
>> uses get_uid () to register all dead cgraph_nodes. That does not work
>> now as cgraph_nodes are directly released via ggc_free and so that one
>> will see a garbage here. Second steps is to register all cgraph hooks
>> and correctly hold add removed nodes. Doing that we'll not need the GGC nodes
>> array.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>> I can also build xalancbmk with -O2 -ffast-math where I previously saw
>> the ICE.
> 
> Just a comment on "style:" to make code more readable, GCC
> coding conventions call for variables to be defined at the same
> time as they're initialized (if possible).  There's still lots
> of legacy C89 code that defines variables at the beginning of
> a function and initializes them much later, but in new C and
> C++ code we have the opportunity to follow it.
> 
> Martin

Sure, I'm sending updated version of the patch.

Martin

> 
> PS For some additional background see DCL19-C in the CERT C
> Coding Standard.  A warning that helped find opportunities to
> reduce the scope of variables would be quite useful.
> 
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-08-09  Martin Liska  
>>
>> PR ipa/91404
>> * passes.c (order): Remove.
>> (uid_hash_t): Likewise).
>> (remove_cgraph_node_from_order): Remove from set
>> of pointers (cgraph_node *).
>> (insert_cgraph_node_to_order): New.
>> (duplicate_cgraph_node_to_order): New.
>> (do_per_function_toporder): Register all 3 cgraph hooks.
>> Skip removed_nodes now as we know about all of them.
>> ---
>>   gcc/passes.c | 69 +---
>>   1 file changed, 44 insertions(+), 25 deletions(-)
>>
>>
> 

>From 1700d3b86e3acd3d6603d9d804f5b0c8938af56e Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Fri, 9 Aug 2019 13:50:31 +0200
Subject: [PATCH] Properly register dead cgraph_nodes in passes.c.

gcc/ChangeLog:

2019-08-09  Martin Liska  

	PR ipa/91404
	* passes.c (order): Remove.
	(uid_hash_t): Likewise).
	(remove_cgraph_node_from_order): Remove from set
	of pointers (cgraph_node *).
	(insert_cgraph_node_to_order): New.
	(duplicate_cgraph_node_to_order): New.
	(do_per_function_toporder): Register all 3 cgraph hooks.
	Skip removed_nodes now as we know about all of them.
---
 gcc/passes.c | 68 +---
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/gcc/passes.c b/gcc/passes.c
index bd56004d909..f715c67ab65 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1646,24 +1646,39 @@ do_per_function (void (*callback) (function *, void *data), void *data)
 }
 }
 
-/* Because inlining might remove no-longer reachable nodes, we need to
-   keep the array visible to garbage collector to avoid reading collected
-   out nodes.  */
-static int nnodes;
-static GTY ((length ("nnodes"))) cgraph_node **order;
-
-#define uid_hash_t hash_set >
-
 /* Hook called when NODE is removed and therefore should be
excluded from order vector.  DATA is a hash set with removed nodes.  */
 
 static void
 remove_cgraph_node_from_order (cgraph_node *node, void *data)
 {
-  uid_hash_t *removed_nodes = (uid_hash_t *)data;
-  removed_nodes->add (node->get_uid ());
+  hash_set *removed_nodes = (hash_set *)data;
+  removed_nodes->add (node);
+}
+
+/* Hook called when NODE is insert and therefore should be
+   excluded from removed_nodes.  DATA is a hash set with removed nodes.  */
+
+static void
+insert_cgraph_node_to_order (cgraph_node *node, void *data)
+{
+  hash_set *removed_nodes = (hash_set *)data;
+  removed_nodes->remove (node);
 }
 
+/* Hook called when NODE is duplicated and therefore should be
+   excluded from removed_nodes.  DATA is a hash set with removed nodes.  */
+
+static void
+duplicate_cgraph_node_to_order (cgraph_node *node, cgraph_node *node2,
+void *data)
+{
+  hash_set *removed_nodes = (hash_set *)data;
+  gcc_checking_assert (!removed_nodes->contains (node));
+  removed_nodes->remove (node2);
+}
+
+
 /* If we are in IPA mode (i.e., current_function_decl is NULL), call
function CALLBACK for every function in the call graph.  Otherwise,
call CALLBACK on the current function.
@@ -1677,26 +1692,30 @@ do_per_function_toporder (void (*callback) (function *, void *data), void *data)
 callback (cfun, data);
   else
 {
-  cgraph_node_hook_list *hook;
-  uid_hash_t removed_nodes;
-  gcc_assert (!order);
-  order = ggc_vec_alloc (symtab->cgraph_count);
+  hash_set removed_nodes;
+  unsigned nnodes = symtab->cgraph_count;
+  cgraph_node **order = XNEWVEC (cgraph_node *, nnodes);
 
   nnodes = ipa_reverse_postorder (order);
   for (i = nnodes - 1; i >= 0; i--)
 	order[i]->process = 1;
-  hook = symtab->add_cgraph_removal_hook (remov