Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

relaxation alignment issues when both rvc and norvc code present #445

Open
jim-wilson opened this issue May 21, 2019 · 6 comments
Open

relaxation alignment issues when both rvc and norvc code present #445

jim-wilson opened this issue May 21, 2019 · 6 comments
Labels

Comments

@jim-wilson
Copy link
Collaborator

Consider this testcase:
.option rvc
add a0, a0, a1
add a1, a1, a2
.balign 4
.option norvc
add a0, a1, a2
call Label # rvc relaxable
.balign 8
var: .dword 100
Label:

Compiling for rv32 gives a linker error
rohan:2213$ riscv32-unknown-elf-gcc -nostdlib -nostartfiles tmp.s
/home/jimw/FOSS/install-riscv32/lib/gcc/riscv32-unknown-elf/8.3.0/../../../../riscv32-unknown-elf/bin/ld: /tmp/ccEKeTeF.o(.text+0xa): 6 bytes required for alignment to 8-byte boundary, but only 4 present
/home/jimw/FOSS/install-riscv32/lib/gcc/riscv32-unknown-elf/8.3.0/../../../../riscv32-unknown-elf/bin/ld: can't relax section: bad value
collect2: error: ld returned 1 exit status
rohan:2214$

In the .o file we have
a: 00000097 auipc ra,0x0
a: R_RISCV_CALL Label
a: R_RISCV_RELAX ABS
e: 000080e7 jalr ra # a
12: 00000013 nop
12: R_RISCV_ALIGN ABS+0x4
00000016 :

The assembler emitted a single 4-byte nop and a R_RISCV_ALIGN reloc because we must have 4-byte instruction alignment in norvc mode. However, it also emitted a R_RISCV_RELAX for the auipc/jalr pair.

In the linker, we see the auipc/jalr pair, the R_RISCV_RELAX reloc, and the fact that the object file contains rvc instructions, and we optimize to a 2-byte c.jalr instruction. Now we have 2-byte alignment in a norvc section, and the alignment relaxation fails.

The only solution I see is to add new relocations. For instance, instead of a single R_RISCV_CALL reloc we need R_RISCV_CALL and R_RISCV_CALL_RVC where the former is used in norvc code and the latter is used in rvc code, and we can only relax to an rvc instruction when R_RISCV_CALL_RVC is present. We have the same problem with lui and the R_RISCV_HI20 reloc. Or alternatively, maybe we add R_RISCV_RELAX_RVC and then we only need one new reloc.

@aswaterman
Copy link
Collaborator

What about treating .option norvc as implying .option norelax when the file contains RVC instructions?

@jim-wilson
Copy link
Collaborator Author

Yes, turning off relaxation solves the problem; that is what I recommended as a workaround to the original bug reporter. But I think that solution is potentially confusing to the end user. It isn't obvious that switching modes should turn off relaxation. And we are losing code size when rvc and norvc are mixed.

@jim-wilson
Copy link
Collaborator Author

There is a related problem where alignment directives can fail.
rohan:2265$ cat tmp2.s
.option rvc
mv a0, a1
.option norvc
.align 3
add a0, a1, a2
rohan:2266$ riscv32-unknown-elf-gcc -nostdlib -nostartfiles tmp2.s
/home/jimw/FOSS/install-riscv32/lib/gcc/riscv32-unknown-elf/8.3.0/../../../../riscv32-unknown-elf/bin/ld: /tmp/ccLQub2H.o(.text+0x2): 6 bytes required for alignment to 8-byte boundary, but only 4 present
/home/jimw/FOSS/install-riscv32/lib/gcc/riscv32-unknown-elf/8.3.0/../../../../riscv32-unknown-elf/bin/ld: can't relax section: bad value
collect2: error: ld returned 1 exit status

Turning off relaxation fixes this problem also.

The error message here is confusing. An alternative solution involves adding a new R_RISCV_ALIGN_RVC reloc, modifying the assembler to always emit N-2 bytes of nops using R_RISCV_ALIGN in norvc mode and R_RISCV_ALIGN_RVC in rvc mode, and then in the linker if we have R_RISCV_ALIGN and need N mod 4 == 2 bytes of nops we can emit a more user friendly error message that explains that we have 2-byte aligned norvc code which is a user error.

@lazyparser
Copy link
Collaborator

@jim-wilson Hi, has it fixed (so we can close this issue)?

@lazyparser lazyparser added the bug label Mar 4, 2021
@jim-wilson
Copy link
Collaborator Author

The fundamental assembler issue is still present.

Nelson had a proposal to introduce new relocs to fix it, but that there has been no resolution of that.
riscv-non-isa/riscv-elf-psabi-doc#116

@TommyMurphyTM1234
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants