This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

rfc: vector_cst, reload etc


hello.

i have the following code that gets miscompiled:

vector int bar;
func()
{
  bar = (vector int){0,0,0,69};
}

since vector initializers are currently treated like arrays, in the above
case, gcc will clear out 4 ints, and try to poke 69 into the last offset
(instead of outputting a symbol with 0,0,0,69).  like this:

(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)
because GPRs are not valid for "movv4si_const0".  but reload will
*not* force a reload of insn 16 because it sees:

	(set (subreg:SI (reg:V4SI r12)) 69)

...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.
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.

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?

aldy

2002-05-13  Aldy Hernandez  <aldyh@redhat.com>

	* expr.c (expand_expr): Output partially zeroed out vectors with
	output_constant_def.


Index: expr.c
===================================================================
RCS file: /cvs/uberbaum/gcc/expr.c,v
retrieving revision 1.452
diff -c -p -r1.452 expr.c
*** expr.c	7 May 2002 05:44:26 -0000	1.452
--- expr.c	13 May 2002 05:12:01 -0000
*************** expand_expr (exp, target, tmode, modifie
*** 6653,6659 ****
  	 fold.  Likewise, if we have a target we can use, it is best to
  	 store directly into the target unless the type is large enough
  	 that memcpy will be used.  If we are making an initializer and
! 	 all operands are constant, put it in memory as well.  */
        else if ((TREE_STATIC (exp)
  		&& ((mode == BLKmode
  		     && ! (target != 0 && safe_from_p (target, exp, 1)))
--- 6653,6665 ----
  	 fold.  Likewise, if we have a target we can use, it is best to
  	 store directly into the target unless the type is large enough
  	 that memcpy will be used.  If we are making an initializer and
! 	 all operands are constant, put it in memory as well.
! 
! 	 FIXME: Avoid trying to fill vector constructors piece-meal.
! 	 Output them with output_constant_def below unless we're sure
! 	 they're zeros.  This should go away when vector initializers
! 	 are treated like VECTOR_CST instead of arrays.
!       */
        else if ((TREE_STATIC (exp)
  		&& ((mode == BLKmode
  		     && ! (target != 0 && safe_from_p (target, exp, 1)))
*************** expand_expr (exp, target, tmode, modifie
*** 6662,6668 ****
  			&& (! MOVE_BY_PIECES_P
  			    (tree_low_cst (TYPE_SIZE_UNIT (type), 1),
  			     TYPE_ALIGN (type)))
! 			&& ! mostly_zeros_p (exp))))
  	       || (modifier == EXPAND_INITIALIZER && TREE_CONSTANT (exp)))
  	{
  	  rtx constructor = output_constant_def (exp, 1);
--- 6668,6676 ----
  			&& (! MOVE_BY_PIECES_P
  			    (tree_low_cst (TYPE_SIZE_UNIT (type), 1),
  			     TYPE_ALIGN (type)))
! 			&& ((TREE_CODE (type) == VECTOR_TYPE
! 			     && !is_zeros_p (exp))
! 			    || ! mostly_zeros_p (exp)))))
  	       || (modifier == EXPAND_INITIALIZER && TREE_CONSTANT (exp)))
  	{
  	  rtx constructor = output_constant_def (exp, 1);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]