[PUSHED] Fix i686 bootstrap by temporarily disabling exporting of global ranges.

[Aldy Hernandez via Gcc-patches]

The patch converting evrp to the get_range_query(fun) API broke i686
bootstrap (commit 57bf37515).  The problem seems to be in a subsequent
pass that has more up-to-date global ranges.  I won't be able to look at
this until next week, so I am reverting the problematic bit of the
patch-- the exporting of global ranges once evrp finishes.  The use of
the new API remains.

Reverting the behavior shouldn't be a problem as we never used to export
global ranges from ranger.  This was new behavior in the patchset.

Tested on x86-64 Linux with a bootstrap and regtest, and on x86-32 with
only a bootstrap and the configure flags from the PR:

--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++ --enable-cet i686-linux 
--enable-bootstrap --with-fpmath=sse --disable-libcc1 --disable-libcilkrts 
--disable-libsanitizer

Sorry for the breakage folks.  I took time off on Friday and Monday is a
US bank holiday, so I wasn't paying much attention.  I will address
this properly starting Tuesday.

Patch pushed.

Aldy

gcc/ChangeLog:

PR tree-optimization/100787
* gimple-ssa-evrp.c: Disable exporting of global ranges.

gcc/testsuite/ChangeLog:

* gcc.dg/Wstringop-overflow-55.c:
* gcc.dg/pr80776-1.c:
---
 gcc/gimple-ssa-evrp.c| 6 --
 gcc/testsuite/gcc.dg/Wstringop-overflow-55.c | 8 
 gcc/testsuite/gcc.dg/pr80776-1.c | 4 +++-
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
index 118d10365a0..f1eea206afd 100644
--- a/gcc/gimple-ssa-evrp.c
+++ b/gcc/gimple-ssa-evrp.c
@@ -127,7 +127,8 @@ public:
 if (dump_file && (dump_flags & TDF_DETAILS))
   m_ranger->dump (dump_file);
 
-m_ranger->export_global_ranges ();
+// FIXME: Do not export ranges until PR100787 is fixed.
+//m_ranger->export_global_ranges ();
 disable_ranger (cfun);
   }
 
@@ -193,7 +194,8 @@ public:
 if (dump_file && (dump_flags & TDF_DETAILS))
   m_ranger->dump (dump_file);
 
-m_ranger->export_global_ranges ();
+// FIXME: Do not export ranges until PR100787 is fixed.
+//m_ranger->export_global_ranges ();
 disable_ranger (cfun);
   }
 
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c 
b/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
index c3c2dbe06dd..5f83af7c57f 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
@@ -66,7 +66,7 @@ void warn_ptrdiff_anti_range_add (ptrdiff_t i)
 {
   i |= 1;
 
-  char ca5[5];  // { dg-message "at offset \\\[1, 5]" "pr?" }
+  char ca5[5];  // { dg-message "at offset \\\[1, 5]" "pr?" { 
xfail *-*-* } }
   char *p0 = ca5;   // offset
   char *p1 = p0 + i;//  1-5
   char *p2 = p1 + i;//  2-5
@@ -74,7 +74,7 @@ void warn_ptrdiff_anti_range_add (ptrdiff_t i)
   char *p4 = p3 + i;//  4-5
   char *p5 = p4 + i;//   5
 
-  memset (p5, 0, 5);// { dg-warning "writing 5 bytes into a region of 
size" "pr?" }
+  memset (p5, 0, 5);// { dg-warning "writing 5 bytes into a region of 
size 0" "pr?" { xfail *-*-* } }
 
   sink (p0, p1, p2, p3, p4, p5);
 }
@@ -83,7 +83,7 @@ void warn_int_anti_range (int i)
 {
   i |= 1;
 
-  char ca5[5];  // { dg-message "at offset \\\[1, 5]" "pr?" }
+  char ca5[5];  // { dg-message "at offset \\\[1, 5]" "pr?" { 
xfail *-*-* } }
   char *p0 = ca5;   // offset
   char *p1 = p0 + i;//  1-5
   char *p2 = p1 + i;//  2-5
@@ -91,7 +91,7 @@ void warn_int_anti_range (int i)
   char *p4 = p3 + i;//  4-5
   char *p5 = p4 + i;//   5
 
-  memset (p5, 0, 5);// { dg-warning "writing 5 bytes into a region of 
size" "pr?" }
+  memset (p5, 0, 5);// { dg-warning "writing 5 bytes into a region of 
size 0" "pr?" { xfail *-*-* } }
 
   sink (p0, p1, p2, p3, p4, p5);
 }
diff --git a/gcc/testsuite/gcc.dg/pr80776-1.c b/gcc/testsuite/gcc.dg/pr80776-1.c
index f3a120b6744..af41c0c2ffa 100644
--- a/gcc/testsuite/gcc.dg/pr80776-1.c
+++ b/gcc/testsuite/gcc.dg/pr80776-1.c
@@ -17,5 +17,7 @@ Foo (void)
 __builtin_unreachable ();
   if (! (0 <= i && i <= 99))
 __builtin_unreachable ();
-  sprintf (number, "%d", i); /* { dg-bogus "writing" "" } */
+  /* The correctness bits for [E]VRP cannot handle chained conditionals
+ when deciding to ignore a unreachable branch for setting SSA range info. 
*/
+  sprintf (number, "%d", i); /* { dg-bogus "writing" "" { xfail *-*-* } } */
 }
-- 
2.31.1

Re: [PATCH 1/5] Common API for accessing global and on-demand ranges.

[Jeff Law via Gcc-patches]

On 5/25/2021 10:16 AM, Aldy Hernandez via Gcc-patches wrote:
The interface is now an inline function for get_range_query() and an 
external function for get_global_range_query), as discussed.


There are no magic cfun uses behind the scenes.

The annoying downcast is gone.

Passes can now decide if they want to export global ranges after they 
use a ranger.


I've adjusted the ChangeLog entries, as well as the commit text.

I've addressed everything discussed.

OK pending tests?

Aldy

0001-Common-API-for-accessing-global-and-on-demand-ranges.patch

 From eeb7627ddf686d5affb08dcad3674b560ef3ce6d Mon Sep 17 00:00:00 2001
From: Aldy Hernandez 
Date: Wed, 19 May 2021 18:27:05 +0200
Subject: [PATCH 1/5] Common API for accessing global and on-demand ranges.

This patch provides a generic API for accessing global ranges.  It is
meant to replace get_range_info() and get_ptr_nonnull() with one
common interface.  It uses the same API as the ranger (class
range_query), so there will now be one API for accessing local and
global ranges alike.

Follow-up patches will convert all users of get_range_info and
get_ptr_nonnull to this API.

For get_range_info, instead of:

   if (!POINTER_TYPE_P (TREE_TYPE (name)) && SSA_NAME_RANGE_INFO (name))
 get_range_info (name, vr);

You can now do:

   get_range_query (cfun)->range_of_expr (vr, name, [stmt]);

...as well as any other of the range_query methods (range_on_edge,
range_of_stmt, value_of_expr, value_on_edge, value_on_stmt, etc).

As per the API, range_of_expr will work on constants, SSA names, and
anything we support in irange::supports_type_p().

For pointers, the interface is the same, so instead of:

   else if (POINTER_TYPE_P (TREE_TYPE (name)) && SSA_NAME_PTR_INFO (name))
 {
   if (get_ptr_nonnull (name))
 stuff();
 }

One can do:

   get_range_query (cfun)->range_of_expr (vr, name, [stmt]);
   if (vr.nonzero_p ())
 stuff ();

Along with this interface, we are providing a mechanism by which a
pass can use an on-demand ranger transparently, without having to
change its code.  Of course, this assumes all get_range_info() and
get_ptr_nonnull() users have been converted to the new API, which
follow-up patches will do.

If a pass would prefer to use an on-demand ranger with finer grained
and context aware ranges, all it would have to do is call
enable_ranger() at the beginning of the pass, and disable_ranger() at
the end of the pass.

Note, that to use context aware ranges, any user of range_of_expr()
would need to pass additional context.  For example, the optional
gimple statement (or perhaps use range_on_edge or range_of_stmt).

The observant reader will note that get_range_query is tied to a
struct function, which may not be available in certain contexts, such
as at RTL time, gimple-fold, or some other places where we may or may
not have cfun set.

For cases where we are sure there is no function, you can use
get_global_range_query() instead of get_range_query(fun).  The API is
the same.

For cases where a function may be called with or without a function,
you could use the following idiom:

   range_query *query = cfun ? get_range_query (cfun)
 : get_global_range_query ();

   query->range_of_expr (range, expr, [stmt]);

The default range query obtained by get_range_query() is the global
range query, unless the user has enabled an on-demand ranger with
enable_ranger(), in which case it will use the currently active ranger.
That is, until disable_ranger() is called, at which point, we revert
back to global ranges.

We think this provides a generic way of accessing ranges, both
globally and locally, without having to keep track of types,
SSA_NAME_RANGE_INFO, and SSA_NAME_PTR_INFO.  We also hope this can be
used to transition passes from global to on-demand ranges when
appropriate.

gcc/ChangeLog:

* function.c (allocate_struct_function): Set cfun->x_range_query.
* function.h (struct function): Declare x_range_query.
(get_range_query): New.
(get_global_range_query): New.
* gimple-range-cache.cc (ssa_global_cache::ssa_global_cache):
Remove call to safe_grow_cleared.
* gimple-range.cc (get_range_global): New.
(gimple_range_global): Move from gimple-range.h.
(get_global_range_query): New.
(global_range_query::range_of_expr): New.
(enable_ranger): New.
(disable_ranger): New.
* gimple-range.h (gimple_range_global): Move to gimple-range.cc.
(class global_range_query): New.
(enable_ranger): New.
(disable_ranger): New.
* gimple-ssa-evrp.c (evrp_folder::~evrp_folder): Rename
dump_all_value_ranges to dump.
* tree-vrp.c (vrp_prop::finalize): Same.
* value-query.cc (range_query::dump): New.
* value-query.h (range_query::dump): New.
* vr-values.c (vr_values::dump_all_value_ranges): Rename to...
(vr_values::dump): ...this.
* vr-values.h (class vr_values): 

Re: [PATCH] Move global range code to value-query.cc.

[Jeff Law via Gcc-patches]

On 5/27/2021 2:39 AM, Aldy Hernandez via Gcc-patches wrote:

As discussed...

This patch moves all the global range code from gimple-range.cc into
value-query.cc.  It also moves get_range_info and get_ptr_nonnull from
tree-ssanames.c into their only uses, and removes external access to them.

No functional changes.

Pushed.

gcc/ChangeLog:

* gimple-range.cc (get_range_global): Move to value-query.cc.
(gimple_range_global): Same.
(get_global_range_query): Same.
(global_range_query::range_of_expr): Same.
* gimple-range.h (class global_range_query): Move to
value-query.h.
(gimple_range_global): Same.
* tree-ssanames.c (get_range_info): Move to value-query.cc.
(get_ptr_nonnull): Same.
* tree-ssanames.h (get_range_info): Remove.
(get_ptr_nonnull): Remove.
* value-query.cc (get_ssa_name_range_info): Move from
tree-ssanames.c.
(get_ssa_name_ptr_info_nonnull): Same.
(get_range_global): Move from gimple-range.cc.
(gimple_range_global): Same.
(get_global_range_query): Same.
(global_range_query::range_of_expr): Same.
* value-query.h (class global_range_query): Move from
gimple-range.h.
(gimple_range_global): Same.

OK
jeff

Re: [PATCH 1/2] Implement generic expression evaluator for range_query.

[Jeff Law via Gcc-patches]

On 5/27/2021 2:55 AM, Aldy Hernandez via Gcc-patches wrote:

Right now, range_of_expr only works with constants, SSA names, and
pointers.  Anything else gets returned as VARYING.  This patch adds the
capability to deal with arbitrary expressions, inasmuch as these
tree codes are implemented in range-ops.cc.

This will give us the ability to ask for the range of any tree expression,
not just constants and SSA names, with range_of_expr().

This is a more generic implementation of determine_value_range in VRP.
A follow-up patch will remove all uses of it in favor of the
range_query API.

Tested on x86-64 Linux.

OK?

gcc/ChangeLog:

* function-tests.c (test_ranges): Call gimple_range_tests.
* gimple-range-gori.cc (gori_compute::expr_range_at_stmt): Use
get_global_range_query instead of get_tree_range.
* gimple-range.cc (fur_source::get_operand): Add argument to
get_tree_range.
(get_arith_expr_range): New.
(get_tree_range): Add gimple and range_query arguments.
Call get_arith_expr_range.
(gimple_ranger::range_of_expr): Add argument to get_tree_range.
Include gimple-range-tests.cc.
* gimple-range.h (get_tree_range): Add argument.
* selftest.h (gimple_range_tests): New.
* value-query.cc (global_range_query::range_of_expr): Add
argument to get_tree_range.
* vr-values.c (vr_values::range_of_expr): Same.
* gimple-range-tests.cc: New file.

Both patches in this series are fine.

Thanks,
Jeff

Re: [PATCH] sim: leverage gnulib

[Jeff Law via Gcc-patches]

On 5/28/2021 8:55 PM, Mike Frysinger via Gcc-patches wrote:

We use getline, so leverage gnulib to provide fallback implementation.

ChangeLog:

* configure.ac: Add gnulib to configdirs for sim.
* configure: Regenerate.

OK
jeff

[PATCH] sim: leverage gnulib

[Mike Frysinger via Gcc-patches]

We use getline, so leverage gnulib to provide fallback implementation.

ChangeLog:

* configure.ac: Add gnulib to configdirs for sim.
* configure: Regenerate.
---
 configure| 3 +++
 configure.ac | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/configure b/configure
index 3715d169dbfe..1224fc4039ec 100755
--- a/configure
+++ b/configure
@@ -9520,6 +9520,9 @@ case " ${configdirs} " in
   *\ gdbserver\ *)
 configdirs="${configdirs} gnulib gdbsupport"
 ;;
+  *\ sim\ *)
+configdirs="${configdirs} gnulib"
+;;
 esac
 
 # Strip out unwanted targets.
diff --git a/configure.ac b/configure.ac
index 626e2106cba7..66d637d70dc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2849,6 +2849,9 @@ case " ${configdirs} " in
   *\ gdbserver\ *)
 configdirs="${configdirs} gnulib gdbsupport"
 ;;
+  *\ sim\ *)
+configdirs="${configdirs} gnulib"
+;;
 esac
 
 # Strip out unwanted targets.
-- 
2.31.1

[Patch, fortran] PR fortran/100120/100816/100818/100819/100821 problems raised by aggregate data types

[José Rui Faustino de Sousa via Gcc-patches]

Hi all!

Proposed patch to:

Bug 100120 - associated intrinsic failure
Bug 100816 - Wrong span on widechar
Bug 100818 - A temporary is passed to associated
Bug 100819 - Wrong code generation with unlimited polymorphic objects 
and character type

Bug 100821 - Deferred character with wrong length

Patch tested only on x86_64-pc-linux-gnu.

This patch mostly deals with setting "span" and "elem_len" for aggregate 
types. For that it is necessary to work around the way in which deferred 
type is implemented, which works fine for assumed character length, but 
doesn't work properly with more dynamic lengths, like with deferred 
character. And to make sure that unlimited polymorphic objects have 
"_len" properly set and receive correct the dynamic type data.
After requiring that no temporaries are created to pass to "associated" 
one notices that the library "associated" implementation relies on 
"elem_len", which may vary for parent types or sub strings, and not on 
the full object size "span", also leading to associated objects not 
being recognized as such.
Finally efforts were made so that the "span" calculation is done on 
descriptor creation and referred to afterwards, only being recalculated 
as a last resort.


Thank you very much.

Best regards,
José Rui

Fortran: Fix some issues with pointers to character.

gcc/fortran/ChangeLog:

PR fortran/100120/100816/100818/100819/100821
* trans-array.c (gfc_get_array_span): rework the way character
array "span" was calculated.
(gfc_conv_expr_descriptor): improve handling of character
sections and unlimited polymorphic objects.
* trans-expr.c (gfc_get_character_len): new function to
calculate character string length.
(gfc_get_character_len_in_bytes): new function to calculate
character string length in bytes.
(gfc_conv_scalar_to_descriptor): add call to set the "span".
(gfc_trans_pointer_assignment): set "_len" and antecipate the
initialization of the deferred character length hidden argument.
* trans-intrinsic.c (gfc_conv_associated): set "force_no_tmp" to
avoid the creation of a temporary.
* trans-types.c (gfc_get_dtype_rank_type): rework type detection
so that unlimited polymorphic objects get proper type
infomation, also important for bind(c).
(gfc_get_dtype): add argument to pass the rank if necessary.
(gfc_get_array_type_bounds): cosmetic change to have character
arrays called character instead of unknown.
* trans-types.h (gfc_get_dtype): modify prototype.
* trans.c (get_array_span): rework the way character array
"span" was calculated.
* trans.h (gfc_get_character_len): new prototype.
(gfc_get_character_len_in_bytes): new prototype.
Add "unlimited_polymorphic" flag to "gfc_se" type to signal when
expression carries an unlimited polymorphic object.

libgfortran/ChangeLog:

PR fortran/100120
* intrinsics/associated.c (associated): have associated verify
if the "span" matches insted of the "elem_len".
* libgfortran.h (GFC_DESCRIPTOR_SPAN): add macro to retrive the
descriptor "span".

gcc/testsuite/ChangeLog:

PR fortran/100120
* gfortran.dg/PR100120.f90: New test.
PR fortran/100816
PR fortran/100818
PR fortran/100819
PR fortran/100821
* gfortran.dg/character_workout_1.f90: New test.
* gfortran.dg/character_workout_4.f90: New test.


diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 7eeef55..a6bcd2b 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -860,16 +860,25 @@ gfc_get_array_span (tree desc, gfc_expr *expr)
 	 size of the array. Attempt to deal with unbounded character
 	 types if possible. Otherwise, return NULL_TREE.  */
   tmp = gfc_get_element_type (TREE_TYPE (desc));
-  if (tmp && TREE_CODE (tmp) == ARRAY_TYPE
-	  && (TYPE_MAX_VALUE (TYPE_DOMAIN (tmp)) == NULL_TREE
-	  || integer_zerop (TYPE_MAX_VALUE (TYPE_DOMAIN (tmp)
-	{
-	  if (expr->expr_type == EXPR_VARIABLE
-	  && expr->ts.type == BT_CHARACTER)
-	tmp = fold_convert (gfc_array_index_type,
-gfc_get_expr_charlen (expr));
-	  else
-	tmp = NULL_TREE;
+  if (tmp && TREE_CODE (tmp) == ARRAY_TYPE && TYPE_STRING_FLAG (tmp))
+	{
+	  gcc_assert (expr->ts.type == BT_CHARACTER);
+	  
+	  tmp = gfc_get_character_len_in_bytes (tmp);
+	  
+	  if (tmp == NULL_TREE || integer_zerop (tmp))
+	{
+	  tree bs;
+
+	  tmp = gfc_get_expr_charlen (expr);
+	  tmp = fold_convert (gfc_array_index_type, tmp);
+	  bs = build_int_cst (gfc_array_index_type, expr->ts.kind);
+	  tmp = fold_build2_loc (input_location, MULT_EXPR,
+ gfc_array_index_type, tmp, bs);
+	}
+	  
+	  tmp = (tmp && !integer_zerop (tmp))
+	? (fold_convert (gfc_array_index_type, tmp)) : (NULL_TREE);
 	}
   else
 	tmp 

[DWARF] Also generate DW_OP_GNU_variable_value at file scope

[Eric Botcazou]

But only for the reference variant (dw_val_class_die_ref).  This is needed for 
variable-sized types declared at library level in Ada.

Tested on x86-64/Linux, both GCC and GDB, OK for the mainline?


2021-05-28  Eric Botcazou  

* dwarf2out.c (loc_list_from_tree_1) : Also generate
DW_OP_GNU_variable_value referencing an existing DIE at file scope.
(type_byte_size): Inline into...
(add_byte_size_attribute): ...this and call add_scalar_info.

-- 
Eric Botcazoudiff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 523735e1521..ebb01ac0cef 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -18829,25 +18829,25 @@ loc_list_from_tree_1 (tree loc, int want_address,
 	  {
 	if (TREE_CODE (loc) != FUNCTION_DECL
 		&& early_dwarf
-		&& current_function_decl
 		&& want_address != 1
 		&& ! DECL_IGNORED_P (loc)
 		&& (INTEGRAL_TYPE_P (TREE_TYPE (loc))
 		|| POINTER_TYPE_P (TREE_TYPE (loc)))
-		&& DECL_CONTEXT (loc) == current_function_decl
 		&& (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (loc)))
 		<= DWARF2_ADDR_SIZE))
 	  {
 		dw_die_ref ref = lookup_decl_die (loc);
-		ret = new_loc_descr (DW_OP_GNU_variable_value, 0, 0);
 		if (ref)
 		  {
+		ret = new_loc_descr (DW_OP_GNU_variable_value, 0, 0);
 		ret->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
 		ret->dw_loc_oprnd1.v.val_die_ref.die = ref;
 		ret->dw_loc_oprnd1.v.val_die_ref.external = 0;
 		  }
-		else
+		else if (current_function_decl
+			 && DECL_CONTEXT (loc) == current_function_decl)
 		  {
+		ret = new_loc_descr (DW_OP_GNU_variable_value, 0, 0);
 		ret->dw_loc_oprnd1.val_class = dw_val_class_decl_ref;
 		ret->dw_loc_oprnd1.v.val_decl_ref = loc;
 		  }
@@ -19588,32 +19588,6 @@ round_up_to_align (const offset_int , unsigned int align)
   return wi::udiv_trunc (t + align - 1, align) * align;
 }
 
-/* Compute the size of TYPE in bytes.  If possible, return NULL and store the
-   size as an integer constant in CST_SIZE.  Otherwise, if possible, return a
-   DWARF expression that computes the size.  Return NULL and set CST_SIZE to -1
-   if we fail to return the size in one of these two forms.  */
-
-static dw_loc_descr_ref
-type_byte_size (const_tree type, HOST_WIDE_INT *cst_size)
-{
-  /* Return a constant integer in priority, if possible.  */
-  *cst_size = int_size_in_bytes (type);
-  if (*cst_size != -1)
-return NULL;
-
-  struct loc_descr_context ctx = {
-const_cast (type),	/* context_type */
-NULL_TREE,	  		/* base_decl */
-NULL,	  		/* dpi */
-false,	  		/* placeholder_arg */
-false,	  		/* placeholder_seen */
-false	  		/* strict_signedness */
-  };
-
-  tree tree_size = TYPE_SIZE_UNIT (TYPE_MAIN_VARIANT (type));
-  return tree_size ? loc_descriptor_from_tree (tree_size, 0, ) : NULL;
-}
-
 /* Helper structure for RECORD_TYPE processing.  */
 struct vlr_context
 {
@@ -21555,7 +21529,6 @@ add_byte_size_attribute (dw_die_ref die, tree tree_node)
 {
   dw_die_ref decl_die;
   HOST_WIDE_INT size;
-  dw_loc_descr_ref size_expr = NULL;
 
   switch (TREE_CODE (tree_node))
 {
@@ -21572,7 +21545,7 @@ add_byte_size_attribute (dw_die_ref die, tree tree_node)
 	  add_AT_die_ref (die, DW_AT_byte_size, decl_die);
 	  return;
 	}
-  size_expr = type_byte_size (tree_node, );
+  size = int_size_in_bytes (tree_node);
   break;
 case FIELD_DECL:
   /* For a data member of a struct or union, the DW_AT_byte_size is
@@ -21585,19 +21558,31 @@ add_byte_size_attribute (dw_die_ref die, tree tree_node)
   gcc_unreachable ();
 }
 
-  /* Support for dynamically-sized objects was introduced by DWARFv3.
- At the moment, GDB does not handle variable byte sizes very well,
- though.  */
-  if ((dwarf_version >= 3 || !dwarf_strict)
-  && gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL
-  && size_expr != NULL)
-add_AT_loc (die, DW_AT_byte_size, size_expr);
-
   /* Note that `size' might be -1 when we get to this point.  If it is, that
- indicates that the byte size of the entity in question is variable and
- that we could not generate a DWARF expression that computes it.  */
+ indicates that the byte size of the entity in question is variable.  */
   if (size >= 0)
 add_AT_unsigned (die, DW_AT_byte_size, size);
+
+  /* Support for dynamically-sized objects was introduced by DWARFv3.  */
+  else if (TYPE_P (tree_node)
+	   && (dwarf_version >= 3 || !dwarf_strict)
+	   && gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL)
+{
+  struct loc_descr_context ctx = {
+	const_cast (tree_node),	/* context_type */
+	NULL_TREE,	  		/* base_decl */
+	NULL,	  		/* dpi */
+	false,	  		/* placeholder_arg */
+	false,	  		/* placeholder_seen */
+	false	  		/* strict_signedness */
+  };
+
+  tree tree_size = TYPE_SIZE_UNIT (TYPE_MAIN_VARIANT (tree_node));
+  add_scalar_info (die, DW_AT_byte_size, tree_size,
+		   dw_scalar_form_exprloc
+		   | 

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Michael Matz]

Hello,

On Fri, 28 May 2021, Bernd Edlinger wrote:

> >> I was wondering, why gimple-match.c and generic-match.c
> >> are not built early but always last, which slows down parallel
> >> makes significantly.
> >>
> >> The reason seems to be that generated_files does not
> >> mention gimple-match.c and generic-match.c.
> >>
> >> This comment in Makefile.in says it all:
> >>
> >> $(ALL_HOST_OBJS) : | $(generated_files)
> >>
> >> So this patch adds gimple-match.c generic-match.c to generated_files.
> >>
> >>
> >> Tested on x86_64-pc-linux-gnu.
> >> Is it OK for trunk?
> > 
> > This should help for what I was complaining about in
> > https://gcc.gnu.org/pipermail/gcc/2021-May/235963.html . I build with
> > -j24 and it was stalling on compiling gimple-match.c for me.
> > Looks like insn-attrtab.c is missed too; I saw genattrtab was running last 
> > too.
> > 
> 
> Yeah, probably insn-automata.c as well, sometimes it is picked up early 
> sometimes not. maybe $(simple_generated_c) should be added to 
> generated_files, but insn-attrtab.c is yet another exception.

You can't put files in there that are sometimes slow to generate (which 
insn-{attrtab,automata}.c are on some targets), as _everything_ then waits 
for them to be created first.

Ideally there would be a way for gnumake to mark some targets as 
"ugh-slow" and back-propagate this to all dependencies so that those are 
put in front of the work queue in a parallel make.  Alas, something like 
that never came into existence :-/  (When order-only deps were introduced 
I got excited, but then came to realize that that wasn't what was really 
needed for this case, a "weak" version of it would be required at least, 
or better yet a specific facility to impose a cost with a target)


Ciao,
Michael.

> 
> 
> Bernd.
> 
> > Thanks,
> > Andrew
> > 
> >>
> >>
> >> Thanks
> >> Bernd.
> >>
> >>
> >> 2021-05-28  Bernd Edlinger  
> >>
> >> * Makefile.in (generated_files): Add gimple-match.c and
> >> generic-match.c
> 

[PATCH][RFC] tree-optimization/100801 - perform final value replacement from VRP

[Richard Biener]

This makes sure to perform final value replacement of constants
when we also are sure to propagate those, like in VRP.  This avoids
spurious diagnostics when doing so only from within SCCP which can
leave unreachable loops in the IL triggering bogus diagnostics.

The choice is VRP because it already defers to SCEV for PHI nodes
so the following makes it analyze final values for loop exit PHIs.
To make that work it arranges for loop-closed SSA to be built only
after inserting ASSERT_EXPRs.

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

It does

FAIL: gcc.dg/ubsan/pr94423.c   -O2  (internal compiler error)

where VRP somehow propagates abnormals in a bogus way.  It also
needs adjustment for a couple of fails like

FAIL: gcc.dg/vect/no-scevccp-outer-11.c scan-tree-dump-times vect "OUTER 
LOOP VECTORIZED." 1

where the testcases do -fno-tree-scev-cprop but that no longer has
the desired effect then (might just guard the final value replacement
in VRP with this).

Any comments?  I probably can plug final_value_at_exit elsewhere
than VRP (to avoid the issues with asserts) but it somewhat felt
naturally because that already uses SCEV.

Thanks,
Richard.

2021-05-28  Richard Biener  

PR tree-optimization/100801
* tree-scalar-evolution.h (final_value_at_exit): Declare.
* tree-scalar-evolution.c (final_value_at_exit): New API,
split out from ...
(final_value_replacement_loop): ... here.
* tree-vrp.c (execute_vrp): Build loop-closed SSA only
after inserting assert expressions.
(vrp_asserts::process_assert_insertions_for): Avoid inserting
asserts for RESULT_DECLs, when building loop-closed SSA
after assert insertion we're not able to propagate out all
PHIs and thus trigger IL validation.
* vr-values.c (vr_values::extract_range_from_phi_node):
For loop exit PHIs also perform final value analysis using
SCEV.

* gcc.target/i386/pr100801.c: New testcase.
---
 gcc/testsuite/gcc.target/i386/pr100801.c | 29 
 gcc/tree-scalar-evolution.c  | 28 +++
 gcc/tree-scalar-evolution.h  |  1 +
 gcc/tree-vrp.c   | 12 --
 gcc/vr-values.c  | 23 +++
 5 files changed, 77 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr100801.c

diff --git a/gcc/testsuite/gcc.target/i386/pr100801.c 
b/gcc/testsuite/gcc.target/i386/pr100801.c
new file mode 100644
index 000..c58f9be6898
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr100801.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx" } */
+
+#include 
+
+void copy_32_unaligned(unsigned int* dest, const unsigned int* src, size_t 
count)
+{
+  // invariant/nop
+  __m128i shufmask =  _mm_set_epi8(15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 
3, 2, 1, 0);
+
+  size_t i;
+  for (i = 0; i + 4 <= count; i += 4) {
+__m128i input = _mm_loadu_si128((const __m128i*)([i]));
+__m128i output = input;
+// no warning without the shuffle:
+output = _mm_shuffle_epi8(input, shufmask);
+_mm_storeu_si128((__m128i*)([i]), output);
+  }
+  for (; i < count; ++i) {  // handle residual elements
+dest[i] = src[i]; /* { dg-bogus "invokes undefined" } */
+  }
+}
+
+void test(unsigned int* buf1, unsigned int* buf2)
+{
+  copy_32_unaligned(buf2, buf1,
+   // multiples of 4 and greater or equal then 12 trigger it:
+   12);
+}
diff --git a/gcc/tree-scalar-evolution.c b/gcc/tree-scalar-evolution.c
index b22d49a0ab6..6917b8c9dab 100644
--- a/gcc/tree-scalar-evolution.c
+++ b/gcc/tree-scalar-evolution.c
@@ -3491,6 +3491,23 @@ expression_expensive_p (tree expr)
  || expanded_size > cache.elements ());
 }
 
+/* Compute the final value of DEF, a DEF inside the loop E exits from.  */
+
+tree
+final_value_at_exit (tree def, edge e, bool *folded_casts)
+{
+  class loop *loop = e->src->loop_father;
+  gcc_assert (loop_exit_edge_p (loop, e));
+  class loop *ex_loop
+= superloop_at_depth (loop, loop_depth (e->dest->loop_father) + 1);
+  tree ev = analyze_scalar_evolution_in_loop (ex_loop, loop, def, 
folded_casts);
+  ev = compute_overall_effect_of_inner_loop (ex_loop, ev);
+  if (!tree_does_not_contain_chrecs (def)
+  || chrec_contains_symbols_defined_in_loop (ev, ex_loop->num))
+ev = chrec_dont_know;
+  return ev;
+}
+
 /* Do final value replacement for LOOP, return true if we did anything.  */
 
 bool
@@ -3513,10 +3530,6 @@ final_value_replacement_loop (class loop *loop)
   /* Set stmt insertion pointer.  All stmts are inserted before this point.  */
   gimple_stmt_iterator gsi = gsi_after_labels (exit->dest);
 
-  class loop *ex_loop
-= superloop_at_depth (loop,
- loop_depth (exit->dest->loop_father) + 1);
-
   bool any = false;
   gphi_iterator psi;
   for (psi = gsi_start_phis (exit->dest); !gsi_end_p (psi); )

Re: Fallout: save/restore target options in handle_optimize_attribute

[Segher Boessenkool]

Hi!

On Fri, May 28, 2021 at 11:48:06AM +0200, Martin Liška wrote:
> There's a fallout after my revision 
> ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e. I would like to analyze
> all case and discuss possible solution. To be honest it's a can of worms 
> and reverting the commit
> is an option on the table.

> $ ./xgcc -B. -Os pr.C
> pr.C:2:11: internal compiler error: ‘global_options’ are modified in 
> local context
> 2 | void main();

So what is called "global" and "local" there?  There are at least three
levels (cannot modify within a TU, cannot modify within a function, and
anything goes).  What is this about?

> What happens: we change from -Os to -O0 and rs6000_isa_flags differ in 
> cl_optimization_compare.
> Problem is that OPTION_MASK_SAVE_TOC_INDIRECT is set based on optimize flag:
> 
>   /* If we can shrink-wrap the TOC register save separately, then use
>  -msave-toc-indirect unless explicitly disabled.  */
>   if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
>   && flag_shrink_wrap_separate
>   && optimize_function_for_speed_p (cfun))
> rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;

Why is that a problem?  This has always been allowed, and disallowing it
now will have a lot of fallout.  Why change the basic features of the
model at all, is there a very good reason for that?

> This is caused by a weird option override:
> 
>   else if (rs6000_long_double_type_size == 128)
> rs6000_long_double_type_size = FLOAT_PRECISION_TFmode; (it's 127)

This is a workaround for the fact that GCC insists all floating point
types can be ordered.  They can not.  There are numbers in both
double-double ("IBM extended long double") and in IEEE QP float that
cannot be represented with the same precision in the other format.  So
Mike made GCC think one format is "better" than the other depending on
which we default to.  This hack works remarkably well, but is of course
a hack.  Until the generic problems here are fixed we cannot do better.

> later when rs6000_option_override_internal is called for saved target flags 
> (127), it complains.
> Possible fix:
> 
>   else if (rs6000_long_double_type_size == 128
>  || rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)

Please propose that as separate patch, and/or open a PR for it.  Having
everything in one mail thread might be easiest for you, but it isn't for
everyone else ;-)

> What target maintainers thing about it?

That we need a lot more background: what do you want to do, and why?


Segher

Re: [PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello again :)  Wanted to shoot a quick status update. Some github issues have
been created for points of feedback, and we've been working on addressing them.
A few changes have been pushed to the contracts-jac-alt branch, while there's
also an active more in depth rewrite branch. Some specific comments inline
below.

On 5/17/21, Jason Merrill  wrote:
> On 5/14/21 4:54 PM, Jason Merrill wrote:
>> On 4/30/21 1:44 PM, Jeff Chapman wrote:
>>> Hello! Looping back around to this. re:
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567334.html
>>>
>>> On 3/25/21, Jason Merrill  wrote:
 On 3/1/21 8:12 AM, Jeff Chapman wrote:
> On 1/18/21, Jason Merrill  wrote:
>> On 1/4/21 9:58 AM, Jeff Chapman wrote:
>>> Ping. re:
>>> https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html
>>> 
>>>
>>> https://github.com/lock3/gcc/tree/contracts-jac-alt
>>> 
>>>
>> Why is some of the code in c-family?  From the modules merge there is
>> now a cp_handle_option function that you could add the option
>> handling
>> to, and I don't see a reason for cxx-contracts.c to be in c-family/
>> rather than cp/.
>
> I've been pushing changes that address the points raised and wanted to
> ping to see if there's more feedback and give a status summary. The
> notable change is making sure the implementation plays nicely with
> modules and a mangling change that did away with a good chunk of code.
>
> The contracts code outside of cp/ has been moved into it, and the
> contract attributes have been altered to work with language
> independent
> handling code. More of the contracts code can probably still be
> moved to
> cxx-contracts which I'll loop back to after other refactoring. The
> naming, spelling, and comments (or lack thereof) have been addressed.

 Sounds good.  I plan to get back to this when GCC 11 branches, which
 should be mid-April.
>>>
>>> Please let me know if you see any more issues when you pick it back up.
>>> Particularly in modules interop, since I don't think you've had a chance
>>> to look at that yet.
>>>
>>> Completed another merge with master earlier this week which didn't bring
>>> to light any new issues or regressions, but I'll keep on that :)
>>>
>>> +  /* If we have contracts, check that they're valid in this
>>> context. */
>>> +  // FIXME: These aren't entirely correct.
>>
>> How not?  Can local extern function decls have contract attributes?
>>
>>> + /* FIXME when we instatiate a template class with
>>> guarded
>>> +  * members, particularly guarded template members,
>>> the resulting
>>> +  * pre/post functions end up being inaccessible
>>> because their
>>> +  * template info's context is the original
>>> uninstantiated class.
>>
>> This sounds like a significant functionality gap.  I'm guessing you
>> want
>> to reconsider the unshare_template approach.
>>
>> One approach would be to only do the pre/post/guarded/unguarded
>> transformation for a fully-instantiated function; a temploid function
>> would leave the contracts as attributes.
>>

There's an in progress rewrite which moves pre/post function generation until
much later which should resolve this. Like you suggested, these are not created
until we're completely out of template processing.

>>> +  /* FIXME do we need magic to perfectly forward this so we
>>> don't clobber
>>> +RVO/NRVO etc?  */
>>
>> Yes.  CALL_FROM_THUNK_P will probably get you some of the magic you
>> want.
>
> These points are still being investigated and addressed; including
> them
> for reference.
>>
>> Any update?
>>

CALL_FROM_THUNK_P is being set now when an actual call is built. Is there a good
way to test that the optimization isn't being broken in general?

>> More soon.
>
> Please let me know what other issues need work.
>>>
>>> If there's anything I can do to make the process smoother please don't
>>> hesitate to ask.
>>
>> Larger-scope comments:
>>
>> Please add an overview of the implementation strategy to the top of
>> cxx-contracts.c.  Particularly to discuss the why and how of
>> pre/post/guarded/unguarded functions.

An initial overview has been added, though it'll need updated with some of the
pending rewrites.

>
>> I'm confused by the approach to late parsing of contracts; it seems like
>> you wait until the end of parsing the function to go back and parse the
>> contracts.  Why not parse them sooner, along with nsdmis, noexcept, and
>> function bodies?

Parsing has been moved forward on the rewrite branch.

>>
>> Smaller-scope comments:
>>
 +   /* If we didn't build a check, insert a NOP so we don't leak
 + 

[PATCH] tree-optimization/100778 - avoid cross-BB vectorization of trapping op

[Richard Biener]

This avoids vectorizing a possibly trapping operation when lanes
are handled in different BBs.  I spotted this when working on the
originally reported issue in PR100778.

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

2021-05-28  Richard Biener  

PR tree-optimization/100778
* tree-vect-slp.c (vect_build_slp_tree_1): Prevent possibly
trapping ops in different BBs.

* gcc.dg/vect/bb-slp-pr100778-1.c: New testcase.
---
 gcc/testsuite/gcc.dg/vect/bb-slp-pr100778-1.c | 18 ++
 gcc/tree-vect-slp.c   |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/bb-slp-pr100778-1.c

diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pr100778-1.c 
b/gcc/testsuite/gcc.dg/vect/bb-slp-pr100778-1.c
new file mode 100644
index 000..9f8b7eecef1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pr100778-1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target vect_double } */
+
+double foo (int x, double *p)
+{
+  double res = p[0] + p[1];
+  double tem = p[0] / x;
+  if (x)
+{
+  p[0] = tem;
+  p[1] /= x;
+}
+  return res + tem;
+}
+
+/* We may not SLP vectorize the FP division because it can trap and it
+   is distributed between two basic-blocks.  */
+/* { dg-final { scan-tree-dump "Build SLP failed: different BB for PHI or 
possibly trapping operation in _\[0-9\]+ = _\[0-9\]+ / _\[0-9\]+;" "slp2" } } */
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 0ec92b0f0ca..ca1539e63f2 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -1214,14 +1214,14 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char 
*swap,
}
}
 
- if (phi_p
+ if ((phi_p || gimple_could_trap_p (stmt_info->stmt))
  && (gimple_bb (first_stmt_info->stmt)
  != gimple_bb (stmt_info->stmt)))
{
  if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
 "Build SLP failed: different BB for PHI "
-"in %G", stmt);
+"or possibly trapping operation in %G", stmt);
  /* Mismatch.  */
  continue;
}
-- 
2.26.2

Re: Fallout: save/restore target options in handle_optimize_attribute

[Richard Biener via Gcc-patches]

On Fri, May 28, 2021 at 11:48 AM Martin Liška  wrote:
>
> Hi.
>
> There's a fallout after my revision ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e. 
> I would like to analyze
> all case and discuss possible solution. To be honest it's a can of worms and 
> reverting the commit
> is an option on the table.
>
> So the cases:
>
> 1) PR100759 - ppc64le
>
> $ cat pr.C
> #pragma GCC optimize 0
> void main();
>
> $ ./xgcc -B. -Os pr.C
> pr.C:2:11: internal compiler error: ‘global_options’ are modified in local 
> context
>  2 | void main();
>
> What happens: we change from -Os to -O0 and rs6000_isa_flags differ in 
> cl_optimization_compare.
> Problem is that OPTION_MASK_SAVE_TOC_INDIRECT is set based on optimize flag:
>
>/* If we can shrink-wrap the TOC register save separately, then use
>   -msave-toc-indirect unless explicitly disabled.  */
>if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
>&& flag_shrink_wrap_separate
>&& optimize_function_for_speed_p (cfun))
>  rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;

So that means that

  /* Restore current options.  */
  cl_optimization_restore (_options, _options_set,
   _opts);
  cl_target_option_restore (_options, _options_set,
TREE_TARGET_OPTION (prev_target_node));

does not result in the same outcome as the original command-line processing?

Given both restore processes could interact (not sure if that's the issue here)
shouldn't we just have a single restore operation and a single target
hook instead of both targetm.override_options_after_change and
targetm.target_option.restore?

Likewise we should probably _always_ set both, DECL_FUNCTION_SPECIFIC_OPT
and _TARGET as a step towards unifying them.

That said, for the above case a more detailed run-down as to how things go wrong
would be nice to see.

> Suggested solution is doing:
>
>if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
>&& flag_shrink_wrap_separate
>  rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;
>
> and add '&& optimize_function_for_speed_p (cfun)' to the place where the 
> option mask is used.
>
> 2) Joseph's case:
>
> $ cat ~/Programming/testcases/opts-bug.i
> extern unsigned long int x;
> extern float f (float);
> extern __typeof (f) f_power8;
> extern __typeof (f) f_power9;
> extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
> static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
> f_ifunc (void)
> {
>__typeof (f) *res = x ? f_power9 : f_power8;
>return res;
> }
>
> $ ./xgcc -B. ~/Programming/testcases/opts-bug.i -c -S -O2 -mlong-double-128 
> -mabi=ibmlongdouble
> /home/marxin/Programming/testcases/opts-bug.i:8:1: error: 
> ‘-mabi=ibmlongdouble’ requires ‘-mlong-double-128’
>
> This is caused by a weird option override:
>
>else if (rs6000_long_double_type_size == 128)
>  rs6000_long_double_type_size = FLOAT_PRECISION_TFmode; (it's 127)
>
> later when rs6000_option_override_internal is called for saved target flags 
> (127), it complains.
> Possible fix:
>
>else if (rs6000_long_double_type_size == 128
>|| rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
>
> 3) ARM issue reported here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20
>
>arm_fp16_inst = bitmap_bit_p (arm_active_target.isa, isa_bit_fp16);
>if (arm_fp16_inst)
>  {
>if (arm_fp16_format == ARM_FP16_FORMAT_ALTERNATIVE)
> error ("selected fp16 options are incompatible");
>arm_fp16_format = ARM_FP16_FORMAT_IEEE;
>  }
>
> there's likely missing else branch which would reset when arm_fp16_inst is 
> null.
> Anyway, can be moved again to the ignored list
>
> 4) Jeff reported the following for v850-elf:
>
> $ cat ~/Programming/testcases/j.c
> typedef __SIZE_TYPE__ size_t;
>
> extern inline __attribute__ ((__always_inline__, __gnu_inline__, 
> __artificial__, __nothrow__, __leaf__)) void *
> memcpy (void *__restrict __dest, const void *__restrict __src, size_t __len)
> {
>return __builtin___memcpy_chk (__dest, __src, __len, __builtin_object_size 
> (__dest, 0));
> }
>
> __attribute__((optimize ("Ofast"))) void
> bar (void *d, void *s, size_t l)
> {
>memcpy (d, s, l);
> }
>
> $ ./xgcc -B. ~/Programming/testcases/j.c  -S
> /home/marxin/Programming/testcases/j.c: In function ‘bar’:
> /home/marxin/Programming/testcases/j.c:4:1: error: inlining failed in call to 
> ‘always_inline’ ‘memcpy’: target specific option mismatch
>  4 | memcpy (void *__restrict __dest, const void *__restrict __src, 
> size_t __len)
>| ^~
> /home/marxin/Programming/testcases/j.c:12:3: note: called from here
> 12 |   memcpy (d, s, l);
>|   ^~~~
>
> This one is pretty clear. The target does:
>
>  { OPT_LEVELS_1_PLUS, OPT_mprolog_function, NULL, 1 },
>
> So it sets a target option based on optimize level.
> This one will need:
>
> diff 

[PATCH] diagnostics: Fix sporadic test failure

[Bernd Edlinger]

Hi,

it turns out to be reproducible this way:

COLUMNS=80 make check-gcc-c RUNTESTFLAGS="plugin.exp=diagnostic*"

Running /home/ed/gnu/gcc-trunk/gcc/testsuite/gcc.dg/plugin/plugin.exp ...
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so  1 blank line(s) in 
output
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so  expected multiline 
pattern lines 550-551 not found: "
__builtin_types_compatible_p \(long, int\) \+ f \(i\)\);.*\n
~\^~~\n"
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so (test for excess 
errors)

a lot more errors happen with COLUMNS=20.

Tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.


2021-05-28  Bernd Edlinger  

* gcc.dg/plugin/diagnostic_plugin_show_trees.c (plugin_init): Fix 
caret_max_with.
* gcc.dg/plugin/diagnostic_plugin_test_inlining.c
(plugin_init): Likewise.
* gcc.dg/plugin/diagnostic_plugin_test_paths.c (plugin_init): Likewise.
* gcc.dg/plugin/diagnostic_plugin_test_string_literals.c
(plugin_init): Likewise.
* gcc.dg/plugin/diagnostic_plugin_test_tree_expression_range.c
(plugin_init): Likewise.
From 50420cb535560ec1388d34c2d3d2a3f0d339a132 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger 
Date: Fri, 28 May 2021 14:26:02 +0200
Subject: [PATCH] diagnostics: Fix sporadic test failure

it turns out to be reproducible this way:

COLUMNS=80 make check-gcc-c RUNTESTFLAGS="plugin.exp=diagnostic*"

Running /home/ed/gnu/gcc-trunk/gcc/testsuite/gcc.dg/plugin/plugin.exp ...
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so  1 blank line(s) in output
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so  expected multiline pattern lines 550-551 not found: "__builtin_types_compatible_p \(long, int\) \+ f \(i\)\);.*\n~\^~~\n"
FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c
 -fplugin=./diagnostic_plugin_test_tree_expression_range.so (test for excess errors)

a lot more errors happen with COLUMNS=20.

2021-05-28  Bernd Edlinger  

	* gcc.dg/plugin/diagnostic_plugin_show_trees.c (plugin_init): Fix caret_max_with.
	* gcc.dg/plugin/diagnostic_plugin_test_inlining.c
	(plugin_init): Likewise.
	* gcc.dg/plugin/diagnostic_plugin_test_paths.c (plugin_init): Likewise.
	* gcc.dg/plugin/diagnostic_plugin_test_string_literals.c
	(plugin_init): Likewise.
	* gcc.dg/plugin/diagnostic_plugin_test_tree_expression_range.c
	(plugin_init): Likewise.
---
 gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c  | 2 ++
 gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_inlining.c   | 2 ++
 gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c  | 2 ++
 gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_string_literals.c| 2 ++
 .../gcc.dg/plugin/diagnostic_plugin_test_tree_expression_range.c| 2 ++
 5 files changed, 10 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c
index 71e6740..ac72503 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c
@@ -115,6 +115,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!plugin_default_version_check (version, _version))
 return 1;
 
+  global_dc->caret_max_width = 80;
+
   register_callback (plugin_name,
 		 PLUGIN_PRE_GENERICIZE,
 		 callback,
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_inlining.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_inlining.c
index 49b78cc..02c4629 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_inlining.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_inlining.c
@@ -169,6 +169,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!plugin_default_version_check (version, _version))
 return 1;
 
+  global_dc->caret_max_width = 80;
+
   pass_info.pass = new pass_test_inlining (g);
   pass_info.reference_pass_name = "*warn_function_noreturn";
   pass_info.ref_pass_instance_number = 1;
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
index 7672875..5c2da02 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
@@ -450,6 +450,8 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!plugin_default_version_check (version, _version))
 return 1;
 
+  global_dc->caret_max_width = 80;
+
   

[PATCH] ipa/100791 - copy fntype when processing __builtin_va_arg_pack

[Richard Biener]

This missing copying exposed a type mismatch in the IL.

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

2021-05-28  Richard Biener  

* tree-inline.c (copy_bb): When processing __builtin_va_arg_pack
copy fntype from original call.

* gcc.dg/pr100791.c: New testcase.
---
 gcc/testsuite/gcc.dg/pr100791.c | 9 +
 gcc/tree-inline.c   | 1 +
 2 files changed, 10 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr100791.c

diff --git a/gcc/testsuite/gcc.dg/pr100791.c b/gcc/testsuite/gcc.dg/pr100791.c
new file mode 100644
index 000..96cf34f14a4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr100791.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+static inline int __attribute__((__always_inline__))
+foo ()
+{
+  return log_bad_request(0, __builtin_va_arg_pack()); /* { dg-warning 
"implicit" } */
+}
+void log_bad_request() { foo (0); } /* { dg-warning "conflicting types" } */
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 1d13e7f5aca..d38e8617e3d 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -2100,6 +2100,7 @@ copy_bb (copy_body_data *id, basic_block bb,
 GF_CALL_VA_ARG_PACK.  */
  gimple_call_copy_flags (new_call, call_stmt);
  gimple_call_set_va_arg_pack (new_call, false);
+ gimple_call_set_fntype (new_call, gimple_call_fntype (call_stmt));
  /* location includes block.  */
  gimple_set_location (new_call, gimple_location (stmt));
  gimple_call_set_lhs (new_call, gimple_call_lhs (call_stmt));
-- 
2.26.2

[PATCH] c/100803 - diagnose invalid GIMPLE condition

[Richard Biener]

another easy fix for GIMPLE FE parser robustness.

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

2021-05-28  Richard Biener   

PR c/100803
gcc/c/
* gimple-parser.c (c_parser_gimple_paren_condition): Diagnose
invalid if conditions.

gcc/testsuite/
* gcc.dg/gimplefe-error-11.c: New testcase.
---
 gcc/c/gimple-parser.c|  8 
 gcc/testsuite/gcc.dg/gimplefe-error-11.c | 12 
 2 files changed, 20 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/gimplefe-error-11.c

diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c
index dfacf23c40a..c8d9db61f0a 100644
--- a/gcc/c/gimple-parser.c
+++ b/gcc/c/gimple-parser.c
@@ -2112,6 +2112,14 @@ c_parser_gimple_paren_condition (gimple_parser )
   if (! c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
 return error_mark_node;
   tree cond = c_parser_gimple_binary_expression (parser).value;
+  if (cond != error_mark_node
+  && ! COMPARISON_CLASS_P (cond)
+  && ! CONSTANT_CLASS_P (cond)
+  && ! SSA_VAR_P (cond))
+{
+  c_parser_error (parser, "comparison required");
+  cond = error_mark_node;
+}
   if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
 return error_mark_node;
   return cond;
diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-11.c 
b/gcc/testsuite/gcc.dg/gimplefe-error-11.c
new file mode 100644
index 000..9c29717676a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-error-11.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple" } */
+
+int bar();
+__GIMPLE
+int foo()
+{
+  if (bar()) /* { dg-error "comparison required" } */
+goto bb1;
+  else
+goto bb2;
+}
-- 
2.26.2

[PATCH] C-SKY: Define HAVE_sync_compare_and_swap*.

[Xianmiao Qu]

From: Cooper Qu 

Tested and pushed.

The SYNC operations are implemented as library functions, not
NSN patterns.  As a result, the HAVE defines for the patterns are
not defined.  We need to define them to generate the corresponding
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_* and __GCC_ATOMIC_*_LOCK_FREE
defines.

gcc/
* config/csky/csky-linux-elf.h (HAVE_sync_compare_and_swapqi):
Defined.
(HAVE_sync_compare_and_swaphi): Likewise.
(HAVE_sync_compare_and_swapsi): Likewise.
---
 gcc/config/csky/csky-linux-elf.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/config/csky/csky-linux-elf.h b/gcc/config/csky/csky-linux-elf.h
index 58a1f3978b1..e94eb8c579e 100644
--- a/gcc/config/csky/csky-linux-elf.h
+++ b/gcc/config/csky/csky-linux-elf.h
@@ -133,3 +133,13 @@
 #ifdef IN_LIBGCC2
 extern int cacheflush (void *__addr, const int __nbytes, const int __op);
 #endif
+
+/* The SYNC operations are implemented as library functions, not
+   INSN patterns.  As a result, the HAVE defines for the patterns are
+   not defined.  We need to define them to generate the corresponding
+   __GCC_HAVE_SYNC_COMPARE_AND_SWAP_* and __GCC_ATOMIC_*_LOCK_FREE
+   defines.  */
+
+#define HAVE_sync_compare_and_swapqi 1
+#define HAVE_sync_compare_and_swaphi 1
+#define HAVE_sync_compare_and_swapsi 1
-- 
2.26.2

Fallout: save/restore target options in handle_optimize_attribute

[Martin Liška]

Hi.

There's a fallout after my revision ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e. I 
would like to analyze
all case and discuss possible solution. To be honest it's a can of worms and 
reverting the commit
is an option on the table.

So the cases:

1) PR100759 - ppc64le

$ cat pr.C
#pragma GCC optimize 0
void main();

$ ./xgcc -B. -Os pr.C
pr.C:2:11: internal compiler error: ‘global_options’ are modified in local 
context
2 | void main();

What happens: we change from -Os to -O0 and rs6000_isa_flags differ in 
cl_optimization_compare.
Problem is that OPTION_MASK_SAVE_TOC_INDIRECT is set based on optimize flag:

  /* If we can shrink-wrap the TOC register save separately, then use
 -msave-toc-indirect unless explicitly disabled.  */
  if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
  && flag_shrink_wrap_separate
  && optimize_function_for_speed_p (cfun))
rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;

Suggested solution is doing:

  if ((rs6000_isa_flags_explicit & OPTION_MASK_SAVE_TOC_INDIRECT) == 0
  && flag_shrink_wrap_separate
rs6000_isa_flags |= OPTION_MASK_SAVE_TOC_INDIRECT;

and add '&& optimize_function_for_speed_p (cfun)' to the place where the option 
mask is used.

2) Joseph's case:

$ cat ~/Programming/testcases/opts-bug.i
extern unsigned long int x;
extern float f (float);
extern __typeof (f) f_power8;
extern __typeof (f) f_power9;
extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
f_ifunc (void)
{
  __typeof (f) *res = x ? f_power9 : f_power8;
  return res;
}

$ ./xgcc -B. ~/Programming/testcases/opts-bug.i -c -S -O2 -mlong-double-128 
-mabi=ibmlongdouble
/home/marxin/Programming/testcases/opts-bug.i:8:1: error: ‘-mabi=ibmlongdouble’ 
requires ‘-mlong-double-128’

This is caused by a weird option override:

  else if (rs6000_long_double_type_size == 128)
rs6000_long_double_type_size = FLOAT_PRECISION_TFmode; (it's 127)

later when rs6000_option_override_internal is called for saved target flags 
(127), it complains.
Possible fix:

  else if (rs6000_long_double_type_size == 128
   || rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)

3) ARM issue reported here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20

  arm_fp16_inst = bitmap_bit_p (arm_active_target.isa, isa_bit_fp16);
  if (arm_fp16_inst)
{
  if (arm_fp16_format == ARM_FP16_FORMAT_ALTERNATIVE)
error ("selected fp16 options are incompatible");
  arm_fp16_format = ARM_FP16_FORMAT_IEEE;
}

there's likely missing else branch which would reset when arm_fp16_inst is null.
Anyway, can be moved again to the ignored list

4) Jeff reported the following for v850-elf:

$ cat ~/Programming/testcases/j.c
typedef __SIZE_TYPE__ size_t;

extern inline __attribute__ ((__always_inline__, __gnu_inline__, 
__artificial__, __nothrow__, __leaf__)) void *
memcpy (void *__restrict __dest, const void *__restrict __src, size_t __len)
{
  return __builtin___memcpy_chk (__dest, __src, __len, __builtin_object_size 
(__dest, 0));
}

__attribute__((optimize ("Ofast"))) void
bar (void *d, void *s, size_t l)
{
  memcpy (d, s, l);
}

$ ./xgcc -B. ~/Programming/testcases/j.c  -S
/home/marxin/Programming/testcases/j.c: In function ‘bar’:
/home/marxin/Programming/testcases/j.c:4:1: error: inlining failed in call to 
‘always_inline’ ‘memcpy’: target specific option mismatch
4 | memcpy (void *__restrict __dest, const void *__restrict __src, size_t 
__len)
  | ^~
/home/marxin/Programming/testcases/j.c:12:3: note: called from here
   12 |   memcpy (d, s, l);
  |   ^~~~

This one is pretty clear. The target does:

{ OPT_LEVELS_1_PLUS, OPT_mprolog_function, NULL, 1 },

So it sets a target option based on optimize level.
This one will need:

diff --git a/gcc/config/v850/v850.c b/gcc/config/v850/v850.c
index e0e5005d865..49f91f12766 100644
--- a/gcc/config/v850/v850.c
+++ b/gcc/config/v850/v850.c
@@ -3140,6 +3140,11 @@ v850_option_override (void)
   /* The RH850 ABI does not (currently) support the use of the CALLT 
instruction.  */
   if (! TARGET_GCC_ABI)
 target_flags |= MASK_DISABLE_CALLT;
+
+  /* Save the initial options in case the user does function specific
+ options.  */
+  target_option_default_node = target_option_current_node
+= build_target_option_node (_options, _options_set);
 }

plus a custom can_inline_p target hook where the MASK_PROLOG_FUNCTION is 
ignored because
caller does not have it set, while callee has.

What target maintainers thing about it?

Martin

[committed] openmp: Fix up handling of reduction clause on constructs combined with target [PR99928]

[Jakub Jelinek via Gcc-patches]

Hi!

The reduction clause should be copied as map (tofrom: ) to combined target if
present, but as we need different handling of array sections between map and 
reduction,
doing that during gimplification would be harder.

So, this patch adds them during splitting, and similarly to firstprivate adds 
them
with a new flag that they should be just ignored/removed if an explicit map 
clause
of the same list item is present.

The exact rules are to be decided in https://github.com/OpenMP/spec/issues/2766
so this patch just implements something that is IMHO reasonable and exact 
detailed
testcases for the cornercases will follow once it is clarified.

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

2021-05-28  Jakub Jelinek  

PR middle-end/99928
gcc/
* tree.h (OMP_CLAUSE_MAP_IMPLICIT): Define.
gcc/c-family/
* c-omp.c (c_omp_split_clauses): For reduction clause if combined with
target add a map tofrom clause with OMP_CLAUSE_MAP_IMPLICIT.
gcc/c/
* c-typeck.c (handle_omp_array_sections): Copy OMP_CLAUSE_MAP_IMPLICIT.
(c_finish_omp_clauses): Move not just OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT
marked clauses last, but also OMP_CLAUSE_MAP_IMPLICIT.  Add
map_firstprivate_head bitmap, set it for GOMP_MAP_FIRSTPRIVATE_POINTER
maps and silently remove OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT if it is
present too.  For OMP_CLAUSE_MAP_IMPLICIT silently remove the clause
if present in map_head, map_field_head or map_firstprivate_head
bitmaps.
gcc/cp/
* semantics.c (handle_omp_array_sections): Copy
OMP_CLAUSE_MAP_IMPLICIT.
(finish_omp_clauses): Move not just OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT
marked clauses last, but also OMP_CLAUSE_MAP_IMPLICIT.  Add
map_firstprivate_head bitmap, set it for GOMP_MAP_FIRSTPRIVATE_POINTER
maps and silently remove OMP_CLAUSE_FIRSTPRIVATE_IMPLICIT if it is
present too.  For OMP_CLAUSE_MAP_IMPLICIT silently remove the clause
if present in map_head, map_field_head or map_firstprivate_head
bitmaps.
gcc/testsuite/
* c-c++-common/gomp/pr99928-8.c: Remove all xfails.
* c-c++-common/gomp/pr99928-9.c: Likewise.
* c-c++-common/gomp/pr99928-10.c: Likewise.
* c-c++-common/gomp/pr99928-16.c: New test.

--- gcc/tree.h.jj   2021-05-26 15:39:15.715315698 +0200
+++ gcc/tree.h  2021-05-26 16:38:26.844260892 +0200
@@ -1654,6 +1654,11 @@ class auto_suppress_location_wrappers
variable.  */
 #define OMP_CLAUSE_MAP_IN_REDUCTION(NODE) \
   TREE_PRIVATE (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP))
+/* Nonzero on map clauses added implicitly for reduction clauses on combined
+   or composite constructs.  They shall be removed if there is an explicit
+   map clause.  */
+#define OMP_CLAUSE_MAP_IMPLICIT(NODE) \
+  (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP)->base.default_def_flag)
 
 /* True on an OMP_CLAUSE_USE_DEVICE_PTR with an OpenACC 'if_present'
clause.  */
--- gcc/c-family/c-omp.c.jj 2021-05-26 15:39:15.638316779 +0200
+++ gcc/c-family/c-omp.c2021-05-26 16:38:26.845260878 +0200
@@ -1989,6 +1989,16 @@ c_omp_split_clauses (location_t loc, enu
"%, %");
  OMP_CLAUSE_REDUCTION_INSCAN (clauses) = 0;
}
+ if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_MAP)) != 0)
+   {
+ c = build_omp_clause (OMP_CLAUSE_LOCATION (clauses),
+   OMP_CLAUSE_MAP);
+ OMP_CLAUSE_DECL (c) = OMP_CLAUSE_DECL (clauses);
+ OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_TOFROM);
+ OMP_CLAUSE_MAP_IMPLICIT (c) = 1;
+ OMP_CLAUSE_CHAIN (c) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
+ cclauses[C_OMP_CLAUSE_SPLIT_TARGET] = c;
+   }
  if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_SCHEDULE)) != 0)
{
  if (code == OMP_SIMD)
--- gcc/c/c-typeck.c.jj 2021-05-21 21:16:03.081850728 +0200
+++ gcc/c/c-typeck.c2021-05-27 15:08:36.348687925 +0200
@@ -13646,6 +13646,7 @@ handle_omp_array_sections (tree c, enum
OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_ATTACH_DETACH);
   else
OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_FIRSTPRIVATE_POINTER);
+  OMP_CLAUSE_MAP_IMPLICIT (c2) = OMP_CLAUSE_MAP_IMPLICIT (c);
   if (OMP_CLAUSE_MAP_KIND (c2) != GOMP_MAP_FIRSTPRIVATE_POINTER
  && !c_mark_addressable (t))
return false;
@@ -13916,7 +13917,8 @@ tree
 c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 {
   bitmap_head generic_head, firstprivate_head, lastprivate_head;
-  bitmap_head aligned_head, map_head, map_field_head, oacc_reduction_head;
+  bitmap_head aligned_head, map_head, map_field_head, map_firstprivate_head;
+  bitmap_head oacc_reduction_head;
   tree c, t, type, *pc;
   tree simdlen = NULL_TREE, safelen = NULL_TREE;
   bool branch_seen = false;
@@ -13936,7 +13938,7 @@ c_finish_omp_clauses 

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Jakub Jelinek via Gcc-patches]

On Fri, May 28, 2021 at 09:33:14AM +0200, Bernd Edlinger wrote:
> > This should help for what I was complaining about in
> > https://gcc.gnu.org/pipermail/gcc/2021-May/235963.html . I build with
> > -j24 and it was stalling on compiling gimple-match.c for me.
> > Looks like insn-attrtab.c is missed too; I saw genattrtab was running last 
> > too.
> > 
> 
> Yeah, probably insn-automata.c as well, sometimes it is picked up early 
> sometimes not.
> maybe $(simple_generated_c) should be added to generated_files,
> but insn-attrtab.c is yet another exception.

The problem with insn-automata.c is that on some targets it is very time
consuming to generate that file (I think e.g. s390x) and by putting it into
generated_files it could actually hurt the build speed because everything
will be waiting on insn-automata.c generation for 1-2 minutes or how long
before anything else can be built.

Jakub

[PATCH] For obj-c stage-final re-use the checksum from the previous stage

[Bernd Edlinger]

Hi Richard,

I've replicated your PR to make the objective-c checksum compare equal

commit fb2647aaf55b453b37badfd40c14c59486a74584
Author: Richard Biener 
Date:   Tue May 3 08:14:27 2016 +

Make-lang.in (cc1-checksum.c): For stage-final re-use the checksum from the 
previous stage.

2016-05-03  Richard Biener  

c/
* Make-lang.in (cc1-checksum.c): For stage-final re-use
the checksum from the previous stage.

cp/
* Make-lang.in (cc1plus-checksum.c): For stage-final re-use
the checksum from the previous stage.

From-SVN: r235804


This silences the stage compare.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.


2021-05-28  Bernd Edlinger  

objc/
* Make-lang.in (cc1obj-checksum.c): For stage-final re-use
the checksum from the previous stage.

objcp/
* Make-lang.in (cc1objplus-checksum.c): For stage-final re-use
the checksum from the previous stage.
From a61184fc909634dba1dca8956ab960998114c644 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger 
Date: Fri, 28 May 2021 06:54:13 +0200
Subject: [PATCH] For obj-c stage-final re-use the checksum from the previous
 stage

This silences the stage compare.

2021-05-28  Bernd Edlinger  

	objc/
	* Make-lang.in (cc1obj-checksum.c): For stage-final re-use
	the checksum from the previous stage.

	objcp/
	* Make-lang.in (cc1objplus-checksum.c): For stage-final re-use
	the checksum from the previous stage.
---
 gcc/objc/Make-lang.in  | 14 +++---
 gcc/objcp/Make-lang.in | 15 +++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/gcc/objc/Make-lang.in b/gcc/objc/Make-lang.in
index c91148a..9011140 100644
--- a/gcc/objc/Make-lang.in
+++ b/gcc/objc/Make-lang.in
@@ -57,11 +57,19 @@ OBJC_OBJS = objc/objc-lang.o objc/objc-act.o hash-table.o \
 
 objc_OBJS = $(OBJC_OBJS) cc1obj-checksum.o
 
+# compute checksum over all object files and the options
+# re-use the checksum from the prev-final stage so it passes
+# the bootstrap comparison and allows comparing of the cc1 binary
 cc1obj-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 $(OBJC_OBJS) $(C_AND_OBJC_OBJS) $(BACKEND) $(LIBDEPS)
-	build/genchecksum$(build_exeext) $(OBJC_OBJS) $(C_AND_OBJC_OBJS) \
-$(BACKEND) $(LIBDEPS) checksum-options > cc1obj-checksum.c.tmp && \
-	$(srcdir)/../move-if-change cc1obj-checksum.c.tmp cc1obj-checksum.c
+	if [ -f ../stage_final ] \
+	   && cmp -s ../stage_current ../stage_final; then \
+	  cp ../prev-gcc/$@ $@; \
+	else \
+	  build/genchecksum$(build_exeext) $(OBJC_OBJS) $(C_AND_OBJC_OBJS) \
+		$(BACKEND) $(LIBDEPS) checksum-options > $@.tmp && \
+	  $(srcdir)/../move-if-change $@.tmp $@; \
+	fi
 
 cc1obj$(exeext): $(OBJC_OBJS) $(C_AND_OBJC_OBJS) cc1obj-checksum.o $(BACKEND) \
 		 $(LIBDEPS) $(objc.prev)
diff --git a/gcc/objcp/Make-lang.in b/gcc/objcp/Make-lang.in
index dfa4d23..3ecc50b 100644
--- a/gcc/objcp/Make-lang.in
+++ b/gcc/objcp/Make-lang.in
@@ -60,12 +60,19 @@ OBJCXX_OBJS = objcp/objcp-act.o objcp/objcp-lang.o objcp/objcp-decl.o \
 
 obj-c++_OBJS = $(OBJCXX_OBJS) cc1objplus-checksum.o
 
+# compute checksum over all object files and the options
+# re-use the checksum from the prev-final stage so it passes
+# the bootstrap comparison and allows comparing of the cc1 binary
 cc1objplus-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 	$(OBJCXX_OBJS) $(BACKEND) $(CODYLIB) $(LIBDEPS)
-	build/genchecksum$(build_exeext) $(OBJCXX_OBJS) $(BACKEND) $(CODYLIB) \
-		$(LIBDEPS) checksum-options > cc1objplus-checksum.c.tmp && \
-	$(srcdir)/../move-if-change cc1objplus-checksum.c.tmp \
-	cc1objplus-checksum.c
+	if [ -f ../stage_final ] \
+	   && cmp -s ../stage_current ../stage_final; then \
+	  cp ../prev-gcc/$@ $@; \
+	else \
+	  build/genchecksum$(build_exeext) $(OBJCXX_OBJS) $(BACKEND) \
+		$(CODYLIB) $(LIBDEPS) checksum-options > $@.tmp && \
+	  $(srcdir)/../move-if-change $@.tmp $@; \
+	fi
 
 cc1objplus$(exeext): $(OBJCXX_OBJS) cc1objplus-checksum.o $(BACKEND) \
 		 $(CODYLIB) $(LIBDEPS) $(obj-c++.prev)
-- 
1.9.1

Re: [PATCH] Add gnu::diagnose_as attribute

[Matthias Kretz]

On Friday, 28 May 2021 05:05:52 CEST Jason Merrill wrote:
> On 5/27/21 6:07 PM, Matthias Kretz wrote:
> > On Thursday, 27 May 2021 23:15:46 CEST Jason Merrill wrote:
> >> On 5/27/21 2:54 PM, Matthias Kretz wrote:
> >>> namespace Vir {
> >>> inline namespace foo {
> >>>   struct A {};
> >>> }
> >>> struct A {};
> >>> }
> >>> using Vir::A;
> >>> 
> >>> :7:12: error: reference to 'A' is ambiguous
> >>> :3:12: note: candidates are: 'struct Vir::A'
> >>> :5:10: note: 'struct Vir::A'
> >> 
> >> That doesn't seem so bad.
> > 
> > As long as you ignore the line numbers, it's a puzzling diagnostic.
> 
> Only briefly puzzling, I think; Vir::A is a valid way of referring to
> both types.

True. But that's also what lead to the error. GCC easily clears it up 
nowadays, but wouldn't anymore if inline namespaces were hidden by default.

> I'd think you could get the same effect from a hypothetical
> 
> namespace [[gnu::diagnose_as]] stdx = std::experimental;
> 
> though we'll need to add support for attributes on namespace aliases to
> the grammar.

Right, but then two of my design goals can't be met:

1. Diagnostics have an improved signal-to-noise ratio out of the box.

2. We can use replacement names that are not valid identifiers.

I don't think libstdc++ would ship with a namespace alias outside of the std 
namespace. So we'd place the "burden" of using diagnose_as correctly on our 
users. Also as a user you'd possibly have to repeat the namespace alias in 
every source file and/or place it in your applications/libraries namespace.

> >> Here it seems like you want to say "use this typedef as the true name of
> >> the type".  Is it useful to have to repeat the name?  Allowing people to
> >> use names that don't correspond to actual declarations seems unnecessary.
> > 
> > Yes, but you could also use it to apply diagnose_as to a template
> > instantiation without introducing a name for users. E.g.
> > 
> >using __only_to_apply_the_attribute [[gnu::diagnose_as("intvector")]]
> >
> >  = std::vector;
> > 
> > Now all diagnostics of 'std::vector' would print 'intvector' instead.
> 
> Yes, but why would you want to?  Making diagnostics print names that the
> user can't use in their own code seems obfuscatory, and requiring users
> to write the same names in two places seems like extra work.

I can imagine using it to make _Internal __names more readable while at the 
same time discouraging users to utter them in their own code. Sorry for the 
bad code obfuscation example above.

An example for consideration from stdx::simd:

  namespace std {
  namespace experimental {
  namespace parallelism_v2 [[gnu::diagnose_as("stdx")]] {
  namespace simd_abi [[gnu::diagnose_as("simd_abi")]] {
template 
  struct _VecBuiltin;

template 
  struct _VecBltnBtmsk;

  #if x86
using __ignore_me_0 [[gnu::diagnose_as("[SSE]")]] = _VecBuiltin<16>;
using __ignore_me_1 [[gnu::diagnose_as("[AVX]")]] = _VecBuiltin<32>;
using __ignore_me_2 [[gnu::diagnose_as("[AVX512]")]] = _VecBltnBtmsk<64>;
  #endif
  

Then diagnostics would print 'stdx::simd' instead 
of 'stdx::simd>'. (Users utter the type by 
saying e.g. 'stdx::native_simd', while compiling with AVX512 flags.)


> > But in general, I tend to agree, for type aliases there's rarely a case
> > where the names wouldn't match.
> > 
> > However, I didn't want to special-case the attribute parameters for type
> > aliases (or introduce another attribute just for this case). The attribute
> > works consistently and with the same interface independent of where it's
> > used. I tried to build a generic, broad feature instead of a narrow
> > one-problem solution.
> 
> "Treat this declaration as the name of the type/namespace it refers to
> in diagnostics" also seems consistent to me.

Sure. In general, I think

  namespace foo [[gnu::this_is_the_name_I_want]] = bar;
  using foo [[gnu::this_is_the_name_I_want]] = bar;

is not a terribly bad idea on its own. But it's not the solution for the 
problems I set out to solve.

> Still, perhaps it would be better to store these aliases in a separate hash
> table instead of *_ATTRIBUTES.

Maybe. For performance reasons or for simplification of the implementation? 
What entity could I use for hashing? The identifier alone wouldn't suffice 
since different instantiations of the same class template can have different 
diagnose_as values (e.g. std::string, std::wstring, ...).

-- 
──
 Dr. Matthias Kretz   https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research   https://gsi.de
 std::experimental::simd  https://github.com/VcDevel/std-simd
──

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Bernd Edlinger]

On 5/28/21 9:26 AM, Andrew Pinski wrote:
> On Thu, May 27, 2021 at 10:10 PM Bernd Edlinger
>  wrote:
>>
>> Hi,
>>
>> I was wondering, why gimple-match.c and generic-match.c
>> are not built early but always last, which slows down parallel
>> makes significantly.
>>
>> The reason seems to be that generated_files does not
>> mention gimple-match.c and generic-match.c.
>>
>> This comment in Makefile.in says it all:
>>
>> $(ALL_HOST_OBJS) : | $(generated_files)
>>
>> So this patch adds gimple-match.c generic-match.c to generated_files.
>>
>>
>> Tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
> 
> This should help for what I was complaining about in
> https://gcc.gnu.org/pipermail/gcc/2021-May/235963.html . I build with
> -j24 and it was stalling on compiling gimple-match.c for me.
> Looks like insn-attrtab.c is missed too; I saw genattrtab was running last 
> too.
> 

Yeah, probably insn-automata.c as well, sometimes it is picked up early 
sometimes not.
maybe $(simple_generated_c) should be added to generated_files,
but insn-attrtab.c is yet another exception.


Bernd.

> Thanks,
> Andrew
> 
>>
>>
>> Thanks
>> Bernd.
>>
>>
>> 2021-05-28  Bernd Edlinger  
>>
>> * Makefile.in (generated_files): Add gimple-match.c and
>> generic-match.c

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Andrew Pinski via Gcc-patches]

On Thu, May 27, 2021 at 10:10 PM Bernd Edlinger
 wrote:
>
> Hi,
>
> I was wondering, why gimple-match.c and generic-match.c
> are not built early but always last, which slows down parallel
> makes significantly.
>
> The reason seems to be that generated_files does not
> mention gimple-match.c and generic-match.c.
>
> This comment in Makefile.in says it all:
>
> $(ALL_HOST_OBJS) : | $(generated_files)
>
> So this patch adds gimple-match.c generic-match.c to generated_files.
>
>
> Tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?

This should help for what I was complaining about in
https://gcc.gnu.org/pipermail/gcc/2021-May/235963.html . I build with
-j24 and it was stalling on compiling gimple-match.c for me.
Looks like insn-attrtab.c is missed too; I saw genattrtab was running last too.

Thanks,
Andrew

>
>
> Thanks
> Bernd.
>
>
> 2021-05-28  Bernd Edlinger  
>
> * Makefile.in (generated_files): Add gimple-match.c and
> generic-match.c

Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

[Richard Biener via Gcc-patches]

On Fri, May 28, 2021 at 7:21 AM Jeff Law via Gcc-patches
 wrote:
>
>
>
> On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote:
> > On 5/26/21 7:07 AM, Andrew Pinski wrote:
> >> On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:
> >>> On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
> >>>  wrote:
>  On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:
> > On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
> >  wrote:
> >> From: Andrew Pinski 
> >>
> >> Instead of some of the more manual optimizations inside phi-opt,
> >> it would be good idea to do a lot of the heavy lifting inside match
> >> and simplify instead. In the process, this moves the three simple
> >> A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.
> >>
> >> OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.
> >>
> >> Differences from V1:
> >> * Use bit_xor 1 instead of bit_not to fix the problem with
> >> boolean types
> >> which are not 1 bit precision.
> > OK.
> >
> > Thanks,
> > Richard.
> >
>  Hmm, sorry, no luck.
> 
>  I think this caused:
> >>> If anything it is a bad interaction with changes between r12-1046 and
> >>> r12-1053; I am suspecting a bug in those changes rather than my
> >>> changes causing the bug.  Debugging it right now.
> >> (gdb) p debug_tree(name)
> >>>>  type  >>  size 
> >>  unit-size 
> >>  align:8 warn_if_not_align:0 symtab:0 alias-set -1
> >> canonical-type 0x76b45b28 precision:1 min  >> 0x76b4a030 0> max >
> >>
> >>  def_stmt _19 = ~_8;
> >>  version:19>
> >>
> >> So what is happening is evrp converted:
> >> ct_12 = ct_5 + -1;
> >> Into
> >> ct_12 = ct_5 == 1 ? 0 : 1;
> >> (this was done before my patch)
> >> And then it gets simplified to:
> >>_8 = ct_5 == 1;
> >>_19 = ~_8;
> >>ct_12 = (int) _19;
> >> (after my match.pd patch)
> Yup.  I've chased this kind of thing down repeatedly through the years.
> It's rare, but some transformations from match.pd create new SSA_NAMEs
> and the various passes need to be prepared to handle that.

You can suppress that at the expense of some missed simplifications.
Using fold_stmt_inplace will, for example, or when passing a NULL seq
arguments to the lower-level functions.

Richard.

> Jeff
>

Re: [PATCH RFA] tree-iterator: C++11 range-for and tree_stmt_iterator

[Richard Biener via Gcc-patches]

On Wed, May 26, 2021 at 3:56 PM Jason Merrill  wrote:
>
> Ping.

The non-C++ parts are OK.

Richard.

> On 5/17/21 1:58 PM, Jason Merrill wrote:
> > On 5/17/21 3:56 AM, Richard Biener wrote:
> >> On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches
> >>  wrote:
> >>>
> >>> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:
>  Ping.
> 
>  On 5/1/21 12:29 PM, Jason Merrill wrote:
> > Like my recent patch to add ovl_range and lkp_range in the C++
> > front end,
> > this patch adds the tsi_range adaptor for using C++11 range-based
> > 'for' with
> > a STATEMENT_LIST, e.g.
> >
> > for (tree stmt : tsi_range (stmt_list)) { ... }
> >
> > This also involves adding some operators to tree_stmt_iterator that
> > are
> > needed for range-for iterators, and should also be useful in code that
> > uses
> > the iterators directly.
> >
> > The patch updates the suitable loops in the C++ front end, but does
> > not
> > touch any loops elsewhere in the compiler.
> >>>
> >>> I like the modernization of the loops.
> >>
> >> The only worry I have (and why I stopped looking at range-for) is that
> >> this adds another style of looping over stmts without opening the
> >> possibility to remove another or even unify all of them.  That's because
> >> range-for isn't powerful enough w/o jumping through hoops and/or
> >> we cannot use what appearantly ranges<> was intended for (fix
> >> this limitation).
> >
> > The range-for enabled by my patch simplifies the common case of simple
> > iteration over elements; that seems worth doing to me even if it doesn't
> > replace all loops.  Just as FOR_EACH_VEC_ELT isn't suitable for all
> > loops over a vector.
> >
> >> That said, if some C++ literate could see if for example
> >> what gimple-iterator.h provides can be completely modernized
> >> then that would be great of course.
> >>
> >> There's stuff like reverse iteration
> >
> > This is typically done with the reverse_iterator<> adaptor, which we
> > could get from  or duplicate.  I didn't see enough reverse
> > iterations to make it seem worth the bother.
> >
> >> iteration skipping debug stmts,
> >
> > There you can move the condition into the loop:
> >
> > if (gimple_code (stmt) == GIMPLE_DEBUG)
> >   continue;
> >
> >> compares of iterators like gsi_one_before_end_p, etc.
> >
> > Certainly anything where you want to mess with the iterators directly
> > doesn't really translate to range-for.
> >
> >> Given my failed tries (but I'm a C++ illiterate) my TODO list now
> >> only contains turning the iterators into STL style ones, thus
> >> gsi_stmt (it) -> *it, gsi_next () -> ++it, etc. - but even
> >> it != end_p looks a bit awkward there.
> >
> > Well, it < end_val is pretty typical for loops involving integer
> > iterators.  But you don't have to use that style if you'd rather not.
> > You could continue to use gsi_end_p, or just *it, since we know that *it
> > is NULL at the end of the sequence.
> >
> >>> I can't find anything terribly wrong with the iterator but let me
> >>> at least pick on some nits ;)
> >>>
> >
> > gcc/ChangeLog:
> >
> >  * tree-iterator.h (struct tree_stmt_iterator): Add operator++,
> >  operator--, operator*, operator==, and operator!=.
> >  (class tsi_range): New.
> >
> > gcc/cp/ChangeLog:
> >
> >  * constexpr.c (build_data_member_initialization): Use tsi_range.
> >  (build_constexpr_constructor_member_initializers): Likewise.
> >  (constexpr_fn_retval, cxx_eval_statement_list): Likewise.
> >  (potential_constant_expression_1): Likewise.
> >  * coroutines.cc (await_statement_expander): Likewise.
> >  (await_statement_walker): Likewise.
> >  * module.cc (trees_out::core_vals): Likewise.
> >  * pt.c (tsubst_expr): Likewise.
> >  * semantics.c (set_cleanup_locs): Likewise.
> > ---
> >gcc/tree-iterator.h  | 28 +++-
> >gcc/cp/constexpr.c   | 42
> > ++
> >gcc/cp/coroutines.cc | 10 --
> >gcc/cp/module.cc |  5 ++---
> >gcc/cp/pt.c  |  5 ++---
> >gcc/cp/semantics.c   |  5 ++---
> >6 files changed, 47 insertions(+), 48 deletions(-)
> >
> > diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h
> > index 076fff8644c..f57456bb473 100644
> > --- a/gcc/tree-iterator.h
> > +++ b/gcc/tree-iterator.h
> > @@ -1,4 +1,4 @@
> > -/* Iterator routines for manipulating GENERIC tree statement list.
> > +/* Iterator routines for manipulating GENERIC tree statement list.
> > -*- C++ -*-
> >   Copyright (C) 2003-2021 Free Software Foundation, Inc.
> >   Contributed by Andrew MacLeod  
> > @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see
> >struct tree_stmt_iterator {
> >  struct 

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Bernd Edlinger]

On 5/28/21 8:41 AM, Richard Biener wrote:
> On Fri, 28 May 2021, Bernd Edlinger wrote:
> 
>>
>>
>> On 5/28/21 6:42 AM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> I was wondering, why gimple-match.c and generic-match.c
>>> are not built early but always last, which slows down parallel
>>> makes significantly.
>>>
>>> The reason seems to be that generated_files does not
>>> mention gimple-match.c and generic-match.c.
>>>
>>> This comment in Makefile.in says it all:
>>>
>>
>> Oh, dear, git commit did eliminate the comments
>> starting with "#"
>> the mentined comment is
>>
>> # Dependency information.
>>
>> # In order for parallel make to really start compiling the expensive
>> # objects from $(OBJS) as early as possible, build all their
>> # prerequisites strictly before all objects.
>>
>>> $(ALL_HOST_OBJS) : | $(generated_files)
>>>
>>> So this patch adds gimple-match.c generic-match.c to generated_files.
>>>
>>>
>>> Tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
> 
> OK.  Does it really help though?
> 

Yes, I guess so, at least a little bit.

Prior to this patch the whole build stage was completed for everything
then those two big files got generated,
and then there are only two large files compiled in parallel for
several minutes at least.

So a make -j8 utilizes only 25 % cpu power
and make -j16 only 12.5 % utilization.
That can certainly be a bit annoying.


Bernd.


> Thanks,
> Richard.
> 
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>> 2021-05-28  Bernd Edlinger  
>>>
>>> * Makefile.in (generated_files): Add gimple-match.c and
>>> generic-match.c
>>>
>>
> 

Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

[Richard Biener]

On Fri, 28 May 2021, Bernd Edlinger wrote:

> 
> 
> On 5/28/21 6:42 AM, Bernd Edlinger wrote:
> > Hi,
> > 
> > I was wondering, why gimple-match.c and generic-match.c
> > are not built early but always last, which slows down parallel
> > makes significantly.
> > 
> > The reason seems to be that generated_files does not
> > mention gimple-match.c and generic-match.c.
> > 
> > This comment in Makefile.in says it all:
> > 
> 
> Oh, dear, git commit did eliminate the comments
> starting with "#"
> the mentined comment is
> 
> # Dependency information.
> 
> # In order for parallel make to really start compiling the expensive
> # objects from $(OBJS) as early as possible, build all their
> # prerequisites strictly before all objects.
> 
> > $(ALL_HOST_OBJS) : | $(generated_files)
> > 
> > So this patch adds gimple-match.c generic-match.c to generated_files.
> > 
> > 
> > Tested on x86_64-pc-linux-gnu.
> > Is it OK for trunk?

OK.  Does it really help though?

Thanks,
Richard.

> > 
> > 
> > Thanks
> > Bernd.
> > 
> > 
> > 2021-05-28  Bernd Edlinger  
> > 
> > * Makefile.in (generated_files): Add gimple-match.c and
> > generic-match.c
> > 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH v2] forwprop: Support vec perm fed by CTOR and CTOR/CST [PR99398]

[Kewen.Lin via Gcc-patches]

on 2021/5/27 下午8:55, Richard Sandiford wrote:
> Sorry for the slow reponse.
> 
> "Kewen.Lin"  writes:
>> diff --git a/gcc/vec-perm-indices.c b/gcc/vec-perm-indices.c
>> index ede590dc5c9..57dd11d723c 100644
>> --- a/gcc/vec-perm-indices.c
>> +++ b/gcc/vec-perm-indices.c
>> @@ -101,6 +101,70 @@ vec_perm_indices::new_expanded_vector (const 
>> vec_perm_indices ,
>>m_encoding.finalize ();
>>  }
>>  
>> +/* Check whether we can switch to a new permutation vector that
>> +   selects the same input elements as ORIG, but with each element
>> +   built up from FACTOR pieces.  Return true if yes, otherwise
>> +   return false.  Every FACTOR permutation indexes should be
>> +   continuous separately and the first one of each batch should
>> +   be able to exactly modulo FACTOR.  For example, if ORIG is
>> +   { 2, 3, 4, 5, 0, 1, 6, 7 } and FACTOR is 2, the new permutation
>> +   is { 1, 2, 0, 3 }.  */
>> +
>> +bool
>> +vec_perm_indices::new_shrunk_vector (const vec_perm_indices ,
>> + unsigned int factor)
>> +{
>> +  gcc_assert (factor > 0);
>> +
>> +  if (maybe_lt (orig.m_nelts_per_input, factor))
>> +return false;
>> +
>> +  poly_uint64 nelts;
>> +  /* Invalid if vector units number isn't multiple of factor.  */
>> +  if (!multiple_p (orig.m_nelts_per_input, factor, ))
>> +return false;
>> +
>> +  /* Only handle the case that npatterns is multiple of factor.
>> + FIXME: Try to see whether we can reshape it by factor npatterns.  */
>> +  if (orig.m_encoding.npatterns () % factor != 0)
>> +return false;
>> +
>> +  unsigned int encoded_nelts = orig.m_encoding.encoded_nelts ();
>> +  auto_vec encodings (encoded_nelts);
> 
> auto_vec would avoid memory allocations in the
> same cases that m_encoding can.  “encoding” might be better than
> “encodings” since there's only really one encoding here.
> 
>> +  /* Separate all encoded elements into batches by size factor,
>> + then ensure the first element of each batch is multiple of
>> + factor and all elements in each batch is consecutive from
>> + the first one.  */
>> +  for (unsigned int i = 0; i < encoded_nelts; i += factor)
>> +{
>> +  element_type first = orig.m_encoding[i];
>> +  element_type new_index;
>> +  if (!multiple_p (first, factor, _index))
>> +return false;
>> +  for (unsigned int j = 1; j < factor; ++j)
>> +{
>> +  if (maybe_ne (first + j, orig.m_encoding[i + j]))
>> +return false;
>> +}
> 
> Formatting nit: unnecessary braces around if.
> 
>> +  encodings.quick_push (new_index);
>> +}
>> +
>> +  m_ninputs = orig.m_ninputs;
>> +  m_nelts_per_input = nelts;
>> +  poly_uint64 full_nelts = exact_div (orig.m_encoding.full_nelts (), 
>> factor);
>> +  unsigned int npatterns = orig.m_encoding.npatterns () / factor;
>> +
>> +  m_encoding.new_vector (full_nelts, npatterns,
>> + orig.m_encoding.nelts_per_pattern ());
>> +
>> +  for (unsigned int i = 0; i < encodings.length (); i++)
>> +m_encoding.quick_push (encodings[i]);
> 
> I think this can be:
> 
>m_encoding.splice (encodings);
> 
> OK with those changes, thanks.  Thanks also for doing it in a
> variable-length-friendly way.
> 


Thanks for the comments, Richard!  The patch was updated as them,
re-tested and committed in r12-1103.

BR,
Kewen