From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11110 invoked by alias); 10 Jun 2011 09:17:30 -0000 Received: (qmail 11101 invoked by uid 22791); 10 Jun 2011 09:17:30 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,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; Fri, 10 Jun 2011 09:17:10 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT2k715jHQaJercGObUOFkj18odoYNahU4Q== X-RZG-CLASS-ID: mo00 Received: from [192.168.0.22] (business-188-111-022-002.static.arcor-ip.net [188.111.22.2]) by post.strato.de (klopstock mo44) (RZmta 25.18) with ESMTPA id j00296n5A8me4G ; Fri, 10 Jun 2011 11:09:34 +0200 (MEST) Message-ID: <4DF1DF4E.3030800@gjlay.de> Date: Fri, 10 Jun 2011 09:27:00 -0000 From: Georg-Johann Lay User-Agent: Thunderbird 2.0.0.24 (X11/20100302) MIME-Version: 1.0 To: Denis Chertykov CC: gcc-patches@gcc.gnu.org, Anatoly Sokolov , "Eric B. Weddington" Subject: Re: [Patch, AVR]: Fix PR46779 References: <4DF0FAB5.6070704@gjlay.de> In-Reply-To: Content-Type: multipart/mixed; boundary="------------050902030007070004030800" 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: 2011-06/txt/msg00812.txt.bz2 This is a multi-part message in MIME format. --------------050902030007070004030800 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-length: 2236 Denis Chertykov schrieb: > 2011/6/9 Georg-Johann Lay : >> This is a tentative patch to fix PR46779 and hopefully also related >> issues like PR45291. >> > - /* Disallow QImode in stack pointer regs. */ > - if ((regno == REG_SP || regno == (REG_SP + 1)) && mode == QImode) > + /* Don't allocate data to non-GENERAL_REGS registers. */ > + > + if (regno >= 32) > return 0; > > I think that we not need in this code. > GCC core must bother about this. > > + > + if (GET_MODE_SIZE (mode) == 1) > return 1; > > I'm agree with this. > > + > + /* Disallow big registers that overlap the frame pointer. > + This will hopefully reduce the number of spill failures. */ > + > + if (GET_MODE_SIZE (mode) > 2 > + && regno <= REG_Y > + && regno + GET_MODE_SIZE (mode) >= REG_Y + 1) > + { > + return 0; > + } > > Fragment from GCC info: > -------------------------------------- > HARD_REGNO_MODE_OK (regno, mode)A C expression that is nonzero if it > is permissible to store a value of mode mode in hard register number > regno (or in several registers starting with that one). For a machine > where all registers are equivalent, a suitable definition is > > #define HARD_REGNO_MODE_OK(REGNO, MODE) 1 > > You need not include code to check for the numbers of fixed registers, > because the allocation mechanism considers them to be always occupied. > ----------------------------------------- > Again, GCC core must bother about this. > > - /* Otherwise disallow all regno/mode combinations that span r28:r29. */ > - if (regno <= (REG_Y + 1) && (regno + GET_MODE_SIZE (mode)) >= (REG_Y + 1)) > - return 0; > - > - if (mode == QImode) > - return 1; > - > - /* Modes larger than QImode occupy consecutive registers. */ > - if (regno + GET_MODE_SIZE (mode) > FIRST_PSEUDO_REGISTER) > - return 0; > - > > This is a right change. > > Denis. So the patch turns avr_hard_regno_mode_ok into plain vanilla... Johann -- PR target/46779 * config/avr/avr.c (avr_hard_regno_mode_ok): Rewrite. In particular, allow 8-bit values in r28 and r29. (avr_hard_regno_scratch_ok): Disallow any register that might be part of the frame pointer. (avr_hard_regno_rename_ok): Same. --------------050902030007070004030800 Content-Type: text/x-patch; name="pr46779.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="pr46779.diff" Content-length: 2999 Index: config/avr/avr.c =================================================================== --- config/avr/avr.c (Revision 174701) +++ config/avr/avr.c (Arbeitskopie) @@ -6276,26 +6276,20 @@ jump_over_one_insn_p (rtx insn, rtx dest int avr_hard_regno_mode_ok (int regno, enum machine_mode mode) { - /* Disallow QImode in stack pointer regs. */ - if ((regno == REG_SP || regno == (REG_SP + 1)) && mode == QImode) - return 0; - - /* The only thing that can go into registers r28:r29 is a Pmode. */ - if (regno == REG_Y && mode == Pmode) - return 1; - - /* Otherwise disallow all regno/mode combinations that span r28:r29. */ - if (regno <= (REG_Y + 1) && (regno + GET_MODE_SIZE (mode)) >= (REG_Y + 1)) - return 0; - - if (mode == QImode) + /* Any GENERAL_REGS register can hold 8-bit values. */ + /* FIXME: + 8-bit values must not be disallowed for R28 or R29. Disallowing + QI et al. in these registers might lead to code like + (set (subreg:QI (reg:HI 28)) ...) + which will result in wrong code because reload does not handle + SUBREGs of hard regsisters like this. This could be fixed in reload. + However, it appears that fixing reload is not wanted by reload people. */ + + if (GET_MODE_SIZE (mode) == 1) return 1; - - /* Modes larger than QImode occupy consecutive registers. */ - if (regno + GET_MODE_SIZE (mode) > FIRST_PSEUDO_REGISTER) - return 0; - - /* All modes larger than QImode should start in an even register. */ + + /* All modes larger than 8 bits should start in an even register. */ + return !(regno & 1); } @@ -6422,13 +6416,23 @@ avr_hard_regno_scratch_ok (unsigned int && !df_regs_ever_live_p (regno)) return false; + /* Don't allow hard registers that might be part of the frame pointer. + Some places in the compiler just test for [HARD_]FRAME_POINTER_REGNUM + and don't care for a frame pointer that spans more than one register. */ + + if ((!reload_completed || frame_pointer_needed) + && (regno == REG_Y || regno == REG_Y + 1)) + { + return false; + } + return true; } /* Return nonzero if register OLD_REG can be renamed to register NEW_REG. */ int -avr_hard_regno_rename_ok (unsigned int old_reg ATTRIBUTE_UNUSED, +avr_hard_regno_rename_ok (unsigned int old_reg, unsigned int new_reg) { /* Interrupt functions can only use registers that have already been @@ -6439,6 +6443,17 @@ avr_hard_regno_rename_ok (unsigned int o && !df_regs_ever_live_p (new_reg)) return 0; + /* Don't allow hard registers that might be part of the frame pointer. + Some places in the compiler just test for [HARD_]FRAME_POINTER_REGNUM + and don't care for a frame pointer that spans more than one register. */ + + if ((!reload_completed || frame_pointer_needed) + && (old_reg == REG_Y || old_reg == REG_Y + 1 + || new_reg == REG_Y || new_reg == REG_Y + 1)) + { + return 0; + } + return 1; } --------------050902030007070004030800--