From patchwork Wed Oct 27 15:15:07 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Artem Shinkarov X-Patchwork-Id: 69364 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 05D55B70D4 for ; Thu, 28 Oct 2010 02:15:41 +1100 (EST) Received: (qmail 27881 invoked by alias); 27 Oct 2010 15:15:38 -0000 Received: (qmail 27865 invoked by uid 22791); 27 Oct 2010 15:15:37 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mail-qy0-f175.google.com (HELO mail-qy0-f175.google.com) (209.85.216.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 27 Oct 2010 15:15:30 +0000 Received: by qyk7 with SMTP id 7so3977561qyk.20 for ; Wed, 27 Oct 2010 08:15:28 -0700 (PDT) Received: by 10.229.225.199 with SMTP id it7mr6105639qcb.33.1288192527242; Wed, 27 Oct 2010 08:15:27 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.182.207 with HTTP; Wed, 27 Oct 2010 08:15:07 -0700 (PDT) In-Reply-To: References: <20101025144714.GZ2806@nightcrawler> <20101025150704.GC2806@nightcrawler> <20101025152737.GD2806@nightcrawler> From: Artem Shinkarov Date: Wed, 27 Oct 2010 16:15:07 +0100 Message-ID: Subject: Re: Vector shifting patch To: "Joseph S. Myers" Cc: Nathan Froyd , gcc-patches@gcc.gnu.org, Richard Guenther X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Wed, Oct 27, 2010 at 1:21 PM, Joseph S. Myers wrote: > On Wed, 27 Oct 2010, Artem Shinkarov wrote: > >> > Most of my comments relate to the scalar case which seems more potentially >> > problematic than the case where both operands are vectors.  But in the >> > case where both operands are vectors I note you say nothing about >> > permitted types of the two operands.  Are they required to be exactly the >> > same?  Or can they differ in signedness?  In the width of elements of the >> > vector? >> >> In case when both operands are vectors my patch requires that they are >> both integer-typed and that the number of elements is the same. On the >> other hand, the existing code c-typeck.c:(build_binary_op):10048 >> >>   if (code0 == VECTOR_TYPE && code1 == VECTOR_TYPE >>       && (!tree_int_cst_equal (TYPE_SIZE (type0), TYPE_SIZE (type1)) >>         || !same_scalar_type_ignoring_signedness (TREE_TYPE (type0), >>                                                   TREE_TYPE (type1)))) >>     { >>       binary_op_error (location, code, type0, type1); >>       return error_mark_node; >>     } >> >> requires that the base type of the vector has the same width. > > But shouldn't the documentation make this explicit for shifts? > > There's a general statement in the documentation already that both length > and signedness must match - but you're describing shifts specially so it > may not be clear what general statements apply to them.  And why is your > paragraph about shifts going in between an example for addition, and a > statement that "Subtraction, multiplication, division, and the logical > operations operate in a similar manner." - that is, in a similar manner to > addition?  The effect would seem to be to change the meaning of "similar" > to refer to shifts rather than addition.  That doesn't seem to be a good > idea.  Work out what the documentation should look like for the extension > as a coherent whole, and it might be clearer what if any new statement > about types is needed. Oh yes, of course you are right, this paragraph must be one paragraph lower. Now it looks like this: > >> > In normal C without vectors, the types of the two operands of a shift are >> > independent.  This would tend to suggest that this should be the case for >> > vectors - they must have the same number of elements, but why constrain >> > things further?  I didn't see any such constraint in the OpenCL >> > specification, although I note that OpenCL doesn't permit scalar << vector >> > or scalar >> vector. >> >> Again, I agree, but then we should do the same stuff for operations +, >> -, *, /, ... >> because we can have an expression in C like: + * >> and the type inference would do the job. > > In C, the integer promotions apply to operands of both + and <<.  For > vector operations they are deliberately excluded - you can operate on > vector char, but not directly on char. > > However, in C the *usual arithmetic conversions* (a superset of the > integer promotions) apply for + but not <<; there is no requirement to > form a common type as there is for arithmetic operations.  So the general > vector rules appear to be being expanded to say: > > * no integer promotions > > * no usual arithmetic conversions > > * must have a common type for vectors (without such promotions / > conversions) even for shifts when ISO C doesn't require a common type for > scalars > >> OpenCL allows scalar vector for most of the arithmetic >> operations, so I thought that it would be nice to have this for shifts >> as well, considering the fact that vector shift where the second >> operator is a scalar is ok. >> >> I don't see how the conversion of the expression 0x12345678 << >> {1,1,1,1} in compile time is different from the conversion of the >> expression: >> vector int x = (vector int){0x12345678, 0x123456778, ...} << {1,1,1,1} >> which is allowed. Ok, in that case we will get warnings if -Woferflow >> is on. > > The point I had was when the shift amount was vector char - so if you > convert the LHS to the type of the RHS then you have (vector char){0x78, > 0x78, 0x78, 0x78}.  Truncating a scalar like that seems a further step > beyond the rules listed above, and it doesn't seem a particularly > desirable step to me. > Ok, I do understand your concern, but my following patch (http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01075.html) implements scalar vector and vector scalar for op in {+, -, /, ...}. In that patch for integer types I use the function int_fits_type_p before I allow scalar to vector conversion. And it would be strange to allow scalar vector everywhere except shifting operators. Would it be ok, if I'll use the same int_fits_type_p checking if scalr fits in the vector type and if not then I'll throw an error. And I'll add this statement in the documentation. What do you think about it? Artem. Index: gcc/doc/extend.texi =================================================================== --- gcc/doc/extend.texi (revision 165913) +++ gcc/doc/extend.texi (working copy) @@ -6315,6 +6315,27 @@ minus or complement operators on a vecto elements are the negative or complemented values of the corresponding elements in the operand. +In C it is possible to use shifting operators @code{<<, >>} on integer-type +vectors. The operation is defined as following: @code{@{a0, a1, @dots{}, +an@} >> @{b0, b1, @dots{}, bn@} == @{a0 >> b0, a1 >> b1, @dots{}, an >> +bn@}}@. Vector operands must have the same number of elements. +Additionally one of the operands can be a scalar integer in which case the +scalar is converted to the type used by the vector operand (with possible +truncation) and each element of this new vector is the scalar's value. +Consider the following code. + +@smallexample +typedef int v4si __attribute__ ((vector_size (16))); + +v4si a, b, c; +int i = 1; + +b = a >> 1; /* b = a >> @{1,1,1,1@}; */ +c = 1 << a; /* c = @{1,1,1,1@} << a; */ +@end smallexample + + + In C vectors can be subscripted as if the vector were an array with the same number of elements and base type. Out of bound accesses invoke undefined behavior at runtime. Warnings for out of bound