From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x133.google.com (mail-il1-x133.google.com [IPv6:2607:f8b0:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 438D6386183A for ; Thu, 9 Nov 2023 15:03:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 438D6386183A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 438D6386183A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::133 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699542214; cv=none; b=qnTJzmCasoFqIRalraUd32d1I4jB6FEP6FVhpJftEq1XEhTom4HGcSi0fQoWA/pAQCT3510qukAxHNtXxlo5ZeO7lNeR9NbqW7Tip6lkAhr9z4yYotQ8vv9gulW6iOM2Kgp7u+bf2u+9LZnUEuqPKX7c37w0wnktHw54SlayFq4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699542214; c=relaxed/simple; bh=i1W4RMZ/O9XDhDmylzhjjL1duxLOD8Tl0wsrJCnClZk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=P9I8CKmuK7e54u1DAvUAcD0PcpSqQ9pmcdeOxq58cHKRPO6JZXRjFNQ/nimQfvd9/vdQEWKz0IWBvTOMqtX6M3CvcbpM2Ryy8ZM1KSR6iiI3+9YiNFm4ZvuE+b39YbNoE0P41CGjq9X6rDnT8ysiTW3pRainV/Uy62vTOJjGGHw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x133.google.com with SMTP id e9e14a558f8ab-35743e88193so3838045ab.3 for ; Thu, 09 Nov 2023 07:03:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699542203; x=1700147003; darn=gcc.gnu.org; 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=5gmaDi1fRlLZj/wEzDW0h0ToKZCzsg5kgxjh/gKWgRY=; b=kaYHnRMjpbJDG0xs6aBWb3vDvtorXo+jkJyxql9rW0RlXI9n9QwfjRENpYpDHYzHci RMJ6AM6oZwNHuFotpVEbz4pT3fm5M6MFpANuElk2pmJNIsFzzgbeKNGTr8jejDZpFAcQ ZSJqht88QfjLx9fpSNzjRxUhO+sJ41i+erA62M2YPfk0AXcfD4ghOVm5JtO03fBG74tT 9/FDMlIUhK4dLBUTXBOxltKDdw8xlh18V46DrX5tzI0221ZZ9BeIxwyJnhJgyAmzSJOh x0jwzX5/T4k+js31mmYmIw+CDqBdKgD/NDDYNRyn6luSPlmgCgfy6gO87vln4EhbqyHC DcLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699542203; x=1700147003; 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=5gmaDi1fRlLZj/wEzDW0h0ToKZCzsg5kgxjh/gKWgRY=; b=r5fx283fV9+gCPJenS50o0Henlo0m0KYoQ3tnLjZvDQh5RLCP6ktuF9/k9zyVDb6x/ agF/QvVgRscvfwRq0leVw1PRiZg6PDGEViNEXuMuClti9aW/a6CRLWRDh5FuEwb3KuC9 bWmF0CkD/JGyJ1Rfgm0CyKyGfIxYGrrPRNnwuI5/vNdpozwuxyskAur9DaYx/JBjIWsJ mtElwM+O7esk7EWE68F3VLZtlLNWQmy+X3FDiTK9B6rcapFo2MQJn5iUs6GH329s7hKh 6FUTLgqSOyHQ3xFJkmHJe91HZO5L/HPqJhf1iCJlmBtas38euMMY1U/zNu/dfrowejwC Raig== X-Gm-Message-State: AOJu0YxIFr2E0enVAN3hWDGNNwNOAVQwaq390/+SJIaA82/uBwpdg197 thyDwmqrAcZ4FKZcg+U2h16pFPoMSMX21w== X-Google-Smtp-Source: AGHT+IGsPbgs72YMGS8JumTpAJDdv4rdvwUYK4N5JPffgqbDQI7DMUoEaqnLzct5kzxXejVZcF6Ojg== X-Received: by 2002:a92:6e12:0:b0:359:4066:a2e7 with SMTP id j18-20020a926e12000000b003594066a2e7mr5254137ilc.0.1699542202729; Thu, 09 Nov 2023 07:03:22 -0800 (PST) Received: from [172.31.0.109] ([136.36.130.248]) by smtp.gmail.com with ESMTPSA id q10-20020a02c8ca000000b0042b10d42c90sm3754352jao.113.2023.11.09.07.03.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Nov 2023 07:03:22 -0800 (PST) Message-ID: <236780fb-a6cc-4e56-b490-0e44e56d5b06@gmail.com> Date: Thu, 9 Nov 2023 08:03:01 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [committed] RISC-V: Fix INSN costing and more zicond tests Content-Language: en-US To: "Maciej W. Rozycki" 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.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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/9/23 07:33, Maciej W. Rozycki wrote: > On Fri, 29 Sep 2023, Jeff Law wrote: > >> So this ends up looking a lot like the bits that I had to revert several weeks >> ago :-) >> >> The core issue we have is given an INSN the generic code will cost the SET_SRC >> and SET_DEST and sum them. But that's far from ideal on a RISC target. >> >> For a register destination, the cost can be determined be looking at just the >> SET_SRC. Which is precisely what this patch does. When the outer code is an >> INSN and we're presented with a SET we take one of two paths. >> >> If the destination is a register, then we recurse just on the SET_SRC and >> we're done. Otherwise we fall back to the existing code which sums the cost >> of the SET_SRC and SET_DEST. That fallback path isn't great and probably >> could be further improved (just costing SET_DEST in that case is probably >> quite reasonable). > > So this actually breaks insn costing for if-conversion, causing all > conditional-move expansions to count as 1 insn regardless of how many > there actually are. This can be easily verified by using various > `-mbranch-cost=' settings. > > Before your change you had to set the branch cost to higher than or equal > to the replacement insn count for if-conversion to trigger. Of course > tuning microarchitectures will have preset this hopefully correctly for > their needs, so normally you don't need to resort to `-mbranch-cost='. > With this change in place only setting `-mbranch-cost=1' will prevent > if-conversion from triggering, which is taking the situation back to GCC > 13 days, where `movMODEcc' patterns were always cost at 1. > > In preparation for an upcoming set of changes I have written numerous > testsuite cases to verify this insn costing to work correctly and now that > I have rebased for the submission all indicate the costing went wrong and > `movMODEcc' sequences of up to 6 insns are all now cost at 1 total. I was > going to post the patch series Fri-Mon, but this seems like a showstopper > to me, because if-conversion now triggers even when the conditional-move > (or for that matter conditional-add, as I have it handled too) sequence is > more expensive than a branched one. > > E.g. the NE operation costs 4 instructions for Zicond: > > sub a1,a0,a1 > czero.eqz a2,a2,a1 > czero.nez a1,a3,a1 > or a0,a2,a1 > ret > > while the branched equivalent costs (branch + 1) instructions: > > beq a0,a1,.L3 > mv a0,a2 > ret > .L3: > mv a0,a3 > ret > > so I'd expect if-conversion only to trigger at `-mbranch-cost=3' or higher > (just as my test cases verify), but now it triggers at `-mbranch-cost=2' > already. > > Can we have the insn costing reverted to correct calculation? What needs to happen is that code needs to be extended, not reverted. Many codes have to be synthesized based on the condition and the true/false arms. That's not currently accounted for. jeff