From 65b3b4687b4f5da25b36d14efb9d222339cbda83 Mon Sep 17 00:00:00 2001 From: Alexis Engelke 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 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 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(); 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(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 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 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(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(); - insert(CEIF); - CEIF->getContents().append(Code.begin(), Code.end()); - CEIF->setHasInstructions(STI); - return; } else { DF = getContext().allocFragment(); 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 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()) {