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


"Ruan Beihong" <ruanbeihong@gmail.com> writes:
> 2008/11/6, Ruan Beihong <ruanbeihong@gmail.com>:
>> Hi Richard,
>> It has been so many days since last talk. I have signed a assignment
>> now (due to my fault that I'd given a wrong address and also the slow
>> mailing service between USA and China), and the assignment is now on
>> its way to Boston for a few days.
>> So can the patch be accept now?
>> I still think that the loongson2ef's <d>multu.g should be divided into
>> a new type of insn (I call it mul3nc), since they don't change or use
>> the HI/LO register.
>>
>> --
>> James Ruan
>>
>
> I've got the reply from FSF. My assignment is completed.

Great!  Thanks for going through the process.

> +(define_insn "<u>div<mode>3"
> +  [(set (match_operand:GPR 0 "register_operand" "=d")
> +	(any_div:GPR
> +		(match_operand:GPR 1 "register_operand" "d")
> +		(match_operand:GPR 2 "register_operand" "d")))]
> +  "TARGET_LOONGSON_2EF"
> +  { return mips_output_division ("<d>div<u>.g\t%0,%1,%2", operands); }
> +  [(set_attr "type" "idiv3")
> +   (set_attr "mode" "<MODE>")]
> +)
> +
> +(define_insn "<u>mod<mode>3"
> +  [(set (match_operand:GPR 0 "register_operand" "=d")
> +	(any_mod:GPR
> +		(match_operand:GPR 1 "register_operand" "d")
> +		(match_operand:GPR 2 "register_operand" "d")))]
> +  "TARGET_LOONGSON_2EF"
> +  { return mips_output_division ("<d>mod<u>.g\t%0,%1,%2", operands); }
> +  [(set_attr "type" "idiv3")
> +   (set_attr "mode" "<MODE>")]

As Eric says, these instructions need to be earlyclobber ("=&d" rather
than "=d") because the divisor might be checked for zero after the
division itself.

> @@ -1358,7 +1365,12 @@
>    ""
>  {
>    if (ISA_HAS_<D>MUL3)
> -    emit_insn (gen_mul<mode>3_mul3 (operands[0], operands[1], operands[2]));
> +  {
> +	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]));
> +  }
>    else if (TARGET_FIX_R4000)
>      emit_insn (gen_mul<mode>3_r4000 (operands[0], operands[1], operands[2]));
>    else

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)
    ...

instead.

> +(define_insn "mul<mode>3_mul3_ls2ef"
> +  [(set (match_operand:GPR 0 "register_operand" "=d")
> +        (mult:GPR (match_operand:GPR 1 "register_operand" "d")
> +                  (match_operand:GPR 2 "register_operand" "d")))]
> +  "TARGET_LOONGSON_2EF"
> +{
> +  return "<d>multu.g\t%0,%1,%2";
> +}

No need for C code here.  Just use:

  (define_insn "mul<mode>3_mul3_ls2ef"
    [(set (match_operand:GPR 0 "register_operand" "=d")
          (mult:GPR (match_operand:GPR 1 "register_operand" "d")
                    (match_operand:GPR 2 "register_operand" "d")))]
    "TARGET_LOONGSON_2EF"
    "<d>multu.g\t%0,%1,%2"

> +(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?

> @@ -6128,6 +6163,9 @@
>  	     (match_operand 2 "const_int_operand" "n"))]
>    "ISA_HAS_PREFETCH && TARGET_EXPLICIT_RELOCS"
>  {
> +  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";

>  /* ISA has a three-operand multiplication instruction.  */
>  #define ISA_HAS_DMUL3		(TARGET_64BIT				\
> -				 && TARGET_OCTEON			\
> +				 && (TARGET_OCTEON			\
> +                                 ||  TARGET_LOONGSON_2EF)		\
>  				 && !TARGET_MIPS16)

Not needed after the change I suggested above.
 
Looks good otherwise, thanks.

Richard


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