Re: Patches ping (toplev.c, cse.c)

[Roger Sayle <roger at eyesopen dot com>]

On Mon, 9 Jan 2006, Hans-Peter Nilsson wrote:
> 2. "[RFA:] Find more equivalents in cse.c:find_comparison_args"
> <URL:http://gcc.gnu.org/ml/gcc-patches/2005-12/msg01745.html>.
>
> 	* cse.c (find_comparison_args): For non-comparison equivalents,
> 	canonicalize on the ARG1 equivalent that has the highest cost.

Sorry for taking so long to review this one.  My initial instincts
told me to be cautious of modifying a part of an RTL optimizer to
return the *most* expensive equivalent expression!?  However, after
your ping I thought that the most expensive equivalent expression
is not a bad heursitic for potential information content.  Clearly
in this case you'd like to see the ABS instead of a pseudo in a
comparison against a constant.  So provided that the cse.c function
find_comparison_args is only used in the analysis of comparisons
and the results never escape, your strategy seems useful.

Unfortunately, this is not the case, and my initial worries are
valid.  Not only is there more than one use of find_comparison_args,
but the critical one directly modifies folded_arg0 and folded_arg1
which do later get used for constructing new RTL.


But I do still like this optimization, and that its in cse.c
rather than combine.c or gcse.c.  However, I would leave the
find_comparison_args function alone, and instead add/duplicate
some of its functionality to fold_rtx (it's caller).

The theory would go something like this.  If the second operand
is a constant value, look through each of the equivalent
expressions of the first operand, and call
simplify_const_relational_operation on each one/pair.  If the
result is non-NULL, you've folded the rtx.  Not only would
this catch "abs(x) >= 0", but also "(x & 7) < 10", where we'd
not normally prefer a AND in find_comparison_args.

Depending on how well this goes, you could even try using
simplify_relational_operation, and then select the result
with the lowest COST, allowing "abs(x) == 0" -> "x == 0".



My apologies to those people that think cse.c should die, and
that adding optimizations to fold_rtx is bizarre.  But there
are a number of optimizations (of the form "we can calculate
x cheaper if we have y available") that fit better here than
anywhere else.  Another good example is "x - y" is cheaply
computed using negation if "y - x" is available in CSE's hash
table.  Or "x * 7" is cheap, if "x * 6" is available.  These
context dependent transformations are higher-level then GCC's
simplify-rtx.c.  They're also perfect candidates for a tree-ssa
pass (hint, hint)!


My apologies again to HP for taking so long to fully investigate
whether his fix was safe or not.  Paolo, StevenB any comments?

Roger
--