[Ada] Fix crash on extension with variable size and representation clause

2019-05-28 Thread Eric Botcazou
The compiler segfaults on the extension of a tagged record type with variable 
size subject to a partial representation clause, which is quite pathological.

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


2019-05-28  Eric Botcazou  

* gcc-interface/decl.c (components_to_record): Set a name on the
created for the REP part, if any.
* gcc-interface/utils.c (finish_record_type): Only take the maximum
when merging sizes for a variant part at offset 0.
(merge_sizes): Rename has_rep parameter into max.


2019-05-28  Eric Botcazou  

* gnat.dg/specs/discr5.ads: New test.

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

with System;

package Discr5 is

   X, Y : Boolean;

   type R (D : Boolean := False) is tagged limited record
  F : Integer;
  case D is
 when True =>
F1, F2 : Integer;
 when False =>
null;
  end case;
   end record;
   for R use record
  F1 at 100 range 0..31;
   end record;
   
   subtype Rt is R(True);
   subtype Rf is R(False);

   type R1 (D1 : Boolean) is new R (X) with record
  FF : Float;
  case D1 is
 when True =>
F3, F4 : Float;
 when False =>
null;
  end case;
   end record;
   for R1 use record
  F4 at 200 range 0..31;
   end record;

   subtype R1t is R1 (True);
   subtype R1f is R1 (False);

   type R2 (D2 : Boolean) is new R1 (Y) with record
  FFF: System.Address;
  case D2 is
 when True =>
F5, F6: System.Address;
 when False =>
null;
  end case;
   end record;
   for R2 use record
  F6 at 300 range 0..63;
   end record;

   subtype R2t is R2 (True);
   subtype R2f is R2 (False);

end Discr5;
Index: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 271679)
+++ gcc-interface/decl.c	(working copy)
@@ -8162,6 +8162,8 @@ components_to_record (Node_Id gnat_compo
 	gnu_field_list = gnu_rep_list;
   else
 	{
+	  TYPE_NAME (gnu_rep_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);
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c	(revision 271680)
+++ gcc-interface/utils.c	(working copy)
@@ -1863,6 +1863,9 @@ finish_record_type (tree record_type, tr
   else
 	this_ada_size = this_size;
 
+  const bool variant_part = (TREE_CODE (type) == QUAL_UNION_TYPE);
+  const bool variant_part_at_zero = variant_part && integer_zerop (pos);
+
   /* Clear DECL_BIT_FIELD for the cases layout_decl does not handle.  */
   if (DECL_BIT_FIELD (field)
 	  && operand_equal_p (this_size, TYPE_SIZE (type), 0))
@@ -1904,9 +1907,7 @@ finish_record_type (tree record_type, tr
   /* Clear DECL_BIT_FIELD_TYPE for a variant part at offset 0, it's simply
 	 not supported by the DECL_BIT_FIELD_REPRESENTATIVE machinery because
 	 the variant part is always the last field in the list.  */
-  if (DECL_INTERNAL_P (field)
-	  && TREE_CODE (TREE_TYPE (field)) == QUAL_UNION_TYPE
-	  && integer_zerop (pos))
+  if (variant_part_at_zero)
 	DECL_BIT_FIELD_TYPE (field) = NULL_TREE;
 
   /* If we still have DECL_BIT_FIELD set at this point, we know that the
@@ -1941,18 +1942,18 @@ finish_record_type (tree record_type, tr
 	case RECORD_TYPE:
 	  /* Since we know here that all fields are sorted in order of
 	 increasing bit position, the size of the record is one
-	 higher than the ending bit of the last field processed
-	 unless we have a rep clause, since in that case we might
-	 have a field outside a QUAL_UNION_TYPE that has a higher ending
-	 position.  So use a MAX in that case.  Also, if this field is a
-	 QUAL_UNION_TYPE, we need to take into account the previous size in
-	 the case of empty variants.  */
+	 higher than the ending bit of the last field processed,
+	 unless we have a variant part at offset 0, since in this
+	 case we might have a field outside the variant part that
+	 has a higher ending position; so use a MAX in this case.
+	 Also, if this field is a QUAL_UNION_TYPE, we need to take
+	 into account the previous size in the case of empty variants.  */
 	  ada_size
-	= merge_sizes (ada_size, pos, this_ada_size,
-			   TREE_CODE (type) == QUAL_UNION_TYPE, rep_level > 0);
+	= merge_sizes (ada_size, pos, this_ada_size, variant_part,
+			   variant_part_at_zero);
 	  size
-	= merge_sizes (size, pos, this_size,
-			   TREE_CODE (type) == QUAL_UNION_TYPE, rep_level > 0);
+	= merge_sizes (size, pos, this_size, variant_part,
+			   variant_part_at_zero);
 	  break;
 
 	default:
@@ -2233,13 +2234,12 @@ rest

[Ada] Add support for __builtin_prefetch

2019-05-28 Thread Eric Botcazou
This adds support for __builtin_prefetch in Ada.

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


2019-05-28  Eric Botcazou  

* gcc-interface/decl.c (intrin_arglists_compatible_p): Do not return
false if the internal builtin uses a variable list.


2019-05-28  Eric Botcazou  

* gnat.dg/prefetch1.ad[sb]: New test.

-- 
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 271528)
+++ gcc-interface/decl.c	(working copy)
@@ -9174,9 +9174,9 @@ intrin_arglists_compatible_p (intrin_bin
   if (!ada_type && !btin_type)
 	break;
 
-  /* If one list is shorter than the other, they fail to match.  */
-  if (!ada_type || !btin_type)
-	return false;
+  /* If the internal builtin uses a variable list, accept anything.  */
+  if (!btin_type)
+	break;
 
   /* If we're done with the Ada args and not with the internal builtin
 	 args, or the other way around, complain.  */
with System;

package Prefetch1 is

  procedure My_Proc1 (Addr : System.Address);
  procedure My_Proc2 (Addr : System.Address);
  procedure My_Proc3 (Addr : System.Address);

end Prefetch1;
-- { dg-do compile }

package body Prefetch1 is

  procedure Prefetch_1 (Addr : System.Address);
  pragma Import (Intrinsic, Prefetch_1, "__builtin_prefetch");

  procedure Prefetch_2 (Addr : System.Address; RW : Integer);
  pragma Import (Intrinsic, Prefetch_2, "__builtin_prefetch");

  procedure Prefetch_3 (Addr : System.Address; RW : Integer; Locality : Integer);
  pragma Import (Intrinsic, Prefetch_3, "__builtin_prefetch");

  procedure My_Proc1 (Addr : System.Address) is
  begin
Prefetch_1 (Addr);
  end;

  procedure My_Proc2 (Addr : System.Address) is
  begin
Prefetch_2 (Addr, 1);
  end;

  procedure My_Proc3 (Addr : System.Address) is
  begin
Prefetch_3 (Addr, 1, 1);
  end;

end Prefetch1;


[Ada] Silence useless -Wuninitialized warning

2019-05-27 Thread Eric Botcazou
This silences a warning issued for the call to the initialization procedure of 
a record type on a misaligned component of another record type.

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


2019-05-27  Eric Botcazou  

* gcc-interface/trans.c (Call_to_gnu): Do not initialize the temporary
created out of addressability concerns if it's for the _Init parameter
of an initialization procedure.

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271658)
+++ gcc-interface/trans.c	(working copy)
@@ -5313,13 +5313,23 @@ Call_to_gnu (Node_Id gnat_node, tree *gn
 	  /* Create an explicit temporary holding the copy.  */
 	  if (atomic_p)
 	gnu_name = build_atomic_load (gnu_name, sync);
-	  gnu_temp
-	= create_init_temporary ("A", gnu_name, _stmt, gnat_actual);
 
-	  /* But initialize it on the fly like for an implicit temporary as
-	 we aren't necessarily having a statement list.  */
-	  gnu_name = build_compound_expr (TREE_TYPE (gnu_name), gnu_stmt,
-	  gnu_temp);
+	  /* Do not initialize it for the _Init parameter of an initialization
+	 procedure since no data is meant to be passed in.  */
+	  if (Ekind (gnat_formal) == E_Out_Parameter
+	  && Is_Entity_Name (Name (gnat_node))
+	  && Is_Init_Proc (Entity (Name (gnat_node
+	gnu_name = gnu_temp = create_temporary ("A", TREE_TYPE (gnu_name));
+
+	  /* Initialize it on the fly like for an implicit temporary in the
+	 other cases, as we don't necessarily have a statement list.  */
+	  else
+	{
+	  gnu_temp = create_init_temporary ("A", gnu_name, _stmt,
+		gnat_actual);
+	  gnu_name = build_compound_expr (TREE_TYPE (gnu_name), gnu_stmt,
+	  gnu_temp);
+	}
 
 	  /* Set up to move the copy back to the original if needed.  */
 	  if (!in_param)


[Ada] Add support for __builtin_expect and friends

2019-05-27 Thread Eric Botcazou
This adds support for __builtin_expect as well as __builtin_[un]likely in Ada.

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


2019-05-27  Eric Botcazou  

* gcc-interface/ada-builtin-types.def: New file.
* gcc-interface/ada-builtins.def: Likewise.
* gcc-interface/ada-tree.h (BUILT_IN_LIKELY): New macro.
(BUILT_IN_UNLIKELY): Likewise.
* gcc-interface/trans.c (independent_iterations_p): Initialize the
auto-vector to 16 elements.
(Call_to_gnu): Remove local variable and change the vector of actual
parameters to an auto-vector.  Do not convert actual parameters to
the argument type for front-end built-in functions.  Add support for
front-end built-in functions.
(build_noreturn_cond): Use internal instead of built-in function.
* gcc-interface/utils.c (c_builtin_type): Include ada-builtin-types.def
(install_builtin_function_types): Likewise.
(install_builtin_functions): Include ada-builtins.def first.


2019-05-27  Eric Botcazou  

* gnat.dg/expect2.adb: New test.
* gnat.dg/expect2_pkg.ads: New helper.

-- 
Eric BotcazouIndex: gcc-interface/ada-builtin-types.def
===
--- gcc-interface/ada-builtin-types.def	(nonexistent)
+++ gcc-interface/ada-builtin-types.def	(revision 336464)
@@ -0,0 +1,25 @@
+/* This file contains the type definitions for the builtins exclusively
+   used in the GNU Ada compiler.
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* See builtin-types.def for details.  */
+
+DEF_FUNCTION_TYPE_1 (BT_FN_BOOL_BOOL, BT_BOOL, BT_BOOL)
+DEF_FUNCTION_TYPE_2 (BT_FN_BOOL_BOOL_BOOL, BT_BOOL, BT_BOOL, BT_BOOL)
Index: gcc-interface/ada-builtins.def
===
--- gcc-interface/ada-builtins.def	(nonexistent)
+++ gcc-interface/ada-builtins.def	(revision 336464)
@@ -0,0 +1,30 @@
+/* This file contains the definitions for the builtins exclusively used
+   in the GNU Ada compiler.
+
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* Before including this file, you should define a macro:
+
+ DEF_ADA_BUILTIN (ENUM, NAME, TYPE, ATTRS)
+
+   See builtins.def for details.  */
+
+DEF_ADA_BUILTIN(BUILT_IN_EXPECT, "expect", BT_FN_BOOL_BOOL_BOOL, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_ADA_BUILTIN(BUILT_IN_LIKELY, "likely", BT_FN_BOOL_BOOL, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_ADA_BUILTIN(BUILT_IN_UNLIKELY, "unlikely", BT_FN_BOOL_BOOL, ATTR_CONST_NOTHROW_LEAF_LIST)
Index: gcc-interface/ada-tree.h
===
--- gcc-interface/ada-tree.h	(revision 336463)
+++ gcc-interface/ada-tree.h	(revision 336464)
@@ -6,7 +6,7 @@
  *  *
  *  C Header File   *
  *  *
- *  Copyright (C) 1992-2018, Free Software Foundation, Inc. *
+ *  Copyright (C) 1992-2019, Free Software Foundation, Inc. *
  *  *
  * GNAT is free software;  you can  redistribute it  and/or modify it under *
  * terms of the  GNU General Public License as published  by the Free Soft- *
@@ -582,3 +582,8 @@ do {		   \
 
 #define EXIT_STMT_COND(NODE) TREE_OPERAND_CHECK_CODE (NODE, EXIT_STMT, 0)
 #define EXIT_STMT_LABEL(NODE)TREE_OPERAND_CHECK_CODE (NODE, EXIT_STMT, 1)
+
+/* Small kludge to be able to define Ada built-

[Ada] Fix minor warning issue

2019-05-27 Thread Eric Botcazou
Tested on x86_64-suse-linux, applied on the mainline, 9 and 8 branches.


2019-05-27  Eric Botcazou  

* gcc-interface/utils.c (maybe_pad_type): Issue the warning for the
specific case of component types preferably.

-- 
Eric Botcazou
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c	(revision 271528)
+++ gcc-interface/utils.c	(working copy)
@@ -1530,14 +1530,14 @@ built:
 	 generated for some other corresponding source entity.  */
   if (Comes_From_Source (gnat_entity))
 	{
-	  if (Present (gnat_error_node))
-	post_error_ne_tree ("{^ }bits of & unused?",
-gnat_error_node, gnat_entity,
-size_diffop (size, orig_size));
-	  else if (is_component_type)
+	  if (is_component_type)
 	post_error_ne_tree ("component of& padded{ by ^ bits}?",
 gnat_entity, gnat_entity,
 size_diffop (size, orig_size));
+	  else if (Present (gnat_error_node))
+	post_error_ne_tree ("{^ }bits of & unused?",
+gnat_error_node, gnat_entity,
+size_diffop (size, orig_size));
 	}
 }
 


[Ada] Do not generate dangling references to bounds

2019-05-27 Thread Eric Botcazou
This prevents gigi from generating dangling references to the bounds of an 
aliased parameter of an unconstrained array type.  This cannot happen in 
strict Ada but you can bypass the rules by means of 'Unchecked_Access.

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


2019-05-27  Eric Botcazou  

* gcc-interface/trans.c (Identifier_to_gnu): Minor tweaks.
(gnat_to_gnu): Do not convert the result if it is a reference to an
unconstrained array used as the prefix of an attribute reference that
requires an lvalue.


2019-05-27  Eric Botcazou  

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

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271650)
+++ gcc-interface/trans.c	(working copy)
@@ -1110,11 +1110,12 @@ Identifier_to_gnu (Node_Id gnat_node, tr
 }
   else
 {
-  /* We want to use the Actual_Subtype if it has already been elaborated,
-	 otherwise the Etype.  Avoid using Actual_Subtype for packed arrays to
-	 simplify things.  */
+  /* We use the Actual_Subtype only if it has already been elaborated,
+	 as we may be invoked precisely during its elaboration, otherwise
+	 the Etype.  Avoid using it for packed arrays to simplify things.  */
   if ((Ekind (gnat_entity) == E_Constant
-	   || Ekind (gnat_entity) == E_Variable || Is_Formal (gnat_entity))
+	   || Ekind (gnat_entity) == E_Variable
+	   || Is_Formal (gnat_entity))
 	  && !(Is_Array_Type (Etype (gnat_entity))
 	   && Present (Packed_Array_Impl_Type (Etype (gnat_entity
 	  && Present (Actual_Subtype (gnat_entity))
@@ -8685,7 +8686,11 @@ gnat_to_gnu (Node_Id gnat_node)
 	  declaration, return the result unmodified because we want to use the
 	  return slot optimization in this case.
 
-   5. Finally, if the type of the result is already correct.  */
+   5. If this is a reference to an unconstrained array which is used as the
+	  prefix of an attribute reference that requires an lvalue, return the
+	  result unmodified because we want return the original bounds.
+
+   6. Finally, if the type of the result is already correct.  */
 
   if (Present (Parent (gnat_node))
   && (lhs_or_actual_p (gnat_node)
@@ -8734,13 +8739,19 @@ gnat_to_gnu (Node_Id gnat_node)
   else if (gnu_result == error_mark_node || gnu_result_type == void_type_node)
 gnu_result = error_mark_node;
 
-  else if (Present (Parent (gnat_node))
+  else if (TREE_CODE (gnu_result) == CALL_EXPR
+	   && Present (Parent (gnat_node))
 	   && (Nkind (Parent (gnat_node)) == N_Object_Declaration
 	   || Nkind (Parent (gnat_node)) == N_Object_Renaming_Declaration)
-	   && TREE_CODE (gnu_result) == CALL_EXPR
 	   && return_type_with_variable_size_p (TREE_TYPE (gnu_result)))
 ;
 
+  else if (TREE_CODE (gnu_result) == UNCONSTRAINED_ARRAY_REF
+	   && Present (Parent (gnat_node))
+	   && Nkind (Parent (gnat_node)) == N_Attribute_Reference
+	   && lvalue_required_for_attribute_p (Parent (gnat_node)))
+;
+
   else if (TREE_TYPE (gnu_result) != gnu_result_type)
 gnu_result = convert (gnu_result_type, gnu_result);
 
-- { dg-do run }

procedure Aliased2 is

  type Rec is record
Data : access constant String;
  end record;

  function Get (S : aliased String) return Rec is
R : Rec := (Data => S'Unchecked_Access);
  begin
return R;
  end;

  S : aliased String := "Hello";

  R : Rec := Get (S);

begin
  if R.Data'Length /= S'Length then
raise Program_Error;
  end if;
end;


[Ada] Fix internal error on limited view of incomplete type

2019-05-27 Thread Eric Botcazou
This fixes an ICE on a package whose spec has a limited_with clause of another 
package, whose body has a with clause of the same other package and uses a 
type which is first declared as incomplete in this other package.

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


2019-05-27  Eric Botcazou  

* gcc-interface/trans.c (Gigi_Types_Compatible): New predicate.
(Identifier_to_gnu): Use it to assert that the type of the identifier
and that of its entity are compatible for gigi.  Rename a couple of
local variables and separate the processing of the result type.


2019-05-27  Eric Botcazou  

* gnat.dg/limited_with7.ad[sb]: New test.
* gnat.dg/limited_with7_pkg.ads: New helper.

-- 
Eric Botcazoulimited with Limited_With7_Pkg;

package Limited_With7 is

   procedure Proc (R : out Limited_With7_Pkg.Rec);

end Limited_With7;
-- { dg-do compile }

with Limited_With7_Pkg; use Limited_With7_Pkg;

package body Limited_With7 is

   procedure Proc (R : out Limited_With7_Pkg.Rec) is
   begin
  R.I := 0;
   end;

end Limited_With7;
package Limited_With7_Pkg is

   type Rec;

   type Rec is record
  I : Integer;
   end record;

end Limited_With7_Pkg;
Index: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271647)
+++ gcc-interface/trans.c	(working copy)
@@ -1021,6 +1021,42 @@ fold_constant_decl_in_expr (tree exp)
   gcc_unreachable ();
 }
 
+/* Return true if TYPE and DEF_TYPE are compatible GNAT types for Gigi.  */
+
+static bool
+Gigi_Types_Compatible (Entity_Id type, Entity_Id def_type)
+{
+  /* The trivial case.  */
+  if (type == def_type)
+return true;
+
+  /* A class-wide type is equivalent to a subtype of itself.  */
+  if (Is_Class_Wide_Type (type))
+return true;
+
+  /* A packed array type is compatible with its implementation type.  */
+  if (Is_Packed (def_type) && type == Packed_Array_Impl_Type (def_type))
+return true;
+
+  /* If both types are Itypes, one may be a copy of the other.  */
+  if (Is_Itype (def_type) && Is_Itype (type))
+return true;
+
+  /* If the type is incomplete and comes from a limited context, then also
+ consider its non-limited view.  */
+  if (Is_Incomplete_Type (def_type)
+  && From_Limited_With (def_type)
+  && Present (Non_Limited_View (def_type)))
+return Gigi_Types_Compatible (type, Non_Limited_View (def_type));
+
+  /* If the type is incomplete/private, then also consider its full view.  */
+  if (Is_Incomplete_Or_Private_Type (def_type)
+  && Present (Full_View (def_type)))
+return Gigi_Types_Compatible (type, Full_View (def_type));
+
+  return false;
+}
+
 /* Subroutine of gnat_to_gnu to translate gnat_node, an N_Identifier,
to a GCC tree, which is returned.  GNU_RESULT_TYPE_P is a pointer
to where we should place the result type.  */
@@ -1028,55 +1064,31 @@ fold_constant_decl_in_expr (tree exp)
 static tree
 Identifier_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p)
 {
-  Node_Id gnat_temp, gnat_temp_type;
-  tree gnu_result, gnu_result_type;
-
-  /* Whether we should require an lvalue for GNAT_NODE.  Needed in
- specific circumstances only, so evaluated lazily.  < 0 means
- unknown, > 0 means known true, 0 means known false.  */
-  int require_lvalue = -1;
-
+  /* The entity of GNAT_NODE and its type.  */
+  Node_Id gnat_entity = (Nkind (gnat_node) == N_Defining_Identifier
+			 || Nkind (gnat_node) == N_Defining_Operator_Symbol)
+			? gnat_node : Entity (gnat_node);
+  Node_Id gnat_entity_type = Etype (gnat_entity);
   /* If GNAT_NODE is a constant, whether we should use the initialization
  value instead of the constant entity, typically for scalars with an
  address clause when the parent doesn't require an lvalue.  */
   bool use_constant_initializer = false;
+  /* Whether we should require an lvalue for GNAT_NODE.  Needed in
+ specific circumstances only, so evaluated lazily.  < 0 means
+ unknown, > 0 means known true, 0 means known false.  */
+  int require_lvalue = -1;
+  Node_Id gnat_result_type;
+  tree gnu_result, gnu_result_type;
 
   /* If the Etype of this node is not the same as that of the Entity, then
  something went wrong, probably in generic instantiation.  However, this
  does not apply to types.  Since we sometime have strange Ekind's, just
- do this test for objects.  Moreover, if the Etype of the Entity is private
- or incomplete coming from a limited context, the Etype of the N_Identifier
- is allowed to be the full/non-limited view and we also consider a packed
- array type to be the same as the original type.  Similarly, a CW type is
- equivalent to a subtype of itself.  Finally, if the types are Itypes, one
- may be a copy of the other, which is also legal.  */
-  gnat_temp = ((Nkind (gnat_node) == N_Defining_Identifier
-

[Ada] Fix spurious error on unchecked conversion to misaligned type

2019-05-27 Thread Eric Botcazou
This is a regression present on the mainline, 9 and 8 branches.  The compiler 
issues a spurious error on the result of an unchecked conversion used as an 
actual In parameter in a subprogram call, if the destination type is a scalar 
type subject to a clause which gives an alignment lower than the natural one.

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


2019-05-27  Eric Botcazou  

* gcc-interface/trans.c (Call_to_gnu): Use the unpadded type when
putting back an intermediate conversion the type of the actuals.


2019-05-27  Eric Botcazou  

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

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 336034)
+++ gcc-interface/trans.c	(revision 336035)
@@ -5354,7 +5354,7 @@ Call_to_gnu (Node_Id gnat_node, tree *gn
 	 since the parent is a procedure call, so put it back here.  Note that
 	 we might have a dummy type here if the actual is the dereference of a
 	 pointer to it, but that's OK if the formal is passed by reference.  */
-  tree gnu_actual_type = gnat_to_gnu_type (Etype (gnat_actual));
+  tree gnu_actual_type = get_unpadded_type (Etype (gnat_actual));
   if (TYPE_IS_DUMMY_P (gnu_actual_type))
 	gcc_assert (is_true_formal_parm && DECL_BY_REF_P (gnu_formal));
   else if (suppress_type_conversion
-- { dg-do compile }

with Ada.Unchecked_Conversion;

procedure Unchecked_Convert13 is

  type B16_T is mod 2 ** 16;
  for B16_T'Size use 16;
  for B16_T'Alignment use 1;

  type Rec_T is record
A : Short_Integer;
  end record;
  for Rec_T use record
A at 0 range 0 .. 15;
  end record;
  for Rec_T'Size use 16;

  Rec : constant Rec_T := (A => 0);

  function Rec_To_B16 is new Ada.Unchecked_Conversion (Rec_T, B16_T);

  procedure Nested (B16 : B16_T) is
  begin
null;
  end;

begin
  Nested (Rec_To_B16 (Rec));
end;


[Ada] Improve code generated for Left_Rotate from Interfaces

2019-05-27 Thread Eric Botcazou
This tweaks the GENERIC code emitted by gigi for the Left_Rotate routine from  
the Interfaces package so as to expose more optimization opportunities.

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


2019-05-27  Eric Botcazou  

* gcc-interface/trans.c (gnat_to_gnu) : Convert the
count to the unsigned version of its base type before proceeding.

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271528)
+++ gcc-interface/trans.c	(working copy)
@@ -7422,7 +7422,7 @@ gnat_to_gnu (Node_Id gnat_node)
 	enum tree_code code = gnu_codes[kind];
 	bool ignore_lhs_overflow = false;
 	location_t saved_location = input_location;
-	tree gnu_type;
+	tree gnu_type, gnu_max_shift = NULL_TREE;
 
 	/* Fix operations set up for boolean types in GNU_CODES above.  */
 	if (Is_Modular_Integer_Type (Underlying_Type (Etype (gnat_node
@@ -7445,6 +7445,17 @@ gnat_to_gnu (Node_Id gnat_node)
 	gnu_rhs = gnat_to_gnu (Right_Opnd (gnat_node));
 	gnu_type = gnu_result_type = get_unpadded_type (Etype (gnat_node));
 
+	/* If this is a shift, take the count as unsigned since that is what
+	   most machines do and will generate simpler adjustments below.  */
+	if (IN (kind, N_Op_Shift))
+	  {
+	tree gnu_count_type
+	  = gnat_unsigned_type_for (get_base_type (TREE_TYPE (gnu_rhs)));
+	gnu_rhs = convert (gnu_count_type, gnu_rhs);
+	gnu_max_shift
+	  = convert (TREE_TYPE (gnu_rhs), TYPE_SIZE (gnu_type));
+	  }
+
 	/* Pending generic support for efficient vector logical operations in
 	   GCC, convert vectors to their representative array type view and
 	   fallthrough.  */
@@ -7468,25 +7479,20 @@ gnat_to_gnu (Node_Id gnat_node)
 
 	/* If this is a shift whose count is not guaranteed to be correct,
 	   we need to adjust the shift count.  */
-	if (IN (kind, N_Op_Shift) && !Shift_Count_OK (gnat_node))
-	  {
-	tree gnu_count_type = get_base_type (TREE_TYPE (gnu_rhs));
-	tree gnu_max_shift
-	  = convert (gnu_count_type, TYPE_SIZE (gnu_type));
-
-	if (kind == N_Op_Rotate_Left || kind == N_Op_Rotate_Right)
-	  gnu_rhs = build_binary_op (TRUNC_MOD_EXPR, gnu_count_type,
-	 gnu_rhs, gnu_max_shift);
-	else if (kind == N_Op_Shift_Right_Arithmetic)
-	  gnu_rhs
-		= build_binary_op
-		  (MIN_EXPR, gnu_count_type,
-		   build_binary_op (MINUS_EXPR,
-gnu_count_type,
-gnu_max_shift,
-build_int_cst (gnu_count_type, 1)),
-		   gnu_rhs);
-	  }
+	if ((kind == N_Op_Rotate_Left || kind == N_Op_Rotate_Right)
+	&& !Shift_Count_OK (gnat_node))
+	  gnu_rhs = build_binary_op (TRUNC_MOD_EXPR, TREE_TYPE (gnu_rhs),
+ gnu_rhs, gnu_max_shift);
+	else if (kind == N_Op_Shift_Right_Arithmetic
+		 && !Shift_Count_OK (gnat_node))
+	  gnu_rhs
+	= build_binary_op (MIN_EXPR, TREE_TYPE (gnu_rhs),
+			   build_binary_op (MINUS_EXPR,
+		TREE_TYPE (gnu_rhs),
+		gnu_max_shift,
+		build_int_cst
+		(TREE_TYPE (gnu_rhs), 1)),
+			   gnu_rhs);
 
 	/* For right shifts, the type says what kind of shift to do,
 	   so we may need to choose a different type.  In this case,
@@ -7533,18 +7539,15 @@ gnat_to_gnu (Node_Id gnat_node)
 
 	/* If this is a logical shift with the shift count not verified,
 	   we must return zero if it is too large.  We cannot compensate
-	   above in this case.  */
+	   beforehand in this case.  */
 	if ((kind == N_Op_Shift_Left || kind == N_Op_Shift_Right)
 	&& !Shift_Count_OK (gnat_node))
 	  gnu_result
-	= build_cond_expr
-	  (gnu_type,
-	   build_binary_op (GE_EXPR, boolean_type_node,
-gnu_rhs,
-convert (TREE_TYPE (gnu_rhs),
-	 TYPE_SIZE (gnu_type))),
-	   build_int_cst (gnu_type, 0),
-	   gnu_result);
+	= build_cond_expr (gnu_type,
+			   build_binary_op (GE_EXPR, boolean_type_node,
+		gnu_rhs, gnu_max_shift),
+			   build_int_cst (gnu_type, 0),
+			   gnu_result);
   }
   break;
 


[patch] Fix ICE in resolve_args_picking_1 with -gsplit-dwarf

2019-05-27 Thread Eric Botcazou
The function simply doesn't handle the new location expression opcodes coming 
from DebugFission (https://gcc.gnu.org/wiki/DebugFission).  Fixed by making it 
handle them like DW_OP_addr and DW_OP_const*.

OK for mainline and the active branches (it's a regression from GCC 5)?


2019-05-27  Eric Botcazou  

* dwarf2out.c (resolve_args_picking_1): Deal with DW_OP_GNU_addr_index
and DW_OP_GNU_const_index opcodes.


2019-05-27  Eric Botcazou  

* gnat.dg/specs/array4.ads: New test.

-- 
Eric BotcazouIndex: dwarf2out.c
===
--- dwarf2out.c	(revision 271106)
+++ dwarf2out.c	(working copy)
@@ -17906,6 +17906,8 @@ resolve_args_picking_1 (dw_loc_descr_ref
 	case DW_OP_push_object_address:
 	case DW_OP_call_frame_cfa:
 	case DW_OP_GNU_variable_value:
+	case DW_OP_GNU_addr_index:
+	case DW_OP_GNU_const_index:
 	  ++frame_offset_;
 	  break;
 
-- { dg-do compile }
-- { dg-skip-if "missing -gsplit-dwarf support" { *-*-darwin* } }
-- { dg-options "-gsplit-dwarf" }

package Array4 is

  type Arr1 is array (Positive range <>) of Boolean;

  Size : Positive := 20;

  type Rec is record
A : Arr1 (1 .. Size);
  end record;

  type Arr2 is array (Positive range <>) of Rec;

end Array4;


Re: On-Demand range technology [3/5] - The Prototype

2019-05-23 Thread Eric Botcazou
> While I agree that symbolic ranges are a complication and that most
> cases it currently handles are not "value-range" things I do not agree
> with the idea that we can simply remove handling them and deal
> with the fallout in some later point in the future.  Eric may also be
> able to show you "real" symbolic value-range magic.

There are a couple of testcases in the testsuite that, I believe, require a 
minimal form of support for symbolic ranges: gcc.dg/tree-ssa/vrp94.c and 
gnat.dg/opt40.adb.  They deal with the following pattern in C:

  if (x >= y)
return 1;

  z = y - x;
  if (z <= 0)
abort ();

  return z;

where we want to eliminate the abort.  Of course the C version doesn't really 
make sense on its own, but it's the translation of the Ada version where the

  if (z <= 0)
abort ();

is generated by the compiler (it's a range check in Ada parlance).

I also agree that symbolic ranges are a complication but I don't understand 
why they would conceptually be treated differently from literal ranges.  Of 
course things may been different practically speaking because I also agree 
that they are of lesser importance than literal ranges in most cases.

> Note that symbolic ranges are already restricted to PLUS_EXPR
> and MINUS_EXPR (and NEGATE_EXPR I think).  There are
> also "symbolic" (non-integer constant) ranges like [, ].

Yes, the current implementation is restricted to additive operations.

-- 
Eric Botcazou


[Ada] Stabilize sort for -fdump-ada-spec

2019-05-23 Thread Eric Botcazou
Self-explanatory.  Tested on x86-64/Lnux, applied on mainline and 9 branch.


2019-05-23  Eric Botcazou  

* c-ada-spec.c (compare_node): Compare the DECL_UIDs as a last resort.

-- 
Eric BotcazouIndex: c-ada-spec.c
===
--- c-ada-spec.c	(revision 271457)
+++ c-ada-spec.c	(working copy)
@@ -679,8 +679,10 @@ compare_node (const void *lp, const void
 {
   const_tree lhs = *((const tree *) lp);
   const_tree rhs = *((const tree *) rp);
+  const int ret
+= compare_location (decl_sloc (lhs, true), decl_sloc (rhs, true));
 
-  return compare_location (decl_sloc (lhs, true), decl_sloc (rhs, true));
+  return ret ? ret : DECL_UID (lhs) - DECL_UID (rhs);
 }
 
 /* Compare two comments (LP and RP) by their source location.  */


[testsuite] Add testcase to gnat.dg testsuite

2019-05-23 Thread Eric Botcazou
This adds the Ada testcase that detected a since then fixed regression on the 
8 branch introduced by a backport.

Tested on x86_64-suse-linux, applied on mainline, 9 and 8 branches.


2019-05-23  Eric Botcazou  

* gnat.dg/opt78.ad[sb]: New test.

-- 
Eric Botcazou-- { dg-do compile }
-- { dg-options "-O" }

package body Opt78 is

   procedure Proc (P : UC; Msg : String) is
  Default : UC := (1, "!");
   begin
  if P = Default then
 raise Program_Error;
  else
 raise Constraint_Error;
  end if;
   end;

end Opt78;
package Opt78 is

   subtype Reasonable is Integer range 1..10;

   type UC (D: Reasonable := 2) is record
  S: String (1 .. D) := "Hi";
   end record;

   type AUC is access all UC;

   procedure Proc (P : UC; Msg : String);

end Opt78;


Re: Backport fix for PR c++/85400

2019-05-22 Thread Eric Botcazou
> Probably also required for GCC 7 then?

The problem is (probably) present there too, but that's not my call.

-- 
Eric Botcazou


Backport fix for PR c++/85400

2019-05-22 Thread Eric Botcazou
Jakub asked me to backport the fix for PR c++/85400 initially intended for 
SPARC/Solaris but which also helps on x86-64/Linux apparently.

Tested on the latter platform, applied on the 8 branch.


2019-05-22  Eric Botcazou  

c-family/
Backport from mainline
2018-05-10  Eric Botcazou  

PR c++/85400
* c-attribs.c (handle_visibility_attribute): Do not set no_add_attrs.


2019-05-22  Eric Botcazou  

cp/
Backport from mainline
2018-05-10  Eric Botcazou  

PR c++/85400
* decl2.c (adjust_var_decl_tls_model): New static function.
(comdat_linkage): Call it on a variable.
(maybe_make_one_only): Likewise.


2019-05-22  Eric Botcazou  

* g++.dg/tls/pr85400.C: New test.

-- 
Eric BotcazouIndex: c-family/c-attribs.c
===
--- c-family/c-attribs.c	(revision 270883)
+++ c-family/c-attribs.c	(working copy)
@@ -2310,14 +2310,13 @@ handle_visibility_attribute (tree *node,
 
 static tree
 handle_tls_model_attribute (tree *node, tree name, tree args,
-			int ARG_UNUSED (flags), bool *no_add_attrs)
+			int ARG_UNUSED (flags),
+			bool *ARG_UNUSED (no_add_attrs))
 {
   tree id;
   tree decl = *node;
   enum tls_model kind;
 
-  *no_add_attrs = true;
-
   if (!VAR_P (decl) || !DECL_THREAD_LOCAL_P (decl))
 {
   warning (OPT_Wattributes, "%qE attribute ignored", name);
Index: cp/decl2.c
===
--- cp/decl2.c	(revision 270883)
+++ cp/decl2.c	(working copy)
@@ -1838,6 +1838,17 @@ mark_vtable_entries (tree decl)
 }
 }
 
+/* Adjust the TLS model on variable DECL if need be, typically after
+   the linkage of DECL has been modified.  */
+
+static void
+adjust_var_decl_tls_model (tree decl)
+{
+  if (CP_DECL_THREAD_LOCAL_P (decl)
+  && !lookup_attribute ("tls_model", DECL_ATTRIBUTES (decl)))
+set_decl_tls_model (decl, decl_default_tls_model (decl));
+}
+
 /* Set DECL up to have the closest approximation of "initialized common"
linkage available.  */
 
@@ -1888,6 +1899,9 @@ comdat_linkage (tree decl)
 
   if (TREE_PUBLIC (decl))
 DECL_COMDAT (decl) = 1;
+
+  if (VAR_P (decl))
+adjust_var_decl_tls_model (decl);
 }
 
 /* For win32 we also want to put explicit instantiations in
@@ -1926,6 +1940,8 @@ maybe_make_one_only (tree decl)
 	  /* Mark it needed so we don't forget to emit it.  */
   node->forced_by_abi = true;
 	  TREE_USED (decl) = 1;
+
+	  adjust_var_decl_tls_model (decl);
 	}
 }
 }


[Ada] Small tweak to -fdump-ada-spec for assignment operators

2019-05-21 Thread Eric Botcazou
The interfacing with C++ assignment operators in Ada is not very clear but we 
should at least make sure that the generated binding compile.

Tested on x86_64-suse-linux, applied on mainline as obvious for the cp/ part.


2019-05-21  Eric Botcazou  

c-family/
* c-ada-spec.h (enum cpp_operation): Add IS_ASSIGNMENT_OPERATOR.
* c-ada-spec.c (print_assignment_operator): New function.
(dump_ada_declaration) : Call it do dump explicit copy
assignment operators declared as methods and filter out the others.


2019-05-21  Eric Botcazou  

cp/
* decl2.c (cpp_check) : New case.

-- 
Eric BotcazouIndex: c-family/c-ada-spec.c
===
--- c-family/c-ada-spec.c	(revision 271106)
+++ c-family/c-ada-spec.c	(working copy)
@@ -2681,6 +2681,17 @@ print_destructor (pretty_printer *buffer
   pp_ada_tree_identifier (buffer, decl_name, t, false);
 }
 
+/* Dump in BUFFER assignment operator spec corresponding to T.  */
+
+static void
+print_assignment_operator (pretty_printer *buffer, tree t, tree type)
+{
+  tree decl_name = DECL_NAME (TYPE_NAME (type));
+
+  pp_string (buffer, "Assign_");
+  pp_ada_tree_identifier (buffer, decl_name, t, false);
+}
+
 /* Return the name of type T.  */
 
 static const char *
@@ -2920,6 +2931,7 @@ dump_ada_declaration (pretty_printer *bu
   bool is_method = TREE_CODE (TREE_TYPE (t)) == METHOD_TYPE;
   tree decl_name = DECL_NAME (t);
   bool is_abstract = false;
+  bool is_assignment_operator = false;
   bool is_constructor = false;
   bool is_destructor = false;
   bool is_copy_constructor = false;
@@ -2931,6 +2943,7 @@ dump_ada_declaration (pretty_printer *bu
   if (cpp_check)
 	{
 	  is_abstract = cpp_check (t, IS_ABSTRACT);
+	  is_assignment_operator = cpp_check (t, IS_ASSIGNMENT_OPERATOR);
 	  is_constructor = cpp_check (t, IS_CONSTRUCTOR);
 	  is_destructor = cpp_check (t, IS_DESTRUCTOR);
 	  is_copy_constructor = cpp_check (t, IS_COPY_CONSTRUCTOR);
@@ -2955,6 +2968,13 @@ dump_ada_declaration (pretty_printer *bu
 	return 0;
 	}
 
+  else if (is_assignment_operator)
+	{
+	  /* ??? Skip implicit or non-method assignment operators for now.  */
+	  if (DECL_ARTIFICIAL (t) || !is_method)
+	return 0;
+	}
+
   /* If this function has an entry in the vtable, we cannot omit it.  */
   else if (!DECL_VINDEX (t) && *IDENTIFIER_POINTER (decl_name) == '_')
 	{
@@ -2977,6 +2997,8 @@ dump_ada_declaration (pretty_printer *bu
 	print_constructor (buffer, t, type);
   else if (is_destructor)
 	print_destructor (buffer, t, type);
+  else if (is_assignment_operator)
+	print_assignment_operator (buffer, t, type);
   else
 	dump_ada_decl_name (buffer, t, false);
 
Index: c-family/c-ada-spec.h
===
--- c-family/c-ada-spec.h	(revision 271106)
+++ c-family/c-ada-spec.h	(working copy)
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.
 enum cpp_operation {
   HAS_DEPENDENT_TEMPLATE_ARGS,
   IS_ABSTRACT,
+  IS_ASSIGNMENT_OPERATOR,
   IS_CONSTRUCTOR,
   IS_DESTRUCTOR,
   IS_COPY_CONSTRUCTOR,
Index: cp/decl2.c
===
--- cp/decl2.c	(revision 271106)
+++ cp/decl2.c	(working copy)
@@ -4247,6 +4247,8 @@ cpp_check (tree t, cpp_operation op)
 	}
   case IS_ABSTRACT:
 	return DECL_PURE_VIRTUAL_P (t);
+  case IS_ASSIGNMENT_OPERATOR:
+	return DECL_ASSIGNMENT_OPERATOR_P (t);
   case IS_CONSTRUCTOR:
 	return DECL_CONSTRUCTOR_P (t);
   case IS_DESTRUCTOR:


Re: [PowerPC 64]r12 is not updated to GEP when control transferred from virtual thunk function .

2019-05-17 Thread Eric Botcazou
> I do think so.  The call is locally, linker should know it's local and
> fix it up with local entry instead.  We don't need to keep r12 always
> hold the entry of being called function.

Yes, but on VxWorks in kernel mode you first do partial linking and then the 
loader resolves the remaining relocations.  None of them plays the usual dance 
with the local and global entry points implied by the ELFv2 ABI.

-- 
Eric Botcazou


Re: [PowerPC 64]r12 is not updated to GEP when control transferred from virtual thunk function .

2019-05-15 Thread Eric Botcazou
> Thank you Eric for the suggestion and say that we support in the  loader
> part ,can you please point on elfv2 reference that says implementation for
> this specific case.

I don't think there is a reference to this specific case in the ABI, only the 
general stuff about local and global entry points.  We have had this patch in 
our tree for some time and it works well, so let me submit it for inclusion in 
the official tree.

-- 
Eric Botcazou


Re: [PowerPC 64]r12 is not updated to GEP when control transferred from virtual thunk function .

2019-05-15 Thread Eric Botcazou
> like above the control  from "_ZThn8_N12Intermediate1vEv" (support
> function for this pointer update)  is transferred
> "_ZN12Intermediate1vEv" by b  inst (where its not updating the r12)
> and in the beginning  of   "_ZN12Intermediate1vEv" we are loading the
> toc base from r12 (which is incorrect ) ,we are investigating the
> issue and one way to fix the issue is that make THUNK to update the
> r12 ,the cal like bctrl  or  load the r12 with the function address in
> the _ZN12Intermediate1vEv prologue code .

Is that on VxWorks in kernel mode?  If so, the loader doesn't abide by the 
ELFv2 ABI so the simple way out is to disable asm thunks altogether:

#undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
#define TARGET_ASM_CAN_OUTPUT_MI_THUNK rs6000_can_output_mi_thunk

/* Return true if rs6000_output_mi_thunk would be able to output the
   assembler code for the thunk function specified by the arguments
   it is passed, and false otherwise.  */

static bool
rs6000_can_output_mi_thunk (const_tree, HOST_WIDE_INT, HOST_WIDE_INT,
const_tree)
{
  /* The only possible issue is for VxWorks in kernel mode.  */
  if (!TARGET_VXWORKS || TARGET_VXWORKS_RTP)
return true;

  /* The loader neither creates the glue code sequence that loads r12 nor uses
 the local entry point for the sibcall's target in the ELFv2 ABI.  */
  return DEFAULT_ABI != ABI_ELFv2;
}

-- 
Eric Botcazou


Re: New .md construct: define_insn_and_rewrite

2019-05-14 Thread Eric Botcazou
> This patch therefore adds a new construct called define_insn_and_rewrite.
> It's basically a define_insn_and_split with an implicit split pattern,
> obtained by copying the insn pattern and replacing match_operands with
> match_dups and match_operators with match_op_dups.

Isn't that what define_subst does in a different context?

> Any comments?  Suggestions for better names?

define_insn_and_subst?

-- 
Eric Botcazou


Re: A bug in vrp_meet?

2019-05-05 Thread Eric Botcazou
> I have now applied this variant.

You backported it onto the 8 branch on Friday:

2019-05-03  Richard Biener  

Backport from mainline
[...]
2019-03-07  Richard Biener  

PR tree-optimization/89595
* tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Take
stmt iterator as reference, take boolean output parameter to
indicate whether the stmt was removed and thus the iterator
already advanced.
(dom_opt_dom_walker::before_dom_children): Re-iterate over
stmts created by folding.

and this introduced a regression for the attached Ada testcase at -O:

Program received signal SIGSEGV, Segmentation fault.
0x0102173c in set_value_range (
vr=0x17747a0 , t=VR_RANGE, min=0x76c3df78, max=, equiv=0x0)
at /home/eric/svn/gcc-8-branch/gcc/tree-vrp.c:298
298   vr->type = t;

on x86-64 at least.  Mainline and 9 branch are not affected.

-- 
Eric Botcazoupackage Opt78 is

   subtype Reasonable is Integer range 1..10;

   type UC (D: Reasonable := 2) is record
  S: String (1 .. D) := "Hi";
   end record;

   type AUC is access all UC;

   procedure Proc (P : UC; Msg : String);

end Opt78;
-- { dg-do compile }
-- { dg-options "-O" }

package body Opt78 is

   procedure Proc (P : UC; Msg : String) is
  Default : UC := (1, "!");
   begin
  if P = Default then
 raise Program_Error;
  else
 raise Constraint_Error;
  end if;
   end;

end Opt78;


Re: A bug in vrp_meet?

2019-05-05 Thread Eric Botcazou
> I have now applied this variant.

You backported it onto the 8 branch on Friday:

2019-05-03  Richard Biener  

Backport from mainline
[...]
2019-03-07  Richard Biener  

PR tree-optimization/89595
* tree-ssa-dom.c (dom_opt_dom_walker::optimize_stmt): Take
stmt iterator as reference, take boolean output parameter to
indicate whether the stmt was removed and thus the iterator
already advanced.
(dom_opt_dom_walker::before_dom_children): Re-iterate over
stmts created by folding.

and this introduced a regression for the attached Ada testcase at -O:

Program received signal SIGSEGV, Segmentation fault.
0x0102173c in set_value_range (
vr=0x17747a0 , t=VR_RANGE, min=0x76c3df78, max=, equiv=0x0)
at /home/eric/svn/gcc-8-branch/gcc/tree-vrp.c:298
298   vr->type = t;

on x86-64 at least.  Mainline and 9 branch are not affected.

-- 
Eric Botcazoupackage Opt78 is

   subtype Reasonable is Integer range 1..10;

   type UC (D: Reasonable := 2) is record
  S: String (1 .. D) := "Hi";
   end record;

   type AUC is access all UC;

   procedure Proc (P : UC; Msg : String);

end Opt78;
-- { dg-do compile }
-- { dg-options "-O" }

package body Opt78 is

   procedure Proc (P : UC; Msg : String) is
  Default : UC := (1, "!");
   begin
  if P = Default then
 raise Program_Error;
  else
 raise Constraint_Error;
  end if;
   end;

end Opt78;


Re: [PATCH] Fix up RTL splitting of asm goto (PR target/90193)

2019-04-24 Thread Eric Botcazou
> 2019-04-24  Jakub Jelinek  
> 
>   PR target/90193
>   * rtl.c (classify_insn): Return JUMP_INSN for asm goto.
>   * emit-rtl.c (try_split): Copy over REG_LABEL_TARGET.
> 
>   * gcc.target/i386/pr90193.c: New test.

OK, thanks.

-- 
Eric Botcazou


Re: [PATCH] Fix ARM exception handling (PR target/89093)

2019-04-24 Thread Eric Botcazou
> The Ada changes need those guards because the file is compiled by both
> the system compiler and by the newly built compilers; when compiled by
> system compiler, as the FE is built with -fno-exceptions I'd hope the EH
> stuff isn't really used there and at least until GCC 9.1 is released we have
> the issue that the system compiler could be some earlier GCC 9.0.1 snapshot
> which doesn't support general-regs-only.

The Ada front-end does use EH on the host, but only the part written in Ada, 
that's why -fno-exceptions is very likely still OK for the C++ part.

The Ada bits are OK and I guess we don't care about earlier 9.0.1 snapshots.

-- 
Eric Botcazou


Re: [PATCH] Fix __builtin_*mul*_overflow* expansion (PR middle-end/90095)

2019-04-16 Thread Eric Botcazou
> The runtime check assures that at runtime, the upper 32 bits of pseudo 104
> must be always 0 (in this case, in some other case could be sign bit
> copies).

OK, as Richard pointed out, that's not sufficient if we allow...

> The question is if it would be valid say for forward propagation to first
> propagate (or combine) the pseudo 97 into the (subreg/s/v:SI (reg:DI 104)
> 0), then hoisting it before the jump_insn 16, have the subreg optimized
> away and miscompile later on.

...this to happen.  So we could clear SUBREG_PROMOTED_VAR_P as soon as the 
SUBREG is rewritten, but this looks quite fragile.  The safest route is 
probably not to use SUBREG_PROMOTED_VAR_P in this conditional context.

> That means either that the hoisting pass is buggy, or that SUBREG_PROMOTED_*
> is only safe at the function boundary (function arguments and return value)
> and not elsewhere.

I think that Richard's characterization is correct:

"Note that likely SUBREG_PROMOTED_VAR_P wasn't designed to communicate
zero-extend info (can't you use a REG_EQUIV note somehow?) but it has
to be information that is valid everywhere in the function unless
data dependences force its motion (thus a conditional doesn't do)."

i.e. this also works for a local variable that is always accessed with the 
SUBREG_PROMOTED_VAR_P semantics.

-- 
Eric Botcazou


Re: [PATCH] Fix __builtin_*mul*_overflow* expansion (PR middle-end/90095)

2019-04-16 Thread Eric Botcazou
> If I remember correctly and read the cfgexpand.c comment
> 
>   /* For a promoted variable, X will not be used directly but wrapped in a
>  SUBREG with SUBREG_PROMOTED_VAR_P set, which means that the RTL land
>  will assume that its upper bits can be inferred from its lower bits.
>  Therefore, if X isn't initialized on every path from the entry, then
>  we must do it manually in order to fulfill the above assumption.  */
> 
> you run into a similar situation but no, I don't think your patch is
> really fixing this given SUBREG_PROMOTED_VAR_P is only true depending
> on a condition (and not a data-dependence).

If the new pseudo is initialized with the above semantics on every possible 
path where it is used, then I think it's OK.

> Note that likely SUBREG_PROMOTED_VAR_P wasn't designed to communicate
> zero-extend info (can't you use a REG_EQUIV note somehow?) but it has
> to be information that is valid everywhere in the function unless
> data dependences force its motion (thus a conditional doesn't do).

SUBREG_PROMOTED_VAR_P is fundamentally a property of the inner pseudo and not 
of the SUBREG it is put on, hence...

> (oh, and yes, I think SUBREG_PROMOTED_VAR_P is a dangerous thing,
> but so is VRP info on SSA names as we've learned...)

...yes, it is delicate to deal with.

-- 
Eric Botcazou


[Ada] Fix wrong translation of C++ virtual destructor

2019-04-09 Thread Eric Botcazou
This is a regression present on all active branches: -fdump-ada-spec no longer 
generates a declaration for the (implicit) deleting destructor in a class, 
which is problematic if it's virtual because it has a slot in the vtable.

Tested on x86_64-suse-linux, applied on all active branches.


2019-04-09  Eric Botcazou  

c-family/
* c-ada-spec.c (print_destructor): Deal with deleting destructors.
(dump_ada_declaration) : Likewise.

-- 
Eric BotcazouIndex: c-ada-spec.c
===
--- c-ada-spec.c	(revision 270188)
+++ c-ada-spec.c	(working copy)
@@ -2676,6 +2676,8 @@ print_destructor (pretty_printer *buffer
   tree decl_name = DECL_NAME (TYPE_NAME (type));
 
   pp_string (buffer, "Delete_");
+  if (strncmp (IDENTIFIER_POINTER (DECL_NAME (t)), "__dt_del", 8) == 0)
+pp_string (buffer, "And_Free_");
   pp_ada_tree_identifier (buffer, decl_name, t, false);
 }
 
@@ -2946,9 +2948,10 @@ dump_ada_declaration (pretty_printer *bu
 	  if (DECL_ARTIFICIAL (t))
 	return 0;
 
-	  /* Only consider constructors/destructors for complete objects.  */
+	  /* Only consider complete constructors and deleting destructors.  */
 	  if (strncmp (IDENTIFIER_POINTER (decl_name), "__ct_comp", 9) != 0
-	  && strncmp (IDENTIFIER_POINTER (decl_name), "__dt_comp", 9) != 0)
+	  && strncmp (IDENTIFIER_POINTER (decl_name), "__dt_comp", 9) != 0
+	  && strncmp (IDENTIFIER_POINTER (decl_name), "__dt_del", 8) != 0)
 	return 0;
 	}
 


[Ada] Small -fdump-ada-spec fix for recent glibc

2019-04-07 Thread Eric Botcazou
This fixes a small issue that may be present when you use -fdump-ada-spec with 
very recent glibc releases.  Tested on Ubuntu 18.04, applied on the mainline.


2019-04-07  Eric Botcazou  

c-family/
* c-ada-spec.c (is_float128): New predicate extracted from...
(dump_ada_node) : Use it to recognize __cfloat128.
: ...here.  Call it.


2019-04-07  Eric Botcazou  

ada/
* libgnat/i-cexten.ads (CFloat_128): New type.

-- 
Eric BotcazouIndex: ada/libgnat/i-cexten.ads
===
--- ada/libgnat/i-cexten.ads	(revision 270181)
+++ ada/libgnat/i-cexten.ads	(working copy)
@@ -74,7 +74,7 @@ package Interfaces.C.Extensions is
for Signed_128'Alignment use unsigned_long_long'Alignment * 2;
 
--  128-bit floating-point type available on x86:
-   --  typedef long_double float_128 __attribute__ ((mode (TF)));
+   --  typedef float float_128 __attribute__ ((mode (TF)));
 
type Float_128 is record
   low, high : unsigned_long_long;
@@ -82,6 +82,14 @@ package Interfaces.C.Extensions is
pragma Convention (C_Pass_By_Copy, Float_128);
for Float_128'Alignment use unsigned_long_long'Alignment * 2;
 
+   --  128-bit complex floating-point type available on x86:
+   --  typedef _Complex float cfloat_128 __attribute__ ((mode (TC)));
+
+   type CFloat_128 is record
+  re, im : Float_128;
+   end record;
+   pragma Convention (C_Pass_By_Copy, CFloat_128);
+
--  Types for bitfields
 
type Unsigned_1 is mod 2 ** 1;
Index: c-family/c-ada-spec.c
===
--- c-family/c-ada-spec.c	(revision 270181)
+++ c-family/c-ada-spec.c	(working copy)
@@ -2014,6 +2014,22 @@ dump_ada_enum_type (pretty_printer *buff
 }
 }
 
+/* Return true if NODE is the __float128/_Float128 type.  */
+
+static bool
+is_float128 (tree node)
+{
+  if (!TYPE_NAME (node) || TREE_CODE (TYPE_NAME (node)) != TYPE_DECL)
+return false;
+
+  tree name = DECL_NAME (TYPE_NAME (node));
+
+  if (IDENTIFIER_POINTER (name) [0] != '_')
+return false;
+
+  return id_equal (name, "__float128") || id_equal (name, "_Float128");
+}
+
 static bool bitfield_used = false;
 
 /* Recursively dump in BUFFER Ada declarations corresponding to NODE of type
@@ -2067,7 +2083,13 @@ dump_ada_node (pretty_printer *buffer, t
   break;
 
 case COMPLEX_TYPE:
-  pp_string (buffer, "");
+  if (is_float128 (TREE_TYPE (node)))
+	{
+	  append_withs ("Interfaces.C.Extensions", false);
+	  pp_string (buffer, "Extensions.CFloat_128");
+	}
+  else
+	pp_string (buffer, "");
   break;
 
 case ENUMERAL_TYPE:
@@ -2078,11 +2100,7 @@ dump_ada_node (pretty_printer *buffer, t
   break;
 
 case REAL_TYPE:
-  if (TYPE_NAME (node)
-	  && TREE_CODE (TYPE_NAME (node)) == TYPE_DECL
-	  && IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (node))) [0] == '_'
-	  && (id_equal (DECL_NAME (TYPE_NAME (node)), "_Float128")
-	  || id_equal (DECL_NAME (TYPE_NAME (node)), "__float128")))
+  if (is_float128 (node))
 	{
 	  append_withs ("Interfaces.C.Extensions", false);
 	  pp_string (buffer, "Extensions.Float_128");


Re: [patch] Fix LRA/reload issue with -fnon-call-exceptions

2019-04-06 Thread Eric Botcazou
> I have backported it onto the 8 branch, where it fixes the failure (timeout)
> of gnat.dg/opt73.adb for PowerPC/Linux, after testing it on this platform.

And now onto the 7 branch, after testing on x86-64/Linux and PowerPC/Linux.

-- 
Eric Botcazou


Re: is re-running bootstrap after a change safe?

2019-04-05 Thread Eric Botcazou
> Say if the first bootstrap succeeds and I then change a single
> GCC .c file and rerun make bootstrap, am I guaranteed to see
> the same fallout of the change as I would if I did a pristine
> build in a clean directory?

No, this would imply deleting the stage2 and stage3 compilers and that isn't 
what happens.  Instead the compiler of each stage is updated in isolation.

-- 
Eric Botcazou


[SPARC] Disable ASAN on 64-bit SPARC/Linux

2019-04-02 Thread Eric Botcazou
With the same error message as on SPARC/Solaris (as I have neither the time 
nor the energy to get through the awkward upstream submission process again).

Tested on SPARC/Solaris and SPARC/Linux, applied on the mainline.


2019-04-02  Eric Botcazou  

* config/sparc/linux64.h (ASAN_REJECT_SPEC): New macro.
(ASAN_CC1_SPEC): Use it in 64-bit mode.
* config/sparc/sol2.h (ASAN_REJECT_SPEC): Remove superfluous colon.

-- 
Eric BotcazouIndex: config/sparc/linux64.h
===
--- config/sparc/linux64.h	(revision 270031)
+++ config/sparc/linux64.h	(working copy)
@@ -143,8 +143,18 @@ extern const char *host_detect_local_cpu
 
 #define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
 
+/* -fsanitize=address is currently only supported for 32-bit.  */
+#define ASAN_REJECT_SPEC \
+  "%{!%:sanitize(thread):%e-fsanitize=address is not supported in this configuration}"
+
 #undef  ASAN_CC1_SPEC
-#define ASAN_CC1_SPEC "%{%:sanitize(address):-funwind-tables}"
+#if DEFAULT_ARCH32_P
+#define ASAN_CC1_SPEC \
+  "%{%:sanitize(address):-funwind-tables %{m64:" ASAN_REJECT_SPEC "}}"
+#else
+#define ASAN_CC1_SPEC \
+  "%{%:sanitize(address):-funwind-tables %{!m32:" ASAN_REJECT_SPEC "}}"
+#endif
 
 #undef  CC1_SPEC
 #if DEFAULT_ARCH32_P
Index: config/sparc/sol2.h
===
--- config/sparc/sol2.h	(revision 270031)
+++ config/sparc/sol2.h	(working copy)
@@ -324,7 +324,7 @@ extern const char *host_detect_local_cpu
 
 /* -fsanitize=address is currently only supported for 32-bit.  */
 #define ASAN_REJECT_SPEC \
-  DEF_ARCH64_SPEC("%e:-fsanitize=address is not supported in this configuration")
+  DEF_ARCH64_SPEC("%e-fsanitize=address is not supported in this configuration")
 
 
 /* Register the Solaris-specific #pragma directives.  */


[patch] Fix build failure with MinGW

2019-03-30 Thread Eric Botcazou
Tested on x86_64-pc-mingw32, OK for the mainline?


2019-03-30  Eric Botcazou  

* src/c++17/fs_ops.cc (fs::permissions): Use std::errc::not_supported.

-- 
Eric Botcazoudiff --git libstdc++-v3/src/c++17/fs_ops.cc libstdc++-v3/src/c++17/fs_ops.cc
index 3ff0ded1c66..5ca523826cb 100644
--- libstdc++-v3/src/c++17/fs_ops.cc
+++ libstdc++-v3/src/c++17/fs_ops.cc
@@ -1127,7 +1127,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
 err = errno;
 #else
   if (nofollow && is_symlink(st))
-ec = std::make_error_code(std::errc::operation_not_supported);
+ec = std::make_error_code(std::errc::not_supported);
   else if (posix::chmod(p.c_str(), static_cast(prms)))
 err = errno;
 #endif


Re: [PR89862] Fix ARM lto bootstrap

2019-03-29 Thread Eric Botcazou
> I have also tested the patch with x86_64-linux-gnu with no new regressions.
> Is this OK for trunk?

Yes, but please put it on all active branches.

-- 
Eric Botcazou


Re: [Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)

2019-03-27 Thread Eric Botcazou
> I modified the patch.
> Please let me know your thoughts.

Thanks.  This version is certainly better, but I cannot approve it myself.

Uros, given that the bug was fixed in 64-bit mode for GCC 9, I think that it 
would make sense to have it fixed in 32-bit mode too.

-- 
Eric Botcazou


Re: Function pointers to a nested function / contained procedure

2019-03-26 Thread Eric Botcazou
> At the moment, I am at a loss of how to try to fix this.  Any ideas?
> Is there any other language which has such a feature, so a bit of
> judicious copy & paste could be applied?

(GNU) C and Ada since the dawn of time.  There is an entire machinery in the 
middle-end and the back-ends to support this (look for trampolines/descriptors 
in the manual and the source code).  This should essentially work out of the 
box for any language front-end.

-- 
Eric Botcazou


Re: [PATCH] rs6000: Make CSE'ing __tls_get_addr calls possible

2019-03-24 Thread Eric Botcazou
> There were REG_EQUAL notes on these tls calls in the past, but I
> recall removing them for one reason or another. So watch out for
> fall-out from this patch! ;-)

Others ports have them though, for example i386, ARM and SPARC.

-- 
Eric Botcazou


Re: [Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)

2019-03-23 Thread Eric Botcazou
> The attached patch (pr85667.patch) fixes the subjected issue for 32-bit.
> Please let me know your thoughts on the patch.

IMO you ought not to duplicate most of function_value_32 here.

-- 
Eric Botcazou


Re: [patch] Fix wrong code for boolean negation in condition at -O

2019-03-23 Thread Eric Botcazou
> Umm, did you look at ssa_name_has_boolean_range?  Isn't the problem that
> Ada's BOOLEAN_TYPE has a wider range than [0..1] and thus this is the
> wrong bit of code:
> 
>   /* Boolean types always have a range [0..1].  */
>   if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
> return true;

You seem to be discovering that boolean types can have a precision > 1; that's 
the case in Ada and Fortran.  They are indeed a bit delicate to handle but the 
invariant is that the 0-or-1 value must be preserved for them too, i.e. you 
cannot create another value out of thin air.

> IIRC there are other places that have similar logic and rely on
> ssa_name_has_boolean_range to filter out anything undesirable.

The other places are more careful, i.e. they explicitly test for 0 or 1.

-- 
Eric Botcazou


Re: [patch] Fix ASAN failures on SPARC64/Linux

2019-03-14 Thread Eric Botcazou
> Not if the >> 3 shift is arithmetic shift.

Sorry, I don't understand how this can work.

-- 
Eric Botcazou


Re: [PATCH] Fix miscompilation due to REG_EQUAL note on a paradoxical subreg operation (PR rtl-optimization/89679)

2019-03-14 Thread Eric Botcazou
> Below is one patch, more in the spirit of
> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg00418.html
> where we refuse to add REG_EQUAL note in that case.
> 
> Attached is another variant, which keeps it around just in case something
> would make use of it, but makes sure we don't fwprop into such REG_EQUAL
> notes replacing a paradoxical subreg in there with something else.
> 
> Both patches were bootstrapped/regtested on
> {x86_64,i686,powerpc64le,s390x,aarch64}-linux, ok for trunk (which one)?
> 
> 2019-03-13  Jakub Jelinek  
> 
>   PR rtl-optimization/89679
>   * expmed.c (expand_mult_const): Don't add a REG_EQUAL note if it
>   would contain a paradoxical SUBREG.

My vote would be for this one.

-- 
Eric Botcazou


Re: [patch] Fix ASAN failures on SPARC64/Linux

2019-03-13 Thread Eric Botcazou
> So, when using the MemToShadow(addr) (1UL << 43) + ((addr << 12) >> (12 +
> 3)) mapping, the first valid address above the hole will have shadow at:
>  0x00020700UL (will not work, as it is inside of the VA hole)
>  0x0001f800UL (will not work, as it is inside of the VA hole)
>  0x00010800UL (this is the only case that will work)
>  0x0800UL (will not work; that would mean that both the low and
>  high memory share the same shadow memory;
>  it could be made to work by doing mmap
>  (0xfff0UL, 0x8UL, MAP_FIXED, 
> PROT_NONE)
>  at libasan initialization and failing if that doesn't
>  succeed)

OK, I can certainly do the last thing.

> Note for the first VA layout even the shadow offset 1UL << 43 will not work
> at all even for the low part of the memory, as all of shadow memory is then
> in the hole.

Yes, you need a kernel configured for SPARC-T4 or later.

> I think hardcoding one of the 4 choices in the ABI is undesirable.

Frankly I'm not sure why you care about the ABI of the AddressSanitizer...

> Another possibility is instead of using constant offset + 2 shifts use a
> variable offset + the normal >> 3, where the offset would be chosen by the
> runtime library depending on the VA hole size and where the shadow for the
> high memory would precede the shadow for the low memory.

But you still need to chop the high bits, otherwise you end up in the hole.

-- 
Eric Botcazou


Re: [patch] Fix ASAN failures on SPARC64/Linux

2019-03-13 Thread Eric Botcazou
> It actually is something that works with all the VA sizes that are
> supported.

Well, there were changes in the past that seem to indicate that this has not 
always been true but, of course, the very specific VM layout on SPARC 64-bit 
(apparently inherited from Solaris) makes things much more convoluted...

Moreover, I'm not sure this is a very important issue, people presumably don't 
run binaries compiled with -fsanitize=address in production, so having to 
recompile with a matching GCC version doesn't seem that much of a hurdle.

-- 
Eric Botcazou


Re: [patch] Fix ASAN failures on SPARC64/Linux

2019-03-13 Thread Eric Botcazou
> Is the size of the virtual address space hole constant though (and will it
> remain constant)?

The kernel sources say that it's constant and with this position for SPARC-T4 
and later.  It's different (larger hole) for SPARC-T3 and earlier but I cannot 
really test.  I don't think that it will change for a given processor.

> E.g. on powerpc64 or aarch64 there are in each case like 4-5 different VA
> size configurations over the last 10+ years of kernel history and
> configuration options and fortunately all that is hidden inside of libasan,
> if you have older gcc and run into an unsupported VA configuration, all it
> takes is update libasan to one that supports it and binaries continue to
> work.

But a few targets have hardcoded VA size in TARGET_ASAN_SHADOW_OFFSET too.

> Could libasan initialization if it detects just PROT_NONE mmap from the end
> of hole to the start of the region it really supports (and fail if that
> fails), so that backward compatibility is ensured?

I'll investigate how targets supporting multiple VA size behave, but I don't 
have access to a large range of SPARC machines...

> Also, don't you need some corresponding libsanitizer changes?

Of course, just merged.

-- 
Eric Botcazou


[libsanitizer] AddressSanitizer: 64-bit SPARC/Linux port

2019-03-13 Thread Eric Botcazou
This patch contains the bits required to make the AddressSanitizer work on 
SPARC64/Linux (SPARC-T4 and later).  It only affects the SPARC ports and has 
been tested on SPARC/Solaris and SPARC64/Linux.

It merges r355980 of the LLVM repository.  Installed on the mainline.


2019-03-13  Eric Botcazou  

PR sanitizer/80953
Merge from LLVM revision 355980
* asan/asan_allocator.h (kAllocatorSpace): Define for SPARC.
(kAllocatorSize): Likewise.
(DefaultSizeClassMap): Likewise.
* asan/asan_mapping.h (kSPARC64_ShadowOffset64): Define.
(SHADOW_OFFSET): Define for SPARC.
Include asan_mapping_sparc64.h for SPARC 64-bit.
* asan/asan_mapping_sparc64.h: New file.

-- 
Eric BotcazouIndex: asan/asan_allocator.h
===
--- asan/asan_allocator.h	(revision 269546)
+++ asan/asan_allocator.h	(working copy)
@@ -132,11 +132,15 @@ const uptr kAllocatorSpace =  ~(uptr)0;
 const uptr kAllocatorSize  =  0x20ULL;  // 128G.
 typedef VeryCompactSizeClassMap SizeClassMap;
 # elif defined(__aarch64__)
-// AArch64/SANITIZER_CAN_USER_ALLOCATOR64 is only for 42-bit VMA
+// AArch64/SANITIZER_CAN_USE_ALLOCATOR64 is only for 42-bit VMA
 // so no need to different values for different VMA.
 const uptr kAllocatorSpace =  0x100ULL;
 const uptr kAllocatorSize  =  0x100ULL;  // 3T.
 typedef DefaultSizeClassMap SizeClassMap;
+# elif defined(__sparc__)
+const uptr kAllocatorSpace = ~(uptr)0;
+const uptr kAllocatorSize  =  0x200ULL;  // 2T.
+typedef DefaultSizeClassMap SizeClassMap;
 # elif SANITIZER_WINDOWS
 const uptr kAllocatorSpace = ~(uptr)0;
 const uptr kAllocatorSize  =  0x80ULL;  // 500G
Index: asan/asan_mapping.h
===
--- asan/asan_mapping.h	(revision 269546)
+++ asan/asan_mapping.h	(working copy)
@@ -99,6 +99,13 @@
 // || `[0x10, 0x11]` || LowShadow  ||
 // || `[0x00, 0x0f]` || LowMem ||
 //
+// Default Linux/SPARC64 (52-bit VMA) mapping:
+// || `[0x8, 0xf]` || HighMem||
+// || `[0x10800, 0x207ff]` || HighShadow ||
+// || `[0x00900, 0x107ff]` || ShadowGap  ||
+// || `[0x00800, 0x008ff]` || LowShadow  ||
+// || `[0x0, 0x007ff]` || LowMem ||
+//
 // Shadow mapping on FreeBSD/x86-64 with SHADOW_OFFSET == 0x4000:
 // || `[0x5000, 0x7fff]` || HighMem||
 // || `[0x4a00, 0x4fff]` || HighShadow ||
@@ -161,6 +168,7 @@ static const u64 kMIPS32_ShadowOffset32
 static const u64 kMIPS64_ShadowOffset64 = 1ULL << 37;
 static const u64 kPPC64_ShadowOffset64 = 1ULL << 41;
 static const u64 kSystemZ_ShadowOffset64 = 1ULL << 52;
+static const u64 kSPARC64_ShadowOffset64 = 1ULL << 43;  // 0x800
 static const u64 kFreeBSD_ShadowOffset32 = 1ULL << 30;  // 0x4000
 static const u64 kFreeBSD_ShadowOffset64 = 1ULL << 46;  // 0x4000
 static const u64 kNetBSD_ShadowOffset32 = 1ULL << 30;  // 0x4000
@@ -223,6 +231,8 @@ static const u64 kMyriadCacheBitMask32 =
 #   define SHADOW_OFFSET kDefaultShadowOffset64
 #  elif defined(__mips64)
 #   define SHADOW_OFFSET kMIPS64_ShadowOffset64
+#  elif defined(__sparc__)
+#   define SHADOW_OFFSET kSPARC64_ShadowOffset64
 #  elif SANITIZER_WINDOWS64
 #   define SHADOW_OFFSET __asan_shadow_memory_dynamic_address
 #  else
@@ -269,6 +279,8 @@ extern uptr kHighMemEnd, kMidMemBeg, kMi
 
 #if SANITIZER_MYRIAD2
 #include "asan_mapping_myriad.h"
+#elif defined(__sparc__) && SANITIZER_WORDSIZE == 64
+#include "asan_mapping_sparc64.h"
 #else
 #define MEM_TO_SHADOW(mem) (((mem) >> SHADOW_SCALE) + (SHADOW_OFFSET))
 
Index: asan/asan_mapping_sparc64.h
===
--- asan/asan_mapping_sparc64.h	(nonexistent)
+++ asan/asan_mapping_sparc64.h	(working copy)
@@ -0,0 +1,100 @@
+//===-- asan_mapping_sparc64.h --*- C++ -*-===//
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details
+//
+//===--===//
+//
+// This file is a part of AddressSanitizer, an address sanity checker.
+//
+// SPARC64-specific definitions for ASan memory mapping.
+//===--===//
+#ifndef ASAN_MAPPING_SPARC64_H
+#define ASAN_MAPPING_SPARC64_H
+
+// This is tailored to the 52-bit VM layout on SPARC-T4 and later.
+// The VM space is split into two 51-bit halves at both ends: the low part
+// has all the bits above the 51st cleared, while the high part has them set.
+//   0xfff8 - 0x
+//   0x - 0x0007
+
+#define VMA_BITS 5

[libsanitizer] AddressSanitizer: fix for SPARC with GCC

2019-03-13 Thread Eric Botcazou
This patch contains a fixlet for the AddressSanitizer on the SPARC with GCC, 
which would otherwise generate a problematic call to the intercepted memcpy 
routine.  It only affects the SPARC ports and has been tested on SPARC/Solaris 
and SPARC64/Linux.

It merges r355979 of the LLVM repository.  Installed on the mainline.


2019-03-13  Eric Botcazou  

PR sanitizer/80953
Merge from LLVM revision 355979
* asan/asan_globals.c (GetGlobalsForAddress): Use internal_memcpy to
copy Global objects for SPARC with GCC.

-- 
Eric BotcazouIndex: asan/asan_globals.cc
===
--- asan/asan_globals.cc	(revision 269546)
+++ asan/asan_globals.cc	(working copy)
@@ -112,7 +112,11 @@ int GetGlobalsForAddress(uptr addr, Glob
 if (flags()->report_globals >= 2)
   ReportGlobal(g, "Search");
 if (IsAddressNearGlobal(addr, g)) {
+#if defined(__GNUC__) && defined(__sparc__)
+  internal_memcpy([res], , sizeof(g));
+#else
   globals[res] = g;
+#endif
   if (reg_sites)
 reg_sites[res] = FindRegistrationSite();
   res++;


[libsanitizer] SanitizerCommon: 64-bit SPARC/Linux port

2019-03-13 Thread Eric Botcazou
This patch contains the bits required to make the common 32-bit allocator work 
on SPARC64/Linux.  It only affects the SPARC ports and has been tested on 
SPARC/Solaris and SPARC64/Linux.

It merges r355978 of the LLVM repository.  Installed on the mainline.


2019-03-13  Eric Botcazou  

PR sanitizer/80953
Merge from LLVM revision 355978
* sanitizer_common/sanitizer_allocator_primary32.h
(class SizeClassAllocator32): Assert that kSpaceSize is power of 2 if
SANITIZER_SIGN_EXTENDED_ADDRESSES is set.
(PointerIsMine): Deal with SANITIZER_SIGN_EXTENDED_ADDRESSES.
(ComputeRegionId): Likewise.
* sanitizer_common/sanitizer_linux.cc (GetMaxVirtualAddress): Return
appropriate value for SPARC 64-bit.
* sanitizer_common/sanitizer_platform.h (SANITIZER_MMAP_RANGE_SIZE):
Define for SPARC.
(SANITIZER_SIGN_EXTENDED_ADDRESSES): Define to 1 for SPARC 64-bit.

-- 
Eric BotcazouIndex: sanitizer_common/sanitizer_allocator_primary32.h
===
--- sanitizer_common/sanitizer_allocator_primary32.h	(revision 269546)
+++ sanitizer_common/sanitizer_allocator_primary32.h	(working copy)
@@ -54,6 +54,9 @@ class SizeClassAllocator32 {
   typedef typename Params::ByteMap ByteMap;
   typedef typename Params::MapUnmapCallback MapUnmapCallback;
 
+  COMPILER_CHECK(!SANITIZER_SIGN_EXTENDED_ADDRESSES ||
+ (kSpaceSize & (kSpaceSize - 1)) == 0);
+
   static const bool kRandomShuffleChunks = Params::kFlags &
   SizeClassAllocator32FlagMasks::kRandomShuffleChunks;
   static const bool kUseSeparateSizeClassForBatch = Params::kFlags &
@@ -175,6 +178,8 @@ class SizeClassAllocator32 {
 
   bool PointerIsMine(const void *p) {
 uptr mem = reinterpret_cast(p);
+if (SANITIZER_SIGN_EXTENDED_ADDRESSES)
+  mem &= (kSpaceSize - 1);
 if (mem < kSpaceBeg || mem >= kSpaceBeg + kSpaceSize)
   return false;
 return GetSizeClass(p) != 0;
@@ -267,6 +272,8 @@ class SizeClassAllocator32 {
   COMPILER_CHECK(sizeof(SizeClassInfo) % kCacheLineSize == 0);
 
   uptr ComputeRegionId(uptr mem) {
+if (SANITIZER_SIGN_EXTENDED_ADDRESSES)
+  mem &= (kSpaceSize - 1);
 const uptr res = mem >> kRegionSizeLog;
 CHECK_LT(res, kNumPossibleRegions);
 return res;
Index: sanitizer_common/sanitizer_linux.cc
===
--- sanitizer_common/sanitizer_linux.cc	(revision 269546)
+++ sanitizer_common/sanitizer_linux.cc	(working copy)
@@ -1064,6 +1064,8 @@ uptr GetMaxVirtualAddress() {
   return (1ULL << 40) - 1;  // 0x00ffUL;
 # elif defined(__s390x__)
   return (1ULL << 53) - 1;  // 0x001fUL;
+# elif defined(__sparc__)
+  return ~(uptr)0;
 # else
   return (1ULL << 47) - 1;  // 0x7fffUL;
 # endif
Index: sanitizer_common/sanitizer_platform.h
===
--- sanitizer_common/sanitizer_platform.h	(revision 269546)
+++ sanitizer_common/sanitizer_platform.h	(working copy)
@@ -239,10 +239,21 @@
 # else
 #  define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 48)
 # endif
+#elif defined(__sparc__)
+# define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 52)
 #else
 # define SANITIZER_MMAP_RANGE_SIZE FIRST_32_SECOND_64(1ULL << 32, 1ULL << 47)
 #endif
 
+// Whether the addresses are sign-extended from the VMA range to the word.
+// The SPARC64 Linux port implements this to split the VMA space into two
+// non-contiguous halves with a huge hole in the middle.
+#if defined(__sparc__) && SANITIZER_WORDSIZE == 64
+# define SANITIZER_SIGN_EXTENDED_ADDRESSES 1
+#else
+# define SANITIZER_SIGN_EXTENDED_ADDRESSES 0
+#endif
+
 // The AArch64 linux port uses the canonical syscall set as mandated by
 // the upstream linux community for all new ports. Other ports may still
 // use legacy syscalls.


[libsanitizer] SanitizerCommon: fixes for unwinding & backtrace on SPARC

2019-03-13 Thread Eric Botcazou
This patch contains various fixes for the unwinding and backtrace machinery 
on the SPARC, which doesn't work correctly in some cases.  It only affects the 
SPARC ports and has been tested on SPARC/Solaris and SPARC64/Linux.

It merges r355965 of the LLVM repository.  Installed on the mainline.


2019-03-13  Eric Botcazou  

PR sanitizer/80953
Merge from LLVM revision 355965
* sanitizer_common/sanitizer_linux.cc (GetWriteFlag): Implement for
SPARC/Linux.
(GetPcSpBp): Likewise.
* sanitizer_common/sanitizer_stacktrace.cc (GetNextInstructionPc):
Adjust for SPARC.
* sanitizer_common/sanitizer_stacktrace.h (SANITIZER_CAN_FAST_UNWIND):
Define to 1 for SPARC.
* sanitizer_common/sanitizer_stacktrace_sparc.cc: Rewrite.
* sanitizer_common/sanitizer_unwind_linux_libcdep.cc (SlowUnwindStack):
Adjust the PC address for SPARC with GCC.

-- 
Eric BotcazouIndex: sanitizer_common/sanitizer_linux.cc
===
--- sanitizer_common/sanitizer_linux.cc	(revision 269546)
+++ sanitizer_common/sanitizer_linux.cc	(working copy)
@@ -1848,10 +1850,20 @@ SignalContext::WriteFlag SignalContext::
   u64 esr;
   if (!Aarch64GetESR(ucontext, )) return UNKNOWN;
   return esr & ESR_ELx_WNR ? WRITE : READ;
-#elif SANITIZER_SOLARIS && defined(__sparc__)
+#elif defined(__sparc__)
   // Decode the instruction to determine the access type.
   // From OpenSolaris $SRC/uts/sun4/os/trap.c (get_accesstype).
+# if SANITIZER_SOLARIS
   uptr pc = ucontext->uc_mcontext.gregs[REG_PC];
+# else
+  // Historical BSDism here.
+  struct sigcontext *scontext = (struct sigcontext *)context;
+#  if defined(__arch64__)
+  uptr pc = scontext->sigc_regs.tpc;
+#  else
+  uptr pc = scontext->si_regs.pc;
+#  endif
+# endif
   u32 instr = *(u32 *)pc;
   return (instr >> 21) & 1 ? WRITE: READ;
 #else
@@ -1942,28 +1954,27 @@ static void GetPcSpBp(void *context, upt
   // pointer, but GCC always uses r31 when we need a frame pointer.
   *bp = ucontext->uc_mcontext.regs->gpr[PT_R31];
 #elif defined(__sparc__)
-  ucontext_t *ucontext = (ucontext_t*)context;
-  uptr *stk_ptr;
-# if defined(__sparcv9) || defined (__arch64__)
-# ifndef MC_PC
-#  define MC_PC REG_PC
-# endif
-# ifndef MC_O6
-#  define MC_O6 REG_O6
+# if defined(__arch64__) || defined(__sparcv9)
+#  define STACK_BIAS 2047
+# else
+#  define STACK_BIAS 0
 # endif
 # if SANITIZER_SOLARIS
-#  define mc_gregs gregs
-# endif
-  *pc = ucontext->uc_mcontext.mc_gregs[MC_PC];
-  *sp = ucontext->uc_mcontext.mc_gregs[MC_O6];
-  stk_ptr = (uptr *) (*sp + 2047);
-  *bp = stk_ptr[15];
-# else
+  ucontext_t *ucontext = (ucontext_t*)context;
   *pc = ucontext->uc_mcontext.gregs[REG_PC];
-  *sp = ucontext->uc_mcontext.gregs[REG_O6];
-  stk_ptr = (uptr *) *sp;
-  *bp = stk_ptr[15];
+  *sp = ucontext->uc_mcontext.gregs[REG_O6] + STACK_BIAS;
+# else
+  // Historical BSDism here.
+  struct sigcontext *scontext = (struct sigcontext *)context;
+#  if defined(__arch64__)
+  *pc = scontext->sigc_regs.tpc;
+  *sp = scontext->sigc_regs.u_regs[14] + STACK_BIAS;
+#  else
+  *pc = scontext->si_regs.pc;
+  *sp = scontext->si_regs.u_regs[14];
+#  endif
 # endif
+  *bp = (uptr) ((uhwptr *) *sp)[14] + STACK_BIAS;
 #elif defined(__mips__)
   ucontext_t *ucontext = (ucontext_t*)context;
   *pc = ucontext->uc_mcontext.pc;
Index: sanitizer_common/sanitizer_stacktrace.cc
===
--- sanitizer_common/sanitizer_stacktrace.cc	(revision 269546)
+++ sanitizer_common/sanitizer_stacktrace.cc	(working copy)
@@ -16,10 +16,9 @@
 namespace __sanitizer {
 
 uptr StackTrace::GetNextInstructionPc(uptr pc) {
-#if defined(__mips__)
+#if defined(__sparc__) || defined(__mips__)
   return pc + 8;
-#elif defined(__powerpc__) || defined(__sparc__) || defined(__arm__) || \
-defined(__aarch64__)
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
   return pc + 4;
 #else
   return pc + 1;
Index: sanitizer_common/sanitizer_stacktrace.h
===
--- sanitizer_common/sanitizer_stacktrace.h	(revision 269546)
+++ sanitizer_common/sanitizer_stacktrace.h	(working copy)
@@ -17,7 +17,7 @@ namespace __sanitizer {
 
 static const u32 kStackTraceMax = 256;
 
-#if defined(__sparc__) || (SANITIZER_LINUX && defined(__mips__))
+#if SANITIZER_LINUX && defined(__mips__)
 # define SANITIZER_CAN_FAST_UNWIND 0
 #elif SANITIZER_WINDOWS
 # define SANITIZER_CAN_FAST_UNWIND 0
Index: sanitizer_common/sanitizer_stacktrace_sparc.cc
===
--- sanitizer_common/sanitizer_stacktrace_sparc.cc	(revision 269546)
+++ sanitizer_common/sanitizer_stacktrace_sparc.cc	(working copy)
@@ -11,9 +11,13 @@
 // Implemention of fast stack unwinding for Sparc.
 //===--

[patch] Fix ASAN failures on SPARC64/Linux

2019-03-11 Thread Eric Botcazou
Hi,

ASAN was enabled for the SPARC architecture during GCC 9 development but it 
doesn't really work on SPARC64/Linux because of the specific layout of the 
virtual memory address space.  Fortunately this is (easily) fixable and the 
fix has been accepted upstream, along with other fixes for SPARC (I have 
attached the asan/asan_mapping_sparc64.h file accepted upstream).

But, since GCC also hardcodes the scaling done by ASAN, this also requires a 
small adjustment to the compiler proper by means of a hook, tentatively called 
TARGET_ASAN_SHADOW_LEFT_SHIFT, which is defined to NULL except for SPARC.  It
yields a 100% clean ASAN testsuite on SPARC64/Linux (32-bit and 64-bit).

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


2019-03-11  Eric Botcazou  

PR sanitizer/80953
* target.def (asan_shadow_left_shift): New hook.
(asan_shadow_offset): Minor tweak.
* doc/tm.texi.in: Add TARGET_ASAN_SHADOW_LEFT_SHIFT.
* doc/tm.texi: Regenerate.
* asan.c (asan_emit_stack_protection): Do a preliminary left shift if
TARGET_ASAN_SHADOW_LEFT_SHIFT is positive.
(build_shadow_mem_access): Likewise.
* config/sparc/sparc.c (TARGET_ASAN_SHADOW_LEFT_SHIFT): Define to...
(sparc_asan_shadow_left_shift): ...this.  New function.

-- 
Eric BotcazouIndex: asan.c
===
--- asan.c	(revision 269546)
+++ asan.c	(working copy)
@@ -1380,6 +1380,7 @@ asan_emit_stack_protection (rtx base, rt
   unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT;
   tree str_cst, decl, id;
   int use_after_return_class = -1;
+  unsigned int shift;
 
   if (shadow_ptr_types[0] == NULL_TREE)
 asan_init_shadow_ptr_types ();
@@ -1524,8 +1525,19 @@ asan_emit_stack_protection (rtx base, rt
   TREE_ASM_WRITTEN (decl) = 1;
   TREE_ASM_WRITTEN (id) = 1;
   emit_move_insn (mem, expand_normal (build_fold_addr_expr (decl)));
-  shadow_base = expand_binop (Pmode, lshr_optab, base,
-			  gen_int_shift_amount (Pmode, ASAN_SHADOW_SHIFT),
+  shadow_base = base;
+  if (targetm.asan_shadow_left_shift
+  && (shift = targetm.asan_shadow_left_shift ()) > 0)
+{
+  shadow_base = expand_binop (Pmode, ashl_optab, shadow_base,
+  gen_int_shift_amount (Pmode, shift),
+  NULL_RTX, 1, OPTAB_DIRECT);
+  shift += ASAN_SHADOW_SHIFT;
+}
+  else
+shift = ASAN_SHADOW_SHIFT;
+  shadow_base = expand_binop (Pmode, lshr_optab, shadow_base,
+			  gen_int_shift_amount (Pmode, shift),
 			  NULL_RTX, 1, OPTAB_DIRECT);
   shadow_base
 = plus_constant (Pmode, shadow_base,
@@ -2023,9 +2035,24 @@ build_shadow_mem_access (gimple_stmt_ite
 {
   tree t, uintptr_type = TREE_TYPE (base_addr);
   tree shadow_type = TREE_TYPE (shadow_ptr_type);
+  unsigned int shift;
   gimple *g;
 
-  t = build_int_cst (uintptr_type, ASAN_SHADOW_SHIFT);
+  if (targetm.asan_shadow_left_shift
+  && (shift = targetm.asan_shadow_left_shift ()) > 0)
+{
+  t = build_int_cst (uintptr_type, shift);
+  g = gimple_build_assign (make_ssa_name (uintptr_type), LSHIFT_EXPR,
+			   base_addr, t);
+  gimple_set_location (g, location);
+  gsi_insert_after (gsi, g, GSI_NEW_STMT);
+  base_addr = gimple_assign_lhs (g);
+  shift += ASAN_SHADOW_SHIFT;
+}
+  else
+shift = ASAN_SHADOW_SHIFT;
+
+  t = build_int_cst (uintptr_type, shift);
   g = gimple_build_assign (make_ssa_name (uintptr_type), RSHIFT_EXPR,
 			   base_addr, t);
   gimple_set_location (g, location);
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 269546)
+++ config/sparc/sparc.c	(working copy)
@@ -674,6 +674,7 @@ static rtx sparc_struct_value_rtx (tree,
 static rtx sparc_function_value (const_tree, const_tree, bool);
 static rtx sparc_libcall_value (machine_mode, const_rtx);
 static bool sparc_function_value_regno_p (const unsigned int);
+static unsigned int sparc_asan_shadow_left_shift (void);
 static unsigned HOST_WIDE_INT sparc_asan_shadow_offset (void);
 static void sparc_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
 static void sparc_file_end (void);
@@ -835,6 +836,9 @@ char sparc_hard_reg_printed[8];
 #undef TARGET_EXPAND_BUILTIN_SAVEREGS
 #define TARGET_EXPAND_BUILTIN_SAVEREGS sparc_builtin_saveregs
 
+#undef TARGET_ASAN_SHADOW_LEFT_SHIFT
+#define TARGET_ASAN_SHADOW_LEFT_SHIFT sparc_asan_shadow_left_shift
+
 #undef TARGET_ASAN_SHADOW_OFFSET
 #define TARGET_ASAN_SHADOW_OFFSET sparc_asan_shadow_offset
 
@@ -12493,7 +12497,16 @@ sparc_init_machine_status (void)
 {
   return ggc_cleared_alloc ();
 }
-
+
+/* Implement the TARGET_ASAN_SHADOW_LEFT_SHIFT hook.  */
+
+static unsigned int
+sparc_asan_shadow_left_shift (void)
+{
+  /* This is tailored to the 52-bit VM layout on SPARC-T4 and later.  */
+  return TARGET_ARCH64 ? 12 : 0;
+}
+
 /* Implement the TARGET_ASAN_SHADOW_OFFSET hook.  */
 
 static 

Re: [patch] Fix PR rtl-optimization/89588

2019-03-11 Thread Eric Botcazou
> Hmm, this looks fragile - isn't the same effect when using
> -fdisable-tree-cunroll?

Maybe.

> That is, it looks like we could "move" the assert to decide_unrolling
> instead, deciding LPT_NONE?

We already have a guard for the assertion but it is bypassed here.

I'm going to commit this (equivalent to Jakub's fixlet) after retesting.


PR rtl-optimization/89588
* loop-unroll.c (decide_unroll_constant_iterations): Make guard for
explicit unrolling factor more robust.

-- 
Eric BotcazouIndex: loop-unroll.c
===
--- loop-unroll.c	(revision 269546)
+++ loop-unroll.c	(working copy)
@@ -400,7 +400,7 @@ decide_unroll_constant_iterations (struc
 {
   /* However we cannot unroll completely at the RTL level a loop with
 	 constant number of iterations; it should have been peeled instead.  */
-  if ((unsigned) loop->unroll - 1 > desc->niter - 2)
+  if (desc->niter < 2 || (unsigned) loop->unroll - 1 > desc->niter - 2)
 	{
 	  if (dump_file)
 	fprintf (dump_file, ";; Loop should have been peeled\n");


[patch] Fix PR rtl-optimization/89588

2019-03-11 Thread Eric Botcazou
Hi,

this is the failure of the assertion:

  /* Should not get here (such loop should be peeled instead).  */
  gcc_assert (niter > max_unroll + 1);

in unroll_loop_constant_iterations on a testcase both containing #pragma GCC 
unroll and compiled with -fno-tree-loop-optimize.  The proposed fix is just to 
disable the pragma altogether when the option is passed.

Tested on x86_64-suse-linux, OK for mainline and 8 branch?


2019-03-11  Eric Botcazou  

PR rtl-optimization/89588
* tree-cfg.c (replace_loop_annotate_in_block) :
Skip annotation and warn if -fno-tree-loop-optimize is specified.


2019-03-11  Eric Botcazou  

* c-c++-common/unroll-6.c: New test.

-- 
Eric BotcazouIndex: tree-cfg.c
===
--- tree-cfg.c	(revision 269546)
+++ tree-cfg.c	(working copy)
@@ -280,9 +280,16 @@ replace_loop_annotate_in_block (basic_bl
 	  loop->safelen = INT_MAX;
 	  break;
 	case annot_expr_unroll_kind:
-	  loop->unroll
-	= (unsigned short) tree_to_shwi (gimple_call_arg (stmt, 2));
-	  cfun->has_unroll = true;
+	  if (flag_tree_loop_optimize)
+	{
+	  loop->unroll
+		= (unsigned short) tree_to_shwi (gimple_call_arg (stmt, 2));
+	  cfun->has_unroll = true;
+	}
+	  else
+	warning_at (gimple_location (stmt), 0,
+			"pragma GCC unroll disabled by "
+			"-fno-tree-loop-optimize");
 	  break;
 	case annot_expr_no_vector_kind:
 	  loop->dont_vectorize = true;
/* { dg-do compile } */
/* { dg-options "-O -fno-tree-loop-optimize" } */

void test (void)
{
  #pragma GCC unroll 2
  for (int nv = 0; nv <= 2; nv += 2) /* { dg-warning "GCC unroll disabled" } */
{}
}


Re: [patch] Fix wrong code for boolean negation in condition at -O

2019-03-01 Thread Eric Botcazou
> The BIT_AND_EXPR case is clearly correct for all possible values.  The code
> says that if the result of BIT_AND_EXPR is known to be a non-zero constant,
> and one or both of the BIT_AND_EXPR arguments have known value ranges [0,1]
> (or boolean or precision 1, not talking about those now), then that value
> with the [0,1] range actually had to be 1, otherwise BIT_AND_EXPR result
> would be 0.
> 
> The BIT_NOT_EXPR case is different, ~0 is -1 and ~1 is -2.  If an SSA_NAME
> has [0,1] range, then BIT_NOT_EXPR of that will be [-2,-1].  If value is one
> of those two, then with your today's patch the assumption will be correct,
> 1 or 0.  If value is some other one, which should hopefully happen only in
> dead code that we haven't been able to optimize, then we'd replace
> different values though.

I don't understand the last sentence: we'll precisely replace a bogus value, 
which we know is impossible given the [0, 1] range, by 0 or 1.

-- 
Eric Botcazou


Re: [patch] Fix wrong code for boolean negation in condition at -O

2019-02-28 Thread Eric Botcazou
> I know Eric has committed a tweak here, but I view this magic handling as
> something meant for boolean types only (if it is correct at all and the
> right fix wouldn't be avoid the BIT_NOT_EXPR for the prec > 1 booleans, I
> believe the expansion of BIT_NOT_EXPR doesn't have any special case for
> BOOLEAN_TYPE).  This patch just restores previous behavior for non-boolean
> types (basically inlines the first two cases from ssa_name_has_boolean_range
> while leaving the problematic third one out, normal integral types with
> just known value range of [0,1]).

IMO you haven't justified why this is problematic in the BIT_NOT_EXPR case and 
not in the BIT_AND_EXPR case...

-- 
Eric Botcazou



Re: [patch] Fix wrong code for boolean negation in condition at -O

2019-02-28 Thread Eric Botcazou
> > Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for
> > boolean types, I think that the same special treatment must be added for
> > boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value
> > invariant.
> > 
> > Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch?
> 
> OK.

Thanks.  However, as reported under PR tree-opt/89536, there is an annoying 
oversight in the reasoning: the predicate to be used is not integer_zerop but 
whether bit #0 is 0 or 1.  I have applied the attached fixlet as obviously 
more correct than the current code, but Jakub has a different opinion on the 
whole change so this will probably be revisited in the near future.


PR tree-optimization/89536
* tree-ssa-dom.c (edge_info::derive_equivalences) : Test
only whether bit #0 of the value is 0 instead of the entire value.


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

-- 
Eric Botcazou/* PR tree-optimization/89536 */
/* Testcase by Zhendong Su  */

int a = 1;

int main (void)
{
  a = ~(a && 1); 
  if (a < -1)
a = ~a;
  
  if (!a)
__builtin_abort ();

  return 0;
}
Index: tree-ssa-dom.c
===
--- tree-ssa-dom.c	(revision 269211)
+++ tree-ssa-dom.c	(working copy)
@@ -348,7 +348,7 @@ edge_info::derive_equivalences (tree nam
 		&& TREE_CODE (rhs) == SSA_NAME
 		&& ssa_name_has_boolean_range (rhs))
 	  {
-		if (integer_zerop (value))
+		if ((TREE_INT_CST_LOW (value) & 1) == 0)
 		  res = build_one_cst (TREE_TYPE (rhs));
 		else
 		  res = build_zero_cst (TREE_TYPE (rhs));


Re: [PATCH] haifa-sched: handle fallthru edge to EXIT block (PR 85899)

2019-02-28 Thread Eric Botcazou
> So it looks to me that the assert has to allow this.  I've bootstrapped the
> following (not that it matters much as it simply relaxes the assert) and
> verified it fixes the testcase.

Yes, fallthrough edges to the exit block exist in RTL, see could_fall_through.

> OK for trunk?
> 
>   * haifa-sched.c (find_fallthru_edge_from): Relax assert to account for
>   fallthru edges leading to the exit block.
> 
>   * gcc.dg/pr85899.c: New test.

OK, thanks.

-- 
Eric Botcazou


[SPARC] Make --help=target -Q output the code model

2019-02-26 Thread Eric Botcazou
--help=target -Q currently outputs the memory model but not the code model, 
because the former is entirely handled by the option machinery while the 
latter is partially handled manually; this patch fixes this discrepancy.

Tested on SPARC/Solaris and SPARC64/Linux, applied on the mainline.


2019-02-26  Eric Botcazou  

* config/sparc/sparc-opts.h (enum processor_type): Rename to...
(enum sparc_processor_type): ...this.
(enum sparc_code_model_type): New enumeration type.
(enum sparc_memory_model_type): Tweak comments.
* config/sparc/sparc.opt (mcpu): Adjust to above renaming.
(mtune): Likewise.
(mcmodel): Use sparc_code_model enumeration and variable.
(sparc_code_model): New enumeration.
(mdebug): Add Undocumented marker.
* config/sparc/sparc.h (enum cmodel): Delete.
(sparc_cmodel): Likewise.
(TARGET_CM_MEDLOW): Adjust to above renaming.
(TARGET_CM_MEDMID): Likewise.
(TARGET_CM_MEDANY): Likewise.
(TARGET_CM_EMBMEDANY): Likewise.
* config/sparc/sparc.c (sparc_cmodel): Delete.
(sparc_option_override): Remove string/value mapping support for the
code model.  Move code and memory model support to after the handling
of target flags.  Do private machine setup last.
(sparc_emit_set_symbolic_const64): Use sparc_code_model.
(sparc_legitimize_reload_address): Likewise.
(sparc_output_mi_thunk): Likewise.
* config/sparc/sparc.md (cpu): Adjust comment to above renaming.

-- 
Eric BotcazouIndex: config/sparc/sparc-opts.h
===
--- config/sparc/sparc-opts.h	(revision 269211)
+++ config/sparc/sparc-opts.h	(working copy)
@@ -20,10 +20,10 @@ along with GCC; see the file COPYING3.
 #ifndef SPARC_OPTS_H
 #define SPARC_OPTS_H
 
-/* Processor type.
+/* SPARC processor type.
These must match the values for the cpu attribute in sparc.md and
the table in sparc_option_override.  */
-enum processor_type {
+enum sparc_processor_type {
   PROCESSOR_V7,
   PROCESSOR_CYPRESS,
   PROCESSOR_V8,
@@ -50,10 +50,19 @@ enum processor_type {
   PROCESSOR_NATIVE
 };
 
-/* Sparc system memory model.  See Appendix D in the Sparc V9 manual
-   for formal specification, and Appendix J for more discussion.  */
+/* SPARC-V9 code model type.  See sparc.h for the full description.  */
+enum sparc_code_model_type {
+  CM_32,	/* 32-bit address space.  */
+  CM_MEDLOW,	/* 32-bit address space.  */
+  CM_MEDMID,	/* 44-bit address space.  */
+  CM_MEDANY,	/* 64-bit address space.  */
+  CM_EMBMEDANY	/* 64-bit address space.  */
+};
+
+/* SPARC memory model type.  See Appendix D in the SPARC-V9 manual
+   for formal specification and Appendix J for more discussion.  */
 enum sparc_memory_model_type {
-  SMM_DEFAULT,	/* Uninitialized.  */
+  SMM_DEFAULT,	/* Processor default.  */
   SMM_RMO,	/* Relaxed Memory Order.  */
   SMM_PSO,	/* Partial Store Order.  */
   SMM_TSO,	/* Total Store Order.  */
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 269211)
+++ config/sparc/sparc.c	(working copy)
@@ -724,11 +724,6 @@ static const struct attribute_spec sparc
 };
 #endif
 
-/* Option handling.  */
-
-/* Parsed value.  */
-enum cmodel sparc_cmodel;
-
 char sparc_hard_reg_printed[8];
 
 /* Initialize the GCC target structure.  */
@@ -1636,22 +1631,10 @@ dump_target_flags (const char *prefix, c
 static void
 sparc_option_override (void)
 {
-  static struct code_model {
-const char *const name;
-const enum cmodel value;
-  } const cmodels[] = {
-{ "32", CM_32 },
-{ "medlow", CM_MEDLOW },
-{ "medmid", CM_MEDMID },
-{ "medany", CM_MEDANY },
-{ "embmedany", CM_EMBMEDANY },
-{ NULL, (enum cmodel) 0 }
-  };
-  const struct code_model *cmodel;
   /* Map TARGET_CPU_DEFAULT to value for -m{cpu,tune}=.  */
   static struct cpu_default {
 const int cpu;
-const enum processor_type processor;
+const enum sparc_processor_type processor;
   } const cpu_default[] = {
 /* There must be one entry here for each TARGET_CPU value.  */
 { TARGET_CPU_sparc, PROCESSOR_CYPRESS },
@@ -1795,30 +1778,6 @@ sparc_option_override (void)
   target_flags |= MASK_LONG_DOUBLE_128;
 }
 
-  /* Code model selection.  */
-  sparc_cmodel = SPARC_DEFAULT_CMODEL;
-
-#ifdef SPARC_BI_ARCH
-  if (TARGET_ARCH32)
-sparc_cmodel = CM_32;
-#endif
-
-  if (sparc_cmodel_string != NULL)
-{
-  if (TARGET_ARCH64)
-	{
-	  for (cmodel = [0]; cmodel->name; cmodel++)
-	if (strcmp (sparc_cmodel_string, cmodel->name) == 0)
-	  break;
-	  if (cmodel->name == NULL)
-	error ("bad value (%s) for -mcmodel= switch", sparc_cmodel_string);
-	  else
-	sparc_cmodel = cmodel->value;
-	}
-  else
-	error ("-mcmodel= is not supported on 

Re: [patch] Fix LRA/reload issue with -fnon-call-exceptions

2019-02-26 Thread Eric Botcazou
>   * rtlanal.c (get_initial_register_offset): Fall back to the raw estimate
>   as long as the epilogue isn't completed.

I have backported it onto the 8 branch, where it fixes the failure (timeout) 
of gnat.dg/opt73.adb for PowerPC/Linux, after testing it on this platform.

-- 
Eric Botcazou


[patch] Fix wrong code for boolean negation in condition at -O

2019-02-25 Thread Eric Botcazou
Hi,

this is a regression present on the mainline and 8 branch, introduced by the 
new code in edge_info::derive_equivalences dealing with BIT_AND_EXPR for SSA 
names with boolean range:

  /* If either operand has a boolean range, then we
 know its value must be one, otherwise we just know it
 is nonzero.  The former is clearly useful, I haven't
 seen cases where the latter is helpful yet.  */
  if (TREE_CODE (rhs1) == SSA_NAME)
{
  if (ssa_name_has_boolean_range (rhs1))
{
  value = build_one_cst (TREE_TYPE (rhs1));
  derive_equivalences (rhs1, value, recursion_limit - 1);
}
}
  if (TREE_CODE (rhs2) == SSA_NAME)
{
  if (ssa_name_has_boolean_range (rhs2))
{
  value = build_one_cst (TREE_TYPE (rhs2));
  derive_equivalences (rhs2, value, recursion_limit - 1);
}
}

and visible on the attached Ada testcase at -O1 or above.

The sequence of events is as follows: for boolean types with precision > 1 
(the normal boolean types in Ada), the gimplifier turns a TRUTH_NOT_EXPR into 
a BIT_XOR_EXPR with 1 in order to preserve the 0-or-1-value invariant:

/* The parsers are careful to generate TRUTH_NOT_EXPR
   only with operands that are always zero or one.
   We do not fold here but handle the only interesting case
   manually, as fold may re-introduce the TRUTH_NOT_EXPR.  */
*expr_p = gimple_boolify (*expr_p);
if (TYPE_PRECISION (TREE_TYPE (*expr_p)) == 1)
  *expr_p = build1_loc (input_location, BIT_NOT_EXPR,
TREE_TYPE (*expr_p),
TREE_OPERAND (*expr_p, 0));
else
  *expr_p = build2_loc (input_location, BIT_XOR_EXPR,
TREE_TYPE (*expr_p),
TREE_OPERAND (*expr_p, 0),
build_int_cst (TREE_TYPE (*expr_p), 1));

Now this TRUTH_NOT_EXPR is part of a conjunction which has been turned into a 
BIT_AND_EXPR by the folder, so this gives BIT_AND_EXPR >.

After some optimization passes, the second operand of the BIT_AND_EXPR is also 
folded into 1 and, consequently, the following match.pd pattern kicks in:

/* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y.  */
(for opo (bit_and bit_xor)
 opi (bit_xor bit_and)
 (simplify
  (opo:c (opi:c @0 @1) @1) 
  (bit_and (bit_not @0) @1)))

and yields BIT_AND_EXPR .  This is still correct, in the 
sense that the 0-or-1-value invariant is preserved.

Then the new code in edge_info::derive_equivalences above deduces from this 
that the BIT_NOT_EXPR has value 1 on one of the edges.  But the same function 
also handles the BIT_NOT_EXPR itself and further deduces that its operand has 
value ~1 or 254 (the precision of boolean types is 8) on this edge, which 
breaks the 0-or-1-value invariant and leads to wrong code downstream.

Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for 
boolean types, I think that the same special treatment must be added for 
boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value invariant.

Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch?


2019-02-25  Eric Botcazou  

* tree-ssa-dom.c (edge_info::derive_equivalences) : Fix
and move around comment.
: Likewise.
: Add specific handling for boolean types.


2019-02-25  Eric Botcazou  

* gnat.dg/opt77.adb: New test.
* gnat.dg/opt77_pkg.ad[sb]: Likewise.

-- 
Eric BotcazouIndex: tree-ssa-dom.c
===
--- tree-ssa-dom.c	(revision 268994)
+++ tree-ssa-dom.c	(working copy)
@@ -170,11 +170,10 @@ edge_info::derive_equivalences (tree nam
   gimple *def_stmt = SSA_NAME_DEF_STMT (name);
   if (is_gimple_assign (def_stmt))
 {
-  /* We know the result of DEF_STMT was zero.  See if that allows
-	 us to deduce anything about the SSA_NAMEs used on the RHS.  */
   enum tree_code code = gimple_assign_rhs_code (def_stmt);
   switch (code)
 	{
+	/* If the result of an OR is zero, then its operands are, too.  */
 	case BIT_IOR_EXPR:
 	  if (integer_zerop (value))
 	{
@@ -188,8 +187,7 @@ edge_info::derive_equivalences (tree nam
 	}
 	  break;
 
-  /* We know the result of DEF_STMT was one.  See if that allows
-	 us to deduce anything about the SSA_NAMEs used on the RHS.  */
+	/* If the result of an AND is nonzero, then its operands are, too.  */
 	case BIT_AND_EXPR:
 	  if (!integer_zerop (value))
 	{
@@ -296,7 +294,6 @@ edge_info::derive_equivalences (tree nam
 	break;
 	  }
 
-
 	case EQ_EX

Re: [PATCH] PR libstdc++/89446 fix null pointer dereference in char_traits

2019-02-23 Thread Eric Botcazou
> I forgot to say that this is a conservative list of targets where
> -fnon-call-exceptions is supported. Maybe it could run on other
> targets, but this should be enough to ensure we don't get a regression
> for this bug.

What do you mean by supported exactly?  Ada and Go set it by default and they 
are supported on a bunch of other targets...

-- 
Eric Botcazou


Re: [patch] Add baseline for SPARC64/Linux

2019-02-22 Thread Eric Botcazou
> Sorry for the delay, I'm in a meeting all week with limited time for
> patch review.

No problem, thanks for reviewing!

> We don't usually include the __once_callable symbols in the baseline
> file, are you sure it's correct to do so for sparc64-linux-gnu?
> 
> +TLS:8:_ZSt11__once_call@@GLIBCXX_3.4.11
> +TLS:8:_ZSt15__once_callable@@GLIBCXX_3.4.11

Now removed for the sake of consistency with the SPARC/Linux twin port.

-- 
Eric Botcazou


Re: [patch] Add baseline for SPARC64/Linux

2019-02-21 Thread Eric Botcazou
> 2019-02-20  Eric Botcazou  
> 
> * configure.host (abi_baseline_pair): Adjust for SPARC64/Linux.
> * config/abi/post/sparc64-linux-gnu: New directory.
> * config/abi/post/sparc64-linux-gnu/baseline_symbols.txt: New file.
> * config/abi/post/sparc64-linux-gnu/32: New directory.
>   * config/abi/post/sparc64-linux-gnu/32/baseline_symbols.txt: New file.

I can probably self-approve it as SPARC maintainer so now installed.

-- 
Eric Botcazou


[patch] Add baseline for SPARC64/Linux

2019-02-20 Thread Eric Botcazou
Tested on SPARC64/Linux, OK for the mainline?


2019-02-20  Eric Botcazou  

* configure.host (abi_baseline_pair): Adjust for SPARC64/Linux.
* config/abi/post/sparc64-linux-gnu: New directory.
* config/abi/post/sparc64-linux-gnu/baseline_symbols.txt: New file.
* config/abi/post/sparc64-linux-gnu/32: New directory.
* config/abi/post/sparc64-linux-gnu/32/baseline_symbols.txt: New file.

-- 
Eric Botcazou


sparc64_libstdc++-v3.diff.xz
Description: application/xz


Re: [patch] Fix LRA/reload issue with -fnon-call-exceptions

2019-02-19 Thread Eric Botcazou
> Doesn't the frame have to be laid out correctly during the final reload
> subpass anyway, in order to get the right elimination choices and
> elimination offsets?  If so, I'm not sure what the " &&
> info->reload_completed" check in rs6000_stack_info achieves.  It seems like
> that's at least partly responsible for the problem.

Clearly a comment would be in order.  I'm not sure it's responsible for the 
problem, unless you mean that it's papering over the underlying issue, in 
which case you're probably right.  This underlying issue is probably that:

  /* Calculate which registers need to be saved & save area size.  */
  info->first_gp_reg_save = first_reg_to_save ();

overlooks the specific status of the frame pointer, i.e. it doesn't test 
frame_pointer_needed explicitly, which I think is pretty much mandatory.
The rest of the code apparently works correctly despite this though.

> I realise you've already applied it and that you saw it as a hack too,
> but this seems like a bit too much.  IMO a cleaner fix would be to define
> TARGET_COMPUTE_FRAME_LAYOUT for rs6000.

Well, it's a hack on top of a big kludge (the get_initial_register_offset 
stuff in rtlanal.c) so it can be viewed as a safety net too. :-)

> I guess that approach means that TARGET_COMPUTE_FRAME_LAYOUT isn't really
> optional though.

IMO another workaround for the underlying issue.

-- 
Eric Botcazou


Re: [patch] Fix LRA/reload issue with -fnon-call-exceptions

2019-02-19 Thread Eric Botcazou
> So, barring the removal of the get_initial_register_offset stuff, the only
> simple fix is probably to prevent it from calling into the back-end too
> early, for example with the attached fixlet.  Tested on x86-64 and
> PowerPC/Linux.
> 
> Thoughts?  Where do we want to fix this?
> 
> 
>   * rtlanal.c (get_initial_register_offset): Fall back to the raw estimate
>   as long as the epilogue isn't completed.

I have installed it on mainline only for now.

-- 
Eric Botcazou


[libgo] Fix alignment issue in persistent allocator

2019-02-16 Thread Eric Botcazou
This gets rid of a bunch of Go failures on SPARC.

Tested on x86-64/Linux, SPARC/Solaris and SPARC64/Linux.


2019-02-16  Eric Botcazou  

* go/runtime/malloc.go (persistentalloc1): Always align the offset.

-- 
Eric BotcazouIndex: go/runtime/malloc.go
===
--- go/runtime/malloc.go	(revision 268849)
+++ go/runtime/malloc.go	(working copy)
@@ -1269,7 +1269,7 @@ func persistentalloc1(size, align uintpt
 break
 			}
 		}
-		persistent.off = sys.PtrSize
+		persistent.off = round(sys.PtrSize, align)
 	}
 	p := persistent.base.add(persistent.off)
 	persistent.off += size


Re: [testsuite] Tweak gcc.target/sparc/struct-ret-check-1.c

2019-02-16 Thread Eric Botcazou
> It cannot pass in PIE mode.

Likewise for the 3 c-c++-common/patchable_function_entry-*.c on SPARC.

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


2019-02-16  Eric Botcazou  

* c-c++-common/patchable_function_entry-decl.c: Add -fno-pie on SPARC.
* c-c++-common/patchable_function_entry-default.c: Likewise.
* c-c++-common/patchable_function_entry-definition.c: Likewise.

-- 
Eric BotcazouIndex: c-c++-common/patchable_function_entry-decl.c
===
--- c-c++-common/patchable_function_entry-decl.c	(revision 268932)
+++ c-c++-common/patchable_function_entry-decl.c	(working copy)
@@ -1,5 +1,6 @@
 /* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
 /* { dg-final { scan-assembler-times "nop|NOP" 2 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 2 { target alpha*-*-* } } } */
 
Index: c-c++-common/patchable_function_entry-default.c
===
--- c-c++-common/patchable_function_entry-default.c	(revision 268932)
+++ c-c++-common/patchable_function_entry-default.c	(working copy)
@@ -1,5 +1,6 @@
 /* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
 /* { dg-final { scan-assembler-times "nop|NOP" 3 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 3 { target alpha*-*-* } } } */
 
Index: c-c++-common/patchable_function_entry-definition.c
===
--- c-c++-common/patchable_function_entry-definition.c	(revision 268932)
+++ c-c++-common/patchable_function_entry-definition.c	(working copy)
@@ -1,5 +1,6 @@
 /* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
 /* { dg-final { scan-assembler-times "nop|NOP" 1 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 1 { target alpha*-*-* } } } */
 


[testsuite] Couple of g++.dg/asan tweaks

2019-02-15 Thread Eric Botcazou
One of the tests in g++.dg/asan/asan_oob_test.cc uses unaligned memory 
accesses and g++.dg/asan/function-argument-3.C assumes a specific kind of 
calling conventions for vectors.

Tested on SPARC64/Linux, applied on the mainline.


2019-02-15  Eric Botcazou  

* g++.dg/asan/asan_oob_test.cc: Skip OOB_int on SPARC.
* g++.dg/asan/function-argument-3.C: Tweak for 32-bit SPARC.

-- 
Eric BotcazouIndex: g++.dg/asan/asan_oob_test.cc
===
--- g++.dg/asan/asan_oob_test.cc	(revision 268849)
+++ g++.dg/asan/asan_oob_test.cc	(working copy)
@@ -68,9 +68,13 @@ TEST(AddressSanitizer, OOB_char) {
   OOBTest();
 }
 
+// The following test uses unaligned memory accesses
+
+#if !defined(__sparc__)
 TEST(AddressSanitizer, OOB_int) {
   OOBTest();
 }
+#endif
 
 TEST(AddressSanitizer, OOBRightTest) {
   for (size_t access_size = 1; access_size <= 8; access_size *= 2) {
Index: g++.dg/asan/function-argument-3.C
===
--- g++.dg/asan/function-argument-3.C	(revision 268849)
+++ g++.dg/asan/function-argument-3.C	(working copy)
@@ -2,7 +2,16 @@
 // { dg-shouldfail "asan" }
 // { dg-additional-options "-Wno-psabi" }
 
+// On SPARC 32-bit, only vectors up to 8 bytes are passed in registers
+#if defined(__sparc__) && !defined(__sparcv9) && !defined(__arch64__)
+#define SMALL_VECTOR
+#endif
+
+#ifdef SMALL_VECTOR
+typedef int v4si __attribute__ ((vector_size (8)));
+#else
 typedef int v4si __attribute__ ((vector_size (16)));
+#endif
 
 static __attribute__ ((noinline)) int
 goo (v4si *a)
@@ -19,10 +28,14 @@ foo (v4si arg)
 int
 main ()
 {
+#ifdef SMALL_VECTOR
+  v4si v = {1,2};
+#else
   v4si v = {1,2,3,4};
+#endif
   return foo (v);
 }
 
 // { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow on address.*(\n|\r\n|\r)" }
 // { dg-output "READ of size . at.*" }
-// { dg-output ".*'arg' \\(line 14\\) <== Memory access at offset \[0-9\]* overflows this variable.*" }
+// { dg-output ".*'arg' \\(line 23\\) <== Memory access at offset \[0-9\]* overflows this variable.*" }


[SPARC] Small ASAN fixes

2019-02-15 Thread Eric Botcazou
This automatically passes -funwind-tables when ASAN is used on Linux, as done 
for other architectures, and also adjusts the shadow offset in 64-bit mode.

Tested on SPARC64/Linux, applied on the mainline.


2019-02-15  Eric Botcazou  

* config/sparc/linux.h (ASAN_CC1_SPEC): Define.
(CC1_SPEC): Use GNU_USER_TARGET_CC1_SPEC and ASAN_CC1_SPEC.
* config/sparc/linux64.h (ASAN_CC1_SPEC): Likewise.
(CC1_SPEC): Likewise.
* config/sparc/sparc.c (sparc_asan_shadow_offset): Adjust for 64-bit.

-- 
Eric BotcazouIndex: config/sparc/linux.h
===
--- config/sparc/linux.h	(revision 268849)
+++ config/sparc/linux.h	(working copy)
@@ -54,10 +54,11 @@ extern const char *host_detect_local_cpu
 
 #define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
 
-/* This is for -profile to use -lc_p instead of -lc.  */
-#undef	CC1_SPEC
-#define	CC1_SPEC "%{profile:-p} \
-"
+#undef  ASAN_CC1_SPEC
+#define ASAN_CC1_SPEC "%{%:sanitize(address):-funwind-tables}"
+
+#undef  CC1_SPEC
+#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC ASAN_CC1_SPEC
 
 #undef SIZE_TYPE
 #define SIZE_TYPE "unsigned int"
Index: config/sparc/linux64.h
===
--- config/sparc/linux64.h	(revision 268849)
+++ config/sparc/linux64.h	(working copy)
@@ -143,24 +143,25 @@ extern const char *host_detect_local_cpu
 
 #define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
 
-#undef	CC1_SPEC
+#undef  ASAN_CC1_SPEC
+#define ASAN_CC1_SPEC "%{%:sanitize(address):-funwind-tables}"
+
+#undef  CC1_SPEC
 #if DEFAULT_ARCH32_P
-#define CC1_SPEC "%{profile:-p} \
-%{m32:%{m64:%emay not use both -m32 and -m64}} \
+#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC ASAN_CC1_SPEC \
+"%{m32:%{m64:%emay not use both -m32 and -m64}} \
 %{m64:-mptr64 -mstack-bias -mlong-double-128 \
   %{!mcpu*:-mcpu=ultrasparc} \
-  %{!mno-vis:%{!mcpu=v9:-mvis}}} \
-"
+  %{!mno-vis:%{!mcpu=v9:-mvis}}}"
 #else
-#define CC1_SPEC "%{profile:-p} \
-%{m32:%{m64:%emay not use both -m32 and -m64}} \
+#define CC1_SPEC GNU_USER_TARGET_CC1_SPEC ASAN_CC1_SPEC \
+"%{m32:%{m64:%emay not use both -m32 and -m64}} \
 %{m32:-mptr32 -mno-stack-bias %{!mlong-double-128:-mlong-double-64} \
   %{!mcpu*:-mcpu=cypress}} \
 %{mv8plus:-mptr32 -mno-stack-bias %{!mlong-double-128:-mlong-double-64} \
   %{!mcpu*:-mcpu=v9}} \
 %{!m32:%{!mcpu*:-mcpu=ultrasparc}} \
-%{!mno-vis:%{!m32:%{!mcpu=v9:-mvis}}} \
-"
+%{!mno-vis:%{!m32:%{!mcpu=v9:-mvis}}}"
 #endif
 
 /* Support for a compile-time default CPU, et cetera.  The rules are:
Index: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 268849)
+++ config/sparc/sparc.c	(working copy)
@@ -12524,7 +12524,7 @@ sparc_init_machine_status (void)
 static unsigned HOST_WIDE_INT
 sparc_asan_shadow_offset (void)
 {
-  return TARGET_ARCH64 ? HOST_WIDE_INT_C (0x7fff8000) : (HOST_WIDE_INT_1 << 29);
+  return TARGET_ARCH64 ? (HOST_WIDE_INT_1 << 43) : (HOST_WIDE_INT_1 << 29);
 }
 
 /* This is called from dwarf2out.c via TARGET_ASM_OUTPUT_DWARF_DTPREL.


Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

2019-02-15 Thread Eric Botcazou
> I'm ready to commit the patch once it's approved, and have been since
> the day the problem was reported.

Maybe CCing whoever approved the previous patch would help?

-- 
Eric Botcazou


Re: [patch] Disable store merging in asan_expand_mark_ifn

2019-02-15 Thread Eric Botcazou
> > OK, revised patch attached.  I have manually verified that it yields the
> > expected result for an array of long doubles on 64-bit SPARC.
> > 
> > 
> > 2019-02-12  Eric Botcazou  
> > 
> > * asan.c (asan_expand_mark_ifn): Take into account the alignment of
> > the object to pick the size of stores on strict-alignment platforms.
> 
> Ok, thanks.

Glad you insisted in the end, because I have ASAN working on SPARC64/Linux, 
but only after fixing another bug on 64-bit strict-alignment platforms:

  /* Align base if target is STRICT_ALIGNMENT.  */
  if (STRICT_ALIGNMENT)
base = expand_binop (Pmode, and_optab, base,
 gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
  << ASAN_SHADOW_SHIFT)
 / BITS_PER_UNIT), Pmode), NULL_RTX,
 1, OPTAB_DIRECT);

GET_MODE_ALIGNMENT is unsigned int so this zero-extends to unsigned long...

Tested on 32-bit and 64-bit SPARC/Linux, applied on mainline as obvious.


2019-02-15  Eric Botcazou  

* asan.c (asan_emit_stack_protection): Use full-sized mask to align
the base address on 64-bit strict-alignment platforms.

-- 
Eric BotcazouIndex: asan.c
===
--- asan.c	(revision 268849)
+++ asan.c	(working copy)
@@ -1440,13 +1441,15 @@ asan_emit_stack_protection (rtx base, rt
 	base_align_bias = ((asan_frame_size + alignb - 1)
 			   & ~(alignb - HOST_WIDE_INT_1)) - asan_frame_size;
 }
+
   /* Align base if target is STRICT_ALIGNMENT.  */
   if (STRICT_ALIGNMENT)
-base = expand_binop (Pmode, and_optab, base,
-			 gen_int_mode (-((GET_MODE_ALIGNMENT (SImode)
-	  << ASAN_SHADOW_SHIFT)
-	 / BITS_PER_UNIT), Pmode), NULL_RTX,
-			 1, OPTAB_DIRECT);
+{
+  const HOST_WIDE_INT align
+	= (GET_MODE_ALIGNMENT (SImode) / BITS_PER_UNIT) << ASAN_SHADOW_SHIFT;
+  base = expand_binop (Pmode, and_optab, base, gen_int_mode (-align, Pmode),
+			   NULL_RTX, 1, OPTAB_DIRECT);
+}
 
   if (use_after_return_class == -1 && pbase)
 emit_move_insn (pbase, base);
@ -1534,7 +1548,7 @@ asan_emit_stack_protection (rtx base, rt
   shadow_mem = gen_rtx_MEM (SImode, shadow_base);
   set_mem_alias_set (shadow_mem, asan_shadow_set);
   if (STRICT_ALIGNMENT)
-set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
+set_mem_align (shadow_mem, GET_MODE_ALIGNMENT (SImode));
   prev_offset = base_offset;
 
   asan_redzone_buffer rz_buffer (shadow_mem, prev_offset);


Re: GCC missing -flto optimizations? SPEC lbm benchmark

2019-02-15 Thread Eric Botcazou
> Hasn't GNAT sorted Ada elements in records (e.g. structures) by size
> since near its initial addition to GCC in the mid-90s? This results in the
> largest elements up front and minimizes the need for alignment gaps.

No, that's a serious misconception, since one of the features of GNAT is to be 
compatible with C by default as much as possible.  But we started to do some 
reordering recently when the records don't have (direct) equivalents in C.

-- 
Eric Botcazou


[testsuite] Tweak gcc.target/sparc/struct-ret-check-1.c

2019-02-15 Thread Eric Botcazou
It cannot pass in PIE mode.

Tested on SPARC64/Linux, applied on all active branches.


2019-02-15  Eric Botcazou  

* gcc.target/sparc/struct-ret-check-1.c: Add -fno-pie option.

-- 
Eric Botcazou
Index: gcc.target/sparc/struct-ret-check-1.c
===
--- gcc.target/sparc/struct-ret-check-1.c	(revision 268849)
+++ gcc.target/sparc/struct-ret-check-1.c	(working copy)
@@ -7,7 +7,7 @@
 
 /* Origin: Carlos O'Donell  */
 /* { dg-do run { target sparc*-*-solaris* sparc*-*-linux* sparc*-*-*bsd* } } */
-/* { dg-options "-mstd-struct-return" } */
+/* { dg-options "-mstd-struct-return -fno-pie" } */
 /* { dg-require-effective-target ilp32 } */
 #include 
 #include 


[testsuite] Small tweaks for Visium

2019-02-15 Thread Eric Botcazou
The only interesting one is gcc.dg/tree-ssa/pr84859.c: for it to pass, the 
undocumented -ftree-cselim must be enabled, which is done automatically only 
on targets with conditional moves, what the Visium is not.

Tested on visium-elf, applied on the mainline and 8 branch.


2019-02-15  Eric Botcazou  

* c-c++-common/patchable_function_entry-decl.c: Do not run on Visium.
* c-c++-common/patchable_function_entry-default.c: Likewise.
* c-c++-common/patchable_function_entry-definition.c: Likewise.
* gcc.dg/tree-ssa/pr84859.c: Add -ftree-cselim switch.

-- 
Eric BotcazouIndex: c-c++-common/patchable_function_entry-decl.c
===
--- c-c++-common/patchable_function_entry-decl.c	(revision 268849)
+++ c-c++-common/patchable_function_entry-decl.c	(working copy)
@@ -1,6 +1,5 @@
-/* { dg-do compile { target { ! nvptx*-*-* } } } */
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
 /* { dg-final { scan-assembler-times "nop|NOP" 2 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 2 { target alpha*-*-* } } } */
 
Index: c-c++-common/patchable_function_entry-default.c
===
--- c-c++-common/patchable_function_entry-default.c	(revision 268849)
+++ c-c++-common/patchable_function_entry-default.c	(working copy)
@@ -1,6 +1,5 @@
-/* { dg-do compile { target { ! nvptx*-*-* } } } */
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
 /* { dg-final { scan-assembler-times "nop|NOP" 3 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 3 { target alpha*-*-* } } } */
 
Index: c-c++-common/patchable_function_entry-definition.c
===
--- c-c++-common/patchable_function_entry-definition.c	(revision 268849)
+++ c-c++-common/patchable_function_entry-definition.c	(working copy)
@@ -1,6 +1,5 @@
-/* { dg-do compile { target { ! nvptx*-*-* } } } */
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
 /* { dg-final { scan-assembler-times "nop|NOP" 1 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 1 { target alpha*-*-* } } } */
 Index: gcc.dg/tree-ssa/pr84859.c
===
--- gcc.dg/tree-ssa/pr84859.c	(revision 268849)
+++ gcc.dg/tree-ssa/pr84859.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -Warray-bounds -fdump-tree-phiopt2" } */
+/* { dg-options "-O2 -ftree-cselim -Warray-bounds -fdump-tree-phiopt2" } */
 
 void
 h (const void *p, unsigned n)


[visium] Adjust to recent assembler change

2019-02-15 Thread Eric Botcazou
This adjusts the compiler to the assembler change I recently istalled:
  https://sourceware.org/ml/binutils/2019-02/msg00035.html

The final.c one-liner is trivial, it changes the test to the exact condition 
under which the fallthrough code won't segfault.

Tested on visium-elf, applied on the mainline.


2019-02-15  Eric Botcazou  

libgcc/
* config/visium/lib2funcs.c (__set_trampoline_parity): Replace
TRAMPOLINE_SIZE with __LIBGCC_TRAMPOLINE_SIZE__.
gcc/
* final.c (insn_current_reference_address): Replace test on JUMP_P
with test on jump_to_label_p.
* config/visium/visium-passes.def: New file.
* config/visium/t-visium (PASSES_EXTRA): Define.
* config/visium/visium-protos.h (make_pass_visium_reorg): Declare.
* config/visium/visium.h (TRAMPOLINE_SIZE): Adjust.
(TRAMPOLINE_ALIGNMENT): Define.
* config/visium/visium.c (visium_option_override): Do not register
the machine-specific reorg pass here.
(visium_trampoline_init): Align the BRA insn on a 64-bit boundary
for the GR6.
(output_branch): Adjust threshold for long branch instruction.
* config/visium/visium.md (cpu): Move around.
(length): Adjust for the GR6.

-- 
Eric BotcazouIndex: libgcc/config/visium/lib2funcs.c
===
--- libgcc/config/visium/lib2funcs.c	(revision 268849)
+++ libgcc/config/visium/lib2funcs.c	(working copy)
@@ -315,7 +315,9 @@ __set_trampoline_parity (UWtype *addr)
 {
   int i;
 
-  for (i = 0; i < (TRAMPOLINE_SIZE * __CHAR_BIT__) / W_TYPE_SIZE; i++)
+  for (i = 0;
+   i < (__LIBGCC_TRAMPOLINE_SIZE__ * __CHAR_BIT__) / W_TYPE_SIZE;
+   i++)
 addr[i] |= parity_bit (addr[i]);
 }
 #endif
Index: gcc/final.c
===
--- gcc/final.c	(revision 268849)
+++ gcc/final.c	(working copy)
@@ -606,7 +606,7 @@ insn_current_reference_address (rtx_insn
 
   rtx_insn *seq = NEXT_INSN (PREV_INSN (branch));
   seq_uid = INSN_UID (seq);
-  if (!JUMP_P (branch))
+  if (!jump_to_label_p (branch))
 /* This can happen for example on the PA; the objective is to know the
offset to address something in front of the start of the function.
Thus, we can treat it like a backward branch.
Index: gcc/config/visium/t-visium
===
--- gcc/config/visium/t-visium	(revision 268849)
+++ gcc/config/visium/t-visium	(working copy)
@@ -1,4 +1,5 @@
-# Multilibs for Visium.
+# General rules that all visium/ targets must have.
+
 # Copyright (C) 2012-2019 Free Software Foundation, Inc.
 #
 # This file is part of GCC.
@@ -17,6 +18,8 @@
 # along with GCC; see the file COPYING3.  If not see
 # <http://www.gnu.org/licenses/>.
 
+PASSES_EXTRA += $(srcdir)/config/visium/visium-passes.def
+
 # The compiler defaults to -mcpu=gr5 but this may be overridden via --with-cpu
 # at configure time so the -mcpu setting must be symmetrical.
 MULTILIB_OPTIONS = mcpu=gr5/mcpu=gr6 muser-mode
Index: gcc/config/visium/visium-passes.def
===
--- gcc/config/visium/visium-passes.def	(nonexistent)
+++ gcc/config/visium/visium-passes.def	(working copy)
@@ -0,0 +1,27 @@
+/* Description of target passes for Visium.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/*
+   Macros that can be used in this file:
+   INSERT_PASS_AFTER (PASS, INSTANCE, TGT_PASS)
+   INSERT_PASS_BEFORE (PASS, INSTANCE, TGT_PASS)
+   REPLACE_PASS (PASS, INSTANCE, TGT_PASS)
+ */
+
+  INSERT_PASS_AFTER (pass_delay_slots, 1, pass_visium_reorg);
Index: gcc/config/visium/visium-protos.h
===
--- gcc/config/visium/visium-protos.h	(revision 268849)
+++ gcc/config/visium/visium-protos.h	(working copy)
@@ -61,4 +61,6 @@ extern int visium_expand_block_set (rtx
 extern unsigned int reg_or_subreg_regno (rtx);
 #endif /* RTX_CODE */
 
+extern rtl_opt_pass * make_pass_visium_reorg (gcc::context *);
+
 #endif
Index: gcc/config/visium/visium.c
===
--- gcc/config/visium/visium.c	(revision 268849)
+++ gcc/config/visium/visium.c	(working copy)
@@ -484,20 +484,6 @@ visium_opt

[patch] Fix LRA/reload issue with -fnon-call-exceptions

2019-02-15 Thread Eric Botcazou
Hi,

this is a regression present on all active branches since the controversial 
get_initial_register_offset stuff was added to rtlanal.c some time ago, and 
visible in the testsuite on PowerPC/Linux under the form of gnat.dg/opt73.adb 
timing out at run time.

The problem is that the compiler generates code that doesn't save the frame 
pointer before clobbering it, because rs6000_stack_info computes a wrong final 
(post-reload) stack layout.  The scenario is as follows: LRA decides to use 
the frame pointer, sets reload_completed to 1 at the end and then does:

  /* We've possibly turned single trapping insn into multiple ones.  */
  if (cfun->can_throw_non_call_exceptions)
{
  auto_sbitmap blocks (last_basic_block_for_fn (cfun));
  bitmap_ones (blocks);
  find_many_sub_basic_blocks (blocks);
}

But find_many_sub_basic_blocks calls control_flow_insn_p, which in turn can 
call rtx_addr_can_trap_p_1, which can call get_initial_register_offset, which 
uses INITIAL_ELIMINATION_OFFSET, IOW rs6000_initial_elimination_offset, which 
calls rs6000_stack_info.  But at this point the DF information hasn't been 
updated so the frame pointer isn't detected as live by df_regs_ever_live_p.

You may think that the fix is just to set reload_completed to 1 after the 
above code in lra, but that's not sufficient because the same issue can arise 
from the do_reload function:

  if (optimize)
cleanup_cfg (CLEANUP_EXPENSIVE);

when checking is enabled, because cleanup_cfg can calls control_flow_insn_p 
and then eventually rtx_addr_can_trap_p_1.  In other words, we would need 
to set reload_completed to 1 only after the above code, which is very late.
As a matter of fact, that's not possible for old reload itself because of:

  /* We must set reload_completed now since the cleanup_subreg_operands call
 below will re-recognize each insn and reload may have generated insns
 which are only valid during and after reload.  */
  reload_completed = 1;

So, barring the removal of the get_initial_register_offset stuff, the only 
simple fix is probably to prevent it from calling into the back-end too early, 
for example with the attached fixlet.  Tested on x86-64 and PowerPC/Linux.

Thoughts?  Where do we want to fix this?


* rtlanal.c (get_initial_register_offset): Fall back to the raw estimate
as long as the epilogue isn't completed.

-- 
Eric BotcazouIndex: rtlanal.c
===
--- rtlanal.c	(revision 268849)
+++ rtlanal.c	(working copy)
@@ -359,10 +359,10 @@ get_initial_register_offset (int from, i
   if (to == from)
 return 0;
 
-  /* It is not safe to call INITIAL_ELIMINATION_OFFSET
- before the reload pass.  We need to give at least
- an estimation for the resulting frame size.  */
-  if (! reload_completed)
+  /* It is not safe to call INITIAL_ELIMINATION_OFFSET before the epilogue
+ is completed, but we need to give at least an estimate for the stack
+ pointer based on the frame size.  */
+  if (!epilogue_completed)
 {
   offset1 = crtl->outgoing_args_size + get_frame_size ();
 #if !STACK_GROWS_DOWNWARD


Re: [PATCH] Avoid assuming valid_constant_size_p argument is a constant expression (PR 89294)

2019-02-14 Thread Eric Botcazou
> The attached patch removes the assumption introduced earlier today
> in my fix for bug 87996 that the valid_constant_size_p argument is
> a constant expression.  I couldn't come up with a C/C++ test case
> where this isn't true but apparently it can happen in Ada which I
> inadvertently didn't build.

Can we do something here?  Our internal testers have been down for 3 days 
because of this blunder...

-- 
Eric Botcazou


Re: [rs6000] 64-bit integer loads/stores and FP instructions

2019-02-14 Thread Eric Botcazou
> Yeah, something like that.  It will need some serious testing, to make
> sure we don't regress (including not regressing what that patch that took
> them away was meant to do).  I can arrange some testing, will you do the
> patch though?

I can do the patch and also (correctness) testing for 32-bit Linux.

Another issue is the extent of the patch: practically speaking, putting back 
the '*' modifier before all the 'd' constraints would be sufficient, but the 
current setting is a bit inconsistent|*] so this could also be adjusted.

[*] For example, in the *movdi_internal32 pattern, 2 'wi' constraints have it 
but not the other 2.  Likewise for "wv'.

-- 
Eric Botcazou


Re: [PATCH] Fix UB in prepare_cmp_insn (PR middle-end/89281)

2019-02-13 Thread Eric Botcazou
> The code will do:
>   size = convert_to_mode (cmp_mode, size, 1);
> i.e. convert size from whatever mode it had before to cmp_mode and the
> test is whether it can do so without changing the behavior.  If size is
> non-constant, then that can be obviously (without using range info etc.)
> done only if the original mode is narrower or at most as wide as cmp_mode.
> We could do the same for CONST_INT_P too, but as we know the constant,
> it wants to make sure that the size can be expressed in cmp_mode.
> As it is unsigned quantity, that can be checked by checking if the value is
> <= GET_MODE_MASK.

That's one interpretationn, the other being that of emit_block_move_via_movmem 
and the latter looks sensible to me too: if the top bit is set, the conversion 
will yield a negative RTL constant which will be sent to the target pattern, 
which could not be prepared for such a negative constant:

`movmemM'
 Block move instruction.  The destination and source blocks of
 memory are the first two operands, and both are `mem:BLK's with an
 address in mode `Pmode'.

 The number of bytes to move is the third operand, in mode M.
 Usually, you specify `Pmode' for M.  However, if you can generate
 better code knowing the range of valid lengths is smaller than
 those representable in a full Pmode pointer, you should provide a
 pattern with a mode corresponding to the range of values you can
 handle efficiently (e.g., `QImode' for values in the range 0-127;
 note we avoid numbers that appear negative) and also a pattern
 with `Pmode'.

That being said, your patch doesn't change the interpretation so is OK.

-- 
Eric Botcazou


Re: Define __sparcv8 in 64-bit-default Solaris/SPARC gcc with -m32

2019-02-13 Thread Eric Botcazou
> Eric, could you have another look if I'm missing something?

Note that this OPTION_DEFAULT_SPECS code is duplicated in linux64.h.

There is this comment:

   In the SPARC_BI_ARCH compiler we cannot pass %{!mcpu=*:-mcpu=%(VALUE)}
   here, otherwise say -mcpu=v7 would be passed even when -m64.
   CC1_SPEC above takes care of this instead.

Why does CC1_SPEC not work apparently?

> Otherwise, I'm going to commit the patch (it's necessary to enable
> SANITIZER_CAN_FAST_UNWIND on Solaris/SPARC and make use of
> libsanitizer/sanitizer_common/sanitizer_stacktrace_sparc.cc).

No, it will not be necessary, I have ported the implementation to 64-bit.

-- 
Eric Botcazou


Re: [PATCH] Fix UB in prepare_cmp_insn (PR middle-end/89281)

2019-02-12 Thread Eric Botcazou
> The following hunk of code results in UB on the recently added testcase,
> because if cmp_mode is SImode or DImode, then 1 << 32 or 1 << 64 is
> undefined.  Fixed by using GET_MODE_MASK, plus UINTVAL because size is
> really unsigned (code later on uses unsignedp=1 too).

Doesn't the current check make sure that the RTL constant is valid for the 
mode though (since RTL constants are sign-extended for their mode)?  See 
emit_block_move_via_movmem for an equivalent check with GET_MODE_MASK >> 1.

-- 
Eric Botcazou


Re: [patch] Disable store merging in asan_expand_mark_ifn

2019-02-12 Thread Eric Botcazou
> Ok, stand corrected on that, 128-bit indeed, but even that is nothing not
> really used.

The irony is that I'm doing this for 32-bit SPARC (we cannot get ASAN to work 
in 64-bit mode for the time being) and the maximum alignment on 32-bit SPARC 
is 64-bit (even long doubles) so this will be totally unused. ;-)

> For STRICT_ALIGNMENT targets store merging pass obviously can't do anything
> with those, because unlike asan.c it can't figure out the alignment.

OK, revised patch attached.  I have manually verified that it yields the 
expected result for an array of long doubles on 64-bit SPARC.


2019-02-12  Eric Botcazou  

* asan.c (asan_expand_mark_ifn): Take into account the alignment of
the object to pick the size of stores on strict-alignment platforms.

-- 
Eric BotcazouIndex: asan.c
===
--- asan.c	(revision 268508)
+++ asan.c	(working copy)
@@ -3218,7 +3218,10 @@ asan_expand_mark_ifn (gimple_stmt_iterat
   /* Generate direct emission if size_in_bytes is small.  */
   if (size_in_bytes <= ASAN_PARAM_USE_AFTER_SCOPE_DIRECT_EMISSION_THRESHOLD)
 {
-  unsigned HOST_WIDE_INT shadow_size = shadow_mem_size (size_in_bytes);
+  const unsigned HOST_WIDE_INT shadow_size
+	= shadow_mem_size (size_in_bytes);
+  const unsigned int shadow_align
+	= (get_pointer_alignment (base) / BITS_PER_UNIT) >> ASAN_SHADOW_SHIFT;
 
   tree shadow = build_shadow_mem_access (iter, loc, base_addr,
 	 shadow_ptr_types[0], true);
@@ -3226,9 +3229,11 @@ asan_expand_mark_ifn (gimple_stmt_iterat
   for (unsigned HOST_WIDE_INT offset = 0; offset < shadow_size;)
 	{
 	  unsigned size = 1;
-	  if (shadow_size - offset >= 4)
+	  if (shadow_size - offset >= 4
+	  && (!STRICT_ALIGNMENT || shadow_align >= 4))
 	size = 4;
-	  else if (shadow_size - offset >= 2)
+	  else if (shadow_size - offset >= 2
+		   && (!STRICT_ALIGNMENT || shadow_align >= 2))
 	size = 2;
 
 	  unsigned HOST_WIDE_INT last_chunk_size = 0;


Re: [rs6000] 64-bit integer loads/stores and FP instructions

2019-02-12 Thread Eric Botcazou
> No, we should allow both integer and floating point insns for integer stores
> always.  We just get the cost estimates slightly wrong now, apparently.

Note that my proof of concept patch doesn't disallow them either...  So what 
do you suggest?  Just putting back the '*' modifiers in the DI patterns?  As a 
matter of fact there are still present in the SI pattern.

-- 
Eric Botcazou


Re: [patch] Disable store merging in asan_expand_mark_ifn

2019-02-11 Thread Eric Botcazou
> No.  64-bit aligned offsets too.  If you know 64-bit alignment of base_addr,
> you can use size 2 stores (though not size 4 stores) on the
> !STRICT_ALIGNMENT targets.  And that is something still pretty common.

But we're talking about STRICT_ALIGNMENT targets here: an array of 2 doubles 
at address 0x1008 will have a shadow address of 0x2001 modulo the 
offset so you cannot use size 2.  Moveover the store merging pass should be 
able to merge the stores so I don't really understand why this matters at all.

-- 
Eric Botcazou


Re: [patch] Disable store merging in asan_expand_mark_ifn

2019-02-11 Thread Eric Botcazou
> So, wouldn't it be better to check for STRICT_ALIGNMENT
> get_pointer_alignment (base_addr) and do this only if that alignment
> (shifted right by ASAN_SHADOW_SHIFT) is not sufficient and e.g. if we would
> know that the shadow is at least 2 byte aligned but not 4 byte aligned, use
> size = 2 instead of always 1?  E.g. compute this before the loop as
> max_size and for !STRICT_ALIGNMENT use always max_size 4?

In practice this makes a difference only for objects aligned on 128-bit or 
above boundaries though.  Moreover, don't you need to take into account the 
offset as well, which can be modified through -fasan-shadow-offset?

-- 
Eric Botcazou


[patch] Disable store merging in asan_expand_mark_ifn

2019-02-11 Thread Eric Botcazou
Hi,

asan_expand_mark_ifn does manual store merging but doesn't take into account 
the alignment, so this can break on strict-alignment platforms.

Tested on SPARC/Solaris 11, where this fixes this regression:

FAIL: gcc.dg/asan/use-after-scope-5.c   -O0  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O1  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O2  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O3 -fomit-frame-pointer -funroll-
loops -fpeel-loops -ftracer -finline-functions  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O3 -g  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -Os  output pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O2 -flto -flto-partition=none  output 
pattern test
FAIL: gcc.dg/asan/use-after-scope-5.c   -O2 -flto  output pattern test

OK for mainline?


2019-02-11  Eric Botcazou  

* asan.c (asan_expand_mark_ifn): Always use a size of 1 byte for the
stores on strict-alignment platforms.

-- 
Eric BotcazouIndex: asan.c
===
--- asan.c	(revision 268508)
+++ asan.c	(working copy)
@@ -3226,10 +3226,13 @@ asan_expand_mark_ifn (gimple_stmt_iterat
   for (unsigned HOST_WIDE_INT offset = 0; offset < shadow_size;)
 	{
 	  unsigned size = 1;
-	  if (shadow_size - offset >= 4)
-	size = 4;
-	  else if (shadow_size - offset >= 2)
-	size = 2;
+	  if (!STRICT_ALIGNMENT)
+	{
+	  if (shadow_size - offset >= 4)
+		size = 4;
+	  else if (shadow_size - offset >= 2)
+		size = 2;
+	}
 
 	  unsigned HOST_WIDE_INT last_chunk_size = 0;
 	  unsigned HOST_WIDE_INT s = (offset + size) * ASAN_SHADOW_GRANULARITY;


Re: [rs6000] 64-bit integer loads/stores and FP instructions

2019-02-11 Thread Eric Botcazou
> You make an arbitrary distinction between certain CPUs and duplicate
> patterns based on that.

Sure, somewhat arbitrary, but that's a proof of concept and IMO better than 
treating every processor the same way.  The alternative would be to complicate 
further the existing patterns by means of the "enabled" attribute or somesuch, 
but I can try it too.

> About the bug: it should behave the same as before, use GPRs only in your
> testcase.

It's fixed by backporting the last part of PR target/85755 on the 7 branch, 
but I can certainly add the testcase to the gcc.target/powerpc testsuite.

-- 
Eric Botcazou


Re: Follow-up-fix to "[PATCH] Move PR84877 fix elsewhere (PR bootstrap/88450)"

2019-02-11 Thread Eric Botcazou
> To fix the r267812 incorrectness we need to use the *stored*
> size, i.e. that which will be stored into using register-sized
> writes.  It's seems like the bug is just a typo, so the fix is
> as simple as follows.  Note the usage of "diff -U 10" to show
> that size_stored is used in the "then"-arm.

Right, thanks for catching it.

> Ok to commit?
> 
> gcc/ChangeLog:
>   * function.c (assign_parm_setup_block): Use the stored
>   size, not the passed size, when allocating stack-space,
>   also for a parameter with alignment larger than
>   MAX_SUPPORTED_STACK_ALIGNMENT.

OK, thanks.

-- 
Eric Botcazou


Re: [testsuite, ada] Don't XPASS gnat.dg/lto19.adb

2019-02-09 Thread Eric Botcazou
> Between 20181106 (r265849) and 20181107 (r265879), gnat.dg/lto19.adb
> started to XPASS everywhere:
> 
> XPASS: gnat.dg/lto19.adb (test for excess errors)
> 
> Fixed as follows, tested on i386-pc-solaris2.11 and sparc-sun-solaris2.11.

Jan just fixed the lto8 failure (thanks!) so you can go ahead with the patch.

-- 
Eric Botcazou


[Ada] Fix out-of-bounds read with wild unchecked conversion

2019-02-08 Thread Eric Botcazou
This is not really a regression (or maybe a very old one) but the behavior of 
the compiler is a bit annoying so IMO worth fixing: when you put a size clause 
to silence the warning about an unchecked conversion from a small type to a 
(very) large one, the compiler effectively disregards the clause if the source 
is an aggregate, thus generating an out-of-bounds read.

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


2019-02-08  Eric Botcazou  

* gcc-interface/trans.c (gnat_to_gnu) : Minor tweak.
* gcc-interface/utils.c (convert): Do not pad when doing an unchecked
conversion here.  Use TREE_CONSTANT throughout the function.
(unchecked_convert): Also pad if the source is a CONSTRUCTOR and the
destination is a more aligned array type or a larger aggregate type,
but not between original and packable versions of a type.

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 268674)
+++ gcc-interface/trans.c	(working copy)
@@ -7230,13 +7230,10 @@ gnat_to_gnu (Node_Id gnat_node)
   {
 	tree gnu_aggr_type;
 
-	/* ??? It is wrong to evaluate the type now, but there doesn't
-	   seem to be any other practical way of doing it.  */
-
+	/* Check that this aggregate has not slipped through the cracks.  */
 	gcc_assert (!Expansion_Delayed (gnat_node));
 
-	gnu_aggr_type = gnu_result_type
-	  = get_unpadded_type (Etype (gnat_node));
+	gnu_result_type = get_unpadded_type (Etype (gnat_node));
 
 	if (TREE_CODE (gnu_result_type) == RECORD_TYPE
 	&& TYPE_CONTAINS_TEMPLATE_P (gnu_result_type))
@@ -7244,6 +7241,8 @@ gnat_to_gnu (Node_Id gnat_node)
 	= TREE_TYPE (DECL_CHAIN (TYPE_FIELDS (gnu_result_type)));
 	else if (TREE_CODE (gnu_result_type) == VECTOR_TYPE)
 	  gnu_aggr_type = TYPE_REPRESENTATIVE_ARRAY (gnu_result_type);
+	else
+	  gnu_aggr_type = gnu_result_type;
 
 	if (Null_Record_Present (gnat_node))
 	  gnu_result = gnat_build_constructor (gnu_aggr_type, NULL);
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c	(revision 268675)
+++ gcc-interface/utils.c	(working copy)
@@ -4356,19 +4356,12 @@ convert (tree type, tree expr)
 
   /* If the inner type is of self-referential size and the expression type
 	 is a record, do this as an unchecked conversion unless both types are
-	 essentially the same.  But first pad the expression if possible to
-	 have the same size on both sides.  */
+	 essentially the same.  */
   if (ecode == RECORD_TYPE
 	  && CONTAINS_PLACEHOLDER_P (DECL_SIZE (TYPE_FIELDS (type)))
 	  && TYPE_MAIN_VARIANT (etype)
 	 != TYPE_MAIN_VARIANT (TREE_TYPE (TYPE_FIELDS (type
-	{
-	  if (TREE_CODE (TYPE_SIZE (etype)) == INTEGER_CST)
-	expr = convert (maybe_pad_type (etype, TYPE_SIZE (type), 0, Empty,
-	false, false, false, true),
-			expr);
-	  return unchecked_convert (type, expr, false);
-	}
+	return unchecked_convert (type, expr, false);
 
   /* If we are converting between array types with variable size, do the
 	 final conversion as an unchecked conversion, again to avoid the need
@@ -4483,9 +4476,9 @@ convert (tree type, tree expr)
 case STRING_CST:
   /* If we are converting a STRING_CST to another constrained array type,
 	 just make a new one in the proper type.  */
-  if (code == ecode && AGGREGATE_TYPE_P (etype)
-	  && !(TREE_CODE (TYPE_SIZE (etype)) == INTEGER_CST
-	   && TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST))
+  if (code == ecode
+	  && !(TREE_CONSTANT (TYPE_SIZE (etype))
+	   && !TREE_CONSTANT (TYPE_SIZE (type
 	{
 	  expr = copy_node (expr);
 	  TREE_TYPE (expr) = type;
@@ -5332,7 +5325,7 @@ unchecked_convert (tree type, tree expr,
  so we skip all expressions that are references.  */
   else if (!REFERENCE_CLASS_P (expr)
 	   && !AGGREGATE_TYPE_P (etype)
-	   && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
+	   && TREE_CONSTANT (TYPE_SIZE (type))
 	   && (c = tree_int_cst_compare (TYPE_SIZE (etype), TYPE_SIZE (type
 {
   if (c < 0)
@@ -5380,16 +5373,36 @@ unchecked_convert (tree type, tree expr,
   return unchecked_convert (type, expr, notrunc_p);
 }
 
-  /* If we are converting a CONSTRUCTOR to a more aligned RECORD_TYPE, bump
- the alignment of the CONSTRUCTOR to speed up the copy operation.  */
+  /* If we are converting a CONSTRUCTOR to a more aligned aggregate type, bump
+ the alignment of the CONSTRUCTOR to speed up the copy operation.  But do
+ not do it for a conversion between original and packable version to avoid
+ an infinite recursion.  */
   else if (TREE_CODE (expr) == CONSTRUCTOR
-	   && code == RECORD_TYPE
+	   && AGGREGATE_TYPE_P (type)
+	   && TYPE_NAME (type) != TYPE_NAME (etype)
 	   && TYPE_ALIGN (etype) <

[Ada] Fix crash on very dynamic record types

2019-02-08 Thread Eric Botcazou
This is a regression present on the mainline and 8 branch: in some specific 
circumstances, you can get a crash when max_size is called from the gimplifier 
on very dynamic record types.  This makes the function a bit more robust.

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


2019-02-08  Eric Botcazou  

* gcc-interface/utils.c (max_size) : Be prepared for an
operand with VOID_TYPE.

-- 
Eric BotcazouIndex: gcc-interface/utils.c
===
--- gcc-interface/utils.c	(revision 268508)
+++ gcc-interface/utils.c	(working copy)
@@ -3642,7 +3642,10 @@ fntype_same_flags_p (const_tree t, tree
 
 /* EXP is an expression for the size of an object.  If this size contains
discriminant references, replace them with the maximum (if MAX_P) or
-   minimum (if !MAX_P) possible value of the discriminant.  */
+   minimum (if !MAX_P) possible value of the discriminant.
+
+   Note that the expression may have already been gimplified,in which case
+   COND_EXPRs have VOID_TYPE and no operands, and this must be handled.  */
 
 tree
 max_size (tree exp, bool max_p)
@@ -3714,11 +3717,15 @@ max_size (tree exp, bool max_p)
   return build_int_cst (type, max_p ? 1 : 0);
 
 case tcc_unary:
+  op0 = TREE_OPERAND (exp, 0);
+
   if (code == NON_LVALUE_EXPR)
-	return max_size (TREE_OPERAND (exp, 0), max_p);
+	return max_size (op0, max_p);
+
+  if (VOID_TYPE_P (TREE_TYPE (op0)))
+	return max_p ? TYPE_MAX_VALUE (type) : TYPE_MIN_VALUE (type);
 
-  op0 = max_size (TREE_OPERAND (exp, 0),
-		  code == NEGATE_EXPR ? !max_p : max_p);
+  op0 = max_size (op0, code == NEGATE_EXPR ? !max_p : max_p);
 
   if (op0 == TREE_OPERAND (exp, 0))
 	return exp;


[Ada] Fix profile mismatch with -fprofile-{generate,use} at -O2

2019-02-08 Thread Eric Botcazou
This is a regression present on all active branches: in some cases, you get 
bogus profile mismatches with -fprofile-{generate,use} at -O2 because some -O3 
optimizations are automatically enabled by -fprofile-use.

Tested on x86_64-suse-linux, applied on all active branches.


2019-02-08  Eric Botcazou  

* gcc-interface/trans.c (Regular_Loop_to_gnu): Replace tests on
individual flag_unswitch_loops and flag_tree_loop_vectorize switches
with test on global optimize switch.
(Raise_Error_to_gnu): Likewise.

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 268508)
+++ gcc-interface/trans.c	(working copy)
@@ -3787,7 +3787,7 @@ Regular_Loop_to_gnu (Node_Id gnat_node,
 	 unswitching is enabled, do not require the loop bounds to be also
 	 invariant, as their evaluation will still be ahead of the loop.  */
   if (vec_safe_length (gnu_loop_info->checks) > 0
-	 && (make_invariant (_low, _high) || flag_unswitch_loops))
+	 && (make_invariant (_low, _high) || optimize >= 3))
 	{
 	  struct range_check_info_d *rci;
 	  unsigned int i, n_remaining_checks = 0;
@@ -3840,22 +3840,21 @@ Regular_Loop_to_gnu (Node_Id gnat_node,
 	  /* Note that loop unswitching can only be applied a small number of
 	 times to a given loop (PARAM_MAX_UNSWITCH_LEVEL default to 3).  */
 	  if (IN_RANGE (n_remaining_checks, 1, 3)
-	  && optimize > 1
+	  && optimize >= 2
 	  && !optimize_size)
 	FOR_EACH_VEC_ELT (*gnu_loop_info->checks, i, rci)
 	  if (rci->invariant_cond != boolean_false_node)
 		{
 		  TREE_OPERAND (rci->inserted_cond, 0) = rci->invariant_cond;
 
-		  if (flag_unswitch_loops)
+		  if (optimize >= 3)
 		add_stmt_with_node_force (rci->inserted_cond, gnat_node);
 		}
 	}
 
   /* Second, if loop vectorization is enabled and the iterations of the
 	 loop can easily be proved as independent, mark the loop.  */
-  if (optimize
-	  && flag_tree_loop_vectorize
+  if (optimize >= 3
 	  && independent_iterations_p (LOOP_STMT_BODY (gnu_loop_stmt)))
 	LOOP_STMT_IVDEP (gnu_loop_stmt) = 1;
 
@@ -6478,7 +6477,7 @@ Raise_Error_to_gnu (Node_Id gnat_node, t
 		= build1 (SAVE_EXPR, boolean_type_node, boolean_true_node);
 	  vec_safe_push (loop->checks, rci);
 	  gnu_cond = build_noreturn_cond (gnat_to_gnu (gnat_cond));
-	  if (flag_unswitch_loops)
+	  if (optimize >= 3)
 		gnu_cond = build_binary_op (TRUTH_ANDIF_EXPR,
 	boolean_type_node,
 	rci->inserted_cond,


Re: [rs6000] 64-bit integer loads/stores and FP instructions

2019-02-08 Thread Eric Botcazou
> Backporting this is okay.  (It was not done because it does not affect
> correctness).  What is the "almost", btw?

The predicate of operand #0 of movdi_internal32 is rs6000_nonimmediate_operand 
on the 7 branch and nonimmediate_operand on the 8 branch and later.

> (In https://gcc.gnu.org/ml/gcc-patches/2016-11/txt4j9hLRLCzf.txt and you
> presumably mean the *movdi_internal32 hunk, and the first line of it:
> 
> - "=Y,r, r, ?m,?*d,?*d,
> + "=Y,r, r, ^m,^d, ^d,
> 
> where the last three are mem<-reg, reg<-mem, reg<-reg, for fp regs).
> 
> This patch was a year before we switched to LRA, and for non-LRA ports *
> does not do any register preferencing.

Are you sure?  See record_reg_classes in ira-costs.c:

case '*':
  /* Ignore the next letter for this pass.  */
  c = *++p;
  break;

> For 6xx/7xx people *wanted* to use FP loads and stores whenever possible,
> because you get twice the bandwidth with them.
> 
> If you do not want the FP registers used (or FP insns used), use
> -msoft-float, that's what it's for.

No disagreement, but you can't force people to use it when they use FP code in 
other parts of their software.  Believe me, we (and probably others) tried...

> Patches are welcome, but complicating this already very complex code by
> introducing an arbitrary distinction between "embedded and other" (or
> actually, "server and other") isn't the way to go.  Sorry.

Not clear what you mean by complicating here...  the approach is rather simple 
and precisely aimed at not disturbing the common path, i.e. server processors,
while preserving the historical path for other processors.  AFAIK nobody has 
evaluated the effects of the original change beyond POWER 7/8/9.

> Could you open a PR please?

Sure, but about what now? ;-)

-- 
Eric Botcazou


Re: [testsuite, ada] Don't XPASS gnat.dg/lto19.adb

2019-02-08 Thread Eric Botcazou
> Between 20181106 (r265849) and 20181107 (r265879), gnat.dg/lto19.adb
> started to XPASS everywhere:
> 
> XPASS: gnat.dg/lto19.adb (test for excess errors)

The idea was to wait until Jan fixes the lto8 failure introduced roughly at 
the same time...

-- 
Eric Botcazou


Re: [PATCH] Fix ICE due to copy_reg_eh_region_note_forward (PR rtl-optimization/89234)

2019-02-08 Thread Eric Botcazou
> The following testcase ICEs on ppc64le.  The problem is that
> copy_reg_eh_region_note_* functions accept either some instruction, or
> REG_EH_REGION note directly.  To differentiate between those it uses INSN_P
> test (and returns early if the insn doesn't contain any REG_EH_REGION
> notes).  If the function is called on a rtx_insn * that isn't INSN_P, like
> on the testcase on a NOTE, it assumes it must be REG_EH_REGION note, uses
> XEXP (note, 0) on it, which is actually PREV_INSN in this case and stores
> an instruction (JUMP_INSN in this case) into REG_EH_REGION notes it creates.

It took me a few minutes to clear the confusion between NOTE & REG_NOTE here.

> I believe we should treat rtx_insn * that aren't INSN_P like instructions
> that don't have any REG_EH_REGION notes.
>
> 2019-02-07  Jakub Jelinek  
> 
>   PR rtl-optimization/89234
>   * except.c (copy_reg_eh_region_note_forward): Return if note_or_insn
>   is a NOTE, CODE_LABEL etc. - rtx_insn * other than INSN_P.
>   (copy_reg_eh_region_note_backward): Likewise.
> 
>   * g++.dg/ubsan/pr89234.C: New test.

OK, thanks.

-- 
Eric Botcazou


[Ada] Fix tasking on SPARC/Linux

2019-02-07 Thread Eric Botcazou
The tasking is terminally broken on SPARC/Linux with the mainline compiler.
The problem is also visible on other branches if you compile the runtime with 
assertions enabled.

Tested on SPARC64/Linux, applied on all active branches.


2019-02-07  Eric Botcazou  

* libgnarl/s-linux__sparc.ads (ETIMEDOUT): Set to correct value.

-- 
Eric BotcazouIndex: libgnarl/s-linux__sparc.ads
===
--- libgnarl/s-linux__sparc.ads	(revision 268508)
+++ libgnarl/s-linux__sparc.ads	(working copy)
@@ -70,7 +70,7 @@ package System.Linux is
EINVAL: constant := 22;
ENOMEM: constant := 12;
EPERM : constant := 1;
-   ETIMEDOUT : constant := 110;
+   ETIMEDOUT : constant := 60;
 
-
-- Signals --


[rs6000] 64-bit integer loads/stores and FP instructions

2019-02-06 Thread Eric Botcazou
Hi,

as reported e.g. at https://gcc.gnu.org/ml/gcc-help/2018-11/msg00038.html, the 
7 series of compilers started to use FP instructions for simple 64-bit integer 
loads/stores in unexpected ways.  Consider:

uint64_t var;

void foo1 (uint64_t *p)
{
  var = *p;
}

void foo2 (uint64_t *p)
{
  *p = var;
}

void foo3 (void * p)
{
  var = *(uint64_t *)p;
}

void foo4 (void *p)
{
  *(uint64_t *)p = var;
}

At -O0, the 32-bit compiler uses a double lwz/stw pairs for the 4 functions.  
At -O1, it uses a lfd/stfd pair for foo2 and foo4, but neither for foo1 nor 
foo3.  At -O2, it again uses a double lwz/stw pairs for the 4 functions.

This was introduced by this change:
  https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01993.html

Now, if you try with the GCC 8 compiler, then you get back the double lwz/stw 
pairs for the 4 functions at every optimization level.

The difference between GCC 7 and GCC 8 comes from this change:
  https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00584.html
which seems to imply that the first change was problematic but which was not 
backported to the 7 branch.

So at a minimum I think that the second change should be backported to the 7 
branch; it applies (almost) cleanly and gives no regressions on PowerPC/Linux.

Backport from mainline
2018-06-11  Segher Boessenkool  

PR target/85755
* config/rs6000/rs6000.md (*movdi_internal32): Put constraint modifiers
on the correct operand.
(*movdi_internal64): Ditto.


But I also think that another part of the first change is problematic, namely 
the removal of the '*' modifier on the alternatives, which means that the 
register preference of the instruction isn't skewed towards integer registers 
any more.  That may be OK for native targets, but that is frowned upon for 
embedded targets, where people are really unhappy when the compiler emits 
floating-point instructions for pure integer code for no apparent reason.

So I suggest decoupling the (recent) native targets from the rest for this 
specific issue, see a proof of concept in the second patch.

Thoughts?

-- 
Eric Botcazou
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 8d0b58cc6d6..89812a03d74 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -8565,16 +8565,16 @@
 
 (define_insn "*movdi_internal32"
   [(set (match_operand:DI 0 "rs6000_nonimmediate_operand"
- "=Y,r, r, ^m,^d, ^d,
-  r, ^wY,   $Z,^wb,   $wv,^wi,
+ "=Y,r, r, m, ^d, ^d,
+  r, wY,Z, ^wb,   $wv,^wi,
   *wo,   *wo,   *wv,   *wi,   *wi,*wv,
   *wv")
 
 	(match_operand:DI 1 "input_operand"
-  "r,Y, r, d, m,  d,
-   IJKnGHF,  wb,wv,wY,Z,  wi,
-   Oj,   wM,OjwM,  Oj,wM, wS,
-   wB"))]
+ "r, Y, r, ^d,m,  ^d,
+  IJKnGHF,   ^wb,   $wv,   wY,Z,  ^wi,
+  Oj,wM,OjwM,  Oj,wM, wS,
+  wB"))]
 
   "! TARGET_POWERPC64
&& (gpc_reg_operand (operands[0], DImode)
@@ -8642,17 +8642,17 @@
 (define_insn "*movdi_internal64"
   [(set (match_operand:DI 0 "nonimmediate_operand"
"=YZ,   r, r, r, r,  r,
-^m,^d,^d,^wY,   $Z, $wb,
+m, ^d,^d,wY,Z,  $wb,
 $wv,   ^wi,   *wo,   *wo,   *wv,*wi,
 *wi,   *wv,   *wv,   r, *h, *h,
 ?*r,   ?*wg,  ?*r,   ?*wj")
 
 	(match_operand:DI 1 "input_operand"
-"r,YZ,r, I, L,  nF,
- d,m, d, wb,wv, wY,
- Z,wi,Oj,wM,OjwM,   Oj,
- wM,   wS,wB,*h,r,  0,
- wg,   r, wj,r"))]
+   "r, YZ,r, I, L,  nF,
+^d,m, ^d,^wb,   $wv,wY,
+Z, ^wi,   Oj,wM,OjwM,   Oj,
+wM,wS,wB,*h,r,  0,
+wg,r, wj,r"))]
 
   "TARGET_POWERPC64
&& (gpc_reg_operand (operands[0], DImode)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 7d399a2

[i386] Fix wrong argument value on Windows

2019-02-06 Thread Eric Botcazou
Hi,

this is a regression present on all active branches: if you compile the 
attached Ada testcase with -O2 -gnatp -fno-omit-frame-pointer for 32-bit 
Windows, you'll see that the compiler swaps a load based on the stack pointer 
with a store based on the frame pointer, thus clobbering a saved argument:

pushl   %ebp
movl%esp, %ebp
pushl   %esi
pushl   %ebx
pushl   %eax  <- %eax save
movl$4108, %eax
call___chkstk_ms
leal8(%ebp), %esi
subl%eax, %esp
movl8(%ebp), %ebx
movl%edx, -20(%ebp)
movl%esi, -12(%ebp)<- fp-based store
movl(%esp,%eax), %eax  <- sp-based load
... wrong value in eax...

The load and the store are swapped because there are not based on the same 
register and the offset between them is seen as variable.  The proposed fix is 
to add a memory blockage, like in other frame-related constructs.

Tested on x86/Windows and x86-64/Windows, OK for all active branches?


2019-02-06  Eric Botcazou  

* config/i386/i386.c (ix86_expand_prologue): Generate a memory blockage
after restoring registers saved to allocate the frame on Windows.


2019-02-06  Eric Botcazou  

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

-- 
Eric BotcazouIndex: config/i386/i386.c
===
--- config/i386/i386.c	(revision 268508)
+++ config/i386/i386.c	(working copy)
@@ -13579,8 +13579,9 @@ ix86_expand_prologue (void)
 	}
   m->fs.sp_offset += allocate;
 
-  /* Use stack_pointer_rtx for relative addressing so that code
-	 works for realigned stack, too.  */
+  /* Use stack_pointer_rtx for relative addressing so that code works for
+	 realigned stack.  But this means that we need a blockage to prevent
+	 stores based on the frame pointer from being scheduled before.  */
   if (r10_live && eax_live)
 {
 	  t = gen_rtx_PLUS (Pmode, stack_pointer_rtx, eax);
@@ -13589,6 +13590,7 @@ ix86_expand_prologue (void)
 	  t = plus_constant (Pmode, t, UNITS_PER_WORD);
 	  emit_move_insn (gen_rtx_REG (word_mode, AX_REG),
 			  gen_frame_mem (word_mode, t));
+	  emit_insn (gen_memory_blockage ());
 	}
   else if (eax_live || r10_live)
 	{
@@ -13596,6 +13598,7 @@ ix86_expand_prologue (void)
 	  emit_move_insn (gen_rtx_REG (word_mode,
    (eax_live ? AX_REG : R10_REG)),
 			  gen_frame_mem (word_mode, t));
+	  emit_insn (gen_memory_blockage ());
 	}
 }
   gcc_assert (m->fs.sp_offset == frame.stack_pointer_offset);
-- { dg-do run }
-- { dg-options "-O2 -gnatp -fno-omit-frame-pointer" }

procedure Opt76 is

   type Integer_Access is access Integer;
   type Registry_Array is array (Natural range <>) of Integer_Access;

   procedure Nested (Input, Parser : Integer; A, B : Boolean) is

  Index : Registry_Array (1 .. 1024);
  Not_B : constant Boolean := not B;

  procedure Inner (Input : Integer) is
  begin
 if Input /= 1 then
raise Program_Error;
 end if;

 if Parser = 128 and then A and then Not_B then
Inner (Input);
Index (Index'First) := null;
 end if;
  end;

   begin
  Inner (Input);
   end;

   Input : Integer := 1 with Volatile;
   Parser : Integer := 2 with Volatile;
  
begin
   Nested (Input, Parser, False, True);
   Nested (Input, Parser, True, False);
end;


[SPARC] Fix fallout of PR target/83368

2019-02-04 Thread Eric Botcazou
The patch for PR target/83368 changed the PIC register from a hard register to 
a pseudo and broke the -mflat mode in the process.  The attached patch fixes 
it and does also a bit of housekeeping in the GOT support.

Tested on SPARC/Solaris 11, applied on the mainline and 8 branch.


2019-02-04  Eric Botcazou  

* config/sparc/sparc.h: Remove superfluous blank lines.
* config/sparc/sparc.c (global_offset_table_rtx): Rename into...
(got_register_rtx): ...this.
(sparc_got): Adjust to above renaming.
(sparc_tls_got): Likewise.
(sparc_delegitimize_address): Likewise.
(sparc_output_mi_thunk): Likewise.
(sparc_init_pic_reg): Likewise.
(save_local_or_in_reg_p): Fix test on the GOT register.
(USE_HIDDEN_LINKONCE): Move around.
(get_pc_thunk_name): Likewise.
(gen_load_pcrel_sym): Likewise.
(load_got_register): Likewise.

-- 
Eric BotcazouIndex: config/sparc/sparc.c
===
--- config/sparc/sparc.c	(revision 268508)
+++ config/sparc/sparc.c	(working copy)
@@ -4269,19 +4269,84 @@ sparc_cannot_force_const_mem (machine_mo
 
 /* Global Offset Table support.  */
 static GTY(()) rtx got_helper_rtx = NULL_RTX;
-static GTY(()) rtx global_offset_table_rtx = NULL_RTX;
+static GTY(()) rtx got_register_rtx = NULL_RTX;
+static GTY(()) rtx got_symbol_rtx = NULL_RTX;
 
 /* Return the SYMBOL_REF for the Global Offset Table.  */
 
-static GTY(()) rtx sparc_got_symbol = NULL_RTX;
-
 static rtx
 sparc_got (void)
 {
-  if (!sparc_got_symbol)
-sparc_got_symbol = gen_rtx_SYMBOL_REF (Pmode, "_GLOBAL_OFFSET_TABLE_");
+  if (!got_symbol_rtx)
+got_symbol_rtx = gen_rtx_SYMBOL_REF (Pmode, "_GLOBAL_OFFSET_TABLE_");
+
+  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
+gen_load_pcrel_sym (rtx op0, rtx op1, rtx op2)
+{
+  int orig_flag_pic = flag_pic;
+  rtx insn;
+
+  /* The load_pcrel_sym{si,di} patterns require absolute addressing.  */
+  flag_pic = 0;
+  if (TARGET_ARCH64)
+insn = gen_load_pcrel_symdi (op0, op1, op2, GEN_INT (REGNO (op0)));
+  else
+insn = gen_load_pcrel_symsi (op0, op1, op2, GEN_INT (REGNO (op0)));
+  flag_pic = orig_flag_pic;
+
+  return insn;
+}
+
+/* Emit code to load the GOT register.  */
+
+void
+load_got_register (void)
+{
+  if (!got_register_rtx)
+got_register_rtx = gen_rtx_REG (Pmode, GLOBAL_OFFSET_TABLE_REGNUM);
+
+  if (TARGET_VXWORKS_RTP)
+emit_insn (gen_vxworks_load_got ());
+  else
+{
+  /* The GOT symbol is subject to a PC-relative relocation so we need a
+	 helper function to add the PC value and thus get the final value.  */
+  if (!got_helper_rtx)
+	{
+	  char name[32];
+	  get_pc_thunk_name (name, GLOBAL_OFFSET_TABLE_REGNUM);
+	  got_helper_rtx = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (name));
+	}
 
-  return sparc_got_symbol;
+  emit_insn (gen_load_pcrel_sym (got_register_rtx, sparc_got (),
+ got_helper_rtx));
+}
 }
 
 /* Ensure that we are not using patterns that are not OK with PIC.  */
@@ -4607,7 +4672,7 @@ sparc_tls_got (void)
   if (TARGET_SUN_TLS && TARGET_ARCH32)
 {
   load_got_register ();
-  return global_offset_table_rtx;
+  return got_register_rtx;
 }
 
   /* In all other cases, we load a new pseudo with the GOT symbol.  */
@@ -4995,7 +5060,7 @@ sparc_delegitimize_address (rtx x)
 
   /* This is generated by mov{si,di}_pic_label_ref in PIC mode.  */
   if (GET_CODE (x) == MINUS
-  && (XEXP (x, 0) == global_offset_table_rtx
+  && (XEXP (x, 0) == got_register_rtx
 	  || sparc_pic_register_p (XEXP (x, 0
 {
   rtx y = XEXP (x, 1);
@@ -5092,72 +5157,6 @@ sparc_mode_dependent_address_p (const_rt
   return false;
 }
 
-#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
-gen_load_pcrel_sym (rtx op0, rtx op1, rtx op2)
-{
-  int orig_flag_pic = flag_pic;

Re: [PATCH] Move stack protector epilogue before loading return hard reg(s) from pseudo(s) (PR rtl-optimization/87485)

2019-02-01 Thread Eric Botcazou
> So, can we e.g. keep emitting the epilogue where it is now for
> naked_return_label != NULL_RTX and move it otherwise?
> For __builtin_return the setter and use of the hard register won't be
> adjacent in any case.

See my comment in the audit trail of the PR; I'd suspend it and go to bed. ;-)

-- 
Eric Botcazou


Re: [PATCH] Move stack protector epilogue before loading return hard reg(s) from pseudo(s) (PR rtl-optimization/87485)

2019-02-01 Thread Eric Botcazou
> As discussed in the PR and suggested by Uros, scheduler has code to keep a
> use of hard register next to the assignment that sets that hard register
> from a pseudo, which is desirable so that RA can deal with it properly.
> Unfortunately, with -fstack-protector* we stick the stack protect epilogue
> in between, which splits the load and use to different basic blocks.
> The code emitted by expand_function_end between these two spots is only the
> loading of the return value into registers, so generally it shouldn't
> contain any stores which stack protection wants to guard against, so I
> believe from security POV this shouldn't weaken anything, but fixes the
> testcase.

This moves the stack protect epilogue from after the naked_return_label to 
before though, so it will be skipped for a naked return.

-- 
Eric Botcazou


[Ada] Fix run time inefficiency with dynamic discriminated record types

2019-01-27 Thread Eric Botcazou
This isn't really a regression but a long standing issue related to the run 
time performance of a class of dynamic discriminated record types.  For them, 
the size and offset calculations contain artificial MIN expressions that are 
introduced to compute maximum sizes at compile time but nevertheless end up in 
the generated code.  Moreover, in some cases, these calculations cannot be 
hoisted out of loops because of fake aliasing conflicts for the discriminant.

The attached patch is aimed at addressing both issues.  We have been using it 
for some time now and is IMO worth having in GCC 9 despite the small risk.

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


2019-01-27  Eric Botcazou  

* repinfo.adb (List_Component_Layout): Remove superfluous space for
zero-sized field.
* gcc-interface/ada-tree.h (TYPE_IS_EXTRA_SUBTYPE_P): New macro.
* gcc-interface/gigi.h (create_extra_subtype): Declare.
* gcc-interface/decl.c (TYPE_ARRAY_SIZE_LIMIT): Likewise.
(update_n_elem): New function.
(gnat_to_gnu_entity): Use create_extra_subtype to create extra subtypes
instead of doing it manually.
: Use update_n_elem to compute the maximum size.  Use the
index type instead of base type for the bounds.  Set TYPE_ARRAY_MAX_SIZE
of the array to the maximum size.
: Create an extra subtype using the index type of the
base array type for self-referential bounds.  Use update_n_elem to
compute the maximum size. Set TYPE_ARRAY_MAX_SIZE of the array to the
maximum size.
(gnat_to_gnu_field): Clear DECL_NONADDRESSABLE_P on discriminants.
* gcc-interface/misc.c (gnat_get_alias_set): Return the alias set of
the base type for an extra subtype.
(gnat_type_max_size): Remove obsolete code.
* gcc-interface/trans.c (Attribute_to_gnu): Minor tweak.
(can_be_lower_p): Deal with pathological types.
* gcc-interface/utils.c (create_extra_subtype): New function.
(create_field_decl): Minor tweak.
(max_size) : Compute a better value by using the extra
subtypes on the self-referential bounds.
: Rewrite.  Deal with "negative value" in unsigned types.
: Likewise.
* gcc-interface/utils2.c (compare_arrays): Retrieve the original bounds
of the arrays upfront.  Swap only if the second length is not constant.
Use comparisons on the original bounds consistently for the null tests.
(build_binary_op): Use TYPE_IS_EXTRA_SUBTYPE_P macro.
(build_allocator): Minor tweak.

-- 
Eric BotcazouIndex: gcc-interface/ada-tree.h
===
--- gcc-interface/ada-tree.h	(revision 268309)
+++ gcc-interface/ada-tree.h	(working copy)
@@ -111,6 +111,9 @@ do {			 \
front-end.  */
 #define TYPE_EXTRA_SUBTYPE_P(NODE) TYPE_LANG_FLAG_2 (INTEGER_TYPE_CHECK (NODE))
 
+#define TYPE_IS_EXTRA_SUBTYPE_P(NODE) \
+  (TREE_CODE (NODE) == INTEGER_TYPE && TYPE_EXTRA_SUBTYPE_P (NODE))
+
 /* Nonzero for an aggregate type if this is a by-reference type.  We also
set this on an ENUMERAL_TYPE that is dummy.  */
 #define TYPE_BY_REFERENCE_P(NODE)   \
Index: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 268314)
+++ gcc-interface/decl.c	(working copy)
@@ -85,6 +85,12 @@
 #define FOREIGN_FORCE_REALIGN_STACK 0
 #endif
 
+/* The largest TYPE_ARRAY_MAX_SIZE value we set on an array type.
+   It's an artibrary limit (256 MB) above which we consider that
+   the allocation is essentially unbounded.  */
+
+#define TYPE_ARRAY_SIZE_LIMIT (1 << 28)
+
 struct incomplete
 {
   struct incomplete *next;
@@ -216,6 +222,7 @@ static bool cannot_be_superflat (Node_Id
 static bool constructor_address_p (tree);
 static bool allocatable_size_p (tree, bool);
 static bool initial_value_needs_conversion (tree, tree);
+static tree update_n_elem (tree, tree, tree);
 static int compare_field_bitpos (const PTR, const PTR);
 static bool components_to_record (Node_Id, Entity_Id, tree, tree, int, bool,
   bool, bool, bool, bool, bool, bool, tree,
@@ -1760,12 +1767,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	if (gnu_high
 	&& !tree_int_cst_equal (gnu_high, TYPE_MAX_VALUE (gnu_type)))
 	  {
-	tree gnu_subtype = make_unsigned_type (esize);
-	SET_TYPE_RM_MAX_VALUE (gnu_subtype, gnu_high);
-	TREE_TYPE (gnu_subtype) = gnu_type;
-	TYPE_EXTRA_SUBTYPE_P (gnu_subtype) = 1;
 	TYPE_NAME (gnu_type) = create_concat_name (gnat_entity, "UMT");
-	gnu_type = gnu_subtype;
+	gnu_type
+	  = create_extra_subtype (gnu_type, TYPE_MIN_VALUE (gnu_type),
+  gnu_high);
 	  }
   }
   goto discrete_type;
@@ -2052,7 +2057,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
 	tree gnu_template_reference, gnu_template_fields, gnu_fat_type;
 	tree *gnu_ind

<    4   5   6   7   8   9   10   11   12   13   >