This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Fix PR 22445
- From: Joern RENNECKE <joern dot rennecke at st dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 13 Jul 2005 18:19:51 +0100
- Subject: Re: RFA: Fix PR 22445
- References: <42D502C6.2050805@st.com> <20050713161845.GA2657@redhat.com>
Richard Henderson wrote:
On Wed, Jul 13, 2005 at 01:02:14PM +0100, Joern RENNECKE wrote:
case 'e':
+ if (i > 0 && fmt[i-1] == 'e'
+ && targetm.commutative_p (x, UNKNOWN)
+ && rtx_equal_for_cselib_p (XEXP (x, i), XEXP (y, i-1))
+ && rtx_equal_for_cselib_p (XEXP (x, i-1), XEXP (y, i)))
Do you actually need the fmt[i-1] == 'e' check, given that
targetm.commutative_p should never return true for a code
that isn't actually fmt="ee" ?
Indeed, from a correctness standpoint, it is not needed. The rtl concept of
commutativity is much narrower (it applies to binary operations only)
than the
constraints / md one (you can specify for any pair of consecutive
operands that
they may be interchanged, and there might be other unrelated operands around
too).
I thought it was still a sensible quick-to-do test to avoid the
targetm.commutative_p
call, but that doesn't hold up under closer scrutiny:
bash-2.05b$ grep '"[^"]*"[^"]*"[^"]*[^e"]e[^"]*"' rtl.def
DEF_RTL_EXPR(INSN_LIST, "insn_list", "ue", RTX_EXTRA)
DEF_RTL_EXPR(INSN, "insn", "iuuBieiee", RTX_INSN)
DEF_RTL_EXPR(JUMP_INSN, "jump_insn", "iuuBieiee0", RTX_INSN)
DEF_RTL_EXPR(CALL_INSN, "call_insn", "iuuBieieee", RTX_INSN)
DEF_RTL_EXPR(ADDR_DIFF_VEC, "addr_diff_vec", "eEee0", RTX_EXTRA)
DEF_RTL_EXPR(VAR_LOCATION, "var_location", "te", RTX_EXTRA)
DEF_RTL_EXPR(DEFINE_PREDICATE, "define_predicate", "ses", RTX_EXTRA)
DEF_RTL_EXPR(DEFINE_SPECIAL_PREDICATE, "define_special_predicate",
"ses", RTX_EXTRA)
DEF_RTL_EXPR(DEFINE_INSN_RESERVATION, "define_insn_reservation", "sies",
RTX_EXTRA)
DEF_RTL_EXPR(DEFINE_ATTR, "define_attr", "sse", RTX_EXTRA)
(set_attr "name" "value") is equivalent to
(set (attr "name") (const_string "value")) */
(set (attr "att") (cond [(eq_attrq "alternative" "1") (const_string "a1")
DEF_RTL_EXPR(COND, "cond", "Ee", RTX_EXTRA)
When we know that commutative rtl operations are always of "ee" type,
the only
value the variable "i" can have for a match is 1, and a successful match
of both
operands implies a successful match of the entire operation.
I have appended an updated patch, which I am re-testing in the
sh-elf-4_1-branch.
To do mainline testing, I still need a resulution to PR 22258 (The
latest patch is still
at http://gcc.gnu.org/ml/gcc-patches/2005-07/msg00053.html,
regtest/bootstrap
has finished successfully long since.)
2005-07-12 J"orn Rennecke <joern.rennecke@st.com>
PR rtl-optimization/22445
* cselib.c (target.h): Include.
(rtx_equal_for_cselib_p): Allow commutative matches.
(cselib_hash_rtx): Don't use MODE for CONST_INT hashing.
Remove MODE parameter. Changed all callers.
Index: cselib.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cselib.c,v
retrieving revision 1.59.4.1
diff -p -r1.59.4.1 cselib.c
*** cselib.c 6 Jul 2005 21:34:18 -0000 1.59.4.1
--- cselib.c 13 Jul 2005 17:02:02 -0000
*************** Software Foundation, 51 Franklin Street,
*** 41,46 ****
--- 41,47 ----
#include "cselib.h"
#include "params.h"
#include "alloc-pool.h"
+ #include "target.h"
static bool cselib_record_memory;
static int entry_and_rtx_equal_p (const void *, const void *);
*************** static int discard_useless_locs (void **
*** 54,60 ****
static int discard_useless_values (void **, void *);
static void remove_useless_values (void);
static rtx wrap_constant (enum machine_mode, rtx);
! static unsigned int cselib_hash_rtx (rtx, enum machine_mode, int);
static cselib_val *new_cselib_val (unsigned int, enum machine_mode);
static void add_mem_for_addr (cselib_val *, cselib_val *, rtx);
static cselib_val *cselib_lookup_mem (rtx, int);
--- 55,61 ----
static int discard_useless_values (void **, void *);
static void remove_useless_values (void);
static rtx wrap_constant (enum machine_mode, rtx);
! static unsigned int cselib_hash_rtx (rtx, int);
static cselib_val *new_cselib_val (unsigned int, enum machine_mode);
static void add_mem_for_addr (cselib_val *, cselib_val *, rtx);
static cselib_val *cselib_lookup_mem (rtx, int);
*************** rtx_equal_for_cselib_p (rtx x, rtx y)
*** 499,504 ****
--- 500,510 ----
break;
case 'e':
+ if (i == 1
+ && targetm.commutative_p (x, UNKNOWN)
+ && rtx_equal_for_cselib_p (XEXP (x, 1), XEXP (y, 0))
+ && rtx_equal_for_cselib_p (XEXP (x, 0), XEXP (y, 1)))
+ return 1;
if (! rtx_equal_for_cselib_p (XEXP (x, i), XEXP (y, i)))
return 0;
break;
*************** wrap_constant (enum machine_mode mode, r
*** 546,556 ****
Possible reasons for return 0 are: the object is volatile, or we couldn't
find a register or memory location in the table and CREATE is zero. If
CREATE is nonzero, table elts are created for regs and mem.
! MODE is used in hashing for CONST_INTs only;
! otherwise the mode of X is used. */
static unsigned int
! cselib_hash_rtx (rtx x, enum machine_mode mode, int create)
{
cselib_val *e;
int i, j;
--- 552,573 ----
Possible reasons for return 0 are: the object is volatile, or we couldn't
find a register or memory location in the table and CREATE is zero. If
CREATE is nonzero, table elts are created for regs and mem.
! N.B. this hash function returns the same hash value for RTXes that
! differ only in the order of operands, thus it is suitable for comparisons
! that take commutativity into account.
! If we wanted to also support associative rules, we'd have to use a different
! strategy to avoid returning spurious 0, e.g. return ~(~0U >> 1) .
! We used to have a MODE argument for hashing for CONST_INTs, but that
! didn't make sense, since it caused spurious hash differences between
! (set (reg:SI 1) (const_int))
! (plus:SI (reg:SI 2) (reg:SI 1))
! and
! (plus:SI (reg:SI 2) (const_int))
! If the mode is important in any context, it must be checked specifically
! in a comparison anyway, since relying on hash differences is unsafe. */
static unsigned int
! cselib_hash_rtx (rtx x, int create)
{
cselib_val *e;
int i, j;
*************** cselib_hash_rtx (rtx x, enum machine_mod
*** 572,578 ****
return e->value;
case CONST_INT:
! hash += ((unsigned) CONST_INT << 7) + (unsigned) mode + INTVAL (x);
return hash ? hash : (unsigned int) CONST_INT;
case CONST_DOUBLE:
--- 589,595 ----
return e->value;
case CONST_INT:
! hash += ((unsigned) CONST_INT << 7) + INTVAL (x);
return hash ? hash : (unsigned int) CONST_INT;
case CONST_DOUBLE:
*************** cselib_hash_rtx (rtx x, enum machine_mod
*** 596,602 ****
for (i = 0; i < units; ++i)
{
elt = CONST_VECTOR_ELT (x, i);
! hash += cselib_hash_rtx (elt, GET_MODE (elt), 0);
}
return hash;
--- 613,619 ----
for (i = 0; i < units; ++i)
{
elt = CONST_VECTOR_ELT (x, i);
! hash += cselib_hash_rtx (elt, 0);
}
return hash;
*************** cselib_hash_rtx (rtx x, enum machine_mod
*** 644,650 ****
case 'e':
{
rtx tem = XEXP (x, i);
! unsigned int tem_hash = cselib_hash_rtx (tem, 0, create);
if (tem_hash == 0)
return 0;
--- 661,667 ----
case 'e':
{
rtx tem = XEXP (x, i);
! unsigned int tem_hash = cselib_hash_rtx (tem, create);
if (tem_hash == 0)
return 0;
*************** cselib_hash_rtx (rtx x, enum machine_mod
*** 656,662 ****
for (j = 0; j < XVECLEN (x, i); j++)
{
unsigned int tem_hash
! = cselib_hash_rtx (XVECEXP (x, i, j), 0, create);
if (tem_hash == 0)
return 0;
--- 673,679 ----
for (j = 0; j < XVECLEN (x, i); j++)
{
unsigned int tem_hash
! = cselib_hash_rtx (XVECEXP (x, i, j), create);
if (tem_hash == 0)
return 0;
*************** cselib_lookup (rtx x, enum machine_mode
*** 936,942 ****
if (MEM_P (x))
return cselib_lookup_mem (x, create);
! hashval = cselib_hash_rtx (x, mode, create);
/* Can't even create if hashing is not possible. */
if (! hashval)
return 0;
--- 953,959 ----
if (MEM_P (x))
return cselib_lookup_mem (x, create);
! hashval = cselib_hash_rtx (x, create);
/* Can't even create if hashing is not possible. */
if (! hashval)
return 0;