From patchwork Tue Dec 13 22:05:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 87952 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp2445772qgi; Tue, 13 Dec 2016 14:06:35 -0800 (PST) X-Received: by 10.99.104.68 with SMTP id d65mr178352969pgc.52.1481666795215; Tue, 13 Dec 2016 14:06:35 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id t5si49469142pgj.143.2016.12.13.14.06.34 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Dec 2016 14:06:35 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-444339-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-444339-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-444339-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=ef5MOQXispw+4Pru le0CE69mGFDxylpvNdrp9yIhTceCq+fZrk1tE+JHXFaZw8tctYS4HO5mA5e35b2a LtraF9O6TBXlUJ8q6CPTdzzGxS+ysitzvsx0jTXiN+YNHGPN6ACihxBvGHL6SRNM oPIvwSiGprE4lFCqTgOJ//4o4Oo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=h+B2wCEfWVdA/7xFZSpxrf v2FWM=; b=lrR2k8WEhnWhQlD3ec1ZUy/eQdyIQ4nOYgFoeQaZlVsn30Ir/u3gHp ZUodiVXqdz2n4JLuDO2YwRkCupjctyMBYEGFai5NkYbKuASRwC6KSI3BB3RkBux3 xNl4JeUNZ0Thhtop6DDHnIFLdbYYTgDgvXYtCwiYaqonJY7/Xg0sk= Received: (qmail 73155 invoked by alias); 13 Dec 2016 22:06:04 -0000 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 Received: (qmail 73133 invoked by uid 89); 13 Dec 2016 22:06:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=typical, sk:ebotcaz, U*ebotcazou, ebotcazouadacorecom X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Dec 2016 22:05:53 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id C65D182051 for ; Tue, 13 Dec 2016 23:05:50 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 73mNNl0qQdTW for ; Tue, 13 Dec 2016 23:05:50 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 977A9812CF for ; Tue, 13 Dec 2016 23:05:50 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [LRA] Fix ICE for paradoxical subregs on strict-alignment platforms Date: Tue, 13 Dec 2016 23:05:50 +0100 Message-ID: <30870806.6LhnUrUFi2@polaris> User-Agent: KMail/4.14.10 (Linux/3.16.7-53-desktop; KDE/4.14.9; x86_64; ; ) MIME-Version: 1.0 Hi, the Ada runtime library doesn't build on SPARC 32-bit with LRA because of an ICE on a couple of files: eric@polaris:~/build/gcc/sparc-sun-solaris2.10> gcc/gnat1 -I gcc/ada/rts - quiet -dumpbase g-debpoo.adb -auxbase g-debpoo -O2 -gnatpg -fPIC -mlra - mcpu=v9 g-debpoo.adb -o g-debpoo.s +===========================GNAT BUG DETECTED==============================+ | 7.0.0 20161213 (experimental) [trunk revision 243595] (sparc-sun- solaris2.10) GCC error:| | in lra_set_insn_recog_data, at lra.c:965 | | Error detected around g-debpoo.adb:1896:8 The problem is that curr_insn_transform attempts to reload subregs before reloading addresses: if (! check_only_p) /* Make equivalence substitution and memory subreg elimination before address processing because an address legitimacy can depend on memory mode. */ Now, on strict-alignment platforms, simplify_operand_subreg has a special provision for paradoxical subregs of memory references: /* If we change the address for a paradoxical subreg of memory, the address might violate the necessary alignment or the access might be slow. So take this into consideration. We need not worry about accesses beyond allocated memory for paradoxical memory subregs as we don't substitute such equiv memory (see processing equivalences in function lra_constraints) and because for spilled pseudos we allocate stack memory enough for the biggest corresponding paradoxical subreg. */ if (!(MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (mode) && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg))) || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode) && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg)))) return true; That is to say, if the adjusted memory reference is not sufficient aligned, the routine will reload the original memory reference instead of the adjusted one on strict-alignment platforms. But there is a hitch: if the address of this original memory reference is invalid, the routine will nevertheless reload the entire memory reference, leading to the aforementioned ICE. The proposed fix is to reload the address, if it is invalid, just before reloading the memory reference in simplify_operand_subreg; this yields the correct sequence of reloads for the case I investigated. The patch also contains formatting fixes and adds a 'return' for the sake of clarity. Tested on x86-64/Linux and SPARC/Solaris with LRA, OK for the mainline? 2016-12-13 Eric Botcazou * lra-constraints.c (process_address): Add forward declaration. (simplify_operand_subreg): In the MEM case, if the adjusted memory reference is not sufficient aligned and the address was invalid, reload the address before reloading the original memory reference. Fix long lines and add a final return for the sake of clarity. -- Eric Botcazou Index: lra-constraints.c =================================================================== --- lra-constraints.c (revision 243595) +++ lra-constraints.c (working copy) @@ -1454,6 +1454,7 @@ insert_move_for_subreg (rtx_insn **befor } static int valid_address_p (machine_mode mode, rtx addr, addr_space_t as); +static bool process_address (int, bool, rtx_insn **, rtx_insn **); /* Make reloads for subreg in operand NOP with internal subreg mode REG_MODE, add new reloads for further processing. Return true if @@ -1480,13 +1481,13 @@ simplify_operand_subreg (int nop, machin type = curr_static_id->operand[nop].type; if (MEM_P (reg)) { - rtx subst; - + const bool addr_was_valid + = valid_address_p (innermode, XEXP (reg, 0), MEM_ADDR_SPACE (reg)); alter_subreg (curr_id->operand_loc[nop], false); - subst = *curr_id->operand_loc[nop]; + rtx subst = *curr_id->operand_loc[nop]; lra_assert (MEM_P (subst)); - if (! valid_address_p (innermode, XEXP (reg, 0), - MEM_ADDR_SPACE (reg)) + + if (!addr_was_valid || valid_address_p (GET_MODE (subst), XEXP (subst, 0), MEM_ADDR_SPACE (subst)) || ((get_constraint_type (lookup_constraint @@ -1503,10 +1504,10 @@ simplify_operand_subreg (int nop, machin ADDRESS, SCRATCH)][0]], MEM_ADDR_SPACE (subst)))) { - /* If we change address for paradoxical subreg of memory, the + /* If we change the address for a paradoxical subreg of memory, the address might violate the necessary alignment or the access might - be slow. So take this into consideration. We should not worry - about access beyond allocated memory for paradoxical memory + be slow. So take this into consideration. We need not worry + about accesses beyond allocated memory for paradoxical memory subregs as we don't substitute such equiv memory (see processing equivalences in function lra_constraints) and because for spilled pseudos we allocate stack memory enough for the biggest @@ -1517,28 +1518,36 @@ simplify_operand_subreg (int nop, machin && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg)))) return true; + *curr_id->operand_loc[nop] = operand; + + /* But if the address was not valid, we cannot reload the MEM without + reloading the address first. */ + if (!addr_was_valid) + process_address (nop, false, &before, &after); + /* INNERMODE is fast, MODE slow. Reload the mem in INNERMODE. */ enum reg_class rclass = (enum reg_class) targetm.preferred_reload_class (reg, ALL_REGS); - if (get_reload_reg (curr_static_id->operand[nop].type, innermode, reg, - rclass, TRUE, "slow mem", &new_reg)) + if (get_reload_reg (curr_static_id->operand[nop].type, innermode, + reg, rclass, TRUE, "slow mem", &new_reg)) { bool insert_before, insert_after; bitmap_set_bit (&lra_subreg_reload_pseudos, REGNO (new_reg)); insert_before = (type != OP_OUT - || GET_MODE_SIZE (innermode) > GET_MODE_SIZE (mode)); + || GET_MODE_SIZE (innermode) + > GET_MODE_SIZE (mode)); insert_after = type != OP_IN; insert_move_for_subreg (insert_before ? &before : NULL, insert_after ? &after : NULL, reg, new_reg); } - *curr_id->operand_loc[nop] = operand; SUBREG_REG (operand) = new_reg; /* Convert to MODE. */ reg = operand; - rclass = (enum reg_class) targetm.preferred_reload_class (reg, ALL_REGS); + rclass + = (enum reg_class) targetm.preferred_reload_class (reg, ALL_REGS); if (get_reload_reg (curr_static_id->operand[nop].type, mode, reg, rclass, TRUE, "slow mem", &new_reg)) { @@ -1561,6 +1570,7 @@ simplify_operand_subreg (int nop, machin the memory. Typical case is when the index scale should correspond the memory. */ *curr_id->operand_loc[nop] = operand; + return false; } else if (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER) {