From patchwork Wed Oct 13 12:47:56 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julian Brown X-Patchwork-Id: 67679 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]) by ozlabs.org (Postfix) with SMTP id C2814B70AA for ; Wed, 13 Oct 2010 23:48:13 +1100 (EST) Received: (qmail 28639 invoked by alias); 13 Oct 2010 12:48:11 -0000 Received: (qmail 28627 invoked by uid 22791); 13 Oct 2010 12:48:10 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 13 Oct 2010 12:48:05 +0000 Received: (qmail 2776 invoked from network); 13 Oct 2010 12:48:03 -0000 Received: from unknown (HELO rex.config) (julian@127.0.0.2) by mail.codesourcery.com with ESMTPA; 13 Oct 2010 12:48:03 -0000 Date: Wed, 13 Oct 2010 13:47:56 +0100 From: Julian Brown To: gcc-patches@gcc.gnu.org Cc: paul@codesourcery.com, richard.sandiford@linaro.org Subject: [PATCH, ARM] Fix Thumb-1 reload ICE with nested functions Message-ID: <20101013134756.193154f7@rex.config> Mime-Version: 1.0 X-IsSubscribed: yes 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 Hi, This patch fixes (fixed) several Fortran tests which ICE'd on 4.4 series compilers (with an "unable to find a register to spill in class 'HI_REGS'" error). Those same tests no longer fail with current mainline and I don't have another test case which fails on mainline at present, but I believe the patch still constitutes a correct change, so should be applied. The problem is related to nested functions rather than Fortran itself, though the testsuite for the latter happened to be where the problem showed up. The problem was originally described in the following mail (but has showed up independently in the meantime): http://gcc.gnu.org/ml/gcc-patches/2008-04/msg02033.html I've based the fix on Richard Sandiford's suggestion in the followup: http://gcc.gnu.org/ml/gcc-patches/2008-05/msg00013.html To recap (*): r12 is referenced as a hard register early in RTL generation, in an instruction which requires reloading. High registers are marked fixed, so are generally unavailable to reload -- but reload chooses to attempt to use a high register for the reload anyway. Now: the thumb1_movsi_insn pattern contains a constraint "..., *lhk". The scope of the '*' modifier is just the following character: during register preferencing, GCC chooses HI_REGS (h) for that alternative. I believe the intention was for the '*' modifier to cover the whole constraint, but this must be written "*l*h*k". This is the first part of the fix. (Other Thumb-1 instructions refer to "*lk", i.e. LO_REGS or the stack pointer, but ignoring LO_REGS for preferencing purposes. Those might be wrong too, but I've left them alone.) The second problem is that only one register class is chosen for each constraint when reloading. The "lhk" combination doesn't correspond to any existing class, because the likely contender (CORE_REGS) also contains the soft frame pointer. Richard S. claims that is unnecessary: removing the soft frame pointer from the class doesn't seem to cause any issues (at least for gcc, g++ & libstdc++ for -mthumb). If this change is omitted, the ICE in the Fortran tests still occurs (HI_REGS is still chosen for the reload). Tested with cross to ARM Linux (gcc, g++, libstdc++, gfortran) with no notable change in results. OK to apply? Thanks, Julian (*) This description was written whilst I had a working test case for the failure. ChangeLog gcc/ * config/arm/arm.h (REG_CLASS_CONTENTS): Remove soft frame pointer from CORE_REGS and GENERAL_REGS classes. * config/arm/arm.md (*thumb1_movsi_insn): Ignore all parts of final constraint for register preferencing. Index: gcc/config/arm/arm.h =================================================================== --- gcc/config/arm/arm.h (revision 165177) +++ gcc/config/arm/arm.h (working copy) @@ -1245,8 +1245,8 @@ enum reg_class { 0x0000DF00, 0x00000000, 0x00000000, 0x00000000 }, /* HI_REGS */ \ { 0x01000000, 0x00000000, 0x00000000, 0x00000000 }, /* CC_REG */ \ { 0x00000000, 0x00000000, 0x00000000, 0x80000000 }, /* VFPCC_REG */ \ - { 0x0200DFFF, 0x00000000, 0x00000000, 0x00000000 }, /* GENERAL_REGS */ \ - { 0x0200FFFF, 0x00000000, 0x00000000, 0x00000000 }, /* CORE_REGS */ \ + { 0x0000DFFF, 0x00000000, 0x00000000, 0x00000000 }, /* GENERAL_REGS */ \ + { 0x0000FFFF, 0x00000000, 0x00000000, 0x00000000 }, /* CORE_REGS */ \ { 0xFAFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x7FFFFFFF } /* ALL_REGS */ \ } Index: gcc/config/arm/arm.md =================================================================== --- gcc/config/arm/arm.md (revision 165177) +++ gcc/config/arm/arm.md (working copy) @@ -5163,8 +5163,8 @@ ) (define_insn "*thumb1_movsi_insn" - [(set (match_operand:SI 0 "nonimmediate_operand" "=l,l,l,l,l,>,l, m,*lhk") - (match_operand:SI 1 "general_operand" "l, I,J,K,>,l,mi,l,*lhk"))] + [(set (match_operand:SI 0 "nonimmediate_operand" "=l,l,l,l,l,>,l, m,*l*h*k") + (match_operand:SI 1 "general_operand" "l, I,J,K,>,l,mi,l,*l*h*k"))] "TARGET_THUMB1 && ( register_operand (operands[0], SImode) || register_operand (operands[1], SImode))"