[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] 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_of_record_type_compilation (tree re
 /* Utility function of above to

[Ada] Fix compiler crash on function returning array at -O

2019-05-28 Thread Eric Botcazou
This is a regression present on all active branches.  The compiler segfaults 
on a function returning an array variable declared locally and used in a 
grandchild function but not in its parent function (which is a child of the 
first function).

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


2019-05-28  Eric Botcazou  

* gcc-interface/trans.c (walk_nesting_tree): New static function.
(finalize_nrv): Use it to walk the entire nesting tree.


2019-05-28  Eric Botcazou  

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

-- 
Eric Botcazoupackage Opt79 is

  type Arr is array (1 .. 8) of Integer;

  function F (I : Integer) return Arr;

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

package body Opt79 is

  function F (I : Integer) return Arr is
A : Arr;

procedure Nested is

  procedure Inner is
  begin
A (1) := 0;
  end;

begin
   Inner;
end;

  begin
Nested;
for J in A'Range loop
  A (J) := I;
end loop;
return A;
  end;

end Opt79;
Index: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271659)
+++ gcc-interface/trans.c	(working copy)
@@ -4280,6 +4280,20 @@ finalize_nrv_unc_r (tree *tp, int *walk_
   return NULL_TREE;
 }
 
+/* Apply FUNC to all the sub-trees of nested functions in NODE.  FUNC is called
+   with the DATA and the address of each sub-tree.  If FUNC returns a non-NULL
+   value, the traversal is stopped.  */
+
+static void
+walk_nesting_tree (struct cgraph_node *node, walk_tree_fn func, void *data)
+{
+  for (node = node->nested; node; node = node->next_nested)
+{
+  walk_tree_without_duplicates (&DECL_SAVED_TREE (node->decl), func, data);
+  walk_nesting_tree (node, func, data);
+}
+}
+
 /* Finalize the Named Return Value optimization for FNDECL.  The NRV bitmap
contains the candidates for Named Return Value and OTHER is a list of
the other return values.  GNAT_RET is a representative return node.  */
@@ -4287,7 +4301,6 @@ finalize_nrv_unc_r (tree *tp, int *walk_
 static void
 finalize_nrv (tree fndecl, bitmap nrv, vec *other, Node_Id gnat_ret)
 {
-  struct cgraph_node *node;
   struct nrv_data data;
   walk_tree_fn func;
   unsigned int i;
@@ -4308,10 +4321,7 @@ finalize_nrv (tree fndecl, bitmap nrv, v
 return;
 
   /* Prune also the candidates that are referenced by nested functions.  */
-  node = cgraph_node::get_create (fndecl);
-  for (node = node->nested; node; node = node->next_nested)
-walk_tree_without_duplicates (&DECL_SAVED_TREE (node->decl), prune_nrv_r,
-  &data);
+  walk_nesting_tree (cgraph_node::get_create (fndecl), prune_nrv_r, &data);
   if (bitmap_empty_p (nrv))
 return;
 


[Ada] Fix minor issue with unconstrained packed arrays

2019-05-28 Thread Eric Botcazou
The compiler allocates the non-packed size for a function returning a 
parameter of an unconstrained packed array type.

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


2019-05-28  Eric Botcazou  

* gcc-interface/trans.c (lvalue_required_for_attribute_p): Return 0
for 'Size too.
(Identifier_to_gnu): Use the actual subtype for a reference to a
packed array in a return statement.
(Attribute_to_gnu) : Do not strip VIEW_CONVERT_EXPRs from
the prefix in every case.

-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 271690)
+++ gcc-interface/trans.c	(revision 271691)
@@ -778,6 +778,7 @@ lvalue_required_for_attribute_p (Node_Id
 case Attr_Range_Length:
 case Attr_Length:
 case Attr_Object_Size:
+case Attr_Size:
 case Attr_Value_Size:
 case Attr_Component_Size:
 case Attr_Descriptor_Size:
@@ -797,7 +798,6 @@ lvalue_required_for_attribute_p (Node_Id
 case Attr_Unrestricted_Access:
 case Attr_Code_Address:
 case Attr_Pool_Address:
-case Attr_Size:
 case Attr_Alignment:
 case Attr_Bit_Position:
 case Attr_Position:
@@ -1112,12 +1112,15 @@ Identifier_to_gnu (Node_Id gnat_node, tr
 {
   /* 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.  */
+	 the Etype.  Avoid using it for packed arrays to simplify things,
+	 except in a return statement because we need the actual size and
+	 the front-end does not make it explicit in this case.  */
   if ((Ekind (gnat_entity) == E_Constant
 	   || Ekind (gnat_entity) == E_Variable
 	   || Is_Formal (gnat_entity))
 	  && !(Is_Array_Type (Etype (gnat_entity))
-	   && Present (Packed_Array_Impl_Type (Etype (gnat_entity
+	   && Present (Packed_Array_Impl_Type (Etype (gnat_entity)))
+	   && Nkind (Parent (gnat_node)) != N_Simple_Return_Statement)
 	  && Present (Actual_Subtype (gnat_entity))
 	  && present_gnu_tree (Actual_Subtype (gnat_entity)))
 	gnat_result_type = Actual_Subtype (gnat_entity);
@@ -2314,21 +2317,24 @@ Attribute_to_gnu (Node_Id gnat_node, tre
 case Attr_Object_Size:
 case Attr_Value_Size:
 case Attr_Max_Size_In_Storage_Elements:
-  gnu_expr = gnu_prefix;
-
-  /* Remove NOPs and conversions between original and packable version
-	 from GNU_EXPR, and conversions from GNU_PREFIX.  We use GNU_EXPR
-	 to see if a COMPONENT_REF was involved.  */
-  while (TREE_CODE (gnu_expr) == NOP_EXPR
-	 || (TREE_CODE (gnu_expr) == VIEW_CONVERT_EXPR
-		 && TREE_CODE (TREE_TYPE (gnu_expr)) == RECORD_TYPE
-		 && TREE_CODE (TREE_TYPE (TREE_OPERAND (gnu_expr, 0)))
+  /* Strip NOPs, conversions between original and packable versions, and
+	 unpadding from GNU_PREFIX.  Note that we cannot simply strip every
+	 VIEW_CONVERT_EXPR because some of them may give the actual size, e.g.
+	 for nominally unconstrained packed array.  We use GNU_EXPR to see
+	 if a COMPONENT_REF was involved.  */
+  while (CONVERT_EXPR_P (gnu_prefix)
+	 || TREE_CODE (gnu_prefix) == NON_LVALUE_EXPR
+	 || (TREE_CODE (gnu_prefix) == VIEW_CONVERT_EXPR
+		 && TREE_CODE (TREE_TYPE (gnu_prefix)) == RECORD_TYPE
+		 && TREE_CODE (TREE_TYPE (TREE_OPERAND (gnu_prefix, 0)))
 		== RECORD_TYPE
-		 && TYPE_NAME (TREE_TYPE (gnu_expr))
-		== TYPE_NAME (TREE_TYPE (TREE_OPERAND (gnu_expr, 0)
-	gnu_expr = TREE_OPERAND (gnu_expr, 0);
-
-  gnu_prefix = remove_conversions (gnu_prefix, true);
+		 && TYPE_NAME (TREE_TYPE (gnu_prefix))
+		== TYPE_NAME (TREE_TYPE (TREE_OPERAND (gnu_prefix, 0)
+	gnu_prefix = TREE_OPERAND (gnu_prefix, 0);
+  gnu_expr = gnu_prefix;
+  if (TREE_CODE (gnu_prefix) == COMPONENT_REF
+	  && TYPE_IS_PADDING_P (TREE_TYPE (TREE_OPERAND (gnu_prefix, 0
+	gnu_prefix = TREE_OPERAND (gnu_prefix, 0);
   prefix_unused = true;
   gnu_type = TREE_TYPE (gnu_prefix);
 
@@ -2391,7 +2397,7 @@ Attribute_to_gnu (Node_Id gnat_node, tre
   /* Deal with a self-referential size by qualifying the size with the
 	 object or returning the maximum size for a type.  */
   if (TREE_CODE (gnu_prefix) != TYPE_DECL)
-	gnu_result = SUBSTITUTE_PLACEHOLDER_IN_EXPR (gnu_result, gnu_expr);
+	gnu_result = SUBSTITUTE_PLACEHOLDER_IN_EXPR (gnu_result, gnu_prefix);
   else if (CONTAINS_PLACEHOLDER_P (gnu_result))
 	gnu_result = max_size (gnu_result, true);
 


[Ada] Add support for selected generic function attributes in Ada

2019-05-28 Thread Eric Botcazou
This adds support for the following function attributes in Ada: no_icf, noipa,
flatten, used, cold, hot, target and target_clones.  They are supported by
means of the pragma Machine_Attribute, whose syntax is extended to accept
more than one optional parameter for the latter attribute.

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


2019-05-28  Eric Botcazou  

* doc/gnat_rm/implementation_defined_pragmas.rst (Machine_Attribute):
Document additional optional parameters.
* sem_prag.adb (Analyze_Pragma) : Accept
more than one optional parameter.
* gcc-interface/decl.c (prepend_one_attribute_pragma): Alphabetize
the list of supported pragmas.  Simplify the handling of parameters
and add support for more than one optional parameter.
* gcc-interface/utils.c (attr_cold_hot_exclusions): New constant.
(gnat_internal_attribute_table): Add entry for no_icf, noipa, flatten,
used, cold, hot, target and target_clones.
(begin_subprog_body): Do not create the RTL for the subprogram here.
(handle_noicf_attribute): New static function.
(handle_noipa_attribute): Likewise.
(handle_flatten_attribute): Likewise.
(handle_used_attribute): Likewise.
(handle_cold_attribute): Likewise.
(handle_hot_attribute): Likewise.
(handle_target_attribute): Likewise.
(handle_target_clones_attribute): Likewise.


2019-05-28  Eric Botcazou  

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

-- 
Eric BotcazouIndex: doc/gnat_rm/implementation_defined_pragmas.rst
===
--- doc/gnat_rm/implementation_defined_pragmas.rst	(revision 271528)
+++ doc/gnat_rm/implementation_defined_pragmas.rst	(working copy)
@@ -3766,18 +3766,19 @@ Syntax:
   pragma Machine_Attribute (
[Entity =>] LOCAL_NAME,
[Attribute_Name =>] static_string_EXPRESSION
-[, [Info   =>] static_EXPRESSION] );
+[, [Info   =>] static_EXPRESSION {, static_EXPRESSION}] );
 
 
 Machine-dependent attributes can be specified for types and/or
 declarations.  This pragma is semantically equivalent to
 :samp:`__attribute__(({attribute_name}))` (if ``info`` is not
 specified) or :samp:`__attribute__(({attribute_name(info})))`
-in GNU C, where *attribute_name* is recognized by the
-compiler middle-end or the ``TARGET_ATTRIBUTE_TABLE`` machine
-specific macro.  A string literal for the optional parameter ``info``
-is transformed into an identifier, which may make this pragma unusable
-for some attributes.
+or :samp:`__attribute__(({attribute_name(info,...})))` in GNU C,
+where *attribute_name* is recognized by the compiler middle-end
+or the ``TARGET_ATTRIBUTE_TABLE`` machine specific macro.  Note
+that a string literal for the optional parameter ``info`` or the
+following ones is transformed by default into an identifier,
+which may make this pragma unusable for some attributes.
 For further information see :title:`GNU Compiler Collection (GCC) Internals`.
 
 Pragma Main
Index: gcc-interface/decl.c
===
--- gcc-interface/decl.c	(revision 271683)
+++ gcc-interface/decl.c	(working copy)
@@ -6458,25 +6458,18 @@ prepend_one_attribute (struct attrib **a
 static void
 prepend_one_attribute_pragma (struct attrib **attr_list, Node_Id gnat_pragma)
 {
-  const Node_Id gnat_arg = Pragma_Argument_Associations (gnat_pragma);
-  tree gnu_arg0 = NULL_TREE, gnu_arg1 = NULL_TREE;
+  const Node_Id gnat_arg = First (Pragma_Argument_Associations (gnat_pragma));
+  Node_Id gnat_next_arg = Next (gnat_arg);
+  tree gnu_arg1 = NULL_TREE, gnu_arg_list = NULL_TREE;
   enum attrib_type etype;
 
   /* Map the pragma at hand.  Skip if this isn't one we know how to handle.  */
   switch (Get_Pragma_Id (Chars (Pragma_Identifier (gnat_pragma
 {
-case Pragma_Machine_Attribute:
-  etype = ATTR_MACHINE_ATTRIBUTE;
-  break;
-
 case Pragma_Linker_Alias:
   etype = ATTR_LINK_ALIAS;
   break;
 
-case Pragma_Linker_Section:
-  etype = ATTR_LINK_SECTION;
-  break;
-
 case Pragma_Linker_Constructor:
   etype = ATTR_LINK_CONSTRUCTOR;
   break;
@@ -6485,58 +6478,58 @@ prepend_one_attribute_pragma (struct att
   etype = ATTR_LINK_DESTRUCTOR;
   break;
 
-case Pragma_Weak_External:
-  etype = ATTR_WEAK_EXTERNAL;
+case Pragma_Linker_Section:
+  etype = ATTR_LINK_SECTION;
+  break;
+
+case Pragma_Machine_Attribute:
+  etype = ATTR_MACHINE_ATTRIBUTE;
   break;
 
 case Pragma_Thread_Local_Storage:
   etype = ATTR_THREAD_LOCAL_STORAGE;
   break;
 
+case Pragma_Weak_External:
+  etype = ATTR_WEAK_EXTERNAL;
+  break;
+
 default:
   return;
 }
 
   /* See what arguments we have and turn them into GCC trees for attribute
- handlers.  These expect identifier for strings.  We handle at most two

Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread Jakub Jelinek
On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > if its address is taken.  Also ix86_init_large_pic_reg shouldn't set
> > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > to keep the label.

Can you explain when is it ever needed to emit ENDBR on
NOTE_INSN_DELETED_LABEL?  Only labels that are proven not to be ever
branched to are turned into deleted labels, so I think the right answer is
never emit any ENDBR on those...

Jakub


Re: Make possible 'scan-tree-dump' of 'lower_omp_target' mapping kinds

2019-05-28 Thread Jakub Jelinek
On Mon, May 27, 2019 at 11:33:00PM +0200, Thomas Schwinge wrote:
> Why exactly are you objecting that the compiler produces useful
> intermediate dumps, understandable my machine and human?

While still not convinced we need this, if we add it, you are still
printing a lot of stuff that is useless to humans and makes it harder to
read, plus you unnecessarily print it in two spots rather than just one.

Printing Mapping on each line just adds clutter, just print the target
stmt with TDF_LINENO followed by "Mappings:\n"
once with the location string, replace the break; at the end of
OMP_CLAUSE_FIRSTPRIVATE: handling with nc = c; tkind_zero = tkind; goto
label; which will be right before if (nc && nc != c) and do the printing
there.  print_generic_expr the clause (c), the name of function seems
completely useless to me (and trying to match it in scan-tree-dump when it
can change any time doesn't make sense), TREE_PURPOSE doesn't seem to be
much useful either, print s expression (size), if tkind == tkind_zero,
just print one number (tkind & ((HOST_WIDE_INT_1U << talign_shift) - 1),
otherwise print two numbers extracted similar way from also tkind_zero,
finally the alignment.

Jakub


[PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread David CARLIER
Hi,

Here a tiny patch to fix few build warnings.

Kind regards.
Index: gcc/ChangeLog
===
--- gcc/ChangeLog	(revision 271684)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2019-05-28  David Carlier 
+
+* coretypes.h: Fix build warning, chaing few classes to struct.
+
 2019-05-27  Jakub Jelinek  
 
 	* gimplify.c (gimplify_scan_omp_clauses): Allow lastprivate conditional
Index: gcc/coretypes.h
===
--- gcc/coretypes.h	(revision 271684)
+++ gcc/coretypes.h	(working copy)
@@ -65,7 +65,7 @@ template class opt_mode;
 typedef opt_mode opt_scalar_mode;
 typedef opt_mode opt_scalar_int_mode;
 typedef opt_mode opt_scalar_float_mode;
-template class pod_mode;
+template struct pod_mode;
 typedef pod_mode scalar_mode_pod;
 typedef pod_mode scalar_int_mode_pod;
 typedef pod_mode fixed_size_mode_pod;
@@ -73,7 +73,7 @@ typedef pod_mode fixed_
 /* Subclasses of rtx_def, using indentation to show the class
hierarchy, along with the relevant invariant.
Where possible, keep this list in the same order as in rtl.def.  */
-class rtx_def;
+struct rtx_def;
   class rtx_expr_list;   /* GET_CODE (X) == EXPR_LIST */
   class rtx_insn_list;   /* GET_CODE (X) == INSN_LIST */
   class rtx_sequence;/* GET_CODE (X) == SEQUENCE */
@@ -138,7 +138,7 @@ struct gomp_teams;
 /* Subclasses of symtab_node, using indentation to show the class
hierarchy.  */
 
-class symtab_node;
+struct symtab_node;
   struct cgraph_node;
   class varpool_node;
 
Index: gcc/hash-table.h
===
--- gcc/hash-table.h	(revision 271684)
+++ gcc/hash-table.h	(working copy)
@@ -347,7 +347,7 @@ hash_table_mod2 (hashval_t hash, unsigne
   return 1 + mul_mod (hash, p->prime - 2, p->inv_m2, p->shift);
 }
 
-class mem_usage;
+struct mem_usage;
 
 /* User-facing hash table type.
 


Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread Segher Boessenkool
On Tue, May 28, 2019 at 09:31:18AM +, David CARLIER wrote:
> Here a tiny patch to fix few build warnings.

Please mention what the warning _is_, and why it is correct / why it is
a good idea to make these changes.


Segher


Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread Martin Liška
On 5/28/19 11:31 AM, David CARLIER wrote:
> Hi,
> 
> Here a tiny patch to fix few build warnings.
> 
> Kind regards.
> 

Hi.

Well, I see a lot of these struct/class discrepancies when building GCC with 
LLVM.
Question is whether it worth changing?

Martin


Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 12:24:12PM +0200, Martin Liška wrote:
> Well, I see a lot of these struct/class discrepancies when building GCC with 
> LLVM.
> Question is whether it worth changing?

No, the warning is just wrong.  C++ clearly states what the struct and class
keyword mean and it is just fine to mix them between declarations and
definitions.

Jakub


Fwd: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread David CARLIER
-- Forwarded message -
From: David CARLIER 
Date: Tue, 28 May 2019 at 10:16
Subject: Re: [PATCH] Fix few build warnings with LLVM toolchain
To: Segher Boessenkool 


All right, here an updated version, hope it looks better.

Thanks.

On Tue, 28 May 2019 at 10:09, Segher Boessenkool
 wrote:
>
> On Tue, May 28, 2019 at 09:31:18AM +, David CARLIER wrote:
> > Here a tiny patch to fix few build warnings.
>
> Please mention what the warning _is_, and why it is correct / why it is
> a good idea to make these changes.
>
>
> Segher
Fixing few build warnings with clang/clang++ of this type:
../.././gcc/coretypes.h:76:1: warning: class 'rtx_def' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
or
../.././gcc/machmode.h:320:1: warning: 'pod_mode' defined as a struct template here but previously declared as a class template; this is valid, but may result in linker errors under the
  Microsoft C++ ABI [-Wmismatched-tags]

The struct/class mismatch is mostly harmless, might be for Microsoft toolchain as mentioned above, but in general for correctness.

Index: gcc/ChangeLog
===
--- gcc/ChangeLog	(revision 271684)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2019-05-28  David Carlier 
+
+* coretypes.h: Fix build warning, chaing few classes to struct.
+
 2019-05-27  Jakub Jelinek  
 
 	* gimplify.c (gimplify_scan_omp_clauses): Allow lastprivate conditional
Index: gcc/coretypes.h
===
--- gcc/coretypes.h	(revision 271684)
+++ gcc/coretypes.h	(working copy)
@@ -65,7 +65,7 @@ template class opt_mode;
 typedef opt_mode opt_scalar_mode;
 typedef opt_mode opt_scalar_int_mode;
 typedef opt_mode opt_scalar_float_mode;
-template class pod_mode;
+template struct pod_mode;
 typedef pod_mode scalar_mode_pod;
 typedef pod_mode scalar_int_mode_pod;
 typedef pod_mode fixed_size_mode_pod;
@@ -73,7 +73,7 @@ typedef pod_mode fixed_
 /* Subclasses of rtx_def, using indentation to show the class
hierarchy, along with the relevant invariant.
Where possible, keep this list in the same order as in rtl.def.  */
-class rtx_def;
+struct rtx_def;
   class rtx_expr_list;   /* GET_CODE (X) == EXPR_LIST */
   class rtx_insn_list;   /* GET_CODE (X) == INSN_LIST */
   class rtx_sequence;/* GET_CODE (X) == SEQUENCE */
@@ -138,7 +138,7 @@ struct gomp_teams;
 /* Subclasses of symtab_node, using indentation to show the class
hierarchy.  */
 
-class symtab_node;
+struct symtab_node;
   struct cgraph_node;
   class varpool_node;
 
Index: gcc/hash-table.h
===
--- gcc/hash-table.h	(revision 271684)
+++ gcc/hash-table.h	(working copy)
@@ -347,7 +347,7 @@ hash_table_mod2 (hashval_t hash, unsigne
   return 1 + mul_mod (hash, p->prime - 2, p->inv_m2, p->shift);
 }
 
-class mem_usage;
+struct mem_usage;
 
 /* User-facing hash table type.
 


Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git

2019-05-28 Thread Maxim Kuvyrkov
Hi Everyone,

What can I say, I was too optimistic about how easy it would be to convert 
GCC's svn repo to git one branch at a time.  After 2 more weeks and several 
re-writes of the scripts I now know more about GCC's svn history than I would 
ever wanted.

The prize for most complicated branch history goes to /branches/ibm/* .  It has 
merges, it has re-creation branches from /trunk and even an accidental deletion 
of all of IBM's branches.

The version of scripts I'm testing right now seems to deal with all of that.

Also, to avoid controversy -- I'm working on these scripts to satisfy my own 
curiosity, and to give GCC community another option to choose from for the 
final migration.  If by end of Summer 2019 we have 2-3 git repos to choose 
from, then we are likely to push GCC [kicking and screaming] into 2010's by the 
end of this decade.

--
Maxim Kuvyrkov
www.linaro.org



> On May 14, 2019, at 7:11 PM, Maxim Kuvyrkov  wrote:
> 
> This patch adds scripts to contrib/ to migrate full history of GCC's 
> subversion repository to git.  My hope is that these scripts will finally 
> allow GCC project to migrate to Git.
> 
> The result of the conversion is at 
> https://github.com/maxim-kuvyrkov/gcc/branches/all .  Branches with "@rev" 
> suffixes represent branch points.  The conversion is still running, so not 
> all branches may appear right away.
> 
> The scripts are not specific to GCC repo and are usable for other projects.  
> In particular, they should be able to convert downstream GCC svn repos.
> 
> The scripts convert svn history branch by branch.  They rely on git-svn on 
> convert individual branches.  Git-svn is a good tool for converting 
> individual branches.  It is, however, either very slow at converting the 
> entire GCC repo, or goes into infinite loop.
> 
> There are 3 scripts:
> 
> - svn-git-repo.sh: top level script to convert entire repo or a part of it 
> (e.g., branches/),
> - svn-list-branches.sh: helper script to output branches and their parents in 
> bottom-up order,
> - svn-git-branch.sh: helper script to convert a single branch.
> 
> Whenever possible, svn-git-branch.sh uses existing git branches as caches.
> 
> What are your questions and comments?
> 
> The attached is cleaned up version, which hasn't been fully tested yet; typos 
> and other silly mistakes are likely.  OK to commit after testing?
> 
> --
> Maxim Kuvyrkov
> www.linaro.org
> 
> 
> <0001-Contrib-SVN-Git-conversion-scripts.patch>



Re: [RFC][PR88838][SVE] Use 32-bit WHILELO in LP64 mode

2019-05-28 Thread Richard Sandiford
Kugan Vivekanandarajah  writes:
> [...]
> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
> index b3fae5b..c15b8a2 100644
> --- a/gcc/tree-vect-loop-manip.c
> +++ b/gcc/tree-vect-loop-manip.c
> @@ -415,10 +415,16 @@ vect_set_loop_masks_directly (struct loop *loop, 
> loop_vec_info loop_vinfo,
> bool might_wrap_p)
>  {
>tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo);
> +  tree iv_type = LOOP_VINFO_MASK_IV_TYPE (loop_vinfo);
>tree mask_type = rgm->mask_type;
>unsigned int nscalars_per_iter = rgm->max_nscalars_per_iter;
>poly_uint64 nscalars_per_mask = TYPE_VECTOR_SUBPARTS (mask_type);
> +  bool convert = false;
>  
> +  /* If the compare_type is not iv_type, we will create an IV with
> + iv_type with truncated use (i.e. converted to the correct type).  */
> +  if (compare_type != iv_type)
> +convert = true;
>/* Calculate the maximum number of scalar values that the rgroup
>   handles in total, the number that it handles for each iteration
>   of the vector loop, and the number that it should skip during the
> @@ -444,12 +450,43 @@ vect_set_loop_masks_directly (struct loop *loop, 
> loop_vec_info loop_vinfo,
>   processed.  */
>tree index_before_incr, index_after_incr;
>gimple_stmt_iterator incr_gsi;
> +  gimple_stmt_iterator incr_gsi2;
>bool insert_after;
> -  tree zero_index = build_int_cst (compare_type, 0);
> +  tree zero_index;
>standard_iv_increment_position (loop, &incr_gsi, &insert_after);
> -  create_iv (zero_index, nscalars_step, NULL_TREE, loop, &incr_gsi,
> -  insert_after, &index_before_incr, &index_after_incr);
>  
> +  if (convert)
> +{
> +  /* If we are creating IV of iv_type and then converting.  */
> +  zero_index = build_int_cst (iv_type, 0);
> +  tree step = build_int_cst (iv_type,
> +  LOOP_VINFO_VECT_FACTOR (loop_vinfo));
> +  /* Creating IV of iv_type.  */
> +  create_iv (zero_index, step, NULL_TREE, loop, &incr_gsi,
> +  insert_after, &index_before_incr, &index_after_incr);
> +  /* Create truncated index_before and after increament.  */
> +  tree index_before_incr_trunc = make_ssa_name (compare_type);
> +  tree index_after_incr_trunc = make_ssa_name (compare_type);
> +  gimple *incr_before_stmt = gimple_build_assign 
> (index_before_incr_trunc,
> +   NOP_EXPR,
> +   index_before_incr);
> +  gimple *incr_after_stmt = gimple_build_assign (index_after_incr_trunc,
> +  NOP_EXPR,
> +  index_after_incr);
> +  incr_gsi2 = incr_gsi;
> +  gsi_insert_before (&incr_gsi2, incr_before_stmt, GSI_NEW_STMT);
> +  gsi_insert_after (&incr_gsi, incr_after_stmt, GSI_NEW_STMT);
> +  index_before_incr = index_before_incr_trunc;
> +  index_after_incr = index_after_incr_trunc;
> +  zero_index = build_int_cst (compare_type, 0);
> +}
> +  else
> +{
> +  /* If the IV is of compare_type, no convertion needed.  */
> +  zero_index = build_int_cst (compare_type, 0);
> +  create_iv (zero_index, nscalars_step, NULL_TREE, loop, &incr_gsi,
> +  insert_after, &index_before_incr, &index_after_incr);
> +}
>tree test_index, test_limit, first_limit;
>gimple_stmt_iterator *test_gsi;
>if (might_wrap_p)

Now that we have an explicit iv_type, there shouldn't be any need to
treat this as two special cases.  I think we should just convert the
IV to the comparison type before passing it to the WHILE.

> @@ -617,6 +654,41 @@ vect_set_loop_masks_directly (struct loop *loop, 
> loop_vec_info loop_vinfo,
>return next_mask;
>  }
>  
> +/* Return the iv_limit for fully masked loop LOOP with LOOP_VINFO.
> +   If it is not possible to calcilate iv_limit, return -1.  */

Maybe:

/* Decide whether it is possible to use a zero-based induction variable
   when vectorizing LOOP_VINFO with a fully-masked loop.  If it is,
   return the value that the induction variable must be able to hold
   in order to ensure that the loop ends with an all-false mask.
   Return -1 otherwise.  */

I think the function should go on in tree-vect-loop.c instead.

> +widest_int
> +vect_get_loop_iv_limit (struct loop *loop, loop_vec_info loop_vinfo)

Maybe: vect_iv_limit_for_full_masking

Probably worth dropping the "loop" parameter and getting it from
LOOP_VINFO.

> +{
> +  /* Convert skip_niters to the right type.  */

Comment no longer applies.

> +  tree niters_skip = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);
> +  unsigned HOST_WIDE_INT max_vf = vect_max_vf (loop_vinfo);
> +
> +  /* Now calculate the value that the induction variable must be able
> + to hit in order to ensure that we end the loop with an all-false mask.
> + This involves adding the maximum number of inactive trailing 

Re: Remove obsolete comment about use_thunk

2019-05-28 Thread Richard Sandiford
Rainer Orth  writes:
> When reviewing how this patch
>
>   [PATCH] gcc: move assemble_start_function / assemble_end_function to 
> output_mi_thunk
>   https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00722.html
>
> might affect Solaris, I noticed an obsolete comment
>
>  Note that use_thunk calls assemble_start_function et al.  */
>
> that in one form or another is present in several targets, but has been
> wrong for almost 10 years when r154736 ripped out those calls to
> assemble_*_function in use_thunk.
>
> The following patch corrects this.  Bootstrapped on i386-pc-solaris2.11
> and sparc-sun-solaris2.11; I didn't bother to try and build cc1 on all
> other affected targets, though.
>
> Ok for mainline?  I guess this is obvious.
>
>   Rainer

OK, thanks.

Richard


Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Martin Liška
On 5/27/19 2:50 PM, Jakub Jelinek wrote:
> On Mon, May 27, 2019 at 02:18:37PM +0200, Martin Liška wrote:
>> +/* Compare loop information for basic blocks BB1 and BB2.  */
>> +
>> +bool
>> +func_checker::compare_loops (basic_block bb1, basic_block bb2)
>> +{
>> +  if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
>> +return return_false ();
>> +
>> +  struct loop *l1 = bb1->loop_father;
>> +  struct loop *l2 = bb2->loop_father;
> 
> Perhaps also
>   if ((bb1 == l1->header) != (bb2 == l2->header))
> return return_false_with_msg ("header");
>   if ((bb1 == l1->latch) != (bb2 == l2->latch))
> return return_false_with_msg ("latch");
> too?

Yes, makes sense.

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

> 
> BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> Say one having early
>   if (arg > 10)
> __builtin_unreachable ();
> and another one
>   if (arg > - 10)
> __builtin_unreachable ();
> both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> do we optimize that away only after IPA?).
> 
>   Jakub
> 

Can you please provide a self-contained test-case?

Thanks,
Martin
>From 82b84914c3d40094be2ff68498a9120e9b7ece0b Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 23 May 2019 12:47:11 +0200
Subject: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

gcc/ChangeLog:

2019-05-23  Martin Liska  

	PR ipa/90555
	* ipa-icf-gimple.c (func_checker::compare_loops): New function.
	* ipa-icf-gimple.h (func_checker::compare_loops): Likewise.
	(func_checker::compare_bb): Call compare_loops.

gcc/testsuite/ChangeLog:

2019-05-23  Martin Liska  

	PR ipa/90555
	* gcc.dg/ipa/pr90555.c: New test.
---
 gcc/ipa-icf-gimple.c   | 38 +
 gcc/ipa-icf-gimple.h   |  3 ++
 gcc/testsuite/gcc.dg/ipa/pr90555.c | 67 ++
 3 files changed, 108 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr90555.c

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 25284936bc3..0713e125898 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-utils.h"
 #include "tree-eh.h"
 #include "builtins.h"
+#include "cfgloop.h"
 
 #include "ipa-icf-gimple.h"
 
@@ -605,6 +606,40 @@ func_checker::compare_variable_decl (tree t1, tree t2)
   return return_with_debug (ret);
 }
 
+/* Compare loop information for basic blocks BB1 and BB2.  */
+
+bool
+func_checker::compare_loops (basic_block bb1, basic_block bb2)
+{
+  if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
+return return_false ();
+
+  struct loop *l1 = bb1->loop_father;
+  struct loop *l2 = bb2->loop_father;
+  if (l1 == NULL)
+return true;
+
+  if ((bb1 == l1->header) != (bb2 == l2->header))
+return return_false_with_msg ("header");
+  if ((bb1 == l1->latch) != (bb2 == l2->latch))
+return return_false_with_msg ("latch");
+  if (l1->simdlen != l2->simdlen)
+return return_false_with_msg ("simdlen");
+  if (l1->safelen != l2->safelen)
+return return_false_with_msg ("safelen");
+  if (l1->can_be_parallel != l2->can_be_parallel)
+return return_false_with_msg ("can_be_parallel");
+  if (l1->dont_vectorize != l2->dont_vectorize)
+return return_false_with_msg ("dont_vectorize");
+  if (l1->force_vectorize != l2->force_vectorize)
+return return_false_with_msg ("force_vectorize");
+  if (l1->unroll != l2->unroll)
+return return_false_with_msg ("unroll");
+  if (!compare_variable_decl (l1->simduid, l2->simduid))
+return return_false_with_msg ("simduid");
+
+  return true;
+}
 
 /* Function visits all gimple labels and creates corresponding
mapping between basic blocks and labels.  */
@@ -727,6 +762,9 @@ func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
   if (!gsi_end_p (gsi2))
 return return_false ();
 
+  if (!compare_loops (bb1->bb, bb2->bb))
+return return_false ();
+
   return true;
 }
 
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 0035db32e66..51aadced9ea 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -226,6 +226,9 @@ public:
   /* Verifies that trees T1 and T2 do correspond.  */
   bool compare_variable_decl (tree t1, tree t2);
 
+  /* Compare loop information for basic blocks BB1 and BB2.  */
+  bool compare_loops (basic_block bb1, basic_block bb2);
+
   /* Return true if types are compatible for polymorphic call analysis.
  COMPARE_PTR indicates if polymorphic type comparsion should be
  done for pointers, too.  */
diff --git a/gcc/testsuite/gcc.dg/ipa/pr90555.c b/gcc/testsuite/gcc.dg/ipa/pr90555.c
new file mode 100644
index 000..a9c751cd63c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
@@ -0,0 +1,67 @@
+/* { dg-do compile } */
+/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
+/* { dg-do compile {

Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
> Yes, makes sense.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ok, thanks.

> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> > Say one having early
> >   if (arg > 10)
> > __builtin_unreachable ();
> > and another one
> >   if (arg > - 10)
> > __builtin_unreachable ();
> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> > do we optimize that away only after IPA?).
> > 
> > Jakub
> > 
> 
> Can you please provide a self-contained test-case?

I don't have one so far, it might be only vrp1 that removes those and thus
be fine for the user written asserts.  E.g. with the patch for C++ ::main argc
range it could perhaps affect code generation if we decide to ICF main with
some other function with the same content (but that patch is not in).
Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
ranges would be set for the parameters and then ICF pick one function body,
or the other way around.

Jakub


[PATCH] rs6000: Improve p9-dimode* testcases

2019-05-28 Thread Segher Boessenkool
This removes the unnecessary restriction to 32-bit (all three ways).
It also scans for mtvsr*, not just mtvsrd.  Finally, it uses the "wa"
constraints instead of "wi" in the inline asm statements.

Tested as usual; committing to trunk.


Segher


2019-05-28  Segher Boessenkool  

gcc/testsuite/
* gcc.target/powerpc/p9-dimode1.c: Don't restrict to -m64.  Check for
all mtvsr*, not just mtvsrd.  Use "wa" instead of "wi" constraints.
* gcc.target/powerpc/p9-dimode2.c: Ditto.

---
 gcc/testsuite/gcc.target/powerpc/p9-dimode1.c | 14 +-
 gcc/testsuite/gcc.target/powerpc/p9-dimode2.c | 14 +-
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/gcc/testsuite/gcc.target/powerpc/p9-dimode1.c 
b/gcc/testsuite/gcc.target/powerpc/p9-dimode1.c
index 424ddb5..b2cd3d6 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-dimode1.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-dimode1.c
@@ -1,21 +1,17 @@
-/* { dg-do compile { target { powerpc64*-*-* && lp64 } } } */
+/* { dg-do compile { target { powerpc*-*-* } } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9 -O2" } */
 
 /* Verify P9 changes to allow DImode into Altivec registers, and generate
constants using XXSPLTIB.  */
 
-#ifndef _ARCH_PPC64
-#error "This code is 64-bit."
-#endif
-
 double
 p9_zero (void)
 {
   long l = 0;
   double ret;
 
-  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wi" (l));
+  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wa" (l));
 
   return ret;
 }
@@ -26,7 +22,7 @@ p9_plus_1 (void)
   long l = 1;
   double ret;
 
-  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wi" (l));
+  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wa" (l));
 
   return ret;
 }
@@ -37,13 +33,13 @@ p9_minus_1 (void)
   long l = -1;
   double ret;
 
-  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wi" (l));
+  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wa" (l));
 
   return ret;
 }
 
 /* { dg-final { scan-assembler {\mxxspltib\M} } } */
-/* { dg-final { scan-assembler-not {\mmtvsrd\M}   } } */
+/* { dg-final { scan-assembler-not {\mmtvsr}  } } */
 /* { dg-final { scan-assembler-not {\mlfd\M}  } } */
 /* { dg-final { scan-assembler-not {\mld\M}   } } */
 /* { dg-final { scan-assembler-not {\mlxsd\M} } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-dimode2.c 
b/gcc/testsuite/gcc.target/powerpc/p9-dimode2.c
index dc3c360..c2196a2 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-dimode2.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-dimode2.c
@@ -1,13 +1,9 @@
-/* { dg-do compile { target { powerpc64*-*-* && lp64 } } } */
+/* { dg-do compile { target { powerpc*-*-* } } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9 -O2" } */
 
-/* Verify that large integer constants are loaded via direct move instead of 
being
-   loaded from memory.  */
-
-#ifndef _ARCH_PPC64
-#error "This code is 64-bit."
-#endif
+/* Verify that large integer constants are loaded via direct move instead of
+   being loaded from memory.  */
 
 double
 p9_large (void)
@@ -15,12 +11,12 @@ p9_large (void)
   long l = 0x12345678;
   double ret;
 
-  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wi" (l));
+  __asm__ ("xxlor %x0,%x1,%x1" : "=&d" (ret) : "wa" (l));
 
   return ret;
 }
 
-/* { dg-final { scan-assembler {\mmtvsrd\M} } } */
+/* { dg-final { scan-assembler {\mmtvsr}} } */
 /* { dg-final { scan-assembler-not {\mld\M} } } */
 /* { dg-final { scan-assembler-not {\mlfd\M}} } */
 /* { dg-final { scan-assembler-not {\mlxsd\M}   } } */
-- 
1.8.3.1



Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Martin Liška
On 5/28/19 1:41 PM, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
>> Yes, makes sense.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ok, thanks.
> 
>>> BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
>>> or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
>>> Say one having early
>>>   if (arg > 10)
>>> __builtin_unreachable ();
>>> and another one
>>>   if (arg > - 10)
>>> __builtin_unreachable ();
>>> both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
>>> do we optimize that away only after IPA?).
>>>
>>> Jakub
>>>
>>
>> Can you please provide a self-contained test-case?
> 
> I don't have one so far, it might be only vrp1 that removes those and thus
> be fine for the user written asserts.  E.g. with the patch for C++ ::main argc
> range it could perhaps affect code generation if we decide to ICF main with
> some other function with the same content (but that patch is not in).

That's handled here:

  3493  if (DECL_NAME (source->decl)
  3494  && MAIN_NAME_P (DECL_NAME (source->decl)))
  3495/* If merge via wrappers, picking main as the target can be
  3496   problematic.  */
  3497source = c->members[1];


> Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
> ranges would be set for the parameters and then ICF pick one function body,
> or the other way around.

I'll discuss that with Martin Jambor.

Martin

> 
>   Jakub
> 



Re: GCC 7 backport

2019-05-28 Thread Martin Liška
Hi.

There are 3 backports that touch g++.dg/ipa/pr89009.C test-case.

Thanks,
Martin
>From 32fea51fa1294abb7c2aface7fc302406ff182cd Mon Sep 17 00:00:00 2001
From: sandra 
Date: Sun, 21 Apr 2019 02:01:36 +
Subject: [PATCH 3/3] Backport r270476

gcc/testsuite/ChangeLog:

2019-04-20  Sandra Loosemore  

	* g++.dg/ipa/pr89009.C: Add dg-require-effective-target fpic.
---
 gcc/testsuite/g++.dg/ipa/pr89009.C | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/testsuite/g++.dg/ipa/pr89009.C b/gcc/testsuite/g++.dg/ipa/pr89009.C
index 70ab0be64a9..ec181d0e8ce 100644
--- a/gcc/testsuite/g++.dg/ipa/pr89009.C
+++ b/gcc/testsuite/g++.dg/ipa/pr89009.C
@@ -1,5 +1,6 @@
 /* PR ipa/89009 */
 /* { dg-do run } */
+/* { dg-require-effective-target fpic } */
 /* { dg-options "-fpic -O2 -fno-inline" } */
 /* { dg-require-visibility "" } */
 
-- 
2.21.0

>From 600f32adacd444162e61796a73f0058a55474356 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Thu, 28 Feb 2019 13:17:09 +
Subject: [PATCH 2/3] Backport r269278

gcc/testsuite/ChangeLog:

2019-02-28  John David Anglin  

	PR testsuite/89441
	* g++.dg/ipa/pr89009.C: Update symbol visibility.
---
 gcc/testsuite/g++.dg/ipa/pr89009.C | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/g++.dg/ipa/pr89009.C b/gcc/testsuite/g++.dg/ipa/pr89009.C
index 6b4fc65a641..70ab0be64a9 100644
--- a/gcc/testsuite/g++.dg/ipa/pr89009.C
+++ b/gcc/testsuite/g++.dg/ipa/pr89009.C
@@ -1,11 +1,12 @@
 /* PR ipa/89009 */
 /* { dg-do run } */
-/* { dg-options "-fvisibility=hidden -fpic -O2 -fno-inline" } */
+/* { dg-options "-fpic -O2 -fno-inline" } */
+/* { dg-require-visibility "" } */
 
-#pragma GCC visibility push(default)
 void foo1() { __builtin_printf ("foo\n"); }
-#pragma GCC visibility pop
+#pragma GCC visibility push(hidden)
 void foo2() { __builtin_printf ("foo\n"); }
+#pragma GCC visibility pop
 
 int main() { foo2(); return 0; }
 
-- 
2.21.0

>From 1d6ee7c522281ceed467f5a5f2278b67b4313369 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Mon, 11 Feb 2019 08:13:03 +
Subject: [PATCH 1/3] Backport r268762 (test-suite)

gcc/testsuite/ChangeLog:

2019-02-11  Martin Liska  

	PR ipa/89009
	* g++.dg/ipa/pr89009.C: New test.
---
 gcc/testsuite/g++.dg/ipa/pr89009.C | 12 
 1 file changed, 12 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr89009.C

diff --git a/gcc/testsuite/g++.dg/ipa/pr89009.C b/gcc/testsuite/g++.dg/ipa/pr89009.C
new file mode 100644
index 000..6b4fc65a641
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr89009.C
@@ -0,0 +1,12 @@
+/* PR ipa/89009 */
+/* { dg-do run } */
+/* { dg-options "-fvisibility=hidden -fpic -O2 -fno-inline" } */
+
+#pragma GCC visibility push(default)
+void foo1() { __builtin_printf ("foo\n"); }
+#pragma GCC visibility pop
+void foo2() { __builtin_printf ("foo\n"); }
+
+int main() { foo2(); return 0; }
+
+/* { dg-output "foo" } */
-- 
2.21.0



Re: [PATCH] Support again multiple --help options (PR other/90315).

2019-05-28 Thread Jakub Jelinek
On Fri, May 03, 2019 at 12:59:02PM +0200, Martin Liška wrote:
> 2019-05-03  Martin Liska  
> 
>   PR other/90315
>   * opts-global.c (decode_options): Print help for all
>   help_option_arguments.
>   * opts.c (print_help): Add new argument.
>   (common_handle_option): Remember all values into
>   help_option_arguments.
>   * opts.h (print_help): Add new argument.

> diff --git a/gcc/opts-global.c b/gcc/opts-global.c
> index e6eaeb20bf7..ce0b1f61603 100644
> --- a/gcc/opts-global.c
> +++ b/gcc/opts-global.c
> @@ -317,8 +317,8 @@ decode_options (struct gcc_options *opts, struct 
> gcc_options *opts_set,
>finish_options (opts, opts_set, loc);
>  
>/* Print --help=* if used.  */
> -  if (help_option_argument != NULL)
> -print_help (opts, lang_mask);
> +  for (unsigned i = 0; i < help_option_arguments.length (); i++)
> +print_help (opts, lang_mask, help_option_arguments[i]);

Use FOR_EACH_VEC_ELT macro?

Ok with that change.

Jakub


Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread David CARLIER
Yes I sort of agree it is pretty harmless even though I cannot tell
for Microsoft toolchain issue here.

On Tue, 28 May 2019 at 10:27, Jakub Jelinek  wrote:
>
> On Tue, May 28, 2019 at 12:24:12PM +0200, Martin Liška wrote:
> > Well, I see a lot of these struct/class discrepancies when building GCC 
> > with LLVM.
> > Question is whether it worth changing?
>
> No, the warning is just wrong.  C++ clearly states what the struct and class
> keyword mean and it is just fine to mix them between declarations and
> definitions.
>
> Jakub


Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Martin Jambor
Hi,

On Tue, May 28 2019, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 01:29:54PM +0200, Martin Liška wrote:
>> Yes, makes sense.
>> 
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ok, thanks.
>
>> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
>> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
>> > Say one having early
>> >   if (arg > 10)
>> > __builtin_unreachable ();
>> > and another one
>> >   if (arg > - 10)
>> > __builtin_unreachable ();
>> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
>> > do we optimize that away only after IPA?).
>> > 
>> >Jakub
>> > 
>> 
>> Can you please provide a self-contained test-case?
>
> I don't have one so far, it might be only vrp1 that removes those and thus
> be fine for the user written asserts.  E.g. with the patch for C++ ::main argc
> range it could perhaps affect code generation if we decide to ICF main with
> some other function with the same content (but that patch is not in).
> Another thing is I'm not sure how IPA-VRP vs. ICF interact, whether first
> ranges would be set for the parameters and then ICF pick one function body,
> or the other way around.

IPA-ICF runs first and unifies call graph nodes.  At that point the
actual VRP information for IPA-VRP information are stored along call
graph edges (the other bit stored along nodes are types of formal
parameters), so assuming the actual call arguments have the same value
ranges in the unified bodies, it should work.

Martin




Re: [PATCH 1/2] Add support for IVOPT

2019-05-28 Thread Richard Sandiford
Kugan Vivekanandarajah  writes:
> +/* Return the preferred mem scale factor for accessing MEM_MODE
> +   of BASE which is optimized for SPEED.  */

Maybe:

/* Return the preferred index scale factor for accessing memory of mode
   MEM_MODE in the address space of pointer BASE.  Assume that we're
   optimizing for speed if SPEED is true and for size otherwise.  */

> @@ -3479,7 +3481,7 @@ add_iv_candidate_derived_from_uses (struct ivopts_data 
> *data)
>data->iv_common_cands.truncate (0);
>  }
>  
> -/* Adds candidates based on the value of USE's iv.  */
> +  /* Adds candidates based on the value of USE's iv.  */
>  
>  static void
>  add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)

Stray change: the original is correct.

> @@ -3500,6 +3502,26 @@ add_iv_candidate_for_use (struct ivopts_data *data, 
> struct iv_use *use)
>  basetype = sizetype;
>record_common_cand (data, build_int_cst (basetype, 0), iv->step, use);
>  
> +  /* Compare the cost of an address with an unscaled index with the cost of
> +an address with a scaled index and add candidate if useful. */

Should be two spaces after "."

OK with those changes, thanks.  OK for part 2 as well.

Richard


Re: C++ PATCH for c++/90548 - ICE with generic lambda and empty pack

2019-05-28 Thread Jason Merrill

On 5/22/19 5:50 PM, Marek Polacek wrote:

On Wed, May 22, 2019 at 12:17:04AM -0400, Jason Merrill wrote:

On 5/20/19 6:08 PM, Marek Polacek wrote:

We ICE here because we are accessing call_args even though it's empty:

(gdb) p (*call_args).is_empty()
$5 = true

It's empty because the pack processed by tsubst_pack_expansion expanded into an
empty vector, so nothing got pushed onto call_args.  So handle that, and also
handle pushing 'this' properly when call_args is empty.


Ah, the problem is that nargs is just wrong in the presence of pack
expansions; in this case it's too large, but it could also end up too small.
What we want is the length of call_args, not the number of args before doing
the expansions.


Oh, right.  I've expanded the test to also detect the case when the number of
arguments grows after any packs have been expanded.  We still need the special
handling when adding 'this', though.

Thanks for catching the other problem!

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


OK, thanks.

Jason



Re: [C/C++ PATCH] Reject __builtin_{add,sub,mul}_overflow with pointer to const integer as last arg (PR c/90628)

2019-05-28 Thread Jason Merrill

On 5/27/19 5:20 PM, Jakub Jelinek wrote:

Hi!

As the testcase shows, we are silently accepting writes into const
variables, because the type generic builtins don't have a prototype.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2019-05-27  Jakub Jelinek  

PR c/90628
* c-common.c (check_builtin_function_arguments)
: Diagnose pointer to const qualified integer
as last argument.

* c-c++-common/builtin-arith-overflow-3.c: New test.

--- gcc/c-family/c-common.c.jj  2019-05-21 16:16:48.068973678 +0200
+++ gcc/c-family/c-common.c 2019-05-27 10:46:25.525968739 +0200
@@ -5995,6 +5995,13 @@ check_builtin_function_arguments (locati
"has pointer to boolean type", fndecl);
  return false;
}
+ else if (TYPE_READONLY (TREE_TYPE (TREE_TYPE (args[2]
+   {
+ error_at (ARG_LOCATION (2), "argument 3 in call to function %qE "
+   "has pointer type to % qualified integer",
+   fndecl);


Is there a reason not to also print the type with %qT?

Jason


Re: [C/C++ PATCH] Reject __builtin_{add,sub,mul}_overflow with pointer to const integer as last arg (PR c/90628)

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 08:59:57AM -0400, Jason Merrill wrote:
> On 5/27/19 5:20 PM, Jakub Jelinek wrote:
> > As the testcase shows, we are silently accepting writes into const
> > variables, because the type generic builtins don't have a prototype.
> > 
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> > 
> > 2019-05-27  Jakub Jelinek  
> > 
> > PR c/90628
> > * c-common.c (check_builtin_function_arguments)
> > : Diagnose pointer to const qualified integer
> > as last argument.
> > 
> > * c-c++-common/builtin-arith-overflow-3.c: New test.
> > 
> > --- gcc/c-family/c-common.c.jj  2019-05-21 16:16:48.068973678 +0200
> > +++ gcc/c-family/c-common.c 2019-05-27 10:46:25.525968739 +0200
> > @@ -5995,6 +5995,13 @@ check_builtin_function_arguments (locati
> > "has pointer to boolean type", fndecl);
> >   return false;
> > }
> > + else if (TYPE_READONLY (TREE_TYPE (TREE_TYPE (args[2]
> > +   {
> > + error_at (ARG_LOCATION (2), "argument 3 in call to function %qE "
> > +   "has pointer type to % qualified integer",
> > +   fndecl);
> 
> Is there a reason not to also print the type with %qT?

So like:
+ error_at (ARG_LOCATION (2), "argument 3 in call to function %qE "
+   "has pointer type to % qualified integer "
+   "(%qT)", fndecl, TREE_TYPE (args[2]));
or some other wording?

I didn't want to say
"argument 3 in call to function %qE has type %qT" because then
users wouldn't know what the actual problem is.

Jakub


[committed] Fix minor testsuite fallout from recent loop changes

2019-05-28 Thread Jeff Law
This fixes light fallout from recent changes in our loop optimizer.  The
tests in question want to look at the codegen for the loop and thus
turning them into mem* calls isn't helpful.

Easiest thing to do is just disable loop distribution.  COmmitted to the
trunk.

Jeff
* testsuite/gcc.target/sh/pr50749-qihisi-predec-3.c: Disable
loop distribution.

diff --git a/gcc/testsuite/gcc.target/sh/pr50749-qihisi-predec-3.c 
b/gcc/testsuite/gcc.target/sh/pr50749-qihisi-predec-3.c
index 541749750e0..f814d7ac940 100644
--- a/gcc/testsuite/gcc.target/sh/pr50749-qihisi-predec-3.c
+++ b/gcc/testsuite/gcc.target/sh/pr50749-qihisi-predec-3.c
@@ -1,7 +1,7 @@
 /* PR target/50749: Verify that pre-decrement addressing is generated
inside a loop.  */
 /* { dg-do compile }  */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -fno-tree-loop-distribute-patterns" } */
 /* { dg-final { scan-assembler-times "mov.b\tr\[0-9]\+,@-r\[0-9]\+" 1 } } */
 /* { dg-final { scan-assembler-times "mov.w\tr\[0-9]\+,@-r\[0-9]\+" 1 } } */
 /* { dg-final { scan-assembler-times "mov.l\tr\[0-9]\+,@-r\[0-9]\+" 1 } } */


Re: [C/C++ PATCH] Reject __builtin_{add,sub,mul}_overflow with pointer to const integer as last arg (PR c/90628)

2019-05-28 Thread Jason Merrill

On 5/28/19 9:11 AM, Jakub Jelinek wrote:

On Tue, May 28, 2019 at 08:59:57AM -0400, Jason Merrill wrote:

On 5/27/19 5:20 PM, Jakub Jelinek wrote:

As the testcase shows, we are silently accepting writes into const
variables, because the type generic builtins don't have a prototype.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2019-05-27  Jakub Jelinek  

PR c/90628
* c-common.c (check_builtin_function_arguments)
: Diagnose pointer to const qualified integer
as last argument.

* c-c++-common/builtin-arith-overflow-3.c: New test.

--- gcc/c-family/c-common.c.jj  2019-05-21 16:16:48.068973678 +0200
+++ gcc/c-family/c-common.c 2019-05-27 10:46:25.525968739 +0200
@@ -5995,6 +5995,13 @@ check_builtin_function_arguments (locati
"has pointer to boolean type", fndecl);
  return false;
}
+ else if (TYPE_READONLY (TREE_TYPE (TREE_TYPE (args[2]
+   {
+ error_at (ARG_LOCATION (2), "argument 3 in call to function %qE "
+   "has pointer type to % qualified integer",
+   fndecl);


Is there a reason not to also print the type with %qT?


So like:
+ error_at (ARG_LOCATION (2), "argument 3 in call to function %qE "
+   "has pointer type to % qualified integer "
+   "(%qT)", fndecl, TREE_TYPE (args[2]));
or some other wording?



I didn't want to say
"argument 3 in call to function %qE has type %qT" because then
users wouldn't know what the actual problem is.


Sure.  Or "has pointer to % type (%qT)" as a terser alternative. 
 OK either way.


Jason


[PATCH V2] A jump threading opportunity for condition branch

2019-05-28 Thread Jiufu Guo
Hi,

This patch implements a new opportunity of jump threading for PR77820.
In this optimization, conditional jumps are merged with unconditional
jump. And then moving CMP result to GPR is eliminated.

This version is based on the proposal of Richard, Jeff and Andrew, and
refined to incorporate comments.  Thanks for the reviews!

Bootstrapped and tested on powerpc64le and powerpc64be with no
regressions (one case is improved) and new testcases are added. Is this
ok for trunk?

Example of this opportunity looks like below:

  
  p0 = a CMP b
  goto ;

  
  p1 = c CMP d
  goto ;

  
  # phi = PHI 
  if (phi != 0) goto ; else goto ;

Could be transformed to:

  
  p0 = a CMP b
  if (p0 != 0) goto ; else goto ;

  
  p1 = c CMP d
  if (p1 != 0) goto ; else goto ;


This optimization eliminates:
1. saving CMP result: p0 = a CMP b.
2. additional CMP on branch: if (phi != 0).
3. converting CMP result if there is phi = (INT_CONV) p0 if there is.

Thanks!
Jiufu Guo


[gcc]
2019-05-28  Jiufu Guo  
Lijia He  

PR tree-optimization/77820
* tree-ssa-threadedge.c
(edge_forwards_cmp_to_conditional_jump_through_empty_bb_p): New
function.
(thread_across_edge): Add call to
edge_forwards_cmp_to_conditional_jump_through_empty_bb_p.

[gcc/testsuite]
2019-05-28  Jiufu Guo  
Lijia He  

PR tree-optimization/77820
* gcc.dg/tree-ssa/phi_on_compare-1.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-2.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-3.c: New testcase.
* gcc.dg/tree-ssa/phi_on_compare-4.c: New testcase.
* gcc.dg/tree-ssa/split-path-6.c: Update testcase.

---
 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c | 30 ++
 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c | 23 +++
 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c | 25 
 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c | 40 +
 gcc/testsuite/gcc.dg/tree-ssa/split-path-6.c |  2 +-
 gcc/tree-ssa-threadedge.c| 76 +++-
 6 files changed, 192 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c 
b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
new file mode 100644
index 000..5227c87
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-vrp1" } */
+
+void g (int);
+void g1 (int);
+
+void
+f (long a, long b, long c, long d, long x)
+{
+  _Bool t;
+  if (x)
+{
+  g (a + 1);
+  t = a < b;
+  c = d + x;
+}
+  else
+{
+  g (b + 1);
+  a = c + d;
+  t = c > d;
+}
+
+  if (t)
+g1 (c);
+
+  g (a);
+}
+
+/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c 
b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
new file mode 100644
index 000..eaf89bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-vrp1" } */
+
+void g (void);
+void g1 (void);
+
+void
+f (long a, long b, long c, long d, int x)
+{
+  _Bool t;
+  if (x)
+t = c < d;
+  else
+t = a < b;
+
+  if (t)
+{
+  g1 ();
+  g ();
+}
+}
+
+/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c 
b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
new file mode 100644
index 000..d5a1e0b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-vrp1" } */
+
+void g (void);
+void g1 (void);
+
+void
+f (long a, long b, long c, long d, int x)
+{
+  int t;
+  if (x)
+t = a < b;
+  else if (d == x)
+t = c < b;
+  else
+t = d > c;
+
+  if (t)
+{
+  g1 ();
+  g ();
+}
+}
+
+/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c 
b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c
new file mode 100644
index 000..53acabc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -fdump-tree-vrp1" } */
+
+void g (int);
+void g1 (int);
+
+void
+f (long a, long b, long c, long d, int x)
+{
+  int t;
+  _Bool l1 = 0, l2 = 0;
+  if (x)
+{
+  g (a);
+  c = a + b;
+  t = a < b;
+  l1 = 1;
+}
+  else
+{
+  g1 (b);
+  t = c > d;
+  d = c + b;
+  l2 = 1;
+}
+
+  if (t)
+{
+  if

Re: [C++ PATCH] P1091R3 - Extending structured bindings to be more like var decls

2019-05-28 Thread Jason Merrill

On 5/20/19 5:44 PM, Jakub Jelinek wrote:

Hi!

The following patch is an attempt at implementing these two papers.
I must be missing something important, because in P1091R3 the rationale
talks about the intent to support structured bindings with static,
thread_local, constexpr, inline and extern, but the actual wording
changes in the paper are only about static and thread_local, because dcl.dcl
still says
If the decl-specifier-seq contains any decl-specifier other than static,
thread_local, auto, or cv-qualifiers, the program is ill-formed.


Right, the original proposal was for more, but it was limited to static 
and thread_local by the committee.



Another thing are the lambda captures of the structured bindings.
The end result is that they are allowed, both capturing values and capturing
references, except when capturing bitfields, but that is actually what gcc
has been implementing for quite a while.  The question is if we shouldn't
reject those for -std=c++17 or -std=c++17 -pedantic-errors, seems e.g.
clang++ rejects those, but looking at the C++17 standard I don't see any
wording that would say those are not allowed.  It was only P0588R1 that
added
"or captures a structured binding (explicitly or implicitly), the program is
ill-formed."
Is P0588R1 a DR that would be applied to older standards?


P0588 was certainly intended to apply to C++17, but I'd treat this as 
overriding it, and not complain.



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

2019-05-20  Jakub Jelinek  

P1091R3 - Extending structured bindings to be more like var decls
P1381R1 - Reference capture of structured bindings
* decl.c (cp_maybe_mangle_decomp): Handle TREE_STATIC decls even at
function scope.
(cp_finish_decomp): Copy over various decl properties from decl to
v[i] in the tuple case.
(grokdeclarator): Allow static, thread_local and __thread for C++2a
and use pedwarn instead of error for older standard revisions.
Make other structured binding diagnostic messages more i18n friendly.

* g++.dg/cpp1z/decomp3.C (test): For static, expect only warning
instead of error and only for c++17_down.  Add a thread_local test.
(z2): Add a __thread test.
* g++.dg/cpp2a/decomp1.C: New test.
* g++.dg/cpp2a/decomp1-aux.cc: New file.
* g++.dg/cpp2a/decomp2.C: New test.
* g++.dg/cpp2a/decomp3.C: New test.

--- gcc/cp/decl.c.jj2019-05-20 14:52:26.871375569 +0200
+++ gcc/cp/decl.c   2019-05-20 15:48:33.563599150 +0200
+  && (DECL_NAMESPACE_SCOPE_P (decl) || TREE_STATIC (decl)))

> +if (TREE_STATIC (v[i]))

When can a structured binding not be TREE_STATIC?

Jason


Re: [C++ PATCH] P1091R3 - Extending structured bindings to be more like var decls

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 10:07:12AM -0400, Jason Merrill wrote:
> > "or captures a structured binding (explicitly or implicitly), the program is
> > ill-formed."
> > Is P0588R1 a DR that would be applied to older standards?
> 
> P0588 was certainly intended to apply to C++17, but I'd treat this as
> overriding it, and not complain.

Ok.

> > --- gcc/cp/decl.c.jj2019-05-20 14:52:26.871375569 +0200
> > +++ gcc/cp/decl.c   2019-05-20 15:48:33.563599150 +0200
> > +  && (DECL_NAMESPACE_SCOPE_P (decl) || TREE_STATIC (decl)))
> > + if (TREE_STATIC (v[i]))
> 
> When can a structured binding not be TREE_STATIC?

All the block scope structured bindings that aren't explicitly static
nor thread_local are not TREE_STATIC, and we really can't mangle them (and
cp_maybe_mangle_decomp is called on them).  And newly we need to mangle
even block scope structured bindings that are static or thread_local.

As extern isn't allowed (yet), perhaps all DECL_NAMESPACE_SCOPE_P (decl)
structured bindings are TREE_STATIC and I could replace the
DECL_NAMESPACE_SCOPE_P (decl) test in cp_maybe_mangle_decomp
with TREE_STATIC (decl) instead.

Jakub


Implement vector average patterns for SVE2

2019-05-28 Thread Alejandro Martinez Vicente
Hi,

This patch implements the [u]avgM3_floor and [u]avgM3_ceil optabs for SVE2.

Alejandro

gcc/ChangeLog:

2019-05-28  Alejandro Martinez  

* config/aarch64/aarch64-sve2.md: New file.
(avg3_floor): New pattern.
(avg3_ceil): Likewise.
(*h): Likewise.
* config/aarch64/aarch64.md: Include aarch64-sve2.md.


2019-05-28  Alejandro Martinez  

gcc/testsuite/
* gcc.target/aarch64/sve2/average_1.c: New test.
* lib/target-supports.exp (check_effective_target_aarch64_sve1_only):
New helper.
(check_effective_target_vect_avg_qi): Check for SVE1 only.


vavg_sve2.patch
Description: vavg_sve2.patch


Re: [C++ Patch] PR 89875 ("[7/8/9/10 Regression] invalid typeof reference to a member of an incomplete struct accepted at function scope")

2019-05-28 Thread Jason Merrill

On 5/10/19 10:29 AM, Paolo Carlini wrote:

Hi,

a while ago Martin noticed that an unintended consequence of an old 
tweak of mine - which avoided redundant error messages emitted from 
cp_parser_init_declarator - is that, in some cases, we started accepting 
ill-formed typeofs. Luckily, decltype isn't affected and that points to 
the real issue: by the time that place in cp_parser_init_declarator is 
reached, for a decltype version we already emitted a correct error 
message. Thus I think the right way to fix the problem is simply 
committing to tentative parse when, toward the end of 
cp_parser_sizeof_operand we know that we must be looking at a (possibly 
ill-formed) expression. Tested x86_64-linux.


The problem with calling cp_parser_commit_to_tentative_parse here is 
that the tentative parse you're committing to is for the enclosing 
scope, which is trying to decide whether e.g. we're parsing a 
declaration or expression.  If the operand of typeof is a well-formed 
expression, and the larger context is an expression, this will break.


Better, I think, to commit and re-parse only if you actually encounter 
an error.


Alternately, cp_parser_decltype_expr deals with this by using a 
tentative firewall and CPP_DECLTYPE; cp_parser_sizeof_operand could do 
the same, but that seems like a bigger hammer.


Jason


Re: [C++ PATCH] P1091R3 - Extending structured bindings to be more like var decls

2019-05-28 Thread Jason Merrill

On 5/28/19 10:19 AM, Jakub Jelinek wrote:

On Tue, May 28, 2019 at 10:07:12AM -0400, Jason Merrill wrote:

"or captures a structured binding (explicitly or implicitly), the program is
ill-formed."
Is P0588R1 a DR that would be applied to older standards?


P0588 was certainly intended to apply to C++17, but I'd treat this as
overriding it, and not complain.


Ok.


--- gcc/cp/decl.c.jj2019-05-20 14:52:26.871375569 +0200
+++ gcc/cp/decl.c   2019-05-20 15:48:33.563599150 +0200
+  && (DECL_NAMESPACE_SCOPE_P (decl) || TREE_STATIC (decl)))
+ if (TREE_STATIC (v[i]))


When can a structured binding not be TREE_STATIC?


All the block scope structured bindings that aren't explicitly static
nor thread_local are not TREE_STATIC


Right, of course!


As extern isn't allowed (yet), perhaps all DECL_NAMESPACE_SCOPE_P (decl)
structured bindings are TREE_STATIC and I could replace the
DECL_NAMESPACE_SCOPE_P (decl) test in cp_maybe_mangle_decomp
with TREE_STATIC (decl) instead.


OK with that change.

Jason



[PATCH v2] aarch64: emit .variant_pcs for aarch64_vector_pcs symbol references

2019-05-28 Thread Szabolcs Nagy
v2:
- use aarch64_simd_decl_p to check for aarch64_vector_pcs.
- emit the .variant_pcs directive even for local functions.
- don't require .variant_pcs asm support in compile only tests.
- add weakref tests.

A dynamic linker with lazy binding support may need to handle vector PCS
function symbols specially, so an ELF symbol table marking was
introduced for such symbols.

Function symbol references and definitions that follow the vector PCS
are marked in the generated assembly with .variant_pcs and then the
STO_AARCH64_VARIANT_PCS st_other flag is set on the symbol in the object
file.  The marking is propagated to the dynamic symbol table by the
static linker so a dynamic linker can handle such symbols specially.

For this to work, the assembler, the static linker and the dynamic
linker has to be updated on a system.  Old assembler does not support
the new .variant_pcs directive, so a toolchain with old binutils won't
be able to compile code that references vector PCS symbols.

gcc/ChangeLog:

2019-05-28  Szabolcs Nagy  

* config/aarch64/aarch64-protos.h (aarch64_asm_output_alias): Declare.
(aarch64_asm_output_external): Declare.
* config/aarch64/aarch64.c (aarch64_asm_output_variant_pcs): New.
(aarch64_declare_function_name): Call aarch64_asm_output_variant_pcs.
(aarch64_asm_output_alias): New.
(aarch64_asm_output_external): New.
* config/aarch64/aarch64.h (ASM_OUTPUT_DEF_FROM_DECLS): Define.
(ASM_OUTPUT_EXTERNAL): Define.

gcc/testsuite/ChangeLog:

2019-05-28  Szabolcs Nagy  

* gcc.target/aarch64/pcs_attribute-2.c: New test.
* gcc.target/aarch64/torture/simd-abi-4.c: Check .variant_pcs support.
* lib/target-supports.exp (check_effective_target_aarch64_variant_pcs):
New.
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index a0723266f22..19e9767c792 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -427,6 +427,8 @@ bool aarch64_is_long_call_p (rtx);
 bool aarch64_is_noplt_call_p (rtx);
 bool aarch64_label_mentioned_p (rtx);
 void aarch64_declare_function_name (FILE *, const char*, tree);
+void aarch64_asm_output_alias (FILE *, const tree, const tree);
+void aarch64_asm_output_external (FILE *, const tree, const char*);
 bool aarch64_legitimate_pic_operand_p (rtx);
 bool aarch64_mask_and_shift_for_ubfiz_p (scalar_int_mode, rtx, rtx);
 bool aarch64_masks_and_shift_for_bfi_p (scalar_int_mode, unsigned HOST_WIDE_INT,
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 83453d03095..16e534b0772 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -15276,6 +15276,19 @@ aarch64_asm_preferred_eh_data_format (int code ATTRIBUTE_UNUSED, int global)
return (global ? DW_EH_PE_indirect : 0) | DW_EH_PE_pcrel | type;
 }
 
+/* Output .variant_pcs for aarch64_vector_pcs function symbols.  */
+
+static void
+aarch64_asm_output_variant_pcs (FILE *stream, const tree decl, const char* name)
+{
+  if (aarch64_simd_decl_p (decl))
+{
+  fprintf (stream, "\t.variant_pcs\t");
+  assemble_name (stream, name);
+  fprintf (stream, "\n");
+}
+}
+
 /* The last .arch and .tune assembly strings that we printed.  */
 static std::string aarch64_last_printed_arch_string;
 static std::string aarch64_last_printed_tune_string;
@@ -15325,11 +15338,33 @@ aarch64_declare_function_name (FILE *stream, const char* name,
   aarch64_last_printed_tune_string = this_tune->name;
 }
 
+  aarch64_asm_output_variant_pcs (stream, fndecl, name);
+
   /* Don't forget the type directive for ELF.  */
   ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
   ASM_OUTPUT_LABEL (stream, name);
 }
 
+/* Implement ASM_OUTPUT_DEF_FROM_DECLS.  Output .variant_pcs for aliases.  */
+
+void
+aarch64_asm_output_alias (FILE *stream, const tree decl, const tree target)
+{
+  const char *name = XSTR (XEXP (DECL_RTL (decl), 0), 0);
+  const char *value = IDENTIFIER_POINTER (target);
+  aarch64_asm_output_variant_pcs (stream, decl, name);
+  ASM_OUTPUT_DEF (stream, name, value);
+}
+
+/* Implement ASM_OUTPUT_EXTERNAL.  Output .variant_pcs for undefined
+   function symbol references.  */
+
+void
+aarch64_asm_output_external (FILE *stream, const tree decl, const char* name)
+{
+  aarch64_asm_output_variant_pcs (stream, decl, name);
+}
+
 /* Implements TARGET_ASM_FILE_START.  Output the assembly header.  */
 
 static void
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index be6981889ab..f0f50e081ac 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -512,6 +512,15 @@ extern unsigned aarch64_architecture_version;
 #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL)	\
   aarch64_declare_function_name (STR, NAME, DECL)
 
+/* Output assembly strings for alias definition.  */
+#define ASM_OUTPUT_DEF_FROM_DECLS(STR, DECL, TARGET) \
+  aarch64_asm_output_alias (STR, DECL, T

Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread H.J. Lu
On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek  wrote:
>
> On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > if its address is taken.  Also ix86_init_large_pic_reg shouldn't set
> > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > to keep the label.
>
> Can you explain when is it ever needed to emit ENDBR on
> NOTE_INSN_DELETED_LABEL?  Only labels that are proven not to be ever
> branched to are turned into deleted labels, so I think the right answer is
> never emit any ENDBR on those...
>

This condition is checked by gcc.target/i386/cet-label-5.c in the
patch.   For

void *
func (void)
{
  return &&bar;
bar:
  return 0;
}

we generate:

(note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
(insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
(label_ref:DI [4 deleted]))
"/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
66 {*movdi_internal}
 (nil))
(insn 11 10 20 2 (use (reg/i:DI 0 ax))
"/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
-1
 (nil))


-- 
H.J.


Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 08:10:19AM -0700, H.J. Lu wrote:
> On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek  wrote:
> >
> > On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > > if its address is taken.  Also ix86_init_large_pic_reg shouldn't set
> > > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is suffcient
> > > > to keep the label.
> >
> > Can you explain when is it ever needed to emit ENDBR on
> > NOTE_INSN_DELETED_LABEL?  Only labels that are proven not to be ever
> > branched to are turned into deleted labels, so I think the right answer is
> > never emit any ENDBR on those...
> >
> 
> This condition is checked by gcc.target/i386/cet-label-5.c in the
> patch.   For
> 
> void *
> func (void)
> {
>   return &&bar;
> bar:
>   return 0;
> }
> 
> we generate:
> 
> (note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
> (insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
> (label_ref:DI [4 deleted]))
> "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> 66 {*movdi_internal}
>  (nil))
> (insn 11 10 20 2 (use (reg/i:DI 0 ax))
> "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> -1
>  (nil))

We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
it would remain a normal label, not a deleted label).

Jakub


Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread H.J. Lu
On Tue, May 28, 2019 at 8:16 AM Jakub Jelinek  wrote:
>
> On Tue, May 28, 2019 at 08:10:19AM -0700, H.J. Lu wrote:
> > On Tue, May 28, 2019 at 1:57 AM Jakub Jelinek  wrote:
> > >
> > > On Sat, Feb 16, 2019 at 07:02:11AM -0800, H.J. Lu wrote:
> > > > > For NOTE_INSN_DELETED_LABEL, we should check if forced_labels to see
> > > > > if its address is taken.  Also ix86_init_large_pic_reg shouldn't set
> > > > > LABEL_PRESERVE_P (in_struct) since NOTE_INSN_DELETED_LABEL is 
> > > > > suffcient
> > > > > to keep the label.
> > >
> > > Can you explain when is it ever needed to emit ENDBR on
> > > NOTE_INSN_DELETED_LABEL?  Only labels that are proven not to be ever
> > > branched to are turned into deleted labels, so I think the right answer is
> > > never emit any ENDBR on those...
> > >
> >
> > This condition is checked by gcc.target/i386/cet-label-5.c in the
> > patch.   For
> >
> > void *
> > func (void)
> > {
> >   return &&bar;
> > bar:
> >   return 0;
> > }
> >
> > we generate:
> >
> > (note/s 4 2 10 2 ("bar") NOTE_INSN_DELETED_LABEL 2)
> > (insn:TI 10 4 11 2 (set (reg/i:DI 0 ax)
> > (label_ref:DI [4 deleted]))
> > "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> > 66 {*movdi_internal}
> >  (nil))
> > (insn 11 10 20 2 (use (reg/i:DI 0 ax))
> > "/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/cet-label-5.c":13:1
> > -1
> >  (nil))
>
> We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
> it would remain a normal label, not a deleted label).
>

But return value of func () may be used with indirect jump.

-- 
H.J.


Re: Simplify more EXACT_DIV_EXPR comparisons

2019-05-28 Thread Martin Sebor

On 5/21/19 3:53 AM, Richard Biener wrote:

On Tue, May 21, 2019 at 4:13 AM Martin Sebor  wrote:


On 5/20/19 3:16 AM, Richard Biener wrote:

On Mon, May 20, 2019 at 10:16 AM Marc Glisse  wrote:


On Mon, 20 May 2019, Richard Biener wrote:


On Sun, May 19, 2019 at 6:16 PM Marc Glisse  wrote:


Hello,

2 pieces:

- the first one handles the case where the denominator is negative. It
doesn't happen often with exact_div, so I don't handle it everywhere, but
this one looked trivial

- handle the case where a pointer difference is cast to an unsigned type
before being compared to a constant (I hit this in std::vector). With some
range info we could probably handle some non-constant cases as well...

The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)

void f (void*);
void g (int *p, int *q)
{
 __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
 if (n < 100)
   f (__builtin_alloca (n));
}

At the time of walloca2, we have

 _1 = p_5(D) - q_6(D);
 # RANGE [-2305843009213693952, 2305843009213693951]
 _2 = _1 /[ex] 4;
 # RANGE ~[2305843009213693952, 16140901064495857663]
 n_7 = (long unsigned intD.10) _2;
 _11 = (long unsigned intD.10) _1;
 if (_11 <= 396)
[...]
 _3 = allocaD.1059 (n_7);

and warn.


That's indeed to complicated relation of _11 to n_7 for
VRP predicate discovery.


However, DOM3 later produces

 _1 = p_5(D) - q_6(D);
 _11 = (long unsigned intD.10) _1;
 if (_11 <= 396)


while _11 vs. _1 works fine.


[...]
 # RANGE [0, 99] NONZERO 127
 _2 = _1 /[ex] 4;
 # RANGE [0, 99] NONZERO 127
 n_7 = (long unsigned intD.10) _2;
 _3 = allocaD.1059 (n_7);

so I am tempted to say that the walloca2 pass is too early, xfail the
testcase and file an issue...


Hmm, there's a DOM pass before walloca2 already and moving
walloca2 after loop opts doesn't look like the best thing to do?
I suppose it's not DOM but sinking that does the important transform
here?  That is,

Index: gcc/passes.def
===
--- gcc/passes.def  (revision 271395)
+++ gcc/passes.def  (working copy)
@@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
NEXT_PASS (pass_optimize_bswap);
NEXT_PASS (pass_laddress);
NEXT_PASS (pass_lim);
-  NEXT_PASS (pass_walloca, false);
NEXT_PASS (pass_pre);
NEXT_PASS (pass_sink_code);
+  NEXT_PASS (pass_walloca, false);
NEXT_PASS (pass_sancov);
NEXT_PASS (pass_asan);
NEXT_PASS (pass_tsan);

fixes it?


I will check, but I don't think walloca uses any kind of on-demand VRP, so
we still need some pass to update the ranges after sinking, which doesn't
seem to happen until the next DOM pass.


Oh, ok...  Aldy, why's this a separate pass anyways?  I think similar
other warnigns are emitted from RTL expansion?  So maybe we can
indeed move the pass towards warn_restrict or late_warn_uninit.


I thought there was a preference to add new middle-end warnings
into passes of their own rather than into existing passes.  Is
that not so (either in general or in this specific case)?


The preference was to add them not into optimization passes.  But
of course having 10+ warning passes, each going over the whole IL
is excessive.  Also each of the locally computing ranges or so.

Given the simplicity of Walloca I wonder why it's not part of another
warning pass - since it's about tracking "sizes" again there are plenty
that fit ;)


-Walloca doesn't need to track object sizes in the same sense
as objsize and strlen do.  It just examines calls to allocation
functions, same as -Walloc-larger-than.  It would make sense to
merge the implementation of two warnings.  They don't need to
run as a pass of their own.


  From my POV, the main (only?) benefit of putting warnings in their
own passes is modularity.  Are there any others?

The biggest drawback I see is that it makes it hard to then share
data across multiple passes.  The sharing can help not just
warnings (reduce both false positive and false negative rates) but
also optimization.  That's why I'm merging the strlen and sprintf
passes, and want to eventually also look into merging
the -Wstringop-overflow warnings there (also emitted just before
RTL expansion.  Did I miss any downsides?


When things fit together they are fine to merge obviously.

One may not like -Warray-bounds inside VRP but it really "fits".


It fits because it has access to data that's not (yet) available
outside VRP, right?  It's not an ideal fit because by being in VRP
it doesn't have access to allocated object sizes so out-of-bounds
access to dynamically allocated objects aren't detected.  To detect
those, I think the only pass it could go in is strlen.  So again
an optimization pass.  It doesn't fit there very well at all, but
there is no other pass that tracks sizes of all objects (objsize
does, though only in response to calls to __builtin_object_size).

I think just like a general

Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE

2019-05-28 Thread Hans-Peter Nilsson
TL;DR: instead of capping TYPE_PRECISION of bitsizetype at
MAX_FIXED_MODE_SIZE, search for the largest fitting size from
scalar_int_mode modes supported by the target using
targetm.scalar_mode_supported_p.

-
In initialize_sizetypes, MAX_FIXED_MODE_SIZE is used as an upper
limit to the *precision* of the bit size of the size-type
(typically address length) of the target, which is wrong.

The effect is that if a 32-bit target says "please don't cook up
pieces larger than a register size", then we don't get more
precision in address-related calculations than that, while the
bit-precision needs to be at least (precision +
LOG2_BITS_PER_UNIT + 1) with precision being the size of the
address, to diagnose overflows.  There are gcc_asserts that
guard this, causing ICE when broken.

This MAX_FIXED_MODE_SIZE usage comes from r118977 (referencing
PR27885 and PR28176) and was introduced as if
MAX_FIXED_MODE_SIZE is the size of the largest supported type
for the target (where "supported" is in the most trivial sense
as in can move and add).  But it's not.

MAX_FIXED_MODE_SIZE is arguably a bit vague, but documented as
"the size in bits of the largest integer machine mode that
should actually be used.  All integer machine modes of this size
or smaller can be used for structures and unions with the
appropriate sizes."  While in general the documentation
sometimes differs from reality, that's mostly right, with
"should actually be" meaning "is preferably": it's the largest
size that the target indicates as beneficial of use besides that
directly mapped from types used in the source code; sort-of a
performance knob.  (I did a static reality code check looking
for direct and indirect uses before imposing this my own
interpretation and recollection.)  Typical use is when gcc finds
that some operations can be combined and synthesized to
optionally use a wider mode than seen in the source (but mostly
copying).  Then this macro sets an upper limit to the those
operations, whether to be done at all or the chunk-size.
Unfortunately some of the effects are unintuitive and I wouldn't
be surprised if this de-facto affects ABI corners.  It's not
something you tweak more than once for a target.

Two tests pass with this fixed for cris-elf (MAX_FIXED_MODE_SIZE
32): gcc.dg/attr-vector_size.c and gcc.dg/pr69973.c, where the
lack of precision (32 bits instead of 64 bits for bitsizetype)
caused an consistency check to ICE, from where I tracked this.

Regarding the change, MAX_FIXED_MODE_SIZE is still mentioned but
just to initialize the fall-back largest-supported precision.
Sometimes the target supports no larger mode than that of the
address, like for a 64-bit target lacking support for larger
sizes (e.g. TImode), as in the motivating PR27885.  I guess they
can still get ICE for overflowing address-calculation checks,
but that's separate problem, fixable by the target.

I considered making a separate convenience function as well as
amending smallest_int_mode_for_size (e.g., an optional argument
to have smallest_mode_for_size neither abort or cap at
MAX_FIXED_MODE_SIZE) but I think the utility is rather specific
to this use.  We rarely want to both settle for a smaller type
than the one requested, and that possibly being larger than
MAX_FIXED_MODE_SIZE.
-

Regtested cris-elf and x86_64-linux-gnu, and a separate
cross-build for hppa64-hp-hpux11.11 to spot-check that I didn't
re-introduce PR27885.  (Not a full cross-build, just building
f951 and following initialize_sizetypes in gdb to see TRT
happening.)

Ok to commit?

gcc:
* stor-layout.c (initialize_sizetypes): Set the precision
of bitsizetype to the size of largest integer mode
supported by target, not necessarily MAX_FIXED_MODE_SIZE.

--- gcc/stor-layout.c.orig  Sat May 25 07:12:49 2019
+++ gcc/stor-layout.c   Tue May 28 04:29:10 2019
@@ -2728,9 +2728,36 @@ initialize_sizetypes (void)
gcc_unreachable ();
 }
 
-  bprecision
-= MIN (precision + LOG2_BITS_PER_UNIT + 1, MAX_FIXED_MODE_SIZE);
-  bprecision = GET_MODE_PRECISION (smallest_int_mode_for_size (bprecision));
+  bprecision = precision + LOG2_BITS_PER_UNIT + 1;
+
+  /* Find the precision of the largest supported mode equal to or larger
+ than needed for the bitsize precision.  This may be larger than
+ MAX_FIXED_MODE_SIZE, which is just the largest preferred size. */
+  machine_mode bpmode;
+  unsigned int largest_target_precision = MAX_FIXED_MODE_SIZE;
+
+  FOR_EACH_MODE_IN_CLASS (bpmode, MODE_INT)
+{
+  scalar_int_mode smode = scalar_int_mode::from_int (bpmode);
+  unsigned int mode_prec = GET_MODE_PRECISION (smode);
+
+  if (!targetm.scalar_mode_supported_p (smode))
+   continue;
+
+  if (mode_prec > largest_target_precision)
+   largest_target_precision = mode_prec;
+
+  if (mode_prec >= (unsigned int) bprecision)
+   {
+ bprecision = mode_prec;
+ break;
+   }
+}
+
+  /* Fall back to the largest know

Re: C++ PATCH to implement deferred parsing of noexcept-specifiers (c++/86476, c++/52869)

2019-05-28 Thread Jason Merrill

On 5/10/19 3:21 PM, Marek Polacek wrote:

Coming back to this.  I didn't think this was suitable for GCC 9.

On Mon, Jan 07, 2019 at 10:44:37AM -0500, Jason Merrill wrote:

On 12/19/18 3:27 PM, Marek Polacek wrote:

Prompted by Jon's observation in 52869, I noticed that we don't treat
a noexcept-specifier as a complete-class context of a class ([class.mem]/6).
As with member function bodies, default arguments, and NSDMIs, names used in
a noexcept-specifier of a member-function can be declared later in the class
body, so we need to wait and parse them at the end of the class.
For that, I've made use of DEFAULT_ARG (now best to be renamed to UNPARSED_ARG).


Or DEFERRED_PARSE, yes.


I didn't change the name but I'm happy to do it as a follow up.


+  /* We can't compare unparsed noexcept-specifiers.  Save the old decl
+ and check this again after we've parsed the noexcept-specifiers
+ for real.  */
+  if (UNPARSED_NOEXCEPT_SPEC_P (new_exceptions))
+{
+  vec_safe_push (DEFARG_INSTANTIATIONS (TREE_PURPOSE (new_exceptions)),
+copy_decl (old_decl));
+  return;
+}


Why copy_decl?


This is so that we don't lose the diagnostic in noexcept46.C.  If I don't use
copy_decl then the tree is shared and subsequent changes to it make us not
detect discrepancies like noexcept(false) vs. noexcept(true) on the same decl.


OK, fair enough.


@@ -20515,7 +20524,13 @@ cp_parser_init_declarator (cp_parser* parser,
/*asmspec=*/NULL_TREE,
attr_chainon (attributes, prefix_attributes));
if (decl && TREE_CODE (decl) == FUNCTION_DECL)
-   cp_parser_save_default_args (parser, decl);
+   {
+ cp_parser_save_default_args (parser, decl);
+ /* Remember if there is a noexcept-specifier to post process.  */
+ tree spec = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (decl));
+ if (UNPARSED_NOEXCEPT_SPEC_P (spec))
+   vec_safe_push (unparsed_noexcepts, decl);


Can we handle this in cp_parser_save_default_args rather than all its 
callers?



+/* Make sure that any member-function parameters are in scope.
+   For instance, a function's noexcept-specifier can use the function's
+   parameters:
+
+   struct S {
+ void fn (int p) noexcept(noexcept(p));
+   };
+
+   so we need to make sure name lookup can find them.  This is used
+   when we delay parsing of the noexcept-specifier.  */
+
+static void
+inject_parm_decls (tree decl)
+{
+  begin_scope (sk_function_parms, decl);
+  tree args = DECL_ARGUMENTS (decl);
+  args = nreverse (args);
+
+  tree next;
+  for (tree parm = args; parm; parm = next)
+{
+  next = DECL_CHAIN (parm);
+  if (TREE_CODE (parm) == PARM_DECL)
+   pushdecl (parm);
+}
+  /* Get the decls in their original chain order and record in the
+ function.  This is all and only the PARM_DECLs that were
+ pushed into scope by the loop above.  */
+  DECL_ARGUMENTS (decl) = get_local_decls ();
+}


Can we share this code with store_parm_decls instead of having two copies?


@@ -25227,6 +25431,18 @@ cp_parser_noexcept_specification_opt (cp_parser* 
parser,
+  /* [class.mem]/6 says that a noexcept-specifer (within the
+member-specification of the class) is a complete-class context of
+a class.  So, if the noexcept-specifier has the optional expression,
+just save the tokens, and reparse this after we're done with the
+class.  */
+  if (cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_OPEN_PAREN
+ && at_class_scope_p ()
+ && TYPE_BEING_DEFINED (current_class_type)
+ && !LAMBDA_TYPE_P (current_class_type))
+   return cp_parser_save_noexcept (parser);


We might optimize the noexcept() case, which should be pretty 
common.



+maybe_check_throw_specifier (tree overrider, tree basefn)


maybe_check_overriding_exception_spec


diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index df002a1664c..8cbc48fb44f 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -2393,6 +2393,12 @@ merge_exception_specifiers (tree list, tree add)
if (list == error_mark_node || add == error_mark_node)
  return error_mark_node;
  
+  /* We don't want to lose the unparsed operand lest we miss diagnostics.  */

+  if (UNPARSED_NOEXCEPT_SPEC_P (list))
+return list;
+  else if (UNPARSED_NOEXCEPT_SPEC_P (add))
+return add;


Here you're throwing away the other side, which seems wrong.

Jason


Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread Jakub Jelinek
On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
> > We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
> > it would remain a normal label, not a deleted label).
> >
> 
> But return value of func () may be used with indirect jump.

No, it may be used say to print that address, but computed goto can't be
used to jump from one function to a different function, see
https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
"You may not use this mechanism to jump to code in a different function.
If you do that, totally unpredictable things happen."

NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
code, the only reason it is kept is that there is or might be something
referencing the label and so you want to emit the label somewhere in the
function, but don't care much where in the function.

Let's look at weirdo testcases where we warn about by default with
-Wreturn-local-addr, but still the indirect jump is within the same
function:

void *
foo (void *x)
{
  if (!x)
{
  return &&bar;
bar:
  return 0;
}
  goto *x;
}

void *
qux (void *x, int y)
{
  if (y == 1)
{
  return &&bar;
bar:
  return (void *) 5;
}
  else if (y == 2)
{
  return &&baz;
baz:
  return (void *) 6;
}
  goto *x;
}

if called with foo (foo (0)) or qux (qux (0, 1)) or qux (qux (0, 2)).
In foo we emit NOTE_INSN_DELETED_LABEL, but just return that and optimize
away the actual goto *x, because we can prove what it will do.  In qux
we can't and so there is no NOTE_INSN_DELETED_LABEL.

Jakub


[PATCH] PR libstdc++/90634 reduce allocations in filesystem::path construction

2019-05-28 Thread Jonathan Wakely

PR libstdc++/90634
* src/filesystem/path.cc (path::_M_split_cmpts()): Fix check for "/".
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New test.

PR libstdc++/90634
* include/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
(path::_S_is_dir_sep()): Add missing 'static' keyword to function.
* include/experimental/bits/fs_path.h: Likewise.
* src/filesystem/path.cc (path::_M_split_cmpts()): Count number of
components and reserve space in vector. Return early when there is
only one component.
* src/filesystem/std-path.cc (path::_M_split_cmpts()): Likewise.

Tested x86_64-linux, committed to gcc-8-branch.


commit 1a21d448211978f10434ddad60bc72f70d1bd9ce
Author: redi 
Date:   Tue May 28 16:08:51 2019 +

Fix check for root-directory path and add tests

PR libstdc++/90634
* src/filesystem/path.cc (path::_M_split_cmpts()): Fix check for 
"/".
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New 
test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-8-branch@271712 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 872e292ecaa..8c876da2875 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,5 +1,10 @@
 2019-05-28  Jonathan Wakely  
 
+   PR libstdc++/90634
+   * src/filesystem/path.cc (path::_M_split_cmpts()): Fix check for "/".
+   * testsuite/27_io/filesystem/path/construct/90634.cc: New test.
+   * testsuite/experimental/filesystem/path/construct/90634.cc: New test.
+
PR libstdc++/90634
* include/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
diff --git a/libstdc++-v3/src/filesystem/path.cc 
b/libstdc++-v3/src/filesystem/path.cc
index ae29b29aab9..e4d339a1208 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -393,7 +393,7 @@ path::_M_split_cmpts()
  _M_add_root_dir(0);
}
}
-  else if (pos == len) // got root directory only
+  else if (len == 1) // got root directory only
{
  _M_type = _Type::_Root_dir;
  return;
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc 
b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc
new file mode 100644
index 000..dc31a86089e
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/90634.cc
@@ -0,0 +1,70 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17 -lstdc++fs" }
+// { dg-do run { target c++17 } }
+// { dg-require-filesystem-ts "" }
+
+#include 
+#include 
+#include 
+
+std::size_t bytes_allocated = 0;
+
+void* operator new(std::size_t n)
+{
+  bytes_allocated += n;
+  return std::malloc(n);
+}
+
+void operator delete(void* p) noexcept { std::free(p); }
+#if __cpp_sized_deallocation
+void operator delete(void* p, std::size_t) noexcept { std::free(p); }
+#endif
+
+void
+test01()
+{
+  std::string s0;
+  std::string s1 = "/";
+  std::string s2 = "///";
+  std::string s3 = "filename";
+  std::string s4 = "C:";
+  std::string s5 = "\\";
+
+  using std::filesystem::path;
+
+  bytes_allocated = 0;
+  path p0 = std::move(s0);
+  VERIFY( bytes_allocated == 0 );
+  path p1 = std::move(s1);
+  VERIFY( bytes_allocated == 0 );
+  path p2 = std::move(s2);
+  VERIFY( bytes_allocated == 0 );
+  path p3 = std::move(s3);
+  VERIFY( bytes_allocated == 0 );
+  path p4 = std::move(s4);
+  VERIFY( bytes_allocated == 0 );
+  path p5 = std::move(s5);
+  VERIFY( bytes_allocated == 0 );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git 
a/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc 
b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc
new file mode 100644
index 000..ef4a804de42
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/90634.cc
@@ -0,0 +1,67 @@
+// 

Re: [libstdc++,doc] doc/xml/manual/support.xml - link adjustment and simplification

2019-05-28 Thread Jonathan Wakely

On 26/05/19 19:46 +0200, Gerald Pfeifer wrote:

The links adjustment I would just have committed right away, but
I'd also like to suggest swe simplify the section: the following
paragraph doesn't really add much, but duplicates the external
link.

Thoughts?


It's the same link, but not the same reference. One is pointing to the
advice in the book, and one is pointing to the accompanying example
code that is available in the CDROM version of the book.

That whole file needs updates, it's all out of date (C++11 adds
nullptr as a better alternative to NULL, and in the next section the
list of six flavours is waaay out of date, as there are several
new overloads in recent standards).



Gerald

2019-05-26  Gerald Pfeifer  

* doc/xml/manual/support.xml: Adjust link to www.aristeia.com.
Shorten the section a bit.

Index: doc/xml/manual/support.xml
===
--- doc/xml/manual/support.xml  (revision 271632)
+++ doc/xml/manual/support.xml  (working copy)
@@ -170,7 +170,7 @@


In his book http://www.w3.org/1999/xlink";
-  xlink:href="http://www.aristeia.com/books.html";>Effective
+  xlink:href="https://www.aristeia.com/books.html";>Effective
  C++, Scott Meyers points out that the best way
to solve this problem is to not overload on pointer-vs-integer
types to begin with.  He also offers a way to make your own magic
@@ -177,11 +177,6 @@
NULL that will match pointers before it
matches integers.

-See the
-  http://www.w3.org/1999/xlink";
-  xlink:href="http://www.aristeia.com/books.html";>Effective
-  C++ CD example.
-
  




Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread Martin Sebor

On 5/28/19 4:24 AM, Martin Liška wrote:

On 5/28/19 11:31 AM, David CARLIER wrote:

Hi,

Here a tiny patch to fix few build warnings.

Kind regards.



Hi.

Well, I see a lot of these struct/class discrepancies when building GCC with 
LLVM.
Question is whether it worth changing?


I think it's nice for these to be spelled consistently and no benefit
to mixing and matching them.  If it cleans up common warnings I see
no reason not to make the change.

FWIW, it's also a common convention to use struct for PODs and class
for types with user-defined ctors, and even if GCC doesn't subscribe
to it, make a change in support of it is an improvement independent
of the Visual C++ warning.  (As might be adding such a warning to
GCC to help enforce the convention on projects that do follow it.)

Martin


[C++ PATCH] template specializations

2019-05-28 Thread Nathan Sidwell
In tracking a surprise on the modules branch, I came across this code in 
duplicate decls.  Why would the newly minted decl already have 
specializations?  That smells wrong.


This chainon call was added by Mark Mitchell back in 1998, (rev 21876) 
to deal with template friends of templates.  Since that time we've 
cleaned a lot of that code up, and in particular do not push the friend 
template until instantiation time.  I believe these changes make this 
chainon unneeded.


This patch changes it to assert there aren't any such specializations to 
concatenate.  I'm putting this on trunk to give it wider exposure than I 
can on the modules branch.  It has survived bootstrap on x86_64-linux.


nathan

--
Nathan Sidwell
2019-05-28  Nathan Sidwell  

	* decl.c (duplicate_decls): Assert a template newdecl has no
	specializations.

Index: cp/decl.c
===
--- cp/decl.c	(revision 271712)
+++ cp/decl.c	(working copy)
@@ -2026,7 +2026,8 @@ duplicate_decls (tree newdecl, tree oldd
   tree new_result = DECL_TEMPLATE_RESULT (newdecl);
   TREE_TYPE (olddecl) = TREE_TYPE (old_result);
-  DECL_TEMPLATE_SPECIALIZATIONS (olddecl)
-	= chainon (DECL_TEMPLATE_SPECIALIZATIONS (olddecl),
-		   DECL_TEMPLATE_SPECIALIZATIONS (newdecl));
+
+  /* The new decl should not already have gathered any
+	 specializations.  */
+  gcc_assert (!DECL_TEMPLATE_SPECIALIZATIONS (newdecl));
 
   DECL_ATTRIBUTES (old_result)


[PATCH][AArch64] Fix PR81800

2019-05-28 Thread Wilco Dijkstra
PR81800 is about the lrint inline giving spurious FE_INEXACT exceptions.
The previous change for PR81800 didn't fix this: when lrint is disabled
in the backend, the midend will simply use llrint.  This actually makes
things worse since llrint now also ignores FE_INVALID exceptions!
The fix is to disable lrint/llrint on double if the size of a long is
smaller (ie. ilp32).

Passes regress and bootstrap on AArch64. OK for commit?

ChangeLog
2018-11-13  Wilco Dijkstra

gcc/
PR target/81800
* gcc/config/aarch64/aarch64.md (lrint): Disable lrint pattern if GPF
operand is larger than a long int.

testsuite/
PR target/81800
* gcc.target/aarch64/no-inline-lrint_3.c: New test.

--

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



[PATCH][AArch64] Fix symbol offset limit

2019-05-28 Thread Wilco Dijkstra
In aarch64_classify_symbol symbols are allowed full-range offsets on 
relocations. 
This means the offset can use all of the +/-4GB offset, leaving no offset 
available
for the symbol itself.  This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xff00.

To avoid this, limit the offset to +/-1MB so that the symbol needs to be within 
a
3.9GB offset from its references.  For the tiny code model use a 64KB offset, 
allowing
most of the 1MB range for code/data between the symbol and its references.

Bootstrapped on AArch64, passes regress, OK for commit?

ChangeLog:
2018-11-09  Wilco Dijkstra  

gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.

testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.

--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
83453d03095018eddd1801e71ef3836849267444..0023cb37bbae5afe9387840c1bb6b43586d4fac2
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13047,26 +13047,26 @@ aarch64_classify_symbol (rtx x, HOST_WIDE_INT offset)
 the offset does not cause overflow of the final address.  But
 we have no way of knowing the address of symbol at compile time
 so we can't accurately say if the distance between the PC and
-symbol + offset is outside the addressible range of +/-1M in the
-TINY code model.  So we rely on images not being greater than
-1M and cap the offset at 1M and anything beyond 1M will have to
-be loaded using an alternative mechanism.  Furthermore if the
-symbol is a weak reference to something that isn't known to
-resolve to a symbol in this module, then force to memory.  */
+symbol + offset is outside the addressible range of +/-1MB in the
+TINY code model.  So we limit the maximum offset to +/-64KB and
+assume the offset to the symbol is not larger than +/-(1MB - 64KB).
+Furthermore force to memory if the symbol is a weak reference to
+something that doesn't resolve to a symbol in this module.  */
  if ((SYMBOL_REF_WEAK (x)
   && !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, -1048575, 1048575))
+ || !IN_RANGE (offset, -0x1, 0x1))
return SYMBOL_FORCE_TO_MEM;
+
  return SYMBOL_TINY_ABSOLUTE;
 
case AARCH64_CMODEL_SMALL:
  /* Same reasoning as the tiny code model, but the offset cap here is
-4G.  */
+1MB, allowing +/-3.9GB for the offset to the symbol.  */
  if ((SYMBOL_REF_WEAK (x)
   && !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (offset, HOST_WIDE_INT_C (-4294967263),
-   HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (offset, -0x10, 0x10))
return SYMBOL_FORCE_TO_MEM;
+
  return SYMBOL_SMALL_ABSOLUTE;
 
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c 
b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index 
d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5
 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=tiny" } */
 
-int fixed_regs[0x0020];
+char fixed_regs[0x0020];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x0008];
+  return fixed_regs[0x000ff000];
 }
 
 /* { dg-final { scan-assembler-not "adr\tx\[0-9\]+, fixed_regs\\\+" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range.c 
b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
index 
6574cf4310430b847e77ea56bf8f20ef312d53e4..75c87c12f08004c153efc5192e5cfab566c089db
 100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range.c
+++ b/gcc/testsuite/gcc.target/aarch64/symbol-range.c
@@ -1,12 +1,12 @@
-/* { dg-do compile } */
+/* { dg-do link } */
 /* { dg-options "-O3 -save-temps -mcmodel=small" } */
 
-int fixed_regs[0x2ULL];
+char fixed_regs[0x2ULL];
 
 int
-foo()
+main ()
 {
-  return fixed_regs[0x1ULL];
+  return fixed_regs[0xf000ULL];
 }
 
 /* { dg-final { scan-assembler-not "adrp\tx\[0-9\]+, fixed_regs\\\+" } } */



[PATCH v2] Fix PR64242

2019-05-28 Thread Wilco Dijkstra
Improve the fix for PR64242.  Various optimizations can change a memory 
reference
into a frame access.  Given there are multiple virtual frame pointers which may
be replaced by multiple hard frame pointers, there are no checks for writes to 
the
various frame pointers.  So updates to a frame pointer tends to generate 
incorrect
code.  Improve the previous fix to also add clobbers of several frame pointers 
and
add a scheduling barrier.  This should work in most cases until GCC supports a
generic "don't optimize across this instruction" feature.

Bootstrap OK. Testcase passes on AArch64 and x86-64.  Inspected x86, Arm,
Thumb-1 and Thumb-2 assembler which looks correct. 

ChangeLog:
2018-12-07  Wilco Dijkstra  

gcc/
PR middle-end/64242
* builtins.c (expand_builtin_longjmp): Add frame clobbers and schedule 
block.
(expand_builtin_nonlocal_goto): Likewise.

testsuite/
PR middle-end/64242
* gcc.c-torture/execute/pr64242.c: Update test.

--
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 
3f32754c4d35fc34af7c53156d2a356f69a94a8f..3463ffb153914a58e5baa3896a244842a28eef09
 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -1137,15 +1137,20 @@ expand_builtin_longjmp (rtx buf_addr, rtx value)
emit_insn (targetm.gen_nonlocal_goto (value, lab, stack, fp));
   else
{
- lab = copy_to_reg (lab);
-
  emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
  emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
+ lab = copy_to_reg (lab);
+
  /* Restore the frame pointer and stack pointer.  We must use a
 temporary since the setjmp buffer may be a local.  */
  fp = copy_to_reg (fp);
  emit_stack_restore (SAVE_NONLOCAL, stack);
+
+ /* Ensure the frame pointer move is not optimized.  */
+ emit_insn (gen_blockage ());
+ emit_clobber (hard_frame_pointer_rtx);
+ emit_clobber (frame_pointer_rtx);
  emit_move_insn (hard_frame_pointer_rtx, fp);
 
  emit_use (hard_frame_pointer_rtx);
@@ -1284,15 +1289,20 @@ expand_builtin_nonlocal_goto (tree exp)
 emit_insn (targetm.gen_nonlocal_goto (const0_rtx, r_label, r_sp, r_fp));
   else
 {
-  r_label = copy_to_reg (r_label);
-
   emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
   emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
+  r_label = copy_to_reg (r_label);
+
   /* Restore the frame pointer and stack pointer.  We must use a
 temporary since the setjmp buffer may be a local.  */
   r_fp = copy_to_reg (r_fp);
   emit_stack_restore (SAVE_NONLOCAL, r_sp);
+
+  /* Ensure the frame pointer move is not optimized.  */
+  emit_insn (gen_blockage ());
+  emit_clobber (hard_frame_pointer_rtx);
+  emit_clobber (frame_pointer_rtx);
   emit_move_insn (hard_frame_pointer_rtx, r_fp);
 
   /* USE of hard_frame_pointer_rtx added for consistency;
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr64242.c 
b/gcc/testsuite/gcc.c-torture/execute/pr64242.c
index 
46a7b23d28d71604d141281c21fb0b77849b1b0d..e6139ede3f34d587ac53d04e286e5d75fd2ca76c
 100644
--- a/gcc/testsuite/gcc.c-torture/execute/pr64242.c
+++ b/gcc/testsuite/gcc.c-torture/execute/pr64242.c
@@ -5,46 +5,31 @@ extern void abort (void);
 __attribute ((noinline)) void
 broken_longjmp (void *p)
 {
-  void *buf[5];
+  void *buf[32];
   __builtin_memcpy (buf, p, 5 * sizeof (void*));
   /* Corrupts stack pointer...  */
   __builtin_longjmp (buf, 1);
 }
 
-__attribute ((noipa)) __UINTPTR_TYPE__
-foo (void *p)
-{
-  return (__UINTPTR_TYPE__) p;
-}
-
-__attribute ((noipa)) void
-bar (void *p)
-{
-  asm volatile ("" : : "r" (p));
-}
-
 volatile int x = 0;
-void *volatile p;
-void *volatile q;
+char *volatile p;
+char *volatile q;
 
 int
 main ()
 {
   void *buf[5];
-  struct __attribute__((aligned (32))) S { int a[4]; } s;
-  bar (&s);
   p = __builtin_alloca (x);
+  q = __builtin_alloca (x);
   if (!__builtin_setjmp (buf))
 broken_longjmp (buf);
 
+  /* Compute expected next alloca offset - some targets don't align properly
+ and allocate too much.  */
+  p = q + (q - p);
+
   /* Fails if stack pointer corrupted.  */
-  q = __builtin_alloca (x);
-  if (foo (p) < foo (q))
-{
-  if (foo (q) - foo (p) >= 1024)
-   abort ();
-}
-  else if (foo (p) - foo (q) >= 1024)
+  if (p != __builtin_alloca (x))
 abort ();
 
   return 0;



Re: V2 [PATCH] i386: Insert ENDBR for NOTE_INSN_DELETED_LABEL only if needed

2019-05-28 Thread Jeff Law
On 5/28/19 9:48 AM, Jakub Jelinek wrote:
> On Tue, May 28, 2019 at 08:30:59AM -0700, H.J. Lu wrote:
>>> We shouldn't generate ENDBR in that case, nothing can goto to bar (otherwise
>>> it would remain a normal label, not a deleted label).
>>>
>>
>> But return value of func () may be used with indirect jump.
> 
> No, it may be used say to print that address, but computed goto can't be
> used to jump from one function to a different function, see
> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> "You may not use this mechanism to jump to code in a different function.
> If you do that, totally unpredictable things happen."
Right.  We disallowed this case some time ago IIRC.  It's essentially
undefined behavior.  I would even claim that in a CET world that we
*want* a CET fault if something tried to use the deleted label as a jump
target and thus an ENDBR is undesirable.


> 
> NOTE_INSN_DELETED_LABEL is not guaranteed to be followed by any sensible
> code, the only reason it is kept is that there is or might be something
> referencing the label and so you want to emit the label somewhere in the
> function, but don't care much where in the function.
Exactly.

Jeff


[PATCH] Fix PR84521

2019-05-28 Thread Wilco Dijkstra
This fixes and simplifies the setjmp and non-local goto implementation.
Currently the virtual frame pointer is saved when using __builtin_setjmp or
a non-local goto.  Depending on whether a frame pointer is used, this may
either save SP or FP with an immediate offset.  However the goto or longjmp
always updates the hard frame pointer.

A receiver veneer in the original function then assigns the hard frame pointer
to the virtual frame pointer, which should, if it works correctly, again assign
SP or FP.  However the special elimination code in eliminate_regs_in_insn
doesn't do this correctly unless the frame pointer is used, and even if it
worked by writing SP, the frame pointer would still be corrupted.

A much simpler implementation is to always save and restore the hard frame
pointer.  This avoids 2 redundant instructions which add/subtract the virtual
frame offset.  A large amount of code can be removed as a result, including all
implementations of TARGET_BUILTIN_SETJMP_FRAME_VALUE (all of which already use
the hard frame pointer).  The expansion of nonlocal_goto on PA can be simplied
to just restore the hard frame pointer. 

This fixes the most obvious issues, however there are still issues on targets
which define HARD_FRAME_POINTER_IS_FRAME_POINTER (arm, mips, xtensa).
Each function could have a different hard frame pointer, so a non-local goto
may restore the wrong frame pointer (TARGET_BUILTIN_SETJMP_FRAME_VALUE could
be useful for this).

The i386 TARGET_BUILTIN_SETJMP_FRAME_VALUE was incorrect: if stack_realign_fp
is true, it would save the hard frame pointer value but restore the virtual
frame pointer which according to ix86_initial_elimination_offset can have a
non-zero offset from the hard frame pointer.

The ia64 implementation of nonlocal_goto seems incorrect since the helper
function moves the the frame pointer value into the static chain register
(so this patch does nothing to make it better or worse).

AArch64 bootstrap OK, new test passes on AArch64, x86-64 and Arm.

ChangeLog:
2018-12-13  Wilco Dijkstra  

gcc/
PR middle-end/84521
* builtins.c (expand_builtin_setjmp_setup): Save hard_frame_pointer_rtx.
(expand_builtin_setjmp_receiver): Do not emit sfp = fp move since we 
restore fp.
* function.c (expand_function_start): Save hard_frame_pointer_rtx for 
non-local goto.
* lra-eliminations.c (eliminate_regs_in_insn): Remove sfp = fp 
elimination code.
(remove_reg_equal_offset_note): Remove unused function.
* reload1.c (eliminate_regs_in_insn): Remove sfp = hfp elimination code.
* config/arc/arc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(arc_builtin_setjmp_frame_value): Remove function.
* config/avr/avr.c  (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(avr_builtin_setjmp_frame_value): Remove function.
* config/i386/i386.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(ix86_builtin_setjmp_frame_value): Remove function.
* config/pa/pa.md (nonlocal_goto): Remove FP adjustment.
* config/sparc/sparc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(sparc_builtin_setjmp_frame_value): Remove function.
* config/vax/vax.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Remove.
(vax_builtin_setjmp_frame_value): Remove function.

testsuite/
PR middle-end/84521
* gcc.c-torture/execute/pr84521.c: New test.

--

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 
3463ffb153914a58e5baa3896a244842a28eef09..4ecfd49d03cb34600e7ac7afb0bf0511ff7ec1fa
 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -981,7 +981,7 @@ expand_builtin_setjmp_setup (rtx buf_addr, rtx 
receiver_label)
 
   mem = gen_rtx_MEM (Pmode, buf_addr);
   set_mem_alias_set (mem, setjmp_alias_set);
-  emit_move_insn (mem, targetm.builtin_setjmp_frame_value ());
+  emit_move_insn (mem, hard_frame_pointer_rtx);
 
   mem = gen_rtx_MEM (Pmode, plus_constant (Pmode, buf_addr,
   GET_MODE_SIZE (Pmode))),
@@ -1023,31 +1023,6 @@ expand_builtin_setjmp_receiver (rtx receiver_label)
   if (chain && REG_P (chain))
 emit_clobber (chain);
 
-  /* Now put in the code to restore the frame pointer, and argument
- pointer, if needed.  */
-  if (! targetm.have_nonlocal_goto ())
-{
-  /* First adjust our frame pointer to its actual value.  It was
-previously set to the start of the virtual area corresponding to
-the stacked variables when we branched here and now needs to be
-adjusted to the actual hardware fp value.
-
-Assignments to virtual registers are converted by
-instantiate_virtual_regs into the corresponding assignment
-to the underlying register (fp in this case) that makes
-the original assignment true.
-So the following insn will actually be decrementing fp by
-TARGET_STARTING_FRAME_OFFSET.  */
-  emit_move_insn (virtual_stack_vars_rtx, hard_frame_pointer_rtx);
-
-  /* 

Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

2019-05-28 Thread Martin Sebor

On 5/24/19 9:49 AM, Alex Henrie wrote:

On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:


I'm not sure we really need a new warning for this.


On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:


I don't think GCC makes a formal distinction between function
attributes that affect only function definitions vs those that
affect its users, or both.  It might be a useful distinction
to introduce, perhaps even for functions as well as variables,
but as it is, users (as well as GCC developers) are on our own
to figure it out.


Then is it preferable to simply silence Wattributes in this case?


This case being PR86407?  I'd say yes.  I think one general problem
to solve is the missing suppression for typedefs.  This could be
useful for other warnings as well.  Another is providing a knob to
control the warning when one kind of an attribute is used with
an entity of a different kind (function vs type, or variable vs
type).  Yet another might be to control warnings for individual
attributes.



On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:



+int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
+


But this is a declaration?


On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:


My first question is: what is the meaning of "function definition
attributes?"  Presumably those that affect only the definition of
a function and not its callers or other users?


As far as I can tell, "fndecl" is a misnomer: these attributes are
more accurately called "function definition attributes", i.e.
attributes that affect the assembly code of the function but do not
affect its calling convention.


That's one possible definition but there are examples that don't
fit it (at least not very neatly).

Attribute malloc attaches only to fndecl and not its type but
doesn't affect the code for a function definition.  FWIW, I
think this is just a bug -- attribute malloc should apply
to a function type for the same reason the closely related
attribute alloc_size does.

Attribute constructor also attaches to a fndecl even though it
doesn't affect the function's codegen but that of its caller
(the runtime).  In this case, though, I'd say that's fine.
Should it be classified as a function definition attribute?

Attributes cold and hot also apply to a fndecl and not its type
but they affect both the function's definition and its callers.
I think this also makes sense.  Should they be classified as
function definition attributes?


On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:


Finally, with this as a prerequisite, if we decided that a warning
like this would be useful, tests to verify that it works for all
the definition attributes and not for the rest would need to be
added (i.e., in addition to ms_hook_prologue).


Okay, I will add tests for the other function attributes that should
behave in the same way, commenting out the tests that will require
more work to pass.

The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
Wine without causing spurious warnings. This will go a long way
towards making Wine compatible with current and future Windows
programs. Thank you for help.


Yes, I understand the goal.  I'm not sure that the proposed change
is the right way to do it.  It seems to me that a more targeted bug
to fix with the broadest benefit is that _Pragma GCC diagnostic (or
some such mechanism) cannot be used to suppress the warning for
typedefs.

Martin


Re: [PATCH] PR libstdc++/90557 fix path assignment that alters source

2019-05-28 Thread Jonathan Wakely

On 22/05/19 23:14 +0100, Jonathan Wakely wrote:

PR libstdc++/90557
* src/c++17/fs_path.cc (path::_List::operator=(const _List&)): Fix
reversed arguments to uninitialized_copy_n.
* testsuite/27_io/filesystem/path/assign/copy.cc: Check that source
is unchanged by copy assignment.
* testsuite/util/testsuite_fs.h (compare_paths): Use std::equal to
compare path components.


I should have used the three-argument version of std::equal, because
the four-argument one is not available in C++11.

Fixed with this patch, tested powerpc64le-linux and committed to
trunk. This needs to be backported togcc-9-branch too.


commit ee46741a1ff05926900d655273ef7f84054e3f62
Author: Jonathan Wakely 
Date:   Tue May 28 15:52:58 2019 +0100

Fix C++14-only code in testsuite utility

* testsuite/util/testsuite_fs.h (compare_paths): Use three-argument
form of std::equals for C++11 compatibility.

diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
index b2a5ee6e655..fe42845ac4f 100644
--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
@@ -67,9 +67,9 @@ namespace __gnu_test
   throw test_fs::filesystem_error(
 	  "distance(begin1, end1) != distance(begin2, end2)", p1, p2,
 	  std::make_error_code(std::errc::invalid_argument) );
-if (!std::equal(p1.begin(), p1.end(), p2.begin(), p2.end()))
+if (!std::equal(p1.begin(), p1.end(), p2.begin()))
   throw test_fs::filesystem_error(
-	  "!equal(begin1, end1, begin2, end2)", p1, p2,
+	  "!equal(begin1, end1, begin2)", p1, p2,
 	  std::make_error_code(std::errc::invalid_argument) );
 
   }


Re: [PATCH] PR libstdc++/90634 reduce allocations in filesystem::path construction

2019-05-28 Thread Jonathan Wakely

On 28/05/19 17:10 +0100, Jonathan Wakely wrote:

PR libstdc++/90634
* src/filesystem/path.cc (path::_M_split_cmpts()): Fix check for "/".
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New test.

PR libstdc++/90634
* include/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
(path::_S_is_dir_sep()): Add missing 'static' keyword to function.
* include/experimental/bits/fs_path.h: Likewise.
* src/filesystem/path.cc (path::_M_split_cmpts()): Count number of
components and reserve space in vector. Return early when there is
only one component.
* src/filesystem/std-path.cc (path::_M_split_cmpts()): Likewise.

Tested x86_64-linux, committed to gcc-8-branch.


This is the simpler patch for trunk and gcc-9-branch, which only needs
to change std::experimental::filesystem::path, because
std::filesystem::path already avoids any unnecessary allocations.

Tested powerpc64le-linux and x86_64-w64-mingw32 Committed to trunk so
far, backport to follow.


commit cdbf389b9485c3212a59605442c392dcde76636a
Author: Jonathan Wakely 
Date:   Tue May 28 16:32:57 2019 +0100

PR libstdc++/90634 reduce allocations in filesystem::path construction

PR libstdc++/90634
* include/experimental/bits/fs_path.h (path::path(path&&)): Only call
_M_split_cmpts() for a path with multiple components.
(path::_S_is_dir_sep()): Add missing 'static' keyword to function.
* src/filesystem/path.cc (path::_M_split_cmpts()): Count number of
components and reserve space in vector. Return early when there is
only one component.
* testsuite/27_io/filesystem/path/construct/90634.cc: New test.
* testsuite/experimental/filesystem/path/construct/90634.cc: New test.

diff --git a/libstdc++-v3/include/experimental/bits/fs_path.h b/libstdc++-v3/include/experimental/bits/fs_path.h
index 588f06822be..9a68a272e34 100644
--- a/libstdc++-v3/include/experimental/bits/fs_path.h
+++ b/libstdc++-v3/include/experimental/bits/fs_path.h
@@ -195,7 +195,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 path(path&& __p) noexcept
 : _M_pathname(std::move(__p._M_pathname)), _M_type(__p._M_type)
 {
-  _M_split_cmpts();
+  if (_M_type == _Type::_Multi)
+	_M_split_cmpts();
   __p.clear();
 }
 
@@ -490,7 +491,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	return _S_convert_loc(__s.data(), __s.data() + __s.size(), __loc);
   }
 
-bool _S_is_dir_sep(value_type __ch)
+static bool _S_is_dir_sep(value_type __ch)
 {
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
   return __ch == L'/' || __ch == preferred_separator;
diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index 92def10e33e..dfc3bd53c00 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -340,6 +340,28 @@ path::_M_split_cmpts()
   if (_M_pathname.empty())
 return;
 
+  {
+// Approximate count of components, to reserve space in _M_cmpts vector:
+int count = 1;
+bool saw_sep_last = _S_is_dir_sep(_M_pathname[0]);
+bool saw_non_sep = !saw_sep_last;
+for (value_type c : _M_pathname)
+  {
+   if (_S_is_dir_sep(c))
+ saw_sep_last = true;
+   else if (saw_sep_last)
+ {
+   ++count;
+   saw_sep_last = false;
+   saw_non_sep = true;
+ }
+  }
+if (saw_non_sep && saw_sep_last)
+  ++count; // empty filename after trailing slash
+if (count > 1)
+  _M_cmpts.reserve(count);
+  }
+
   size_t pos = 0;
   const size_t len = _M_pathname.size();
 
@@ -362,9 +384,13 @@ path::_M_split_cmpts()
 	  pos = 3;
 	  while (pos < len && !_S_is_dir_sep(_M_pathname[pos]))
 		++pos;
+	  if (pos == len)
+		{
+		  _M_type = _Type::_Root_name;
+		  return;
+		}
 	  _M_add_root_name(pos);
-	  if (pos < len) // also got root directory
-		_M_add_root_dir(pos);
+	  _M_add_root_dir(pos);
 	}
 	  else
 	{
@@ -373,6 +399,11 @@ path::_M_split_cmpts()
 	  _M_add_root_dir(0);
 	}
 	}
+  else if (len == 1) // got root directory only
+	{
+	  _M_type = _Type::_Root_dir;
+	  return;
+	}
   else // got root directory
 	_M_add_root_dir(0);
   ++pos;
@@ -381,12 +412,28 @@ path::_M_split_cmpts()
   else if (len > 1 && _M_pathname[1] == L':')
 {
   // got disk designator
+  if (len == 2)
+	{
+	  _M_type = _Type::_Root_name;
+	  return;
+	}
   _M_add_root_name(2);
   if (len > 2 && _S_is_dir_sep(_M_pathname[2]))
 	_M_add_root_dir(2);
   pos = 2;
 }
 #endif
+  else
+{
+  size_t n = 1;
+  for (; n < _M_pathname.size() && !_S_is_dir_sep(_M_pathname[n]); ++n)
+	{ }
+  if (n == _M_pathname.size())
+	{
+	  _M_type = _Type::_Filename;
+	  return;
+	}
+}
 
   

Re: Question on adding an option control to libcpp

2019-05-28 Thread Qing Zhao
Hi, David,

for this new option, do I need to add a new testing case for it?
if so, where should I put this new testing case?

thanks.

Qing
> On May 24, 2019, at 1:47 PM, David Malcolm  wrote:
> 
> On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:
>> Hi, 
>> 
>> in order to fix PR90581: (provide an option to adjust the maximum
>> depth of nested #include)
>> 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 > rg/bugzilla/show_bug.cgi?id=90581>
>> 
>> we need to add a new option to preprocessor.  where should I put this
>> option?
>> 
>> I tried to add a new option into:
>> 
>> gcc/c-family/c.opt
>> 
>> [qinzhao@localhost c-family]$ git diff c.opt
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 046d489..4f237b6 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -1598,6 +1598,10 @@ flocal-ivars
>> ObjC ObjC++ Var(flag_local_ivars) Init(1)
>> Allow access to instance variables as if they were local
>> declarations within instance method implementati
>> 
>> +finclude-nest-limit=
>> +C ObjC C++ ObjC++ Joined RejectNegative UInteger
>> Var(include_nest_limit) Init(200)
>> +Set the maximum number of depth of nested #include.
>> +
>> 
>> However, don’t know how to refer this new variable
>> “include_nest_limit” from libcpp.
>> 
> 
> You probably want to add a new field to cpp_options, and copy over the
> value from the GCC options to the new field; see e.g.
> c_common_handle_option where various cases write to fields of cpp_opts.
> 
> Hope this is helpful
> Dave



Re: [PATCH] Fix few build warnings with LLVM toolchain

2019-05-28 Thread Eric Gallager
On 5/28/19, Martin Sebor  wrote:
> On 5/28/19 4:24 AM, Martin Liška wrote:
>> On 5/28/19 11:31 AM, David CARLIER wrote:
>>> Hi,
>>>
>>> Here a tiny patch to fix few build warnings.
>>>
>>> Kind regards.
>>>
>>
>> Hi.
>>
>> Well, I see a lot of these struct/class discrepancies when building GCC
>> with LLVM.
>> Question is whether it worth changing?
>
> I think it's nice for these to be spelled consistently and no benefit
> to mixing and matching them.  If it cleans up common warnings I see
> no reason not to make the change.
>
> FWIW, it's also a common convention to use struct for PODs and class
> for types with user-defined ctors, and even if GCC doesn't subscribe
> to it, make a change in support of it is an improvement independent
> of the Visual C++ warning.  (As might be adding such a warning to
> GCC to help enforce the convention on projects that do follow it.)
>
> Martin
>

The bug for discussing -Wmismatched-tags is bug 61339, FWIW:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61339


Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Christophe Lyon
Hi Martin,


On Tue, 28 May 2019 at 13:30, Martin Liška  wrote:
>
> On 5/27/19 2:50 PM, Jakub Jelinek wrote:
> > On Mon, May 27, 2019 at 02:18:37PM +0200, Martin Liška wrote:
> >> +/* Compare loop information for basic blocks BB1 and BB2.  */
> >> +
> >> +bool
> >> +func_checker::compare_loops (basic_block bb1, basic_block bb2)
> >> +{
> >> +  if ((bb1->loop_father == NULL) != (bb2->loop_father == NULL))
> >> +return return_false ();
> >> +
> >> +  struct loop *l1 = bb1->loop_father;
> >> +  struct loop *l2 = bb2->loop_father;
> >
> > Perhaps also
> >   if ((bb1 == l1->header) != (bb2 == l2->header))
> > return return_false_with_msg ("header");
> >   if ((bb1 == l1->latch) != (bb2 == l2->latch))
> > return return_false_with_msg ("latch");
> > too?
>
> Yes, makes sense.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>

The new testcase contains:
/* { dg-do compile } */
/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
/* { dg-do compile { target i?86-*-* x86_64-*-* } } */

I suspect line 3 should actually be line 1, because the test fails to
compile on non-x86 targets:
xgcc: error: unrecognized command line option '-mavx512f'

Was it your intention?

Thanks,

Christophe

> >
> > BTW, unrelated to this patch, what does ICF do if e.g. SSA_NAME_PTR_INFO
> > or SSA_NAME_RANGE_INFO is different between otherwise identical functions?
> > Say one having early
> >   if (arg > 10)
> > __builtin_unreachable ();
> > and another one
> >   if (arg > - 10)
> > __builtin_unreachable ();
> > both optimized away into SSA_NAME_RANGE_INFO of the argument before IPA (or
> > do we optimize that away only after IPA?).
> >
> >   Jakub
> >
>
> Can you please provide a self-contained test-case?
>
> Thanks,
> Martin


Re: [C++ Patch] PR 89875 ("[7/8/9/10 Regression] invalid typeof reference to a member of an incomplete struct accepted at function scope")

2019-05-28 Thread Paolo Carlini

Hi,

On 28/05/19 16:47, Jason Merrill wrote:

On 5/10/19 10:29 AM, Paolo Carlini wrote:

Hi,

a while ago Martin noticed that an unintended consequence of an old 
tweak of mine - which avoided redundant error messages emitted from 
cp_parser_init_declarator - is that, in some cases, we started 
accepting ill-formed typeofs. Luckily, decltype isn't affected and 
that points to the real issue: by the time that place in 
cp_parser_init_declarator is reached, for a decltype version we 
already emitted a correct error message. Thus I think the right way 
to fix the problem is simply committing to tentative parse when, 
toward the end of cp_parser_sizeof_operand we know that we must be 
looking at a (possibly ill-formed) expression. Tested x86_64-linux.


The problem with calling cp_parser_commit_to_tentative_parse here is 
that the tentative parse you're committing to is for the enclosing 
scope, which is trying to decide whether e.g. we're parsing a 
declaration or expression.  If the operand of typeof is a well-formed 
expression, and the larger context is an expression, this will break.


Better, I think, to commit and re-parse only if you actually encounter 
an error.


Alternately, cp_parser_decltype_expr deals with this by using a 
tentative firewall and CPP_DECLTYPE; cp_parser_sizeof_operand could do 
the same, but that seems like a bigger hammer.


Today I spent quite a bit of time on this and eventually decided to 
follow the example of decltype as closely as possible. Then I started 
tweaking those drafts which laready passed the testsuite and after a 
while ended up with the below, rather close to the current code, in 
fact. Testing !cp_parser_error_occurred and in case calling 
cp_parser_abort_tentative_parse by hand (closer to the decltype example) 
also works. What do you think? Thanks, Paolo.


///


Index: cp/parser.c
===
--- cp/parser.c (revision 271692)
+++ cp/parser.c (working copy)
@@ -28942,6 +28942,8 @@ cp_parser_sizeof_operand (cp_parser* parser, enum
 {
   tree type = NULL_TREE;
 
+  tentative_firewall firewall (parser);
+
   /* We can't be sure yet whether we're looking at a type-id or an
 expression.  */
   cp_parser_parse_tentatively (parser);
@@ -28969,11 +28971,15 @@ cp_parser_sizeof_operand (cp_parser* parser, enum
   /* If all went well, then we're done.  */
   if (cp_parser_parse_definitely (parser))
expr = type;
+  else
+   {
+ /* Commit to the tentative_firewall so we get syntax errors.  */
+ cp_parser_commit_to_tentative_parse (parser);
+
+ expr = cp_parser_unary_expression (parser);
+   }
 }
-
-  /* If the type-id production did not work out, then we must be
- looking at the unary-expression production.  */
-  if (!expr)
+  else
 expr = cp_parser_unary_expression (parser);
 
   /* Go back to evaluating expressions.  */
Index: testsuite/g++.dg/cpp0x/decltype-pr66548.C
===
--- testsuite/g++.dg/cpp0x/decltype-pr66548.C   (revision 271692)
+++ testsuite/g++.dg/cpp0x/decltype-pr66548.C   (working copy)
@@ -11,7 +11,7 @@ struct Meow {};
 
 void f ()
 {
-  decltype (Meow.purr ()) d;   // { dg-error "expected primary-expression" 
"pr89875" { xfail c++98_only } }
+  decltype (Meow.purr ()) d;   // { dg-error "expected primary-expression" }
   (void)&d;
 }
 
Index: testsuite/g++.dg/template/sizeof-template-argument.C
===
--- testsuite/g++.dg/template/sizeof-template-argument.C(revision 
271692)
+++ testsuite/g++.dg/template/sizeof-template-argument.C(working copy)
@@ -3,9 +3,9 @@
 
 template struct A {};
 
-template struct B : A  {}; /* { dg-error "template 
argument" } */
+template struct B : A  {}; /* { dg-error "expected 
primary-expression" } */
 
-template struct C : A  {}; /* { dg-error "template 
argument" } */
+template struct C : A  {}; /* { dg-error "expected 
primary-expression" } */
 
 int a;
 


libgomp: long known as the GNU Offloading and Multi Processing Runtime Library (was: libgomp: Now known as the GNU Offloading and Multi Processing Runtime Library (was: libgomp: "GNU OpenMP Runtime Li

2019-05-28 Thread Thomas Schwinge
Hi!

On Thu, 29 Jan 2015 09:28:38 +0100, I wrote:
> On Sat, 10 Jan 2015 20:21:46 +0100, I wrote:
> > On Wed, 12 Nov 2014 15:43:06 -0500, David Malcolm  
> > wrote:
> > > On Wed, 2014-11-12 at 21:30 +0100, Jakub Jelinek wrote:
> > > > On Wed, Nov 12, 2014 at 03:22:21PM -0500, David Malcolm wrote:
> > > > > On Wed, 2014-11-12 at 14:47 +0100, Jakub Jelinek wrote:
> > > > > > On Wed, Nov 12, 2014 at 08:33:34AM -0500, David Malcolm wrote:
> > > > > > > Apologies for bikeshedding, and I normally dislike "cute" names, 
> > > > > > > but
> > > > > > > renaming it to
> > > > > > > 
> > > > > > >"GNU Offloading and Multi Processing library"
> > 
> > Oh, how cute!  ;-P
> > 
> > > > > > > would allow a backronym of "libgomp", thus preserving the existing
> > > > > > > filenames/SONAME etc.

> [...] and committed to wwwdocs:
> 
> Index: htdocs/onlinedocs/index.html
> ===
> RCS file: /cvs/gcc/wwwdocs/htdocs/onlinedocs/index.html,v
> retrieving revision 1.145
> retrieving revision 1.146
> diff -u -p -r1.145 -r1.146
> --- htdocs/onlinedocs/index.html  19 Dec 2014 13:24:58 -  1.145
> +++ htdocs/onlinedocs/index.html  29 Jan 2015 08:26:06 -  1.146
> @@ -957,8 +957,8 @@ existing release.
> href="https://gcc.gnu.org/onlinedocs/gccgo.ps.gz";>PostScript 
> or  href="https://gcc.gnu.org/onlinedocs/gccgo-html.tar.gz";>an
> HTML tarball)
> -https://gcc.gnu.org/onlinedocs/libgomp/";>GNU OpenMP
> -   Manual ( +https://gcc.gnu.org/onlinedocs/libgomp/";>GNU Offloading and
> +   Multi Processing Runtime Library Manual ( href="https://gcc.gnu.org/onlinedocs/libgomp.pdf";>also in
> PDF or  
> href="https://gcc.gnu.org/onlinedocs/libgomp.ps.gz";>PostScript or  
> It remains to be seen if for the GCC 5.0 release, the text is copied from
> the 4.9.2 section at the top of the file, or from the current development
> section at the bottom (which I just changed).  ;-)

Well... ;-) -- that didn't work out.  I now committed the following
changes, "libgomp: long known as the GNU Offloading and Multi Processing
Runtime Library":

Index: htdocs/onlinedocs/index.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/onlinedocs/index.html,v
retrieving revision 1.177
retrieving revision 1.178
diff -u -p -r1.177 -r1.178
--- htdocs/onlinedocs/index.html3 May 2019 09:05:54 -   1.177
+++ htdocs/onlinedocs/index.html28 May 2019 21:01:16 -  1.178
@@ -78,7 +78,7 @@
href="https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-9.1.0/libgomp/";>GCC 9.1
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-9.1.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-9.1.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-8.3.0/libgomp/";>GCC 8.3
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-8.3.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-8.3.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-7.4.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-7.4.0/libgomp/";>GCC 7.4
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-7.4.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-7.4.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-6.5.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-6.5.0/libgomp/";>GCC 6.5
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-6.5.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-6.5.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-5.5.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-5.5.0/libgomp/";>GCC 5.5
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-5.5.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-5.5.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-5.1.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-5.1.0/libgomp/";>GCC 5.1
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-5.1.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-5.1.0/libgomp.ps.gz";>PostScript or 
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gccgo-html.tar.gz";>an
HTML tarball)
 https://gcc.gnu.org/onlinedocs/gcc-5.2.0/libgomp/";>GCC 5.2
- GNU OpenMP Manual ( (https://gcc.gnu.org/onlinedocs/gcc-5.2.0/libgomp.pdf";>also in
  PDF or https://gcc.gnu.org/onlinedocs/gcc-5.2.0/libgomp.ps.gz";>Post

Re: Question on adding an option control to libcpp

2019-05-28 Thread David Malcolm
On Tue, 2019-05-28 at 15:08 -0500, Qing Zhao wrote:
> Hi, David,
> 
> for this new option, do I need to add a new testing case for it?

Yes please.  Ideally we should add at least one test case for any new
option, especially if it's easy to test for.

> if so, where should I put this new testing case?

It's probably testable for in DejaGnu with a testcase that sets the
maximum depth to some very low limit (say just a single include), and
then have a testcase that has a depth of two.

Or something like that, though DejaGnu's line-based directives aren't a
perfect fit for diagnostics occurring in included files.

Maybe such tests should live in gcc/testsuite/c-c++-common/cpp/

I think there's something similar in:
  gcc/testsuite/c-c++-common/inc-from-1.c

Dave

> thanks.
> 
> Qing
> > On May 24, 2019, at 1:47 PM, David Malcolm 
> > wrote:
> > 
> > On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:
> > > Hi, 
> > > 
> > > in order to fix PR90581: (provide an option to adjust the maximum
> > > depth of nested #include)
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581  > > nu.o
> > > rg/bugzilla/show_bug.cgi?id=90581>
> > > 
> > > we need to add a new option to preprocessor.  where should I put
> > > this
> > > option?
> > > 
> > > I tried to add a new option into:
> > > 
> > > gcc/c-family/c.opt
> > > 
> > > [qinzhao@localhost c-family]$ git diff c.opt
> > > diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> > > index 046d489..4f237b6 100644
> > > --- a/gcc/c-family/c.opt
> > > +++ b/gcc/c-family/c.opt
> > > @@ -1598,6 +1598,10 @@ flocal-ivars
> > > ObjC ObjC++ Var(flag_local_ivars) Init(1)
> > > Allow access to instance variables as if they were local
> > > declarations within instance method implementati
> > > 
> > > +finclude-nest-limit=
> > > +C ObjC C++ ObjC++ Joined RejectNegative UInteger
> > > Var(include_nest_limit) Init(200)
> > > +Set the maximum number of depth of nested #include.
> > > +
> > > 
> > > However, don’t know how to refer this new variable
> > > “include_nest_limit” from libcpp.
> > > 
> > 
> > You probably want to add a new field to cpp_options, and copy over
> > the
> > value from the GCC options to the new field; see e.g.
> > c_common_handle_option where various cases write to fields of
> > cpp_opts.
> > 
> > Hope this is helpful
> > Dave
> 
> 


Re: Question on adding an option control to libcpp

2019-05-28 Thread Qing Zhao
Okay.

thanks, I will add testing cases.

Qing
> On May 28, 2019, at 4:26 PM, David Malcolm  wrote:
> 
> On Tue, 2019-05-28 at 15:08 -0500, Qing Zhao wrote:
>> Hi, David,
>> 
>> for this new option, do I need to add a new testing case for it?
> 
> Yes please.  Ideally we should add at least one test case for any new
> option, especially if it's easy to test for.
> 
>> if so, where should I put this new testing case?
> 
> It's probably testable for in DejaGnu with a testcase that sets the
> maximum depth to some very low limit (say just a single include), and
> then have a testcase that has a depth of two.
> 
> Or something like that, though DejaGnu's line-based directives aren't a
> perfect fit for diagnostics occurring in included files.
> 
> Maybe such tests should live in gcc/testsuite/c-c++-common/cpp/
> 
> I think there's something similar in:
>  gcc/testsuite/c-c++-common/inc-from-1.c
> 
> Dave
> 
>> thanks.
>> 
>> Qing
>>> On May 24, 2019, at 1:47 PM, David Malcolm 
>>> wrote:
>>> 
>>> On Fri, 2019-05-24 at 11:59 -0500, Qing Zhao wrote:
 Hi, 
 
 in order to fix PR90581: (provide an option to adjust the maximum
 depth of nested #include)
 
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90581 
 
 we need to add a new option to preprocessor.  where should I put
 this
 option?
 
 I tried to add a new option into:
 
 gcc/c-family/c.opt
 
 [qinzhao@localhost c-family]$ git diff c.opt
 diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
 index 046d489..4f237b6 100644
 --- a/gcc/c-family/c.opt
 +++ b/gcc/c-family/c.opt
 @@ -1598,6 +1598,10 @@ flocal-ivars
 ObjC ObjC++ Var(flag_local_ivars) Init(1)
 Allow access to instance variables as if they were local
 declarations within instance method implementati
 
 +finclude-nest-limit=
 +C ObjC C++ ObjC++ Joined RejectNegative UInteger
 Var(include_nest_limit) Init(200)
 +Set the maximum number of depth of nested #include.
 +
 
 However, don’t know how to refer this new variable
 “include_nest_limit” from libcpp.
 
>>> 
>>> You probably want to add a new field to cpp_options, and copy over
>>> the
>>> value from the GCC options to the new field; see e.g.
>>> c_common_handle_option where various cases write to fields of
>>> cpp_opts.
>>> 
>>> Hope this is helpful
>>> Dave
>> 
>> 



[PATCH] PR71482: Add -Wglobal-constructors

2019-05-28 Thread Sean Gillespie
This adds a new warning, -Wglobal-constructors, that warns whenever a
decl requires a global constructor or destructor. This new warning fires
whenever a thread_local or global variable is declared whose type has a
non-trivial constructor or destructor. When faced with both a constructor
and a destructor, the error message mentions the destructor and is only
fired once.

This warning mirrors the Clang option -Wglobal-constructors, which warns
on the same thing. -Wglobal-constructors was present in Apple's GCC and
later made its way into Clang.

Bootstrapped and regression-tested on x86-64 linux, new tests passing.

gcc/ChangeLog:

2019-05-28  Sean Gillespie  

* doc/invoke.texi: Add new flag -Wglobal-constructors.

gcc/c-family/ChangeLog:

2019-05-28  Sean Gillespie  

* c.opt: Add new flag -Wglobal-constructors.

gcc/cp/ChangeLog:

2019-05-28  Sean Gillespie  

* decl.c (expand_static_init): Warn if a thread local or static decl
requires a non-trivial constructor or destructor.

gcc/testsuite/ChangeLog:

2019-05-28  Sean Gillespie  

* g++.dg/warn/global-constructors-1.C: New test.
* g++.dg/warn/global-constructors-2.C: New test.
---
 gcc/c-family/c.opt|  4 +++
 gcc/cp/decl.c | 17 ++---
 gcc/doc/invoke.texi   |  7 ++
 .../g++.dg/warn/global-constructors-1.C   | 25 +++
 .../g++.dg/warn/global-constructors-2.C   | 23 +
 5 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/global-constructors-2.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..cf62625ca42 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -613,6 +613,10 @@ Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) 
Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) 
IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wglobal-constructors
+C++ ObjC++ Var(warn_global_constructors) Warning
+Warn when a global declaration requires a constructor to initialize.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 19d14a6a5e9..c1d66195bd7 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -8467,10 +8467,21 @@ expand_static_init (tree decl, tree init)
   finish_then_clause (if_stmt);
   finish_if_stmt (if_stmt);
 }
-  else if (CP_DECL_THREAD_LOCAL_P (decl))
-tls_aggregates = tree_cons (init, decl, tls_aggregates);
   else
-static_aggregates = tree_cons (init, decl, static_aggregates);
+   {
+if (CP_DECL_THREAD_LOCAL_P (decl))
+  tls_aggregates = tree_cons (init, decl, tls_aggregates);
+else
+  static_aggregates = tree_cons (init, decl, static_aggregates);
+
+if (warn_global_constructors)
+ {
+  if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl)))
+warning(OPT_Wglobal_constructors, "declaration requires a global 
destructor");
+  else
+warning(OPT_Wglobal_constructors, "declaration requires a global 
constructor");
+ }
+   }
 }
 
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4964cc41ba3..a9b2e47a302 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -312,6 +312,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-signedness  -Wformat-truncation=@var{n} @gol
 -Wformat-y2k  -Wframe-address @gol
 -Wframe-larger-than=@var{byte-size}  -Wno-free-nonheap-object @gol
+-Wglobal-constructors @gol
 -Wjump-misses-init @gol
 -Whsa  -Wif-not-aligned @gol
 -Wignored-qualifiers  -Wignored-attributes  -Wincompatible-pointer-types @gol
@@ -6509,6 +6510,12 @@ to @option{-Wframe-larger-than=}@samp{SIZE_MAX} or 
larger.
 Do not warn when attempting to free an object that was not allocated
 on the heap.
 
+@item -Wglobal-constructors @r{(C++ and Objective-C++ only)}
+@opindex Wglobal-constructors
+@opindex Wno-global-constructors
+Warn if the compiler detects a global declaration that requires
+a constructor to initialize.
+
 @item -Wstack-usage=@var{byte-size}
 @opindex Wstack-usage
 @opindex Wno-stack-usage
diff --git a/gcc/testsuite/g++.dg/warn/global-constructors-1.C 
b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
new file mode 100644
index 000..5a6869e9dcd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/global-constructors-1.C
@@ -0,0 +1,25 @@
+// { dg-do compile }
+// { dg-options "-Wglobal-constructors" }
+
+struct with_ctor {
+int x;
+with_ctor() : x(42) {}
+};
+
+struct with_dtor {
+~with_dtor() {}
+};
+
+struct with_both {
+int x;
+with_both() : x(42) {}
+~with_both() {}
+};
+
+with_ctor global_var; /* { dg-warning "declaration requires a global 
construct

Re: libbacktrace integration for _GLIBCXX_DEBUG mode

2019-05-28 Thread Jonathan Wakely

On 23/05/19 07:39 +0200, François Dumont wrote:

Hi

    So here what I come up with.

    _GLIBCXX_DEBUG_BACKTRACE controls the feature. If the user define 


Thanks for making this opt-in.

it and there is a detectable issue with libbacktrace then I generate a 
compilation error. I want to avoid users defining it but having no 
backtrace in the end in the debug assertion.


Why do you want to avoid that?

This means users can't just define the macro in their makefiles
unconditionally, they have to check if libbacktrace is installed and
supported. That might mean having platform-specific conditionals in
the makefile to only enable it sometimes.

What harm does it do to just ignore the _GLIBCXX_DEBUG_BACKTRACE macro
if backtraces can't be enabled? Or just use #warning instead of
#error?



    With this new setup I manage to run testsuite with it like that:

export LD_LIBRARY_PATH=/home/fdt/dev/gcc/install/lib/
make CXXFLAGS='-D_GLIBCXX_DEBUG_BACKTRACE 
-I/home/fdt/dev/gcc/install/include -lbacktrace' check-debug


    An example of result:

/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:606:
In function:
    std::__debug::vector<_Tp, _Allocator>::iterator
    std::__debug::vector<_Tp, 
_Allocator>::insert(std::__debug::vector<_Tp,

    _Allocator>::const_iterator, _InputIterator, _InputIterator) [with
    _InputIterator = int*;  = void; _Tp = int;
    _Allocator = std::allocator; std::__debug::vector<_Tp,
    _Allocator>::iterator =
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator >, std::__debug::vector,
    std::random_access_iterator_tag>; typename 
std::iterator_traits
    std::vector<_Tp, _Alloc>::iterator>::iterator_category =
    std::random_access_iterator_tag; typename std::vector<_Tp,
    _Alloc>::iterator = __gnu_cxx::__normal_iteratorstd::vector

    >; std::__debug::vector<_Tp, _Allocator>::const_iterator =
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator >, std::__debug::vector,
    std::random_access_iterator_tag>; typename 
std::iterator_traits
    std::vector<_Tp, _Alloc>::const_iterator>::iterator_category =
    std::random_access_iterator_tag; typename std::vector<_Tp,
    _Alloc>::const_iterator = __gnu_cxx::__normal_iteratorstd::

    vector >]

Backtrace:
    0x402718 
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iteratorstd::vector >> std::__debug::vector::insertvoid>(__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iteratorconst*, std::vector >>, int*, int*)

/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/vector:606
    0x402718 test01()
/home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/debug/57779_neg.cc:29
    0x401428 main
/home/fdt/dev/gcc/git/libstdc++-v3/testsuite/23_containers/vector/debug/57779_neg.cc:34

Error: attempt to insert with an iterator range [__first, __last) from this
container.

Objects involved in the operation:
    iterator "__first" @ 0x0x7fff730b96b0 {
  type = int* (mutable iterator);
    }
    iterator "__last" @ 0x0x7fff730b96b8 {
  type = int* (mutable iterator);
    }
    sequence "this" @ 0x0x7fff730b9720 {
  type = std::__debug::vector;
    }


This is nice.


diff --git a/libstdc++-v3/doc/xml/manual/debug_mode.xml 
b/libstdc++-v3/doc/xml/manual/debug_mode.xml
index 570c17ba28a..27873151dae 100644
--- a/libstdc++-v3/doc/xml/manual/debug_mode.xml
+++ b/libstdc++-v3/doc/xml/manual/debug_mode.xml
@@ -104,9 +104,11 @@
The following library components provide extra debugging
  capabilities in debug mode:

+  std::array (no safe iterators)
  std::basic_string (no safe iterators and see note 
below)
  std::bitset
  std::deque
+  std::forward_list
  std::list
  std::map
  std::multimap
@@ -160,6 +162,13 @@ which always works correctly.
  GLIBCXX_DEBUG_MESSAGE_LENGTH can be used to request a
  different length.

+Starting with GCC 10 libstdc++ has integrated


I think "integrated" is the wrong word, because you haven't made
libbacktrace and libstdc++ into a single thing. You've just added the
ability for libstdc++ to use libbacktrace.

I suggest "libstdc++ is able to use"


+  http://www.w3.org/1999/xlink";
+  
xlink:href="https://github.com/ianlancetaylor/libbacktrace";>libbacktrace
+  to produce backtrace on error. Use -D_GLIBCXX_DEBUG_BACKTRACE to


Should be "produce backtraces".


+  activate it. Note that if not properly installed or if libbacktrace is not
+  supported compilation will fail. You'll also have to use the


Comma after "supported". Remove "the" at the end of the line.



+  -lbacktrace to build your application.



The new _GLIBCXX_DEBUG_BACKTRACE macro should be documented in
doc/xml/manual/using.xml too. I think we also have a table in that
chapter which lists relevant GCC options, so -lbacktrace could go in
there too.

Enabling backtrace support doesn't alter the ABI of the debug mode
containers, right? It only affects the size of the _Error_formatter
type, which is not a member of any container or iterator.




[PATCH] hppa: Improve mcount argument setup

2019-05-28 Thread John David Anglin
We don't need to pass to mcount the exact start of the called routine.  This 
saves
one instruction when long calls are being used.

Tested on hppa-unknown-linux-gnu, hppa2.0w-hp-hpux11.11 and hppa64-hp-hpux11.11.
Committed to trunk, gcc-9 and gcc-8 branches.

Dave
-- 
John David Anglin  dave.ang...@bell.net

2019-05-28  John David Anglin  

* config/pa/pa.c (hppa_profile_hook): Remove offset adjustment.

Index: config/pa/pa.c
===
--- config/pa/pa.c  (revision 271614)
+++ config/pa/pa.c  (working copy)
@@ -4569,10 +4581,6 @@
 void
 hppa_profile_hook (int label_no)
 {
-  /* We use SImode for the address of the function in both 32 and
- 64-bit code to avoid having to provide DImode versions of the
- lcla2 and load_offset_label_address insn patterns.  */
-  rtx reg = gen_reg_rtx (SImode);
   rtx_code_label *label_rtx = gen_label_rtx ();
   int reg_parm_stack_space = REG_PARM_STACK_SPACE (NULL_TREE);
   rtx arg_bytes, begin_label_rtx, mcount, sym;
@@ -4604,18 +4612,13 @@
   if (!use_mcount_pcrel_call)
 {
   /* The address of the function is loaded into %r25 with an instruction-
-relative sequence that avoids the use of relocations.  The sequence
-is split so that the load_offset_label_address instruction can
-occupy the delay slot of the call to _mcount.  */
+relative sequence that avoids the use of relocations.  We use SImode
+for the address of the function in both 32 and 64-bit code to avoid
+having to provide DImode versions of the lcla2 pattern.  */
   if (TARGET_PA_20)
-   emit_insn (gen_lcla2 (reg, label_rtx));
+   emit_insn (gen_lcla2 (gen_rtx_REG (SImode, 25), label_rtx));
   else
-   emit_insn (gen_lcla1 (reg, label_rtx));
-
-  emit_insn (gen_load_offset_label_address (gen_rtx_REG (SImode, 25),
-   reg,
-   begin_label_rtx,
-   label_rtx));
+   emit_insn (gen_lcla1 (gen_rtx_REG (SImode, 25), label_rtx));
 }

   if (!NO_DEFERRED_PROFILE_COUNTERS)


Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git

2019-05-28 Thread Joseph Myers
On Fri, 24 May 2019, Segher Boessenkool wrote:

> IMO the best we can do is use what we already have: what CVS or SVN used
> as the committer identity.  *That* info is *correct* at least.

CVS and SVN have a local identity.  git has a global identity.  I consider 
it simply *incorrect* to put a local identity in a git committer or author 
- I think this is just like any other data format conversion necessarily 
involved in a repository conversion.  There's an argument that (real name 
from /etc/passwd on the repository system at the time of commit) plus 
(username @ hostname of repository system at the time of commit) 
corresponds most accurately to the old local identities, but we don't have 
the contemporaneous /etc/passwd and I don't see doing something like that 
as an improvement on using whatever each person's preferred identity for 
git commits is (or some name and email address we've found, in the absence 
of an expressed preference from a given committer), if there wasn't a 
ChangeLog entry added in that commit.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH V2] A jump threading opportunity for condition branch

2019-05-28 Thread Jiufu Guo
Jiufu Guo  writes:

> Hi,
>
> This patch implements a new opportunity of jump threading for PR77820.
> In this optimization, conditional jumps are merged with unconditional
> jump. And then moving CMP result to GPR is eliminated.
>
> This version is based on the proposal of Richard, Jeff and Andrew, and
> refined to incorporate comments.  Thanks for the reviews!
>
> Bootstrapped and tested on powerpc64le and powerpc64be with no
> regressions (one case is improved) and new testcases are added. Is this
> ok for trunk?
For accurate, tested targets are powerpc64-unknown-none and
powerpc64le-unknown-none.  split-path-6.c is the case improved, since
one jump threading opportunity also meet, and then more following
optimization happen on it. 
>
> Example of this opportunity looks like below:
>
>   
>   p0 = a CMP b
>   goto ;
>
>   
>   p1 = c CMP d
>   goto ;
>
>   
>   # phi = PHI 
>   if (phi != 0) goto ; else goto ;
>
> Could be transformed to:
>
>   
>   p0 = a CMP b
>   if (p0 != 0) goto ; else goto ;
>
>   
>   p1 = c CMP d
>   if (p1 != 0) goto ; else goto ;
>
>
> This optimization eliminates:
> 1. saving CMP result: p0 = a CMP b.
> 2. additional CMP on branch: if (phi != 0).
> 3. converting CMP result if there is phi = (INT_CONV) p0 if there is.
>
> Thanks!
> Jiufu Guo
>
>
> [gcc]
> 2019-05-28  Jiufu Guo  
>   Lijia He  
>
>   PR tree-optimization/77820
>   * tree-ssa-threadedge.c
>   (edge_forwards_cmp_to_conditional_jump_through_empty_bb_p): New
>   function.
>   (thread_across_edge): Add call to
>   edge_forwards_cmp_to_conditional_jump_through_empty_bb_p.
>
> [gcc/testsuite]
> 2019-05-28  Jiufu Guo  
>   Lijia He  
>
>   PR tree-optimization/77820
>   * gcc.dg/tree-ssa/phi_on_compare-1.c: New testcase.
>   * gcc.dg/tree-ssa/phi_on_compare-2.c: New testcase.
>   * gcc.dg/tree-ssa/phi_on_compare-3.c: New testcase.
>   * gcc.dg/tree-ssa/phi_on_compare-4.c: New testcase.
>   * gcc.dg/tree-ssa/split-path-6.c: Update testcase.
>
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c | 30 ++
>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c | 23 +++
>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c | 25 
>  gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c | 40 +
>  gcc/testsuite/gcc.dg/tree-ssa/split-path-6.c |  2 +-
>  gcc/tree-ssa-threadedge.c| 76 
> +++-
>  6 files changed, 192 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> new file mode 100644
> index 000..5227c87
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-1.c
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (int);
> +void g1 (int);
> +
> +void
> +f (long a, long b, long c, long d, long x)
> +{
> +  _Bool t;
> +  if (x)
> +{
> +  g (a + 1);
> +  t = a < b;
> +  c = d + x;
> +}
> +  else
> +{
> +  g (b + 1);
> +  a = c + d;
> +  t = c > d;
> +}
> +
> +  if (t)
> +g1 (c);
> +
> +  g (a);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> new file mode 100644
> index 000..eaf89bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-2.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (void);
> +void g1 (void);
> +
> +void
> +f (long a, long b, long c, long d, int x)
> +{
> +  _Bool t;
> +  if (x)
> +t = c < d;
> +  else
> +t = a < b;
> +
> +  if (t)
> +{
> +  g1 ();
> +  g ();
> +}
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c 
> b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> new file mode 100644
> index 000..d5a1e0b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-vrp1" } */
> +
> +void g (void);
> +void g1 (void);
> +
> +void
> +f (long a, long b, long c, long d, int x)
> +{
> +  int t;
> +  if (x)
> +t = a < b;
> +  else if (d == x)
> +t = c < b;
> +  else
> +t = d > c;
> +
> +  if (t)
> +{
> +  g1 ();
> +  g ();
> +}
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "vrp1" } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-4.c 
> b/gcc/testsuite/

Re: [PATCH] rs6000: Call flow implementation for PC-relative addressing

2019-05-28 Thread Bill Schmidt
Hi,

Please go ahead and review this.  In the test case
gcc.target/powerpc/notoc-direct-1.c, I accidentally left in '+'
characters in column 1 of the first three lines, which caused the test
case failure.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with that
fixed.  Is this okay for trunk?

Thanks,
Bill

On 5/24/19 9:06 AM, Bill Schmidt wrote:
> New test case ICEs, so consider this withdrawn.  Sorry again about this.
>
> Bill
>
> On 5/23/19 9:17 PM, Bill Schmidt wrote:
>> Hm, I got ahead of myself on this one.  I haven't done the regstrap yet,
>> so please hold off reviewing for now.
>>
>> Sorry for the noise.  I shouldn't post when I'm tired...
>>
>> Thanks,
>> Bill
>>
>> On 5/23/19 9:11 PM, Bill Schmidt wrote:
>>> Hi,
>>>
>>> This patch contains the changes to implement call flow for PC-relative 
>>> addressing.  
>>> It's an amalgam of several internal patches that Alan and I worked on, and 
>>> as a 
>>> result it's hard to tease apart individual pieces much further.  So I 
>>> apologize 
>>> that this patch is a little larger than the others.  Also, I've CC'd Alan 
>>> so he 
>>> can help answer questions about the patch, particularly the PLT bits I'm 
>>> not very
>>> familiar with.
>>>
>>> Following are descriptions of the individual patches that are combined here.
>>>
>>> (1) When a function uses PC-relative code generation, all direct calls 
>>> (other than 
>>> sibcalls) that the function makes to local or external callees should 
>>> appear as
>>> "bl sym@notoc" and should not be followed by a nop instruction.  @notoc 
>>> indicates
>>> that the assembler should annotate the call with R_PPC64_REL24_NOTOC, 
>>> meaning
>>> that the caller does not guarantee that r2 contains a valid TOC pointer.  
>>> Thus
>>> the linker should not try to replace a subsequent "nop" with a TOC restore
>>> instruction.
>>>
>>> I've added a test case for the four cases handled here:  local calls 
>>> with/without
>>> a return value, and external cases with/without a return value.
>>>
>>> (2) If a caller preserves the TOC pointer and the callee does not, or vice 
>>> versa,
>>> then a sibcall will cause an inconsistency.  Don't allow that.
>>>
>>> (3) The linker needs a @notoc directive on sibcall targets when the caller 
>>> does not
>>> provide or preserve a TOC pointer.  This patch provides for that.
>>>
>>> In creating the new sibcall patterns, I did not duplicate the "c" 
>>> alternatives
>>> that allow for bctr or blr sibcalls.  I don't think there's a way to 
>>> generate
>>> those currently.  The bctr would be legitimate for PC-relative sibcalls if 
>>> you
>>> can prove that the target function is in the same binary, but we don't 
>>> appear
>>> to detect that possibility today.
>>>
>>> (4) This patch deletes all the extra insns added to handle pcrel calls,
>>> instead opting to use existing insns but making their output
>>> conditional on rs6000_pcrel_p(cfun).  There isn't a need to
>>> differentiate between pcrel and non-pcrel calls at the point rtl is
>>> created; rs6000_pcrel_p is valid right up to the final pass, as
>>> evidenced by use of rs6000_pcrel_p to emit .localentry.
>>> 
>>> There is one case however where we do need new insns: The existing
>>> nonlocal indirect call insns mention r2 in their rtl.  That isn't
>>> correct for pcrel indirect calls, and may cause problems when/if r2
>>> is allocated as any other volatile gpr in pcrel code.
>>> 
>>> The patch also fixes pcrel inline PLT calls (which are used for
>>> -fno-plt and -mlongcall) to use a pcrel load from the PLT rather than
>>> attempting (and failing) to use TOC-relative loads.  This requires
>>> some changes in the way relocs are emitted.  For prefix insns we can't
>>> write
>>>.reloc .,R_PPC64_PLT_PCREL34_NOTOC,foo
>>>pld 12,0(0),1
>>> since the pld may require a padding nop.  Instead it's necessary to
>>> put the .reloc after the instruction or use a label on the insn.  Like
>>> this (which is what the patch does):
>>>pld 12,0(0),1
>>>.reloc .-8,R_PPC64_PLT_PCREL34_NOTOC,foo
>>> or this:
>>>.reloc 0f,R_PPC64_PLT_PCREL34_NOTOC,foo
>>> 0: pld 12,0(0),1
>>>
>>> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no 
>>> regressions.
>>> Is this okay for trunk?
>>>
>>> Thanks!
>>> Bill
>>>
>>>
>>> [gcc]
>>>
>>> 2019-05-23  Bill Schmidt  
>>> Alan Modra  
>>>
>>> * config/rs6000/rs6000.c (rs6000_call_template_1): Handle pcrel
>>> calls here...
>>> (rs6000_indirect_call_template_1): ...and here.
>>> (rs6000_indirect_sibcall_template): Handle plt_pcrel34.  Rework
>>> tocsave, plt16_ha, plt16_lo, mtctr indirect calls.
>>> (rs6000_decl_ok_for_sibcall): New function.
>>> (rs6000_function_ok_for_sibcall): Refactor.
>>> (rs6000_longcall_ref): Use UNSPEC_PLT_PCREL when pcrel.
>>> (rs6000_call_aix): Don't emit toc restore rtl for indirect calls
>>> whe

[PATCH][OBVIOUS] Remove duplicate dg-compile (PR testsuite/90657).

2019-05-28 Thread Martin Liška
Hi.

It's an obvious error that I've just tested on ppc64le.

I'm going to install the patch.
Martin

gcc/testsuite/ChangeLog:

2019-05-29  Martin Liska  

PR testsuite/90657
* gcc.dg/ipa/pr90555.c: Remove duplicate dg-compile.
---
 gcc/testsuite/gcc.dg/ipa/pr90555.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


diff --git a/gcc/testsuite/gcc.dg/ipa/pr90555.c b/gcc/testsuite/gcc.dg/ipa/pr90555.c
index a9c751cd63c..327cff9dcdd 100644
--- a/gcc/testsuite/gcc.dg/ipa/pr90555.c
+++ b/gcc/testsuite/gcc.dg/ipa/pr90555.c
@@ -1,6 +1,5 @@
-/* { dg-do compile } */
-/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
 /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-fopenmp-simd -O2 -mavx512f -fdump-ipa-icf" } */
 
 #define N 1024
 int a[N];



Re: [PATCH] Handle loop fields in IPA ICF (PR ipa/90555).

2019-05-28 Thread Martin Liška
On 5/28/19 10:31 PM, Christophe Lyon wrote:
> Was it your intention?

Hi.

No ;) fixed as r271729.

Thanks for reporting,
Martin


Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

2019-05-28 Thread Richard Biener
On Tue, 28 May 2019, Martin Sebor wrote:

> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:
> > > 
> > > I'm not sure we really need a new warning for this.
> > 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> > > 
> > > I don't think GCC makes a formal distinction between function
> > > attributes that affect only function definitions vs those that
> > > affect its users, or both.  It might be a useful distinction
> > > to introduce, perhaps even for functions as well as variables,
> > > but as it is, users (as well as GCC developers) are on our own
> > > to figure it out.
> > 
> > Then is it preferable to simply silence Wattributes in this case?
> 
> This case being PR86407?  I'd say yes.  I think one general problem
> to solve is the missing suppression for typedefs.  This could be
> useful for other warnings as well.  Another is providing a knob to
> control the warning when one kind of an attribute is used with
> an entity of a different kind (function vs type, or variable vs
> type).  Yet another might be to control warnings for individual
> attributes.
> 
> > 
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener  wrote:
> > > 
> > > > +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> > > > +
> > > 
> > > But this is a declaration?
> > 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> > > 
> > > My first question is: what is the meaning of "function definition
> > > attributes?"  Presumably those that affect only the definition of
> > > a function and not its callers or other users?
> > 
> > As far as I can tell, "fndecl" is a misnomer: these attributes are
> > more accurately called "function definition attributes", i.e.
> > attributes that affect the assembly code of the function but do not
> > affect its calling convention.
> 
> That's one possible definition but there are examples that don't
> fit it (at least not very neatly).
> 
> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.

Yeah, we have existing attributes that do not apply well to
indirect calls due to mistakes made in the past.  Of course
for things like 'malloc' this isn't a correctness issue.

> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?

I think the issue is that internally we know what a fndecl is
but that doesn't match what the C standard says is a function
declaration.  So it's important to not mix terms for GCC internals
and terms used in diagnostics which generally should use terms
from the language standard.

> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?
> 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor  wrote:
> > > 
> > > Finally, with this as a prerequisite, if we decided that a warning
> > > like this would be useful, tests to verify that it works for all
> > > the definition attributes and not for the rest would need to be
> > > added (i.e., in addition to ms_hook_prologue).
> > 
> > Okay, I will add tests for the other function attributes that should
> > behave in the same way, commenting out the tests that will require
> > more work to pass.
> > 
> > The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
> > Wine without causing spurious warnings. This will go a long way
> > towards making Wine compatible with current and future Windows
> > programs. Thank you for help.
> 
> Yes, I understand the goal.  I'm not sure that the proposed change
> is the right way to do it.  It seems to me that a more targeted bug
> to fix with the broadest benefit is that _Pragma GCC diagnostic (or
> some such mechanism) cannot be used to suppress the warning for
> typedefs.
> 
> Martin
> 

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