[PATCH] D82826: [X86] support .nops directive

[Jian Cai via Phabricator via llvm-commits]

jcai19 marked an inline comment as done.
jcai19 added inline comments.


================
Comment at: llvm/include/llvm/MC/MCFragment.h:358
+  /// Maximum number of bytes of each instruction.
+  int64_t NopLength;
+
----------------
craig.topper wrote:
> reames wrote:
> > Can you call this MaxNopLength or something?
> Was this comment addressed? I'm not sure what the variable was called when @reames made this comment.
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!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82826/new/

https://reviews.llvm.org/D82826