> The only thing I think we want for the patch (as Pan also raised last time) > is > the param to set those .vx costs to zero in order to ensure the tests test > the > right thing (--param=vx_preferred/gr2vr_cost or something).
I see, shall we start a new series for this? AFAIK, we may need some more alignment for something like --param=xx that exposing to end-user. > According to patchwork the tests you add pass but shouldn't they actually > fail > with a GR2VR cost of 2? I must be missing something. For now the cost of GR2VR is 2, take test vx_vadd-1-i64.c for example, the vec_dup + vadd.vv has higher cost than vadd.vx, thus perform the late-combine as below. Feel free to correct me if I mis-understand how cost model works here. 51 trying to combine definition of r135 in: 52 11: r135:RVVM1DI=vec_duplicate(r150:DI) 53 into: 54 18: r147:RVVM1DI=r146:RVVM1DI+r135:RVVM1DI 55 REG_DEAD r146:RVVM1DI 56 successfully matched this instruction to *add_vx_rvvm1di: 57 (set (reg:RVVM1DI 147 [ vect__6.8_16 ]) 58 (plus:RVVM1DI (vec_duplicate:RVVM1DI (reg:DI 150 [ x ])) 59 (reg:RVVM1DI 146))) 60 original cost = 12 + 4 (weighted: 43.043637), replacement cost = 4 (weighted: 32.363637); keeping replacement 61 rescanning insn with uid = 18. 62 updating insn 18 in-place 63 verify found no changes in insn with uid = 18. 64 deleting insn 11 65 deleting insn with uid = 11 Pan -----Original Message----- From: Robin Dapp <rdapp....@gmail.com> Sent: Tuesday, April 22, 2025 11:10 PM To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; jeffreya...@gmail.com; rdapp....@gmail.com; Chen, Ken <ken.c...@intel.com>; Liu, Hongtao <hongtao....@intel.com>; Robin Dapp <rdapp....@gmail.com> Subject: Re: [PATCH v2 1/3] RISC-V: Combine vec_duplicate + vadd.vv to vadd.vx on GR2VR cost > /* TODO: We set RVV instruction cost as 1 by default. > Cost Model need to be well analyzed and supported in the future. */ > + int cost_val = 1; > + enum rtx_code rcode = GET_CODE (x); > + > + /* Aka (vec_duplicate:RVVM1DI (reg/v:DI 143 [ x ])) */ > + if (rcode == VEC_DUPLICATE && SCALAR_INT_MODE_P (GET_MODE (XEXP (x, 0)))) > + cost_val += get_vector_costs ()->regmove->GR2VR; > + > + *total = COSTS_N_INSNS (cost_val); > + > + return true; > +} I first read this wrong and thought we'd use GR2VR costs literally. IMHO the costing makes sense like now. The only thing I think we want for the patch (as Pan also raised last time) is the param to set those .vx costs to zero in order to ensure the tests test the right thing (--param=vx_preferred/gr2vr_cost or something). According to patchwork the tests you add pass but shouldn't they actually fail with a GR2VR cost of 2? I must be missing something. -- Regards Robin
