Skip to content

[RISCV] Fix alignment when mixing rvc/norvc relax/norelax code #150159

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

Closed
wants to merge 2 commits into from

Conversation

kito-cheng
Copy link
Member

The issue we want to show is the alignment problem when mixing RVC and non-RVC code, especially when the RVC code is relaxed and the alignment with in the non-RVC region will not got expected alignment requirement.

Give an example, this example will build with rv64gc and -mrelax option, and assume text section is start from 0x1000:

_start:
        lui a0, %hi(foo)
        addi a0, a0, %lo(foo)
        mul a0, a1, a4
        .option push

        .option norelax
        .option norvc
        .balign 8
SHOULD_ALIGN_8_HERE:
        .word 0x12345678

Then when we assemble this file, assembler will insert NOP and R_RISCV_RELAX/R_RISCV_ALIGN like below:

Disassembly of section .text:

0000000000000000 <_start-0x2>:
   0:   0001                    nop
                        0: R_RISCV_ALIGN        *ABS*+0x2

0000000000000002 <_start>:
   2:   00000537                lui     a0,0x0
                        2: R_RISCV_HI20 foo
                        2: R_RISCV_RELAX        *ABS*
   6:   00050513                mv      a0,a0
                        6: R_RISCV_LO12_I       foo
                        6: R_RISCV_RELAX        *ABS*
   a:   8082                    ret
   c:   00000013                nop

0000000000000010 <SHOULD_ALIGN_8_HERE>:
  10:   12345678                .word   0x12345678

And we didn't insert R_RISCV_ALIGN before SHOULD_ALIGN_8_HERE, because norelax option are given and the alignment seems already meet.

However...the linker relaxation will remove first NOP in the text section before the _start symbol for meet the alignment requirement, then SHOULD_ALIGN_8_HERE no longer aligned to 8 bytes.

So this should be fixed in the MC assembler by inserting R_RISCV_ALIGN correctly before SHOULD_ALIGN_8_HERE, even if the norelax option is given, but that not means we should always inserting that, we just need insert when relax has ever enabled on that section.

Also it should emit N-2 bytes NOPs even norvc status if this section has enable RVC ever, so that linker has enough NOPs to remove for meet the alignment requirement.

This is not really lld bug, but this must be put within lld testcases so that we can verify the alignment result is wrong after linking.

Not all testcase are fail with out this fix, however some of them may fail on binutils side, so I think it worth to put all testcase to lld test to prevent future regression.

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-mc
@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-backend-risc-v

Author: Kito Cheng (kito-cheng)

Changes

The issue we want to show is the alignment problem when mixing RVC and non-RVC code, especially when the RVC code is relaxed and the alignment with in the non-RVC region will not got expected alignment requirement.

Give an example, this example will build with rv64gc and -mrelax option, and assume text section is start from 0x1000:

_start:
        lui a0, %hi(foo)
        addi a0, a0, %lo(foo)
        mul a0, a1, a4
        .option push

        .option norelax
        .option norvc
        .balign 8
SHOULD_ALIGN_8_HERE:
        .word 0x12345678

Then when we assemble this file, assembler will insert NOP and R_RISCV_RELAX/R_RISCV_ALIGN like below:

Disassembly of section .text:

0000000000000000 &lt;_start-0x2&gt;:
   0:   0001                    nop
                        0: R_RISCV_ALIGN        *ABS*+0x2

0000000000000002 &lt;_start&gt;:
   2:   00000537                lui     a0,0x0
                        2: R_RISCV_HI20 foo
                        2: R_RISCV_RELAX        *ABS*
   6:   00050513                mv      a0,a0
                        6: R_RISCV_LO12_I       foo
                        6: R_RISCV_RELAX        *ABS*
   a:   8082                    ret
   c:   00000013                nop

0000000000000010 &lt;SHOULD_ALIGN_8_HERE&gt;:
  10:   12345678                .word   0x12345678

And we didn't insert R_RISCV_ALIGN before SHOULD_ALIGN_8_HERE, because norelax option are given and the alignment seems already meet.

However...the linker relaxation will remove first NOP in the text section before the _start symbol for meet the alignment requirement, then SHOULD_ALIGN_8_HERE no longer aligned to 8 bytes.

So this should be fixed in the MC assembler by inserting R_RISCV_ALIGN correctly before SHOULD_ALIGN_8_HERE, even if the norelax option is given, but that not means we should always inserting that, we just need insert when relax has ever enabled on that section.

Also it should emit N-2 bytes NOPs even norvc status if this section has enable RVC ever, so that linker has enough NOPs to remove for meet the alignment requirement.

This is not really lld bug, but this must be put within lld testcases so that we can verify the alignment result is wrong after linking.

Not all testcase are fail with out this fix, however some of them may fail on binutils side, so I think it worth to put all testcase to lld test to prevent future regression.


Patch is 26.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150159.diff

15 Files Affected:

  • (added) lld/test/ELF/riscv-relax-align-1.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-10.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-11.s (+70)
  • (added) lld/test/ELF/riscv-relax-align-12.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-2.s (+68)
  • (added) lld/test/ELF/riscv-relax-align-3.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-4.s (+68)
  • (added) lld/test/ELF/riscv-relax-align-5.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-6.s (+68)
  • (added) lld/test/ELF/riscv-relax-align-7.s (+70)
  • (added) lld/test/ELF/riscv-relax-align-8.s (+69)
  • (added) lld/test/ELF/riscv-relax-align-9.s (+70)
  • (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+23)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp (+5-2)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h (+14)
diff --git a/lld/test/ELF/riscv-relax-align-1.s b/lld/test/ELF/riscv-relax-align-1.s
new file mode 100644
index 0000000000000..c7ab4b03539eb
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-1.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_8_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_8_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_8_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_8_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 4
+        .global _start
+        .type _start, @function
+_start:
+        lui a0, %hi(foo)
+        addi a0, a0, %lo(foo)
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 8
+SHOULD_ALIGN_8_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-10.s b/lld/test/ELF/riscv-relax-align-10.s
new file mode 100644
index 0000000000000..dc816f0b03b77
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-10.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+	call foo
+        .option norvc
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 16
+SHOULD_ALIGN_16_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-11.s b/lld/test/ELF/riscv-relax-align-11.s
new file mode 100644
index 0000000000000..456ecf1e04456
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-11.s
@@ -0,0 +1,70 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+        lui a0, %hi(foo)
+        addi a0, a0, %lo(foo)
+        .option norvc
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 16
+SHOULD_ALIGN_16_HERE:
+	ret
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-12.s b/lld/test/ELF/riscv-relax-align-12.s
new file mode 100644
index 0000000000000..b9e48d3323db5
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-12.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+	call foo
+        .option norvc
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 16
+SHOULD_ALIGN_16_HERE:
+	ret
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-2.s b/lld/test/ELF/riscv-relax-align-2.s
new file mode 100644
index 0000000000000..401f6e338845b
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-2.s
@@ -0,0 +1,68 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001008 t SHOULD_ALIGN_8_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001008 t SHOULD_ALIGN_8_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001008 t SHOULD_ALIGN_8_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001008 t SHOULD_ALIGN_8_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 4
+        .global _start
+        .type _start, @function
+_start:
+	call foo
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 8
+SHOULD_ALIGN_8_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-3.s b/lld/test/ELF/riscv-relax-align-3.s
new file mode 100644
index 0000000000000..1f4ebeef095f3
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-3.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+        lui a0, %hi(foo)
+        addi a0, a0, %lo(foo)
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 4
+SHOULD_ALIGN_4_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-4.s b/lld/test/ELF/riscv-relax-align-4.s
new file mode 100644
index 0000000000000..2db50fa264f28
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-4.s
@@ -0,0 +1,68 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+	call foo
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 4
+SHOULD_ALIGN_4_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-5.s b/lld/test/ELF/riscv-relax-align-5.s
new file mode 100644
index 0000000000000..b3474811c1554
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-5.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+        lui a0, %hi(foo)
+        addi a0, a0, %lo(foo)
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 16
+SHOULD_ALIGN_16_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-6.s b/lld/test/ELF/riscv-relax-align-6.s
new file mode 100644
index 0000000000000..66e58f490d49b
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-6.s
@@ -0,0 +1,68 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 0000000000001010 t SHOULD_ALIGN_16_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+	call foo
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 16
+SHOULD_ALIGN_16_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-7.s b/lld/test/ELF/riscv-relax-align-7.s
new file mode 100644
index 0000000000000..d192e7129f99b
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-7.s
@@ -0,0 +1,70 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=RELAX-RVC
+
+# RELAX-RVC: 000000000000100c t SHOULD_ALIGN_4_HERE
+
+#--- a.s
+        .text
+        .option relax
+        .balign 2
+        .global _start
+        .type _start, @function
+_start:
+        lui a0, %hi(foo)
+        addi a0, a0, %lo(foo)
+	.option norvc
+	mul a0, a1, a4
+        .option push
+
+.ifdef NORELAX
+        .option norelax
+.endif
+.ifdef NORVC
+        .option norvc
+.endif
+        .balign 4
+SHOULD_ALIGN_4_HERE:
+        .word 0x12345678
+
+        .option pop
+
+foo:
+        ret
+
+
+
+#--- lds
+ENTRY(_start)
+SECTIONS {
+	.text 0x0001000 : {
+		*(.text*)
+	}
+}
diff --git a/lld/test/ELF/riscv-relax-align-8.s b/lld/test/ELF/riscv-relax-align-8.s
new file mode 100644
index 0000000000000..6f8095917d55e
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-align-8.s
@@ -0,0 +1,69 @@
+# REQUIRES: riscv
+## Testing the aligment is correct when mixing with rvc/norvc relax/norelax
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+## NORVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1 --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC-NORELAX
+
+# NORVC-NORELAX: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## NORVC, RELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORVC=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORVC
+
+# NORVC: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## RVC, NORELAX
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m a.s -o a.o --defsym=NORELAX=1
+# RUN: ld.lld -T lds a.o -o a.out
+# RUN: llvm-nm a.out | FileCheck %s --check-prefix=NORELAX
+
+# NORELAX: 0000000000001008 t SHOULD_ALIGN_4_HERE
+
+## RVC, RELAX
+# RUN: llvm-mc -filetype=o...
[truncated]

@jrtc27
Copy link
Collaborator

jrtc27 commented Jul 23, 2025

This is not really lld bug, but this must be put within lld testcases so that we can verify the alignment result is wrong after linking.

Or you can just write a test case using llvm-mc to verify that the section layout and relocations are correct? You're just using ld.lld as an unnecessarily complicated "is my object file correct" predicate.

@kito-cheng
Copy link
Member Author

Or you can just write a test case using llvm-mc to verify that the section layout and relocations are correct? You're just using ld.lld as an unnecessarily complicated "is my object file correct" predicate.

Hmmm, maybe I can keep lld testcase, but also add testcase for MC assembler

@topperc
Copy link
Collaborator

topperc commented Jul 23, 2025

Does the same issue exist with .option exact?

@MaskRay
Copy link
Member

MaskRay commented Jul 24, 2025

The 10+ lld tests do not look right. If this is an assembler issue, the test ought to be in llvm/test/MC/RISCV.
Please can you remove the lld/test/ELF changes from this PR?

I believe there is a lot of duplication in these tests (numbered from 1 to 10? We don't adopt the gcc style naming...)
https://maskray.me/blog/2021-08-08-toolchain-testing#i-dont-know-an-existing-test-can-be-enhanced

@MaskRay
Copy link
Member

MaskRay commented Jul 27, 2025

I think the example is missing a .balign 4 at _start. Without the alignment, the initial alignment is 2, <= the minimum nop size (2 due to the C extension), and assemblers (both GAS and LLVM) will not emit a R_RISCV_ALIGN relocation.

ELF assembles have the subsection feature. While I haven't tested, I think the patch will not work with

        .text 1
        .option push
        .option norelax
        .option norvc
        .balign 8
SHOULD_ALIGN_8_HERE:
        .word 0x12345678
        .option pop

        .text 0
_start:
.balign 4
        lui a0, %hi(foo)
        addi a0, a0, %lo(foo)
        mul a0, a1, a4

I think we need to modify MCSection::LinkerRelaxable. It should be set to a value that indicates the LayoutOrder of the first linker-relaxable fragment. Then, relaxAlign should be updated to verify if the FT_Align fragment appears after this first linker-relaxable fragment.

@MaskRay
Copy link
Member

MaskRay commented Jul 27, 2025

I think we need to modify MCSection::LinkerRelaxable. It should be set to a value that indicates the LayoutOrder of the first linker-relaxable fragment. Then, relaxAlign should be updated to verify if the FT_Align fragment appears after this first linker-relaxable fragment.

See #150816

The issue we want to show is the alignment problem when mixing RVC and non-RVC
code, especially when the RVC code is relaxed and the alignment with in the
non-RVC region will not got expected alignment requirement.

Give an example, this example will build with `rv64gc` and `-mrelax` option,
and assume text section is start from `0x1000`:

```
_start:
        lui a0, %hi(foo)
        addi a0, a0, %lo(foo)
        mul a0, a1, a4
        .option push

        .option norelax
        .option norvc
        .balign 8
SHOULD_ALIGN_8_HERE:
        .word 0x12345678
```

Then when we assemble this file, assembler will insert NOP and
`R_RISCV_RELAX`/`R_RISCV_ALIGN` like below:
```
Disassembly of section .text:

0000000000000000 <_start-0x2>:
   0:   0001                    nop
                        0: R_RISCV_ALIGN        *ABS*+0x2

0000000000000002 <_start>:
   2:   00000537                lui     a0,0x0
                        2: R_RISCV_HI20 foo
                        2: R_RISCV_RELAX        *ABS*
   6:   00050513                mv      a0,a0
                        6: R_RISCV_LO12_I       foo
                        6: R_RISCV_RELAX        *ABS*
   a:   8082                    ret
   c:   00000013                nop

0000000000000010 <SHOULD_ALIGN_8_HERE>:
  10:   12345678                .word   0x12345678
```

And we didn't insert `R_RISCV_ALIGN` before `SHOULD_ALIGN_8_HERE`, because
`norelax` option are given and the alignment seems already meet.

However...the linker relaxation will remove first NOP in the text section
before the `_start` symbol for meet the alignment requirement, then
`SHOULD_ALIGN_8_HERE` no longer aligned to 8 bytes.

So this should be fixed in the MC assembler by inserting `R_RISCV_ALIGN`
correctly before `SHOULD_ALIGN_8_HERE`, even if the norelax option is given,
but that not means we should always inserting that, we just need insert when
relax has ever enabled on that section.

Also it should emit N-2 bytes NOPs even norvc status if this section has enable
RVC ever, so that linker has enough NOPs to remove for meet the alignment
requirement.

This is not really lld bug, but this must be put within lld testcases so that we
can verify the alignment result is wrong after linking.

Not all testcase are fail with out this fix, however some of them may fail on
binutils side, so I think it worth to put all testcase to lld test to prevent
future regression.
Changes:
- Add const to MCSection* for the RelaxEverSections and RVCEverSections
- Fix and simplify condition in RISCVAsmBackend::relaxAlign.
- Drop lld tests.
- Adding MC tests
@kito-cheng kito-cheng force-pushed the kitoc/fix-aligment-issue branch from 60936d9 to 58dd9c2 Compare July 31, 2025 14:41
@llvmbot llvmbot added the mc Machine (object) code label Jul 31, 2025
@kito-cheng
Copy link
Member Author

Changes:

  • Add const to MCSection* for the RelaxEverSections and RVCEverSections
  • Fix and simplify condition in RISCVAsmBackend::relaxAlign.
  • Drop lld tests.
  • Adding MC tests

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
View the diff from clang-format here.
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 5583a2d73..5095603dc 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -312,9 +312,8 @@ bool RISCVAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
   if (!STI->hasFeature(RISCV::FeatureRelax) && !hasRelaxEver(F.getParent()))
     return false;
   unsigned MinNopLen =
-      hasRVCEver(F.getParent()) || STI->hasFeature(RISCV::FeatureStdExtZca)
-          ? 2
-          : 4;
+      hasRVCEver(F.getParent()) || STI->hasFeature(RISCV::FeatureStdExtZca) ? 2
+                                                                            : 4;
   if (F.getAlignment() <= MinNopLen)
     return false;
 

return false;
unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
unsigned MinNopLen =
hasRVCEver(F.getParent()) || STI->hasFeature(RISCV::FeatureStdExtZca)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check STI->hasFeature(RISCV::FeatureStdExtZca) first?

@lenary
Copy link
Member

lenary commented Aug 1, 2025

I don't know of the status of this vs #150816. I presume we're only getting one of these, not both.

In terms of naming, I would prefer we don't reference rvc, and instead we reference this in terms of the minimal IALIGN - which for any section will be 4 until someone enables Zca at which point it will drop to 2. I think conceptually that's a bit more specific than noting something about "rvc" when the check is for zca, a subset of rvc. I'm happy to accept the code would be equivalent, but I think naming in terms of IALIGN would help clarify what's really going on.

@kito-cheng
Copy link
Member Author

I believe #150816 is better solution than mine, and I am happy to see that moving forward, so let me close this PR by myself to prevent confusing people that we are competing :)

@kito-cheng kito-cheng closed this Aug 6, 2025
MaskRay added a commit that referenced this pull request Aug 8, 2025
Each section now tracks the index of the first linker-relaxable
fragment, enabling two changes:

* Delete redundant ALIGN relocations before the first linker-relaxable
  instruction in a section. The primary example is the offset 0
  R_RISCV_ALIGN relocation for a text section aligned by 4.
* For alignments larger than the NOP size after the first
  linker-relaxable instruction, ALIGN relocations are now generated, even in
  norelax regions. This fixes the issue #150159.

The new test llvm/test/MC/RISCV/Relocations/align-after-relax.s
verifies the required ALIGN in a norelax region following
linker-relaxable instructions.
By using a fragment index within the subsection (which is less than or
equal to the section's index), the implementation may generate redundant
ALIGN relocations in lower-numbered subsections before the first
linker-relaxable instruction.

align-option-relax.s demonstrates the ALIGN optimization.
Add an initial `call` to a few tests to prevent the ALIGN optimization.

---

When the alignment exceeds 2, we insert $alignment-2 bytes of NOPs, even
in non-RVC code. This enables non-RVC code following RVC code to handle
a 2-byte adjustment without requiring an additional state in MCSection
or AsmParser.

```
.globl _start
_start:
// GNU ld can relax this to  6505          lui     a0, 0x1
// LLD hasn't implemented this transformation.
  lui a0, %hi(foo)

.option push
.option norelax
.option norvc
// Now we generate R_RISCV_ALIGN with addend 2, even if this is a norvc region.
.balign 4
b0:
  .word 0x3a393837
.option pop
foo:
```

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

Successfully merging this pull request may close these issues.

6 participants