-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[LoongArch] Allow delayed decision for ADD/SUB relocations #72960
Conversation
Refer to RISCV [1], LoongArch also need delayed decision for ADD/SUB relocations. In handleAddSubRelocations, just return directly if SecA != SecB, handleFixup usually will finish the the rest of creating PCRel relocations works. Otherwise we emit relocs depends on whether relaxation is enabled. If not, we return true and avoid record ADD/SUB relocations. Now the two symbols separated by alignment directive will return without folding symbol offset in AttemptToFoldSymbolOffsetDifference, which has the same effect when relaxation is enabled. [1] https://reviews.llvm.org/D155357
@llvm/pr-subscribers-backend-loongarch @llvm/pr-subscribers-mc Author: Jinyang He (MQ-mengqing) ChangesRefer to RISCV [1], LoongArch also need delayed decision for ADD/SUB relocations. In handleAddSubRelocations, just return directly if SecA != SecB, handleFixup usually will finish the the rest of creating PCRel relocations works. Otherwise we emit relocs depends on whether relaxation is enabled. If not, we return true and avoid record ADD/SUB relocations. [1] https://reviews.llvm.org/D155357 Full diff: https://github.com/llvm/llvm-project/pull/72960.diff 6 Files Affected:
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index 73e6569f96e4630..061f2ad13ffa78b 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -632,7 +632,8 @@ static void AttemptToFoldSymbolOffsetDifference(
// instructions and InSet is false (not expressions in directive like
// .size/.fill), disable the fast path.
if (Layout && (InSet || !SecA.hasInstructions() ||
- !Asm->getContext().getTargetTriple().isRISCV())) {
+ !(Asm->getContext().getTargetTriple().isRISCV() ||
+ Asm->getContext().getTargetTriple().isLoongArch()))) {
// If both symbols are in the same fragment, return the difference of their
// offsets. canGetFragmentOffset(FA) may be false.
if (FA == FB && !SA.isVariable() && !SB.isVariable()) {
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index a35916d2ad2197d..2732f61b534f890 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -176,6 +176,29 @@ bool LoongArchAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
}
}
+static inline std::pair<MCFixupKind, MCFixupKind>
+getRelocPairForSize(unsigned Size) {
+ switch (Size) {
+ default:
+ llvm_unreachable("unsupported fixup size");
+ case 6:
+ return std::make_pair(MCFixupKind(LoongArch::fixup_loongarch_add6),
+ MCFixupKind(LoongArch::fixup_loongarch_sub6));
+ case 8:
+ return std::make_pair(MCFixupKind(LoongArch::fixup_loongarch_add8),
+ MCFixupKind(LoongArch::fixup_loongarch_sub8));
+ case 16:
+ return std::make_pair(MCFixupKind(LoongArch::fixup_loongarch_add16),
+ MCFixupKind(LoongArch::fixup_loongarch_sub16));
+ case 32:
+ return std::make_pair(MCFixupKind(LoongArch::fixup_loongarch_add32),
+ MCFixupKind(LoongArch::fixup_loongarch_sub32));
+ case 64:
+ return std::make_pair(MCFixupKind(LoongArch::fixup_loongarch_add64),
+ MCFixupKind(LoongArch::fixup_loongarch_sub64));
+ }
+}
+
bool LoongArchAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
const MCSubtargetInfo *STI) const {
// We mostly follow binutils' convention here: align to 4-byte boundary with a
@@ -190,6 +213,56 @@ bool LoongArchAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
return true;
}
+bool LoongArchAsmBackend::handleAddSubRelocations(const MCAsmLayout &Layout,
+ const MCFragment &F,
+ const MCFixup &Fixup,
+ const MCValue &Target,
+ uint64_t &FixedValue) const {
+ std::pair<MCFixupKind, MCFixupKind> FK;
+ uint64_t FixedValueA, FixedValueB;
+ const MCSection &SecA = Target.getSymA()->getSymbol().getSection();
+ const MCSection &SecB = Target.getSymB()->getSymbol().getSection();
+
+ // We need record relocation if SecA != SecB. Usually SecB is same as the
+ // section of Fixup, which will be record the relocation as PCRel. If SecB
+ // is not same as the section of Fixup, it will report error. Just return
+ // false and then this work can be finished by handleFixup.
+ if (&SecA != &SecB)
+ return false;
+
+ // In SecA == SecB case. If the linker relaxation is enabled, we need record
+ // the ADD, SUB relocations. Otherwise the FixedValue has already been
+ // calculated out in evaluateFixup, return true and avoid record relocations.
+ if (!STI.hasFeature(LoongArch::FeatureRelax))
+ return true;
+
+ switch (Fixup.getKind()) {
+ case llvm::FK_Data_1:
+ FK = getRelocPairForSize(8);
+ break;
+ case llvm::FK_Data_2:
+ FK = getRelocPairForSize(16);
+ break;
+ case llvm::FK_Data_4:
+ FK = getRelocPairForSize(32);
+ break;
+ case llvm::FK_Data_8:
+ FK = getRelocPairForSize(64);
+ break;
+ default:
+ llvm_unreachable("unsupported fixup size");
+ }
+ MCValue A = MCValue::get(Target.getSymA(), nullptr, Target.getConstant());
+ MCValue B = MCValue::get(Target.getSymB());
+ auto FA = MCFixup::create(Fixup.getOffset(), nullptr, std::get<0>(FK));
+ auto FB = MCFixup::create(Fixup.getOffset(), nullptr, std::get<1>(FK));
+ auto &Asm = Layout.getAssembler();
+ Asm.getWriter().recordRelocation(Asm, Layout, &F, FA, A, FixedValueA);
+ Asm.getWriter().recordRelocation(Asm, Layout, &F, FB, B, FixedValueB);
+ FixedValue = FixedValueA - FixedValueB;
+ return true;
+}
+
std::unique_ptr<MCObjectTargetWriter>
LoongArchAsmBackend::createObjectTargetWriter() const {
return createLoongArchELFObjectWriter(
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
index f840f9fa2b6a007..b1cc40034df68a4 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
@@ -31,10 +31,15 @@ class LoongArchAsmBackend : public MCAsmBackend {
public:
LoongArchAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options)
- : MCAsmBackend(llvm::endianness::little), STI(STI), OSABI(OSABI),
- Is64Bit(Is64Bit), TargetOptions(Options) {}
+ : MCAsmBackend(llvm::endianness::little,
+ LoongArch::fixup_loongarch_relax),
+ STI(STI), OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {}
~LoongArchAsmBackend() override {}
+ bool handleAddSubRelocations(const MCAsmLayout &Layout, const MCFragment &F,
+ const MCFixup &Fixup, const MCValue &Target,
+ uint64_t &FixedValue) const override;
+
void applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
const MCValue &Target, MutableArrayRef<char> Data,
uint64_t Value, bool IsResolved,
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchFixupKinds.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchFixupKinds.h
index ba2d6718cdf9a27..238d4c5044af77a 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchFixupKinds.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchFixupKinds.h
@@ -106,7 +106,20 @@ enum Fixups {
// 20-bit fixup corresponding to %gd_pc_hi20(foo) for instruction pcalau12i.
fixup_loongarch_tls_gd_pc_hi20,
// 20-bit fixup corresponding to %gd_hi20(foo) for instruction lu12i.w.
- fixup_loongarch_tls_gd_hi20
+ fixup_loongarch_tls_gd_hi20,
+ // ADD and SUB fixups.
+ fixup_loongarch_add6 = FirstLiteralRelocationKind + ELF::R_LARCH_ADD6,
+ fixup_loongarch_sub6 = FirstLiteralRelocationKind + ELF::R_LARCH_SUB6,
+ fixup_loongarch_add8 = FirstLiteralRelocationKind + ELF::R_LARCH_ADD8,
+ fixup_loongarch_sub8 = FirstLiteralRelocationKind + ELF::R_LARCH_SUB8,
+ fixup_loongarch_add16 = FirstLiteralRelocationKind + ELF::R_LARCH_ADD16,
+ fixup_loongarch_sub16 = FirstLiteralRelocationKind + ELF::R_LARCH_SUB16,
+ fixup_loongarch_add32 = FirstLiteralRelocationKind + ELF::R_LARCH_ADD32,
+ fixup_loongarch_sub32 = FirstLiteralRelocationKind + ELF::R_LARCH_SUB32,
+ fixup_loongarch_add64 = FirstLiteralRelocationKind + ELF::R_LARCH_ADD64,
+ fixup_loongarch_sub64 = FirstLiteralRelocationKind + ELF::R_LARCH_SUB64,
+ // Generate an R_LARCH_RELAX which indicates the linker may relax here.
+ fixup_loongarch_relax = FirstLiteralRelocationKind + ELF::R_LARCH_RELAX
};
} // end namespace LoongArch
} // end namespace llvm
diff --git a/llvm/test/MC/LoongArch/Misc/subsection.s b/llvm/test/MC/LoongArch/Misc/subsection.s
new file mode 100644
index 000000000000000..0bd22b474536c0e
--- /dev/null
+++ b/llvm/test/MC/LoongArch/Misc/subsection.s
@@ -0,0 +1,38 @@
+# RUN: not llvm-mc --filetype=obj --triple=loongarch64 --mattr=-relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefixes=ERR,NORELAX --implicit-check-not=error:
+## TODO: not llvm-mc --filetype=obj --triple=loongarch64 --mattr=+relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefixes=ERR,RELAX --implicit-check-not=error:
+
+a:
+ nop
+b:
+ la.pcrel $t0, a
+c:
+ nop
+d:
+
+.data
+## Positive subsection numbers
+## With relaxation, report an error as c-b is not an assemble-time constant.
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection c-b
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection d-b
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection c-a
+
+.subsection b-a
+.subsection d-c
+
+## Negative subsection numbers
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -8 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection b-c
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -12 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection b-d
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -12 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
+.subsection a-c
+# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
+.subsection a-b
+# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
+.subsection c-d
diff --git a/llvm/test/MC/LoongArch/Relocations/relax-addsub.s b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
new file mode 100644
index 000000000000000..e090fbaee81c365
--- /dev/null
+++ b/llvm/test/MC/LoongArch/Relocations/relax-addsub.s
@@ -0,0 +1,68 @@
+# RUN: llvm-mc --filetype=obj --triple=loongarch64 --mattr=-relax < %s \
+# RUN: | llvm-readobj -r -x .data - | FileCheck %s --check-prefix=NORELAX
+# RUN: llvm-mc --filetype=obj --triple=loongarch64 --mattr=+relax < %s \
+# RUN: | llvm-readobj -r -x .data - | FileCheck %s --check-prefix=RELAX
+
+# NORELAX: Relocations [
+# NORELAX-NEXT: Section ({{.*}}) .rela.text {
+# NORELAX-NEXT: 0x10 R_LARCH_PCALA_HI20 .text 0x0
+# NORELAX-NEXT: 0x14 R_LARCH_PCALA_LO12 .text 0x0
+# NORELAX-NEXT: }
+# NORELAX-NEXT: ]
+
+# NORELAX: Hex dump of section '.data':
+# NORELAX-NEXT: 0x00000000 04040004 00000004 00000000 0000000c
+# NORELAX-NEXT: 0x00000010 0c000c00 00000c00 00000000 00000808
+# NORELAX-NEXT: 0x00000020 00080000 00080000 00000000 00
+
+# RELAX: Relocations [
+# RELAX-NEXT: Section ({{.*}}) .rela.text {
+# RELAX-NEXT: 0x10 R_LARCH_PCALA_HI20 .L1 0x0
+# RELAX-NEXT: 0x14 R_LARCH_PCALA_LO12 .L1 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: }
+# RELAX-NEXT: ]
+
+# RELAX: Hex dump of section '.data':
+# RELAX-NEXT: 0x00000000 04040004 00000004 00000000 00000000
+# RELAX-NEXT: 0x00000010 00000000 00000000 00000000 00000808
+# RELAX-NEXT: 0x00000020 00080000 00080000 00000000 00
+
+.text
+.L1:
+ nop
+.L2:
+ .align 4
+.L3:
+ la.pcrel $t0, .L1
+.L4:
+ ret
+
+.data
+## Not emit relocs
+.byte .L2 - .L1
+.short .L2 - .L1
+.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().
+.byte .L3 - .L2
+.short .L3 - .L2
+.word .L3 - .L2
+.dword .L3 - .L2
+## TODO
+## With relaxation, emit relocs because la.pcrel is a linker-relaxable inst.
+.byte .L4 - .L3
+.short .L4 - .L3
+.word .L4 - .L3
+.dword .L4 - .L3
|
Add: @MaskRay @SixWeining @wangleiat @xry111 @xen0n . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…nts. 1, update relax-addsub test to check `la.pcrel` RELAX. 2, update subsection test to check RELAX. 3, add check reloc start/end flags. 4, change non-first CHECK to CHECK-NEXT and add CHECK-EMPTY.
This patch causes an error while building the Linux kernel's vDSO:
A trivial reproducer from struct vdso_timestamp {
long sec;
};
struct {
long cycle_last;
struct vdso_timestamp basetime[];
} do_hres_vd;
int vdso_calc_delta_mult, do_hres_clk;
long do_hres_sec;
long vdso_calc_delta() { return vdso_calc_delta_mult; }
int do_hres() {
struct vdso_timestamp vdso_ts = do_hres_vd.basetime[do_hres_clk];
do_hres_sec = vdso_ts.sec;
return 0;
} With GCC 13.2.0:
At the parent of this change:
At this change:
|
Seems a temporary workaround is using DWARF v4 by passing RISCV- currently defaults to DWARF v4: https://reviews.llvm.org/D157663 |
I got the same |
Buildbot failed after this: https://lab.llvm.org/staging/#/builders/106/builds/2820 |
We may want to implement proper |
Follow RISCV 1df5ea2, we can fix it by the diff. I'll add some test cases. BTW, add tests is messy for this RELAX series patches. I reduced the correlation between patches so that patches could be reviewed earlier. Commit in progress may be affected by a previous commit. #72961 is an example where I need to re-modify the test case after #72960 is merged. diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
index 6d8ef1bf96cb..9c080aba2c96 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp
@@ -173,6 +173,7 @@ bool LoongArchAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
case FK_Data_2:
case FK_Data_4:
case FK_Data_8:
+ case FK_Data_leb128:
return !Target.isAbsolute();
}
}
@@ -202,9 +203,25 @@ getRelocPairForSize(unsigned Size) {
return std::make_pair(
MCFixupKind(FirstLiteralRelocationKind + ELF::R_LARCH_ADD64),
MCFixupKind(FirstLiteralRelocationKind + ELF::R_LARCH_SUB64));
+ case 128:
+ return std::make_pair(
+ MCFixupKind(FirstLiteralRelocationKind + ELF::R_LARCH_ADD_ULEB128),
+ MCFixupKind(FirstLiteralRelocationKind + ELF::R_LARCH_SUB_ULEB128));
}
}
+bool LoongArchAsmBackend::relaxLEB128(MCLEBFragment &LF, MCAsmLayout &Layout,
+ int64_t &Value) const {
+ if (LF.isSigned())
+ return false;
+ const MCExpr &Expr = LF.getValue();
+// if (ULEB128Reloc) {
+ LF.getFixups().push_back(
+ MCFixup::create(0, &Expr, FK_Data_leb128, Expr.getLoc()));
+// }
+ return Expr.evaluateKnownAbsolute(Value, Layout);
+}
+
bool LoongArchAsmBackend::writeNopData(raw_ostream &OS, uint64_t Count,
const MCSubtargetInfo *STI) const {
// We mostly follow binutils' convention here: align to 4-byte boundary with a
@@ -255,6 +272,9 @@ bool LoongArchAsmBackend::handleAddSubRelocations(const MCAsmLayout &Layout,
case llvm::FK_Data_8:
FK = getRelocPairForSize(64);
break;
+ case llvm::FK_Data_leb128:
+ FK = getRelocPairForSize(128);
+ break;
default:
llvm_unreachable("unsupported fixup size");
}
diff --git a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
index fef0e84600a7..238f5b931f58 100644
--- a/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
+++ b/llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.h
@@ -66,6 +66,10 @@ public:
void relaxInstruction(MCInst &Inst,
const MCSubtargetInfo &STI) const override {}
+
+ bool relaxLEB128(MCLEBFragment &LF, MCAsmLayout &Layout,
+ int64_t &Value) const override;
+
bool writeNopData(raw_ostream &OS, uint64_t Count,
const MCSubtargetInfo *STI) const override; |
Thanks for volunteering to fix the problem. It's blocking integration work right now.
FYI |
…relocs when sym is not in section 1, Follow RISCV 1df5ea2 to support generates relocs for .uleb128 which can not be folded. Unlike RISCV, the located content of LoongArch should be zero. LoongArch fixup uleb128 value by in-place addition and subtraction reloc types named R_LARCH_{ADD,SUB}_ULEB128. The located content can affect the result and R_LARCH_ADD_ULEB128 has enough info to represent the first symbol value, so it needs to be set to zero. 2, Force relocs if sym is not in section so that it can emit relocs for external symbol. Fixes: llvm#72960 (comment)
Seems that this reduced case and buildbot failures (https://lab.llvm.org/staging/#/builders/106/builds/2820) have been addressed by 0731567. But I'm not sure about the vDSO case. |
While 0731567 does appear to address the vDSO error, as I note on its pull request, that change has other issues. Additionally, this change (a8081ed) has an assertion failure that I only uncovered when working around the vDSO error (as that causes the build to error quite early):
Luckily, that issue appears to be addressed by #76433 as well, so getting that merged would be good (I didn't bother with a reproducer due to this but I can come up with one if desired). |
…relocs when sym is not in section (#76433) 1, Follow RISCV 1df5ea2 to support generates relocs for .uleb128 which can not be folded. Unlike RISCV, the located content of LoongArch should be zero. LoongArch fixup uleb128 value by in-place addition and subtraction reloc types named R_LARCH_{ADD,SUB}_ULEB128. The located content can affect the result and R_LARCH_ADD_ULEB128 has enough info to represent the first symbol value, so it needs to be set to zero. 2, Force relocs if sym is not in section so that it can emit relocs for external symbol. Fixes: #72960 (comment)
…relocs when sym is not in section (llvm#76433) 1, Follow RISCV 1df5ea2 to support generates relocs for .uleb128 which can not be folded. Unlike RISCV, the located content of LoongArch should be zero. LoongArch fixup uleb128 value by in-place addition and subtraction reloc types named R_LARCH_{ADD,SUB}_ULEB128. The located content can affect the result and R_LARCH_ADD_ULEB128 has enough info to represent the first symbol value, so it needs to be set to zero. 2, Force relocs if sym is not in section so that it can emit relocs for external symbol. Fixes: llvm#72960 (comment)
Refer to RISCV [1], LoongArch also need delayed decision for ADD/SUB relocations. In handleAddSubRelocations, just return directly if SecA != SecB, handleFixup usually will finish the the rest of creating PCRel relocations works. Otherwise we emit relocs depends on whether relaxation is enabled. If not, we return true and avoid record ADD/SUB relocations.
Now the two symbols separated by alignment directive will return without folding symbol offset in AttemptToFoldSymbolOffsetDifference, which has the same effect when relaxation is enabled.
[1] https://reviews.llvm.org/D155357