On Wed, Apr 8, 2015 at 10:52 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> This patch extends the PR65220 patch: > 1) there is really no reason to limit the divisor to 32 or 64, > we can divide/modulo by pow2 constants up to 2G (0x7fffffff is > then still representable in 32-bit signed immediate) > 2) on the testcase RTL cprop unfortunately isn't performed, because > the function contains only a single basic block, and the combiner > when the constant is propagated into it simplifies it to the > shift and and; so the patch adds another pattern similar to the previous > one to handle this case > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2015-04-08 Jakub Jelinek <ja...@redhat.com> > > PR target/65693 > * config/i386/i386.md (*udivmod<mode>4_pow2): Allow > any pow2 integer in between 2 and 0x80000000U inclusive. > (*udivmod<mode>4_pow2_1): New define_insn_and_split. > > * gcc.target/i386/pr65693.c: New test. Part 1) is OK and is IMO a good improvement. The combiner part 2) has just been fixed in a generic way [1], so it looks that the new pattern is not needed. There still remains the issue with "asm(%rdx")" that effectively blocks usage of rdx register in the function. Does RA declares the register as fixed in this case? [1] https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00359.html Thanks, Uros. > --- gcc/config/i386/i386.md.jj 2015-04-03 15:32:30.000000000 +0200 > +++ gcc/config/i386/i386.md 2015-04-08 17:30:10.615369134 +0200 > @@ -7340,7 +7340,7 @@ (define_insn_and_split "*udivmod<mode>4_ > (set (match_operand:SWI48 1 "register_operand" "=r") > (umod:SWI48 (match_dup 2) (match_dup 3))) > (clobber (reg:CC FLAGS_REG))] > - "UINTVAL (operands[3]) - 2 < <MODE_SIZE> * BITS_PER_UNIT > + "IN_RANGE (INTVAL (operands[3]), 2, HOST_WIDE_INT_UC (0x80000000)) > && (UINTVAL (operands[3]) & (UINTVAL (operands[3]) - 1)) == 0" > "#" > "&& 1" > @@ -7357,6 +7357,27 @@ (define_insn_and_split "*udivmod<mode>4_ > [(set_attr "type" "multi") > (set_attr "mode" "<MODE>")]) > > +;; Similarly, but for the case when the combiner simplifies it. > +(define_insn_and_split "*udivmod<mode>4_pow2_1" > + [(set (match_operand:SWI48 0 "register_operand" "=r") > + (lshiftrt:SWI48 (match_operand:SWI48 2 "register_operand" "0") > + (match_operand:SWI48 3 "const_int_operand" "n"))) > + (set (match_operand:SWI48 1 "register_operand" "=r") > + (and:SWI48 (match_dup 2) (match_operand:SWI48 4 "const_int_operand" > "n"))) > + (clobber (reg:CC FLAGS_REG))] > + "IN_RANGE (INTVAL (operands[3]), 1, 31) > + && UINTVAL (operands[4]) == (HOST_WIDE_INT_1U << INTVAL (operands[3])) - > 1" > + "#" > + "&& 1" > + [(set (match_dup 1) (match_dup 2)) > + (parallel [(set (match_dup 0) (lshiftrt:<MODE> (match_dup 2) (match_dup > 3))) > + (clobber (reg:CC FLAGS_REG))]) > + (parallel [(set (match_dup 1) (and:<MODE> (match_dup 1) (match_dup 4))) > + (clobber (reg:CC FLAGS_REG))])] > + "" > + [(set_attr "type" "multi") > + (set_attr "mode" "<MODE>")]) > + > (define_insn "*udivmod<mode>4_noext" > [(set (match_operand:SWIM248 0 "register_operand" "=a") > (udiv:SWIM248 (match_operand:SWIM248 2 "register_operand" "0") > --- gcc/testsuite/gcc.target/i386/pr65693.c.jj 2015-04-08 17:42:14.146727788 > +0200 > +++ gcc/testsuite/gcc.target/i386/pr65693.c 2015-04-08 17:41:22.000000000 > +0200 > @@ -0,0 +1,13 @@ > +/* PR target/65693 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +int a; > + > +void > +foo (int (*fn) (int, int, int), unsigned int b) > +{ > + unsigned long *c = (unsigned long *) __builtin_alloca (b); > + a = *c; > + register int d asm ("edx") = fn (0, 0, d); > +} > > Jakub