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]

Re: [trunk]: patch to properly build the conflict graph


On Tue, Oct 02, 2007 at 09:17:06AM -0400, Kenneth Zadeck wrote:
> (build_insn_chain): Rewritten from scratch and made local.

   My ia16 port no longer builds libiberty with this patch. There's
something wrong with the insn chains built by the new implementation. The
problem is with the live_throughout set. It builds

(gdb) call print_insn_chain (stderr, c)
insn=64, live_throughout: 12, 38, 40, 41, 43, 59, 65, dead_or_set: 42, 65

out of

Register 65 used 4 times across 6 insns in block 6; set 2 times; QI_REGS or none.
[...]
Basic block 6 , prev 5, next 7, loop_depth 1, count 0, freq 764, maybe hot.
Predecessors:  5 [16.8%]  (fallthru)
;; bb 6 artificial_defs: { }
;; bb 6 artificial_uses: { u41(10){ }u42(12){ }u43(14){ }}
;; lr  in  	 10 [bp] 12 [sp] 14 [argp] 38 39 40 41 43 59 73
;; lr  use 	 10 [bp] 12 [sp] 14 [argp] 39 43 59 73
;; lr  def 	 0 [c] 1 [ch] 2 [a] 3 [ah] 4 [d] 5 [dh] 6 [b] 7 [bh] 12 [sp] 13 [cc] 37 42 65 68
;; live  in  	 10 [bp] 12 [sp] 14 [argp] 38 39 40 41 43 59 73
;; live  gen 	 2 [a] 12 [sp] 37 42 65 68
;; live  kill	 13 [cc]

Successors:  7 [100.0%]  (fallthru)
;; lr  out 	 10 [bp] 12 [sp] 14 [argp] 38 39 40 41 73
;; live  out 	 10 [bp] 12 [sp] 14 [argp] 38 39 40 41 73
[...]
;; Register 65 in 4.
[...]

(call_insn:HI 62 61 63 6 delta1/regex.tmp.i:41 (set (reg:QI 2 a)
        (call (mem:PQI (symbol_ref:HI ("byte_group_match_null_string_p") [flags 0x3] <function_decl 0xb7bc3b80 byte_group_match_null_string_p>) [0 S1 A8])
            (const_int 6 [0x6]))) 542 {*call_value} (nil)
    (nil))

(insn:HI 63 62 64 6 delta1/regex.tmp.i:41 (set (reg:QI 42 [ D.1219 ])
        (reg:QI 2 a)) 8 {*movqi} (expr_list:REG_DEAD (reg:QI 2 a)
        (nil)))

(insn:HI 64 63 65 6 delta1/regex.tmp.i:41 (set (subreg:QI (reg:HI 65 [ D.1219 ]) 0)
        (reg:QI 42 [ D.1219 ])) 8 {*movqi} (expr_list:REG_DEAD (reg:QI 42 [ D.1219 ])
        (nil)))

(note:HI 65 64 68 6 NOTE_INSN_DELETED)

(insn:HI 68 65 69 6 delta1/regex.tmp.i:41 (parallel [
            (set (reg:HI 65 [ D.1219 ])
                (and:HI (reg:HI 65 [ D.1219 ])
                    (reg:HI 59)))
            (clobber (reg:CC 13 cc))
        ]) 77 {*andhi3} (expr_list:REG_DEAD (reg:HI 59)
        (expr_list:REG_UNUSED (reg:CC 13 cc)
            (nil))))

(insn:HI 69 68 71 6 delta1/regex.tmp.i:41 (set (reg:HI 68)
        (mem/s/j:HI (reg/f:HI 43 [ D.1216 ]) [0 S1 A16])) 6 {*movhi} (nil))

(insn:HI 71 69 72 6 delta1/regex.tmp.i:41 (parallel [
            (set (reg:HI 68)
                (and:HI (reg:HI 68)
                    (const_int -4 [0xfffffffc])))
            (clobber (reg:CC 13 cc))
        ]) 77 {*andhi3} (expr_list:REG_UNUSED (reg:CC 13 cc)
        (nil)))

(insn:HI 72 71 73 6 delta1/regex.tmp.i:41 (parallel [
            (set (reg:HI 68)
                (ior:HI (reg:HI 68)
                    (reg:HI 65 [ D.1219 ])))
            (clobber (reg:CC 13 cc))
        ]) 79 {*iorhi3} (expr_list:REG_DEAD (reg:HI 65 [ D.1219 ])
        (expr_list:REG_UNUSED (reg:CC 13 cc)
            (nil))))

Register 65 is not live throughout insn 64! Eventually,
caller-save.c/insert_save() aborts when asked to save register 4
(reg_renumber[65] == 4) for call_insn 62 because the register isn't live
across the call (and so has no stack slot allocated to it).

> +		    if ((regno < FIRST_PSEUDO_REGISTER || reg_renumber[regno] >= 0)
> +			&& (!DF_REF_FLAGS_IS_SET (def, DF_REF_CONDITIONAL)))
> +		      {
> +			rtx reg = DF_REF_REG (def);
> +			/* We can model subregs, but not if they are
> +			   wrapped in ZERO_EXTRACTS.  */
> +			if (GET_CODE (reg) == SUBREG
> +			    && !DF_REF_FLAGS_IS_SET (def, DF_REF_EXTRACT))
> +			  {
> +			    unsigned int start = SUBREG_BYTE (reg);
> +			    unsigned int last = start + GET_MODE_SIZE (GET_MODE (reg));
>  
> -	      /* Mark anything that is set in this insn and then unused as dying.  */
> +			    ra_init_live_subregs (bitmap_bit_p (live_relevant_regs, regno), 
> +						  live_subregs, live_subregs_used,
> +						  regno, reg);
> +			    /* Ignore the paradoxical bits.  */
> +			    if ((int)last > live_subregs_used[regno])
> +			      last = live_subregs_used[regno];
> +
> +			    while (start < last)
> +			      {
> +				RESET_BIT (live_subregs[regno], start);
> +				start++;
> +			      }
> +			    
> +			    if (sbitmap_empty_p (live_subregs[regno]))
> +			      {
> +				live_subregs_used[regno] = 0;
> +				bitmap_clear_bit (live_relevant_regs, regno);
> +			      }
> +			    else
> +			      /* Set live_relevant_regs here because
> +				 that bit has to be true to get us to
> +				 look at the live_subregs fields.  */
> +			      bitmap_set_bit (live_relevant_regs, regno);
> +			  }
> +			else
> +			  {
> +			    /* DF_REF_PARTIAL is generated for
> +			       subregs, STRICT_LOW_PART, and
> +			       ZERO_EXTRACT.  We handle the subreg
> +			       case above so here we have to keep from
> +			       modeling the def as a killing def.  */
> +			    if (!DF_REF_FLAGS_IS_SET (def, DF_REF_PARTIAL))
> +			      {
> +				bitmap_clear_bit (live_relevant_regs, regno);
> +				live_subregs_used[regno] = 0;
> +			      }
> +			  }
> +		      }
> +		  }

   Shouldn't there be a check of DF_REF_PARTIAL, DF_REF_STRICT_LOWER_PART,
DF_REF_READ_WRITE or some such in this block? At the sbitmap_empty_p() check
the bitmap is wrong:

(gdb) call debug_sbitmap (live_subregs[65])
n_bits = 2, set = {1 }

Subreg byte 1 is not live before insn 64 because the insn clobbers it. No
bits should be set.

(gdb) print *def
$187 = {reg = 0xb7bffa2c, bb = 0xb7b6d618, insn = 0xb7c00b88,
 loc = 0xb7bffa3c, chain = 0x0, id = 57, ref_order = 107, regno = 65,
 type = DF_REF_REG_DEF, flags = DF_REF_SUBREG, next_reg = 0x0,
 prev_reg = 0xa6e8474}

   This patch appears to fix the problem (but needs testing):

Index: gcc/global.c
===================================================================
--- gcc/global.c	(revision 128957)
+++ gcc/global.c	(working copy)
@@ -1395,6 +1395,10 @@ build_insn_chain (void)
 			    if ((int)last > live_subregs_used[regno])
 			      last = live_subregs_used[regno];
 
+			    /* Round subregs without strict_low_part to a word.  */
+			    if (!DF_REF_FLAGS_IS_SET (def, DF_REF_STRICT_LOWER_PART))
+			      last = (last + UNITS_PER_WORD - 1) & ~(UNITS_PER_WORD - 1);
+
 			    while (start < last)
 			      {
 				RESET_BIT (live_subregs[regno], start);

-- 
Rask Ingemann Lambertsen
Danish law requires addresses in e-mail to be logged and stored for a year


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