Re: [RFC][PATCH] A change to do_while_loop_p()

[Richard Guenther <rguenther at suse dot de>]

On Wed, 21 Mar 2012, Razya Ladelsky wrote:

> Hi,
> 
> I need to use do_while_loop_p, but I'm not sure its functionality is what 
> I expected it to be.
> 
> This is the part that I do not understand:
> 
> /* If the header contains just a condition, it is not a do-while loop.  */
>   stmt = last_and_only_stmt (loop->header);
>  if (stmt
>       && gimple_code (stmt) == GIMPLE_COND)
>     return false;
> 
> The header could contain a condition which is not the loop's exit 
> condition,
> but rather a part of its body, then  why do we rule out this loop as a 
> do_while loop?
> 
> I ran into this in a loop (the outer loop) extracted from bwaves 
> benchmark:
> 
>       do k=1,nz
>          km1=mod(k+nz-2,nz)+1
>          kp1=mod(k,nz)+1
>          do j=1,ny
>             jm1=mod(j+ny-2,ny)+1
>             jp1=mod(j,ny)+1
>         .....
>          enddo
>       enddo
>  
> which was translated to:
> 
> D.2361_17 = *ny_16(D);
> 
> <bb 5>:
>   # k_3 = PHI <1(4), k_562(25)>
>   if (D.2361_17 > 0)
>     goto <bb 8>;
>   else
>     goto <bb 6>;
> 
> <bb 6>:
>   k_562 = k_3 + 1;
>   # DEBUG k => k_562
>   if (k_3 == D.1583_270)
>     goto <bb 7>;  ------->   return
>   else
>     goto <bb 25>;
> 
> <bb 25>:
>   goto <bb 5>;
> 
> <bb 8>:  --> starting the body of the the second loop
>   pretmp.318_776 = (integer(kind=8)) k_3;
>   pretmp.318_777 = stride.92_20 * pretmp.318_776;
> ....... 
> 
> 
> 
> bb 5 is the header of the outer loop, and bb 25 is the latch.
> According to do_while_loop_p ()  this is NOT a do while loop, but it
> seems that it should be.
> 
>  I am attaching a patch to change do_while_loop_p() assuming that what I 
> understand is indeed correct,
>  Please let me know if I'm right,

Not sure what your new tests should do - the patch lacks an update
to the comment.  Note that this predicate is first of all used
in loop header copying to say whether we should avoid copying
a header.  In this case the condition is not what should prevent
copying of the header, but your patch seems to make it so, no?

So, please clarify your change.

Richard.