public inbox for
 help / color / mirror / Atom feed
From: Palmer Dabbelt <>
Subject: Re: [PATCH] RISC-V: cost model for loading 64bit constant in rv32
Date: Tue, 08 Nov 2022 20:37:56 -0800 (PST)	[thread overview]
Message-ID: <mhng-00adf0a8-091b-4044-a6d1-511678fca607@palmer-ri-x1c9a> (raw)
In-Reply-To: <>

On Tue, 08 Nov 2022 19:26:01 PST (-0800), wrote:
> loading constant 0x739290001LL in rv32 can be done with three instructions
> output:
> li a1, 7
> lui a1, 234128
> addi a1, a1, 1

Probably more useful to load the constant into two different registers?  
The point is the same, though, so just a commit message issue.

> Similarly, loading 0x839290001LL in rv32 can be done within three instructions
> expected output:
> li a1, 8
> lui a1, 234128
> addi a1, a1, 1
> However, riscv_build_integer does not handle this case well and makes a wrong prediction about the number of instructions needed and then the constant is forced to put in the memory via riscv_const_insns and emit_move_insn.
> real output:
> lui a4,%hi(.LC0)
> lw a2,%lo(.LC0)(a4)
> lw a3,%lo(.LC0+4)(a4)
> .LC0:
>  .word958988289
>  .word8

That's still 3 instructions, but having loads and a constant is worse so 
that's just another commit message issue.

> comparison with clang:
> < >

IIUC the rules are generally no compiler explorer links (so we can 
preserve history) and no attachment patches (ie, inline them like 
git-send-email does).  There's some documenation on sending patches at 

Given those usually show up for first-time contributors there's also 
some copyright/DCO procedures to bo followed.  I see some other emails called out explicitly, but then also a generic 
"GCC Alibaba Group Holding Limited".  I think that means we're OK for 
copyright assignment here?  There's still the "you wrote the code" bits 
that are worth reading, though.

Looking at the attached patch:

> +  if ((value > INT32_MAX || value < INT32_MIN) && !TARGET_64BIT)
> +    {
> +      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],
> +			   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;
> +	   }
> +	}
> +    }

This kind of stuff always seems like it should be possible to handle 
with generic middle-end optimizations: it's essentially just splitting 
the 64-bit constant into two 32-bit constants, which is free because 
it's going into two registers anyway.  That's not a RISC-V specific 
problem, it's just the case any time a constant is going to be split 
between two registers -- it'd even apply for 128-bit constants on rv64, 
though those are probably rare enough they don't matter all that much.

I'm not opposed to doing this in the backend, but maybe it's a sign 
we've just screwed something else up and can avoid adding the code?

> +++ b/gcc/testsuite/
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv32gc -mabi=ilp32 -Os" } */

This has the same library/abi problems we've had before, but in this 
case I think it's fine to just leave the -march/-mabi out: the test 
cases won't be that exciting on rv64 (unless we add the 128-bit splits 
too?), but they should still stay away from the constant pool.

IIUC this should work on more than Os, at least O2/above?  Not that 
exciting for the test case, though.

> +/* { dg-final { scan-assembler-not "\.LC\[0-9\]" } } */

That's a bit fragile, maybe just match on load-word?

  reply	other threads:[~2022-11-09  4:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09  3:26 Sinan
2022-11-09  4:37 ` Palmer Dabbelt [this message]
2022-11-09 13:03 Sinan

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:

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

  git send-email \
    --in-reply-to=mhng-00adf0a8-091b-4044-a6d1-511678fca607@palmer-ri-x1c9a \ \ \

* 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).