diff mbox

[Ada] Improve array packing for small record component

Message ID 91464303.MTIrPuzqCP@polaris
State New
Headers show

Commit Message

Eric Botcazou May 16, 2016, 10:14 a.m. UTC
This change ensures that the packing of array types subject to pragma Pack
and whose component type is a record with a size in the range 33 .. 63 bits
is optimal, in the sense that the 'Component_Size is equal to the 'Size of
the component type.

The following package P must yield the specified output with -gnatR1:

package P is

  type R is record
    I : Integer;
    B : Boolean;
  end record;
  pragma Pack (R);

  type A1 is array (1 .. 8) of R;
  pragma Pack (A1);

  type A2 is array (1 .. 8) of R;
  for A2'Component_Size use 33;

end P;

Representation information for unit P (spec)
--------------------------------------------

for R'Object_Size use 40;
for R'Value_Size use 33;
for R'Alignment use 1;
for R use record
   I at 0 range  0 .. 31;
   B at 4 range  0 ..  0;
end record;

for A1'Size use 264;
for A1'Alignment use 1;
for A1'Component_Size use 33;

for A2'Size use 264;
for A2'Alignment use 1;
for A2'Component_Size use 33;


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


2016-05-16  Eric Botcazou  <ebotcazou@adacore.com>

	* exp_util.adb (Remove_Side_Effects): Also make a constant if we need
	to capture the value for a small not by-reference record type.
	* freeze.ads (Check_Compile_Time_Size): Adjust comment.
	* freeze.adb (Set_Small_Size): Likewise.  Accept a size in the range
	of 33 .. 64 bits.
	(Check_Compile_Time_Size): Merge scalar and access type cases. Change
	variable name in array type case.  For the computation of the packed
	size, deal with record components and remove redundant test.
	(Freeze_Array_Type): Also adjust packing status when the size of the
	component type is in the range 33 .. 64 bits.
	* doc/gnat_rm/representation_clauses_and_pragmas.rst: Turn primitive
	into elementary type throughout.  Minor tweaks.
	(Alignment Clauses): Document actual alignment of packed array types.
	(Pragma Pack for Arrays): List only the 3 main cases and adjust.  Add
	"simple" to the record case.  Document effect on non packable types.
	(Pragma Pack for Records): Likewise.  Add record case and adjust.
diff mbox

Patch

Index: doc/gnat_rm/representation_clauses_and_pragmas.rst
===================================================================
--- doc/gnat_rm/representation_clauses_and_pragmas.rst	(revision 236264)
+++ doc/gnat_rm/representation_clauses_and_pragmas.rst	(working copy)
@@ -32,9 +32,9 @@  GNAT requires that all alignment clauses
 default alignments are always a power of 2.  The default alignment
 values are as follows:
 
-* *Primitive Types*.
+* *Elementary Types*.
 
-  For primitive types, the alignment is the minimum of the actual size of
+  For elementary types, the alignment is the minimum of the actual size of
   objects of the type divided by `Storage_Unit`,
   and the maximum alignment supported by the target.
   (This maximum alignment is given by the GNAT-specific attribute
@@ -53,10 +53,11 @@  values are as follows:
   For arrays, the alignment is equal to the alignment of the component type
   for the normal case where no packing or component size is given.  If the
   array is packed, and the packing is effective (see separate section on
-  packed arrays), then the alignment will be one for long packed arrays,
-  or arrays whose length is not known at compile time.  For short packed
+  packed arrays), then the alignment will be either 4, 2 or 1 for long packed
+  arrays or arrays whose length is not known at compile time, depending on
+  whether the component size is divisible by 4, 2 or is odd.  For short packed
   arrays, which are handled internally as modular types, the alignment
-  will be as described for primitive types, e.g., a packed array of length
+  will be as described for elementary types, e.g. a packed array of length
   31 bits will have an object size of four bytes, and an alignment of 4.
 
 * *Records*.
@@ -789,7 +790,7 @@  restrictions placed on component clauses
   little-endian machines, this must be explicitly programmed.  This capability
   is not provided by `Bit_Order`.
 
-* Components that are positioned across byte boundaries
+* Components that are positioned across byte boundaries.
 
   but do not occupy an integral number of bytes.  Given that bytes are not
   reordered, such fields would occupy a non-contiguous sequence of bits
@@ -1069,22 +1070,23 @@  Pragma Pack for Arrays
 
 .. index:: Pragma Pack (for arrays)
 
-Pragma `Pack` applied to an array has no effect unless the component type
-is packable.  For a component type to be packable, it must be one of the
-following cases:
+Pragma `Pack` applied to an array has an effect that depends upon whether the
+component type is *packable*.  For a component type to be *packable*, it must
+be one of the following cases:
 
-*
-  Any scalar type
-*
-  Any type whose size is specified with a size clause
-*
-  Any packed array type with a static size
-*
-  Any record type padded because of its default alignment
+* Any elementary type.
+
+* Any small packed array type with a static size.
+
+* Any small simple record type with a static size.
 
 For all these cases, if the component subtype size is in the range
-1 through 63, then the effect of the pragma `Pack` is exactly as though a
+1 through 64, then the effect of the pragma `Pack` is exactly as though a
 component size were specified giving the component subtype size.
+
+All other types are non-packable, they occupy an integral number of storage
+units and the only effect of pragma Pack is to remove alignment gaps.
+
 For example if we have:
 
 .. code-block:: ada
@@ -1095,7 +1097,7 @@  For example if we have:
      pragma Pack (ar);
 
 Then the component size of `ar` will be set to 5 (i.e., to `r'size`,
-and the size of the array `ar` will be exactly 40 bits.
+and the size of the array `ar` will be exactly 40 bits).
 
 Note that in some cases this rather fierce approach to packing can produce
 unexpected effects.  For example, in Ada 95 and Ada 2005,
@@ -1184,23 +1186,21 @@  taken by components.  We distinguish bet
 *non-packable* components.
 Components of the following types are considered packable:
 
-*
-  Components of a primitive type are packable unless they are aliased
-  or of an atomic type.
+* Components of an elementary type are packable unless they are aliased,
+  independent or of an atomic type.
 
-*
-  Small packed arrays, whose size does not exceed 64 bits, and where the
-  size is statically known at compile time, are represented internally
-  as modular integers, and so they are also packable.
+* Small packed arrays, where the size is statically known, are represented
+  internally as modular integers, and so they are also packable.
+
+* Small simple records, where the size is statically known, are also packable.
 
+For all these cases, if the 'Size value is in the range 1 through 64, the
+components occupy the exact number of bits corresponding to this value
+and are packed with no padding bits, i.e. they can start on an arbitrary
+bit boundary.
 
-All packable components occupy the exact number of bits corresponding to
-their `Size` value, and are packed with no padding bits, i.e., they
-can start on an arbitrary bit boundary.
-
-All other types are non-packable, they occupy an integral number of
-storage units, and
-are placed at a boundary corresponding to their alignment requirements.
+All other types are non-packable, they occupy an integral number of storage
+units and the only effect of pragma Pack is to remove alignment gaps.
 
 For example, consider the record
 
@@ -1331,7 +1331,7 @@  so for example, the following is permitt
 
 Note: the above rules apply to recent releases of GNAT 5.
 In GNAT 3, there are more severe restrictions on larger components.
-For non-primitive types, including packed arrays with a size greater than
+For composite types, including packed arrays with a size greater than
 64 bits, component clauses must respect the alignment requirement of the
 type, in particular, always starting on a byte boundary, and the length
 must be a multiple of the storage unit.
Index: exp_util.adb
===================================================================
--- exp_util.adb	(revision 236264)
+++ exp_util.adb	(working copy)
@@ -7711,15 +7711,22 @@  package body Exp_Util is
 
       Scope_Suppress.Suppress := (others => True);
 
-      --  If it is an elementary type and we need to capture the value, just
-      --  make a constant. Likewise if this is not a name reference, except
-      --  for a type conversion because we would enter an infinite recursion
-      --  with Checks.Apply_Predicate_Check if the target type has predicates.
-      --  And type conversions need a specific treatment anyway, see below.
-      --  Also do it if we have a volatile reference and Name_Req is not set
-      --  (see comments for Side_Effect_Free).
+      --  If this is an elementary or a small not by-reference record type, and
+      --  we need to capture the value, just make a constant; this is cheap and
+      --  objects of both kinds of types can be bit aligned, so it might not be
+      --  possible to generate a reference to them. Likewise if this is not a
+      --  name reference, except for a type conversion because we would enter
+      --  an infinite recursion with Checks.Apply_Predicate_Check if the target
+      --  type has predicates (and type conversions need a specific treatment
+      --  anyway, see below). Also do it if we have a volatile reference and
+      --  Name_Req is not set (see comments for Side_Effect_Free).
 
-      if Is_Elementary_Type (Exp_Type)
+      if (Is_Elementary_Type (Exp_Type)
+           or else (Is_Record_Type (Exp_Type)
+                     and then Known_Static_RM_Size (Exp_Type)
+                     and then RM_Size (Exp_Type) <= 64
+                     and then not Has_Discriminants (Exp_Type)
+                     and then not Is_By_Reference_Type (Exp_Type)))
         and then (Variable_Ref
                    or else (not Is_Name_Reference (Exp)
                              and then Nkind (Exp) /= N_Type_Conversion)
Index: freeze.adb
===================================================================
--- freeze.adb	(revision 236264)
+++ freeze.adb	(working copy)
@@ -745,9 +745,9 @@  package body Freeze is
    procedure Check_Compile_Time_Size (T : Entity_Id) is
 
       procedure Set_Small_Size (T : Entity_Id; S : Uint);
-      --  Sets the compile time known size (32 bits or less) in the Esize
-      --  field, of T checking for a size clause that was given which attempts
-      --  to give a smaller size, and also checking for an alignment clause.
+      --  Sets the compile time known size (64 bits or less) in the RM_Size
+      --  field of T, checking for a size clause that was given which attempts
+      --  to give a smaller size.
 
       function Size_Known (T : Entity_Id) return Boolean;
       --  Recursive function that does all the work
@@ -765,7 +765,7 @@  package body Freeze is
 
       procedure Set_Small_Size (T : Entity_Id; S : Uint) is
       begin
-         if S > 32 then
+         if S > 64 then
             return;
 
          --  Check for bad size clause given
@@ -800,14 +800,12 @@  package body Freeze is
          if Size_Known_At_Compile_Time (T) then
             return True;
 
-         --  Always True for scalar types. This is true even for generic formal
-         --  scalar types. We used to return False in the latter case, but the
-         --  size is known at compile time, even in the template, we just do
-         --  not know the exact size but that's not the point of this routine.
+         --  Always True for elementary types, even generic formal elementary
+         --  types. We used to return False in the latter case, but the size
+         --  is known at compile time, even in the template, we just do not
+         --  know the exact size but that's not the point of this routine.
 
-         elsif Is_Scalar_Type (T)
-           or else Is_Task_Type (T)
-         then
+         elsif Is_Elementary_Type (T) or else Is_Task_Type (T) then
             return True;
 
          --  Array types
@@ -817,8 +815,8 @@  package body Freeze is
             --  String literals always have known size, and we can set it
 
             if Ekind (T) = E_String_Literal_Subtype then
-               Set_Small_Size (T, Component_Size (T)
-                               * String_Literal_Length (T));
+               Set_Small_Size
+                 (T, Component_Size (T) * String_Literal_Length (T));
                return True;
 
             --  Unconstrained types never have known at compile time size
@@ -839,10 +837,10 @@  package body Freeze is
             end if;
 
             --  Check for all indexes static, and also compute possible size
-            --  (in case it is less than 32 and may be packable).
+            --  (in case it is not greater than 64 and may be packable).
 
             declare
-               Esiz : Uint := Component_Size (T);
+               Size : Uint := Component_Size (T);
                Dim  : Uint;
 
             begin
@@ -869,24 +867,19 @@  package body Freeze is
                      Dim := Expr_Value (High) - Expr_Value (Low) + 1;
 
                      if Dim >= 0 then
-                        Esiz := Esiz * Dim;
+                        Size := Size * Dim;
                      else
-                        Esiz := Uint_0;
+                        Size := Uint_0;
                      end if;
                   end if;
 
                   Next_Index (Index);
                end loop;
 
-               Set_Small_Size (T, Esiz);
+               Set_Small_Size (T, Size);
                return True;
             end;
 
-         --  Access types always have known at compile time sizes
-
-         elsif Is_Access_Type (T) then
-            return True;
-
          --  For non-generic private types, go to underlying type if present
 
          elsif Is_Private_Type (T)
@@ -1074,11 +1067,10 @@  package body Freeze is
 
                   if Packed_Size_Known then
 
-                     --  We can only deal with elementary types, since for
-                     --  non-elementary components, alignment enters into the
-                     --  picture, and we don't know enough to handle proper
-                     --  alignment in this context. Packed arrays count as
-                     --  elementary if the representation is a modular type.
+                     --  We can deal with elementary types, small packed arrays
+                     --  if the representation is a modular type and also small
+                     --  record types (if the size is not greater than 64, but
+                     --  the condition is checked by Set_Small_Size).
 
                      if Is_Elementary_Type (Ctyp)
                        or else (Is_Array_Type (Ctyp)
@@ -1086,33 +1078,14 @@  package body Freeze is
                                             (Packed_Array_Impl_Type (Ctyp))
                                  and then Is_Modular_Integer_Type
                                             (Packed_Array_Impl_Type (Ctyp)))
+                       or else Is_Record_Type (Ctyp)
                      then
-                        --  Packed size unknown if we have an atomic/VFA type
-                        --  or a by-reference type, since the back end knows
-                        --  how these are layed out.
-
-                        if Is_Atomic_Or_VFA (Ctyp)
-                          or else Is_By_Reference_Type (Ctyp)
-                        then
-                           Packed_Size_Known := False;
-
                         --  If RM_Size is known and static, then we can keep
-                        --  accumulating the packed size
+                        --  accumulating the packed size.
 
-                        elsif Known_Static_RM_Size (Ctyp) then
+                        if Known_Static_RM_Size (Ctyp) then
 
-                           --  A little glitch, to be removed sometime ???
-                           --  gigi does not understand zero sizes yet.
-
-                           if RM_Size (Ctyp) = Uint_0 then
-                              Packed_Size_Known := False;
-
-                           --  Normal case where we can keep accumulating the
-                           --  packed array size.
-
-                           else
-                              Packed_Size := Packed_Size + RM_Size (Ctyp);
-                           end if;
+                           Packed_Size := Packed_Size + RM_Size (Ctyp);
 
                         --  If we have a field whose RM_Size is not known then
                         --  we can't figure out the packed size here.
@@ -1121,8 +1094,7 @@  package body Freeze is
                            Packed_Size_Known := False;
                         end if;
 
-                     --  If we have a non-elementary type we can't figure out
-                     --  the packed array size (alignment issues).
+                     --  For other types we can't figure out the packed size
 
                      else
                         Packed_Size_Known := False;
@@ -2475,13 +2447,15 @@  package body Freeze is
                      end if;
 
                      --  Actual packing is not needed for 8, 16, 32, 64. Also
-                     --  not needed for 24 if alignment is 1.
+                     --  not needed for multiples of 8 if alignment is 1, and
+                     --  for multiples of 16 (i.e. only 48) if alignment is 2.
 
                      if        Csiz = 8
                        or else Csiz = 16
                        or else Csiz = 32
                        or else Csiz = 64
-                       or else (Csiz = 24 and then Alignment (Ctyp) = 1)
+                       or else (Csiz mod 8 = 0 and then Alignment (Ctyp) = 1)
+                       or else (Csiz = 48 and then Alignment (Ctyp) = 2)
                      then
                         --  Here the array was requested to be packed, but
                         --  the packing request had no effect, so Is_Packed
Index: freeze.ads
===================================================================
--- freeze.ads	(revision 236264)
+++ freeze.ads	(working copy)
@@ -151,15 +151,15 @@  package Freeze is
    --    fact Gigi decides it is known, but the opposite situation can never
    --    occur.
    --
-   --    Size is known at compile time, but the actual value of the size is
-   --    not known to the front end or is definitely 32 or more. In this case
-   --    Size_Known_At_Compile_Time is set, but the Esize field is left set
+   --    Size is known at compile time, but the actual value of the size is not
+   --    known to the front end or is definitely greater than 64. In this case,
+   --    Size_Known_At_Compile_Time is set, but the RM_Size field is left set
    --    to zero (to be set by Gigi).
    --
    --    Size is known at compile time, and the actual value of the size is
-   --    known to the front end and is less than 32. In this case, the flag
-   --    Size_Known_At_Compile_Time is set, and in addition Esize is set to
-   --    the required size, allowing for possible front end packing of an
+   --    known to the front end and is not greater than 64. In this case, the
+   --    flag Size_Known_At_Compile_Time is set, and in addition RM_Size is set
+   --    to the required size, allowing for possible front end packing of an
    --    array using this type as a component type.
    --
    --  Note: the flag Size_Known_At_Compile_Time is used to determine if the