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.

(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
Thanks,
Richard

Reply via email to