From mboxrd@z Thu Jan 1 00:00:00 1970 From: "John David Anglin" To: dave@hiauly1.hia.nrc.ca (John David Anglin) Cc: gcc-patches@gcc.gnu.org, gcc-bugs@gcc.gnu.org, alan@linuxcare.com.au Subject: Re: [PATCH] Re: REG_DEAD/REG_EQUIV problem. Date: Thu, 15 Feb 2001 11:33:00 -0000 Message-id: <200102151932.OAA05634@hiauly1.hia.nrc.ca> References: X-SW-Source: 2001-02/msg00893.html > > > On Thu, 1 Feb 2001, Alan Modra wrote: > > > > > (insn 17 15 21 (set (reg/v:DI 69) > > > (mem/u:DI (lo_sum:DI (reg:DI 70) > > > (unspec:DI[ > > > (symbol_ref/v/f:DI ("*L$C0001")) > > > ] 0)) 0)) 83 {*pa.md:2368} (insn_list 15 (nil)) > > > (expr_list:REG_EQUIV (mem/u:DI (lo_sum:DI (reg:DI 70) > > > (unspec:DI[ > > > (symbol_ref/v/f:DI ("*L$C0001")) > > > ] 0)) 0) > > > (expr_list:REG_DEAD (reg:DI 70) > > > (nil)))) > > > > Whee, maybe I'll be a gcc hacker one day. > > > > This fixes the problem for me. I've no idea whether it's the ideal > > solution - it may well be possible and desirable to prevent these > > conflicting notes being issued. > > > > gcc/ChangeLog > > * rtlanal.c (reg_death_note_p): New function. > > (reg_mentioned_dies): New function. > > * rtl.h (reg_mentioned_dies): Declare. > > * reload1.c (reload): Don't set reg_equiv_memory_loc for an insn > > that references memory via a reg that dies. I am not certain this is totally sufficient. I think it is possible for this type of situation to occur: (set (reg: A) (plus (reg: PIC) (high:SI (symbol_ref:SI ("X"))))) (set (reg: B) (mem/u:SI (lo_sum:SI (reg/f: A) (unspec:SI[(symbol_ref:SI ("X")])))) (set (reg: C) (mem/u:SI (lo_sum:SI (reg/f: A) (unspec:SI[(symbol_ref:SI ("X")])))) (expr_list:REG_DEAD (reg: A) (nil)) ... (set (reg: D) (reg: B)) The last set may be in a different basic block than the one in which B is set. Reload may try to substitute the MEM for B. It won't notice that the register mentioned in the MEM is dead. It would appear that we need to check whether any registers mentioned in the MEM are still alive at the point where D is set before substituting. I have a modified patch which stops the generation of the REG_EQUIV note in local-alloc (see below). However, it won't work if the above situation can occur. There is also a problem with the above MEM being incorrectly substuted into the insn at line 2099 of pa.md during reload. I believe this is because the Q constraint is relaxed during reload: /* Subroutine for EXTRA_CONSTRAINT. Return 1 iff OP is a pseudo which did not get a hard register and we are running the reload pass. */ #define IS_RELOADING_PSEUDO_P(OP) \ ((reload_in_progress \ && GET_CODE (OP) == REG \ && REGNO (OP) >= FIRST_PSEUDO_REGISTER \ && reg_renumber [REGNO (OP)] < 0)) /* Optional extra constraints for this machine. Borrowed from sparc.h. For the HPPA, `Q' means that this is a memory operand but not a symbolic memory operand. Note that an unassigned pseudo register is such a memory operand. Needed because reload will generate these things in insns and then not re-recognize the insns, causing constrain_operands to fail. `R' is used for scaled indexed addresses. `S' is the constant 31. `T' is for fp loads and stores. */ #define EXTRA_CONSTRAINT(OP, C) \ ((C) == 'Q' ? \ (IS_RELOADING_PSEUDO_P (OP) \ || (GET_CODE (OP) == MEM \ && (memory_address_p (GET_MODE (OP), XEXP (OP, 0))\ || reload_in_progress) \ && ! symbolic_memory_operand (OP, VOIDmode) \ && !(GET_CODE (XEXP (OP, 0)) == PLUS \ && (GET_CODE (XEXP (XEXP (OP, 0), 0)) == MULT\ || GET_CODE (XEXP (XEXP (OP, 0), 1)) == MULT))))\ : ((C) == 'R' ? \ (GET_CODE (OP) == MEM \ && GET_CODE (XEXP (OP, 0)) == PLUS \ && (GET_CODE (XEXP (XEXP (OP, 0), 0)) == MULT \ || GET_CODE (XEXP (XEXP (OP, 0), 1)) == MULT) \ && (move_operand (OP, GET_MODE (OP)) \ || memory_address_p (GET_MODE (OP), XEXP (OP, 0))\ || reload_in_progress)) \ I have modified symbolic_memory_operand to detect pic symbolic references and would like to get rid of the reload_in_progress stuff in EXTRA_CONSTRAINT. This seems bogus to me and an attempt to work around some kind of reload bug. Maybe Jeff can recall why it is there? A simple way to avoid the problem is to turn off the unchanging bit in pic MEMs. This prevents the REG_EQUIV notes from being generated. I have done a complete build with the unchanging bit off. There are two testsuite failures that I don't think are present in non PIC builds and checks. These are gcc.c-torture/compile/900313-1.c and gcc.c-torture/execute/941014-1.c: Executing on host: /xxx/gnu/gcc-2.97/objdir/gcc/xgcc -B/xxx/gnu/gcc-2.97/objdir/gcc/ /xxx/gnu/gcc-2.97/gcc/testsuite/gcc.c-torture/compile/900313-1.c -w -O0 -c -fPIC -o /xxx/gnu/gcc-2.97/objdir/gcc/testsuite/900313-1.o (timeout = -1) /xxx/gnu/gcc-2.97/gcc/testsuite/gcc.c-torture/compile/900313-1.c: In function `main': /xxx/gnu/gcc-2.97/gcc/testsuite/gcc.c-torture/compile/900313-1.c:7: Insn does not satisfy its constraints: (insn 104 55 107 (set (reg:SI 111) (reg:SI 1 %r1)) 69 {*pa.md:2099} (nil) (nil)) /xxx/gnu/gcc-2.97/gcc/testsuite/gcc.c-torture/compile/900313-1.c:7: confused by earlier errors, bailing out I will try to figure out why SI 111 didn't get a hard register. Comments? Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) 2001-02-13 John David Anglin * local-alloc.c (update_equiv_regs): Don't change a REG_EQUAL note to REG_EQUIV note if the MEM contains a reference to a register which dies in the insn. * rtlanal.c (reg_death_note_p): New function. (reg_mentioned_dies): New function. * rtl.h (reg_mentioned_dies): Declare. --- local-alloc.c.orig Fri Feb 2 00:54:57 2001 +++ local-alloc.c Tue Feb 13 17:42:12 2001 @@ -822,6 +822,7 @@ rtx set; rtx dest, src; int regno; + int mem_is_dead = 0; if (GET_CODE (insn) == NOTE) { @@ -956,9 +957,19 @@ reg_equiv[regno].init_insns = gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns); + /* Make sure that a MEM doesn't use a register that dies in + this insn. */ + if (GET_CODE (src) == MEM) + { + rtx death = find_reg_note (insn, REG_DEAD, NULL_RTX); + if (death) + if (reg_mentioned_dies (src, death)) + mem_is_dead = 1; + } + /* If this register is known to be equal to a constant, record that it is always equivalent to the constant. */ - if (note && ! rtx_varies_p (XEXP (note, 0), 0)) + if (note && ! mem_is_dead && ! rtx_varies_p (XEXP (note, 0), 0)) PUT_MODE (note, (enum machine_mode) REG_EQUIV); /* If this insn introduces a "constant" register, decrease the priority --- rtl.h.orig Sun Jan 7 13:39:19 2001 +++ rtl.h Mon Feb 12 15:08:06 2001 @@ -1403,6 +1403,7 @@ extern int computed_jump_p PARAMS ((rtx)); typedef int (*rtx_function) PARAMS ((rtx *, void *)); extern int for_each_rtx PARAMS ((rtx *, rtx_function, void *)); +extern int reg_mentioned_dies PARAMS ((rtx, rtx)); extern rtx regno_use_in PARAMS ((unsigned int, rtx)); extern int auto_inc_p PARAMS ((rtx)); extern void remove_node_from_expr_list PARAMS ((rtx, rtx *)); --- rtlanal.c.orig Thu Feb 8 14:05:50 2001 +++ rtlanal.c Mon Feb 12 15:08:06 2001 @@ -27,6 +27,7 @@ static void set_of_1 PARAMS ((rtx, rtx, void *)); static void insn_dependent_p_1 PARAMS ((rtx, rtx, void *)); +static int reg_death_note_p PARAMS ((rtx *, void *)); /* Forward declarations */ static int computed_jump_p_1 PARAMS ((rtx)); @@ -2327,6 +2328,34 @@ return 0; } +/* Predicate function for reg_mentioned_dies. */ + +static int +reg_death_note_p (reg, notes) + rtx *reg; + void *notes; +{ + rtx link; + + if (GET_CODE (*reg) != REG) + return 0; + + for (link = (rtx) notes; link; link = XEXP (link, 1)) + if (REG_NOTE_KIND (link) == REG_DEAD && *reg == XEXP (link, 0)) + return 1; + return 0; +} + +/* Return 1 if any register found in INSN has a REG_DEAD note in NOTES. */ + +int +reg_mentioned_dies (insn, notes) + rtx insn; + rtx notes; +{ + return for_each_rtx (&insn, reg_death_note_p, notes); +} + /* Searches X for any reference to REGNO, returning the rtx of the reference found if any. Otherwise, returns NULL_RTX. */