Re: RFA: Tweak optabs.c's use of constant rtx_costs

[Richard Sandiford <richard at codesourcery dot com>]

Rask Ingemann Lambertsen <rask@sygehus.dk> writes:
> On Fri, Aug 17, 2007 at 04:28:18PM +0100, Richard Sandiford wrote:
>> expand_binop has code like the following:
>> 
>>   /* If we are inside an appropriately-short loop and we are optimizing,
>>      force expensive constants into a register.  */
>>   if (CONSTANT_P (op0) && optimize
>>       && rtx_cost (op0, binoptab->code) > COSTS_N_INSNS (1))
>>     {
>>       if (GET_MODE (op0) != VOIDmode)
>> 	op0 = convert_modes (mode, VOIDmode, op0, unsignedp);
>>       op0 = force_reg (mode, op0);
>>     }
>> 
>> Outdated comment aside, what this code is doing seems perfectly
>> reasonable in itself.  However, I think it's in the wrong place.
>> The code is one of the first things that expand_binop does,
>> so we run it even if we end up splitting the binary operation
>> into smaller word-sized pieces or using a sequence of completely
>> different optabs altogether.  There are two direct problems with this:
>> 
>>   - The target has no real way of knowing whether a CONST_INT passed
>>     to TARGET_RTX_COSTS is for a word or double-word operation.
>> 
>>   - If we force a constant into a register before reducing the operation
>>     into smaller pieces (such as word-mode pieces), we will end up using
>>     SUBREGs of a multiword REG for those smaller pieces, instead of using
>>     individual constants.  That's harder to optimise, and effectively
>>     hides the individual constants until after the lower-subreg pass.
>
>    There is similiar code in prepare_cmp_insn(). It interferes with
> SELECT_CC_MODE() on targets which choose the mode of the comparison result
> depending on the constant (if any) in the comparison.
>
>    Additionally, emitting
>
> (insn (set (tmp) (const_int 4)))
> (insn (set (sp) (plus (sp) (tmp))))
>
> instead of
>
> (insn (set (sp) (plus (sp) (const_int 4))))
>
> disables frame pointer elimination for a function. Fwprop, gcse and
> sometimes even combine can clean it up, but that of course requires those
> passes to be run. It's a nuisance when trying to find a code gen bug which
> only happens with frame pointer elimination.

Yeah, I'd noticed the prepare_cmp_insn insn thing too; it means that
the MIPS patch I mentioned has to treat constants with a COMPARE outer
code specially -- giving them zero cost -- even though the port never
actually uses COMPARE itself.  I didn't actually find the MIPS check
too gross, but I did wonder if the code was still worthwhile on targets
that do use COMPARE.  It's interesting that it's a pain for yours too.

(For avoidance of doubt, I think this should be treated as a separate
issue, and I don't think you were suggesting otherwise.)

Richard