This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [vect] Ask for review and approving the patch about vect and loongson


Eric Fisher <joefoxreal@gmail.com> writes:
> 2010/7/10 Richard Sandiford <rdsandiford@googlemail.com>:
>> 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).
>>
>> Richard
>
> 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.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]