| Message ID | 0a23778f-57c2-5f01-4919-231ec7d001e0@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Add limit for maximal alignment options (PR c/84310). | expand |
PING^1 On 02/12/2018 01:09 PM, Martin Liška wrote: > Hi. > > Following patch fixes 2 issues with -falign-*: > 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared > as code in final.c can deal just with limited alignment. > 2) thus I also documented and limited the maximum value of -falign-* options. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > i386.exp test-suite works fine on x86_64 machine. > > Ready to be installed? > Martin > > gcc/ChangeLog: > > 2018-02-09 Martin Liska <mliska@suse.cz> > > PR c/84310 > PR target/79747 > * final.c (shorten_branches): Build align_tab array with one > more element. > * opts.c (finish_options): Add alignment option limit check. > (MAX_CODE_ALIGN): Likewise. > (MAX_CODE_ALIGN_VALUE): Likewise. > * doc/invoke.texi: Document maximum allowed option value for > all -falign-* options. > > gcc/testsuite/ChangeLog: > > 2018-02-12 Martin Liska <mliska@suse.cz> > > PR c/84310 > PR target/79747 > * gcc.target/i386/pr84310.c: New test. > * gcc.target/i386/pr84310-2.c: Likewise. > --- > gcc/doc/invoke.texi | 4 ++++ > gcc/final.c | 4 ++-- > gcc/opts.c | 20 ++++++++++++++++++++ > gcc/testsuite/gcc.target/i386/pr84310-2.c | 10 ++++++++++ > gcc/testsuite/gcc.target/i386/pr84310.c | 8 ++++++++ > 5 files changed, 44 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr84310-2.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr84310.c > >
On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote: > Following patch fixes 2 issues with -falign-*: > 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared > as code in final.c can deal just with limited alignment. > 2) thus I also documented and limited the maximum value of -falign-* options. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > i386.exp test-suite works fine on x86_64 machine. > > Ready to be installed? > Martin > > gcc/ChangeLog: > > 2018-02-09 Martin Liska <mliska@suse.cz> > > PR c/84310 > PR target/79747 > * final.c (shorten_branches): Build align_tab array with one > more element. > * opts.c (finish_options): Add alignment option limit check. > (MAX_CODE_ALIGN): Likewise. > (MAX_CODE_ALIGN_VALUE): Likewise. > * doc/invoke.texi: Document maximum allowed option value for > all -falign-* options. > > gcc/testsuite/ChangeLog: > > 2018-02-12 Martin Liska <mliska@suse.cz> > > PR c/84310 > PR target/79747 > * gcc.target/i386/pr84310.c: New test. > * gcc.target/i386/pr84310-2.c: Likewise. Ok, thanks. Jakub
On 02/20/2018 09:43 AM, Jakub Jelinek wrote: > On Mon, Feb 12, 2018 at 01:09:43PM +0100, Martin Liška wrote: >> Following patch fixes 2 issues with -falign-*: >> 1) when using -malign-x=16 (or corresponding -falign-* value) then ICE appeared >> as code in final.c can deal just with limited alignment. >> 2) thus I also documented and limited the maximum value of -falign-* options. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> i386.exp test-suite works fine on x86_64 machine. >> >> Ready to be installed? >> Martin >> >> gcc/ChangeLog: >> >> 2018-02-09 Martin Liska <mliska@suse.cz> >> >> PR c/84310 >> PR target/79747 >> * final.c (shorten_branches): Build align_tab array with one >> more element. >> * opts.c (finish_options): Add alignment option limit check. >> (MAX_CODE_ALIGN): Likewise. >> (MAX_CODE_ALIGN_VALUE): Likewise. >> * doc/invoke.texi: Document maximum allowed option value for >> all -falign-* options. >> >> gcc/testsuite/ChangeLog: >> >> 2018-02-12 Martin Liska <mliska@suse.cz> >> >> PR c/84310 >> PR target/79747 >> * gcc.target/i386/pr84310.c: New test. >> * gcc.target/i386/pr84310-2.c: Likewise. > > Ok, thanks. > > Jakub > Thanks Jakub! Would it be possible to backport that to active branches? Martin
On Tue, Feb 20, 2018 at 11:05:50AM +0100, Martin Liška wrote: > Thanks Jakub! > Would it be possible to backport that to active branches? Yes, but give it some time on the trunk first. Jakub
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index df357bea7dc..edfa9d5ada1 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9096,6 +9096,7 @@ Some assemblers only support this flag when @var{n} is a power of two; in that case, it is rounded up. If @var{n} is not specified or is zero, use a machine-dependent default. +The maximum allowed @var{n} option value is 65536. Enabled at levels @option{-O2}, @option{-O3}. @@ -9121,6 +9122,7 @@ are greater than this value, then their values are used instead. If @var{n} is not specified or is zero, use a machine-dependent default which is very likely to be @samp{1}, meaning no alignment. +The maximum allowed @var{n} option value is 65536. Enabled at levels @option{-O2}, @option{-O3}. @@ -9134,6 +9136,7 @@ operations. @option{-fno-align-loops} and @option{-falign-loops=1} are equivalent and mean that loops are not aligned. +The maximum allowed @var{n} option value is 65536. If @var{n} is not specified or is zero, use a machine-dependent default. @@ -9151,6 +9154,7 @@ need be executed. equivalent and mean that loops are not aligned. If @var{n} is not specified or is zero, use a machine-dependent default. +The maximum allowed @var{n} option value is 65536. Enabled at levels @option{-O2}, @option{-O3}. diff --git a/gcc/final.c b/gcc/final.c index 99a7cadd7c9..933c613cabf 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -911,7 +911,7 @@ shorten_branches (rtx_insn *first) char *varying_length; rtx body; int uid; - rtx align_tab[MAX_CODE_ALIGN]; + rtx align_tab[MAX_CODE_ALIGN + 1]; /* Compute maximum UID and allocate label_align / uid_shuid. */ max_uid = get_max_uid (); @@ -1016,7 +1016,7 @@ shorten_branches (rtx_insn *first) alignment of n. */ uid_align = XCNEWVEC (rtx, max_uid); - for (i = MAX_CODE_ALIGN; --i >= 0;) + for (i = MAX_CODE_ALIGN + 1; --i >= 0;) align_tab[i] = NULL_RTX; seq = get_last_insn (); for (; seq; seq = PREV_INSN (seq)) diff --git a/gcc/opts.c b/gcc/opts.c index f2795f98bf4..33efcc0d6e7 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1039,6 +1039,26 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, if ((opts->x_flag_sanitize & SANITIZE_KERNEL_ADDRESS) && opts->x_flag_tm) sorry ("transactional memory is not supported with " "%<-fsanitize=kernel-address%>"); + + /* Comes from final.c -- no real reason to change it. */ +#define MAX_CODE_ALIGN 16 +#define MAX_CODE_ALIGN_VALUE (1 << MAX_CODE_ALIGN) + + if (opts->x_align_loops > MAX_CODE_ALIGN_VALUE) + error_at (loc, "-falign-loops=%d is not between 0 and %d", + opts->x_align_loops, MAX_CODE_ALIGN_VALUE); + + if (opts->x_align_jumps > MAX_CODE_ALIGN_VALUE) + error_at (loc, "-falign-jumps=%d is not between 0 and %d", + opts->x_align_jumps, MAX_CODE_ALIGN_VALUE); + + if (opts->x_align_functions > MAX_CODE_ALIGN_VALUE) + error_at (loc, "-falign-functions=%d is not between 0 and %d", + opts->x_align_functions, MAX_CODE_ALIGN_VALUE); + + if (opts->x_align_labels > MAX_CODE_ALIGN_VALUE) + error_at (loc, "-falign-labels=%d is not between 0 and %d", + opts->x_align_labels, MAX_CODE_ALIGN_VALUE); } #define LEFT_COLUMN 27 diff --git a/gcc/testsuite/gcc.target/i386/pr84310-2.c b/gcc/testsuite/gcc.target/i386/pr84310-2.c new file mode 100644 index 00000000000..e39a421e8d2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr84310-2.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -malign-loops=16" } */ +/* { dg-warning "is obsolete" "" { target *-*-* } 0 } */ + +void +c (void) +{ + for (;;) + ; +} diff --git a/gcc/testsuite/gcc.target/i386/pr84310.c b/gcc/testsuite/gcc.target/i386/pr84310.c new file mode 100644 index 00000000000..f82327e45f3 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr84310.c @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -falign-functions=100000" } */ +/* { dg-error "is not between 0 and 65536" "" { target *-*-* } 0 } */ + +void +test_func (void) +{ +}