Hello Richard: On 14/06/24 4:26 pm, Richard Sandiford wrote: > Ajit Agarwal <aagar...@linux.ibm.com> writes: >> Hello Richard: >> >> All comments are addressed. > > I don't think this addresses the following comments from the previous > reviews: > > (1) It is not correct to mark existing insn uses as live-out. > The patch mustn't try to do this. >
Addressed in v3 of the patch. > (2) To quote a previous review: > > It's probably better to create a fresh OO register, rather than > change an existing 128-bit register to 256 bits. If we do that, > and if reg:V16QI 125 is the destination of the second load > (which I assume it is from the 16 offset in the subreg), > then the new RTL should be: > > (vec_select:HI (subreg:V8HI (reg:OO NEW_REG) 16) ...) > > It's possible to get this by using insn_propagation to replace > (reg:V16QI 125) with (subreg:V16QI (reg:OO NEW_REG) 16). > insn_propagation should then take care of the rest. > > There are no existing rtl-ssa routines for handling new registers > though. (The idea was to add things as the need arose.) > > The reason for (2) is that changing the mode of an existing pseudo > invalidates all existing references to that pseudo. Although the > patch tries to fix things up, it's doing that at a stage where > there is already "garbage in" (in the sense that the starting > RTL is invalid). Just changing the mode would also invalidate > things like REG_EXPR, for example. > > In contrast, the advantage of creating a new pseudo means that every > insn transformation is from structurally valid RTL to structurally > valid RTL. It also prevents information being incorrectly carried > over from the old pseudo. > x Addressed in v3 of the patch. > Thanks, > Richard Thanks & Regards Ajit
