From patchwork Fri Dec 6 18:13:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kenneth Zadeck X-Patchwork-Id: 298170 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id D61622C00AD for ; Sat, 7 Dec 2013 05:14:09 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; q=dns; s=default; b=N7xr2SD4i9L/5kNch s+23/zMEMjre72tPhrzHRcq6nuEsENY0VC04pSVaesRlyfs+QngwQMNrU5tRKyA9 slXuW7RJV0q2H07ylfhtab5/nzUd07ltId/xb0b7S5USJCu9cAiA59ZWvqFoiOcs Cu42XzxNHgS8WzSd1JWU7dp1is= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; s=default; bh=JNFNKdzCI7+34iJ8cI0/6sF w8ic=; b=b9AnaYF5nE1HoKGGUAn6EIhKldaZD8dw4ei5k6r+5CMUv7fglqqwy9j PlG5HAHYnXwvZTOnb+YGO51OSZGuZkHIYxn4v8asfDw7Q8UU6bH0+pRLqt6C5WnU WSfxwJL1knD/u/ioFcA0EgDySZTpKLlZIA09DqKCxIhArIB7grg4= Received: (qmail 3948 invoked by alias); 6 Dec 2013 18:14:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 3925 invoked by uid 89); 6 Dec 2013 18:14:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-yh0-f42.google.com Received: from Unknown (HELO mail-yh0-f42.google.com) (209.85.213.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 06 Dec 2013 18:14:00 +0000 Received: by mail-yh0-f42.google.com with SMTP id z6so749815yhz.29 for ; Fri, 06 Dec 2013 10:13:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type; bh=YOkc2XYS2V4+ghEbVIgO9f6c+mHlZncmg4RHS1PwjXc=; b=JdHIynHJbU+9fUfmaGViGbOsQr0ZHhAUR4TYvheTjwZKccH8dACSwYv5frMT6ty4UZ MYZMNpWmoZBjtxfdBIHi8lVpwBUBoKEB2L5FCR6pAilxtWExsrgw6WDly4PRuGB6oCV7 o8J4ANhCkF7z/hkJb091j6XqJHUEVZ2bQ8FHahNMj2kK762oea26uWgyqdDTQ1s43bpl GvYXv7PwBtYD9k/d9bWIAVRGkaXOB7g7duaaVi3GanywqEmjuzEwSfgoX8ze/QarRET7 OOUF0sFROjxLEJPRpHPIdiMolgRNoUDW42oEX2X3kNh38iaQ6FK/08mvlgJewgCTkKtt VHeA== X-Gm-Message-State: ALoCoQlEsceb9uz4y3ZsAoOm5sZDESplbOA0E7dN4bCI3FVCMkEtDjGGuG8kZlaOXYU8RDFyOIsB X-Received: by 10.236.76.138 with SMTP id b10mr3283086yhe.152.1386353632817; Fri, 06 Dec 2013 10:13:52 -0800 (PST) Received: from moria.site (pool-98-113-157-168.nycmny.fios.verizon.net. [98.113.157.168]) by mx.google.com with ESMTPSA id h66sm99732140yhb.7.2013.12.06.10.13.51 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 06 Dec 2013 10:13:51 -0800 (PST) Message-ID: <52A213DE.2050100@naturalbridge.com> Date: Fri, 06 Dec 2013 13:13:50 -0500 From: Kenneth Zadeck User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: Richard Biener , gcc-patches , Mike Stump , rsandifo@linux.vnet.ibm.com Subject: Re: [wide-int] small cleanup in wide-int.* References: <52977927.5010008@naturalbridge.com> <529E04A3.9060806@naturalbridge.com> <87txepanch.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <52A1FF29.3050909@naturalbridge.com> In-Reply-To: <52A1FF29.3050909@naturalbridge.com> Richard asked to see the patch where i did the changes the way that he asked in his email. Doing it the way he wants potentially has advantages over the way that i did it, but his technique fails for non obvious reasons. The failure in building is: g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gccBadMulVrp/gcc -I../../gccBadMulVrp/gcc/. -I../../gccBadMulVrp/gcc/../include -I../../gccBadMulVrp/gcc/../libcpp/include -I../../gccBadMulVrp/gcc/../libdecnumber -I../../gccBadMulVrp/gcc/../libdecnumber/bid -I../libdecnumber -I../../gccBadMulVrp/gcc/../libbacktrace -o tree-vrp.o -MT tree-vrp.o -MMD -MP -MF ./.deps/tree-vrp.TPo ../../gccBadMulVrp/gcc/tree-vrp.c In file included from ../../gccBadMulVrp/gcc/double-int.h:23:0, from ../../gccBadMulVrp/gcc/tree-core.h:28, from ../../gccBadMulVrp/gcc/tree.h:23, from ../../gccBadMulVrp/gcc/tree-vrp.c:26: ../../gccBadMulVrp/gcc/wide-int.h: In member function ‘generic_wide_int& generic_wide_int::operator=(const T&) [with T = generic_wide_int >, storage = wi::extended_tree<1152>]’: ../../gccBadMulVrp/gcc/wide-int.h:701:3: instantiated from ‘generic_wide_int& generic_wide_int::operator-=(const T&) [with T = generic_wide_int >, storage = wi::extended_tree<1152>, generic_wide_int = generic_wide_int >]’ ../../gccBadMulVrp/gcc/tree-vrp.c:2646:13: instantiated from here ../../gccBadMulVrp/gcc/wide-int.h:860:3: error: no matching function for call to ‘generic_wide_int >::operator=(const generic_wide_int >&)’ ../../gccBadMulVrp/gcc/wide-int.h:860:3: note: candidate is: ../../gccBadMulVrp/gcc/tree.h:4529:9: note: wi::extended_tree<1152>& wi::extended_tree<1152>::operator=(const wi::extended_tree<1152>&) ../../gccBadMulVrp/gcc/tree.h:4529:9: note: no known conversion for argument 1 from ‘const generic_wide_int >’ to ‘const wi::extended_tree<1152>&’ make[3]: *** [tree-vrp.o] Error 1 make[3]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp/gcc' make[2]: *** [all-stage1-gcc] Error 2 make[2]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp' make[1]: *** [stage1-bubble] Error 2 make[1]: Leaving directory `/home/zadeck/gcc/gbbBadMulVrp' make: *** [all] Error 2 heracles:~/gcc/gbbBadMulVrp(9) cd ../gccBadMulVrp/ On 12/06/2013 11:45 AM, Kenneth Zadeck wrote: > On 12/03/2013 11:52 AM, Richard Sandiford wrote: >> Kenneth Zadeck writes: >>> Index: tree-vrp.c >>> =================================================================== >>> --- tree-vrp.c (revision 205597) >>> +++ tree-vrp.c (working copy) >>> @@ -2611,22 +2611,28 @@ extract_range_from_binary_expr_1 (value_ >>> signop sign = TYPE_SIGN (expr_type); >>> unsigned int prec = TYPE_PRECISION (expr_type); >>> - unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0); >>> if (range_int_cst_p (&vr0) >>> && range_int_cst_p (&vr1) >>> && TYPE_OVERFLOW_WRAPS (expr_type)) >>> { >>> - wide_int sizem1 = wi::mask (prec, false, prec2); >>> - wide_int size = sizem1 + 1; >>> + /* vrp_int is twice as wide as anything that the target >>> + supports so it can support a full width multiply. No >>> + need to add any more padding for an extra sign bit >>> + because that comes with the way that WIDE_INT_MAX_ELTS is >>> + defined. */ >>> + typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) >>> + vrp_int; >>> + vrp_int sizem1 = wi::mask (prec, false); >>> + vrp_int size = sizem1 + 1; >>> /* Extend the values using the sign of the result to PREC2. >>> From here on out, everthing is just signed math no matter >>> what the input types were. */ >>> - wide_int min0 = wide_int::from (vr0.min, prec2, sign); >>> - wide_int max0 = wide_int::from (vr0.max, prec2, sign); >>> - wide_int min1 = wide_int::from (vr1.min, prec2, sign); >>> - wide_int max1 = wide_int::from (vr1.max, prec2, sign); >>> + vrp_int min0 = wi::to_vrp (vr0.min); >>> + vrp_int max0 = wi::to_vrp (vr0.max); >>> + vrp_int min1 = wi::to_vrp (vr1.min); >>> + vrp_int max1 = wi::to_vrp (vr1.max); >> I think we should avoid putting to_vrp in tree.h if vrp_int is only >> local to this block. Instead you could have: >> >> typedef generic_wide_int >> > vrp_int_cst; >> ... >> vrp_int_cst min0 = vr0.min; >> vrp_int_cst max0 = vr0.max; >> vrp_int_cst min1 = vr1.min; >> vrp_int_cst max1 = vr1.max; >> > i did this in a different way because i had trouble doing it as you > suggested. the short answer is that all of the vrp_int code is now > local to tree-vrp.c which i think was your primary goal >>> @@ -228,15 +228,16 @@ along with GCC; see the file COPYING3. >>> #endif >>> /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very >>> - early examination of the target's mode file. Thus it is safe that >>> - some small multiple of this number is easily larger than any number >>> - that that target could compute. The place in the compiler that >>> - currently needs the widest ints is the code that determines the >>> - range of a multiply. This code needs 2n + 2 bits. */ >>> - >>> + early examination of the target's mode file. The WIDE_INT_MAX_ELTS >>> + can accomodate at least 1 more bit so that unsigned numbers of that >>> + mode can be represented. This will accomodate every place in the >>> + compiler except for a multiply routine in tree-vrp. That function >>> + makes its own arrangements for larger wide-ints. */ >> I think we should drop the "This will accomodate..." bit, since it'll >> soon >> get out of date. Maybe something like: >> >> Note that it is still possible to create fixed_wide_ints that have >> precisions greater than MAX_BITSIZE_MODE_ANY_INT. This can be useful >> when representing a double-width multiplication result, for example. */ > done >>> #define WIDE_INT_MAX_ELTS \ >>> - ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \ >>> - / HOST_BITS_PER_WIDE_INT) >>> + (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \ >>> + / HOST_BITS_PER_WIDE_INT) + 1) >> I think this should be: >> >> (MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_WIDE_INT + 1) >> >> We only need an extra HWI if MAX_BITSIZE_MODE_ANY_INT is an exact >> multiple >> of HOST_BITS_PER_WIDE_INT. > we will do this later when some other issues that Eric B raised are > settled. > > ok to commit to the branch? > > kenny >> Looks good to me otherwise FWIW. >> >> You probably already realise this, but for avoidance of doubt, Richard >> was also asking that we reduce MAX_BITSIZE_MODE_ANY_INT to 128 on >> x86_64, >> since that's the largest scalar_mode_supported_p mode. >> >> Thanks, >> Richard >> > Index: gcc/wide-int.h =================================================================== --- gcc/wide-int.h (revision 205726) +++ gcc/wide-int.h (working copy) @@ -228,15 +228,17 @@ along with GCC; see the file COPYING3. #endif /* The MAX_BITSIZE_MODE_ANY_INT is automatically generated by a very - early examination of the target's mode file. Thus it is safe that - some small multiple of this number is easily larger than any number - that that target could compute. The place in the compiler that - currently needs the widest ints is the code that determines the - range of a multiply. This code needs 2n + 2 bits. */ - + early examination of the target's mode file. The WIDE_INT_MAX_ELTS + can accomodate at least 1 more bit so that unsigned numbers of that + mode can be represented. Note that it is still possible to create + fixed_wide_ints that have precisions greater than + MAX_BITSIZE_MODE_ANY_INT. This can be useful when representing a + double-width multiplication result, for example. */ #define WIDE_INT_MAX_ELTS \ - ((4 * MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \ - / HOST_BITS_PER_WIDE_INT) + (((MAX_BITSIZE_MODE_ANY_INT + HOST_BITS_PER_WIDE_INT - 1) \ + / HOST_BITS_PER_WIDE_INT) + 1) + +#define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT) /* This is the max size of any pointer on any machine. It does not seem to be as easy to sniff this out of the machine description as @@ -294,7 +296,7 @@ struct wide_int_storage; typedef generic_wide_int wide_int; typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int; -typedef FIXED_WIDE_INT (MAX_BITSIZE_MODE_ANY_INT) widest_int; +typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION) widest_int; template struct wide_int_ref_storage; Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 205726) +++ gcc/tree-vrp.c (working copy) @@ -2620,23 +2620,24 @@ extract_range_from_binary_expr_1 (value_ signop sign = TYPE_SIGN (expr_type); unsigned int prec = TYPE_PRECISION (expr_type); - unsigned int prec2 = (prec * 2) + (sign == UNSIGNED ? 2 : 0); if (range_int_cst_p (&vr0) && range_int_cst_p (&vr1) && TYPE_OVERFLOW_WRAPS (expr_type)) { - wide_int sizem1 = wi::mask (prec, false, prec2); - wide_int size = sizem1 + 1; + typedef FIXED_WIDE_INT (WIDE_INT_MAX_PRECISION * 2) vrp_int; + typedef generic_wide_int + > vrp_int_cst; + vrp_int sizem1 = wi::mask (prec, false); + vrp_int size = sizem1 + 1; /* Extend the values using the sign of the result to PREC2. From here on out, everthing is just signed math no matter what the input types were. */ - wide_int min0 = wide_int::from (vr0.min, prec2, sign); - wide_int max0 = wide_int::from (vr0.max, prec2, sign); - wide_int min1 = wide_int::from (vr1.min, prec2, sign); - wide_int max1 = wide_int::from (vr1.max, prec2, sign); - + vrp_int_cst min0 = vr0.min; + vrp_int_cst max0 = vr0.max; + vrp_int_cst min1 = vr1.min; + vrp_int_cst max1 = vr1.max; /* Canonicalize the intervals. */ if (sign == UNSIGNED) { @@ -2653,17 +2654,17 @@ extract_range_from_binary_expr_1 (value_ } } - wide_int prod0 = min0 * min1; - wide_int prod1 = min0 * max1; - wide_int prod2 = max0 * min1; - wide_int prod3 = max0 * max1; + vrp_int prod0 = min0 * min1; + vrp_int prod1 = min0 * max1; + vrp_int prod2 = max0 * min1; + vrp_int prod3 = max0 * max1; /* Sort the 4 products so that min is in prod0 and max is in prod3. */ /* min0min1 > max0max1 */ if (wi::gts_p (prod0, prod3)) { - wide_int tmp = prod3; + vrp_int tmp = prod3; prod3 = prod0; prod0 = tmp; } @@ -2671,21 +2672,21 @@ extract_range_from_binary_expr_1 (value_ /* min0max1 > max0min1 */ if (wi::gts_p (prod1, prod2)) { - wide_int tmp = prod2; + vrp_int tmp = prod2; prod2 = prod1; prod1 = tmp; } if (wi::gts_p (prod0, prod1)) { - wide_int tmp = prod1; + vrp_int tmp = prod1; prod1 = prod0; prod0 = tmp; } if (wi::gts_p (prod2, prod3)) { - wide_int tmp = prod3; + vrp_int tmp = prod3; prod3 = prod2; prod2 = tmp; } Index: gcc/tree.h =================================================================== --- gcc/tree.h (revision 205726) +++ gcc/tree.h (working copy) @@ -4549,7 +4549,7 @@ namespace wi static const unsigned int precision = N; }; - generic_wide_int > + generic_wide_int > to_widest (const_tree); generic_wide_int > to_offset (const_tree); @@ -4570,7 +4570,7 @@ wi::int_traits ::decompose ( precision); } -inline generic_wide_int > +inline generic_wide_int > wi::to_widest (const_tree t) { return t; @@ -4609,7 +4609,7 @@ wi::extended_tree ::get_len () const { if (N == ADDR_MAX_PRECISION) return TREE_INT_CST_OFFSET_NUNITS (m_t); - else if (N == MAX_BITSIZE_MODE_ANY_INT) + else if (N >= WIDE_INT_MAX_PRECISION) return TREE_INT_CST_EXT_NUNITS (m_t); else /* This class is designed to be used for specific output precisions