From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126912 invoked by alias); 14 Sep 2018 15:37:58 -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 126897 invoked by uid 89); 14 Sep 2018 15:37:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=Yeung, yeung, clobber, bitsize X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Sep 2018 15:37:56 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 183DF80D; Fri, 14 Sep 2018 08:37:55 -0700 (PDT) Received: from localhost (unknown [10.32.99.101]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 703C93F557; Fri, 14 Sep 2018 08:37:54 -0700 (PDT) From: Richard Sandiford To: Mail-Followup-To: ,, richard.sandiford@arm.com Cc: Subject: Re: [PATCH 01/25] Handle vectors that don't fit in an integer. References: Date: Fri, 14 Sep 2018 16:03:00 -0000 In-Reply-To: (ams's message of "Wed, 5 Sep 2018 12:48:49 +0100") Message-ID: <87musk5bud.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-09/txt/msg00755.txt.bz2 writes: > GCN vector sizes range between 64 and 512 bytes, none of which have > correspondingly sized integer modes. This breaks a number of assumptions > throughout the compiler, but I don't really want to create modes just for this > purpose. > > Instead, this patch fixes up the cases that I've found, so far, such that the > compiler tries something else, or fails to optimize, rather than just ICE. > > 2018-09-05 Andrew Stubbs > Kwok Cheung Yeung > Jan Hubicka > Martin Jambor > > gcc/ > * combine.c (gen_lowpart_or_truncate): Return clobber if there is > not a integer mode if the same size as x. > (gen_lowpart_for_combine): Fail if there is no integer mode of the > same size. > * expr.c (expand_expr_real_1): Force first operand to be in memory > if it is a vector register and the result is in BLKmode. > * tree-vect-stmts.c (vectorizable_store): Don't ICE when > int_mode_for_size fails. > (vectorizable_load): Likewise. > --- > gcc/combine.c | 13 ++++++++++++- > gcc/expr.c | 8 ++++++++ > gcc/tree-vect-stmts.c | 8 ++++---- > 3 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/gcc/combine.c b/gcc/combine.c > index a2649b6..cbf9dae 100644 > --- a/gcc/combine.c > +++ b/gcc/combine.c > @@ -8621,7 +8621,13 @@ gen_lowpart_or_truncate (machine_mode mode, rtx x) > { > /* Bit-cast X into an integer mode. */ > if (!SCALAR_INT_MODE_P (GET_MODE (x))) > - x = gen_lowpart (int_mode_for_mode (GET_MODE (x)).require (), x); > + { > + enum machine_mode imode = > + int_mode_for_mode (GET_MODE (x)).require (); > + if (imode == BLKmode) > + return gen_rtx_CLOBBER (mode, const0_rtx); > + x = gen_lowpart (imode, x); require () will ICE if there isn't an integer mode and always returns a scalar_int_mode, so this looks like a no-op. I think you want something like: scalar_int_mode imode; if (!int_mode_for_mode (GET_MODE (x)).exists (&imode)) ... > @@ -11698,6 +11704,11 @@ gen_lowpart_for_combine (machine_mode omode, rtx x) > if (omode == imode) > return x; > > + /* This can happen when there is no integer mode corresponding > + to a size of vector mode. */ > + if (omode == BLKmode) > + goto fail; > + > /* We can only support MODE being wider than a word if X is a > constant integer or has a mode the same size. */ > if (maybe_gt (GET_MODE_SIZE (omode), UNITS_PER_WORD) This seems like it's working around a bug in ther caller. > diff --git a/gcc/expr.c b/gcc/expr.c > index cd5cf12..776254a 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -10569,6 +10569,14 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode, > || maybe_gt (bitpos + bitsize, > GET_MODE_BITSIZE (mode2))); > > + /* If the result is in BLKmode and the underlying object is a > + vector in a register, and the size of the vector is larger than > + the largest integer mode, then we must force OP0 to be in memory > + as this is assumed in later code. */ > + if (REG_P (op0) && VECTOR_MODE_P (mode2) && mode == BLKmode > + && maybe_gt (bitsize, MAX_FIXED_MODE_SIZE)) > + must_force_mem = 1; > + > /* Handle CONCAT first. */ > if (GET_CODE (op0) == CONCAT && !must_force_mem) > { Are you sure this is still needed after: 2018-06-04 Richard Sandiford * expr.c (expand_expr_real_1): Force the operand into memory if its TYPE_MODE is BLKmode and if there is no integer mode for the number of bits being extracted. If so, what case is it handling differently? > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c > index 8d94fca..607a2bd 100644 > --- a/gcc/tree-vect-stmts.c > +++ b/gcc/tree-vect-stmts.c > @@ -6702,12 +6702,12 @@ vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi, > supported. */ > unsigned lsize > = group_size * GET_MODE_BITSIZE (elmode); > - elmode = int_mode_for_size (lsize, 0).require (); > unsigned int lnunits = const_nunits / group_size; > /* If we can't construct such a vector fall back to > element extracts from the original vector type and > element size stores. */ > - if (mode_for_vector (elmode, lnunits).exists (&vmode) > + if (int_mode_for_size (lsize, 0).exists (&elmode) > + && mode_for_vector (elmode, lnunits).exists (&vmode) > && VECTOR_MODE_P (vmode) > && targetm.vector_mode_supported_p (vmode) > && (convert_optab_handler (vec_extract_optab, > @@ -7839,11 +7839,11 @@ vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi, > to a larger load. */ > unsigned lsize > = group_size * TYPE_PRECISION (TREE_TYPE (vectype)); > - elmode = int_mode_for_size (lsize, 0).require (); > unsigned int lnunits = const_nunits / group_size; > /* If we can't construct such a vector fall back to > element loads of the original vector type. */ > - if (mode_for_vector (elmode, lnunits).exists (&vmode) > + if (int_mode_for_size (lsize, 0).exists (&elmode) > + && mode_for_vector (elmode, lnunits).exists (&vmode) > && VECTOR_MODE_P (vmode) > && targetm.vector_mode_supported_p (vmode) > && (convert_optab_handler (vec_init_optab, vmode, elmode) These two are OK independently of the rest (if that's convenient). Thanks, Richard