Re: [PATCH] RISC-V: Fix error combine of pred_mov pattern

[Jeff Law <jeffreyalaw@gmail.com>]

On 2/19/24 21:21, Alexandre Oliva wrote:
> This backport is the second of two required for the pr111935 testcase,
> already backported to gcc-13, to pass on riscv64-elf and riscv32-elf.
> The V_VLS mode iterator, used in the original patch, is not available in
> gcc-13, and I thought that would be too much to backport (and maybe so
> are these two patches, WDYT?), so I changed it to V, to match the
> preexisting gcc-13 pattern.  Comments also needed manual adjustment.
> Regstrapped on x86_64-linux-gnu, along with other backports, and tested
> manually on riscv64-elf.  Ok to install?
> 
> From: Lehua Ding <lehua.ding@rivai.ai>
> 
> This patch fix PR110943 which will produce some error code. This is because
> the error combine of some pred_mov pattern. Consider this code:
> 
> ```
> 
> void foo9 (void *base, void *out, size_t vl)
> {
>      int64_t scalar = *(int64_t*)(base + 100);
>      vint64m2_t v = __riscv_vmv_v_x_i64m2 (0, 1);
>      *(vint64m2_t*)out = v;
> }
> ```
> 
> RTL before combine pass:
> 
> ```
> (insn 11 10 12 2 (set (reg/v:RVVM2DI 134 [ v ])
>          (if_then_else:RVVM2DI (unspec:RVVMF32BI [
>                      (const_vector:RVVMF32BI repeat [
>                              (const_int 1 [0x1])
>                          ])
>                      (const_int 1 [0x1])
>                      (const_int 2 [0x2]) repeated x2
>                      (const_int 0 [0])
>                      (reg:SI 66 vl)
>                      (reg:SI 67 vtype)
>                  ] UNSPEC_VPREDICATE)
>              (const_vector:RVVM2DI repeat [
>                      (const_int 0 [0])
>                  ])
>              (unspec:RVVM2DI [
>                      (reg:SI 0 zero)
>                  ] UNSPEC_VUNDEF))) "/app/example.c":6:20 1089 {pred_movrvvm2di})
> (insn 14 13 0 2 (set (mem:RVVM2DI (reg/v/f:DI 136 [ out ]) [1 MEM[(vint64m2_t *)out_4(D)]+0 S[32, 32] A128])
>          (reg/v:RVVM2DI 134 [ v ])) "/app/example.c":7:23 717 {*movrvvm2di_whole})
> ```
> 
> RTL after combine pass:
> ```
> (insn 14 13 0 2 (set (mem:RVVM2DI (reg:DI 138) [1 MEM[(vint64m2_t *)out_4(D)]+0 S[32, 32] A128])
>          (if_then_else:RVVM2DI (unspec:RVVMF32BI [
>                      (const_vector:RVVMF32BI repeat [
>                              (const_int 1 [0x1])
>                          ])
>                      (const_int 1 [0x1])
>                      (const_int 2 [0x2]) repeated x2
>                      (const_int 0 [0])
>                      (reg:SI 66 vl)
>                      (reg:SI 67 vtype)
>                  ] UNSPEC_VPREDICATE)
>              (const_vector:RVVM2DI repeat [
>                      (const_int 0 [0])
>                  ])
>              (unspec:RVVM2DI [
>                      (reg:SI 0 zero)
>                  ] UNSPEC_VUNDEF))) "/app/example.c":7:23 1089 {pred_movrvvm2di})
> ```
> 
> This combine change the semantics of insn 14. I split @pred_mov pattern and
> restrict the conditon of @pred_mov.
> 
> 	PR target/110943
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/predicates.md (vector_const_int_or_double_0_operand):
> 	New predicate.
> 	* config/riscv/riscv-vector-builtins.cc (function_expander::function_expander):
> 	force_reg mem target operand.
> 	* config/riscv/vector.md (@pred_mov<mode>): Wrapper.
> 	(*pred_mov<mode>): Remove imm -> reg pattern.
> 	(*pred_broadcast<mode>_imm): Add imm -> reg pattern.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/rvv/base/pr110943.c: New test.
I'd leave this alone as well.  I just don't see much value in the backports.

jeff