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

[Richard Guenther]

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);
> 
> :
>   # k_3 = PHI <1(4), k_562(25)>
>   if (D.2361_17 > 0)
> goto ;
>   else
> goto ;
> 
> :
>   k_562 = k_3 + 1;
>   # DEBUG k => k_562
>   if (k_3 == D.1583_270)
> goto ;  --->   return
>   else
> goto ;
> 
> :
>   goto ;
> 
> :  --> 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.

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

[Razya Ladelsky]

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

:
  # k_3 = PHI <1(4), k_562(25)>
  if (D.2361_17 > 0)
goto ;
  else
goto ;

:
  k_562 = k_3 + 1;
  # DEBUG k => k_562
  if (k_3 == D.1583_270)
goto ;  --->   return
  else
goto ;

:
  goto ;

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

Thank you,
Razya

Index: tree-ssa-loop-ch.c
===
--- tree-ssa-loop-ch.c  (revision 185604)
+++ tree-ssa-loop-ch.c  (working copy)
@@ -107,6 +107,8 @@ should_duplicate_loop_header_p (basic_block header
 bool
 do_while_loop_p (struct loop *loop)
 {
+  edge exit_edge;
+  gimple cond_stmt;
   gimple stmt = last_stmt (loop->latch);
 
   /* If the latch of the loop is not empty, it is not a do-while loop.  */
@@ -116,8 +118,14 @@ do_while_loop_p (struct loop *loop)
 
   /* If the header contains just a condition, it is not a do-while loop.  */
   stmt = last_and_only_stmt (loop->header);
+  exit_edge = single_dom_exit (loop);
+  if (exit_edge)
+cond_stmt = last_stmt (exit_edge->src);
+  else
+cond_stmt =stmt;
   if (stmt
-  && gimple_code (stmt) == GIMPLE_COND)
+  && gimple_code (stmt) == GIMPLE_COND
+  && stmt == cond_stmt)
 return false;
 
   return true;
=