Add an interface emitPrefix for MCCodeEmitter

[skan]

Summary

To support for prefix padding to align branches within 32-Byte boundary D70157, we need to know the number of the existing prefixes of instructions , since too many prefixes before one instruction can be a performance issue on some microarchitecture.

The code of emitting prefixes is extracted out from the function encodeInstruction as emitPrefix, then we can emit the prefixes into a dummy stream, and count the prefixes of each instruction.

[craig.topper]

This description needs more context about what the end goal of this is for. Prefixes are a very X86 specific concept. Most of the targets in tree use fixed instruction widths.

[craig.topper]

Can we move the X86MCCodeEmitter definition to a header file and create a separate X86MCCodeEmitter belonging to the X86AsmBackend.cpp that that we can call the emitPrefix on? I'm not sure we want to expose the emitPrefix interface on the generic MCCodeEmitter interface.

[skan]

In D72047#1800290, @craig.topper wrote:

Can we move the X86MCCodeEmitter definition to a header file and create a separate X86MCCodeEmitter belonging to the X86AsmBackend.cpp that that we can call the emitPrefix on? I'm not sure we want to expose the emitPrefix interface on the generic MCCodeEmitter interface.

Currently, MCObjectStreamer owns a MCAssembler ,which owns a MCAsmBackend and a MCCodeEmitter, so I don't think making X86AsmBackend have a X86MCCodeEmitter object member is a good design. Currently, I plan to call the emitPrefix as OS.getAssembler().getEmitter().emitPrefix() in X86AsmBackend.cpp, where OS is a MCObjectStreamer object.

[craig.topper]

My thought was just that the encoder only has 2 member variables in it so its quite small to create. And it would avoid exposing the concept of prefixes to the MCCodeEmitter interface. But lets see what other reviewers think.

[craig.topper]

In D72047#1800301, @craig.topper wrote:

My thought was just that the encoder only has 2 member variables in it so its quite small to create. And it would avoid exposing the concept of prefixes to the MCCodeEmitter interface. But lets see what other reviewers think.

But I guess we don't have a MCContext object in X86AsmBackend so we can't run the X86MCCodeEmitter constructor so maybe that doesn't work at all.

[skan]

Rebase

[craig.topper]

llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

1365

When would emitPrefix be called on a X86II::Pseudo?

1419–1422

Why is Pseudo no longer in this switch?

[skan]

llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

1365

I don't know, encodeInstruction checks (TSFlags & X86II::FormMask) == X86II::Pseudo, so I do the same for emitPrefix. My thought is emitPrefix can be used anywhere where encodeInstruction can be used.

1419–1422

encodeInstruction always calls emitPrefixImpl, which already checks
Pseudo in the corresponding switch.

[craig.topper]

llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

1365

Ok thanks. I missed that.

1419–1422

Can you put it back just so that all possible values are mentioned by name in the switch. I think you can remove it from the emitPrefixImpl switch and just let it go to the default case.

[craig.topper]

LGTM

llvm/include/llvm/MC/MCCodeEmitter.h

 public:
   MCCodeEmitter(const MCCodeEmitter &) = delete;
   MCCodeEmitter &operator=(const MCCodeEmitter &) = delete;
   virtual ~MCCodeEmitter();
 
   /// Lifetime management
   virtual void reset() {}
 
+  /// Emit the prefixes of given instruction on the output stream.
+  ///
+  /// \param Inst a single low-level machine instruction.
+  /// \param OS output stream.
+  virtual void emitPrefix(const MCInst &Inst, raw_ostream &OS,
+                          const MCSubtargetInfo &STI) const {}
   /// EncodeInstruction - Encode the given \p Inst to bytes on the output
   /// stream \p OS.
   virtual void encodeInstruction(const MCInst &Inst, raw_ostream &OS,
                                  SmallVectorImpl<MCFixup> &Fixups,
                                  const MCSubtargetInfo &STI) const = 0;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCCODEEMITTER_H

llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

 
 public:
   X86MCCodeEmitter(const MCInstrInfo &mcii, MCContext &ctx)
       : MCII(mcii), Ctx(ctx) {}
   X86MCCodeEmitter(const X86MCCodeEmitter &) = delete;
   X86MCCodeEmitter &operator=(const X86MCCodeEmitter &) = delete;
   ~X86MCCodeEmitter() override = default;
 
+  void emitPrefix(const MCInst &MI, raw_ostream &OS,
+                  const MCSubtargetInfo &STI) const override;
+
   void encodeInstruction(const MCInst &MI, raw_ostream &OS,
                          SmallVectorImpl<MCFixup> &Fixups,
                          const MCSubtargetInfo &STI) const override;
 
 private:
   unsigned getX86RegNum(const MCOperand &MO) const {
     return Ctx.getRegisterInfo()->getEncodingValue(MO.getReg()) & 0x7;
   }
void emitSIBByte(unsigned SS, unsigned Index, unsigned Base,
     emitByte(modRMByte(SS, Index, Base), CurByte, OS);
   }
 
   void emitMemModRMByte(const MCInst &MI, unsigned Op, unsigned RegOpcodeField,
                         uint64_t TSFlags, bool Rex, unsigned &CurByte,
                         raw_ostream &OS, SmallVectorImpl<MCFixup> &Fixups,
                         const MCSubtargetInfo &STI) const;
 
+  void emitPrefixImpl(uint64_t TSFlags, unsigned &CurOp, unsigned &CurByte,
+                  bool &Rex, const MCInst &MI, const MCInstrDesc &Desc,
+                  const MCSubtargetInfo &STI, raw_ostream &OS) const;
+
   void emitVEXOpcodePrefix(uint64_t TSFlags, unsigned &CurByte, int MemOperand,
                            const MCInst &MI, const MCInstrDesc &Desc,
                            raw_ostream &OS) const;
 
   void emitSegmentOverridePrefix(unsigned &CurByte, unsigned SegOperand,
                                  const MCInst &MI, raw_ostream &OS) const;
 
   bool emitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte, int MemOperand,
void X86MCCodeEmitter::emitMemModRMByte(const MCInst &MI, unsigned Op,
   if (ForceDisp8)
     emitImmediate(Disp, MI.getLoc(), 1, FK_Data_1, CurByte, OS, Fixups,
                   ImmOffset);
   else if (ForceDisp32 || Disp.getImm() != 0)
     emitImmediate(Disp, MI.getLoc(), 4, MCFixupKind(X86::reloc_signed_4byte),
                   CurByte, OS, Fixups);
 }
 
+void X86MCCodeEmitter::emitPrefixImpl(uint64_t TSFlags, unsigned &CurOp,
+                                  unsigned &CurByte, bool &Rex,
+                                  const MCInst &MI, const MCInstrDesc &Desc,
+                                  const MCSubtargetInfo &STI,
+                                  raw_ostream &OS) const {
+  // Determine where the memory operand starts, if present.
+  int MemoryOperand = X86II::getMemoryOperandNo(TSFlags);
+  if (MemoryOperand != -1)
+    MemoryOperand += CurOp;
+
+  // Emit segment override opcode prefix as needed.
+  if (MemoryOperand >= 0)
+    emitSegmentOverridePrefix(CurByte, MemoryOperand + X86::AddrSegmentReg, MI,
+                              OS);
+
+  // Emit the repeat opcode prefix as needed.
+  unsigned Flags = MI.getFlags();
+  if (TSFlags & X86II::REP || Flags & X86::IP_HAS_REPEAT)
+    emitByte(0xF3, CurByte, OS);
+  if (Flags & X86::IP_HAS_REPEAT_NE)
+    emitByte(0xF2, CurByte, OS);
+
+  // Emit the address size opcode prefix as needed.
+  bool need_address_override;
+  uint64_t AdSize = TSFlags & X86II::AdSizeMask;
+  if ((STI.hasFeature(X86::Mode16Bit) && AdSize == X86II::AdSize32) ||
+      (STI.hasFeature(X86::Mode32Bit) && AdSize == X86II::AdSize16) ||
+      (STI.hasFeature(X86::Mode64Bit) && AdSize == X86II::AdSize32)) {
+    need_address_override = true;
+  } else if (MemoryOperand < 0) {
+    need_address_override = false;
+  } else if (STI.hasFeature(X86::Mode64Bit)) {
+    assert(!is16BitMemOperand(MI, MemoryOperand, STI));
+    need_address_override = is32BitMemOperand(MI, MemoryOperand);
+  } else if (STI.hasFeature(X86::Mode32Bit)) {
+    assert(!is64BitMemOperand(MI, MemoryOperand));
+    need_address_override = is16BitMemOperand(MI, MemoryOperand, STI);
+  } else {
+    assert(STI.hasFeature(X86::Mode16Bit));
+    assert(!is64BitMemOperand(MI, MemoryOperand));
+    need_address_override = !is16BitMemOperand(MI, MemoryOperand, STI);
+  }
+
+  if (need_address_override)
+    emitByte(0x67, CurByte, OS);
+
+  // Encoding type for this instruction.
+  uint64_t Encoding = TSFlags & X86II::EncodingMask;
+  if (Encoding == 0)
+    Rex = emitOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, STI, OS);
+  else
+    emitVEXOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS);
+
+  uint64_t Form = TSFlags & X86II::FormMask;
+  switch (Form) {
+  default:
+    break;
+  case X86II::RawFrmDstSrc: {
+    unsigned siReg = MI.getOperand(1).getReg();
+    assert(((siReg == X86::SI && MI.getOperand(0).getReg() == X86::DI) ||
+            (siReg == X86::ESI && MI.getOperand(0).getReg() == X86::EDI) ||
+            (siReg == X86::RSI && MI.getOperand(0).getReg() == X86::RDI)) &&
+           "SI and DI register sizes do not match");
+    // Emit segment override opcode prefix as needed (not for %ds).
+    if (MI.getOperand(2).getReg() != X86::DS)
+      emitSegmentOverridePrefix(CurByte, 2, MI, OS);
+    // Emit AdSize prefix as needed.
+    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
+        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
+      emitByte(0x67, CurByte, OS);
+    CurOp += 3; // Consume operands.
+    break;
+  }
+  case X86II::RawFrmSrc: {
+    unsigned siReg = MI.getOperand(0).getReg();
+    // Emit segment override opcode prefix as needed (not for %ds).
+    if (MI.getOperand(1).getReg() != X86::DS)
+      emitSegmentOverridePrefix(CurByte, 1, MI, OS);
+    // Emit AdSize prefix as needed.
+    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
+        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
+      emitByte(0x67, CurByte, OS);
+    CurOp += 2; // Consume operands.
+    break;
+  }
+  case X86II::RawFrmDst: {
+    unsigned siReg = MI.getOperand(0).getReg();
+    // Emit AdSize prefix as needed.
+    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::EDI) ||
+        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::DI))
+      emitByte(0x67, CurByte, OS);
+    ++CurOp; // Consume operand.
+    break;
+  }
+  case X86II::RawFrmMemOffs: {
+    // Emit segment override opcode prefix as needed.
+    emitSegmentOverridePrefix(CurByte, 1, MI, OS);
+    break;
+  }
+  }
+}
+
 /// emitVEXOpcodePrefix - AVX instructions are encoded using a opcode prefix
 /// called VEX.
 void X86MCCodeEmitter::emitVEXOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
                                            int MemOperand, const MCInst &MI,
                                            const MCInstrDesc &Desc,
                                            raw_ostream &OS) const {
   assert(!(TSFlags & X86II::LOCK) && "Can't have LOCK VEX.");
 
case X86II::T8: // 0F 38
     break;
   case X86II::TA: // 0F 3A
     emitByte(0x3A, CurByte, OS);
     break;
   }
   return Ret;
 }
 
+void X86MCCodeEmitter::emitPrefix(const MCInst &MI, raw_ostream &OS,
+                                  const MCSubtargetInfo &STI) const {
+  unsigned Opcode = MI.getOpcode();
+  const MCInstrDesc &Desc = MCII.get(Opcode);
+  uint64_t TSFlags = Desc.TSFlags;
+
+  // Pseudo instructions don't get encoded.
+  if ((TSFlags & X86II::FormMask) == X86II::Pseudo)
+    return;
+
+  unsigned CurOp = X86II::getOperandBias(Desc);
+
+  // Keep track of the current byte being emitted.
+  unsigned CurByte = 0;
+
+  bool Rex = false;
+  emitPrefixImpl(TSFlags, CurOp, CurByte, Rex, MI, Desc, STI, OS);
+}
+
 void X86MCCodeEmitter::encodeInstruction(const MCInst &MI, raw_ostream &OS,
                                          SmallVectorImpl<MCFixup> &Fixups,
                                          const MCSubtargetInfo &STI) const {
   unsigned Opcode = MI.getOpcode();
   const MCInstrDesc &Desc = MCII.get(Opcode);
   uint64_t TSFlags = Desc.TSFlags;
-  unsigned Flags = MI.getFlags();
 
   // Pseudo instructions don't get encoded.
   if ((TSFlags & X86II::FormMask) == X86II::Pseudo)
     return;
 
   unsigned NumOps = Desc.getNumOperands();
   unsigned CurOp = X86II::getOperandBias(Desc);
 
   // Keep track of the current byte being emitted.
   unsigned CurByte = 0;
 
-  // Encoding type for this instruction.
-  uint64_t Encoding = TSFlags & X86II::EncodingMask;
+  bool Rex = false;
+  emitPrefixImpl(TSFlags, CurOp, CurByte, Rex, MI, Desc, STI, OS);
 
   // It uses the VEX.VVVV field?
   bool HasVEX_4V = TSFlags & X86II::VEX_4V;
   bool HasVEX_I8Reg = (TSFlags & X86II::ImmMask) == X86II::Imm8Reg;
 
   // It uses the EVEX.aaa field?
   bool HasEVEX_K = TSFlags & X86II::EVEX_K;
   bool HasEVEX_RC = TSFlags & X86II::EVEX_RC;
 
   // Used if a register is encoded in 7:4 of immediate.
   unsigned I8RegNum = 0;
 
-  // Determine where the memory operand starts, if present.
-  int MemoryOperand = X86II::getMemoryOperandNo(TSFlags);
-  if (MemoryOperand != -1)
-    MemoryOperand += CurOp;
-
-  // Emit segment override opcode prefix as needed.
-  if (MemoryOperand >= 0)
-    emitSegmentOverridePrefix(CurByte, MemoryOperand + X86::AddrSegmentReg, MI,
-                              OS);
-
-  // Emit the repeat opcode prefix as needed.
-  if (TSFlags & X86II::REP || Flags & X86::IP_HAS_REPEAT)
-    emitByte(0xF3, CurByte, OS);
-  if (Flags & X86::IP_HAS_REPEAT_NE)
-    emitByte(0xF2, CurByte, OS);
-
-  // Emit the address size opcode prefix as needed.
-  bool need_address_override;
-  uint64_t AdSize = TSFlags & X86II::AdSizeMask;
-  if ((STI.hasFeature(X86::Mode16Bit) && AdSize == X86II::AdSize32) ||
-      (STI.hasFeature(X86::Mode32Bit) && AdSize == X86II::AdSize16) ||
-      (STI.hasFeature(X86::Mode64Bit) && AdSize == X86II::AdSize32)) {
-    need_address_override = true;
-  } else if (MemoryOperand < 0) {
-    need_address_override = false;
-  } else if (STI.hasFeature(X86::Mode64Bit)) {
-    assert(!is16BitMemOperand(MI, MemoryOperand, STI));
-    need_address_override = is32BitMemOperand(MI, MemoryOperand);
-  } else if (STI.hasFeature(X86::Mode32Bit)) {
-    assert(!is64BitMemOperand(MI, MemoryOperand));
-    need_address_override = is16BitMemOperand(MI, MemoryOperand, STI);
-  } else {
-    assert(STI.hasFeature(X86::Mode16Bit));
-    assert(!is64BitMemOperand(MI, MemoryOperand));
-    need_address_override = !is16BitMemOperand(MI, MemoryOperand, STI);
-  }
-
-  if (need_address_override)
-    emitByte(0x67, CurByte, OS);
-
-  bool Rex = false;
-  if (Encoding == 0)
-    Rex = emitOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, STI, OS);
-  else
-    emitVEXOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS);
-
   uint8_t BaseOpcode = X86II::getBaseOpcodeFor(TSFlags);
 
   if ((TSFlags & X86II::OpMapMask) == X86II::ThreeDNow)
     BaseOpcode = 0x0F; // Weird 3DNow! encoding.
 
   unsigned OpcodeOffset = 0;
 
   uint64_t Form = TSFlags & X86II::FormMask;
   switch (Form) {
   default:
     errs() << "FORM: " << Form << "\n";
     llvm_unreachable("Unknown FormMask value in X86MCCodeEmitter!");
   case X86II::Pseudo:
     llvm_unreachable("Pseudo instruction shouldn't be emitted");
-  case X86II::RawFrmDstSrc: {
-    unsigned siReg = MI.getOperand(1).getReg();
-    assert(((siReg == X86::SI && MI.getOperand(0).getReg() == X86::DI) ||
-            (siReg == X86::ESI && MI.getOperand(0).getReg() == X86::EDI) ||
-            (siReg == X86::RSI && MI.getOperand(0).getReg() == X86::RDI)) &&
-           "SI and DI register sizes do not match");
-    // Emit segment override opcode prefix as needed (not for %ds).
-    if (MI.getOperand(2).getReg() != X86::DS)
-      emitSegmentOverridePrefix(CurByte, 2, MI, OS);
-    // Emit AdSize prefix as needed.
-    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
-        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
-      emitByte(0x67, CurByte, OS);
-    CurOp += 3; // Consume operands.
+  case X86II::RawFrmDstSrc:
+  case X86II::RawFrmSrc:
+  case X86II::RawFrmDst:
     emitByte(BaseOpcode, CurByte, OS);
     break;
-  }
-  case X86II::RawFrmSrc: {
-    unsigned siReg = MI.getOperand(0).getReg();
-    // Emit segment override opcode prefix as needed (not for %ds).
-    if (MI.getOperand(1).getReg() != X86::DS)
-      emitSegmentOverridePrefix(CurByte, 1, MI, OS);
-    // Emit AdSize prefix as needed.
-    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
-        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
-      emitByte(0x67, CurByte, OS);
-    CurOp += 2; // Consume operands.
-    emitByte(BaseOpcode, CurByte, OS);
-    break;
-  }
-  case X86II::RawFrmDst: {
-    unsigned siReg = MI.getOperand(0).getReg();
-    // Emit AdSize prefix as needed.
-    if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::EDI) ||
-        (STI.hasFeature(X86::Mode32Bit) && siReg == X86::DI))
-      emitByte(0x67, CurByte, OS);
-    ++CurOp; // Consume operand.
-    emitByte(BaseOpcode, CurByte, OS);
-    break;
-  }
   case X86II::AddCCFrm: {
     // This will be added to the opcode in the fallthrough.
     OpcodeOffset = MI.getOperand(NumOps - 1).getImm();
     assert(OpcodeOffset < 16 && "Unexpected opcode offset!");
     --NumOps; // Drop the operand from the end.
     LLVM_FALLTHROUGH;
   case X86II::RawFrm:
     emitByte(BaseOpcode + OpcodeOffset, CurByte, OS);
 
     if (!STI.hasFeature(X86::Mode64Bit) || !isPCRel32Branch(MI, MCII))
       break;
 
     const MCOperand &Op = MI.getOperand(CurOp++);
     emitImmediate(Op, MI.getLoc(), X86II::getSizeOfImm(TSFlags),
                   MCFixupKind(X86::reloc_branch_4byte_pcrel), CurByte, OS,
                   Fixups);
     break;
   }
   case X86II::RawFrmMemOffs:
-    // Emit segment override opcode prefix as needed.
-    emitSegmentOverridePrefix(CurByte, 1, MI, OS);
     emitByte(BaseOpcode, CurByte, OS);
     emitImmediate(MI.getOperand(CurOp++), MI.getLoc(),
                   X86II::getSizeOfImm(TSFlags), getImmFixupKind(TSFlags),
                   CurByte, OS, Fixups);
     ++CurOp; // skip segment operand
     break;
   case X86II::RawFrmImm8:
     emitByte(BaseOpcode, CurByte, OS);