Commit 591d8571 by Richard Sandiford

cse: Use simplify_replace_fn_rtx to process notes [PR94740]

cse_process_notes did a very simple substitution, which in the wrong
circumstances could create non-canonical RTL and invalid MEMs.
Various sticking plasters have been applied to cse_process_notes_1
to handle cases like ZERO_EXTEND, SIGN_EXTEND and UNSIGNED_FLOAT,
but I think this PR is a plaster too far.

The code is trying hard to avoid creating unnecessary rtl, which of
course is a good thing.  If we continue to do that, then we can end
up changing subexpressions while keeping the containing rtx.
This in turn means that validate_change will be a no-op on the
containing rtx, even if its contents have changed.  So in these
cases we have to apply validate_change to the individual subexpressions.

On the other hand, if we always apply validate_change to the
individual subexpressions, we'll end up calling validate_change
on something before it has been simplified and canonicalised.
And that's one of the situations we're trying to avoid.

There might be a middle ground in which we queue the validate_changes
as part of a group, and so can cancel the pending validate_changes
for subexpressions if there's a change in the outer expression.
But that seems even more ad-hoc than the current code.
It would also be quite an invasive change.

I think the best thing is just to hook into the existing
simplify_replace_fn_rtx function, keeping the REG and MEM handling
from cse_process_notes_1 essentially unchanged.  It can generate
more redundant rtl when a simplification takes place, but it has
the advantage of being relative well-used code (both directly
and via simplify_replace_rtx).

2020-04-30  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR rtl-optimization/94740
	* cse.c (cse_process_notes_1): Replace with...
	(cse_process_note_1): ...this new function, acting as a
	simplify_replace_fn_rtx callback to process_note.  Handle only
	REGs and MEMs directly.  Validate the MEM if cse_process_note
	changes its address.
	(cse_process_notes): Replace with...
	(cse_process_note): ...this new function.
	(cse_extended_basic_block): Update accordingly, iterating over
	the register notes and passing individual notes to cse_process_note.
parent e72cfef3
2020-04-30 Richard Sandiford <richard.sandiford@arm.com>
PR rtl-optimization/94740
* cse.c (cse_process_notes_1): Replace with...
(cse_process_note_1): ...this new function, acting as a
simplify_replace_fn_rtx callback to process_note. Handle only
REGs and MEMs directly. Validate the MEM if cse_process_note
changes its address.
(cse_process_notes): Replace with...
(cse_process_note): ...this new function.
(cse_extended_basic_block): Update accordingly, iterating over
the register notes and passing individual notes to cse_process_note.
2020-04-30 Martin Jambor <mjambor@suse.cz>
PR ipa/94856
......
......@@ -585,7 +585,6 @@ static void cse_insn (rtx_insn *);
static void cse_prescan_path (struct cse_basic_block_data *);
static void invalidate_from_clobbers (rtx_insn *);
static void invalidate_from_sets_and_clobbers (rtx_insn *);
static rtx cse_process_notes (rtx, rtx, bool *);
static void cse_extended_basic_block (struct cse_basic_block_data *);
extern void dump_class (struct table_elt*);
static void get_cse_reg_info_1 (unsigned int regno);
......@@ -6222,75 +6221,28 @@ invalidate_from_sets_and_clobbers (rtx_insn *insn)
}
}
/* Process X, part of the REG_NOTES of an insn. Look at any REG_EQUAL notes
and replace any registers in them with either an equivalent constant
or the canonical form of the register. If we are inside an address,
only do this if the address remains valid.
static rtx cse_process_note (rtx);
OBJECT is 0 except when within a MEM in which case it is the MEM.
/* A simplify_replace_fn_rtx callback for cse_process_note. Process X,
part of the REG_NOTES of an insn. Replace any registers with either
an equivalent constant or the canonical form of the register.
Only replace addresses if the containing MEM remains valid.
Return the replacement for X. */
Return the replacement for X, or null if it should be simplified
recursively. */
static rtx
cse_process_notes_1 (rtx x, rtx object, bool *changed)
cse_process_note_1 (rtx x, const_rtx, void *)
{
enum rtx_code code = GET_CODE (x);
const char *fmt = GET_RTX_FORMAT (code);
int i;
switch (code)
{
case CONST:
case SYMBOL_REF:
case LABEL_REF:
CASE_CONST_ANY:
case PC:
case CC0:
case LO_SUM:
return x;
case MEM:
validate_change (x, &XEXP (x, 0),
cse_process_notes (XEXP (x, 0), x, changed), 0);
return x;
case EXPR_LIST:
if (REG_NOTE_KIND (x) == REG_EQUAL)
XEXP (x, 0) = cse_process_notes (XEXP (x, 0), NULL_RTX, changed);
/* Fall through. */
case INSN_LIST:
case INT_LIST:
if (XEXP (x, 1))
XEXP (x, 1) = cse_process_notes (XEXP (x, 1), NULL_RTX, changed);
return x;
case SIGN_EXTEND:
case ZERO_EXTEND:
case SUBREG:
if (MEM_P (x))
{
rtx new_rtx = cse_process_notes (XEXP (x, 0), object, changed);
/* We don't substitute VOIDmode constants into these rtx,
since they would impede folding. */
if (GET_MODE (new_rtx) != VOIDmode)
validate_change (object, &XEXP (x, 0), new_rtx, 0);
validate_change (x, &XEXP (x, 0), cse_process_note (XEXP (x, 0)), false);
return x;
}
case UNSIGNED_FLOAT:
if (REG_P (x))
{
rtx new_rtx = cse_process_notes (XEXP (x, 0), object, changed);
/* We don't substitute negative VOIDmode constants into these rtx,
since they would impede folding. */
if (GET_MODE (new_rtx) != VOIDmode
|| (CONST_INT_P (new_rtx) && INTVAL (new_rtx) >= 0)
|| (CONST_DOUBLE_P (new_rtx) && CONST_DOUBLE_HIGH (new_rtx) >= 0))
validate_change (object, &XEXP (x, 0), new_rtx, 0);
return x;
}
case REG:
i = REG_QTY (REGNO (x));
int i = REG_QTY (REGNO (x));
/* Return a constant or a constant register. */
if (REGNO_QTY_VALID_P (REGNO (x)))
......@@ -6309,26 +6261,19 @@ cse_process_notes_1 (rtx x, rtx object, bool *changed)
/* Otherwise, canonicalize this register. */
return canon_reg (x, NULL);
default:
break;
}
for (i = 0; i < GET_RTX_LENGTH (code); i++)
if (fmt[i] == 'e')
validate_change (object, &XEXP (x, i),
cse_process_notes (XEXP (x, i), object, changed), 0);
return x;
return NULL_RTX;
}
/* Process X, part of the REG_NOTES of an insn. Replace any registers in it
with either an equivalent constant or the canonical form of the register.
Only replace addresses if the containing MEM remains valid. */
static rtx
cse_process_notes (rtx x, rtx object, bool *changed)
cse_process_note (rtx x)
{
rtx new_rtx = cse_process_notes_1 (x, object, changed);
if (new_rtx != x)
*changed = true;
return new_rtx;
return simplify_replace_fn_rtx (x, NULL_RTX, cse_process_note_1, NULL);
}
......@@ -6623,14 +6568,19 @@ cse_extended_basic_block (struct cse_basic_block_data *ebb_data)
{
/* Process notes first so we have all notes in canonical forms
when looking for duplicate operations. */
if (REG_NOTES (insn))
{
bool changed = false;
REG_NOTES (insn) = cse_process_notes (REG_NOTES (insn),
NULL_RTX, &changed);
for (rtx note = REG_NOTES (insn); note; note = XEXP (note, 1))
if (REG_NOTE_KIND (note) == REG_EQUAL)
{
rtx newval = cse_process_note (XEXP (note, 0));
if (newval != XEXP (note, 0))
{
XEXP (note, 0) = newval;
changed = true;
}
}
if (changed)
df_notes_rescan (insn);
}
cse_insn (insn);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment