Patchwork [Ada] Ignore Optimize_Alignment (Space) for packed variable length record

login
register
mail settings
Submitter Arnaud Charlet
Date Oct. 29, 2012, 11:39 a.m.
Message ID <20121029113928.GA1927@adacore.com>
Download mbox | patch
Permalink /patch/194980/
State New
Headers show

Comments

Arnaud Charlet - Oct. 29, 2012, 11:39 a.m.
Normally pragma Optimize_Alignment (Space) forces an alignment of 1
for any packed record, but we can't do that for a variable length
record, since we can't implement that case, so ignore the pragma
in this case with a warning.

The following test program shows the patch in action:

     1. pragma optimize_alignment (space);
     2. procedure testalign is
     3.    subtype Continuous_Values is float;
     4.    type Continuous_Values_Map is
     5.      array ( integer range <> ) of Continuous_Values;
     6.
     7.    type my_record (s : integer) is record
                |
        >>> Optimize_Alignment has no effect for "my_record"
        >>> pragma is ignored for variable length record

     8.       x : continuous_values_map (1 .. s);
     9.    end record;
    10.    pragma pack (my_record);
    11.
    12.    type my_record2 (s : integer) is record
    13.       x : continuous_values_map (1 .. 1000);
    14.    end record;
    15.    pragma pack (my_record2);
    16.
    17. begin
    18.     null;
    19. end;

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

2012-10-29  Robert Dewar  <dewar@adacore.com>

	* layout.adb (Set_Composite_Alignment): Ignore pragma
	Optimize_Alignment (Space) for packed variable length records.
Diego Novillo - Oct. 29, 2012, 2:33 p.m.
Arnaud,

After updating my tree to rev 192942 this morning, I'm getting a
bootstrap failure with ada enabled:

raised CONSTRAINT_ERROR : a-comlin.adb:65 explicit raise
make[6]: *** [rts/s-oscons.ads] Error 1
make[6]: Leaving directory `bld-gcc/gcc/ada'
make[5]: *** [gnatlib-shared-default] Error 2

I've configured with

$ configure --enable-languages=all,ada,go,obj-c++ --enable-checking=release

Does this look familiar?


Thanks.  Diego.
Arnaud Charlet - Oct. 29, 2012, 2:36 p.m.
> After updating my tree to rev 192942 this morning, I'm getting a
> bootstrap failure with ada enabled:

Looks like make did not rebuild (or too late?) xoscons.

I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and
see if this fixes the error.

If not, you might need to do a make -s -k clean

Arno
Diego Novillo - Oct. 29, 2012, 2:38 p.m.
On Mon, Oct 29, 2012 at 10:36 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>> After updating my tree to rev 192942 this morning, I'm getting a
>> bootstrap failure with ada enabled:
>
> Looks like make did not rebuild (or too late?) xoscons.
>
> I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and
> see if this fixes the error.
>
> If not, you might need to do a make -s -k clean

Well, this was a build from scratch.  Maybe there is a parallel make
bug in the new ada changes?  Some undeclared dependency?


Diego.
Arnaud Charlet - Oct. 29, 2012, 2:39 p.m.
> Well, this was a build from scratch.  Maybe there is a parallel make
> bug in the new ada changes?  Some undeclared dependency?

There is not in the recent changes. Maybe there's a latent bug, although I
have never experienced it.

At which revision are you?

Arno
Diego Novillo - Oct. 29, 2012, 2:41 p.m.
On Mon, Oct 29, 2012 at 10:39 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>> Well, this was a build from scratch.  Maybe there is a parallel make
>> bug in the new ada changes?  Some undeclared dependency?
>
> There is not in the recent changes. Maybe there's a latent bug, although I
> have never experienced it.
>
> At which revision are you?

rev 192942.

Removing bld/gcc/ada/bldtools did not help.  I'm trying a -j1 build
after doing 'make clean'.


Diego.
Arnaud Charlet - Oct. 29, 2012, 2:44 p.m.
> >> Well, this was a build from scratch.  Maybe there is a parallel make
> >> bug in the new ada changes?  Some undeclared dependency?
> >
> > There is not in the recent changes. Maybe there's a latent bug, although I
> > have never experienced it.
> >
> > At which revision are you?
> 
> rev 192942.

Can you confirm that gcc/ada/Make-generated.in contains the following
line:

	./xoscons s-oscons ) ; \

and NOT:

	./xoscons ) ; \

Arno
Diego Novillo - Oct. 29, 2012, 2:48 p.m.
On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>> >> Well, this was a build from scratch.  Maybe there is a parallel make
>> >> bug in the new ada changes?  Some undeclared dependency?
>> >
>> > There is not in the recent changes. Maybe there's a latent bug, although I
>> > have never experienced it.
>> >
>> > At which revision are you?
>>
>> rev 192942.
>
> Can you confirm that gcc/ada/Make-generated.in contains the following
> line:
>
>         ./xoscons s-oscons ) ; \
>
> and NOT:
>
>         ./xoscons ) ; \
>

Yup, it does:

 94                 $(OSCONS_CPP) ; \
 95                 $(OSCONS_EXTRACT) ; \
 96                 ./xoscons s-oscons ) ; \
 97                 $(MOVE_IF_CHANGE)
$(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads
$(ADA_GEN_SUBDIR)/s-oscons.ads ; \
 98                 $(MOVE_IF_CHANGE)
$(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h
$(ADA_GEN_SUBDIR)/s-oscons.h
Diego Novillo - Oct. 29, 2012, 2:48 p.m.
On Mon, Oct 29, 2012 at 10:48 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>>> >> Well, this was a build from scratch.  Maybe there is a parallel make
>>> >> bug in the new ada changes?  Some undeclared dependency?
>>> >
>>> > There is not in the recent changes. Maybe there's a latent bug, although I
>>> > have never experienced it.
>>> >
>>> > At which revision are you?
>>>
>>> rev 192942.
>>
>> Can you confirm that gcc/ada/Make-generated.in contains the following
>> line:
>>
>>         ./xoscons s-oscons ) ; \
>>
>> and NOT:
>>
>>         ./xoscons ) ; \
>>
>
> Yup, it does:
>
>  94                 $(OSCONS_CPP) ; \
>  95                 $(OSCONS_EXTRACT) ; \
>  96                 ./xoscons s-oscons ) ; \
>  97                 $(MOVE_IF_CHANGE)
> $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads
> $(ADA_GEN_SUBDIR)/s-oscons.ads ; \
>  98                 $(MOVE_IF_CHANGE)
> $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h
> $(ADA_GEN_SUBDIR)/s-oscons.h
Arnaud Charlet - Oct. 29, 2012, 2:51 p.m.
> Yup, it does:

OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons
vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and
$(srcdir)/gcc/ada/xoscons.adb?

Arno
Diego Novillo - Oct. 29, 2012, 3:18 p.m.
On Mon, Oct 29, 2012 at 10:51 AM, Arnaud Charlet <charlet@adacore.com> wrote:
>> Yup, it does:
>
> OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons
> vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and
> $(srcdir)/gcc/ada/xoscons.adb?

I have no directory called gcc/ada/bldtools/oscons/xoscons.  That's an
executable.  I suppose you meant gcc/ada/bldtools/oscons/xoscons.adb?

$ ls -l gcc/ada/bldtools/oscons/xoscons
gcc/ada/bldtools/oscons/xoscons.adb
~/gcc/clean/trunk/gcc/ada/xoscons.adb
-rw-r----- 1 dnovillo eng 17767 Oct 29 08:32
/home/dnovillo/gcc/clean/trunk/gcc/ada/xoscons.adb
-rwxr-x--- 1 dnovillo eng 59793 Oct 29 10:38 gcc/ada/bldtools/oscons/xoscons*
-rw-r----- 1 dnovillo eng 17767 Oct 29 08:32 gcc/ada/bldtools/oscons/xoscons.adb


Still happens with -j1 (after starting from scratch), so it's probably
not a parallel make issue.

We chatted about this on IRC, and Richi is seeing the same problem on
his builds.


Thanks.  Diego.
Arnaud Charlet - Oct. 29, 2012, 3:27 p.m.
> I have no directory called gcc/ada/bldtools/oscons/xoscons.  That's an
> executable.

Yes, that's as expected, I asked you to check the timestamp of this file.

> Still happens with -j1 (after starting from scratch), so it's probably
> not a parallel make issue.

Right, no surprise here.

> We chatted about this on IRC, and Richi is seeing the same problem on
> his builds.

Very strange. I'm looking at it.

Arno

Patch

Index: layout.adb
===================================================================
--- layout.adb	(revision 192918)
+++ layout.adb	(working copy)
@@ -2882,7 +2882,12 @@ 
         and then Is_Packed (E)
         and then not Is_Atomic (E)
       then
-         Align := 1;
+         if not Size_Known_At_Compile_Time (E) then
+            Error_Msg_N ("Optimize_Alignment has no effect for &", E);
+            Error_Msg_N ("\pragma is ignored for variable length record?", E);
+         else
+            Align := 1;
+         end if;
 
       --  Not a record, or not packed