"Robin Dapp" <[email protected]> writes: >> Yeah, that was the idea, sorry, but I kept putting it off. >> >> I suppose this is all due to the RISC-V decision to forbid integer modes >> in vector registers. For example, V2SI is a valid mode but SI and DI >> are not. If integer modes were allowed, it would make sense to define >> a TI move pattern and make the vr alternatives behave like V4SI. >> TARGET_SECONDARY_MEMORY_NEEDED could say that moves between general >> registers and vector registers must go via memory. > > Yes, that's my understanding. If we had TI, etc. move patterns we would > never loop but emit_move_multi_word could perform something sensible. > Also, it's not like the RISC-V decision was to deliberately not > introduce them. We just didn't see the need for it, initially. > If at all, then the rationale was "getting scalars to/from vector > registers is slow and rare, so let's not bother and have the middle-end > spill as required". > > Through other ways, I guess crypto/crc, we started defining the larger > integer modes then, but without introducing move patterns. > > On the one hand, I don't see any major obstacles just adding the moves > but if we do that, I feel the documentation would need to be amended to > indicate why it's necessary to have them. (I wouldn't know where and > how, though :) ) > > On the other hand I'd prefer expmed/expand to handle the situation > properly and if we're touching it anyway, let's do it (mostly) right.
Yeah, normally I'd be all in favour of dealing with this in target-independent code, rather than forcing each backend to do similar work. The reason I think this case is different is that it's not always obvious what target-independent code should do. We have the combination: - the target encourages the use of TImode for general work (via MAX_FIXED_MODE_SIZE). - (subreg:TI (reg:V4SF R) 0) is valid. - (subreg:TI (reg:V4SF R) 0) cannot be further subdivided. - TImode cannot be moved directly either. I don't think having emit_move_insn spill to the stack is the way out. A lot of code uses emit_move_insn to move one register to another, including via copy_to_reg and force_reg, and until now there has been no risk (AFAIK) that doing so could create new stack slots. Spilling during expand should be avoided if possible in any case. AArch64 has been plagued in the past with issues where expand forced something to the stack only for later RTL optimisers to remove the stack references. It then wasn't possible to reclaim the stack space and so we got overly large frames or pointless stack manipulation. As I mentioned in the earlier review, I see the bitfield insertion & extraction parts of the patch as a temporary measure that could be improved in future. The current changes are better than the status quo (ICEs or loops), but there is a specific plan for improving things further (operate on REGMODE_NATURAL_SIZE chunks instead). But the move and operand_subword_force changes are different. It isn't obvious what the long-term direction should be. For example: (set (reg:TI x) (subreg:TI (reg:V4SF y) 0)) could become: (set (subreg:V4SF (reg:TI x) 0) (reg:V4SF y)) In general, it should be possible to look through subregs in this way if: - the subreg mode is the same size as the move mode - changing mode does not increase alignment requirements for any MEM in the move or: - this is a register-to-register move, - the destination is not a read-modify-write subreg, and - the source is either a REG or a subreg lowpart. But that leaves open what should happen for: (set (subreg:TI (reg:OI x) 8) (subreg:TI (reg:V8SF y) 8)) The move doesn't mention a mode that could be used instead of TI. We could replace REGMODE_NATURAL_SIZE with a hook that returns a "natural mode". And that sounds like it would work well if we were using the natural mode of the move mode. But here everything hinges on using the natural mode of a subreg's mode, which doesn't always correspond nicely with the move mode. For example, suppose a 32-bit target provided SImode moves but no DImode or TImode moves, and suppose that the target had 128-bit vector registers. If we tried to emit: (set (mem:DI (reg:SI addr)) (subreg:DI (reg:V4SI x) 0)) then knowing that x's natural mode is V4SI wouldn't help. We would need something no bigger than 64 bits. On an AArch32-like target, V2SI might be a realistic choice, but it's not obvious how we'd arrive at that. Maybe we could keep REGMODE_NATURAL_SIZE as-is and provide a hook that helps to select the move mode? Then there's the question of what to do on targets with (say) three different REGMODE_NATURAL_SIZE values. All three could even come into play in a single move (the move mode and the two subreg modes). So it just seems simpler to say that the target must provide integer moves up to the minimum of: - MAX_FIXED_MODE_SIZE - the largest REGMODE_NATURAL_SIZE It also seems like a natural extension of the existing requirement to provide word_mode moves. As for where to document that: REGMODE_NATURAL_SIZE itself is probably a good place. I suppose it doesn't really matter whether the RISC-V TImode moves provide vector register alternatives. That would just be an optimisation. The moves could start out as being GPR-only. IIUC, even that would be on a par with spilling in expand. It might even be better, if the move is optimised away later. Richard
