public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Lin Sinan <sinan.lin@linux.alibaba.com>, gcc-patches@gcc.gnu.org
Cc: palmer@dabbelt.com, kito.cheng@gmail.com, andrew@sifive.com
Subject: Re: [PATCH RESEND] riscv: improve the cost model for loading a 64bit constant in rv32.
Date: Mon, 28 Nov 2022 12:44:56 -0700	[thread overview]
Message-ID: <2399c0bf-e8dd-0430-432b-b8fb30c0da12@gmail.com> (raw)
In-Reply-To: <6787a2d509d2b8ef27083d3b9806661eb8f56102.1668090837.git.sinan.lin@linux.alibaba.com>

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]



On 11/10/22 07:37, Lin Sinan via Gcc-patches wrote:
> The motivation of this patch is to correct the wrong estimation of the number of instructions needed for loading a 64bit constant in rv32 in the current cost model(riscv_interger_cost). According to the current implementation, if a constant requires more than 3 instructions(riscv_const_insn and riscv_legitimate_constant_p), then the constant will be put into constant pool when expanding gimple to rtl(legitimate_constant_p hook and emit_move_insn). So the inaccurate cost model leads to the suboptimal codegen in rv32 and the wrong estimation part could be corrected through this fix.
> 
> e.g. the current codegen for loading 0x839290001 in rv32
> 
>    lui     a5,%hi(.LC0)
>    lw      a0,%lo(.LC0)(a5)
>    lw      a1,%lo(.LC0+4)(a5)
> .LC0:
>    .word   958988289
>    .word   8
> 
> output after this patch
> 
>    li a0,958988288
>    addi a0,a0,1
>    li a1,8
> 
> gcc/ChangeLog:
> 
>          * config/riscv/riscv.cc (riscv_build_integer): Handle the case of loading 64bit constant in rv32.
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.target/riscv/rv32-load-64bit-constant.c: New test.
> 
> Signed-off-by: Lin Sinan <sinan.lin@linux.alibaba.com>
I fixed up the ChangeLog and some minor formatting issues in the new 
code in riscv_build_integer.  I also twiddled the test so that it 
iterates over the optimization levels properly while skipping -O0.

Attached is the patch I committed.

jeff

[-- Attachment #2: P --]
[-- Type: text/plain, Size: 3541 bytes --]

commit 940d5b56990fdf171f49517ae102673817b9c869
Author: Sinan <sinan.lin@linux.alibaba.com>
Date:   Mon Nov 28 12:41:17 2022 -0700

    riscv: improve cost model for loading 64bit constant in rv32
    
    The motivation of this patch is to correct the wrong estimation of the number of instructions needed for loading a 64bit constant in rv32 in the current cost model(riscv_interger_cost). According to the current implementation, if a constant requires more than 3 instructions(riscv_const_insn and riscv_legitimate_constant_p), then the constant will be put into constant pool when expanding gimple to rtl(legitimate_constant_p hook and emit_move_insn). So the inaccurate cost model leads to the suboptimal codegen in rv32 and the wrong estimation part could be corrected through this fix.
    
    e.g. the current codegen for loading 0x839290001 in rv32
    
      lui     a5,%hi(.LC0)
      lw      a0,%lo(.LC0)(a5)
      lw      a1,%lo(.LC0+4)(a5)
    .LC0:
      .word   958988289
      .word   8
    
    output after this patch
    
      li a0,958988288
      addi a0,a0,1
      li a1,8
    
    gcc/ChangeLog:
    
            * config/riscv/riscv.cc (riscv_build_integer): Improve some cases
            of loading 64bit constants for rv32.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.target/riscv/rv32-load-64bit-constant.c: New test.

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index ab02a81e152..05bdba5ab4d 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -625,6 +625,30 @@ riscv_build_integer (struct riscv_integer_op *codes, HOST_WIDE_INT value,
 	}
     }
 
+  if (!TARGET_64BIT
+      && (value > INT32_MAX || value < INT32_MIN))
+    {
+      unsigned HOST_WIDE_INT loval = sext_hwi (value, 32);
+      unsigned HOST_WIDE_INT hival = sext_hwi ((value - loval) >> 32, 32);
+      struct riscv_integer_op alt_codes[RISCV_MAX_INTEGER_OPS];
+      struct riscv_integer_op hicode[RISCV_MAX_INTEGER_OPS];
+      int hi_cost, lo_cost;
+
+      hi_cost = riscv_build_integer_1 (hicode, hival, mode);
+      if (hi_cost < cost)
+	{
+	  lo_cost = riscv_build_integer_1 (alt_codes, loval, mode);
+	  if (lo_cost + hi_cost < cost)
+	    {
+	      memcpy (codes, alt_codes,
+		      lo_cost * sizeof (struct riscv_integer_op));
+	      memcpy (codes + lo_cost, hicode,
+		      hi_cost * sizeof (struct riscv_integer_op));
+	      cost = lo_cost + hi_cost;
+	    }
+	}
+    }
+
   return cost;
 }
 
diff --git a/gcc/testsuite/gcc.target/riscv/rv32-load-64bit-constant.c b/gcc/testsuite/gcc.target/riscv/rv32-load-64bit-constant.c
new file mode 100644
index 00000000000..954e1ddf1c0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rv32-load-64bit-constant.c
@@ -0,0 +1,40 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32im -mabi=ilp32" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } } */
+
+
+/* This test only applies to RV32. Some of 64bit constants in this test will be put
+into the constant pool in RV64, since RV64 might need one extra instruction to load
+64bit constant. */
+
+unsigned long long
+rv32_mov_64bit_int1 (void)
+{
+  return 0x739290001LL;
+}
+
+unsigned long long
+rv32_mov_64bit_int2 (void)
+{
+  return 0x839290001LL;
+}
+
+unsigned long long
+rv32_mov_64bit_int3 (void)
+{
+  return 0x3929000139290000LL;
+}
+
+unsigned long long
+rv32_mov_64bit_int4 (void)
+{
+  return 0x3929001139290000LL;
+}
+
+unsigned long long
+rv32_mov_64bit_int5 (void)
+{
+  return 0x14736def39290000LL;
+}
+
+/* { dg-final { scan-assembler-not "lw\t" } } */

      parent reply	other threads:[~2022-11-28 19:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10 14:37 Lin Sinan
2022-11-17  7:32 ` [PING] " Lin Sinan
2022-11-22 22:23 ` Jeff Law
     [not found]   ` <e026c2e0-04ec-4477-bd91-441a6f160251.sinan.lin@linux.alibaba.com>
2022-11-28 19:15     ` 回复:[PING] " Jeff Law
2022-11-28 19:18       ` Palmer Dabbelt
2022-11-28 19:44 ` Jeff Law [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2399c0bf-e8dd-0430-432b-b8fb30c0da12@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=andrew@sifive.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kito.cheng@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=sinan.lin@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).