From b994624fa57b3314098656e110781d4d0f77b4e4 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Fri, 23 Aug 2024 09:22:30 +0200 Subject: [PATCH] RISC-V: process rs_align_code also when relaxing riscv_handle_align() runs after all input was processed. Whether relaxation is enabled for any particular piece of code is not recorded anywhere. (This issue was even "worked around" in a gas testcase, which is adjusted accordingly.) Furthermore, as demonstrated by an ld testcase, tail padding in an object file's executable sections depended on whether relaxation was enabled at the end of assembly: NOPs were emitted only when relaxation was off; zeroes were emitted with relaxation enabled. (It could probably be either way, but it should be independent of relaxation state at the end of assembly. Except of course write.c, in a comment ahead of #define-ing SUB_SEGMENT_ALIGN(), explicitly says "proper nop-filling".) While re-indenting, drop the "odd_padding" variable. It's used exactly once, and having the actual expression right in the if() is imo helping readers to understand what the intentions are. While touching the ld testcase, also tighten the expectations for the addresses of the two symbols: The last two digits have to have fixed values. --- gas/config/tc-riscv.c | 61 +++++++++---------- gas/testsuite/gas/riscv/mapping.s | 3 - .../ld-riscv-elf/relax-max-align-gp.d | 32 +++++++++- 3 files changed, 58 insertions(+), 38 deletions(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index 15244beafaa..4fc980a82d0 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -5023,40 +5023,37 @@ riscv_handle_align (fragS *fragP) switch (fragP->fr_type) { case rs_align_code: - /* When relaxing, riscv_frag_align_code handles code alignment. */ - if (!riscv_opts.relax) - { - bfd_signed_vma bytes = (fragP->fr_next->fr_address - - fragP->fr_address - fragP->fr_fix); - /* We have 4 byte uncompressed nops. */ - bfd_signed_vma size = 4; - bfd_signed_vma excess = bytes % size; - bfd_boolean odd_padding = (excess % 2 == 1); - char *p = fragP->fr_literal + fragP->fr_fix; - - if (bytes <= 0) - break; + { + bfd_signed_vma bytes = (fragP->fr_next->fr_address + - fragP->fr_address - fragP->fr_fix); + /* We have 4 byte uncompressed nops. */ + bfd_signed_vma size = 4; + bfd_signed_vma excess = bytes % size; + char *p = fragP->fr_literal + fragP->fr_fix; + + if (bytes <= 0) + break; - /* Insert zeros or compressed nops to get 4 byte alignment. */ - if (excess) - { - if (odd_padding) - riscv_add_odd_padding_symbol (fragP); - riscv_make_nops (p, excess); - fragP->fr_fix += excess; - p += excess; - } + /* Insert zeros or compressed nops to get 4 byte alignment. */ + if (excess) + { + if (excess % 2) + riscv_add_odd_padding_symbol (fragP); + riscv_make_nops (p, excess); + fragP->fr_fix += excess; + p += excess; + } - /* The frag will be changed to `rs_fill` later. The function - `write_contents` will try to fill the remaining spaces - according to the patterns we give. In this case, we give - a 4 byte uncompressed nop as the pattern, and set the size - of the pattern into `fr_var`. The nop will be output to the - file `fr_offset` times. However, `fr_offset` could be zero - if we don't need to pad the boundary finally. */ - riscv_make_nops (p, size); - fragP->fr_var = size; - } + /* The frag will be changed to `rs_fill` later. The function + `write_contents` will try to fill the remaining spaces + according to the patterns we give. In this case, we give + a 4 byte uncompressed nop as the pattern, and set the size + of the pattern into `fr_var`. The nop will be output to the + file `fr_offset` times. However, `fr_offset` could be zero + if we don't need to pad the boundary finally. */ + riscv_make_nops (p, size); + fragP->fr_var = size; + } break; default: diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s index 3014a69e792..6882a9e6105 100644 --- a/gas/testsuite/gas/riscv/mapping.s +++ b/gas/testsuite/gas/riscv/mapping.s @@ -1,7 +1,4 @@ .attribute arch, "rv32ic" -.option norelax # FIXME: assembler fill the paddings after parsing everything, - # so we probably won't fill anything for the norelax region when - # the riscv_opts.relax is enabled at somewhere. .section .text.cross.section.A, "ax" .option push diff --git a/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d index b62d3883858..58328350ebc 100644 --- a/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d +++ b/ld/testsuite/ld-riscv-elf/relax-max-align-gp.d @@ -7,7 +7,7 @@ Disassembly of section .text: -0+[0-9a-f]+ <_start>: +0+[0-9a-f]+00 <_start>: .*:[ ]+[0-9a-f]+[ ]+addi[ ]+.* .*:[ ]+[0-9a-f]+[ ]+jal[ ]+.* .*:[ ]+[0-9a-f]+[ ]+j[ ]+.* @@ -41,6 +41,32 @@ Disassembly of section .text: .*:[ ]+[0-9a-f]+[ ]+nop .*:[ ]+[0-9a-f]+[ ]+nop -0+[0-9a-f]+ : +0+[0-9a-f]+80 : .*:[ ]+[0-9a-f]+[ ]+ret -[ ]+... +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop +.*:[ ]+[0-9a-f]+[ ]+nop -- 2.39.5