Re: [PATCH] Check rtx_costs in combine.c's try_combine

[Roger Sayle <roger at eyesopen dot com>]

On Mon, 28 Jun 2004, Richard Kenner wrote:
>     The following patch adds an additional test to combine.c's try_combine
>     to only allow instructions to be merged/combined if the backend's
>     TARGET_RTX_COST macro indictates that the replacement is atleast as
>     cheap as the original.  Historically, GCC's "combine" pass has used
>     a greedy algorithm that always attempts to build complex composite
>     instructions that are recognized by the back-end independent of any
>     cost metric.  Increasingly with modern processors, the more complex
>     instructions supported are an ISA are not automatically better than
>     their component parts.
>
> Yes, but in such a case, those complex instructions should not in the MD
> file as there is no situation in which they should be used.
>
> I disagree with this patch.


It's often the case that these instructions are not profitable for some
combination of operand values, but generally that the instruction is
useful.  A good example, is the use of the x86's "imul" instruction
with a immediate constant operand (but the argument applies to any
processor with such a pattern).  Clearly this instruction should never
be used where ever a synthetic multiply sequence of shifts and adds is
cheaper.  For example, we'd never expect the GCC backend to generate
the assembler instructions "imul $0, %eax", "imul $1, %eax" or even
"imul $2, %eax".  But clearly, a multiplication should be defined in
the i386.md backend, and it needs to be able to match CONST_INT operands.
The question now becomes where do we disallow the "bad" coefficients.

Your proposal would seem to imply that we duplicate the entire cost
logic of synth_mult into each backends expander for multiply patterns,
so that they can themselves FAIL if there potentially exists a better
replacement.


I'll admit I've a patch still pending review that tries to catch some
of these cases in the i386.c backend, but using peepholes rather than
FAILS: http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00066.html


The issue is also one of relative costs.  Yes, in theory the x86 should
never emit an "imul $4, %eax" instruction, so the backend should fail
it.  However it is still cheaper than variable multiplications, such
as "imul %ebx, %eax", hence should CSE, GCSE or reload manage to
determine that the value of %ebx is always 4, we'd be deteriorating the
quality of code we produce, as the GCC backends provide no mechanism
when they fail to match a pattern to describe the replacement sequence
that should be used instead.



Similar arguments apply to shifts or the use of memory operands.  It
would probably break the compiler to reject patterns for particular
operand values (though certainly deteriorate code quality).  For example,
the middle-end's expand_shift currently doesn't have a way of falling
back to using additions if using the ashl_optab fails, hence a backend
that fails to recognize shifts-by-two in an attempt to get the compiler
to generate two additions, is shooting itself in the foot, and may even
end up generating calls to ashlsi3 in libgcc!


And finally, there's even the destablizing issue with latent compiler
bugs with your approach.  There are have been plenty of bugs in the
RTL optimizers, where we'd fail to re-recognize and instruction after
making a change.  Most of these have been fixed.  But its commonly,
the case that the RTL optimizers assume they don't need to rerecognize
an instruction when replacing a pseudo with a constant, or one constant
with another.  All of GCSE for example.  Adding constraints on operand
values will open up a whole can of worms with "unrecognizable insn"
and "insn doesn't satisfy it's constraint" ICEs.


Roger
--