This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/3] Extend -falign-FOO=N to N[:M[:N2[:M2]]]


On Tue, Jul 03, 2018 at 02:51:27PM +0200, Martin Liška wrote:
> On 07/03/2018 12:58 PM, Segher Boessenkool wrote:
> > On Tue, Jul 03, 2018 at 12:15:48PM +0200, Martin Liška wrote:
> >>> toplev.c already has (in init_alignments):
> >>>
> >>>   if (align_jumps_max_skip > align_jumps)
> >>>     align_jumps_max_skip = align_jumps - 1;
> >>
> >> I'm rewriting this logic in the patch set. Issue is that 
> >> checking for value of align_jumps_max_skip is done
> >> in rs6000_option_override_internal, which is place before
> >> align_jumps_max_skip is parsed.
> >>
> >> That said, 'align_jumps_max_skip <= 0' is always true.
> > 
> > It's not clear to me what you want me to do.
> > 
> > You should write your patch so that the end result behaves the same as
> > before, on all targets.  If that requires changing (or at least checking)
> > all targets, then you have a lot of work to do.
> > 
> > If you think the rs6000 backend is doing something wrong, please say
> > what exactly?  I don't see it.
> 
> Uf, it's quite complicated I would say.
> So first I believe for all -falign-{labels,loops,jumps} we don't handle properly
> value of the argument. More precisely for a value of N (not power of 2),
> we don't respect max_skip and we generate alignment to M, where M is first bigger
> power of 2 number. Example:
> 
> $ gcc /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/params/blocksort-part.c -O2 -falign-labels=1025 -c -S  -o /dev/stdout | grep align | sort | uniq -c
>       1 	.align 32
>     132 	.p2align 11
>       7 	.p2align 4,,15
> 
> 2^11 == 2048, but I would expect '.p2align 11,,1024' to be generated. That's what you get for function alignment:
> 
> $ gcc /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/params/blocksort-part.c -O2 -falign-functions=1025 -c -S  -o /dev/stdout | grep align | sort | uniq -c
>       1 	.align 32
>       7 	.p2align 11,,1024
>      55 	.p2align 3
>      48 	.p2align 4,,10
> 
> Do I understand that correctly that it's broken?

Yes, this behaviour contradicts our documentation:

'-falign-labels=N'
     Align all branch targets to a power-of-two boundary, skipping up to
     N bytes like '-falign-functions'.

'-falign-functions=N'
     Align the start of functions to the next power-of-two greater than
     N, skipping up to N bytes.  For instance, '-falign-functions=32'
     aligns functions to the next 32-byte boundary, but
     '-falign-functions=24' aligns to the next 32-byte boundary only if
     this can be done by skipping 23 bytes or less.

> On powerpc, because align_jumps_max_skip is set to 15, then we see inconsistency like:
> 
> ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/params/blocksort-part.c -O2 -falign-jumps=14 -c -S  -o /dev/stdout | grep align | sort | uniq -c
> ...
>      27 	.p2align 4,,13
> ...
> 
> which is correct.
> 
> but:
> 
> ./xgcc -B. /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/params/blocksort-part.c -O2 -falign-jumps=1025 -c -S  -o /dev/stdout | grep align | sort | uniq -c
> ...
>      27 	.p2align 11,,15
> ...
> 
> Here 11,,15 is completely broken value.

Yup.

This is specific to align-jumps...  Not many people ever change that :-)


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]