From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x12a.google.com (mail-il1-x12a.google.com [IPv6:2607:f8b0:4864:20::12a]) by sourceware.org (Postfix) with ESMTPS id D48DF3858D3C for ; Thu, 9 Nov 2023 15:47:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D48DF3858D3C 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 D48DF3858D3C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::12a ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699544834; cv=none; b=sYpeoCZOtklM4smPBwh9sTHKGK+G2qNI7TOHXhIZWMhmy5NaqttQofYtfyJ/mfIOYe571SvBmFWVt04yfT5QZbwSAYBYin6tVtzgZ11L0G4WbSlz1oECOFKgNcPrnhS96Wx1O/JzTVLW9STi6SJYVkr75f24zcbqB+vVHVZf5lI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699544834; c=relaxed/simple; bh=wfceGvrgj05U6tVtx27Ew2R3hfRLBqTyVBMGCgvW7/Q=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=vskp3SGoNqoTanP7wrcls3Z4B6EieRg5kusvvSR5AW/s2lIRdjidnCBBV/voHmslnG9Oa8cnIuOrWBl85NlV6M9ToHtFB5uNDV6IQTaEOKEs78J3hdazIMhRfc1f7m4NRLRCiskezKcuAXpaAuJSwVOcGLI99YWqWdBvPHgAyFc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-il1-x12a.google.com with SMTP id e9e14a558f8ab-359d796abd6so885845ab.0 for ; Thu, 09 Nov 2023 07:47:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1699544832; x=1700149632; 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=72KW6eYj840UuUH3753qbWPCmSz1EkGZQ8FyECGc1rc=; b=gOO0CqqREmEOBmO5DQbPCUWXXp8Zm/N7Mg78Lzjwlr3DAncMQJAbpzLmvMVaehtZRc C8dR4q5XOH2BAP6VMngclK1BWH73rRQjr8MWwuduAjMxlk6mGOcWtKIXWDKgYEkVw2l/ Mo+jrxcRrIO5usU5eZrh6HHhIHxp+Aa0oZterdFZ/2WboUsttNbWfUO6PDtsVMl7mmco 10BN4joqxcbJdHcE7Prdc3P07BUUioS8jX2OD9KyapR20E3albmBW5TbDJYo8FBjPP5m 3Uxnx5NBYnloLmDCXJ0aNmGE2VDeGQs/M+yQitacPubRrn7z9vBBWQLjQhVZ20ft5393 ibCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699544832; x=1700149632; 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=72KW6eYj840UuUH3753qbWPCmSz1EkGZQ8FyECGc1rc=; b=a4zLQ20hXz/Yd10xj10oiB3HcFKG+c8zpMjPFBD5EKgYgUDhViaNO6AxTvG031f5ua Qq2hZFBGUgqz4CqWSsly20zXIfAGPeVrl57G9Q+LzsLK7j31+bdksg1Q7+wlrokLXxEa Qw3OT7GU/Wc0pLx7kn0ZaoA7fJGAb0RPAoAZIhNyOcH9FPumtNeSBJBV6a/G49hszBDN nJAwG1D/RaTvXomUjGhJBizaZx4gWadXxqehVoHuSXyeMGH9pvVYKikvS9Xpi3TDCW25 r5EPxqhah+U0xgm3veUJZNSq1nixuzfQ5Kt41bDGpUZ00etW4pnaZkKT3HgCK5F3DuNF TPkA== X-Gm-Message-State: AOJu0YxOQrNz2elkzsEAC5kXetYNKmG028vz6rbaehRNNnX7r/OwcKsM QTbPSFJlvtpzrqqlei7ZS9s= X-Google-Smtp-Source: AGHT+IHnrYPuTOcOEmYwhsQK8dGXk+FhSvBii28PS1uRNHmW48ZyefkBy8FuRDJzkaKewZoxiDIHfA== X-Received: by 2002:a92:c54d:0:b0:359:3c20:4a89 with SMTP id a13-20020a92c54d000000b003593c204a89mr9890278ilj.5.1699544831903; Thu, 09 Nov 2023 07:47:11 -0800 (PST) Received: from [172.31.0.109] ([136.36.130.248]) by smtp.gmail.com with ESMTPSA id p12-20020a92d48c000000b00357e53527afsm4504990ilg.61.2023.11.09.07.47.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Nov 2023 07:47:09 -0800 (PST) Message-ID: <0e68aec8-b075-49d9-bc6c-afac72195529@gmail.com> Date: Thu, 9 Nov 2023 08:47:08 -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? FYI, I've opened a bug for this issue so it doesn't get lost. I don't think the extensions are terribly hard. It's really a matter of deciding if we can re-use any of the logic from the expander or if we just mirror its logic and keep the expander and costing in sync. Jeff > > Maciej