This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Clean up MIPS branch handling, fix mips-ps-*.c failures
Adam Nemet <anemet@caviumnetworks.com> writes:
> Richard Sandiford writes:
>> So the change on its own was a net loss for you?
>
> To be precise this is the change I had in my tree:
>
> Index: config/mips/mips.c
> ===================================================================
> --- config/mips/mips.c (revision 111703)
> +++ config/mips/mips.c (working copy)
> @@ -3193,7 +3193,7 @@ gen_conditional_branch (rtx *operands, e
>
> mips_emit_compare (&code, &op0, &op1, TARGET_MIPS16);
> target = gen_rtx_IF_THEN_ELSE (VOIDmode,
> - gen_rtx_fmt_ee (code, GET_MODE (op0),
> + gen_rtx_fmt_ee (code, GET_MODE (op0) == CCmode? CCmode : VOIDmode,
> op0, op1),
> gen_rtx_LABEL_REF (VOIDmode, operands[0]),
> pc_rtx);
Out of interest, why did you keep CCmode for floating-point comparisons?
> And I was getting this diffstat on assembly generated from
> GCC preprocessed files:
>
> 39 files changed, 8229 insertions(+), 8118 deletions(-)
How does that break down into individual files? Are they all regressions?
Like I said in my original post, my run had some tests that improved and some
that regressed. One improvement was in gcc.c-torture/compile/20011029-1.c,
which came from combining:
(insn 27 46 28 4 (set (reg:DI 193 [ D.1484 ])
(sign_extend:DI (reg:SI 195))) 154 {extendsidi2} (nil)
(expr_list:REG_DEAD (reg:SI 195)
(nil)))
(jump_insn 28 27 31 4 (set (pc)
(if_then_else (ne (reg:DI 193 [ D.1484 ])
(const_int 0 [0x0]))
(label_ref:DI 43)
(pc))) 275 {*branch_equalitydi} (insn_list:REG_DEP_TRUE 27 (nil))
(expr_list:REG_DEAD (reg:DI 193 [ D.1484 ])
(expr_list:REG_BR_PROB (const_int 9900 [0x26ac])
(nil))))
into:
(jump_insn 28 27 31 4 (set (pc)
(if_then_else (ne (reg:SI 195)
(const_int 0 [0x0]))
(label_ref:DI 43)
(pc))) 274 {*branch_equalitysi} (nil)
(expr_list:REG_DEAD (reg:SI 195)
(expr_list:REG_BR_PROB (const_int 9900 [0x26ac])
(nil))))
One testcase that regressed was gcc.c-torture/compile/pr26425.c,
which was because of the simplify_relational_operation thing
(i.e. comparison of (plus x 1) and -1).
> After I removed the early return in simplify_relational_operation for
> mode == VOIDmode I am still down but less:
>
> 10 files changed, 192 insertions(+), 185 deletions(-)
Yeah, I wondered about doing that too, but I wasn't sure whether
it was a good idea. The assumption might have been that the
operands to VOIDmode comparisons are more restricted, so changing
them would not be helpful. I suppose the commonest case will be
where targets require a comparison against zero and where converting
(op foo 0) into (op foo' x), x != 0, would not be helpful.
I vaguely wondered about:
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c (revision 111867)
+++ gcc/simplify-rtx.c (working copy)
@@ -3504,7 +3504,7 @@ simplify_relational_operation (enum rtx_
return simplify_relational_operation (code, mode, VOIDmode,
XEXP (op0, 0), XEXP (op0, 1));
- if (mode == VOIDmode
+ if ((mode == VOIDmode && op1 == const0_rtx)
|| GET_MODE_CLASS (cmp_mode) == MODE_CC
|| CC0_P (op0))
return NULL_RTX;
In my runs, this gives the same testsuite output as removing the mode,
but I haven't thought much about it yet.
It's a shame that you're still seeing some regressions with the mode
test removed though. Can you point at a specific example? It actually
seemed to be a consistent win in my runs (compared to the output both
before and after yesterday's patch).
Richard