Re: rfc: vector_cst, reload etc

[Geoff Keating <geoffk at geoffk dot org>]

Aldy Hernandez <aldyh@redhat.com> writes:

...
> (insn 13 9 16 (set (reg/v:V4SI 118)
>         (const_vector:V4SI[ 
>                 (const_int 0 [0x0])
>                 (const_int 0 [0x0])
>                 (const_int 0 [0x0])
>                 (const_int 0 [0x0])
>             ] )) 544 {*movv4si_const0} (nil)
> 
> (insn 16 13 18 (set (subreg:SI (reg/v:V4SI 118) 12)
>         (const_int 69 [0x45])) 294 {*movsi_internal1} (nil)
>     (nil))
> 
> however, this last SUBREG is wrong since (afaict), no architectures allow 
> subregs of SIMD registers.
> 
> later on, reload will choose GPRs to store pseudo 118 (r8-r12).
> and insn 16 will become a set into r12.
> 
> find_reloads will cause a reload of R8-R12 (in insn 13)

???  You must mean R9-R12.

> because GPRs are not valid for "movv4si_const0".

Y'know, there's no reason for that.  Loading zero into a GPR is pretty easy.

>  but reload will
> *not* force a reload of insn 16 because it sees:
> 
> 	(set (subreg:SI (reg:V4SI r12)) 69)

You mean '(set (reg:SI r12) (const_int 69))'.  There shouldn't be a
SUGREG at this point.

> ..as perfectly valid (after all, it's just a set into an SI register).  
> then flow deletes the set into r12 because it's no longer needed.

???  Of course it's needed.  The code should look, at this point, like:

(set (reg:V4SI v...) (const_vector 0))
(set (reg:V4SI r9) (reg:V4SI v...))
(set (reg:SI r12) (const_int 12))
(subsequent use of r9-r12).

r12 should be live due to later uses of (reg:V4SI r9).

> so... we end up loosing the 69, and consequently store {0,0,0,0} into bar.
> 
> we could:
> 
> a) fix reload to reload insn 16 as well (probably an ugly hack).
> 
> b) disallow the CONSTRUCTOR code from clearing a SIMD register and then 
> filling in individual elements, which will eventually give us a subreg
> of a SIMD register (like my patch below).
> 
> or, we could DTRT, and fix vector initializers to not behave like arrays.  
> the REAL problem is that vector initializers are currently behaving like 
> arrays, but we still end up creating CONST_VECTOR's in special cases.  so we
> have an amalgamation in which we sometimes treat them like arrays, and
> sometimes treat them like vector constants.

All these are trying to avoid the real problem, which is that the SET
is not dead, and yet it's getting deleted.  I'm sure it'd be possible
to come up with a different testcase, probably involving unions, where
you could reproduce that RTL sequence.

> however, this last option is an entire rewrite of vector initializers,
> which is in my [future] TODO list...
> 
> in the meantime, is option (b) below acceptable for mainline?

Maybe, but please try to fix the real problem first.

> aldy
> 
> 2002-05-13  Aldy Hernandez  <aldyh@redhat.com>
> 
> 	* expr.c (expand_expr): Output partially zeroed out vectors with
> 	output_constant_def.

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>