From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2521 invoked by alias); 30 Apr 2012 15:37:44 -0000 Received: (qmail 2344 invoked by uid 22791); 30 Apr 2012 15:37:40 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mo-p00-ob.rzone.de (HELO mo-p00-ob.rzone.de) (81.169.146.161) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 30 Apr 2012 15:37:26 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT2k715jHQaJercGOZE+TiTS5oCa8h49Png== X-RZG-CLASS-ID: mo00 Received: from [192.168.2.100] (dslb-084-058-251-010.pools.arcor-ip.net [84.58.251.10]) by smtp.strato.de (josoe mo80) (RZmta 28.16 DYNA|AUTH) with ESMTPA id L0285co3UF8k9b ; Mon, 30 Apr 2012 17:37:22 +0200 (CEST) Message-ID: <4F9EB161.2090506@gjlay.de> Date: Mon, 30 Apr 2012 15:37:00 -0000 From: Georg-Johann Lay User-Agent: Mozilla Thunderbird 1.0.7 (Windows/20050923) MIME-Version: 1.0 To: Richard Earnshaw CC: Jim MacArthur , "gcc-patches@gcc.gnu.org" , rdsandiford@googlemail.com Subject: Re: [patch] More thorough checking in reg_fits_class_p References: <4F994B99.1010606@arm.com> <4F9E9772.8080700@arm.com> <4F9EA258.5030006@arm.com> In-Reply-To: <4F9EA258.5030006@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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 X-SW-Source: 2012-04/txt/msg01888.txt.bz2 Richard Earnshaw schrieb: > On 30/04/12 15:07, Richard Sandiford wrote: > >>Richard Earnshaw writes: >> >>> Jim MacArthur wrote: >>> >>>>The current code in reg_fits_class_p appears to be incorrect; since >>>>offset may be negative, it's necessary to check both ends of the range >>>>otherwise an array overrun or underrun may occur when calling >>>>in_hard_reg_set_p. in_hard_reg_set_p should also be checked for each >>>>register in the range of regno .. regno+offset. >>>> >>>>A negative offset can occur on a big-endian target. For example, on >>>>AArch64, subreg_regno_offset (0, DImode, 0, TImode) returns -1. >>>> >>>>We discovered this problem while developing unrelated code for >>>>big-endian support in the AArch64 back end. >>>> >>>>I've tested this with an x86 bootstrap which shows no errors, and with >>>>our own AArch64 back end. >>>> >>>>Ok for trunk? >>>> >>>>gcc/Changelog entry: >>>> >>>>2012-04-26 Jim MacArthur >>>> * recog.c (reg_fits_class_p): Check every register between regno and >>>> regno+offset is in the hard register set. >>> >>>OK. >>> >>>R. >>> >>>>reg-fits-class-9 >>>> >>>>diff --git a/gcc/recog.c b/gcc/recog.c >>>>index 8fb96a0..825bfb1 100644 >>>>--- a/gcc/recog.c >>>>+++ b/gcc/recog.c >>>>@@ -2759,14 +2759,28 @@ bool >>>> reg_fits_class_p (const_rtx operand, reg_class_t cl, int offset, >>>> enum machine_mode mode) >>>> { >>>>- int regno = REGNO (operand); >>>>+ unsigned int regno = REGNO (operand); >>>> >>>> if (cl == NO_REGS) >>>> return false; >>>> >>>>- return (HARD_REGISTER_NUM_P (regno) >>>>- && in_hard_reg_set_p (reg_class_contents[(int) cl], >>>>- mode, regno + offset)); >>>>+ /* We should not assume all registers in the range regno to regno + offset are >>>>+ valid just because each end of the range is. */ >>>>+ if (HARD_REGISTER_NUM_P (regno) && HARD_REGISTER_NUM_P (regno + offset)) >>>>+ { >>>>+ unsigned int i; >>>>+ >>>>+ unsigned int start = MIN (regno, regno + offset); >>>>+ unsigned int end = MAX (regno, regno + offset); >>>>+ for (i = start; i <= end; i++) >>>>+ { >>>>+ if (!in_hard_reg_set_p (reg_class_contents[(int) cl], >>>>+ mode, i)) >>>>+ return false; >>>>+ } >> >>This doesn't look right to me. We should still only need to check >>in_hard_reg_set_p for one register number. I'd have expected >>something like: >> >> return (HARD_REGISTER_NUM_P (regno) >> && HARD_REGISTER_NUM_P (regno + offset) >> && in_hard_reg_set_p (reg_class_contents[(int) cl], >> mode, regno + offset)); >> >>instead. >> >>Richard > > Hmm, looking at the comment that precedes the function makes me think > the actual implementation should be: > > { > int regno = REGNO (operand) + offset; > ... > > return (HARD_REGISTER_NUM_P (regno) > && HARD_REGISTER_NUM_P (end_hard_regno (regno, mode)) > && in_hard_reg_set_p (...., regno); > > } > > That is, the original regno isn't really interesting and what really > counts is the range regno + offset ... regno + offset + > NUM_HARD_REGS(mode) - 1 Shouldn't this be HARD_REGNO_NREGS? Johann