[i386] Do not omit the frame pointer at -O0

[Eric Botcazou]

Ping for https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01228.html

Thanks in advance.

-- 
Eric Botcazou

Re: [i386] Do not omit the frame pointer at -O0

[Eric Botcazou]

> LGTM, but please note that the patch was already approved by Jeff on
> 22th of June [1].

Sorry, I missed that...  Thanks for pointing it out.

-- 
Eric Botcazou

Fix old thinko in choose_multiplier

[Eric Botcazou]

As spotted by the reporter of the bug, there is a small thinko at the end of 
choose_multiplier whereby the (N + 1)th bit of the result is set when the 
computed value is exactly 2**N.  But it turns out that this case can never 
actually happen given how the function is invoked in the compiler.

Bootstrapped/regtested on x86-64/Linux, applied on the mainline as obvious.


2018-07-04  Eric Botcazou  

PR middle-end/86380
* expmed.c (choose_multiplier): Fix incorrect comparison with mask.

-- 
Eric BotcazouIndex: expmed.c
===
--- expmed.c	(revision 262339)
+++ expmed.c	(working copy)
@@ -3678,7 +3678,7 @@ choose_multiplier (unsigned HOST_WIDE_IN
 {
   unsigned HOST_WIDE_INT mask = (HOST_WIDE_INT_1U << n) - 1;
   *multiplier_ptr = mhigh.to_uhwi () & mask;
-  return mhigh.to_uhwi () >= mask;
+  return mhigh.to_uhwi () > mask;
 }
   else
 {

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Eric Botcazou]

> The result is good enough to bootstrap natively and seems to give reasonable
> native testsuite results for a first attempt.  The machine I'm running on
> has broken icache flushing, so trampolines won't work, and I suspect that
> is causing a lot of the testsuite failures.

Ada doesn't use trampolines if you define...

> +   Always_Compatible_Rep : constant Boolean := False;

...this to False.

-- 
Eric Botcazou

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Eric Botcazou]

> Ada doesn't use trampolines if you define...
> 
> > +   Always_Compatible_Rep : constant Boolean := False;
> 
> ...this to False.

And also define TARGET_CUSTOM_FUNCTION_DESCRIPTORS for the architecture.

-- 
Eric Botcazou

[Ada] Optimize calls to pure functions with by-ref In parameter

[Eric Botcazou]

With GNAT you can declare a function as pure with a dedicated pragma, even if 
it takes a parameter passed by reference.  In this case, if the parameter is 
declared as In (the default), the language additionally guarantees that it is 
not modified, thus making the function also pure in the GCC sense.

Tested on x86-64/Linux, applied on the mainline.


2018-07-07  Eric Botcazou  

* gcc-interface/decl.c (gnat_to_gnu_param): Minor tweak.
(gnat_to_gnu_subprog_type): New pure_flag local variable.  Set it for
a pure Ada function with a by-ref In parameter.  Propagate it onto the
function type by means of the TYPE_QUAL_RESTRICT flag.
* gcc-interface/utils.c (finish_subprog_decl): Set DECL_PURE_P if the
function type has the TYPE_QUAL_RESTRICT flag set.


2018-07-07  Eric Botcazou  

* gnat.dg/pure_function3a.adb: New test.
* gnat.dg/pure_function3b.adb: Likewise.
* gnat.dg/pure_function3c.adb: Likewise.
* gnat.dg/pure_function3_pkg.ads: New helper.

-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 262468)
+++ gcc-interface/decl.c	(working copy)
@@ -5228,7 +5228,6 @@ gnat_to_gnu_param (Entity_Id gnat_param,
 	 && TYPE_MULTI_ARRAY_P (TREE_TYPE (gnu_param_type)))
 	gnu_param_type = TREE_TYPE (gnu_param_type);
 
-  by_component_ptr = true;
   gnu_param_type = TREE_TYPE (gnu_param_type);
 
   if (ro_param)
@@ -5236,6 +5235,7 @@ gnat_to_gnu_param (Entity_Id gnat_param,
 	  = change_qualified_type (gnu_param_type, TYPE_QUAL_CONST);
 
   gnu_param_type = build_pointer_type (gnu_param_type);
+  by_component_ptr = true;
 }
 
   /* Fat pointers are passed as thin pointers for foreign conventions.  */
@@ -5561,14 +5561,15 @@ gnat_to_gnu_subprog_type (Entity_Id gnat
   /* Fields in return type of procedure with copy-in copy-out parameters.  */
   tree gnu_field_list = NULL_TREE;
   /* The semantics of "pure" in Ada essentially matches that of "const"
- in the back-end.  In particular, both properties are orthogonal to
- the "nothrow" property if the EH circuitry is explicit in the
- internal representation of the back-end.  If we are to completely
+ or "pure" in GCC.  In particular, both properties are orthogonal
+ to the "nothrow" property if the EH circuitry is explicit in the
+ internal representation of the middle-end.  If we are to completely
  hide the EH circuitry from it, we need to declare that calls to pure
  Ada subprograms that can throw have side effects since they can
- trigger an "abnormal" transfer of control flow; thus they can be
- neither "const" nor "pure" in the back-end sense.  */
+ trigger an "abnormal" transfer of control flow; therefore, they can
+ be neither "const" nor "pure" in the GCC sense.  */
   bool const_flag = (Back_End_Exceptions () && Is_Pure (gnat_subprog));
+  bool pure_flag = false;
   bool return_by_direct_ref_p = false;
   bool return_by_invisi_ref_p = false;
   bool return_unconstrained_p = false;
@@ -5849,13 +5850,19 @@ gnat_to_gnu_subprog_type (Entity_Id gnat
 	  gnu_param_list = chainon (gnu_param, gnu_param_list);
 	  save_gnu_tree (gnat_param, gnu_param, false);
 
-	  /* If a parameter is a pointer, a function may modify memory through
-	 it and thus shouldn't be considered a const function.   Also, the
-	 memory may be modified between two calls, so they can't be CSE'ed.
-	 The latter case also handles by-ref parameters.  */
-	  if (POINTER_TYPE_P (gnu_param_type)
-	  || TYPE_IS_FAT_POINTER_P (gnu_param_type))
-	const_flag = false;
+	  /* A pure function in the Ada sense which takes an access parameter
+	 may modify memory through it and thus need be considered neither
+	 const nor pure in the GCC sense.  Likewise it if takes a by-ref
+	 In Out or Out parameter.  But if it takes a by-ref In parameter,
+	 then it may only read memory through it and can be considered
+	 pure in the GCC sense.  */
+	  if ((const_flag || pure_flag)
+	  && (POINTER_TYPE_P (gnu_param_type)
+		  || TYPE_IS_FAT_POINTER_P (gnu_param_type)))
+	{
+	  const_flag = false;
+	  pure_flag = DECL_POINTS_TO_READONLY_P (gnu_param);
+	}
 	}
 
   /* If the parameter uses the copy-in copy-out mechanism, allocate a field
@@ -6007,6 +6014,9 @@ gnat_to_gnu_subprog_type (Entity_Id gnat
   if (const_flag)
 	gnu_type = change_qualified_type (gnu_type, TYPE_QUAL_CONST);
 
+  if (pure_flag)
+	gnu_type = change_qualified_type (gnu_type, TYPE_QUAL_RESTRICT);
+
   if (No_Return (gnat_subprog))
 	gnu_type = change_qualified_type (gnu_type, TYPE_QUAL_VOLATILE);
 
Index: gcc-interface/utils.c
==

[Ada] Do not generate debug info for actual subtypes

[Eric Botcazou]

These actual subtypes are artificial subtypes generated for parameters whose 
nominal subtype is an unconstrained array type in order to expose the bounds.

There is no point in generating debug info for them so avoid doing it now.

Tested on x86-64/Linux, applied on the mainline.


2018-07-07  Eric Botcazou  

* gcc-interface/gigi.h (add_decl_expr): Adjust prototype.
* gcc-interface/decl.c (gnat_to_gnu_entity): Remove useless test.
* gcc-interface/trans.c (add_stmt_with_node): Remove exceptions.
(add_decl_expr): Change type of second parameter and rename it.
(renaming_from_instantiation_p): New function moved from...
(set_expr_location_from_node): Test for exceptions here and add one
for actual subtypes built for unconstrained composite actuals.
* gcc-interface/utils.c (renaming_from_instantiation_p): ...here.

-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 262495)
+++ gcc-interface/decl.c	(working copy)
@@ -430,11 +430,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	  || Is_Public (gnat_entity));
 
   /* Get the name of the entity and set up the line number and filename of
- the original definition for use in any decl we make.  Make sure we do not
- inherit another source location.  */
+ the original definition for use in any decl we make.  Make sure we do
+ not inherit another source location.  */
   gnu_entity_name = get_entity_name (gnat_entity);
-  if (Sloc (gnat_entity) != No_Location
-  && !renaming_from_instantiation_p (gnat_entity))
+  if (!renaming_from_instantiation_p (gnat_entity))
 Sloc_to_locus (Sloc (gnat_entity), &input_location);
 
   /* For cases when we are not defining (i.e., we are referencing from
Index: gcc-interface/gigi.h
===
--- gcc-interface/gigi.h	(revision 262468)
+++ gcc-interface/gigi.h	(working copy)
@@ -77,9 +77,9 @@ extern tree end_stmt_group (void);
 /* Set the BLOCK node corresponding to the current code group to GNU_BLOCK.  */
 extern void set_block_for_group (tree);
 
-/* Add a declaration statement for GNU_DECL to the current BLOCK_STMT node.
-   Get SLOC from GNAT_ENTITY.  */
-extern void add_decl_expr (tree gnu_decl, Entity_Id gnat_entity);
+/* Add a declaration statement for GNU_DECL to the current statement group.
+   Get the SLOC to be put onto the statement from GNAT_NODE.  */
+extern void add_decl_expr (tree gnu_decl, Node_Id gnat_node);
 
 /* Mark nodes rooted at T with TREE_VISITED and types as having their
sized gimplified.  We use this to indicate all variable sizes and
Index: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 262496)
+++ gcc-interface/trans.c	(working copy)
@@ -8119,9 +8119,7 @@ add_stmt_force (tree gnu_stmt)
 void
 add_stmt_with_node (tree gnu_stmt, Node_Id gnat_node)
 {
-  /* Do not emit a location for renamings that come from generic instantiation,
- they are likely to disturb debugging.  */
-  if (Present (gnat_node) && !renaming_from_instantiation_p (gnat_node))
+  if (Present (gnat_node))
 set_expr_location_from_node (gnu_stmt, gnat_node);
   add_stmt (gnu_stmt);
 }
@@ -8137,10 +8135,10 @@ add_stmt_with_node_force (tree gnu_stmt,
 }
 
 /* Add a declaration statement for GNU_DECL to the current statement group.
-   Get SLOC from Entity_Id.  */
+   Get the SLOC to be put onto the statement from GNAT_NODE.  */
 
 void
-add_decl_expr (tree gnu_decl, Entity_Id gnat_entity)
+add_decl_expr (tree gnu_decl, Node_Id gnat_node)
 {
   tree type = TREE_TYPE (gnu_decl);
   tree gnu_stmt, gnu_init;
@@ -8179,7 +8177,7 @@ add_decl_expr (tree gnu_decl, Entity_Id
 	MARK_VISITED (TYPE_ADA_SIZE (type));
 }
   else
-add_stmt_with_node (gnu_stmt, gnat_entity);
+add_stmt_with_node (gnu_stmt, gnat_node);
 
   /* If this is a variable and an initializer is attached to it, it must be
  valid for the context.  Similar to init_const in create_var_decl.  */
@@ -8203,7 +8201,7 @@ add_decl_expr (tree gnu_decl, Entity_Id
 	gnu_decl = convert (TREE_TYPE (TYPE_FIELDS (type)), gnu_decl);
 
   gnu_stmt = build_binary_op (INIT_EXPR, NULL_TREE, gnu_decl, gnu_init);
-  add_stmt_with_node (gnu_stmt, gnat_entity);
+  add_stmt_with_node (gnu_stmt, gnat_node);
 }
 }
 
@@ -10005,6 +10003,32 @@ Sloc_to_locus (Source_Ptr Sloc, location
   return true;
 }
 
+/* Return whether GNAT_NODE is a defining identifier for a renaming that comes
+   from the parameter association for the instantiation of a generic.  We do
+   not want to emit source location for them: the code generated for their
+   initialization is likely to disturb debugging.  */
+
+bool
+renaming_from_instantiation_p (Node_Id gnat_node)
+{
+  if (Nkind (gnat_node) != N_Defining_Identifier
+  || !

[Ada] Reduce -Wstack-usage false positives on variant records

[Eric Botcazou]

This reduces the number of false positives of -Wstack-usage in the presence of 
variables whose nominal subtype is a discriminated record with a variant part.

Tested on x86-64/Linux, applied on the mainline.


2018-07-07  Eric Botcazou  

* gcc-interface/decl.c (gnat_to_gnu_entity): Add GNAT_DECL local
variable and use it throughout.
: If the nominal subtype of the object is unconstrained,
compute the Ada size separately and put in on the padding type if the
size is not fixed.
: Minor tweak.
* gcc-interface/misc.c (gnat_type_max_size): Rename max_size_unit
into max_size_unit throughout.


2018-07-07  Eric Botcazou  

* gnat.dg/stack_usage6.adb: New test.
* gnat.dg/stack_usage6_pkg.ads: New helper.

-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 262497)
+++ gcc-interface/decl.c	(working copy)
@@ -273,7 +273,9 @@ static bool intrin_profiles_compatible_p
 tree
 gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 {
-  /* Contains the kind of the input GNAT node.  */
+  /* The construct that declared the entity.  */
+  const Node_Id gnat_decl = Declaration_Node (gnat_entity);
+  /* The kind of the entity.  */
   const Entity_Kind kind = Ekind (gnat_entity);
   /* True if this is a type.  */
   const bool is_type = IN (kind, Type_Kind);
@@ -578,7 +580,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
   if (definition
 	  && !gnu_expr
 	  && No (Address_Clause (gnat_entity))
-	  && !No_Initialization (Declaration_Node (gnat_entity))
+	  && !No_Initialization (gnat_decl)
 	  && No (Renamed_Object (gnat_entity)))
 	{
 	  gnu_decl = error_mark_node;
@@ -611,9 +613,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	 may contain N_Expression_With_Actions nodes and thus declarations of
 	 objects from other units that we need to discard.  */
   if (!definition
-	  && !No_Initialization (Declaration_Node (gnat_entity))
+	  && !No_Initialization (gnat_decl)
 	  && !Is_Dispatch_Table_Entity (gnat_entity)
-	  && Present (gnat_temp = Expression (Declaration_Node (gnat_entity)))
+	  && Present (gnat_temp = Expression (gnat_decl))
 	  && Nkind (gnat_temp) != N_Allocator
 	  && (!type_annotate_only || Compile_Time_Known_Value (gnat_temp)))
 	gnu_expr = gnat_to_gnu_external (gnat_temp);
@@ -634,9 +636,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	 && !(kind == E_Variable
 		  && Present (Linker_Section_Pragma (gnat_entity)))
 	 && !Treat_As_Volatile (gnat_entity)
-	 && (((Nkind (Declaration_Node (gnat_entity))
-		   == N_Object_Declaration)
-		  && Present (Expression (Declaration_Node (gnat_entity
+	 && (((Nkind (gnat_decl) == N_Object_Declaration)
+		  && Present (Expression (gnat_decl)))
 		 || Present (Renamed_Object (gnat_entity))
 		 || imported_p));
 	bool inner_const_flag = const_flag;
@@ -650,7 +651,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	bool used_by_ref = false;
 	tree gnu_ext_name = NULL_TREE;
 	tree renamed_obj = NULL_TREE;
-	tree gnu_object_size;
+	tree gnu_ada_size = NULL_TREE;
 
 	/* We need to translate the renamed object even though we are only
 	   referencing the renaming.  But it may contain a call for which
@@ -755,8 +756,9 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	  {
 	if (gnu_expr && kind == E_Constant)
 	  {
-		tree size = TYPE_SIZE (TREE_TYPE (gnu_expr));
-		if (CONTAINS_PLACEHOLDER_P (size))
+		gnu_size = TYPE_SIZE (TREE_TYPE (gnu_expr));
+		gnu_ada_size = TYPE_ADA_SIZE (TREE_TYPE (gnu_expr));
+		if (CONTAINS_PLACEHOLDER_P (gnu_size))
 		  {
 		/* If the initializing expression is itself a constant,
 		   despite having a nominal type with self-referential
@@ -768,27 +770,38 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 			&& (TREE_READONLY (TREE_OPERAND (gnu_expr, 0))
 			|| DECL_READONLY_ONCE_ELAB
 			   (TREE_OPERAND (gnu_expr, 0
-		  gnu_size = DECL_SIZE (TREE_OPERAND (gnu_expr, 0));
+		  {
+			gnu_size = DECL_SIZE (TREE_OPERAND (gnu_expr, 0));
+			gnu_ada_size = gnu_size;
+		  }
 		else
-		  gnu_size
-			= SUBSTITUTE_PLACEHOLDER_IN_EXPR (size, gnu_expr);
+		  {
+			gnu_size
+			  = SUBSTITUTE_PLACEHOLDER_IN_EXPR (gnu_size,
+			gnu_expr);
+			gnu_ada_size
+			  = SUBSTITUTE_PLACEHOLDER_IN_EXPR (gnu_ada_size,
+			gnu_expr);
+		  }
 		  }
-		else
-		  gnu_size = size;
 	  }
 	/* We may have no GNU_EXPR because No_Initialization is
 	   set even though there's an Expression.  */
 	else if (kind == E_Constant
-		 && (Nkind (Declaration_Node (gnat_entity))
-			 == N_Object_Declaration)
-		 && Present (Expression (Declaration_Node (gnat_entity
-	  gnu_size
-		= TYPE_SIZ

[c-family] Swich -fdump-ada-spec output for Ada 2012

[Eric Botcazou]

The aspect syntax introduced in Ada 2012 makes it much easier to support 
function overloading in particular, so the patch removes a lot of lines:

 c-ada-spec.c |  322 ++---
 1 file changed, 105 insertions(+), 217 deletions(-)

Tested on x86-64/Linux, applied on the mainline.


2018-07-07  Eric Botcazou  

* c-ada-spec.c (to_ada_name): Remove index parameter.
(pp_ada_tree_identifier): Likewise.
(dump_ada_macros): Adjust call to to_ada_name.
(struct overloaded_name_hash): Delete.
(struct overloaded_name_hasher): Likewise.
(overloaded_names): Likewise.
(compute_overloading_index): Likewise.
(dump_ada_decl_name): Do not call compute_overloading_index and
adjust calls to pp_ada_tree_identifier.
(dump_ada_double_name): Adjust calls to pp_ada_tree_identifier.
(dump_ada_import): Add spc parameter and switch to aspect syntax.
(dump_ada_function_declaration): Adjust call to pp_ada_tree_identifier.
(dump_ada_enum_type): Remove type and display_convention parameters.
Adjust calls to pp_ada_tree_identifier.
(dump_ada_node): Likewise and for dump_ada_structure.
(dump_nested_type) : Adjust call to dump_ada_enum_type
and tidy up.
: Adjust call to dump_ada_structure and switch to aspect
syntax.
(print_constructor): Adjust call to pp_ada_tree_identifier.
(print_destructor): Likewise.
(dump_ada_declaration): Switch to aspect syntax.
(dump_ada_structure): Likewise and tidy up.  Replace display_convention
parameter with nested parameter.
(dump_ads): Emit pragma Ada_2012 in lieu of pragma Ada_2005.
(dump_ada_specs): Do not delete overloaded_names table.

-- 
Eric BotcazouIndex: c-ada-spec.c
===
--- c-ada-spec.c	(revision 262468)
+++ c-ada-spec.c	(working copy)
@@ -34,8 +34,8 @@ along with GCC; see the file COPYING3.
 /* Local functions, macros and variables.  */
 static int  dump_ada_node (pretty_printer *, tree, tree, int, bool, bool);
 static int  dump_ada_declaration (pretty_printer *, tree, tree, int);
-static void dump_ada_structure (pretty_printer *, tree, tree, int, bool);
-static char *to_ada_name (const char *, unsigned int, bool *);
+static void dump_ada_structure (pretty_printer *, tree, tree, bool, int);
+static char *to_ada_name (const char *, bool *);
 
 #define INDENT(SPACE) \
   do { int i; for (i = 0; i homonyms;
-};
-
-struct overloaded_name_hasher : delete_ptr_hash
-{
-  static inline hashval_t hash (overloaded_name_hash *t)
-{ return t->hash; }
-  static inline bool equal (overloaded_name_hash *a, overloaded_name_hash *b)
-{ return a->name == b->name && a->context == b->context; }
-};
-
-static hash_table *overloaded_names;
-
-/* Compute the overloading index of function DECL in its context.  */
-
-static unsigned int
-compute_overloading_index (tree decl)
-{
-  const hashval_t hashcode
-= iterative_hash_hashval_t (htab_hash_pointer (DECL_NAME (decl)),
-			htab_hash_pointer (DECL_CONTEXT (decl)));
-  struct overloaded_name_hash in, *h, **slot;
-  unsigned int index, *iter;
-
-  if (!overloaded_names)
-overloaded_names = new hash_table (512);
-
-  /* Look up the list of homonyms in the table.  */
-  in.hash = hashcode;
-  in.name = DECL_NAME (decl);
-  in.context = DECL_CONTEXT (decl);
-  slot = overloaded_names->find_slot_with_hash (&in, hashcode, INSERT);
-  if (*slot)
-h = *slot;
-  else
-{
-  h = new overloaded_name_hash;
-  h->hash = hashcode;
-  h->name = DECL_NAME (decl);
-  h->context = DECL_CONTEXT (decl);
-  h->homonyms.create (0);
-  *slot = h;
-}
-
-  /* Look up the function in the list of homonyms.  */
-  FOR_EACH_VEC_ELT (h->homonyms, index, iter)
-if (*iter == DECL_UID (decl))
-  break;
-
-  /* If it is not present, push it onto the list.  */
-  if (!iter)
-h->homonyms.safe_push (DECL_UID (decl));
-
-  return index;
-}
-
 /* Dump in BUFFER the name of a DECL node if set, in Ada syntax.
LIMITED_ACCESS indicates whether NODE can be accessed via a
limited 'with' clause rather than a regular 'with' clause.  */
@@ -1519,13 +1450,7 @@ static void
 dump_ada_decl_name (pretty_printer *buffer, tree decl, bool limited_access)
 {
   if (DECL_NAME (decl))
-{
-  const unsigned int index
-	= (TREE_CODE (decl) == FUNCTION_DECL && cpp_check)
-	  ? compute_overloading_index (decl) : 0;
-  pp_ada_tree_identifier (buffer, DECL_NAME (decl), decl, index,
-			  limited_access);
-}
+pp_ada_tree_identifier (buffer, DECL_NAME (decl), decl, limited_access);
   else
 {
   tree type_name = TYPE_NAME (TREE_TYPE (decl));
@@ -1539,7 +1464,7 @@ dump_ada_decl_name (pretty_printer *buff
 	pp_scalar (buffer, 

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Eric Botcazou]

> I tried adding the missing definition.  I now get
> 
> === acats Summary ===
> # of expected passes2320
> # of unexpected failures0
> 
> === gnat Summary ===
> 
> # of expected passes2779
> # of unexpected failures4
> # of expected failures  24
> # of unsupported tests  25
> 
> So yes, that solved my problem, and we have a working RISC-V Ada port
> now.  Thanks for the help.

You're welcome.  Are the 4 remaining failures related to stack checking?

-- 
Eric Botcazou

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Eric Botcazou]

> I haven't tried looking at the failures yet, and might not spend much
> more time on this.  Two of them are debug related, and debug support
> is a work in progress.  I need to finish the native riscv64-linux
> support before we can do anything useful there, and I'd like to get
> back to working on that as soon as possible.

No clue about debug11.adb, maybe Pierre-Marie could shed some light on it.

> The GNU-stack error looks a little worrisome.  I'd expect a linux port to
> get GNU-stack stuff right without much trouble.

The test checks that -ftrampolines forces the use of trampolines instead of 
descriptors.  If trampolines are generated on the stack for RISC-V, then the 
stack should be made executable when there are built.  Otherwise you can add 
the appropriate triplet to the dg-skip-if line.

In any case, that's benign since trampolines are not generated by default now.

> The last one is warn5.adb:29:30: warning: source alignment (4) < alignment
> of "Element_Type" (8) Maybe something I copied from the mips linux port is
> wrong for riscv64 linux.

No, this looks as expected, you just need to add the appropriate triplet to 
the list on line 29 when you have 15 seconds to kill.  Patchlet preapproved.

-- 
Eric Botcazou

[SPARC] Minor tweak

[Eric Botcazou]

Tested on SPARC/Solaris, applied on the mainline.


2018-07-13  Eric Botcazou  

* config/sparc/sparc-protos.h (sparc_compute_frame_size): Delete.
* config/sparc/sparc.c (sparc_compute_frame_size): Make static.

-- 
Eric BotcazouIndex: config/sparc/sparc-protos.h
===
--- config/sparc/sparc-protos.h	(revision 262551)
+++ config/sparc/sparc-protos.h	(working copy)
@@ -31,7 +31,6 @@ extern unsigned long sparc_type_code (tr
 #endif /* TREE_CODE */
 
 extern void order_regs_for_local_alloc (void);
-extern HOST_WIDE_INT sparc_compute_frame_size (HOST_WIDE_INT, int);
 extern int sparc_initial_elimination_offset (int);
 extern void sparc_expand_prologue (void);
 extern void sparc_flat_expand_prologue (void);
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 262551)
+++ config/sparc/sparc.c	(working copy)
@@ -5459,7 +5459,7 @@ save_local_or_in_reg_p (unsigned int reg
 /* Compute the frame size required by the function.  This function is called
during the reload pass and also by sparc_expand_prologue.  */
 
-HOST_WIDE_INT
+static HOST_WIDE_INT
 sparc_compute_frame_size (HOST_WIDE_INT size, int leaf_function)
 {
   HOST_WIDE_INT frame_size, apparent_frame_size;

[testsuite] Minor tweak to 4 Aarch64 testcases

[Eric Botcazou]

These 4 Aarch64 testcases use dg-xfail-if to disable themselves on ARM, while 
all the other equivalent testcases use dg-skip-if.  The latter form is better 
because it doesn't unnecessarily pollute the testsuite report.

Tested on arm-eabi, OK for the mainline?


2018-07-13  Eric Botcazou  

* gcc.target/aarch64/advsimd-intrinsics/vld1x2.c: Replace dg-xfail-if
with dg-skip-if for ARM targets.
* gcc.target/aarch64/advsimd-intrinsics/vld1x3.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vst1x2.c: Likewise.
* gcc.target/aarch64/advsimd-intrinsics/vst1x3.c: Likewise.

-- 
Eric BotcazouIndex: gcc.target/aarch64/advsimd-intrinsics/vld1x2.c
===
--- gcc.target/aarch64/advsimd-intrinsics/vld1x2.c	(revision 262551)
+++ gcc.target/aarch64/advsimd-intrinsics/vld1x2.c	(working copy)
@@ -1,7 +1,7 @@
 /* We haven't implemented these intrinsics for arm yet.  */
-/* { dg-xfail-if "" { arm*-*-* } } */
 /* { dg-do run } */
 /* { dg-options "-O3" } */
+/* { dg-skip-if "" { arm*-*-* } } */
 
 #include 
 
Index: gcc.target/aarch64/advsimd-intrinsics/vld1x3.c
===
--- gcc.target/aarch64/advsimd-intrinsics/vld1x3.c	(revision 262551)
+++ gcc.target/aarch64/advsimd-intrinsics/vld1x3.c	(working copy)
@@ -1,7 +1,7 @@
 /* We haven't implemented these intrinsics for arm yet.  */
-/* { dg-xfail-if "" { arm*-*-* } } */
 /* { dg-do run } */
 /* { dg-options "-O3" } */
+/* { dg-skip-if "" { arm*-*-* } } */
 
 #include 
 #include "arm-neon-ref.h"
Index: gcc.target/aarch64/advsimd-intrinsics/vst1x2.c
===
--- gcc.target/aarch64/advsimd-intrinsics/vst1x2.c	(revision 262551)
+++ gcc.target/aarch64/advsimd-intrinsics/vst1x2.c	(working copy)
@@ -1,7 +1,7 @@
 /* We haven't implemented these intrinsics for arm yet.  */
-/* { dg-xfail-if "" { arm*-*-* } } */
 /* { dg-do run } */
 /* { dg-options "-O3" } */
+/* { dg-skip-if "" { arm*-*-* } } */
 
 #include 
 #include "arm-neon-ref.h"
Index: gcc.target/aarch64/advsimd-intrinsics/vst1x3.c
===
--- gcc.target/aarch64/advsimd-intrinsics/vst1x3.c	(revision 262551)
+++ gcc.target/aarch64/advsimd-intrinsics/vst1x3.c	(working copy)
@@ -1,7 +1,7 @@
 /* We haven't implemented these intrinsics for arm yet.  */
-/* { dg-xfail-if "" { arm*-*-* } } */
 /* { dg-do run } */
 /* { dg-options "-O3" } */
+/* { dg-skip-if "" { arm*-*-* } } */
 
 #include 
 #include "arm-neon-ref.h"

[testsuite] Robustify target_tls_runtime check

[Eric Botcazou]

As witnessed by the kludge added for MSP430 and Visium, the check doesn't 
really work (that's also visible for arm-eabi).

Tested on x86_64-suse-linux, visium-elf and arm-eabi, OK for mainline?


2018-07-13  Eric Botcazou  

* lib/target-supports.exp (check_effective_target_tls_runtime): Force
global-dynamic model for thread variable and remove kludge.

-- 
Eric BotcazouIndex: lib/target-supports.exp
===
--- lib/target-supports.exp	(revision 262551)
+++ lib/target-supports.exp	(working copy)
@@ -878,13 +878,8 @@ proc check_effective_target_tls_emulated
 # Return 1 if TLS executables can run correctly, 0 otherwise.
 
 proc check_effective_target_tls_runtime {} {
-# The runtime does not have TLS support, but just
-# running the test below is insufficient to show this.
-if { [istarget msp430-*-*] || [istarget visium-*-*] } {
-	return 0
-}
 return [check_runtime tls_runtime {
-	__thread int thr = 0;
+	__thread int thr __attribute__((tls_model("global-dynamic"))) = 0;
 	int main (void) { return thr; }
 } [add_options_for_tls ""]]
 }

Re: [testsuite] Minor tweak to 4 Aarch64 testcases

[Eric Botcazou]

> I used xfail for these testcases in particular because the intrinsics that
> they test should be available for both arm and aarch64.
> They are currently not implemented on arm, even though they should be.
> The other tests that are skipped instead of xfailed test intrinsics that
> should only be available on aarch64 and not arm.

OK, thanks for the explanation.

-- 
Eric Botcazou

Re: [PATCH, Ada] RISC-V: Initial riscv linux Ada port.

[Eric Botcazou]

> I poked at this a little and noticed a difference between the x86_64
> support and the RISC-V support.  The RISC-V C language port has char
> as unsigned by default.  The x86_64 port has char signed by default.
> If I add a -fsigned-char option, then the testcase works as expected
> for RISC-V.  Curiously, the Ada compiler accepts -fsigned-char but not
> -funsigned-char.

But it accepts -fno-signed-char, which is equivalent. :-)  In any case, I 
agree that it should also accept -funsigned-char, now done.

> I tried hacking in a -funsigned-char flag, but when
> I use it with the x86_64 port the result is still correct.  Maybe my
> quick hack wasn't quite right.  Anyways, the default signedness of
> char has something to do with the problem.

I don't seem to be able to reproduce the failure with a cross-compiler though 
so that's really weird.


* gcc-interface/lang.opt (funsigned-char): New option.
* gcc-interface/misc.c (gnat_handle_option): Accept it.
* gcc-interface/utils.c (finish_character_type): Tweak comment.


-- 
Eric BotcazouIndex: gcc-interface/lang.opt
===
--- gcc-interface/lang.opt	(revision 262551)
+++ gcc-interface/lang.opt	(working copy)
@@ -80,6 +80,10 @@ fsigned-char
 Ada AdaWhy AdaSCIL
 Make \"char\" signed by default.
 
+funsigned-char
+Ada AdaWhy AdaSCIL
+Make \"char\" unsigned by default.
+
 gant
 Ada AdaWhy AdaSCIL Driver Joined Undocumented RejectNegative
 Catch typos.
Index: gcc-interface/misc.c
===
--- gcc-interface/misc.c	(revision 262551)
+++ gcc-interface/misc.c	(working copy)
@@ -170,6 +170,7 @@ gnat_handle_option (size_t scode, const
 
 case OPT_fshort_enums:
 case OPT_fsigned_char:
+case OPT_funsigned_char:
   /* These are handled by the middle-end.  */
   break;
 
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c	(revision 262551)
+++ gcc-interface/utils.c	(working copy)
@@ -1684,7 +1684,7 @@ record_builtin_type (const char *name, t
   integral types are unsigned.
 
   Unfortunately the signedness of 'char' in C is implementation-defined
-  and GCC even has the option -fsigned-char to toggle it at run time.
+  and GCC even has the option -f{un}signed-char to toggle it at run time.
   Since GNAT's philosophy is to be compatible with C by default, to wit
   Interfaces.C.char is defined as a mere copy of Character, we may need
   to declare character types as signed types in GENERIC and generate the

Re: [PATCH] Properly unshare TYPE_SIZE_UNIT/DECL_FIELD_OFFSET (PR86216)

[Eric Botcazou]

> It breaks Ada bootstrap.  I guess Ada has variable-size types in
> non-function scope (not sure how TYPE_SIZES_GIMPLIFIED works then
> though).  That said, r92495 moved the unshare_expr from layout_type
> to gimplify_one_sizepos.

See gimplify.c:763 and below.

-- 
Eric Botcazou

[patch] Fix PR tree-optimization/86514

[Eric Botcazou]

Hi,

this is a regression present on the mainline and 8 branch in the form of wrong 
code generated for an Ada program manipulating bit-packed boolean array types.

The problem is in the new range optimization code of the reassoc pass: from

  _64 = _63 | 4;
  _73 = _64 & 191;
  _76 = _64 >> 6;
  _77 = (boolean) _76;
  _78 = (boolean) _64;
  _79 = _77 | _78;

it deduces:

Optimizing range tests _76 +[0, 0] and _64 +[0, 0]
|...]
  _64 = _63 | 4;
  _73 = _64 & 191;
  _76 = _64 >> 6;
  _90 = _76 | _64;
  _19 = _90 != 0;
  _77 = (boolean) _76;
  _78 = (boolean) _64;
  _79 = _19;

which is not equivalent.  The proposed fix is to avoid bypassing a conversion 
to a boolean type from a type with greater precision in init_range_entry.

Tested on x86_64-suse-linux, OK for the mainline?


2018-07-16  Eric Botcazou  

PR tree-optimization/86514
* tree-ssa-reassoc.c (init_range_entry) : Return for a
conversion to a boolean type from a type with greater precision.


2018-07-16  Eric Botcazou  

* gnat.dg/opt73.adb: New test.

-- 
Eric BotcazouIndex: tree-ssa-reassoc.c
===
--- tree-ssa-reassoc.c	(revision 262658)
+++ tree-ssa-reassoc.c	(working copy)
@@ -2168,8 +2168,13 @@ init_range_entry (struct range_entry *r,
 	  continue;
 	CASE_CONVERT:
 	  if (is_bool)
-	goto do_default;
-	  if (TYPE_PRECISION (TREE_TYPE (arg0)) == 1)
+	{
+	  if ((TYPE_PRECISION (exp_type) == 1
+		   || TREE_CODE (exp_type) == BOOLEAN_TYPE)
+		  && TYPE_PRECISION (TREE_TYPE (arg0)) > 1)
+		return;
+	}
+	  else if (TYPE_PRECISION (TREE_TYPE (arg0)) == 1)
 	{
 	  if (TYPE_UNSIGNED (TREE_TYPE (arg0)))
 		is_bool = true;
-- { dg-do run }
-- { dg-options "-O" }

procedure Opt73 is

   type Terminal_Set_Indexed_By_Non_Terminal is
 array (Natural range <>, Natural  range <>) of Boolean with Pack;

   type Terminal_Set_Per_Non_Terminal
 (Last_Terminal : Natural;
  Last_Non_Terminal : Natural) is
   record
  Map : Terminal_Set_Indexed_By_Non_Terminal
(1 .. Last_Non_Terminal, 0 .. Last_Terminal);
   end record;

   Follow : Terminal_Set_Per_Non_Terminal (5, 4);
   Expect : Terminal_Set_Per_Non_Terminal :=
 (5, 4, (1 => (2 => True, others => False),
 others => (others => False)));

   procedure Get_Follow (Value : out Terminal_Set_Per_Non_Terminal) is
   begin
  Value.Map := (others => (others => False));
  Value.Map (1, 2) := True;
  Value.Map (2, 0) := Value.Map (2, 0) or Value.Map (1, 0);
   end;

begin
   Get_Follow (Follow);
   if Follow /= Expect then
  raise Program_Error;
   end if;
end;

Re: [PATCH] Properly unshare TYPE_SIZE_UNIT/DECL_FIELD_OFFSET (PR86216)

[Eric Botcazou]

> Thanks. In that light the unsharing at the places the FE builds expressions
> using TYPE_SIZE and friends looks like the way to go.

Probably, yes.

> I still wonder why unsharing in gimplify_one_sizepos is necessary though.
> Ist that because even deep unsharing doesn't walk types?

walk_tree walks the DECL_EXPR of TYPE_DECL.  I think that it's an old band-aid 
for types defined at library level in Ada.  Let me experiment a bit with that.

-- 
Eric Botcazou

[Ada] Fix spurious check failure for Character discriminant

[Eric Botcazou]

This is a regression present on the mainline, 8 and 7 branches.  The compiler 
generates a spurious check failure for a Character discriminant declared in a 
discriminated record type with variant part if one of the variants is selected 
by a range of values which contains at least the values at position 127 & 128.

It's also indirectly responsible for failure of gnat.dg/debug11.adb on RISC-V.

Tested on x86-64/Linux, applied on the mainline, 8 and 7 branches.


2018-07-17  Eric Botcazou  

* gcc-interface/decl.c (choices_to_gnu): Rename parameters.  Deal with
an operand of Character type.  Factor out range generation to the end.
Check that the bounds are literals and convert them to the type of the
operand before building the ranges.
* gcc-interface/utils.c (make_dummy_type): Minor tweak.
(make_packable_type): Propagate TYPE_DEBUG_TYPE.
(maybe_pad_type): Likewise.


2018-07-17  Eric Botcazou  

* gnat.dg/discr55.adb: New test.

-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 262803)
+++ gcc-interface/decl.c	(working copy)
@@ -6705,65 +6705,44 @@ elaborate_reference (tree ref, Entity_Id
the value passed against the list of choices.  */
 
 static tree
-choices_to_gnu (tree operand, Node_Id choices)
+choices_to_gnu (tree gnu_operand, Node_Id gnat_choices)
 {
-  Node_Id choice;
-  Node_Id gnat_temp;
-  tree result = boolean_false_node;
-  tree this_test, low = 0, high = 0, single = 0;
+  tree gnu_result = boolean_false_node, gnu_type;
 
-  for (choice = First (choices); Present (choice); choice = Next (choice))
+  gnu_operand = maybe_character_value (gnu_operand);
+  gnu_type = TREE_TYPE (gnu_operand);
+
+  for (Node_Id gnat_choice = First (gnat_choices);
+   Present (gnat_choice);
+   gnat_choice = Next (gnat_choice))
 {
-  switch (Nkind (choice))
+  tree gnu_low = NULL_TREE, gnu_high = NULL_TREE;
+  tree gnu_test;
+
+  switch (Nkind (gnat_choice))
 	{
 	case N_Range:
-	  low = gnat_to_gnu (Low_Bound (choice));
-	  high = gnat_to_gnu (High_Bound (choice));
-
-	  this_test
-	= build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
-			   build_binary_op (GE_EXPR, boolean_type_node,
-		operand, low, true),
-			   build_binary_op (LE_EXPR, boolean_type_node,
-		operand, high, true),
-			   true);
-
+	  gnu_low = gnat_to_gnu (Low_Bound (gnat_choice));
+	  gnu_high = gnat_to_gnu (High_Bound (gnat_choice));
 	  break;
 
 	case N_Subtype_Indication:
-	  gnat_temp = Range_Expression (Constraint (choice));
-	  low = gnat_to_gnu (Low_Bound (gnat_temp));
-	  high = gnat_to_gnu (High_Bound (gnat_temp));
-
-	  this_test
-	= build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
-			   build_binary_op (GE_EXPR, boolean_type_node,
-		operand, low, true),
-			   build_binary_op (LE_EXPR, boolean_type_node,
-		operand, high, true),
-			   true);
+	  gnu_low = gnat_to_gnu (Low_Bound (Range_Expression
+	(Constraint (gnat_choice;
+	  gnu_high = gnat_to_gnu (High_Bound (Range_Expression
+	  (Constraint (gnat_choice;
 	  break;
 
 	case N_Identifier:
 	case N_Expanded_Name:
-	  /* This represents either a subtype range, an enumeration
-	 literal, or a constant  Ekind says which.  If an enumeration
-	 literal or constant, fall through to the next case.  */
-	  if (Ekind (Entity (choice)) != E_Enumeration_Literal
-	  && Ekind (Entity (choice)) != E_Constant)
+	  /* This represents either a subtype range or a static value of
+	 some kind; Ekind says which.  */
+	  if (Is_Type (Entity (gnat_choice)))
 	{
-	  tree type = gnat_to_gnu_type (Entity (choice));
+	  tree gnu_type = get_unpadded_type (Entity (gnat_choice));
 
-	  low = TYPE_MIN_VALUE (type);
-	  high = TYPE_MAX_VALUE (type);
-
-	  this_test
-		= build_binary_op (TRUTH_ANDIF_EXPR, boolean_type_node,
-   build_binary_op (GE_EXPR, boolean_type_node,
-		operand, low, true),
-   build_binary_op (LE_EXPR, boolean_type_node,
-		operand, high, true),
-   true);
+	  gnu_low = TYPE_MIN_VALUE (gnu_type);
+	  gnu_high = TYPE_MAX_VALUE (gnu_type);
 	  break;
 	}
 
@@ -6771,27 +6750,49 @@ choices_to_gnu (tree operand, Node_Id ch
 
 	case N_Character_Literal:
 	case N_Integer_Literal:
-	  single = gnat_to_gnu (choice);
-	  this_test = build_binary_op (EQ_EXPR, boolean_type_node, operand,
-   single, true);
+	  gnu_low = gnat_to_gnu (gnat_choice);
 	  break;
 
 	case N_Others_Choice:
-	  this_test = boolean_true_node;
 	  break;
 
 	default:
 	  gcc_unreachable ();
 	}
 
-  if (result == boolean_false_node)
-	result = this_test;
+  /* Everything should be folded into constants at this point.  */
+  gcc_assert (!gnu_low  || TREE_CODE (gnu_low)  == INTEGER_CST);
+  gcc_assert (!gnu_high || TREE_CODE (gnu_hig

Re: [PATCH] enhance strlen to understand MEM_REF and partial overlaps (PR 86042, 86043)

[Eric Botcazou]

> I missed your approval and didn't get to committing the patch
> until today.  While retesting it on top of fresh trunk I noticed
> a few test failures due to other recent strlen changes.  I made
> adjustments to the patch to avoid most of them and opened bug
> 86688 for one that I think needs a separate code change and
> xfailed the test cases until the bug gets resolved.

But it has introduced a couple of regressions in the ACATS testsuite:

FAIL:   c52103c
FAIL:   cde0001

=== acats Summary ===
# of expected passes2318
# of unexpected failures2
Native configuration is x86_64-suse-linux-gnu


+===GNAT BUG DETECTED==+
| 9.0.0 20180727 (experimental) [trunk revision 263028] (x86_64-suse-linux) 
GCC error:|
| in handle_char_store, at tree-ssa-strlen.c:3332  |
| Error detected around c52103c.adb:43:1       |

-- 
Eric Botcazou

Re: [PATCH] enhance strlen to understand MEM_REF and partial overlaps (PR 86042, 86043)

[Eric Botcazou]

> FWIW, there are 128 failures in the GCC test suite on x86_64.
> Many of these have been there for weeks (e.g., the lto failures
> due to PR86004), even years (guality).  My script to compare
> the results against a baseline uses the following regular
> expression to extract the names of failing (and other
> "interesting") tests. 

Why not just use "make mail-report.log" ?

-- 
Eric Botcazou

Re: [PATCH] enhance strlen to understand MEM_REF and partial overlaps (PR 86042, 86043)

[Eric Botcazou]

> What I was trying to highlight is that rolling my own solution like
> this makes missing regressions more likely than having a shared
> solution would.

But 'make mail-report.log' is precisely the shared solution, no need to 
reinvent the wheel here.

-- 
Eric Botcazou

Re: [patch] improve internals documentation for nested function descriptors

[Eric Botcazou]

> Apropos of the discussion about improving the docs for
> TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port
> submission,
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html
> 
> here is the patch I've come up with based on reading the source.  Is
> this technically accurate?  Any suggestions on how to improve it further?

""Define this hook to 0 if the target implements custom support"

"custom" was precisely chosen to distinguish this kind of descriptors from the 
standard descriptors used on IA-64 or HP-PA, so it's contradictory.  Moreover 
I would really start with the "custom" case and not the standard case as was 
originally written, the "custom" case being the common case for targets.

I'm not really convinced by the substitution misalignment -> tag either, but 
if others find the new version more understandable, then OK with me.

-- 
Eric Botcazou

Re: [patch] improve internals documentation for nested function descriptors

[Eric Botcazou]

> This is precisely what I found so confusing about the original text.  To
> me, "custom" implies that the back end is *customized* to have its own
> descriptor implementation to conform to target-specific ABI standards,
> not that it uses a generic implementation in common code.

To me, "custom" sounds contradictory with "conform to" in this sentence.

And I don't follow your interpretation, back-ends are by definition customized 
to follow external constraints and not every macro/hook has "custom" in it.

> I could make the docs say both things, but from the perspective of a
> back end implementor, being explicit that it's a bit mask used to
> differentiate descriptors from any valid function pointer (so you can
> figure out what an appropriate value to define it to is) is more
> critical than describing what target-independent code does with a
> descriptor once it has identified that's what it's got.

Fair enough.

-- 
Eric Botcazou

[Ada] Fix internal error on extension with interface at -O2

[Eric Botcazou]

This is a regression present on the mainline, 10 and 9 branches, in the form 
of an internal error when a covariant-only thunk is inlined into its caller.

Tested on x86-64/Linux, applied on the mainline, 10 and 9 branches.


2021-01-25  Eric Botcazou  

* gcc-interface/trans.c (make_covariant_thunk): Set DECL_CONTEXT
of the parameters and do not set TREE_PUBLIC on the thunk.
(maybe_make_gnu_thunk): Pass the alias to the covariant thunk.
* gcc-interface/utils.c (finish_subprog_decl): Set the DECL_CONTEXT
of the parameters here...
(begin_subprog_body): ...instead of here.


2021-01-25  Eric Botcazou  

* gnat.dg/thunk2.adb, gnat.dg/thunk2.ads: New test.
* gnat.dg/thunk2_pkg.ads: New helper.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 6402c73ded0..ae7a52f3ca2 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -10612,7 +10612,7 @@ make_alias_for_thunk (tree target)
   return alias;
 }
 
-/* Create the covariant part of the {GNAT,GNU}_THUNK.  */
+/* Create the local covariant part of {GNAT,GNU}_THUNK.  */
 
 static tree
 make_covariant_thunk (Entity_Id gnat_thunk, tree gnu_thunk)
@@ -10623,6 +10623,11 @@ make_covariant_thunk (Entity_Id gnat_thunk, tree gnu_thunk)
 		  gnu_name, TREE_TYPE (gnu_thunk));
 
   DECL_ARGUMENTS (gnu_cv_thunk) = copy_list (DECL_ARGUMENTS (gnu_thunk));
+  for (tree param_decl = DECL_ARGUMENTS (gnu_cv_thunk);
+   param_decl;
+   param_decl = DECL_CHAIN (param_decl))
+DECL_CONTEXT (param_decl) = gnu_cv_thunk;
+
   DECL_RESULT (gnu_cv_thunk) = copy_node (DECL_RESULT (gnu_thunk));
   DECL_CONTEXT (DECL_RESULT (gnu_cv_thunk)) = gnu_cv_thunk;
 
@@ -10630,7 +10635,6 @@ make_covariant_thunk (Entity_Id gnat_thunk, tree gnu_thunk)
   DECL_CONTEXT (gnu_cv_thunk) = DECL_CONTEXT (gnu_thunk);
   TREE_READONLY (gnu_cv_thunk) = TREE_READONLY (gnu_thunk);
   TREE_THIS_VOLATILE (gnu_cv_thunk) = TREE_THIS_VOLATILE (gnu_thunk);
-  TREE_PUBLIC (gnu_cv_thunk) = TREE_PUBLIC (gnu_thunk);
   DECL_ARTIFICIAL (gnu_cv_thunk) = 1;
 
   return gnu_cv_thunk;
@@ -10760,6 +10764,12 @@ maybe_make_gnu_thunk (Entity_Id gnat_thunk, tree gnu_thunk)
 
   cgraph_node *target_node = cgraph_node::get_create (gnu_target);
 
+  /* We may also need to create an alias for the target in order to make
+ the call local, depending on the linkage of the target.  */
+  tree gnu_alias = use_alias_for_thunk_p (gnu_target)
+		  ? make_alias_for_thunk (gnu_target)
+		  : gnu_target;
+
   /* If the return type of the target is a controlling type, then we need
  both an usual this thunk and a covariant thunk in this order:
 
@@ -10772,17 +10782,11 @@ maybe_make_gnu_thunk (Entity_Id gnat_thunk, tree gnu_thunk)
   tree gnu_cv_thunk = make_covariant_thunk (gnat_thunk, gnu_thunk);
   target_node->create_thunk (gnu_cv_thunk, gnu_target, false,
  - fixed_offset, 0, 0,
- NULL_TREE, gnu_target);
+ NULL_TREE, gnu_alias);
 
-  gnu_target = gnu_cv_thunk;
+  gnu_alias = gnu_target = gnu_cv_thunk;
 }
 
-  /* We may also need to create an alias for the target in order to make
- the call local, depending on the linkage of the target.  */
-  tree gnu_alias = use_alias_for_thunk_p (gnu_target)
-		  ? make_alias_for_thunk (gnu_target)
-		  : gnu_target;
-
   target_node->create_thunk (gnu_thunk, gnu_target, true,
 			 fixed_offset, virtual_value, indirect_offset,
 			 virtual_offset, gnu_alias);
diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 494f60e0879..d52220a675e 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -3521,6 +3521,12 @@ create_subprog_decl (tree name, tree asm_name, tree type, tree param_decl_list,
 void
 finish_subprog_decl (tree decl, tree asm_name, tree type)
 {
+  /* DECL_ARGUMENTS is set by the caller, but not its context.  */
+  for (tree param_decl = DECL_ARGUMENTS (decl);
+   param_decl;
+   param_decl = DECL_CHAIN (param_decl))
+DECL_CONTEXT (param_decl) = decl;
+
   tree result_decl
 = build_decl (DECL_SOURCE_LOCATION (decl), RESULT_DECL, NULL_TREE,
 		  TREE_TYPE (type));
@@ -3566,8 +3572,6 @@ finish_subprog_decl (tree decl, tree asm_name, tree type)
 void
 begin_subprog_body (tree subprog_decl)
 {
-  tree param_decl;
-
   announce_function (subprog_decl);
 
   /* This function is being defined.  */
@@ -3583,10 +3587,6 @@ begin_subprog_body (tree subprog_decl)
   /* Enter a new binding level and show that all the parameters belong to
  this function.  */
   gnat_pushlevel ();
-
-  for (param_decl = DECL_ARGUMENTS (subprog_decl); param_decl;
-   param_decl = DECL_CHAIN (param_decl))
-DECL_CONTEXT (param_decl) = subprog_decl;
 }
 
 /* Finish translating the current subprogram and set its BODY.  */
package Thunk2_Pkg is

  type Root is tagged record
A : Integer;
  end record;

  type I is interface;

  function Element (Se

[Ada] Fix PR ada/98228

[Eric Botcazou]

This is the profiled bootstrap failure for s390x/Linux on the mainline, which 
has been introduced by the modref pass but actually exposing an existing issue 
in the maybe_pad_type function that is visible only on s390x.  Marius did all 
the heavy lifting and posted a fairly extensive investigation report in the 
audit trail (thanks!), so I don't repeat it here; the ultimate issue is too 
weak a test for the addressability of the inner component.

Marius bootstrapped/regtested it on s930x and x86-64 and I conducted a bit 
more testing on my side.  Applied on mainline, 10 and 9 branches.


2021-01-26  Eric Botcazou  
Marius Hillenbrand  

PR ada/98228
* gcc-interface/utils.c (maybe_pad_type): Test the size of the new
packable type instead of its alignment for addressability's sake.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 494f60e0879..95e509c6e8b 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -1571,7 +1571,7 @@ maybe_pad_type (tree type, tree size, unsigned int align,
 {
   tree packable_type = make_packable_type (type, true, align);
   if (TYPE_MODE (packable_type) != BLKmode
-	  && align >= TYPE_ALIGN (packable_type))
+	  && compare_tree_int (TYPE_SIZE (packable_type), align) <= 0)
 type = packable_type;
 }
 

Fix LTO bootstrap on Windows (PR lto/85574)

[Eric Botcazou]

The last fix made for PR lto/85574 introduced a comparison of executables and 
this cannot directly work on Windows because they are timestamped.  Moreover 
nobody sets $(exeext) at top level, at least on MinGW, so you get a weird 
behavior because some tools add the implicit .exe suffix and others don't.

Bootstrapped in LTO mode on Windows, OK for all active branches?


2021-01-28  Eric Botcazou  

contrib/
PR lto/85574
* compare-lto: Deal with PE-COFF executables specifically.

-- 
Eric Botcazoudiff --git a/contrib/compare-lto b/contrib/compare-lto
index 17379e196a7..c0bb71c0765 100755
--- a/contrib/compare-lto
+++ b/contrib/compare-lto
@@ -32,7 +32,7 @@ case $1 in
 esac
 
 if test $# != 2; then
-  echo 'usage: compare-lto file1.o file2.o' >&2
+  echo 'usage: compare-lto file1 file2' >&2
   exit 1
 fi
 
@@ -101,6 +101,25 @@ else
 else
   status=1
 fi
+
+  # PE-COFF executables are timestamped so skip leading bytes for them.
+  else
+case "$1" in
+  *.exe)
+if cmp -i 256 "$1" "$2"; then
+  status=0
+else
+  status=1
+fi
+;;
+  *)
+if test -f "$1.exe" && cmp -i 256 "$1.exe" "$2.exe"; then
+  status=0
+else
+  status=1
+fi
+;;
+esac
   fi
 fi
 

Re: [PATCH] tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775]

[Eric Botcazou]

> Bootstrapped/regtested on
> * x86_64-pc-linux-gnu
> * powerpc64le-unknown-linux-gnu
> * aarch64-linux-gnu
> ok for trunk?

None of them is strict alignment though, isn't it?

-- 
Eric Botcazou

Re: [PATCH] tree: Don't reuse types if TYPE_USER_ALIGN differ [PR94775]

[Eric Botcazou]

> Aware.  I don't have access to, e.g., a sparc box.  But the test I've added
> uses -mstrict-align where possible to check that the issue is resolved.

There are relatively fast SPARC64/Linux (gcc202) and SPARC/Solaris machines 
(gcc210 and gcc211) in the Compile Farm:
  https://cfarm.tetaneutral.net/machines/list/

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> The important difference is for strn{,case}cmp folding, we pass that s2
> value as the last argument to the host functions comparing the c_getstr
> results.  If s2 fits into size_t, then my patch makes no difference,
> but if it is larger, we know the 2 c_getstr objects need to fit into the
> host address space, so larger s2 should just act essentially as strcmp
> or strcasecmp; as none of those objects can occupy 100% of the address
> space, using MIN (SIZE_MAX, s2) achieves that.

But SIZE_MAX is a host value, isn't it?  As a matter of fact, it breaks the 
build with somewhat ancient glibcs:

In file included from ../../src/gcc/fold-const-call.c:21:

../../src/gcc/fold-const-call.c: In function 'tree_node* 
fold_const_call(combined_fn, tree, tree, tree, tree)':

../../src/gcc/fold-const-call.c:1777:56: error: 'SIZE_MAX' was not declared in 
this scope

 1777 |  return build_int_cst (type, strncmp (p0, p1, MIN (s2, SIZE_MAX)));

because /usr/include/stdint.h has:

/* The ISO C99 standard specifies that in C++ implementations these
   macros should only be defined if explicitly requested.  */
#if !defined __cplusplus || defined __STDC_LIMIT_MACROS

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> In patch form now (for ~ we'd need to use (size_t) ~(size_t) 0 to be
> fully portable, I think nothing in the standard requires that size_t isn't
> e.g. unsigned char or unsigned short that would then promote to int.
> 
> Ok for trunk?
> 
> 2021-01-31  Jakub Jelinek  
> 
>   * fold-const-call.c (fold_const_call): Use INTTYPE_MAXIMUM (size_t)
>   instead of SIZE_MAX.

Why not just:

#ifndef SIZE_MAX
# define SIZE_MAX INTTYPE_MAXIMUM (size_t)
#endif

just below UCHAR_MAX in system.h?

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> Why not just:
> 
> #ifndef SIZE_MAX
> # define SIZE_MAX INTTYPE_MAXIMUM (size_t)
> #endif
> 
> just below UCHAR_MAX in system.h?

Or rather just below

#ifdef HAVE_STDINT_H
#include 
#endif

#ifdef HAVE_INTTYPES_H
#include 
#endif

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> If it doesn't break anything (note, there can be system headers included
> even after this spot), why not.

OK, then maybe move them to the end.  Or define

#define __STDC_LIMIT_MACROS

just before the two aforementioned includes, so that this doesn't happen again 
with another macro.

> Note, it now affects GCC 10 branch too.

Yep, that's where it breaks things for us. :-)

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> Whatever works, I can't test such patches except on Linux, so can you just
> create a patch and test it on Solaris where it failed before?

It fails on old Linux distros, e.g. RHES 5, not on Solaris as far as I know.

-- 
Eric Botcazou

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> Whatever works, I can't test such patches except on Linux, so can you just
> create a patch and test it on Solaris where it failed before?

Maybe a safer fix is the attached one.  Tested on old RedHat and SuSE distros.


* fold-const-call.c: Define __STDC_LIMIT_MACROS at the top.

-- 
Eric Botcazoudiff --git a/gcc/fold-const-call.c b/gcc/fold-const-call.c
index da01759d9c8..c5f5566fa06 100644
--- a/gcc/fold-const-call.c
+++ b/gcc/fold-const-call.c
@@ -17,6 +17,8 @@ You should have received a copy of the GNU General Public License
 along with GCC; see the file COPYING3.  If not see
 .  */
 
+#define __STDC_LIMIT_MACROS /* For SIZE_MAX in C++.  */
+
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"

Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]

[Eric Botcazou]

> But next time we use SIZE_MAX somewhere it is going to break again this way.
> If we just define SIZE_MAX if not defined after all includes in system.h, I
> think it is better than this.

The existing practice seems to define the missing constants right after the 
corresponding include, e.g. MAP_FAILED, so I have successfully tested this.


* system.h (SIZE_MAX): Define if not already defined.

-- 
Eric Botcazoudiff --git a/gcc/system.h b/gcc/system.h
index d04f8fd3360..a2218834b43 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -528,6 +528,10 @@ extern void *realloc (void *, size_t);
 #include 
 #endif
 
+#ifndef SIZE_MAX
+# define SIZE_MAX INTTYPE_MAXIMUM (size_t)
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif

[Ada] Assorted LTO fixes

[Eric Botcazou]

This polishes a few rough edges visible in LTO mode.

Tested on x86-64/Linux, applied on mainline, 10 and 9 branches.


2021-02-03  Eric Botcazou  

* gcc-interface/decl.c (gnat_to_gnu_entity) : Make the
two fields of the fat pointer type addressable, and do not make the
template type read-only.
: If the type has discriminants, mark it as may_alias.
* gcc-interface/utils.c (make_dummy_type): Likewise.
(build_dummy_unc_pointer_types): Likewise.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index 5ea1b16af67..8120d4e33cf 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -2197,14 +2197,16 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	  }
 	else
 	  {
+	/* We make the fields addressable for the sake of compatibility
+	   with languages for which the regular fields are addressable.  */
 	tem
 	  = create_field_decl (get_identifier ("P_ARRAY"),
    ptr_type_node, gnu_fat_type,
-   NULL_TREE, NULL_TREE, 0, 0);
+   NULL_TREE, NULL_TREE, 0, 1);
 	DECL_CHAIN (tem)
 	  = create_field_decl (get_identifier ("P_BOUNDS"),
    gnu_ptr_template, gnu_fat_type,
-   NULL_TREE, NULL_TREE, 0, 0);
+   NULL_TREE, NULL_TREE, 0, 1);
 	finish_fat_pointer_type (gnu_fat_type, tem);
 	SET_TYPE_UNCONSTRAINED_ARRAY (gnu_fat_type, gnu_type);
 	  }
@@ -2327,7 +2329,6 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	finish_record_type (gnu_template_type, gnu_template_fields, 0,
 			debug_info_p);
 	TYPE_CONTEXT (gnu_template_type) = current_function_decl;
-	TYPE_READONLY (gnu_template_type) = 1;
 
 	/* If Component_Size is not already specified, annotate it with the
 	   size of the component.  */
@@ -3054,15 +3055,24 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 		|| type_annotate_only);
 	  }
 
-	/* Make a node for the record.  If we are not defining the record,
-	   suppress expanding incomplete types.  */
+	/* Make a node for the record type.  */
 	gnu_type = make_node (tree_code_for_record_type (gnat_entity));
 	TYPE_NAME (gnu_type) = gnu_entity_name;
 	TYPE_PACKED (gnu_type) = (packed != 0) || has_align || has_rep;
 	TYPE_REVERSE_STORAGE_ORDER (gnu_type)
 	  = Reverse_Storage_Order (gnat_entity);
+
+	/* If the record type has discriminants, pointers to it may also point
+	   to constrained subtypes of it, so mark it as may_alias for LTO.  */
+	if (has_discr)
+	  prepend_one_attribute
+	(&attr_list, ATTR_MACHINE_ATTRIBUTE,
+	 get_identifier ("may_alias"), NULL_TREE,
+	 gnat_entity);
+
 	process_attributes (&gnu_type, &attr_list, true, gnat_entity);
 
+	/* If we are not defining it, suppress expanding incomplete types.  */
 	if (!definition)
 	  {
 	defer_incomplete_level++;
diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index c503bfbb36d..2656f117fa9 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -467,6 +467,11 @@ make_dummy_type (Entity_Id gnat_type)
 = create_type_stub_decl (TYPE_NAME (gnu_type), gnu_type);
   if (Is_By_Reference_Type (gnat_equiv))
 TYPE_BY_REFERENCE_P (gnu_type) = 1;
+  if (Has_Discriminants (gnat_equiv))
+decl_attributes (&gnu_type,
+		 tree_cons (get_identifier ("may_alias"), NULL_TREE,
+NULL_TREE),
+		 ATTR_FLAG_TYPE_IN_PLACE);
 
   SET_DUMMY_NODE (gnat_equiv, gnu_type);
 
@@ -516,10 +521,10 @@ build_dummy_unc_pointer_types (Entity_Id gnat_desig_type, tree gnu_desig_type)
 = create_type_stub_decl (create_concat_name (gnat_desig_type, "XUP"),
 			 gnu_fat_type);
   fields = create_field_decl (get_identifier ("P_ARRAY"), gnu_ptr_array,
-			  gnu_fat_type, NULL_TREE, NULL_TREE, 0, 0);
+			  gnu_fat_type, NULL_TREE, NULL_TREE, 0, 1);
   DECL_CHAIN (fields)
 = create_field_decl (get_identifier ("P_BOUNDS"), gnu_ptr_template,
-			 gnu_fat_type, NULL_TREE, NULL_TREE, 0, 0);
+			 gnu_fat_type, NULL_TREE, NULL_TREE, 0, 1);
   finish_fat_pointer_type (gnu_fat_type, fields);
   SET_TYPE_UNCONSTRAINED_ARRAY (gnu_fat_type, gnu_desig_type);
   /* Suppress debug info until after the type is completed.  */

[Ada] Fix regression with partial rep clause on variant record type

[Eric Botcazou]

It is present on the mainline, 10 and 9 branches, and can yield an incorrect
layout when there is a partial representation clause on a discriminated record
type with a variant part.

Tested on x86-64/Linux, applied on mainline, 10 and 9 branches.


2021-02-03  Eric Botcazou  

* gcc-interface/decl.c (components_to_record): If the first component
with rep clause is the _Parent field with variable size, temporarily
set it aside when computing the internal layout of the REP part again.
* gcc-interface/utils.c (finish_record_type): Revert to taking the
maximum when merging sizes for all record types with rep clause.
(merge_sizes): Put SPECIAL parameter last and adjust recursive calls.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index 8120d4e33cf..aea191c7ecb 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -8330,12 +8330,12 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
   if (p_gnu_rep_list && gnu_rep_list)
 *p_gnu_rep_list = chainon (*p_gnu_rep_list, gnu_rep_list);
 
-  /* Deal with the annoying case of an extension of a record with variable size
- and partial rep clause, for which the _Parent field is forced at offset 0
- and has variable size, which we do not support below.  Note that we cannot
- do it if the field has fixed size because we rely on the presence of the
- REP part built below to trigger the reordering of the fields in a derived
- record type when all the fields have a fixed position.  */
+  /* Deal with the case of an extension of a record type with variable size and
+ partial rep clause, for which the _Parent field is forced at offset 0 and
+ has variable size.  Note that we cannot do it if the field has fixed size
+ because we rely on the presence of the REP part built below to trigger the
+ reordering of the fields in a derived record type when all the fields have
+ a fixed position.  */
   else if (gnu_rep_list
 	   && !DECL_CHAIN (gnu_rep_list)
 	   && TREE_CODE (DECL_SIZE (gnu_rep_list)) != INTEGER_CST
@@ -8353,33 +8353,52 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
  record, before the others, if we also have fields without rep clause.  */
   else if (gnu_rep_list)
 {
-  tree gnu_rep_type, gnu_rep_part;
-  int i, len = list_length (gnu_rep_list);
-  tree *gnu_arr = XALLOCAVEC (tree, len);
+  tree gnu_parent, gnu_rep_type;
 
   /* If all the fields have a rep clause, we can do a flat layout.  */
   layout_with_rep = !gnu_field_list
 			&& (!gnu_variant_part || variants_have_rep);
+
+  /* Same as above but the extension itself has a rep clause, in which case
+	 we need to set aside the _Parent field to lay out the REP part.  */
+  if (TREE_CODE (DECL_SIZE (gnu_rep_list)) != INTEGER_CST
+	  && !layout_with_rep
+	  && !variants_have_rep
+	  && first_free_pos
+	  && integer_zerop (first_free_pos)
+	  && integer_zerop (bit_position (gnu_rep_list)))
+	{
+	  gnu_parent = gnu_rep_list;
+	  gnu_rep_list = DECL_CHAIN (gnu_rep_list);
+	}
+  else
+	gnu_parent = NULL_TREE;
+
   gnu_rep_type
 	= layout_with_rep ? gnu_record_type : make_node (RECORD_TYPE);
 
-  for (gnu_field = gnu_rep_list, i = 0;
-	   gnu_field;
-	   gnu_field = DECL_CHAIN (gnu_field), i++)
-	gnu_arr[i] = gnu_field;
+  /* Sort the fields in order of increasing bit position.  */
+  const int len = list_length (gnu_rep_list);
+  tree *gnu_arr = XALLOCAVEC (tree, len);
+
+  gnu_field = gnu_rep_list;
+  for (int i = 0; i < len; i++)
+	{
+	  gnu_arr[i] = gnu_field;
+	  gnu_field = DECL_CHAIN (gnu_field);
+	}
 
   qsort (gnu_arr, len, sizeof (tree), compare_field_bitpos);
 
-  /* Put the fields in the list in order of increasing position, which
-	 means we start from the end.  */
   gnu_rep_list = NULL_TREE;
-  for (i = len - 1; i >= 0; i--)
+  for (int i = len - 1; i >= 0; i--)
 	{
 	  DECL_CHAIN (gnu_arr[i]) = gnu_rep_list;
 	  gnu_rep_list = gnu_arr[i];
 	  DECL_CONTEXT (gnu_arr[i]) = gnu_rep_type;
 	}
 
+  /* Do the layout of the REP part, if any.  */
   if (layout_with_rep)
 	gnu_field_list = gnu_rep_list;
   else
@@ -8388,14 +8407,36 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
 	= create_concat_name (gnat_record_type, "REP");
 	  TYPE_REVERSE_STORAGE_ORDER (gnu_rep_type)
 	= TYPE_REVERSE_STORAGE_ORDER (gnu_record_type);
-	  finish_record_type (gnu_rep_type, gnu_rep_list, 1, debug_info);
+	  finish_record_type (gnu_rep_type, gnu_rep_list, 1, false);
 
 	  /* If FIRST_FREE_POS is nonzero, we need to ensure that the fields
 	 without rep clause are laid out starting from this position.
 	 Therefore, we force it as a minimal size on the

Fix PR rtl-optimization/96015

[Eric Botcazou]

This is the miscompilation of Python on HP-PA/Linux present on the mainline 
and 10 branch, caused by the presence of a call to __builtin_unreachable () in 
the middle of a heavily branchy code, which confuses the reorg pass.

Bootstrapped/regtested on SPARC/Solaris, applied on mainline and 10 branch.


2021-02-09  Eric Botcazou 

PR rtl-optimization/96015
* reorg.c (skip_consecutive_labels): Minor comment tweaks.
(relax_delay_slots): When deleting a jump to the next active
instruction over a barrier, first delete the barrier if the
jump is the only way to reach the target label.

-- 
Eric Botcazoudiff --git a/gcc/reorg.c b/gcc/reorg.c
index 84beb9395aa..d468fa698ba 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -139,9 +139,9 @@ skip_consecutive_labels (rtx label_or_return)
   /* __builtin_unreachable can create a CODE_LABEL followed by a BARRIER.
 
  Since reaching the CODE_LABEL is undefined behavior, we can return
- any code label and we're OK at runtime.
+ any code label and we're OK at run time.
 
- However, if we return a CODE_LABEL which leads to a shrinked wrapped
+ However, if we return a CODE_LABEL which leads to a shrink-wrapped
  epilogue, but the path does not have a prologue, then we will trip
  a sanity check in the dwarf2 cfi code which wants to verify that
  the CFIs are all the same on the traces leading to the epilogue.
@@ -3175,6 +3175,23 @@ relax_delay_slots (rtx_insn *first)
 	  && ! condjump_in_parallel_p (jump_insn)
 	  && ! (next && switch_text_sections_between_p (jump_insn, next)))
 	{
+	  rtx_insn *direct_label = as_a (JUMP_LABEL (insn));
+	  rtx_insn *prev = prev_nonnote_insn (direct_label);
+
+	  /* If the insn jumps over a BARRIER and is the only way to reach
+		 its target, then we need to delete the BARRIER before the jump
+		 because, otherwise, the target may end up being considered as
+		 unreachable and thus also deleted.  */
+	  if (BARRIER_P (prev) && LABEL_NUSES (direct_label) == 1)
+		{
+		  delete_related_insns (prev);
+
+		  /* We have just removed a BARRIER, which means that the block
+		 number of the next insns has effectively been changed (see
+		 find_basic_block in resource.c), so clear it.  */
+		  clear_hashed_info_until_next_barrier (direct_label);
+		}
+
 	  delete_jump (jump_insn);
 	  continue;
 	}

[x86] Fix -freorder-blocks-and-partition glitch with Windows SEH

[Eric Botcazou]

Since GCC 8, the -freorder-blocks-and-partition pass can split a function into 
hot and cold parts, thus generating 2 CIEs for a single function in DWARF for 
exception purposes and doing an equivalent trick for Windows SEH on x86-64.

Now the Windows system unwinder is picky when it comes to the boundary between 
an active EH region and the end of the function and, therefore, a nop may need 
to be added in specific cases, see e.g. ix86_seh_fixup_eh_fallthru.

I overlooked that when implementing the -freorder-blocks-and-partition support 
back in 2018 and this results in e.g. a non-functioning Ada compiler when you 
do a profiled bootstrap.

Bootstrapped on x86-64/Windows, applied on all active branches as obvious.


2021-02-11  Eric Botcazou 

* config/i386/winnt.c (i386_pe_seh_unwind_emit): When switching to
the cold section, emit a nop before the directive if the previous
active instruction can throw.

-- 
Eric Botcazoudiff --git a/gcc/config/i386/winnt.c b/gcc/config/i386/winnt.c
index 962c88e3c1b..adc3f36f13b 100644
--- a/gcc/config/i386/winnt.c
+++ b/gcc/config/i386/winnt.c
@@ -1231,6 +1231,10 @@ i386_pe_seh_unwind_emit (FILE *asm_out_file, rtx_insn *insn)
   seh = cfun->machine->seh;
   if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS)
 {
+  /* See ix86_seh_fixup_eh_fallthru for the rationale.  */
+  rtx_insn *prev = prev_active_insn (insn);
+  if (prev && !insn_nothrow_p (prev))
+	fputs ("\tnop\n", asm_out_file);
   fputs ("\t.seh_endproc\n", asm_out_file);
   seh->in_cold_section = true;
   return;

Fix cast in df_worklist_dataflow_doublequeue

[Eric Botcazou]

The existing cast to float gives weird results in the RTL dump files on x86 
when the compiler is configured --with-fpmath=sse.

Bootstrapped on x86/Linux, applied on the mainline as obvious.


2021-02-15  Eric Botcazou 

* df-core.c (df_worklist_dataflow_doublequeue): Use proper cast.

-- 
Eric Botcazoudiff --git a/gcc/df-core.c b/gcc/df-core.c
index 8536e5e1f51..b4407eec460 100644
--- a/gcc/df-core.c
+++ b/gcc/df-core.c
@@ -1064,7 +1064,7 @@ df_worklist_dataflow_doublequeue (struct dataflow *dataflow,
 	 " n_basic_blocks %d n_edges %d"
 	 " count %d (%5.2g)\n",
 	 n_basic_blocks_for_fn (cfun), n_edges_for_fn (cfun),
-	 dcount, dcount / (float)n_basic_blocks_for_fn (cfun));
+	 dcount, dcount / (double)n_basic_blocks_for_fn (cfun));
 }
 
 /* Worklist-based dataflow solver. It uses sbitmap as a worklist,

Re: [PATCH 2/2] sparc: Run SUBTARGET_INIT_BUILTINS if it exists

[Eric Botcazou]

> Some subtargets don't provide the canonical function names as
> the symbol name in C libraries, and libcalls will only work if
> the builtins are patched to emit the correct library name.
> 
> For example, on NetBSD, cabsl has the symbol name __c99_cabsl,
> and the patching is done via netbsd_patch_builtin.
> 
> With this change, libgfortran.so is correctly built with a
> reference to __c99_cabsl, instead of "cabsl" which is not defined.

The change is OK modulo a couple of nits:

> gcc/ChangeLog:
> * config/sparc/sparc.c
>   (sparc_init_builtins): Call SUBTARGET_INIT_BUILTINS.

The name of the function on the first line.

> ---
>  gcc/config/sparc/sparc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
> index f3557936114..fe6475f2520 100644
> --- a/gcc/config/sparc/sparc.c
> +++ b/gcc/config/sparc/sparc.c
> @@ -10962,6 +10962,9 @@ sparc_init_builtins (void)
> 
>if (TARGET_VIS)
>  sparc_vis_init_builtins ();
> +#ifdef SUBTARGET_INIT_BUILTINS
> +  SUBTARGET_INIT_BUILTINS;
> +#endif
>  }

Missing blank line before the change.

-- 
Eric Botcazou

[x86] Fix PR target/99264

[Eric Botcazou]

Hi,

this wrong-code PR for the C++ compiler on x86-64/Windows is a regression
in GCC 9 and later, but the underlying issue has probably been there since
SEH was implemented and is exposed by this comment in config/i386/winnt.c:

  /* SEH records saves relative to the "current" stack pointer, whether
 or not there's a frame pointer in place.  This tracks the current
 stack pointer offset from the CFA.  */
  HOST_WIDE_INT sp_offset;

That's not what the (current) Microsoft documentation says; instead it says:

  /* SEH records offsets relative to the lowest address of the fixed stack
 allocation.  If there is no frame pointer, these offsets are from the
 stack pointer; if there is a frame pointer, these offsets are from the
 value of the stack pointer when the frame pointer was established, i.e.
 the frame pointer minus the offset in the .seh_setframe directive.

That's why the implementation is correct only under the condition that the 
frame pointer be established *after* the fixed stack allocation; as a matter 
of fact, that's clearly the model underpinning SEH, but is the opposite of 
what is done e.g. on Linux.

However the issue is mostly papered over in practice because:

  1. SEH forces use_fast_prologue_epilogue to false, which in turns forces 
save_regs_using_mov to false, so the general regs are always pushed when they 
need to be saved, which eliminates the offset computation for them.

  2. As soon as a frame is larger than 240 bytes, the frame pointer is fixed 
arbitrarily to 128 bytes above the stack pointer, which of course requires 
that it be established after the fixed stack allocation.

So you need a very small frame clobbering one of the call-saved XMM registers 
in order to generate wrong SEH unwind info.

The attached fix makes sure that the frame pointer is always established after 
the fixed stack allocation by pointing it at or below the lowest used register 
save area, i.e. the SSE save area, and removing the special early saves in the 
prologue; the end result is a uniform prologue sequence for SEH whatever the 
frame size.  And it avoids a weird discrepancy between cases where the number 
of saved general regs is even and cases where it is odd.

Tested on x86_64-w64-mingw32, OK for mainline, 10 and 9 branches?


2021-02-26 Eric Botcazou  

PR target/99264
* config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
point the hard frame pointer to the SSE register save area instead
of the general register save area.  Perform only minimal adjustment
for small frames if it is initially not correctly aligned.
(ix86_expand_prologue): Remove early saves for a SEH target.
* config/i386/winnt.c (struct seh_frame_state): Document constraint.


2021-02-26 Eric Botcazou  

* g++.dg/eh/seh-xmm-unwind.C: New test.

-- 
Eric Botcazoudiff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index a8f8bcd9105..25d943c1759 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6184,11 +6184,6 @@ ix86_compute_frame_layout (void)
   offset += frame->nregs * UNITS_PER_WORD;
   frame->reg_save_offset = offset;
 
-  /* On SEH target, registers are pushed just before the frame pointer
- location.  */
-  if (TARGET_SEH)
-frame->hard_frame_pointer_offset = offset;
-
   /* Calculate the size of the va-arg area (not including padding, if any).  */
   frame->va_arg_size = ix86_varargs_gpr_size + ix86_varargs_fpr_size;
 
@@ -6355,14 +6350,21 @@ ix86_compute_frame_layout (void)
  the unwind data structure.  */
   if (TARGET_SEH)
 {
-  HOST_WIDE_INT diff;
+  /* Force the frame pointer to point at or below the lowest register save
+	 area, see the SEH code in config/i386/winnt.c for the rationale.  */
+  frame->hard_frame_pointer_offset = frame->sse_reg_save_offset;
 
-  /* If we can leave the frame pointer where it is, do so.  Also, returns
+  /* If we can leave the frame pointer where it is, do so.  Also, return
 	 the establisher frame for __builtin_frame_address (0).  */
-  diff = frame->stack_pointer_offset - frame->hard_frame_pointer_offset;
-  if (diff <= SEH_MAX_FRAME_SIZE
-	  && (diff > 240 || (diff & 15) != 0)
-	  && !crtl->accesses_prior_frames)
+  const HOST_WIDE_INT diff
+	= frame->stack_pointer_offset - frame->hard_frame_pointer_offset;
+  if (diff <= 255)
+	{
+	  /* The resulting diff will be a multiple of 16 lower than 255,
+	 i.e. at most 240 as required by the unwind data structure.  */
+	  frame->hard_frame_pointer_offset += (diff & 15);
+	}
+  else if (diff <= SEH_MAX_FRAME_SIZE && !crtl->accesses_prior_frames)
 	{
 	  /* Ideally we'd determine what portion of the local stack frame
 	 (within the constraint of the lowest 240) is most heavily used.
@@ -8070,17 +8072,6 @@ ix86_expand_prologue (void

[Ada] Fix PR ada/99095

[Eric Botcazou]

This is a regression present on the mainline and 10 branch, where we fail
to make the bounds explicit for the return value of a function returning
an unconstrained array of a limited record type.

Tested on x86-64/Linux, applied on mainline and 10 branch.


2021-03-02  Eric Botcazou  

PR ada/99095
* sem_ch8.adb (Check_Constrained_Object): Restrict again the special
optimization for limited types to non-array types except in the case
of an extended return statement.


2021-03-02  Eric Botcazou  

* gnat.dg/limited5.adb: New test.

-- 
Eric Botcazoudiff --git a/gcc/ada/sem_ch8.adb b/gcc/ada/sem_ch8.adb
index 4689ae4ba18..efff7145337 100644
--- a/gcc/ada/sem_ch8.adb
+++ b/gcc/ada/sem_ch8.adb
@@ -830,11 +830,19 @@ package body Sem_Ch8 is
 --  that are used in iterators. This is an optimization, but it
 --  also prevents typing anomalies when the prefix is further
 --  expanded.
+
 --  Note that we cannot just use the Is_Limited_Record flag because
 --  it does not apply to records with limited components, for which
 --  this syntactic flag is not set, but whose size is also fixed.
 
-elsif Is_Limited_Type (Typ) then
+--  Note also that we need to build the constrained subtype for an
+--  array in order to make the bounds explicit in most cases, but
+--  not if the object comes from an extended return statement, as
+--  this would create dangling references to them later on.
+
+elsif Is_Limited_Type (Typ)
+  and then (not Is_Array_Type (Typ) or else Is_Return_Object (Id))
+then
null;
 
 else
--  { dg-do compile }

procedure Limited5 is

   type Command   is limited null record;
   type Command_Array is array (Positive range <>) of Command;
  
   function To_Commands return Command_Array is
   begin
  return Result : Command_Array (1 .. 2);
   end To_Commands;
   
   The_Commands : aliased Command_Array := To_Commands;

begin
   null;
end;

Re: [x86] Fix PR target/99264

[Eric Botcazou]

> LGTM.

Thanks.  However, I overlooked an issue with pathologically large frames 
(larger than SEH_MAX_FRAME_SIZE, i.e. 2GB and for which we cannot emit CFI 
directives) that is visible in the gnat.dg testsuite under the form of an ICE.

Tested on x86_64-w64-mingw32, applied on mainline/10/9 branches as obvious.


2021-03-03 Eric Botcazou  

PR target/99234
* config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
point back the hard frame pointer to its default location when the
frame is larger than SEH_MAX_FRAME_SIZE.

-- 
Eric Botcazoudiff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e85b06b6824..6672da597ba 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6660,7 +6660,8 @@ ix86_compute_frame_layout (void)
   frame->hard_frame_pointer_offset = frame->sse_reg_save_offset;
 
   /* If we can leave the frame pointer where it is, do so.  Also, return
-	 the establisher frame for __builtin_frame_address (0).  */
+	 the establisher frame for __builtin_frame_address (0) or else if the
+	 frame overflows the SEH maximum frame size.  */
   const HOST_WIDE_INT diff
 	= frame->stack_pointer_offset - frame->hard_frame_pointer_offset;
   if (diff <= 255)
@@ -6678,6 +6679,8 @@ ix86_compute_frame_layout (void)
 	 frame that is addressable with 8-bit offsets.  */
 	  frame->hard_frame_pointer_offset = frame->stack_pointer_offset - 128;
 	}
+  else
+	frame->hard_frame_pointer_offset = frame->hfp_save_offset;
 }
 }
 

Re: [PATCH] sparcv9: Disable -Wuninitialized warnings breaking bootstrap [PR92002]

[Eric Botcazou]

> sparcv9 bootstrap has been broken for 1 1/2 years now by spurious
> -Wuninitialized warnings:

IIRC we used to have 3 of them, now we have 2 so there is some progress. ;-)

> Before we ship yet another release with this issue, I suggest to at
> least include a workaround of demoting them to warnings.
> 
> Tested on sparcv9-sun-solaris2.11.
> 
> Ok for master?

Sure.

> I originally meant to propose the patch for the gcc-10 branch as well,
> but when I tried a sparcv9-sun-solaris2.11 bootstrap there some time
> ago, it wasn't affected any longer.

But release branches do not have -Werror set, do they?

-- 
Eric Botcazou

[patch] Fix PR rtl-optimization/99376

[Eric Botcazou]

Hi,

this is an undefined behavior spotted by the sanitizer that has managed to go 
unnoticed until now.  Tested on x86-64/Linux, OK for the mainline?


2021-03-04  Eric Botcazou  

PR rtl-optimization/99376
* rtlanal.c (nonzero_bits1) : If the number
of low-order zero bits is too large, set the result to 0 directly.

-- 
Eric Botcazoudiff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index d1240b0b7c5..a8ea1d72636 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5053,11 +5053,17 @@ nonzero_bits1 (const_rtx x, scalar_int_mode mode, const_rtx known_x,
 	gcc_unreachable ();
 	  }
 
+	/* Note that mode_width <= HOST_BITS_PER_WIDE_INT, see above.  */
 	if (result_width < mode_width)
 	  nonzero &= (HOST_WIDE_INT_1U << result_width) - 1;
 
 	if (result_low > 0)
-	  nonzero &= ~((HOST_WIDE_INT_1U << result_low) - 1);
+	  {
+	if (result_low < HOST_BITS_PER_WIDE_INT)
+	  nonzero &= ~((HOST_WIDE_INT_1U << result_low) - 1);
+	else
+	  nonzero = 0;
+	  }
   }
   break;
 

[Ada] Fix PR ada/99264

[Eric Botcazou]

This fixes the build breakage introduced by the latest glibc release.

Tested on x86-64/Linux, applied on mainline, 10 and 9 branches.


2021-03-05  Eric Botcazou  

PR ada/99264
* init.c (__gnat_alternate_sta) [Linux]: Remove preprocessor test on
MINSIGSTKSZ and bump size to 32KB.
* libgnarl/s-osinte__linux.ads (Alternate_Stack_Size): Bump to 32KB.

-- 
Eric Botcazoudiff --git a/gcc/ada/init.c b/gcc/ada/init.c
index e76aa79c5a8..3ceb1a31b02 100644
--- a/gcc/ada/init.c
+++ b/gcc/ada/init.c
@@ -579,12 +579,8 @@ __gnat_error_handler (int sig, siginfo_t *si ATTRIBUTE_UNUSED, void *ucontext)
 
 #ifndef __ia64__
 #define HAVE_GNAT_ALTERNATE_STACK 1
-/* This must be in keeping with System.OS_Interface.Alternate_Stack_Size.
-   It must be larger than MINSIGSTKSZ and hopefully near 2 * SIGSTKSZ.  */
-# if 16 * 1024 < MINSIGSTKSZ
-#  error "__gnat_alternate_stack too small"
-# endif
-char __gnat_alternate_stack[16 * 1024];
+/* This must be in keeping with System.OS_Interface.Alternate_Stack_Size.  */
+char __gnat_alternate_stack[32 * 1024];
 #endif
 
 #ifdef __XENO__
diff --git a/gcc/ada/libgnarl/s-osinte__linux.ads b/gcc/ada/libgnarl/s-osinte__linux.ads
index f7af00bf5e2..2272f83d68d 100644
--- a/gcc/ada/libgnarl/s-osinte__linux.ads
+++ b/gcc/ada/libgnarl/s-osinte__linux.ads
@@ -328,7 +328,7 @@ package System.OS_Interface is
   oss : access stack_t) return int;
pragma Import (C, sigaltstack, "sigaltstack");
 
-   Alternate_Stack_Size : constant := 16 * 1024;
+   Alternate_Stack_Size : constant := 32 * 1024;
--  This must be in keeping with init.c:__gnat_alternate_stack
 
Alternate_Stack : aliased char_array (1 .. Alternate_Stack_Size);

Re: [pushed] c++: Fix class NTTP constness handling [PR98810]

[Eric Botcazou]

> Here, when substituting still-dependent args into an alias template, we see
> a non-const type because the default argument is non-const, and is not a
> template parm object because it's still dependent.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> gcc/cp/ChangeLog:
> 
>   PR c++/98810
>   * pt.c (tsubst_copy) [VIEW_CONVERT_EXPR]: Add const
>   to a class non-type template argument that needs it.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c++/98810
>   * g++.dg/cpp2a/nontype-class-defarg1.C: New test.

This apparently went down to the 9 branch as well and introduced:

Running /home/eric/cvs/gcc-9/gcc/testsuite/g++.dg/dg.exp ...
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++98: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++14: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "
ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

ERROR: g++.dg/cpp2a/nontype-class-defarg1.C  -std=c++17: syntax error in 
target selector "target c++20" for " dg-do 2 compile { target c++20 } "

On the contrary, the 10 branch is a in good shape.

-- 
Eric Botcazou

Re: [pushed] c++: Fix class NTTP constness handling [PR98810]

[Eric Botcazou]

> GCC 9 doesn't have the "c++20" target yet.
> 
>   * g++.dg/cpp2a/nontype-class-defarg1.C: Use target c++2a.

Thanks!

-- 
Eric Botcazou

Re: [PATCH v2] fix Ada bootstrap on Cygwin64 (PR bootstrap/94918)

[Eric Botcazou]

> I wonder why we include  from this file at all,
> and why it is not included from {t,}system.h instead which
> is where system header specific fixups should be made
> (and this one could be avoided because system headers
> are included _before_ poisoning anything).

 is the Mother of All Things on the platform so you don't want to 
include it liberally (although it can be tamed e.g. with WIN32_LEAN_AND_MEAN).
Therefore including it from tsystem.h might be worse than the actual disease.

Mikael, can you work around the problem by adding

#ifdef __CYGWIN__
#include "mingw32.h"
#endif

at the appropriate spot in raise-gcc.c instead?

-- 
Eric Botcazou

[patch] Fix PR C++/90448

[Eric Botcazou]

Hi,

this is a regression present on the mainline and 10 branch for architectures
that pass all structure types by reference, e.g. 32-bit PowerPC or SPARC.

Jakub posted a detailed analysis in the audit trail and this boils down to
the RTL expander trying to take the address of a DECL whose RTX is a register.

Bootstrapped/regtested on x86-64/Linux, PowerPC64/Linux and SPARC/Solaris,
OK for the mainline and 10 branch?


2021-03-08  Eric Botcazou  

PR C++/90448
* calls.c (initialize_argument_information): When the argument
is passed by reference, do not make a copy in a thunk only if
the argument is already in memory.  Remove redundant test for
the case of callee copy.

-- 
Eric Botcazoudiff --git a/gcc/calls.c b/gcc/calls.c
index 1fea022ad8a..ff606204772 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -2388,19 +2388,17 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
   function_arg_info arg (type, argpos < n_named_args);
   if (pass_by_reference (args_so_far_pnt, arg))
 	{
-	  bool callee_copies;
-	  tree base = NULL_TREE;
-
-	  callee_copies = reference_callee_copied (args_so_far_pnt, arg);
-
-	  /* If we're compiling a thunk, pass through invisible references
-	 instead of making a copy.  */
-	  if (call_from_thunk_p
-	  || (callee_copies
-		  && !TREE_ADDRESSABLE (type)
-		  && (base = get_base_address (args[i].tree_value))
-		  && TREE_CODE (base) != SSA_NAME
-		  && (!DECL_P (base) || MEM_P (DECL_RTL (base)
+	  const bool callee_copies
+	= reference_callee_copied (args_so_far_pnt, arg);
+	  tree base;
+
+	  /* If we're compiling a thunk, pass directly the address of an object
+	 already in memory, instead of making a copy.  Likewise if we want
+	 to make the copy in the callee instead of the caller.  */
+	  if ((call_from_thunk_p || callee_copies)
+	  && (base = get_base_address (args[i].tree_value))
+	  && TREE_CODE (base) != SSA_NAME
+	  && (!DECL_P (base) || MEM_P (DECL_RTL (base
 	{
 	  /* We may have turned the parameter value into an SSA name.
 		 Go back to the original parameter so we can take the

[patch] Fix PR fortran/96983

[Eric Botcazou]

Hi,

AFAICS the code in build_round_expr implicitly assumes that __float128 exists, 
which is *not* the common case among 64-bit architectures since "long double" 
is generally already 128-bit for them.

Tested on x86-64/Linux and SPARC64/Linux, OK for the mainline?


2021-03-08  Eric Botcazou  

PR fortran/96983
* trans-intrinsic.c (build_round_expr): Do not implicitly assume
that __float128 is the 128-bit floating-point type.

-- 
Eric Botcazoudiff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 5c9258c65c3..ae359a07973 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -407,7 +407,10 @@ build_round_expr (tree arg, tree restype)
   if (kind < 0)
 	gfc_internal_error ("Could not find real kind with at least %d bits",
 			resprec);
-  arg = fold_convert (gfc_float128_type_node, arg);
+  if (gfc_real16_is_float128)
+	arg = fold_convert (gfc_float128_type_node, arg);
+  else
+	arg = fold_convert (long_double_type_node, arg);
   fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind);
 }
   else

Re: [PATCH v2] fix Ada bootstrap on Cygwin64 (PR bootstrap/94918)

[Eric Botcazou]

> This one worked. Is that what you had in mind?
> 
> * raise-gcc.c: On Cygwin include mingw32.h to prevent
> windows.h from including x86intrin.h or emmintrin.h.

Yep, exactly, thanks, you may put it on whichever branch you need.

-- 
Eric Botcazou

Re: [patch] Fix PR C++/90448

[Eric Botcazou]

> The whole point of thunks is that they do not require things like copying
> ... is this case somehow IPA-SRA/CPed in an odd way?  Otherwise it seems
> like the passed through reference was mishandled on the GIMPLE level?

Maybe, but the change looks correct on its own and nobody has cared about the 
issue for months, so please let's not the best be the enemy of the good...

-- 
Eric Botcazou

Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Eric Botcazou]

> It was pointed out in PR101598 to be inappropriate, that
> ignored Ada decls receive the source line number which was
> recorded in the function decl's DECL_SOURCE_LOCATION.
> Therefore set all front-end-generated Ada decls with
> DECL_IGNORED_P to UNKNOWN_LOCATION.
> 
> 2021-07-24  Bernd Edlinger  
> 
>   PR debug/101598
>   * gcc-interface/trans.c (Subprogram_Body_to_gnu): Set the
>   DECL_SOURCE_LOCATION of DECL_IGNORED_P gnu_subprog_decl to
>   UNKNOWN_LOCATION.

Is that really needed in DWARF 5?  If no, I'm not sure that we want it.

-- 
Eric Botcazou

Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Eric Botcazou]

> The location of these ignored Ada decls looks completely sane to me.
> However, it was an unintentional side effect of the patch to allow
> minimal debugging of ignored decls.  This means we can now step into
> those functions or set line breakpoints there, while previously that
> was not possible.  And I guess it could be considered an improvement.
> 
> So it's your choice, how you want these functions to be debugged.

The requirement on the GDB side is that these functions *cannot* be stepped 
into, i.e. that they be completely transparent for the GDB user.  But we still 
want to have location information in the compiler itself to debug it.

-- 
Eric Botcazou

Fix PR tree-optimization/101626

[Eric Botcazou]

This is a regression present on the mainline, caused by an oversight of mine 
in an earlier fix for SRA, whereby I forgot to exclude cases for reverse SSO.

Tested on x86-64/Linux, applied on the mainline as obvious.


2021-08-05  Eric Botcazuo  

PR tree-optimization/101626
* tree-sra.c (propagate_subaccesses_from_rhs): Do not set the
reverse scalar storage order on a pointer or vector component.


2021-08-05  Eric Botcazuo  

* gcc.dg/sso-15.c: New test.

-- 
Eric Botcazoudiff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index c05d22f3e8f..3a9e14f50a0 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -2790,7 +2790,10 @@ propagate_subaccesses_from_rhs (struct access *lacc, struct access *racc)
 	{
 	  /* We are about to change the access type from aggregate to scalar,
 	 so we need to put the reverse flag onto the access, if any.  */
-	  const bool reverse = TYPE_REVERSE_STORAGE_ORDER (lacc->type);
+	  const bool reverse
+	= TYPE_REVERSE_STORAGE_ORDER (lacc->type)
+	  && !POINTER_TYPE_P (racc->type)
+	  && !VECTOR_TYPE_P (racc->type);
 	  tree t = lacc->base;
 
 	  lacc->type = racc->type;
/* { dg-do compile } */
/* { dg-options "-O2" } */

#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
#define REV_ENDIANNESS __attribute__((scalar_storage_order("big-endian")))
#else
#define REV_ENDIANNESS __attribute__((scalar_storage_order("little-endian")))
#endif

struct X { int *p; } REV_ENDIANNESS;

struct X x;

struct X __attribute__((noinline)) foo (int *p)
{
  struct X x;
  x.p = p;
  return x;
}

void __attribute((noinline)) bar (void)
{
  *x.p = 1;
}

extern void abort (void);

int main (void)
{
  int i = 0;
  x = foo(&i);
  bar();
  if (i != 1)
abort ();
  return 0;
}

Re: [PATCH] Fix ICE when mixing VLAs and statement expressions [PR91038]

[Eric Botcazou]

> I think the patch makes sense but the comment says
> 
>  Java requires that we elaborated nodes in source order.  That
>  means we must gimplify the inner expression followed by each of
>  the indices, in order.  But we can't gimplify the inner
>  expression until we deal with any variable bounds, sizes, or
>  positions in order to deal with PLACEHOLDER_EXPRs.
> 
> and I don't really understand that (not to say Java is no longer supported
> by GCC, but Ada does use PLACEHOLDER_EXPRs).  In fact the comment
> suggests that we _should_ gimplify the inner expression first ... at least
> it seems to after your explanations ;)

We already gimplify the inner expression first, i.e. before indices and 
operands, but we gimplify the "annotations" (special operands) before.

> Eric - do you know anything about this?

If the comment is right, then Martin's patch should break Ada indeed.

-- 
Eric Botcazou

Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Eric Botcazou]

> But it is okay that we can set a breakpoint on defs__struct1IP,
> in the test case of PR 101598.
> And the debugger shall only show assembler here.
> Right?

The defs__struct1IP procedure should be totally transparent for the user, so 
setting a breakpoint in it is not supposed to come into play.

> Do you have an example where this location information is used in the
> compiler itself for debugging?

That's useful when you compile the code with -gnatD, i.e when you debug the 
intermediate code generated by the front-end.

> I assume You would agree that having the location for Test2 is better
> than no debug info at all?

But we want no debug info at all for these IP functions.

> What do you think?

I guess I still don't understand why DECL_IGNORED_P was changed.

-- 
Eric Botcazou

Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Eric Botcazou]

> ISTR it was changed because at least with location info generated
> by gas there's no way to have "no location" for a portion of code.
> Instead the assigned location will be that of the previous .loc
> directive which results in random and confusing results for the
> pc range of the DECL_INGORED_P function.

Yes in the general case, but no if you put them at the beginning of the 
assembly file (what the Ada compiler precisely does), at least if you do not 
pass any -gdwarf-n switch now.  This had worked for 2 decades at least...

> I guess we should really revisit the decision to rely on gas
> to produce line info.  What's the advantage of doing so (apart
> from "nice" annotated assembly)?

Not a small advantage in my opinion, and I don't think that we want to change 
it because of a corner case in Ada in any case.

I guess Bernd's patch is acceptable, modulo a small tweak for -gnatD.  Let me 
experiment a little bit though.

-- 
Eric Botcazou

Re: [PATCH] Fix ICE when mixing VLAs and statement expressions [PR91038]

[Eric Botcazou]

> Yes, it breaks Ada. I already found this out in the meanwhile.

OK, thanks for checking.

> My understanding of this is that this is for referring
> to some field of an outer struct which is then used in the
> size expression, e.g. something like this (using
> C syntax):
> 
> struct foo {
>   int len;
>   float (*x)[3][len];
> };

Yes, just

struct foo {
  int len;
  float a[len];
}

but it's also used for unconstrained array types and this seems to be the 
problem here, e.g. for:

package Vect1 is

   type Varray is array (Integer range <>) of Long_Float;
   for Varray'Alignment use 16;

   procedure Add (X, Y : not null access Varray; R : not null access Varray);

end Vect1;

package body Vect1 is

   procedure Add (X, Y : not null access Varray; R : not null access Varray) 
is
   begin
  for I in X'Range loop
 R(I) := X(I) + Y(I);
  end loop;
   end;

end Vect1;

> But then why would you gimplify the size expression before
> the base expression?  I would assume that you also want
> to process the base expression first, because it is the
> source of the struct which we access in the size expression.

For the above testcase, we have a dangling PLACEHOLDER_EXPRs

+===GNAT BUG DETECTED==+
| 12.0.0 20210805 (experimental) [master revision e314cfc371d:
5eb84b79079:ead235f60139edc6eb408d8d083cbb15e417b447] (x86_64-suse-linux) GCC 
error:|
| in gimplify_expr, at gimplify.c:15019|
| Error detected around vect1.adb:6:23

probably because array_ref_low_bound, where it is normally eliminated, cannot 
do its jub properly when it is called after the base expression is gimplified.

-- 
Eric Botcazou

Re: [PATCH 2/2] Ada: Remove debug line number for DECL_IGNORED_P functions

[Eric Botcazou]

> Ah, thanks, I tried it but the defs__struct1IP function has
> DECL_IGNORED_P = false when I build it with -gnatD.
> 
> So that would not be affected by setting DECL_SOURCE_LOCATION to
> UNKNOWN_LOCATION as done in the proposed patch, because that is only
> done for functions with DECL_IGNORED_P = true.

Ouch, I should have checked it myself...  Thanks for doing the investigation.

> Then we have ordinary functions with DECL_IGNORED_P = false,
> but they are morphed into a thunk calling a similar looking function
> and the thunk has now DECL_IGNORED_P = true, and all debug info
> removed, except the DECL_SOURCE_LOCATION.  To make this even worse,
> the similar looking function is inlined into the thunk again, and
> all debug info is again stripped away.
> 
> And when this happens it is desirable to at least see the source line where
> the original function was declared.
> 
> 
> As an example, please consider this test case:
> 
> $ cat test.ads
> package test is
> 
>type Func_Ptr is access function (X : Integer) return Integer;
> 
>function Test1 (X : Integer) return Integer;
>function Test2 (X : Integer) return Integer;
>function DoIt (X : Integer; Func : Func_Ptr) return Integer;
> 
> end test;
> $ cat test.ads
> package test is
> 
>type Func_Ptr is access function (X : Integer) return Integer;
> 
>function Test1 (X : Integer) return Integer;
>function Test2 (X : Integer) return Integer;
>function DoIt (X : Integer; Func : Func_Ptr) return Integer;
> 
> end test;
> 
> $ cat test.adb
> package body test is
> 
>function Test1 (X : Integer) return Integer is
>begin
>   return X;
>end Test1;
> 
>function Test2 (X : Integer) return Integer is
>begin
>   return X;
>end Test2;
> 
>function DoIt (X : Integer; Func : Func_Ptr) return Integer is
>begin
>   return Func (X);
>end DoIt;
> 
> end test;
> 
> $ cat main.adb
> with Ada.Text_IO; use Ada.Text_IO;
> with test; use test;
> 
> procedure Main is
> 
>X : Integer := 7;
>Y : Integer := DoIt (X, Test1'Access);
>Z : Integer := DoIt (X, Test2'Access);
> 
> begin
>Put_Line (X'Img & " " & Y'Img & " " & Z'Img);
> end Main;
> 
> $ gnatmake -f -g -O2 main.adb -save-temp -fdump-tree-all-lineno
> 
> Now Test1 and Test2 are ordinary functions, but Test2 ends up as
> DECL_IGNORED_P = true, that happens between test.adb.052t.local-fnsummary2
> and test.adb.092t.fixup_cfg3:

Ouch (bis).  Quite unexpected that DECL_IGNORED_P is set out of nowhere.  It's 
Identical Code Folding which turns Test2 into a thunk?

> in test.adb.052t.local-fnsummary2 we have:
> 
> integer test.test1 (integer x)
> {
>[local count: 1073741824]:
>   [test.adb:5:7] return x_1(D);
> 
> }
> 
> and
> 
> integer test.test2 (integer x)
> {
>[local count: 1073741824]:
>   [test.adb:10:7] return x_1(D);
> 
> }
> 
> 
> but in test.adb.092t.fixup_cfg3
> 
> we have
> 
> integer test.test1 (integer x)
> {
>[local count: 1073741824]:
>   [test.adb:5:7] return x_1(D);
> 
> }
> 
> and
> 
> integer test.test2 (integer x)
> {
>   integer D.4674;
>   integer retval.5;
>   integer _4;
> 
>[local count: 1073741824]:
>   # DEBUG x => x_1(D)
>   _4 = x_1(D);
>   # DEBUG x => NULL
>   retval.5_2 = _4;
>   return retval.5_2;
> 
> }
> 
> 
> the line numbers are gone, and the function has DECL_IGNORED_P,
> but still a useful DECL_SOURCE_LOCATION, I don't know
> where the DECL_SOURCE_LOCATION can be seen in the dump files,
> but from debugging this effect, I know that quite well.
> 
> This second effect is why as a special case DECL_IGNORED_P functions
> with valid looking DECL_SOURCE_LOCATION have now a .loc statement,
> because that is less surprising to a user than having no line numbers
> at all here.

OK, thanks for the explanation, the patch is OK then.

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> The question is whether we instead want to amend build3 to
> set TREE_THIS_VOLATILE automatically when the FIELD_DECL has
> it set.  At least for the Fortran FE cases the gimplifier
> fails to see some volatile references and thus can generate
> wrong code which is a latent issue.

What do we do for other similar flags, e.g. TREE_READONLY?

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> build3 currently does no special processing for the FIELD_DECL operand,
> it just sets TREE_THIS_VOLATILE from operand zero for tcc_references.
> 
> The C and C++ frontends have repeated patterns like
> 
>   ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
> NULL_TREE);
>   SET_EXPR_LOCATION (ref, loc);
>   if (TREE_READONLY (subdatum)
> 
>   || (use_datum_quals && TREE_READONLY (datum)))
> 
> TREE_READONLY (ref) = 1;
>   if (TREE_THIS_VOLATILE (subdatum)
> 
>   || (use_datum_quals && TREE_THIS_VOLATILE (datum)))
> 
> TREE_THIS_VOLATILE (ref) = 1;

Likewise in the Ada front-end (gigi).

> Now - I wonder if there's a reason a frontend might _not_ want to
> set TREE_THIS_VOLATILE on a COMPONENT_REF when the FIELD_DECL has
> TREE_THIS_VOLATILE set.

This would be weird semantics in my opinion.

> I guess I'll do one more experiment and add verification that
> TREE_THIS_VOLATILE on COMPONENT_REFs and FIELD_DECLs is consistent
> and see where that trips.

Sounds good to me.

-- 
Eric Botcazou

Re: [PATCH][v2] Adjust volatile handling of the operand scanner

[Eric Botcazou]

> So I'm leaning towards leaving build3 alone and fixing up frontends
> as issues pop up.

FWIW fine with me.

-- 
Eric Botcazou

Small tweak to expand_used_vars

[Eric Botcazou]

This completes the replacement of DECL_ATTRIBUTES (current_function_decl) with 
the attribs local variable.

Tested on x86-64/Linux, applied on the mainline as obvious.


2021-08-11  Eric Botcazuo  

* cfgexpand.c (expand_used_vars): Reuse attribs local variable.

-- 
Eric Botcazoudiff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 818328071db..03260b019e5 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -2294,22 +2294,19 @@ expand_used_vars (bitmap forced_stack_vars)
 	if (gen_stack_protect_signal
 	|| cfun->calls_alloca
 	|| has_protected_decls
-	|| lookup_attribute ("stack_protect",
- DECL_ATTRIBUTES (current_function_decl)))
+	|| lookup_attribute ("stack_protect", attribs))
 	  create_stack_guard ();
 	break;
 
   case SPCT_FLAG_DEFAULT:
 	if (cfun->calls_alloca
 	|| has_protected_decls
-	|| lookup_attribute ("stack_protect",
- DECL_ATTRIBUTES (current_function_decl)))
+	|| lookup_attribute ("stack_protect", attribs))
 	  create_stack_guard ();
 	break;
 
   case SPCT_FLAG_EXPLICIT:
-	if (lookup_attribute ("stack_protect",
-			  DECL_ATTRIBUTES (current_function_decl)))
+	if (lookup_attribute ("stack_protect", attribs))
 	  create_stack_guard ();
 	break;
 

[patch] Make -no-pie option work for native Windows

[Eric Botcazou]

Hi,

as already mentioned on the list, binutils 2.36 generates PIE executables by
default on native Windows (because --dynamicbase is the default) so it makes
sense to have a simple way to counter that and -no-pie seems appropriate,
all the more so that it is automatically passed when building the compiler.

Bootstrapped on x86 and x86-64/Windows, w/ and w/o binutils 2.36, OK for the
mainline and 11 branch?


2021-08-11  Eric Botcazou  

* configure.ac (PE linker --disable-dynamicbase support): New check.
* configure: Regenerate.
* config.in: Likewise.
* config/i386/mingw32.h (LINK_SPEC_DISABLE_DYNAMICBASE): New define.
(LINK_SPEC): Use it.
* config/i386/mingw-w64.h (LINK_SPEC_DISABLE_DYNAMICBASE): Likewise.
(LINK_SPEC): Likewise.

-- 
Eric Botcazoudiff --git a/gcc/config/i386/mingw-w64.h b/gcc/config/i386/mingw-w64.h
index 0cec6b02787..6cc7ac54fdd 100644
--- a/gcc/config/i386/mingw-w64.h
+++ b/gcc/config/i386/mingw-w64.h
@@ -89,6 +89,14 @@ along with GCC; see the file COPYING3.  If not see
 # define LINK_SPEC_LARGE_ADDR_AWARE ""
 #endif
 
+#undef LINK_SPEC_DISABLE_DYNAMICBASE
+#if HAVE_LD_PE_DISABLE_DYNAMICBASE
+# define LINK_SPEC_DISABLE_DYNAMICBASE \
+  "%{!shared:%{!mdll:%{no-pie:--disable-dynamicbase}}}"
+#else
+# define LINK_SPEC_DISABLE_DYNAMICBASE ""
+#endif
+
 #undef LINK_SPEC
 #define LINK_SPEC SUB_LINK_SPEC " %{mwindows:--subsystem windows} \
   %{mconsole:--subsystem console} \
@@ -97,6 +105,7 @@ along with GCC; see the file COPYING3.  If not see
   %{static:-Bstatic} %{!static:-Bdynamic} \
   %{shared|mdll: " SUB_LINK_ENTRY " --enable-auto-image-base} \
   " LINK_SPEC_LARGE_ADDR_AWARE "\
+  " LINK_SPEC_DISABLE_DYNAMICBASE "\
   %(shared_libgcc_undefs)"
 
 /* Enable sincos optimization, overriding cygming.h.  sincos, sincosf
diff --git a/gcc/config/i386/mingw32.h b/gcc/config/i386/mingw32.h
index 36e7bae5e1b..779c9335711 100644
--- a/gcc/config/i386/mingw32.h
+++ b/gcc/config/i386/mingw32.h
@@ -148,6 +148,13 @@ along with GCC; see the file COPYING3.  If not see
   "%{!shared:%{!mdll:%{!m64:--large-address-aware}}}"
 #endif
 
+#if HAVE_LD_PE_DISABLE_DYNAMICBASE
+# define LINK_SPEC_DISABLE_DYNAMICBASE \
+  "%{!shared:%{!mdll:%{no-pie:--disable-dynamicbase}}}"
+#else
+# define LINK_SPEC_DISABLE_DYNAMICBASE ""
+#endif
+
 #define LINK_SPEC "%{mwindows:--subsystem windows} \
   %{mconsole:--subsystem console} \
   %{shared: %{mdll: %eshared and mdll are not compatible}} \
@@ -155,6 +162,7 @@ along with GCC; see the file COPYING3.  If not see
   %{static:-Bstatic} %{!static:-Bdynamic} \
   %{shared|mdll: " SUB_LINK_ENTRY " --enable-auto-image-base} \
   " LINK_SPEC_LARGE_ADDR_AWARE "\
+  " LINK_SPEC_DISABLE_DYNAMICBASE "\
   %(shared_libgcc_undefs)"
 
 /* Include in the mingw32 libraries with libgcc */
diff --git a/gcc/configure.ac b/gcc/configure.ac
index c8e0d63fe70..653a1cc561d 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -6383,6 +6383,23 @@ case $target_os in
 	[Define if the PE linker has broken DWARF 5 support.])
 fi
 AC_MSG_RESULT($gcc_cv_ld_broken_pe_dwarf5)
+
+AC_MSG_CHECKING(PE linker --disable-dynamicbase support)
+gcc_cv_ld_disable_dynamicbase=no
+if test $in_tree_ld = yes; then
+  if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 36 -o "$gcc_cv_gld_major_version" -gt 2; then \
+gcc_cv_ld_disable_dynamicbase=yes
+  fi
+else
+  if $gcc_cv_ld --help 2>&1 | grep -q 'disable\-]dynamicbase' > /dev/null; then
+gcc_cv_ld_disable_dynamicbase=yes
+  fi
+fi
+if test x"$gcc_cv_ld_disable_dynamicbase" = xyes; then
+  AC_DEFINE(HAVE_LD_PE_DISABLE_DYNAMICBASE, 1,
+[Define if the PE linker supports --disable-dynamicbase option.])
+fi
+AC_MSG_RESULT($gcc_cv_ld_disable_dynamicbase)
 ;;
 esac
 

Re: [patch] Make -no-pie option work for native Windows

[Eric Botcazou]

> Looks good to me. Do you have push permissions?

Thanks.  Yes, see the preceding message on gcc-patches@, so applied.

-- 
Eric Botcazou

Disable GNAT encodings by default

[Eric Botcazou]

Given the latest work in the compiler and debugger, we no longer need to use
most GNAT-specific encodings in the debug info generated for an Ada program,
so the attached patch disables them, except with -fgnat-encodings=all.

Tested on x86-64/Linux, applied on the mainline as obvious.


2021-08-16  Eric Botcazou  

* dwarf2out.c (add_data_member_location_attribute): Use GNAT
encodings only when -fgnat-encodings=all is specified.
(add_bound_info): Likewise.
(add_byte_size_attribute): Likewise.
(gen_member_die): Likewise.

-- 
Eric Botcazoudiff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b91a9b5abaa..ba0a6d6ed60 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -19915,22 +19915,23 @@ add_data_member_location_attribute (dw_die_ref die,
 {
   loc_descr = field_byte_offset (decl, ctx, &offset);
 
-  /* If loc_descr is available then we know the field offset is dynamic.
-	 However, GDB does not handle dynamic field offsets very well at the
-	 moment.  */
-  if (loc_descr != NULL && gnat_encodings != DWARF_GNAT_ENCODINGS_MINIMAL)
+  if (!loc_descr)
+	;
+
+  /* If loc_descr is available, then we know the offset is dynamic.  */
+  else if (gnat_encodings == DWARF_GNAT_ENCODINGS_ALL)
 	{
 	  loc_descr = NULL;
 	  offset = 0;
 	}
 
-  /* Data member location evalutation starts with the base address on the
+  /* Data member location evaluation starts with the base address on the
 	 stack.  Compute the field offset and add it to this base address.  */
-  else if (loc_descr != NULL)
+  else
 	add_loc_descr (&loc_descr, new_loc_descr (DW_OP_plus, 0, 0));
 }
 
-  if (! loc_descr)
+  if (!loc_descr)
 {
   /* While DW_AT_data_bit_offset has been added already in DWARF4,
 	 e.g. GDB only added support to it in November 2016.  For DWARF5
@@ -21389,12 +21391,9 @@ add_bound_info (dw_die_ref subrange_die, enum dwarf_attribute bound_attr,
 	/* FALLTHRU */
 
   default:
-	/* Because of the complex interaction there can be with other GNAT
-	   encodings, GDB isn't ready yet to handle proper DWARF description
-	   for self-referencial subrange bounds: let GNAT encodings do the
-	   magic in such a case.  */
+	/* Let GNAT encodings do the magic for self-referential bounds.  */
 	if (is_ada ()
-	&& gnat_encodings != DWARF_GNAT_ENCODINGS_MINIMAL
+	&& gnat_encodings == DWARF_GNAT_ENCODINGS_ALL
 	&& contains_placeholder_p (bound))
 	  return;
 
@@ -21566,7 +21565,7 @@ add_byte_size_attribute (dw_die_ref die, tree tree_node)
   /* Support for dynamically-sized objects was introduced in DWARF3.  */
   else if (TYPE_P (tree_node)
 	   && (dwarf_version >= 3 || !dwarf_strict)
-	   && gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL)
+	   && gnat_encodings != DWARF_GNAT_ENCODINGS_ALL)
 {
   struct loc_descr_context ctx = {
 	const_cast (tree_node),	/* context_type */
@@ -25629,11 +25628,11 @@ gen_member_die (tree type, dw_die_ref context_die)
 	splice_child_die (context_die, child);
 	}
 
-  /* Do not generate standard DWARF for variant parts if we are generating
-	 the corresponding GNAT encodings: DIEs generated for both would
-	 conflict in our mappings.  */
+  /* Do not generate DWARF for variant parts if we are generating the
+	 corresponding GNAT encodings: DIEs generated for the two schemes
+	 would conflict in our mappings.  */
   else if (is_variant_part (member)
-	   && gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL)
+	   && gnat_encodings != DWARF_GNAT_ENCODINGS_ALL)
 	{
 	  vlr_ctx.variant_part_offset = byte_position (member);
 	  gen_variant_part (member, &vlr_ctx, context_die);

[patch] Fix regression in debug info for Ada with DWARF 5

[Eric Botcazou]

Hi,

add_scalar_info can directly generate a reference to an existing DIE for a 
scalar attribute, e.g the upper bound of a VLA, but it does so only if this 
existing DIE has a location or is a constant:

  if (get_AT (decl_die, DW_AT_location)
  || get_AT (decl_die, DW_AT_data_member_location)
  || get_AT (decl_die, DW_AT_const_value))

Now, in DWARF 5, members of a structure that are bitfields no longer have a 
DW_AT_data_member_location but a DW_AT_data_bit_offset attribute instead, so 
the condition is bypassed.

Tested on x86-64/Linux, OK for mainline and 11 branch?


2021-08-16  Eric Botcazou  

* dwarf2out.c (add_scalar_info): Deal with DW_AT_data_bit_offset.

-- 
Eric Botcazoudiff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 4bcd3313fee..ba0a6d6ed60 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -21253,6 +21253,7 @@ add_scalar_info (dw_die_ref die, enum dwarf_attribute attr, tree value,
 	{
 	  if (get_AT (decl_die, DW_AT_location)
 		  || get_AT (decl_die, DW_AT_data_member_location)
+		  || get_AT (decl_die, DW_AT_data_bit_offset)
 		  || get_AT (decl_die, DW_AT_const_value))
 		{
 		  add_AT_die_ref (die, attr, decl_die);

Re: [PATCH] Make sure -fexceptions is enabled when -fnon-call-exceptions is

[Eric Botcazou]

> This makes -fexceptions enabled by -fnon-call-exceptions, removing
> the odd state of !flag_exceptions && flag_non_call_exceptions from
> middle-end consideration.

FWIW fine with me.

-- 
Eric Botcazou

[Ada] Add preliminary support for 128-bit integer types

[Eric Botcazou]

This is only the gigi part, in preparation for the bulk of the implementation.

Tested on x86_64-suse-linux, applied on the mainline.


2020-09-12  Eric Botcazou  

* gcc-interface/gigi.h (standard_datatypes): Add ADT_mulv128_decl.
(mulv128_decl): New macro.
(get_target_long_long_long_size): Declare.
* gcc-interface/decl.c (gnat_to_gnu_entity): Use a maximum size of
128 bits for discrete types if Enable_128bit_Types is true.
* gcc-interface/targtyps.c: Include target.h.
(get_target_long_long_long_size): New function.
* gcc-interface/trans.c (gigi): Initialize mulv128_decl if need be.
(build_binary_op_trapv): Call it for 128-bit multiplication.
* gcc-interface/utils.c (make_type_from_size): Enforce a maximum
size of 128 bits if Enable_128bit_Types is true.

-- 
Eric Botcazou
diff --git a/gcc/ada/fe.h b/gcc/ada/fe.h
index 520301e4c3e..858a28acb8e 100644
--- a/gcc/ada/fe.h
+++ b/gcc/ada/fe.h
@@ -213,6 +213,7 @@ typedef enum {
 extern Ada_Version_Type Ada_Version;
 extern Boolean Back_End_Inlining;
 extern Boolean Debug_Generated_Code;
+extern Boolean Enable_128bit_Types;
 extern Boolean Exception_Extra_Info;
 extern Boolean Exception_Locations_Suppressed;
 extern Exception_Mechanism_Type Exception_Mechanism;
diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index d19f5aac81f..c9c2a95170f 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -524,7 +524,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	  else if (IN (kind, Access_Kind))
 	max_esize = POINTER_SIZE * 2;
 	  else
-	max_esize = LONG_LONG_TYPE_SIZE;
+	max_esize = Enable_128bit_Types ? 128 : LONG_LONG_TYPE_SIZE;
 
 	  if (esize > max_esize)
 	   esize = max_esize;
diff --git a/gcc/ada/gcc-interface/gigi.h b/gcc/ada/gcc-interface/gigi.h
index e43b3db59a9..355178e284f 100644
--- a/gcc/ada/gcc-interface/gigi.h
+++ b/gcc/ada/gcc-interface/gigi.h
@@ -390,6 +390,9 @@ enum standard_datatypes
   /* Function decl node for 64-bit multiplication with overflow checking.  */
   ADT_mulv64_decl,
 
+  /* Function decl node for 128-bit multiplication with overflow checking.  */
+  ADT_mulv128_decl,
+
   /* Identifier for the name of the _Parent field in tagged record types.  */
   ADT_parent_name_id,
 
@@ -462,6 +465,7 @@ extern GTY(()) tree gnat_raise_decls_ext[(int) LAST_REASON_CODE + 1];
 #define free_decl gnat_std_decls[(int) ADT_free_decl]
 #define realloc_decl gnat_std_decls[(int) ADT_realloc_decl]
 #define mulv64_decl gnat_std_decls[(int) ADT_mulv64_decl]
+#define mulv128_decl gnat_std_decls[(int) ADT_mulv128_decl]
 #define parent_name_id gnat_std_decls[(int) ADT_parent_name_id]
 #define exception_data_name_id gnat_std_decls[(int) ADT_exception_data_name_id]
 #define jmpbuf_type gnat_std_decls[(int) ADT_jmpbuf_type]
@@ -1035,6 +1039,7 @@ extern Pos get_target_short_size (void);
 extern Pos get_target_int_size (void);
 extern Pos get_target_long_size (void);
 extern Pos get_target_long_long_size (void);
+extern Pos get_target_long_long_long_size (void);
 extern Pos get_target_pointer_size (void);
 extern Pos get_target_maximum_default_alignment (void);
 extern Pos get_target_system_allocator_alignment (void);
diff --git a/gcc/ada/gcc-interface/targtyps.c b/gcc/ada/gcc-interface/targtyps.c
index 9b2d24146a1..60a37e1463b 100644
--- a/gcc/ada/gcc-interface/targtyps.c
+++ b/gcc/ada/gcc-interface/targtyps.c
@@ -29,6 +29,7 @@
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
+#include "target.h"
 #include "tree.h"
 
 #include "ada.h"
@@ -94,6 +95,15 @@ get_target_long_long_size (void)
   return LONG_LONG_TYPE_SIZE;
 }
 
+Pos
+get_target_long_long_long_size (void)
+{
+  if (targetm.scalar_mode_supported_p (TImode))
+return GET_MODE_BITSIZE (TImode);
+  else
+return LONG_LONG_TYPE_SIZE;
+}
+
 Pos
 get_target_pointer_size (void)
 {
diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 39d4d28fa67..9be12952c5b 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -439,6 +439,19 @@ gigi (Node_Id gnat_root,
 			   NULL_TREE, is_default, true, true, true, false,
 			   false, NULL, Empty);
 
+  if (Enable_128bit_Types)
+{
+  tree int128_type = gnat_type_for_size (128, 0);
+  mulv128_decl
+	= create_subprog_decl (get_identifier ("__gnat_mulv128"), NULL_TREE,
+			   build_function_type_list (int128_type,
+			 int128_type,
+			 int128_type,
+			 NULL_TREE),
+			   NULL_TREE, is_default, true, true, true, false,
+			   false, NULL, Empty);
+}
+
   /* Name of the _Parent field in tagged record types.  */
   parent_name_id = get_identifier (Get_Name_String (Name_uParent));
 
@@ -9388,6 +9401,15 @@ build_binary_op_trapv (enum tree_code code, tree gnu_type, tree left,
 		   convert (int64, rhs)));
 	}
 
+   

[Ada] Minor tweak to line debug info

[Eric Botcazou]

This prevents the SLOC of the expression for a tag from being present in the 
line debug info every time it is referenced for coverage purposes.

Tested on x86_64-suse-linux, applied on the mainline.


2020-09-12  Eric Botcazou  

* gcc-interface/trans.c (gnat_to_gnu) : Clear
the SLOC of the expression of a tag.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index e3d71bca40f..aa4393c24b7 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -6476,6 +6476,7 @@ gnat_to_gnu (Node_Id gnat_node)
 
 	  gnu_expr = gnat_to_gnu (Expression (gnat_node));
 
+	  /* First deal with erroneous expressions.  */
 	  if (TREE_CODE (gnu_expr) == ERROR_MARK)
 	{
 	  /* If this is a named number for which we cannot manipulate
@@ -6485,6 +6486,11 @@ gnat_to_gnu (Node_Id gnat_node)
 	  else if (type_annotate_only)
 		gnu_expr = NULL_TREE;
 	}
+
+	  /* Then a special case: we do not want the SLOC of the expression
+	 of the tag to pop up every time it is referenced somewhere.  */
+	  else if (EXPR_P (gnu_expr) && Is_Tag (gnat_temp))
+	SET_EXPR_LOCATION (gnu_expr, UNKNOWN_LOCATION);
 	}
   else
 	gnu_expr = NULL_TREE;

[Ada] Accept absolute address clause for array of UNC nominal subtype

[Eric Botcazou]

This changes the compiler to accept again absolute address clause for aliased 
array of unconstrained nominal subtype, instead of erroring out in this case.

Tested on x86_64-suse-linux, applied on the mainline.


2020-09-12  Eric Botcazou  

* gcc-interface/decl.c (gnat_to_gnu_entity) : Only give
a warning for the overlay of an aliased array with an unconstrained
nominal subtype if the address is absolute.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index 2b7392c62c0..d19f5aac81f 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -1245,6 +1245,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 		if (TREE_CODE (gnu_address) == POINTER_PLUS_EXPR
 		&& TREE_OPERAND (gnu_address, 1) == off)
 		  gnu_address = TREE_OPERAND (gnu_address, 0);
+
 		/* This is the pattern built for an overaligned object.  */
 		else if (TREE_CODE (gnu_address) == POINTER_PLUS_EXPR
 			 && TREE_CODE (TREE_OPERAND (gnu_address, 1))
@@ -1255,6 +1256,18 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 		= build2 (POINTER_PLUS_EXPR, gnu_type,
 			  TREE_OPERAND (gnu_address, 0),
 			  TREE_OPERAND (TREE_OPERAND (gnu_address, 1), 0));
+
+		/* We make an exception for an absolute address but we warn
+		   that there is a descriptor at the start of the object.  */
+		else if (TREE_CODE (gnu_address) == INTEGER_CST)
+		  {
+		post_error_ne ("??aliased object& with unconstrained "
+   "array nominal subtype", gnat_clause,
+   gnat_entity);
+		post_error ("\\starts with a descriptor whose size is "
+"given by ''Descriptor_Size", gnat_clause);
+		  }
+
 		else
 		  {
 		post_error_ne ("aliased object& with unconstrained array "

[Ada] Fix small inconsistency in new predicate

[Eric Botcazou]

This can result on the mainline in a segfault when an object declared at 
library level is used in the declaration of another, local object.

Tested on x86_64-suse-linux, applied on the mainline.


2020-09-12  Eric Botcazou  

* gcc-interface/trans.c (lvalue_for_aggr_p) :
Return false unconditionally.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index aa4393c24b7..39d4d28fa67 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -968,12 +968,8 @@ lvalue_for_aggregate_p (Node_Id gnat_node, tree gnu_type)
  get_unpadded_type (Etype (gnat_parent)));
 
 case N_Object_Declaration:
-  /* For an aggregate object declaration, return the constant at top level
-	 in order to avoid generating elaboration code.  */
-  if (global_bindings_p ())
-	return false;
-
-  /* ... fall through ... */
+  /* For an aggregate object declaration, return false consistently.  */
+  return false;
 
 case N_Assignment_Statement:
   /* For an aggregate assignment, decide based on the size.  */

[patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

Hi,

when a thunk cannot be emitted in assembly directly, cgraph_node::expand_thunk 
generates regular GIMPLE code but unconditionally forces a tail call to the 
target of the thunk.  That's theoretically OK because the thunk essentially 
forwards its parameters to the target, but in practice the RTL expander can 
spill parameters passed by reference on the stack, see assign_parm_setup_reg:

  /* If we were passed a pointer but the actual value can safely live
 in a register, retrieve it and use it directly.  */
  if (data->arg.pass_by_reference && TYPE_MODE (TREE_TYPE (parm)) != BLKmode)
{
  /* We can't use nominal_mode, because it will have been set to
 Pmode above.  We must use the actual mode of the parm.  */
  if (use_register_for_decl (parm))
{
  parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm)));
  mark_user_reg (parmreg);
}
  else
{
  int align = STACK_SLOT_ALIGNMENT (TREE_TYPE (parm),
TYPE_MODE (TREE_TYPE (parm)),
TYPE_ALIGN (TREE_TYPE (parm)));
  parmreg
= assign_stack_local (TYPE_MODE (TREE_TYPE (parm)),
  GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (parm))),
  align);
  set_mem_attributes (parmreg, parm, 1);
}

use_register_for_decl always return false at -O0 so, in this case, the thunk 
will pass an address within its frame to its target, so it cannot use a tail 
call to invoke it.

Tested on x86_64-suse-linux, OK for the mainline?


2020-09-14  Eric Botcazou  

* cgraphunit.c (cgraph_node::expand_thunk): Force a tail call only
when optimizing.


2020-09-14  Eric Botcazou  

* gnat.dg/thunk1.adb: New test.
* gnat.dg/thunk1_pkg1.ads: New helper.
* gnat.dg/thunk1_pkg2.ads: Likewise.
* gnat.dg/thunk1_pkg2.adb: Likewise.

-- 
Eric Botcazoudiff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 26d3995a0c0..fe8552e3cf2 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2170,8 +2170,11 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 		  bsi = gsi_last_bb (return_bb);
 		}
 	}
+
+	  /* We can use a tail call, but only when optimizing to be sure that
+	 the RTL expander doesn't spill parameters passed by reference.  */
 	  else
-	gimple_call_set_tail (call, true);
+	gimple_call_set_tail (call, optimize);
 
 	  /* Build return value.  */
 	  if (!DECL_BY_REFERENCE (resdecl))
@@ -2183,7 +2186,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 	}
   else
 	{
-	  gimple_call_set_tail (call, true);
+	  gimple_call_set_tail (call, optimize);
 	  remove_edge (single_succ_edge (bb));
 	}
 
package Thunk1_Pkg2 is

  type Root is tagged record
I : Integer;
  end record;

  type Iface is interface;
  procedure Op (This : in out Iface; S : String) is abstract;

  type Ext is new Root and Iface with null record;

  procedure Op (This : in out Ext; S : String);

end Thunk1_Pkg2;
-- { dg-do run }

with Thunk1_Pkg1; use Thunk1_Pkg1;

procedure Thunk1 is
  D: Derived;
begin
  D.Op ("Message");
end;
with Thunk1_Pkg2; use Thunk1_Pkg2;

package Thunk1_Pkg1 is

  type Derived is new Ext with null record;

end Thunk1_Pkg1;
package body Thunk1_Pkg2 is

  procedure Op (This : in out Ext; S : String) is
  begin
if S /= "Message" then
  raise Program_Error;
end if;
  end;

end Thunk1_Pkg2;

Re: [patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

> ISTR the tailcall flag is only a hint and RTL expansion can decide to
> not tailcall based on targets.  So to me it looks like a missed
> disqualification on the RTL expansion side.

That's IMO debatable: the GIMPLE tailcall optimization pass e.g. does:

  /* Make sure the tail invocation of this function does not indirectly
 refer to local variables.  (Passing variables directly by value
 is OK.)  */
  FOR_EACH_LOCAL_DECL (cfun, idx, var)
{
  if (TREE_CODE (var) != PARM_DECL
  && auto_var_in_fn_p (var, cfun->decl)
  && may_be_aliased (var)
  && (ref_maybe_used_by_stmt_p (call, var)
  || call_may_clobber_ref_p (call, var)))

Do you want to replace it with something at the RTL level too?  I don't think 
that you can disqualify things at the RTL as easily as at the GIMPLE level.

> Or do we, besides from this very single spot, simply never tailcall at -O0
> and thus never hit this latent issue?

Presumably yes, tail calling is an optimization like the others.

> How does this change the debug experience at -O0 when GIMPLE thunks
> are used?

In Ada this doesn't change much since thunks have line debug info.

-- 
Eric Botcazou

Re: [patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

> No, so you're right - validity analysis is split.  Still the cause you
> reference comes from RTL expansion which means RTL expansion should
> possibly do the disqualification here.  The question is what makes the case
> you run into at -O0 impossible to trigger with -O1+?

The key test in use_register_for_decl is:

  if (optimize)
return true;

I think that all the preceding tests that return false cannot trigger in this 
context - a parameter passed by invisible reference - so, by returning true 
here, you're pretty much covered I think.

> Maybe we can also avoid this spilling at -O0?

The code wants to retrieve the parameter at all optimization levels.  The 
question is: register or stack slot?  It uses use_register_for_decl to decide 
as in other circumstances, but so you want to always force a register?

Note that, since cgraph_node::expand_thunk has expanded the thunk in GIMPLE, 
the function is no longer a thunk for the middle-end (cfun->is_thunk false).

That being said, I can add another bit to cfun, e.g. is_gimple_thunk, and 
force the use of a register above only in case the bit is true.

> Yeah, but the asm thunk tail-calls also at -O0.  I guess tail-calling is
> forced here because gimple analysis sometimes would not mark the call.

The low-level assembly thunks simply support the most simple thunks, see the 
condition in expand_thunk.  Moreover targets can opt out as they wish.

-- 
Eric Botcazou

Re: [patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

> That being said, I can add another bit to cfun, e.g. is_gimple_thunk, and
> force the use of a register above only in case the bit is true.

In fact I can probably reuse cfun->tail_call_marked for this purpose.

-- 
Eric Botcazou

Re: [patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

> In fact I can probably reuse cfun->tail_call_marked for this purpose.

Like so.

* cgraphunit.c (cgraph_node::expand_thunk): Make sure to set
cfun->tail_call_marked when forcing a tail call.
* function.c (assign_parm_setup_reg): Always use a register to
retrieve a parameter passed by reference if cfun->tail_call_marked.

-- 
Eric Botcazoudiff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 26d3995a0c0..bedb6e2eea1 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2171,7 +2171,10 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 		}
 	}
 	  else
-	gimple_call_set_tail (call, true);
+	{
+	  gimple_call_set_tail (call, true);
+	  cfun->tail_call_marked = true;
+	}
 
 	  /* Build return value.  */
 	  if (!DECL_BY_REFERENCE (resdecl))
@@ -2184,6 +2187,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
   else
 	{
 	  gimple_call_set_tail (call, true);
+	  cfun->tail_call_marked = true;
 	  remove_edge (single_succ_edge (bb));
 	}
 
diff --git a/gcc/function.c b/gcc/function.c
index 2c8fa217f1f..314fd01c195 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -3322,13 +3322,15 @@ assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm,
   else
 emit_move_insn (parmreg, validated_mem);
 
-  /* If we were passed a pointer but the actual value can safely live
- in a register, retrieve it and use it directly.  */
+  /* If we were passed a pointer but the actual value can live in a register,
+ retrieve it and use it directly.  Note that we cannot use nominal_mode,
+ because it will have been set to Pmode above, we must use the actual mode
+ of the parameter instead.  */
   if (data->arg.pass_by_reference && TYPE_MODE (TREE_TYPE (parm)) != BLKmode)
 {
-  /* We can't use nominal_mode, because it will have been set to
-	 Pmode above.  We must use the actual mode of the parm.  */
-  if (use_register_for_decl (parm))
+  /* Use a stack slot for debugging purposes, except if a tail call is
+	 involved because this would create a dangling reference.  */
+  if (use_register_for_decl (parm) || cfun->tail_call_marked)
 	{
 	  parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm)));
 	  mark_user_reg (parmreg);

[patch] Fix pessimization in EH cleanup pass

[Eric Botcazou]

Hi,

the cleanup_all_empty_eh function was originally doing a post-order traversal 
of the EH region tree to optimize it:

/* Do a post-order traversal of the EH region tree.  Examine each
   post_landing_pad block and see if we can eliminate it as empty.  */

That's sensible since the worker function cleanup_empty_eh punts if the number 
of outgoing edges from landing pads is not 0 or 1:

  /* There can be zero or one edges out of BB.  This is the quickest test.  */
  switch (EDGE_COUNT (bb->succs))
{
case 0:
  e_out = NULL;
  break;
case 1:
  e_out = single_succ_edge (bb);
  break;
default:
  return false;
}

This was recently changed to use another order and this trivially breaks 
testcases with nested regions like the attached one.  So the attached patch 
restores the post-order traversal and it also contains a small tweak to the 
line debug info to avoid a problematic inheritance for coverage measurement.

Tested on x86_64-suse-linux, OK for the mainline?


2020-09-15  Eric Botcazou  
Pierre-Marie Derodat  

* tree-eh.c (lower_try_finally_dup_block): Backward propagate slocs 
to stack restore builtin calls.
(cleanup_all_empty_eh): Do again a post-order traversal of the EH
region tree.


2020-09-15  Eric Botcazou  

* gnat.dg/concat4.adb: New test.

-- 
Eric Botcazou-- { dg-do compile }

with Ada.Text_IO; use Ada.Text_IO;

procedure Concat4 (X : Integer) is
   Ximg : constant String := Integer'Image (X);
begin
   if X > 0 then
  Put_Line (Ximg & " is Positive");
   elsif X < 0 then
  Put_Line (Ximg & " is Negative");
   else
  Put_Line (Ximg & " is Null");
   end if;
end;

-- { dg-final { scan-assembler-not "_Unwind_Resume" } }
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 4246dca8806..ed32f80220d 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -899,23 +899,26 @@ lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state,
   gtry *region = NULL;
   gimple_seq new_seq;
   gimple_stmt_iterator gsi;
+  location_t last_loc = UNKNOWN_LOCATION;
 
   new_seq = copy_gimple_seq_and_replace_locals (seq);
 
-  for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi))
+  for (gsi = gsi_last (new_seq); !gsi_end_p (gsi); gsi_prev (&gsi))
 {
   gimple *stmt = gsi_stmt (gsi);
   /* We duplicate __builtin_stack_restore at -O0 in the hope of eliminating
-	 it on the EH paths.  When it is not eliminated, make it transparent in
-	 the debug info.  */
+	 it on the EH paths.  When it is not eliminated, give it the next
+	 location in the sequence or make it transparent in the debug info.  */
   if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
-	gimple_set_location (stmt, UNKNOWN_LOCATION);
+	gimple_set_location (stmt, last_loc);
   else if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
 	{
 	  tree block = gimple_block (stmt);
 	  gimple_set_location (stmt, loc);
 	  gimple_set_block (stmt, block);
 	}
+  else
+	last_loc = gimple_location (stmt);
 }
 
   if (outer_state->tf)
@@ -4751,15 +4754,9 @@ cleanup_all_empty_eh (void)
   eh_landing_pad lp;
   int i;
 
-  /* Ideally we'd walk the region tree and process LPs inner to outer
- to avoid quadraticness in EH redirection.  Walking the LP array
- in reverse seems to be an approximation of that.  */
-  for (i = vec_safe_length (cfun->eh->lp_array) - 1; i >= 1; --i)
-{
-  lp = (*cfun->eh->lp_array)[i];
-  if (lp)
-	changed |= cleanup_empty_eh (lp);
-}
+  for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)
+if (lp)
+  changed |= cleanup_empty_eh (lp);
 
   return changed;
 }

Re: [patch] Fix pessimization in EH cleanup pass

[Eric Botcazou]

> So it breaks PR93199 again?

Indeed, although there is no regression in the testsuite AFAICS.  I guess that 
we can do the new walk before and not instead of the post-order traversal.

Revised patch attached, same ChangeLog.

-- 
Eric Botcazoudiff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 4246dca8806..46df7af7bf2 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -899,23 +899,26 @@ lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state,
   gtry *region = NULL;
   gimple_seq new_seq;
   gimple_stmt_iterator gsi;
+  location_t last_loc = UNKNOWN_LOCATION;
 
   new_seq = copy_gimple_seq_and_replace_locals (seq);
 
-  for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi))
+  for (gsi = gsi_last (new_seq); !gsi_end_p (gsi); gsi_prev (&gsi))
 {
   gimple *stmt = gsi_stmt (gsi);
   /* We duplicate __builtin_stack_restore at -O0 in the hope of eliminating
-	 it on the EH paths.  When it is not eliminated, make it transparent in
-	 the debug info.  */
+	 it on the EH paths.  When it is not eliminated, give it the next
+	 location in the sequence or make it transparent in the debug info.  */
   if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
-	gimple_set_location (stmt, UNKNOWN_LOCATION);
+	gimple_set_location (stmt, last_loc);
   else if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
 	{
 	  tree block = gimple_block (stmt);
 	  gimple_set_location (stmt, loc);
 	  gimple_set_block (stmt, block);
 	}
+  else
+	last_loc = gimple_location (stmt);
 }
 
   if (outer_state->tf)
@@ -4751,9 +4754,8 @@ cleanup_all_empty_eh (void)
   eh_landing_pad lp;
   int i;
 
-  /* Ideally we'd walk the region tree and process LPs inner to outer
- to avoid quadraticness in EH redirection.  Walking the LP array
- in reverse seems to be an approximation of that.  */
+  /* The post-order traversal may lead to quadraticness in EH redirection
+ so first try to walk the EH region tree from inner to outer LPs.  */
   for (i = vec_safe_length (cfun->eh->lp_array) - 1; i >= 1; --i)
 {
   lp = (*cfun->eh->lp_array)[i];
@@ -4761,6 +4763,10 @@ cleanup_all_empty_eh (void)
 	changed |= cleanup_empty_eh (lp);
 }
 
+  for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)
+if (lp)
+  changed |= cleanup_empty_eh (lp);
+
   return changed;
 }
 

Re: [patch] Fix dangling references in thunks at -O0

[Eric Botcazou]

> This introduces an ICE building the glibc testsuite for alpha (bisected),
> s390 and sparc (symptoms appear the same, not bisected to confirm the
> exact revision).  See bug 97078.

Fixed thus, tested on x86_64-suse-linux, applied on mainline as obvious.


PR middle-end/97078
* function.c (use_register_for_decl): Test cfun->tail_call_marked
for a parameter here instead of...
(assign_parm_setup_reg): ...here.

* gcc.dg/pr97078.c: New test.

-- 
Eric Botcazoudiff --git a/gcc/function.c b/gcc/function.c
index c4c9930d725..c6129593b9b 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -2237,6 +2237,11 @@ use_register_for_decl (const_tree decl)
   if (optimize)
 return true;
 
+  /* Thunks force a tail call even at -O0 so we need to avoid creating a
+ dangling reference in case the parameter is passed by reference.  */
+  if (TREE_CODE (decl) == PARM_DECL && cfun->tail_call_marked)
+return true;
+
   if (!DECL_REGISTER (decl))
 return false;
 
@@ -3328,9 +,8 @@ assign_parm_setup_reg (struct assign_parm_data_all *all, tree parm,
  of the parameter instead.  */
   if (data->arg.pass_by_reference && TYPE_MODE (TREE_TYPE (parm)) != BLKmode)
 {
-  /* Use a stack slot for debugging purposes, except if a tail call is
-	 involved because this would create a dangling reference.  */
-  if (use_register_for_decl (parm) || cfun->tail_call_marked)
+  /* Use a stack slot for debugging purposes if possible.  */
+  if (use_register_for_decl (parm))
 	{
 	  parmreg = gen_reg_rtx (TYPE_MODE (TREE_TYPE (parm)));
 	  mark_user_reg (parmreg);
/* { dg-do compile } */
/* { dg-options "-O2 -ffloat-store" } */

extern void foo (long double);

void bar (long double d)
{
  foo (d);
}

Re: [PATCH] optabs: Don't reuse target for multi-word expansions if it overlaps operand(s) [PR97073]

[Eric Botcazou]

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> release branches?
> 
> 2020-09-18  Jakub Jelinek  
> 
>   PR middle-end/97073
>   * optabs.c (expand_binop, expand_absneg_bit, expand_unop,
>   expand_copysign_bit): Check reg_overlap_mentioned_p between target
>   and operand(s) and if it returns true, force a pseudo as target.
> 
>   * gcc.c-torture/execute/pr97073.c: New test.

This looks good to me.

-- 
Eric Botcazou

[Ada] Fix bogus alignment warning on address clause

[Eric Botcazou]

This is a regression present on the mainline and 10 branch: the compiler gives 
a bogus alignment warning on an address clause and a discriminated record type 
with variable size.

Tested on x86_64-suse-linux, applied on the mainline and 10 branch.


2020-09-28  Eric Botcazou  

* gcc-interface/decl.c (maybe_saturate_size): Add ALIGN parameter
and round down the result to ALIGN.
(gnat_to_gnu_entity): Adjust calls to maybe_saturate_size.


2020-09-28  Eric Botcazou  

* gnat.dg/addr16.adb: New test.
* gnat.dg/addr16_pkg.ads: New helper.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index c9c2a95170f..cd0a50b2083 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -232,7 +232,7 @@ static tree build_position_list (tree, bool, tree, tree, unsigned int, tree);
 static vec build_subst_list (Entity_Id, Entity_Id, bool);
 static vec build_variant_list (tree, Node_Id, vec,
 	 vec);
-static tree maybe_saturate_size (tree);
+static tree maybe_saturate_size (tree, unsigned int align);
 static tree validate_size (Uint, tree, Entity_Id, enum tree_code, bool, bool,
 			   const char *, const char *);
 static void set_rm_size (Uint, tree, Entity_Id);
@@ -4425,7 +4425,12 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 	  /* If the size is self-referential, annotate the maximum value
 	 after saturating it, if need be, to avoid a No_Uint value.  */
 	  if (CONTAINS_PLACEHOLDER_P (gnu_size))
-	gnu_size = maybe_saturate_size (max_size (gnu_size, true));
+	{
+	  const unsigned int align
+		= UI_To_Int (Alignment (gnat_entity)) * BITS_PER_UNIT;
+	  gnu_size
+		= maybe_saturate_size (max_size (gnu_size, true), align);
+	}
 
 	  /* If we are just annotating types and the type is tagged, the tag
 	 and the parent components are not generated by the front-end so
@@ -4461,7 +4466,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entity, tree gnu_expr, bool definition)
 		  gnu_size = size_binop (PLUS_EXPR, gnu_size, offset);
 		}
 
-	  gnu_size = maybe_saturate_size (round_up (gnu_size, align));
+	  gnu_size
+		= maybe_saturate_size (round_up (gnu_size, align), align);
 	  Set_Esize (gnat_entity, annotate_value (gnu_size));
 
 	  /* Tagged types are Strict_Alignment so RM_Size = Esize.  */
@@ -8946,15 +8952,21 @@ build_variant_list (tree gnu_qual_union_type, Node_Id gnat_variant_part,
 }
 
 /* If SIZE has overflowed, return the maximum valid size, which is the upper
-   bound of the signed sizetype in bits; otherwise return SIZE unmodified.  */
+   bound of the signed sizetype in bits, rounded down to ALIGN.  Otherwise
+   return SIZE unmodified.  */
 
 static tree
-maybe_saturate_size (tree size)
+maybe_saturate_size (tree size, unsigned int align)
 {
   if (TREE_CODE (size) == INTEGER_CST && TREE_OVERFLOW (size))
-size = size_binop (MULT_EXPR,
-		   fold_convert (bitsizetype, TYPE_MAX_VALUE (ssizetype)),
-		   build_int_cst (bitsizetype, BITS_PER_UNIT));
+{
+  size
+	= size_binop (MULT_EXPR,
+		  fold_convert (bitsizetype, TYPE_MAX_VALUE (ssizetype)),
+		  build_int_cst (bitsizetype, BITS_PER_UNIT));
+  size = round_down (size, align);
+}
+
   return size;
 }
 
-- { dg-do compile }

with Aggr16_Pkg; use Aggr16_Pkg;

package body Aggr16 is

  type Arr is array (1 .. 4) of Time;

  type Change_Type is (One, Two, Three);

  type Change (D : Change_Type) is record
case D is
  when Three =>
A : Arr;
  when Others =>
B : Boolean;
end case;
  end record;

  procedure Proc is
C : Change (Three);
  begin
C.A := (others => Null_Time);
  end;

end Aggr16;
package Aggr16_Pkg is

  type Time_Type is (A, B);

  type Time (D : Time_Type := A) is private;

  Null_Time : constant Time;

private

  type Hour is record
I1 : Integer;
I2 : Integer;
  end record;

  type Time (D : Time_Type := A) is record
case D is
  when A =>
A_Time : Integer;
  when B =>
B_Time : Hour;
end case;
  end record;

  Null_Time : constant Time := (A, 0);

end Aggr16_Pkg;

[Ada] Add missing end location information

[Eric Botcazou]

In some cases we would fail to put the end location information on the 
outermost BIND_EXPR of a function, which is problematic when there is a 
dynamic stack allocation.

Tested on x86_64-suse-linux, applied on the mainline.  This makes the 
following tweak to lower_try_finally_dup_block obsolete:
  https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553951.html
so I reverted it.


2020-09-28  Eric Botcazou  

* gcc-interface/trans.c (Subprogram_Body_to_gnu): Set the end locus
of body and declaration earlier.

-- 
Eric Botcazoudiff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 3491451cc3d..f03d591a323 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -4017,6 +4017,11 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
   gnat_poplevel ();
   gnu_result = end_stmt_group ();
 
+  /* Attempt setting the end_locus of our GCC body tree, typically a BIND_EXPR,
+ then the end_locus of our GCC subprogram declaration tree.  */
+  set_end_locus_from_node (gnu_result, gnat_node);
+  set_end_locus_from_node (gnu_subprog_decl, gnat_node);
+
   /* If we populated the parameter attributes cache, we need to make sure that
  the cached expressions are evaluated on all the possible paths leading to
  their uses.  So we force their evaluation on entry of the function.  */
@@ -4111,12 +4116,6 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
 
   gnu_return_label_stack->pop ();
 
-  /* Attempt setting the end_locus of our GCC body tree, typically a
- BIND_EXPR or STATEMENT_LIST, then the end_locus of our GCC subprogram
- declaration tree.  */
-  set_end_locus_from_node (gnu_result, gnat_node);
-  set_end_locus_from_node (gnu_subprog_decl, gnat_node);
-
   /* On SEH targets, install an exception handler around the main entry
  point to catch unhandled exceptions.  */
   if (DECL_NAME (gnu_subprog_decl) == main_identifier_node

[patch] Do not use doloop pattern with pragma Unroll

[Eric Botcazou]

Hi,

this fixes the following Ada failure on 64-bit PowerPC:

-FAIL: gnat.dg/unroll4.adb scan-rtl-dump-times loop2_unroll "optimized: loop 
unrolled 7 times" 2

The IVOPTS pass detects a doloop pattern and consequently discombobulates the 
loop sufficiently as to make it hard for the RTL unrolling pass to compute the 
best number of iterations.

Tested on PowerPC64/Linux and x86-64/Linux, OK for the mainline?


2020-09-28  Eric Botcazou  

* tree-ssa-loop-ivopts.c (analyze_and_mark_doloop_use): Bail out if
the loop is subject to a pragma Unroll with no specific count.

-- 
Eric Botcazoudiff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 5acf044161c..da04dd5ba0a 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -7947,6 +7947,9 @@ analyze_and_mark_doloop_use (struct ivopts_data *data)
   if (!flag_branch_on_count_reg)
 return;
 
+  if (data->current_loop->unroll == USHRT_MAX)
+return;
+
   if (!generic_predict_doloop_p (data))
 return;
 

[rs6000] Avoid useless masking of count operand for rotation

[Eric Botcazou]

Hi,

the Interfaces package of the Ada library defines a pair of rotation operators

   function Rotate_Left  (Value : Unsigned_n; Amount : Natural)
  return Unsigned_n;
   function Rotate_Right (Value : Unsigned_n; Amount : Natural)
  return Unsigned_n;

on modular (aka unsigned) types of n bits.  The translation in GENERIC for the 
Rotate_Left on a 32-bit modular type is:

  Value r<< ((UNSIGNED_32) Amount & 31)

and the masking is present all the way down to the assembly at -O2:

rlwinm 4,4,0,27,31
rotlw 3,3,4

Now this masking is redundant since it's done by the hardware so it would be 
nice to get rid of it.  I have attached a couple of patches to that effect: 
the first one adds new instructions while the second one only adds splitters.

Tested on PowerPC64/Linux, OK (which one) for the mainline?


2020-09-29  Eric Botcazou  

* config/rs6000/rs6000.md (*rotl3_mask): New.
(*rotlsi3_mask_64): Likewise.
(*rotl3_dot): Change to use P mode iterator.
(*rotl3_mask_dot): New.
(*rotl3_dot2): Change to use P mode iterator.
(*rotl3_mask_dot2): New.


2020-09-29  Eric Botcazou  

* gnat.dg/rotate1.adb: New test.

-- 
Eric Botcazoudiff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 694ff70635e..b6c185b80b7 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -4306,6 +4306,18 @@
   [(set_attr "type" "shift")
(set_attr "maybe_var_shift" "yes")])
 
+;; Avoid useless masking of count operand
+(define_insn "*rotl3_mask"
+  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+	(rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
+		(and:GPR (match_operand:GPR 2 "gpc_reg_operand" "r")
+			 (match_operand:GPR 3 "const_int_operand" "n"]
+  "(UINTVAL (operands[3]) & (GET_MODE_BITSIZE (mode) - 1))
+   == (unsigned HOST_WIDE_INT) (GET_MODE_BITSIZE (mode) - 1)"
+  "rotl%I2 %0,%1,%2"
+  [(set_attr "type" "shift")
+   (set_attr "var_shift" "yes")])
+
 (define_insn "*rotlsi3_64"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
 	(zero_extend:DI
@@ -4316,20 +4328,34 @@
   [(set_attr "type" "shift")
(set_attr "maybe_var_shift" "yes")])
 
+;; Avoid useless masking of count operand
+(define_insn "*rotlsi3_mask_64"
+  [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
+	(zero_extend:DI
+	(rotate:SI (match_operand:SI 1 "gpc_reg_operand" "r")
+		   (and:SI (match_operand:SI 2 "gpc_reg_operand" "r")
+			   (match_operand:SI 3 "const_int_operand" "n")]
+  "TARGET_POWERPC64
+   && (UINTVAL (operands[3]) & (GET_MODE_BITSIZE (SImode) - 1))
+  == (unsigned HOST_WIDE_INT) (GET_MODE_BITSIZE (SImode) - 1)"
+  "rotlw%I2 %0,%1,%h2"
+  [(set_attr "type" "shift")
+   (set_attr "var_shift" "yes")])
+
 (define_insn_and_split "*rotl3_dot"
   [(set (match_operand:CC 3 "cc_reg_operand" "=x,?y")
-	(compare:CC (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r,r")
-(match_operand:SI 2 "reg_or_cint_operand" "rn,rn"))
+	(compare:CC (rotate:P (match_operand:P 1 "gpc_reg_operand" "r,r")
+			  (match_operand:SI 2 "reg_or_cint_operand" "rn,rn"))
 		(const_int 0)))
-   (clobber (match_scratch:GPR 0 "=r,r"))]
-  "mode == Pmode"
+   (clobber (match_scratch:P 0 "=r,r"))]
+  ""
   "@
rotl%I2. %0,%1,%2
#"
-  "&& reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)"
+  "reload_completed && cc_reg_not_cr0_operand (operands[3], CCmode)"
   [(set (match_dup 0)
-	(rotate:GPR (match_dup 1)
-		(match_dup 2)))
+	(rotate:P (match_dup 1)
+		  (match_dup 2)))
(set (match_dup 3)
 	(compare:CC (match_dup 0)
 		(const_int 0)))]
@@ -4339,22 +4365,49 @@
(set_attr "dot" "yes")
(set_attr "length" "4,8")])
 
+;; Avoid useless masking of count operand
+(define_insn_and_split "*rotl3_mask_dot"
+  [(set (match_operand:CC 4 "cc_reg_operand" "=x,?y")
+	(compare:CC (rotate:P (match_operand:P 1 "gpc_reg_operand" "r,r")
+			  (and:P (match_operand:P 2 "gpc_reg_operand" "r,r")
+ (match_operand:P 3 "const_int_operand" "n,n")))
+		(const_int 0)))
+   (clobber (match_scratch:P 0 "=r,r"))]
+  "(UINTVAL (operands[3]) & (GET_MODE_BITSIZE (mode) - 1))
+   == (unsigned HOST_WIDE_

Re: [rs6000] Avoid useless masking of count operand for rotation

[Eric Botcazou]

> Don't call it "mask" please: *all* of our basic rotate instructions
> already have something called "mask" (that is the "m" in "rlwnm" for
> example; and "rotlw d,a,b" is just an extended mnemonic for
> "rlwnm d,a,b,0,31").  The hardware also does not mask the shift amount
> at all (instead, only the low 5 bits of RB *are* the shift amount).

"Mask" is a common terminology though, used e.g. in the x86 back-end.  That 
being said, I agree that it conflicts with PowerPC specific stuff so please 
suggest an alternative here.

> > +(define_insn "*rotl3_mask"
> > +  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> > +   (rotate:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
> > +   (and:GPR (match_operand:GPR 2 "gpc_reg_operand" "r")
> > +(match_operand:GPR 3 "const_int_operand" 
"n"]
> > +  "(UINTVAL (operands[3]) & (GET_MODE_BITSIZE (mode) - 1))
> > +   == (unsigned HOST_WIDE_INT) (GET_MODE_BITSIZE (mode) - 1)"
> 
> (Useless casts are useless.)

OK, I can remove it.

> Don't mask operands[3] please (in the UINTVAL): RTL with the number
> outside of that range is *undefined*.  So just check that it is equal?

Copied from the x86 back-end as well, so I'd rather keep it that way, unless 
you really insist...

> Why?  This diverges the "dot" version from the non-dot for no reason.

OK, let's drop it.

> I don't see a patch with splitters only?  Huh.  Did you attach the same
> patch twice?

AFAICS no, 2 different patches were attached.

> Since this won't be handled before combine (or what do I miss?), it is
> fine to do splitters only (splitters for combine).  But the other
> approach is fine as well.

Patch #2 uses define_and_split like the x86 back-end; moreover, we already 
have define_and_split for the dot variants so maybe it's the best way to go?

-- 
Eric Botcazou

[patch] Fix PR tree-optimization/92131

[Eric Botcazou]

This is a regression present on mainline, 9 and 8 branches, but the underlying 
issue is probably latent on the 7 branch.  compare_values in tree-vrp.c can 
rely on undefined overflow to compare symbolic expressions with constants so 
we must make sure not to introduce such an overflow when combining ranges with 
symbolic expressions.

Tested on x86_64-suse-linux, OK for all active branches?


2019-10-23  Eric Botcazou  

PR tree-optimization/92131
* tree-vrp.c (extract_range_from_plus_minus_expr): If the resulting
range would be symbolic, drop to varying for any explicit overflow
in the constant part or if one of the ranges is not a singleton.


2019-10-23  Eric Botcazou  

* gcc.c-torture/execute/20191023-1.c: New test.

-- 
Eric BotcazouIndex: tree-vrp.c
===
--- tree-vrp.c	(revision 277149)
+++ tree-vrp.c	(working copy)
@@ -1652,7 +1652,7 @@ extract_range_from_plus_minus_expr (valu
   value_range_kind vr0_kind = vr0.kind (), vr1_kind = vr1.kind ();
   tree vr0_min = vr0.min (), vr0_max = vr0.max ();
   tree vr1_min = vr1.min (), vr1_max = vr1.max ();
-  tree min = NULL, max = NULL;
+  tree min = NULL_TREE, max = NULL_TREE;
 
   /* This will normalize things such that calculating
  [0,0] - VR_VARYING is not dropped to varying, but is
@@ -1715,18 +1715,19 @@ extract_range_from_plus_minus_expr (valu
   combine_bound (code, wmin, min_ovf, expr_type, min_op0, min_op1);
   combine_bound (code, wmax, max_ovf, expr_type, max_op0, max_op1);
 
-  /* If we have overflow for the constant part and the resulting
-	 range will be symbolic, drop to VR_VARYING.  */
-  if (((bool)min_ovf && sym_min_op0 != sym_min_op1)
-	  || ((bool)max_ovf && sym_max_op0 != sym_max_op1))
+  /* If the resulting range will be symbolic, we need to eliminate any
+	 explicit or implicit overflow introduced in the above computation
+	 because compare_values could make an incorrect use of it.  That's
+	 why we require one of the ranges to be a singleton.  */
+  if ((sym_min_op0 != sym_min_op1 || sym_max_op0 != sym_max_op1)
+	  && ((bool)min_ovf || (bool)max_ovf
+	  || (min_op0 != max_op0 && min_op1 != max_op1)))
 	{
 	  vr->set_varying (expr_type);
 	  return;
 	}
 
   /* Adjust the range for possible overflow.  */
-  min = NULL_TREE;
-  max = NULL_TREE;
   set_value_range_with_overflow (kind, min, max, expr_type,
  wmin, wmax, min_ovf, max_ovf);
   if (kind == VR_VARYING)
/* PR tree-optimization/92131 */
/* Testcase by Armin Rigo  */

long b, c, d, e, f, i;
char g, h, j, k;
int *aa;

static void error (void) __attribute__((noipa));
static void error (void) { __builtin_abort(); }

static void see_me_here (void) __attribute__((noipa));
static void see_me_here (void) {}

static void aaa (void) __attribute__((noipa));
static void aaa (void) {}

static void a (void) __attribute__((noipa));
static void a (void) {
  long am, ao;
  if (aa == 0) {
aaa();
if (j)
  goto ay;
  }
  return;
ay:
  aaa();
  if (k) {
aaa();
goto az;
  }
  return;
az:
  if (i)
if (g)
  if (h)
if (e)
  goto bd;
  return;
bd:
  am = 0;
  while (am < e) {
switch (c) {
case 8:
  goto bh;
case 4:
  return;
}
  bh:
if (am >= 0)
  b = -am;
ao = am + b;
f = ao & 7;
if (f == 0)
  see_me_here();
if (ao >= 0)
  am++;
else
  error();
  }
}

int main (void)
{
j++;
k++;
i++;
g++;
h++;
e = 1;
a();
return 0;
}

Re: [RFA][1/3] Remove Cell Broadband Engine SPU targets

[Eric Botcazou]

> OK for all 3 patches.  Similarly for removing any other remnants you
> might find later.

I have removed left-overs in htdocs/backends.html

-- 
Eric Botcazoudiff --git a/htdocs/backends.html b/htdocs/backends.html
index 6e212b24..c9449065 100644
--- a/htdocs/backends.html
+++ b/htdocs/backends.html
@@ -112,7 +112,6 @@ rx |  s
 s390   |   ? Qqr gia e
 sh | Q   CB   qr pi
 sparc  | Q   CB   qr  b   ia
-spu|   ? Q  *C  mgi
 stormy16   | ???L  FIC D lb   i
 tilegx |   S Q   Cq  gi  e
 tilepro|   S   F C   gi  e
@@ -130,10 +129,5 @@ https://gcc.gnu.org/ml/gcc/2003-10/msg00027.html.
 href="https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb";>
 https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb.
 
-* SPU's float is special; it has the same format as IEEE float but has
-an extended range and no infinity or NaNs. SPU's float will not produce
-denormal or negative zeros; both are treated as positive zero.  On the
-other hand, SPU's double is IEEE floating point.
-
 
 

Fix PR testsuite/92302

[Eric Botcazou]

This makes the test more robust by relaxing the regexp.

Tested on SPARC/Solaris 11, applied on the mainline.


2019-11-04  Eric Botcazou  

PR testsuite/92302
* gcc.target/sparc/sparc-ret-3.c: Accept more registers in address.

-- 
Eric BotcazouIndex: gcc.target/sparc/sparc-ret-3.c
===
--- gcc.target/sparc/sparc-ret-3.c	(revision 277436)
+++ gcc.target/sparc/sparc-ret-3.c	(working copy)
@@ -50,4 +50,4 @@ unsigned int bug(unsigned int crc, const
 
 	return *ctx;
 }
-/* { dg-final { scan-assembler "ld\[ \t\]*\\\[%i5\\+8\\\], %i0\n\[^\n\]*return\[ \t\]*%i7\\+8" } } */
+/* { dg-final { scan-assembler "ld\[ \t\]*\\\[%\[a-z0-9\]*\\+\[0-9\]*\\\], %i0\n\[^\n\]*return\[ \t\]*%i7\\+8" } } */

[SPARC] Fix PR target/92095

[Eric Botcazou]

This is a regression present on mainline and 9/8 branches which was introduced 
by my fix for PR target/91472, another regression present on the branches:
  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00012.html
They are ultimately all fallout of the transition to a pseudo PIC register for 
the SPARC port, which turns out to be more disruptive than initially thought.

Bootstrapped/regtested on SPARC64/Linux and SPARC/Solaris, applied on mainline 
and 9/8 branches.


2019-11-08  Eric Botcazou  

PR target/92095
* config/sparc/sparc-protos.h (output_load_pcrel_sym): Declare.
* config/sparc/sparc.c (sparc_cannot_force_const_mem): Revert latest
change.
(got_helper_needed): New static variable.
(output_load_pcrel_sym): New function.
(get_pc_thunk_name): Remove after inlining...
(load_got_register): ...here.  Rework the initialization of the GOT
register and of the GOT helper.
(save_local_or_in_reg_p): Test the REGNO of the GOT register.
(sparc_file_end): Test got_helper_needed to decide whether the GOT
helper must be emitted.  Use output_asm_insn instead of fprintf.
(sparc_init_pic_reg): In PIC mode, always initialize the PIC register
if optimization is enabled.
* config/sparc/sparc.md (load_pcrel_sym): Emit the assembly
by calling output_load_pcrel_sym.


2019-11-08  Eric Botcazou  

* gcc.c-torture/compile/20191108-1.c: New test.
* gcc.target/sparc/overflow-3.c: Add -fno-pie to the options.
* gcc.target/sparc/overflow-4.c: Likewise.
* gcc.target/sparc/overflow-5.c: Likewise.

-- 
Eric Botcazou
/* PR target/92095 */
/* Testcase by Sergei Trofimovich  */

typedef union {
  double a;
  int b[2];
} c;

double d(int e)
{
  c f;
  (&f)->b[0] = 15728640;
  return e ? -(&f)->a : (&f)->a;
}
Index: config/sparc/sparc-protos.h
===
--- config/sparc/sparc-protos.h	(revision 277906)
+++ config/sparc/sparc-protos.h	(working copy)
@@ -69,6 +69,7 @@ extern void sparc_split_reg_mem (rtx, rt
 extern void sparc_split_mem_reg (rtx, rtx, machine_mode);
 extern int sparc_split_reg_reg_legitimate (rtx, rtx);
 extern void sparc_split_reg_reg (rtx, rtx, machine_mode);
+extern const char *output_load_pcrel_sym (rtx *);
 extern const char *output_ubranch (rtx, rtx_insn *);
 extern const char *output_cbranch (rtx, rtx, int, int, int, rtx_insn *);
 extern const char *output_return (rtx_insn *);
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 277906)
+++ config/sparc/sparc.c	(working copy)
@@ -4201,13 +4201,6 @@ eligible_for_sibcall_delay (rtx_insn *tr
 static bool
 sparc_cannot_force_const_mem (machine_mode mode, rtx x)
 {
-  /* After IRA has run in PIC mode, it is too late to put anything into the
- constant pool if the PIC register hasn't already been initialized.  */
-  if ((lra_in_progress || reload_in_progress)
-  && flag_pic
-  && !crtl->uses_pic_offset_table)
-return true;
-
   switch (GET_CODE (x))
 {
 case CONST_INT:
@@ -4243,9 +4236,11 @@ sparc_cannot_force_const_mem (machine_mo
 }
 
 /* Global Offset Table support.  */
-static GTY(()) rtx got_helper_rtx = NULL_RTX;
-static GTY(()) rtx got_register_rtx = NULL_RTX;
 static GTY(()) rtx got_symbol_rtx = NULL_RTX;
+static GTY(()) rtx got_register_rtx = NULL_RTX;
+static GTY(()) rtx got_helper_rtx = NULL_RTX;
+
+static GTY(()) bool got_helper_needed = false;
 
 /* Return the SYMBOL_REF for the Global Offset Table.  */
 
@@ -4258,27 +4253,6 @@ sparc_got (void)
   return got_symbol_rtx;
 }
 
-#ifdef HAVE_GAS_HIDDEN
-# define USE_HIDDEN_LINKONCE 1
-#else
-# define USE_HIDDEN_LINKONCE 0
-#endif
-
-static void
-get_pc_thunk_name (char name[32], unsigned int regno)
-{
-  const char *reg_name = reg_names[regno];
-
-  /* Skip the leading '%' as that cannot be used in a
- symbol name.  */
-  reg_name += 1;
-
-  if (USE_HIDDEN_LINKONCE)
-sprintf (name, "__sparc_get_pc_thunk.%s", reg_name);
-  else
-ASM_GENERATE_INTERNAL_LABEL (name, "LADDPC", regno);
-}
-
 /* Wrapper around the load_pcrel_sym{si,di} patterns.  */
 
 static rtx
@@ -4298,30 +4272,78 @@ gen_load_pcrel_sym (rtx op0, rtx op1, rt
   return insn;
 }
 
+/* Output the load_pcrel_sym{si,di} patterns.  */
+
+const char *
+output_load_pcrel_sym (rtx *operands)
+{
+  if (flag_delayed_branch)
+{
+  output_asm_insn ("sethi\t%%hi(%a1-4), %0", operands);
+  output_asm_insn ("call\t%a2", operands);
+  output_asm_insn (" add\t%0, %%lo(%a1+4), %0", operands);
+}
+  else
+{
+  output_asm_insn ("sethi\t%%hi(%a1-8), %0", operands);
+  output_asm_insn ("add\t%0, %%lo(%a1-4), %0", operands);
+  output_asm_insn ("call\t%a2", oper

Re: [PATCH, GCC] Fix unrolling check.

[Eric Botcazou]

> I was fiddling around with the loop unrolling pass and noticed a check 
> in decide_unroll_* functions (in the patch). The comment on top of this 
> check says
> "/* If we were not asked to unroll this loop, just return back silently. 
>   */"
> However the check returns when loop->unroll == 0 rather than 1.
> 
> The check was added in r255106 where the ChangeLog suggests that the 
> actual intention was probably to check the value 1 and not 0.

No, this is intended, 0 is the default value of the field, not 1.  And note 
that decide_unroll_constant_iterations, decide_unroll_runtime_iterations and 
decide_unroll_stupid *cannot* be called with loop->unroll == 1 because of this 
check in decide_unrolling:

  if (loop->unroll == 1)
{
  if (dump_file)
fprintf (dump_file,
 ";; Not unrolling loop, user didn't want it unrolled\n");
  continue;
}

> Tested on aarch64-none-elf with one new regression:
> FAIL: gcc.dg/pr40209.c (test for excess errors)
> This fails because the changes cause the loop to unroll 3 times using 
> unroll_stupid and that shows up as excess error due -fopt-info. This 
> option was added in r202077 but I am not sure why this particular test 
> was chosen for it.

That's a regression, there should be no unrolling.

-- 
Eric Botcazou

Re: [PATCH, GCC] Fix unrolling check.

[Eric Botcazou]

> Thanks for the explanation. However, I do not understand why are we 
> returning with the default value.

The regression you reported should be clear enough though: if we don't do 
that, we will unroll in cases where we would not have before.  Try with a 
compiler that predates the pragma and compare, there should be no changes.

> What "do we always do"?

What we do in the absence of specific unrolling directives for the loop.

-- 
Eric Botcazou

Re: Fix ICE in ipa-cp when mixing -O0 and -O2 code in LTO

[Eric Botcazou]

> this fixes second ICE seen during profiledbootstrap with Ada enabled.
> The problem is that Ada builds some object files with -O2 -O0 (not sure
> why) and these functions get flag_ipa_cp but !optimize.   

# Compile s-excdeb.o without optimization and with debug info to let the
# debugger set breakpoints and inspect subprogram parameters on exception
# related events.

We could probably try wih -Og these days.

-- 
Eric Botcazou

[Ada] Fix -fdump-ada-spec issue with array field

[Eric Botcazou]

This is a regression present on mainline and 9 branch: if the component type 
of array type is a structure declared in another file, then the binding would 
contain a local declaration for this structure type.

Tested on x86_64-suse-linux, applied on the mainline and 9 branch.


2019-11-13  Eric Botcazou  

c-family/
* c-ada-spec.c (get_underlying_decl): Do not look through typedefs.
(dump_forward_type): Do not generate a declaration for function types.
(dump_nested_type) : Do not generate a nested declaration
of the component type if it is declared in another file.

-- 
Eric BotcazouIndex: c-ada-spec.c
===
--- c-ada-spec.c	(revision 277906)
+++ c-ada-spec.c	(working copy)
@@ -1025,7 +1025,9 @@ get_underlying_decl (tree type)
 
   if (TYPE_P (type))
 {
-  type = TYPE_MAIN_VARIANT (type);
+  /* Strip qualifiers but do not look through typedefs.  */
+  if (TYPE_QUALS_NO_ADDR_SPACE (type))
+	type = TYPE_MAIN_VARIANT (type);
 
   /* type is a typedef.  */
   if (TYPE_NAME (type) && DECL_P (TYPE_NAME (type)))
@@ -2454,6 +2456,9 @@ dump_forward_type (pretty_printer *buffe
   if (DECL_SOURCE_FILE (decl) != DECL_SOURCE_FILE (t))
 return;
 
+  if (TREE_CODE (type) == FUNCTION_TYPE)
+return;
+
   /* Generate an incomplete type declaration.  */
   pp_string (buffer, "type ");
   dump_ada_node (buffer, decl, NULL_TREE, spc, false, true);
@@ -2522,7 +2527,10 @@ dump_nested_type (pretty_printer *buffer
   while (TREE_CODE (tmp) == ARRAY_TYPE)
 	tmp = TREE_TYPE (tmp);
   decl = get_underlying_decl (tmp);
-  if (decl && !DECL_NAME (decl) && !TREE_VISITED (decl))
+  if (decl
+	  && !DECL_NAME (decl)
+	  && DECL_SOURCE_FILE (decl) == DECL_SOURCE_FILE (t)
+	  && !TREE_VISITED (decl))
 	{
 	  /* Generate full declaration.  */
 	  dump_nested_type (buffer, decl, t, parent, spc);

Re: [pushed] Ada : Fix bootstrap after r11-4793.

[Eric Botcazou]

> The patch omitted a change for Ada, fixed thus.
> 
> tested on x86_64-darwin,
> pushed to master as obvious/bootstrap fix.
> thanks
> Iain
> 
> gcc/ada/ChangeLog:
> 
>   * gcc-interface/misc.c (gnat_printable_name): Change
>   DECL_IS_BUILTIN -> DECL_IS_UNDECLARED_BUILTIN.

Thanks for fixing this!

-- 
Eric Botcazou