This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[dataflow]: Re: Dataflow branch review #2


Did everything except that FOR_EACH_BB_IN_REGION,
FOR_EACH_BB_REVERSE_IN_REGION were removed since they were
unused. 

Richards follow up comments were not addressed because that is Danny's
code and he can respond to it.

This was bootstrapped and regression tested on x86-64.


2007-05-31  Kenneth Zadeck <zadeck@naturalbridge.com>

    * bitmap.c (bitmap_elt_copy): Fixed comment.
    * cfganal.c (inverted_post_order_compute): Fixed comment and
    formatting of test.
    * basic_block.h: Removed include rtl.h.
    (enum bb_flags): Renumbered.
    (FOR_EACH_BB_IN_REGION, FOR_EACH_BB_REVERSE_IN_REGION): Removed.
    (FOR_BB_INSNS_SAFE, FOR_BB_INSNS_REVERSE_SAFE): Fixed formatting.
   
Kenny



Ian Lance Taylor wrote:
> My second set of comments on the dataflow branch.
>
> Ian
>
> In basic-block.h:
>
> +#include "rtl.h"
>
> Why?  Please omit if not required.
>
>
> -
> -  /* Set if insns in BB have are modified.  Used for updating liveness info.  */
> -  BB_DIRTY = 1,
> -
>    /* Only set on blocks that have just been created by create_bb.  */
>    BB_NEW = 2,
>
> Please either renumber the remaining flags, or put in a comment
> explanining why flag 1 is no longer used.
>
>
> +#define FOR_EACH_BB_IN_REGION(BB, REGION) \
> +  for (BB= (REGION)->region_info->first_bb_in_region; BB; BB = BB->next_bb_in_region)
> +
> +#define FOR_EACH_BB_REVERSE_IN_REGION(BB, REGION) \
> +  for (BB= (REGION)->region_info->last_bb_in_region; BB; BB = BB->prev_bb_in_region)
>
> Space after BB in both.
>
>
> +/* For iterating over insns in basic block when we might remove the
> +   current insn.  */
> +#define FOR_BB_INSNS_SAFE(BB, INSN, CURR)			\
> +  for ((INSN) = BB_HEAD (BB), (CURR)=(INSN) ? NEXT_INSN ((INSN)): NULL;	\
> +       (INSN) && (INSN) != NEXT_INSN (BB_END (BB));	\
> +       (INSN) = (CURR), (CURR) = (INSN) ? NEXT_INSN ((INSN)) : NULL)
> +       
> +
>  #define FOR_BB_INSNS_REVERSE(BB, INSN)		\
>    for ((INSN) = BB_END (BB);			\
>         (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB));	\
>         (INSN) = PREV_INSN (INSN))
>  
> +#define FOR_BB_INSNS_REVERSE_SAFE(BB, INSN, CURR)	\
> +  for ((INSN) = BB_END (BB),(CURR)=(INSN) ? PREV_INSN ((INSN)) : NULL;	\
> +       (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB));	\
> +       (INSN) = (CURR), (CURR) = (INSN) ? PREV_INSN ((INSN)) : NULL)
> +
>
> Space around = in (CURR)=(INSN).  No need for double parenthesis when
> calling NEXT_INSN.
>
>
> In bitmap.c:
>
> +/* Insert an element equal to DST_ELT after DST_PREV, overwriting DST_ELT
> +   if non-NULL.  CHANGED is true if the destination bitmap had already been
> +   changed; the new value of CHANGED is returned.  */
> +
> +static inline bool
> +bitmap_elt_copy (bitmap dst, bitmap_element *dst_elt, bitmap_element *dst_prev,
> +		 bitmap_element *src_elt, bool changed)
>
> The comment should mention SRC_ELT.  Maybe the first DST_ELT should
> really be SRC_ELT.
>
>
> In cfganal.c:
>
> In inverted_post_order_compute:
>
> +      /* Detect any inifinite loop and activate the kludge. 
> +         Note that this doesn't check EXIT_BLOCK itself
> +         since EXIT_BLOCK is always added after the outer do-while loop.  */
>
> Typo: "inifinite" => "infinite".
>
> +  /* Put all blocks that have no successor into the initial work list.  */
> +  FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR, NULL, next_bb)
>
> Just write FOR_ALL_BB (bb).
>
> +      if (has_unvisited_bb == true && sp == 0)
>
> Don't compare to true, just write
>     if (has_unvisited_bb && sp == 0)
>   

Index: bitmap.c
===================================================================
--- bitmap.c	(revision 125210)
+++ bitmap.c	(working copy)
@@ -853,7 +853,7 @@ bitmap_and_into (bitmap a, bitmap b)
 }
 
 
-/* Insert an element equal to DST_ELT after DST_PREV, overwriting DST_ELT
+/* Insert an element equal to SRC_ELT after DST_PREV, overwriting DST_ELT
    if non-NULL.  CHANGED is true if the destination bitmap had already been
    changed; the new value of CHANGED is returned.  */
 
Index: cfganal.c
===================================================================
--- cfganal.c	(revision 125210)
+++ cfganal.c	(working copy)
@@ -878,7 +878,7 @@ inverted_post_order_compute (int *post_o
             }
         }
 
-      /* Detect any inifinite loop and activate the kludge. 
+      /* Detect any infinite loop and activate the kludge. 
          Note that this doesn't check EXIT_BLOCK itself
          since EXIT_BLOCK is always added after the outer do-while loop.  */
       FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR, EXIT_BLOCK_PTR, next_bb)
@@ -910,7 +910,7 @@ inverted_post_order_compute (int *post_o
               }
           }
 
-      if (has_unvisited_bb == true && sp == 0)
+      if (has_unvisited_bb && sp == 0)
         {
           /* No blocks are reachable from EXIT at all. 
              Find a dead-end from the ENTRY, and restart the iteration. */
Index: basic-block.h
===================================================================
--- basic-block.h	(revision 125210)
+++ basic-block.h	(working copy)
@@ -30,7 +30,6 @@ Software Foundation, 51 Franklin Street,
 #include "predict.h"
 #include "vec.h"
 #include "function.h"
-#include "rtl.h"
 
 /* Head of register set linked list.  */
 typedef bitmap_head regset_head;
@@ -295,44 +294,44 @@ DEF_VEC_ALLOC_P(basic_block,heap);
 enum bb_flags
 {
   /* Only set on blocks that have just been created by create_bb.  */
-  BB_NEW = 2,
+  BB_NEW = 1 << 0,
 
   /* Set by find_unreachable_blocks.  Do not rely on this being set in any
      pass.  */
-  BB_REACHABLE = 4,
+  BB_REACHABLE = 1 << 1,
 
   /* Set for blocks in an irreducible loop by loop analysis.  */
-  BB_IRREDUCIBLE_LOOP = 8,
+  BB_IRREDUCIBLE_LOOP = 1 << 2,
 
   /* Set on blocks that may actually not be single-entry single-exit block.  */
-  BB_SUPERBLOCK = 16,
+  BB_SUPERBLOCK = 1 << 3,
 
   /* Set on basic blocks that the scheduler should not touch.  This is used
      by SMS to prevent other schedulers from messing with the loop schedule.  */
-  BB_DISABLE_SCHEDULE = 32,
+  BB_DISABLE_SCHEDULE = 1 << 4,
 
   /* Set on blocks that should be put in a hot section.  */
-  BB_HOT_PARTITION = 64,
+  BB_HOT_PARTITION = 1 << 5,
 
   /* Set on blocks that should be put in a cold section.  */
-  BB_COLD_PARTITION = 128,
+  BB_COLD_PARTITION = 1 << 6,
 
   /* Set on block that was duplicated.  */
-  BB_DUPLICATED = 256,
+  BB_DUPLICATED = 1 << 7,
 
   /* Set if the label at the top of this block is the target of a non-local goto.  */
-  BB_NON_LOCAL_GOTO_TARGET = 512,
+  BB_NON_LOCAL_GOTO_TARGET = 1 << 8,
 
   /* Set on blocks that are in RTL format.  */
-  BB_RTL = 1024,
+  BB_RTL = 1 << 9 ,
 
   /* Set on blocks that are forwarder blocks.
      Only used in cfgcleanup.c.  */
-  BB_FORWARDER_BLOCK = 2048,
+  BB_FORWARDER_BLOCK = 1 << 10,
 
   /* Set on blocks that cannot be threaded through.
      Only used in cfgcleanup.c.  */
-  BB_NONTHREADABLE_BLOCK = 4096
+  BB_NONTHREADABLE_BLOCK = 1 << 11
 };
 
 /* Dummy flag for convenience in the hot/cold partitioning code.  */
@@ -423,12 +422,6 @@ struct control_flow_graph GTY(())
 
 #define FOR_EACH_BB_REVERSE(BB) FOR_EACH_BB_REVERSE_FN(BB, cfun)
 
-#define FOR_EACH_BB_IN_REGION(BB, REGION) \
-  for (BB= (REGION)->region_info->first_bb_in_region; BB; BB = BB->next_bb_in_region)
-
-#define FOR_EACH_BB_REVERSE_IN_REGION(BB, REGION) \
-  for (BB= (REGION)->region_info->last_bb_in_region; BB; BB = BB->prev_bb_in_region)
-
 /* For iterating over insns in basic block.  */
 #define FOR_BB_INSNS(BB, INSN)			\
   for ((INSN) = BB_HEAD (BB);			\
@@ -438,18 +431,17 @@ struct control_flow_graph GTY(())
 /* For iterating over insns in basic block when we might remove the
    current insn.  */
 #define FOR_BB_INSNS_SAFE(BB, INSN, CURR)			\
-  for ((INSN) = BB_HEAD (BB), (CURR)=(INSN) ? NEXT_INSN ((INSN)): NULL;	\
+  for ((INSN) = BB_HEAD (BB), (CURR) = (INSN) ? NEXT_INSN ((INSN)): NULL;	\
        (INSN) && (INSN) != NEXT_INSN (BB_END (BB));	\
        (INSN) = (CURR), (CURR) = (INSN) ? NEXT_INSN ((INSN)) : NULL)
        
-
 #define FOR_BB_INSNS_REVERSE(BB, INSN)		\
   for ((INSN) = BB_END (BB);			\
        (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB));	\
        (INSN) = PREV_INSN (INSN))
 
 #define FOR_BB_INSNS_REVERSE_SAFE(BB, INSN, CURR)	\
-  for ((INSN) = BB_END (BB),(CURR)=(INSN) ? PREV_INSN ((INSN)) : NULL;	\
+  for ((INSN) = BB_END (BB),(CURR) = (INSN) ? PREV_INSN ((INSN)) : NULL;	\
        (INSN) && (INSN) != PREV_INSN (BB_HEAD (BB));	\
        (INSN) = (CURR), (CURR) = (INSN) ? PREV_INSN ((INSN)) : NULL)
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]