From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 4AEDF1581EE for ; Fri, 21 Mar 2025 16:23:56 +0000 (UTC) Received: from lists.gentoo.org (bobolink.gentoo.org [140.211.166.189]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: relay-lists.gentoo.org@gentoo.org) by smtp.gentoo.org (Postfix) with ESMTPSA id 379DA34317F for ; Fri, 21 Mar 2025 16:23:56 +0000 (UTC) Received: from bobolink.gentoo.org (localhost [127.0.0.1]) by bobolink.gentoo.org (Postfix) with ESMTP id 31CB21104AB; Fri, 21 Mar 2025 16:23:55 +0000 (UTC) Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bobolink.gentoo.org (Postfix) with ESMTPS id 2574E1104AB for ; Fri, 21 Mar 2025 16:23:55 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id C8E3F34317F for ; Fri, 21 Mar 2025 16:23:54 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id 1B24E1C96 for ; Fri, 21 Mar 2025 16:23:53 +0000 (UTC) From: "Sam James" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Sam James" Message-ID: <1742574215.346097d6dbc0fe0fc42cacf029a18204f0a54415.sam@gentoo> Subject: [gentoo-commits] proj/gcc-patches:master commit in: 15.0.0/gentoo/ X-VCS-Repository: proj/gcc-patches X-VCS-Files: 15.0.0/gentoo/78_all_PR118615.patch X-VCS-Directories: 15.0.0/gentoo/ X-VCS-Committer: sam X-VCS-Committer-Name: Sam James X-VCS-Revision: 346097d6dbc0fe0fc42cacf029a18204f0a54415 X-VCS-Branch: master Date: Fri, 21 Mar 2025 16:23:53 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-Archives-Salt: be2eeac0-e878-45ad-b10a-5f9fb3996814 X-Archives-Hash: ac118c5a29357ee7210120d79ba9d395 commit: 346097d6dbc0fe0fc42cacf029a18204f0a54415 Author: Sam James gentoo org> AuthorDate: Fri Mar 21 16:23:35 2025 +0000 Commit: Sam James gentoo org> CommitDate: Fri Mar 21 16:23:35 2025 +0000 URL: https://gitweb.gentoo.org/proj/gcc-patches.git/commit/?id=346097d6 15.0.0: update 78_all_PR118615.patch (cosmetic) This just adds the changelog etc. Signed-off-by: Sam James gentoo.org> 15.0.0/gentoo/78_all_PR118615.patch | 97 ++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/15.0.0/gentoo/78_all_PR118615.patch b/15.0.0/gentoo/78_all_PR118615.patch index 9ae8200..bea0c22 100644 --- a/15.0.0/gentoo/78_all_PR118615.patch +++ b/15.0.0/gentoo/78_all_PR118615.patch @@ -1,3 +1,94 @@ +https://inbox.sourceware.org/gcc-patches/Z91i4oSOe2t9Wsu5@tucnak/ + +Date: Fri, 21 Mar 2025 14:00:18 +0100 +From: Jakub Jelinek +To: Vladimir Makarov , Jeff Law , + Richard Sandiford +Cc: gcc-patches@gcc.gnu.org, Surya Kumari Jangala , + Peter Bergner +Subject: [PATCH] lra, v2: emit caller-save register spills before call insn + [PR116028] +Message-ID: +Reply-To: Jakub Jelinek +MIME-Version: 1.0 +X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 +X-Mimecast-Spam-Score: 0 +X-Mimecast-MFC-PROC-ID: _tXP__7Gkm9pvYzp53m4LpOJ23nLH9dr5dUlzvuS5ts_1742562026 +X-Mimecast-Originator: redhat.com +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00,DKIM_INVALID,DKIM_SIGNED,KAM_DMARC_QUARANTINE,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_W,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 +X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org +List-Id: + +Hi! + +Here is an updated version of Surya's PR116028 fix from August, which got +reverted because it caused bootstrap failures on aarch64, later on bootstrap +comparison errors there as well and problems on other targets as well. + +The changes compared to the +https://gcc.gnu.org/pipermail/gcc-patches/2024-August/658973.html +version are: +1) the reason for aarch64 miscompilations and later on bootstrap comparison + issues as can be seen on the pr118615.c testcase in the patch was that + when curr_insn is a JUMP_INSN or some cases of CALL_INSNs, + split_if_necessary is called with before_p true and if it is successful, + the code set use_insn = PREV_INSN (curr_insn); instead of use_insn = + curr_insn; and that use_insn is then what is passed to + add_next_usage_insn; now, if the patch decides to emit the save + instruction(s) before the first call after curr_insn in the ebb rather + than before the JUMP_INSN/CALL_INSN, PREV_INSN (curr_insn) is some random + insn before it, not anything related to the split_reg actions. + If it is e.g. a DEBUG_INSN in one case vs. some unrelated other insn + otherwise, that can affect further split_reg within the same function +2) as suggested by Surya in PR118615, it makes no sense to try to change + behavior if the first call after curr_insn is in the same bb as curr_insn +3) split_reg is actually called sometimes from within inherit_in_ebb but + sometimes from elsewhere; trying to use whatever last call to + inherit_in_ebb saw last is a sure way to run into wrong-code issues, + so instead of clearing the rtx var at the start of inherit_in_ebb it is + now cleared at the end of it +4) calling the var latest_call_insn was weird, inherit_in_ebb walks the ebb + backwards, so what the var contains is the first call insn within the + ebb (after curr_insn) +5) the patch was using + lra_process_new_insns (PREV_INSN (latest_call_insn), NULL, save, + "Add save<-reg"); + to emit the save insn before latest_call_insn. That feels quite weird + given that latest_call_insn has explicit support for adding stuff + before some insn or after some insn, adding something before some + insn doesn't really need to be done as addition after PREV_INSN +6) some formatting nits + new testcase + removal of xfail even on arm32 + +Bootstrapped/regtested on x86_64-linux/i686-linux (my usual +--enable-checking=yes,rtl,extra builds), aarch64-linux (normal default +bootstrap) and our distro scratch build +({x86_64,i686,aarch64,powerpc64le,s390x}-linux --enable-checking=release +LTO profiledbootstrap/regtest), I think Sam James tested on 32-bit arm +too. +On aarch64-linux this results in +-FAIL: gcc.dg/pr10474.c scan-rtl-dump pro_and_epilogue "Performing shrink-wrapping" + +I admit I don't know the code well nor understood everything it is doing. + +I have some concerns: +1) I wonder if there is a guarantee that first_call_insn if non-NULL will be + always in between curr_insn and usage_insn when call_save_p; I'd hope + yes because if usage_insn is before first_call_insn in the ebb, + presumably it wouldn't need to find call save regs because the range + wouldn't cross any calls +2) I wonder whether it wouldn't be better instead of inserting the saves + before first_call_insn insert it at the start of the bb containing that + call (after labels of course); emitting it right before a call could + mislead code looking for argument slot initialization of the call +3) even when avoiding the use_insn = PREV_INSN (curr_insn);, I wonder + if it is ok to use use_insn equal to curr_insn rather than the insns + far later where we actually inserted it, but primarily because I don't + understand the code much; I think for the !before_p case it is doing + similar thing on a shorter distance, the saves were emitted after + curr_insn and we record it on curr_insn + 2025-03-21 Surya Kumari Jangala Jakub Jelinek @@ -26,7 +117,7 @@ + or NULL outside of that function. */ +static rtx_insn *first_call_insn; - + @@ -6373,12 +6376,26 @@ split_reg (bool before_p, int original_r lra_process_new_insns (as_a (usage_insn), @@ -147,3 +238,7 @@ + } + baz (&n); +} + + Jakub + +