[PATCH] D81301: [X86] Emit two-byte NOP when possible

Sat Jun 6 07:26:54 PDT 2020

aganea added a comment.

In D81301#2077585 <https://reviews.llvm.org/D81301#2077585>, @reames wrote:

> Are two byte nops legal on all 32 bit x86?  The comment seems to imply no.
>
> Once this question is answered and we're happy with the general design, I will ask you to split out the NFC signature change and land it separately, but let's *not* do that yet.

My understanding is that the story goes like this:

- `90 NOP` - introduced with 8086.
- `66 90 XCHG AX, AX` - supported starting with 80386, with the introduction of instruction prefixes [1]
- `0F 1F NOP DWORD ptr` - long NOP variants were introduced with SSE, in Pentium III Katmai [2]

`llvm/lib/Target/X86/X86.td` specifies `FeatureNOPL` being supported in Pentium Pro [3] and Pentium 2 [4], but looking at the respective manuals that seems incorrect. There's no trace of long NOP instructions for those CPUs. @craig.topper could you please confirm?

[1] http://css.csail.mit.edu/6.858/2014/readings/i386/s02_04.htm
[2] https://en.wikipedia.org/wiki/X86_instruction_listings
[3] http://www.mathcs.emory.edu/~cheung/Courses/355/Syllabus/6-io/Docs/Intel-instructions.pdf
[4] http://www.archive.ece.cmu.edu/~ece548/localcpy/24281603.pdf

In D81301#2077964 <https://reviews.llvm.org/D81301#2077964>, @cdavis5x wrote:

> > Whenever using the "patchable-function" attribute, a PATCHABLE_OP now lowers to a xchg ax, ax, like MSVC does.
>
> Is this true? Last I checked--and admittedly, this was a long time ago--MSVC emitted `mov edi, edi`--more specifically, opcode bytes `8b ff`. GCC also emits those same bytes.

VS2012 and VS2013 generate `8B FF mov edi,edi`. After VS2015, `66 90 xchg ax,ax` is generated, as recommended by Intel, see [5] Vol. 2B page 4-167.

F12093135: image.png <https://reviews.llvm.org/F12093135>

[5] https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html#combined

> There's also a compatibility angle here. The reason this was added to GCC in the first place, and the reason I wrote the original change, is that Wine needed this to make certain Windows programs run correctly. I think they specifically check for that `8b ff` signature before installing their hotpatches--which I admit they really shouldn't be doing, and may be a factor in why MS started using `xchg ax, ax` (`66 90`), but they do it, and we need specifically that sequence to make them work correctly.

I'm a bit confused. Wine lists support up to Windows 10, which means recently-compiled applications should be supported. Meanning they (Wine) should see `66 90`, and support that. I asked the question anyway on the wine-devel mailing list.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81301