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]

[PATCH] Fix four bugs in combine


This patches fixes four related bugs in combine.  Some of these might have
been introduced by the cfglayout conversion, I am not sure though.

1. To decide whether to start a new EBB, the code goes by basic-block numbers;
a block is considered to be part of the current EBB if the basic-block number
is one more than the previous and the block has only one predecessor.  This
does not hold between the ENTRY_BLOCK (0) and the first block (2) due to
EXIT_BLOCK being number 1.

Since incoming promotions get the tick from the initial tick (0), they would
no longer be valid in the first block.

I fixed this by using the label tick of the block from the previous iteration
of the BB loop.

2. Since everything inside reg_stat is initialized to zero.  It's unfortunate
to start the tick value at 0.  For example in record_value_for_reg we
invalidate the value of a promotion:

  /* Now update the status of each register being set.
     If someone is using this register in this block, set this register
     to invalid since we will get confused between the two lives in this
     basic block.  This makes using this register always invalid.  In cse, we
     scan the table to invalidate all entries using this register, but this
     is too much work for us.  */

  for (i = regno; i < endregno; i++)
    {
      rsp = VEC_index (reg_stat_type, reg_stat, i);
      rsp->las->int_set_label = label_tick;
      if (!insn
	  || (value && rsp->last_set_table_tick >= label_tick_ebb_start))
	rsp->last_set_invalid = 1;
      else
	rsp->last_set_invalid = 0;
    }

because both rsp->last_set_table_tick (unmodified at this point) and
label_tick_ebb_start are 0.

I changed label_ticks to start from 1.

3. setup_incoming_promotions() calls record_value_for_reg, which uses
label_tick and label_tick_ebb_start.  Hence we need to initialize the value of
label_tick_ebb_start before calling setup_incoming_promotions in the first
pass.

4. Same thing is broken WRT to label_tick and setup_incoming_promotions in the
second pass.

Bootstrapped and regtested on {x86_64,mips64octeon}-linux-gnu.

OK to install?

Adam


	* combine.c (bb_to_label_tick): New function.
	(combine_instructions): Use it.  Set label_tick and
	label_tick_ebb_start before calling setup_incoming_promotions.  As the
	label tick of the previous BB use label tick from the previous
	iteration rather than computing it.

testsuite/
	* gcc.target/mips/truncate-6.c: New test.

Index: gcc/combine.c
===================================================================
--- gcc.orig/combine.c	2009-10-01 14:39:50.000000000 -0700
+++ gcc/combine.c	2009-10-01 16:52:35.000000000 -0700
@@ -1010,10 +1010,15 @@ clear_log_links (void)
     if (INSN_P (insn))
       free_INSN_LIST_list (&LOG_LINKS (insn));
 }
+
+/* Return the label tick for a basic block BB.  */
 
+static inline int bb_to_label_tick (basic_block bb)
+{
+  /* Avoid 0 because that is the unset value for ticks.  */
+  return bb->index + 1;
+}
 
-
-
 /* Main entry point for combiner.  F is the first insn of the function.
    NREGS is the first unused pseudo-reg number.
 
@@ -1058,6 +1063,7 @@ combine_instructions (rtx f, unsigned in
      problems when, for example, we have j <<= 1 in a loop.  */
 
   nonzero_sign_valid = 0;
+  label_tick = label_tick_ebb_start = bb_to_label_tick (ENTRY_BLOCK_PTR);
 
   /* Scan all SETs and see if we can deduce anything about what
      bits are known to be zero for some registers and how many copies
@@ -1069,16 +1075,15 @@ combine_instructions (rtx f, unsigned in
   setup_incoming_promotions (first);
 
   create_log_links ();
-  label_tick_ebb_start = ENTRY_BLOCK_PTR->index;
   FOR_EACH_BB (this_basic_block)
     {
       optimize_this_for_speed_p = optimize_bb_for_speed_p (this_basic_block);
       last_call_luid = 0;
       mem_last_set = -1;
-      label_tick = this_basic_block->index;
       if (!single_pred_p (this_basic_block)
-	  || single_pred (this_basic_block)->index != label_tick - 1)
-	label_tick_ebb_start = label_tick;
+	  || bb_to_label_tick (single_pred (this_basic_block)) != label_tick)
+	label_tick_ebb_start = bb_to_label_tick (this_basic_block);
+      label_tick = bb_to_label_tick (this_basic_block);
       FOR_BB_INSNS (this_basic_block, insn)
         if (INSN_P (insn) && BLOCK_FOR_INSN (insn))
 	  {
@@ -1110,7 +1115,7 @@ combine_instructions (rtx f, unsigned in
 
   /* Now scan all the insns in forward order.  */
 
-  label_tick_ebb_start = ENTRY_BLOCK_PTR->index;
+  label_tick = label_tick_ebb_start = bb_to_label_tick (ENTRY_BLOCK_PTR);
   init_reg_last ();
   setup_incoming_promotions (first);
 
@@ -1119,10 +1124,10 @@ combine_instructions (rtx f, unsigned in
       optimize_this_for_speed_p = optimize_bb_for_speed_p (this_basic_block);
       last_call_luid = 0;
       mem_last_set = -1;
-      label_tick = this_basic_block->index;
       if (!single_pred_p (this_basic_block)
-	  || single_pred (this_basic_block)->index != label_tick - 1)
-	label_tick_ebb_start = label_tick;
+	  || bb_to_label_tick (single_pred (this_basic_block)) != label_tick)
+	label_tick_ebb_start = bb_to_label_tick (this_basic_block);
+      label_tick = bb_to_label_tick (this_basic_block);
       rtl_profile_for_bb (this_basic_block);
       for (insn = BB_HEAD (this_basic_block);
 	   insn != NEXT_INSN (BB_END (this_basic_block));
Index: gcc/testsuite/gcc.target/mips/truncate-6.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/truncate-6.c	2009-10-01 17:20:18.000000000 -0700
@@ -0,0 +1,12 @@
+/* setup_incoming_promotions should detect x to be already sign-extended due
+   to PROMOTE_MODE.  Thus the truncation should be removed by combine.  Based
+   on gcc.c-torture/execute/pr34070-2.c.  */
+/* { dg-options "-O -mgp64" } */
+/* { dg-final { scan-assembler-not "\tsll\t\[^\n\]*,0" } } */
+
+NOMIPS16 int f(unsigned int x, int n, int *p)
+{
+  if (p)
+    *p = 1;
+  return ((int)x) / (1 << n);
+}


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