Patchwork [PR,57800] Waste work in gfc_match_call()

login
register
mail settings
Submitter pchang9@cs.wisc.edu
Date July 23, 2013, 9:32 p.m.
Message ID <759459e99760aaae0ab1d9905fe57939.squirrel@webmail.cs.wisc.edu>
Download mbox | patch
Permalink /patch/261220/
State New
Headers show

Comments

pchang9@cs.wisc.edu - July 23, 2013, 9:32 p.m.
Hi,

The problem appears in revision 201034 in version 4.9. I attached a
one-line patch that fixes it.  I also reported this problem
at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57800

Bootstrap and regression-tested on x86_64-linux.

In method "gfc_match_call()" in gcc/fortran/match.c, the loop on line
4189 should break immediately after "i" is set to 1.


2013-07-22  Chang  <pchang9@cs.wisc.edu>

        * match.c (gfc_match_call): Exit loop after setting i.



-Chang
Index: gcc/fortran/match.c
===================================================================
--- gcc/fortran/match.c	(revision 201034)
+++ gcc/fortran/match.c	(working copy)
@@ -4188,7 +4188,10 @@
   i = 0;
   for (a = arglist; a; a = a->next)
     if (a->expr == NULL)
-      i = 1;
+      {
+	i = 1;
+	break;
+      }
 
   if (i)
     {
aldot - July 24, 2013, 4:12 p.m.
On 23 July 2013 23:32:27 pchang9@cs.wisc.edu wrote:
> Hi,
>
> The problem appears in revision 201034 in version 4.9. I attached a
> one-line patch that fixes it.  I also reported this problem
> at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57800
>
> Bootstrap and regression-tested on x86_64-linux.
>
> In method "gfc_match_call()" in gcc/fortran/match.c, the loop on line
> 4189 should break immediately after "i" is set to 1.
>
>
> 2013-07-22  Chang  <pchang9@cs.wisc.edu>
>
>         * match.c (gfc_match_call): Exit loop after setting i.
>
>
> Index: gcc/fortran/match.c
> ===================================================================
> --- gcc/fortran/match.c	(revision 201034)
> +++ gcc/fortran/match.c	(working copy)
> @@ -4188,7 +4188,10 @@
>    i = 0;
>    for (a = arglist; a; a = a->next)
    for (a = arglist; a || i; a = a->next)

Sounds more reasonable to me.

To repeat the previous question, curious how you get at these? scev one-liner?

Thanks,

>      if (a->expr == NULL)
> -      i = 1;
> +      {
> +	i = 1;
> +	break;
> +      }
>
>    if (i)
>      {
>
> -Chang


Sent with AquaMail for Android
http://www.aqua-mail.com
aldot - July 24, 2013, 4:42 p.m.
On 24 July 2013 18:12:00 "Bernhard Reutner-Fischer" <rep.dot.nop@gmail.com> 
wrote:
> On 23 July 2013 23:32:27 pchang9@cs.wisc.edu wrote:
> > Hi,
> >
> > The problem appears in revision 201034 in version 4.9. I attached a
> > one-line patch that fixes it.  I also reported this problem
> > at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57800
> >
> > Bootstrap and regression-tested on x86_64-linux.
> >
> > In method "gfc_match_call()" in gcc/fortran/match.c, the loop on line
> > 4189 should break immediately after "i" is set to 1.
> >
> >
> > 2013-07-22  Chang  <pchang9@cs.wisc.edu>
> >
> >         * match.c (gfc_match_call): Exit loop after setting i.
> >
> >
> > Index: gcc/fortran/match.c
> > ===================================================================
> > --- gcc/fortran/match.c	(revision 201034)
> > +++ gcc/fortran/match.c	(working copy)
> > @@ -4188,7 +4188,10 @@
> >    i = 0;
> >    for (a = arglist; a; a = a->next)

    for (a = arglist; !i || a; a = a->next)
Swap that, obviously..
>
> Sounds more reasonable to me.
>
> To repeat the previous question, curious how you get at these? scev one-liner?
>
> Thanks,
>
> >      if (a->expr == NULL)
> > -      i = 1;
> > +      {
> > +	i = 1;
> > +	break;
> > +      }
> >
> >    if (i)
> >      {
> >
> > -Chang


Sent with AquaMail for Android
http://www.aqua-mail.com
pchang9@cs.wisc.edu - July 25, 2013, 1:01 a.m.
> On 23 July 2013 23:32:27 pchang9@cs.wisc.edu wrote:
>> Hi,
>>
>> The problem appears in revision 201034 in version 4.9. I attached a
>> one-line patch that fixes it.  I also reported this problem
>> at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57800
>>
>> Bootstrap and regression-tested on x86_64-linux.
>>
>> In method "gfc_match_call()" in gcc/fortran/match.c, the loop on line
>> 4189 should break immediately after "i" is set to 1.
>>
>>
>> 2013-07-22  Chang  <pchang9@cs.wisc.edu>
>>
>>         * match.c (gfc_match_call): Exit loop after setting i.
>>
>>
>> Index: gcc/fortran/match.c
>> ===================================================================
>> --- gcc/fortran/match.c	(revision 201034)
>> +++ gcc/fortran/match.c	(working copy)
>> @@ -4188,7 +4188,10 @@
>>    i = 0;
>>    for (a = arglist; a; a = a->next)
>     for (a = arglist; a || i; a = a->next)
>
> Sounds more reasonable to me.
>
> To repeat the previous question, curious how you get at these? scev one-liner?
>
> Thanks,

We found these by analysis tool using LLVM, but not so-fancy as scev one-linear.
Of course, at very beginning we inspected the code manually.

Thanks,
-Chang
Jeff Law - July 29, 2013, 7:08 p.m.
On 07/23/2013 03:32 PM, pchang9@cs.wisc.edu wrote:
> Hi,
>
> The problem appears in revision 201034 in version 4.9. I attached a
> one-line patch that fixes it.  I also reported this problem
> at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57800
>
> Bootstrap and regression-tested on x86_64-linux.
>
> In method "gfc_match_call()" in gcc/fortran/match.c, the loop on line
> 4189 should break immediately after "i" is set to 1.
>
>
> 2013-07-22  Chang  <pchang9@cs.wisc.edu>
>
>          * match.c (gfc_match_call): Exit loop after setting i.
Installed.  Thanks!

Jeff

Patch

Index: gcc/fortran/match.c
===================================================================
--- gcc/fortran/match.c	(revision 201034)
+++ gcc/fortran/match.c	(working copy)
@@ -4188,7 +4188,10 @@ 
   i = 0;
   for (a = arglist; a; a = a->next)
     if (a->expr == NULL)
-      i = 1;
+      {
+	i = 1;
+	break;
+      }

   if (i)
     {