Re: [PATCH v3 1/5] LoongArch: Fix usage of LSX and LASX frint/ftint instructions [PR112578]

[chenglulu <chenglulu@loongson.cn>]

在 2023/11/23 下午3:31, chenglulu 写道:
>
> 在 2023/11/23 下午3:11, Xi Ruoyao 写道:
>> On Thu, 2023-11-23 at 14:35 +0800, chenglulu wrote:
>>> Hi,
>>>
>>>    I don’t quite understand this part. Is it because define_insn 
>>> would be
>>> duplicated with the above implementation,
>>>
>>> so define_insn_and_split is used?
>> Yes, but if you think duplicating the above implementation is better I
>> can dup it as well (as it's just a single line).
>>
>> (I wrote it as a define_expand but it didn't work, then I modified it to
>> define_insn_and_split).
>>
> I just thought it was weird when I was looking at the code.
>
> I modified this code to use define_expand:
>
>     (define_expand "fix_trunc<mode><vimode>2"
>       [(set (match_operand:<VIMODE> 0 "register_operand" "=f")
>             (fix:<VIMODE> (match_operand:FVEC 1 "register_operand" 
> "f")))]
>       ""
>       {
>         emit_insn 
> (gen_<simd_isa>_<x>vftintrz_<simdifmt_for_f>_<simdfmt> (
>           operands[0], operands[1]));
>         DONE;
>       }
>       [(set_attr "type" "simd_fcvt")
>        (set_attr "mode" "<MODE>")])
>
> Here are my test cases:
>
>     typedef float __attribute__ ((mode (SF))) float_t;
>     typedef int __attribute__ ((mode (SI))) int_t;
>
>     extern int_t v[4];
>     int_t
>     lt_fixdfsi (float_t *x)
>     {
>
>       for (int i=0;i<4;i++)
>         v[i] = x[i];
>     }
>
> This still achieves the desired effect, generating the following 
> assembly code:
>
> lt_fixdfsi:
> .LFB0 = .
>     .cfi_startproc
>
>     or    $r13,$r4,$r0     # 16    [c=4 l=4]  *movdi_64bit/0
>     la.global    $r12,v     # 8    [c=4 l=12]  *movdi_64bit/1
>     vld    $vr0,$r13,0     # 6    [c=12 l=4]  movv4sf_lsx/1
>     vftintrz.w.s    $vr0,$vr0     # 7    [c=12 l=4] lsx_vftintrz_w_s
>     vst    $vr0,$r12,0     # 9    [c=4 l=4]  movv4si_lsx/2
>
> So I don't know if I'm getting it right?:-(
>
The fix_truncv4sfv4si2 template is indeed called when debugging with gdb.

So I think we can use define_expand here.

>>>> +(define_insn_and_split "fix_trunc<mode><vimode>2"
>>>> +  [(set (match_operand:<VIMODE> 0 "register_operand" "=f")
>>>> +    (fix:<VIMODE> (match_operand:FVEC 1 "register_operand" "f")))]
>>>> +  ""
>>>> +  "#"
>>>> +  ""
>>>> +  [(const_int 0)]
>>>> +  {
>>>> +    emit_insn 
>>>> (gen_<simd_isa>_<x>vftintrz_<simdifmt_for_f>_<simdfmt> (
>>>> +      operands[0], operands[1]));
>>>> +    DONE;
>>>> +  }
>>>> +  [(set_attr "type" "simd_fcvt")
>>>> +   (set_attr "mode" "<MODE>")])