[PATCH] fix SPEC ICE - if-conversion

[PATCH] fix SPEC ICE - if-conversion

[Devang Patel]

Recent (in last 3-4 weeks) changes in mainline exposed one bug in  
tree if-conversion.
It causes ICE in 4 SPEC benchmarks, gcc, perlbmk, crafty and bzip2.  
While replacing
phi node with cond expr, for example

Input:
     P1 = PHI < A(3), B(4) >

Output:
     P1 = C1 ? A : B
or
     P1 = C2 ? B : A

one of the predecessor's condition is selected and it determines  
order of operands.
[ This conditions are propagated during each basic block's if- 
conversion. When one
block is if-converted, its condition is propagated into destination  
block (stored
in aux field.) ]

Now, if one of the predecessor is loop header then corresponding  
condition is
defined in latch node. This results in verify_ssa() failure, because  
definition
follows use when all loop blocks are merged into one big block. This  
patch fixes this
by adding one additional loop header check during condition selection.

2005-03-31  Devang Patel  <dpatel@apple.com>

         * tree-if-conv.c (find_phi_replacement_condition): New  
parameter, loop.
         Avoid condition from loop header.

Bootstrapped and tested on powerpc-darwin, tested on SPEC (with auto  
vectorizer ON).

OK for mainline?
Thanks,
-
Devang


Index: gcc/tree-if-conv.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-if-conv.c,v
retrieving revision 2.34
diff -Idpatel.pbxuser -c -3 -p -r2.34 tree-if-conv.c
*** gcc/tree-if-conv.c  12 Mar 2005 18:35:48 -0000      2.34
--- gcc/tree-if-conv.c  31 Mar 2005 23:00:52 -0000
*************** static void add_to_predicate_list (basic
*** 117,123 ****
   static tree add_to_dst_predicate_list (struct loop * loop,  
basic_block, tree, tree,
                                        block_stmt_iterator *);
   static void clean_predicate_lists (struct loop *loop);
! static basic_block find_phi_replacement_condition (basic_block,  
tree *,
                                                     
block_stmt_iterator *);
   static void replace_phi_with_cond_modify_expr (tree, tree,  
basic_block,
                                                  block_stmt_iterator  
*);
--- 117,123 ----
   static tree add_to_dst_predicate_list (struct loop * loop,  
basic_block, tree, tree,
                                        block_stmt_iterator *);
   static void clean_predicate_lists (struct loop *loop);
! static basic_block find_phi_replacement_condition (struct loop*,  
basic_block, tree *,
                                                     
block_stmt_iterator *);
   static void replace_phi_with_cond_modify_expr (tree, tree,  
basic_block,
                                                  block_stmt_iterator  
*);
*************** clean_predicate_lists (struct loop *loop
*** 683,689 ****
      whose phi arguments are selected when cond is true.  */

   static basic_block
! find_phi_replacement_condition (basic_block bb, tree *cond,
                                   block_stmt_iterator *bsi)
   {
     edge e;
--- 683,689 ----
      whose phi arguments are selected when cond is true.  */

   static basic_block
! find_phi_replacement_condition (struct loop *loop, basic_block bb,  
tree *cond,
                                   block_stmt_iterator *bsi)
   {
     edge e;
*************** find_phi_replacement_condition (basic_bl
*** 704,721 ****
         }
       }

!   /* Use condition that is not TRUTH_NOT_EXPR in conditional modify  
expr.  */
     tmp_cond = p1->aux;
!   if (TREE_CODE (tmp_cond) == TRUTH_NOT_EXPR)
!     {
!       *cond  = p2->aux;
!       true_bb = p2;
!     }
!   else
       {
!       *cond  = p1->aux;
!       true_bb = p1;
!     }

     /* Create temp. for the condition. Vectorizer prefers to have  
gimple
        value as condition. Various targets use different means to  
communicate
--- 704,725 ----
         }
       }

!   /* Use condition that is not TRUTH_NOT_EXPR in conditional modify  
expr.
!      However, do not use condition from loop header.  */
     tmp_cond = p1->aux;
!   if (loop->header != p1 && loop->header != p2)
       {
!       if (TREE_CODE (tmp_cond) == TRUTH_NOT_EXPR)
!       true_bb = p2;
!       else
!       true_bb = p1;
!     }
!   else if (loop->header == p1)
!     true_bb = p2;
!   else
!     true_bb = p1;
!
!   *cond = true_bb->aux;

     /* Create temp. for the condition. Vectorizer prefers to have  
gimple
        value as condition. Various targets use different means to  
communicate
*************** process_phi_nodes (struct loop *loop)
*** 834,840 ****
         /* BB has two predecessors. Using predecessor's aux field, set
          appropriate condition for the PHI node replacement.  */
         if (phi)
!       true_bb = find_phi_replacement_condition (bb, &cond, &bsi);

         while (phi)
         {
--- 838,844 ----
         /* BB has two predecessors. Using predecessor's aux field, set
          appropriate condition for the PHI node replacement.  */
         if (phi)
!       true_bb = find_phi_replacement_condition (loop, bb, &cond,  
&bsi);

         while (phi)
         {

Re: [PATCH] fix SPEC ICE - if-conversion

[Diego Novillo]

On Thu, Mar 31, 2005 at 04:18:53PM -0800, Devang Patel wrote:
> 
>         * tree-if-conv.c (find_phi_replacement_condition): New  
> parameter, loop.
>         Avoid condition from loop header.
> 
This is the same patch you had added to TCB in December, right?
Yes, this is fine for mainline.


Diego.

Re: [PATCH] fix SPEC ICE - if-conversion

[Devang Patel]

On Apr 6, 2005, at 10:26 AM, Diego Novillo wrote:

> On Thu, Mar 31, 2005 at 04:18:53PM -0800, Devang Patel wrote:
>>
>>         * tree-if-conv.c (find_phi_replacement_condition): New
>> parameter, loop.
>>         Avoid condition from loop header.

> This is the same patch you had added to TCB in December, right?

Yes.

> Yes, this is fine for mainline.

Thanks
-
Devang