* [PATCH] Fix endless recursion in decide_alg (PR target/69888)
@ 2016-02-22 21:16 Jakub Jelinek
2016-02-22 21:27 ` Uros Bizjak
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2016-02-22 21:16 UTC (permalink / raw)
To: Uros Bizjak, Jan Hubicka; +Cc: gcc-patches
Hi!
On the following testcase, we ICE due to infinite recursion in decide_alg
- max is 0, and expected_size is the same (2048) in the second and following
recursive calls.
Fixed by avoiding recursing with the same arguments as before.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-02-22 Jakub Jelinek <jakub@redhat.com>
PR target/69888
* config/i386/i386.c (decide_alg): Ensure we don't recurse with
identical arguments. Formatting and spelling fixes.
* gcc.target/i386/pr69888.c: New test.
--- gcc/config/i386/i386.c.jj 2016-02-12 00:50:55.000000000 +0100
+++ gcc/config/i386/i386.c 2016-02-22 10:25:53.564338145 +0100
@@ -26032,11 +26032,12 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
bool memset, bool zero_memset, bool have_as,
int *dynamic_check, bool *noalign)
{
- const struct stringop_algs * algs;
+ const struct stringop_algs *algs;
bool optimize_for_speed;
int max = 0;
const struct processor_costs *cost;
int i;
+ HOST_WIDE_INT orig_expected_size = expected_size;
bool any_alg_usable_p = false;
*noalign = false;
@@ -26066,7 +26067,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
any_alg_usable_p |= usable;
if (candidate != libcall && candidate && usable)
- max = algs->size[i].max;
+ max = algs->size[i].max;
}
/* If expected size is not known but max size is small enough
@@ -26076,7 +26077,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
&& expected_size == -1)
expected_size = min_size / 2 + max_size / 2;
- /* If user specified the algorithm, honnor it if possible. */
+ /* If user specified the algorithm, honor it if possible. */
if (ix86_stringop_alg != no_stringop
&& alg_usable_p (ix86_stringop_alg, memset, have_as))
return ix86_stringop_alg;
@@ -26152,20 +26153,20 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
|| !alg_usable_p (algs->unknown_size, memset, have_as)))
{
enum stringop_alg alg;
+ HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
- /* If there aren't any usable algorithms, then recursing on
- smaller sizes isn't going to find anything. Just return the
- simple byte-at-a-time copy loop. */
- if (!any_alg_usable_p)
+ /* If there aren't any usable algorithms or if recursing with the
+ same arguments as before, then recursing on smaller sizes or
+ same size isn't going to find anything. Just return the simple
+ byte-at-a-time copy loop. */
+ if (!any_alg_usable_p || orig_expected_size == new_expected_size)
{
/* Pick something reasonable. */
if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
*dynamic_check = 128;
return loop_1_byte;
}
- if (max <= 0)
- max = 4096;
- alg = decide_alg (count, max / 2, min_size, max_size, memset,
+ alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
zero_memset, have_as, dynamic_check, noalign);
gcc_assert (*dynamic_check == -1);
if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
--- gcc/testsuite/gcc.target/i386/pr69888.c.jj 2016-02-22 10:31:52.529401396 +0100
+++ gcc/testsuite/gcc.target/i386/pr69888.c 2016-02-22 10:34:10.852502628 +0100
@@ -0,0 +1,10 @@
+/* PR target/69888 */
+/* { dg-do compile } */
+/* { dg-options "-minline-all-stringops -mmemset-strategy=no_stringop:-1:noalign" } */
+/* { dg-additional-options "-march=geode" { target ia32 } } */
+
+void
+foo (char *p)
+{
+ __builtin_memset (p, 0, 32);
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix endless recursion in decide_alg (PR target/69888)
2016-02-22 21:16 [PATCH] Fix endless recursion in decide_alg (PR target/69888) Jakub Jelinek
@ 2016-02-22 21:27 ` Uros Bizjak
2016-02-22 21:32 ` Uros Bizjak
0 siblings, 1 reply; 3+ messages in thread
From: Uros Bizjak @ 2016-02-22 21:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
On Mon, Feb 22, 2016 at 10:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the following testcase, we ICE due to infinite recursion in decide_alg
> - max is 0, and expected_size is the same (2048) in the second and following
> recursive calls.
>
> Fixed by avoiding recursing with the same arguments as before.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-02-22 Jakub Jelinek <jakub@redhat.com>
>
> PR target/69888
> * config/i386/i386.c (decide_alg): Ensure we don't recurse with
> identical arguments. Formatting and spelling fixes.
>
> * gcc.target/i386/pr69888.c: New test.
OK with a nit below.
Thanks,
Uros.
>
> --- gcc/config/i386/i386.c.jj 2016-02-12 00:50:55.000000000 +0100
> +++ gcc/config/i386/i386.c 2016-02-22 10:25:53.564338145 +0100
> @@ -26032,11 +26032,12 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
> bool memset, bool zero_memset, bool have_as,
> int *dynamic_check, bool *noalign)
> {
> - const struct stringop_algs * algs;
> + const struct stringop_algs *algs;
> bool optimize_for_speed;
> int max = 0;
> const struct processor_costs *cost;
> int i;
> + HOST_WIDE_INT orig_expected_size = expected_size;
> bool any_alg_usable_p = false;
>
> *noalign = false;
> @@ -26066,7 +26067,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
> any_alg_usable_p |= usable;
>
> if (candidate != libcall && candidate && usable)
> - max = algs->size[i].max;
> + max = algs->size[i].max;
> }
>
> /* If expected size is not known but max size is small enough
> @@ -26076,7 +26077,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
> && expected_size == -1)
> expected_size = min_size / 2 + max_size / 2;
>
> - /* If user specified the algorithm, honnor it if possible. */
> + /* If user specified the algorithm, honor it if possible. */
> if (ix86_stringop_alg != no_stringop
> && alg_usable_p (ix86_stringop_alg, memset, have_as))
> return ix86_stringop_alg;
> @@ -26152,20 +26153,20 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
> || !alg_usable_p (algs->unknown_size, memset, have_as)))
> {
> enum stringop_alg alg;
> + HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
HOST_WIDE_INT new_expected_size = max > 0 ? max / 2 : 2048;
> - /* If there aren't any usable algorithms, then recursing on
> - smaller sizes isn't going to find anything. Just return the
> - simple byte-at-a-time copy loop. */
> - if (!any_alg_usable_p)
> + /* If there aren't any usable algorithms or if recursing with the
> + same arguments as before, then recursing on smaller sizes or
> + same size isn't going to find anything. Just return the simple
> + byte-at-a-time copy loop. */
> + if (!any_alg_usable_p || orig_expected_size == new_expected_size)
> {
> /* Pick something reasonable. */
> if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> *dynamic_check = 128;
> return loop_1_byte;
> }
> - if (max <= 0)
> - max = 4096;
> - alg = decide_alg (count, max / 2, min_size, max_size, memset,
> + alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
> zero_memset, have_as, dynamic_check, noalign);
> gcc_assert (*dynamic_check == -1);
> if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> --- gcc/testsuite/gcc.target/i386/pr69888.c.jj 2016-02-22 10:31:52.529401396 +0100
> +++ gcc/testsuite/gcc.target/i386/pr69888.c 2016-02-22 10:34:10.852502628 +0100
> @@ -0,0 +1,10 @@
> +/* PR target/69888 */
> +/* { dg-do compile } */
> +/* { dg-options "-minline-all-stringops -mmemset-strategy=no_stringop:-1:noalign" } */
> +/* { dg-additional-options "-march=geode" { target ia32 } } */
> +
> +void
> +foo (char *p)
> +{
> + __builtin_memset (p, 0, 32);
> +}
>
> Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix endless recursion in decide_alg (PR target/69888)
2016-02-22 21:27 ` Uros Bizjak
@ 2016-02-22 21:32 ` Uros Bizjak
0 siblings, 0 replies; 3+ messages in thread
From: Uros Bizjak @ 2016-02-22 21:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
On Mon, Feb 22, 2016 at 10:27 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Feb 22, 2016 at 10:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> Hi!
>>
>> On the following testcase, we ICE due to infinite recursion in decide_alg
>> - max is 0, and expected_size is the same (2048) in the second and following
>> recursive calls.
>>
>> Fixed by avoiding recursing with the same arguments as before.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> 2016-02-22 Jakub Jelinek <jakub@redhat.com>
>>
>> PR target/69888
>> * config/i386/i386.c (decide_alg): Ensure we don't recurse with
>> identical arguments. Formatting and spelling fixes.
>>
>> * gcc.target/i386/pr69888.c: New test.
>
> OK with a nit below.
On the second thought, the patch is OK as is. It documents that new
expected size is half of the value.
Thanks,
Uros.
>>
>> --- gcc/config/i386/i386.c.jj 2016-02-12 00:50:55.000000000 +0100
>> +++ gcc/config/i386/i386.c 2016-02-22 10:25:53.564338145 +0100
>> @@ -26032,11 +26032,12 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
>> bool memset, bool zero_memset, bool have_as,
>> int *dynamic_check, bool *noalign)
>> {
>> - const struct stringop_algs * algs;
>> + const struct stringop_algs *algs;
>> bool optimize_for_speed;
>> int max = 0;
>> const struct processor_costs *cost;
>> int i;
>> + HOST_WIDE_INT orig_expected_size = expected_size;
>> bool any_alg_usable_p = false;
>>
>> *noalign = false;
>> @@ -26066,7 +26067,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
>> any_alg_usable_p |= usable;
>>
>> if (candidate != libcall && candidate && usable)
>> - max = algs->size[i].max;
>> + max = algs->size[i].max;
>> }
>>
>> /* If expected size is not known but max size is small enough
>> @@ -26076,7 +26077,7 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
>> && expected_size == -1)
>> expected_size = min_size / 2 + max_size / 2;
>>
>> - /* If user specified the algorithm, honnor it if possible. */
>> + /* If user specified the algorithm, honor it if possible. */
>> if (ix86_stringop_alg != no_stringop
>> && alg_usable_p (ix86_stringop_alg, memset, have_as))
>> return ix86_stringop_alg;
>> @@ -26152,20 +26153,20 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
>> || !alg_usable_p (algs->unknown_size, memset, have_as)))
>> {
>> enum stringop_alg alg;
>> + HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
>
> HOST_WIDE_INT new_expected_size = max > 0 ? max / 2 : 2048;
>
>> - /* If there aren't any usable algorithms, then recursing on
>> - smaller sizes isn't going to find anything. Just return the
>> - simple byte-at-a-time copy loop. */
>> - if (!any_alg_usable_p)
>> + /* If there aren't any usable algorithms or if recursing with the
>> + same arguments as before, then recursing on smaller sizes or
>> + same size isn't going to find anything. Just return the simple
>> + byte-at-a-time copy loop. */
>> + if (!any_alg_usable_p || orig_expected_size == new_expected_size)
>> {
>> /* Pick something reasonable. */
>> if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
>> *dynamic_check = 128;
>> return loop_1_byte;
>> }
>> - if (max <= 0)
>> - max = 4096;
>> - alg = decide_alg (count, max / 2, min_size, max_size, memset,
>> + alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
>> zero_memset, have_as, dynamic_check, noalign);
>> gcc_assert (*dynamic_check == -1);
>> if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
>> --- gcc/testsuite/gcc.target/i386/pr69888.c.jj 2016-02-22 10:31:52.529401396 +0100
>> +++ gcc/testsuite/gcc.target/i386/pr69888.c 2016-02-22 10:34:10.852502628 +0100
>> @@ -0,0 +1,10 @@
>> +/* PR target/69888 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-minline-all-stringops -mmemset-strategy=no_stringop:-1:noalign" } */
>> +/* { dg-additional-options "-march=geode" { target ia32 } } */
>> +
>> +void
>> +foo (char *p)
>> +{
>> + __builtin_memset (p, 0, 32);
>> +}
>>
>> Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-22 21:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 21:16 [PATCH] Fix endless recursion in decide_alg (PR target/69888) Jakub Jelinek
2016-02-22 21:27 ` Uros Bizjak
2016-02-22 21:32 ` 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).