Re: [PATCH] RISC-V: cost model for loading 64bit constant in rv32

[Palmer Dabbelt <palmer@dabbelt.com>]

On Tue, 08 Nov 2022 19:26:01 PST (-0800), gcc-patches@gcc.gnu.org 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:
> https://godbolt.org/z/v5nxTbKe9 <https://godbolt.org/z/v5nxTbKe9 >

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 
<https://gcc.gnu.org/contribute.html>.

Given those usually show up for first-time contributors there's also 
some copyright/DCO procedures to bo followed.  I see some other 
linux.alibaba.com 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/gcc.target/riscv/rv32-load-64bit-constant.c
> @@ -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?