[Ada] No_Stream_Optimizations ignored for 'Class'Input

2019-07-05 Thread Pierre-Marie de Rodat
This patch fixes a bug in which if pragma Restrictions
(No_Stream_Optimizations) is in effect, it is ignored for T'Class'Input.
Revision 251886  was causing the compiler to bypass
No_Stream_Optimizations.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Bob Duff  

gcc/ada/

* exp_attr.adb (Input): Take the No_Stream_Optimizations
restriction into account.--- gcc/ada/exp_attr.adb
+++ gcc/ada/exp_attr.adb
@@ -3997,11 +3997,13 @@ package body Exp_Attr is
 
declare
   Rtyp : constant Entity_Id := Root_Type (P_Type);
-  Expr : Node_Id;
+  Get_Tag : Node_Id; -- expression to read the 'Tag
+  Expr : Node_Id; -- call to Descendant_Tag
 
begin
   --  Read the internal tag (RM 13.13.2(34)) and use it to
-  --  initialize a dummy tag value. We used to generate:
+  --  initialize a dummy tag value. We used to unconditionally
+  --  generate:
   --
   -- Descendant_Tag (String'Input (Strm), P_Type);
   --
@@ -4012,6 +4014,11 @@ package body Exp_Attr is
   --  String_Input_Blk_IO, except that if the String is
   --  absurdly long, it raises an exception.
   --
+  --  However, if the No_Stream_Optimizations restriction
+  --  is active, we disable this unnecessary attempt at
+  --  robustness; we really need to read the string
+  --  character-by-character.
+  --
   --  This value is used only to provide a controlling
   --  argument for the eventual _Input call. Descendant_Tag is
   --  called rather than Internal_Tag to ensure that we have a
@@ -4026,18 +4033,30 @@ package body Exp_Attr is
   --  this constant in Cntrl, but this caused a secondary stack
   --  leak.
 
+  if Restriction_Active (No_Stream_Optimizations) then
+ Get_Tag :=
+   Make_Attribute_Reference (Loc,
+ Prefix =>
+   New_Occurrence_Of (Standard_String, Loc),
+ Attribute_Name => Name_Input,
+ Expressions=> New_List (
+   Relocate_Node (Duplicate_Subexpr (Strm;
+  else
+ Get_Tag :=
+   Make_Function_Call (Loc,
+ Name   =>
+   New_Occurrence_Of
+ (RTE (RE_String_Input_Tag), Loc),
+ Parameter_Associations => New_List (
+   Relocate_Node (Duplicate_Subexpr (Strm;
+  end if;
+
   Expr :=
 Make_Function_Call (Loc,
   Name   =>
 New_Occurrence_Of (RTE (RE_Descendant_Tag), Loc),
   Parameter_Associations => New_List (
-Make_Function_Call (Loc,
-  Name   =>
-New_Occurrence_Of
-  (RTE (RE_String_Input_Tag), Loc),
-  Parameter_Associations => New_List (
-Relocate_Node (Duplicate_Subexpr (Strm,
-
+Get_Tag,
 Make_Attribute_Reference (Loc,
   Prefix => New_Occurrence_Of (P_Type, Loc),
   Attribute_Name => Name_Tag)));



[Ada] Fix inlining in GNATprove inside quantified expressions

2019-07-05 Thread Pierre-Marie de Rodat
Calls to local subprograms in GNATprove may be inlined in some case, but
it should not be the case inside quantified expressions which are
handled as expressions inside GNATprove. Because quantified expressions
are only preanalayzed, the detection of the impossible inlining was not
performed.  Now fixed.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Yannick Moy  

gcc/ada/

* sem_res.adb (Resolve_Call): Cannot inline in quantified
expressions.
* sem_util.adb, sem_util.ads (In_Quantified_Expression): New
function.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -6768,6 +6768,15 @@ package body Sem_Res is
Cannot_Inline
  ("cannot inline & (in default expression)?", N, Nam_UA);
 
+--  Calls cannot be inlined inside quantified expressions, which
+--  are left in expression form for GNATprove. Since these
+--  expressions are only preanalyzed, we need to detect the failure
+--  to inline outside of the case for Full_Analysis below.
+
+elsif In_Quantified_Expression (N) then
+   Cannot_Inline
+ ("cannot inline & (in quantified expression)?", N, Nam_UA);
+
 --  Inlining should not be performed during preanalysis
 
 elsif Full_Analysis then

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -12305,6 +12305,25 @@ package body Sem_Util is
   end if;
end In_Pre_Post_Condition;
 
+   --
+   -- In_Quantified_Expression --
+   --
+
+   function In_Quantified_Expression (N : Node_Id) return Boolean is
+  P : Node_Id;
+   begin
+  P := Parent (N);
+  loop
+ if No (P) then
+return False;
+ elsif Nkind (P) = N_Quantified_Expression then
+return True;
+ else
+P := Parent (P);
+ end if;
+  end loop;
+   end In_Quantified_Expression;
+
-
-- In_Reverse_Storage_Order_Object --
-

--- gcc/ada/sem_util.ads
+++ gcc/ada/sem_util.ads
@@ -1410,6 +1410,9 @@ package Sem_Util is
--  Returns True if node N appears within a pre/postcondition pragma. Note
--  the pragma Check equivalents are NOT considered.
 
+   function In_Quantified_Expression (N : Node_Id) return Boolean;
+   --  Returns true if the expression N occurs within a quantified expression
+
function In_Reverse_Storage_Order_Object (N : Node_Id) return Boolean;
--  Returns True if N denotes a component or subcomponent in a record or
--  array that has Reverse_Storage_Order.



[Ada] Compiler abort on a dynamic predicate used in a precondition

2019-07-05 Thread Pierre-Marie de Rodat
This patch suppresses the generation of a predicate check when the
expression is a formal IN parameter of a subprogram S. If the check is
being applied to the actual in a call, the call is either in the body of
S, or in an aspect specfication for S, e.g. a precondition, In both
cases the check is redundant bevause it will be applied on any call to
S. In the second case the expansion of the predicate check may lead to
out-of-scope references the the formal.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Ed Schonberg  

gcc/ada/

* checks.adb (Apply_Predicate_Check): Except within the
subprogram body that defines the formal, do not apply predicate
check on a formal IN parameter: such a check is redundant and
its expansion can lead to out-of-scope references when it is
originates in a function call in a precondition,

gcc/testsuite/

* gnat.dg/predicate7.adb, gnat.dg/predicate7.ads,
gnat.dg/predicate7_pkg.ads: New testcase.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -2707,6 +2707,41 @@ package body Checks is
  --  Here for normal case of predicate active
 
  else
+--  If the expression is an IN parameter, the predicate will have
+--  been applied at the point of call. An additional check would
+--  be redundant, or will lead to out-of-scope references if the
+--  call appears within an aspect specification for a precondition.
+
+--  However, if the reference is within the body of the subprogram
+--  that declares the formal, the predicate can safely be applied,
+--  which may be necessary for a nested call whose formal has a
+--  different predicate.
+
+if Is_Entity_Name (N)
+  and then Ekind (Entity (N)) = E_In_Parameter
+then
+   declare
+  In_Body : Boolean := False;
+  P : Node_Id := Parent (N);
+
+   begin
+  while Present (P) loop
+ if Nkind (P) = N_Subprogram_Body
+   and then Corresponding_Spec (P) = Scope (Entity (N))
+ then
+In_Body := True;
+exit;
+ end if;
+
+ P := Parent (P);
+  end loop;
+
+  if not In_Body then
+ return;
+  end if;
+   end;
+end if;
+
 --  If the type has a static predicate and the expression is known
 --  at compile time, see if the expression satisfies the predicate.
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate7.adb
@@ -0,0 +1,6 @@
+--  { dg-do compile }
+--  { dg-options "-gnata" }
+
+package body Predicate7 is
+   procedure Foo is null;
+end;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate7.ads
@@ -0,0 +1,13 @@
+with Predicate7_Pkg; use Predicate7_Pkg;
+
+package Predicate7 is
+   function Always_True (I : My_Int) return Boolean;
+
+   function Identity (I : My_Int ) return Integer with Pre => Always_True (I);
+
+   procedure Foo;
+
+private
+   function Identity (I : My_Int ) return Integer is (I);
+   function Always_True (I : My_Int) return Boolean is (True);
+end;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate7_pkg.ads
@@ -0,0 +1,3 @@
+package Predicate7_Pkg is
+  subtype My_Int is Integer with Dynamic_Predicate => My_Int /= 0;
+end Predicate7_Pkg;



[Ada] Spurious error on aggregate with choice that is predicted subtype

2019-07-05 Thread Pierre-Marie de Rodat
This patch fixes a spurious error on a record aggregate for a variant
record when a choice in the aggregate is given by a subtype with a
static predicate. The same expansion mechanism for such a variant, used
in case statements, must be used here as well.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Ed Schonberg  

gcc/ada/

* sem_util.adb (Encloing_Subprogram): If Enclosing_Dynamic_Scope
is a loop, continue climbing the scope stack to find the
enclosing subprogram.
(Gather_Components): Handle properly a choice in a record
aggregate that is given by a subtype with a static predicate.

gcc/testsuite/

* gnat.dg/aggr25.adb, gnat.dg/aggr25.ads: New testcase.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -6895,7 +6895,7 @@ package body Sem_Util is
   elsif Ekind (Dyn_Scop) = E_Subprogram_Body then
  return Corresponding_Spec (Parent (Parent (Dyn_Scop)));
 
-  elsif Ekind_In (Dyn_Scop, E_Block, E_Return_Statement) then
+  elsif Ekind_In (Dyn_Scop, E_Block, E_Return_Statement, E_Loop) then
  return Enclosing_Subprogram (Dyn_Scop);
 
   elsif Ekind (Dyn_Scop) = E_Entry then
@@ -8939,6 +8939,12 @@ package body Sem_Util is
 
   begin
  Find_Discrete_Value : while Present (Variant) loop
+
+--  If a choice is a subtype with a static predicate, it must
+--  be rewritten as an explicit list of non-predicated choices.
+
+Expand_Static_Predicates_In_Choices (Variant);
+
 Discrete_Choice := First (Discrete_Choices (Variant));
 while Present (Discrete_Choice) loop
exit Find_Discrete_Value when

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/aggr25.adb
@@ -0,0 +1,7 @@
+--  { dg-do compile }
+
+package body Aggr25 is
+
+  procedure Foo is null;
+
+end Aggr25;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/aggr25.ads
@@ -0,0 +1,23 @@
+package Aggr25 is
+
+  type T_A is (A, B , C ,D);
+
+  subtype Has_B_D is T_A with Static_Predicate => Has_B_D in B | D;
+
+  type Obj_T (Kind : T_A) is
+record
+   case Kind is
+--OK-- when A | C => null; --OK--
+when Has_B_D  =>  Value : Boolean;
+--BAD-- when A | C => null;
+when others => null;
+  end case;
+end record;
+
+  type T is access Obj_T;
+
+  Unavailable : constant T := new Obj_T'(Kind => A);
+
+  procedure Foo;
+
+end Aggr25;



[Ada] Crash on exported build-in-place function

2019-07-05 Thread Pierre-Marie de Rodat
This patch fixes a bug where if a function is build-in-place, and is
exported, and contains an extended_return_statement whose object is
initialized with another build-in-place function call, then the compiler
will crash.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Bob Duff  

gcc/ada/

* exp_ch6.adb (Is_Build_In_Place_Function): Narrow the check for
Has_Foreign_Convention to the imported case only.  If a
build-in-place function is exported, and called from Ada code,
build-in-place protocols should be used.

gcc/testsuite/

* gnat.dg/bip_export.adb, gnat.dg/bip_export.ads: New testcase.--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -7765,22 +7765,20 @@ package body Exp_Ch6 is
 
   --  For now we test whether E denotes a function or access-to-function
   --  type whose result subtype is inherently limited. Later this test
-  --  may be revised to allow composite nonlimited types. Functions with
-  --  a foreign convention or whose result type has a foreign convention
-  --  never qualify.
+  --  may be revised to allow composite nonlimited types.
 
   if Ekind_In (E, E_Function, E_Generic_Function)
 or else (Ekind (E) = E_Subprogram_Type
   and then Etype (E) /= Standard_Void_Type)
   then
- --  Note: If the function has a foreign convention, it cannot build
- --  its result in place, so you're on your own. On the other hand,
- --  if only the return type has a foreign convention, its layout is
- --  intended to be compatible with the other language, but the build-
- --  in place machinery can ensure that the object is not copied.
+ --  If the function is imported from a foreign language, we don't do
+ --  build-in-place. Note that Import (Ada) functions can do
+ --  build-in-place. Note that it is OK for a build-in-place function
+ --  to return a type with a foreign convention; the build-in-place
+ --  machinery will ensure there is no copying.
 
  return Is_Build_In_Place_Result_Type (Etype (E))
-   and then not Has_Foreign_Convention (E)
+   and then not (Has_Foreign_Convention (E) and then Is_Imported (E))
and then not Debug_Flag_Dot_L;
   else
  return False;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/bip_export.adb
@@ -0,0 +1,15 @@
+--  { dg-do compile }
+
+package body Bip_Export is
+   function F return T is
+   begin
+  return Result : constant T := G do
+ null;
+  end return;
+   end F;
+
+   function G return T is
+   begin
+  return (null record);
+   end G;
+end Bip_Export;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/bip_export.ads
@@ -0,0 +1,6 @@
+package Bip_Export is
+   type T is limited null record;
+   function F return T;
+   pragma Export (C, F);
+   function G return T;
+end Bip_Export;



[Ada] Diagnostics in Elaboration order v4.0

2019-07-05 Thread Pierre-Marie de Rodat
This patch introduces several changes to the new elaboration order
mechanism:

  * The library graph can now discover, store, and organize the various
cycles it contains.

  * The elaboration order mechanism can now diagnose one or all cycles
within the library graph. Diagnostics consist of describing the
reason for the cycle, listing all units comprising the circuit, and
offering suggestions on how to break the cycle.

The patch also modifies unit ALI to hide all invocation-related data
structures and several implementation-specific types by relocating them
in the body of the unit.

The patch cleans up most children of Bindo by using better names of
routines and formal parameters.


-- Source --


--  a.ads

with B; pragma Elaborate_All (B);
with C; pragma Elaborate_All (C);

package A is
end A;

--  b.ads

package B is
   procedure Force_Body;
end B;

--  b.adb

with D;

package body B is
   procedure Force_Body is null;

   Elab : constant Integer := D.Func;
end B;

--  c.ads

package C is
   procedure Force_Body;
end C;

--  c.adb

with E;

package body C is
   procedure Force_Body is null;
end C;

--  d.ads

package D is
   function Func return Integer;
end D;

--  d.adb

with A;

package body D is
   Local : Integer := 123;

   function Func return Integer is
   begin
  return Local;
   end Func;
end D;

--  e.ads

with A;

package E is
end E;

--  main.adb

with B;

-- Elaborate_All Elaborate_All   with
--C spec <--- A spec -> B spec <-- Main
--  ^  ^  ^   ^
--  |  |  |   |
--  sbb |  |  |   | sbb
--  |  |  |   |
--C body ---> E spec  |   D spec <- B body
--   with | ^   with  |
--| | |
--| sbb | |
--| | |
--+-- D body <+
--  with   Invocation
--
--  The cycles are
--
--A spec --> C spec --> E spec --> A spec
--   C body
--
--A spec --> B spec --> D body --> A spec
--   B body

procedure Main is begin null; end Main;


-- Compilation and output --


$ gnatmake -q main.adb -bargs -d_C -d_N
error: Elaboration circularity detected
info:
info:Reason:
info:
info:  unit "a (spec)" depends on its own elaboration
info:
info:Circularity:
info:
info:  unit "a (spec)" has with clause and pragma Elaborate_All for unit
 "b (spec)"
info:  unit "b (body)" is in the closure of pragma Elaborate_All
info:  unit "b (body)" has with clause for unit "d (spec)"
info:  unit "d (body)" is in the closure of pragma Elaborate_All
info:  unit "d (body)" has with clause for unit "a (spec)"
info:
info:Suggestions:
info:
info:  change pragma Elaborate_All for unit "b (spec)" to Elaborate in unit
 "a (spec)"
info:  remove pragma Elaborate_All for unit "b (spec)" in unit "a (spec)"
info:
error: Elaboration circularity detected
info:
info:Reason:
info:
info:  unit "a (spec)" depends on its own elaboration
info:
info:Circularity:
info:
info:  unit "a (spec)" has with clause and pragma Elaborate_All for unit
 "c (spec)"
info:  unit "c (body)" is in the closure of pragma Elaborate_All
info:  unit "c (body)" has with clause for unit "e (spec)"
info:  unit "e (spec)" has with clause for unit "a (spec)"
info:
info:Suggestions:
info:
info:  change pragma Elaborate_All for unit "c (spec)" to Elaborate in unit
 "a (spec)"
info:  remove pragma Elaborate_All for unit "c (spec)" in unit "a (spec)"
info:
gnatmake: *** bind failed.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Hristian Kirtchev  

gcc/ada/

* ali.adb: Relocate types Invocation_Construct_Record,
Invocation_Relation_Record, and Invocation_Signature_Record to
the body of ALI.  Relocate tables Invocation_Constructs,
Invocation_Relations, and Invocation_Signatures to the body of
ALI.  Remove type Body_Placement_Codes.  Add new types
Declaration_Placement_Codes, and
Invocation_Graph_Encoding_Codes.  Update the literals of type
Invocation_Graph_Line_Codes.
(Add_Invocation_Construct): Update the parameter profile. Add an
invocation construct built from all attributes provided.
(Add_Invocation_Relation): Update the parameter profile. Add an
invocation relation built from all attributes provided.
(Body_Placement): New routine.
(Body_P

[Ada] Missing range check on assignment to bit-packed array

2019-07-05 Thread Pierre-Marie de Rodat
This patch adds an explicit range check on an assignment to a component
of a bit-packed array, when the index type of the array is an
enumeration type with a non-standard representation,

Executing the following:

   gnatmake -f -gnata -q main
   ./main

must yield:

   1 is invalid
4097 is invalid
4116 is invalid
4117 is invalid
4118 is invalid
4119 is invalid
4120 is invalid
4121 is invalid


with Example; use Example;
with My_Types;use My_Types;
with Text_IO; use Text_IO;

procedure main is
begin
   --We try to access an invalid array location.
begin
 dummy(idx=> 1,action => DISABLE);
exception
   when others => Text_IO.Put_Line ("1 is invalid");
end;

  for I in typ_uint32'(16#1000#) .. 16#101E#  loop
 declare
 begin
--  Text_IO.Put_Line (typ_uint32'image(I) & " OK");
Dummy (Idx => I, action => Enable);
exception
when others => put_line (typ_uint32'Image (I) & " is invalid");
 end;
  end loop;
end;

with Interfaces; use Interfaces;

package My_Types is

   subtype typ_bool is boolean;

   type typ_uint32 is new Interfaces.Unsigned_32;
   subtype typ_uint16 is typ_uint32 range 0..2**16 - 1;

   type typ_dis_en is ( DISABLE, ENABLE );
   for typ_dis_en'size use 32;
   for typ_dis_en use ( DISABLE => 0, ENABLE  => 1 );

type typ_rid is
   (
  RID_0,
  RID_2,
  RID_3,
  RID_4,
  RID_5,
  RID_6,
  RID_7,
  RID_8,
  RID_9,
  RID_10,
  RID_11,
  RID_12,
  RID_13,
  RID_14,
  RID_15,
  RID_16,
  RID_17,
  RID_18,
  RID_19,
  RID_26,
  RID_27,
  RID_28,
  RID_29,
  RID_30
   );
for typ_rid use
   (
  RID_0   =>  16#1000#,
  RID_2   =>  16#1002#,
  RID_3   =>  16#1003#,
  RID_4   =>  16#1004#,
  RID_5   =>  16#1005#,
  RID_6   =>  16#1006#,
  RID_7   =>  16#1007#,
  RID_8   =>  16#1008#,
  RID_9   =>  16#1009#,
  RID_10  =>  16#100A#,
  RID_11  =>  16#100B#,
  RID_12  =>  16#100C#,
  RID_13  =>  16#100D#,
  RID_14  =>  16#100E#,
  RID_15  =>  16#100F#,
  RID_16  =>  16#1010#,
  RID_17  =>  16#1011#,
  RID_18  =>  16#1012#,
  RID_19  =>  16#1013#,
  RID_26  =>  16#101A#,
  RID_27  =>  16#101B#,
  RID_28  =>  16#101C#,
  RID_29  =>  16#101D#,
  RID_30  =>  16#101E#
   );
for typ_rid'size use 16;

end My_Types;


with My_Types;

package  Example is

procedure Check;
procedure dummy
   (
 idx: in My_Types.typ_uint32;
 action : in My_Types.typ_dis_en
   );

end Example;

with Text_IO; use Text_IO;
with Unchecked_Conversion;
with my_types; use my_types;
package body Example is

   type typ_rid_sts is array (My_Types.typ_rid)
  of My_Types.typ_bool;
   for typ_rid_sts'component_size use 1;

   is_rid_en : typ_rid_sts :=
  (TRUE, false, True, False, true, False, True, false, True, False,
  TRUE, false, True, False, true, False, True, false, True, False,
  TRUE, false, True, False);

   procedure Check is
   begin
 pragma Assert (for all I in is_rid_en'range => is_rid_en (I));
   end Check;

   function toRidEvt is new Unchecked_Conversion
  (
 -- Defining source and target types
 source => My_Types.typ_uint16,
 target => My_Types.typ_rid
  );

   procedure dummy (
 idx: in My_Types.typ_uint32;
 action : in My_Types.typ_dis_en)
   is
  rid_evt  : My_Types.typ_rid;

   begin

  rid_evt := toRidEvt(idx);

  if action = My_Types.ENABLE
  then
 is_rid_en(rid_evt) := TRUE;
  else
 is_rid_en(rid_evt) := FALSE;
  end if;

   end dummy;
end Example;

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Ed Schonberg  

gcc/ada/

* exp_pakd.adb (Expand_Bit_Packed_Element_Set): Add explicit
range checks when the index type of the bit-packed array is an
enumeration type with a non-standard representation,--- gcc/ada/exp_pakd.adb
+++ gcc/ada/exp_pakd.adb
@@ -1022,7 +1022,9 @@ package body Exp_Pakd is
   Ass_OK : constant Boolean := Assignment_OK (Lhs);
   --  Used to preserve assignment OK status when assignment is rewritten
 
-  Rhs : Node_Id := Expression (N);
+  Expr : Node_Id;
+
+  Rhs  : Node_Id := Expression (N);
   --  Initially Rhs is the right hand side value, it will be replaced
   --  later by an appropriate unchecked conversion for the assignment.
 
@@ -1140,6 +1142,35 @@ package body Exp_Pakd is
  Analyze_And_Resolve (Rhs, Ctyp);
   end if;
 
+  --  If any of the indices has a nonstandard representation, introduce
+  --  the proper Rep_To_Pos conversion, which in turn will generate index
+  --  checks when needed. We do this on a copy of the index expression,
+  --  rather that rewriting the LHS altogether.
+
+  Expr := First (Expressions (Lhs));
+  

[Ada] Crash on deallocating component with discriminated task

2019-07-05 Thread Pierre-Marie de Rodat
This patch modifies the generation of task deallocation code to examine
the underlying type for task components.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Hristian Kirtchev  

gcc/ada/

* exp_ch7.adb (Cleanup_Record): Use the underlying type when
checking for components with tasks.

gcc/testsuite/

* gnat.dg/task3.adb, gnat.dg/task3.ads, gnat.dg/task3_pkg1.ads,
gnat.dg/task3_pkg2.ads: New testcase.--- gcc/ada/exp_ch7.adb
+++ gcc/ada/exp_ch7.adb
@@ -3893,11 +3893,12 @@ package body Exp_Ch7 is
   Typ  : Entity_Id) return List_Id
is
   Loc   : constant Source_Ptr := Sloc (N);
-  Tsk   : Node_Id;
-  Comp  : Entity_Id;
   Stmts : constant List_Id:= New_List;
   U_Typ : constant Entity_Id  := Underlying_Type (Typ);
 
+  Comp : Entity_Id;
+  Tsk  : Node_Id;
+
begin
   if Has_Discriminants (U_Typ)
 and then Nkind (Parent (U_Typ)) = N_Full_Type_Declaration
@@ -3918,7 +3919,7 @@ package body Exp_Ch7 is
  return New_List (Make_Null_Statement (Loc));
   end if;
 
-  Comp := First_Component (Typ);
+  Comp := First_Component (U_Typ);
   while Present (Comp) loop
  if Has_Task (Etype (Comp))
or else Has_Simple_Protected_Object (Etype (Comp))
@@ -3937,8 +3938,8 @@ package body Exp_Ch7 is
 
 elsif Is_Record_Type (Etype (Comp)) then
 
-   --  Recurse, by generating the prefix of the argument to
-   --  the eventual cleanup call.
+   --  Recurse, by generating the prefix of the argument to the
+   --  eventual cleanup call.
 
Append_List_To (Stmts, Cleanup_Record (N, Tsk, Etype (Comp)));
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/task3.adb
@@ -0,0 +1,11 @@
+--  { dg-do compile }
+
+with Ada.Unchecked_Deallocation;
+
+package body Task3 is
+   procedure Destroy (Obj : in out Child_Wrapper) is
+  procedure Free is new Ada.Unchecked_Deallocation (Child, Child_Ptr);
+   begin
+  Free (Obj.Ptr);
+   end Destroy;
+end Task3;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/task3.ads
@@ -0,0 +1,12 @@
+with Task3_Pkg2; use Task3_Pkg2;
+
+package Task3 is
+   type Child is new Root with null record;
+   type Child_Ptr is access Child;
+
+   type Child_Wrapper is record
+  Ptr : Child_Ptr := null;
+   end record;
+
+   procedure Destroy (Obj : in out Child_Wrapper);
+end Task3;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/task3_pkg1.ads
@@ -0,0 +1,11 @@
+package Task3_Pkg1 is
+   type Task_Wrapper (Discr : Integer) is tagged limited private;
+
+private
+   task type Task_Typ (Discr : Integer) is
+   end Task_Typ;
+
+   type Task_Wrapper (Discr : Integer) is tagged limited record
+  Tsk : Task_Typ (Discr);
+   end record;
+end Task3_Pkg1;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/task3_pkg2.ads
@@ -0,0 +1,7 @@
+with Task3_Pkg1; use Task3_Pkg1;
+
+package Task3_Pkg2 is
+   type Root (Discr : Integer) is tagged limited record
+  Wrap : Task_Wrapper (Discr);
+   end record;
+end Task3_Pkg2;



[Ada] Removing support for SCIL "contract-only" subprogram bodies

2019-07-05 Thread Pierre-Marie de Rodat
Remove support added for CodePeer (which was never enabled by default;
it was controlled by the -gnatd.K option) for generation of SCIL
"contract-only" subprogram bodies. These were intended for use when a
subprogram's "real" body is unavailable but the subprogram spec has
pre/post-conditions specified.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Javier Miranda  

gcc/ada/

* debug.adb (-gnatd.K): Leave available this switch.
* contracts.adb (Build_And_Analyze_Contract_Only_Subprograms):
Remove.
* scil_ll.ads, scil_ll.adb (Contract_Only_Body_Flag,
Contract_Only_Body_Nodes, Get_Contract_Only_Body,
Is_Contract_Only_Body, Set_Contract_Only_Body): Remove.--- gcc/ada/contracts.adb
+++ gcc/ada/contracts.adb
@@ -25,7 +25,6 @@
 
 with Aspects;  use Aspects;
 with Atree;use Atree;
-with Debug;use Debug;
 with Einfo;use Einfo;
 with Elists;   use Elists;
 with Errout;   use Errout;
@@ -51,7 +50,6 @@ with Sinfo;use Sinfo;
 with Snames;   use Snames;
 with Stand;use Stand;
 with Stringt;  use Stringt;
-with SCIL_LL;  use SCIL_LL;
 with Tbuild;   use Tbuild;
 
 package body Contracts is
@@ -63,11 +61,6 @@ package body Contracts is
--
--Part_Of
 
-   procedure Build_And_Analyze_Contract_Only_Subprograms (L : List_Id);
-   --  (CodePeer): Subsidiary procedure to Analyze_Contracts which builds the
-   --  contract-only subprogram body of eligible subprograms found in L, adds
-   --  them to their corresponding list of declarations, and analyzes them.
-
procedure Expand_Subprogram_Contract (Body_Id : Entity_Id);
--  Expand the contracts of a subprogram body and its correspoding spec (if
--  any). This routine processes all [refined] pre- and postconditions as
@@ -354,10 +347,6 @@ package body Contracts is
   Decl : Node_Id;
 
begin
-  if CodePeer_Mode and then Debug_Flag_Dot_KK then
- Build_And_Analyze_Contract_Only_Subprograms (L);
-  end if;
-
   Decl := First (L);
   while Present (Decl) loop
 
@@ -1305,490 +1294,6 @@ package body Contracts is
   Restore_SPARK_Mode (Saved_SM, Saved_SMP);
end Analyze_Task_Contract;
 
-   -
-   -- Build_And_Analyze_Contract_Only_Subprograms --
-   -
-
-   procedure Build_And_Analyze_Contract_Only_Subprograms (L : List_Id) is
-  procedure Analyze_Contract_Only_Subprograms;
-  --  Analyze the contract-only subprograms of L
-
-  procedure Append_Contract_Only_Subprograms (Subp_List : List_Id);
-  --  Append the contract-only bodies of Subp_List to its declarations list
-
-  function Build_Contract_Only_Subprogram (E : Entity_Id) return Node_Id;
-  --  If E is an entity for a non-imported subprogram specification with
-  --  pre/postconditions and we are compiling with CodePeer mode, then
-  --  this procedure will create a wrapper to help Gnat2scil process its
-  --  contracts. Return Empty if the wrapper cannot be built.
-
-  function Build_Contract_Only_Subprograms (L : List_Id) return List_Id;
-  --  Build the contract-only subprograms of all eligible subprograms found
-  --  in list L.
-
-  function Has_Private_Declarations (N : Node_Id) return Boolean;
-  --  Return True for package specs, task definitions, and protected type
-  --  definitions whose list of private declarations is not empty.
-
-  ---
-  -- Analyze_Contract_Only_Subprograms --
-  ---
-
-  procedure Analyze_Contract_Only_Subprograms is
- procedure Analyze_Contract_Only_Bodies;
- --  Analyze all the contract-only bodies of L
-
- --
- -- Analyze_Contract_Only_Bodies --
- --
-
- procedure Analyze_Contract_Only_Bodies is
-Decl : Node_Id;
-
- begin
-Decl := First (L);
-while Present (Decl) loop
-   if Nkind (Decl) = N_Subprogram_Body
- and then Is_Contract_Only_Body
-(Defining_Unit_Name (Specification (Decl)))
-   then
-  Analyze (Decl);
-   end if;
-
-   Next (Decl);
-end loop;
- end Analyze_Contract_Only_Bodies;
-
-  --  Start of processing for Analyze_Contract_Only_Subprograms
-
-  begin
- if Ekind (Current_Scope) /= E_Package then
-Analyze_Contract_Only_Bodies;
-
- else
-declare
-   Pkg_Spec : constant Node_Id :=
-Package_Specification (Current_Scope);
-
-begin
-   if not Has_Private_Declarations (Pkg_Spec) then
-  Analyze_Contract_Only_Bodies;
-
-   --  For packages with private declarations, the contract-only
- 

[Ada] Accept compilation switches -Og/-Ofast in non-GCC backends

2019-07-05 Thread Pierre-Marie de Rodat
Tools like GNATprove built as GNAT backends rely on adabkend.adb to
handle generic switches like the optimisation switches -Oxxx.
This patch adds support for -Og and -Ofast that was missing.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Yannick Moy  

gcc/ada/

* adabkend.adb (Scan_Back_End_Switches): Accept -Og and -Ofast
switches.--- gcc/ada/adabkend.adb
+++ gcc/ada/adabkend.adb
@@ -117,9 +117,11 @@ package body Adabkend is
 
  --  Set optimization indicators appropriately. In gcc-based GNAT this
  --  is picked up from imported variables set by the gcc driver, but
- --  for compilers with non-gcc back ends we do it here to allow use
- --  of these switches by the front end. Allowed optimization switches
- --  are -Os (optimize for size), -O[0123], and -O (same as -O1).
+ --  for compilers with non-gcc back ends we do it here to allow use of
+ --  these switches by the front end. Allowed optimization switches are
+ --  -Os (optimize for size), -O[0123], -O (same as -O1), -Ofast
+ --  (disregard strict standards compliance), and -Og (optimize
+ --  debugging experience).
 
  elsif Switch_Chars (First) = 'O' then
 if First = Last then
@@ -134,10 +136,21 @@ package body Adabkend is
   Optimization_Level :=
 Character'Pos (Switch_Chars (Last)) - Character'Pos ('0');
 
+   --  Switch -Og is between -O0 and -O1 in GCC. Consider it like
+   --  -O0 for other back ends.
+
+   elsif Switch_Chars (Last) = 'g' then
+  Optimization_Level := 0;
+
else
   Fail ("invalid switch: " & Switch_Chars);
end if;
 
+--  Switch -Ofast enables -O3
+
+elsif Switch_Chars (First + 1 .. Last) = "fast" then
+   Optimization_Level := 3;
+
 else
Fail ("invalid switch: " & Switch_Chars);
 end if;
@@ -169,7 +182,7 @@ package body Adabkend is
 
 return;
 
- --  Special check, the back end switch -fno-inline also sets the
+ --  Special check, the back-end switch -fno-inline also sets the
  --  front end flags to entirely inhibit all inlining. So we store it
  --  and set the appropriate flags.
 
@@ -206,7 +219,7 @@ package body Adabkend is
end case;
 end if;
 
- --  Ignore all other back end switches
+ --  Ignore all other back-end switches
 
  elsif Is_Back_End_Switch (Switch_Chars) then
 null;



[Ada] Compiler loop on illegal nested accept statement

2019-07-05 Thread Pierre-Marie de Rodat
This patch fixes a "Compilation abandoned" message in a compiler built
with assertions, or a compiler loop otherwise, when an accept statement
contains an illegal accept statement for the same entry.

Compiling accept_in_accept.adb must yield:

accept_in_accept.adb:12:13:
  duplicate accept statement for same entry (RM 9.5.2 (15))


procedure accept_in_accept is

   task a_in_a is
  entry a (i : Integer);
   end a_in_a;

   task body a_in_a is
   begin
  select
 accept a (i : Integer) do
null;
accept a (i : integer) do
  null;
end a;
 end a;
  or
 terminate;
  end select;
   end a_in_a;

begin
   a_in_a.a (1);
end accept_in_accept;

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Ed Schonberg  

gcc/ada/

* sem_ch9.adb (Analyze_Accept_Statement): If this is an illegal
accept statement for an enclosing entry abandon analysis to
prevent scope mismatches and potential infinite loops in
compiler.--- gcc/ada/sem_ch9.adb
+++ gcc/ada/sem_ch9.adb
@@ -883,7 +883,13 @@ package body Sem_Ch9 is
  exit when Task_Nam = Scope_Stack.Table (J).Entity;
 
  if Entry_Nam = Scope_Stack.Table (J).Entity then
-Error_Msg_N ("duplicate accept statement for same entry", N);
+Error_Msg_N
+  ("duplicate accept statement for same entry (RM 9.5.2 (15))", N);
+
+--  Do not continue analysis of accept statement, to prevent
+--  cascaded errors.
+
+return;
  end if;
   end loop;
 



[Ada] Fix internal error on packed array In/Out actual parameter

2019-07-05 Thread Pierre-Marie de Rodat
This fixes an issue introduced in Ada 2012 for calls to functions taking
an In/Out parameter and for which the actual is the component of a
packed array.  In this case, the front-end needs to create a temporary
for the actual, initialize it before the call and assign it back after
it, because operations on bit-packed arrays are converted into
mask-and-shift sequences.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Eric Botcazou  

gcc/ada/

* exp_ch4.adb (Expand_N_Indexed_Component): Do not expand actual
parameters of function calls here either.

gcc/testsuite/

* gnat.dg/pack23.adb, gnat.dg/pack23_pkg.ads: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -6762,7 +6762,7 @@ package body Exp_Ch4 is
   --Renaming objects in renaming associations
   --  This case is handled when a use of the renamed variable occurs
 
-  --Actual parameters for a procedure call
+  --Actual parameters for a subprogram call
   --  This case is handled in Exp_Ch6.Expand_Actuals
 
   --The second expression in a 'Read attribute reference
@@ -6783,11 +6783,12 @@ package body Exp_Ch4 is
 if Nkind (Parnt) = N_Unchecked_Expression then
null;
 
-elsif Nkind_In (Parnt, N_Object_Renaming_Declaration,
-   N_Procedure_Call_Statement)
+elsif Nkind (Parnt) = N_Object_Renaming_Declaration then
+   return;
+
+elsif Nkind (Parnt) in N_Subprogram_Call
   or else (Nkind (Parnt) = N_Parameter_Association
-and then
-  Nkind (Parent (Parnt)) = N_Procedure_Call_Statement)
+and then Nkind (Parent (Parnt)) in N_Subprogram_Call)
 then
return;
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/pack23.adb
@@ -0,0 +1,14 @@
+--  { dg-do compile }
+--  { dg-options "-gnatws" }
+
+with Pack23_Pkg;
+
+function Pack23 return Integer is
+
+  type Arr is array (1 .. 32) of Boolean with Size => 32, Pack;
+
+  A : Arr;
+
+begin
+  return Pack23_Pkg.Func (A (1));
+end;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/pack23_pkg.ads
@@ -0,0 +1,5 @@
+package Pack23_Pkg is
+
+  function Func (B : in out Boolean) return Integer;
+
+end Pack23_Pkg;



[Ada] Wrong accessibility level under -gnat12

2019-07-05 Thread Pierre-Marie de Rodat
For an anonymous allocator whose type is that of a stand-alone object of
an anonymous access-to-object type, the accessibility level is that of
the declaration of the stand-alone object; however it was incorrectly
computed as being library level compiling under -gnat12 mode.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Javier Miranda  

gcc/ada/

* exp_ch4.adb (Expand_N_Type_Conversion): Do not apply an
accessibility check when the conversion is an access to
class-wide interface type and it is an actual parameter.
* exp_ch6.adb (Expand_Call_Helper): Add documentation on the
accessibility level of an anonymous allocator defining the value
of an access parameter.
* sem_util.ads, sem_util.adb (Dynamic_Accessibility_Level): Add
support for an anonymous allocator whose type is that of a
stand-alone object of an anonymous access to object type.

gcc/testsuite/

* gnat.dg/access6.adb: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -11471,7 +11471,8 @@ package body Exp_Ch4 is
  then
 if not Comes_From_Source (N)
   and then Nkind_In (Parent (N), N_Function_Call,
- N_Procedure_Call_Statement)
+ N_Procedure_Call_Statement,
+ N_Parameter_Association)
   and then Is_Interface (Designated_Type (Target_Type))
   and then Is_Class_Wide_Type (Designated_Type (Target_Type))
 then

--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -3271,7 +3271,10 @@ package body Exp_Ch6 is
 
   --  For allocators we pass the level of the execution of the
   --  called subprogram, which is one greater than the current
-  --  scope level.
+  --  scope level. However, according to RM 3.10.2(14/3) this
+  --  is wrong since for an anonymous allocator defining the
+  --  value of an access parameter, the accessibility level is
+  --  that of the innermost master of the call???
 
   when N_Allocator =>
  Add_Extra_Actual

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -6452,8 +6452,8 @@ package body Sem_Util is
-- Dynamic_Accessibility_Level --
-
 
-   function Dynamic_Accessibility_Level (Expr : Node_Id) return Node_Id is
-  Loc : constant Source_Ptr := Sloc (Expr);
+   function Dynamic_Accessibility_Level (N : Node_Id) return Node_Id is
+  Loc : constant Source_Ptr := Sloc (N);
 
   function Make_Level_Literal (Level : Uint) return Node_Id;
   --  Construct an integer literal representing an accessibility level
@@ -6473,7 +6473,12 @@ package body Sem_Util is
 
   --  Local variables
 
-  E : Entity_Id;
+  Expr : constant Node_Id := Original_Node (N);
+  --  Expr references the original node because at this stage N may be the
+  --  reference to a variable internally created by the frontend to remove
+  --  side effects of an expression.
+
+  E: Entity_Id;
 
--  Start of processing for Dynamic_Accessibility_Level
 
@@ -6530,12 +6535,66 @@ package body Sem_Util is
 
  when N_Allocator =>
 
---  Unimplemented: depends on context. As an actual parameter where
---  formal type is anonymous, use
---Scope_Depth (Current_Scope) + 1.
---  For other cases, see 3.10.2(14/3) and following. ???
+--  This is not fully implemented since it depends on context (see
+--  3.10.2(14/3-14.2/3). More work is needed in the following cases
+--
+--  1) For an anonymous allocator defining the value of an access
+-- parameter, the accessibility level is that of the innermost
+-- master of the call; however currently we pass the level of
+-- execution of the called subprogram, which is one greater
+-- than the current scope level (see Expand_Call_Helper).
+--
+-- For example, a statement is a master and a declaration is
+-- not a master; so we should not pass in the same level for
+-- the following cases:
+--
+-- function F (X : access Integer) return T is ... ;
+-- Decl : T := F (new Integer); -- level is off by one
+--  begin
+-- Decl := F (new Integer); -- we get this case right
+--
+--  2) For an anonymous allocator that defines the result of a
+-- function with an access result, the accessibility level is
+-- determined as though the allocator were in place of the call
+-- of the function. In the special case of a call that is th

[Ada] Add contracts to Ada.Text_IO for SPARK

2019-07-05 Thread Pierre-Marie de Rodat
This change removes the warnings returned when using Ada.Text_IO library
in SPARK. An abstract state and global contracts were added to modelize
the action of Text_IO procedures and function on the memory and the
files.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Joffrey Huguet  

gcc/ada/

* libgnat/a-textio.adb: Add abstract state refinment.
* libgnat/a-textio.ads: Add File_System abstract state.  Add
global contracts, contract cases, preconditions and
postconditions to procedures and functions.
(Set_Input, Set_Output, Set_Error, Standard_Input,
Standard_Output, Standard_Error, Current_Input, Current_Output,
Current_Error): Turn SPARK_Mode off.
(Get_Line): Turn SPARK_Mode off on Get_Line functions.
* libgnat/a-tideio.ads, libgnat/a-tienio.ads,
libgnat/a-tifiio.ads, libgnat/a-tiflio.ads,
libgnat/a-tiinio.ads, libgnat/a-timoio.ads: Add global
contracts, contract cases, preconditions and postconditions to
procedures and functions.

patch.diff.gz
Description: application/gzip


[Ada] Failure to detect trivial infinite recursion

2019-07-05 Thread Pierre-Marie de Rodat
This patch includes delay statements in the set of control flow
statements since their expressions may have side effects, which in turn
may affect an infinite recursion.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Hristian Kirtchev  

gcc/ada/

* sem_res.adb (Is_Control_Flow_Statement): Delay statements
contain an expression, which in turn may have side effects and
affect the infinite recursion. As a result, delay statements
should not be treated specially.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -816,19 +816,11 @@ package body Sem_Res is
 
   function Is_Control_Flow_Statement (N : Node_Id) return Boolean is
   begin
- --  Delay statements do not affect the control flow because they
- --  simply postpone the execution of all subsequent statements.
+ --  It is assumed that all statements may affect the control flow in
+ --  some way. A raise statement may be expanded into a non-statement
+ --  node.
 
- if Nkind (N) in N_Delay_Statement then
-return False;
-
- --  Otherwise it is assumed that all other statements may affect the
- --  control flow in some way. A raise statement may be expanded into
- --  a non-statement node.
-
- else
-return Is_Statement (N) or else Is_Raise_Statement (N);
- end if;
+ return Is_Statement (N) or else Is_Raise_Statement (N);
   end Is_Control_Flow_Statement;
 
   



[Ada] Incorrect accessibility check

2019-07-05 Thread Pierre-Marie de Rodat
This patch fixes an issue whereby anonymous access result types were
treated as having the same accessibility level as typed results instead
of having the level determined by the "master of the call" as per RM
3.10.2 (10).


-- Source --


--  main.adb

with Pack_12; use Pack_12;
with Pack_05; use Pack_05;

procedure Main is
   Obj : aliased Integer;
begin
   Test_Alloc
 (new Rec_T'(Disc => Id_A (Obj'Access))); --  OK

   Id_A (Obj'Access).all := 0;--  OK
   Id_B (Obj'Access).all := 0;--  OK
   Id_C (Obj'Access).all := 0;--  ERROR
end Main;

--  pack_12.ads

pragma Ada_2012;

with Ada.Unchecked_Conversion;

package Pack_12 is
   function Id_A (I : access Integer)
 return access Integer
 is (I);

   type Obj_Ptr is access all Integer;

   function Id_C (I : access Integer)
 return Obj_Ptr
 is (I.all'Access);

   type Rec_T (Disc : access Integer) is null record;

   procedure Test_Alloc (Access_Param : access Rec_T);
end Pack_12;

--  pack_12.adb

package body Pack_12 is
   Dummy : Integer;

   procedure Test_Alloc (Access_Param : access Rec_T) is
   begin
  Dummy := Access_Param.Disc.all;
   end Test_Alloc;
end Pack_12;

--  pack_05.ads

pragma Ada_2005;

with Pack_12; use Pack_12;

package Pack_05 is
   function Id_B (I : access Integer)
 return access Integer
 renames Id_A;
end Pack_05;

-
-- Compilation --
-

$ gnatmake -q main.adb
$ main
raised PROGRAM_ERROR : pack_12.ads:14 accessibility check failed

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Justin Squirek  

gcc/ada/

* checks.adb (Apply_Accessibility_Check): Add logic to fetch the
function result accessibility level if one is required within
the generated check.
* exp_ch6.adb (Needs_Result_Accessibility_Level): Modify
controlling elsif block to handle more cases such as anonymous
access results and disable checking for coextensions.--- gcc/ada/checks.adb
+++ gcc/ada/checks.adb
@@ -617,8 +617,23 @@ package body Checks is
  Param_Level :=
New_Occurrence_Of (Extra_Accessibility (Param_Ent), Loc);
 
- Type_Level :=
-   Make_Integer_Literal (Loc, Deepest_Type_Access_Level (Typ));
+ --  Use the dynamic accessibility parameter for the function's result
+ --  when one has been created instead of statically referring to the
+ --  deepest type level so as to appropriatly handle the rules for
+ --  RM 3.10.2 (10.1/3).
+
+ if Ekind_In (Scope (Param_Ent), E_Function,
+ E_Operator,
+ E_Subprogram_Type)
+   and then Present (Extra_Accessibility_Of_Result (Scope (Param_Ent)))
+ then
+Type_Level :=
+  New_Occurrence_Of
+(Extra_Accessibility_Of_Result (Scope (Param_Ent)), Loc);
+ else
+Type_Level :=
+  Make_Integer_Literal (Loc, Deepest_Type_Access_Level (Typ));
+ end if;
 
  --  Raise Program_Error if the accessibility level of the access
  --  parameter is deeper than the level of the target access type.

--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -9236,8 +9236,9 @@ package body Exp_Ch6 is
  return False;
   end Has_Unconstrained_Access_Discriminant_Component;
 
-  Feature_Disabled : constant Boolean := True;
-  --  Temporary
+  Disable_Coextension_Cases : constant Boolean := True;
+  --  Flag used to temporarily disable a "True" result for types with
+  --  access discriminants and related coextension cases.
 
--  Start of processing for Needs_Result_Accessibility_Level
 
@@ -9247,9 +9248,6 @@ package body Exp_Ch6 is
   if not Present (Func_Typ) then
  return False;
 
-  elsif Feature_Disabled then
- return False;
-
   --  False if not a function, also handle enum-lit renames case
 
   elsif Func_Typ = Standard_Void_Type
@@ -9274,23 +9272,37 @@ package body Exp_Ch6 is
   elsif Ada_Version < Ada_2012 then
  return False;
 
-  elsif Ekind (Func_Typ) = E_Anonymous_Access_Type
-or else Is_Tagged_Type (Func_Typ)
-  then
- --  In the case of, say, a null tagged record result type, the need
- --  for this extra parameter might not be obvious. This function
- --  returns True for all tagged types for compatibility reasons.
- --  A function with, say, a tagged null controlling result type might
- --  be overridden by a primitive of an extension having an access
- --  discriminant and the overrider and overridden must have compatible
- --  calling conventions (including implicitly declared parameters).
- --  Similarly, values of one access-to-subprogram type might designate
- --  both a primitive subprogram of a given type and a functio

[Ada] Failure to detect trivial infinite recursion

2019-07-05 Thread Pierre-Marie de Rodat
This patch reimplements the detection of trivial infinite recursion to
remove the implicit assumptions concenring the structure and contents of
the enclosing subprogram statements.


-- Source --


--  infinite.adb

procedure Infinite with SPARK_Mode is
   function Func_1 (Val : Integer) return Integer;
   function Func_2 (Val : Integer) return Integer;
   function Func_3 (Val : Integer) return Integer;
   function Func_4 (Val : Integer) return Integer;
   function Func_5 (Val : Integer) return Integer;
   function Func_6 (Val : Integer) return Integer;
   function Func_7 (Val : Integer) return Integer;
   function Func_8 (Val_1 : Integer; Val_2 : Integer) return Integer;
   procedure Proc_1 (Val : Integer);

   function Func_1 (Val : Integer) return Integer is
   begin
  return Func_1 (Val);   --  WARN
   end Func_1;

   function Func_2 (Val : Integer) return Integer is
   begin
  return Func_2 (123);   --  none
   end Func_2;

   function Func_3 (Val : Integer) return Integer is
  Temp : Integer;
   begin
  Temp := Func_3 (Val);  --  WARN
  return Temp;
   end Func_3;

   function Func_4 (Val : Integer) return Integer is
  Temp : Integer;
   begin
  Temp := Func_4 (123);  --  none
  return Temp;
   end Func_4;

   function Func_5 (Val : Integer) return Integer is
   begin
  Proc_1 (Val);
  return Func_5 (Val);   --  none
   end Func_5;

   function Func_6 (Val : Integer) return Integer is
   begin
  Proc_1 (Val);
  return Func_6 (123);   --  none
   end Func_6;

   function Func_7 (Val : Integer) return Integer is
   begin
  raise Program_Error;
  return Func_7 (Val);   --  none
   end Func_7;

   function Func_8 (Val_1 : Integer; Val_2 : Integer) return Integer is
   begin
  return Func_8 (Val_1, 123);--  none
   end Func_8;

   procedure Proc_1 (Val : Integer) is
   begin
  Proc_1 (Val);  --  WARN
   end Proc_1;

begin null; end Infinite;


-- Compilation and output --


$ gcc -c infinite.adb
infinite.adb:14:14: infinite recursion
infinite.adb:14:14: Storage_Error would have been raised at run time
infinite.adb:25:15: infinite recursion
infinite.adb:25:15: Storage_Error would have been raised at run time
infinite.adb:61:07: infinite recursion
infinite.adb:61:07: Storage_Error would have been raised at run time

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Hristian Kirtchev  

gcc/ada/

* sem_res.adb (Check_Infinite_Recursion): Reimplemented.
(Enclosing_Declaration_Or_Statement,
Invoked_With_Different_Arguments, Is_Conditional_Statement,
Is_Control_Flow_Statement, Is_Immediately_Within_Body,
Is_Raise_Idiom, Is_Raise_Statement, Is_Sole_Statement,
Preceded_By_Control_Flow_Statement,
Within_Conditional_Statement): New routines.--- gcc/ada/sem_res.adb
+++ gcc/ada/sem_res.adb
@@ -111,8 +111,8 @@ package body Sem_Res is
   Pref : Node_Id);
--  Check that the type of the prefix of a dereference is not incomplete
 
-   function Check_Infinite_Recursion (N : Node_Id) return Boolean;
-   --  Given a call node, N, which is known to occur immediately within the
+   function Check_Infinite_Recursion (Call : Node_Id) return Boolean;
+   --  Given a call node, Call, which is known to occur immediately within the
--  subprogram being called, determines whether it is a detectable case of
--  an infinite recursion, and if so, outputs appropriate messages. Returns
--  True if an infinite recursion is detected, and False otherwise.
@@ -695,164 +695,406 @@ package body Sem_Res is
-- Check_Infinite_Recursion --
--
 
-   function Check_Infinite_Recursion (N : Node_Id) return Boolean is
-  P : Node_Id;
-  C : Node_Id;
+   function Check_Infinite_Recursion (Call : Node_Id) return Boolean is
+  function Enclosing_Declaration_Or_Statement (N : Node_Id) return Node_Id;
+  --  Return the nearest enclosing declaration or statement that houses
+  --  arbitrary node N.
 
-  function Same_Argument_List return Boolean;
-  --  Check whether list of actuals is identical to list of formals of
-  --  called function (which is also the enclosing scope).
+  function Invoked_With_Different_Arguments (N : Node_Id) return Boolean;
+  --  Determine whether call N invokes the related enclosing subprogram
+  --  with actuals that differ from the subprogram's formals.
 
-  
-  -- Same_Argument_List --
-  
+  function 

[Ada] Stabilization of Elaboration order v4.0

2019-07-05 Thread Pierre-Marie de Rodat
This patch introduces several changes to the new elaboration order
mechanism:

   * Instantiations processed in the context of invocation graph
 encoding now yield a relation which is later transformed into an
 invocation edge. This ensures that the unit where the instantiation
 resides properly depends on the unit where the body of the generic
 is.

   * The diagnostics of cycles that involve invocation edges now use a
 set to avoid infinite recursion when visiting paths that represent
 recursive code.

   * Various diagnostics that suggest the use of switches have been
 updated to indicate which tool the switches apply to.

   * Bindo can now output the dependencies of various units that specify
 why a predecessor unit must be elaborated prior to a successor
 unit. This functionality implements binder switch -e (output
 complete list of elaboration order dependencies).

   * The output of the elaboration order is now identical to that
 emitted by Binde.

   * The nature of the invocation graph encoding is now recorded in the
 ALI record rather than the Unit record of a unit. This ensures that
 both the spec and body share the same encoding kind.

   * A section on debugging elaboration order issues is now available in
 Bindo.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-05  Hristian Kirtchev  

gcc/ada/

* ali.adb (For_Each_Invocation_Construct,
For_Each_Invocation_Relation): New version.
(Scan_ALI): Initialize field Invocation_Graph_Encoding.
(Set_Invocation_Graph_Encoding): Update the setting of the
invocation graph encoding.
* ali.ads: Move field Invocation_Graph_Encoding from Unit_Record
to ALI_Record because the encoding applies to the whole ALI,
rather than one of the units (spec or body) for which the ALI
file was created.
(For_Each_Invocation_Construct, For_Each_Invocation_Relation):
New version.
* bindo.adb: Update the section on switches.  Complete the
section of debugging elaboration order issues.
(Find_Elaboration_Order): Prepare the routine for the switch
from the old to the new elaboration order mechanism.
* bindo-diagnostics.adb (Find_And_Output_Invocation_Paths):
Manage a visited set used by Visit_Vertex.
(Output_All_Cycles_Suggestions,
Output_Dynamic_Model_Suggestions): Clarify the nature of the
suggested switch.
(Output_Elaborate_Body_Transition): Update the diagnostic to
emit a better message.
(Output_Forced_Suggestions, Output_Full_Encoding_Suggestions):
Clarify the nature of the suggested switch.
(Visit_Vertex): Update the parameter profile to add a set of
invokers visited during the transition. This set prevents
infinite exploration of the graph in case the invocations are
recursive.
* bindo-elaborators.adb: Add a use clause for
Bindo.Writers.Dependency_Writers.
(Elaborate_Units_Common): Output the library graph after it has
been augmented with invocation edges. Output just the components
instead of outputting the whole library graph again.
(Elaborate_Units_Dynamic, Elaborate_Units_Static): Output the
dependencies as expressed in the library graph.
* bindo-units.adb (Invocation_Graph_Encoding): Update the
extraction of the invocation graph encoding.
* bindo-writers.adb: Add with and use clauses for Binderr and
Butil.
(palgc, plgc): New debug routine.
(Write_Components): Moved to the spec. Add a header for the
output.
(Write_Dependencies, Write_Dependencies_Of_Vertex,
Write_Dependency_Edge): New routine.
(Write_Elaboration_Order): Update the logic to follow the format
of Binde's order output.
(Write_Library_Graph): Do not output the components every time
the graph is written.
(Write_Unit): Output the invocation graph encoding of the unit.
Output the invocation constructs and relations for the unit
only.
* bindo-writers.ads (Write_Components): Moved from the body.
(Write_Dependencies): New routine.
* bindusg.adb: Prepare the routine for the switch from the old
to the new elaboration order mechanism.
* debug.adb: Binder switch -d_O is now not associated with any
functionality.
* einfo.adb (Is_Elaboration_Target): The attribute applies to
packages, as specified by the comment on the attribute usage.
* opt.ads: Add a global flag which controls the choice between
the new and the legacy elaboration order mechanism.
* sem_elab.adb: Add Package_Target to type Target_Kind.
(Build_Elaborate_Body_Procedure, Build_Elaborate_Procedure,
Build_Elaborate_Spec_Procedure, Check_Elaboration_Scenarios,
Check_SPARK_Model_In_

Fix uninitialised use in mips_split_move

2019-07-05 Thread Richard Sandiford
While testing the fix for PR91068, I hit an rtl checking failure
while building newlib.  mips_split_move was decomposing an address that
happened to be symbolic and then tried to access the REGNO of the base
register field, which wasn't initialised but which by chance pointed to
valid memory.

Tested on mipsisa64-elf.  OK to install?

Richard


2019-07-05  Richard Sandiford  

gcc/
* config/mips/mips.c (mips_split_move): Zero-initialize addr
and check whether addr.reg is nonnull before using it.

Index: gcc/config/mips/mips.c
===
--- gcc/config/mips/mips.c  2019-07-01 09:37:07.300523814 +0100
+++ gcc/config/mips/mips.c  2019-07-05 09:46:46.407526855 +0100
@@ -4849,7 +4849,7 @@ mips_split_move (rtx dest, rtx src, enum
  can forward SRC for DEST.  This is most useful if the next insn is a
  simple store.   */
   rtx_insn *insn = (rtx_insn *)insn_;
-  struct mips_address_info addr;
+  struct mips_address_info addr = {};
   if (insn)
 {
   rtx_insn *next = next_nonnote_nondebug_insn_bb (insn);
@@ -4862,7 +4862,7 @@ mips_split_move (rtx dest, rtx src, enum
{
  rtx tmp = XEXP (src, 0);
  mips_classify_address (&addr, tmp, GET_MODE (tmp), true);
- if (REGNO (addr.reg) != REGNO (dest))
+ if (addr.reg && REGNO (addr.reg) != REGNO (dest))
validate_change (next, &SET_SRC (set), src, false);
}
  else


PR91068: Fix MIPS fallout from IRA matched operand changes

2019-07-05 Thread Richard Sandiford
PR91068 is a case in which we have (ignoring non-LRA alternatives):

  [(set (match_operand:SI 0 "register_operand" "=l,d?")
(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d")
  (match_operand:SI 2 "register_operand" "d,d"))
 (match_operand:SI 3 "register_operand" "0,d")))
   (clobber (match_scratch:SI 4 "=X,l"))
   (clobber (match_scratch:SI 5 "=X,&d"))]

where the first alternative is one instruction but the second is two.
This is very similar to the case that my recent IRA patches were
supposed to help.  The crucial difference is that the cheap
alternative requires a single-register class while the expensive
alternative uses general registers.

This makes a difference when one of operand 0 or 3 can naturally be
allocated to LO but the other can't.  If IRA makes that allocation,
both alternatives require one reload of equal cost and so the first
alternative clearly wins.

However, if we say that tying operands 0 and 3 saves the cost of a full
move, then all other things being equal, IRA will prefer to allocate
both registers to the same GPR.  The registers will then naturally
fit the second alternative.

This has a more drastic effect in the MIPS case than it should because
using the GPR alternative is much more expensive there than it appears
to the RA.  But that's really a separate problem and something we were
able to live with before my IRA patch.

What makes tying less useful here is the fact that the tied register is
a single-register class.  I think in those circumstances it's better not
to use tied operands at all and instead use "l" for the inputs.
Allocating the input to LO, and allocating the output to LO, then depend
naturally on class costs.  If we decide to allocate at least one of them
to LO, we'll use the cheap alternative, otherwise we'll (correctly) use
the expensive alternative.  This effectively restores the situation
before my IRA patch, but this time making the preference on the input
register more explicit.

I originally wrote the patterns in the early days of IRA, and certainly
well before LRA.  I think they were largely influened by reload rather
than RA proper (see the comment above *mul_acc_si, which is all about
the reload behaviour).  LRA copes with the two-"l" case just fine.

The patch may well cause problems for -mno-lra, but I think we should
cull that option anyway.

Tested on mipsisa64-elf.  OK to install?

Richard


2019-07-05  Richard Sandiford  

gcc/
PR target/91068
* config/mips/mips.md (*mul_acc_si, *mul_acc_si_r3900, *macc)
(*msac, *msac_using_macc, *mul_sub_si): Use "l" for input operands
instead of matching them to "l" output operands.

Index: gcc/config/mips/mips.md
===
--- gcc/config/mips/mips.md 2019-07-01 09:37:07.292523884 +0100
+++ gcc/config/mips/mips.md 2019-07-05 09:46:55.219455545 +0100
@@ -1749,7 +1749,7 @@ (define_insn "*mul_acc_si"
   [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d?")
(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d")
  (match_operand:SI 2 "register_operand" "d,d,d"))
-(match_operand:SI 3 "register_operand" "0,0,d")))
+(match_operand:SI 3 "register_operand" "l,l,d")))
(clobber (match_scratch:SI 4 "=X,X,l"))
(clobber (match_scratch:SI 5 "=X,X,&d"))]
   "GENERATE_MADD_MSUB && !TARGET_MIPS16"
@@ -1778,7 +1778,7 @@ (define_insn "*mul_acc_si_r3900"
   [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d*?,d?")
(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d,d")
  (match_operand:SI 2 "register_operand" "d,d,d,d"))
-(match_operand:SI 3 "register_operand" "0,0,l,d")))
+(match_operand:SI 3 "register_operand" "l,l,l,d")))
(clobber (match_scratch:SI 4 "=X,X,3,l"))
(clobber (match_scratch:SI 5 "=X,X,X,&d"))]
   "TARGET_MIPS3900 && !TARGET_MIPS16"
@@ -1822,7 +1822,7 @@ (define_insn "*macc"
   [(set (match_operand:SI 0 "register_operand" "=l,d")
(plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d")
  (match_operand:SI 2 "register_operand" "d,d"))
-(match_operand:SI 3 "register_operand" "0,l")))
+(match_operand:SI 3 "register_operand" "l,l")))
(clobber (match_scratch:SI 4 "=X,3"))]
   "ISA_HAS_MACC"
 {
@@ -1842,7 +1842,7 @@ (define_insn "*macc"
 
 (define_insn "*msac"
   [(set (match_operand:SI 0 "register_operand" "=l,d")
-(minus:SI (match_operand:SI 1 "register_operand" "0,l")
+(minus:SI (match_operand:SI 1 "register_operand" "l,l")
   (mult:SI (match_operand:SI 2 "register_operand" "d,d")
(match_operand:SI 3 "register_operand" "d,d"
(clobber (match_scratch:SI 4 "=X,1"))]
@@ -1862,7 +1862,7 @@ (define_insn "*msac"
 ;; An msac-like instruction implemented 

Add early exit to access path oracle

2019-07-05 Thread Jan Hubicka
Hi,
this patch adds early exit into nonoverlapping_component_refs_since_match_p
so we do not try to parse the access paths when there is obviously nothing to 
do.
It also improves statistics byt not accounting the early exit as a querry
and also to account cases we found both paths to be matching (so we have kind
of must alias assuming that bases are the same modulo array refs that I plan
to fix soon too).

Bootstrapped/regtested x86_64-linux, comitted.

The stats now look as follows:

Alias oracle query stats:
  refs_may_alias_p: 4392494 disambiguations, 4762170 queries
  ref_maybe_used_by_call_p: 6790 disambiguations, 4418867 queries
  call_may_clobber_ref_p: 883 disambiguations, 883 queries
  nonoverlapping_component_refs_p: 0 disambiguations, 33 queries
  nonoverlapping_component_refs_since_match_p: 66 disambiguations, 2330 must 
overlaps, 2586 queries
  aliasing_component_refs_p: 932 disambiguations, 30391 queries
  TBAA oracle: 1860937 disambiguations 3844917 queries
   774883 are in alias set 0
   713704 queries asked about the same object
   0 queries asked about the same alias set
   0 access volatile
   356107 are dependent in the DAG
   139286 are aritificially in conflict with void *

PTA query stats:
  pt_solution_includes: 911807 disambiguations, 1226678 queries
  pt_solutions_intersect: 121134 disambiguations, 551599 queries

So we have relatively little work left for nonoverlapping_component_refs_p
and also nonoverlapping_component_refs_since_match_p has overal 256 querries
it may disambiguate because there are different paths, and it suceeds in 66 of 
them.

Honza

* tree-ssa-alias.c (alias_stats): Add
nonoverlapping_component_refs_since_match_p_must_overlap.
(dump_alias_stats): Print it.
(nonoverlapping_component_refs_since_match_p): Add early exit.
(nonoverlapping_component_refs_p): Do not account early exit.
Index: tree-ssa-alias.c
===
--- tree-ssa-alias.c(revision 273130)
+++ tree-ssa-alias.c(working copy)
@@ -105,6 +105,7 @@ static struct {
   unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_may_alias;
   unsigned HOST_WIDE_INT nonoverlapping_component_refs_p_no_alias;
   unsigned HOST_WIDE_INT nonoverlapping_component_refs_since_match_p_may_alias;
+  unsigned HOST_WIDE_INT 
nonoverlapping_component_refs_since_match_p_must_overlap;
   unsigned HOST_WIDE_INT nonoverlapping_component_refs_since_match_p_no_alias;
 } alias_stats;
 
@@ -138,10 +139,13 @@ dump_alias_stats (FILE *s)
   + alias_stats.nonoverlapping_component_refs_p_may_alias);
   fprintf (s, "  nonoverlapping_component_refs_since_match_p: "
   HOST_WIDE_INT_PRINT_DEC" disambiguations, "
+  HOST_WIDE_INT_PRINT_DEC" must overlaps, "
   HOST_WIDE_INT_PRINT_DEC" queries\n",
   alias_stats.nonoverlapping_component_refs_since_match_p_no_alias,
+  alias_stats.nonoverlapping_component_refs_since_match_p_must_overlap,
   alias_stats.nonoverlapping_component_refs_since_match_p_no_alias
-  + alias_stats.nonoverlapping_component_refs_since_match_p_may_alias);
+  + alias_stats.nonoverlapping_component_refs_since_match_p_may_alias
+  + 
alias_stats.nonoverlapping_component_refs_since_match_p_must_overlap);
   fprintf (s, "  aliasing_component_refs_p: "
   HOST_WIDE_INT_PRINT_DEC" disambiguations, "
   HOST_WIDE_INT_PRINT_DEC" queries\n",
@@ -1149,6 +1153,17 @@ static int
 nonoverlapping_component_refs_since_match_p (tree match1, tree ref1,
 tree match2, tree ref2)
 {
+  /* Early return if there are no references to match, we do not need
+ to walk the access paths.
+
+ Do not consider this as may-alias for stats - it is more useful
+ to have information how many disambiguations happened provided that
+ the query was meaningful.  */
+
+  if (match1 == ref1 || !handled_component_p (ref1)
+  || match2 == ref2 || !handled_component_p (ref2))
+return -1;
+
   auto_vec component_refs1;
   auto_vec component_refs2;
 
@@ -1214,7 +1229,7 @@ nonoverlapping_component_refs_since_matc
  if (component_refs1.is_empty ())
{
  ++alias_stats
-   .nonoverlapping_component_refs_since_match_p_may_alias;
+   .nonoverlapping_component_refs_since_match_p_must_overlap;
  return 0;
}
  ref1 = component_refs1.pop ();
@@ -1226,7 +1241,7 @@ nonoverlapping_component_refs_since_matc
  if (component_refs2.is_empty ())
{
  ++alias_stats
-   .nonoverlapping_component_refs_since_match_p_may_alias;
+   .nonoverlapping_component_refs_since_match_p_must_overlap;
  return 0;
}
  ref2 = component_refs2.pop ();
@@ -1266,7 +1281,7 @@ nonoverlapping_component_ref

Re: [PATCH] Add generic support for "noinit" attribute

2019-07-05 Thread Christophe Lyon
On Thu, 4 Jul 2019 at 23:46, Jozef Lawrynowicz  wrote:
>
> Hi,
>
> > diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
> > index 365e9eb..8266fa0 100644
> > --- a/gcc/config/msp430/msp430.c
> > +++ b/gcc/config/msp430/msp430.c
> > @@ -1807,7 +1807,6 @@ const char * const  ATTR_CRIT   = "critical";
> >  const char * const  ATTR_LOWER  = "lower";
> >  const char * const  ATTR_UPPER  = "upper";
> >  const char * const  ATTR_EITHER = "either";
> > -const char * const  ATTR_NOINIT = "noinit";
> >  const char * const  ATTR_PERSIST = "persistent";
> >
> >  static inline bool
>
> With this change removed so that ATTR_NOINIT is still defined, your patch is
> ok for msp430 - the attribute still behaves as expected.
Oops sorry for this.

> I'm holding off using default_elf_select_section instead of
> default_select_section in msp430.c since there could be some possible 
> conflicts
> with the MSPABI introduced by using elf_select_section that need to be 
> properly
> considered before making the change.
>
> I think default_select_section is supposed to be very terse, so I'm not sure
> if it would be even be suitable to extend it to handle noinit.
Yes, that was my worry too.

> With that said, could you please add the following FIXME to your patch:
OK

> diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
> index 365e9eba747..b403b1f5a42 100644
> --- a/gcc/config/msp430/msp430.c
> +++ b/gcc/config/msp430/msp430.c
> @@ -2338,6 +2336,8 @@ msp430_select_section (tree decl, int reloc, unsigned
> HOST_WIDE_INT align) {
>if (TREE_CODE (decl) == FUNCTION_DECL)
> return text_section;
> +  /* FIXME: ATTR_NOINIT is handled generically in
> + default_elf_select_section.  */
>else if (has_attr (ATTR_NOINIT, decl))
> return noinit_section;
>else if (has_attr (ATTR_PERSIST, decl))
>
> Also, the gcc.target/arm/noinit-attribute.c test works with msp430.
> Why not create a effective-target keyword which checks for noinit support, so
> the test can be gated on that condition and put in a generic part of the
> testsuite?

That was my intention, and I was waiting for feedback on this. In this
case, I suppose the effective-target check would test a hardcoded list
of targets, like many other effective-targets?
(eg check_weak_available)

> After doing some further testing, I noticed the attribute does not work on
> static variables. The attribute handling code allows the attribute to be set 
> on
> a static variable, but then that variable does not get placed in the .noinit
> section.
>
> What are your thoughts on this?
>
> Does it even makes sense for a static local variable to be declared as 
> "noinit"?
> One should want a static local variable to be initialized, as having it not
> initialized and hold a random value could cause problems.
> Perhaps a user could think of some use for this, but I can't.
>
I added:
static int __attribute__((noinit)) var_noinit2;
in global scope, and
static int __attribute__((noinit)) var_noinit3;
in main(), and the generate code contains:
.section.noinit,"aw"
.align  2
.set.LANCHOR2,. + 0
.type   var_noinit2, %object
.size   var_noinit2, 4
var_noinit2:
.space  4
.type   var_noinit3.4160, %object
.size   var_noinit3.4160, 4
var_noinit3.4160:
.space  4
.type   var_noinit, %object
.size   var_noinit, 4
var_noinit:
.space  4

So, all three of them are in the .noinit section, but only var_noinit has
.global var_noinit

So it seems to work?

> Finally, I would point out that "contrib/check_GNU_style.sh" reports some 
> style
> issues with your patch.

Thanks for noticing.

>
> Thanks and regards,
> Jozef


[PATCH] Fix no-strict-aliasing condition in VN

2019-07-05 Thread Richard Biener


There's a path I added where get_alias_set () != 0 was meant to
cover -fno-strict-aliasing but it doesn't since quite some time
(oops).  Fixed as follows - preprartory for PR91091 where we'd
otherwise break the testcase below.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

I'll also backport this to the GCC 8/9 branches where the issue
is latent.

Richard.

2019-07-05  Richard Biener  

PR tree-optimization/91091
* tree-ssa-sccvn.c (vn_reference_lookup_3): Overlap of
accesses can happen with -fno-strict-aliasing.

* gcc.dg/tree-ssa/pr91091-1.c: New testcase.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273133)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -1996,7 +1996,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
 activation of a union member via a store makes the
 values of untouched bytes unspecified.  */
  && (known_eq (ref->size, BITS_PER_UNIT)
- || (get_alias_set (lhs) != 0
+ || (flag_strict_aliasing
+ && get_alias_set (lhs) != 0
  && ao_ref_alias_set (ref) != 0)))
{
  tree *saved_last_vuse_ptr = data->last_vuse_ptr;
Index: gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c   (working copy)
@@ -0,0 +1,23 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-strict-aliasing" } */
+
+struct s { int x; } __attribute__((packed));
+struct t { int x; };
+
+void __attribute__((noinline,noipa))
+swap(struct s* p, struct t* q)
+{
+  p->x = q->x;
+  q->x = p->x;
+}
+
+int main()
+{
+  struct t a[2];
+  a[0].x = 0x12345678;
+  a[1].x = 0x98765432;
+  swap ((struct s *)((char *)a + 1), a);
+  if (a[0].x != 0x12345678)
+__builtin_abort ();
+  return 0;
+}


Re: [PATCH][GCC][AARCH64] PR target/90712 Fix gcc.dg/rtl/aarch64/subs_adds_sp.c regression

2019-07-05 Thread Richard Earnshaw


On 02/07/2019 12:00, Sam Tebbs wrote:
> Hi all,
> 
> This patch fixes the regression to gcc.dg/rtl/aarch64/subs_adds_sp.c that
> r271735 caused. This was done by ensuring that the current function's
> frame has
> been laid out before checking if return address signing is enabled.
> 
> Tested and built on aarch64-none-elf and aarch64-none-linux-gnu.
> 
> OK for trunk?
> 
> Sam Tebbs
> 
> gcc/
> 2019-06-20  Sam Tebbs
> 
>   PR target/90712
>   * aarch64/aarch64.c (aarch64_post_cfi_startproc): Replace thunk check
>   with a frame laid out check.
> 


OK.

R.


[PATCH] Fix PR91091 and more

2019-07-05 Thread Richard Biener


When working on handling partial defs in VN I noticed that
we do not treat MEM[a + 1] and MEM[a + 2] as equal bases.
The following patch fixes that, together with three testcases
illustrating the fixed missed optimizations.

This then caused bogus redundant store removal by FRE again
which made me refactor how we handle its !tbaa_p handling,
fixing a few omissions on the way as well.  Which turned
out to fix PR91091 (see the separate fix for another issue
_that_ triggered...).

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

Richard.

2019-07-05  Richard Biener  

PR tree-optimization/91091
* tree-ssa-alias.h (get_continuation_for_phi): Add tbaa_p parameter.
(walk_non_aliased_vuses): Likewise.
* tree-ssa-alias.c (maybe_skip_until): Pass down tbaa_p.
(get_continuation_for_phi): New tbaa_p parameter and pass
it down.
(walk_non_aliased_vuses): Likewise.
* ipa-prop.c (determine_known_aggregate_parts): Adjust.
* tree-ssa-pre.c (translate_vuse_through_block): Likewise.
* tree-ssa-scopedtables.c (avail_exprs_stack::lookup_avail_expr):
Likewise.
* tree-ssa-sccvn.c (struct vn_walk_cb_data): Add tbaa_p flag.
(adjust_offsets_for_equal_base_address): New function.
(vn_reference_lookup_3): Use it to catch more base equivalences.
Handle and pass down tbaa_p flag.
(vn_reference_lookup_pieces): Adjust.
(vn_reference_lookup): Remove alias-set altering, instead pass
down false as tbaa_p.

* gcc.dg/tree-ssa/pr91091-2.c: New testcase.
* gcc.dg/tree-ssa/ssa-fre-70.c: Likewise.
* gcc.dg/tree-ssa/ssa-fre-71.c: Likewise.
* gcc.dg/tree-ssa/ssa-fre-72.c: Likewise.

Index: gcc/ipa-prop.c
===
--- gcc/ipa-prop.c  (revision 273041)
+++ gcc/ipa-prop.c  (working copy)
@@ -1678,7 +1678,8 @@ determine_known_aggregate_parts (gcall *
 
   if (gimple_code (stmt) == GIMPLE_PHI)
{
- dom_vuse = get_continuation_for_phi (stmt, &r, *aa_walk_budget_p,
+ dom_vuse = get_continuation_for_phi (stmt, &r, true,
+  *aa_walk_budget_p,
   &visited, false, NULL, NULL);
  continue;
}
Index: gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c   (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+struct s { int x; };
+struct t { int x; };
+
+void swap(struct s* p, struct t* q)
+{
+  p->x = q->x;
+  q->x = p->x;
+}
+
+/* The second statement is redundant.  */
+/* { dg-final { scan-tree-dump-times "x = " 1 "fre1" } } */
+/* { dg-final { scan-tree-dump-times " = \[^;\]*x;" 1 "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1" } */
+
+__GIMPLE (ssa, startwith("fre")) char foo(char *p)
+{
+  char _1;
+
+__BB(2):
+  __MEM  (p) = _Literal (char[4]) {};
+  _1 = __MEM  (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1-details" } */
+
+__GIMPLE (ssa, startwith("fre")) char foo(char *p)
+{
+  char _1;
+
+__BB(2):
+  __MEM  (p) = 0;
+  _1 = __MEM  (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1" } */
+
+__GIMPLE (ssa,startwith("fre")) char foo(char *p, int i)
+{
+  char _1;
+
+__BB(2):
+  __MEM  (p) = i_2(D);
+  _1 = __MEM  (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "BIT_FIELD_REF" "fre1" } } */
Index: gcc/tree-ssa-alias.c
===
--- gcc/tree-ssa-alias.c(revision 273041)
+++ gcc/tree-ssa-alias.c(working copy)
@@ -3003,8 +3003,8 @@ stmt_kills_ref_p (gimple *stmt, tree ref
 
 static bool
 maybe_skip_until (gimple *phi, tree &target, basic_block target_bb,
-

[PATCH] Yet another VN improvement

2019-07-05 Thread Richard Biener


I noticed we fail to valueize the RHS when looking for same-valued
stores.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-07-05  Richard Biener  

* tree-ssa-sccvn.c (vn_reference_lookup_3): Valueize RHS
when comparing against a store with possibly the same value.

* gcc.dg/tree-ssa/ssa-fre-77.c: New testcase.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273083)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -2012,9 +2268,11 @@ vn_reference_lookup_3 (ao_ref *ref, tree
  if (res && res != (void *)-1)
{
  vn_reference_t vnresult = (vn_reference_t) res;
+ tree rhs = gimple_assign_rhs1 (def_stmt);
+ if (TREE_CODE (rhs) == SSA_NAME)
+   rhs = SSA_VAL (rhs);
  if (vnresult->result
- && operand_equal_p (vnresult->result,
- gimple_assign_rhs1 (def_stmt), 0))
+ && operand_equal_p (vnresult->result, rhs, 0))
return res;
}
}
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-77.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-77.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-77.c  (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+int foo (int *p, int *q)
+{
+  int x;
+  *p = 1;
+  x = *p;
+  *q = x;
+  return *p;
+}
+
+/* { dg-final { scan-tree-dump "return 1;" "fre1" } } */


Re: [PATCH] Add generic support for "noinit" attribute

2019-07-05 Thread Jozef Lawrynowicz
On Fri, 5 Jul 2019 11:26:20 +0200
Christophe Lyon  wrote:

> On Thu, 4 Jul 2019 at 23:46, Jozef Lawrynowicz  
> wrote:
> >
> > Also, the gcc.target/arm/noinit-attribute.c test works with msp430.
> > Why not create a effective-target keyword which checks for noinit support, 
> > so
> > the test can be gated on that condition and put in a generic part of the
> > testsuite?  
> 
> That was my intention, and I was waiting for feedback on this. In this
> case, I suppose the effective-target check would test a hardcoded list
> of targets, like many other effective-targets?
> (eg check_weak_available)

Were there previous discussions on whether the noinit attribute should be
explicitly enabled for certain targets? e.g. so it will error if you try to use
it on x86_64.
If it will just be enabled by default for all targets then a hardcoded
list for check-effective target sounds ok. Otherwise if it does cause an error
when used on an unsupported target you could do a check that the
attribute compiles successfully instead.

> 
> > After doing some further testing, I noticed the attribute does not work on
> > static variables. The attribute handling code allows the attribute to be 
> > set on
> > a static variable, but then that variable does not get placed in the .noinit
> > section.
> >
> > What are your thoughts on this?
> >
> > Does it even makes sense for a static local variable to be declared as 
> > "noinit"?
> > One should want a static local variable to be initialized, as having it not
> > initialized and hold a random value could cause problems.
> > Perhaps a user could think of some use for this, but I can't.
> >  
> I added:
> static int __attribute__((noinit)) var_noinit2;
> in global scope, and
> static int __attribute__((noinit)) var_noinit3;
> in main(), and the generate code contains:
> .section.noinit,"aw"
> .align  2
> .set.LANCHOR2,. + 0
> .type   var_noinit2, %object
> .size   var_noinit2, 4
> var_noinit2:
> .space  4
> .type   var_noinit3.4160, %object
> .size   var_noinit3.4160, 4
> var_noinit3.4160:
> .space  4
> .type   var_noinit, %object
> .size   var_noinit, 4
> var_noinit:
> .space  4
> 
> So, all three of them are in the .noinit section, but only var_noinit has
> .global var_noinit
> 
> So it seems to work?

Hmm yes there seems to be an issue with trunks handling of noinit for msp430.
Even before your patch the static noinit variables are marked as
common symbols with ".comm" and are not placed in .noinit.

These static noinit variables work with my downstream msp430-gcc (which doesn't
yet have parity with upstream), so this is just something I'll fix separately,
and doesn't represent any issues with your patch.

Jozef


Re: Enable nonoverallping_component_refs even after the base pointers are equivalent

2019-07-05 Thread Richard Biener
On Thu, Jul 4, 2019 at 2:47 PM Jan Hubicka  wrote:
>
> >
> > Why does this only happen in fre3?!
> After fre1 we have
>
> test (int i, int j, int k, int l)
> {
>   struct c * cptr.0_1;
>   struct c * cptr2.1_2;
>   int _11;
>
>:
>   cptr.0_1 = cptr;
>   cptr.0_1->b[i_5(D)].a[j_6(D)].val = 123;
>   cptr2.1_2 = cptr2;
>   cptr2.1_2->b[k_8(D)].a2[l_9(D)].val = 2;
>   _11 = cptr.0_1->b[i_5(D)].a[j_6(D)].val;
>   return _11;
> }
>
> I think it is the same issue with AO querries not being valueized
> you fixed for the other testcase and later reverted.

Ah, yes - there's now a quite easy way to fix those.  Mind open a bugreport
so I remember?

Thanks,
Richard.

> Honza


Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC

2019-07-05 Thread Richard Biener
On Fri, Jul 5, 2019 at 12:21 AM Indu Bhagat  wrote:
>
> On 07/04/2019 03:43 AM, Richard Biener wrote:
>
> On Thu, Jul 4, 2019 at 2:36 AM Indu Bhagat  wrote:
>
> [...]
>
> RE subset of C : It is true that CTF format currently does leave out a very
> small subset of C like FIXED_POINT as you noted ( CTF does have representation
> for COMPLEX_TYPE, if my code paths culminate to gcc_unreachable () for that, I
> should fix them ).  The end goal is to make it support all of C, and not just 
> a
> subset.
>
> What about other languages?  GCC supports C++, Ada, Objective-C, Go, D,
> Fortran, Modula-2, BRIG (this list is not necessarily complete and may change
> in the future).
>
> The format supports C only at this time. Other languages are not on the radar
> yet. However, we have no intrinsic objection to them. Although, languages
> that already have fully-fledged type introspection and interpreted/
> managed languages are probably out of scope, since they already have
> what CTF provides.
>
>
>
> Given it appears to generate only debug info for symbols and no locations
> or whatnot it should be sufficient to introspect the compilation to generate
> the CTF info on the side and then merge it in at link-time.  Which makes
> me wonder if this shouldn't be a plugin for now until it is more complete
> and can be evaluated better (comments in the patches indicate even the
> on-disk format is in flux?).  Adding plugin hook invocations to the three
> places the CTF info generation hooks off should be easy.
>
> Yes, some bits of the on-disk format are being adapted to make it easier to
> adopt the CTF format across the board. E.g., we recently added CU name in the
> CTF header. As another example, we added CTF_K_SLICE type because there 
> existed
> no way in CTF to represent enum bitfields. For the most part though, CTF 
> format
> has stayed as is.
>
> I hope the format is versioned at least.
>
> Yes, the format is versioned. The current version is CTF_VERSION_3.  All these
> format changes I talked about above are a part of CTF_VERSION_3.
>
> libctf handles backward compatibility for users of CTF in the toolchain; all
> transparently to the user. This means that, in future, when CTF version needs
> to be bumped, libctf will either support older version and/or transparently
> upgrade to the new version for further consumers.
>
> It also means that the compiler does not always need to change merely because
> the format has changed: (depending on the change) the linker can transparently
> adjust, as will all consumers if they try to read unlinked object files.
>
>
> That said, the patch series isn't ready for integration since it will
> crash left and right -- did you bootstrap and run the testsuite
> with -gt?
>
>
> Bootstrap and Testsuite : Yes, I have.  On x86_64/linux, sparc64/linux,
>aarch64/linux.
> Run testsuite with -gt : Not yet. Believe me, it's on my plate. And I already
>   regret not having done it sooner :)
> Bootstrap with -gt : Not yet. I should try soon.
>
> (I have compiled libdtrace-ctf with -gt and parsed the .ctf sections with the
> patch set.)
>
> About the patch being not ready for integration : Yes, you're right.
> That's why I chose to retain 'RFC' for this patch series as well. I am working
> on issues, testing the compiler, and closing on the open ends in the
> implementation.
>
> I will refresh the patch series when I have made a meaningful stride ahead. 
> Any
> further suggestions on functional/performance testing will be helpful too.
>
> What's the functional use of CTF?  Print nice backtraces (without showing
> function argument values)?
>
> CTF, at this time, is type information for entities at global or file scope.
> This can be used by online debuggers, program tracers (dynamic tracing); More
> generally, it provides type introspection for C programs, with an optional
> library API to allow them to get at their own types quite more easily than
> DWARF. So, the umbrella usecases are - all C programs that want to introspect
> their own types quickly; and applications that want to introspect other
> programs's types quickly.

What makes it superior to DWARF stripped down to the above feature set?

> (Even with the exception of its embedded string table, it is already small
>  enough to  be kept around in stripped binaries so that it can be relied upon
>  to be present.)

So for distributing a program/library for SUSE we usually split the
distribution into two pieces - the binaries and separated debug information.
With CTF we'd then create both, continue stripping out the DWARF information
but keep the CTF in the binaries?

When a program contains CTF only, can gdb do anything to help debugging
of a running program or a core file?  Do you have gdb support in the works?

> We are also extending the format so it is useful for other on-line debugging
> tools, such as backtracers.

So you become more complex similar to DWARF?

Richard.

>
> In

[patch 1/2][aarch64]: redefine aes patterns

2019-07-05 Thread Sylvia Taylor
Greetings,

This first patch removes aarch64 usage of the aese/aesmc and aesd/aesimc
fusions (i.e. aes fusion) implemented in the scheduler due to unpredictable
behaviour observed in cases such as:
- when register allocation goes bad (e.g. extra movs)
- aes operations with xor and zeroed keys among interleaved operations

A more stable version should be provided by instead doing the aes fusion 
during the combine pass. Since the aese and aesd patterns have been 
rewritten as encapsulating a xor operation, the existing combine fusion 
patterns have also been updated. The purpose is to simplify the need of 
having additional combine patterns for cases like the ones below:

For AESE (though it also applies to AESD as both have a xor operation):

data = data ^ key;
data = vaeseq_u8(data, zero);
---
eor v1.16b, v0.16b, v1.16b
aesev1.16b, v2.16b

Should mean and generate the same as:

data = vaeseq_u8(data, key);
---
aesev1.16b, v0.16b

Bootstrapped and tested on aarch64-none-linux-gnu.

Cheers,
Syl

gcc/ChangeLog:

2019-07-05  Sylvia Taylor  

* config/aarch64/aarch64-simd.md
(aarch64_crypto_aesv16qi): Redefine pattern with xor.
(aarch64_crypto_aesv16qi): Remove attribute enabled.
(*aarch64_crypto_aesv16qi_xor_combine): Remove both.
(*aarch64_crypto_aese_fused,
*aarch64_crypto_aesd_fused): Update to new definition.
* config/aarch64/aarch64.c
(aarch_macro_fusion_pair_p): Remove aese/aesmc fusion check.

gcc/testsuite/ChangeLog:

2019-07-05  Sylvia Taylor  

* gcc.target/aarch64/crypto-fuse-1.c: Remove.
* gcc.target/aarch64/crypto-fuse-2.c: Remove.
* gcc.target/aarch64/aes-fuse-1.c: New testcase.
* gcc.target/aarch64/aes-fuse-2.c: New testcase.
diff --git a/gcc/config/aarch64/aarch64-simd.md 
b/gcc/config/aarch64/aarch64-simd.md
index 
83f5c1fc2c27b265d528e9d5a02c05cc7fe5001f..1bcc50081f50a89f4951b15d7c465e03d8d9fb81
 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -6053,56 +6053,23 @@
 
 (define_insn "aarch64_crypto_aesv16qi"
   [(set (match_operand:V16QI 0 "register_operand" "=w")
-   (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "%0")
-  (match_operand:V16QI 2 "register_operand" "w")]
+   (unspec:V16QI
+   [(xor:V16QI
+(match_operand:V16QI 1 "register_operand" "%0")
+(match_operand:V16QI 2 "register_operand" "w"))]
  CRYPTO_AES))]
   "TARGET_SIMD && TARGET_AES"
   "aes\\t%0.16b, %2.16b"
   [(set_attr "type" "crypto_aese")]
 )
 
-(define_insn "*aarch64_crypto_aesv16qi_xor_combine"
-  [(set (match_operand:V16QI 0 "register_operand" "=w")
-   (unspec:V16QI [(xor:V16QI
-   (match_operand:V16QI 1 "register_operand" "%0")
-   (match_operand:V16QI 2 "register_operand" "w"))
-  (match_operand:V16QI 3 "aarch64_simd_imm_zero" "")]
-  CRYPTO_AES))]
-  "TARGET_SIMD && TARGET_AES"
-  "aes\\t%0.16b, %2.16b"
-  [(set_attr "type" "crypto_aese")]
-)
-
-(define_insn "*aarch64_crypto_aesv16qi_xor_combine"
-  [(set (match_operand:V16QI 0 "register_operand" "=w")
-   (unspec:V16QI [(match_operand:V16QI 3 "aarch64_simd_imm_zero" "")
-   (xor:V16QI (match_operand:V16QI 1 "register_operand" "%0")
-  (match_operand:V16QI 2 "register_operand" "w"))]
-   CRYPTO_AES))]
-  "TARGET_SIMD && TARGET_AES"
-  "aes\\t%0.16b, %2.16b"
-  [(set_attr "type" "crypto_aese")]
-)
-
-;; When AES/AESMC fusion is enabled we want the register allocation to
-;; look like:
-;;AESE Vn, _
-;;AESMC Vn, Vn
-;; So prefer to tie operand 1 to operand 0 when fusing.
-
 (define_insn "aarch64_crypto_aesv16qi"
-  [(set (match_operand:V16QI 0 "register_operand" "=w,w")
-   (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0,w")]
+  [(set (match_operand:V16QI 0 "register_operand" "=w")
+   (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "w")]
 CRYPTO_AESMC))]
   "TARGET_SIMD && TARGET_AES"
   "aes\\t%0.16b, %1.16b"
-  [(set_attr "type" "crypto_aesmc")
-   (set_attr_alternative "enabled"
- [(if_then_else (match_test
-  "aarch64_fusion_enabled_p (AARCH64_FUSE_AES_AESMC)")
-(const_string "yes" )
-(const_string "no"))
-  (const_string "yes")])]
+  [(set_attr "type" "crypto_aesmc")]
 )
 
 ;; When AESE/AESMC fusion is enabled we really want to keep the two together
@@ -6111,12 +6078,14 @@
 ;;  Mash the two together during combine.
 
 (define_insn "*aarch64_crypto_aese_fused"
-  [(set (match_operand:V16QI 0 "register_operand" "=&w")
+  [(set (match_operand:V16QI 0 "register_operand" "=w")
(unspec:V16QI
  [(unspec:V16QI
-   [(match_operand:V16QI 1 "register_operand" "0")
-(match_operand:V16QI 2 "register_operand" "w")] UNSPEC_AESE)
- ] UN

[patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Andrew Stubbs
This patch allows Fortran "parameter" named constants to be named in 
OpenMP map and OpenACC copy directives.


Right now, the compiler just says something like:

 !$omp target data map(tofrom:N,x)
 1
  Error: Object 'n' is not a variable at (1)

This is technically correct, but is surprising to the user and AFAICT 
the OpenMP/OpenACC documents don't say you can't name parameters, so I'd 
like it to be allowed.


Also, I suspect the support request that led me here originated from 
code originally written for another toolchain, so this is a portability 
issue.


With this patch the compiler allows the parameter in the directive, but 
just ignores it (deletes it), so that it has no effect. This is safe 
since the named constant will have been substituted for the actual value 
everywhere it occurs. If that is not true in some case (can it happen?), 
I believe it will be automatically mapped in the same way other 
variables are.


Using parameters in device_resident, deviceptr, private, reduction, 
etc., remains an error.


OK to commit?

Andrew
Allow Fortran parameters in map/copy directives.

2019-07-05  Andrew Stubbs  

	gcc/fortran/
	* gfortran.h (gfc_free_omp_namelist_entry): New prototype.
	* match.c (gfc_free_omp_namelist): Break contents out into ...
	(gfc_free_omp_namelist_entry): ... here.
	* openmp.c (resolve_omp_clauses): Delete parameters in map to and from.

	gcc/testsuite/
	* gfortran.dg/goacc/parameter.f95: Allow parameters in copy.
	* gfortran.dg/gomp/map-1.f90: Allow parameters in map, only.

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index b1f7bd0604a..5363cbd331b 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3181,6 +3181,7 @@ void gfc_free_iterator (gfc_iterator *, int);
 void gfc_free_forall_iterator (gfc_forall_iterator *);
 void gfc_free_alloc_list (gfc_alloc *);
 void gfc_free_namelist (gfc_namelist *);
+void gfc_free_omp_namelist_entry (gfc_omp_namelist *);
 void gfc_free_omp_namelist (gfc_omp_namelist *);
 void gfc_free_equiv (gfc_equiv *);
 void gfc_free_equiv_until (gfc_equiv *, gfc_equiv *);
diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index 0f3b2132122..51c9e9ef5ed 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -5373,6 +5373,23 @@ gfc_free_namelist (gfc_namelist *name)
 }
 
 
+/* Free an OpenMP namelist entry.  */
+
+void
+gfc_free_omp_namelist_entry (gfc_omp_namelist *name)
+{
+  gfc_free_expr (name->expr);
+  if (name->udr)
+{
+  if (name->udr->combiner)
+	gfc_free_statement (name->udr->combiner);
+  if (name->udr->initializer)
+	gfc_free_statement (name->udr->initializer);
+  free (name->udr);
+}
+  free (name);
+}
+
 /* Free an OpenMP namelist structure.  */
 
 void
@@ -5382,17 +5399,8 @@ gfc_free_omp_namelist (gfc_omp_namelist *name)
 
   for (; name; name = n)
 {
-  gfc_free_expr (name->expr);
-  if (name->udr)
-	{
-	  if (name->udr->combiner)
-	gfc_free_statement (name->udr->combiner);
-	  if (name->udr->initializer)
-	gfc_free_statement (name->udr->initializer);
-	  free (name->udr);
-	}
   n = name->next;
-  free (name);
+  gfc_free_omp_namelist_entry (name);
 }
 }
 
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 1c7bce6c300..d06221b41ed 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -4165,6 +4165,22 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 		   "clause at %L", &code->loc);
 }
 
+  /* Remove no-op mappings, such as parameters.  */
+  gfc_omp_namelist **parent = &omp_clauses->lists[OMP_LIST_MAP];
+  for (n = omp_clauses->lists[OMP_LIST_MAP]; n; n = n->next)
+{
+  if ((n->u.map_op == OMP_MAP_TO
+	   || n->u.map_op == OMP_MAP_FROM
+	   || n->u.map_op == OMP_MAP_TOFROM)
+	  && n->sym->attr.flavor == FL_PARAMETER)
+	{
+	  *parent = n->next;
+	  gfc_free_omp_namelist_entry (n);
+	}
+  else
+	parent = &n->next;
+}
+
   /* Check that no symbol appears on multiple clauses, except that
  a symbol can appear on both firstprivate and lastprivate.  */
   for (list = 0; list < OMP_LIST_NUM; list++)
diff --git a/gcc/testsuite/gfortran.dg/goacc/parameter.f95 b/gcc/testsuite/gfortran.dg/goacc/parameter.f95
index 84274611915..edfaeeadc76 100644
--- a/gcc/testsuite/gfortran.dg/goacc/parameter.f95
+++ b/gcc/testsuite/gfortran.dg/goacc/parameter.f95
@@ -7,7 +7,7 @@ contains
 integer :: i
 integer, parameter :: a = 1
 !$acc declare device_resident (a) ! { dg-error "PARAMETER" }
-!$acc data copy (a) ! { dg-error "not a variable" }
+!$acc data copy (a) ! This is permitted.
 !$acc end data
 !$acc data deviceptr (a) ! { dg-error "not a variable" }
 !$acc end data
diff --git a/gcc/testsuite/gfortran.dg/gomp/map-1.f90 b/gcc/testsuite/gfortran.dg/gomp/map-1.f90
index e78b56c8f39..abe448a33f9 100644
--- a/gcc/testsuite/gfortran.dg/gomp/map-1.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/map-1.f90
@@ -18,7 +18,7 @@ subroutine test(aas

[patch 2/2][arm]: redefine aes patterns

2019-07-05 Thread Sylvia Taylor
Greetings,

This patch removes the arch-common aese/aesmc and aesd/aesimc fusions
(i.e. aes fusion) implemented in the scheduling phase through the
aarch_crypto_can_dual function. The reason is due to observing
undesired behaviour in cases such as:
- when register allocation goes bad (e.g. extra movs)
- aes operations with xor and zeroed keys among interleaved operations

A more stable version should be provided by instead doing the aes fusion
during the combine pass. As such, new combine patterns have been added to
enable this.

The second change is the aese and aesd patterns have been rewritten as
encapsulating a xor operation. The purpose is to simplify the need of
having additional combine patterns for cases like the ones below:

For AESE (though it also applies to AESD as both have a xor operation):

data = data ^ key;
data = vaeseq_u8(data, zero);
---
veorq1, q0, q1
aese.8  q1, q9

Should mean and generate the same as:

data = vaeseq_u8(data, key);
---
aese.8  q1, q0

Bootstrapped and tested on arm-none-linux-gnueabihf.

Cheers,
Syl

gcc/ChangeLog:

2019-07-05  Sylvia Taylor  

* config/arm/crypto.md:
(crypto_): Redefine aese/aesd pattern with xor.
(crypto_): Remove attribute enabled for aesmc.
(crypto_): Split CRYPTO_BINARY into 2 patterns.
(*aarch32_crypto_aese_fused, *aarch32_crypto_aesd_fused): New.
* config/arm/arm.c
(aarch_macro_fusion_pair_p): Remove aes/aesmc fusion check.
* config/arm/aarch-common-protos.h
(aarch_crypto_can_dual_issue): Remove.
* config/arm/aarch-common.c 
(aarch_crypto_can_dual_issue): Likewise.
* config/arm/exynos-m1.md: Remove aese/aesmc fusion.
* config/arm/cortex-a53.md: Likewise.
* config/arm/cortex-a57.md: Likewise.
* config/arm/iterators.md:
(CRYPTO_BINARY): Redefine.
(CRYPTO_UNARY): Removed.
(CRYPTO_AES, CRYPTO_AESMC): New.

gcc/testsuite/ChangeLog:

2019-07-05  Sylvia Taylor  

* gcc.target/arm/aes-fuse-1.c: New.
* gcc.target/arm/aes-fuse-2.c: New.
* gcc.target/arm/aes_xor_combine.c: New.
diff --git a/gcc/config/arm/aarch-common-protos.h 
b/gcc/config/arm/aarch-common-protos.h
index 
11cd5145bbc77ab35e7874a75a93ec0e7bb0ea28..3bf38a104f6941eec1ce88db7d6b6ceb7da0af92
 100644
--- a/gcc/config/arm/aarch-common-protos.h
+++ b/gcc/config/arm/aarch-common-protos.h
@@ -24,7 +24,6 @@
 #define GCC_AARCH_COMMON_PROTOS_H
 
 extern int aarch_accumulator_forwarding (rtx_insn *, rtx_insn *);
-extern int aarch_crypto_can_dual_issue (rtx_insn *, rtx_insn *);
 extern bool aarch_rev16_p (rtx);
 extern bool aarch_rev16_shleft_mask_imm_p (rtx, machine_mode);
 extern bool aarch_rev16_shright_mask_imm_p (rtx, machine_mode);
diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index 
c7af12d4cd1714c70ebc6d6c7d4454606d15f864..965a07a43e3129dd1743d4a79813a597feca0b71
 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -31,46 +31,6 @@
 #include "rtl-iter.h"
 #include "memmodel.h"
 
-/* In ARMv8-A there's a general expectation that AESE/AESMC
-   and AESD/AESIMC sequences of the form:
-
-   AESE Vn, _
-   AESMC Vn, Vn
-
-   will issue both instructions in a single cycle on super-scalar
-   implementations.  This function identifies such pairs.  */
-
-int
-aarch_crypto_can_dual_issue (rtx_insn *producer_insn, rtx_insn *consumer_insn)
-{
-  rtx producer_set, consumer_set;
-  rtx producer_src, consumer_src;
-
-  producer_set = single_set (producer_insn);
-  consumer_set = single_set (consumer_insn);
-
-  producer_src = producer_set ? SET_SRC (producer_set) : NULL;
-  consumer_src = consumer_set ? SET_SRC (consumer_set) : NULL;
-
-  if (producer_src && consumer_src
-  && GET_CODE (producer_src) == UNSPEC && GET_CODE (consumer_src) == UNSPEC
-  && ((XINT (producer_src, 1) == UNSPEC_AESE
-   && XINT (consumer_src, 1) == UNSPEC_AESMC)
-  || (XINT (producer_src, 1) == UNSPEC_AESD
-  && XINT (consumer_src, 1) == UNSPEC_AESIMC)))
-  {
-unsigned int regno = REGNO (SET_DEST (producer_set));
-
-/* Before reload the registers are virtual, so the destination of
-   consumer_set doesn't need to match.  */
-
-return (REGNO (SET_DEST (consumer_set)) == regno || !reload_completed)
-   && REGNO (XVECEXP (consumer_src, 0, 0)) == regno;
-  }
-
-  return 0;
-}
-
 /* Return TRUE if X is either an arithmetic shift left, or
is a multiplication by a power of two.  */
 bool
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
e9aba65c70563f23ba3049702072a59cf555b9ce..5c5129a8e52adb07bb431eb51c6f6239b9b0c941
 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -30565,10 +30565,6 @@ aarch_macro_fusion_pair_p (rtx_insn* prev, rtx_insn* 
curr)
   if (!arm_macro_fusion_p ())
 return false;
 
-  if (current_tune->fusible_ops & tune_params::FUSE_AES_AESMC
-  && aarch_cr

Re: [PATCH] Add generic support for "noinit" attribute

2019-07-05 Thread Christophe Lyon
On Fri, 5 Jul 2019 at 12:57, Jozef Lawrynowicz  wrote:
>
> On Fri, 5 Jul 2019 11:26:20 +0200
> Christophe Lyon  wrote:
>
> > On Thu, 4 Jul 2019 at 23:46, Jozef Lawrynowicz  
> > wrote:
> > >
> > > Also, the gcc.target/arm/noinit-attribute.c test works with msp430.
> > > Why not create a effective-target keyword which checks for noinit 
> > > support, so
> > > the test can be gated on that condition and put in a generic part of the
> > > testsuite?
> >
> > That was my intention, and I was waiting for feedback on this. In this
> > case, I suppose the effective-target check would test a hardcoded list
> > of targets, like many other effective-targets?
> > (eg check_weak_available)
>
> Were there previous discussions on whether the noinit attribute should be
> explicitly enabled for certain targets? e.g. so it will error if you try to 
> use
> it on x86_64.

Not formally. I submitted a patch for arm only
https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00771.html
and got requests to make it generic instead, starting with:
https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00065.html

> If it will just be enabled by default for all targets then a hardcoded
> list for check-effective target sounds ok. Otherwise if it does cause an error
> when used on an unsupported target you could do a check that the
> attribute compiles successfully instead.

Currently, I think it will be accepted for all targets, with various
effects depending on the OS etc...

> >
> > > After doing some further testing, I noticed the attribute does not work on
> > > static variables. The attribute handling code allows the attribute to be 
> > > set on
> > > a static variable, but then that variable does not get placed in the 
> > > .noinit
> > > section.
> > >
> > > What are your thoughts on this?
> > >
> > > Does it even makes sense for a static local variable to be declared as 
> > > "noinit"?
> > > One should want a static local variable to be initialized, as having it 
> > > not
> > > initialized and hold a random value could cause problems.
> > > Perhaps a user could think of some use for this, but I can't.
> > >
> > I added:
> > static int __attribute__((noinit)) var_noinit2;
> > in global scope, and
> > static int __attribute__((noinit)) var_noinit3;
> > in main(), and the generate code contains:
> > .section.noinit,"aw"
> > .align  2
> > .set.LANCHOR2,. + 0
> > .type   var_noinit2, %object
> > .size   var_noinit2, 4
> > var_noinit2:
> > .space  4
> > .type   var_noinit3.4160, %object
> > .size   var_noinit3.4160, 4
> > var_noinit3.4160:
> > .space  4
> > .type   var_noinit, %object
> > .size   var_noinit, 4
> > var_noinit:
> > .space  4
> >
> > So, all three of them are in the .noinit section, but only var_noinit has
> > .global var_noinit
> >
> > So it seems to work?
>
> Hmm yes there seems to be an issue with trunks handling of noinit for msp430.
> Even before your patch the static noinit variables are marked as
> common symbols with ".comm" and are not placed in .noinit.
>
> These static noinit variables work with my downstream msp430-gcc (which 
> doesn't
> yet have parity with upstream), so this is just something I'll fix separately,
> and doesn't represent any issues with your patch.
>
> Jozef


Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Jakub Jelinek
On Fri, Jul 05, 2019 at 12:31:17PM +0100, Andrew Stubbs wrote:
> This patch allows Fortran "parameter" named constants to be named in OpenMP
> map and OpenACC copy directives.
> 
> Right now, the compiler just says something like:
> 
>  !$omp target data map(tofrom:N,x)
>  1
>   Error: Object 'n' is not a variable at (1)
> 
> This is technically correct, but is surprising to the user and AFAICT the
> OpenMP/OpenACC documents don't say you can't name parameters, so I'd like it
> to be allowed.

I don't think you can do it in OpenMP, I can discuss in the language
committee, but the definition of variable is:

"A named data storage block, for which the value can be defined and redefined 
during
the execution of a program."

so I don't believe Fortran parameters are OpenMP variables.
Scalar Fortran parameters don't even have any data storage block associated
with them at all.

Jakub


Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Andrew Stubbs

On 05/07/2019 12:49, Jakub Jelinek wrote:

On Fri, Jul 05, 2019 at 12:31:17PM +0100, Andrew Stubbs wrote:

This patch allows Fortran "parameter" named constants to be named in OpenMP
map and OpenACC copy directives.

Right now, the compiler just says something like:

  !$omp target data map(tofrom:N,x)
  1
   Error: Object 'n' is not a variable at (1)

This is technically correct, but is surprising to the user and AFAICT the
OpenMP/OpenACC documents don't say you can't name parameters, so I'd like it
to be allowed.


I don't think you can do it in OpenMP, I can discuss in the language
committee, but the definition of variable is:

"A named data storage block, for which the value can be defined and redefined 
during
the execution of a program."

so I don't believe Fortran parameters are OpenMP variables.
Scalar Fortran parameters don't even have any data storage block associated
with them at all.


Agreed, they are not variables (nor variable), and copying them about is 
entirely pointless (although, if you simply remove the error check GCC 
will happily create storage for 'N' and do the copies).


And yet, this issue has generated a support request from a blocked 
customer, and I'd like to improve their experience.


I could generate a warning, or add a note to the error message, but in 
the interests of portability I thought just doing the right thing would 
be nicer.


Any suggestions?

Andrew


Re: [patch] Small improvements to coverage info (1/n)

2019-07-05 Thread Jakub Jelinek
On Wed, Jul 03, 2019 at 12:46:37PM +0200, Eric Botcazou wrote:
> 2019-07-03  Eric Botcazou  
> 
>   * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on
>   phi nodes if possible.
>   * tree-scalar-evolution.c (final_value_replacement_loop): Propagate
>   location info on the newly created statement.
>   * tree-ssa-loop-manip.c (create_iv): Propagate location info on the
>   newly created increment if needed.

> --- tree-ssa-loop-manip.c (revision 272930)
> +++ tree-ssa-loop-manip.c (working copy)
> @@ -126,10 +126,22 @@ create_iv (tree base, tree step, tree va
>  gsi_insert_seq_on_edge_immediate (pe, stmts);
>  
>stmt = gimple_build_assign (va, incr_op, vb, step);
> +  /* Prevent the increment from inheriting a bogus location if it is not put
> + immediately after a statement whose location is known.  */
>if (after)
> -gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT);
> +{
> +  if (gsi_end_p (*incr_pos))
> + {
> +   edge e = single_succ_edge (gsi_bb (*incr_pos));
> +   gimple_set_location (stmt, e->goto_locus);
> + }
> +  gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT);
> +}
>else
> -gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT);
> +{
> +  gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos)));
> +  gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT);
> +}

This change broke gomp/pr88107.c test:
FAIL: gcc.dg/gomp/pr88107.c (internal compiler error)
FAIL: gcc.dg/gomp/pr88107.c (test for excess errors)
Excess errors:
during GIMPLE pass: graphite
/usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler error: 
Segmentation fault
0x11942a4 crash_signal
../../gcc/toplev.c:326
0x13b5861 gimple_location
../../gcc/gimple.h:1805
0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*, 
gimple_stmt_iterator*, bool, tree_node**, tree_node**)
../../gcc/tree-ssa-loop-manip.c:142
0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*, 
tree_node*, tree_node*, tree_node**, tree_node**, loop*)
../../gcc/cfgloopmanip.c:831

Apparently gsi_end_p (*incr_pos) is true and after is false, so
gsi_stmt (*incr_pos) is NULL.  One needs graphite enabled.

Jakub


Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Jakub Jelinek
On Fri, Jul 05, 2019 at 01:10:00PM +0100, Andrew Stubbs wrote:
> > so I don't believe Fortran parameters are OpenMP variables.
> > Scalar Fortran parameters don't even have any data storage block associated
> > with them at all.
> 
> Agreed, they are not variables (nor variable), and copying them about is
> entirely pointless (although, if you simply remove the error check GCC will
> happily create storage for 'N' and do the copies).
> 
> And yet, this issue has generated a support request from a blocked customer,
> and I'd like to improve their experience.
> 
> I could generate a warning, or add a note to the error message, but in the
> interests of portability I thought just doing the right thing would be
> nicer.
> 
> Any suggestions?

I don't know what OpenACC says, perhaps it is different.

For OpenMP, a warning is not good enough, because if Fortran PARAMETER is
not a variable, then the program is invalid and needs to be rejected.
You could improve the diagnostics by adding some explanation message that 
fortran
PARAMETER is not a variable.

Jakub


[wwwdocs] Use whitespace between sourceware.org and gcc.gnu.org

2019-07-05 Thread Shakthi Kannan
Use "sourceware.org / gcc.gnu.org" for more clarity (bug #90334).

Shakthi Kannan

Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.43
diff -u -p -r1.43 svnwrite.html
--- htdocs/svnwrite.html12 Jan 2019 16:27:27 -  1.43
+++ htdocs/svnwrite.html5 Jul 2019 10:27:54 -
@@ -34,7 +34,7 @@ maintainers and significant developers.<
 be sponsored by an existing maintainer (someone with "write after approval"
 is not sufficient).

-If you already have an account on sourceware.org/gcc.gnu.org, ask
+If you already have an account on sourceware.org / gcc.gnu.org, ask
 overse...@gcc.gnu.org to add access to the GCC repository.
 Include the name of your sponsor and CC: her.
 If you do not have an account yet, use 

Re: [wwwdocs] Use whitespace between sourceware.org and gcc.gnu.org

2019-07-05 Thread Jonathan Wakely

On 05/07/19 12:28 +, Shakthi Kannan wrote:

Use "sourceware.org / gcc.gnu.org" for more clarity (bug #90334).


Committed to CVS - thanks!


Shakthi Kannan

Index: htdocs/svnwrite.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/svnwrite.html,v
retrieving revision 1.43
diff -u -p -r1.43 svnwrite.html
--- htdocs/svnwrite.html12 Jan 2019 16:27:27 -  1.43
+++ htdocs/svnwrite.html5 Jul 2019 10:27:54 -
@@ -34,7 +34,7 @@ maintainers and significant developers.<
be sponsored by an existing maintainer (someone with "write after approval"
is not sufficient).

-If you already have an account on sourceware.org/gcc.gnu.org, ask
+If you already have an account on sourceware.org / gcc.gnu.org, ask
overse...@gcc.gnu.org to add access to the GCC repository.
Include the name of your sponsor and CC: her.
If you do not have an account yet, use 

Re: [PATCH] Fix no-strict-aliasing condition in VN

2019-07-05 Thread Richard Biener
On Fri, 5 Jul 2019, Richard Biener wrote:

> 
> There's a path I added where get_alias_set () != 0 was meant to
> cover -fno-strict-aliasing but it doesn't since quite some time
> (oops).  Fixed as follows - preprartory for PR91091 where we'd
> otherwise break the testcase below.
> 
> Bootstrap / regtest running on x86_64-unknown-linux-gnu.
> 
> I'll also backport this to the GCC 8/9 branches where the issue
> is latent.

Applied with adjusting gcc.dg/tree-ssa/ssa-fre-61.c to use
-fstrict-aliasing.  The testcase could be handled without
if alignment were considered, I'll leave that or a followup.

Richard.

> Richard.
> 
> 2019-07-05  Richard Biener  
> 
>   PR tree-optimization/91091
>   * tree-ssa-sccvn.c (vn_reference_lookup_3): Overlap of
>   accesses can happen with -fno-strict-aliasing.
> 
>   * gcc.dg/tree-ssa/pr91091-1.c: New testcase.
> 
> Index: gcc/tree-ssa-sccvn.c
> ===
> --- gcc/tree-ssa-sccvn.c  (revision 273133)
> +++ gcc/tree-ssa-sccvn.c  (working copy)
> @@ -1996,7 +1996,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
>activation of a union member via a store makes the
>values of untouched bytes unspecified.  */
> && (known_eq (ref->size, BITS_PER_UNIT)
> -   || (get_alias_set (lhs) != 0
> +   || (flag_strict_aliasing
> +   && get_alias_set (lhs) != 0
> && ao_ref_alias_set (ref) != 0)))
>   {
> tree *saved_last_vuse_ptr = data->last_vuse_ptr;
> Index: gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c (nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr91091-1.c (working copy)
> @@ -0,0 +1,23 @@
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-strict-aliasing" } */
> +
> +struct s { int x; } __attribute__((packed));
> +struct t { int x; };
> +
> +void __attribute__((noinline,noipa))
> +swap(struct s* p, struct t* q)
> +{
> +  p->x = q->x;
> +  q->x = p->x;
> +}
> +
> +int main()
> +{
> +  struct t a[2];
> +  a[0].x = 0x12345678;
> +  a[1].x = 0x98765432;
> +  swap ((struct s *)((char *)a + 1), a);
> +  if (a[0].x != 0x12345678)
> +__builtin_abort ();
> +  return 0;
> +}
> 

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

[PATCH] Fix PR83518, track partial definitions for VN

2019-07-05 Thread Richard Biener


This implements tracking of partial definitions in value-numbering
so that we can handle

typedef int v2si __attribute__((vector_size(__SIZEOF_INT__ * 2)));
v2si foo (int *a)
{
  a[0] = 1;
  a[1] = 2;
  return *(v2si *)a;
}

and value-number the load to { 1, 2 }.

This happens for example for the testcase in the PR where there
is a vectorized tail that can be constant folded after unrolling
of a non-vectorized head.

We track partial definitions in two pieces - first a vector
of partial defs is collected that can be native_encoded in
order to support overlaps of the partial defs.  Second, a
splay-tree is used to track the range(s) those partial defs
cover so we can stop once we have gathered enough of them.

The patch only implements byte-aligned/sized constant partial
defs for now but it should be possible to extend it to non-constant
partial defs to for example value-number a load to a
vector constuctor - not too useful in itself but for followup
simplifications inside VN.  It should be also possible to
extend this to cover bitfields with a bit of twiddling for
the native_encode/interpret glue since that works on byte-granularity
only.

I do plan to look at the non-constant (vector/complex) defs, bot
at the bitfield ones.

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

Re-testing with graphite disabled (too noisy at the moment) and after
committing all the prerequesites.

Comments?

Thanks,
Richard.

2019-07-05  Richard Biener  

PR tree-optimization/83518
* tree-ssa-sccvn.c: Include splay-tree.h.
(struct pd_range, struct pd_data): New.
(struct vn_walk_cb_data): Add data to track partial definitions.
(vn_walk_cb_data::~vn_walk_cb_data): New.
(vn_walk_cb_data::push_partial_def): New.
(pd_tree_alloc, pd_tree_dealloc, pd_range_compare): New.
(vn_reference_lookup_2): When partial defs are registered give up.
(vn_reference_lookup_3): Track partial defs for memset and
constructor zeroing and for defs from constants.

* gcc.dg/tree-ssa/ssa-fre-73.c: New testcase.
* gcc.dg/tree-ssa/ssa-fre-74.c: Likewise.
* gcc.dg/tree-ssa/ssa-fre-75.c: Likewise.
* gcc.dg/tree-ssa/ssa-fre-76.c: Likewise.
* g++.dg/tree-ssa/pr83518.C: Likewise.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273136)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "splay-tree.h"
 #include "backend.h"
 #include "rtl.h"
 #include "tree.h"
@@ -360,6 +361,8 @@ static void init_vn_nary_op_from_stmt (v
 static void init_vn_nary_op_from_pieces (vn_nary_op_t, unsigned int,
 enum tree_code, tree, tree *);
 static tree vn_lookup_simplify_result (gimple_match_op *);
+static vn_reference_t vn_reference_lookup_or_insert_for_pieces
+ (tree, alias_set_type, tree, vec, tree);
 
 /* Return whether there is value numbering information for a given SSA name.  
*/
 
@@ -1646,20 +1649,245 @@ vn_reference_lookup_1 (vn_reference_t vr
   return NULL_TREE;
 }
 
+
+/* Partial definition tracking support.  */
+
+struct pd_range
+{
+  HOST_WIDE_INT offset;
+  HOST_WIDE_INT size;
+};
+
+struct pd_data
+{
+  tree rhs;
+  HOST_WIDE_INT offset;
+  HOST_WIDE_INT size;
+};
+
+/* Context for alias walking.  */
+
 struct vn_walk_cb_data
 {
   vn_walk_cb_data (vn_reference_t vr_, tree *last_vuse_ptr_,
-   vn_lookup_kind vn_walk_kind_, bool tbaa_p_)
+  vn_lookup_kind vn_walk_kind_, bool tbaa_p_)
 : vr (vr_), last_vuse_ptr (last_vuse_ptr_), vn_walk_kind (vn_walk_kind_),
-  tbaa_p (tbaa_p_)
-{}
+  tbaa_p (tbaa_p_), known_ranges (NULL)
+   {}
+  ~vn_walk_cb_data ();
+  void *push_partial_def (const pd_data& pd, tree, HOST_WIDE_INT);
 
   vn_reference_t vr;
   tree *last_vuse_ptr;
   vn_lookup_kind vn_walk_kind;
   bool tbaa_p;
+
+  /* The VDEFs of partial defs we come along.  */
+  auto_vec partial_defs;
+  /* The first defs range to avoid splay tree setup in most cases.  */
+  pd_range first_range;
+  tree first_vuse;
+  splay_tree known_ranges;
+  obstack ranges_obstack;
 };
 
+vn_walk_cb_data::~vn_walk_cb_data ()
+{
+  if (known_ranges)
+{
+  splay_tree_delete (known_ranges);
+  obstack_free (&ranges_obstack, NULL);
+}
+}
+
+/* pd_range splay-tree helpers.  */
+
+static int
+pd_range_compare (splay_tree_key offset1p, splay_tree_key offset2p)
+{
+  HOST_WIDE_INT offset1 = *(HOST_WIDE_INT *)offset1p;
+  HOST_WIDE_INT offset2 = *(HOST_WIDE_INT *)offset2p;
+  if (offset1 < offset2)
+return -1;
+  else if (offset1 > offset2)
+return 1;
+  return 0;
+}
+
+static void *
+pd_tree_alloc (int size, void *data_)
+{
+  vn_walk_cb_data *data = (vn_walk_cb_data *)data_;
+  return obstack_alloc (&data->ranges_obstack, size);
+}
+
+static 

Re: [PATCH][GCC][AARCH64] PR target/90712 Fix gcc.dg/rtl/aarch64/subs_adds_sp.c regression

2019-07-05 Thread Sam Tebbs

On 05/07/2019 11:33, Richard Earnshaw wrote:
>
> On 02/07/2019 12:00, Sam Tebbs wrote:
>> Hi all,
>>
>> This patch fixes the regression to gcc.dg/rtl/aarch64/subs_adds_sp.c that
>> r271735 caused. This was done by ensuring that the current function's
>> frame has
>> been laid out before checking if return address signing is enabled.
>>
>> Tested and built on aarch64-none-elf and aarch64-none-linux-gnu.
>>
>> OK for trunk?
>>
>> Sam Tebbs
>>
>> gcc/
>> 2019-06-20  Sam Tebbs
>>
>>  PR target/90712
>>  * aarch64/aarch64.c (aarch64_post_cfi_startproc): Replace thunk check
>>  with a frame laid out check.
>>
>
> OK.
>
> R.

Thank you Richard, committed as r273138.

Sam




Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Jakub Jelinek
On Fri, Jul 05, 2019 at 02:15:12PM +0200, Jakub Jelinek wrote:
> On Fri, Jul 05, 2019 at 01:10:00PM +0100, Andrew Stubbs wrote:
> > > so I don't believe Fortran parameters are OpenMP variables.
> > > Scalar Fortran parameters don't even have any data storage block 
> > > associated
> > > with them at all.
> > 
> > Agreed, they are not variables (nor variable), and copying them about is
> > entirely pointless (although, if you simply remove the error check GCC will
> > happily create storage for 'N' and do the copies).
> > 
> > And yet, this issue has generated a support request from a blocked customer,
> > and I'd like to improve their experience.
> > 
> > I could generate a warning, or add a note to the error message, but in the
> > interests of portability I thought just doing the right thing would be
> > nicer.
> > 
> > Any suggestions?
> 
> I don't know what OpenACC says, perhaps it is different.
> 
> For OpenMP, a warning is not good enough, because if Fortran PARAMETER is
> not a variable, then the program is invalid and needs to be rejected.
> You could improve the diagnostics by adding some explanation message that 
> fortran
> PARAMETER is not a variable.

Got it confirmed, it might change in a future OpenMP release; and there has
been a change already in OpenMP 3.1 that might have intended to allow
Fortran PARAMETERs in firstprivate clause, but because the glossary hasn't
been changed, it is not valid even in firstprivate clause.

Note, if there is adjusted wording for Fortran PARAMETERs, it shouldn't be
done just for map clause, but all other OpenMP data sharing clauses.  Even
if PARAMETERs are eventually allowed as variables, it will be still limited
to firstprivate clause and perhaps some sorts of map clauses, nothing else,
e.g. for non-firstprivate privatization clauses you really need a definable
object or allocatable.

Jakub


Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")

2019-07-05 Thread Jason Merrill

On 7/4/19 8:56 AM, Paolo Carlini wrote:

Hi,

On 27/06/19 23:19, Jason Merrill wrote:
Ah, thanks.  Then perhaps we want to change the CLASSTYPE_FINAL in 
build_over_call to resolves_to_fixed_type_p (arg), to also handle the 
other reasons we might know the dynamic type of the argument, and 
remove the related code from build_new_method_call_1?


You could avoid needing to move the conditional lower by comparing 
DECL_CONTEXT (fn) and BINFO_TYPE (cand->conversion_path) rather than 
parmtype and TREE_TYPE (converted_arg).


Sorry for late replying, a few days off.

Anyway, great, it looks like we are reaching a nice synthesis. I must 
admit that until yesterday I hadn't noticed that Fabien dealt precisely 
with using declarations in order to fix c++/11750, thus the existing 
check in build_new_method_call_1 is exactly what we need. The below does 
that and passes testing, in it I didn't keep the checks of DECL_VINDEX 
(fn) && ! (flags & LOOKUP_NONVIRTUAL) which don't seem necessary, might 
avoid function calls, though. Let me know...


Yeah, they're an optimization to avoid the calls when they aren't 
necessary but I wouldn't expect that the difference is measurable.  The 
patch is OK, thanks.


Jason


Re: C++ PATCH for DR 1813, c++/83374 - __is_standard_layout wrong for a class with repeated bases.

2019-07-05 Thread Jason Merrill

On 7/3/19 6:25 PM, Marek Polacek wrote:

This resolves DR 1813 which says that for a class to be a standard-layout
class, it may not have two (possibly indirect) base class subobjects of the
same type.  I was going to play DFS games but then I noticed we'd already set
CLASSTYPE_REPEATED_BASE_P, making this significantly easier.

There are 3 related DRs which I opened PRs for:

91079 - [DR 1881] Standard-layout classes and unnamed bit-fields
91080 - [DR 1672] Layout compatibility with multiple empty bases
91081 - [DR 2120] Array as first non-static data member in standard-layout class

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

2019-07-03  Marek Polacek  

DR 1813
PR c++/83374 - __is_standard_layout wrong for a class with repeated 
bases.
* class.c (check_bases): Set CLASSTYPE_NON_STD_LAYOUT for a class if
CLASSTYPE_REPEATED_BASE_P is true.


OK.

Jason



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

2019-07-05 Thread Szabolcs Nagy
On 28/05/2019 16:01, Szabolcs Nagy wrote:
> 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.
> 

i'd like to backport this and follow up fixes to the gcc-9 branch,
see attached patch.
From 54f408f666f7224563d2a1ff10eda6ab4d8864b1 Mon Sep 17 00:00:00 2001
From: nsz 
Date: Mon, 3 Jun 2019 13:50:53 +
Subject: [PATCH] aarch64: emit .variant_pcs for aarch64_vector_pcs symbol
 references

Backport r271869
Backport r271913
Backport r272414

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:

	* 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:

	* 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.

gcc/ChangeLog:

	* config/aarch64/aarch64-protos.h (aarch64_asm_output_external): Remove
	const.
	* config/aarch64/aarch64.c (aarch64_asm_output_external): Call
	default_elf_asm_output_external.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/pcs_attribute-2.c: Remove ifunc usage.
	* gcc.target/aarch64/pcs_attribute-3.c: New test.
---
 gcc/ChangeLog | 21 +
 gcc/config/aarch64/aarch64-protos.h   |  2 +
 gcc/config/aarch64/aarch64.c  | 36 +++
 gcc/config/aarch64/aarch64.h  |  9 ++
 gcc/testsuite/ChangeLog   | 15 +++
 .../gcc.target/aarch64/pcs_attribute-2.c  | 93 +++
 .../gcc.target/aarch64/pcs_attribute-3.c  | 58 
 .../gcc.target/aarch64/torture/simd-abi-4.c   |  3 +-
 gcc/testsuite/lib/target-supports.exp | 11 +++
 9 files changed, 247 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pcs_attribute-2.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pcs_attribute-3.c

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 82010817b6a..c0ec9df3db1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,24 @@
+2019-07-05  Szabolcs Nagy  
+
+	Backport from mainline
+	2019-06-03  Szabolcs Nagy  
+
+	* config/

Re: [patch] Small improvements to coverage info (1/n)

2019-07-05 Thread Jeff Law
On 7/5/19 6:12 AM, Jakub Jelinek wrote:
> On Wed, Jul 03, 2019 at 12:46:37PM +0200, Eric Botcazou wrote:
>> 2019-07-03  Eric Botcazou  
>>
>>  * tree-cfg.c (gimple_make_forwarder_block): Propagate location info on
>>  phi nodes if possible.
>>  * tree-scalar-evolution.c (final_value_replacement_loop): Propagate
>>  location info on the newly created statement.
>>  * tree-ssa-loop-manip.c (create_iv): Propagate location info on the
>>  newly created increment if needed.
> 
>> --- tree-ssa-loop-manip.c(revision 272930)
>> +++ tree-ssa-loop-manip.c(working copy)
>> @@ -126,10 +126,22 @@ create_iv (tree base, tree step, tree va
>>  gsi_insert_seq_on_edge_immediate (pe, stmts);
>>  
>>stmt = gimple_build_assign (va, incr_op, vb, step);
>> +  /* Prevent the increment from inheriting a bogus location if it is not put
>> + immediately after a statement whose location is known.  */
>>if (after)
>> -gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT);
>> +{
>> +  if (gsi_end_p (*incr_pos))
>> +{
>> +  edge e = single_succ_edge (gsi_bb (*incr_pos));
>> +  gimple_set_location (stmt, e->goto_locus);
>> +}
>> +  gsi_insert_after (incr_pos, stmt, GSI_NEW_STMT);
>> +}
>>else
>> -gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT);
>> +{
>> +  gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos)));
>> +  gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT);
>> +}
> 
> This change broke gomp/pr88107.c test:
> FAIL: gcc.dg/gomp/pr88107.c (internal compiler error)
> FAIL: gcc.dg/gomp/pr88107.c (test for excess errors)
> Excess errors:
> during GIMPLE pass: graphite
> /usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler 
> error: Segmentation fault
> 0x11942a4 crash_signal
> ../../gcc/toplev.c:326
> 0x13b5861 gimple_location
> ../../gcc/gimple.h:1805
> 0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*, 
> gimple_stmt_iterator*, bool, tree_node**, tree_node**)
> ../../gcc/tree-ssa-loop-manip.c:142
> 0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*, 
> tree_node*, tree_node*, tree_node**, tree_node**, loop*)
> ../../gcc/cfgloopmanip.c:831
> 
> Apparently gsi_end_p (*incr_pos) is true and after is false, so
> gsi_stmt (*incr_pos) is NULL.  One needs graphite enabled.
Which might explain the massive failures I'm seeing with graphite
enabled on various *-elf targets.

ft32-elf for example:
> Tests that now fail, but worked before (34 tests):
> 
> ft32-sim: gcc.dg/graphite/id-10.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/id-16.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/id-25.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/id-pr46834.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/id-pr47046.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/id-pr48805.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/interchange-8.c scan-tree-dump graphite "tiled by"
> ft32-sim: gcc.dg/graphite/pr29330.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr30565.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr31183.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr33576.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr36287.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr42211.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr42914.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr42917.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr46168.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr46966.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr68428.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr68493.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr68756.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr69068.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr69292.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr70045.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr77362.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr80906.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr80906.c scan-tree-dump graphite "isl AST to 
> Gimple succeeded"
> ft32-sim: gcc.dg/graphite/pr81373-2.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr81373.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr82421.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr83255.c scan-tree-dump graphite "loop nest 
> optimized"
> ft32-sim: gcc.dg/graphite/pr84204.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr84205.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr85935.c (test for excess errors)
> ft32-sim: gcc.dg/graphite/pr87931.c (test for excess errors)


SImilarly for c6x, arc, & nds32 (everything that's run in the last few
hours).

I suspect there'll be more as the day goes on.

jeff


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

2019-07-05 Thread Richard Sandiford
Szabolcs Nagy  writes:
> On 28/05/2019 16:01, Szabolcs Nagy wrote:
>> 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.
>> 
>
> i'd like to backport this and follow up fixes to the gcc-9 branch,
> see attached patch.

OK.  I agree this makes sense since vector PCS functions were a new
feature in GCC 9 and since not backporting it could lead to silent
wrong code.

Richard


Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Andrew Stubbs

On 05/07/2019 15:04, Jakub Jelinek wrote:

For OpenMP, a warning is not good enough, because if Fortran PARAMETER is
not a variable, then the program is invalid and needs to be rejected.
You could improve the diagnostics by adding some explanation message that 
fortran
PARAMETER is not a variable.


Got it confirmed, it might change in a future OpenMP release; and there has
been a change already in OpenMP 3.1 that might have intended to allow
Fortran PARAMETERs in firstprivate clause, but because the glossary hasn't
been changed, it is not valid even in firstprivate clause.


OK, here is an alternative patch that merely tries to make the error 
message more informative.


Basically, the user needs to get past "it isn't working but I need that 
value in this kernel", so hopefully this will help get them there.


WDYT?

Andrew
Tweak error message for mapped parameters.

2019-07-05  Andrew Stubbs  

	gcc/fortran/
	* openmp.c (resolve_omp_clauses): Add custom error messages for
	parameters in map clauses.

	gcc/testsuite/
	* gfortran.dg/gomp/map-1.f90: Update error message text.

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 1c7bce6c300..e05bf84faf2 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -4208,8 +4208,21 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 		  continue;
 	  }
 	  }
-	gfc_error ("Object %qs is not a variable at %L", n->sym->name,
-		   &n->where);
+	if (list == OMP_LIST_MAP
+	&& n->sym->attr.flavor == FL_PARAMETER)
+	  {
+	if (openacc)
+	  gfc_error ("Parameter %qs is not a variable at %L; it probably"
+			 " doesn't need to be copied", n->sym->name,
+			 &n->where);
+	else
+	  gfc_error ("Parameter %qs is not a variable at %L; it probably"
+			 " doesn't need to be mapped", n->sym->name,
+			 &n->where);
+	  }
+	else
+	  gfc_error ("Object %qs is not a variable at %L", n->sym->name,
+		 &n->where);
   }
 
   for (list = 0; list < OMP_LIST_NUM; list++)
diff --git a/gcc/testsuite/gfortran.dg/gomp/map-1.f90 b/gcc/testsuite/gfortran.dg/gomp/map-1.f90
index e78b56c8f39..8967b4dd0b5 100644
--- a/gcc/testsuite/gfortran.dg/gomp/map-1.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/map-1.f90
@@ -18,7 +18,7 @@ subroutine test(aas)
   !$omp target map(j)
   !$omp end target
 
-  !$omp target map(p) ! { dg-error "Object 'p' is not a variable" }
+  !$omp target map(p) ! { dg-error "Parameter 'p' is not a variable" }
   !$omp end target
 
   !$omp target map(j(1))


Re: [patch, openmp/openacc] Allow Fortran parameters in map/copy.

2019-07-05 Thread Jakub Jelinek
On Fri, Jul 05, 2019 at 03:40:55PM +0100, Andrew Stubbs wrote:
> On 05/07/2019 15:04, Jakub Jelinek wrote:
> > > For OpenMP, a warning is not good enough, because if Fortran PARAMETER is
> > > not a variable, then the program is invalid and needs to be rejected.
> > > You could improve the diagnostics by adding some explanation message that 
> > > fortran
> > > PARAMETER is not a variable.
> > 
> > Got it confirmed, it might change in a future OpenMP release; and there has
> > been a change already in OpenMP 3.1 that might have intended to allow
> > Fortran PARAMETERs in firstprivate clause, but because the glossary hasn't
> > been changed, it is not valid even in firstprivate clause.
> 
> OK, here is an alternative patch that merely tries to make the error message
> more informative.
> 
> Basically, the user needs to get past "it isn't working but I need that
> value in this kernel", so hopefully this will help get them there.
> 
> WDYT?

I don't like it, I'd end the message where you put ;, the rest doesn't make
sense and will just confuse users.  For one, the compiler should know if the
parameter needs to be copied or not, so doesn't need to talk about
probability thereof, and if it needs to be copied, it should be the compiler
that arranges that (PR90779) and the user has no way to overide or force
that behavior.

Jakub


[00/11] Diagnose ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
While doing the SVE ACLE work, I hit a case in which a pattern had
two iterators and used an attribute without saying which iterator
it was referring to.  The choice of iterator made a difference and
in this case the implicit choice happened to be the "wrong" one.

This series raises an error for cases in which an unprefixes attribute
could expand to different things depending on the iterator chosen.
It also tries to fix all instances in the ports.

Bootstrapped & regression-tested on aarch64-linux-gnu and
x86_64-linux-gnu.  Also tested by building at least one target
per CPU directory and (a) checking for errors and (b) diffing the
autogenerated files before and after the patch.

Richard


[01/11] [arch64] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change except for dropping the unused SVE
divide permutations.

I can self-approve the SVE bits, but OK for the rest?

Richard


2019-07-05  Richard Sandiford  

gcc/
* config/aarch64/aarch64.md (*compare_condjump)
(loadwb_pair_, loadwb_pair_)
(storewb_pair_, storewb_pair_)
(*ands_compare0): Fix ambiguous uses of .md attributes.
* config/aarch64/aarch64-simd.md
(*aarch64_get_lane_extend): Likewise.
(*aarch64_get_lane_zero_extend): Likewise.
* config/aarch64/aarch64-sve.md
(while_ult): Likewise.
(*cond__any): Fix SVE_I/SVE_SDI typo.

Index: gcc/config/aarch64/aarch64.md
===
--- gcc/config/aarch64/aarch64.md   2019-07-03 20:51:55.225747145 +0100
+++ gcc/config/aarch64/aarch64.md   2019-07-05 15:04:46.680794809 +0100
@@ -567,14 +567,14 @@ (define_insn "condjump"
 ;; sub x0, x1, #(CST & 0xfff000)
 ;; subsx0, x0, #(CST & 0x000fff)
 ;; b .Label
-(define_insn_and_split "*compare_condjump"
+(define_insn_and_split "*compare_condjump"
   [(set (pc) (if_then_else (EQL
  (match_operand:GPI 0 "register_operand" "r")
  (match_operand:GPI 1 "aarch64_imm24" "n"))
   (label_ref:P (match_operand 2 "" ""))
   (pc)))]
-  "!aarch64_move_imm (INTVAL (operands[1]), mode)
-   && !aarch64_plus_operand (operands[1], mode)
+  "!aarch64_move_imm (INTVAL (operands[1]), mode)
+   && !aarch64_plus_operand (operands[1], mode)
&& !reload_completed"
   "#"
   "&& true"
@@ -582,11 +582,12 @@ (define_insn_and_split "*compare_condjum
   {
 HOST_WIDE_INT lo_imm = UINTVAL (operands[1]) & 0xfff;
 HOST_WIDE_INT hi_imm = UINTVAL (operands[1]) & 0xfff000;
-rtx tmp = gen_reg_rtx (mode);
-emit_insn (gen_add3 (tmp, operands[0], GEN_INT (-hi_imm)));
-emit_insn (gen_add3_compare0 (tmp, tmp, GEN_INT (-lo_imm)));
+rtx tmp = gen_reg_rtx (mode);
+emit_insn (gen_add3 (tmp, operands[0], GEN_INT (-hi_imm)));
+emit_insn (gen_add3_compare0 (tmp, tmp, GEN_INT (-lo_imm)));
 rtx cc_reg = gen_rtx_REG (CC_NZmode, CC_REGNUM);
-rtx cmp_rtx = gen_rtx_fmt_ee (, mode, cc_reg, const0_rtx);
+rtx cmp_rtx = gen_rtx_fmt_ee (, mode,
+ cc_reg, const0_rtx);
 emit_jump_insn (gen_condjump (cmp_rtx, cc_reg, operands[2]));
 DONE;
   }
@@ -1505,8 +1506,8 @@ (define_insn "loadwb_pair_mode)"
-  "ldp\\t%2, %3, [%1], %4"
-  [(set_attr "type" "load_")]
+  "ldp\\t%2, %3, [%1], %4"
+  [(set_attr "type" "load_")]
 )
 
 (define_insn "loadwb_pair_"
@@ -1520,7 +1521,7 @@ (define_insn "loadwb_pair_mode)"
-  "ldp\\t%2, %3, [%1], %4"
+  "ldp\\t%2, %3, [%1], %4"
   [(set_attr "type" "neon_load1_2reg")]
 )
 
@@ -1553,8 +1554,8 @@ (define_insn "storewb_pair_mode)"
-  "stp\\t%2, %3, [%0, %4]!"
-  [(set_attr "type" "store_")]
+  "stp\\t%2, %3, [%0, %4]!"
+  [(set_attr "type" "store_")]
 )
 
 (define_insn "storewb_pair_"
@@ -1569,7 +1570,7 @@ (define_insn "storewb_pair_mode)"
-  "stp\\t%2, %3, [%0, %4]!"
+  "stp\\t%2, %3, [%0, %4]!"
   [(set_attr "type" "neon_store1_2reg")]
 )
 
@@ -4782,7 +4783,7 @@ (define_insn "*and_compare0"
   [(set_attr "type" "alus_imm")]
 )
 
-(define_insn "*ands_compare0"
+(define_insn "*ands_compare0"
   [(set (reg:CC_NZ CC_REGNUM)
(compare:CC_NZ
 (zero_extend:GPI (match_operand:SHORT 1 "register_operand" "r"))
Index: gcc/config/aarch64/aarch64-simd.md
===
--- gcc/config/aarch64/aarch64-simd.md  2019-07-03 20:51:55.225747145 +0100
+++ gcc/config/aarch64/aarch64-simd.md  2019-07-05 15:04:46.676794843 +0100
@@ -3135,30 +3135,31 @@ (define_expand "vcondu"
   [(set (match_operand:GPI 0 "register_operand" "=r")
(sign_extend:GPI
- (vec_select:
+ (vec_select:
(match_operand:VDQQH 1 "register_operand" "w")
(parallel [(match_operand:SI 2 "immediate_operand" "i")]]
   "TARGET_SIMD"
   {
-operands[2] = aarch64_endian_lane_rtx (mode, INTVAL (operands[2]));
+operands[2] = aarch64_endian_lane_rtx (mode,
+  INTVAL (operands[2]));
 return "smov\\t%0, %1.[%2]";
   }
-  [(set_attr "type" "neon_to_gp")]
-)
-
-(define_insn "*aarch64_get_lane_zero_extend"
-  [(set (match_operand:GPI 0 "register_operand" "=r")
-   (zero_extend:GPI
- (vec_select:
-   (match_operand:VDQQH 1 "register_operand" "w")
-   (parallel [(match_operand:SI 2 "immediate_operand" "i")]]
-  "TARGET_SIMD"
-  {
-operands[2] = aarch64_endian_lane_rtx (mode,
-  INTVAL (operands[2]));
-  

[02/11] [arm] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for Thumb-1, since previously the
LDREX width was taken from the SImode success result rather than the
memory mode:

-#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREXBH && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREXBH && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREXD && 
ARM_DOUBLEWORD_ALIGN \
+   && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))

The same goes for the predicate and constraints in
@atomic_compare_and_swapt1di_1, which previously used the
SI values from the success result.


2019-07-05  Richard Sandiford  

gcc/
* config/arm/sync.md
(@atomic_compare_and_swap_1): Use
 instead of (implicitly) .
(@atomic_compare_and_swap_1): Likewise
.  Use  and
.

Index: gcc/config/arm/sync.md
===
--- gcc/config/arm/sync.md  2019-07-01 09:37:07.224524452 +0100
+++ gcc/config/arm/sync.md  2019-07-05 15:05:09.088609956 +0100
@@ -201,7 +201,7 @@ (define_insn_and_split "@atomic_compare_
   (match_operand:SI 7 "const_int_operand")];; mod_f
  VUNSPEC_ATOMIC_CAS))
(clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  ""
+  ""
   "#"
   "&& reload_completed"
   [(const_int 0)]
@@ -225,14 +225,14 @@ (define_insn_and_split "@atomic_compare_
(match_operand:SIDI 2 "mem_noofs_operand" "+Ua,Ua,Ua,Ua"))  ;; 
memory
(set (match_dup 2)
(unspec_volatile:SIDI
- [(match_operand:SIDI 3 "" 
",lIL*h,J,*r") ;; expect
+ [(match_operand:SIDI 3 "" 
",lIL*h,J,*r") ;; expect
   (match_operand:SIDI 4 "s_register_operand" "r,r,r,r");; 
desired
   (match_operand:SI 5 "const_int_operand") ;; is_weak
   (match_operand:SI 6 "const_int_operand") ;; mod_s
   (match_operand:SI 7 "const_int_operand")];; mod_f
  VUNSPEC_ATOMIC_CAS))
(clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  ""
+  ""
   "#"
   "&& reload_completed"
   [(const_int 0)]


[03/11] [amdgcn] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for the case in which the 1REG_MODE
and 1REG_ALT are different, since previously we would use the 1REG_MODE
for both the comparison and the select, even though the operands being
compared are 1REG_ALT rather than 1REG_MODE.


2019-07-05  Richard Sandiford  

gcc/
* config/gcn/gcn-valu.md
(vcond): Use
gen_vec_cmpdi rather than (implicitly)
gen_vec_cmpdi.  Explicitly use
gen_vcond_mask_di.
(vcond_exec): Likewise,
but using the _exec comparison patterns.
(vcondu): Use
gen_vec_cmpdi rather than (implicitly)
gen_vec_cmpdi.  Explicitly use
gen_vcond_mask_di.
(vcondu_exec): Likewise,
but using the _exec comparison patterns.

Index: gcc/config/gcn/gcn-valu.md
===
--- gcc/config/gcn/gcn-valu.md  2019-03-08 18:15:37.764736305 +
+++ gcc/config/gcn/gcn-valu.md  2019-07-05 15:05:24.076486317 +0100
@@ -2574,10 +2574,10 @@ (define_expand "vconddi (tmp, operands[3], operands[4],
-   operands[5]));
-emit_insn (gen_vcond_mask_di (operands[0], operands[1], operands[2],
-   tmp));
+emit_insn (gen_vec_cmpdi
+  (tmp, operands[3], operands[4], operands[5]));
+emit_insn (gen_vcond_mask_di
+  (operands[0], operands[1], operands[2], tmp));
 DONE;
   })
 
@@ -2592,10 +2592,10 @@ (define_expand "vconddi_exec (tmp, operands[3], operands[4],
-operands[5], operands[6]));
-emit_insn (gen_vcond_mask_di (operands[0], operands[1], operands[2],
-   tmp));
+emit_insn (gen_vec_cmpdi_exec
+  (tmp, operands[3], operands[4], operands[5], operands[6]));
+emit_insn (gen_vcond_mask_di
+  (operands[0], operands[1], operands[2], tmp));
 DONE;
   })
 
@@ -2609,10 +2609,10 @@ (define_expand "vcondudi (tmp, operands[3], operands[4],
-   operands[5]));
-emit_insn (gen_vcond_mask_di (operands[0], operands[1], operands[2],
-   tmp));
+emit_insn (gen_vec_cmpdi
+  (tmp, operands[3], operands[4], operands[5]));
+emit_insn (gen_vcond_mask_di
+  (operands[0], operands[1], operands[2], tmp));
 DONE;
   })
 
@@ -2627,10 +2627,10 @@ (define_expand "vcondudi_exec (tmp, operands[3], operands[4],
-operands[5], operands[6]));
-emit_insn (gen_vcond_mask_di (operands[0], operands[1], operands[2],
-   tmp));
+emit_insn (gen_vec_cmpdi_exec
+  (tmp, operands[3], operands[4], operands[5], operands[6]));
+emit_insn (gen_vcond_mask_di
+  (operands[0], operands[1], operands[2], tmp));
 DONE;
   })
 


[04/11] [h8300] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  

gcc/
* config/h8300/h8300.md (*push1_h8300hs_): Explicitly
specify the mode iterator referenced by , giving...
(*push1_h8300hs_): ...this.

Index: gcc/config/h8300/h8300.md
===
--- gcc/config/h8300/h8300.md   2019-07-01 09:37:07.328523581 +0100
+++ gcc/config/h8300/h8300.md   2019-07-05 15:06:32.491921996 +0100
@@ -728,7 +728,7 @@ (define_insn "*pushqi1_h8300"
   "mov.w\\t%T0,@-r7"
   [(set_attr "length" "2")])
 
-(define_insn "*push1_h8300hs_"
+(define_insn "*push1_h8300hs_"
   [(set (mem:QHI
(pre_modify:P
  (reg:P SP_REG)


[05/11] [i386] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change except for dropping the unused *andnot3_bcst
permutations.


2019-07-05  Richard Sandiford  

gcc/
* config/i386/i386.md (*fop__3_i387)
(l2): Fix ambiguous uses
of .md attributes.
* config/i386/sse.md (*avx512pf_gatherpfsf_mask)
(*avx512pf_gatherpfdf_mask, *avx512pf_scatterpfsf_mask)
(*avx512pf_scatterpfdf_mask, *avx2_gathersi)
(*avx2_gathersi_2, *avx2_gatherdi)
(*avx2_gatherdi_2, *avx2_gatherdi_3): Likewise.
(*avx2_gatherdi_4, *avx512f_gathersi): Likewise.
(*avx512f_gathersi_2, *avx512f_gatherdi): Likewise.
(*avx512f_gatherdi_2, *avx512f_scattersi): Likewise.
(*avx512f_scatterdi): Likewise.
(*andnot3_bcst): Fix VI/VI48_AVX512VL typo.

Index: gcc/config/i386/i386.md
===
--- gcc/config/i386/i386.md 2019-07-03 20:50:46.074319519 +0100
+++ gcc/config/i386/i386.md 2019-07-05 15:05:55.216229452 +0100
@@ -14755,7 +14755,7 @@ (define_insn "*fop__3_i38
  ]
  (const_string "fop")))
(set_attr "fp_int_src" "true")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 (define_insn "*fop_xf_4_i387"
   [(set (match_operand:XF 0 "register_operand" "=f,f")
@@ -16457,7 +16457,7 @@ (define_expand "lmode);
 
-  emit_insn (gen_sse4_1_round2
+  emit_insn (gen_sse4_1_round2
 (tmp, operands[1], GEN_INT (ROUND_
 | ROUND_NO_EXC)));
   emit_insn (gen_fix_trunc2
Index: gcc/config/i386/sse.md
===
--- gcc/config/i386/sse.md  2019-07-03 20:50:46.078319486 +0100
+++ gcc/config/i386/sse.md  2019-07-05 15:05:55.220229422 +0100
@@ -12702,8 +12702,8 @@ (define_insn "*andnot3"
  (const_string "")))])
 
 (define_insn "*andnot3_bcst"
-  [(set (match_operand:VI 0 "register_operand" "=v")
-   (and:VI
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+   (and:VI48_AVX512VL
  (not:VI48_AVX512VL
 (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
  (vec_duplicate:VI48_AVX512VL
@@ -18085,7 +18085,7 @@ (define_expand "avx512pf_gatherpfs
operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_gatherpfsf_mask"
+(define_insn "*avx512pf_gatherpfsf_mask"
   [(unspec
  [(match_operand: 0 "register_operand" "Yk")
   (match_operator: 5 "vsib_mem_operator"
@@ -18132,7 +18132,7 @@ (define_expand "avx512pf_gatherpfd
operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_gatherpfdf_mask"
+(define_insn "*avx512pf_gatherpfdf_mask"
   [(unspec
  [(match_operand: 0 "register_operand" "Yk")
   (match_operator:V8DF 5 "vsib_mem_operator"
@@ -18179,7 +18179,7 @@ (define_expand "avx512pf_scatterpf
operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_scatterpfsf_mask"
+(define_insn "*avx512pf_scatterpfsf_mask"
   [(unspec
  [(match_operand: 0 "register_operand" "Yk")
   (match_operator: 5 "vsib_mem_operator"
@@ -18228,7 +18228,7 @@ (define_expand "avx512pf_scatterpf
operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_scatterpfdf_mask"
+(define_insn "*avx512pf_scatterpfdf_mask"
   [(unspec
  [(match_operand: 0 "register_operand" "Yk")
   (match_operator:V8DF 5 "vsib_mem_operator"
@@ -21017,7 +21017,7 @@ (define_expand "avx2_gathersi"
operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx2_gathersi"
+(define_insn "*avx2_gathersi"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
(unspec:VEC_GATHER_MODE
  [(match_operand:VEC_GATHER_MODE 2 "register_operand" "0")
@@ -21037,7 +21037,7 @@ (define_insn "*avx2_gathersi"
(set_attr "prefix" "vex")
(set_attr "mode" "")])
 
-(define_insn "*avx2_gathersi_2"
+(define_insn "*avx2_gathersi_2"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
(unspec:VEC_GATHER_MODE
  [(pc)
@@ -21078,7 +21078,7 @@ (define_expand "avx2_gatherdi"
operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx2_gatherdi"
+(define_insn "*avx2_gatherdi"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
(unspec:VEC_GATHER_MODE
  [(match_operand: 2 "register_operand" "0")
@@ -21098,7 +21098,7 @@ (define_insn "*avx2_gatherdi"
(set_attr "prefix" "vex")
(set_attr "mode" "")])
 
-(define_insn "*avx2_gatherdi_2"
+(define_insn "*avx2_gatherdi_2"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_ope

[06/11] [mips] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  

gcc/
* config/mips/micromips.md (*movep):
Explicitly use  for the mode attribute.

Index: gcc/config/mips/micromips.md
===
--- gcc/config/mips/micromips.md2019-03-08 18:15:39.000731609 +
+++ gcc/config/mips/micromips.md2019-07-05 15:07:29.083455268 +0100
@@ -133,5 +133,5 @@ (define_insn "*movep")
+   (set_attr "mode" "")
(set_attr "can_delay" "no")])


[07/11] [riscv] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  

gcc/
* config/riscv/pic.md (*local_pic_load_s)
(*local_pic_load_u): Explicitly specify the mode iterator
referenced by , giving...
(*local_pic_load_s, *local_pic_load_u): ...these.
* config/riscv/riscv.md (*sge_)
(*slt_, *sle_): Explicitly
use  for the mode attribute.

Index: gcc/config/riscv/pic.md
===
--- gcc/config/riscv/pic.md 2019-03-08 18:15:38.084735089 +
+++ gcc/config/riscv/pic.md 2019-07-05 15:07:46.695310028 +0100
@@ -29,14 +29,14 @@ (define_insn "*local_pic_load"
   "\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load_s"
+(define_insn "*local_pic_load_s"
   [(set (match_operand:SUPERQI 0 "register_operand" "=r")
(sign_extend:SUPERQI (mem:SUBX (match_operand 1 
"absolute_symbolic_operand" ""]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
   "\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load_u"
+(define_insn "*local_pic_load_u"
   [(set (match_operand:SUPERQI 0 "register_operand" "=r")
(zero_extend:SUPERQI (mem:SUBX (match_operand 1 
"absolute_symbolic_operand" ""]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
Index: gcc/config/riscv/riscv.md
===
--- gcc/config/riscv/riscv.md   2019-07-01 09:37:06.632529410 +0100
+++ gcc/config/riscv/riscv.md   2019-07-05 15:07:46.699309994 +0100
@@ -2054,7 +2054,7 @@ (define_insn "*sge_
   ""
   "slt%i2\t%0,zero,%1"
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 (define_insn "*slt_"
   [(set (match_operand:GPR   0 "register_operand" "= r")
@@ -2063,7 +2063,7 @@ (define_insn "*slt_
   ""
   "slt%i2\t%0,%1,%2"
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 (define_insn "*sle_"
   [(set (match_operand:GPR   0 "register_operand" "=r")
@@ -2075,7 +2075,7 @@ (define_insn "*sle_
   return "slt%i2\t%0,%1,%2";
 }
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 ;;
 ;;  


[08/11] [rs6000] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before except
for formatting and line numbers.


2019-07-05  Richard Sandiford  

gcc/
* config/rs6000/rs6000.md (*mov_update1): Explicitly
use , ,  and  rather than
leaving the choice between SFDF and P implicit.
(*mov_update2): Likewise.
(*cmp_internal2): Explicitly use 
rather than leaving the choice betweem IBM128 and GPR implicit.
(*fix_trunc2_mem): Explicitly use
 rather than leaving the choice between IEEE128 and
QHSI implicit.
(AltiVec define_peephole2s): Explicitly use 
rather than leaving the choice between ALTIVEC_DFORM and P implicit.
* config/rs6000/vsx.md
(*vsx_ext__fl_)
(*vsx_ext__ufl_): Explicitly
use  rather than leaving the choice between FL_CONV
and VSX_EXTRACT_I implicit.

Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md 2019-07-03 20:50:46.390316903 +0100
+++ gcc/config/rs6000/rs6000.md 2019-07-05 15:08:06.639145562 +0100
@@ -9312,14 +9312,14 @@ (define_insn "*movqi_update3"
(set_attr "update" "yes")
(set_attr "indexed" "yes,no")])
 
-(define_insn "*mov_update1"
-  [(set (match_operand:SFDF 3 "gpc_reg_operand" "=,")
+(define_insn "*mov_update1"
+  [(set (match_operand:SFDF 3 "gpc_reg_operand" "=,")
(mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
  (match_operand:P 2 "reg_or_short_operand" "r,I"
(set (match_operand:P 0 "gpc_reg_operand" "=b,b")
(plus:P (match_dup 1) (match_dup 2)))]
   "TARGET_HARD_FLOAT && TARGET_UPDATE
-   && (!avoiding_indexed_address_p (mode)
+   && (!avoiding_indexed_address_p (mode)
|| !gpc_reg_operand (operands[2], Pmode))"
   "@
lfux %3,%0,%2
@@ -9327,16 +9327,16 @@ (define_insn "*mov_update1"
   [(set_attr "type" "fpload")
(set_attr "update" "yes")
(set_attr "indexed" "yes,no")
-   (set_attr "size" "")])
+   (set_attr "size" "")])
 
-(define_insn "*mov_update2"
+(define_insn "*mov_update2"
   [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
  (match_operand:P 2 "reg_or_short_operand" "r,I")))
-   (match_operand:SFDF 3 "gpc_reg_operand" ","))
+   (match_operand:SFDF 3 "gpc_reg_operand" ","))
(set (match_operand:P 0 "gpc_reg_operand" "=b,b")
(plus:P (match_dup 1) (match_dup 2)))]
   "TARGET_HARD_FLOAT && TARGET_UPDATE
-   && (!avoiding_indexed_address_p (mode)
+   && (!avoiding_indexed_address_p (mode)
|| !gpc_reg_operand (operands[2], Pmode))"
   "@
stfux %3,%0,%2
@@ -9344,7 +9344,7 @@ (define_insn "*mov_update2"
   [(set_attr "type" "fpstore")
(set_attr "update" "yes")
(set_attr "indexed" "yes,no")
-   (set_attr "size" "")])
+   (set_attr "size" "")])
 
 (define_insn "*movsf_update3"
   [(set (match_operand:SF 3 "gpc_reg_operand" "=r,r")
@@ -11558,7 +11558,7 @@ (define_insn "*cmp_internal1"
   [(set_attr "type" "fpcompare")
(set_attr "length" "12")])
 
-(define_insn_and_split "*cmp_internal2"
+(define_insn_and_split "*cmp_internal2"
   [(set (match_operand:CCFP 0 "cc_reg_operand" "=y")
(compare:CCFP (match_operand:IBM128 1 "gpc_reg_operand" "d")
  (match_operand:IBM128 2 "gpc_reg_operand" "d")))
@@ -11571,7 +11571,7 @@ (define_insn_and_split "*cmp_inter
 (clobber (match_scratch:DF 9 "=d"))
 (clobber (match_scratch:DF 10 "=d"))
 (clobber (match_scratch:GPR 11 "=b"))]
-  "TARGET_XL_COMPAT && FLOAT128_IBM_P (mode)
+  "TARGET_XL_COMPAT && FLOAT128_IBM_P (mode)
&& TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
   "#"
   "&& reload_completed"
@@ -11595,10 +11595,14 @@ (define_insn_and_split "*cmp_inter
   const int lo_word = LONG_DOUBLE_LARGE_FIRST ? GET_MODE_SIZE (DFmode) : 0;
   const int hi_word = LONG_DOUBLE_LARGE_FIRST ? 0 : GET_MODE_SIZE (DFmode);
 
-  operands[5] = simplify_gen_subreg (DFmode, operands[1], mode, hi_word);
-  operands[6] = simplify_gen_subreg (DFmode, operands[1], mode, lo_word);
-  operands[7] = simplify_gen_subreg (DFmode, operands[2], mode, hi_word);
-  operands[8] = simplify_gen_subreg (DFmode, operands[2], mode, lo_word);
+  operands[5] = simplify_gen_subreg (DFmode, operands[1],
+mode, hi_word);
+  operands[6] = simplify_gen_subreg (DFmode, operands[1],
+mode, lo_word);
+  operands[7] = simplify_gen_subreg (DFmode, operands[2],
+mode, hi_word);
+  operands[8] = simplify_gen_subreg (DFmode, operands[2],
+mode, lo_word);
   operands[12] = gen_label_rtx ();
   operands[13] = gen_label_rtx ();
   real_inf 

[09/11] [s390] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

The vx-builtins.md part changes the choice of  from the
implicit  to an explicit  (i.e. from the
mode of the comparison result to the mode of the operands being
compared).  That seemed like the intended behaviour given later
patterns like vec_cmpeq_cc.

The use of BFP in the s390.md LNDFR pattern looks like a typo,
since the operand to (abs ...) has to have the same mode as the result.
The only effect before this series was to create some extra variants
that would never match, making it harmless apart from very minor code
bloat.


2019-07-05  Richard Sandiford  

gcc/
* config/s390/s390.md (*negabs2_nocc): Use FP for
operand 1.
* config/s390/vx-builtins.md (*vec_cmp_cconly):
Make the choice of  explicit, giving...
(*vec_cmp_cconly): ...this.

Index: gcc/config/s390/s390.md
===
--- gcc/config/s390/s390.md 2019-07-01 09:37:06.016534568 +0100
+++ gcc/config/s390/s390.md 2019-07-05 15:08:27.018977508 +0100
@@ -8821,8 +8821,8 @@ (define_insn "*negabs2_cconly"
 
 ; lndfr
 (define_insn "*negabs2_nocc"
-  [(set (match_operand:FP 0 "register_operand"  "=f")
-(neg:FP (abs:FP (match_operand:BFP 1 "register_operand" ""]
+  [(set (match_operand:FP 0 "register_operand" "=f")
+(neg:FP (abs:FP (match_operand:FP 1 "register_operand" ""]
   "TARGET_DFP"
   "lndfr\t%0,%1"
   [(set_attr "op_type"  "RRE")
Index: gcc/config/s390/vx-builtins.md
===
--- gcc/config/s390/vx-builtins.md  2019-04-18 13:28:51.035821466 +0100
+++ gcc/config/s390/vx-builtins.md  2019-07-05 15:08:27.018977508 +0100
@@ -2046,7 +2046,7 @@ (define_insn "*vec_cmphl_cc"
 ;;
 
 ; vfcesbs, vfcedbs, wfcexbs, vfchsbs, vfchdbs, wfchxbs, vfchesbs, vfchedbs, 
wfchexbs
-(define_insn "*vec_cmp_cconly"
+(define_insn "*vec_cmp_cconly"
   [(set (reg:VFCMP CC_REGNUM)
(compare:VFCMP (match_operand:VF_HW 0 "register_operand" "v")
   (match_operand:VF_HW 1 "register_operand" "v")))


[10/11] Use file_location for md_reader's ptr_loc

2019-07-05 Thread Richard Sandiford
Use file_location for md_reader's ptr_loc.  Also make it public, so that
clients can use the location for error reporting.

I'll apply this once the port changes are in.  (It could go in now,
but it's Friday afternoon...)


2019-07-05  Richard Sandiford  

gcc/
* read-md.h (md_reader::ptr_loc): Moved from read-md.c.
Use file_location instead of separate fields.
(md_reader::set_md_ptr_loc): Take a file_location instead of a
separate filename and line number.
* read-md.c (ptr_loc): As above.
(md_reader::copy_md_ptr_loc): Update for new ptr_loc layout.
(md_reader::fprint_md_ptr_loc): Likewise.
(md_reader::set_md_ptr_loc): Likewise.  Take a file_location
instead of a separate filename and line number.
(md_reader::read_string): Update call accordingly.

Index: gcc/read-md.h
===
--- gcc/read-md.h   2019-05-12 12:27:15.753897237 +0100
+++ gcc/read-md.h   2019-07-05 15:08:52.698765764 +0100
@@ -148,6 +148,13 @@ struct mapping;
 class md_reader
 {
  public:
+  /* Associates PTR (which can be a string, etc.) with the file location
+ specified by LOC.  */
+  struct ptr_loc {
+const void *ptr;
+file_location loc;
+  };
+
   md_reader (bool compact);
   virtual ~md_reader ();
 
@@ -182,7 +189,7 @@ struct mapping;
   void require_word_ws (const char *expected);
   int peek_char (void);
 
-  void set_md_ptr_loc (const void *ptr, const char *filename, int lineno);
+  void set_md_ptr_loc (const void *ptr, file_location);
   const struct ptr_loc *get_md_ptr_loc (const void *ptr);
   void copy_md_ptr_loc (const void *new_ptr, const void *old_ptr);
   void fprint_md_ptr_loc (FILE *outf, const void *ptr);
Index: gcc/read-md.c
===
--- gcc/read-md.c   2019-03-08 18:15:36.692740380 +
+++ gcc/read-md.c   2019-07-05 15:08:52.698765764 +0100
@@ -43,14 +43,6 @@ int have_error = 0;
 #endif /* #ifndef GENERATOR_FILE */
 
 
-/* Associates PTR (which can be a string, etc.) with the file location
-   specified by FILENAME and LINENO.  */
-struct ptr_loc {
-  const void *ptr;
-  const char *filename;
-  int lineno;
-};
-
 /* This callback will be invoked whenever an md include directive is
processed.  To be used for creation of the dependency file.  */
 void (*include_callback) (const char *);
@@ -94,25 +86,24 @@ leading_ptr_eq_p (const void *def1, cons
   return *(const void *const *) def1 == *(const void *const *) def2;
 }
 
-/* Associate PTR with the file position given by FILENAME and LINENO.  */
+/* Associate PTR with the file position given by FILE_LOC.  */
 
 void
-md_reader::set_md_ptr_loc (const void *ptr, const char *filename, int lineno)
+md_reader::set_md_ptr_loc (const void *ptr, file_location file_loc)
 {
   struct ptr_loc *loc;
 
   loc = (struct ptr_loc *) obstack_alloc (&m_ptr_loc_obstack,
  sizeof (struct ptr_loc));
   loc->ptr = ptr;
-  loc->filename = filename;
-  loc->lineno = lineno;
+  loc->loc = file_loc;
   *htab_find_slot (m_ptr_locs, loc, INSERT) = loc;
 }
 
 /* Return the position associated with pointer PTR.  Return null if no
position was set.  */
 
-const struct ptr_loc *
+const md_reader::ptr_loc *
 md_reader::get_md_ptr_loc (const void *ptr)
 {
   return (const struct ptr_loc *) htab_find (m_ptr_locs, &ptr);
@@ -125,7 +116,7 @@ md_reader::copy_md_ptr_loc (const void *
 {
   const struct ptr_loc *loc = get_md_ptr_loc (old_ptr);
   if (loc != 0)
-set_md_ptr_loc (new_ptr, loc->filename, loc->lineno);
+set_md_ptr_loc (new_ptr, loc->loc);
 }
 
 /* If PTR is associated with a known file position, print a #line
@@ -136,7 +127,7 @@ md_reader::fprint_md_ptr_loc (FILE *outf
 {
   const struct ptr_loc *loc = get_md_ptr_loc (ptr);
   if (loc != 0)
-fprintf (outf, "#line %d \"%s\"\n", loc->lineno, loc->filename);
+fprintf (outf, "#line %d \"%s\"\n", loc->loc.lineno, loc->loc.filename);
 }
 
 /* Special fprint_md_ptr_loc for writing to STDOUT.  */
@@ -672,7 +663,7 @@ md_reader::read_string (int star_if_brac
 {
   char *stringbuf;
   int saw_paren = 0;
-  int c, old_lineno;
+  int c;
 
   c = read_skip_spaces ();
   if (c == '(')
@@ -681,7 +672,7 @@ md_reader::read_string (int star_if_brac
   c = read_skip_spaces ();
 }
 
-  old_lineno = get_lineno ();
+  file_location loc = get_current_location ();
   if (c == '"')
 stringbuf = read_quoted_string ();
   else if (c == '{')
@@ -704,7 +695,7 @@ md_reader::read_string (int star_if_brac
   if (saw_paren)
 require_char_ws (')');
 
-  set_md_ptr_loc (stringbuf, get_filename (), old_lineno);
+  set_md_ptr_loc (stringbuf, loc);
   return stringbuf;
 }
 


[11/11] Report ambiguous uses of .md attributes

2019-07-05 Thread Richard Sandiford
This patch reports an error if the .md file has an unscoped
attribute that maps to more than one possible value.

I'll apply this once the port patches are in.


2019-07-05  Richard Sandiford  

gcc/
* read-md.h (md_reader::record_potential_iterator_use): Add a
file_location parameter.
* read-rtl.c (attribute_use::loc): New field.
(map_attr_string): Take a file_location parameter.  Report cases
in which attributes map to multiple distinct values.
(apply_attribute_uses): Update call accordingly.
(md_reader::handle_overloaded_name): Likewise.
(md_reader::apply_iterator_to_string): Likewise.  Skip empty
nonnull strings.
(record_attribute_use): Take a file_location parameter.
Initialize attribute_use::loc.
(md_reader::record_potential_iterator_use): Take a file_location
parameter.  Update call to record_attribute_use.
(rtx_reader::rtx_alloc_for_name): Update call accordingly.
(rtx_reader::read_rtx_code): Likewise.
(rtx_reader::read_rtx_operand): Likewise.  Record a location
for implicitly-expanded empty strings.

Index: gcc/read-md.h
===
--- gcc/read-md.h   2019-07-05 15:08:52.698765764 +0100
+++ gcc/read-md.h   2019-07-05 15:09:08.694633872 +0100
@@ -211,8 +211,8 @@ struct mapping;
   rtx copy_rtx_for_iterators (rtx original);
   void read_conditions ();
   void record_potential_iterator_use (struct iterator_group *group,
- rtx x, unsigned int index,
- const char *name);
+ file_location loc, rtx x,
+ unsigned int index, const char *name);
   struct mapping *read_mapping (struct iterator_group *group, htab_t table);
   overloaded_name *handle_overloaded_name (rtx, vec *);
 
Index: gcc/read-rtl.c
===
--- gcc/read-rtl.c  2019-07-01 09:37:05.780536545 +0100
+++ gcc/read-rtl.c  2019-07-05 15:09:08.694633872 +0100
@@ -106,6 +106,9 @@ struct attribute_use {
   /* The group that describes the use site.  */
   struct iterator_group *group;
 
+  /* The location at which the use occurs.  */
+  file_location loc;
+
   /* The name of the attribute, possibly with an "iterator:" prefix.  */
   const char *value;
 
@@ -361,10 +364,10 @@ find_subst_iter_by_attr (const char *att
 
 /* Map attribute string P to its current value.  Return null if the attribute
isn't known.  If ITERATOR_OUT is nonnull, store the associated iterator
-   there.  */
+   there.  Report any errors against location LOC.  */
 
 static struct map_value *
-map_attr_string (const char *p, mapping **iterator_out = 0)
+map_attr_string (file_location loc, const char *p, mapping **iterator_out = 0)
 {
   const char *attr;
   struct mapping *iterator;
@@ -372,6 +375,8 @@ map_attr_string (const char *p, mapping
   struct mapping *m;
   struct map_value *v;
   int iterator_name_len;
+  struct map_value *res = NULL;
+  struct mapping *prev = NULL;
 
   /* Peel off any "iterator:" prefix.  Set ATTR to the start of the
  attribute name.  */
@@ -414,13 +419,22 @@ map_attr_string (const char *p, mapping
  for (v = m->values; v; v = v->next)
if (v->number == iterator->current_value->number)
  {
+   if (res && strcmp (v->string, res->string) != 0)
+ {
+   error_at (loc, "ambiguous attribute '%s'; could be"
+ " '%s' (via '%s:%s') or '%s' (via '%s:%s')",
+ attr, res->string, prev->name, attr,
+ v->string, iterator->name, attr);
+   return v;
+ }
if (iterator_out)
  *iterator_out = iterator;
-   return v;
+   prev = iterator;
+   res = v;
  }
}
 }
-  return NULL;
+  return res;
 }
 
 /* Apply the current iterator values to STRING.  Return the new string
@@ -432,16 +446,17 @@ md_reader::apply_iterator_to_string (con
   char *base, *copy, *p, *start, *end;
   struct map_value *v;
 
-  if (string == 0)
+  if (string == 0 || string[0] == 0)
 return string;
 
+  file_location loc = get_md_ptr_loc (string)->loc;
   base = p = copy = ASTRDUP (string);
   while ((start = strchr (p, '<')) && (end = strchr (start, '>')))
 {
   p = start + 1;
 
   *end = 0;
-  v = map_attr_string (p);
+  v = map_attr_string (loc, p);
   *end = '>';
   if (v == 0)
continue;
@@ -572,7 +587,7 @@ apply_attribute_uses (void)
 
   FOR_EACH_VEC_ELT (attribute_uses, i, ause)
 {
-  v = map_attr_string (ause->value);
+  v = map_attr_string (ause->loc, ause->value);
   if (!v)
fatal_with_file_and_line ("unknown iterator value `%s'", ause->value);

Re: [03/11] [amdgcn] Fix ambiguous .md attribute uses

2019-07-05 Thread Andrew Stubbs

On 05/07/2019 16:12, Richard Sandiford wrote:

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for the case in which the 1REG_MODE
and 1REG_ALT are different, since previously we would use the 1REG_MODE
for both the comparison and the select, even though the operands being
compared are 1REG_ALT rather than 1REG_MODE.


2019-07-05  Richard Sandiford  

gcc/
* config/gcn/gcn-valu.md
(vcond): Use
gen_vec_cmpdi rather than (implicitly)
gen_vec_cmpdi.  Explicitly use
gen_vcond_mask_di.
(vcond_exec): Likewise,
but using the _exec comparison patterns.
(vcondu): Use
gen_vec_cmpdi rather than (implicitly)
gen_vec_cmpdi.  Explicitly use
gen_vcond_mask_di.
(vcondu_exec): Likewise,
but using the _exec comparison patterns.


This seems logically correct to me.

Thanks Richard.

Andrew


Re: [05/11] [i386] Fix ambiguous .md attribute uses

2019-07-05 Thread Uros Bizjak
On Fri, Jul 5, 2019 at 5:15 PM Richard Sandiford
 wrote:
>
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
>  to specify an iterator, and in which  could
> have different values depending on the iterator chosen.
>
> No behavioural change except for dropping the unused *andnot3_bcst
> permutations.
>
>
> 2019-07-05  Richard Sandiford  
>
> gcc/
> * config/i386/i386.md (*fop__3_i387)
> (l2): Fix ambiguous uses
> of .md attributes.
> * config/i386/sse.md (*avx512pf_gatherpfsf_mask)
> (*avx512pf_gatherpfdf_mask, *avx512pf_scatterpfsf_mask)
> (*avx512pf_scatterpfdf_mask, *avx2_gathersi)
> (*avx2_gathersi_2, *avx2_gatherdi)
> (*avx2_gatherdi_2, *avx2_gatherdi_3): Likewise.
> (*avx2_gatherdi_4, *avx512f_gathersi): Likewise.
> (*avx512f_gathersi_2, *avx512f_gatherdi): Likewise.
> (*avx512f_gatherdi_2, *avx512f_scattersi): Likewise.
> (*avx512f_scatterdi): Likewise.
> (*andnot3_bcst): Fix VI/VI48_AVX512VL typo.

OK.

Thanks,
Uros.

> Index: gcc/config/i386/i386.md
> ===
> --- gcc/config/i386/i386.md 2019-07-03 20:50:46.074319519 +0100
> +++ gcc/config/i386/i386.md 2019-07-05 15:05:55.216229452 +0100
> @@ -14755,7 +14755,7 @@ (define_insn "*fop__3_i38
>   ]
>   (const_string "fop")))
> (set_attr "fp_int_src" "true")
> -   (set_attr "mode" "")])
> +   (set_attr "mode" "")])
>
>  (define_insn "*fop_xf_4_i387"
>[(set (match_operand:XF 0 "register_operand" "=f,f")
> @@ -16457,7 +16457,7 @@ (define_expand "l  {
>rtx tmp = gen_reg_rtx (mode);
>
> -  emit_insn (gen_sse4_1_round2
> +  emit_insn (gen_sse4_1_round2
>  (tmp, operands[1], GEN_INT (ROUND_
>  | ROUND_NO_EXC)));
>emit_insn (gen_fix_trunc2
> Index: gcc/config/i386/sse.md
> ===
> --- gcc/config/i386/sse.md  2019-07-03 20:50:46.078319486 +0100
> +++ gcc/config/i386/sse.md  2019-07-05 15:05:55.220229422 +0100
> @@ -12702,8 +12702,8 @@ (define_insn "*andnot3"
>   (const_string "")))])
>
>  (define_insn "*andnot3_bcst"
> -  [(set (match_operand:VI 0 "register_operand" "=v")
> -   (and:VI
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
> +   (and:VI48_AVX512VL
>   (not:VI48_AVX512VL
>  (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
>   (vec_duplicate:VI48_AVX512VL
> @@ -18085,7 +18085,7 @@ (define_expand "avx512pf_gatherpfs
> operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_gatherpfsf_mask"
> +(define_insn "*avx512pf_gatherpfsf_mask"
>[(unspec
>   [(match_operand: 0 "register_operand" "Yk")
>(match_operator: 5 "vsib_mem_operator"
> @@ -18132,7 +18132,7 @@ (define_expand "avx512pf_gatherpfd
> operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_gatherpfdf_mask"
> +(define_insn "*avx512pf_gatherpfdf_mask"
>[(unspec
>   [(match_operand: 0 "register_operand" "Yk")
>(match_operator:V8DF 5 "vsib_mem_operator"
> @@ -18179,7 +18179,7 @@ (define_expand "avx512pf_scatterpf
> operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_scatterpfsf_mask"
> +(define_insn "*avx512pf_scatterpfsf_mask"
>[(unspec
>   [(match_operand: 0 "register_operand" "Yk")
>(match_operator: 5 "vsib_mem_operator"
> @@ -18228,7 +18228,7 @@ (define_expand "avx512pf_scatterpf
> operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_scatterpfdf_mask"
> +(define_insn "*avx512pf_scatterpfdf_mask"
>[(unspec
>   [(match_operand: 0 "register_operand" "Yk")
>(match_operator:V8DF 5 "vsib_mem_operator"
> @@ -21017,7 +21017,7 @@ (define_expand "avx2_gathersi"
> operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx2_gathersi"
> +(define_insn "*avx2_gathersi"
>[(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
> (unspec:VEC_GATHER_MODE
>   [(match_operand:VEC_GATHER_MODE 2 "register_operand" "0")
> @@ -21037,7 +21037,7 @@ (define_insn "*avx2_gathersi"
> (set_attr "prefix" "vex")
> (set_attr "mode" "")])
>
> -(define_insn "*avx2_gathersi_2"
> +(define_insn "*avx2_gathersi_2"
>[(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
> (unspec:VEC_GATHER_MODE
>   [(pc)
> @@ -21078,7 +21078,7 @@ (define_expand "avx2_gatherdi"
> operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx2_gatherdi"
> +(define_insn "*avx2_gatherdi"
>[(set (match_operand:VEC_GATHER_MODE 0 "registe

Re: [02/11] [arm] Fix ambiguous .md attribute uses

2019-07-05 Thread Kyrill Tkachov

Hi Richard,

On 7/5/19 4:10 PM, Richard Sandiford wrote:

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
 to specify an iterator, and in which  could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for Thumb-1, since previously the
LDREX width was taken from the SImode success result rather than the
memory mode:

-#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREX && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREXBH && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREXBH && 
TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREXD && 
ARM_DOUBLEWORD_ALIGN \

+   && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))

The same goes for the predicate and constraints in
@atomic_compare_and_swapt1di_1, which previously used the
SI values from the success result.



Ok.

Thanks,

Kyrill


2019-07-05  Richard Sandiford 

gcc/
    * config/arm/sync.md
(@atomic_compare_and_swap_1): Use
     instead of (implicitly) .
(@atomic_compare_and_swap_1): Likewise
    .  Use  and
    .

Index: gcc/config/arm/sync.md
===
--- gcc/config/arm/sync.md  2019-07-01 09:37:07.224524452 +0100
+++ gcc/config/arm/sync.md  2019-07-05 15:05:09.088609956 +0100
@@ -201,7 +201,7 @@ (define_insn_and_split "@atomic_compare_
    (match_operand:SI 7 "const_int_operand")] ;; mod_f
   VUNSPEC_ATOMIC_CAS))
    (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  ""
+  ""
   "#"
   "&& reload_completed"
   [(const_int 0)]
@@ -225,14 +225,14 @@ (define_insn_and_split "@atomic_compare_
 (match_operand:SIDI 2 "mem_noofs_operand" 
"+Ua,Ua,Ua,Ua"))  ;; memory

    (set (match_dup 2)
 (unspec_volatile:SIDI
- [(match_operand:SIDI 3 "" 
",lIL*h,J,*r") ;; expect
+ [(match_operand:SIDI 3 "" 
",lIL*h,J,*r") ;; expect
    (match_operand:SIDI 4 "s_register_operand" "r,r,r,r") ;; 
desired
    (match_operand:SI 5 "const_int_operand")  ;; 
is_weak

    (match_operand:SI 6 "const_int_operand")  ;; mod_s
    (match_operand:SI 7 "const_int_operand")] ;; mod_f
   VUNSPEC_ATOMIC_CAS))
    (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  ""
+  ""
   "#"
   "&& reload_completed"
   [(const_int 0)]


Re: [09/11] [s390] Fix ambiguous .md attribute uses

2019-07-05 Thread Andreas Krebbel
On 05.07.19 17:22, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
>  to specify an iterator, and in which  could
> have different values depending on the iterator chosen.
> 
> The vx-builtins.md part changes the choice of  from the
> implicit  to an explicit  (i.e. from the
> mode of the comparison result to the mode of the operands being
> compared).  That seemed like the intended behaviour given later
> patterns like vec_cmpeq_cc.
> 
> The use of BFP in the s390.md LNDFR pattern looks like a typo,
> since the operand to (abs ...) has to have the same mode as the result.
> The only effect before this series was to create some extra variants
> that would never match, making it harmless apart from very minor code
> bloat.
> 
> 
> 2019-07-05  Richard Sandiford  
> 
> gcc/
>   * config/s390/s390.md (*negabs2_nocc): Use FP for
>   operand 1.
>   * config/s390/vx-builtins.md (*vec_cmp_cconly):
>   Make the choice of  explicit, giving...
>   (*vec_cmp_cconly): ...this.

Ok. Thanks for fix!

Andreas



Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

2019-07-05 Thread Segher Boessenkool
Hi Richard,

On Fri, Jul 05, 2019 at 04:20:37PM +0100, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
>  to specify an iterator, and in which  could
> have different values depending on the iterator chosen.

I have sometimes wondered which iterator is chosen in ambiguous cases.
So I finally looked it up, and the doc says
  The @code{@var{iterator}:} prefix may be omitted, in which case the
  substitution will be attempted for every iterator expansion.

Well ugh :-)  I always thought there was some rule, but nope.

Maybe there should be some way of indicating what iterator you want if
none is mentioned?  For the whole pattern, or some global priority scheme
even?  Changes like (random example)

> -(define_insn "*mov_update2"
> +(define_insn "*mov_update2"
>[(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
> (match_operand:P 2 "reg_or_short_operand" "r,I")))
> - (match_operand:SFDF 3 "gpc_reg_operand" ","))
> + (match_operand:SFDF 3 "gpc_reg_operand" ","))
> (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>   (plus:P (match_dup 1) (match_dup 2)))]
>"TARGET_HARD_FLOAT && TARGET_UPDATE
> -   && (!avoiding_indexed_address_p (mode)
> +   && (!avoiding_indexed_address_p (mode)

do not make the code more readable.  A rule like "Do not use P if any
other mode iterator is available" would work for us.


(The patch is fine if the whole series is approved, of course).

Thanks,


Segher


[committed, openmp/openacc] Tweak error message for mapped parameters

2019-07-05 Thread Andrew Stubbs

On 05/07/2019 15:49, Jakub Jelinek wrote:

OK, here is an alternative patch that merely tries to make the error message
more informative.

Basically, the user needs to get past "it isn't working but I need that
value in this kernel", so hopefully this will help get them there.

WDYT?


I don't like it, I'd end the message where you put ;, the rest doesn't make
sense and will just confuse users.  For one, the compiler should know if the
parameter needs to be copied or not, so doesn't need to talk about
probability thereof, and if it needs to be copied, it should be the compiler
that arranges that (PR90779) and the user has no way to overide or force
that behavior.


Here's what Jakub approved via IRC. Now committed.

Thanks Jakub

Andrew
Tweak error message for mapped parameters.

2019-07-05  Andrew Stubbs  

	gcc/fortran/
	* openmp.c (resolve_omp_clauses): Add custom error messages for
	parameters in map clauses.

diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 1c7bce6c300..44fcb9db8c6 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -4208,8 +4208,21 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 		  continue;
 	  }
 	  }
-	gfc_error ("Object %qs is not a variable at %L", n->sym->name,
-		   &n->where);
+	if (list == OMP_LIST_MAP
+	&& n->sym->attr.flavor == FL_PARAMETER)
+	  {
+	if (openacc)
+	  gfc_error ("Object %qs is not a variable at %L; parameters"
+			 " cannot be and need not be copied", n->sym->name,
+			 &n->where);
+	else
+	  gfc_error ("Object %qs is not a variable at %L; parameters"
+			 " cannot be and need not be mapped", n->sym->name,
+			 &n->where);
+	  }
+	else
+	  gfc_error ("Object %qs is not a variable at %L", n->sym->name,
+		 &n->where);
   }
 
   for (list = 0; list < OMP_LIST_NUM; list++)


Re: [PATCH] Fix ODR violations in code using

2019-07-05 Thread Jonathan Wakely

On 21/06/19 18:13 +0100, Jonathan Wakely wrote:

On 21/06/19 18:08 +0100, Jonathan Wakely wrote:

On 21/06/19 13:01 -0400, Nathan Sidwell wrote:

On 6/21/19 12:01 PM, Jonathan Wakely wrote:

Nathan noticed that the 'static inline' functions in 
cause ODR violations when used from inline functions or templates (see
[basic.def.odr] p12 bullet (12.2)). His modules branch now diagnoses
those violations, so we need a fix.

Looking at the history (r114044 by Paolo) I believe the idea was indeed
to allow different definitions to be used in different TUs. I think
__attribute__((__always_inline__)) is a better match for that than
'static inline', and doesn't violate the ODR (at least, not if all TUs
have the same values for __GTHREADS and _GLIBCXX_ATOMIC_BUILTINS).

These functions still violate this rule in [dcl.inline]:

C++17: "If a function with external linkage is declared inline in one
translation unit, it shall be declared inline in all translation units
in which it appears; no diagnostic is required."

C++2a WP: "If a function or variable with external or module linkage
is declared inline in one translation unit, there shall be a reachable
inline declaration in all translation units in which it is declared;
no diagnostic is required."

But that doesn't seem to be diagnosable by today's implementations.

Does this change seem reasonable?


yes, thanks!


Actually, I think I prefer this version. This produces identical code
to the always_inline version, but without the indirection to
additional inline functions, i.e. just inline the relevant code into
the _dispatch functions.

Tests are still running but no failures so far, as expected.


Oops, I spoke too soon:

FAIL: tr1/2_general_utilities/shared_ptr/thread/default_weaktoshared.cc (test 
for excess errors)
UNRESOLVED: tr1/2_general_utilities/shared_ptr/thread/default_weaktoshared.cc 
compilation failed to produce executable
FAIL: tr1/2_general_utilities/shared_ptr/thread/mutex_weaktoshared.cc (test for 
excess errors)
UNRESOLVED: tr1/2_general_utilities/shared_ptr/thread/mutex_weaktoshared.cc 
compilation failed to produce executable

Those tests explicitly use the __atomic_add function that the patch
removes. But those would be easy to adjust.


I decided against the simplification in the second patch, and
committed the attached one which is closer to the first patch I sent
(preserving the __atomic_add and __exchange_and_add functions even
when they just call the built-ins).

Tested x86_64-linux, powerpc64-linux, powerpc-aix. Committed to trunk.

commit 0d26b6506f20dcb1c956338baba0c26f623e25f2
Author: Jonathan Wakely 
Date:   Fri Jul 5 15:15:46 2019 +0100

Fix ODR violations in code using 

Because the inline versions of __exchange_and_add and __atomic_add are
also marked static, they cannot be used from templates or other inline
functions without ODR violations. This change gives them external
linkage, but adds the always_inline attribute.

* include/ext/atomicity.h [_GLIBCXX_ATOMIC_BUILTINS] (__atomic_add)
(__exchange_and_add): Replace static specifier with always_inline
attribute.
(__exchange_and_add_single, __atomic_add_single): Likewise.
(__exchange_and_add_dispatch, __atomic_add_dispatch): Likewise. Also
combine !__gthread_active_p() and !__GTHREADS branches.

diff --git a/libstdc++-v3/include/ext/atomicity.h b/libstdc++-v3/include/ext/atomicity.h
index 0783a58e01a..73225b3de20 100644
--- a/libstdc++-v3/include/ext/atomicity.h
+++ b/libstdc++-v3/include/ext/atomicity.h
@@ -44,24 +44,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // __exchange_and_add_dispatch
   // __atomic_add_dispatch
 #ifdef _GLIBCXX_ATOMIC_BUILTINS
-  static inline _Atomic_word 
+  inline _Atomic_word
+  __attribute__((__always_inline__))
   __exchange_and_add(volatile _Atomic_word* __mem, int __val)
   { return __atomic_fetch_add(__mem, __val, __ATOMIC_ACQ_REL); }
 
-  static inline void
+  inline void
+  __attribute__((__always_inline__))
   __atomic_add(volatile _Atomic_word* __mem, int __val)
   { __atomic_fetch_add(__mem, __val, __ATOMIC_ACQ_REL); }
 #else
   _Atomic_word
-  __attribute__ ((__unused__))
   __exchange_and_add(volatile _Atomic_word*, int) throw ();
 
   void
-  __attribute__ ((__unused__))
   __atomic_add(volatile _Atomic_word*, int) throw ();
 #endif
 
-  static inline _Atomic_word
+  inline _Atomic_word
+  __attribute__((__always_inline__))
   __exchange_and_add_single(_Atomic_word* __mem, int __val)
   {
 _Atomic_word __result = *__mem;
@@ -69,36 +70,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 return __result;
   }
 
-  static inline void
+  inline void
+  __attribute__((__always_inline__))
   __atomic_add_single(_Atomic_word* __mem, int __val)
   { *__mem += __val; }
 
-  static inline _Atomic_word
-  __attribute__ ((__unused__))
+  inline _Atomic_word
+  __attribute__ ((__always_inline__))
   __exchange_and_add_dispatch(_Atomic_word* __mem, int __val)
   {

Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
Segher Boessenkool  writes:
> Hi Richard,
>
> On Fri, Jul 05, 2019 at 04:20:37PM +0100, Richard Sandiford wrote:
>> This patch is part of a series that fixes ambiguous attribute
>> uses in .md files, i.e. cases in which attributes didn't use
>>  to specify an iterator, and in which  could
>> have different values depending on the iterator chosen.
>
> I have sometimes wondered which iterator is chosen in ambiguous cases.
> So I finally looked it up, and the doc says
>   The @code{@var{iterator}:} prefix may be omitted, in which case the
>   substitution will be attempted for every iterator expansion.
>
> Well ugh :-)  I always thought there was some rule, but nope.
>
> Maybe there should be some way of indicating what iterator you want if
> none is mentioned?  For the whole pattern, or some global priority scheme
> even?  Changes like (random example)
>
>> -(define_insn "*mov_update2"
>> +(define_insn "*mov_update2"
>>[(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
>>(match_operand:P 2 "reg_or_short_operand" "r,I")))
>> -(match_operand:SFDF 3 "gpc_reg_operand" ","))
>> +(match_operand:SFDF 3 "gpc_reg_operand" ","))
>> (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>>  (plus:P (match_dup 1) (match_dup 2)))]
>>"TARGET_HARD_FLOAT && TARGET_UPDATE
>> -   && (!avoiding_indexed_address_p (mode)
>> +   && (!avoiding_indexed_address_p (mode)
>
> do not make the code more readable.  A rule like "Do not use P if any
> other mode iterator is available" would work for us.

Yeah, it's a bit ugly, but I think it's better to be explicit even
for P.  E.g. there are pattern names like:

vsx_extract___load

in which using that rule and dropping the iterator names would silently
do something unexpected.  (Not a big deal for "*" patterns of course.)

But I think the bigger problem is that attributes tend to grow over
time, and that something that used to be unambiguous can suddenly
become ambiguous without anyone noticing.  E.g. an attribute A might
start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
pattern.  But then it might be useful to add vector modes to A for some
unrelated pattern.

So even if it the order was selectable, it would still be easy to get
things wrong when you're not thinking about it.  And the series is only
forcing an iterator to be specified if there's genuine ambiguity.
E.g. if Ff was specific to floating-point modes, using  would
still be OK.

> (The patch is fine if the whole series is approved, of course).

Thanks!

Richard

> Thanks,
>
>
> Segher


Re: [04/11] [h8300] Fix ambiguous .md attribute uses

2019-07-05 Thread Jeff Law
On 7/5/19 9:13 AM, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
>  to specify an iterator, and in which  could
> have different values depending on the iterator chosen.
> 
> No behavioural change -- produces the same code as before.
> 
> 
> 2019-07-05  Richard Sandiford  
> 
> gcc/
>   * config/h8300/h8300.md (*push1_h8300hs_): Explicitly
>   specify the mode iterator referenced by , giving...
>   (*push1_h8300hs_): ...this.
OK
jeff


Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

2019-07-05 Thread Segher Boessenkool
On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote:
> Segher Boessenkool  writes:
> > Maybe there should be some way of indicating what iterator you want if
> > none is mentioned?  For the whole pattern, or some global priority scheme
> > even?  Changes like (random example)
> >
> >> -(define_insn "*mov_update2"
> >> +(define_insn "*mov_update2"
> >>[(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
> >>  (match_operand:P 2 "reg_or_short_operand" "r,I")))
> >> -  (match_operand:SFDF 3 "gpc_reg_operand" ","))
> >> +  (match_operand:SFDF 3 "gpc_reg_operand" ","))
> >> (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
> >>(plus:P (match_dup 1) (match_dup 2)))]
> >>"TARGET_HARD_FLOAT && TARGET_UPDATE
> >> -   && (!avoiding_indexed_address_p (mode)
> >> +   && (!avoiding_indexed_address_p (mode)
> >
> > do not make the code more readable.  A rule like "Do not use P if any
> > other mode iterator is available" would work for us.
> 
> Yeah, it's a bit ugly, but I think it's better to be explicit even
> for P.  E.g. there are pattern names like:
> 
> vsx_extract___load
> 
> in which using that rule and dropping the iterator names would silently
> do something unexpected.  (Not a big deal for "*" patterns of course.)

This would be

 vsx_extract___load

in that case.  It *can* still be confused, sure.

> But I think the bigger problem is that attributes tend to grow over
> time, and that something that used to be unambiguous can suddenly
> become ambiguous without anyone noticing.  E.g. an attribute A might
> start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
> pattern.  But then it might be useful to add vector modes to A for some
> unrelated pattern.
> 
> So even if it the order was selectable, it would still be easy to get
> things wrong when you're not thinking about it.  And the series is only
> forcing an iterator to be specified if there's genuine ambiguity.

Yeah.  And there aren't very many places being explicit is needed.  Do
you have some estimate how much that "only disallow if actually ambiguous"
helped?  I expected more changes needed :-)

> E.g. if Ff was specific to floating-point modes, using  would
> still be OK.

Our Ff unfortunately is not for FP modes only, but for modes that can go
in FP registers :-/


Segher


[PATCH, RISC-V] Fix ambiguous mode of some compare insn

2019-07-05 Thread Katsuhiro Suzuki

Hello,

This patch fixes ambiguous mode of some compare insns of RISC-V.
Only sge, slt and sle are using  but other compare insns use
. It seems first group mode settings are ambiguous.

Best Regards,
Katsuhiro Suzuki
Index: gcc/config/riscv/riscv.md
===
--- gcc/config/riscv/riscv.md	(revision 272852)
+++ gcc/config/riscv/riscv.md	(working copy)
@@ -2054,7 +2054,7 @@
   ""
   "slt%i2\t%0,zero,%1"
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 (define_insn "*slt_"
   [(set (match_operand:GPR   0 "register_operand" "= r")
@@ -2063,7 +2063,7 @@
   ""
   "slt%i2\t%0,%1,%2"
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 (define_insn "*sle_"
   [(set (match_operand:GPR   0 "register_operand" "=r")
@@ -2075,7 +2075,7 @@
   return "slt%i2\t%0,%1,%2";
 }
   [(set_attr "type" "slt")
-   (set_attr "mode" "")])
+   (set_attr "mode" "")])
 
 ;;
 ;;  


Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

2019-07-05 Thread Richard Sandiford
Segher Boessenkool  writes:
> On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool  writes:
>> > Maybe there should be some way of indicating what iterator you want if
>> > none is mentioned?  For the whole pattern, or some global priority scheme
>> > even?  Changes like (random example)
>> >
>> >> -(define_insn "*mov_update2"
>> >> +(define_insn "*mov_update2"
>> >>[(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
>> >> (match_operand:P 2 "reg_or_short_operand" "r,I")))
>> >> - (match_operand:SFDF 3 "gpc_reg_operand" ","))
>> >> + (match_operand:SFDF 3 "gpc_reg_operand" ","))
>> >> (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>> >>   (plus:P (match_dup 1) (match_dup 2)))]
>> >>"TARGET_HARD_FLOAT && TARGET_UPDATE
>> >> -   && (!avoiding_indexed_address_p (mode)
>> >> +   && (!avoiding_indexed_address_p (mode)
>> >
>> > do not make the code more readable.  A rule like "Do not use P if any
>> > other mode iterator is available" would work for us.
>> 
>> Yeah, it's a bit ugly, but I think it's better to be explicit even
>> for P.  E.g. there are pattern names like:
>> 
>> vsx_extract___load
>> 
>> in which using that rule and dropping the iterator names would silently
>> do something unexpected.  (Not a big deal for "*" patterns of course.)
>
> This would be
>
>  vsx_extract___load
>
> in that case.  It *can* still be confused, sure.
>
>> But I think the bigger problem is that attributes tend to grow over
>> time, and that something that used to be unambiguous can suddenly
>> become ambiguous without anyone noticing.  E.g. an attribute A might
>> start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
>> pattern.  But then it might be useful to add vector modes to A for some
>> unrelated pattern.
>> 
>> So even if it the order was selectable, it would still be easy to get
>> things wrong when you're not thinking about it.  And the series is only
>> forcing an iterator to be specified if there's genuine ambiguity.
>
> Yeah.  And there aren't very many places being explicit is needed.  Do
> you have some estimate how much that "only disallow if actually ambiguous"
> helped?  I expected more changes needed :-)

No real estimate, but I imagine loads :-)  Having two mode iterators
is pretty common, and forcing a qualifier even when there's no
ambiguity (e.g. using vector queries in a vector x P pattern)
would probably be over the top.

And yeah, I was pleasantly surprised how few places needed changing.
The bug-fix to make-work ratio seemed pretty high in the end (but not
for rs6000).

Richard

>
>> E.g. if Ff was specific to floating-point modes, using  would
>> still be OK.
>
> Our Ff unfortunately is not for FP modes only, but for modes that can go
> in FP registers :-/
>
>
> Segher


Re: [patch] Small improvements to coverage info (1/n)

2019-07-05 Thread Eric Botcazou
> This change broke gomp/pr88107.c test:
> FAIL: gcc.dg/gomp/pr88107.c (internal compiler error)
> FAIL: gcc.dg/gomp/pr88107.c (test for excess errors)
> Excess errors:
> during GIMPLE pass: graphite
> /usr/src/gcc/gcc/testsuite/gcc.dg/gomp/pr88107.c:26:1: internal compiler
> error: Segmentation fault 0x11942a4 crash_signal
> ../../gcc/toplev.c:326
> 0x13b5861 gimple_location
> ../../gcc/gimple.h:1805
> 0x13b76f8 create_iv(tree_node*, tree_node*, tree_node*, loop*,
> gimple_stmt_iterator*, bool, tree_node**, tree_node**)
> ../../gcc/tree-ssa-loop-manip.c:142
> 0xaa2c95 create_empty_loop_on_edge(edge_def*, tree_node*, tree_node*,
> tree_node*, tree_node*, tree_node**, tree_node**, loop*)
> ../../gcc/cfgloopmanip.c:831
> 
> Apparently gsi_end_p (*incr_pos) is true and after is false, so
> gsi_stmt (*incr_pos) is NULL.  One needs graphite enabled.

OK, thanks for the heads up.  I have installed the attached fixlet for now.


* tree-ssa-loop-manip.c (create_iv): Add missing guard for gsi_end_p.

-- 
Eric BotcazouIndex: tree-ssa-loop-manip.c
===
--- tree-ssa-loop-manip.c	(revision 273133)
+++ tree-ssa-loop-manip.c	(working copy)
@@ -139,7 +139,8 @@ create_iv (tree base, tree step, tree va
 }
   else
 {
-  gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos)));
+  if (!gsi_end_p (*incr_pos))
+	gimple_set_location (stmt, gimple_location (gsi_stmt (*incr_pos)));
   gsi_insert_before (incr_pos, stmt, GSI_NEW_STMT);
 }
 


Re: [PATCH] Fix ODR violations in code using

2019-07-05 Thread Daniel Krügler
Am Fr., 5. Juli 2019 um 18:13 Uhr schrieb Jonathan Wakely :
>
[..]
> I decided against the simplification in the second patch, and
> committed the attached one which is closer to the first patch I sent
> (preserving the __atomic_add and __exchange_and_add functions even
> when they just call the built-ins).
>
> Tested x86_64-linux, powerpc64-linux, powerpc-aix. Committed to trunk.

Unrelated to the actual patch, I noticed some explicit "throw()" forms
used as exception specifications - shouldn't these be replaced by
either explicit "noexcept" or at least by a library macro that expands
to one or the other? (I'm sorry, if such unrelated questions are
considered as inappropriate for this list).

- Daniel


Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC

2019-07-05 Thread Nix
On 5 Jul 2019, Richard Biener said:

> On Fri, Jul 5, 2019 at 12:21 AM Indu Bhagat  wrote:
>> CTF, at this time, is type information for entities at global or file scope.
>> This can be used by online debuggers, program tracers (dynamic tracing); More
>> generally, it provides type introspection for C programs, with an optional
>> library API to allow them to get at their own types quite more easily than
>> DWARF. So, the umbrella usecases are - all C programs that want to introspect
>> their own types quickly; and applications that want to introspect other
>> programs's types quickly.
>
> What makes it superior to DWARF stripped down to the above feature set?

Increased compactness. DWARF fundamentally trades off compactness in
favour of its regular structure, which makes it easier to parse (but not
easier to interpret) but very hard to make it much smaller than it is
now. Where DWARF uses word-sized and larger entities for everything, CTF
packs everything much more tightly -- and this is quite visible in the
resulting file sizes, once deduplicated. (CTF for the entire Linux
kernel is about 6MiB after gzipping, and that includes not only complete
descriptions of its tens of thousands of types but also type and string
table entries for every structure and union member name, every
enumeration member, and every global variable. More conventional
programs will be able to eschew spending space on some of these because
the ELF string table already contains their names, and we reuse those
where possible. Insofar as it is possible to tell, the DWARF type info
for the entire kernel, even after deduplication, would be many times
larger: it is certainly much larger as it comes out of the compiler. You
could define a "restricted DWARF" with smaller tags etc that is smaller,
but frankly that would no longer be DWARF at all.)

(I'm using the kernel as an example a lot not because CTF is
kernel-specific but because our *existing deduplicator* happens to be
targetted at the kernel. This is already an annoying limitation: we want
to be able to use CTF in userspace more easily and more widely, without
kludges and without incurring huge costs to generate gigabytes of DWARF
we otherwise aren't using: hence this project.)

When programs try to consume DWARF from large programs the size of the
kernel, even with indexes I observe a multi-second lag and significant
memory usage: no program I have tried has increased its RSS by less than
100MiB. CTF consumers can suck in the CTF for the core kernel in well
under a third of a second, and can traverse the CTF for the kernel and
all modules (multiple CTF sections in an archive, sharing common types
wiht a parent section) in about a second and a half (from a cold cache):
RSS goes up by about 15MiB. If DWARF usage can impose a burden that low
on consumers, it's the first I've ever heard of it.

>> (Even with the exception of its embedded string table, it is already small
>>  enough to  be kept around in stripped binaries so that it can be relied upon
>>  to be present.)
>
> So for distributing a program/library for SUSE we usually split the
> distribution into two pieces - the binaries and separated debug information.
> With CTF we'd then create both, continue stripping out the DWARF information
> but keep the CTF in the binaries?
>
> When a program contains CTF only, can gdb do anything to help debugging
> of a running program or a core file?  Do you have gdb support in the works?

Yes, and it works well enough already to extract types from programs
(going all the way from symbols to types requires some code on the GCC
and linker side that is being written right now, and we can't test the
GDB code that relies on that until then: equally, I'm still working on
the linker so this is a demo on a randomly-chosen object file. This also
means you don't see any benefits from strtab reuse with the containing
ELF object, CTF section compression or deduplication in the following
example's .ctf section size):

[nix@ca-tools3 libiberty]$ /home/ibhagat/GCC/install/gcc-ctf/bin/gcc -c 
-DHAVE_CONFIG_H -gt -O2  -I. -I../../libiberty/../include  -W -Wall 
-Wwrite-strings -Wc++-compat -Wstrict-prototypes -Wshadow=local -pedantic  
-D_GNU_SOURCE ../../libiberty/hashtab.c -o hashtab.o

[nix@ca-tools3 libiberty]$ size -A hashtab.o
hashtab.o  :
sectionsize   addr
.text  4112  0
.data16  0
.bss  0  0
.ctf  11907  0
.rodata.str1.8   40  0
.rodata.cst8  8  0
.rodata 480  0
.comment 43  0
.note.GNU-stack   0  0
Total 16606

[nix@ca-tools3 libiberty]$ ../gdb/gdb hashtab.o 
GNU gdb (GDB) 8.2.50.20190214-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "s

[Darwin, PPC, committed] Remove dead code.

2019-07-05 Thread Iain Sandoe
TARGET_LINK_STACK is unused on Darwin, and only relevant to a processor on
which the port was never released.

tested on powerpc-darwin9, 
applied to mainline
thanks
Iain

2019-07-05  Iain Sandoe  

* config/rs6000/rs6000-logue.c: Remove unused code.


diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
index 3fe6230..8454f96 100644
--- a/gcc/config/rs6000/rs6000-logue.c
+++ b/gcc/config/rs6000/rs6000-logue.c
@@ -5100,24 +5100,11 @@ macho_branch_islands (void)
 #endif /* DBX_DEBUGGING_INFO || XCOFF_DEBUGGING_INFO */
   if (flag_pic)
{
- if (TARGET_LINK_STACK)
-   {
- char name[32];
- get_ppc476_thunk_name (name);
- strcat (tmp_buf, ":\n\tmflr r0\n\tbl ");
- strcat (tmp_buf, name);
- strcat (tmp_buf, "\n");
- strcat (tmp_buf, label);
- strcat (tmp_buf, "_pic:\n\tmflr r11\n");
-   }
- else
-   {
- strcat (tmp_buf, ":\n\tmflr r0\n\tbcl 20,31,");
- strcat (tmp_buf, label);
- strcat (tmp_buf, "_pic\n");
- strcat (tmp_buf, label);
- strcat (tmp_buf, "_pic:\n\tmflr r11\n");
-   }
+ strcat (tmp_buf, ":\n\tmflr r0\n\tbcl 20,31,");
+ strcat (tmp_buf, label);
+ strcat (tmp_buf, "_pic\n");
+ strcat (tmp_buf, label);
+ strcat (tmp_buf, "_pic:\n\tmflr r11\n");
 
  strcat (tmp_buf, "\taddis r11,r11,ha16(");
  strcat (tmp_buf, name_buf);



C++ Modules

2019-07-05 Thread Nathan Sidwell

Hi all,
I have achieved a major milestone in adding C++ Modules support to GCC. 
Namely the iostream header can be built and used as a header unit.
#including  includes an awful lot of the STL, so this has 
pushed a large swathe of C++ through the module machinery.


Because 'hello world' now works, it's good for people to play with. 
some assembly is required, not least because it is not on trunk.


Beware, this is lots of new code, it is full of FIXMEs and it will break 
on your code.  I'll be working on cleaning it up next.


Refer to https://gcc.gnu.org/wiki/cxx-modules for details.

Testcase g++.dg/modules/iostream-1_{a.H,b.C} is the iostream example, FWIW.

nathan

--
Nathan Sidwell


Re: [PATCH] Fix ODR violations in code using

2019-07-05 Thread Jonathan Wakely

On 05/07/19 20:23 +0200, Daniel Krügler wrote:

Am Fr., 5. Juli 2019 um 18:13 Uhr schrieb Jonathan Wakely :



[..]

I decided against the simplification in the second patch, and
committed the attached one which is closer to the first patch I sent
(preserving the __atomic_add and __exchange_and_add functions even
when they just call the built-ins).

Tested x86_64-linux, powerpc64-linux, powerpc-aix. Committed to trunk.


Unrelated to the actual patch, I noticed some explicit "throw()" forms
used as exception specifications - shouldn't these be replaced by
either explicit "noexcept" or at least by a library macro that expands
to one or the other?


Yes, they should be _GLIBCXX_NOTHROW.


(I'm sorry, if such unrelated questions are
considered as inappropriate for this list).


Entirely appropriate, thanks!



Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC

2019-07-05 Thread Jakub Jelinek
On Fri, Jul 05, 2019 at 07:28:12PM +0100, Nix wrote:
> > What makes it superior to DWARF stripped down to the above feature set?
> 
> Increased compactness. DWARF fundamentally trades off compactness in
> favour of its regular structure, which makes it easier to parse (but not
> easier to interpret) but very hard to make it much smaller than it is
> now. Where DWARF uses word-sized and larger entities for everything, CTF
> packs everything much more tightly -- and this is quite visible in the

That is just not true, most of the data in DWARF are uleb128/sleb128
encoded, or often even present just in the abbreviation table
(DW_FORM_flag_present, DW_FORM_implicit_const), word-sized is typically only
stuff that needs relocations (at assembly time and more importantly at link
time).

> could define a "restricted DWARF" with smaller tags etc that is smaller,
> but frankly that would no longer be DWARF at all.)

You can certainly decide what kind of information you emit and what you
don't, it is done already (look at -g1 vs. -g2 vs. -g3), and can be done for
other stuff, say if you aren't interested in any locations,
DW_AT_{decl,call}_{file,line,column} can be omitted, if you aren't
interested in debug info within functions, a lot of debug info can be
emitted etc.  And it still will be DWARF.
For DWARF you also have various techniques at reducing size and optimizing
redundancies away, look e.g. at the dwz utility.

Jakub


[C++ Patch PING] Re: [C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl

2019-07-05 Thread Paolo Carlini

Hi,

On 23/06/19 13:58, Paolo Carlini wrote:

Hi,

here there are a couple of rather straightforward improvements in the 
second half of grokdeclarator plus a check_tag_decl change consistent 
with the other existing case of multiple_types_p diagnostic. Tested 
x86_64-linux.


Gently pinging this...

    https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01391.html

Thanks, Paolo.



Re: C++ Modules

2019-07-05 Thread Jason Merrill
Awesome!

On Fri, Jul 5, 2019, 2:30 PM Nathan Sidwell  wrote:

> Hi all,
> I have achieved a major milestone in adding C++ Modules support to GCC.
> Namely the iostream header can be built and used as a header unit.
> #including  includes an awful lot of the STL, so this has
> pushed a large swathe of C++ through the module machinery.
>
> Because 'hello world' now works, it's good for people to play with.
> some assembly is required, not least because it is not on trunk.
>
> Beware, this is lots of new code, it is full of FIXMEs and it will break
> on your code.  I'll be working on cleaning it up next.
>
> Refer to https://gcc.gnu.org/wiki/cxx-modules for details.
>
> Testcase g++.dg/modules/iostream-1_{a.H,b.C} is the iostream example, FWIW.
>
> nathan
>
> --
> Nathan Sidwell
>


Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")

2019-07-05 Thread Jakub Jelinek
On Thu, Jul 04, 2019 at 02:56:47PM +0200, Paolo Carlini wrote:
> --- cp/call.c (revision 273076)
> +++ cp/call.c (working copy)
> @@ -9845,17 +9852,6 @@ build_new_method_call_1 (tree instance, tree fns,
>  
> if (call != error_mark_node)
>   {
> -   /* Optimize away vtable lookup if we know that this
> -  function can't be overridden.  We need to check if
> -  the context and the type where we found fn are the same,
> -  actually FN might be defined in a different class
> -  type because of a using-declaration. In this case, we
> -  do not want to perform a non-virtual call.  */
> -   if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
> -   && same_type_ignoring_top_level_qualifiers_p
> -   (DECL_CONTEXT (fn), BINFO_TYPE (binfo))
> -   && resolves_to_fixed_type_p (instance, 0))
> - flags |= LOOKUP_NONVIRTUAL;
>if (explicit_targs)
>  flags |= LOOKUP_EXPLICIT_TMPL_ARGS;
> /* Now we know what function is being called.  */

This change broke bootstrap, as it removes the last use of binfo
variable besides the setter of that variable.

I'll commit following as obvious if I get successfully past that point in
bootstrap:

2019-07-05  Jakub Jelinek  

PR c++/67184
PR c++/69445
* call.c (build_new_method_call_1): Remove set but not used variable
binfo.

--- gcc/call.c.jj   2019-07-05 22:09:49.694367815 +0200
+++ gcc/call.c  2019-07-05 22:25:58.476016114 +0200
@@ -9564,7 +9564,7 @@ build_new_method_call_1 (tree instance,
   struct z_candidate *candidates = 0, *cand;
   tree explicit_targs = NULL_TREE;
   tree basetype = NULL_TREE;
-  tree access_binfo, binfo;
+  tree access_binfo;
   tree optype;
   tree first_mem_arg = NULL_TREE;
   tree name;
@@ -9603,7 +9603,6 @@ build_new_method_call_1 (tree instance,
   if (!conversion_path)
 conversion_path = BASELINK_BINFO (fns);
   access_binfo = BASELINK_ACCESS_BINFO (fns);
-  binfo = BASELINK_BINFO (fns);
   optype = BASELINK_OPTYPE (fns);
   fns = BASELINK_FUNCTIONS (fns);
   if (TREE_CODE (fns) == TEMPLATE_ID_EXPR)


Jakub


Re: [PATCH,RFC] collect2 LTO for AIX

2019-07-05 Thread David Edelsohn
On Thu, Jul 4, 2019 at 11:43 AM Martin Liška  wrote:
>
> On 7/4/19 5:03 PM, David Edelsohn wrote:
> > On Thu, Jul 4, 2019 at 10:38 AM Martin Liška  wrote:
> >>
> >> Hi.
> >>
> >> Recently I've introduced a new .gnu.lto_.lto section that
> >> is supposed to provide meta information about a LTO bytecode.
> >>
> >> As a further step, I'm planning to teach binutils about
> >> existence of the section and I'll remove in the future
> >> emission of __gnu_lto_slim and __gnu_lto_v1 symbols.
> >> The former one is used by binutils to identify if
> >> an object is a slim LTO object. The later one is currently
> >> used only in gcc/collect2.c and was added by David's patch.
> >>
> >> My question is: Can we remove __gnu_lto_v1 right now and
> >> XCOFF will use something similar to ELF (has_lto_section)?
> >> Can you David help me with that please?
> >
> > LTO currently does not work on AIX. I added the __gnu_lto_v1 as a
> > test. You can rip it out and XCOFF will follow a different path when
> > implementing LTO.
>
> Great. Then I'm sending revert of the patch.
>
> Ready to be installed?

Okay.

Thanks, David


Re: [PATCH 1/3] C++20 constexpr lib part 1/3

2019-07-05 Thread Ed Smith-Rowland via gcc-patches

On 7/2/19 8:11 AM, Jonathan Wakely wrote:

One more comment. In  this:

+#if __cplusplus > 201703L \
+?? && defined(_GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED)
+?? if (__builtin_is_constant_evaluated())

can be simplified to just:

#ifdef __cpp_lib_is_constant_evaluated
 if (std::is_constant_evaluated())

The feature test macro is exactly the check we want here


This is done and the test cases for the non-modifying algorithms are done.

I'm was stumped on the modifying algos though.?? Consider std::copy:

-

constexpr bool
test()
{
?? constexpr std::array ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 
11}};

?? std::array ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};

?? constexpr auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, 
ma0.begin() + 2);


?? return out6 == ma0.begin() + 10;
}

static_assert(test());
-

This is essentially the same as the Boost test case referenced in p0202.

I've also taken the arrays out as globals with the same result either way:

-

ed@bad-horse:~/cxx_constexpr_lib$ ../bin_constexpr_lib/bin/g++ 
-std=gnu++2a -c testsuite/25_algorithms/copy/constexpr.cc
testsuite/25_algorithms/copy/constexpr.cc: In function ???constexpr bool 
test()???:
testsuite/25_algorithms/copy/constexpr.cc:36:34: in ???constexpr??? 
expansion of ???std::copy(ca0.std::array12>::begin(), (ca0.std::array::begin() + 32), 
(ma0.std::array::begin() + 8))???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:535:7: 
in ???constexpr??? expansion of ???std::__copy_move_a2int*>(std::__miter_base(__first), std::__miter_baseint*>(__last), __result)???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:501:30: 
in ???constexpr??? expansion of ???std::__copy_move_aint*>(std::__niter_base(__first), std::__niter_baseint*>(__last), std::__niter_base(__result))???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:463:30: 
in ???constexpr??? expansion of ???std::__copy_movestd::random_access_iterator_tag>::__copy_m(__first, __last, __result)???
/home/ed/bin_constexpr_lib/include/c++/10.0.0/bits/stl_algobase.h:445:24: 
in ???constexpr??? expansion of ???std::__memmove(__result, 
__first, ((std::size_t)((std::ptrdiff_t)_Num)))???
testsuite/25_algorithms/copy/constexpr.cc:36:80: error: modification of 
???ma0??? is not a constant expression
 36 | constexpr auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, 
ma0.begin() + 2);

| ^
testsuite/25_algorithms/copy/constexpr.cc: At global scope:
testsuite/25_algorithms/copy/constexpr.cc:41:19: error: non-constant 
condition for static assertion

 41 | static_assert(test());
?? | ^~

-

By my reckoning, you have a constexpr source array, an output array that 
is initialized as it must be for constexpr.?? You have to have a 
deterministic result after the copy.?? In the local array version the 
actual iterator is not leaking out - just the results of a calculation 
that must return one result.


The only thing that 'helps' is removing the constexpr from the iterator 
return and of course that's the whole point of the thing.


Blargh!

So, you can't modify a constexpr sequence. No actual surprise.?? The 
returned iterators into non-constexpr sequences can never be literals.?? 
What you *can* do is make the modifying algorithms so you can make pure 
functions with them.?? In this connection my previous testcases for 
non-modifying?? were correct, just not complete because there the 
returned iterators into constexpr sequences can be (must be) literals.


So here is a new patch for chunk 1 of constexpr library.

Tested with default settings and with -std=gnu++2a on x86_64-linux.

OK?

Ed


2019-07-08  Edward Smith-Rowland  <3dw...@verizon.net>

Implement C++20 p0202 - Add Constexpr Modifiers to Functions
in  and  Headers.
Implement C++20 p1023 - constexpr comparison operators for std::array.
* include/bits/algorithmfwd.h (all_of, any_of, binary_search, copy,
copy_backward, copy_if, copy_n, equal_range, fill, find_end,
find_if_not, includes, is_heap, is_heap_until, is_partitioned,
is_permutation, is_sorted, is_sorted_until, iter_swap, lower_bound,
none_of, partition_copy, partition_point, remove, remove_if,
remove_copy, remove_copy_if, replace_copy, replace_copy_if,
reverse_copy, rotate_copy, uunique, upper_bound, adjacent_find, count,
count_if, equal, find, find_first_of, find_if, for_each, generate,
generate_n, lexicographical_compare, merge, mismatch, replace,
replace_if, search, search_n, set_difference, set_intersection,
set_symmetric_difference, set_union, transform, unique_copy):
Mark constexpr.
* include/bits/cpp_type_traits.h (__miter_base): Mark constexpr.
* include/bits/predefined_ops.h