From patchwork Sat Nov 19 09:15:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 23240 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp626276wrr; Sat, 19 Nov 2022 01:17:52 -0800 (PST) X-Google-Smtp-Source: AA0mqf4WSMGjTFGXG3W6Xj2PKS6tmxqoxSKAnHret4piVp5EII8hCd0eiDQdMzOzAPWSwi+j0Ud2 X-Received: by 2002:a05:6402:4286:b0:458:7489:34ea with SMTP id g6-20020a056402428600b00458748934eamr9554014edc.264.1668849472642; Sat, 19 Nov 2022 01:17:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668849472; cv=none; d=google.com; s=arc-20160816; b=diGjH8uc7pJw7Wkq6Kuk4RfsyPU2xyKfFanMPkHXCPj4SEkzIfwEf7VGNtteOUelwH 4IN0VxrxrbuNXRhEEuKEyhV1JoWtyo5U30jU6/CrysZXAZUl3LI9VFXwLWwTsFFjiW/+ nEiaEdMarOo3zxewGvXX1L9Xwzd2JO1btmDMoY/1/7FAnF7g7Ae33e+YdcSbrj2eXWuH 7FaS7jkzfHuavBi3sDt8ngqyWJBYU2nyagoONCBcV/cutI6J5CH05r7QjDkkf4rx7Upq I9k6viUd/KmkILR8+BUpGKxP3HQMXOeQfH7InaYdp2jrp673Evis/rdKRTCHMMwHPrwb 4Q5g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:message-id:subject:cc:to:date :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=MLsGYi+wymE2qsFwXDDiHk7BgiQ5lIFnKb+3Ps91UUI=; b=zdN+fiVmZ6oS2aWVdYIWAtVboYZcnuxRX60yMoemKQJ/h6771VK4KX6EJHr9uupMs3 KbxmwaCTGjjM73K5KddiOAF2Cafzh5rrQZu8YRwXE5bAVVtlNqyQDFYptSgQHq9pPHW/ 0wxp3RKYlQkCd7NTIIfnJjg3rtJZ/wcUm6Eul9pJ67oZ5510sajyV3hSjMt9292bLso8 h8YRAk+Y7iIh9WmAt5v6EmbmPpWfzsYvnYUNJgYC7jw2mK90wD6G86GtzxyFV6DGPNYT pQWjdlCxs+W2CKWmNk4tFJCgMvz6RG7I4a7BlbaTZHd72pqBc7zQ5oO3jRp13Abht0l8 8HPQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=ZjIpLS3V; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id tc14-20020a1709078d0e00b0078ddf8926d2si4348585ejc.827.2022.11.19.01.17.52 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 19 Nov 2022 01:17:52 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=ZjIpLS3V; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 835C43858280 for ; Sat, 19 Nov 2022 09:17:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 835C43858280 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1668849470; bh=MLsGYi+wymE2qsFwXDDiHk7BgiQ5lIFnKb+3Ps91UUI=; h=Date:To:Cc:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=ZjIpLS3VymB7H9lpM4tDzcMmOYp1JZiWIaVL1LqHgbfbSfRPBEs6eQhKfA2mSVU4R UNfseCLznciExQ+4n7iOjhkE+WJi/NE+3ClNJLBm9zJ1/JjZrqdPkUNNPPTtRFxaaw Onfv3kHn+/Qvsyw2Jwc26OV+U9qgPvb+cdCF/9QI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id BBA4F3858D1E for ; Sat, 19 Nov 2022 09:16:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BBA4F3858D1E Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-34--i-wq1tRMOekgfYLO6sVXw-1; Sat, 19 Nov 2022 04:15:56 -0500 X-MC-Unique: -i-wq1tRMOekgfYLO6sVXw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9C047101A528; Sat, 19 Nov 2022 09:15:56 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 58EDBC50925; Sat, 19 Nov 2022 09:15:56 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 2AJ9FpLD3761204 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 19 Nov 2022 10:15:51 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 2AJ9Foi43761203; Sat, 19 Nov 2022 10:15:50 +0100 Date: Sat, 19 Nov 2022 10:15:50 +0100 To: Richard Biener , Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] reg-stack: Fix a -fcompare-debug bug in reg-stack [PR107183] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-15.1 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749915504699481790?= X-GMAIL-MSGID: =?utf-8?q?1749915504699481790?= Hi! As the following testcase shows, the swap_rtx_condition function in reg-stack can result in different code generation between -g and -g0. The function is doing the changes as it goes, so does analysis and changes together, which makes it harder to deal with DEBUG_INSNs, where normally analysis phase ignores them and the later phase doesn't. swap_rtx_condition walks instructions two different ways, one is using next_flags_user function which stops on non-call instructions that mention the flags register, and the other is a loop on fnstsw where it stops on instructions mentioning it and tries to find sahf instruction that uses it (in both cases calls stop it and so does end of basic block). Now both of these currently stop on DEBUG_INSNs that mention the flags register resp. the fnstsw result register. On success the function recurses on next flags user instruction if still live and if the recursion failed, reverts the changes it did too and fails. If it were just for the next_flags_user case, the fix could be just not doing INSN_CODE (insn) = -1; if (recog_memoized (insn) == -1) fail = 1; on DEBUG_INSNs (assuming all changes to those are fine), swap_rtx_condition_1 just changes one comparison to a different one. But due to the possibility of fnstsw result being used in theory before sahf in some DEBUG_INSNs, this patch takes a different approach. swap_rtx_condition has now a new argument and two modes. The first mode is when debug_seen is >= 0, in this case both next_flags_user and the loop for fnstsw -> sahf will ignore but note DEBUG_INSNs (that mention flags register or fnstsw result). If no such DEBUG_INSN is found during the whole call including recursive invocations (so e.g. for -g0 but probably most often for -g as well), it behaves as before, if it returns true all the changes are done and nothing further needs to be done later. If any DEBUG_INSNs are seen along the way, even when returning success all the changes are reverted, so it just reports that the function would be successful if DEBUG_INSNs were ignored. In this case, compare_for_stack_reg needs to call it again in debug_seen = -1 mode, which tells the function to update everything including DEBUG_INSNs. For the fnstsw -> sahf case which I hope will be very rare I just reset the DEBUG_INSNs, I don't really know how to express it easily otherwise. For the rest swap_rtx_condition_1 is done even on the DEBUG_INSNs. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? And after some time for release branches too? 2022-11-19 Jakub Jelinek PR target/107183 * reg-stack.cc (next_flags_user): Add DEBUG_SEEN argument. If >= 0 and a DEBUG_INSN would be otherwise returned, set DEBUG_SEEN to 1 and ignore it. (swap_rtx_condition): Add DEBUG_SEEN argument. In >= 0 mode only set DEBUG_SEEN to 1 if problematic DEBUG_ISNSs were seen and revert all changes on success in that case. Don't try to recog_memoized DEBUG_INSNs. (compare_for_stack_reg): Adjust swap_rtx_condition caller. If it returns true and debug_seen is 1, call swap_rtx_condition again with debug_seen -1. * gcc.dg/ubsan/pr107183.c: New test. Jakub --- gcc/reg-stack.cc.jj 2022-09-14 12:21:39.594178160 +0200 +++ gcc/reg-stack.cc 2022-11-18 16:54:43.090381887 +0100 @@ -263,14 +263,14 @@ static void swap_to_top (rtx_insn *, sta static bool move_for_stack_reg (rtx_insn *, stack_ptr, rtx); static bool move_nan_for_stack_reg (rtx_insn *, stack_ptr, rtx); static int swap_rtx_condition_1 (rtx); -static int swap_rtx_condition (rtx_insn *); +static int swap_rtx_condition (rtx_insn *, int &); static void compare_for_stack_reg (rtx_insn *, stack_ptr, rtx, bool); static bool subst_stack_regs_pat (rtx_insn *, stack_ptr, rtx); static void subst_asm_stack_regs (rtx_insn *, stack_ptr); static bool subst_stack_regs (rtx_insn *, stack_ptr); static void change_stack (rtx_insn *, stack_ptr, stack_ptr, enum emit_where); static void print_stack (FILE *, stack_ptr); -static rtx_insn *next_flags_user (rtx_insn *); +static rtx_insn *next_flags_user (rtx_insn *, int &); /* Return nonzero if any stack register is mentioned somewhere within PAT. */ @@ -336,7 +336,7 @@ stack_regs_mentioned (const_rtx insn) static rtx ix86_flags_rtx; static rtx_insn * -next_flags_user (rtx_insn *insn) +next_flags_user (rtx_insn *insn, int &debug_seen) { /* Search forward looking for the first use of this value. Stop at block boundaries. */ @@ -346,7 +346,14 @@ next_flags_user (rtx_insn *insn) insn = NEXT_INSN (insn); if (INSN_P (insn) && reg_mentioned_p (ix86_flags_rtx, PATTERN (insn))) - return insn; + { + if (DEBUG_INSN_P (insn) && debug_seen >= 0) + { + debug_seen = 1; + continue; + } + return insn; + } if (CALL_P (insn)) return NULL; @@ -1248,8 +1255,22 @@ swap_rtx_condition_1 (rtx pat) return r; } +/* This function swaps condition in cc users and returns true + if successful. It is invoked in 2 different modes, one with + DEBUG_SEEN set initially to 0. In this mode, next_flags_user + will skip DEBUG_INSNs that it would otherwise return and just + sets DEBUG_SEEN to 1 in that case. If DEBUG_SEEN is 0 at + the end of toplevel swap_rtx_condition which returns true, + it means no problematic DEBUG_INSNs were seen and all changes + have been applied. If it returns true but DEBUG_SEEN is 1, + it means some problematic DEBUG_INSNs were seen and no changes + have been applied so far. In that case one needs to call + swap_rtx_condition again with DEBUG_SEEN set to -1, in which + case it doesn't skip DEBUG_INSNs, but instead adjusts the + flags related condition in them or resets them as needed. */ + static int -swap_rtx_condition (rtx_insn *insn) +swap_rtx_condition (rtx_insn *insn, int &debug_seen) { rtx pat = PATTERN (insn); @@ -1259,7 +1280,7 @@ swap_rtx_condition (rtx_insn *insn) && REG_P (SET_DEST (pat)) && REGNO (SET_DEST (pat)) == FLAGS_REG) { - insn = next_flags_user (insn); + insn = next_flags_user (insn, debug_seen); if (insn == NULL_RTX) return 0; pat = PATTERN (insn); @@ -1281,7 +1302,18 @@ swap_rtx_condition (rtx_insn *insn) { insn = NEXT_INSN (insn); if (INSN_P (insn) && reg_mentioned_p (dest, insn)) - break; + { + if (DEBUG_INSN_P (insn)) + { + if (debug_seen >= 0) + debug_seen = 1; + else + /* Reset the DEBUG insn otherwise. */ + INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC (); + continue; + } + break; + } if (CALL_P (insn)) return 0; } @@ -1301,7 +1333,7 @@ swap_rtx_condition (rtx_insn *insn) return 0; /* Now we are prepared to handle this. */ - insn = next_flags_user (insn); + insn = next_flags_user (insn, debug_seen); if (insn == NULL_RTX) return 0; pat = PATTERN (insn); @@ -1310,23 +1342,25 @@ swap_rtx_condition (rtx_insn *insn) if (swap_rtx_condition_1 (pat)) { int fail = 0; - INSN_CODE (insn) = -1; - if (recog_memoized (insn) == -1) - fail = 1; - /* In case the flags don't die here, recurse to try fix - following user too. */ - else if (! dead_or_set_p (insn, ix86_flags_rtx)) + if (DEBUG_INSN_P (insn)) + gcc_assert (debug_seen < 0); + else { - insn = next_flags_user (insn); - if (!insn || !swap_rtx_condition (insn)) + INSN_CODE (insn) = -1; + if (recog_memoized (insn) == -1) fail = 1; } - if (fail) + /* In case the flags don't die here, recurse to try fix + following user too. */ + if (!fail && !dead_or_set_p (insn, ix86_flags_rtx)) { - swap_rtx_condition_1 (pat); - return 0; + insn = next_flags_user (insn, debug_seen); + if (!insn || !swap_rtx_condition (insn, debug_seen)) + fail = 1; } - return 1; + if (fail || debug_seen == 1) + swap_rtx_condition_1 (pat); + return !fail; } return 0; } @@ -1345,6 +1379,7 @@ compare_for_stack_reg (rtx_insn *insn, s { rtx *src1, *src2; rtx src1_note, src2_note; + int debug_seen = 0; src1 = get_true_reg (&XEXP (pat_src, 0)); src2 = get_true_reg (&XEXP (pat_src, 1)); @@ -1354,8 +1389,17 @@ compare_for_stack_reg (rtx_insn *insn, s if ((! STACK_REG_P (*src1) || (STACK_REG_P (*src2) && get_hard_regnum (regstack, *src2) == FIRST_STACK_REG)) - && swap_rtx_condition (insn)) + && swap_rtx_condition (insn, debug_seen)) { + /* If swap_rtx_condition succeeded but some debug insns + were seen along the way, it has actually reverted all the + changes. Rerun swap_rtx_condition in a mode where DEBUG_ISNSs + will be adjusted as well. */ + if (debug_seen) + { + debug_seen = -1; + swap_rtx_condition (insn, debug_seen); + } std::swap (XEXP (pat_src, 0), XEXP (pat_src, 1)); src1 = get_true_reg (&XEXP (pat_src, 0)); --- gcc/testsuite/gcc.dg/ubsan/pr107183.c.jj 2022-11-18 17:07:04.441096505 +0100 +++ gcc/testsuite/gcc.dg/ubsan/pr107183.c 2022-11-18 17:06:37.471470688 +0100 @@ -0,0 +1,12 @@ +/* PR target/107183 */ +/* { dg-do compile } */ +/* { dg-options "-O -fsanitize=float-cast-overflow -fcompare-debug" } */ + +long double a, b, c; + +int +foo (void) +{ + unsigned u = b || __builtin_rintl (c); + return u + (unsigned) a; +}