This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] RISC-V: Improve li expansion for better code density.
- From: Jim Wilson <jimw at sifive dot com>
- To: Kito Cheng <kito dot cheng at sifive dot com>
- Cc: Binutils <binutils at sourceware dot org>, Kito Cheng <kito dot cheng at gmail dot com>
- Date: Thu, 22 Aug 2019 19:09:38 -0700
- Subject: Re: [PATCH] RISC-V: Improve li expansion for better code density.
- References: <1566456017-9152-1-git-send-email-kito.cheng@sifive.com>
On Wed, Aug 21, 2019 at 11:40 PM Kito Cheng <kito.cheng@sifive.com> wrote:
> - macro_build (&lower, "addi", "d,s,j", reg, reg, BFD_RELOC_RISCV_LO12_I);
> + md_assemblef ("%s x%d, x%d, %" BFD_VMA_FMT "d", ADD32_INSN, reg, reg,
> + lower.X_add_number);
You are replacing addi with ADD32_INSN which maps to an addiw on a
64-bit target. addiw always sign-extends the 32-bit result, and hence
can't be used to load a 64-bit constant, which is what this code is
doing. Fixing this requires fixing the 64-bit testcases.
> + /* Dicard low part and zero-extend uppper immediate. */
uppper -> upper
> - macro_build (ep, ADD32_INSN, "d,s,j", reg, hi_reg,
> - BFD_RELOC_RISCV_LO12_I);
> + md_assemblef ("%s x%d, x%d, %" BFD_VMA_FMT "d", ADD32_INSN, reg, hi_reg,
> + lower.X_add_number);
This one should be ADD32_INSN, because it is loading a 32-bit
constant, which requires addiw on a 64-bit target, so this is correct.
Otherwise, this looks good with the changes Andreas and Andrew asked
for in addition to the changes I asked for.
FYI a testcase to show this is broken. This works with an unpatched
binutils. WIth your patch I get this result
gamma05:2022$ cat tmp.s
.file "tmp.c"
.option nopic
.attribute arch, "rv64i2p0_m2p0_a2p0_f2p0_d2p0_c2p0"
.attribute unaligned_access, 0
.attribute stack_align, 16
.text
.section .rodata.str1.8,"aMS",@progbits,1
.align 3
.LC0:
.string "0x%lx\n"
.text
.align 1
.globl main
.type main, @function
main:
addi sp,sp,-16
sd ra,8(sp)
li a1,0x1234567887654321
lui a0,%hi(.LC0)
addi a0,a0,%lo(.LC0)
call printf
li a0,0
ld ra,8(sp)
addi sp,sp,16
jr ra
.size main, .-main
.ident "GCC: (GNU) 9.2.0"
gamma05:2023$ build-install/bin/riscv64-unknown-elf-gcc tmp.s
gamma05:2024$ build-install/bin/qemu-riscv64 ./a.out
0xffffffff87654321
gamma05:2025$
We can't put an execution testcase like this into the binutils
testsuite though. I just created a small C testcase, and then edited
the .s file to add a 64-bit li macro instruction.
Jim