[X86][MC][NFC] Reduce the parameters of functions in X86MCCodeEmitter(Part I)

[skan]

Summary

The function in X86MCCodeEmitter has too many parameters to make it look
messy, and some parameters are unnecessary. This is the first patch to
reduce their parameters.

The follwing operations are cheap

unsigned Opcode = MI.getOpcode();
const MCInstrDesc &Desc = MCII.get(Opcode);
uint64_t TSFlags = Desc.TSFlags;

So if we pass a MCInst, we don't need to pass MCInstrDesc;
if we pass a MCInstrDesc, we don't need to pass TSFlags.

[skan]

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

1309–1310

Comment to myself: Plan to fix this "FIXME" in Part2

[pengfei]

It's better to add NFC in the title.
The type of TSFlags is uint64_t instead of const uint64_t, is it possible this value been changed during function call?

[skan]

In D78180#1983011, @pengfei wrote:

It's better to add NFC in the title.
The type of TSFlags is uint64_t instead of const uint64_t, is it possible this value been changed during function call?

As far as I know, TSFlags is constant during function call. (At least in this file it's constant) @craig.topper

[craig.topper]

This patch LGTM. The comments are all for future work.

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

644

While you're cleaning up things in this file? Can you merge this if with the "MemoryOperand != -1" above. The only negative value returned by getMemoryOperandNo is -1 so these ifs are the same. Separate patch.

656

Also fix this to be CamelCase not snake_case.

1309–1310

Pretty sure REX prefix has to come after 0xF2 so I don't think that can be fixed.

1312

Should we just make determineREXPrefix be emitREXPrefix and move the emitByte into it. Its more arguments, but more consistent.

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

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 emitPrefixImpl(unsigned &CurOp, unsigned &CurByte, bool &Rex,
+                      const MCInst &MI, const MCSubtargetInfo &STI,
+                      raw_ostream &OS) const;
 
-  void emitVEXOpcodePrefix(uint64_t TSFlags, unsigned &CurByte, int MemOperand,
-                           const MCInst &MI, const MCInstrDesc &Desc,
+  void emitVEXOpcodePrefix(unsigned &CurByte, int MemOperand, const MCInst &MI,
                            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,
-                        const MCInst &MI, const MCInstrDesc &Desc,
+  bool emitOpcodePrefix(unsigned &CurByte, int MemOperand, const MCInst &MI,
                         const MCSubtargetInfo &STI, raw_ostream &OS) const;
 
-  uint8_t determineREXPrefix(const MCInst &MI, uint64_t TSFlags, int MemOperand,
-                             const MCInstrDesc &Desc) const;
+  uint8_t determineREXPrefix(const MCInst &MI, int MemOperand) const;
 };
 
 } // end anonymous namespace
 
 /// \returns true if this signed displacement fits in a 8-bit sign-extended
 /// field.
 static bool isDisp8(int Value) { return Value == (int8_t)Value; }
 
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,
+void X86MCCodeEmitter::emitPrefixImpl(unsigned &CurOp, unsigned &CurByte,
+                                      bool &Rex, const MCInst &MI,
                                       const MCSubtargetInfo &STI,
                                       raw_ostream &OS) const {
+  uint64_t TSFlags = MCII.get(MI.getOpcode()).TSFlags;
   // 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)) {
void X86MCCodeEmitter::emitPrefixImpl(unsigned &CurOp, unsigned &CurByte,
   }
 
   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);
+    Rex = emitOpcodePrefix(CurByte, MemoryOperand, MI, STI, OS);
   else
-    emitVEXOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS);
+    emitVEXOpcodePrefix(CurByte, MemoryOperand, MI, 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) ||
case X86II::RawFrmMemOffs: {
     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,
+void X86MCCodeEmitter::emitVEXOpcodePrefix(unsigned &CurByte, int MemOperand,
+                                           const MCInst &MI,
                                            raw_ostream &OS) const {
+  const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
+  uint64_t TSFlags = Desc.TSFlags;
+
   assert(!(TSFlags & X86II::LOCK) && "Can't have LOCK VEX.");
 
   uint64_t Encoding = TSFlags & X86II::EncodingMask;
   bool HasEVEX_K = TSFlags & X86II::EVEX_K;
   bool HasVEX_4V = TSFlags & X86II::VEX_4V;
   bool HasEVEX_RC = TSFlags & X86II::EVEX_RC;
 
   // VEX_R: opcode externsion equivalent to REX.R in
else
                    (EVEX_V2 << 3) | EVEX_aaa,
                CurByte, OS);
   }
 }
 
 /// Determine if the MCInst has to be encoded with a X86-64 REX prefix which
 /// specifies 1) 64-bit instructions, 2) non-default operand size, and 3) use
 /// of X86-64 extended registers.
-uint8_t X86MCCodeEmitter::determineREXPrefix(const MCInst &MI, uint64_t TSFlags,
-                                             int MemOperand,
-                                             const MCInstrDesc &Desc) const {
+uint8_t X86MCCodeEmitter::determineREXPrefix(const MCInst &MI,
+                                             int MemOperand) const {
   uint8_t REX = 0;
   bool UsesHighByteReg = false;
 
+  const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
+  uint64_t TSFlags = Desc.TSFlags;
+
   if (TSFlags & X86II::REX_W)
     REX |= 1 << 3; // set REX.W
 
   if (MI.getNumOperands() == 0)
     return REX;
 
   unsigned NumOps = MI.getNumOperands();
   unsigned CurOp = X86II::getOperandBias(Desc);

 }
 
 /// Emit all instruction prefixes prior to the opcode.
 ///
 /// \param MemOperand the operand # of the start of a memory operand if present.
 /// If not present, it is -1.
 ///
 /// \returns true if a REX prefix was used.
-bool X86MCCodeEmitter::emitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
-                                        int MemOperand, const MCInst &MI,
-                                        const MCInstrDesc &Desc,
+bool X86MCCodeEmitter::emitOpcodePrefix(unsigned &CurByte, int MemOperand,
+                                        const MCInst &MI,
                                         const MCSubtargetInfo &STI,
                                         raw_ostream &OS) const {
+  const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
+  uint64_t TSFlags = Desc.TSFlags;
+
   bool Ret = false;
   // Emit the operand size opcode prefix as needed.
   if ((TSFlags & X86II::OpSizeMask) ==
       (STI.hasFeature(X86::Mode16Bit) ? X86II::OpSize32 : X86II::OpSize16))
     emitByte(0x66, CurByte, OS);
 
   // Emit the LOCK opcode prefix.
   if (TSFlags & X86II::LOCK || MI.getFlags() & X86::IP_HAS_LOCK)
bool X86MCCodeEmitter::emitOpcodePrefix(unsigned &CurByte, int MemOperand,
   case X86II::XS: // F3
     emitByte(0xF3, CurByte, OS);
     break;
   case X86II::XD: // F2
     emitByte(0xF2, CurByte, OS);
     break;
   }
 
   // Handle REX prefix.
   // FIXME: Can this come before F2 etc to simplify emission?
   if (STI.hasFeature(X86::Mode64Bit)) {
-    if (uint8_t REX = determineREXPrefix(MI, TSFlags, MemOperand, Desc)) {
+    if (uint8_t REX = determineREXPrefix(MI, MemOperand)) {
       emitByte(0x40 | REX, CurByte, OS);
       Ret = true;
     }
   } else {
     assert(!(TSFlags & X86II::REX_W) && "REX.W requires 64bit mode.");
   }
 
   // 0x0F escape code must be emitted just before the opcode.
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);
+  emitPrefixImpl(CurOp, CurByte, Rex, MI, 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;
 
   // 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;
 
   bool Rex = false;
-  emitPrefixImpl(TSFlags, CurOp, CurByte, Rex, MI, Desc, STI, OS);
+  emitPrefixImpl(CurOp, CurByte, Rex, MI, 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;