On 05/24/2011 03:34 PM, Jeff Law wrote:
>
> This has gone latent on the trunk, but the underlying issue hasn't been
> resolved.
>
> ira.c::update_equiv_regs can create REG_EQUIV notes for equivalences
> which are local to a block rather than the traditional function-wide
> equivalences we typically work with.
>
> This occurs when we have an insn that loads a pseudo from a MEM and the
> pseudo is used within only a single block and the MEM remains unchanged
> through the life of the pseudo.
>
> Starting with the assumption that we're going to create a block local
> pseudo under the rules noted above, consider this RTL:
>
> (set (reg X) (some address))
> (set (reg Y) (mem (reg X)))
> (use Y)
>
>
> We're going to create an equivalence between (reg Y) and its memory
> location in update_equiv_regs. Assume IRA is able to allocate a hard
> reg for reg X, but not reg Y.
>
> reload's strategy in this situation will be to remove the insn which
> creates the equivalence between reg Y and the memory location. Uses of
> reg Y will be replaced with the equivalent memory location.
>
> That's all fine and good, except reload uses delete_dead_insn, which
> deletes the equivalencing insn, but also recursively tries to remove the
> prior insn if it becomes dead as a result of removing the equivalencing
> insn.
>
> Anyway, continuing with our example, reg X gets a hard reg, so our RTL
> will look something like
>
> (set (reg 0) (some address))
> (set (reg Y) (mem (reg 0)))
> (use Y)
>
> Then we remove the equivalencing insn resulting in
>
> (set (reg 0) (some address)
> (use Y)
>
> And we recurse from delete_dead_insn and determine that the first insn
> was dead as well, so it gets removed leaving:
>
> (use Y)
>
> We then replace Y with its equivalent memory location
>
> (use (mem (reg 0))
>
> At which point we lose because hard reg 0 is no longer initialized.
>
>
> The code in question is literally 20 years old and predates running any
> real dead code elimination after reload. ISTM the right thing to do is
> stop using delete_dead_insn in this code and let the post-reload DCE
> pass do its job. That allows us to continue to record the block local
> equivalence.
Sounds like the right thing to do. OK. (Can we eliminate the other caller?)
I've looked at code generation; it appears unchanged on i686-linux,
which I think is the expected result. There are minor differences in
assembly output on mips64-linux. If you want to look at it, I'm
attaching a testcase - compile with "-O2 -fno-reorder-blocks".
Bernd
typedef _Bool bool;
typedef struct { volatile int counter; }
atomic_t;
struct list_head { struct list_head *next, *prev; };
typedef void (*ctor_fn_t)(void);
struct kref { atomic_t refcount; };
struct kobject { const char *name; struct list_head entry; struct kobject
*parent; struct kset *kset; struct kobj_type *ktype; struct sysfs_dirent
*sd; struct kref kref; unsigned int state_initialized:1; unsigned int
state_in_sysfs:1; unsigned int state_add_uevent_sent:1; unsigned int
state_remove_uevent_sent:1; unsigned int uevent_suppress:1; };
static inline __attribute__((always_inline)) void _do_trace_module_get
(void (*probe)(char *name, bool wait, unsigned long ip)) { return -38; }
struct module_kobject { struct kobject kobj; struct module *mod; struct
kobject *drivers_dir; struct module_param_attrs *mp; };
enum module_state { MODULE_STATE_LIVE, MODULE_STATE_COMING,
MODULE_STATE_GOING, };
struct module { enum module_state state; struct list_head list; char
name[(64 - sizeof(unsigned long))]; struct module_kobject mkobj; struct
module_attribute *modinfo_attrs; const char *version; const char *srcversion;
struct kobject *holders_dir; const struct kernel_symbol *syms; const
unsigned long *crcs; unsigned int num_syms; struct kernel_param *kp;
unsigned int num_kp; unsigned int num_gpl_syms; const struct kernel_symbol
*gpl_syms; const unsigned long *gpl_crcs;
struct list_head modules_which_use_me; struct task_struct *waiter; void
(*exit)(void); struct module_ref { unsigned int incs; unsigned int decs;
} *refptr; ctor_fn_t *ctors; unsigned int num_ctors; };
static inline __attribute__((always_inline)) int bscnl_emit(char *buf, int
buflen, int rbot, int rtop, int len) { if (len > 0) len += scnprintf(buf +
len, buflen - len, ","); if (rbot == rtop) len += scnprintf(buf + len,
buflen - len, "%d", rbot); else len += scnprintf(buf + len, buflen - len,
"%d-%d", rbot, rtop); return len; }
int bitmap_scnlistprintf(char *buf, unsigned int buflen, const unsigned long
*maskp, int nmaskbits) { int len = 0; int cur, rbot, rtop; if (buflen ==
0) return 0; buf[0] = 0; rbot = cur = find_next_bit((maskp), (nmaskbits),
0); while (cur < nmaskbits) { rtop = cur; cur = find_next_bit(maskp,
nmaskbits, cur+1); if (cur >= nmaskbits || cur > rtop + 1) { len =
bscnl_emit(buf, buflen, rbot, rtop, len); rbot = cur; } } return len; }
