"Jin Ma" <[email protected]> writes: > Hi Robin, > > Thank you for pointing this out. You are right that the x86 workaround > alone does not establish that the RISC-V target is the right place for > the fix. > > After looking further, I think it is important to distinguish the scalar > and vector uses. g:8e3239e3e92f explicitly retained the ability to > construct SUBREGs of CONST, SYMBOL_REF, and LABEL_REF. The existing > AVR, MSP430, and RL78 uses are all in scalar or pointer modes. They > only show that symbolic SUBREGs should not be unconditionally rejected > or materialized in the generic middle end. > > The direct vector precedent remains PR71626. Its x86_64 testcase > represents a DImode function address as a V1DImode vector, and > ix86_expand_vector_move legalizes the SUBREG of a constant > (g:6a20b538f2a02e01b311af1dade9cea6a58d815e). Although its generation > path differs from this case, both are equal-sized bitcasts from a scalar > address to a vector when they reach the vector move expander.
I can understand the justification for generating the V1DI<->DI subregs, although it's a bit on the edge. V1DI is really just a scalar in vector robes. But DI<->V2SI seems less reasonable. >> > More precisely, the expression can be constructed by the current >> > validate_subreg and simplify_gen_subreg and used as a temporary input to >> > a move expander. However, it is not a general_operand and cannot remain >> > in a final move insn. legitimize_move legalizes it before emitting the >> > final move. >> >> Then we rather shouldn't have done that in the first place, I'd say. >> >> -- >> Regards >> Robin > > I agree that it must not remain in a final move insn for a target that > does not support this operand. However, the existing scalar uses and > the x86 vector precedent show that the generic middle end should not > unconditionally reject or materialize this RTL. > > riscv_vector::legitimize_move is called by the RVV mov<mode> expander > and already legalizes vector constants, SUBREGs, and special moves > according to the RISC-V operand constraints. Materializing the inner > symbolic constant there therefore falls within its existing role. > Targets that already support this RTL need no change. Targets that > cannot accept it directly can legalize it according to their own move > definitions. That's fair if the RISC-V predicates reject subregs that the general target-independent predicates wouldn't. But AIUI, that isn't the case here. It seems like the patch is instead having to handle something that doesn't conform to general_operand. > In contrast, changing force_subreg would affect multiple middle-end > paths and existing target handling. For these reasons, I think the > RISC-V target fix is the more appropriate and conservative solution. FWIW, I agree with Robin that this isn't something that every target should need to deal with. I think we should reject subregs involving symbolic constants if the nunits of either the original mode or the new mode != 1. Andrew is working on patches to support vector addresses, where it might make sense for symbol_ref:VnSI to duplicate the symbol into all lanes. But I don't think a subreg would be useful even there. Thanks, Richard
