Re: [patch] SPARC VIS instructions or PR1820

[ja2morri at csclub dot uwaterloo dot ca (James A. Morrison)]

Eric Botcazou <ebotcazou@libertysurf.fr> writes:

> >  Hi, this patch allows the sparc backend to generate VIS instructions for
> > vector operations.  I've added the arithmetic VIS instructions.  For any
> > other VIS instruction I've changed the mode to be created from a mode macro
> > without having to change anything.  So far I've only tested this with
> > bubblestraps and running the tests that are attached.  I'm running a
> > bootstrap and regtest now on sparc-linux.  Then I'm going to do the same
> > thing on sparc64-linux.
> 
> First of all, this is nice.  I always have feared that adding VIS support 
> would bloat sparc.md, but the new mode macro feature has obliterated the 
> problem.
> 
> [I'll privately send you a copy of a sketch of VIS support by David S. Miller]
> 
> 
> I have a few remarks:
> 
> --- gcc/config/sparc/sparc-modes.def	13 Oct 2003 21:16:32 -0000	1.4
> +++ gcc/config/sparc/sparc-modes.def	31 Oct 2004 12:30:33 -0000
> @@ -42,3 +42,7 @@ CC_MODE (CC_NOOV);
>  CC_MODE (CCX_NOOV);
>  CC_MODE (CCFP);
>  CC_MODE (CCFPE);
> +
> +/* Vector modes.  */
> +VECTOR_MODES (INT, 8);        /*       V8QI V4HI V2SI */
> +VECTOR_MODES (INT, 4);        /*       V4QI V2HI */
> 
> I think we should support V1SI too.  This would correspond to the fpxxx32s VIS 
> instructions.

 Really?  I haven't figured out how these are any different than SImode.
I've updated the comment anyway.

> 
> +/* Vector modes are only supported with VIS.  */
> +static bool
> +sparc_vector_mode_supported_p (enum machine_mode mode)
> +{
> +
> +  if (TARGET_VIS && VECTOR_MODE_P (mode))
> +    return true;
> +  else
> +    return false;
> 
> The comment should be inside the function.  The head comment should be 
> something like:
> /* Implement the TARGET_VECTOR_MODE_SUPPORTED_P target hook.
>    Specify whether the vector mode is supported by the hardware.  */
> 
> And I think
> 
>   return TARGET_VIS && VECTOR_MODE_P (mode) ? true : false;
> 
> is even better.

 I've changed this to return TARGET_VIS && VECTOR_MODE_P (mode);

> 
> +(define_expand "and<V64I:mode>3"
> +  [(set (match_operand:V64I 0 "register_operand" "")
> +	(and:V64I (match_operand:V64I 1 "arith_double_operand" "")
> +		(match_operand:V64I 2 "arith_double_operand" "")))]
> 
> +(define_expand "xor<V64I:mode>3"
> +  [(set (match_operand:V64I 0 "register_operand" "")
> +	(xor:V64I (match_operand:V64I 1 "arith_double_operand" "")
> +		(match_operand:V64I 2 "arith_double_operand" "")))]
> 
> Any specific reason why you didn't convert the ior, xnor,... patterns too?

 I didn't know of them, I haven't found extensive VIS documentation anywhere.
 
> +;; VIS instructions
> +(define_insn "addv2si3"
> +  [(set (match_operand:V2SI 0 "register_operand" "=e")
> +        (plus:V2SI (match_operand:V2SI 1 "register_operand" "e")
> +                   (match_operand:V2SI 2 "register_operand" "e")))]
> +  "TARGET_VIS"
> +  "fpadd32\t%1, %2, %0"
> +  [(set_attr "type" "fga")])
> 
> Perhaps using the VIS-specific constraint letters would be better.  On the 
> other hand, this would certainly be redundant with the condition.

 At some point I was having trouble using "b", so I switch to using "e" and
things stopped failing.
 
> You need to set the "fptype" attribute too.

 Ok.

> 
> I'm testing a patch that will bring the back-end's function calling machinery 
> into a working state wrt to vector types.  We need to again make sure that 
> the calling conventions are not changed by -mvis/-mno-vis.

 As long as vectors can be packed into integer registers for function passing
we shouldn't have any trouble.  Perhaps vector loads and stores should be
available with or without -mvis.

> -- 
> Eric Botcazou
> 


-- 
Thanks,
Jim

http://www.student.cs.uwaterloo.ca/~ja2morri/
http://phython.blogspot.com
http://open.nit.ca/wiki/?page=jim