| Message ID | 20180301231040.GU5867@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix ICE on invalid inline asm with "X" constraint and non-zero CONST_VECTOR (PR inline-asm/84625) | expand |
On Fri, Mar 2, 2018 at 12:10 AM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > Assertions are only useful when inline asm is not involved, otherwise users > can write anything they want. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2018-03-02 Jakub Jelinek <jakub@redhat.com> > > PR inline-asm/84625 > * config/i386/i386.c (ix86_print_operand): Use conditional > output_operand_lossage instead of gcc_assert if CONST_VECTOR is not > zero vector. > > * gcc.target/i386/pr84625.c: New test. OK. Thanks, Uros. > --- gcc/config/i386/i386.c.jj 2018-02-26 20:49:57.345331383 +0100 > +++ gcc/config/i386/i386.c 2018-03-01 12:01:17.820587068 +0100 > @@ -18743,7 +18743,8 @@ ix86_print_operand (FILE *file, rtx x, i > since we can in fact encode that into an immediate. */ > if (GET_CODE (x) == CONST_VECTOR) > { > - gcc_assert (x == CONST0_RTX (GET_MODE (x))); > + if (x != CONST0_RTX (GET_MODE (x))) > + output_operand_lossage ("invalid vector immediate"); > x = const0_rtx; > } > > --- gcc/testsuite/gcc.target/i386/pr84625.c.jj 2018-03-01 12:00:06.254636914 +0100 > +++ gcc/testsuite/gcc.target/i386/pr84625.c 2018-03-01 12:14:54.044024998 +0100 > @@ -0,0 +1,12 @@ > +/* PR inline-asm/84625 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -msse2" } */ > + > +typedef int V __attribute__((vector_size (16))); > + > +void > +foo (void) > +{ > + asm volatile ("# %0" : : "X" ((V) { 1, 2, 3, 4 })); // { dg-error "invalid vector immediate" } > + asm volatile ("# %0" : : "" ((V) { 2, 3, 4, 5 })); // { dg-error "invalid vector immediate" } > +} > > Jakub
On Fri, Mar 2, 2018 at 8:16 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, Mar 2, 2018 at 12:10 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> Hi! >> >> Assertions are only useful when inline asm is not involved, otherwise users >> can write anything they want. IIRC, we can also handle { -1, -1, ... , -1 } in certain cases, but I don't think it is worth to complicate here. Uros.
On Fri, Mar 02, 2018 at 08:19:40AM +0100, Uros Bizjak wrote: > On Fri, Mar 2, 2018 at 8:16 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > > On Fri, Mar 2, 2018 at 12:10 AM, Jakub Jelinek <jakub@redhat.com> wrote: > >> Hi! > >> > >> Assertions are only useful when inline asm is not involved, otherwise users > >> can write anything they want. > > IIRC, we can also handle { -1, -1, ... , -1 } in certain cases, but I > don't think it is worth to complicate here. We can handle that as whole instruction special-casing all ones CONST_VECTOR, sure, but as an operand in inline-asm? Even the { 0, 0, ... , 0 } case is weird, we print it just as 0, dunno where exactly it would make sense, but we were doing that in the past already. Jakub
On Fri, Mar 2, 2018 at 8:47 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Mar 02, 2018 at 08:19:40AM +0100, Uros Bizjak wrote: >> On Fri, Mar 2, 2018 at 8:16 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> > On Fri, Mar 2, 2018 at 12:10 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> >> Hi! >> >> >> >> Assertions are only useful when inline asm is not involved, otherwise users >> >> can write anything they want. >> >> IIRC, we can also handle { -1, -1, ... , -1 } in certain cases, but I >> don't think it is worth to complicate here. > > We can handle that as whole instruction special-casing all ones > CONST_VECTOR, sure, but as an operand in inline-asm? > > Even the { 0, 0, ... , 0 } case is weird, we print it just as 0, dunno > where exactly it would make sense, but we were doing that in the past > already. Probably QImode/HImode vector zero can be represented as DImode immediate in an integer move insn. Anyway, let's proceed with your original patch. Uros.
--- gcc/config/i386/i386.c.jj 2018-02-26 20:49:57.345331383 +0100 +++ gcc/config/i386/i386.c 2018-03-01 12:01:17.820587068 +0100 @@ -18743,7 +18743,8 @@ ix86_print_operand (FILE *file, rtx x, i since we can in fact encode that into an immediate. */ if (GET_CODE (x) == CONST_VECTOR) { - gcc_assert (x == CONST0_RTX (GET_MODE (x))); + if (x != CONST0_RTX (GET_MODE (x))) + output_operand_lossage ("invalid vector immediate"); x = const0_rtx; } --- gcc/testsuite/gcc.target/i386/pr84625.c.jj 2018-03-01 12:00:06.254636914 +0100 +++ gcc/testsuite/gcc.target/i386/pr84625.c 2018-03-01 12:14:54.044024998 +0100 @@ -0,0 +1,12 @@ +/* PR inline-asm/84625 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -msse2" } */ + +typedef int V __attribute__((vector_size (16))); + +void +foo (void) +{ + asm volatile ("# %0" : : "X" ((V) { 1, 2, 3, 4 })); // { dg-error "invalid vector immediate" } + asm volatile ("# %0" : : "" ((V) { 2, 3, 4, 5 })); // { dg-error "invalid vector immediate" } +}