From: Alan Modra <amodra@gmail.com>
To: Jeff Law <law@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: PR79286, ira combine_and_move_insns in loops
Date: Sat, 11 Feb 2017 00:52:00 -0000 [thread overview]
Message-ID: <20170211002955.GR3731@bubble.grove.modra.org> (raw)
In-Reply-To: <e7d07419-4c05-a11a-ce99-75b2e4bf6c28@redhat.com>
On Fri, Feb 03, 2017 at 01:55:33AM -0700, Jeff Law wrote:
> That seems pretty pessimistic -- do we have dominance information at this
> point? If so we could check that the assignment to the register dominates
> the use. If they are in the same block, then you have to look at LUIDs or
> somesuch.
>
> That would address the problem you're trying to solve without pessimizing
> the code.
Fair enough. Revised and regression tested x86_64-linux.
PR rtl-optimization/79286
* ira.c (def_dominates_uses): New function.
(update_equiv_regs): Don't create an equivalence for insns that
may trap where the register def does not dominate the use.
testsuite/
* gcc.c-torture/execute/pr79286.c: New.
diff --git a/gcc/ira.c b/gcc/ira.c
index 96b4b62..6fb8aaf 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -3300,6 +3300,49 @@ adjust_cleared_regs (rtx loc, const_rtx old_rtx ATTRIBUTE_UNUSED, void *data)
return NULL_RTX;
}
+/* Given register REGNO is set only once, return true if the defining
+ insn dominates all uses. */
+
+static bool
+def_dominates_uses (int regno)
+{
+ df_ref def = DF_REG_DEF_CHAIN (regno);
+
+ struct df_insn_info *def_info = DF_REF_INSN_INFO (def);
+ /* If this is an artificial def (eh handler regs, hard frame pointer
+ for non-local goto, regs defined on function entry) then def_info
+ is NULL and the reg is always live before any use. We might
+ reasonably return true in that case, but since the only call
+ of this function is currently here in ira.c when we are looking
+ at a defining insn we can't have an artificial def as that would
+ bump DF_REG_DEF_COUNT. */
+ gcc_assert (DF_REG_DEF_COUNT (regno) == 1 && def_info != NULL);
+
+ rtx_insn *def_insn = DF_REF_INSN (def);
+ basic_block def_bb = BLOCK_FOR_INSN (def_insn);
+
+ for (df_ref use = DF_REG_USE_CHAIN (regno);
+ use;
+ use = DF_REF_NEXT_REG (use))
+ {
+ struct df_insn_info *use_info = DF_REF_INSN_INFO (use);
+ /* Only check real uses, not artificial ones. */
+ if (use_info)
+ {
+ rtx_insn *use_insn = DF_REF_INSN (use);
+ if (!DEBUG_INSN_P (use_insn))
+ {
+ basic_block use_bb = BLOCK_FOR_INSN (use_insn);
+ if (use_bb != def_bb
+ ? !dominated_by_p (CDI_DOMINATORS, use_bb, def_bb)
+ : DF_INSN_INFO_LUID (use_info) < DF_INSN_INFO_LUID (def_info))
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
/* Find registers that are equivalent to a single value throughout the
compilation (either because they can be referenced in memory or are
set once from a single constant). Lower their priority for a
@@ -3498,9 +3541,18 @@ update_equiv_regs (void)
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
/* If this register is known to be equal to a constant, record that
- it is always equivalent to the constant. */
+ it is always equivalent to the constant.
+ Note that it is possible to have a register use before
+ the def in loops (see gcc.c-torture/execute/pr79286.c)
+ where the reg is undefined on first use. If the def insn
+ won't trap we can use it as an equivalence, effectively
+ choosing the "undefined" value for the reg to be the
+ same as the value set by the def. */
if (DF_REG_DEF_COUNT (regno) == 1
- && note && ! rtx_varies_p (XEXP (note, 0), 0))
+ && note
+ && !rtx_varies_p (XEXP (note, 0), 0)
+ && (!may_trap_p (XEXP (note, 0))
+ || def_dominates_uses (regno)))
{
rtx note_value = XEXP (note, 0);
remove_note (insn, note);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr79286.c b/gcc/testsuite/gcc.c-torture/execute/pr79286.c
new file mode 100644
index 0000000..e6d0e93
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr79286.c
@@ -0,0 +1,15 @@
+int a = 0, c = 0;
+static int d[][8] = {};
+
+int main ()
+{
+ int e;
+ for (int b = 0; b < 4; b++)
+ {
+ __builtin_printf ("%d\n", b, e);
+ while (a && c++)
+ e = d[300000000000000000][0];
+ }
+
+ return 0;
+}
--
Alan Modra
Australia Development Lab, IBM
next prev parent reply other threads:[~2017-02-11 0:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-01 13:48 Alan Modra
2017-02-01 21:37 ` Alan Modra
2017-02-02 9:31 ` Alan Modra
2017-02-03 8:55 ` Jeff Law
2017-02-11 0:52 ` Alan Modra [this message]
2017-02-16 23:08 ` Jeff Law
2017-02-04 0:08 ` Jeff Law
2017-02-18 14:28 Dominique d'Humières
2017-02-21 23:35 ` Alan Modra
2017-02-22 12:10 ` Dominique d'Humières
2017-02-22 16:32 ` Jeff Law
2017-02-22 16:57 ` Dominique d'Humières
2017-02-23 8:55 ` Jeff Law
2017-02-23 10:13 ` Alan Modra
2017-02-23 15:22 ` Jeff Law
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=20170211002955.GR3731@bubble.grove.modra.org \
--to=amodra@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
/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).