From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29491 invoked by alias); 3 May 2013 11:28:24 -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 29479 invoked by uid 89); 3 May 2013 11:28:23 -0000 X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-we0-f176.google.com (HELO mail-we0-f176.google.com) (74.125.82.176) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 03 May 2013 11:28:22 +0000 Received: by mail-we0-f176.google.com with SMTP id r6so1189030wey.21 for ; Fri, 03 May 2013 04:28:20 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.194.58.78 with SMTP id o14mr13204560wjq.39.1367580500825; Fri, 03 May 2013 04:28:20 -0700 (PDT) Received: by 10.194.56.100 with HTTP; Fri, 3 May 2013 04:28:20 -0700 (PDT) In-Reply-To: <87ip3bncsk.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> References: <506C72C7.7090207@naturalbridge.com> <50891AAC.8090301@naturalbridge.com> <87y5im3orb.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87pq3y3kyk.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <50912D85.1070002@naturalbridge.com> <5091331C.3030504@naturalbridge.com> <512D686B.90000@naturalbridge.com> <515EC4E7.7040907@naturalbridge.com> <871ua0qer8.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87y5c8nhz2.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87fvygngsw.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87txmwm0r9.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87ip3bncsk.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> Date: Fri, 03 May 2013 11:28:00 -0000 Message-ID: Subject: Re: patch to fix constant math -5th patch, rtl From: Richard Biener To: Richard Biener , Kenneth Zadeck , Mike Stump , gcc-patches , Lawrence Crowl , Ian Lance Taylor , rdsandiford@googlemail.com Content-Type: text/plain; charset=ISO-8859-1 X-SW-Source: 2013-05/txt/msg00111.txt.bz2 On Wed, Apr 24, 2013 at 5:55 PM, Richard Sandiford wrote: > Richard Biener writes: >> On Wed, Apr 24, 2013 at 5:00 PM, Richard Sandiford >> wrote: >>> Richard Biener writes: >>>> On Wed, Apr 24, 2013 at 4:29 PM, Richard Sandiford >>>> wrote: >>>>> In other words, one of the reasons wide_int can't be exactly 1:1 >>>>> in practice is because it is clearing out these mistakes (GEN_INT >>>>> rather than gen_int_mode) and missing features (non-power-of-2 widths). >>>> >>>> Note that the argument should be about CONST_WIDE_INT here, >>>> not wide-int. Indeed CONST_WIDE_INT has the desired feature >>>> and can be properly truncated/extended according to mode at the time >>>> we build it >>>> via immed_wide_int_cst (w, mode). I don't see the requirement that >>>> wide-int itself is automagically providing that truncation/extension >>>> (though it is a possibility, one that does not match existing behavior of >>>> HWI for CONST_INT or double-int for CONST_DOUBLE). >>> >>> I agree it doesn't match the existing behaviour of HWI for CONST_INT or >>> double-int for CONST_DOUBLE, but I think that's very much a good thing. >>> The model for HWIs at the moment is that you have to truncate results >>> to the canonical form after every operation where it matters. As you >>> proved in your earlier message about the plus_constant bug, that's easily >>> forgotten. I don't think the rtl code is doing all CONST_INT arithmetic >>> on full HWIs because it wants to: it's doing it because that's the way >>> C/C++ arithmetic on primitive types works. In other words, the current >>> CONST_INT code is trying to emulate N-bit arithmetic (for gcc runtime N) >>> using a single primitive integer type. wide_int gives us N-bit arithmetic >>> directly; no emulation is needed. >> >> Ok, so what wide-int provides is integer values encoded in 'len' HWI >> words that fit in 'precision' or more bits (and often in less). wide-int >> also provides N-bit arithmetic operations. IMHO both are tied >> too closely together. A give constant doesn't really have a precision. > > I disagree. All rtl objects have a precision. REGs, MEMs, SYMBOL_REFs, > LABEL_REFs and CONSTs all have precisions, and the last three are > run-time constants. Why should CONST_INT and CONST_DOUBLE be different? Well - they _are_ different. They don't even have a mode at the moment. If you want to change that be my guest (or well, it's unfortunate to lose the sharing then) - but Kenny always repeats that this is "impossible to fix". Having CONST_INT and CONST_DOUBLE without a precision but CONST_WIDE_INT with a precision would be at least odd. > See e.g. the hoops that cselib has to jump through: > > /* We need to pass down the mode of constants through the hash table > functions. For that purpose, wrap them in a CONST of the appropriate > mode. */ > static rtx > wrap_constant (enum machine_mode mode, rtx x) > { > if ((!CONST_SCALAR_INT_P (x)) && GET_CODE (x) != CONST_FIXED) > return x; > gcc_assert (mode != VOIDmode); > return gen_rtx_CONST (mode, x); > } > > That is, cselib locally converts (const_int X) into (const:M (const_int X)), > purely so that it doesn't lose track of the CONST_INT's mode. > (const:M (const_int ...)) is invalid rtl elsewhere, but a necessary > hack here all the same. Indeed ugly. But I wonder why cselib needs to store constants in hashtables at all ... they should be VALUEs themselves. So the fix for the above might not necessarily be to assign the CONST_INT a mode (not that CONST_WIDE_INT would fix the above). >> What RTL currently has looks better to me - operations have >> explicitely specified precisions. > > But that isn't enough to determine the precision of all operands. > A classic case is ZERO_EXTEND. Something like: > > (zero_extend:DI (reg:SI X)) > > is unambiguous. But if you substitute (reg:SI X) with a CONST_INT, > the result becomes ambiguous. E.g. we could end up with: > > (zero_extend:DI (const_int -1)) > > The ZERO_EXTEND operand still has SImode, but that fact is not explicit > in the rtl, and is certainly not explicit in the ZERO_EXTEND operation. > So if we just see the result above, we no longer know whether the result > should be (const_int 0xff), (const_int 0xffff), or what. The same goes for: That situation only occurs when you have "unfolded" RTX. You should have never generated the above (and hopefully the RTL verifier doesn't allow it), but instead called sth like simplify_gen_zero_extend (DImode, SImode, x); with x being the constant substituted for X. It's probably unfortunate that parts of the RTL machinery work like (if I remember correctly) for_each_rtx (x, replace-interesting-stuff); simplify (x); > (zero_extend:DI (const_int 256)) > > where (const_int 0) and (const_int 256) are both potential results. > > It's not just ZERO_EXTEND. E.g.: > > (zero_extract:SI ...) > > tells you that an SImode value is being extracted, but it doesn't tell > you what precision you're extracting from. So for: > > (zero_extract:SI (const_int -1) (const_int X) (const_int 3)) > > how many 1 bits should be the result have? Because of the sign-extension > canonicalisation, the answer depends on the precision of the (const_int -1), > which has now been lost. If instead CONST_INTs were stored in zero-extended > form, the same ambiguity would apply to SIGN_EXTRACT. > > This sort of thing has been a constant headache in rtl. I can't stress > how much I feel it is _not_ better than recording the precision of > the constant :-) Ok, so please then make all CONST_INTs and CONST_DOUBLEs have a mode! The solution is not to have a CONST_WIDE_INT (again with VOIDmode and no precision in the RTX object(!)) and only have wide_int have a precision. So, the proper order to fix things is to get CONST_INTs and CONST_DOUBLEs to have a mode. And to make CONST_WIDE_INT inherit that property. Richard. > Richard