[llvm] [MC][ELF] Emit instructions directly into fragment (PR #94950)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-systemz

Author: Alexis Engelke (aengelke)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/94950.diff


3 Files Affected:

- (modified) llvm/lib/MC/MCELFStreamer.cpp (+12-23) 
- (modified) llvm/lib/MC/MCObjectStreamer.cpp (+2-4) 
- (modified) llvm/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp (-1) 


``````````diff
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!");

``````````

</details>


https://github.com/llvm/llvm-project/pull/94950