> This patch makes emit_inc_dec_insn_before use add3_insn / gen_move_insn
> so that the appropriate expanders are used to create the new instructions,
> and for dse it use the available register liveness information to check
> that no live fixed hard register, like a flags register, is clobbered in
> the process. For postreload, there is no such information available, so we
> give up when we see a clobber / set that might be problematic.
2011-10-31 Joern Rennecke <joern.renne...@embecosm.com>
* regset.h (fixed_regset): Declare.
* dse.c: Include regset.h .
(struct insn_info): Add member fixed_regs_live.
(note_add_store_info): New typedef.
(note_add_store): New function.
(emit_inc_dec_insn_before): Expect arg to be of type insn_info_t .
Use gen_add3_insn / gen_move_insn.
Check new insn for unwanted clobbers before emitting it.
(check_for_inc_dec): Rename to...
(check_for_inc_dec_1:) ... this. Return bool. Take insn_info
parameter. Changed all callers in file.
(check_for_inc_dec, copy_fixed_regs): New functions.
(scan_insn): Set fixed_regs_live field of insn_info.
* rtl.h (check_for_inc_dec): Update prototype.
* postreload.c (reload_cse_simplify): Take new signature of
check_ind_dec into account.
* reginfo.c (fixed_regset): New variable.
(init_reg_sets_1): Initialize it.
OK modulo the following:
+typedef struct
+{
+ rtx insert_before;
This field is never read.
+ rtx first, current;
+ regset fixed_regs_live;
+ bool failure;
+} note_add_store_info;
+
+/* Callback for emit_inc_dec_insn_before via note_stores.
+ Check if a register is clobbered which is life afterwards. */
"live"
+static void
+note_add_store (rtx loc, const_rtx expr ATTRIBUTE_UNUSED, void *data)
Missing blank line. The functions in dse.c have a blank line between head
comment and body.
+{
+ rtx insn, *nextp;
+ note_add_store_info *info = (note_add_store_info *) data;
+ int r, n;
+
+ if (!REG_P (loc))
+ return;
+ /* If this register is referenced by the current or an earlier insn,
+ that's OK. E.g. this applies to the register that is being incremented
+ with this addition. */
Blank line before the comment.
+ nextp = &info->first;
+ do
+ {
+ insn = *nextp;
+ nextp = &NEXT_INSN (insn);
+ if (reg_referenced_p (loc, PATTERN (insn)))
+ return;
+ }
+ while (insn != info->current);
Isn't that a convoluted way of writing this?
for (insn = info->first;
insn != NEXT_INSN (info->current);
insn = NEXT_INSN (insn))
if (reg_referenced_p (loc, PATTERN (insn)))
return;
+ if (!info->fixed_regs_live)
+ {
+ info->failure = true;
+ return;
+ }
Missing comment explaining why we do that.
+ /* Now check if this is a live fixed register. */
+ r = REGNO (loc);
+ n = HARD_REGNO_NREGS (r, GET_MODE (loc));
+ while (--n >= 0)
+ if (REGNO_REG_SET_P (info->fixed_regs_live, r+n))
+ info->failure = true;
Blank line before the comment. What's the point in the reverse iteration?
for (i = 0; i < hard_regno_nregs[regno][GET_MODE (loc)]; i++)
if (REGNO_REG_SET_P (info->fixed_regs_live, regno + i))
{
info->failure = true;
return;
}
hard_regno_nregs in small letters.
+ info.insert_before = insn;
+ info.first = new_insn;
+ info.fixed_regs_live = insn_info->fixed_regs_live;
+ info.failure = false;
+ for (cur = new_insn; cur; cur = NEXT_INSN (cur))
+ {
+ info.current = cur;
+ note_stores (PATTERN (cur), note_add_store, &info);
+ }
+ if (info.failure)
+ return 1;
Missing comment explaining what we're doing.
/* Before we delete INSN, make sure that the auto inc/dec, if it is
- there, is split into a separate insn. */
+ there, is split into a separate insn.
+ Return true on success (or if there was nothing to do), false on failure.
*/
-void
-check_for_inc_dec (rtx insn)
+static bool
+check_for_inc_dec_1 (insn_info_t insn_info)
Missing adjustment in the comment: "Before we delete the insn described by
INSN_INFO, make sure..."
+/* Entry point for postreload. */
+bool
+check_for_inc_dec (rtx insn, regset fixed_regs_live)
No point in adding an argument if it is always null. Missing blank line and
head comment: "Same as above, but take a naked INSN instead. This is used by
passes like that don't compute precise liveness information."
+/* Return a bitmap of the fixed registers contained in IN. */
+static bitmap
+copy_fixed_regs (const_bitmap in)
+{
+ bitmap ret;
+
+ ret = ALLOC_REG_SET (NULL);
+ bitmap_and (ret, in, fixed_regset);
+ return ret;
+}
Missing blank line.
+/* Same information as fixed_reg_set but in regset form. */
+regset fixed_regset;
Hum, you'd better have a good trick to remember which is which. This isn't
pretty, but let's mimic what is just above:
/* Same information as FIXED_REG_SET but in regset form. */
regset fixed_reg_set_regset;
--
Eric Botcazou
