diff mbox

not too big an alignment

Message ID 29224C40-B124-4C42-BB57-F0E7604A9BF7@comcast.net
State New
Headers show

Commit Message

Mike Stump Nov. 12, 2013, 9:11 p.m. UTC
Alignments are stored in a byte, large alignments don't actually work nicely.  This caps the alignment to 128, as most ports would define BIGGEST_ALIGNMENT to be smaller than this.  The competing change would to be to make it a short, but, I'd be happy to punt that until such time as someone actually needs that.

Ports break down this way currently:

  12 #define BIGGEST_ALIGNMENT 64
  10 #define BIGGEST_ALIGNMENT 32
   6 #define BIGGEST_ALIGNMENT 128
   3 #define BIGGEST_ALIGNMENT 8
   8 #define BIGGEST_ALIGNMENT 16

Ok?

Comments

Jakub Jelinek Nov. 12, 2013, 9:16 p.m. UTC | #1
On Tue, Nov 12, 2013 at 01:11:04PM -0800, Mike Stump wrote:
> Alignments are stored in a byte, large alignments don't actually work nicely.  This caps the alignment to 128, as most ports would define BIGGEST_ALIGNMENT to be smaller than this.  The competing change would to be to make it a short, but, I'd be happy to punt that until such time as someone actually needs that.
> 
> Ports break down this way currently:
> 
>   12 #define BIGGEST_ALIGNMENT 64
>   10 #define BIGGEST_ALIGNMENT 32
>    6 #define BIGGEST_ALIGNMENT 128
>    3 #define BIGGEST_ALIGNMENT 8
>    8 #define BIGGEST_ALIGNMENT 16

You are missing i386 that has BIGGEST_ALIGNMENT 512 (or less, depending on
compiler options).  So this doesn't look right.

> --- a/gcc/genmodes.c
> +++ b/gcc/genmodes.c
> @@ -1178,7 +1178,7 @@ emit_mode_base_align (void)
>                           alignment);
>  
>    for_all_modes (c, m)
> -    tagged_printf ("%u", m->alignment, m->name);
> +    tagged_printf ("%u", m->alignment > 128 ? 128 : m->alignment, m->name);
>  
>    print_closer ();
>  }

	Jakub
Mike Stump Nov. 12, 2013, 9:42 p.m. UTC | #2
On Nov 12, 2013, at 1:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 12, 2013 at 01:11:04PM -0800, Mike Stump wrote:
>> Alignments are stored in a byte, large alignments don't actually work nicely.  This caps the alignment to 128, as most ports would define BIGGEST_ALIGNMENT to be smaller than this.  The competing change would to be to make it a short, but, I'd be happy to punt that until such time as someone actually needs that.
>> 
>> Ports break down this way currently:
>> 
>>  12 #define BIGGEST_ALIGNMENT 64
>>  10 #define BIGGEST_ALIGNMENT 32
>>   6 #define BIGGEST_ALIGNMENT 128
>>   3 #define BIGGEST_ALIGNMENT 8
>>   8 #define BIGGEST_ALIGNMENT 16
> 
> You are missing i386 that has BIGGEST_ALIGNMENT 512 (or less, depending on
> compiler options).  So this doesn't look right.

And yet alignments for modes with sizes like 256 won't work and i386 has no mode with alignment bigger than 128 in this table.

So, do you prefer that we use short instead?  I can do that patch, if you prefer.  Just, there is no port that currently requires it in tree.
Jakub Jelinek Nov. 12, 2013, 9:53 p.m. UTC | #3
On Tue, Nov 12, 2013 at 01:42:00PM -0800, Mike Stump wrote:
> On Nov 12, 2013, at 1:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, Nov 12, 2013 at 01:11:04PM -0800, Mike Stump wrote:
> >> Alignments are stored in a byte, large alignments don't actually work nicely.  This caps the alignment to 128, as most ports would define BIGGEST_ALIGNMENT to be smaller than this.  The competing change would to be to make it a short, but, I'd be happy to punt that until such time as someone actually needs that.
> >> 
> >> Ports break down this way currently:
> >> 
> >>  12 #define BIGGEST_ALIGNMENT 64
> >>  10 #define BIGGEST_ALIGNMENT 32
> >>   6 #define BIGGEST_ALIGNMENT 128
> >>   3 #define BIGGEST_ALIGNMENT 8
> >>   8 #define BIGGEST_ALIGNMENT 16
> > 
> > You are missing i386 that has BIGGEST_ALIGNMENT 512 (or less, depending on
> > compiler options).  So this doesn't look right.
> 
> And yet alignments for modes with sizes like 256 won't work and i386 has no mode with alignment bigger than 128 in this table.

Well, BIGGEST_ALIGNMENT is in bits, while mode_base_align seems to be in bytes it
seems:

unsigned int
get_mode_alignment (enum machine_mode mode)
{
  return MIN (BIGGEST_ALIGNMENT, MAX (1, mode_base_align[mode]*BITS_PER_UNIT));
}

So, supposedly it works up to 1024 bit alignment right now.

	Jakub
Richard Biener Nov. 13, 2013, 10:18 a.m. UTC | #4
On Tue, Nov 12, 2013 at 10:53 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 12, 2013 at 01:42:00PM -0800, Mike Stump wrote:
>> On Nov 12, 2013, at 1:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Tue, Nov 12, 2013 at 01:11:04PM -0800, Mike Stump wrote:
>> >> Alignments are stored in a byte, large alignments don't actually work nicely.  This caps the alignment to 128, as most ports would define BIGGEST_ALIGNMENT to be smaller than this.  The competing change would to be to make it a short, but, I'd be happy to punt that until such time as someone actually needs that.
>> >>
>> >> Ports break down this way currently:
>> >>
>> >>  12 #define BIGGEST_ALIGNMENT 64
>> >>  10 #define BIGGEST_ALIGNMENT 32
>> >>   6 #define BIGGEST_ALIGNMENT 128
>> >>   3 #define BIGGEST_ALIGNMENT 8
>> >>   8 #define BIGGEST_ALIGNMENT 16
>> >
>> > You are missing i386 that has BIGGEST_ALIGNMENT 512 (or less, depending on
>> > compiler options).  So this doesn't look right.
>>
>> And yet alignments for modes with sizes like 256 won't work and i386 has no mode with alignment bigger than 128 in this table.
>
> Well, BIGGEST_ALIGNMENT is in bits, while mode_base_align seems to be in bytes it
> seems:
>
> unsigned int
> get_mode_alignment (enum machine_mode mode)
> {
>   return MIN (BIGGEST_ALIGNMENT, MAX (1, mode_base_align[mode]*BITS_PER_UNIT));
> }
>
> So, supposedly it works up to 1024 bit alignment right now.

Either storing log2 (alignment) in mode_base_align or increasing its
size is possible if required.  IIRC we store alignment log2 elsewhere.

Richard.

>         Jakub
diff mbox

Patch

diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index b4dc0d2..8f4980c 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -1178,7 +1178,7 @@  emit_mode_base_align (void)
                          alignment);
 
   for_all_modes (c, m)
-    tagged_printf ("%u", m->alignment, m->name);
+    tagged_printf ("%u", m->alignment > 128 ? 128 : m->alignment, m->name);
 
   print_closer ();
 }