From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id E2C983857C4D for ; Tue, 1 Sep 2020 14:27:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E2C983857C4D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 56C51ACBF; Tue, 1 Sep 2020 14:27:47 +0000 (UTC) Subject: Re: [PATCH] VEC_COND_EXPR: fix ICE in gimple_expand_vec_cond_expr To: Richard Biener Cc: GCC Patches References: From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <73f905a3-cef1-e9e5-0764-0105e7c1c2d6@suse.cz> Date: Tue, 1 Sep 2020 16:27:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Sep 2020 14:27:50 -0000 On 8/31/20 10:01 AM, Richard Biener wrote: > On Fri, Aug 28, 2020 at 4:18 PM Martin Liška wrote: >> >> Hey. >> >> The patch is about VEC_COND_EXP comparison of a SSA_NAME with a constant >> that is artifact of -fno-tree-ccp. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed? > > Err, no - we shouldn't do this at RTL expansion time. And piecewise > expansion is gross - in this the case is a constant boolean vector IIRC > (not sure what mode?) Yes, it's: (gdb) p op0.typed.type $11 = (gdb) p mode $13 = E_DImode and TYPE_MODE (TREE_TYPE (op0)) is E_BLKmode > and ISEL only tries matching up with > a comparison operand. But ISEL should also handle matching up with > a constant "boolean vector" ("boolean vector"s are artifacts of VEC_COND_EXPRs, > but some targets allow bitwise arithmetic on them...). So the compensation > needs to happen in ISEL, recognizing > > _1 = {0} > _2 = _1 ? ...; > > as > > _2 = .VCOND (0 == 0, ...) Do we really want to do a scalar arguments of .VCOND? Note that in PR96453 (a similar bug), we end up with: _5 = { -1, -1 }; _6 = VEC_COND_EXPR <_5, { -1, -1 }, { 0, 0 }>; Thanks for hints, Martin > > or so. > > Richard. > >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> PR tree-optimization/96466 >> * gimple-fold.c (expand_cmp_piecewise): New. >> * gimple-fold.h (nunits_for_known_piecewise_op): New. >> (expand_cmp_piecewise): Moved from ... >> * tree-vect-generic.c (expand_vector_comparison): ... here. >> (nunits_for_known_piecewise_op): Moved to gimple-fold.h. >> * gimple-isel.cc (gimple_expand_vec_cond_expr): Use >> expand_cmp_piecewise fallback for constants. >> >> gcc/testsuite/ChangeLog: >> >> PR tree-optimization/96466 >> * gcc.dg/vect/pr96466.c: New test. >> --- >> gcc/gimple-fold.c | 28 ++++++++++++++++++++ >> gcc/gimple-fold.h | 14 ++++++++++ >> gcc/gimple-isel.cc | 10 ++++--- >> gcc/testsuite/gcc.dg/vect/pr96466.c | 18 +++++++++++++ >> gcc/tree-vect-generic.c | 41 ++--------------------------- >> 5 files changed, 69 insertions(+), 42 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/vect/pr96466.c >> >> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c >> index dcc1b56a273..86d5d0ed7d8 100644 >> --- a/gcc/gimple-fold.c >> +++ b/gcc/gimple-fold.c >> @@ -8056,3 +8056,31 @@ gimple_stmt_integer_valued_real_p (gimple *stmt, int depth) >> return false; >> } >> } >> + >> +tree >> +expand_cmp_piecewise (gimple_stmt_iterator *gsi, tree type, tree op0, tree op1) >> +{ >> + tree inner_type = TREE_TYPE (TREE_TYPE (op0)); >> + tree part_width = vector_element_bits_tree (TREE_TYPE (op0)); >> + tree index = bitsize_int (0); >> + int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0)); >> + int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type)); >> + tree ret_type = build_nonstandard_integer_type (prec, 1); >> + tree ret_inner_type = boolean_type_node; >> + int i; >> + tree t = build_zero_cst (ret_type); >> + >> + if (TYPE_PRECISION (ret_inner_type) != 1) >> + ret_inner_type = build_nonstandard_integer_type (1, 1); >> + for (i = 0; i < nunits; >> + i++, index = int_const_binop (PLUS_EXPR, index, part_width)) >> + { >> + tree a = tree_vec_extract (gsi, inner_type, op0, part_width, index); >> + tree b = tree_vec_extract (gsi, inner_type, op1, part_width, index); >> + tree result = gimplify_build2 (gsi, NE_EXPR, ret_inner_type, a, b); >> + t = gimplify_build3 (gsi, BIT_INSERT_EXPR, ret_type, t, result, >> + bitsize_int (i)); >> + } >> + >> + return gimplify_build1 (gsi, VIEW_CONVERT_EXPR, type, t); >> +} >> diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h >> index 0ed1d1ffe83..7e843b34f53 100644 >> --- a/gcc/gimple-fold.h >> +++ b/gcc/gimple-fold.h >> @@ -147,6 +147,20 @@ gimple_build_vector (gimple_seq *seq, tree_vector_builder *builder) >> extern bool gimple_stmt_nonnegative_warnv_p (gimple *, bool *, int = 0); >> extern bool gimple_stmt_integer_valued_real_p (gimple *, int = 0); >> >> +/* Return the number of elements in a vector type TYPE that we have >> + already decided needs to be expanded piecewise. We don't support >> + this kind of expansion for variable-length vectors, since we should >> + always check for target support before introducing uses of those. */ >> + >> +static inline unsigned int >> +nunits_for_known_piecewise_op (const_tree type) >> +{ >> + return TYPE_VECTOR_SUBPARTS (type).to_constant (); >> +} >> + >> +extern tree expand_cmp_piecewise (gimple_stmt_iterator *gsi, tree lhs, >> + tree op0, tree op1); >> + >> /* In gimple-match.c. */ >> extern tree gimple_simplify (enum tree_code, tree, tree, >> gimple_seq *, tree (*)(tree)); >> diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc >> index b330cf4c20e..32e3bc31f7f 100644 >> --- a/gcc/gimple-isel.cc >> +++ b/gcc/gimple-isel.cc >> @@ -33,8 +33,8 @@ along with GCC; see the file COPYING3. If not see >> #include "gimplify-me.h" >> #include "gimplify.h" >> #include "tree-cfg.h" >> -#include "bitmap.h" >> #include "tree-ssa-dce.h" >> +#include "gimple-fold.h" >> >> /* Expand all VEC_COND_EXPR gimple assignments into calls to internal >> function based on type of selected expansion. */ >> @@ -119,8 +119,12 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi, >> /* Fake op0 < 0. */ >> else >> { >> - gcc_assert (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (op0))) >> - == MODE_VECTOR_INT); >> + if (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (op0))) != MODE_VECTOR_INT) >> + { >> + tree t = expand_cmp_piecewise (gsi, TREE_TYPE (lhs), op0, op1); >> + return gimple_build_assign (lhs, NOP_EXPR, t); >> + } >> + >> op0a = op0; >> op0b = build_zero_cst (TREE_TYPE (op0)); >> tcode = LT_EXPR; >> diff --git a/gcc/testsuite/gcc.dg/vect/pr96466.c b/gcc/testsuite/gcc.dg/vect/pr96466.c >> new file mode 100644 >> index 00000000000..8cca5e12ff2 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/vect/pr96466.c >> @@ -0,0 +1,18 @@ >> +/* PR tree-optimization/96466 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-Og -finline-functions-called-once -fno-tree-ccp" } */ >> + >> +typedef unsigned long __attribute__ ((__vector_size__ (8))) V; >> + >> +V >> +bar (unsigned long x, V v) >> +{ >> + v &= x >= v; >> + return (V) v; >> +} >> + >> +V >> +foo (void) >> +{ >> + return bar (5, (V) 4441221375); >> +} >> diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c >> index 6d5d65195ae..b01aa301baa 100644 >> --- a/gcc/tree-vect-generic.c >> +++ b/gcc/tree-vect-generic.c >> @@ -42,20 +42,11 @@ along with GCC; see the file COPYING3. If not see >> #include "insn-config.h" >> #include "tree-ssa-dce.h" >> #include "recog.h" /* FIXME: for insn_data */ >> +#include "gimple-fold.h" >> >> >> static void expand_vector_operations_1 (gimple_stmt_iterator *, bitmap); >> >> -/* Return the number of elements in a vector type TYPE that we have >> - already decided needs to be expanded piecewise. We don't support >> - this kind of expansion for variable-length vectors, since we should >> - always check for target support before introducing uses of those. */ >> -static unsigned int >> -nunits_for_known_piecewise_op (const_tree type) >> -{ >> - return TYPE_VECTOR_SUBPARTS (type).to_constant (); >> -} >> - >> /* Return true if TYPE1 has more elements than TYPE2, where either >> type may be a vector or a scalar. */ >> >> @@ -427,35 +418,7 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0, >> TYPE_VECTOR_SUBPARTS (type) >> * GET_MODE_BITSIZE (SCALAR_TYPE_MODE >> (TREE_TYPE (type))))) >> - { >> - tree inner_type = TREE_TYPE (TREE_TYPE (op0)); >> - tree part_width = vector_element_bits_tree (TREE_TYPE (op0)); >> - tree index = bitsize_int (0); >> - int nunits = nunits_for_known_piecewise_op (TREE_TYPE (op0)); >> - int prec = GET_MODE_PRECISION (SCALAR_TYPE_MODE (type)); >> - tree ret_type = build_nonstandard_integer_type (prec, 1); >> - tree ret_inner_type = boolean_type_node; >> - int i; >> - location_t loc = gimple_location (gsi_stmt (*gsi)); >> - t = build_zero_cst (ret_type); >> - >> - if (TYPE_PRECISION (ret_inner_type) != 1) >> - ret_inner_type = build_nonstandard_integer_type (1, 1); >> - warning_at (loc, OPT_Wvector_operation_performance, >> - "vector operation will be expanded piecewise"); >> - for (i = 0; i < nunits; >> - i++, index = int_const_binop (PLUS_EXPR, index, part_width)) >> - { >> - tree a = tree_vec_extract (gsi, inner_type, op0, part_width, >> - index); >> - tree b = tree_vec_extract (gsi, inner_type, op1, part_width, >> - index); >> - tree result = gimplify_build2 (gsi, code, ret_inner_type, a, b); >> - t = gimplify_build3 (gsi, BIT_INSERT_EXPR, ret_type, t, result, >> - bitsize_int (i)); >> - } >> - t = gimplify_build1 (gsi, VIEW_CONVERT_EXPR, type, t); >> - } >> + t = expand_cmp_piecewise (gsi, type, op0, op1); >> else >> t = expand_vector_piecewise (gsi, do_compare, type, >> TREE_TYPE (TREE_TYPE (op0)), op0, op1, >> -- >> 2.28.0 >>