public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix ICE with x86_64 alloca (PR target/65693)
@ 2015-04-08 20:52 Jakub Jelinek
  2015-04-09  6:23 ` Uros Bizjak
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2015-04-08 20:52 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

Hi!

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  <jakub@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.

--- 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix ICE with x86_64 alloca (PR target/65693)
  2015-04-08 20:52 [PATCH] Fix ICE with x86_64 alloca (PR target/65693) Jakub Jelinek
@ 2015-04-09  6:23 ` Uros Bizjak
  0 siblings, 0 replies; 2+ messages in thread
From: Uros Bizjak @ 2015-04-09  6:23 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Segher Boessenkool, Vladimir Makarov

On Wed, Apr 8, 2015 at 10:52 PM, Jakub Jelinek <jakub@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  <jakub@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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-04-09  6:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-08 20:52 [PATCH] Fix ICE with x86_64 alloca (PR target/65693) Jakub Jelinek
2015-04-09  6:23 ` Uros Bizjak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).