On Tue, 18 Feb 2025 21:40:06 -0700, Jeff Law wrote: > > > On 2/18/25 7:30 PM, Jin Ma wrote: > > > > > I apologize for not explaining things more clearly. I also discovered that > > the issue is caused by CSE. I think that during the substitution process, > > CSE recognized the syntax of if_then_else and concluded that the expressions > > in the "then" and "else" branches are equivalent, resulting in both yielding > > (reg/v:RVVMF2SF 140 [ vreg_memory ]): > > > > (minus:RVVMF2SF (reg/v:RVVMF2SF 140 [ vreg_memory ]) > > (float_extend:RVVMF2SF (vec_duplicate:RVVMF4HF (const_double:HF 0.0 > > [0x0.0p+0])))) > > > > is considered equivalent to: > > > > (reg/v:RVVMF2SF 140 [ vreg_memory ]) > > > > Clearly, there wasn鈥檛 a deeper consideration of the fact that float_extend > > requires > > a rounding mode(frm). Therefore, I attempted to use UNSPEC in the pattern > > to inform > > CSE that we have a rounding mode. > Right. It worked, but there's a deeper issue here. > > > > > As I mentioned before, this may not be a good solution, as it risks missing > > other > > optimization opportunities. As you pointed out, we need a more general > > approach > > to fix it. Unfortunately, while I鈥檓 still trying to find a solution, I > > currently > > don't have any other good ideas. > Changing the rounding modes isn't common, but it's not unheard of. My > suspicion is that we need to expose the rounding mode assignment earlier > (at RTL generation time). > > That may not work well with the current optimization of FRM, but I think > early exposure is the only viable path forward in my mind. Depending on > the depth of the problems it may not be something we can fix in the > gcc-15 space. > > You might experiment with emitting the FRM assignment in the > insn_expander class in the risc-v backend. This code: > > /* Add rounding mode operand. */ > > if (m_insn_flags & FRM_DYN_P) > > add_rounding_mode_operand (FRM_DYN); > > else if (m_insn_flags & FRM_RUP_P) > > add_rounding_mode_operand (FRM_RUP); > > else if (m_insn_flags & FRM_RDN_P) > > add_rounding_mode_operand (FRM_RDN); > > else if (m_insn_flags & FRM_RMM_P) > > add_rounding_mode_operand (FRM_RMM); > > else if (m_insn_flags & FRM_RNE_P) > > add_rounding_mode_operand (FRM_RNE); > > else if (m_insn_flags & VXRM_RNU_P) > > add_rounding_mode_operand (VXRM_RNU); > > else if (m_insn_flags & VXRM_RDN_P) > > add_rounding_mode_operand (VXRM_RDN); > > For anything other than FRM_DYN_P emit the appropriate insn to set FRM. > This may generate poor code in the presence of explicit rounding modes, > but I think something along these lines is ultimately going to be needed.
Are you suggesting that we should emit the rounding mode insn earlier or incorporate the rounding mode into the pattern (in fact, there are already operands[9]/reg FRM_REGNUM)? However, this doesn't seem to be effective because the side effects of the rounding mode do not take effect in float_extend, and CSE will always optimize away pred_single_widen_subrvvmf2sf_scalar, just like before :) Best regards, Jin Ma > jeff
