From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id 1EAAC39730E2 for ; Sun, 20 Nov 2022 15:01:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1EAAC39730E2 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-x42a.google.com with SMTP id v28so9149421pfi.12 for ; Sun, 20 Nov 2022 07:01:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=gmzArt3q/GYDg2wy1A0dnEiMGMMkn0IipUo7AhtI0cM=; b=CzSFejjyODA2lj+zCIbMySYAgrwkXXFLS6IWBZSmvEXBtfvRF6nd6O0ph4+X4m26ZW MWB0fzGA4/qRH6SgaNCu3m/I76eAX9sX2mr//nXEwkPsbWIaUuWcwjPPZ39tuU02BLl7 gcXke3llTia/2ZdsxXBAZWRgmOXxGRSRNlViKmXW/uswasDfnP018jVJ4p4oZmkWLcYN yiBqi1jb/Jv2tfoiYM05rXGkizVcJvc5qIWiodsvj6+RM/0ElbZ2gE9cukQjd/xO0yQC sPNlh1ZMLnWDlOnVZ0S84uHrVWIgeXV0Dl5s6u3IAKw8GqzfJfPzcOePooaVzeAIa1rG 03IA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gmzArt3q/GYDg2wy1A0dnEiMGMMkn0IipUo7AhtI0cM=; b=rA2YRHgSxnJUd/jFJ0gOiwopmmH650ztAxYvhKooMLddSYx3FVMiXOKoL6Z6BDZIxC RIkxNLWrHzrw5vgsT2QebNPxVWfhUAIt/jFh8y8cR9gpKq/iL184pyZIQlTpPNAGzW8Q 7D06I0t4e1S9pWZP6MlSRWeT6G4STGIr7BqNln9qJefcuYuCShEjvAonSeVbYDwFJMb9 wypafKwk4KNaUgTrALAAnBKoGY7G9gfwZzfYZ89hwKEJPK6JT8pfOY+u4JjGfaBaZqpY xcH6kVid1JY/+vqXdAsCjrOo3p/ylG2pNmPHXKBO2WUy9ff+5XvBU4AkJVhCoFEAcBgN mMdg== X-Gm-Message-State: ANoB5pkdWDXbs4FP3v75XARqfZgT89s8BYOnTuAgvFt1j/7n4P/w4dag 50ovLMsBiYvJXEVX2UcWuxo= X-Google-Smtp-Source: AA0mqf4GSy/cr+w+RAh9ULcNDDtw5iLC68YHIhs1Z3NnPb71f2E5oWGwPnqfZ1EQa7azaVf2LntD2g== X-Received: by 2002:a62:140a:0:b0:572:84a4:d797 with SMTP id 10-20020a62140a000000b0057284a4d797mr16281606pfu.42.1668956497583; Sun, 20 Nov 2022 07:01:37 -0800 (PST) Received: from ?IPV6:2601:681:8600:13d0::f0a? ([2601:681:8600:13d0::f0a]) by smtp.gmail.com with ESMTPSA id b10-20020a170902650a00b0016d773aae60sm2288849plk.19.2022.11.20.07.01.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 20 Nov 2022 07:01:36 -0800 (PST) Message-ID: <85ca38be-5e3b-2671-e45d-5f2f86912e9c@gmail.com> Date: Sun, 20 Nov 2022 08:01:35 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH] reg-stack: Fix a -fcompare-debug bug in reg-stack [PR107183] Content-Language: en-US To: Jakub Jelinek , Richard Biener Cc: gcc-patches@gcc.gnu.org References: From: Jeff Law In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/19/22 02:15, Jakub Jelinek wrote: > 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. OK jeff