> Thanks for the feedback. Here's how this patch enables cleaner usage with a > concrete example. > Without this patch, for an instruction like vfqwdota that needs to pass its > altfmt operand (at operands[9]) to the vsetvl pass, we must add > per-instruction boilerplate like this:
> > (define_insn "@pred_vfqwdota<mode>" > [(set (match_operand:RVVM1SF 0 "register_operand" "=vr, > vr") > (if_then_else:RVVM1SF > (unspec:<VM> > [(match_operand:<VM> 1 "vector_mask_operand" > "vmWc1,vmWc1") > (match_operand 5 "vector_length_operand" " rK, > rK") > (match_operand 6 "const_int_operand" " i, > i") > (match_operand 7 "const_int_operand" " i, > i") > (match_operand 8 "const_int_operand" " i, > i") > (match_operand 9 "const_int_operand" " i, > i") > (reg:SI VL_REGNUM) > (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) > (unspec:RVVM1SF [ > (match_operand:VQI_DOT 3 "register_operand" " vr, > vr") > (match_operand:VQI_DOT 4 "register_operand" " vr, > vr") > ] UNSPEC_VFQWDOTA) > (match_operand:RVVM1SF 2 "vector_merge_operand" " vu, > 0")))] > "TARGET_VECTOR" > "vfqwdota.vv\t%0,%3,%4%p1" > [(set_attr "type" "vdot") > (set_attr "mode" "<MODE>") > ;; Per-instruction boilerplate required for every altfmt instruction > (set (attr "altfmt") > (symbol_ref "INTVAL (operands[9]) > ? riscv_vector::ALTFMT_ALT > : riscv_vector::ALTFMT_NONE"))]) > > This becomes repetitive and error-prone when many instructions need similar > handling, especially since each may use a different operand index. > With this patch, we centralize all altfmt logic in the define_attr "altfmt" > and simply let the attribute machinery route the correct operand value. The > same vfqwdota instruction now needs no altfmt-related code at all: > > (define_insn "@pred_vfqwdota<mode>" > [(set (match_operand:RVVM1SF 0 "register_operand" "=vr, > vr") > (if_then_else:RVVM1SF > (unspec:<VM> > [(match_operand:<VM> 1 "vector_mask_operand" > "vmWc1,vmWc1") > (match_operand 5 "vector_length_operand" " rK, > rK") > (match_operand 6 "const_int_operand" " i, > i") > (match_operand 7 "const_int_operand" " i, > i") > (match_operand 8 "const_int_operand" " i, > i") > (match_operand 9 "const_int_operand" " i, > i") > (reg:SI VL_REGNUM) > (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE) > (unspec:RVVM1SF [ > (match_operand:VQI_DOT 3 "register_operand" " vr, > vr") > (match_operand:VQI_DOT 4 "register_operand" " vr, > vr") > ] UNSPEC_VFQWDOTA) > (match_operand:RVVM1SF 2 "vector_merge_operand" " vu, > 0")))] > "TARGET_VECTOR" > "vfqwdota.vv\t%0,%3,%4%p1" > [(set_attr "type" "vdot") > (set_attr "mode" "<MODE>")]) > > > ;; Centralized: all altfmt routing logic lives here, keyed by type > (define_attr "altfmt" "" > (cond [(eq_attr "type" "vdot") > (symbol_ref "riscv_vector::get_altfmt(operands[9])")] > (const_int INVALID_ATTRIBUTE))) > > The get_altfmt() helper extracts the operand value and handles ALTFMT_ANY → > INVALID_ATTRIBUTE mapping, so the vsetvl pass receives the correct altfmt > preference without any per-instruction duplication. When adding a new > instruction that needs altfmt propagation, one only needs to add a new clause > in the centralized define_attr "altfmt" specifying its type and the > corresponding operand index, while the instruction pattern itself remains > clean. Yes that makes sense and matches what we're already doing. Also that's the same 8-bit "integer" altfmt that Lino is proposing? And here we already have arithmetic, rather than just conversions :) This strengthens the point of preferring real modes rather than an altfmt bit. If we have real modes we can just get the format from the mode, rather than carrying an additional operand. -- Regards Robin
