On Wed, Jan 22, 2020 at 08:06:03AM -0600, Segher Boessenkool wrote:
> On Mon, Jan 06, 2020 at 09:24:10AM +0100, Jakub Jelinek wrote:
> > 1) always use gen_add3_insn, but if it returns NULL use the extra
> > emit_move_insn (end_addr, GEN_INT (-rounded_size));
> > and retry with end_addr instead of rs
> > 2) if the first gen_add3_insn returned NULL or if it created more than one
> > instruction, add the REG_FRAME_RELATED_EXPR note on the last insn
>
> 3) Don't use gen_add3_insn, generate the wanted insns directly. Why
> can't we do that? gen_add3_insn is fine for generic expansion, but that
> is not what we are doing here at all. And since not everything it can do
> is okay here at all apparently, it is a bad choice.
I don't see how not using gen_add3_insn would make anything easier, the
problem isn't gen_add3_insn, but that the add expander has predicates that
need to be satisfied, and even if I build the insn by hand, I'll still need
to make sure the predicates are satisfied and for that will need to check
them anyway.
> It looks like your patch will pessimise code in some cases as well, btw?
No, it will solely turn previous wrong-codes into something that works
(== cases where gen_addr3_insn would previously fail).
The 1)+2) variant could even improve code, as gen_addr3_insn could succeed
even if we currently don't call it (perhaps generate more than one insn,
but it still might be better than forcing the constant into register and
then performing add that way).
Jakub
