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

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

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,

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