Go patch committed: Sort packages in export data more deterministically

[Ian Lance Taylor]

Different Go packages can have the same package name, but the sorting
done when generating export data was only sorting by package name.
This could cause nondeterministic output.  This patch fixes the
problem by extending the sort comparison function.  Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 270214)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-a69f7c05f1880bb90544fb0c3577109cb1d7f3ab
+8822487ed776d55eafed44de7d89ee54bbfbab47
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/export.cc
===
--- gcc/go/gofrontend/export.cc (revision 270212)
+++ gcc/go/gofrontend/export.cc (working copy)
@@ -509,7 +509,24 @@ Export::set_type_index(Type* type)
 static bool
 packages_compare(const Package* a, const Package* b)
 {
-  return a->package_name() < b->package_name();
+  if (a->package_name() < b->package_name())
+return true;
+  else if (a->package_name() > b->package_name())
+return false;
+
+  if (a->pkgpath() < b->pkgpath())
+return true;
+  else if (a->pkgpath() > b->pkgpath())
+return false;
+
+  // In principle if we get here then a == b.  Try to do something sensible
+  // even if the import information is inconsistent.
+  if (a->pkgpath_symbol() < b->pkgpath_symbol())
+return true;
+  else if (a->pkgpath_symbol() > b->pkgpath_symbol())
+return false;
+
+  return a < b;
 }
 
 // Write out all the known packages whose pkgpath symbol is not a

Re: [PATCH v2] elf.c: initialize struct stat

[Khem Raj]

On Mon, Apr 8, 2019 at 6:47 PM Yu, Mingli  wrote:
>
>
>
> On 2019年04月08日 22:21, Jeff Law wrote:
> > On 4/8/19 12:34 AM, mingli...@windriver.com wrote:
> >> From: Mingli Yu 
> >>
> >> Initialize struct stat to fix the below
> >> build failure when -Og included in compiler flag.
> >> | 
> >> ./../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:
> >>  In function 'elf_is_symlink':
> >> | 
> >> ../../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21:
> >>  error: 'st.st_mode' may be used uninitialized in this function 
> >> [-Werror=maybe-uninitialized]
> >> return S_ISLNK (st.st_mode);
> >>
> >> Signed-off-by: Mingli Yu 
> >> ---
> >>   libbacktrace/elf.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> > This is a false positive in -Wuninitialized that can be worked around by
> > compiling with -O2.  Please do that rather than compiling with -Og.
>
> Thanks Jeff for your respond!
>
> Hi Khem,
>
> If so, do we need to just to add some logic as DEBUG_OPTIMIZATION_append
> = " -Wno-error" for gcc-sanitizers to avoid -Og indicating the
> -Werror=maybe-uninitialized message?
>

yeah that would be fine.

> Thanks,
>
> >
> > jeff
> >

Re: [PATCH v2] elf.c: initialize struct stat

[Yu, Mingli]

On 2019年04月08日 22:21, Jeff Law wrote:

On 4/8/19 12:34 AM, mingli...@windriver.com wrote:

From: Mingli Yu 

Initialize struct stat to fix the below
build failure when -Og included in compiler flag.
| 
./../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:
 In function 'elf_is_symlink':
| 
../../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21:
 error: 'st.st_mode' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
return S_ISLNK (st.st_mode);

Signed-off-by: Mingli Yu 
---
  libbacktrace/elf.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

This is a false positive in -Wuninitialized that can be worked around by
compiling with -O2.  Please do that rather than compiling with -Og.


Thanks Jeff for your respond!

Hi Khem,

If so, do we need to just to add some logic as DEBUG_OPTIMIZATION_append 
= " -Wno-error" for gcc-sanitizers to avoid -Og indicating the 
-Werror=maybe-uninitialized message?


Thanks,



jeff

Re: [PATCH][OBVIOUS] Fix expected scanned pattern.

[Jim Wilson]

On Mon, Apr 8, 2019 at 1:40 AM Martin Liška  wrote:
> * gcc.target/riscv/arch-1.c: Fix expected scanned pattern.

Thanks.  I verified it is OK with cross testing.

Jim

Re: [PATCH] Remove usage of apostrophes in error and warning messages (PR translation/89935).

[Jim Wilson]

On Mon, Apr 8, 2019 at 1:10 PM Jim Wilson  wrote:
> The testcase has to be fixed, but there is a locale issue that has me
> confused at the moment, because this testcase is actually working on one
> of my machines and failing on another one.

Confusion sorted.  Turns out one of the trees I was looking at didn't
have the patch yet.  I thought I had already updated it.

Jim

[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

[Nicholas Krause]

This fixes the caller in tsubst_requires_expr to
tsubst_constraint_variables to wrap their respective
trees in PARM_CONSTR_PARMS. This is to get the correct
parmeter constraints from the tree before calling
tsubst_constraint_variables like other callers
in constraint.cc and to fix the bug id, 88395 on
the gcc bugzilla. OK for merge?

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..a78d0a9a49b 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,7 +1882,7 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
-- 
2.17.1

[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

[Nicholas Krause]

This fixes both callers in tsubst_requires_expr to
tsubst_constraint_variables to wrap their respective
trees in PARM_CONSTR_PARMS. This is to get the correct
parmeter constraints from the tree before calling
tsubst_constraint_variables like other callers
in constraint.cc and to fix the bug id, 88395 on
the gcc bugzilla.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..cb06d0ec6e2 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,13 +1882,13 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
 
   tree reqs = TREE_OPERAND (t, 1);
-  reqs = tsubst_requirement_body (reqs, args, complain, in_decl);
+  reqs = tsubst_requirement_body (PARM_CONSTR_PARMS (reqs), args, complain, 
in_decl);
   if (reqs == error_mark_node)
 return error_mark_node;
 
-- 
2.17.1

Re: [PATCH] Fix PR90006

[Richard Sandiford]

Richard Biener  writes:
> The following fixes SLP vectorization to properly consider
> calls like lrint demoting on ilp32 targets.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Richard.
>
> 2019-04-08  Richard Biener  
>
>   PR tree-optimization/90006
>   * tree-vect-data-refs.c (vect_get_smallest_scalar_type): Handle
>   calls like lrint.
>
>   * gcc.dg/vect/bb-slp-pr90006.c: New testcase.
>
> Index: gcc/tree-vect-data-refs.c
> ===
> --- gcc/tree-vect-data-refs.c (revision 270202)
> +++ gcc/tree-vect-data-refs.c (working copy)
> @@ -144,6 +144,15 @@ vect_get_smallest_scalar_type (gimple *s
>if (rhs < lhs)
>  scalar_type = rhs_type;
>  }
> +  else if (is_gimple_call (stmt)
> +&& gimple_call_num_args (stmt) > 0)
> +{
> +  tree rhs_type = TREE_TYPE (gimple_call_arg (stmt, 0));
> +
> +  rhs = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (rhs_type));
> +  if (rhs < lhs)
> +scalar_type = rhs_type;
> +}
>  
>*lhs_size_unit = lhs;
>*rhs_size_unit = rhs;

Looks like this causes a few regressions on SVE.  One is that we reach
here before doing much sanity checking, so for SLP we can see vectorised
calls with variable-length parameters.  That's easily fixed with the
same tree_fits_uhwi_p as for lhs.

The more difficult one is that we have various internal functions
whose first parameter isn't interesting for finding vector types.
E.g. for conditional internal functions like IFN_COND_DIV, the first
parameter is the boolean condition.  Using that here meant we ended up
thinking we needed vectors of 8-bit data, and so had multiple copies for
wider elements.  (The idea instead is that the condition width adapts
to match the data.)

I think the same thing could happen for masked loads and stores,
where the first parameter is a pointer.  E.g. if we have a 64-bit
load or store on an ILP32 target, we might end up thinking that we
need vectors of 32-bit elements when we don't.

The patch below fixes the cases caught by the testsuite, but I'm
not sure whether there are others lurking.  I guess the problem is
that returning a narrower type than necessary is really a QoI thing:
we just end up unrolling/interleaving the loop more times than
necessary, and most tests wouldn't pick that up.  (Given the PRs
about unrolling, it might accidentally be a good thing. :-))

Tested on aarch64-linux-gnu (with and without SVE) and x86_64-linux-gnu.
OK to install?

Richard


2019-04-08  Richard Sandiford  

gcc/
* tree-vect-data-refs.c (vect_get_smallest_scalar_type): Always
use gimple_expr_type for load and store calls.  Skip over the
condition argument in a conditional internal function.
Protect use of TREE_INT_CST_LOW.

Index: gcc/tree-vect-data-refs.c
===
--- gcc/tree-vect-data-refs.c   2019-04-08 21:55:28.062370229 +0100
+++ gcc/tree-vect-data-refs.c   2019-04-08 21:55:34.382348514 +0100
@@ -145,14 +145,29 @@ vect_get_smallest_scalar_type (stmt_vec_
   if (rhs < lhs)
 scalar_type = rhs_type;
 }
-  else if (is_gimple_call (stmt_info->stmt)
-  && gimple_call_num_args (stmt_info->stmt) > 0)
+  else if (gcall *call = dyn_cast  (stmt_info->stmt))
 {
-  tree rhs_type = TREE_TYPE (gimple_call_arg (stmt_info->stmt, 0));
-
-  rhs = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (rhs_type));
-  if (rhs < lhs)
-scalar_type = rhs_type;
+  unsigned int i = 0;
+  if (gimple_call_internal_p (call))
+   {
+ internal_fn ifn = gimple_call_internal_fn (call);
+ if (internal_load_fn_p (ifn) || internal_store_fn_p (ifn))
+   /* gimple_expr_type already picked the type of the loaded
+  or stored data.  */
+   i = ~0U;
+ else if (internal_fn_mask_index (ifn) == 0)
+   i = 1;
+   }
+  if (i < gimple_call_num_args (call))
+   {
+ tree rhs_type = TREE_TYPE (gimple_call_arg (call, i));
+ if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (rhs_type)))
+   {
+ rhs = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (rhs_type));
+ if (rhs < lhs)
+   scalar_type = rhs_type;
+   }
+   }
 }
 
   *lhs_size_unit = lhs;

Re: Issue with webpage

[Ann Sykes]

Thank you!

It is also happening on this too page
http://repos.inaccess.com/embian/ha/docs/components/embian/1.7/ref_docs/gcc_doc/libstdc++_html/20_util/allocator.html
when I click on ‘the standard librarian: what are allocators good for?'

Have a great day

On Sun, Apr 7, 2019 at 2:20 PM Bernhard Reutner-Fischer <
rep.dot@gmail.com> wrote:

> On 7 April 2019 14:08:39 CEST, Ann Sykes  wrote:
> >Good Afternoon!
> >
> >I’ve just been looking at your website and I came across this webpage:
> >http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02198.html
> >
> >Unfortunately, when I click the link ‘
> >http://www.cuj.com/documents/s=8011/cuj0207stroustr/,’ it redirects me
> >to a
> >payday loan site.
>
> Try the archives. E.g.:
>
>
> https://web.archive.org/web/20041214091933/http://www.cuj.com/documents/s=8011/cuj0207stroustr/
>
> HTH,
> >
> >I thought I should let you know so that you can remove the link.
> >
> >With best regards,
> >
> >Ann
>
>

Re: [PATCH] Remove usage of apostrophes in error and warning messages (PR translation/89935).

[Jim Wilson]

On 4/8/19 1:09 AM, Andreas Schwab wrote:

FAIL: gcc.target/riscv/arch-1.c  (test for errors, line )
FAIL: gcc.target/riscv/arch-1.c (test for excess errors)
Excess errors:
cc1: error: '-march=rv32I': first ISA subset must be 'e', 'i' or 'g'


The testcase has to be fixed, but there is a locale issue that has me 
confused at the moment, because this testcase is actually working on one 
of my machines and failing on another one.  Looking at them, I see that 
one is printing a backquote and one is printing a regular single quote. 
I had thought that the gcc testsuite set the locale environment 
variables to avoid this problem, but maybe there is a problem with the 
riscv.exp file not including something.  I've also seen this testcase 
fail before Martin's patch went in, so this isn't a new problem, just 
shifted around a bit.  Since this is apparently just a testsuite 
problem, it isn't at the top of my todo list at the moment.  Or maybe it 
is a termcap problem not a locale problem.  Or maybe the right fix is to 
just use a regexp to match any quote character.


Jim

New German PO file for 'gcc' (version 9.1-b20190324)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the German team of translators.  The file is available at:

https://translationproject.org/latest/gcc/de.po

(This file, 'gcc-9.1-b20190324.de.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/gcc/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/gcc.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases

[Marek Polacek]

On Mon, Apr 08, 2019 at 03:24:13PM -0400, Jason Merrill wrote:
> On 4/1/19 11:23 AM, Marek Polacek wrote:
> > On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote:
> > > On Mär 28 2019, Marek Polacek  wrote:
> > > 
> > > > Andreas, could you please find out why we're not hitting this code in
> > > > digest_init_r:
> > > > 
> > > > 1210   tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
> > > > 1211   if (reference_related_p (type, TREE_TYPE (elt)))
> > > 
> > > This is never executed if flag_checking is false, of course.
> > 
> > It's certainly wrong for a warning to depend on flag_checking, so this
> > patch corrects it, and I hope will fix the ia64 problem as well.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > 2019-04-01  Marek Polacek  
> > 
> > * typeck2.c (digest_init_r): Don't condition the object slicing warning
> > on flag_checking.
> 
> OK.

Thanks, committed.  Andreas -- I hope the failure is fixed now.

Marek

Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases

[Jason Merrill]

On 4/1/19 11:23 AM, Marek Polacek wrote:

On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote:

On Mär 28 2019, Marek Polacek  wrote:


Andreas, could you please find out why we're not hitting this code in
digest_init_r:

1210   tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
1211   if (reference_related_p (type, TREE_TYPE (elt)))


This is never executed if flag_checking is false, of course.


It's certainly wrong for a warning to depend on flag_checking, so this
patch corrects it, and I hope will fix the ia64 problem as well.

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

2019-04-01  Marek Polacek  

* typeck2.c (digest_init_r): Don't condition the object slicing warning
on flag_checking.


OK.

Jason

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Jonathan Wakely]

On 08/04/19 19:54 +0100, Jonathan Wakely wrote:

On 08/04/19 17:36 +0100, Jonathan Wakely wrote:

On 08/04/19 19:20 +0300, Ville Voutilainen wrote:

On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
 wrote:


On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely  wrote:

The attached patch implements the same thing with totally separate
__gen_vtable_r and __gen_vtable_r_impl class templates, instead of
adding all the visit functionality into the existing code (and then
needing to tease it apart again with if-constexpr).

The visit implementation doesn't need to care about the
__variant_cookie or __variant_idx_cookie cases, which simplifies
things.

This also adjusts some whitespace, for correct indentation and for
readability. And removes a redundant && from a type.

What do you think?


I hate the duplication of __gen_vtable with a burning passion, because
*that* is the part that causes me heartburn,
not the compile-time ifs in the other bits. But if this is what you
want to ship, I can live with it.


A bit of elaboration: in all of this, the most dreadful parts to
understand were _Multi_array and __gen_vtable.


Completely agreed, that's why I found extending __gen_vtable_impl to
handle a new feature made it even harder to understand.


Whereas the attached patch *removes* code from __gen_vtable, but fixes


Patch actually attached this time.



the bug in my original std::visit implementation, *and* fixes a bug
in __visitor_result_type (it doesn't use INVOKE).

The downside of this patch is that it fails to diagnose some
ill-formed visitors where not all invocations return the same type. So
it's not acceptable. But I still think there's a simpler design
struggling to get out.

Please commit your patch as-is. We can revisit (pun intended) this
later if necessary.


diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf624a..42c4bbcc54f 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,7 +138,7 @@ namespace __variant
 constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
 get(const variant<_Types...>&&);
 
-  template
+  template
 constexpr decltype(auto)
 __do_visit(_Visitor&& __visitor, _Variants&&... __variants);
 
@@ -177,8 +177,6 @@ namespace __variant
   struct __variant_cookie {};
   // used for raw visitation with indices passed in
   struct __variant_idx_cookie {};
-  // a more explanatory name than 'true'
-  inline constexpr auto __visit_with_index = bool_constant{};
 
   // _Uninitialized is guaranteed to be a literal type, even if T is not.
   // We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
@@ -377,7 +375,7 @@ namespace __variant
 
   constexpr void _M_reset_impl()
   {
-	__do_visit([](auto&& __this_mem) mutable
+	__do_visit<__detail::__variant::__variant_cookie>([](auto&& __this_mem) mutable
 		   -> __detail::__variant::__variant_cookie
 	  {
 	if constexpr (!is_same_v,
@@ -466,7 +464,7 @@ namespace __variant
 void __variant_construct(_Tp&& __lhs, _Up&& __rhs)
 {
   __lhs._M_index = __rhs._M_index;
-  __do_visit([&__lhs](auto&& __rhs_mem) mutable
+  __do_visit<__detail::__variant::__variant_cookie>([&__lhs](auto&& __rhs_mem) mutable
 		 -> __detail::__variant::__variant_cookie
 {
 	  __variant_construct_single(std::forward<_Tp>(__lhs),
@@ -594,9 +592,10 @@ namespace __variant
   operator=(const _Copy_assign_base& __rhs)
 	  noexcept(_Traits<_Types...>::_S_nothrow_copy_assign)
   {
-	__do_visit<__visit_with_index>([this](auto&& __rhs_mem,
+	using __detail::__variant::__variant_idx_cookie;
+	__do_visit<__variant_idx_cookie>([this](auto&& __rhs_mem,
 	  auto __rhs_index) mutable
-	-> __detail::__variant::__variant_idx_cookie
+	-> __variant_idx_cookie
 	  {
 	if constexpr (__rhs_index != variant_npos)
 	  {
@@ -663,9 +662,10 @@ namespace __variant
   operator=(_Move_assign_base&& __rhs)
 	  noexcept(_Traits<_Types...>::_S_nothrow_move_assign)
   {
-	__do_visit<__visit_with_index>([this](auto&& __rhs_mem,
+	using __detail::__variant::__variant_idx_cookie;
+	__do_visit<__variant_idx_cookie>([this](auto&& __rhs_mem,
 	  auto __rhs_index) mutable
-	-> __detail::__variant::__variant_idx_cookie
+	-> __variant_idx_cookie
 	  {
 	if constexpr (__rhs_index != variant_npos)
 	  {
@@ -951,14 +951,18 @@ namespace __variant
 	return __variant_cookie{};
 	}
 
-  static constexpr decltype(auto)
+  static constexpr _Result_type
   __visit_invoke(_Visitor&& __visitor, _Variants... __vars)
   {
-	if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
-	  return std::__invoke(std::forward<_Visitor>(__visitor),
+   if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
+ return std::__invoke(std::forward<_Visitor>(__visitor),
+__element_by_index_or_cookie<__indices>(
+  std::forward<_Variants>(__vars))...,
+  

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Jonathan Wakely]

On 08/04/19 17:36 +0100, Jonathan Wakely wrote:

On 08/04/19 19:20 +0300, Ville Voutilainen wrote:

On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
 wrote:


On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely  wrote:

The attached patch implements the same thing with totally separate
__gen_vtable_r and __gen_vtable_r_impl class templates, instead of
adding all the visit functionality into the existing code (and then
needing to tease it apart again with if-constexpr).

The visit implementation doesn't need to care about the
__variant_cookie or __variant_idx_cookie cases, which simplifies
things.

This also adjusts some whitespace, for correct indentation and for
readability. And removes a redundant && from a type.

What do you think?


I hate the duplication of __gen_vtable with a burning passion, because
*that* is the part that causes me heartburn,
not the compile-time ifs in the other bits. But if this is what you
want to ship, I can live with it.


A bit of elaboration: in all of this, the most dreadful parts to
understand were _Multi_array and __gen_vtable.


Completely agreed, that's why I found extending __gen_vtable_impl to
handle a new feature made it even harder to understand.


Whereas the attached patch *removes* code from __gen_vtable, but fixes
the bug in my original std::visit implementation, *and* fixes a bug
in __visitor_result_type (it doesn't use INVOKE).

The downside of this patch is that it fails to diagnose some
ill-formed visitors where not all invocations return the same type. So
it's not acceptable. But I still think there's a simpler design
struggling to get out.

Please commit your patch as-is. We can revisit (pun intended) this
later if necessary.

libgo patch committed: Update to 1.12.2

[Ian Lance Taylor]

This patch updates libgo to the 1.12.2 release.  Bootstrapped and ran
Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 270212)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-392e9b3da473070f24dbe6c12c282a0e06e73b54
+a69f7c05f1880bb90544fb0c3577109cb1d7f3ab
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/MERGE
===
--- libgo/MERGE (revision 270212)
+++ libgo/MERGE (working copy)
@@ -1,4 +1,4 @@
-0380c9ad38843d523d9c9804fe300cb7edd7cd3c
+ac02fdec7cd16ea8d3de1fc33def9cfabec5170d
 
 The first line of this file holds the git revision number of the
 last merge done from the master library sources.
Index: libgo/VERSION
===
--- libgo/VERSION   (revision 270212)
+++ libgo/VERSION   (working copy)
@@ -1 +1 @@
-go1.12.1
+go1.12.2
Index: libgo/go/cmd/go/internal/load/pkg.go
===
--- libgo/go/cmd/go/internal/load/pkg.go(revision 270212)
+++ libgo/go/cmd/go/internal/load/pkg.go(working copy)
@@ -1184,6 +1184,36 @@ var cgoSyscallExclude = map[string]bool{
 
 var foldPath = make(map[string]string)
 
+// DefaultExecName returns the default executable name
+// for a package with the import path importPath.
+//
+// The default executable name is the last element of the import path.
+// In module-aware mode, an additional rule is used. If the last element
+// is a vN path element specifying the major version, then the second last
+// element of the import path is used instead.
+func DefaultExecName(importPath string) string {
+   _, elem := pathpkg.Split(importPath)
+   if cfg.ModulesEnabled {
+   // If this is example.com/mycmd/v2, it's more useful to install 
it as mycmd than as v2.
+   // See golang.org/issue/24667.
+   isVersion := func(v string) bool {
+   if len(v) < 2 || v[0] != 'v' || v[1] < '1' || '9' < 
v[1] {
+   return false
+   }
+   for i := 2; i < len(v); i++ {
+   if c := v[i]; c < '0' || '9' < c {
+   return false
+   }
+   }
+   return true
+   }
+   if isVersion(elem) {
+   _, elem = pathpkg.Split(pathpkg.Dir(importPath))
+   }
+   }
+   return elem
+}
+
 // load populates p using information from bp, err, which should
 // be the result of calling build.Context.Import.
 func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
@@ -1226,7 +1256,7 @@ func (p *Package) load(stk *ImportStack,
}
_, elem := filepath.Split(p.Dir)
if cfg.ModulesEnabled {
-   // NOTE(rsc): Using p.ImportPath instead of p.Dir
+   // NOTE(rsc,dmitshur): Using p.ImportPath instead of 
p.Dir
// makes sure we install a package in the root of a
// cached module directory as that package name
// not name@v1.2.3.
@@ -1235,26 +1265,9 @@ func (p *Package) load(stk *ImportStack,
// even for non-module-enabled code,
// but I'm not brave enough to change the
// non-module behavior this late in the
-   // release cycle. Maybe for Go 1.12.
+   // release cycle. Can be done for Go 1.13.
// See golang.org/issue/26869.
-   _, elem = pathpkg.Split(p.ImportPath)
-
-   // If this is example.com/mycmd/v2, it's more useful to 
install it as mycmd than as v2.
-   // See golang.org/issue/24667.
-   isVersion := func(v string) bool {
-   if len(v) < 2 || v[0] != 'v' || v[1] < '1' || 
'9' < v[1] {
-   return false
-   }
-   for i := 2; i < len(v); i++ {
-   if c := v[i]; c < '0' || '9' < c {
-   return false
-   }
-   }
-   return true
-   }
-   if isVersion(elem) {
-   _, elem = 
pathpkg.Split(pathpkg.Dir(p.ImportPath))
-   }
+   elem = DefaultExecName(p.ImportPath)
}
   

Re: Re : add tsv110 pipeline scheduling

[James Greenhalgh]

Thank you for the ChangeLog entry for your patch.

I have applied it to trunk as revision 270212.

We're very late in GCC 9 development, but this patch only impacts TSV
scheduling.

Thanks,
James


On Thu, Apr 04, 2019 at 02:11:12AM +0100, wuyuan (E) wrote:
> Hi ,James:
>  Thank you for your review, Please attach the following author 
> information to the patch.
> 
> 2019-04-04  wu yuan 
> 
>   * config/aarch64/aarch64-cores.def (tsv1100): Change scheduling model.
>   * config/aarch64/aarch64.md : Add "tsv110.md"
>   * config/aarch64/tsv110.md: New file.
>   
> Thanks,
>   wuyuan
> 
> -邮件原件-
> 发件人: James Greenhalgh [mailto:james.greenha...@arm.com] 
> 发送时间: 2019年4月4日 1:58
> 收件人: wuyuan (E) 
> 抄送: Kyrill Tkachov ; gcc-patches@gcc.gnu.org; 
> Zhangyichao (AB) ; Zhanghaijian (A) 
> ; nd 
> 主题: Re: Re : add tsv110 pipeline scheduling
> 
> On Tue, Apr 02, 2019 at 03:26:22PM +0100, wuyuan (E) wrote:
> > Hi ,James:
> > Has the submitted patch been merged into the trunk?  Looking forward to 
> > your reply , thank you very much!   
> > 
> > 
> > 
> > Best Regards,
> > 
> > wuyuan
> 
> Hi Wuyuan,
> 
> This patch is OK for trunk. Thank you for your many clarifications.
> 
> Will you need one of us to apply this to trunk on your behalf?
> 
> If you would like me to apply your patch, please provide the full ChangeLog 
> with author information, like so:
> 
> 2019-04-03  James Greenhalgh  
>   Second Author  
>   Third Author  
> 
>   * config/aarch64/aarch64-cores.def (tsv1100): Change scheduling model.
>   * config/aarch64/aarch64.md : Add "tsv110.md"
>   * config/aarch64/tsv110.md: New file.
> 
> Thanks,
> James
> 
> 
> > -邮件原件-
> > 发件人: wuyuan (E)
> > 发送时间: 2019年3月15日 21:57
> > 收件人: 'James Greenhalgh' 
> > 抄送: Kyrill Tkachov ; 
> > gcc-patches@gcc.gnu.org; Zhangyichao (AB) 
> > ; Zhanghaijian (A) 
> > ; nd ; wufeng (O) 
> > ; Yangfei (Felix) 
> > 主题: Re : add tsv110 pipeline scheduling
> > 
> > Hi , James:
> >  Thank you very much for your meticulous review work. The explanation 
> > of the two questions as follows:
> >  The first problem is caused by my negligence and should be changed to 
> > " crypto_sha256_fast" .
> >   The second question I have verified with the hardware engineer. Only 
> > ALU2/ALU3 could support PSTATE register update so any instruction intends 
> > to update NZCV will be issued to ALU2/ALU3.   MDU could provide a better 
> > pipeline efficiency for multi cycle ALU instruction so we issue 2 cycles 
> > ALU w/o PSTATE update to MDU unit.  the current pipeline processing is  ok  
> > , except the pipeline " tsv110_alu2" should replace with " tsv110_alu2| 
> > tsv110_alu3".
> > 
> > 
> >  
> > 
> > The detailed patches are as follows:
> > 
> >   * config/aarch64/aarch64-cores.def (tsv1100): Change scheduling model.
> >   * config/aarch64/aarch64.md : Add "tsv110.md"
> >   * config/aarch64/tsv110.md: New file.
> > 
> > 
> > diff --git a/gcc/config/aarch64/aarch64-cores.def 
> > b/gcc/config/aarch64/aarch64-cores.def
> > index ed56e5e..82d91d6
> > --- a/gcc/config/aarch64/aarch64-cores.def
> > +++ b/gcc/config/aarch64/aarch64-cores.def
> > @@ -105,7 +105,7 @@ AARCH64_CORE("neoverse-n1",  neoversen1, 
> > cortexa57, 8_2A,  AARCH64_FL_FOR_ARCH8_  AARCH64_CORE("neoverse-e1",  
> > neoversee1, cortexa53, 8_2A,  AARCH64_FL_FOR_ARCH8_2 | AARCH64_FL_F16 
> > | AARCH64_FL_RCPC | AARCH64_FL_DOTPROD | AARCH64_FL_SSBS, cortexa53, 
> > 0x41, 0xd4a, -1)
> >  
> >  /* HiSilicon ('H') cores. */
> > -AARCH64_CORE("tsv110",  tsv110, cortexa57, 8_2A,  AARCH64_FL_FOR_ARCH8_2 | 
> > AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2, 
> > tsv110,   0x48, 0xd01, -1)
> > +AARCH64_CORE("tsv110",  tsv110, tsv110, 8_2A,  AARCH64_FL_FOR_ARCH8_2 | 
> > AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2, 
> > tsv110,   0x48, 0xd01, -1)
> >  
> >  /* ARMv8.4-A Architecture Processors.  */
> >  
> > diff --git a/gcc/config/aarch64/aarch64.md 
> > b/gcc/config/aarch64/aarch64.md index b7cd9fc..861f059 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -361,6 +361,7 @@
> >  (include "thunderx.md")
> >  (include "../arm/xgene1.md")
> >  (include "thunderx2t99.md")
> > +(include "tsv110.md")
> >  
> >  ;; 
> > 

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Jonathan Wakely]

On 08/04/19 19:20 +0300, Ville Voutilainen wrote:

On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
 wrote:


On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely  wrote:
> The attached patch implements the same thing with totally separate
> __gen_vtable_r and __gen_vtable_r_impl class templates, instead of
> adding all the visit functionality into the existing code (and then
> needing to tease it apart again with if-constexpr).
>
> The visit implementation doesn't need to care about the
> __variant_cookie or __variant_idx_cookie cases, which simplifies
> things.
>
> This also adjusts some whitespace, for correct indentation and for
> readability. And removes a redundant && from a type.
>
> What do you think?

I hate the duplication of __gen_vtable with a burning passion, because
*that* is the part that causes me heartburn,
not the compile-time ifs in the other bits. But if this is what you
want to ship, I can live with it.


A bit of elaboration: in all of this, the most dreadful parts to
understand were _Multi_array and __gen_vtable.


Completely agreed, that's why I found extending __gen_vtable_impl to
handle a new feature made it even harder to understand.


The latter is already a recursive template, with this patch we have
two of them. I knew that I could split the hierarchy
for  and non-, but I instinctively went with a pass-through
template parameter that I then handled
in __visit_invoke, so as to specifically avoid splitting the recursive
template into two recursive templates.

That recursive template and how it recurses is not something we are
going to change often, or really reason about
often. But it's something that I need to rack my brain over every
time, whereas looking at an if-constexpr is simple as pie
to me. I suppose there are different levels of familiarity/comfort here.


The if-constexpr itself is easy to understand, but how we get from
__do_visit to a specific specialization of a recursive class template
and a specific if-constexpr condition is the hard part for me to
follow.

But now that I've wrapped my head around the existing code, I'll spend
some more time getting acquainted with your proposed changes and see
if they grow on me overnight.

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Jonathan Wakely]

On 08/04/19 17:02 +0100, Jonathan Wakely wrote:

+  template
+struct __gen_vtable_r_impl<
+  _Multi_array<_Result_type (*)(_Visitor, _Variants...)>,
+  tuple<_Variants...>, std::index_sequence<__indices...>>
+{
+  using _Array_type =
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
+
+  template
+   static constexpr decltype(auto)
+   __element_by_index_or_cookie(_Variant&& __var)
+{
+ if constexpr (__index != variant_npos)
+   return __variant::__get<__index>(std::forward<_Variant>(__var));
+ else
+   return __variant_cookie{};
+   }


I don't think we need __element_by_index_or_cookie for the visit
case, so the function above can go, and ...


+  static constexpr _Result_type
+  __visit_r_invoke(_Visitor&& __visitor, _Variants... __vars)
+  {
+   if constexpr (is_void_v<_Result_type>)
+ return (void) std::__invoke(std::forward<_Visitor>(__visitor),
+   __element_by_index_or_cookie<__indices>(


this just becomes __variant::__get<__indices>(


+ std::forward<_Variants>(__vars))...);
+   else
+ return std::__invoke(std::forward<_Visitor>(__visitor),
+   __element_by_index_or_cookie<__indices>(


and here too.


+ std::forward<_Variants>(__vars))...);
+  }
+
+  static constexpr _Array_type
+  _S_apply()
+  { return _Array_type{&__visit_r_invoke}; }
+};


With the above simplification the visit code is a lot simpler than
the existing __gen_vtable code, let alone the version that adds
visit support to it.

I'll spend some more time thinking about it.



+  template
+struct __gen_vtable_r
+{
+  using _Array_type =
+ _Multi_array<_Result_type (*)(_Visitor&&, _Variants...),
+  variant_size_v>...>;
+
+  static constexpr _Array_type _S_vtable
+   = __gen_vtable_r_impl<_Array_type, tuple<_Variants...>,
+ std::index_sequence<>>::_S_apply();


N.B. here I just declare a variable and its default initializer. I
don't see any benefit to how the original in __gen_vtable does it:

 static constexpr _Array_type
 _S_apply()
 {
return __gen_vtable_impl<_Array_type, tuple<_Variants...>,
 std::index_sequence<>>::_S_apply();
 }

 static constexpr auto _S_vtable = _S_apply();

We only ever use that function in the default initializer, so might as
well get rid of it.

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Ville Voutilainen]

On Mon, 8 Apr 2019 at 19:12, Ville Voutilainen
 wrote:
>
> On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely  wrote:
> > The attached patch implements the same thing with totally separate
> > __gen_vtable_r and __gen_vtable_r_impl class templates, instead of
> > adding all the visit functionality into the existing code (and then
> > needing to tease it apart again with if-constexpr).
> >
> > The visit implementation doesn't need to care about the
> > __variant_cookie or __variant_idx_cookie cases, which simplifies
> > things.
> >
> > This also adjusts some whitespace, for correct indentation and for
> > readability. And removes a redundant && from a type.
> >
> > What do you think?
>
> I hate the duplication of __gen_vtable with a burning passion, because
> *that* is the part that causes me heartburn,
> not the compile-time ifs in the other bits. But if this is what you
> want to ship, I can live with it.

A bit of elaboration: in all of this, the most dreadful parts to
understand were _Multi_array and __gen_vtable.
The latter is already a recursive template, with this patch we have
two of them. I knew that I could split the hierarchy
for  and non-, but I instinctively went with a pass-through
template parameter that I then handled
in __visit_invoke, so as to specifically avoid splitting the recursive
template into two recursive templates.

That recursive template and how it recurses is not something we are
going to change often, or really reason about
often. But it's something that I need to rack my brain over every
time, whereas looking at an if-constexpr is simple as pie
to me. I suppose there are different levels of familiarity/comfort here.

Re: [Patch] [arm] Ensure *neon_mov constraint uses correct mode.

[Kyrill Tkachov]

On 4/8/19 4:51 PM, Matthew Malcomson wrote:

On 08/04/19 14:00, Kyrill Tkachov wrote:

Hi Matthew,

On 4/5/19 12:06 PM, Matthew Malcomson wrote:

Bootstrapped and regtested on arm-none-linux-gnueabihf
Regtested on cross-compiler arm-none-eabi


Does this fix a PR?

If so, could you add the information for it to the ChangeLog?

Does this need backporting to the branches as well?

The patch looks good to me otherwise.


Hi Kyrill,

The bug isn't up on bugzilla, but it is in all branches after gcc-4_9
(i.e. 5,6,7,8) so I do need to backport.


Would you mind filing an appropriate bug report (as a 7/8/9 Regression) 
so that we can keep track of where this is fixed?




Err ... how does one get permission for backporting?


You usually just mention that you want to backport it in your email and 
the approver will approve (or not).


If the branch is in the final steps of releasing (RC created) then you'd 
need release manager approval, but we're not at this stage for the branches.


Kyrill




Cheers,
Matthew


Thanks,

Kyrill

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Ville Voutilainen]

On Mon, 8 Apr 2019 at 19:02, Jonathan Wakely  wrote:
> The attached patch implements the same thing with totally separate
> __gen_vtable_r and __gen_vtable_r_impl class templates, instead of
> adding all the visit functionality into the existing code (and then
> needing to tease it apart again with if-constexpr).
>
> The visit implementation doesn't need to care about the
> __variant_cookie or __variant_idx_cookie cases, which simplifies
> things.
>
> This also adjusts some whitespace, for correct indentation and for
> readability. And removes a redundant && from a type.
>
> What do you think?

I hate the duplication of __gen_vtable with a burning passion, because
*that* is the part that causes me heartburn,
not the compile-time ifs in the other bits. But if this is what you
want to ship, I can live with it.

Re: [PATCH] Implement std::visit for C++2a (P0655R1)

[Jonathan Wakely]

On 06/04/19 03:07 +0300, Ville Voutilainen wrote:

On Sat, 6 Apr 2019 at 02:55, Ville Voutilainen
 wrote:


Just in case that cast looks scary: the implicit conversion is also
deep down in __visit_invoke, so
we do actually require implicit convertibility as we are supposed to.
If that's still too scary,
we can just do

-  return (_Res)
+  if constexpr (!is_void_v<_Res>)
+return
+ __do_visit(std::forward<_Visitor>(__visitor),
+  std::forward<_Variants>(__variants)...);
+  else

like in the attached patch.


Okay, I merged that with the original. I also renamed the neg-test, so
here goes the hopefully final variant change for GCC 9:

2019-04-06  Ville Voutilainen  

   Fix visit for variant.
   * include/std/variant (__do_visit): Add a template parameter
   for enforcing same return types for visit.
   (__gen_vtable_impl): Likewise.
   (_S_apply_single_alt): Adjust.
   (__visit_invoke_impl): New. Handle casting to void.
   (__do_visit_invoke): New. Enforces same return types.
   (__do_visit_invoke_r): New. Converts return types.
   (__visit_invoke): Adjust.
   (__gen_vtable):  Add a template parameter for enforcing
   same return types for visit.
   * testsuite/20_util/variant/visit_r.cc: Add a test for a visitor with
   different return types.
   * testsuite/20_util/variant/visit_neg.cc: New. Ensures that
   visitors with different return types don't accidentally
   compile with regular visitation.


I'm concerned about how dense and impenetrable this code is (the
absence of comments certainly doesn't help).

The attached patch implements the same thing with totally separate
__gen_vtable_r and __gen_vtable_r_impl class templates, instead of
adding all the visit functionality into the existing code (and then
needing to tease it apart again with if-constexpr).

The visit implementation doesn't need to care about the
__variant_cookie or __variant_idx_cookie cases, which simplifies
things.

This also adjusts some whitespace, for correct indentation and for
readability. And removes a redundant && from a type.

What do you think?

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index fdf04cf624a..8370b6c742d 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -138,12 +138,14 @@ namespace __variant
 constexpr variant_alternative_t<_Np, variant<_Types...>> const&&
 get(const variant<_Types...>&&);
 
-  template
+  template
 constexpr decltype(auto)
 __do_visit(_Visitor&& __visitor, _Variants&&... __variants);
 
   template 
-decltype(auto) __variant_cast(_Tp&& __rhs)
+decltype(auto)
+__variant_cast(_Tp&& __rhs)
 {
   if constexpr (is_lvalue_reference_v<_Tp>)
 	{
@@ -829,14 +831,21 @@ namespace __variant
 {
   static constexpr size_t __index =
 	sizeof...(_Variants) - sizeof...(__rest) - 1;
+
   using _Variant = typename _Nth_type<__index, _Variants...>::type;
+
   static constexpr int __do_cookie =
 	_Extra_visit_slot_needed<_Ret, _Variant>::value ? 1 : 0;
+
   using _Tp = _Ret(*)(_Visitor, _Variants...);
+
   template
 	constexpr const _Tp&
 	_M_access(size_t __first_index, _Args... __rest_indices) const
-{ return _M_arr[__first_index + __do_cookie]._M_access(__rest_indices...); }
+	{
+	  return _M_arr[__first_index + __do_cookie]
+	._M_access(__rest_indices...);
+	}
 
   _Multi_array<_Tp, __rest...> _M_arr[__first + __do_cookie];
 };
@@ -880,6 +889,7 @@ namespace __variant
   using _Next =
 	  remove_reference_t::type>;
+
   using _Array_type =
 	  _Multi_array<_Result_type (*)(_Visitor, _Variants...),
 		   __dimensions...>;
@@ -939,7 +949,7 @@ namespace __variant
 		   tuple<_Variants...>, std::index_sequence<__indices...>>
 {
   using _Array_type =
-	  _Multi_array<_Result_type (*)(_Visitor&&, _Variants...)>;
+	  _Multi_array<_Result_type (*)(_Visitor, _Variants...)>;
 
   template
 	static constexpr decltype(auto)
@@ -956,9 +966,9 @@ namespace __variant
   {
 	if constexpr (is_same_v<_Result_type, __variant_idx_cookie>)
 	  return std::__invoke(std::forward<_Visitor>(__visitor),
-	__element_by_index_or_cookie<__indices>(
-	  std::forward<_Variants>(__vars))...,
-	  integral_constant()...);
+__element_by_index_or_cookie<__indices>(
+  std::forward<_Variants>(__vars))...,
+  integral_constant()...);
 	else
 	  return std::__invoke(std::forward<_Visitor>(__visitor),
 	__element_by_index_or_cookie<__indices>(
@@ -988,6 +998,104 @@ namespace __variant
   static constexpr auto _S_vtable = _S_apply();
 };
 
+#if __cplusplus > 201703L
+  // Similar to __gen_vtable_impl but for visit
+  template
+struct __gen_vtable_r_impl;
+
+  template
+struct __gen_vtable_r_impl<
+	_Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
+	tuple<_Variants...>, std::index_sequence<__indices...>>
+{
+  using 

Re: [Patch] [arm] Ensure *neon_mov constraint uses correct mode.

[Matthew Malcomson]

On 08/04/19 14:00, Kyrill Tkachov wrote:
> Hi Matthew,
> 
> On 4/5/19 12:06 PM, Matthew Malcomson wrote:
>> 
>> Bootstrapped and regtested on arm-none-linux-gnueabihf
>> Regtested on cross-compiler arm-none-eabi
>>
> 
> Does this fix a PR?
> 
> If so, could you add the information for it to the ChangeLog?
> 
> Does this need backporting to the branches as well?
> 
> The patch looks good to me otherwise.
> 

Hi Kyrill,

The bug isn't up on bugzilla, but it is in all branches after gcc-4_9 
(i.e. 5,6,7,8) so I do need to backport.

Err ... how does one get permission for backporting?

Cheers,
Matthew

> Thanks,
> 
> Kyrill
> 

[PATCH] netbsd EABI support

[coypu]

Pinging again in the hope of getting the patch in, I'd like to have
less outstanding patches :) (I have quite a few and new releases
can become painful!)

gcc/ChangeLog

config.gcc (arm*-*-netbsdelf*) Add support for EABI configuration
config.host (arm*-*-netbsd*): Build driver-arm.o
config/arm/netbsd-eabi.h: New file.
config/arm/netbsd-elf.h
config/netbsd-elf.h: Define SUBTARGET_EXTRA_SPECS.

libgcc/ChangeLog

config.host (arm*-*-netbsdelf*): Add support for EABI configuration
config/arm/t-netbsd: LIB1ASMFUNCS: Append to existing set.
 HOST_LIBGCC2_CFLAGS: workaround possible bug
config/arm/t-netbsd-eabi: New file.

---
 gcc/config.gcc  | 29 +-
 gcc/config.host |  2 +-
 gcc/config/arm/netbsd-eabi.h| 97 +
 gcc/config/arm/netbsd-elf.h | 10 
 gcc/config/netbsd-elf.h | 15 +
 libgcc/config.host  | 10 +++-
 libgcc/config/arm/t-netbsd  | 10 +++-
 libgcc/config/arm/t-netbsd-eabi | 18 ++
 8 files changed, 184 insertions(+), 7 deletions(-)
 create mode 100644 gcc/config/arm/netbsd-eabi.h
 create mode 100644 libgcc/config/arm/t-netbsd-eabi

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 3ee31c5993d..736b2163a24 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1161,10 +1161,33 @@ arm*-*-freebsd*)# ARM FreeBSD EABI
with_tls=${with_tls:-gnu}
;;
 arm*-*-netbsdelf*)
-   tm_file="dbxelf.h elfos.h ${nbsd_tm_file} arm/elf.h arm/aout.h 
${tm_file} arm/netbsd-elf.h"
-   extra_options="${extra_options} netbsd.opt netbsd-elf.opt"
-   tmake_file="${tmake_file} arm/t-arm"
target_cpu_cname="strongarm"
+   tmake_file="${tmake_file} arm/t-arm"
+   tm_file="dbxelf.h elfos.h ${nbsd_tm_file} arm/elf.h"
+   extra_options="${extra_options} netbsd.opt netbsd-elf.opt"
+   case ${target} in
+   arm*eb-*) tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" ;;
+   esac
+   case ${target} in
+   arm*-*-netbsdelf-*eabi*)
+   tm_file="$tm_file arm/bpabi.h arm/netbsd-elf.h arm/netbsd-eabi.h"
+   tmake_file="$tmake_file arm/t-bpabi arm/t-netbsdeabi"
+   ;;
+   *)
+   tm_file="$tm_file arm/netbsd-elf.h"
+   tmake_file="$tmake_file arm/t-netbsd"
+   ;;
+   esac
+   tm_file="${tm_file} arm/aout.h arm/arm.h"
+   case ${target} in
+   arm*-*-netbsdelf-*eabihf*)
+   tm_defines="${tm_defines} 
TARGET_DEFAULT_FLOAT_ABI=ARM_FLOAT_ABI_HARD"
+   ;;
+   esac
+   case ${target} in
+   armv6*) target_cpu_cname="arm1176jzf-s";;
+   armv7*) target_cpu_cname="generic-armv7-a";;
+   esac
;;
 arm*-*-linux-*)# ARM GNU/Linux with ELF
tm_file="dbxelf.h elfos.h gnu-user.h linux.h linux-android.h 
glibc-stdint.h arm/elf.h arm/linux-gas.h arm/linux-elf.h"
diff --git a/gcc/config.host b/gcc/config.host
index 816a0f06cb7..5077c0ee33a 100644
--- a/gcc/config.host
+++ b/gcc/config.host
@@ -107,7 +107,7 @@ case ${host} in
;;
 esac
 ;;
-  arm*-*-freebsd* | arm*-*-linux* | arm*-*-fuchsia*)
+  arm*-*-freebsd* | arm*-*-netbsd* | arm*-*-linux* | arm*-*-fuchsia*)
 case ${target} in
   arm*-*-*)
host_extra_gcc_objs="driver-arm.o"
diff --git a/gcc/config/arm/netbsd-eabi.h b/gcc/config/arm/netbsd-eabi.h
new file mode 100644
index 000..13bc274175d
--- /dev/null
+++ b/gcc/config/arm/netbsd-eabi.h
@@ -0,0 +1,97 @@
+/* Definitions of target machine for GNU compiler, NetBSD/arm ELF version.
+   Copyright (C) 2002, 2003, 2004, 2005, 2007 Free Software Foundation, Inc.
+   Contributed by Wasabi Systems, Inc.
+
+   This file is part of GCC.
+
+   GCC 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.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   .  */
+
+/* Run-time Target Specification.  */
+#undef MULTILIB_DEFAULTS
+#define MULTILIB_DEFAULTS { "mabi=aapcs-linux" }
+
+#define TARGET_LINKER_EABI_SUFFIX \
+(TARGET_DEFAULT_FLOAT_ABI == ARM_FLOAT_ABI_SOFT \
+ ? "%{!mabi=apcs-gnu:%{!mabi=atpcs:%{mfloat-abi=hard:_eabihf;:_eabi}}}" \
+ : "%{!mabi=apcs-gnu:%{!mabi=atpcs:%{mfloat-abi=soft:_eabi;:_eabihf}}}")
+#define TARGET_LINKER_BIG_EMULATION "armelfb_nbsd%(linker_eabi_suffix)"
+#define TARGET_LINKER_LITTLE_EMULATION "armelf_nbsd%(linker_eabi_suffix)"
+
+/* TARGET_BIG_ENDIAN_DEFAULT is set in
+   config.gcc for big endian configurations.  */
+#undef  

Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases

[Marek Polacek]

Any comments?

On Mon, Apr 01, 2019 at 11:23:29AM -0400, Marek Polacek wrote:
> On Mon, Apr 01, 2019 at 10:15:11AM +0200, Andreas Schwab wrote:
> > On Mär 28 2019, Marek Polacek  wrote:
> > 
> > > Andreas, could you please find out why we're not hitting this code in
> > > digest_init_r:
> > >
> > > 1210   tree elt = CONSTRUCTOR_ELT (stripped_init, 0)->value;
> > > 1211   if (reference_related_p (type, TREE_TYPE (elt)))
> > 
> > This is never executed if flag_checking is false, of course.
> 
> It's certainly wrong for a warning to depend on flag_checking, so this
> patch corrects it, and I hope will fix the ia64 problem as well.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-04-01  Marek Polacek  
> 
>   * typeck2.c (digest_init_r): Don't condition the object slicing warning
>   on flag_checking.
> 
> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
> index fa98b1cb8b5..55b84f043f4 100644
> --- gcc/cp/typeck2.c
> +++ gcc/cp/typeck2.c
> @@ -1200,8 +1200,7 @@ digest_init_r (tree type, tree init, int nested, int 
> flags,
>/* "If T is a class type and the initializer list has a single
>   element of type cv U, where U is T or a class derived from T,
>   the object is initialized from that element."  */
> -  if (flag_checking
> -  && cxx_dialect >= cxx11
> +  if (cxx_dialect >= cxx11
>&& BRACE_ENCLOSED_INITIALIZER_P (stripped_init)
>&& CONSTRUCTOR_NELTS (stripped_init) == 1
>&& ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
> @@ -1228,7 +1227,7 @@ digest_init_r (tree type, tree init, int nested, int 
> flags,
> "results in object slicing", TREE_TYPE (field)))
>   inform (loc, "remove %<{ }%> around initializer");
>   }
> -   else
> +   else if (flag_checking)
>   /* We should have fixed this in reshape_init.  */
>   gcc_unreachable ();
>   }

Marek

Re: [PATCH] claim ifunc support on several NetBSD architectures

[coypu]

Small addition for ARM. Since it doesn't have a geneirc way to detect
CPU features the code in libatomic relies on a linux-specific behaviour,
the ifunc condition is only defined for linux.

To unbreak compilation, I'd like to exclude netbsd/arm from the
libatomic ifunc camp :)

libatomic/ChangeLog:
* configure.tgt: Exclude arm*-*-netbsd* from try_ifunc.

diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index ecbb7d33cc1..4a1294bc1ff 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -53,7 +53,7 @@ case "${target_cpu}" in
   arm*)
ARCH=arm
case "${target}" in
-arm*-*-freebsd*)
+arm*-*-freebsd* | arm*-*-netbsd*)
 ;;
 *)
 # ??? Detect when -march=armv7 is already enabled.

Re: [PATCH v2] elf.c: initialize struct stat

[Jeff Law]

On 4/8/19 12:34 AM, mingli...@windriver.com wrote:
> From: Mingli Yu 
> 
> Initialize struct stat to fix the below
> build failure when -Og included in compiler flag.
> | 
> ./../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:
>  In function 'elf_is_symlink':
> | 
> ../../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21:
>  error: 'st.st_mode' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>return S_ISLNK (st.st_mode);
> 
> Signed-off-by: Mingli Yu 
> ---
>  libbacktrace/elf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
This is a false positive in -Wuninitialized that can be worked around by
compiling with -O2.  Please do that rather than compiling with -Og.

jeff

Re: [PATCH] Fix PR83033

[Andrea Corallo]

Richard Earnshaw (lists) writes:

> On 08/04/2019 09:59, Andrea Corallo wrote:
>>
>> Richard Earnshaw (lists) writes:
>>
>>> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
>>> the source code, so that it's clear these don't exist.  ChangeLog update
>>> is good too.
>>>
>>> R.
>>
>> Hi Richard,
>> sorry my misunderstanding.
>>
>
> NP.  I've put this in.  For future reference, I've made a few very minor
> tweaks before applying...

Thanks for that!

Bests
  Andrea

Re: [PATCH] Fix PR83033

[Richard Earnshaw (lists)]

On 08/04/2019 09:59, Andrea Corallo wrote:
> 
> Richard Earnshaw (lists) writes:
> 
>> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
>> the source code, so that it's clear these don't exist.  ChangeLog update
>> is good too.
>>
>> R.
> 
> Hi Richard,
> sorry my misunderstanding.
> 

NP.  I've put this in.  For future reference, I've made a few very minor
tweaks before applying...

> Bests
>   Andrea
> 
> 
> 2019-03-29  Andrea Corallo  andrea.cora...@arm.com

email address should be enclosed in angle brackets .

> 
> PR target/83033
> * config/aarch64/cortex-a57-fma-steering.c
> (fma_forest): Prohibit copy construction.
> (fma_root_node): Prohibit copy construction.

Generally speaking we use 'Likewise' when the same change is made to
multiple functions.

> (func_fma_steering): Prohibit copy construction.
> 
> 
> PR83033.patch
> 
> From 372ffe36fd0d59aa3c0ba1f8794c7aff9cee84a6 Mon Sep 17 00:00:00 2001
> From: Andrea Corallo 
> Date: Thu, 28 Mar 2019 23:17:17 +0100
> Subject: [PATCH] imporove cortex-a57-fma-steering.c
> 
> ---
>  gcc/config/aarch64/cortex-a57-fma-steering.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c 
> b/gcc/config/aarch64/cortex-a57-fma-steering.c
> index f2da03a..4792d1f 100644
> --- a/gcc/config/aarch64/cortex-a57-fma-steering.c
> +++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
> @@ -114,6 +114,9 @@ public:
>void dispatch ();
>  
>  private:
> +  /* Prohibit copy construction.  */
> +  fma_forest (const fma_forest &);
> +
>/* The list of roots that form this forest.  */
>std::list *m_roots;
>  
> @@ -147,6 +150,9 @@ public:
>void set_head (du_head *);
>void rename (fma_forest *);
>void dump_info (fma_forest *);
> +private:

Blank line before 'private:'.

> +  /* Prohibit copy construction.  */
> +  fma_node (const fma_node &);
>  
>  protected:
>/* Root node that lead to this node.  */
> @@ -203,6 +209,9 @@ public:
>void execute_fma_steering ();
>  
>  private:
> +  /* Prohibit copy construction.  */
> +  func_fma_steering (const func_fma_steering &);
> +
>void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node 
> *),
>   void (*) (fma_forest *, fma_node *), bool);
>void analyze ();
> 

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Richard Biener]

On Mon, Apr 8, 2019 at 3:53 PM Martin Liška  wrote:
>
> On 4/8/19 3:50 PM, Martin Liška wrote:
> > On 4/8/19 2:42 PM, Richard Biener wrote:
> >> On Mon, Apr 8, 2019 at 2:26 PM Martin Liška  wrote:
> >>>
> >>> On 4/8/19 2:18 PM, Richard Biener wrote:
>  On Mon, Apr 8, 2019 at 1:30 PM Martin Liška  wrote:
> >
> > On 4/8/19 12:08 PM, Richard Biener wrote:
> >> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
> >>>
> >>> Hi.
> >>>
> >>> The patch adds a new config that makes LTO+PGO bootstrap faster by
> >>> using LTO only in stage4. In stage3, generators are build with LTO
> >>> in order to collect a reasonable profile for LTO FE.
> >>>
> >>> Ready for trunk?
> >>
> >> I wonder if you need the
> >>
> >> +AC_SUBST(GENERATOR_CFLAGS)
> >>
> >> at all, can't you just use
> >>
> >> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >>
> >> ?
> >
> > I've just tested that and it does not work for me.
> 
>  Ah, you probably need to move the
> 
>  @@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
>  CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
>  LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
>  prefix +] \
>  CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
>  +   GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
>  GENERATOR_CFLAGS; \
>  CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev 
>  +] \
> 
>  change to the respective build targets.
> >>>
> >>> Can you please point me to a location in Makefile.tpl where
> >>> is the target? It's all Greek to me :)
> >>
> >> I think it's
> >>
> >> all-stage[+id+]-[+prefix+][+module+]: 
> >> configure-stage[+id+]-[+prefix+][+module+]
> >> @[ $(current_stage) = stage[+id+] ] || $(MAKE) stage[+id+]-start
> >> @r=`${PWD_COMMAND}`; export r; \
> >> s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> >> TFLAGS="$(STAGE[+id+]_TFLAGS)"; \
> >> [+exports+][+ IF prev +] \
> >> [+poststage1_exports+][+ ENDIF prev +] [+extra_exports+] \
> >> cd [+subdir+]/[+module+] && \
> >> [+autoprofile+] \
> >> $(MAKE) $(BASE_FLAGS_TO_PASS)[+ IF prefix +] \
> >> CFLAGS="$(CFLAGS_FOR_TARGET)" \
> >> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)" \
> >> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"[+ ELSE prefix +] \
> >> CFLAGS="$(STAGE[+id+]_CFLAGS)" \
> >> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"[+ IF prev +] \
> >> LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
> >> LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +][+ ENDIF prefix +] 
> >> \
> >> CFLAGS_FOR_TARGET="$(CFLAGS_FOR_TARGET)" \
> >> CXXFLAGS_FOR_TARGET="$(CXXFLAGS_FOR_TARGET)" \
> >> LIBCFLAGS_FOR_TARGET="$(LIBCFLAGS_FOR_TARGET)" \
> >> [+args+] [+IF prev +][+poststage1_args+][+ ELSE prev +] \
> >> [+stage1_args+][+ ENDIF prev +] [+extra_make_flags+] \
> >> TFLAGS="$(STAGE[+id+]_TFLAGS)" [+profile_data+] \
> >> $(TARGET-stage[+id+]-[+prefix+][+module+])
> >>
> >> where you maybe can simply add a
> >> GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)
> >>
> >> Richard.
> >>
> >>> Martin
> >>>
> 
> >>
> >> Please mention in both bootstrap-lto-lean.mk and the documentation
> >> that the intended make target for this config is profiledbootstrap
> >> since for non-profiledbootstrap it ends up not using LTO at all.  A 
> >> "lean"
> >> mode for non-profiledbootstrap would need to set up things to
> >> use LTO only for stage3 which means not doing a bootstrap comparison
> >> which means we could "skip" stage2 as well here.  So partial support
> >> for that would be to have
> >>
> >> STAGE2_CFLAGS += -frandom-seed=1
> >> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> >> ...
> >> do-compare = true
> >
> > Changed to tihs.
> >
> >>
> >> So if this works for non-profiledbootstrap the docs could be
> >> changed to say "but is intended for faster build by only
> >> using LTO in the final bootstrap stage.  With @samp{...}
> >> the LTO frontend is trained only on generator files."
> >
> > Likewise.
> >
> >>
> >> (why do we need -frandom-seed=1?  IIRC that was only for the
> >> comparison step which is elided in all cases for -lean.mk).
> >
> > Yep, it's not neded now.
> 
>  I see it still on STAGE[23]_CFLAGS?  I think you can drop
>  STAGE2_CFLAGS adjustment completely.
> 
>  Otherwise looks OK - mind trying one more time with the
>  above suggestion for GENERATOR_CFLAGS?
> 
>  Thanks,
>  

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Martin Liška]

On 4/8/19 2:42 PM, Richard Biener wrote:
> On Mon, Apr 8, 2019 at 2:26 PM Martin Liška  wrote:
>>
>> On 4/8/19 2:18 PM, Richard Biener wrote:
>>> On Mon, Apr 8, 2019 at 1:30 PM Martin Liška  wrote:

 On 4/8/19 12:08 PM, Richard Biener wrote:
> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
>>
>> Hi.
>>
>> The patch adds a new config that makes LTO+PGO bootstrap faster by
>> using LTO only in stage4. In stage3, generators are build with LTO
>> in order to collect a reasonable profile for LTO FE.
>>
>> Ready for trunk?
>
> I wonder if you need the
>
> +AC_SUBST(GENERATOR_CFLAGS)
>
> at all, can't you just use
>
> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
>
> ?

 I've just tested that and it does not work for me.
>>>
>>> Ah, you probably need to move the
>>>
>>> @@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
>>> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
>>> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
>>> prefix +] \
>>> CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
>>> +   GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
>>> GENERATOR_CFLAGS; \
>>> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev +] \
>>>
>>> change to the respective build targets.
>>
>> Can you please point me to a location in Makefile.tpl where
>> is the target? It's all Greek to me :)
> 
> I think it's
> 
> all-stage[+id+]-[+prefix+][+module+]: 
> configure-stage[+id+]-[+prefix+][+module+]
> @[ $(current_stage) = stage[+id+] ] || $(MAKE) stage[+id+]-start
> @r=`${PWD_COMMAND}`; export r; \
> s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
> TFLAGS="$(STAGE[+id+]_TFLAGS)"; \
> [+exports+][+ IF prev +] \
> [+poststage1_exports+][+ ENDIF prev +] [+extra_exports+] \
> cd [+subdir+]/[+module+] && \
> [+autoprofile+] \
> $(MAKE) $(BASE_FLAGS_TO_PASS)[+ IF prefix +] \
> CFLAGS="$(CFLAGS_FOR_TARGET)" \
> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)" \
> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"[+ ELSE prefix +] \
> CFLAGS="$(STAGE[+id+]_CFLAGS)" \
> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"[+ IF prev +] \
> LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
> LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +][+ ENDIF prefix +] \
> CFLAGS_FOR_TARGET="$(CFLAGS_FOR_TARGET)" \
> CXXFLAGS_FOR_TARGET="$(CXXFLAGS_FOR_TARGET)" \
> LIBCFLAGS_FOR_TARGET="$(LIBCFLAGS_FOR_TARGET)" \
> [+args+] [+IF prev +][+poststage1_args+][+ ELSE prev +] \
> [+stage1_args+][+ ENDIF prev +] [+extra_make_flags+] \
> TFLAGS="$(STAGE[+id+]_TFLAGS)" [+profile_data+] \
> $(TARGET-stage[+id+]-[+prefix+][+module+])
> 
> where you maybe can simply add a
> GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)
> 
> Richard.
> 
>> Martin
>>
>>>
>
> Please mention in both bootstrap-lto-lean.mk and the documentation
> that the intended make target for this config is profiledbootstrap
> since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
> mode for non-profiledbootstrap would need to set up things to
> use LTO only for stage3 which means not doing a bootstrap comparison
> which means we could "skip" stage2 as well here.  So partial support
> for that would be to have
>
> STAGE2_CFLAGS += -frandom-seed=1
> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> ...
> do-compare = true

 Changed to tihs.

>
> So if this works for non-profiledbootstrap the docs could be
> changed to say "but is intended for faster build by only
> using LTO in the final bootstrap stage.  With @samp{...}
> the LTO frontend is trained only on generator files."

 Likewise.

>
> (why do we need -frandom-seed=1?  IIRC that was only for the
> comparison step which is elided in all cases for -lean.mk).

 Yep, it's not neded now.
>>>
>>> I see it still on STAGE[23]_CFLAGS?  I think you can drop
>>> STAGE2_CFLAGS adjustment completely.
>>>
>>> Otherwise looks OK - mind trying one more time with the
>>> above suggestion for GENERATOR_CFLAGS?
>>>
>>> Thanks,
>>> Richard.
>>>
 Martin

>
> Richard.
>
>> Thanks,
>> Martin
>>
>> ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * Makefile.in: Regenerate.
>> * Makefile.tpl: Pass GENERATOR_CFLAGS
>> in all stages.
>>
>> config/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * bootstrap-lto-lean.mk: New file.
>>
>> gcc/ChangeLog:
>>
>> 2019-04-05  Martin 

Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE.

[Martin Liška]

On 4/8/19 11:11 AM, Richard Biener wrote:
> On Fri, Apr 5, 2019 at 2:32 PM Martin Liška  wrote:
>>
>> Hi.
>>
>> The patch adds support for profile for GIMPLE FE. That can be useful
>> in the future.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed after stage1 opens?
> 
> Hmm, I guess to be useful we need the profile quality indicators as well
> (and think about a proper syntax here since it seems related).

I wanted to make it easy, buy yes. We should implement that.

> 
> Btw, do BB counts and edge probabilities not have a relation?

Yes, they should match. But you have optimizations that make some discrepancies.
So that I would implement support for both of them.

>  If so why
> do we need both?  In predict.c we also look at ENTRY_BLOCK->count
> so we need a place to set that as well.  We should probably set
> edge probabilities of fallthru edges properly during our "CFG build".
> 
> +goto __BB3(44739243);
> 
> since we eventually want to add other flags this syntactically
> should maybe be goto __BB3(guessed(44739243)); and similar
> for the __BB count case (just s/count/quality/).

The syntax works for me.

> 
> The entry block count could be sticked to __GIMPLE (ssa,guessed(N))
> for example.  There's also the exit block, not sure if we ever look at
> its count, so __GIMPLE (ssa,guessed(N[,M])) might be a possibility
> if we always have the same quality here (probably not...).

I'll check it.

> 
> Otherwise thanks for trying ;)
> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * gimple-pretty-print.c (dump_gimple_bb_header):
>> Dump BB count.
>> (pp_cfg_jump): Dump edge probability.
>> * profile-count.h (get_raw_value): New function.
>> (from_raw_value): Likewise.
>>
>> gcc/c/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * gimple-parser.c (struct gimple_parser): Add frequency
>> for gimple_parser_edge.
>> (gimple_parser::push_edge): Add new argument frequency.
>> (c_parser_gimple_parse_bb_spec): Parse also frequency
>> if present.
>> (c_parser_parse_gimple_body): Set edge probability.
>> (c_parser_gimple_compound_statement): Consume token
>> before calling c_parser_gimple_goto_stmt.
>> Parse BB counts.
>> (c_parser_gimple_statement): Pass new argument.
>> (c_parser_gimple_goto_stmt): Likewise.
>> (c_parser_gimple_if_stmt): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * gcc.dg/gimplefe-37.c: New test.
>> ---
>>  gcc/c/gimple-parser.c  | 116 +++--
>>  gcc/gimple-pretty-print.c  |   8 ++
>>  gcc/profile-count.h|  15 
>>  gcc/testsuite/gcc.dg/gimplefe-37.c |  27 +++
>>  4 files changed, 144 insertions(+), 22 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c
>>
>>

[PATCH] Fix PR90006

[Richard Biener]

The following fixes SLP vectorization to properly consider
calls like lrint demoting on ilp32 targets.

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

Richard.

2019-04-08  Richard Biener  

PR tree-optimization/90006
* tree-vect-data-refs.c (vect_get_smallest_scalar_type): Handle
calls like lrint.

* gcc.dg/vect/bb-slp-pr90006.c: New testcase.

Index: gcc/tree-vect-data-refs.c
===
--- gcc/tree-vect-data-refs.c   (revision 270202)
+++ gcc/tree-vect-data-refs.c   (working copy)
@@ -144,6 +144,15 @@ vect_get_smallest_scalar_type (gimple *s
   if (rhs < lhs)
 scalar_type = rhs_type;
 }
+  else if (is_gimple_call (stmt)
+  && gimple_call_num_args (stmt) > 0)
+{
+  tree rhs_type = TREE_TYPE (gimple_call_arg (stmt, 0));
+
+  rhs = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (rhs_type));
+  if (rhs < lhs)
+scalar_type = rhs_type;
+}
 
   *lhs_size_unit = lhs;
   *rhs_size_unit = rhs;
Index: gcc/testsuite/gcc.dg/vect/bb-slp-pr90006.c
===
--- gcc/testsuite/gcc.dg/vect/bb-slp-pr90006.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/vect/bb-slp-pr90006.c  (working copy)
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-math-errno" } */
+/* { dg-additional-options "-march=x86-64" { target x86_64-*-* i?86-*-* } } */
+
+long int lrint(double x);
+
+int a, b;
+union c {
+int d;
+};
+
+int e()
+{
+  int f, g, h;
+  long i, j, k;
+  double l, m = b = lrint(0.3127);
+  a = b >> 16 >> 8 & 255;
+  ((union c *)e)->d = a;
+  k = m;
+  h = k >> 16 >> 8 & 255;
+  ((union c *)(e + 4))->d = h;
+  j = lrint(l);
+  g = j >> 16 >> 8 & 255;
+  ((union c *)(e + 8))->d = g;
+  i = lrint(0.292);
+  f = i >> 16 >> 8 & 255;
+  ((union c *)(e + 12))->d = f;
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target { { 
x86_64-*-* i?86-*-* } && ilp32 } } } } */

Re: [Patch] [arm] Ensure *neon_mov constraint uses correct mode.

[Kyrill Tkachov]

Hi Matthew,

On 4/5/19 12:06 PM, Matthew Malcomson wrote:

Hi there,

The "*neon_mov" patterns for 128 bit sized quantities uses the "Dn"
constraint to match vmov.f32 and vmov.i patterns.
This constraint boils down to using the `neon_immediate_valid` function.
Once the constraint has matched, the output C statement asserts that 
function

passes.

The output C statement calls `neon_immediate_valid` with the mode 
taken from the

iterator, while the constraint takes the mode from the operand.
This can cause a discrepency when the operand is a CONST_INT, as the 
constraint

passes VOIDmode which `neon_immediate_valid` treats as DImode, while the C
statement passes the mode of the iterator which can be TImode.
When this happens, the `neon_immediate_valid` can fail in the second 
call (if
e.g. the CONST_INT is a valid immediate in DImode but not TImode) 
which would

trigger the assertion.

The testcase added with this patch triggers this when compiled with an 
arm cross

compiler using the command line below.
gcc -march=armv8-a -c neon-immediate-timode.c -O1 -mfloat-abi=hard 
-mfpu=neon-fp-armv8


This patch splits the original "Dn" constraint into three new 
constraints, "DN"
for TImode CONST_INT, "Dn" for DImode CONST_INT, and "Dm" for 
CONST_VECTOR.

Splitting things up this way requires using one extra alternative in the
"*neon_mov" patterns, but makes it clear from the constraint 
what mode is

being used.

We also remove the behaviour of treating VOIDmode as DImode in
`neon_valid_immediate` since the original "Dn" constraint was the only 
place
that functionality was used.  VOIDmode is now never passed to that 
function.
An assertion has been added to the function to ensure this problem is 
caught

earlier on.

Bootstrapped and regtested on arm-none-linux-gnueabihf
Regtested on cross-compiler arm-none-eabi



Does this fix a PR?

If so, could you add the information for it to the ChangeLog?

Does this need backporting to the branches as well?

The patch looks good to me otherwise.

Thanks,

Kyrill



gcc/ChangeLog:

2019-04-05  Matthew Malcomson 

    * config/arm/arm.c (neon_valid_immediate): Disallow VOIDmode 
parameter.
    * config/arm/constraints.md (Dm, DN, Dn): Split previous Dn 
constraint

    into three.
    * config/arm/neon.md (*neon_mov): Account for TImode and 
DImode

    differences directly.
    (*smax3_neon, vashl3, vashr3_imm): Use Dm 
constraint.


gcc/testsuite/ChangeLog:

2019-04-05  Matthew Malcomson 

    * gcc.dg/torture/neon-immediate-timode.c: New test.



### Attachment also inlined for ease of reply    
###



diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
69b74a237a5f10b4137aa995ad43b77d6ecd04db..65eb4c6e00b1820dedcdf3977651aa423643cc9b 
100644

--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -11994,8 +11994,7 @@ neon_valid_immediate (rtx op, machine_mode 
mode, int inverse,

   else
 {
   n_elts = 1;
-  if (mode == VOIDmode)
-   mode = DImode;
+  gcc_assert (mode != VOIDmode);
 }

   innersize = GET_MODE_UNIT_SIZE (mode);
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index 
57ec06396220b9f7f356943dae1443c4c763741f..d4a4c5967aea09816485d77f9ab90020aa085e73 
100644

--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -31,8 +31,8 @@
 ;; 'H' was previously used for FPA.

 ;; The following multi-letter normal constraints have been used:
-;; in ARM/Thumb-2 state: Da, Db, Dc, Dd, Dn, Dl, DL, Do, Dv, Dy, Di, 
Dt, Dp,

-;;  Dz, Tu
+;; in ARM/Thumb-2 state: Da, Db, Dc, Dd, Dn, DN, Dm, Dl, DL, Do, Dv, 
Dy, Di,

+;;  Dt, Dp, Dz, Tu
 ;; in Thumb-1 state: Pa, Pb, Pc, Pd, Pe
 ;; in Thumb-2 state: Ha, Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py, Pz
 ;; in all states: Pf
@@ -298,14 +298,28 @@ (define_constraint "Di"
  (and (match_code "const_double,const_int")
   (match_test "TARGET_32BIT && arm_const_double_by_immediates 
(op)")))


-(define_constraint "Dn"
+(define_constraint "Dm"
  "@internal
-  In ARM/Thumb-2 state a const_vector or const_int which can be 
loaded with a

-  Neon vmov immediate instruction."
- (and (match_code "const_vector,const_int")
+  In ARM/Thumb-2 state a const_vector which can be loaded with a Neon 
vmov

+  immediate instruction."
+ (and (match_code "const_vector")
   (match_test "TARGET_32BIT
    && imm_for_neon_mov_operand (op, GET_MODE (op))")))

+(define_constraint "Dn"
+ "@internal
+  In ARM/Thumb-2 state a DImode const_int which can be loaded with a 
Neon vmov

+  immediate instruction."
+ (and (match_code "const_int")
+  (match_test "TARGET_32BIT && imm_for_neon_mov_operand (op, 
DImode)")))

+
+(define_constraint "DN"
+ "@internal
+  In ARM/Thumb-2 state a TImode const_int which can be loaded with a 
Neon vmov

+  immediate instruction."
+ (and (match_code "const_int")
+  (match_test "TARGET_32BIT && imm_for_neon_mov_operand (op, 

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Richard Biener]

On Mon, Apr 8, 2019 at 2:26 PM Martin Liška  wrote:
>
> On 4/8/19 2:18 PM, Richard Biener wrote:
> > On Mon, Apr 8, 2019 at 1:30 PM Martin Liška  wrote:
> >>
> >> On 4/8/19 12:08 PM, Richard Biener wrote:
> >>> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
> 
>  Hi.
> 
>  The patch adds a new config that makes LTO+PGO bootstrap faster by
>  using LTO only in stage4. In stage3, generators are build with LTO
>  in order to collect a reasonable profile for LTO FE.
> 
>  Ready for trunk?
> >>>
> >>> I wonder if you need the
> >>>
> >>> +AC_SUBST(GENERATOR_CFLAGS)
> >>>
> >>> at all, can't you just use
> >>>
> >>> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >>> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >>>
> >>> ?
> >>
> >> I've just tested that and it does not work for me.
> >
> > Ah, you probably need to move the
> >
> > @@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
> > CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
> > LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
> > prefix +] \
> > CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
> > +   GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
> > GENERATOR_CFLAGS; \
> > CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev +] \
> >
> > change to the respective build targets.
>
> Can you please point me to a location in Makefile.tpl where
> is the target? It's all Greek to me :)

I think it's

all-stage[+id+]-[+prefix+][+module+]: configure-stage[+id+]-[+prefix+][+module+]
@[ $(current_stage) = stage[+id+] ] || $(MAKE) stage[+id+]-start
@r=`${PWD_COMMAND}`; export r; \
s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
TFLAGS="$(STAGE[+id+]_TFLAGS)"; \
[+exports+][+ IF prev +] \
[+poststage1_exports+][+ ENDIF prev +] [+extra_exports+] \
cd [+subdir+]/[+module+] && \
[+autoprofile+] \
$(MAKE) $(BASE_FLAGS_TO_PASS)[+ IF prefix +] \
CFLAGS="$(CFLAGS_FOR_TARGET)" \
CXXFLAGS="$(CXXFLAGS_FOR_TARGET)" \
LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"[+ ELSE prefix +] \
CFLAGS="$(STAGE[+id+]_CFLAGS)" \
CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"[+ IF prev +] \
LIBCFLAGS="$(STAGE[+id+]_CFLAGS)"[+ ELSE prev +] \
LIBCFLAGS="$(LIBCFLAGS)"[+ ENDIF prev +][+ ENDIF prefix +] \
CFLAGS_FOR_TARGET="$(CFLAGS_FOR_TARGET)" \
CXXFLAGS_FOR_TARGET="$(CXXFLAGS_FOR_TARGET)" \
LIBCFLAGS_FOR_TARGET="$(LIBCFLAGS_FOR_TARGET)" \
[+args+] [+IF prev +][+poststage1_args+][+ ELSE prev +] \
[+stage1_args+][+ ENDIF prev +] [+extra_make_flags+] \
TFLAGS="$(STAGE[+id+]_TFLAGS)" [+profile_data+] \
$(TARGET-stage[+id+]-[+prefix+][+module+])

where you maybe can simply add a
GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)

Richard.

> Martin
>
> >
> >>>
> >>> Please mention in both bootstrap-lto-lean.mk and the documentation
> >>> that the intended make target for this config is profiledbootstrap
> >>> since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
> >>> mode for non-profiledbootstrap would need to set up things to
> >>> use LTO only for stage3 which means not doing a bootstrap comparison
> >>> which means we could "skip" stage2 as well here.  So partial support
> >>> for that would be to have
> >>>
> >>> STAGE2_CFLAGS += -frandom-seed=1
> >>> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> >>> ...
> >>> do-compare = true
> >>
> >> Changed to tihs.
> >>
> >>>
> >>> So if this works for non-profiledbootstrap the docs could be
> >>> changed to say "but is intended for faster build by only
> >>> using LTO in the final bootstrap stage.  With @samp{...}
> >>> the LTO frontend is trained only on generator files."
> >>
> >> Likewise.
> >>
> >>>
> >>> (why do we need -frandom-seed=1?  IIRC that was only for the
> >>> comparison step which is elided in all cases for -lean.mk).
> >>
> >> Yep, it's not neded now.
> >
> > I see it still on STAGE[23]_CFLAGS?  I think you can drop
> > STAGE2_CFLAGS adjustment completely.
> >
> > Otherwise looks OK - mind trying one more time with the
> > above suggestion for GENERATOR_CFLAGS?
> >
> > Thanks,
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
>  Thanks,
>  Martin
> 
>  ChangeLog:
> 
>  2019-04-05  Martin Liska  
> 
>  * Makefile.in: Regenerate.
>  * Makefile.tpl: Pass GENERATOR_CFLAGS
>  in all stages.
> 
>  config/ChangeLog:
> 
>  2019-04-05  Martin Liska  
> 
>  * bootstrap-lto-lean.mk: New file.
> 
>  gcc/ChangeLog:
> 
>  2019-04-05  Martin Liska  
> 
>  * Makefile.in: Use GENERATOR_CFLAGS for all generators.
>  * configure: 

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Martin Liška]

On 4/8/19 2:18 PM, Richard Biener wrote:
> On Mon, Apr 8, 2019 at 1:30 PM Martin Liška  wrote:
>>
>> On 4/8/19 12:08 PM, Richard Biener wrote:
>>> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:

 Hi.

 The patch adds a new config that makes LTO+PGO bootstrap faster by
 using LTO only in stage4. In stage3, generators are build with LTO
 in order to collect a reasonable profile for LTO FE.

 Ready for trunk?
>>>
>>> I wonder if you need the
>>>
>>> +AC_SUBST(GENERATOR_CFLAGS)
>>>
>>> at all, can't you just use
>>>
>>> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
>>> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
>>>
>>> ?
>>
>> I've just tested that and it does not work for me.
> 
> Ah, you probably need to move the
> 
> @@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
> CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
> LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
> prefix +] \
> CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
> +   GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
> GENERATOR_CFLAGS; \
> CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev +] \
> 
> change to the respective build targets.

Can you please point me to a location in Makefile.tpl where
is the target? It's all Greek to me :)

Martin

> 
>>>
>>> Please mention in both bootstrap-lto-lean.mk and the documentation
>>> that the intended make target for this config is profiledbootstrap
>>> since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
>>> mode for non-profiledbootstrap would need to set up things to
>>> use LTO only for stage3 which means not doing a bootstrap comparison
>>> which means we could "skip" stage2 as well here.  So partial support
>>> for that would be to have
>>>
>>> STAGE2_CFLAGS += -frandom-seed=1
>>> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
>>> ...
>>> do-compare = true
>>
>> Changed to tihs.
>>
>>>
>>> So if this works for non-profiledbootstrap the docs could be
>>> changed to say "but is intended for faster build by only
>>> using LTO in the final bootstrap stage.  With @samp{...}
>>> the LTO frontend is trained only on generator files."
>>
>> Likewise.
>>
>>>
>>> (why do we need -frandom-seed=1?  IIRC that was only for the
>>> comparison step which is elided in all cases for -lean.mk).
>>
>> Yep, it's not neded now.
> 
> I see it still on STAGE[23]_CFLAGS?  I think you can drop
> STAGE2_CFLAGS adjustment completely.
> 
> Otherwise looks OK - mind trying one more time with the
> above suggestion for GENERATOR_CFLAGS?
> 
> Thanks,
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
 Thanks,
 Martin

 ChangeLog:

 2019-04-05  Martin Liska  

 * Makefile.in: Regenerate.
 * Makefile.tpl: Pass GENERATOR_CFLAGS
 in all stages.

 config/ChangeLog:

 2019-04-05  Martin Liska  

 * bootstrap-lto-lean.mk: New file.

 gcc/ChangeLog:

 2019-04-05  Martin Liska  

 * Makefile.in: Use GENERATOR_CFLAGS for all generators.
 * configure: Regenerate.
 * configure.ac: Pass GENERATOR_CFLAGS.
 * doc/install.texi: Document the new config.
 ---
  Makefile.in  | 207 +++
  Makefile.tpl |   2 +
  config/bootstrap-lto-lean.mk |  19 
  gcc/Makefile.in  |   4 +-
  gcc/configure|   6 +-
  gcc/configure.ac |   1 +
  gcc/doc/install.texi |   6 +
  7 files changed, 241 insertions(+), 4 deletions(-)
  create mode 100644 config/bootstrap-lto-lean.mk


>>

Re: [PATCH] Add missing libsanitizer extra patch (r259664) (PR sanitizer/89941).

[Jakub Jelinek]

On Wed, Apr 03, 2019 at 10:09:19AM +0200, Martin Liška wrote:
> Hi.
> 
> The patch is about re-application of what we've already had
> on top trunk libsanitizer. I'll then include the patch in 
> libsanitizer/LOCAL_PATCHES.
> 
> Ready for trunk?
> Thanks,
> Martin
> 
> libsanitizer/ChangeLog:
> 
> 2019-04-03  Martin Liska  
> 
>   PR sanitizer/89941
>   * sanitizer_common/sanitizer_platform_limits_linux.cc (defined):
>   Reapply patch from r259664.
>   * sanitizer_common/sanitizer_platform_limits_posix.h (defined):
>   Likewise.

Ok.

Jakub

Re: [PATCH] Add peephole2s to improve pr49095.c f{char,short,int,long}minus on ia32 (PR rtl-optimization/89865)

[Uros Bizjak]

On Mon, Apr 8, 2019 at 1:32 PM Jakub Jelinek  wrote:
>
> On Sat, Mar 30, 2019 at 11:21:33AM +0100, Uros Bizjak wrote:
> > > OK.  One might ask if there's a way to share a bit of code here since
> > > there's a fair amount of duplication.  But I'll trust that you've
> > > pondered that and decided it wasn't really worth the effort.
> >
> > I think that Vladimir n is looking into the PR. So, if RA can avoid
> > register copies by itself, then these extra peepholes won't be needed.
> > Let's ask Vladimir for his opinion.
>
> While Vlad's patch improved the code generation on the testcase, it was
> orthogonal to the two peephole2 patches:
> https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01441.html
> https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01442.html
> as can be seen even from that Vlad didn't have to adjust the counts in the
> pr49095.c testcase, while both of those patches do that (both together down
> to zero RMW sequences).
>
> Are you ok with those patches, or do you have other suggestions?

Yes, they are OK.

Thanks,
Uros.

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Richard Biener]

On Mon, Apr 8, 2019 at 1:30 PM Martin Liška  wrote:
>
> On 4/8/19 12:08 PM, Richard Biener wrote:
> > On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
> >>
> >> Hi.
> >>
> >> The patch adds a new config that makes LTO+PGO bootstrap faster by
> >> using LTO only in stage4. In stage3, generators are build with LTO
> >> in order to collect a reasonable profile for LTO FE.
> >>
> >> Ready for trunk?
> >
> > I wonder if you need the
> >
> > +AC_SUBST(GENERATOR_CFLAGS)
> >
> > at all, can't you just use
> >
> > +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> > +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> >
> > ?
>
> I've just tested that and it does not work for me.

Ah, you probably need to move the

@@ -1124,6 +1125,7 @@ configure-stage[+id+]-[+prefix+][+module+]:
CXXFLAGS="$(CXXFLAGS_FOR_TARGET)"; export CXXFLAGS; \
LIBCFLAGS="$(LIBCFLAGS_FOR_TARGET)"; export LIBCFLAGS;[+ ELSE
prefix +] \
CFLAGS="$(STAGE[+id+]_CFLAGS)"; export CFLAGS; \
+   GENERATOR_CFLAGS="$(STAGE[+id+]_GENERATOR_CFLAGS)"; export
GENERATOR_CFLAGS; \
CXXFLAGS="$(STAGE[+id+]_CXXFLAGS)"; export CXXFLAGS;[+ IF prev +] \

change to the respective build targets.

> >
> > Please mention in both bootstrap-lto-lean.mk and the documentation
> > that the intended make target for this config is profiledbootstrap
> > since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
> > mode for non-profiledbootstrap would need to set up things to
> > use LTO only for stage3 which means not doing a bootstrap comparison
> > which means we could "skip" stage2 as well here.  So partial support
> > for that would be to have
> >
> > STAGE2_CFLAGS += -frandom-seed=1
> > STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> > ...
> > do-compare = true
>
> Changed to tihs.
>
> >
> > So if this works for non-profiledbootstrap the docs could be
> > changed to say "but is intended for faster build by only
> > using LTO in the final bootstrap stage.  With @samp{...}
> > the LTO frontend is trained only on generator files."
>
> Likewise.
>
> >
> > (why do we need -frandom-seed=1?  IIRC that was only for the
> > comparison step which is elided in all cases for -lean.mk).
>
> Yep, it's not neded now.

I see it still on STAGE[23]_CFLAGS?  I think you can drop
STAGE2_CFLAGS adjustment completely.

Otherwise looks OK - mind trying one more time with the
above suggestion for GENERATOR_CFLAGS?

Thanks,
Richard.

> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >> ChangeLog:
> >>
> >> 2019-04-05  Martin Liska  
> >>
> >> * Makefile.in: Regenerate.
> >> * Makefile.tpl: Pass GENERATOR_CFLAGS
> >> in all stages.
> >>
> >> config/ChangeLog:
> >>
> >> 2019-04-05  Martin Liska  
> >>
> >> * bootstrap-lto-lean.mk: New file.
> >>
> >> gcc/ChangeLog:
> >>
> >> 2019-04-05  Martin Liska  
> >>
> >> * Makefile.in: Use GENERATOR_CFLAGS for all generators.
> >> * configure: Regenerate.
> >> * configure.ac: Pass GENERATOR_CFLAGS.
> >> * doc/install.texi: Document the new config.
> >> ---
> >>  Makefile.in  | 207 +++
> >>  Makefile.tpl |   2 +
> >>  config/bootstrap-lto-lean.mk |  19 
> >>  gcc/Makefile.in  |   4 +-
> >>  gcc/configure|   6 +-
> >>  gcc/configure.ac |   1 +
> >>  gcc/doc/install.texi |   6 +
> >>  7 files changed, 241 insertions(+), 4 deletions(-)
> >>  create mode 100644 config/bootstrap-lto-lean.mk
> >>
> >>
>

[RFC, doc] Note variable shadowing at max macro using statement expression

[Tom de Vries]

Hi,

When suggesting to rewrite the unsafe (with respect to multiple evaluation of
arguments) macro definition:
...
  #define max(a,b) ((a) > (b) ? (a) : (b))
...
into the safe macro definition:
...
  #define maxint(a,b) \
({int _a = (a), _b = (b); _a > _b ? _a : _b; })
...
mention the variable shadowing problem for:
...
  #define maxint3(a, b, c) \
({int _a = (a), _b = (b), _c = (c); maxint (maxint (_a, _b), _c); })
...

Any comments?

Thanks,
- Tom

[doc] Note variable shadowing at max macro using statement expression

2019-04-08  Tom de Vries  

* doc/extend.texi (@node Statement Exprs): Note variable shadowing at
max macro using statement expression.

---
 gcc/doc/extend.texi | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8e0deac26c3..27ed0fb014f 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -143,13 +143,34 @@ follows:
 But this definition computes either @var{a} or @var{b} twice, with bad
 results if the operand has side effects.  In GNU C, if you know the
 type of the operands (here taken as @code{int}), you can define
-the macro safely as follows:
+the macro safe (from evaluating operands more than once) as follows:
 
 @smallexample
 #define maxint(a,b) \
   (@{int _a = (a), _b = (b); _a > _b ? _a : _b; @})
 @end smallexample
 
+Note that introducing variable declarations (as we do in maxint) can
+cause variable shadowing, so while this example using the max macro will
+produce correct results:
+@smallexample
+int _a = 1, _b = 2, c;
+c = max (_a, _b);
+@end smallexample
+this example using maxint will not:
+@smallexample
+int _a = 1, _b = 2, c;
+c = maxint (_a, _b);
+@end smallexample
+
+This problem may for instance occur when we use this pattern recursively, like
+so:
+
+@smallexample
+#define maxint3(a, b, c) \
+  (@{int _a = (a), _b = (b), _c = (c); maxint (maxint (_a, _b), _c); @})
+@end smallexample
+
 Embedded statements are not allowed in constant expressions, such as
 the value of an enumeration constant, the width of a bit-field, or
 the initial value of a static variable.

Re: [PATCH] Add peephole2s to improve pr49095.c f{char,short,int,long}minus on ia32 (PR rtl-optimization/89865)

[Jakub Jelinek]

On Sat, Mar 30, 2019 at 11:21:33AM +0100, Uros Bizjak wrote:
> > OK.  One might ask if there's a way to share a bit of code here since
> > there's a fair amount of duplication.  But I'll trust that you've
> > pondered that and decided it wasn't really worth the effort.
> 
> I think that Vladimir n is looking into the PR. So, if RA can avoid
> register copies by itself, then these extra peepholes won't be needed.
> Let's ask Vladimir for his opinion.

While Vlad's patch improved the code generation on the testcase, it was
orthogonal to the two peephole2 patches:
https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01441.html
https://gcc.gnu.org/ml/gcc-patches/2019-03/msg01442.html
as can be seen even from that Vlad didn't have to adjust the counts in the
pr49095.c testcase, while both of those patches do that (both together down
to zero RMW sequences).

Are you ok with those patches, or do you have other suggestions?

Jakub

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Martin Liška]

On 4/8/19 12:08 PM, Richard Biener wrote:
> On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
>>
>> Hi.
>>
>> The patch adds a new config that makes LTO+PGO bootstrap faster by
>> using LTO only in stage4. In stage3, generators are build with LTO
>> in order to collect a reasonable profile for LTO FE.
>>
>> Ready for trunk?
> 
> I wonder if you need the
> 
> +AC_SUBST(GENERATOR_CFLAGS)
> 
> at all, can't you just use
> 
> +BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> +BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> 
> ?

I've just tested that and it does not work for me.

> 
> Please mention in both bootstrap-lto-lean.mk and the documentation
> that the intended make target for this config is profiledbootstrap
> since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
> mode for non-profiledbootstrap would need to set up things to
> use LTO only for stage3 which means not doing a bootstrap comparison
> which means we could "skip" stage2 as well here.  So partial support
> for that would be to have
> 
> STAGE2_CFLAGS += -frandom-seed=1
> STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
> ...
> do-compare = true

Changed to tihs.

> 
> So if this works for non-profiledbootstrap the docs could be
> changed to say "but is intended for faster build by only
> using LTO in the final bootstrap stage.  With @samp{...}
> the LTO frontend is trained only on generator files."

Likewise.

> 
> (why do we need -frandom-seed=1?  IIRC that was only for the
> comparison step which is elided in all cases for -lean.mk).

Yep, it's not neded now.

Martin

> 
> Richard.
> 
>> Thanks,
>> Martin
>>
>> ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * Makefile.in: Regenerate.
>> * Makefile.tpl: Pass GENERATOR_CFLAGS
>> in all stages.
>>
>> config/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * bootstrap-lto-lean.mk: New file.
>>
>> gcc/ChangeLog:
>>
>> 2019-04-05  Martin Liska  
>>
>> * Makefile.in: Use GENERATOR_CFLAGS for all generators.
>> * configure: Regenerate.
>> * configure.ac: Pass GENERATOR_CFLAGS.
>> * doc/install.texi: Document the new config.
>> ---
>>  Makefile.in  | 207 +++
>>  Makefile.tpl |   2 +
>>  config/bootstrap-lto-lean.mk |  19 
>>  gcc/Makefile.in  |   4 +-
>>  gcc/configure|   6 +-
>>  gcc/configure.ac |   1 +
>>  gcc/doc/install.texi |   6 +
>>  7 files changed, 241 insertions(+), 4 deletions(-)
>>  create mode 100644 config/bootstrap-lto-lean.mk
>>
>>

>From 93542f291b7e6e8a7a6a54c8a915ee4b4e0262ed Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 3 Apr 2019 13:23:46 +0200
Subject: [PATCH] Come up with bootstrap-lto-lean config.

ChangeLog:

2019-04-05  Martin Liska  

	* Makefile.in: Regenerate.
	* Makefile.tpl: Pass GENERATOR_CFLAGS
	in all stages.

config/ChangeLog:

2019-04-05  Martin Liska  

	* bootstrap-lto-lean.mk: New file.

gcc/ChangeLog:

2019-04-05  Martin Liska  

	* Makefile.in: Use GENERATOR_CFLAGS for all generators.
	* configure: Regenerate.
	* configure.ac: Pass GENERATOR_CFLAGS.
	* doc/install.texi: Document the new config.
---
 Makefile.in  | 207 +++
 Makefile.tpl |   2 +
 config/bootstrap-lto-lean.mk |  17 +++
 gcc/Makefile.in  |   4 +-
 gcc/configure|   6 +-
 gcc/configure.ac |   1 +
 gcc/doc/install.texi |   6 +
 7 files changed, 239 insertions(+), 4 deletions(-)
 create mode 100644 config/bootstrap-lto-lean.mk

diff --git a/Makefile.in b/Makefile.in
index 231cc07cc0f..4445ad27efa 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -837,30 +837,39 @@ BASE_FLAGS_TO_PASS = \
 	"LEAN=$(LEAN)" \
 	"STAGE1_CFLAGS=$(STAGE1_CFLAGS)" \
 	"STAGE1_CXXFLAGS=$(STAGE1_CXXFLAGS)" \
+	"STAGE1_GENERATOR_CFLAGS=$(STAGE1_GENERATOR_CFLAGS)" \
 	"STAGE1_TFLAGS=$(STAGE1_TFLAGS)" \
 	"STAGE2_CFLAGS=$(STAGE2_CFLAGS)" \
 	"STAGE2_CXXFLAGS=$(STAGE2_CXXFLAGS)" \
+	"STAGE2_GENERATOR_CFLAGS=$(STAGE2_GENERATOR_CFLAGS)" \
 	"STAGE2_TFLAGS=$(STAGE2_TFLAGS)" \
 	"STAGE3_CFLAGS=$(STAGE3_CFLAGS)" \
 	"STAGE3_CXXFLAGS=$(STAGE3_CXXFLAGS)" \
+	"STAGE3_GENERATOR_CFLAGS=$(STAGE3_GENERATOR_CFLAGS)" \
 	"STAGE3_TFLAGS=$(STAGE3_TFLAGS)" \
 	"STAGE4_CFLAGS=$(STAGE4_CFLAGS)" \
 	"STAGE4_CXXFLAGS=$(STAGE4_CXXFLAGS)" \
+	"STAGE4_GENERATOR_CFLAGS=$(STAGE4_GENERATOR_CFLAGS)" \
 	"STAGE4_TFLAGS=$(STAGE4_TFLAGS)" \
 	"STAGEprofile_CFLAGS=$(STAGEprofile_CFLAGS)" \
 	"STAGEprofile_CXXFLAGS=$(STAGEprofile_CXXFLAGS)" \
+	"STAGEprofile_GENERATOR_CFLAGS=$(STAGEprofile_GENERATOR_CFLAGS)" \
 	"STAGEprofile_TFLAGS=$(STAGEprofile_TFLAGS)" \
 	"STAGEtrain_CFLAGS=$(STAGEtrain_CFLAGS)" \
 	"STAGEtrain_CXXFLAGS=$(STAGEtrain_CXXFLAGS)" \
+	"STAGEtrain_GENERATOR_CFLAGS=$(STAGEtrain_GENERATOR_CFLAGS)" \
 	"STAGEtrain_TFLAGS=$(STAGEtrain_TFLAGS)" \
 	

Re: [PATCH] Come up with bootstrap-lto-lean config.

[Richard Biener]

On Fri, Apr 5, 2019 at 12:42 PM Martin Liška  wrote:
>
> Hi.
>
> The patch adds a new config that makes LTO+PGO bootstrap faster by
> using LTO only in stage4. In stage3, generators are build with LTO
> in order to collect a reasonable profile for LTO FE.
>
> Ready for trunk?

I wonder if you need the

+AC_SUBST(GENERATOR_CFLAGS)

at all, can't you just use

+BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
+BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE

?

Please mention in both bootstrap-lto-lean.mk and the documentation
that the intended make target for this config is profiledbootstrap
since for non-profiledbootstrap it ends up not using LTO at all.  A "lean"
mode for non-profiledbootstrap would need to set up things to
use LTO only for stage3 which means not doing a bootstrap comparison
which means we could "skip" stage2 as well here.  So partial support
for that would be to have

STAGE2_CFLAGS += -frandom-seed=1
STAGE3_CFLAGS += -flto=jobserver -frandom-seed=1
...
do-compare = true

So if this works for non-profiledbootstrap the docs could be
changed to say "but is intended for faster build by only
using LTO in the final bootstrap stage.  With @samp{...}
the LTO frontend is trained only on generator files."

(why do we need -frandom-seed=1?  IIRC that was only for the
comparison step which is elided in all cases for -lean.mk).

Richard.

> Thanks,
> Martin
>
> ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * Makefile.in: Regenerate.
> * Makefile.tpl: Pass GENERATOR_CFLAGS
> in all stages.
>
> config/ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * bootstrap-lto-lean.mk: New file.
>
> gcc/ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * Makefile.in: Use GENERATOR_CFLAGS for all generators.
> * configure: Regenerate.
> * configure.ac: Pass GENERATOR_CFLAGS.
> * doc/install.texi: Document the new config.
> ---
>  Makefile.in  | 207 +++
>  Makefile.tpl |   2 +
>  config/bootstrap-lto-lean.mk |  19 
>  gcc/Makefile.in  |   4 +-
>  gcc/configure|   6 +-
>  gcc/configure.ac |   1 +
>  gcc/doc/install.texi |   6 +
>  7 files changed, 241 insertions(+), 4 deletions(-)
>  create mode 100644 config/bootstrap-lto-lean.mk
>
>

Re: [PATCH PR89725]Handle DR's access functions of loops not in DDR's loop_nest

[Richard Biener]

On Mon, Apr 8, 2019 at 8:17 AM bin.cheng  wrote:
>
> >
> >
> > --
> > Sender:Richard Biener 
> > Sent At:2019 Apr. 1 (Mon.) 21:07
> > Recipient:bin.cheng 
> > Cc:GCC Patches 
> > Subject:Re: [PATCH PR89725]Handle DR's access functions of loops not in 
> > DDR's loop_nest
> >
> >
> > On Mon, Apr 1, 2019 at 5:10 AM bin.cheng  
> > wrote:
> > >
> > > Hi,
> > >
> > > As described in comments of PR89725, this patch fixes out-of-bound access 
> > > during
> > > computing classic dist/dir vector for DDR.  Basically it does two things: 
> > > A) Handle
> > > relevant chrec of outer loop in multivariate access function as invariant 
> > > symbol during
> > > DDR analysis; B) Bypass relevant univariate access functions.
> > >
> > > Bootstrap and test on x86_64, any comments?
> >
> > LGTM, as Jakub said elsewhere index_in_loop_nest is better written as
> >
> > static inline int
> > index_in_loop_nest (int var, vec loop_nest)
> > {
> >   struct loop *loopi;
> >   int var_index;
> >
> >   for (var_index = 0; loop_nest.iterate (var_index, );
> >var_index++)
> > if (loopi->num == var)
> >   return var_index;
> >
> >   gcc_unreachable ();
> > }
> >
> > OK with that change.
>
> Updated patch according to this review.  Also, could anyone help me commit 
> this please?  Write-access on my system seems broken.  Thanks.

Will do.

Richard.

> Thanks,
> bin
> 2019-04-01  Bin Cheng  
>
> PR tree-optimization/89725
> * tree-chrec.c (chrec_contains_symbols): New parameter.  Handle outer
> loop's chrec as invariant symbol.
> * tree-chrec.h (chrec_contains_symbols): New parameter.
> * tree-data-ref.c (analyze_miv_subscript): Pass new argument.
> (build_classic_dist_vector_1, add_other_self_distances): Bypass access
> function of loops not in DDR's loop_nest.
> * tree-data-ref.h (index_in_loop_nest): Add unreachable check.
>
> 2018-04-01  Bin Cheng  
>
> PR tree-optimization/89725
> * gcc/testsuite/gcc.dg/tree-ssa/pr89725.c: New test.

[PATCH] Make gcov docs more precise (PR gcov-profile/89959).

[Martin Liška]

Hi.

The patch is about option documentation improvement as seen in the PR.
I'm adding the reported to CC. I'll install it if there are no comments.

Martin

gcc/ChangeLog:

2019-04-08  Martin Liska  

PR gcov-profile/89959
* doc/gcov.texi: Make documentation of -x option
more precise.
---
 gcc/doc/gcov.texi | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)


diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index ecad5d11847..0b8a1c78bfa 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -427,13 +427,15 @@ Print verbose informations related to basic blocks and arcs.
 
 @item -x
 @itemx --hash-filenames
-By default, gcov uses the full pathname of the source files to create
+When using @var{--preserve-paths},
+gcov uses the full pathname of the source files to create
 an output filename.  This can lead to long filenames that can overflow
 filesystem limits.  This option creates names of the form
 @file{@var{source-file}##@var{md5}.gcov},
 where the @var{source-file} component is the final filename part and
 the @var{md5} component is calculated from the full mangled name that
-would have been used otherwise.
+would have been used otherwise.  The option is an alternative
+to the @var{--preserve-paths} on systems which have a filesystem limit.
 
 @end table
 

Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE.

[Richard Biener]

On Fri, Apr 5, 2019 at 2:32 PM Martin Liška  wrote:
>
> Hi.
>
> The patch adds support for profile for GIMPLE FE. That can be useful
> in the future.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed after stage1 opens?

Hmm, I guess to be useful we need the profile quality indicators as well
(and think about a proper syntax here since it seems related).

Btw, do BB counts and edge probabilities not have a relation?  If so why
do we need both?  In predict.c we also look at ENTRY_BLOCK->count
so we need a place to set that as well.  We should probably set
edge probabilities of fallthru edges properly during our "CFG build".

+goto __BB3(44739243);

since we eventually want to add other flags this syntactically
should maybe be goto __BB3(guessed(44739243)); and similar
for the __BB count case (just s/count/quality/).

The entry block count could be sticked to __GIMPLE (ssa,guessed(N))
for example.  There's also the exit block, not sure if we ever look at
its count, so __GIMPLE (ssa,guessed(N[,M])) might be a possibility
if we always have the same quality here (probably not...).

Otherwise thanks for trying ;)

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * gimple-pretty-print.c (dump_gimple_bb_header):
> Dump BB count.
> (pp_cfg_jump): Dump edge probability.
> * profile-count.h (get_raw_value): New function.
> (from_raw_value): Likewise.
>
> gcc/c/ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * gimple-parser.c (struct gimple_parser): Add frequency
> for gimple_parser_edge.
> (gimple_parser::push_edge): Add new argument frequency.
> (c_parser_gimple_parse_bb_spec): Parse also frequency
> if present.
> (c_parser_parse_gimple_body): Set edge probability.
> (c_parser_gimple_compound_statement): Consume token
> before calling c_parser_gimple_goto_stmt.
> Parse BB counts.
> (c_parser_gimple_statement): Pass new argument.
> (c_parser_gimple_goto_stmt): Likewise.
> (c_parser_gimple_if_stmt): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2019-04-05  Martin Liska  
>
> * gcc.dg/gimplefe-37.c: New test.
> ---
>  gcc/c/gimple-parser.c  | 116 +++--
>  gcc/gimple-pretty-print.c  |   8 ++
>  gcc/profile-count.h|  15 
>  gcc/testsuite/gcc.dg/gimplefe-37.c |  27 +++
>  4 files changed, 144 insertions(+), 22 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c
>
>

[PATCH] Add data_file to GCOV interm. format (PR gcov-profile/89961).

[Martin Liška]

Hi.

The patch extends JSON intermediate format where data_file column is added.
I'm going to install the patch as obvious.

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

Martin

gcc/ChangeLog:

2019-04-05  Martin Liska  

PR gcov-profile/89961
* doc/gcov.texi: Document data_file.
* gcov.c (generate_results): Add data_info into JSON output.
---
 gcc/doc/gcov.texi | 4 
 gcc/gcov.c| 1 +
 2 files changed, 5 insertions(+)


diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index ecad5d11847..e086c3084f9 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -192,6 +192,7 @@ Structure of the JSON is following:
 @smallexample
 @{
   "current_working_directory": @var{current_working_directory},
+  "data_file": @var{data_file},
   "format_version": @var{format_version},
   "gcc_version": @var{gcc_version}
   "files": [@var{file}]
@@ -205,6 +206,9 @@ Fields of the root element have following semantics:
 @var{current_working_directory}: working directory where
 a compilation unit was compiled
 
+@item
+@var{data_file}: name of the data file (GCDA)
+
 @item
 @var{format_version}: semantic version of the format
 
diff --git a/gcc/gcov.c b/gcc/gcov.c
index 1d576552a45..1fc37a07c34 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -1421,6 +1421,7 @@ generate_results (const char *file_name)
 
   if (bbg_cwd != NULL)
 root->set ("current_working_directory", new json::string (bbg_cwd));
+  root->set ("data_file", new json::string (file_name));
 
   json::array *json_files = new json::array ();
   root->set ("files", json_files);

Re: [PATCH] Fix a missed case in predicate analysis of the late uninit pass

[Richard Biener]

On Fri, Apr 5, 2019 at 2:26 PM Vladislav Ivanishin  wrote:
>
> Richard Biener  writes:
>
> > On Thu, Apr 4, 2019 at 4:05 PM Vladislav Ivanishin  wrote:
> >>
> >> Richard Biener  writes:
> >>
> >> > On Mon, Apr 1, 2019 at 5:36 PM Vladislav Ivanishin  
> >> > wrote:
> >> >>
> >> >> Hi!
> >> >>
> >> >> This is a fairly trivial change fixing a false negative in
> >> >> -Wmaybe-uninitialized.  I am pretty sure this is simply an overlooked
> >> >> case (is_value_included_in() is not meant to deal with the case where
> >> >> both compare codes are NE_EXPRs, neither does it need to, since their
> >> >> handling is trivial).
> >> >>
> >> >> In a nutshell, what happens is values of v restricted by (v != 2) are
> >> >> incorrectly considered a subset of values of v restricted by (v != 1).
> >> >> As if "v != 2, therefore v != 1".
> >> >>
> >> >> This is by no means a gcc-9 regression; I'll ping the patch once stage4
> >> >> is over, if needed.
> >> >>
> >> >> This came up when I was experimenting with moving the uninit passes
> >> >> around.  On mainline, the late uninit pass runs very late, so reliably
> >> >> triggering the affected path is a bit tricky.  So I created a GIMPLE
> >> >> test (it reproduces the behavior precisely, but might be fragile
> >> >> w.r.t. future versions of the textual representation) and then with a
> >> >> hint from Alexander managed to produce a simple C test.  [By the way,
> >> >> the first take was to insert an asm with a lot of newlines to prevent
> >> >> the dom pass from rewriting the CFG due to high cost of duplicating
> >> >> instructions.  This didn't work out; I think the dom pass does not
> >> >> respect sizes of inline asms.  I plan to create a testcase and file a
> >> >> bug later.]
> >> >>
> >> >> I ran regression testing on x86_64-pc-linux-gnu and saw no new
> >> >> regressions modulo a handful of flaky UNRESOLVED tests under
> >> >> gcc.dg/tree-prof.  `BOOT_CFLAGS="-O -Wno-error=maybe-uninitialized
> >> >> -Wmaybe-uninitialized" bootstrap` also succeeded producing no new
> >> >> warnings.
> >> >>
> >> >> OK for stage1?
> >> >
> >> > Hum.  While definitely two NE_EXPR do not work correctly I'd like
> >> > to see a positive test since LT_EXPR doesn't work either?
> >>
> >> Right, thanks.  The case that was not covered well is actually when
> >> cond2 == NE_EXPR (arbitrary cond1).  I created 2 new tests: uninit-26.c
> >> demonstrates a false negative, and uninit-27-gimple.c a false positive
> >> with trunk.
> >>
> >> > Specifically the code falls through to test is_value_included_in which
> >> > seems to assume code1 == code2.
> >>
> >> The function is_value_included_in itself only cares about one condition
> >> code (I'll expound on this below).  I agree though that the second part
> >> of the comment seems to assume that.
> >>
> >> Please take a look at the updated patch.  The rationale is as follows.
> >>
> >> When we have 2 potentially comparable predicates in
> >> is_pred_expr_subset_of, there are basically two things we want to check.
> >>
> >> 1) Whether two ranges with identical condition codes are nested.  This
> >> is done straightforwardly with is_value_included_in.
> >>
> >> 2) Whether X CMPC VAL1 is nested in X != VAL2.  Which is easy: this
> >> happens iff the set (a.k.a "range") {X: X CMPC VAL1 is true} doesn't
> >> contain ("cover") VAL2, in other words iff VAL2 CMPC VAL1 is false.
> >>
> >> Only, the logic of 2) is faulty when X CMPC VAL1 is not a range bounded
> >> on one end (this happens when, and only when CMPC is NE_EXPR; the range
> >> is then unbounded on both ends and can only be nested in X != VAL2, if
> >> VAL1 == VAL2).
> >>
> >> 3) Whether X != VAL1 is nested in X CMPC VAL2, but that is always false
> >> unless CMPC is NE_EXPR, which is already considered.
> >
> > OK.  Your patch does
> >
> > +  if (code2 == NE_EXPR && code1 == NE_EXPR)
> > +return false;
> >
> > but it should instead return operand_equal_p (expr1.pred_rhs,
> > expr2.pred_rhs, 0)?
>
> This doesn't change the semantics, because the case with equal rhs's is
> already considered at the beginning of this function:
>
> static bool
> is_pred_expr_subset_of (pred_info expr1, pred_info expr2)
> {
>   enum tree_code code1, code2;
>
>   if (pred_equal_p (expr1, expr2))
> return true;
>
> So I think, leaving this part of the patch as is results in less
> localized/explicit code, but better matches the overall style of this
> function.  Or perhaps add a checking assert?

Ah, I looked for but missed this check...

>   if (code1 == code2)
> gcc_checking_assert (!operand_equal_p (expr1.pred_rhs,
>expr2.pred_rhs, 0))

No, I don't think that's needed.

> >> > To me it looks like is_value_includeds comment should be clarified to
> >> > say
> >> >
> >> > /* Returns true if all values X satisfying X CMPC VAL satisfy
> >> >X CMPC BOUNDARY.  */
> >>
> >> This is indeed more clear than the current comment, and the meaning is
> >> the same.  I 

Re: [PATCH] Fix PR83033

[Andrea Corallo]

Richard Earnshaw (lists) writes:

> Ah, you've just changed the ChangeLog entries.  By comments, I meant in
> the source code, so that it's clear these don't exist.  ChangeLog update
> is good too.
>
> R.

Hi Richard,
sorry my misunderstanding.

Bests
  Andrea


2019-03-29  Andrea Corallo  andrea.cora...@arm.com

PR target/83033
* config/aarch64/cortex-a57-fma-steering.c
(fma_forest): Prohibit copy construction.
(fma_root_node): Prohibit copy construction.
(func_fma_steering): Prohibit copy construction.

From 372ffe36fd0d59aa3c0ba1f8794c7aff9cee84a6 Mon Sep 17 00:00:00 2001
From: Andrea Corallo 
Date: Thu, 28 Mar 2019 23:17:17 +0100
Subject: [PATCH] imporove cortex-a57-fma-steering.c

---
 gcc/config/aarch64/cortex-a57-fma-steering.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c b/gcc/config/aarch64/cortex-a57-fma-steering.c
index f2da03a..4792d1f 100644
--- a/gcc/config/aarch64/cortex-a57-fma-steering.c
+++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
@@ -114,6 +114,9 @@ public:
   void dispatch ();
 
 private:
+  /* Prohibit copy construction.  */
+  fma_forest (const fma_forest &);
+
   /* The list of roots that form this forest.  */
   std::list *m_roots;
 
@@ -147,6 +150,9 @@ public:
   void set_head (du_head *);
   void rename (fma_forest *);
   void dump_info (fma_forest *);
+private:
+  /* Prohibit copy construction.  */
+  fma_node (const fma_node &);
 
 protected:
   /* Root node that lead to this node.  */
@@ -203,6 +209,9 @@ public:
   void execute_fma_steering ();
 
 private:
+  /* Prohibit copy construction.  */
+  func_fma_steering (const func_fma_steering &);
+
   void dfs (void (*) (fma_forest *), void (*) (fma_forest *, fma_root_node *),
 	void (*) (fma_forest *, fma_node *), bool);
   void analyze ();
-- 
2.7.4

Re: [PATCH] Fix directory_iterator handling of DT_UNKNOWN

[Jonathan Wakely]

On 08/04/19 09:18 +0200, Christophe Lyon wrote:

On Fri, 5 Apr 2019 at 18:57, Jonathan Wakely  wrote:


We need to handle DT_UNKNOWN earlier, not only during directory
recursion, so that the cached file_type value in the directory_entry
won't be used.

* src/c++17/fs_dir.cc (_Dir::advance(bool, error_code&)): Handle
d_type == DT_UNKNOWN immediately.
(_Dir::should_recurse(bool, error_code&)): Remove file_type::unknown
handling here.
* testsuite/27_io/filesystem/iterators/caching.cc: New test.

Tested powerpc64le-linux, committed to trunk.



Hi,

The new caching.cc test was lacking
+// { dg-require-filesystem-ts "" }
which I committed as obvious as r270199.


Thanks. I've added a Git pre-commit hook to stop me doing that again.

[PATCH][OBVIOUS] Fix expected scanned pattern.

[Martin Liška]

Hi.

One obvious fix for the test-case. I'm going to install the patch.

Martin

gcc/testsuite/ChangeLog:

2019-04-08  Martin Liska  

* gcc.target/riscv/arch-1.c: Fix expected scanned pattern.
---
 gcc/testsuite/gcc.target/riscv/arch-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/testsuite/gcc.target/riscv/arch-1.c b/gcc/testsuite/gcc.target/riscv/arch-1.c
index f12879c84d5..945897723dd 100644
--- a/gcc/testsuite/gcc.target/riscv/arch-1.c
+++ b/gcc/testsuite/gcc.target/riscv/arch-1.c
@@ -3,4 +3,4 @@
 int foo()
 {
 }
-/* { dg-error ".'-march=rv32I': first ISA subset must be `e', `i' or `g'" "" { target *-*-* } 0 } */
+/* { dg-error ".'-march=rv32I': first ISA subset must be 'e', 'i' or 'g'" "" { target *-*-* } 0 } */

Re: [PATCH] Remove usage of apostrophes in error and warning messages (PR translation/89935).

[Andreas Schwab]

FAIL: gcc.target/riscv/arch-1.c  (test for errors, line )
FAIL: gcc.target/riscv/arch-1.c (test for excess errors)
Excess errors:
cc1: error: '-march=rv32I': first ISA subset must be 'e', 'i' or 'g'

Andreas.

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

Re: [PATCH] Fix directory_iterator handling of DT_UNKNOWN

[Christophe Lyon]

On Fri, 5 Apr 2019 at 18:57, Jonathan Wakely  wrote:
>
> We need to handle DT_UNKNOWN earlier, not only during directory
> recursion, so that the cached file_type value in the directory_entry
> won't be used.
>
> * src/c++17/fs_dir.cc (_Dir::advance(bool, error_code&)): Handle
> d_type == DT_UNKNOWN immediately.
> (_Dir::should_recurse(bool, error_code&)): Remove file_type::unknown
> handling here.
> * testsuite/27_io/filesystem/iterators/caching.cc: New test.
>
> Tested powerpc64le-linux, committed to trunk.
>

Hi,

The new caching.cc test was lacking
+// { dg-require-filesystem-ts "" }
which I committed as obvious as r270199.

Christophe

[PATCH v2] elf.c: initialize struct stat

[mingli.yu]

From: Mingli Yu 

Initialize struct stat to fix the below
build failure when -Og included in compiler flag.
| 
./../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:
 In function 'elf_is_symlink':
| 
../../../../../../../../../work-shared/gcc-8.3.0-r0/gcc-8.3.0/libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21:
 error: 'st.st_mode' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
   return S_ISLNK (st.st_mode);

Signed-off-by: Mingli Yu 
---
 libbacktrace/elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index f3988ec..4ba8826 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -765,7 +765,7 @@ elf_syminfo (struct backtrace_state *state, uintptr_t addr,
 static int
 elf_is_symlink (const char *filename)
 {
-  struct stat st;
+  struct stat st = {0};
 
   if (lstat (filename, ) < 0)
 return 0;
-- 
2.7.4

Re: [PATCH PR89725]Handle DR's access functions of loops not in DDR's loop_nest

[bin.cheng]

> 
> 
> --
> Sender:Richard Biener 
> Sent At:2019 Apr. 1 (Mon.) 21:07
> Recipient:bin.cheng 
> Cc:GCC Patches 
> Subject:Re: [PATCH PR89725]Handle DR's access functions of loops not in DDR's 
> loop_nest
> 
> 
> On Mon, Apr 1, 2019 at 5:10 AM bin.cheng  wrote:
> >
> > Hi,
> >
> > As described in comments of PR89725, this patch fixes out-of-bound access 
> > during
> > computing classic dist/dir vector for DDR.  Basically it does two things: 
> > A) Handle
> > relevant chrec of outer loop in multivariate access function as invariant 
> > symbol during
> > DDR analysis; B) Bypass relevant univariate access functions.
> >
> > Bootstrap and test on x86_64, any comments?
> 
> LGTM, as Jakub said elsewhere index_in_loop_nest is better written as
> 
> static inline int
> index_in_loop_nest (int var, vec loop_nest)
> {
>   struct loop *loopi;
>   int var_index;
> 
>   for (var_index = 0; loop_nest.iterate (var_index, );
>var_index++)
> if (loopi->num == var)
>   return var_index;
> 
>   gcc_unreachable ();
> }
> 
> OK with that change.

Updated patch according to this review.  Also, could anyone help me commit this 
please?  Write-access on my system seems broken.  Thanks.

Thanks,
bin
2019-04-01  Bin Cheng  

PR tree-optimization/89725
* tree-chrec.c (chrec_contains_symbols): New parameter.  Handle outer
loop's chrec as invariant symbol.
* tree-chrec.h (chrec_contains_symbols): New parameter.
* tree-data-ref.c (analyze_miv_subscript): Pass new argument.
(build_classic_dist_vector_1, add_other_self_distances): Bypass access
function of loops not in DDR's loop_nest.
* tree-data-ref.h (index_in_loop_nest): Add unreachable check.

2018-04-01  Bin Cheng  

PR tree-optimization/89725
* gcc/testsuite/gcc.dg/tree-ssa/pr89725.c: New test.

0001-pr89725.patch
Description: Binary data