[MC][ELF] Emit instructions directly into fragment #94950
Merged
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
Is that related to @dschuff https://discourse.llvm.org/t/state-of-nacl-in-monorepo/55118 |
Yeah, exactly that. I'd also be in favor of removing. |
MaskRay
added a commit
that referenced
this pull request
Jun 14, 2024
When both aligned bundling and RelaxAll are enabled, bundle padding is directly written into fragments (https://reviews.llvm.org/D8072). (The original motivation was memory usage, which has been achieved from different angles with recent assembler improvement). The code presents challenges with the work to replace fragment representation (e.g. #94950 #95077). This patch removes the special handling. RelaxAll still works but the behavior seems slightly different as revealed by 2 changed tests. However, most `-mc-relax-all` tests are unchanged. RelaxAll used to be the default for clang -O0. This mode has significant code size drawbacks and newer Clang doesn't use it (#90013). --- flushPendingLabels: The FOffset parameter can be removed: pending labels will be assigned to the incoming fragment at offset 0. Pull Request: #95188
Avoid needless copying of instructions and fixups.
aengelke
force-pushed
the
perf/mc-emit-into-fragment
branch
from
49f835c to
65b3b46
Compare
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-systemz Author: Alexis Engelke (aengelke) ChangesAvoid needless copying of instructions and fixups and directly emit into the fragment small vectors. This (optionally, second commit) also removes the single use of the MCCompactEncodedInstFragment to simplify code. Full diff: https://github.com/llvm/llvm-project/pull/94950.diff 3 Files Affected:
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index e6e6b7d19dff4..66b9aff64b7b9 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:
//
@@ -526,15 +520,15 @@ 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
// 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()) {
@@ -542,16 +536,6 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
// 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.
- 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);
@@ -571,17 +555,22 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
DF = getOrCreateDataFragment(&STI);
}
- // Add the fixups and data.
+ // 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!");
|
|
Nice! With I think there will be some memory consumption bump for aligned bundling, but it is unlikely anyone cares about that particularly now. |
MaskRay
approved these changes
Jul 3, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/801 Here is the relevant piece of the build log for the reference: |
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
Avoid needless copying of instructions and fixups and directly emit into the fragment small vectors. This (optionally, second commit) also removes the single use of the MCCompactEncodedInstFragment to simplify code.
MaskRay
added a commit
that referenced
this pull request
Jul 29, 2024
This has been used after #94950.
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.
Avoid needless copying of instructions and fixups and directly emit into the fragment small vectors.
This (optionally, second commit) also removes the single use of the MCCompactEncodedInstFragment to simplify code.