On Thu, Jan 4, 2018 at 10:27 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> This ICEs because split2 splits a HImode rotate by 8 into an instruction
> that isn't really valid (bswaphi_lowpart insn, which has preferred_for_size
> enabled just the first alternative that uses +Q, while the splitter creates
> insn with %r9) and the second alternative with +r should be disabled.
>
> For some reason we don't ICE on that, but when -mmitigate-rop mach subpass
> uses regrename framework and changes that %r9 register to yet another non-Q
> register (%si) it isn't recognized because the second alternative is
> disabled.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2018-01-04 Jakub Jelinek <ja...@redhat.com>
> Uros Bizjak <ubiz...@gmail.com>
>
> PR target/83554
> * config/i386/i386.md (*<rotate_insn>hi3_1 splitter): Use
> QIreg_operand instead of register_operand predicate.
> * config/i386/i386.c (ix86_rop_should_change_byte_p,
> set_rop_modrm_reg_bits, ix86_mitigate_rop): Use -mmitigate-rop in
> comments instead of -fmitigate[-_]rop.
>
> * gcc.target/i386/pr83554.c: New test.
OK for mainline and backports.
Thanks,
Uros.
> --- gcc/config/i386/i386.md.jj 2018-01-04 12:37:23.609487117 +0100
> +++ gcc/config/i386/i386.md 2018-01-04 14:23:09.436675176 +0100
> @@ -11643,7 +11643,7 @@ (define_insn "*<rotate_insn>qi3_1_slp"
> (set_attr "mode" "QI")])
>
> (define_split
> - [(set (match_operand:HI 0 "register_operand")
> + [(set (match_operand:HI 0 "QIreg_operand")
> (any_rotate:HI (match_dup 0) (const_int 8)))
> (clobber (reg:CC FLAGS_REG))]
> "reload_completed
> --- gcc/config/i386/i386.c.jj 2018-01-04 12:37:23.611487117 +0100
> +++ gcc/config/i386/i386.c 2018-01-04 14:22:24.490657996 +0100
> @@ -3089,7 +3089,7 @@ ix86_debug_options (void)
> }
>
> /* Return true if T is one of the bytes we should avoid with
> - -fmitigate-rop. */
> + -mmitigate-rop. */
>
> static bool
> ix86_rop_should_change_byte_p (int t)
> @@ -41175,7 +41175,7 @@ ix86_seh_fixup_eh_fallthru (void)
> /* Given a register number BASE, the lowest of a group of registers, update
> regsets IN and OUT with the registers that should be avoided in input
> and output operands respectively when trying to avoid generating a modr/m
> - byte for -fmitigate-rop. */
> + byte for -mmitigate-rop. */
>
> static void
> set_rop_modrm_reg_bits (int base, HARD_REG_SET &in, HARD_REG_SET &out)
> @@ -41186,7 +41186,7 @@ set_rop_modrm_reg_bits (int base, HARD_R
> SET_HARD_REG_BIT (in, base + 3);
> }
>
> -/* Called if -fmitigate_rop is in effect. Try to rewrite instructions so
> +/* Called if -mmitigate-rop is in effect. Try to rewrite instructions so
> that certain encodings of modr/m bytes do not occur. */
> static void
> ix86_mitigate_rop (void)
> --- gcc/testsuite/gcc.target/i386/pr83554.c.jj 2018-01-04 14:22:24.491657997
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr83554.c 2018-01-04 14:22:24.491657997
> +0100
> @@ -0,0 +1,15 @@
> +/* PR target/83554 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-Os -mmitigate-rop" } */
> +
> +unsigned a;
> +unsigned __int128
> +foo (unsigned char c, unsigned short d, unsigned e, unsigned long f,
> + unsigned __int128 g, unsigned h, unsigned short i, unsigned long j)
> +{
> + j /= (unsigned)-2;
> + h += h &= c == c;
> + h -= g;
> + i = i >> 8 | i << 8;
> + return c + d + e + f + g + h + i + j + a;
> +}
>
> Jakub
