* [Ada] Remove propagation of atomicity from object to type
@ 2015-05-27 14:00 Arnaud Charlet
2015-05-27 18:04 ` Eric Botcazou
0 siblings, 1 reply; 2+ messages in thread
From: Arnaud Charlet @ 2015-05-27 14:00 UTC (permalink / raw)
To: gcc-patches; +Cc: Eric Botcazou
[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]
This change removes an old trick which was propagating the Atomic (and now
Volatile_Full_Access) setting from an object to a locally-defined type, in
order to coax gigi into accepting more atomic objects.
This trick is now obsolete since gigi should be able to rewrite the type of
the objects to meet the atomicity requirements on its own.
The change also rewrites Is_Atomic_VFA_Aggregate to check for the presence
of the flag on the object as well, which was missing but largely mitigated
by the aforementioned trick.
No functional changes.
Tested on x86_64-pc-linux-gnu, committed on trunk
2015-05-26 Eric Botcazou <ebotcazou@adacore.com>
* freeze.ads (Is_Atomic_VFA_Aggregate): Adjust profile.
* freeze.adb (Is_Atomic_VFA_Aggregate): Change Entity
parameter into Node parameter and remove Type parameter.
Look at Is_Atomic_Or_VFA both on the type and on the object.
(Freeze_Entity): Adjust call to Is_Atomic_VFA_Aggregate.
* exp_aggr.adb (Expand_Record_Aggregate): Likewise.
(Process_Atomic_Independent_Shared_Volatile): Remove code
propagating Atomic or VFA from object to locally-defined type.
[-- Attachment #2: difs --]
[-- Type: text/plain, Size: 7139 bytes --]
Index: sem_prag.adb
===================================================================
--- sem_prag.adb (revision 223750)
+++ sem_prag.adb (working copy)
@@ -5875,7 +5875,6 @@
E : Entity_Id;
E_Id : Node_Id;
K : Node_Kind;
- Utyp : Entity_Id;
procedure Set_Atomic_VFA (E : Entity_Id);
-- Set given type as Is_Atomic or Is_Volatile_Full_Access. Also, if
@@ -6053,46 +6052,6 @@
then
Set_Has_Delayed_Freeze (E);
end if;
-
- -- An interesting improvement here. If an object of composite
- -- type X is declared atomic, and the type X isn't, that's a
- -- pity, since it may not have appropriate alignment etc. We
- -- can rescue this in the special case where the object and
- -- type are in the same unit by just setting the type as
- -- atomic, so that the back end will process it as atomic.
-
- -- Note: we used to do this for elementary types as well,
- -- but that turns out to be a bad idea and can have unwanted
- -- effects, most notably if the type is elementary, the object
- -- a simple component within a record, and both are in a spec:
- -- every object of this type in the entire program will be
- -- treated as atomic, thus incurring a potentially costly
- -- synchronization operation for every access.
-
- -- For Volatile_Full_Access we can do this for elementary types
- -- too, since there is no issue of atomic synchronization.
-
- -- Of course it would be best if the back end could just adjust
- -- the alignment etc for the specific object, but that's not
- -- something we are capable of doing at this point.
-
- Utyp := Underlying_Type (Etype (E));
-
- if Present (Utyp)
- and then (Is_Composite_Type (Utyp)
- or else Prag_Id = Pragma_Volatile_Full_Access)
- and then Sloc (E) > No_Location
- and then Sloc (Utyp) > No_Location
- and then
- Get_Source_File_Index (Sloc (E)) =
- Get_Source_File_Index (Sloc (Utyp))
- then
- if Prag_Id = Pragma_Volatile_Full_Access then
- Set_Is_Volatile_Full_Access (Utyp);
- else
- Set_Is_Atomic (Utyp);
- end if;
- end if;
end if;
-- Atomic/Shared/Volatile_Full_Access imply Independent
Index: freeze.adb
===================================================================
--- freeze.adb (revision 223750)
+++ freeze.adb (working copy)
@@ -1459,17 +1459,15 @@
-- Is_Atomic_VFA_Aggregate --
-----------------------------
- function Is_Atomic_VFA_Aggregate
- (E : Entity_Id;
- Typ : Entity_Id) return Boolean
- is
- Loc : constant Source_Ptr := Sloc (E);
+ function Is_Atomic_VFA_Aggregate (N : Node_Id) return Boolean is
+ Loc : constant Source_Ptr := Sloc (N);
New_N : Node_Id;
Par : Node_Id;
Temp : Entity_Id;
+ Typ : Entity_Id;
begin
- Par := Parent (E);
+ Par := Parent (N);
-- Array may be qualified, so find outer context
@@ -1477,24 +1475,45 @@
Par := Parent (Par);
end if;
- if Nkind_In (Par, N_Object_Declaration, N_Assignment_Statement)
- and then Comes_From_Source (Par)
- then
- Temp := Make_Temporary (Loc, 'T', E);
- New_N :=
- Make_Object_Declaration (Loc,
- Defining_Identifier => Temp,
- Object_Definition => New_Occurrence_Of (Typ, Loc),
- Expression => Relocate_Node (E));
- Insert_Before (Par, New_N);
- Analyze (New_N);
+ if not Comes_From_Source (Par) then
+ return False;
+ end if;
- Set_Expression (Par, New_Occurrence_Of (Temp, Loc));
- return True;
+ case Nkind (Par) is
+ when N_Assignment_Statement =>
+ Typ := Etype (Name (Par));
- else
- return False;
- end if;
+ if not Is_Atomic_Or_VFA (Typ)
+ and then not (Is_Entity_Name (Name (Par))
+ and then Is_Atomic_Or_VFA (Entity (Name (Par))))
+ then
+ return False;
+ end if;
+
+ when N_Object_Declaration =>
+ Typ := Etype (Defining_Identifier (Par));
+
+ if not Is_Atomic_Or_VFA (Typ)
+ and then not Is_Atomic_Or_VFA (Defining_Identifier (Par))
+ then
+ return False;
+ end if;
+
+ when others =>
+ return False;
+ end case;
+
+ Temp := Make_Temporary (Loc, 'T', N);
+ New_N :=
+ Make_Object_Declaration (Loc,
+ Defining_Identifier => Temp,
+ Object_Definition => New_Occurrence_Of (Typ, Loc),
+ Expression => Relocate_Node (N));
+ Insert_Before (Par, New_N);
+ Analyze (New_N);
+
+ Set_Expression (Par, New_Occurrence_Of (Temp, Loc));
+ return True;
end Is_Atomic_VFA_Aggregate;
-----------------------------------------------
@@ -4821,8 +4840,7 @@
and then Nkind (Parent (E)) = N_Object_Declaration
and then Present (Expression (Parent (E)))
and then Nkind (Expression (Parent (E))) = N_Aggregate
- and then
- Is_Atomic_VFA_Aggregate (Expression (Parent (E)), Etype (E))
+ and then Is_Atomic_VFA_Aggregate (Expression (Parent (E)))
then
null;
end if;
Index: freeze.ads
===================================================================
--- freeze.ads (revision 223750)
+++ freeze.ads (working copy)
@@ -174,9 +174,7 @@
-- do not allow a size clause if the size would not otherwise be known at
-- compile time in any case.
- function Is_Atomic_VFA_Aggregate
- (E : Entity_Id;
- Typ : Entity_Id) return Boolean;
+ function Is_Atomic_VFA_Aggregate (N : Node_Id) return Boolean;
-- If an atomic/VFA object is initialized with an aggregate or is assigned
-- an aggregate, we have to prevent a piecemeal access or assignment to the
-- object, even if the aggregate is to be expanded. We create a temporary
Index: exp_aggr.adb
===================================================================
--- exp_aggr.adb (revision 223750)
+++ exp_aggr.adb (working copy)
@@ -5950,10 +5950,7 @@
-- temporary instead, so that the back end can generate an atomic move
-- for it.
- if Is_Atomic_Or_VFA (Typ)
- and then Comes_From_Source (Parent (N))
- and then Is_Atomic_VFA_Aggregate (N, Typ)
- then
+ if Is_Atomic_VFA_Aggregate (N) then
return;
-- No special management required for aggregates used to initialize
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Ada] Remove propagation of atomicity from object to type
2015-05-27 14:00 [Ada] Remove propagation of atomicity from object to type Arnaud Charlet
@ 2015-05-27 18:04 ` Eric Botcazou
0 siblings, 0 replies; 2+ messages in thread
From: Eric Botcazou @ 2015-05-27 18:04 UTC (permalink / raw)
To: gcc-patches; +Cc: Arnaud Charlet
[-- Attachment #1: Type: text/plain, Size: 389 bytes --]
> This trick is now obsolete since gigi should be able to rewrite the type of
> the objects to meet the atomicity requirements on its own.
It just needs the following patchlet, applied on the mainline.
2015-05-27 Eric Botcazou <ebotcazou@adacore.com>
* gcc-interface/decl.c (gnat_to_gnu_entity) <E_Variable>: Move down
code applying atomic checks to the object.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1442 bytes --]
Index: gcc-interface/decl.c
===================================================================
--- gcc-interface/decl.c (revision 223766)
+++ gcc-interface/decl.c (working copy)
@@ -881,15 +881,6 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
check_ok_for_atomic_type (gnu_inner, gnat_entity, true);
}
- /* Now check if the type of the object allows atomic access. Note
- that we must test the type, even if this object has size and
- alignment to allow such access, because we will be going inside
- the padded record to assign to the object. We could fix this by
- always copying via an intermediate value, but it's not clear it's
- worth the effort. */
- if (Is_Atomic_Or_VFA (gnat_entity))
- check_ok_for_atomic_type (gnu_type, gnat_entity, false);
-
/* If this is an aliased object with an unconstrained nominal subtype,
make a type that includes the template. */
if (Is_Constr_Subt_For_UN_Aliased (Etype (gnat_entity))
@@ -955,6 +946,10 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
debug_info_p, gnat_entity);
}
+ /* Now check if the type of the object allows atomic access. */
+ if (Is_Atomic_Or_VFA (gnat_entity))
+ check_ok_for_atomic_type (gnu_type, gnat_entity, false);
+
/* If this is a renaming, avoid as much as possible to create a new
object. However, in some cases, creating it is required because
renaming can be applied to objects that are not names in Ada.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-05-27 18:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-27 14:00 [Ada] Remove propagation of atomicity from object to type Arnaud Charlet
2015-05-27 18:04 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).