From patchwork Wed Nov 27 16:24:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 294615 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 1ADCE2C0086 for ; Thu, 28 Nov 2013 03:26:05 +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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; q=dns; s= default; b=QUvA1Xn3khYH1wdvd5l7blWBmvhxKTeneMHSic2VuRNoh13C69uGs 77H9SNljAeTox91O3qAYjUyC0jxCumCxolAw8C2ulQ2kk2k94wM4RtNx+ERW3jK5 W1lo5fUAjTYc1P4ucmIO1MfeLWuV7t+PA3OeG4TQ6BAouaVFNlWGJY= 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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=default; bh=O93o7eyX0ku66hY6tNOLm5VFkLQ=; b=t0BB1VX6n9ES4UR9uBljRGpw2Gkg P0900tYmOMaPCORvxcwiNglfJOsBUjtWD21v5pvK3/B1/TtjdhaHECar6fgQiI+c jvHRoBnV2K7K6P3l3cVfQQDIlhD2lXt4iQ1wShVXBMlsQZLf7hA+LceAo96NzI2P UXnIgrelqS7/4e0= Received: (qmail 30927 invoked by alias); 27 Nov 2013 16:25:54 -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 30915 invoked by uid 89); 27 Nov 2013 16:25:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL, BAYES_50, RDNS_NONE autolearn=no version=3.3.2 X-HELO: smtp.eu.adacore.com Received: from Unknown (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 27 Nov 2013 16:25:53 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 2FB5126AD3C8; Wed, 27 Nov 2013 17:25:44 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PG5EKm75yHzH; Wed, 27 Nov 2013 17:25:44 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id F321826AD3B5; Wed, 27 Nov 2013 17:25:43 +0100 (CET) From: Eric Botcazou To: Mike Stump Cc: gcc-patches@gcc.gnu.org, Kenneth Zadeck Subject: Re: wide-int, rtl Date: Wed, 27 Nov 2013 17:24:52 +0100 Message-ID: <1673383.yVHhgiMuu2@polaris> User-Agent: KMail/4.7.2 (Linux/3.1.10-1.29-desktop; KDE/4.7.2; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 > Richi has asked the we break the wide-int patch so that the individual port > and front end maintainers can review their parts without have to go through > the entire patch. This patch covers the first half of the rtl code. @@ -336,6 +348,14 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"), 1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.c. */ unsigned return_val : 1; + union { + /* RTXs are free to use up to 32 bit from here. */ + + /* In a CONST_WIDE_INT (aka hwivec_def), this is the number of + HOST_WIDE_INTs in the hwivec_def. */ + unsigned GTY ((tag ("CONST_WIDE_INT"))) num_elem:32; + } GTY ((desc ("GET_CODE (&%0)"))) u2; + /* The first element of the operands of this rtx. The number of operands and their types are controlled by the `code' field, according to rtl.def. */ Why can't this be put into hwivec_def? --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c + if (op_mode == VOIDmode) + { + /* CONST_INT have VOIDmode as the mode. We assume that all + the bits of the constant are significant, though, this is + a dangerous assumption as many times CONST_INTs are + created and used with garbage in the bits outside of the + precision of the implied mode of the const_int. */ + op_mode = MAX_MODE_INT; + } Well, if that's really dangerous, we should find another way or at least stop the compiler before generating wrong code. +#if TARGET_SUPPORTS_WIDE_INT == 0 + /* This assert keeps the simplification from producing a result + that cannot be represented in a CONST_DOUBLE but a lot of + upstream callers expect that this function never fails to + simplify something and so you if you added this to the test + above the code would die later anyway. If this assert + happens, you just need to make the port support wide int. */ + gcc_assert (width <= HOST_BITS_PER_DOUBLE_INT); +#endif Can't we be more forgiving here and in the other similar places? + /* We support any size mode. */ + max_bitsize = MAX (GET_MODE_BITSIZE (outermode), + GET_MODE_BITSIZE (innermode)); max_bitsize = MAX (GET_MODE_BITSIZE (outermode), GET_MODE_BITSIZE (innermode)); --- a/gcc/cse.c +++ b/gcc/cse.c @@ -2336,15 +2336,23 @@ hash_rtx_cb (const_rtx x, enum machine_mode mode, + (unsigned int) INTVAL (x)); return hash; + case CONST_WIDE_INT: + { + int i; + for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++) + hash += CONST_WIDE_INT_ELT (x, i); + } + return hash; You can write "for (int i = 0; ..." now and remove the parentheses. --- a/gcc/cselib.c +++ b/gcc/cselib.c @@ -1121,15 +1120,23 @@ cselib_hash_rtx (rtx x, int create, enum machine_mode memmode) hash += ((unsigned) CONST_INT << 7) + INTVAL (x); return hash ? hash : (unsigned int) CONST_INT; + case CONST_WIDE_INT: + { + int i; + for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++) + hash += CONST_WIDE_INT_ELT (x, i); + } + return hash; + Likewise. --- a/gcc/expmed.c +++ b/gcc/expmed.c @@ -3298,23 +3268,13 @@ choose_multiplier (unsigned HOST_WIDE_INT d, int n, int precision, pow = n + lgup; pow2 = n + lgup - precision; - /* We could handle this with some effort, but this case is much - better handled directly with a scc insn, so rely on caller using - that. */ - gcc_assert (pow != HOST_BITS_PER_DOUBLE_INT); Why removing it? --- a/gcc/expr.c +++ b/gcc/expr.c @@ -722,64 +722,33 @@ convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x, int uns if (mode == oldmode) return x; - /* There is one case that we must handle specially: If we are converting - a CONST_INT into a mode whose size is twice HOST_BITS_PER_WIDE_INT and - we are to interpret the constant as unsigned, gen_lowpart will do - the wrong if the constant appears negative. What we want to do is - make the high-order word of the constant zero, not all ones. */ - - if (unsignedp && GET_MODE_CLASS (mode) == MODE_INT - && GET_MODE_BITSIZE (mode) == HOST_BITS_PER_DOUBLE_INT - && CONST_INT_P (x) && INTVAL (x) < 0) + if (CONST_SCALAR_INT_P (x) + && GET_MODE_CLASS (mode) == MODE_INT) On a single line. { - double_int val = double_int::from_uhwi (INTVAL (x)); - - /* We need to zero extend VAL. */ - if (oldmode != VOIDmode) - val = val.zext (GET_MODE_BITSIZE (oldmode)); - - return immed_double_int_const (val, mode); + /* If the caller did not tell us the old mode, then there is + not much to do with respect to canonization. We have to assume + that all the bits are significant. */ + if (GET_MODE_CLASS (oldmode) != MODE_INT) + oldmode = MAX_MODE_INT; + wide_int w = wide_int::from (std::make_pair (x, oldmode), + GET_MODE_PRECISION (mode), + unsignedp ? UNSIGNED : SIGNED); + return immed_wide_int_const (w, mode); canonicalization? @@ -5301,10 +5271,10 @@ store_expr (tree exp, rtx target, int call_param_p, bool nontemporal) &alt_rtl); } - /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not - the same as that of TARGET, adjust the constant. This is needed, for - example, in case it is a CONST_DOUBLE and we want only a word-sized - value. */ + /* If TEMP is a VOIDmode constant and the mode of the type of EXP is + not the same as that of TARGET, adjust the constant. This is + needed, for example, in case it is a CONST_DOUBLE or + CONST_WIDE_INT and we want only a word-sized value. */ if (CONSTANT_P (temp) && GET_MODE (temp) == VOIDmode && TREE_CODE (exp) != ERROR_MARK && GET_MODE (target) != TYPE_MODE (TREE_TYPE (exp))) Why reformatting the whole comment? @@ -9481,11 +9459,19 @@ expand_expr_real_1 (tree exp, rtx target, enum machine_mode tmode, return decl_rtl; case INTEGER_CST: - temp = immed_double_const (TREE_INT_CST_LOW (exp), - TREE_INT_CST_HIGH (exp), mode); - - return temp; - + { + tree type = TREE_TYPE (exp); Redundant, see the beginning of the function. + /* One could argue that GET_MODE_PRECISION (TYPE_MODE (type)) + should always be the same as TYPE_PRECISION (type). + However, it is not. Since we are converting from tree to + rtl, we have to expose this ugly truth here. */ + temp = immed_wide_int_const (wide_int::from + (exp, + GET_MODE_PRECISION (TYPE_MODE (type)), + TYPE_SIGN (type)), + TYPE_MODE (type)); + return temp; + } I don't really see how one could argue that, given that there are much fewer modes than possible type precisions, so please rewrite the comment, e.g.: "Given that TYPE_PRECISION (type) is not always equal to GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former to the latter according to the signedness of the type". What about a fast track where the precisions are indeed equal? @@ -11145,8 +11131,8 @@ const_vector_from_tree (tree exp) RTVEC_ELT (v, i) = CONST_FIXED_FROM_FIXED_VALUE (TREE_FIXED_CST (elt), inner); else - RTVEC_ELT (v, i) = immed_double_int_const (tree_to_double_int (elt), - inner); + RTVEC_ELT (v, i) + = immed_wide_int_const (elt, TYPE_MODE (TREE_TYPE (elt))); } return gen_rtx_CONST_VECTOR (mode, v); Why replacing inner? --- a/gcc/machmode.def +++ b/gcc/machmode.def @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */ UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */ UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */ +/* Should be overridden by EXTRA_MODES_FILE if wrong. */ +#define MAX_BITS_PER_UNIT 8 + What is it for? It's not documented at all. --- a/gcc/print-rtl.c +++ b/gcc/print-rtl.c @@ -617,6 +617,12 @@ print_rtx (const_rtx in_rtx) fprintf (outfile, " [%s]", s); } break; + + case CONST_WIDE_INT: + if (! flag_simple) + fprintf (outfile, " "); + cwi_output_hex (outfile, in_rtx); + break; #endif Remove the if (! flag_simple) test. --- a/gcc/recog.c +++ b/gcc/recog.c /* Returns 1 if OP is an operand that is a constant integer or constant - floating-point number. */ + floating-point number of MODE. */ + +int +const_double_operand (rtx op, enum machine_mode mode) +{ + return (GET_CODE (op) == CONST_DOUBLE) + && (GET_MODE (op) == mode || mode == VOIDmode); +} Can CONST_DOUBLEs still represent constant integers? diff --git a/gcc/rtl.def b/gcc/rtl.def index 15a997b..a76b28b 100644 --- a/gcc/rtl.def +++ b/gcc/rtl.def @@ -345,6 +345,9 @@ DEF_RTL_EXPR(TRAP_IF, "trap_if", "ee", RTX_EXTRA) /* numeric integer constant */ DEF_RTL_EXPR(CONST_INT, "const_int", "w", RTX_CONST_OBJ) +/* numeric integer constant */ +DEF_RTL_EXPR(CONST_WIDE_INT, "const_wide_int", "", RTX_CONST_OBJ) + I think that we should consider using a new letter here, for example W. "" is reserved to very specific cases. --- a/gcc/rtl.h +++ b/gcc/rtl.h @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_RTL_H #define GCC_RTL_H +#include Ouch. Can't this really be avoided?