* [PATCH] Another fix for decide_alg (PR target/70062)
@ 2016-03-03 20:16 Jakub Jelinek
2016-03-04 9:59 ` Uros Bizjak
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2016-03-03 20:16 UTC (permalink / raw)
To: Jan Hubicka, Uros Bizjak; +Cc: gcc-patches
Hi!
Before my recent decide_alg change, *dynamic_check == -1 was indeed
guaranteed, because any_alg_usable_p doesn't depend on the arguments of
decide_alg that might change during recursive call, so we'd only recurse if
it wouldn't set *dynamic_check. But, if we give up because we'd otherwise
recurse infinitely, we can set *dynamic_check to 128.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2016-03-03 Jakub Jelinek <jakub@redhat.com>
PR target/70062
* config/i386/i386.c (decide_alg): If
TARGET_INLINE_STRINGOPS_DYNAMICALLY, allow *dynamic_check to be also
128 from the recursive call.
* gcc.target/i386/pr70062.c: New test.
--- gcc/config/i386/i386.c.jj 2016-03-02 14:08:00.000000000 +0100
+++ gcc/config/i386/i386.c 2016-03-03 17:48:18.587450348 +0100
@@ -26170,11 +26170,15 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
}
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)
- *dynamic_check = max;
+ {
+ /* *dynamic_check could be set 128 above because we avoided
+ infinite recursion. */
+ gcc_assert (*dynamic_check == -1 || *dynamic_check == 128);
+ *dynamic_check = max;
+ }
else
- gcc_assert (alg != libcall);
+ gcc_assert (alg != libcall && *dynamic_check == -1);
return alg;
}
return (alg_usable_p (algs->unknown_size, memset, have_as)
--- gcc/testsuite/gcc.target/i386/pr70062.c.jj 2016-03-03 17:54:13.167642050 +0100
+++ gcc/testsuite/gcc.target/i386/pr70062.c 2016-03-03 17:54:58.753023808 +0100
@@ -0,0 +1,11 @@
+/* PR target/70062 */
+/* { dg-options "-minline-all-stringops -minline-stringops-dynamically -mmemcpy-strategy=libcall:-1:noalign -Wno-psabi" } */
+/* { dg-additional-options "-mtune=k6-2" { target ia32 } } */
+
+typedef int V __attribute__ ((vector_size (32)));
+
+V
+foo (V x)
+{
+ return (V) { x[0] };
+}
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-03 20:16 [PATCH] Another fix for decide_alg (PR target/70062) Jakub Jelinek
@ 2016-03-04 9:59 ` Uros Bizjak
2016-03-04 10:06 ` Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Uros Bizjak @ 2016-03-04 9:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
On Thu, Mar 3, 2016 at 9:16 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Before my recent decide_alg change, *dynamic_check == -1 was indeed
> guaranteed, because any_alg_usable_p doesn't depend on the arguments of
> decide_alg that might change during recursive call, so we'd only recurse if
> it wouldn't set *dynamic_check. But, if we give up because we'd otherwise
> recurse infinitely, we can set *dynamic_check to 128.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2016-03-03 Jakub Jelinek <jakub@redhat.com>
>
> PR target/70062
> * config/i386/i386.c (decide_alg): If
> TARGET_INLINE_STRINGOPS_DYNAMICALLY, allow *dynamic_check to be also
> 128 from the recursive call.
>
> * gcc.target/i386/pr70062.c: New test.
I don't like the fact that *dynamic_check is set to max (which is 0
with your testcase) when recursion avoidance code already set it to
"something reasonable", together with loop_1_byte alg. What do you
think about attached (lightly tested) patch?
Uros.
[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 956 bytes --]
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 8a026ae..ded9951 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
}
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)
- *dynamic_check = max;
+ {
+ /* *dynamic_check could be set to 128 above because we avoided
+ infinite recursion. */
+ if (*dynamic_check == 128)
+ gcc_assert (alg == loop_1_byte);
+ else
+ {
+ gcc_assert (*dynamic_check == -1);
+ *dynamic_check = max;
+ }
+ }
else
- gcc_assert (alg != libcall);
+ gcc_assert (alg != libcall && *dynamic_check == -1);
+
return alg;
}
return (alg_usable_p (algs->unknown_size, memset, have_as)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 9:59 ` Uros Bizjak
@ 2016-03-04 10:06 ` Jakub Jelinek
2016-03-04 10:16 ` Uros Bizjak
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2016-03-04 10:06 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
> I don't like the fact that *dynamic_check is set to max (which is 0
> with your testcase) when recursion avoidance code already set it to
> "something reasonable", together with loop_1_byte alg. What do you
> think about attached (lightly tested) patch?
But that can still set *dynamic_check to 0 if the recursive call has
not set *dynamic_check.
So, perhaps we want *dynamic_check = max ? max : 128;
?
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 8a026ae..ded9951 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
> }
> 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)
> - *dynamic_check = max;
> + {
> + /* *dynamic_check could be set to 128 above because we avoided
> + infinite recursion. */
> + if (*dynamic_check == 128)
> + gcc_assert (alg == loop_1_byte);
> + else
> + {
> + gcc_assert (*dynamic_check == -1);
> + *dynamic_check = max;
> + }
> + }
> else
> - gcc_assert (alg != libcall);
> + gcc_assert (alg != libcall && *dynamic_check == -1);
> +
> return alg;
> }
> return (alg_usable_p (algs->unknown_size, memset, have_as)
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 10:06 ` Jakub Jelinek
@ 2016-03-04 10:16 ` Uros Bizjak
2016-03-04 10:34 ` Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Uros Bizjak @ 2016-03-04 10:16 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> I don't like the fact that *dynamic_check is set to max (which is 0
>> with your testcase) when recursion avoidance code already set it to
>> "something reasonable", together with loop_1_byte alg. What do you
>> think about attached (lightly tested) patch?
>
> But that can still set *dynamic_check to 0 if the recursive call has
> not set *dynamic_check.
> So, perhaps we want *dynamic_check = max ? max : 128;
> ?
I believe that recursive call set *dynamic_check to zero for a reason.
The sent patch deals with recursion breaking issues only, leaving all
other functionality as it was before. So, your issue is IMO orthogonal
to the PR70062 and should be fixed (if at all) in a separate patch.
Uros.
>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 8a026ae..ded9951 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -26170,11 +26170,22 @@ decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
>> }
>> 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)
>> - *dynamic_check = max;
>> + {
>> + /* *dynamic_check could be set to 128 above because we avoided
>> + infinite recursion. */
>> + if (*dynamic_check == 128)
>> + gcc_assert (alg == loop_1_byte);
>> + else
>> + {
>> + gcc_assert (*dynamic_check == -1);
>> + *dynamic_check = max;
>> + }
>> + }
>> else
>> - gcc_assert (alg != libcall);
>> + gcc_assert (alg != libcall && *dynamic_check == -1);
>> +
>> return alg;
>> }
>> return (alg_usable_p (algs->unknown_size, memset, have_as)
>
>
> Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 10:16 ` Uros Bizjak
@ 2016-03-04 10:34 ` Jakub Jelinek
2016-03-04 10:42 ` Uros Bizjak
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2016-03-04 10:34 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
> >> I don't like the fact that *dynamic_check is set to max (which is 0
> >> with your testcase) when recursion avoidance code already set it to
> >> "something reasonable", together with loop_1_byte alg. What do you
> >> think about attached (lightly tested) patch?
> >
> > But that can still set *dynamic_check to 0 if the recursive call has
> > not set *dynamic_check.
> > So, perhaps we want *dynamic_check = max ? max : 128;
> > ?
>
> I believe that recursive call set *dynamic_check to zero for a reason.
> The sent patch deals with recursion breaking issues only, leaving all
> other functionality as it was before. So, your issue is IMO orthogonal
> to the PR70062 and should be fixed (if at all) in a separate patch.
The recursive call should never set *dynamic_check to 0, only to
-1 or 128 (the last one newly, since my fix).
And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
is ok, or it is not.
Perhaps better would be to add an extra argument to decide_alg, false
for toplevel invocation, true for recursive invocation, and if recursive
call, just never try to recurse again (thus we could revert the previous
change) and don't set *dynamic_check to anything in that case during the
recursion.
And then at the caller side decide what we want for max == -1 and what
for max == 0 with *dynamic_check.
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 10:34 ` Jakub Jelinek
@ 2016-03-04 10:42 ` Uros Bizjak
2016-03-04 11:11 ` Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Uros Bizjak @ 2016-03-04 10:42 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
>> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> >> I don't like the fact that *dynamic_check is set to max (which is 0
>> >> with your testcase) when recursion avoidance code already set it to
>> >> "something reasonable", together with loop_1_byte alg. What do you
>> >> think about attached (lightly tested) patch?
>> >
>> > But that can still set *dynamic_check to 0 if the recursive call has
>> > not set *dynamic_check.
>> > So, perhaps we want *dynamic_check = max ? max : 128;
>> > ?
>>
>> I believe that recursive call set *dynamic_check to zero for a reason.
>> The sent patch deals with recursion breaking issues only, leaving all
>> other functionality as it was before. So, your issue is IMO orthogonal
>> to the PR70062 and should be fixed (if at all) in a separate patch.
>
> The recursive call should never set *dynamic_check to 0, only to
> -1 or 128 (the last one newly, since my fix).
> And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
> is ok, or it is not.
> Perhaps better would be to add an extra argument to decide_alg, false
> for toplevel invocation, true for recursive invocation, and if recursive
> call, just never try to recurse again (thus we could revert the previous
> change) and don't set *dynamic_check to anything in that case during the
> recursion.
> And then at the caller side decide what we want for max == -1 and what
> for max == 0 with *dynamic_check.
I think that would work, and considering that we have only one
non-recursive callsite of decide_alg, it looks like the cleanest
solution.
Uros.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 10:42 ` Uros Bizjak
@ 2016-03-04 11:11 ` Jakub Jelinek
2016-03-04 11:19 ` Uros Bizjak
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2016-03-04 11:11 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 04, 2016 at 11:42:34AM +0100, Uros Bizjak wrote:
> On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
> >> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> >> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
> >> >> I don't like the fact that *dynamic_check is set to max (which is 0
> >> >> with your testcase) when recursion avoidance code already set it to
> >> >> "something reasonable", together with loop_1_byte alg. What do you
> >> >> think about attached (lightly tested) patch?
> >> >
> >> > But that can still set *dynamic_check to 0 if the recursive call has
> >> > not set *dynamic_check.
> >> > So, perhaps we want *dynamic_check = max ? max : 128;
> >> > ?
> >>
> >> I believe that recursive call set *dynamic_check to zero for a reason.
> >> The sent patch deals with recursion breaking issues only, leaving all
> >> other functionality as it was before. So, your issue is IMO orthogonal
> >> to the PR70062 and should be fixed (if at all) in a separate patch.
> >
> > The recursive call should never set *dynamic_check to 0, only to
> > -1 or 128 (the last one newly, since my fix).
> > And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
> > is ok, or it is not.
> > Perhaps better would be to add an extra argument to decide_alg, false
> > for toplevel invocation, true for recursive invocation, and if recursive
> > call, just never try to recurse again (thus we could revert the previous
> > change) and don't set *dynamic_check to anything in that case during the
> > recursion.
> > And then at the caller side decide what we want for max == -1 and what
> > for max == 0 with *dynamic_check.
>
> I think that would work, and considering that we have only one
> non-recursive callsite of decide_alg, it looks like the cleanest
> solution.
So like this?
2016-03-03 Jakub Jelinek <jakub@redhat.com>
PR target/70062
* config/i386/i386.c (decide_alg): Add RECUR argument. Revert
2016-02-22 changes, instead don't recurse if RECUR is already true.
Don't change *dynamic_check if RECUR. Adjust recursive caller
to pass true to the new argument.
(ix86_expand_set_or_movmem): Adjust decide_alg caller.
* gcc.target/i386/pr70062.c: New test.
--- gcc/config/i386/i386.c.jj 2016-03-03 21:49:38.535744904 +0100
+++ gcc/config/i386/i386.c 2016-03-04 12:06:40.075300426 +0100
@@ -26032,14 +26032,13 @@ static enum stringop_alg
decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
unsigned HOST_WIDE_INT min_size, unsigned HOST_WIDE_INT max_size,
bool memset, bool zero_memset, bool have_as,
- int *dynamic_check, bool *noalign)
+ int *dynamic_check, bool *noalign, bool recur)
{
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;
@@ -26157,19 +26156,18 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
enum stringop_alg alg;
HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
- /* 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 there aren't any usable algorithms or if recursing already,
+ 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 || recur)
+ {
+ /* Pick something reasonable. */
+ if (TARGET_INLINE_STRINGOPS_DYNAMICALLY && !recur)
+ *dynamic_check = 128;
+ return loop_1_byte;
+ }
alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
- zero_memset, have_as, dynamic_check, noalign);
+ zero_memset, have_as, dynamic_check, noalign, true);
gcc_assert (*dynamic_check == -1);
if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
*dynamic_check = max;
@@ -26430,7 +26428,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx
alg = decide_alg (count, expected_size, min_size, probable_max_size,
issetmem,
issetmem && val_exp == const0_rtx, have_as,
- &dynamic_check, &noalign);
+ &dynamic_check, &noalign, false);
if (alg == libcall)
return false;
gcc_assert (alg != no_stringop);
--- gcc/testsuite/gcc.target/i386/pr70062.c.jj 2016-03-04 12:03:55.240561744 +0100
+++ gcc/testsuite/gcc.target/i386/pr70062.c 2016-03-04 12:03:55.240561744 +0100
@@ -0,0 +1,11 @@
+/* PR target/70062 */
+/* { dg-options "-minline-all-stringops -minline-stringops-dynamically -mmemcpy-strategy=libcall:-1:noalign -Wno-psabi" } */
+/* { dg-additional-options "-mtune=k6-2" { target ia32 } } */
+
+typedef int V __attribute__ ((vector_size (32)));
+
+V
+foo (V x)
+{
+ return (V) { x[0] };
+}
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Another fix for decide_alg (PR target/70062)
2016-03-04 11:11 ` Jakub Jelinek
@ 2016-03-04 11:19 ` Uros Bizjak
0 siblings, 0 replies; 8+ messages in thread
From: Uros Bizjak @ 2016-03-04 11:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches
On Fri, Mar 4, 2016 at 12:11 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 04, 2016 at 11:42:34AM +0100, Uros Bizjak wrote:
>> On Fri, Mar 4, 2016 at 11:34 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Fri, Mar 04, 2016 at 11:16:44AM +0100, Uros Bizjak wrote:
>> >> On Fri, Mar 4, 2016 at 11:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> >> > On Fri, Mar 04, 2016 at 10:59:48AM +0100, Uros Bizjak wrote:
>> >> >> I don't like the fact that *dynamic_check is set to max (which is 0
>> >> >> with your testcase) when recursion avoidance code already set it to
>> >> >> "something reasonable", together with loop_1_byte alg. What do you
>> >> >> think about attached (lightly tested) patch?
>> >> >
>> >> > But that can still set *dynamic_check to 0 if the recursive call has
>> >> > not set *dynamic_check.
>> >> > So, perhaps we want *dynamic_check = max ? max : 128;
>> >> > ?
>> >>
>> >> I believe that recursive call set *dynamic_check to zero for a reason.
>> >> The sent patch deals with recursion breaking issues only, leaving all
>> >> other functionality as it was before. So, your issue is IMO orthogonal
>> >> to the PR70062 and should be fixed (if at all) in a separate patch.
>> >
>> > The recursive call should never set *dynamic_check to 0, only to
>> > -1 or 128 (the last one newly, since my fix).
>> > And, my issue is IMO not orghogonal to that, either *dynamic_check == 0
>> > is ok, or it is not.
>> > Perhaps better would be to add an extra argument to decide_alg, false
>> > for toplevel invocation, true for recursive invocation, and if recursive
>> > call, just never try to recurse again (thus we could revert the previous
>> > change) and don't set *dynamic_check to anything in that case during the
>> > recursion.
>> > And then at the caller side decide what we want for max == -1 and what
>> > for max == 0 with *dynamic_check.
>>
>> I think that would work, and considering that we have only one
>> non-recursive callsite of decide_alg, it looks like the cleanest
>> solution.
>
> So like this?
Yes, this looks like much better and cleaner solution.
> 2016-03-03 Jakub Jelinek <jakub@redhat.com>
>
> PR target/70062
> * config/i386/i386.c (decide_alg): Add RECUR argument. Revert
> 2016-02-22 changes, instead don't recurse if RECUR is already true.
> Don't change *dynamic_check if RECUR. Adjust recursive caller
> to pass true to the new argument.
> (ix86_expand_set_or_movmem): Adjust decide_alg caller.
>
> * gcc.target/i386/pr70062.c: New test.
OK.
Thanks,
Uros.
> --- gcc/config/i386/i386.c.jj 2016-03-03 21:49:38.535744904 +0100
> +++ gcc/config/i386/i386.c 2016-03-04 12:06:40.075300426 +0100
> @@ -26032,14 +26032,13 @@ static enum stringop_alg
> decide_alg (HOST_WIDE_INT count, HOST_WIDE_INT expected_size,
> unsigned HOST_WIDE_INT min_size, unsigned HOST_WIDE_INT max_size,
> bool memset, bool zero_memset, bool have_as,
> - int *dynamic_check, bool *noalign)
> + int *dynamic_check, bool *noalign, bool recur)
> {
> 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;
> @@ -26157,19 +26156,18 @@ decide_alg (HOST_WIDE_INT count, HOST_WI
> enum stringop_alg alg;
> HOST_WIDE_INT new_expected_size = (max > 0 ? max : 4096) / 2;
>
> - /* 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 there aren't any usable algorithms or if recursing already,
> + 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 || recur)
> + {
> + /* Pick something reasonable. */
> + if (TARGET_INLINE_STRINGOPS_DYNAMICALLY && !recur)
> + *dynamic_check = 128;
> + return loop_1_byte;
> + }
> alg = decide_alg (count, new_expected_size, min_size, max_size, memset,
> - zero_memset, have_as, dynamic_check, noalign);
> + zero_memset, have_as, dynamic_check, noalign, true);
> gcc_assert (*dynamic_check == -1);
> if (TARGET_INLINE_STRINGOPS_DYNAMICALLY)
> *dynamic_check = max;
> @@ -26430,7 +26428,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx
> alg = decide_alg (count, expected_size, min_size, probable_max_size,
> issetmem,
> issetmem && val_exp == const0_rtx, have_as,
> - &dynamic_check, &noalign);
> + &dynamic_check, &noalign, false);
> if (alg == libcall)
> return false;
> gcc_assert (alg != no_stringop);
> --- gcc/testsuite/gcc.target/i386/pr70062.c.jj 2016-03-04 12:03:55.240561744 +0100
> +++ gcc/testsuite/gcc.target/i386/pr70062.c 2016-03-04 12:03:55.240561744 +0100
> @@ -0,0 +1,11 @@
> +/* PR target/70062 */
> +/* { dg-options "-minline-all-stringops -minline-stringops-dynamically -mmemcpy-strategy=libcall:-1:noalign -Wno-psabi" } */
> +/* { dg-additional-options "-mtune=k6-2" { target ia32 } } */
> +
> +typedef int V __attribute__ ((vector_size (32)));
> +
> +V
> +foo (V x)
> +{
> + return (V) { x[0] };
> +}
>
>
> Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-04 11:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-03 20:16 [PATCH] Another fix for decide_alg (PR target/70062) Jakub Jelinek
2016-03-04 9:59 ` Uros Bizjak
2016-03-04 10:06 ` Jakub Jelinek
2016-03-04 10:16 ` Uros Bizjak
2016-03-04 10:34 ` Jakub Jelinek
2016-03-04 10:42 ` Uros Bizjak
2016-03-04 11:11 ` Jakub Jelinek
2016-03-04 11:19 ` 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).