On 7/8/24 14:03, Jeff Law wrote: > > On 6/30/24 6:47 PM, Vineet Gupta wrote: >> Changes since v1: >> - Removed UNSPEC_{INFINITE,ISNORMAL} >> - Don't hardcode SI in patterns, try to keep X to avoid potential >> sign extension pitfalls. Implementation wise requires skipping >> :MODE specifier in match_operand which is flagged as missing mode >> warning. >> >> --- >> Hmm, I'm wondering if I gave you a slightly bad steer. The fclass insn >> is unlikely to ever be used as-is since the result is target specific. >> So we probably don't need to worry too much about optimizing it the >> fclass insn itself for extension removal... But we probably do want to >> improve the expansion side a bit....
No worries. >> +(define_insn "fclass<ANYF:mode>" >> + [(set (match_operand 0 "register_operand" "=r") >> + (unspec [(match_operand:ANYF 1 "register_operand" " f")] >> + UNSPEC_FCLASS))] >> + "TARGET_HARD_FLOAT" >> + "fclass.<fmt>\t%0,%1"; >> + [(set_attr "type" "fcmp") >> + (set_attr "mode" "<UNITMODE>")]) > So I think just make operand0 :X here so that only word_mode is allowed. > > > >> + >> +(define_expand "isfinite<ANYF:mode>2" >> + [(set (match_operand 0 "register_operand" "=r") >> + (match_operand:ANYF 1 "register_operand" " f"))] >> + "TARGET_HARD_FLOAT" >> +{ >> + rtx tmp = gen_reg_rtx (word_mode); >> + emit_insn (gen_fclass<ANYF:mode> (tmp, operands[1])); >> + riscv_emit_binary (AND, tmp, tmp, GEN_INT (0x7e)); >> + rtx cmp = gen_rtx_NE (word_mode, tmp, const0_rtx); >> + emit_insn (gen_cstore<mode>4 (operands[0], cmp, tmp, const0_rtx)); >> + >> + DONE; >> +}) > Leave the modes in the RTL template of the expander as you've got them > above. That looks good. > > > In the C fragment I think you'll want to reject all the undesirable > output modes. The expander is allowed to FAIL, so you can do something > like this: > > if (GET_MODE (operands[0]) != SImode > && GET_MODE (operands[0]) != word_mode) > FAIL; > > Which should allow SI for rv32/rv64 and DI for rv64. > > > +> +(define_expand "isnormal<ANYF:mode>2" >> + [(set (match_operand 0 "register_operand" "=r") >> + (match_operand:ANYF 1 "register_operand" " f"))] >> + "TARGET_HARD_FLOAT" >> +{ >> + rtx tmp = gen_reg_rtx (word_mode); >> + emit_insn (gen_fclass<ANYF:mode> (tmp, operands[1])); >> + riscv_emit_binary (AND, tmp, tmp, GEN_INT (0x42)); >> + rtx cmp = gen_rtx_NE (word_mode, tmp, const0_rtx); >> + emit_insn (gen_cstore<mode>4 (operands[0], cmp, tmp, const0_rtx)); > So I would use tmp (or another word_mode pseudo register) for the > destination of that emit_insn. Then something like: > > t = gen_lowpart (SImode, tmp); > SUBREG_PROMOTED_VAR_P (tmp) = 1; > SUBREG_PROMOTED_SET (tmp, SRP_SIGNED); > emit_move_insn (operands[0], tmp); > > To the output into operands[0] with enough magic to allow us to > potentially remove a subsequent sign extension. Right looks like this does the trick for most part. Thx, -Vineet