public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Henderson <rth@redhat.com>
To: Artem Shinkarov <artyom.shinkaroff@gmail.com>
Cc: gcc-patches@gcc.gnu.org, Richard Guenther <richard.guenther@gmail.com>
Subject: Re: Scalar vector binary operation
Date: Mon, 16 Aug 2010 22:31:00 -0000	[thread overview]
Message-ID: <4C69B7CD.3040208@redhat.com> (raw)
In-Reply-To: <AANLkTikEihhuCipZwHYMzPjAvL9H7h0rxy9ovTsYThfc@mail.gmail.com>

On 08/14/2010 09:37 AM, Artem Shinkarov wrote:
> +/* Build a vector of type VECTYPE where all the elements are SCs.  */
> +tree
> +build_vector_from_val (const tree sc, const tree vectype) 
> +{
> +  tree t = NULL_TREE;
> +  int i, nunits = TYPE_VECTOR_SUBPARTS (vectype);
> +
> +  if (sc == error_mark_node)
> +    return sc;
> +
> +  gcc_assert (TREE_TYPE (sc) == TREE_TYPE (vectype));
> +
> +  for (i = 0; i < nunits; ++i)
> +    t = tree_cons (NULL_TREE, sc, t);
> +
> +  if (CONSTANT_CLASS_P (sc))
> +    return build_vector (vectype, t);
> +  else 
> +    return build_constructor_from_list (vectype, t);
> +}
> +

Better to use build_constructor and build_vector_from_ctor.
We've been trying to eliminate TREE_LIST.

> +static inline bool
> +expr_fits_type_p (const tree expr, const tree type )

No explicit inline marker.

This function doesn't handle real/integer type crosses.
I.e. v4sf * long or v4si * 2.0.  It will crash for some
of the combinations.

> +static inline int
> +scalar_to_vector (location_t loc, enum tree_code code, tree op0, tree op1)

No explicit inline marker.

> +#define MSG "Conversion of scalar to vector involves truncation"
> +#define MSG_FLOAT "Truncating floating point constant to vector " \
> +                  "precision does not preserve value"
> +#define ERROR(MSG, LOC, RET) { error_at (LOC, MSG); return RET; }

This is, frankly, gross.

> +      case RSHIFT_EXPR:
> +      case LSHIFT_EXPR:
> +        if (TREE_CODE (TREE_TYPE (op0)) == INTEGER_TYPE)
> +          return 0;

			  code0

I think you also want to check for vect << int, which is directly
supported by most cpus.  Also, you don't want a fallthru to the
real format checks.

You could probably simplify the code a bit for the arithmetic ops
if you first force op0 to be the vector operation.  Since we're not
actually emitting code here, it doesn't matter if we temporarily
"swap" non-commutative operands.

Some commentary that one of op0 or op1 is known to be vector type
and the other is known to be scalar type would also help here.

Using FP scalar types with integer vector types is probably suspect
and should generate an error.


> +  /* For 'vector <shift> scalar' or 'scalar <shift> vector', we convert 
> +     a scalar to a vector. Truncating the shift amount is ok.  */
> +  if ((code0 == VECTOR_TYPE || code1 == VECTOR_TYPE)
> +      && (code0 != code1))

  if ((code0 == VECTOR_TYPE) != (code1 == VECTOR_TYPE))

> +    {
> +        int convert_flag = scalar_to_vector (location, code, op0, op1);
> +        
> +        if (convert_flag == -2)

Definitely use a switch here.  Possibly better with an enum.


r~

  reply	other threads:[~2010-08-16 22:12 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-14 16:41 Artem Shinkarov
2010-08-16 22:31 ` Richard Henderson [this message]
2010-11-01 16:21 Artem Shinkarov
2010-11-03 13:25 ` Artem Shinkarov
2010-11-03 13:27 ` Richard Guenther
2010-11-03 14:23   ` Artem Shinkarov
2010-11-04 21:15     ` Artem Shinkarov
2010-11-06 18:07     ` Joseph S. Myers
2010-11-08 17:49       ` Artem Shinkarov
2010-11-12 21:39         ` Joseph S. Myers
2010-11-15 18:00           ` Artem Shinkarov
2010-11-16 17:10             ` Joseph S. Myers
2010-11-18 14:01               ` Artem Shinkarov
2010-11-19 22:44                 ` Joseph S. Myers
2010-11-22 16:26                   ` Artem Shinkarov
2010-12-03 18:00                     ` Joseph S. Myers
2010-12-17 16:16                       ` Artem Shinkarov
2011-01-21 15:24                         ` Richard Guenther
2011-01-26  1:13                           ` Joseph S. Myers
2011-02-09 17:09                             ` Artem Shinkarov
2011-08-09 23:54 Artem Shinkarov
2011-08-10  0:00 ` Artem Shinkarov
2011-08-10 15:02   ` Richard Guenther
2011-09-28 14:33     ` Ian Lance Taylor
2011-09-28 14:55       ` Artem Shinkarov
2011-09-28 15:48         ` Ian Lance Taylor
2011-09-28 20:46           ` Artem Shinkarov
2011-12-05 11:27             ` Gerald Pfeifer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C69B7CD.3040208@redhat.com \
    --to=rth@redhat.com \
    --cc=artyom.shinkaroff@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).