[X86] support .nops directive

[jcai19]

Summary

Add support of .nops on X86. This addresses llvm.org/PR45788.

[craig.topper]

Would it be possible to emit a new Fragment type and leverage the existing NOP emission code in X86AsmBackend.cpp. The code here appears to be copied from X86MCInstLower. Which would mean we would now have 3 places that do almost the same thing.

[jcai19]

In D82826#2121595, @craig.topper wrote:

Would it be possible to emit a new Fragment type and leverage the existing NOP emission code in X86AsmBackend.cpp. The code here appears to be copied from X86MCInstLower. Which would mean we would now have 3 places that do almost the same thing.

Thanks for the suggestion. Did not know there was a second copy already. Will try to reuse the code from X86AsmBackend.cpp.

[MaskRay]

I asked what the largest operand .nops supports https://sourceware.org/bugzilla/show_bug.cgi?id=25789 but did not get a clear answer. Maybe some reviewer can help me ask for a clarification...

llvm/test/MC/X86/x86-directive-nops.s

2

#

8

# X32

[nickdesaulniers]

If NopSize is basically equivalent for NumBytes except for 2 cases, then I'd do:

if (!NopSize) {
  print_some_error();
  return;
}
unsigned NopSize = NumBytes;
switch (NumBytes) {
...
default:
  NopSize = 10;
  ...
}

So that you don't have to assign NopSize = NumBytes for each other case.

[jcai19]

@craig.topper I tried to add a new Fragment type and use X86AsmBackend::writeNopData to insert NOP instructions. It however changed the outcome of the test cases I used, as X86AsmBackend::writeNopData only generatssingle-byte nop instructions if X86::FeatureNOPL feature bit is not true, while the maxLongNopLength in the current implementation (from X86MCInstLower) allows long nop instructions up to 10 bytes on a 64-bit processor. Which one is preferred? Also, which target triple should I use to test multiple-byte nop instructions? Thanks.

[jcai19]

@MaskRay @nickdesaulniers Thanks for the comments. Will refactor the code once we decide which way to go with the implementation.

[craig.topper]

In D82826#2124581, @jcai19 wrote:

@craig.topper I tried to add a new Fragment type and use X86AsmBackend::writeNopData to insert NOP instructions. It however changed the outcome of the test cases I used, as X86AsmBackend::writeNopData only generatssingle-byte nop instructions if X86::FeatureNOPL feature bit is not true, while the maxLongNopLength in the current implementation (from X86MCInstLower) allows long nop instructions up to 10 bytes on a 64-bit processor. Which one is preferred? Also, which target triple should I use to test multiple-byte nop instructions? Thanks.

That seems like a bug in writeNopData. All real 64-bit CPUs should have FeatureNOPL. But I think llvm-mc and llc default to "generic" as a CPU which isn't a real CPU and doesn't have the feature. clang never uses the "generic". The default 64-bit CPU for clang is "x86-64" which does have the feature. I'll see what happens if we check for 64-bit mode explicitly in writeNopData and post a patch tonight or tomorrow.

[jcai19]

Thanks! Checking if the CPU is 64-bit along FeatureNOPL seemed to work. Will verify more. Also I'd like to point out the difference of 32-bit CPUs.

[craig.topper]

writeNopsData should always use multibyte nops in 64-bit mode now.

[craig.topper]

is the 32-bit different the 2 byte case?

[jcai19]

In D82826#2126190, @craig.topper wrote:

is the 32-bit different the 2 byte case?

Yes.

[MaskRay]

I have a concern that we haven't figured out the largest operand .nops supports. We should clarify this (https://sourceware.org/bugzilla/show_bug.cgi?id=25789 )

[jcai19]

(1) This is a temporary implementation as it breaks the following tests:

LLVM :: MC/COFF/align-nops.s
LLVM :: MC/MachO/x86_32-optimal_nop.s
LLVM :: MC/X86/AlignedBundling/misaligned-bundle-group.s
LLVM :: MC/X86/AlignedBundling/misaligned-bundle.s
LLVM :: MC/X86/align-branch-bundle.s
LLVM :: MC/X86/align-branch-pad-max-prefix.s
LLVM :: MC/X86/x86_long_nop.s
LLVM :: MC/X86/x86_nop.s

The breakage was caused by checking X86::Mode32Bit and X86::Mode64Bit in the newly introduced X86AsmBackend::getMaximumNopSize() function. All the above tests would pass if I removed the two checks, but .nops calls would then emit byte-long nop instructions regardless of its second argument.

(2) Will address error messages in the next iteration.

[nickdesaulniers]

I'm having trouble posting comments inline; it seems fabricator won't allow me to "save" comments.

Is it too painful to make the member of MCNopsFragment uint64_t? a la MCStreamer::emitFill

[craig.topper]

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

1073

Mode32Bit is only relevant if FeatureNOPL is false. So I think it should be

if (!STI.getFeatureBits()[X86::FeatureNOPL] &&
    !STI.getFeatureBits()[X86::Mode64Bit])
  return STI.getFeatureBits()[X86::Mode32Bit] ? 2 : 1;

[jcai19]

In D82826#2126955, @nickdesaulniers wrote:

I'm having trouble posting comments inline; it seems fabricator won't allow me to "save" comments.

Is it too painful to make the member of MCNopsFragment uint64_t? a la MCStreamer::emitFill

I suppose it's easy to do but I'll have to explicitly cast them to signed integers to check if they are smaller than 0. That is probably more error-prone IMO. WDYT?

[jcai19]

Refactor X86AsmBackend::getMaximumNopSize based on @craig.topper's comment.

[jcai19]

Issuing multiple-byte NOP for 32-bit mode broke many tests and probably worth a separate patch itself. So this patch will keep the behavior on 32-bit mode unchange for now. This built and passed all the tests.

[reames]

llvm/include/llvm/MC/MCFragment.h

358

Can you call this MaxNopLength or something?

llvm/lib/MC/MCAssembler.cpp

625

When parsing asm, you reject negative lengths. Should these simply be asserts?

633

Does this behaviour match existing gnu? I'd have expected the result of specifying a "too large" maximum size to simply clamp to the target's maximum.

This is important as if the result is semantic, then the difference between "largest encodeable" and "largest profitable" becomes a thing the rest of the code has to care about. 15 byte nops are almost always *legal* they're just not *fast*.

644

This loop is duplicated from within emitNops. Can you pass in a MaxNopLength parameter instead?

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

1072

Rename this function to getMaximumProfitableNop()

There's a difference between legality and profit here. As commented earlier, if that matters you'll have a harder task implementation wise.

llvm/test/MC/X86/align-branch-bundle.s

9 ↗

(On Diff #275853)

Having a test delta in a file without .nops is highly suspicious.

I'd suggest splitting your patch into a trivial version which emits single byte nops, and an change which adds the multiple byte support. That would allow us to separate the directive mechanics from the interesting profit bits.

[jcai19]

llvm/lib/MC/MCAssembler.cpp

633

Does this behaviour match existing gnu?

Appears so.

$ cat foo.s
.nops 16, 15

$ gcc -c foo.s
foo.s: Assembler messages:
foo.s:1: Error: invalid single nop size: 15 (expect within [0, 11])

With the patch applied,
$ llvm-mc -filetype=obj -triple=x86_64 foo.s
foo.s:1:1: error: illegal NOP size 15. (expected within [0, 10])
.nops 16, 15
^

llvm/test/MC/X86/align-branch-bundle.s

9 ↗

(On Diff #275853)

How about we also print out instruction bytes here. If 64-bit processors can generate a two-byte long nop instruction here, shouldn't we emit that instead of two single-byte nop? Thanks.

[jcai19]

llvm/lib/MC/MCAssembler.cpp

644

There isn't any loop in emitNops. Do you by any chance refer to the loop in writeNopData? In that case, they are not duplicate as this loop will break total bytes into nop instructions no longer than specified by the second argument of .nops if provided, while the loop in writeNopData makes sure each instruction emitted is no longer than the maximum length allowed by the target.

llvm/test/MC/X86/align-branch-bundle.s

9 ↗

(On Diff #275853)

On second thought, I agree that splitting the patch is the better approach in case the multiple-byte support causes any regression. Will address this in the next iteration.

[jcai19]

Address some of @reames's concerns, and rename variables to avoid confusion.

[aganea]

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

1072

Any reason for not reusing maxLongNopLength() rather than rewriting the same thing here? https://github.com/llvm/llvm-project/blob/b2eb1c5793d78d70c1223b098aefc87050f69a8c/llvm/lib/Target/X86/X86MCInstLower.cpp#L1085
That function could perhaps be moved to llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h ?

[jcai19]

Fixed an assertion message.

[craig.topper]

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

1072

That function can't be moved as is. It uses X86Subtarget which isn't available to MC. It does something different than for 32-bit mode than what is currently in this patch as that causes additional test failures as discussed elsewhere in this review. That function also uses ProcIntelSLM instead of Feature7ByteNOP. And the FeatureFast flags being set assumes FeatureNOPL is set which is backwards of how it should be.

I think the function here is closer to how it should be except for the 32-bit difference.

[jcai19]

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

1072

Any reason for not reusing maxLongNopLength() rather than rewriting the same thing here? https://github.com/llvm/llvm-project/blob/b2eb1c5793d78d70c1223b098aefc87050f69a8c/llvm/lib/Target/X86/X86MCInstLower.cpp#L1085

Yes I'm all for merging these two functions although there are some differences on both 32-bit and 64-bit mode that would break some unit tests, such as https://reviews.llvm.org/D82826?id=275853 on 64-bit mode. Maybe we can address that in a separate patch as previously discussed.

That function could perhaps be moved to llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h ?

SG. Will start to work on merging them once this patch is checked in.

[jcai19]

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

1072

@craig.topper Okay I missed your comment. Thanks for the clarification.

[jcai19]

@reames @craig.topper Hi just want to double check if there are any additional issues I should address. Thanks.

[MaskRay]

I think I have a very old (I emphasized it again in another comment) comment which isn't addressed (https://reviews.llvm.org/D82826#2126406 )
We should align with GNU as on the largest operand .nops supports.

[jcai19]

In D82826#2154511, @MaskRay wrote:

I think I have a very old (I emphasized it again in another comment) comment which isn't addressed (https://reviews.llvm.org/D82826#2126406 )
We should align with GNU as on the largest operand .nops supports.

I think it's probably better to address that in a separate patch as the this patch is focused on adding support to .nops directive, similar to this comment https://reviews.llvm.org/D82826?id=275853#inline-765965.

[craig.topper]

llvm/include/llvm/MC/MCFragment.h

358

Was this comment addressed? I'm not sure what the variable was called when @reames made this comment.

llvm/lib/MC/MCAssembler.cpp

628

Should we put Asm.getBackend().getMaximumNopSize() into a variable? We call it 3 times.

634

Should we clamp the NopLength if we hit the error case? reportError won't stop immediately. Or are we relying on writeNopData to not exceed the maximum size internally?

637

Maybe just use a if statement here for !NopLength

644

I think he was refering to writeNopData. I suppose we could add a limit parameter to writeNopData, but every target would need to be updated for it.

llvm/test/MC/X86/x86-directive-nops-errors.s

6

Please use X86 rather than X32. X32 gets confusing with gnux32 where 32-bit pointers are used on a 64-bit target.

[jcai19]

llvm/include/llvm/MC/MCFragment.h

358

It was called NopLength. The name was confusing because I called it NopLength here but the same value MaxNopLength somewhere so @reames suggested me to rename it to MaxNopLength . While working on that I realized this value was meant to specify a soft cap on the size limit of a no-op instruction, i.e. the second argument (control) as https://sourceware.org/binutils/docs/as/Nops.html#Nops specified, so I kept the name to avoid the confusion with the function name getMaximumNopSize, which returns a "hard cap" LLVM can accept. Any suggestion on what a better name could be? Also I realized MaxNopLen in the newly introduced emitNops function should be renamed once we make a final decision on the naming. Will address other comments together in the next iteration. Thanks!

[jcai19]

Renamed a variable name and addressed comments. @craig.topper Please let me know if it looks any better. And thanks for enabling multibyte-NOPs in 64-bit mode.

[craig.topper]

LGTM

[jcai19]

In D82826#2189118, @craig.topper wrote:

LGTM

Thank you!

llvm/include/llvm/MC/MCAsmBackend.h

public:
   /// @}
 
   /// Returns the minimum size of a nop in bytes on this target. The assembler
   /// will use this to emit excess padding in situations where the padding
   /// required for simple alignment would be less than the minimum nop size.
   ///
   virtual unsigned getMinimumNopSize() const { return 1; }
 
+  /// Returns the maximum size of a nop in bytes on this target.
+  ///
+  virtual unsigned getMaximumNopSize() const { return 0; }
+
   /// Write an (optimal) nop sequence of Count bytes to the given output. If the
   /// target cannot generate such a sequence, it should return an error.
   ///
   /// \return - True on success.
   virtual bool writeNopData(raw_ostream &OS, uint64_t Count) const = 0;
 
   /// Give backend an opportunity to finish layout after relaxation
   virtual void finishLayout(MCAssembler const &Asm,

llvm/include/llvm/MC/MCFragment.h

class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
   friend class MCAsmLayout;
 
 public:
   enum FragmentType : uint8_t {
     FT_Align,
     FT_Data,
     FT_CompactEncodedInst,
     FT_Fill,
+    FT_Nops,
     FT_Relaxable,
     FT_Org,
     FT_Dwarf,
     FT_DwarfFrame,
     FT_LEB,
     FT_BoundaryAlign,
     FT_SymbolId,
     FT_CVInlineLines,
public:
 
   SMLoc getLoc() const { return Loc; }
 
   static bool classof(const MCFragment *F) {
     return F->getKind() == MCFragment::FT_Fill;
   }
 };
 
+class MCNopsFragment : public MCFragment {
+  /// The number of bytes to insert.
+  int64_t Size;
+  /// Maximum number of bytes allowed in each NOP instruction.
+  int64_t ControlledNopLength;
+
+  /// Source location of the directive that this fragment was created for.
+  SMLoc Loc;
+
+public:
+  MCNopsFragment(int64_t NumBytes, int64_t ControlledNopLength, SMLoc L,
+                 MCSection *Sec = nullptr)
+      : MCFragment(FT_Nops, false, Sec), Size(NumBytes),
+        ControlledNopLength(ControlledNopLength), Loc(L) {}
+
+  int64_t getNumBytes() const { return Size; }
+  int64_t getControlledNopLength() const { return ControlledNopLength; }
+
+  SMLoc getLoc() const { return Loc; }
+
+  static bool classof(const MCFragment *F) {
+    return F->getKind() == MCFragment::FT_Nops;
+  }
+};
+
 class MCOrgFragment : public MCFragment {
   /// Value to use for filling bytes.
   int8_t Value;
 
   /// The offset this fragment should start at.
   const MCExpr *Offset;
 
   /// Source location of the directive that this fragment was created for.

llvm/include/llvm/MC/MCObjectStreamer.h

public:
   Optional<std::pair<bool, std::string>>
   emitRelocDirective(const MCExpr &Offset, StringRef Name, const MCExpr *Expr,
                      SMLoc Loc, const MCSubtargetInfo &STI) override;
   using MCStreamer::emitFill;
   void emitFill(const MCExpr &NumBytes, uint64_t FillValue,
                 SMLoc Loc = SMLoc()) override;
   void emitFill(const MCExpr &NumValues, int64_t Size, int64_t Expr,
                 SMLoc Loc = SMLoc()) override;
+  void emitNops(int64_t NumBytes, int64_t ControlledNopLength,
+                SMLoc Loc) override;
   void emitFileDirective(StringRef Filename) override;
 
   void emitAddrsig() override;
   void emitAddrsigSym(const MCSymbol *Sym) override;
 
   void finishImpl() override;
 
   /// Emit the absolute difference between two symbols if possible.

llvm/include/llvm/MC/MCStreamer.h

public:
   /// This is used to implement assembler directives such as .fill.
   ///
   /// \param NumValues - The number of copies of \p Size bytes to emit.
   /// \param Size - The size (in bytes) of each repeated value.
   /// \param Expr - The expression from which \p Size bytes are used.
   virtual void emitFill(const MCExpr &NumValues, int64_t Size, int64_t Expr,
                         SMLoc Loc = SMLoc());
 
+  virtual void emitNops(int64_t NumBytes, int64_t ControlledNopLength,
+                        SMLoc Loc);
+
   /// Emit NumBytes worth of zeros.
   /// This function properly handles data in virtual sections.
   void emitZeros(uint64_t NumBytes);
 
   /// Emit some number of copies of \p Value until the byte alignment \p
   /// ByteAlignment is reached.
   ///
   /// If the number of bytes need to emit for the alignment is not a multiple

llvm/lib/MC/MCAssembler.cpp

 STATISTIC(EmittedDataFragments,
           "Number of emitted assembler fragments - data");
 STATISTIC(EmittedCompactEncodedInstFragments,
           "Number of emitted assembler fragments - compact encoded inst");
 STATISTIC(EmittedAlignFragments,
           "Number of emitted assembler fragments - align");
 STATISTIC(EmittedFillFragments,
           "Number of emitted assembler fragments - fill");
-STATISTIC(EmittedOrgFragments,
-          "Number of emitted assembler fragments - org");
+STATISTIC(EmittedNopsFragments, "Number of emitted assembler fragments - nops");
+STATISTIC(EmittedOrgFragments, "Number of emitted assembler fragments - org");
 STATISTIC(evaluateFixup, "Number of evaluated fixups");
 STATISTIC(FragmentLayouts, "Number of fragment layouts");
 STATISTIC(ObjectBytes, "Number of emitted object file bytes");
 STATISTIC(RelaxationSteps, "Number of assembler layout and relaxation steps");
 STATISTIC(RelaxedInstructions, "Number of relaxed instructions");
 
 } // end namespace stats
 } // end anonymous namespace
case MCFragment::FT_Fill: {
     int64_t Size = NumValues * FF.getValueSize();
     if (Size < 0) {
       getContext().reportError(FF.getLoc(), "invalid number of bytes");
       return 0;
     }
     return Size;
   }
 
+  case MCFragment::FT_Nops:
+    return cast<MCNopsFragment>(F).getNumBytes();
+
   case MCFragment::FT_LEB:
     return cast<MCLEBFragment>(F).getContents().size();
 
   case MCFragment::FT_BoundaryAlign:
     return cast<MCBoundaryAlignFragment>(F).getSize();
 
   case MCFragment::FT_SymbolId:
     return 4;
case MCFragment::FT_Fill: {
 
     // do remainder if needed.
     unsigned TrailingCount = FragmentSize % ChunkSize;
     if (TrailingCount)
       OS.write(Data, TrailingCount);
     break;
   }
 
+  case MCFragment::FT_Nops: {
+    ++stats::EmittedNopsFragments;
+    const MCNopsFragment &NF = cast<MCNopsFragment>(F);
+    int64_t NumBytes = NF.getNumBytes();
+    int64_t ControlledNopLength = NF.getControlledNopLength();
+    int64_t MaximumNopLength = Asm.getBackend().getMaximumNopSize();
+
+    assert(NumBytes > 0 && "Expected positive NOPs fragment size");
+    assert(ControlledNopLength >= 0 && "Expected non-negative NOP size");
+
+    if (ControlledNopLength > MaximumNopLength) {
+      Asm.getContext().reportError(NF.getLoc(),
+                                   "illegal NOP size " +
+                                       std::to_string(ControlledNopLength) +
+                                       ". (expected within [0, " +
+                                       std::to_string(MaximumNopLength) + "])");
+      // Clamp the NOP length as reportError does not stop the execution
+      // immediately.
+      ControlledNopLength = MaximumNopLength;
+    }
+
+    // Use maximum value if the size of each NOP is not specified
+    if (!ControlledNopLength)
+      ControlledNopLength = MaximumNopLength;
+
+    while (NumBytes) {
+      uint64_t NumBytesToEmit =
+          (uint64_t)std::min(NumBytes, ControlledNopLength);
+      assert(NumBytesToEmit && "try to emit empty NOP instruction");
+      if (!Asm.getBackend().writeNopData(OS, NumBytesToEmit)) {
+        report_fatal_error("unable to write nop sequence of the remaining " +
+                           Twine(NumBytesToEmit) + " bytes");
+        break;
+      }
+      NumBytes -= NumBytesToEmit;
+    }
+    break;
+  }
+
   case MCFragment::FT_LEB: {
     const MCLEBFragment &LF = cast<MCLEBFragment>(F);
     OS << LF.getContents();
     break;
   }
 
   case MCFragment::FT_BoundaryAlign: {
     if (!Asm.getBackend().writeNopData(OS, FragmentSize))

llvm/lib/MC/MCFragment.cpp

case FT_Data:
       delete cast<MCDataFragment>(this);
       return;
     case FT_CompactEncodedInst:
       delete cast<MCCompactEncodedInstFragment>(this);
       return;
     case FT_Fill:
       delete cast<MCFillFragment>(this);
       return;
+    case FT_Nops:
+      delete cast<MCNopsFragment>(this);
+      return;
     case FT_Relaxable:
       delete cast<MCRelaxableFragment>(this);
       return;
     case FT_Org:
       delete cast<MCOrgFragment>(this);
       return;
     case FT_Dwarf:
       delete cast<MCDwarfLineAddrFragment>(this);
LLVM_DUMP_METHOD void MCFragment::dump() const {
 
   OS << "<";
   switch (getKind()) {
   case MCFragment::FT_Align: OS << "MCAlignFragment"; break;
   case MCFragment::FT_Data:  OS << "MCDataFragment"; break;
   case MCFragment::FT_CompactEncodedInst:
     OS << "MCCompactEncodedInstFragment"; break;
   case MCFragment::FT_Fill:  OS << "MCFillFragment"; break;
+  case MCFragment::FT_Nops:
+    OS << "MCFNopsFragment";
+    break;
   case MCFragment::FT_Relaxable:  OS << "MCRelaxableFragment"; break;
   case MCFragment::FT_Org:   OS << "MCOrgFragment"; break;
   case MCFragment::FT_Dwarf: OS << "MCDwarfFragment"; break;
   case MCFragment::FT_DwarfFrame: OS << "MCDwarfCallFrameFragment"; break;
   case MCFragment::FT_LEB:   OS << "MCLEBFragment"; break;
   case MCFragment::FT_BoundaryAlign: OS<<"MCBoundaryAlignFragment"; break;
   case MCFragment::FT_SymbolId:    OS << "MCSymbolIdFragment"; break;
   case MCFragment::FT_CVInlineLines: OS << "MCCVInlineLineTableFragment"; break;
LLVM_DUMP_METHOD void MCFragment::dump() const {
   }
   case MCFragment::FT_Fill:  {
     const auto *FF = cast<MCFillFragment>(this);
     OS << " Value:" << static_cast<unsigned>(FF->getValue())
        << " ValueSize:" << static_cast<unsigned>(FF->getValueSize())
        << " NumValues:" << FF->getNumValues();
     break;
   }
+  case MCFragment::FT_Nops: {
+    const auto *NF = cast<MCNopsFragment>(this);
+    OS << " NumBytes:" << NF->getNumBytes()
+       << " ControlledNopLength:" << NF->getControlledNopLength();
+    break;
+  }
   case MCFragment::FT_Relaxable:  {
     const auto *F = cast<MCRelaxableFragment>(this);
     OS << "\n       ";
     OS << " Inst:";
     F->getInst().dump_pretty(OS);
     OS << " (" << F->getContents().size() << " bytes)";
     break;
   }

llvm/lib/MC/MCObjectStreamer.cpp

void MCObjectStreamer::emitFill(const MCExpr &NumValues, int64_t Size,
   // Otherwise emit as fragment.
   MCDataFragment *DF = getOrCreateDataFragment();
   flushPendingLabels(DF, DF->getContents().size());
 
   assert(getCurrentSectionOnly() && "need a section");
   insert(new MCFillFragment(Expr, Size, NumValues, Loc));
 }
 
+void MCObjectStreamer::emitNops(int64_t NumBytes, int64_t ControlledNopLength,
+                                SMLoc Loc) {
+  // Emit an NOP fragment.
+  MCDataFragment *DF = getOrCreateDataFragment();
+  flushPendingLabels(DF, DF->getContents().size());
+
+  assert(getCurrentSectionOnly() && "need a section");
+  insert(new MCNopsFragment(NumBytes, ControlledNopLength, Loc));
+}
+
 void MCObjectStreamer::emitFileDirective(StringRef Filename) {
   getAssembler().addFileName(Filename);
 }
 
 void MCObjectStreamer::emitAddrsig() {
   getAssembler().getWriter().emitAddrsigSection();
 }
 

llvm/lib/MC/MCStreamer.cpp

 }
 
 /// Emit NumBytes bytes worth of the value specified by FillValue.
 /// This implements directives such as '.space'.
 void MCStreamer::emitFill(uint64_t NumBytes, uint8_t FillValue) {
   emitFill(*MCConstantExpr::create(NumBytes, getContext()), FillValue);
 }
 
+void llvm::MCStreamer::emitNops(int64_t NumBytes, int64_t ControlledNopLen,
+                                llvm::SMLoc) {}
+
 /// The implementation in this class just redirects to emitFill.
 void MCStreamer::emitZeros(uint64_t NumBytes) { emitFill(NumBytes, 0); }
 
 Expected<unsigned>
 MCStreamer::tryEmitDwarfFileDirective(unsigned FileNo, StringRef Directory,
                                       StringRef Filename,
                                       Optional<MD5::MD5Result> Checksum,
                                       Optional<StringRef> Source,

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

private:
   bool CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp,
                                unsigned BaseReg, unsigned IndexReg,
                                unsigned Scale, SMLoc Start, SMLoc End,
                                unsigned Size, StringRef Identifier,
                                const InlineAsmIdentifierInfo &Info,
                                OperandVector &Operands);
 
   bool parseDirectiveArch();
+  bool parseDirectiveNops(SMLoc L);
   bool parseDirectiveEven(SMLoc L);
   bool ParseDirectiveCode(StringRef IDVal, SMLoc L);
 
   /// CodeView FPO data directives.
   bool parseDirectiveFPOProc(SMLoc L);
   bool parseDirectiveFPOSetFrame(SMLoc L);
   bool parseDirectiveFPOPushReg(SMLoc L);
   bool parseDirectiveFPOStackAlloc(SMLoc L);
if (getLexer().isNot(AsmToken::EndOfStatement)) {
       if (Parser.getTok().getString() == "noprefix")
         Parser.Lex();
       else if (Parser.getTok().getString() == "prefix")
         return Error(DirectiveID.getLoc(), "'.intel_syntax prefix' is not "
                                            "supported: registers must not have "
                                            "a '%' prefix in .intel_syntax");
     }
     return false;
-  } else if (IDVal == ".even")
+  } else if (IDVal == ".nops")
+    return parseDirectiveNops(DirectiveID.getLoc());
+  else if (IDVal == ".even")
     return parseDirectiveEven(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_proc")
     return parseDirectiveFPOProc(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_setframe")
     return parseDirectiveFPOSetFrame(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_pushreg")
     return parseDirectiveFPOPushReg(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_stackalloc")

 }
 
 bool X86AsmParser::parseDirectiveArch() {
   // Ignore .arch for now.
   getParser().parseStringToEndOfStatement();
   return false;
 }
 
+/// parseDirectiveNops
+///  ::= .nops size[, control]
+bool X86AsmParser::parseDirectiveNops(SMLoc L) {
+  int64_t NumBytes = 0, Control = 0;
+  SMLoc NumBytesLoc, ControlLoc;
+  const MCSubtargetInfo STI = getSTI();
+  NumBytesLoc = getTok().getLoc();
+  if (getParser().checkForValidSection() ||
+      getParser().parseAbsoluteExpression(NumBytes))
+    return true;
+
+  if (parseOptionalToken(AsmToken::Comma)) {
+    ControlLoc = getTok().getLoc();
+    if (getParser().parseAbsoluteExpression(Control))
+      return true;
+  }
+  if (getParser().parseToken(AsmToken::EndOfStatement,
+                             "unexpected token in '.nops' directive"))
+    return true;
+
+  if (NumBytes <= 0) {
+    Error(NumBytesLoc, "'.nops' directive with non-positive size");
+    return false;
+  }
+
+  if (Control < 0) {
+    Error(ControlLoc, "'.nops' directive with negative NOP size");
+    return false;
+  }
+
+  /// Emit nops
+  getParser().getStreamer().emitNops(NumBytes, Control, L);
+
+  return false;
+}
+
 /// parseDirectiveEven
 ///  ::= .even
 bool X86AsmParser::parseDirectiveEven(SMLoc L) {
   if (parseToken(AsmToken::EndOfStatement, "unexpected token in directive"))
     return false;
 
   const MCSection *Section = getStreamer().getCurrentSectionOnly();
   if (!Section) {

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

public:
   bool padInstructionViaPrefix(MCRelaxableFragment &RF, MCCodeEmitter &Emitter,
                                unsigned &RemainingSize) const;
 
   bool padInstructionEncoding(MCRelaxableFragment &RF, MCCodeEmitter &Emitter,
                               unsigned &RemainingSize) const;
 
   void finishLayout(MCAssembler const &Asm, MCAsmLayout &Layout) const override;
 
+  unsigned getMaximumNopSize() const override;
+
   bool writeNopData(raw_ostream &OS, uint64_t Count) const override;
 };
 } // end anonymous namespace
 
 static unsigned getRelaxedOpcodeBranch(const MCInst &Inst, bool Is16BitMode) {
   unsigned Op = Inst.getOpcode();
   switch (Op) {
   default:
#endif
   // The layout is done. Mark every fragment as valid.
   for (unsigned int i = 0, n = Layout.getSectionOrder().size(); i != n; ++i) {
     MCSection &Section = *Layout.getSectionOrder()[i];
     Layout.getFragmentOffset(&*Section.getFragmentList().rbegin());
     Asm.computeFragmentSize(Layout, *Section.getFragmentList().rbegin());
   }
 }
 
+unsigned X86AsmBackend::getMaximumNopSize() const {
+  if (!STI.hasFeature(X86::FeatureNOPL) && !STI.hasFeature(X86::Mode64Bit))
+    return 1;
+  if (STI.getFeatureBits()[X86::FeatureFast7ByteNOP])
+    return 7;
+  if (STI.getFeatureBits()[X86::FeatureFast15ByteNOP])
+    return 15;
+  if (STI.getFeatureBits()[X86::FeatureFast11ByteNOP])
+    return 11;
+  // FIXME: handle 32-bit mode
+  // 15-bytes is the longest single NOP instruction, but 10-bytes is
+  // commonly the longest that can be efficiently decoded.
+  return 10;
+}
+
 /// Write a sequence of optimal nops to the output, covering \p Count
 /// bytes.
 /// \return - true on success, false on failure
 bool X86AsmBackend::writeNopData(raw_ostream &OS, uint64_t Count) const {
   static const char Nops[10][11] = {
     // nop
     "\x90",
     // xchg %ax,%ax
static const char Nops[10][11] = {
     // nopl 0L(%[re]ax,%[re]ax,1)
     "\x0f\x1f\x84\x00\x00\x00\x00\x00",
     // nopw 0L(%[re]ax,%[re]ax,1)
     "\x66\x0f\x1f\x84\x00\x00\x00\x00\x00",
     // nopw %cs:0L(%[re]ax,%[re]ax,1)
     "\x66\x2e\x0f\x1f\x84\x00\x00\x00\x00\x00",
   };
 
-  // This CPU doesn't support long nops. If needed add more.
-  // FIXME: We could generated something better than plain 0x90.
-  if (!STI.hasFeature(X86::FeatureNOPL) && !STI.hasFeature(X86::Mode64Bit)) {
-    for (uint64_t i = 0; i < Count; ++i)
-      OS << '\x90';
-    return true;
-  }
-
-  // 15-bytes is the longest single NOP instruction, but 10-bytes is
-  // commonly the longest that can be efficiently decoded.
-  uint64_t MaxNopLength = 10;
-  if (STI.getFeatureBits()[X86::FeatureFast7ByteNOP])
-    MaxNopLength = 7;
-  else if (STI.getFeatureBits()[X86::FeatureFast15ByteNOP])
-    MaxNopLength = 15;
-  else if (STI.getFeatureBits()[X86::FeatureFast11ByteNOP])
-    MaxNopLength = 11;
+  uint64_t MaxNopLength = (uint64_t)getMaximumNopSize();
 
   // Emit as many MaxNopLength NOPs as needed, then emit a NOP of the remaining
   // length.
   do {
     const uint8_t ThisNopLength = (uint8_t) std::min(Count, MaxNopLength);
     const uint8_t Prefixes = ThisNopLength <= 10 ? 0 : ThisNopLength - 10;
     for (uint8_t i = 0; i < Prefixes; i++)
       OS << '\x66';

llvm/test/MC/X86/x86-directive-nops-errors.s

+# RUN: not llvm-mc -triple i386 %s -filetype=obj -o /dev/null 2>&1 | FileCheck --check-prefix=X86 %s
+# RUN: not llvm-mc -triple=x86_64 %s -filetype=obj -o /dev/null 2>&1 | FileCheck --check-prefix=X64 %s
+
+.nops 4, 3
+# X86: :[[@LINE-1]]:1: error: illegal NOP size 3.
+.nops 4, 4
+# X86: :[[@LINE-1]]:1: error: illegal NOP size 4.
+.nops 4, 5
+# X86: :[[@LINE-1]]:1: error: illegal NOP size 5.
+.nops 16, 15
+# X86: :[[@LINE-1]]:1: error: illegal NOP size 15.
+# X64: :[[@LINE-2]]:1: error: illegal NOP size 15.

llvm/test/MC/X86/x86-directive-nops.s

+# RUN: llvm-mc -triple i386 %s -filetype=obj | llvm-objdump -d - | FileCheck %s
+
+.nops 4
+# CHECK:       0: 90 nop
+# CHECK-NEXT:  1: 90 nop
+# CHECK-NEXT:  2: 90 nop
+# CHECK-NEXT:  3: 90 nop
+.nops 4, 1
+# CHECK:       4: 90 nop
+# CHECK-NEXT:  5: 90 nop
+# CHECK-NEXT:  6: 90 nop
+# CHECK-NEXT:  7: 90 nop

llvm/test/MC/X86/x86_64-directive-nops.s

+# RUN: llvm-mc -triple=x86_64 %s -filetype=obj | llvm-objdump -d - | FileCheck %s
+
+.nops 4, 1
+# CHECK:       0: 90 nop
+# CHECK-NEXT:  1: 90 nop
+# CHECK-NEXT:  2: 90 nop
+# CHECK-NEXT:  3: 90 nop
+.nops 4, 2
+# CHECK-NEXT:  4: 66 90 nop
+# CHECK-NEXT:  6: 66 90 nop
+.nops 4, 3
+# CHECK-NEXT:  8: 0f 1f 00 nopl (%rax)
+# CHECK-NEXT:  b: 90 nop
+.nops 4, 4
+# CHECK-NEXT:  c: 0f 1f 40 00 nopl (%rax)
+.nops 4, 5
+# CHECK-NEXT:  10: 0f 1f 40 00 nopl (%rax)
+.nops 4
+# CHECK-NEXT:  14: 0f 1f 40 00 nopl (%rax)