This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]