This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][MIPS][Loongson][SIMD] fix reduc_uplus_v8qi (biadd)
Ok, I agree with you.
2009/2/14 Richard Sandiford <rdsandiford@googlemail.com>:
> Hi,
>
> Sorry for the slow response.
>
> Ruan Beihong <ruanbeihong@gmail.com> writes:
>> 2009/2/4 Ruan Beihong <ruanbeihong@gmail.com>:
>>> The standard name 'reduc_uplus_m' require the same mode of input and output.
>>> When compiling some file in Mplayer, following error occurred:
>>>
>>> libfaad2/sbr_hfgen.c: In function 'hf_generation':
>>> libfaad2/sbr_hfgen.c:184: error: unrecognizable insn:
>>> (insn 562 561 563 69 libfaad2/sbr_hfgen.c:98 (set (reg:V8QI 618)
>>> (unspec:V4HI [
>>> (reg:V8QI 403 [ vect_var_.214 ])
>>> ] 515)) -1 (nil))
>>> libfaad2/sbr_hfgen.c:184: internal compiler error: in extract_insn, at
>>> recog.c:2027
>>>
>>> This patch fix the problem.
>>>
>>> The patch attached below:
>>> Index: gcc/config/mips/loongson.md
>>> ===================================================================
>>> --- gcc/config/mips/loongson.md (revision 143919)
>>> +++ gcc/config/mips/loongson.md (working copy)
>>> @@ -358,10 +358,13 @@
>>> [(set_attr "type" "fadd")])
>>>
>>> ;; Sum of unsigned byte integers.
>>> +;; NOTE: reduce_uplus_m need the same mode of input and output,
>>> +;; while biadd here actually return a V4HI with the lowerest field
>>> +;; set to the sum of 8 byte.
>>> (define_insn "reduc_uplus_<mode>"
>>> - [(set (match_operand:<V_stretch_half> 0 "register_operand" "=f")
>>> - (unspec:<V_stretch_half> [(match_operand:VB 1 "register_operand" "f")]
>>> - UNSPEC_LOONGSON_BIADD))]
>>> + [(set (match_operand:VB 0 "register_operand" "=f")
>>> + (unspec:VB [(match_operand:VB 1 "register_operand" "f")]
>>> + UNSPEC_LOONGSON_BIADD))]
>>> "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>>> "biadd\t%0,%1"
>>> [(set_attr "type" "fabs")])
>>>
>>> --
>>> James Ruan
>>>
>>
>>
>> Ok. Here is the finished patch:
>>
>> Index: config/mips/loongson.md
>> ===================================================================
>> --- config/mips/loongson.md (revision 143947)
>> +++ config/mips/loongson.md (working copy)
>> @@ -358,7 +358,19 @@
>> [(set_attr "type" "fadd")])
>>
>> ;; Sum of unsigned byte integers.
>> -(define_insn "reduc_uplus_<mode>"
>> +;; NOTE: reduce_uplus_m need the same mode of input and output,
>> +;; while biadd here actually return a V4HI with the lowerest field
>> +;; set to the sum of 8 byte.
>> +(define_insn "reduc_uplus_v8qi"
>> + [(set (match_operand:VB 0 "register_operand" "=f")
>> + (unspec:VB [(match_operand:VB 1 "register_operand" "f")]
>> + UNSPEC_LOONGSON_BIADD))]
>> + "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>> + "biadd\t%0,%1"
>> + [(set_attr "type" "fabs")])
>> +
>> +;; This is the actual biadd which can be used through builtin function call.
>> +(define_insn "loongson_biadd"
>> [(set (match_operand:<V_stretch_half> 0 "register_operand" "=f")
>> (unspec:<V_stretch_half> [(match_operand:VB 1 "register_operand" "f")]
>> UNSPEC_LOONGSON_BIADD))]
>
> It occurred to me later that the MIPS backend doesn't allow an FPR
> value to be treated as both a V4HI and a V8QI value. That is, an FPR
> set in one mode cannot be used in the other mode; we would first have
> to move the FPR into a GPR and back again(!). Using "biadd" for
> reduc_uplus_v8qi seems to violate that.
>
> So the question is: does the Loongson ISA specification allow vectors to
> be reinterpreted in this way or not? I imagine it does, in which case:
>
> (a) your patch is right and
> (b) mips_cannot_change_mode_class should return false when FROM and TO
> are integer vector modes of equal size.
>
> But we should make certain that this is explicitly allowed first.
> (This is a tricky area because the MIPS ISA _doesn't_ allow these kinds
> of reinterpretation for the standard FPR formats (W, L, S, PS, etc.))
>
> I'm a bit reluctant to have (a) without (b) because of the inconsistency.
> I'm also a bit reluctant to do (b) at this stage in the 4.4 cycle because
> it's a relatively invasive change.
>
> I think the safest thing for 4.4 is to drop the reduc optimisation
> altogether. There'd then be plenty of time to improve things for 4.5.
> (This includes the FPR logical ops you posted earlier. It'd be great
> to get those in 4.5 too.)
>
> Here's the patch I'm testing and would like to apply for 4.4.
> My testing is on non-Loongson, so is only a sanity check at best.
>
> Does that sound OK?
>
> Richard
>
>
> gcc/
> 2009-02-xx Ruan Beihong <ruanbeihong@gmail.com>
> Richard Sandiford <rdsandiford@googlemail.com>
>
> * config/mips/mips.c (CODE_FOR_loongson_biadd): Delete.
> * config/mips/loongson.md (reduc_uplus_<mode>): Rename to...
> (loongson_biadd): ...this.
>
> Index: gcc/config/mips/loongson.md
> ===================================================================
> --- gcc/config/mips/loongson.md 2009-02-14 10:27:37.000000000 +0000
> +++ gcc/config/mips/loongson.md 2009-02-14 10:27:45.000000000 +0000
> @@ -358,7 +358,7 @@ (define_insn "loongson_pasubub"
> [(set_attr "type" "fadd")])
>
> ;; Sum of unsigned byte integers.
> -(define_insn "reduc_uplus_<mode>"
> +(define_insn "loongson_biadd"
> [(set (match_operand:<V_stretch_half> 0 "register_operand" "=f")
> (unspec:<V_stretch_half> [(match_operand:VB 1 "register_operand" "f")]
> UNSPEC_LOONGSON_BIADD))]
> Index: gcc/config/mips/mips.c
> ===================================================================
> --- gcc/config/mips/mips.c 2009-02-14 10:27:19.000000000 +0000
> +++ gcc/config/mips/mips.c 2009-02-14 10:27:25.000000000 +0000
> @@ -11253,7 +11253,6 @@ #define CODE_FOR_loongson_pminsh CODE_FO
> #define CODE_FOR_loongson_pminub CODE_FOR_uminv8qi3
> #define CODE_FOR_loongson_pmulhuh CODE_FOR_umulv4hi3_highpart
> #define CODE_FOR_loongson_pmulhh CODE_FOR_smulv4hi3_highpart
> -#define CODE_FOR_loongson_biadd CODE_FOR_reduc_uplus_v8qi
> #define CODE_FOR_loongson_psubw CODE_FOR_subv2si3
> #define CODE_FOR_loongson_psubh CODE_FOR_subv4hi3
> #define CODE_FOR_loongson_psubb CODE_FOR_subv8qi3
>
--
James Ruan