[llvm] [MC][ELF] Emit instructions directly into fragment (PR #94950)
Alexis Engelke via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 4 07:46:07 PDT 2024
https://github.com/aengelke updated https://github.com/llvm/llvm-project/pull/94950
>From 65b3b4687b4f5da25b36d14efb9d222339cbda83 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Sat, 8 Jun 2024 13:56:31 +0200
Subject: [PATCH 1/3] [MC][ELF] Emit instructions directly into fragment
Avoid needless copying of instructions and fixups.
---
llvm/lib/MC/MCELFStreamer.cpp | 48 ++++++++++++++-----
llvm/lib/MC/MCObjectStreamer.cpp | 6 +--
.../MCTargetDesc/SystemZMCCodeEmitter.cpp | 1 -
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index e6e6b7d19dff4..4741b0c3a9c4e 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -511,12 +511,6 @@ static void CheckBundleSubtargets(const MCSubtargetInfo *OldSTI,
void MCELFStreamer::emitInstToData(const MCInst &Inst,
const MCSubtargetInfo &STI) {
MCAssembler &Assembler = getAssembler();
- SmallVector<MCFixup, 4> Fixups;
- SmallString<256> Code;
- Assembler.getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
-
- for (auto &Fixup : Fixups)
- fixSymbolsInTLSFixups(Fixup.getValue());
// There are several possibilities here:
//
@@ -535,7 +529,16 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// the group, though.
MCDataFragment *DF;
+ // When bundling is enabled, we can't just append to the data fragment, as it
+ // might need to be a MCCompactEncodedInstFragment for zero fixups.
if (Assembler.isBundlingEnabled()) {
+ SmallVector<MCFixup, 4> Fixups;
+ SmallString<256> Code;
+ Assembler.getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
+
+ for (auto &Fixup : Fixups)
+ fixSymbolsInTLSFixups(Fixup.getValue());
+
MCSection &Sec = *getCurrentSectionOnly();
if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
// If we are bundle-locked, we re-use the current fragment.
@@ -546,6 +549,9 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// Optimize memory usage by emitting the instruction to a
// MCCompactEncodedInstFragment when not in a bundle-locked group and
// there are no fixups registered.
+ //
+ // Apparently, this is not just a performance optimization? Using an
+ // MCDataFragment at this point causes test failures.
MCCompactEncodedInstFragment *CEIF =
getContext().allocFragment<MCCompactEncodedInstFragment>();
insert(CEIF);
@@ -567,21 +573,39 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// We're now emitting an instruction in a bundle group, so this flag has
// to be turned off.
Sec.setBundleGroupBeforeFirstInst(false);
- } else {
- DF = getOrCreateDataFragment(&STI);
+
+ for (auto &Fixup : Fixups) {
+ Fixup.setOffset(Fixup.getOffset() + DF->getContents().size());
+ DF->getFixups().push_back(Fixup);
+ }
+
+ DF->setHasInstructions(STI);
+ if (!Fixups.empty() && Fixups.back().getTargetKind() ==
+ getAssembler().getBackend().RelaxFixupKind)
+ DF->setLinkerRelaxable();
+
+ DF->getContents().append(Code.begin(), Code.end());
+ return;
}
- // Add the fixups and data.
+ DF = getOrCreateDataFragment(&STI);
+
+ // Emit instruction directly into data fragment.
+ size_t FixupStartIndex = DF->getFixups().size();
+ size_t CodeOffset = DF->getContents().size();
+ Assembler.getEmitter().encodeInstruction(Inst, DF->getContents(),
+ DF->getFixups(), STI);
+
+ auto Fixups = MutableArrayRef(DF->getFixups()).slice(FixupStartIndex);
for (auto &Fixup : Fixups) {
- Fixup.setOffset(Fixup.getOffset() + DF->getContents().size());
- DF->getFixups().push_back(Fixup);
+ Fixup.setOffset(Fixup.getOffset() + CodeOffset);
+ fixSymbolsInTLSFixups(Fixup.getValue());
}
DF->setHasInstructions(STI);
if (!Fixups.empty() && Fixups.back().getTargetKind() ==
getAssembler().getBackend().RelaxFixupKind)
DF->setLinkerRelaxable();
- DF->getContents().append(Code.begin(), Code.end());
}
void MCELFStreamer::emitBundleAlignMode(Align Alignment) {
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index afe5da6e5fbb9..a72e34fe6fd33 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -393,10 +393,8 @@ void MCObjectStreamer::emitInstToFragment(const MCInst &Inst,
getContext().allocFragment<MCRelaxableFragment>(Inst, STI);
insert(IF);
- SmallString<128> Code;
- getAssembler().getEmitter().encodeInstruction(Inst, Code, IF->getFixups(),
- STI);
- IF->getContents().append(Code.begin(), Code.end());
+ getAssembler().getEmitter().encodeInstruction(Inst, IF->getContents(),
+ IF->getFixups(), STI);
}
#ifndef NDEBUG
diff --git a/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp b/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
index a6285a2ccf9d1..b161eed95d6e2 100644
--- a/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
+++ b/llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
@@ -172,7 +172,6 @@ uint64_t SystemZMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNum,
uint32_t BitOffset = MIBitSize - RawBitOffset - OpBitSize;
Fixups.push_back(MCFixup::create(BitOffset >> 3, MO.getExpr(),
(MCFixupKind)Kind, MI.getLoc()));
- assert(Fixups.size() <= 2 && "More than two memory operands in MI?");
return 0;
}
llvm_unreachable("Unexpected operand type!");
>From 2e10b945bd17b7980661f9345847ca9263313628 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Tue, 2 Jul 2024 16:28:43 +0000
Subject: [PATCH 2/3] Simplify code by removing MCCompactEncodedInstFragment
---
llvm/lib/MC/MCELFStreamer.cpp | 41 +++--------------------------------
1 file changed, 3 insertions(+), 38 deletions(-)
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 4741b0c3a9c4e..66b9aff64b7b9 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -520,9 +520,7 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
//
// If bundling is enabled:
// - If we're not in a bundle-locked group, emit the instruction into a
- // fragment of its own. If there are no fixups registered for the
- // instruction, emit a MCCompactEncodedInstFragment. Otherwise, emit a
- // MCDataFragment.
+ // fragment of its own.
// - If we're in a bundle-locked group, append the instruction to the current
// data fragment because we want all the instructions in a group to get into
// the same fragment. Be careful not to do that for the first instruction in
@@ -532,32 +530,12 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// When bundling is enabled, we can't just append to the data fragment, as it
// might need to be a MCCompactEncodedInstFragment for zero fixups.
if (Assembler.isBundlingEnabled()) {
- SmallVector<MCFixup, 4> Fixups;
- SmallString<256> Code;
- Assembler.getEmitter().encodeInstruction(Inst, Code, Fixups, STI);
-
- for (auto &Fixup : Fixups)
- fixSymbolsInTLSFixups(Fixup.getValue());
-
MCSection &Sec = *getCurrentSectionOnly();
if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
// If we are bundle-locked, we re-use the current fragment.
// The bundle-locking directive ensures this is a new data fragment.
DF = cast<MCDataFragment>(getCurrentFragment());
CheckBundleSubtargets(DF->getSubtargetInfo(), &STI);
- } else if (!isBundleLocked() && Fixups.size() == 0) {
- // Optimize memory usage by emitting the instruction to a
- // MCCompactEncodedInstFragment when not in a bundle-locked group and
- // there are no fixups registered.
- //
- // Apparently, this is not just a performance optimization? Using an
- // MCDataFragment at this point causes test failures.
- MCCompactEncodedInstFragment *CEIF =
- getContext().allocFragment<MCCompactEncodedInstFragment>();
- insert(CEIF);
- CEIF->getContents().append(Code.begin(), Code.end());
- CEIF->setHasInstructions(STI);
- return;
} else {
DF = getContext().allocFragment<MCDataFragment>();
insert(DF);
@@ -573,23 +551,10 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// We're now emitting an instruction in a bundle group, so this flag has
// to be turned off.
Sec.setBundleGroupBeforeFirstInst(false);
-
- for (auto &Fixup : Fixups) {
- Fixup.setOffset(Fixup.getOffset() + DF->getContents().size());
- DF->getFixups().push_back(Fixup);
- }
-
- DF->setHasInstructions(STI);
- if (!Fixups.empty() && Fixups.back().getTargetKind() ==
- getAssembler().getBackend().RelaxFixupKind)
- DF->setLinkerRelaxable();
-
- DF->getContents().append(Code.begin(), Code.end());
- return;
+ } else {
+ DF = getOrCreateDataFragment(&STI);
}
- DF = getOrCreateDataFragment(&STI);
-
// Emit instruction directly into data fragment.
size_t FixupStartIndex = DF->getFixups().size();
size_t CodeOffset = DF->getContents().size();
>From de0a823edf92cde44bfcd15fed6bd913f730b439 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Thu, 4 Jul 2024 16:44:49 +0200
Subject: [PATCH 3/3] remove outdated comment
---
llvm/lib/MC/MCELFStreamer.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 66b9aff64b7b9..a30a64ba3df5e 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -527,8 +527,6 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// the group, though.
MCDataFragment *DF;
- // When bundling is enabled, we can't just append to the data fragment, as it
- // might need to be a MCCompactEncodedInstFragment for zero fixups.
if (Assembler.isBundlingEnabled()) {
MCSection &Sec = *getCurrentSectionOnly();
if (isBundleLocked() && !Sec.isBundleGroupBeforeFirstInst()) {
More information about the llvm-commits
mailing list