From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57271 invoked by alias); 8 Jan 2019 13:09:34 -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 57260 invoked by uid 89); 8 Jan 2019 13:09:33 -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=6029 X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 08 Jan 2019 13:09:30 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id AB914B051; Tue, 8 Jan 2019 13:09:28 +0000 (UTC) Subject: Re: [PATCH] Use proper type in linear transformation in tree-switch-conversion (PR tree-optimization/88753). To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org References: <1b7be4a1-525c-101d-d897-76b637fd14b1@suse.cz> <20190108125235.GA30353@tucnak> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <559fe855-4093-3004-fe97-e53c5aec8e58@suse.cz> Date: Tue, 08 Jan 2019 13:09:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <20190108125235.GA30353@tucnak> Content-Type: multipart/mixed; boundary="------------6887535C3394517D82614B1E" X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00393.txt.bz2 This is a multi-part message in MIME format. --------------6887535C3394517D82614B1E Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 1252 On 1/8/19 1:52 PM, Jakub Jelinek wrote: > On Tue, Jan 08, 2019 at 01:40:41PM +0100, Martin Liška wrote: >> As seen in the PR, when doing switch convert linear transformation, >> one needs to first convert to unsigned type for constructor values. >> >> Patch survives regression tests and bootstrap on x86_64-linux-gnu. >> Ready for trunk? >> Thanks, >> Martin >> >> >> gcc/ChangeLog: >> >> 2019-01-08 Martin Liska >> >> PR tree-optimization/88753 >> * tree-switch-conversion.c (switch_conversion::build_one_array): >> Come up with local variable constructor. Convert first to >> type of constructor values. > > Why is the testcase missing? Because I forgot to call git add ;) > > Otherwise LGTM. > > Also, note contains_linear_function_p > wide_int range_min = wi::to_wide (fold_convert (TREE_TYPE (elt0), > m_range_min)); > could be written on wide_ints directly: > wide_int range_min > = wide_int::from (wi::to_wide (m_range_min), > TYPE_PRECISION (TREE_TYPE (elt0)), > TYPE_SIGN (TREE_TYPE (m_range_min))); > > Jakub > Changed that. I verified that tree-ssa.exp tests work fine. May I install the hunk you suggested without testing? Martin --------------6887535C3394517D82614B1E Content-Type: text/x-patch; name="0001-Use-proper-type-in-linear-transformation-in-tree-swi.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Use-proper-type-in-linear-transformation-in-tree-swi.pa"; filename*1="tch" Content-length: 4420 >From 52f8961361f894cca35c616a38486f69c48c194c Mon Sep 17 00:00:00 2001 From: marxin Date: Tue, 8 Jan 2019 12:56:03 +0100 Subject: [PATCH] Use proper type in linear transformation in tree-switch-conversion (PR tree-optimization/88753). gcc/ChangeLog: 2019-01-08 Martin Liska PR tree-optimization/88753 * tree-switch-conversion.c (switch_conversion::build_one_array): Come up with local variable constructor. Convert first to type of constructor values. gcc/testsuite/ChangeLog: 2019-01-08 Martin Liska * gcc.dg/tree-ssa/pr88753.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr88753.c | 57 +++++++++++++++++++++++++ gcc/tree-switch-conversion.c | 17 +++++--- 2 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr88753.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr88753.c b/gcc/testsuite/gcc.dg/tree-ssa/pr88753.c new file mode 100644 index 00000000000..eaefc38962f --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88753.c @@ -0,0 +1,57 @@ +/* PR tree-optimization/88753 */ +/* { dg-options "-O2 -fdump-tree-switchconv" } */ +/* { dg-do run { target nonpic } } */ + +typedef unsigned short int uint16_t; +typedef unsigned char uint8_t; + +uint16_t length; +uint16_t enc_method_global; + +uint8_t +__attribute__((noipa)) +_zip_buffer_get_8(int buffer) +{ + return buffer; +} + + int + __attribute__((noipa)) +foo(int v) +{ + uint16_t enc_method; + switch (_zip_buffer_get_8(v)) { + case 1: + enc_method = 0x0101; + break; + case 2: + enc_method = 0x0102; + break; + case 3: + enc_method = 0x0103; + break; + default: + __builtin_abort (); + } + + enc_method_global = enc_method; +} + +int main(int argc, char **argv) +{ + foo (1); + if (enc_method_global != 0x0101) + __builtin_abort (); + + foo (2); + if (enc_method_global != 0x0102) + __builtin_abort (); + + foo (3); + if (enc_method_global != 0x0103) + __builtin_abort (); + + return 0; +} + +/* { dg-final { scan-tree-dump-times "Linear transformation with A = 1 and B = 256" 1 "switchconv" } } */ diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c index 614c450dd4d..c3f2baf39d7 100644 --- a/gcc/tree-switch-conversion.c +++ b/gcc/tree-switch-conversion.c @@ -473,8 +473,10 @@ switch_conversion::contains_linear_function_p (vec *vec, if (TREE_CODE (elt0) != INTEGER_CST || TREE_CODE (elt1) != INTEGER_CST) return false; - wide_int range_min = wi::to_wide (fold_convert (TREE_TYPE (elt0), - m_range_min)); + wide_int range_min + = wide_int::from (wi::to_wide (m_range_min), + TYPE_PRECISION (TREE_TYPE (elt0)), + TYPE_SIGN (TREE_TYPE (m_range_min))); wide_int y1 = wi::to_wide (elt0); wide_int y2 = wi::to_wide (elt1); wide_int a = y2 - y1; @@ -600,9 +602,9 @@ switch_conversion::build_one_array (int num, tree arr_index_type, name = copy_ssa_name (PHI_RESULT (phi)); m_target_inbound_names[num] = name; + vec *constructor = m_constructors[num]; wide_int coeff_a, coeff_b; - bool linear_p = contains_linear_function_p (m_constructors[num], &coeff_a, - &coeff_b); + bool linear_p = contains_linear_function_p (constructor, &coeff_a, &coeff_b); if (linear_p) { if (dump_file && coeff_a.to_uhwi () > 0) @@ -610,7 +612,8 @@ switch_conversion::build_one_array (int num, tree arr_index_type, " and B = %" PRId64 "\n", coeff_a.to_shwi (), coeff_b.to_shwi ()); - tree t = unsigned_type_for (TREE_TYPE (m_index_expr)); + /* We must use type of constructor values. */ + tree t = unsigned_type_for (TREE_TYPE ((*constructor)[0].value)); gimple_seq seq = NULL; tree tmp = gimple_convert (&seq, t, m_index_expr); tree tmp2 = gimple_build (&seq, MULT_EXPR, t, @@ -633,10 +636,10 @@ switch_conversion::build_one_array (int num, tree arr_index_type, unsigned int i; constructor_elt *elt; - FOR_EACH_VEC_SAFE_ELT (m_constructors[num], i, elt) + FOR_EACH_VEC_SAFE_ELT (constructor, i, elt) elt->value = fold_convert (value_type, elt->value); } - ctor = build_constructor (array_type, m_constructors[num]); + ctor = build_constructor (array_type, constructor); TREE_CONSTANT (ctor) = true; TREE_STATIC (ctor) = true; -- 2.20.1 --------------6887535C3394517D82614B1E--