From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) by sourceware.org (Postfix) with ESMTPS id A970A3858C36 for ; Thu, 9 Nov 2023 14:33:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A970A3858C36 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A970A3858C36 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::232 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699540434; cv=none; b=k/uRVz5hiBlpU9GVTxE/MDKA6RwgARGkF2ERGg2GzAgHx4LUxYxkESbEPEG1I//P5twI77i+19vTS0LBTE+5rsoXNevUOeWv3uo/70jFnfq6ZK1f+Exjgy6Y+WpDJAoK509KLD8cTl+E/8S8w7JYFXkO2mDLIyFyqlPODB5OEQQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699540434; c=relaxed/simple; bh=Xe+vN1EPNjwKsZmt5ImXvjJpv1wi7Ib5wb8VSiH/1Tg=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=LhZlexy6fx3Bg3rmlakg3KmtLK/s2EC0dTje4NsivSvLIocUucwUNFKkagcXnbKkGNHL+pu1b1kM9tcr4q1wothzvKxR79Fib2tHU0vvO6vCCavuuuauk5er2qwHaWcjVhmreTBQ6l+25ltNDLRpLo2oMljahGcLITXYY7HwVY8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x232.google.com with SMTP id 38308e7fff4ca-2c50cf61f6dso12098131fa.2 for ; Thu, 09 Nov 2023 06:33:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; t=1699540430; x=1700145230; darn=gcc.gnu.org; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=me7vAQ8zTGBsE8GRN6ZWobnPOubQzXD/z08viPfm0gs=; b=b80aKNxkQHmxGnvH2TA6HCmb6XEp34Q4rrLR9scbu0k0O78wBLDOpqrZ1yw4eElHFJ EdjF663qbKmqsJehKyGVIQX8qic99JT0kvoCPxVKNqSVqj/EKC8TsuOZNUt6LQw8uUla Y5wyfF3P5c0LdMAV54JyPPXC5gMdI4bvDP3QqURFvuZd0q4mUV9S/fC86zCyoeWy1E20 rR88ZrGehLTc3era3k/QcPqtBZ5MmwgM73rYiGYBjDCCirBQ8U0YSHtlBjuth5UWG7Td FQddjz0W2OPOSlGtGEP747bJGBoUmQGkuI6WahE3eaB992sTXS39xR7p4/s6E8T8u+rc rhKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699540430; x=1700145230; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=me7vAQ8zTGBsE8GRN6ZWobnPOubQzXD/z08viPfm0gs=; b=JejE4N5X3rCYLFOaJ/4ZCQ8fYzIQ789iRnuRD340VqV3doskccYNoOpUZjCIpux+Ya p7tkXam4Bti1XzlM6SLez2XYEgqVC9NtCUAJCNqGQNUYteBAazxbHoVWAXrg+KfSjyZ6 uX/BcPj2r8FwxEEUoI7FyZki+5WigzVDjlJR9d/AITJmSy5NRfKVfW/LAPRWBKq8OybC DvrKimzvubaZkVZxtC3zLYP6oreTqkjMLo5cGeTkvyeo+rMnghNXIb+N5av86Tg4ZW/0 2flAuA6SYs7j7kWeKthbm7znFNxMwPYz4YqlxqmNjMKbl7sWB9ZKiQkFgSeOfZ+OM0zY lRBg== X-Gm-Message-State: AOJu0YyJ+Zmu9gTIsm2Bcz+BqIUaDVo5L2wNf808eUYFtpnkRdlaD4o6 dAC4Z/Y9JHCY/q51dYaRNVXqew== X-Google-Smtp-Source: AGHT+IEX1rg/yzjg9CQbEGYLxZN6IDJU/65s6rGXnhyQqz8qBm/fF5qiD977kHY+KfwfWtO9du6eHQ== X-Received: by 2002:a05:651c:b26:b0:2c6:ed74:4055 with SMTP id b38-20020a05651c0b2600b002c6ed744055mr5211999ljr.6.1699540429980; Thu, 09 Nov 2023 06:33:49 -0800 (PST) Received: from tpp.orcam.me.uk (tpp.orcam.me.uk. [2001:8b0:154:0:ea6a:64ff:fe24:f2fc]) by smtp.gmail.com with ESMTPSA id j16-20020a05600c485000b004083bc9ac90sm2204808wmo.24.2023.11.09.06.33.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Nov 2023 06:33:49 -0800 (PST) Date: Thu, 9 Nov 2023 14:33:48 +0000 (GMT) From: "Maciej W. Rozycki" To: Jeff Law cc: gcc-patches@gcc.gnu.org Subject: Re: [committed] RISC-V: Fix INSN costing and more zicond tests In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 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? Maciej