x86-64 bootstrap broken (was Re: [PATCH] tree level if-conversion for vectorizer)

Devang Patel dpatel@apple.com
Tue Sep 7 01:37:00 GMT 2004


On Sep 4, 2004, at 1:39 PM, Jan Hubicka wrote:

>> On Sep 4, 2004, at 5:55 AM, Jan Hubicka wrote:
>>
>>> Devang,
>>> it looks like your patch is responsible for misscompilation of stage2
>>> at
>>> x86-64 compiler (so we die building libgcc with memory corruption).  
>>> I
>>> checked it by disabling the pass from tree-optimize and we got past 
>>> it.
>>>
>>> Would be possible to look into it soon?  I've got similarly looking
>>> failure on PPC-linux too:

[snip]

>>
>> Would it be possible to send me preprocessed source ?
>> I do not have access to x86-64 box and ppc-linux box
>> in the lab has decided to not respond at the moment.
>
> Well, the problem is that the same source works fine with stage1
> compiler and crash in stage2, so what we are facing is an
> misscompilation of gcc itself.  It however crashed for me on
> i686-pc-linux box too.

It managed to mis-compile code that is not used on powerpc-darwin :)

While replacing phi node with conditional modify expr, if-convertor
walks pred list and determines which one represents true condition.
And based on this info. it replaces

	a = PHI <a1(0), a2(3)>
with
	a = cond ? a1 : a2
or
	a = cond ? a2 : a1

Now, in following block

;; basic block 5, loop depth 1, count 0
;; prev block 4, next block 35
;; pred:       34 [100.0%]  (fallthru) 4 [100.0%]  (fallthru,exec)
;; succ:       6 [11.0%]  (loop_exit,false,exec) 35 [89.0%]  
(dfs_back,true,exec)
# high_10 = PHI <high_155(4), pos_111(34)>;
# low_47 = PHI <low_115(4), low_156(34)>;
<L5>:;
_ifc_.1462_138 = low_47 != high_10;
if (_ifc_.1462_138) goto <L50>; else goto <L7>;

34 is first predecessor for basic block 5.
But in phi nodes, value from edge whose source is basic block 34 is
listed as 2nd argument.

Relying on order of phi node arguments to match order of pred nodes 
turned
out to be a bad idea in this example.

Following patch cures it by remembering actual source basic block 
instead
of order number. Bootstrapped finished, without error, on 
i686-pc-linux-gnu.

powerpc-darwin bootstrap plus dejagnu run is in progress.
OK to commit if it succeeds ?

Thanks,
-
Devang

2004-09-06  Devang Patel  <dpatel@apple.com>

         * tree-if-conv.c (find_phi_replacement_condition): Return true 
edge block.
         (replace_phi_with_cond_modify_expr): Select conditional expr 
args based on
         true edge basic block.


Index: tree-if-conv.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-if-conv.c,v
retrieving revision 2.1
diff -c -3 -p -r2.1 tree-if-conv.c
*** tree-if-conv.c      4 Sep 2004 03:26:57 -0000       2.1
--- tree-if-conv.c      6 Sep 2004 23:15:33 -0000
*************** static void add_to_predicate_list (basic
*** 117,125 ****
   static tree add_to_dst_predicate_list (struct loop * loop, tree, 
tree, tree,
                                        block_stmt_iterator *);
   static void clean_predicate_lists (struct loop *loop);
! static bool find_phi_replacement_condition (basic_block, tree *,
!                                             block_stmt_iterator *);
! static void replace_phi_with_cond_modify_expr (tree, tree, bool,
                                                  block_stmt_iterator *);
   static void process_phi_nodes (struct loop *);
   static void combine_blocks (struct loop *);
--- 117,125 ----
   static tree add_to_dst_predicate_list (struct loop * loop, tree, 
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 *);
   static void process_phi_nodes (struct loop *);
   static void combine_blocks (struct loop *);
*************** clean_predicate_lists (struct loop *loop
*** 671,687 ****
   }

   /* Basic block BB has two predecessors. Using predecessor's aux 
field, set
!    appropriate condition COND for the PHI node replacement. Return 
true if
!    phi arguments are condition is selected from second predecessor.  
*/

! static bool
   find_phi_replacement_condition (basic_block bb, tree *cond,
                                   block_stmt_iterator *bsi)
   {
     edge e;
     basic_block p1 = NULL;
     basic_block p2 = NULL;
!   bool switch_args = false;
     tree tmp_cond;

     for (e = bb->pred; e; e = e->pred_next)
--- 671,687 ----
   }

   /* Basic block BB has two predecessors. Using predecessor's aux 
field, set
!    appropriate condition COND for the PHI node replacement. Return 
true block
!    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;
     basic_block p1 = NULL;
     basic_block p2 = NULL;
!   basic_block true_bb = NULL;
     tree tmp_cond;

     for (e = bb->pred; e; e = e->pred_next)
*************** find_phi_replacement_condition (basic_bl
*** 700,711 ****
     if (TREE_CODE (tmp_cond) == TRUTH_NOT_EXPR)
       {
         *cond  = p2->aux;
!       switch_args = true;
       }
     else
       {
         *cond  = p1->aux;
!       switch_args = false;
       }

     /* Create temp. for the condition. Vectorizier prefers to have 
gimple
--- 700,711 ----
     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. Vectorizier prefers to have 
gimple
*************** find_phi_replacement_condition (basic_bl
*** 727,733 ****
       abort ();
   #endif

!   return switch_args;
   }


--- 727,733 ----
       abort ();
   #endif

!   return true_bb;
   }


*************** find_phi_replacement_condition (basic_bl
*** 738,748 ****
      is converted into,
        S2: A = cond ? x1 : x2;
      S2 is inserted at the top of basic block's statement list.
!    PHI arguments are switched if SWITCH_ARGS is true.
   */

   static void
! replace_phi_with_cond_modify_expr (tree phi, tree cond, bool 
switch_args,
                                      block_stmt_iterator *bsi)
   {
     tree new_stmt;
--- 738,748 ----
      is converted into,
        S2: A = cond ? x1 : x2;
      S2 is inserted at the top of basic block's statement list.
!    When COND is true, phi arg from TRUE_BB is selected.
   */

   static void
! replace_phi_with_cond_modify_expr (tree phi, tree cond, basic_block 
true_bb,
                                      block_stmt_iterator *bsi)
   {
     tree new_stmt;
*************** replace_phi_with_cond_modify_expr (tree
*** 767,773 ****
     arg_1 = NULL_TREE;

     /* Use condition that is not TRUTH_NOT_EXPR in conditional modify 
expr.  */
!   if (switch_args)
       {
         arg_0 = PHI_ARG_DEF (phi, 1);
         arg_1 = PHI_ARG_DEF (phi, 0);
--- 767,773 ----
     arg_1 = NULL_TREE;

     /* Use condition that is not TRUTH_NOT_EXPR in conditional modify 
expr.  */
!   if (PHI_ARG_EDGE(phi, 1)->src == true_bb)
       {
         arg_0 = PHI_ARG_DEF (phi, 1);
         arg_1 = PHI_ARG_DEF (phi, 0);
*************** process_phi_nodes (struct loop *loop)
*** 820,826 ****
       {
         tree phi, cond;
         block_stmt_iterator bsi;
!       bool switch_args = false;
         bb = ifc_bbs[i];

         if (bb == loop->header || bb == loop->latch)
--- 820,826 ----
       {
         tree phi, cond;
         block_stmt_iterator bsi;
!       basic_block true_bb = NULL;
         bb = ifc_bbs[i];

         if (bb == loop->header || bb == loop->latch)
*************** process_phi_nodes (struct loop *loop)
*** 832,843 ****
         /* BB has two predecessors. Using predecessor's aux field, set
          appropriate condition for the PHI node replacement.  */
         if (phi)
!       switch_args = find_phi_replacement_condition (bb, &cond, &bsi);

         while (phi)
         {
           tree next = TREE_CHAIN (phi);
!         replace_phi_with_cond_modify_expr (phi, cond, switch_args, 
&bsi);
           release_phi_node (phi);
           phi = next;
         }
--- 832,843 ----
         /* 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)
         {
           tree next = TREE_CHAIN (phi);
!         replace_phi_with_cond_modify_expr (phi, cond, true_bb, &bsi);
           release_phi_node (phi);
           phi = next;
         }



More information about the Gcc-patches mailing list