On 11/1/19 7:16 AM, Mikael Tillenius wrote:
>
> On 10/30/19 8:59 PM, Jeff Law wrote:
>>> 1) Teach CSE to work even for calls to __mulhi3
>> THe name shouldn't matter at all to CSE. What's important to CSE is
>> interpreting the REG_EQUAL note in this case and finding the bounds of
>> the insn chain that computes the value in the REG_EQUAL note.
> My guess would be that it is the *call* that is the problem, not exactly
> what it calls. But I really don't know what I am talking about. I never
> looked at the GCC sources until yesterday.
There is certainly a problem in that CSE should know how to remove the
redundant call and failure to do so is likely a (low priority) bug.
But at least on the H8/300H and later processors the calls should be
totally unnecessary as we should be generating the multiply instruction.
>> Note that GCC should know how to generate mulxu.w for the H8/300H and
>> above variants. Just quickly looking at the MD file I see:
>>
>>
>>> (define_expand "umulhisi3"
>>> [(set (match_operand:SI 0 "register_operand" "")
>>> (mult:SI (zero_extend:SI (match_operand:HI 1
>>> "register_operand" ""))
>>> ;; intentionally-mismatched modes
>>> (match_operand:HI 2 "reg_or_nibble_operand" "")))]
>>> "TARGET_H8300H || TARGET_H8300S"
>>> {
>>> if (GET_MODE (operands[2]) != VOIDmode)
>>> operands[2] = gen_rtx_ZERO_EXTEND (SImode, operands[2]);
>>> })
>> Note the HImode on operand 2. That seems wrong. Hopefully it wasn't
>> something *I* did eons ago. Anyway, that might be a place to start
>> looking. I wouldn't be surprised if all the work we've done in the last
>> decade to tighten up our checking of predicates is somehow causing us to
>> not use umulhisi3 in this context.
>
> I think the problem is the predicate "reg_or_nibble_operand". It only
> matches integers <= 15 and only on H8SX. So it cannot match 13 on my
> target (which is H8S) and cannot match sizeof(...) in general even on H8SX.
It's not the range of values, but the *modes* of those values. ie, it's
perfectly fine to have a predicate that restricts the values to some
integer range.
The general structure of that expander is suspect because of the
difference in the structure of the two operands of the MULT rtx. ie,
one is a (zero_extend:SI ...) and the other is a HImode
reg_or_nibble_operand. I strongly suspect the generic expansion code is
claiming that umulhisi3 isn't supported for any cases because of this issue.
To further complicate matters GCC does not generally allow for
constructs such as ({zero,sign}_extend (const_int ...))) as those are
inherently constants and would be simplified by removing the extension.
So it's actually fairly difficult to write that expander correctly.
Anyway, someone would need to debug the expander and recognition code.
It's not a high priority issue though. But at least with the bug filed
it won't get lost.
Jeff
