[MC][RISCV][LoongArch] Add AlignFragment size if layout is available and not need insert nops #76552
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-loongarch @llvm/pr-subscribers-mc Author: Jinyang He (MQ-mengqing) ChangesDue to delayed decision for ADD/SUB relocations, RISCV and LoongArch may go slow fragment walk path with available layout. When RISCV (or LoongArch in the future) don't need insert nops, that means relax is disabled. With available layout and not needing insert nops, the size of AlignFragment should be a constant. So we can add it to Displacement for folding A-B. Full diff: https://github.com/llvm/llvm-project/pull/76552.diff 5 Files Affected:
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index a85182aa06ad52..9dae026535ccfb 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -704,8 +704,14 @@ static void AttemptToFoldSymbolOffsetDifference(
}
int64_t Num;
+ unsigned Count;
if (DF) {
Displacement += DF->getContents().size();
+ } else if (auto *AF = dyn_cast<MCAlignFragment>(FI);
+ AF && Layout &&
+ !Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
+ *AF, Count)) {
+ Displacement += Asm->computeFragmentSize(*Layout, *AF);
} else if (auto *FF = dyn_cast<MCFillFragment>(FI);
FF && FF->getNumValues().evaluateAsAbsolute(Num)) {
Displacement += Num * FF->getValueSize();
diff --git a/llvm/test/MC/LoongArch/Misc/cfi-advance.s b/llvm/test/MC/LoongArch/Misc/cfi-advance.s
new file mode 100644
index 00000000000000..662c43e6bceafd
--- /dev/null
+++ b/llvm/test/MC/LoongArch/Misc/cfi-advance.s
@@ -0,0 +1,27 @@
+# RUN: llvm-mc --filetype=obj --triple=loongarch64 -mattr=-relax %s -o %t.o
+# RUN: llvm-readobj -r %t.o | FileCheck --check-prefix=RELOC %s
+# RUN: llvm-dwarfdump --debug-frame %t.o | FileCheck --check-prefix=DWARFDUMP %s
+
+# RELOC: Relocations [
+# RELOC-NEXT: .rela.eh_frame {
+# RELOC-NEXT: 0x1C R_LARCH_32_PCREL .text 0x0
+# RELOC-NEXT: }
+# RELOC-NEXT: ]
+# DWARFDUMP: DW_CFA_advance_loc: 4
+# DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
+# DWARFDUMP-NEXT: DW_CFA_advance_loc: 8
+# DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
+
+ .text
+ .globl test
+ .p2align 2
+ .type test,@function
+test:
+ .cfi_startproc
+ nop
+ .cfi_def_cfa_offset 8
+ .p2align 3
+ nop
+ .cfi_def_cfa_offset 8
+ nop
+ .cfi_endproc
diff --git a/llvm/test/MC/LoongArch/Relocations/relax-addsub.s b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
index c4454f5bb98d11..c30a16562b80bc 100644
--- a/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
+++ b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
@@ -23,14 +23,6 @@
# RELAX-NEXT: 0x14 R_LARCH_RELAX - 0x0
# RELAX-NEXT: }
# RELAX-NEXT: Section ({{.*}}) .rela.data {
-# RELAX-NEXT: 0xF R_LARCH_ADD8 .L3 0x0
-# RELAX-NEXT: 0xF R_LARCH_SUB8 .L2 0x0
-# RELAX-NEXT: 0x10 R_LARCH_ADD16 .L3 0x0
-# RELAX-NEXT: 0x10 R_LARCH_SUB16 .L2 0x0
-# RELAX-NEXT: 0x12 R_LARCH_ADD32 .L3 0x0
-# RELAX-NEXT: 0x12 R_LARCH_SUB32 .L2 0x0
-# RELAX-NEXT: 0x16 R_LARCH_ADD64 .L3 0x0
-# RELAX-NEXT: 0x16 R_LARCH_SUB64 .L2 0x0
# RELAX-NEXT: 0x1E R_LARCH_ADD8 .L4 0x0
# RELAX-NEXT: 0x1E R_LARCH_SUB8 .L3 0x0
# RELAX-NEXT: 0x1F R_LARCH_ADD16 .L4 0x0
@@ -43,8 +35,8 @@
# RELAX-NEXT: ]
# RELAX: Hex dump of section '.data':
-# RELAX-NEXT: 0x00000000 04040004 00000004 00000000 00000000
-# RELAX-NEXT: 0x00000010 00000000 00000000 00000000 00000000
+# RELAX-NEXT: 0x00000000 04040004 00000004 00000000 0000000c
+# RELAX-NEXT: 0x00000010 0c000c00 00000c00 00000000 00000000
# RELAX-NEXT: 0x00000020 00000000 00000000 00000000 00
.text
@@ -64,8 +56,7 @@
.word .L2 - .L1
.dword .L2 - .L1
## With relaxation, emit relocs because of the .align making the diff variable.
-## TODO Handle alignment directive. Why they emit relocs now? They returns
-## without folding symbols offset in AttemptToFoldSymbolOffsetDifference().
+## TODO Handle alignment directive.
.byte .L3 - .L2
.short .L3 - .L2
.word .L3 - .L2
diff --git a/llvm/test/MC/RISCV/cfi-advance.s b/llvm/test/MC/RISCV/cfi-advance.s
index d9224fd2ae1c9f..c4af390be757da 100644
--- a/llvm/test/MC/RISCV/cfi-advance.s
+++ b/llvm/test/MC/RISCV/cfi-advance.s
@@ -5,12 +5,16 @@
# CHECK: .rela.eh_frame {
# CHECK-NEXT: 0x1C R_RISCV_32_PCREL <null> 0x0
+# CHECK-NEXT: 0x35 R_RISCV_SET6 <null> 0x0
+# CHECK-NEXT: 0x35 R_RISCV_SUB6 <null> 0x0
# CHECK-NEXT: }
# CHECK-DWARFDUMP: DW_CFA_advance_loc1: 104
# CHECK-DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
# CHECK-DWARFDUMP-NEXT: DW_CFA_advance_loc2: 259
# CHECK-DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
# CHECK-DWARFDUMP-NEXT: DW_CFA_advance_loc4: 65539
+# CHECK-DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
+# CHECK-DWARFDUMP-NEXT: DW_CFA_advance_loc: 10
# CHECK-DWARFDUMP-NEXT: DW_CFA_def_cfa_offset: +8
.text
.globl test # -- Begin function test
@@ -28,4 +32,7 @@ test:
.zero 65535, 0x90
.cfi_def_cfa_offset 8
nop
+ .p2align 3
+ .cfi_def_cfa_offset 8
+ nop
.cfi_endproc
diff --git a/llvm/test/MC/RISCV/fixups-expr.s b/llvm/test/MC/RISCV/fixups-expr.s
index 20e5aacac61928..c850c0a6c0116e 100644
--- a/llvm/test/MC/RISCV/fixups-expr.s
+++ b/llvm/test/MC/RISCV/fixups-expr.s
@@ -16,11 +16,15 @@
.globl G1
.globl G2
+.globl G3
.L1:
G1:
call extern
.L2:
G2:
+ .p2align 3
+.L3:
+G3:
.data
.dword .L2-.L1
@@ -47,3 +51,28 @@ G2:
# RELAX: 0x1C R_RISCV_SUB8 .L1 0x0
# RELAX: 0x1D R_RISCV_ADD8 G2 0x0
# RELAX: 0x1D R_RISCV_SUB8 G1 0x0
+
+.dword .L3-.L2
+.dword G3-G2
+.word .L3-.L2
+.word G3-G2
+.half .L3-.L2
+.half G3-G2
+.byte .L3-.L2
+.byte G3-G2
+# RELAX: 0x1E R_RISCV_ADD64 .L3 0x0
+# RELAX: 0x1E R_RISCV_SUB64 .L2 0x0
+# RELAX: 0x26 R_RISCV_ADD64 G3 0x0
+# RELAX: 0x26 R_RISCV_SUB64 G2 0x0
+# RELAX: 0x2E R_RISCV_ADD32 .L3 0x0
+# RELAX: 0x2E R_RISCV_SUB32 .L2 0x0
+# RELAX: 0x32 R_RISCV_ADD32 G3 0x0
+# RELAX: 0x32 R_RISCV_SUB32 G2 0x0
+# RELAX: 0x36 R_RISCV_ADD16 .L3 0x0
+# RELAX: 0x36 R_RISCV_SUB16 .L2 0x0
+# RELAX: 0x38 R_RISCV_ADD16 G3 0x0
+# RELAX: 0x38 R_RISCV_SUB16 G2 0x0
+# RELAX: 0x3A R_RISCV_ADD8 .L3 0x0
+# RELAX: 0x3A R_RISCV_SUB8 .L2 0x0
+# RELAX: 0x3B R_RISCV_ADD8 G3 0x0
+# RELAX: 0x3B R_RISCV_SUB8 G2 0x0
|
|
Add: @MaskRay @SixWeining |
SixWeining
reviewed
Dec 29, 2023
MaskRay
reviewed
Jan 1, 2024
llvm/test/MC/RISCV/fixups-expr.s
Outdated
| .half G3-G2 | ||
| .byte .L3-.L2 | ||
| .byte G3-G2 | ||
| # RELAX: 0x1E R_RISCV_ADD64 .L3 0x0 |
MaskRay
approved these changes
Jan 1, 2024
SixWeining
approved these changes
Jan 2, 2024
…and not need insert nops Due to delayed decision for ADD/SUB relocations, RISCV and LoongArch may go slow fragment walk path with available layout. When RISCV (or LoongArch in the future) don't need insert nops, that means relax is disabled. With available layout and not needing insert nops, the size of AlignFragment should be a constant. So we can add it to Displacement for folding A-B.
MQ-mengqing
force-pushed
the
faligndiff
branch
from
6c483e1 to
0f2de38
Compare
|
I bisected a crash that I see in a particular translation unit when building the Linux kernel for RISC-V,
.section ""
riscv_kexec_relocate:
la s6, f
.align 2
riscv_kexec_relocate_end:
.long riscv_kexec_relocate_end - riscv_kexec_relocateI've included the full preprocessed file below, in case you want full introspection. Full
|
I find it is because the AF->STI is uninitialized in this section. (Not call MCAlignFragment::setEmitNops). I think it can fix by the following diff, diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 9dae026535cc..80def6dfc24b 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -708,7 +708,7 @@ static void AttemptToFoldSymbolOffsetDifference(
if (DF) {
Displacement += DF->getContents().size();
} else if (auto *AF = dyn_cast<MCAlignFragment>(FI);
- AF && Layout &&
+ AF && Layout && AF->hasEmitNops() &&
!Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(
*AF, Count)) {
Displacement += Asm->computeFragmentSize(*Layout, *AF); |
MQ-mengqing
added a commit
to MQ-mengqing/llvm-project
that referenced
this pull request
Jan 7, 2024
…rCodeAlign The shouldInsertExtraNopBytesForCodeAlign() need STI to check whether relax is enabled or not. It is initialized when call setEmitNops. The setEmitNops may not be called in a section which has instructions but is not executable. In this case uninitialized STI will cause problems. Thus, check hasEmitNops before call it. Fixes: llvm#76552 (comment)
SixWeining
pushed a commit
that referenced
this pull request
Jan 9, 2024
…rCodeAlign (#77236) The shouldInsertExtraNopBytesForCodeAlign() need STI to check whether relax is enabled or not. It is initialized when call setEmitNops. The setEmitNops may not be called in a section which has instructions but is not executable. In this case uninitialized STI will cause problems. Thus, check hasEmitNops before call it. Fixes: #76552 (comment)
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
…rCodeAlign (llvm#77236) The shouldInsertExtraNopBytesForCodeAlign() need STI to check whether relax is enabled or not. It is initialized when call setEmitNops. The setEmitNops may not be called in a section which has instructions but is not executable. In this case uninitialized STI will cause problems. Thus, check hasEmitNops before call it. Fixes: llvm#76552 (comment)
MaskRay
added a commit
that referenced
this pull request
May 29, 2025
Commit bb03cdc caused a Linux kernel regression ClangBuiltLinux/linux#2091 When a section contains linker-relaxable MCAlignmentFragment but no linker-relaxable instructions, the RISCVAsmBackend::isPCRelFixupResolved code path should be taken as well. The #76552 condition in the fragment walk code will make the fixup unresolvable, leading to a relocation.
svkeerthy
pushed a commit
that referenced
this pull request
May 29, 2025
Commit bb03cdc caused a Linux kernel regression ClangBuiltLinux/linux#2091 When a section contains linker-relaxable MCAlignmentFragment but no linker-relaxable instructions, the RISCVAsmBackend::isPCRelFixupResolved code path should be taken as well. The #76552 condition in the fragment walk code will make the fixup unresolvable, leading to a relocation.
google-yfyang
pushed a commit
to google-yfyang/llvm-project
that referenced
this pull request
May 29, 2025
Commit bb03cdc caused a Linux kernel regression ClangBuiltLinux/linux#2091 When a section contains linker-relaxable MCAlignmentFragment but no linker-relaxable instructions, the RISCVAsmBackend::isPCRelFixupResolved code path should be taken as well. The llvm#76552 condition in the fragment walk code will make the fixup unresolvable, leading to a relocation.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
Commit bb03cdc caused a Linux kernel regression ClangBuiltLinux/linux#2091 When a section contains linker-relaxable MCAlignmentFragment but no linker-relaxable instructions, the RISCVAsmBackend::isPCRelFixupResolved code path should be taken as well. The llvm#76552 condition in the fragment walk code will make the fixup unresolvable, leading to a relocation.
leecheechen
pushed a commit
to leecheechen/llvm-project
that referenced
this pull request
Jun 9, 2025
… need insert nops (llvm#76552) Due to delayed decision for ADD/SUB relocations, RISCV and LoongArch may go slow fragment walk path with available layout. When RISCV (or LoongArch in the future) don't need insert nops, that means relax is disabled. With available layout and not needing insert nops, the size of AlignFragment should be a constant. So we can add it to Displacement for folding A-B. (cherry picked from commit 0731567) Change-Id: I554d6766bd7f688204e956e4a6431574b4c511c9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to delayed decision for ADD/SUB relocations, RISCV and LoongArch may go slow fragment walk path with available layout. When RISCV (or LoongArch in the future) don't need insert nops, that means relax is disabled. With available layout and not needing insert nops, the size of AlignFragment should be a constant. So we can add it to Displacement for folding A-B.