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: PATCH for [ARM] subsequent use of plus and minus operators couldbe improved


> +       next = NEXT_INSN (insn);

If you only handle adjacent instructions, and don't use the data
available in reload_cse_move2add, putting the code there is just
confusing.  You could put that code into any old loop that traverses
the insns.  In that respect, I think your previous patch that did
a separate traversal was better.
If we really save a worthwhile amount of time by traversing the insns
only once - that would have to be because of better memory locality,
as putting too much into a single function tends to screw up register
allocation - then we should just merge all the post-reload operations
that don't need to run sequentially into one huge glob of code.

Doing your transformation without a cost check is also a bit dubious,
although with a well-written machine description, any disadvantagous
combination will probably be undone in the pre-sched2 instruction
splitting pass.

> +               rtx new_value = GEN_INT (INTVAL (XEXP (insn_src, 1)) +
> +                                        INTVAL (XEXP (next_src, 1)));

This new constant that you create can end up with incorrect sign extension.


FWIW, on the SH, due to LEGITIMIZE_RELOAD_ADDRESS, we sometimes end up with
(set (REGX) (CONST_INT A))
(add (REGX) (REGY))
(... (mem (plus ((REGX) (CONST_INT B)))  ...)

which, if r0 is available, is simplified by a peephole to:

(set (r0) (CONST_INT A+B))
(... (mem (plus ((REGY) (r0)))  ...)

-- 
--------------------------
SuperH (UK) Ltd.
2410 Aztec West / Almondsbury / BRISTOL / BS32 4QX
T:+44 1454 465658


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