From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19056 invoked by alias); 5 Nov 2019 16:09:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 19044 invoked by uid 89); 5 Nov 2019 16:09:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-lj1-f170.google.com Received: from mail-lj1-f170.google.com (HELO mail-lj1-f170.google.com) (209.85.208.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Nov 2019 16:09:19 +0000 Received: by mail-lj1-f170.google.com with SMTP id q2so15941098ljg.7 for ; Tue, 05 Nov 2019 08:09:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IhI48mZySiqDuFzl2CsPOdtLdI4rN7gcm9zBOMahbYw=; b=DsEbX6umgFxse1f8grdr7FFzdqEUdsdcM6NmuFcozlArW534+lH1Ad/YE/7ArpDtWZ eLyux7H8pCrYce4x3GdBSGuxV+tErBL5S1C4pfovIdzso+D8apKOzZa+0FHM+fosva5v zWme7gVaAXjpHossAjSazsftTbpOw6fSK1pbtlQfiEmc4U6+0ReT3JIigshwYdUjCOIB hb+cERaXxB+GhmWCXFmb0w1u6S3/Y+ZvLGifhzWiCY/pv8v3pegOUolmIOMBMZJmx1QX DzeNJv0XbM71GfGjikDry1+nbZ/xAi8k7Bpmn1vS74DOl4XF8Y4Osqj7uD+aiIGRVk9n R/iQ== MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Tue, 05 Nov 2019 16:09:00 -0000 Message-ID: Subject: Re: [10/n] Make less use of get_same_sized_vectype To: Richard Sandiford Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00316.txt.bz2 On Tue, Nov 5, 2019 at 4:34 PM Richard Sandiford wrote: > > Richard Biener writes: > > On Fri, Oct 25, 2019 at 2:41 PM Richard Sandiford > > wrote: > >> > >> Some callers of get_same_sized_vectype were dealing with operands that > >> are constant or defined externally, and so have no STMT_VINFO_VECTYPE > >> available. Under the current model, using get_same_sized_vectype for > >> that case is equivalent to using get_vectype_for_scalar_type, since > >> get_vectype_for_scalar_type always returns vectors of the same size, > >> once a size is fixed. > >> > >> Using get_vectype_for_scalar_type is arguably more obvious though: > >> if we're using the same scalar type as we would for internal > >> definitions, we should use the same vector type too. (Constant and > >> external definitions sometimes let us change the original scalar type > >> to a "nicer" scalar type, but that isn't what's happening here.) > >> > >> This is a prerequisite to supporting multiple vector sizes in the same > >> vec_info. > > > > This might change the actual type we get back, IIRC we mass-changed > > it in the opposite direction from your change in the past, because it's > > more obvious to relate the type used to another vector type on the > > stmt. So isn't it better to use the new related_vector_type thing here? > > I guess this is a downside of the patch order. Hopefully this looks > like a more sensible decision after 16/n, where we also pass the > group size to get_vectype_for_scalar_type. If not: :-) > > At the moment, there can only ever be one vector type for a given scalar > type within a vector loop. We don't e.g. allow one loop vector stmt to > use V2SI and another to V4SI, because all vectorised SIs have to be > compatible in the same way that the original scalar SIs were. So once > we have a loop_vec_info and once we have a scalar type, there's only one > valid choice of vector type. I think trying to circumvent that by using > get_related_vectype_for_scalar_type instead of get_vectype_for_scalar_type > would run the risk of introducing accidental mismatches. I.e. if we do > it right, calling get_related_vectype_for_scalar_type would give the same > result as calling get_vectype_for_scalar_type (and so we might as well > just call get_vectype_for_scalar_type). If we do it wrong we can end up > with a different type from the one that other statements were expecting. > > BB vectorisation currently works the same way. But after 16/n, there > is instead one vector type for each (bb_vinfo, scalar_type, group_size) > triple. So that patch makes it possible for different SLP instances to > have different vector types for the same scalar type, but the choice is > still fixed within an SLP instance, in a similar way to loop > vectorisation. > > So I think using anything other than get_vectype_for_scalar_type > would give a false sense of freedom. Calling it directly is mostly > useful for temporaries, e.g. in epilogue reduction handling. OK, I guess we'll see how it plays out in the end of the series(es). What I think should be there in the end is vectorizable_* asking for the vector type that matches what they implement in terms of the operation. Say, if it is a PLUS and one operand has a vector type set in the internal_def definition then the constant/external_def should get the very same vector type. That is, ideally those routines would _not_ call "get me a vector type" but simply compute the required type. That is, vectorizable_operation should not need to call these functions at unless we run into a completely invariant operation (in which case we might as well fail vectorization). I see it doesn't care for the vector type of op1/2 which is probably a bug since with multiple sizes we may get V2SI on one op and V4SI on another? Richard. > Thanks, > Richard