This is the mail archive of the
mailing list for the GCC project.
Re: [vect] Ask for review and approving the patch about vect and loongson
Eric Fisher <email@example.com> writes:
> 2010/7/10 Richard Sandiford <firstname.lastname@example.org>:
>> The updated vect.patch is OK.
>> The target-supports.patch is OK if you break the line immediately before
>> the "&&"; the lines are too long as-is.
> OK, I've updated the patch in the attachment.
Looks good. OK to apply.
>> As far as loongson.patch goes:
>> Index: config/mips/loongson.md
>> --- config/mips/loongson.md Â Â (revision 161865)
>> +++ config/mips/loongson.md Â Â (working copy)
>> @@ -352,6 +352,16 @@
>> Â "pmulh<V_suffix>\t%0,%1,%2"
>> Â [(set_attr "type" "fmul")])
>> +;; Standard pattern mulm3
>> +(define_expand "mul<mode>3"
>> + Â[(set (match_operand:VH 0 "register_operand" "=f")
>> + Â Â Â Â(unspec:VH [(match_operand:VH 1 "register_operand" "f")
>> + Â Â Â Â Â Â Â Â Â Â(match_operand:VH 2 "register_operand" "f")]
>> + Â Â Â Â Â Â Â Â Â UNSPEC_LOONGSON_PMULL))]
>> + Â"TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>> + Â"")
>> Â;; Multiply signed integers and store low result.
>> Â(define_insn "loongson_pmull<V_suffix>"
>> Â [(set (match_operand:VH 0 "register_operand" "=f")
>> Instead rename loongson_pmull<V_suffix> to mul<mode>3 and add #defines
>> to mips.c to make CODE_FOR_loongson_pmullh an alias for CODE_FOR_mulhi3,
>> etc. ÂGrep for CODE_FOR_loongson to see where I mean.
> Thanks for the helpful information. I've changed the patch in the attachment.
Sorry, I forgot to ask you to change the pattern from an UNSPEC to
a MULT at the same time. The multiplication part is OK with that fixed.
>> The same rename-and-#define common would apply to the shifts, but I'm
>> worried. ÂThe loongson insns are all "infinite precision" shifts
>> (any V2HI << 16 == 0), whereas MIPS is a SHIFT_COUNT_TRUNCATED
>> target (implying V2HI << 16 should be an identity operation).
>> This could in theory cause us to miscompile things like:
>> Â v2hi >> (shift & 15)
>> and although I can't come up with a testcase, I think the problem
>> is still there.
>> Admittedly this means the current code is wrong too. ÂIt should be
>> using UNSPECs instead of shift rtxes.
>> One fix would be to make SHIFT_COUNT_TRUNCATED take a mode argument
>> (and turn it into a target hook at the same time).
> I'm going to take a look at this problem. Do you think that it is all
> right to first submit the current patch and leave this problem in the
> next one separately?
No, I'm afraid the truncation needs to be sorted out before the shift
changes go in. As things stand, the shift changes would in principle
introduce a wrong-code regression.