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?
