From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com [IPv6:2a00:1450:4864:20::22e]) by sourceware.org (Postfix) with ESMTPS id C7E203865C11 for ; Wed, 22 May 2024 15:30:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C7E203865C11 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 C7E203865C11 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::22e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716391811; cv=none; b=tlSR4IZecxMMySpQJ4/v0BOpG4qnsPZX8Mg1m7p5H3J6uiR9TM8llN5U2D7AGDxmK2Vk6qd3wuTHMsa8tx+lS05D83dvfqRPJCoXCjtQlZq1P+WpAkLDN6Sbpe4CiPf1SlpZg1PcpE5e4mc6YgUKLc7e4KlzDYTpaBSq/YmgaMs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716391811; c=relaxed/simple; bh=rSjU7ahAuI0G8nFackds3/dayRTAuUy9MDqLFN0tUAY=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=t6Rgx7PfOQjlOMCQWXdepTa/S9uyPngv7Du9eIpSNhTAp2tUa8KptfnjJOLErqE7EbxjHbozlO5SZZiLVjUSca9JWMMmuYa5BGifJwYgonIAlAboWUSjTdF4zuMg3msYphCCf5oNr8C5V/R4g3V1wM9bFQeAN0iClrpBxG9WUwY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lj1-x22e.google.com with SMTP id 38308e7fff4ca-2e428242a38so96282691fa.2 for ; Wed, 22 May 2024 08:30:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716391805; x=1716996605; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=NqEEz0w/5d7VwzKpcFyoZGB3gu8h4ArxRmXYM7UK6Io=; b=edhh8It39QnBt2V91nfZgenYNelgvNWhluC9tpdKCZ9PZn4dkPfDaPN2Efjc85rhrR kmqlW8ZecgFRXQiRlDxQsRpKvQv+YfNvHRm4rDnTeQHAs0+SIe1sDZPyCUB/HLQoRDOi CKeuNANvGg54qH6vFR8DD3vYzZIwFyVRt2M5i+fJTk9fOmnB8aKSek2/EpHbU9rAhZ5U CF8+W+RPya3gVtcfRrcSmSZn50nspyxrYedgNdDRr9xdUvz8Xp2ROq/cvZYjjY8h1b69 LT3B5gKJGzTVRgdEvdRL55TttMAzFQV57p35auW9hSvB7XeRU/RgXFWAiq30KEb/a3tn Tsmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716391805; x=1716996605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NqEEz0w/5d7VwzKpcFyoZGB3gu8h4ArxRmXYM7UK6Io=; b=no1sdrjLwjS3agvHoEycV5hIkFzErW7l7AnFa95bC3IHgrPgGX3mvTrR4oyI+TkpR9 snk+PQ/sLOZn1COKQsuWjs8WlJh3muQcbLyHoSw4s4cTEpumiH+u0ozokCYGqR59U4lc iopuptIhgtxhHXKJfKmNPFRaV6emg3wqz2UASdcK84Buat3wIljUD/YWhBnk49sLjguc n+ENTLtFgFdbtdJaw0DLcxscbjAHIDq6IaDFOUNR+H2ycUX9wQMxYY9lYwa/EzhP6ftz 3z6B50UYVNlx7AiyJU9Mv54ApCoH6Gpct97esP7Hq8O/VpieJHoxGH8slFBRzevC+jxx 5JSw== X-Gm-Message-State: AOJu0YxX7rhSap5P0PWk+DUWis6rnsYVQWsSci6Kk2BYjVkYNgt937rY 9K3SVQm7UgNn9c+lexS7c+q6XlsvvkDpNt7j8OltOxAsXqDQSQ9tWPgIydVvVr2O/Z9o8u7ihpE lHE/PUdQQNaZCnwjZ5G77q0NXj1wZr1c96p4= X-Google-Smtp-Source: AGHT+IHxW8jvUkZk6m0nJvew9vyvpHrXGJpGwLNCo3z0f07ceSRQwrnkaO0b5vU2lf1P7awyVdnk/i4ArY55h3u5C6U= X-Received: by 2002:a2e:a385:0:b0:2e2:9842:a9d3 with SMTP id 38308e7fff4ca-2e949563f7cmr19665651fa.46.1716391804660; Wed, 22 May 2024 08:30:04 -0700 (PDT) MIME-Version: 1.0 References: <007701daac5a$d94d0af0$8be720d0$@nextmovesoftware.com> In-Reply-To: <007701daac5a$d94d0af0$8be720d0$@nextmovesoftware.com> From: Uros Bizjak Date: Wed, 22 May 2024 17:29:51 +0200 Message-ID: Subject: Re: [x86_64 PATCH] Correct insn_cost of movabsq. To: Roger Sayle Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,LOTS_OF_MONEY,MONEY_NOHTML,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 Wed, May 22, 2024 at 5:15=E2=80=AFPM Roger Sayle wrote: > > This single line patch fixes a strange quirk/glitch in i386's rtx_costs, > which considers an instruction loading a 64-bit constant to be significan= tly > cheaper than loading a 32-bit (or smaller) constant. > > Consider the two functions: > unsigned long long foo() { return 0x0123456789abcdefULL; } > unsigned int bar() { return 10; } > > and the corresponding lines from combine's dump file: > insn_cost 1 for #: r98:DI=3D0x123456789abcdef > insn_cost 4 for #: ax:SI=3D0xa > > The same issue can be seen in -dP assembler output. > movabsq $81985529216486895, %rax # 5 [c=3D1 l=3D10] *movdi_interna= l/4 > > The problem is that pattern_costs interpretation of rtx_costs contains > "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for > example a register or small immediate constant) is considered special, > and equivalent to a single instruction, but all other values are treated > as verbatim. Hence to make x86_64's 10-byte long movabsq instruction > slightly more expensive than a simple constant, rtx_costs needs to > return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost > of movabsq is the intended value 5: > insn_cost 5 for #: r98:DI=3D0x123456789abcdef > and > movabsq $81985529216486895, %rax # 5 [c=3D5 l=3D10] *movdi_interna= l/4 > > > [I'd originally tried fixing this by adding a ix86_insn_cost target > hook, but the testsuite is very sensitive to the costing of insns]. > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check, both with and without --target_board=3Dunix{-m32} > with no new failures. Ok for mainline? > > > 2024-05-22 Roger Sayle > > gcc/ChangeLog > * config/i386/i386.cc (ix86_rtx_costs) : > A CONST_INT that isn't x86_64_immediate_operand requires an extra > (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1= . 1 of 20,796 [x86_64 PATCH] Correct insn_cost of movabsq. Inbox Roger Sayle 5:15=E2=80=AFPM (12 minutes ago) to gcc-patches, me This single line patch fixes a strange quirk/glitch in i386's rtx_costs, which considers an instruction loading a 64-bit constant to be significantl= y cheaper than loading a 32-bit (or smaller) constant. Consider the two functions: unsigned long long foo() { return 0x0123456789abcdefULL; } unsigned int bar() { return 10; } and the corresponding lines from combine's dump file: insn_cost 1 for #: r98:DI=3D0x123456789abcdef insn_cost 4 for #: ax:SI=3D0xa The same issue can be seen in -dP assembler output. movabsq $81985529216486895, %rax # 5 [c=3D1 l=3D10] *movdi_internal/= 4 The problem is that pattern_costs interpretation of rtx_costs contains "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for example a register or small immediate constant) is considered special, and equivalent to a single instruction, but all other values are treated as verbatim. Hence to make x86_64's 10-byte long movabsq instruction slightly more expensive than a simple constant, rtx_costs needs to return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost of movabsq is the intended value 5: insn_cost 5 for #: r98:DI=3D0x123456789abcdef and movabsq $81985529216486895, %rax # 5 [c=3D5 l=3D10] *movdi_internal/= 4 [I'd originally tried fixing this by adding a ix86_insn_cost target hook, but the testsuite is very sensitive to the costing of insns]. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=3Dunix{-m32} with no new failures. Ok for mainline? 2024-05-22 Roger Sayle gcc/ChangeLog * config/i386/i386.cc (ix86_rtx_costs) : A CONST_INT that isn't x86_64_immediate_operand requires an extra (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1. Thanks in advance, Roger -- One attachment =E2=80=A2 Scanned by Gmail Roger Sayle (nextmovesoftware.com), gcc-patches@gcc.gnu.org On Wed, May 22, 2024 at 5:15=E2=80=AFPM Roger Sayle wrote: > > This single line patch fixes a strange quirk/glitch in i386's rtx_costs, > which considers an instruction loading a 64-bit constant to be significan= tly > cheaper than loading a 32-bit (or smaller) constant. > > Consider the two functions: > unsigned long long foo() { return 0x0123456789abcdefULL; } > unsigned int bar() { return 10; } > > and the corresponding lines from combine's dump file: > insn_cost 1 for #: r98:DI=3D0x123456789abcdef > insn_cost 4 for #: ax:SI=3D0xa > > The same issue can be seen in -dP assembler output. > movabsq $81985529216486895, %rax # 5 [c=3D1 l=3D10] *movdi_interna= l/4 > > The problem is that pattern_costs interpretation of rtx_costs contains > "return cost > 0 ? cost : COSTS_N_INSNS (1)" where a zero value (for > example a register or small immediate constant) is considered special, > and equivalent to a single instruction, but all other values are treated > as verbatim. Hence to make x86_64's 10-byte long movabsq instruction > slightly more expensive than a simple constant, rtx_costs needs to > return COSTS_N_INSNS(1)+1 and not 1. With this change, the insn_cost > of movabsq is the intended value 5: > insn_cost 5 for #: r98:DI=3D0x123456789abcdef > and > movabsq $81985529216486895, %rax # 5 [c=3D5 l=3D10] *movdi_interna= l/4 > > > [I'd originally tried fixing this by adding a ix86_insn_cost target > hook, but the testsuite is very sensitive to the costing of insns]. > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > and make -k check, both with and without --target_board=3Dunix{-m32} > with no new failures. Ok for mainline? > > > 2024-05-22 Roger Sayle > > gcc/ChangeLog > * config/i386/i386.cc (ix86_rtx_costs) : > A CONST_INT that isn't x86_64_immediate_operand requires an extra > (expensive) movabsq insn to load, so return COSTS_N_INSNS (1) + 1= . OK, with a small comment added. Thanks, Uros. > diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc > index b4838b7..b4a9519 100644 > --- a/gcc/config/i386/i386.cc > +++ b/gcc/config/i386/i386.cc > @@ -21569,7 +21569,7 @@ ix86_rtx_costs (rtx x, machine_mode mode, int out= er_code_i, int opno, > if (x86_64_immediate_operand (x, VOIDmode)) > *total =3D 0; > else > - *total =3D 1; > + *total =3D COSTS_N_INSNS (1) + 1; > return true; Please add a small comment that this cost belongs to movabs.