From: Eric Botcazou <ebotcazou@adacore.com>
To: Joern Rennecke <amylaar@spamcop.net>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: RFA: Fix dse / postreload not to bypass add expanders
Date: Thu, 03 Nov 2011 19:09:00 -0000 [thread overview]
Message-ID: <201111032001.00867.ebotcazou@adacore.com> (raw)
In-Reply-To: <20111101045348.qp9lxw7jhkogcook-nzlynne@webmail.spamcop.net>
> 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.rennecke@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
next prev parent reply other threads:[~2011-11-03 19:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-01 8:54 Joern Rennecke
2011-11-03 19:09 ` Eric Botcazou [this message]
2011-11-04 10:27 ` Paolo Bonzini
2011-11-04 10:37 ` Eric Botcazou
2011-11-04 10:53 ` amylaar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201111032001.00867.ebotcazou@adacore.com \
--to=ebotcazou@adacore.com \
--cc=amylaar@spamcop.net \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).