diff mbox

[Ada] Adjustments to setting alignment when size is set

Message ID 20111121115945.GA31065@adacore.com
State New
Headers show

Commit Message

Arnaud Charlet Nov. 21, 2011, 11:59 a.m. UTC
This patch reworks the handling when the alignment is not set
but the size is set. Basically in this case, we always set a
reasonable alignment from the size. This is true even if the
size is confirming (we really have no way of telling if a size
clause is or is not confirming in the general case).

And in the case where neither size nor alignment is set for
a subtype or derived type, we inherit both object size and
alignment from the parent type. This avoids some anomolies
that were previously showing up.

The following example:

     1.  package Pkg is
     2.    type Time_Xtype is delta 0.1 range 0.0 .. 999_999.9;
     3.    for Time_Xtype'Small use 0.1;
     4.    for Time_Xtype'Size use 24;
     5.    for Time_Xtype'Alignment use 1;
     6.
     7.    subtype Ddc_Delay_Stype is Time_Xtype range 0.0 .. 51.1;
     8. end;

Was one of the anomolous cases, the result of -gnatR2 is

Representation information for unit Pkg (spec)

for Time_Xtype'Object_Size use 32;
for Time_Xtype'Value_Size use 24;
for Time_Xtype'Alignment use 1;
for Time_Xtype'Small use 0.1;
for Time_Xtype'Range use 0.0 .. 999999.9;

for Ddc_Delay_Stype'Object_Size use 32;
for Ddc_Delay_Stype'Value_Size use 9;
for Ddc_Delay_Stype'Alignment use 1;
for Ddc_Delay_Stype'Small use 0.1;
for Ddc_Delay_Stype'Range use 0.0 .. 51.1;

Previously the subtype Ddc_Delay had an alignment of 4, which was
definitely strange. The impact of this change is small, it had no
visible effect on our entire test suite.

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

2011-11-21  Robert Dewar  <dewar@adacore.com>

	* freeze.adb (Freeze_Enumeration_Type): Make sure to set both
	size and alignment for foreign convention enumeration types.
	* layout.adb (Set_Elem_Alignment): Redo setting of alignment
	when size is set.
diff mbox

Patch

Index: layout.adb
===================================================================
--- layout.adb	(revision 181556)
+++ layout.adb	(working copy)
@@ -3088,7 +3088,7 @@ 
       end if;
 
       --  Here we calculate the alignment as the largest power of two multiple
-      --  of System.Storage_Unit that does not exceed either the actual size of
+      --  of System.Storage_Unit that does not exceed either the object size of
       --  the type, or the maximum allowed alignment.
 
       declare
@@ -3126,21 +3126,101 @@ 
             A := 2 * A;
          end loop;
 
-         --  Now we think we should set the alignment to A, but we skip this if
-         --  an alignment is already set to a value greater than A (happens for
-         --  derived types).
+         --  If alignment is currently not set, then we can safetly set it to
+         --  this new calculated value.
 
-         --  However, if the alignment is known and too small it must be
-         --  increased, this happens in a case like:
+         if Unknown_Alignment (E) then
+            Init_Alignment (E, A);
 
-         --     type R is new Character;
-         --     for R'Size use 16;
+         --  Cases where we have inherited an alignment
 
-         --  Here the alignment inherited from Character is 1, but it must be
-         --  increased to 2 to reflect the increased size.
+         --  For constructed types, always reset the alignment, these are
+         --  Generally invisible to the user anyway, and that way we are
+         --  sure that no constructed types have weird alignments.
 
-         if Unknown_Alignment (E) or else Alignment (E) < A then
+         elsif not Comes_From_Source (E) then
             Init_Alignment (E, A);
+
+         --  If this inherited alignment is the same as the one we computed,
+         --  then obviously everything is fine, and we do not need to reset it.
+
+         elsif Alignment (E) = A then
+            null;
+
+         --  Now we come to the difficult cases where we have inherited an
+         --  alignment and size, but overridden the size but not the alignment.
+
+         elsif Has_Size_Clause (E) or else Has_Object_Size_Clause (E) then
+
+            --  This is tricky, it might be thought that we should try to
+            --  inherit the alignment, since that's what the RM implies, but
+            --  that leads to complex rules and oddities. Consider for example:
+
+            --    type R is new Character;
+            --    for R'Size use 16;
+
+            --  It seems quite bogus in this case to inherit an alignment of 1
+            --  from the parent type Character. Furthermore, if that's what the
+            --  programmer really wanted for some odd reason, then they could
+            --  specify the alignment they wanted.
+
+            --  Furthermore we really don't want to inherit the alignment in
+            --  the case of a specified Object_Size for a subtype, since then
+            --  there would be no way of overriding to give a reasonable value
+            --  (we don't have an Object_Subtype attribute). Consider:
+
+            --    subtype R is new Character;
+            --    for R'Object_Size use 16;
+
+            --  If we inherit the alignment of 1, then we have an odd
+            --  inefficient alignment for the subtype, which cannot be fixed.
+
+            --  So we make the decision that if Size (or Object_Size) is given
+            --  (and, in the case of a first subtype, the alignment is not set
+            --  with a specific alignment clause). We reset the alignment to
+            --  the appropriate value for the specified size. This is a nice
+            --  simple rule to implement and document.
+
+            --  There is one slight glitch, which is that a confirming size
+            --  clause can now change the alignment, which, if we really think
+            --  that confirming rep clauses should have no effect, is a no-no.
+
+            --    type R is new Character;
+            --    for R'Alignment use 2;
+            --    type S is new R;
+            --    for S'Size use Character'Size;
+
+            --  Now the alignment of S is 1 instead of 2, as a result of
+            --  applying the above rule to the confirming rep clause for S. Not
+            --  clear this is worth worrying about. If we recorded whether a
+            --  size clause was confirming we could avoid this, but right now
+            --  we have no way of doing that or easily figuring it out, so we
+            --  don't bother.
+
+            --  Historical note. In versions of GNAT prior to Nov 6th, 2010, an
+            --  odd distinction was made between inherited alignments greater
+            --  than the computed alignment (where the larger alignment was
+            --  inherited) and inherited alignments smaller than the computed
+            --  alignment (where the smaller alignment was overridden). This
+            --  was a dubious fix to get around an ACATS problem which seems
+            --  to have disappeared anyway, and in any case, this peculiarity
+            --  was never documented.
+
+            Init_Alignment (E, A);
+
+         --  If no Size (or Object_Size) was specified, then we inherited the
+         --  object size, so we should inherit the alignment as well and not
+         --  modify it. This takes care of cases like:
+
+         --    type R is new Integer;
+         --    for R'Alignment use 1;
+         --    subtype S is R;
+
+         --  Here we have R has a default Object_Size of 32, and a specified
+         --  alignment of 1, and it seeems right for S to inherit both values.
+
+         else
+            null;
          end if;
       end;
    end Set_Elem_Alignment;
Index: freeze.adb
===================================================================
--- freeze.adb	(revision 181556)
+++ freeze.adb	(working copy)
@@ -4239,7 +4239,8 @@ 
       --  By default, if no size clause is present, an enumeration type with
       --  Convention C is assumed to interface to a C enum, and has integer
       --  size. This applies to types. For subtypes, verify that its base
-      --  type has no size clause either.
+      --  type has no size clause either. Treat other foreign conventions
+      --  in the same way, and also make sure alignment is set right.
 
       if Has_Foreign_Convention (Typ)
         and then not Has_Size_Clause (Typ)
@@ -4247,6 +4248,7 @@ 
         and then Esize (Typ) < Standard_Integer_Size
       then
          Init_Esize (Typ, Standard_Integer_Size);
+         Set_Alignment (Typ, Alignment (Standard_Integer));
 
       else
          --  If the enumeration type interfaces to C, and it has a size clause