[PATCH] PR opt/12260: Avoid (const (neg (const (plus X Y))))
Roger Sayle
roger@eyesopen.com
Wed Sep 24 18:09:00 GMT 2003
The following patch is my proposed fix for PR optimization/12260
which is an ICE on legal compiling the attached testcase on alpha.
I believe the fundamental problem, which this patch doesn't attempt
to address is that the semantics of the RTL operator CONST, i.e.
which expressions it is allowed to wrap, are insufficiently documented.
Ultimately, alpha.md tries to call output_addr_const on the RTX
(const:DI (neg:DI (const:DI (plus:DI (symbol_ref ...) (const_int ...)))))
Unfortunately, output_addr_const can't handle NEG expressions [the
assembler syntax would require parentheses] and so we abort.
I'd like to assume the fault is in the alpha back-end somewhere for
assuming that just because an expression matches CONSTANT_P that it
is valid as an operand in assembler. Failing that it is fault of
fold_rtx for calling gen_rtx_CONST to wrap arbitrary unary operations
on line 3606. Perhaps the reviewer could cast an opinion either way.
My contribution is to bypass the above issue altogether to resolve
this failure by improving our RTL simplification routines so that we
never create anything as hideous as (const (neg (const (plus X Y)))).
Firstly, for any unary operator (op (const (foo))) should x-ray through
the const and try to simplify "(op (foo))". Then in addition to our
current "normalization" (neg (plus X Y)) -> (minus (neg X) Y), which
doesn't automatically reduce operation count, i.e add the optimization
that (neg (plus X C)) -> (minus -C X) for integer/real constants C,
and then for good measure (minus (neg X) C) -> (minus -C X). Then
finally, I discovered that simplify_plus_minus, we currently prefer
the first form with more operations than the second! A quick tweak
here avoids undoing all of our good work to improve code.
For our testcase (taken from the bugzilla PR), our expression now becomes
(const:DI (minus (const_int ...) (symbol_ref ...))) which is both far
less ugly, but also happily handled by output_addr_const. There's
more that can be done optimization wise, at one point we have:
(insn 32 31 33 2 (set (reg:DI 77)
(minus:DI (reg:DI 78)
(reg:DI 85))) 22 {subdi3} (nil)
(expr_list:REG_EQUAL (minus:DI (const_int 1 [0x1])
(symbol_ref:DI ("buf") [flags 0x2] <var_decl 0x4024db54 buf>))
(nil)))
(insn 33 32 34 2 (set (reg:DI 79)
(plus:DI (reg:DI 77)
(reg:DI 85))) 7 {*adddi_internal} (nil)
(expr_list:REG_EQUAL (plus:DI (reg:DI 77)
(symbol_ref:DI ("buf") [flags 0x2] <var_decl 0x4024db54 buf>))
(nil)))
Where clearly the expression (1-buf)+buf should simplify to just 1.
Unfortunately, we ignore REG_EQUAL notes in local_cprop, and then
the loop optimizers ruin everything by hoisting the first insn out
of the loop but not the second. A craftsman's work is never done!
The following patch has been tested on i686-pc-linux-gnu with a complete
"make bootstrap", all languages except treelang, and regression tested
with a top-level "make -k check" with no new failures. I've also
confirmed that it fixes the ICE in the new gcc.c-torture/compile testcase
in a cross-compiler gcc to alphaev68-unknown-linux-gnu hosted on the
same system.
Ok for mainline?
2003-09-24 Roger Sayle <roger@eyesopen.com>
PR optimization/12260
* simplify-rtx.c (simplify_unary_operation): Simplify all unary
operations through CONST nodes. Optimize (neg (plus X C)) as
(minus -C X) for constant values C.
(simplify_binary_operation): Optimize (minus (neg X) C) as
(minus -C X) for constant values C.
(simplify_plus_minus): Avoid creating (neg (const (plus X C)),
instead create (minus -C X).
* gcc.c-torture/compile/20030924-1.c: New test case.
Index: simplify-rtx.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
retrieving revision 1.160
diff -c -3 -p -r1.160 simplify-rtx.c
*** simplify-rtx.c 18 Sep 2003 19:07:04 -0000 1.160
--- simplify-rtx.c 24 Sep 2003 11:34:12 -0000
*************** simplify_unary_operation (enum rtx_code
*** 407,412 ****
--- 407,414 ----
return gen_rtx_CONST_VECTOR (mode, v);
}
}
+ else if (GET_CODE (op) == CONST)
+ return simplify_unary_operation (code, mode, XEXP (op, 0), op_mode);
if (VECTOR_MODE_P (mode) && GET_CODE (trueop) == CONST_VECTOR)
{
*************** simplify_unary_operation (enum rtx_code
*** 891,901 ****
return simplify_gen_binary (MINUS, mode, XEXP (op, 1),
XEXP (op, 0));
- /* (neg (plus A B)) is canonicalized to (minus (neg A) B). */
if (GET_CODE (op) == PLUS
&& !HONOR_SIGNED_ZEROS (mode)
&& !HONOR_SIGN_DEPENDENT_ROUNDING (mode))
{
temp = simplify_gen_unary (NEG, mode, XEXP (op, 0), mode);
return simplify_gen_binary (MINUS, mode, temp, XEXP (op, 1));
}
--- 893,914 ----
return simplify_gen_binary (MINUS, mode, XEXP (op, 1),
XEXP (op, 0));
if (GET_CODE (op) == PLUS
&& !HONOR_SIGNED_ZEROS (mode)
&& !HONOR_SIGN_DEPENDENT_ROUNDING (mode))
{
+ /* (neg (plus A C)) is simplified to (minus -C A). */
+ if (GET_CODE (XEXP (op, 1)) == CONST_INT
+ || GET_CODE (XEXP (op, 1)) == CONST_DOUBLE)
+ {
+ temp = simplify_unary_operation (NEG, mode, XEXP (op, 1),
+ mode);
+ if (temp)
+ return simplify_gen_binary (MINUS, mode, temp,
+ XEXP (op, 0));
+ }
+
+ /* (neg (plus A B)) is canonicalized to (minus (neg A) B). */
temp = simplify_gen_unary (NEG, mode, XEXP (op, 0), mode);
return simplify_gen_binary (MINUS, mode, temp, XEXP (op, 1));
}
*************** simplify_binary_operation (enum rtx_code
*** 1492,1497 ****
--- 1505,1520 ----
if (GET_CODE (op1) == NEG)
return simplify_gen_binary (PLUS, mode, op0, XEXP (op1, 0));
+ /* (-x - c) may be simplified as (-c - x). */
+ if (GET_CODE (op0) == NEG
+ && (GET_CODE (op1) == CONST_INT
+ || GET_CODE (op1) == CONST_DOUBLE))
+ {
+ tem = simplify_unary_operation (NEG, mode, op1, mode);
+ if (tem)
+ return simplify_gen_binary (MINUS, mode, tem, XEXP (op0, 0));
+ }
+
/* If one of the operands is a PLUS or a MINUS, see if we can
simplify this by the associative law.
Don't use the associative law for floating point.
*************** simplify_plus_minus (enum rtx_code code,
*** 2295,2300 ****
--- 2318,2330 ----
/* Sort the operations based on swap_commutative_operands_p. */
qsort (ops, n_ops, sizeof (*ops), simplify_plus_minus_op_data_cmp);
+ /* Create (minus -C X) instead of (neg (const (plus X C))). */
+ if (n_ops == 2
+ && GET_CODE (ops[1].op) == CONST_INT
+ && CONSTANT_P (ops[0].op)
+ && ops[0].neg)
+ return gen_rtx_fmt_ee (MINUS, mode, ops[1].op, ops[0].op);
+
/* We suppressed creation of trivial CONST expressions in the
combination loop to avoid recursion. Create one manually now.
The combination loop should have ensured that there is exactly
/* PR optimization/12260. */
extern int f(void);
extern int g(int);
static char buf[512];
void h(int l) {
while (l) {
char *op = buf;
if (f() == 0)
break;
if (g(op - buf + 1))
break;
}
}
Roger
--
Roger Sayle, E-mail: roger@eyesopen.com
OpenEye Scientific Software, WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road, Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507. Fax: (+1) 505-473-0833
More information about the Gcc-patches
mailing list