From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70888 invoked by alias); 30 Oct 2019 14:32:07 -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 70460 invoked by uid 89); 30 Oct 2019 14:32:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-lj1-f178.google.com Received: from mail-lj1-f178.google.com (HELO mail-lj1-f178.google.com) (209.85.208.178) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Oct 2019 14:32:05 +0000 Received: by mail-lj1-f178.google.com with SMTP id 139so2955413ljf.1 for ; Wed, 30 Oct 2019 07:32:05 -0700 (PDT) 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=asdJpedykLZrphHhxhfOEqyC2BLbPQ/4goqWS4DCnuA=; b=GoqvPSu9EOD3xL75v399CjH8uNpP64wRHRCj/KFO8dRGyRnkg9aFr14cR6JAtfifGG D5fzzfhDUJjDm9bzwb8gtMETk01CMZjG5pUF2ySGKrVQXAeNeEDZM5wK6qc/cswHOAg9 071oVclolihT2h1mqi43TGXg/rKYNF2nfjNzKmaGLJlh0lGY1e6sFDtcSe6HXZnCHMTa ptahIMA+Y4+SBfg803k6tBA8Rkd5EFRbiDT4lZe4XIKGqm+Qtdo38ZRMo/kM/EArd/6X uKu/+CE0s6luGL88dO2H5TNCrWUFGzigapL1oGevxd2VOlOQQXQdEdZghHInih+kb9a+ vIDw== MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Wed, 30 Oct 2019 14:33:00 -0000 Message-ID: Subject: Re: [7/n] Use consistent compatibility checks in vectorizable_shift To: Richard Sandiford Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg02138.txt.bz2 On Fri, Oct 25, 2019 at 2:34 PM Richard Sandiford wrote: > > The validation phase of vectorizable_shift used TYPE_MODE to check > whether the shift amount vector was compatible with the shifted vector: > > if ((op1_vectype == NULL_TREE > || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype)) > && (!slp_node > || SLP_TREE_DEF_TYPE > (SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def)) > > But the generation phase was stricter and required the element types to > be equivalent: > > && !useless_type_conversion_p (TREE_TYPE (vectype), > TREE_TYPE (op1))) > > This difference led to an ICE with a later patch. > > The first condition seems a bit too lax given that the function > supports vect_worthwhile_without_simd_p, where two different vector > types could have the same integer mode. But it seems too strict > to reject signed shifts by unsigned amounts or unsigned shifts by > signed amounts; verify_gimple_assign_binary is happy with those. > > This patch therefore goes for a middle ground of checking both TYPE_MODE > and TYPE_VECTOR_SUBPARTS, using the same condition in both places. OK. The whole vectorizable_shift needs a rewrite ;) (no good reason to not support widening/narrowing of a shift argument) Richard. > > 2019-10-24 Richard Sandiford > > gcc/ > * tree-vect-stmts.c (vectorizable_shift): Check the number > of vector elements as well as the type mode when deciding > whether an op1_vectype is compatible. Reuse the result of > this check when generating vector statements. > > Index: gcc/tree-vect-stmts.c > =================================================================== > --- gcc/tree-vect-stmts.c 2019-10-25 13:27:08.653811531 +0100 > +++ gcc/tree-vect-stmts.c 2019-10-25 13:27:12.121787027 +0100 > @@ -5522,6 +5522,7 @@ vectorizable_shift (stmt_vec_info stmt_i > bool scalar_shift_arg = true; > bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info); > vec_info *vinfo = stmt_info->vinfo; > + bool incompatible_op1_vectype_p = false; > > if (!STMT_VINFO_RELEVANT_P (stmt_info) && !bb_vinfo) > return false; > @@ -5666,8 +5667,12 @@ vectorizable_shift (stmt_vec_info stmt_i > > if (!op1_vectype) > op1_vectype = get_same_sized_vectype (TREE_TYPE (op1), vectype_out); > - if ((op1_vectype == NULL_TREE > - || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype)) > + incompatible_op1_vectype_p > + = (op1_vectype == NULL_TREE > + || maybe_ne (TYPE_VECTOR_SUBPARTS (op1_vectype), > + TYPE_VECTOR_SUBPARTS (vectype)) > + || TYPE_MODE (op1_vectype) != TYPE_MODE (vectype)); > + if (incompatible_op1_vectype_p > && (!slp_node > || SLP_TREE_DEF_TYPE > (SLP_TREE_CHILDREN (slp_node)[1]) != vect_constant_def)) > @@ -5813,9 +5818,7 @@ vectorizable_shift (stmt_vec_info stmt_i > } > } > } > - else if (slp_node > - && !useless_type_conversion_p (TREE_TYPE (vectype), > - TREE_TYPE (op1))) > + else if (slp_node && incompatible_op1_vectype_p) > { > if (was_scalar_shift_arg) > {