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: [PATCH] [MIPS] [Loongson] Specific integer instructions and prefetch added


I understand that.
Formatting is very important.
I think  this update attached will be nicer.
As to the peephole2, did you mean that every mult would be
 replace with multu.g unless it's the LO is the demanding target,
so that no peephole2 is need for it? If so, I deleted this part.

2008/11/12, Richard Sandiford <rdsandiford@googlemail.com>:
> Thanks for the update.
>
> "Ruan Beihong" <ruanbeihong@gmail.com> writes:
>> 2008/11/11 Richard Sandiford <rdsandiford@googlemail.com>:
>>> Formatting; this should be:
>>>
>>>    {
>>>      if (TARGET_LOONGSON_2EF)
>>>        emit_insn (gen_mul<mode>3_mul3_ls2ef (operands[0], operands[1],
>>>                                              operands[2]));
>>>      else
>>>        emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1],
>>> operands[2]));
>>>    }
>>>
>>> (GCC's quite strict about formatting.)  But I don't see any reason to
>>> make
>>> the TARGET_LOONSON_2EF code depend on ISA_HAS_<D>MUL3; it seems easier
>>> to have:
>>>
>>>  if (TARGET_LOONGSON_2EF)
>>>    emit_insn (gen_mul<mode>3_mul3_ls2ef (operands[0], operands[1],
>>>                                          operands[2]));
>>>  else if (ISA_HAS_<D>MUL3)
>>>    emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1],
>>> operands[2]));
>>>  else if (TARGET_FIX_R4000)
>>>    ...
>
> In the patch you had:
>
>   if(TARGET_LOONGSON_2EF)
>     emit_insn(gen_mul<mode>3_mul3_ls2ef(operands[0], operands[1],
> operands[2]));
>
> instead, but the coding standards require spaces before the "("s.
> (See what I meant when I said GCC was quite strict about formatting? ;))
> The code really should be formatted the way I had it above.
>
> (Lines shouldn't be longer than 80 characters, which is why I split
> the call over two lines.)
>
> I know this must seem overly picky, but bear in mind that, if a hundred
> people made changes in their own preferred style, the whole thing would
> become a right mess.
>
>>>> +  if(TARGET_LOONGSON_2EF)
>>>> +  /*Loongson 2[ef] use load to $0 to perform prefetching*/
>>>> +    return "ld\t$0,%a0";
>>>
>>> Formatting; should be:
>>>
>>>  if (TARGET_LOONGSON_2EF)
>>>    /* Loongson 2[ef] use load to $0 to perform prefetching.  */
>>>    return "ld\t$0,%a0";
>
> Looks like you forgot this bit.  (Comments should start with a
> captial letter and end with ".  ".)
>
>>>> +(define_peephole2
>>>> +  [(set (match_operand:SI 0 "lo_operand")
>>>> +        (mult:SI (match_operand:SI 1 "d_operand")
>>>> +                 (match_operand:SI 2 "d_operand")))
>>>> +   (set (match_operand:SI 3 "d_operand")
>>>> +        (match_dup 0))]
>>>> +   "TARGET_LOONGSON_2EF && peep2_reg_dead_p(2, operands[0])"
>>>> +  [(set (match_dup 3)
>>>> +        (mult:SI (match_dup 1)
>>>> +                 (match_dup 2)))]
>>>
>>> When do we need this?  How do we get a LO-target multiplication
>>> after the mul<mode>3 change above?
>>
>> In traditional mips ISA, mult's result is stored in LO/HI.
>> If we need to get the result, we need an extra mflo.
>> Loongson2's multu.g is used to save the extra mflo in such case.
>> So I want use this peephole2 to turn insns like:
>>  mult $x, $y
>>  mflo $z
>> into
>>  multu.g $z, $x, $y
>> when the LO is used only once in such case.
>
> Sure, but what I meant was: define_peephole2s operate on instructions
> that have already been generated, and that already match some define_insn.
> So this define_peephole2 is only necessary if we can still sometimes
> generate a "normal" LO-based multiplication INSN for Loongson.
> So how could that happen after your changes to the mul<mode>3 expanders?
> Surely we should always be using mul<mode>3_mul3_ls2ef for Loongson,
> in which case every multiplication will already write to a GPR
> instead of LO.
>
> Richard
>


-- 
James Ruan

Attachment: new_i_insn_and_prefetch.patch.gz
Description: GNU Zip compressed data


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