From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18783 invoked by alias); 7 Mar 2018 23:04: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 18106 invoked by uid 89); 7 Mar 2018 23:04:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=logically X-HELO: foss.arm.com Received: from usa-sjc-mx-foss1.foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Mar 2018 23:04:32 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4D1C1435; Wed, 7 Mar 2018 15:04:29 -0800 (PST) Received: from localhost (unknown [10.32.99.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BAC983F53D; Wed, 7 Mar 2018 15:04:28 -0800 (PST) From: Richard Sandiford To: Martin Sebor Mail-Followup-To: Martin Sebor ,Jeff Law , Jakub Jelinek , Gcc Patch List , richard.sandiford@arm.com Cc: Jeff Law , Jakub Jelinek , Gcc Patch List Subject: Re: [PATCH] fix ICE in generic_overlap (PR 84526) References: <20180223201302.GJ5867@tucnak> <63d4e234-627f-c713-202c-1aac7ba32b2f@gmail.com> Date: Wed, 07 Mar 2018 23:04:00 -0000 In-Reply-To: (Martin Sebor's message of "Wed, 7 Mar 2018 15:52:39 -0700") Message-ID: <87po4fmqrt.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-03/txt/msg00360.txt.bz2 Martin Sebor writes: > @@ -409,23 +412,33 @@ builtin_memref::set_base_and_offset (tree expr) > base = get_inner_reference (expr, &bitsize, &bitpos, &var_off, > &mode, &sign, &reverse, &vol); > > + /* get_inner_reference is not expected to return null. */ > + gcc_assert (base != NULL); > + > poly_int64 bytepos = exact_div (bitpos, BITS_PER_UNIT); > > - HOST_WIDE_INT const_off; > - if (!base || !bytepos.is_constant (&const_off)) > + /* Convert the poly_int64 offset to to offset_int. The offset > + should be constant but be prepared for it not to be just in > + case. */ I know it's just repeating what I said in the other reply, but I think we should drop this comment. It doesn't add anything and it will quickly become out of date once the frontend supports variable-length types. > + offset_int cstoff; > + if (bytepos.is_constant (&cstoff)) Also, same comment about this being less efficient than using the natural type of HOST_WIDE_INT. I think this and... > { > - base = get_base_address (TREE_OPERAND (expr, 0)); > - return; > + offrange[0] += cstoff; > + offrange[1] += cstoff; > + > + /* Besides the reference saved above, also stash the offset > + for validation. */ > + if (TREE_CODE (expr) == COMPONENT_REF) > + refoff = cstoff; > } > + else > + offrange[1] += maxobjsize; > > - offrange[0] += const_off; > - offrange[1] += const_off; > - > if (var_off) > { > if (TREE_CODE (var_off) == INTEGER_CST) > { > - offset_int cstoff = wi::to_offset (var_off); > + cstoff = wi::to_offset (var_off); > offrange[0] += cstoff; > offrange[1] += cstoff; ...this are logically separate variables, so there's no need for them to have the same type and name. Richard