[X86] Add a private member function determinePaddingPrefix for X86AsmBackend

[skan]

Summary

X86 can reduce the bytes of NOP by padding instructions with prefixes to get a better peformance in some cases. So a private member function determinePaddingPrefix is added to determine which prefix is the most suitable.

[skan]

Lowercase the first character of the function name and make the function private

[skan]

delete the unintentional static

[skan]

Add one comment for the encoding values for segment overide prefixes.

[skan]

Fix the error reported by clang-tidy

[reames]

LGTM

[skan]

@craig.topper the logic of checking the existing segment override prefix for instruction is kind of complicated, does it look good to you?

[MaskRay]

Do you have a plan how determinePaddingPrefix will be used? Without a plan, there may be some churn that moves things around. The plan should also include how the complex logic will be tested eventually.

[skan]

In D75357#1902293, @MaskRay wrote:

Do you have a plan how determinePaddingPrefix will be used? Without a plan, there may be some churn that moves things around. The plan should also include how the complex logic will be tested eventually.

Philip prototyped what prefix padding for relaxable instructions might look like in D75203, determinePaddingPrefix is planed to be used to determine the appropriate prefix in the framework.

[MaskRay]

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

357

The variable Opcode can be omitted.

359

Consider omitting TSFlags. It is used just twice.

361

The variable CurOp can be omitted.

375

The variable Form can be omitted.

[skan]

Address review comments

[skan]

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

359

Prefer to keep unchanged.

[glider]

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

350

Hi Shengchen,

Out of curiosity, why does this feature use CS instead of DS for instruction padding?

Also, is there an easy way to trigger this behavior in Clang? I couldn't find any tests for this functionality.

Thanks!

[skan]

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

350

Hi @glider. the behavior here is to align with GAS, I don't know if there is a specific reason to use CS here. I doubt it's more like a decision.

You can find the tests here

llvm/test/MC/X86/prefix-padding-64.s
llvm/test/MC/X86/prefix-padding-32.s

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

 };
 
 class X86AsmBackend : public MCAsmBackend {
   const MCSubtargetInfo &STI;
   std::unique_ptr<const MCInstrInfo> MCII;
   X86AlignBranchKind AlignBranchType;
   Align AlignBoundary;
 
+  uint8_t determinePaddingPrefix(const MCInst &Inst) const;
+
   bool isMacroFused(const MCInst &Cmp, const MCInst &Jcc) const;
 
   bool needAlign(MCObjectStreamer &OS) const;
   bool needAlignInst(const MCInst &Inst) const;
   MCInst PrevInst;
 
 public:
   X86AsmBackend(const Target &T, const MCSubtargetInfo &STI)
static bool isFirstMacroFusibleInst(const MCInst &Inst,
   // An Intel instruction with RIP relative addressing is not macro fusible.
   if (isRIPRelative(Inst, MCII))
     return false;
   X86::FirstMacroFusionInstKind FIK =
       X86::classifyFirstOpcodeInMacroFusion(Inst.getOpcode());
   return FIK != X86::FirstMacroFusionInstKind::Invalid;
 }
 
+/// X86 can reduce the bytes of NOP by padding instructions with prefixes to
+/// get a better peformance in some cases. Here, we determine which prefix is
+/// the most suitable.
+///
+/// If the instruction has a segment override prefix, use the existing one.
+/// If the target is 64-bit, use the CS.
+/// If the target is 32-bit,
+///   - If the instruction has a ESP/EBP base register, use SS.
+///   - Otherwise use DS.
+uint8_t X86AsmBackend::determinePaddingPrefix(const MCInst &Inst) const {
+  assert((STI.hasFeature(X86::Mode32Bit) || STI.hasFeature(X86::Mode64Bit)) &&
+         "Prefixes can be added only in 32-bit or 64-bit mode.");
+  const MCInstrDesc &Desc = MCII->get(Inst.getOpcode());
+  uint64_t TSFlags = Desc.TSFlags;
+
+  // Determine where the memory operand starts, if present.
+  int MemoryOperand = X86II::getMemoryOperandNo(TSFlags);
+  if (MemoryOperand != -1)
+    MemoryOperand += X86II::getOperandBias(Desc);
+
+  unsigned SegmentReg = 0;
+  if (MemoryOperand >= 0) {
+    // Check for explicit segment override on memory operand.
+    SegmentReg = Inst.getOperand(MemoryOperand + X86::AddrSegmentReg).getReg();
+  }
+
+  switch (TSFlags & X86II::FormMask) {
+  default:
+    break;
+  case X86II::RawFrmDstSrc: {
+    // Check segment override opcode prefix as needed (not for %ds).
+    if (Inst.getOperand(2).getReg() != X86::DS)
+      SegmentReg = Inst.getOperand(2).getReg();
+    break;
+  }
+  case X86II::RawFrmSrc: {
+    // Check segment override opcode prefix as needed (not for %ds).
+    if (Inst.getOperand(1).getReg() != X86::DS)
+      SegmentReg = Inst.getOperand(1).getReg();
+    break;
+  }
+  case X86II::RawFrmMemOffs: {
+    // Check segment override opcode prefix as needed.
+    SegmentReg = Inst.getOperand(1).getReg();
+    break;
+  }
+  }
+
+  switch (SegmentReg) {
+  case 0:
+    break;
+  case X86::CS:
+    return X86::CS_Encoding;
+  case X86::DS:
+    return X86::DS_Encoding;
+  case X86::ES:
+    return X86::ES_Encoding;
+  case X86::FS:
+    return X86::FS_Encoding;
+  case X86::GS:
+    return X86::GS_Encoding;
+  case X86::SS:
+    return X86::SS_Encoding;
+  }
+
+  if (STI.hasFeature(X86::Mode64Bit))
+    return X86::CS_Encoding;
+
+  if (MemoryOperand >= 0) {
+    unsigned BaseRegNum = MemoryOperand + X86::AddrBaseReg;
+    unsigned BaseReg = Inst.getOperand(BaseRegNum).getReg();
+    if (BaseReg == X86::ESP || BaseReg == X86::EBP)
+      return X86::SS_Encoding;
+  }
+  return X86::DS_Encoding;
+}
+
 /// Check if the two instructions will be macro-fused on the target cpu.
 bool X86AsmBackend::isMacroFused(const MCInst &Cmp, const MCInst &Jcc) const {
   const MCInstrDesc &InstDesc = MCII->get(Jcc.getOpcode());
   if (!InstDesc.isConditionalBranch())
     return false;
   if (!isFirstMacroFusibleInst(Cmp, *MCII))
     return false;
   const X86::FirstMacroFusionInstKind CmpKind =

llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h

enum AlignBranchBoundaryKind : uint8_t {
     AlignBranchNone = 0,
     AlignBranchFused = 1U << 0,
     AlignBranchJcc = 1U << 1,
     AlignBranchJmp = 1U << 2,
     AlignBranchCall = 1U << 3,
     AlignBranchRet = 1U << 4,
     AlignBranchIndirect = 1U << 5
   };
+
+  /// Defines the encoding values for segment override prefix.
+  enum EncodingOfSegmentOverridePrefix : uint8_t {
+    CS_Encoding = 0x2E,
+    DS_Encoding = 0x3E,
+    ES_Encoding = 0x26,
+    FS_Encoding = 0x64,
+    GS_Encoding = 0x65,
+    SS_Encoding = 0x36
+  };
 } // end namespace X86;
 
 /// X86II - This namespace holds all of the target specific flags that
 /// instruction info tracks.
 ///
 namespace X86II {
   /// Target Operand Flag enum.
   enum TOF {