[PATCH] D97268: [X86] Use correct padding when in 16-bit mode

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

In D97268#2583270 <https://reviews.llvm.org/D97268#2583270>, @jyknight wrote:

> In D97268#2582826 <https://reviews.llvm.org/D97268#2582826>, @void wrote:
>
>> In D97268#2581555 <https://reviews.llvm.org/D97268#2581555>, @jyknight wrote:
>>
>>> This change also modifies (breaks!) the 32-bit NOP set, definitely shouldn't do that. (But I think all that can be reverted, in any case).
>>
>> Which part did it break?
>
> It replaced the single-instruction NOPs with new sequences consisting of multiple instructions.

That...shouldn't matter since a NOP is a NOP. Regardless, I removed that modification to simplify the patch.



================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:1128
+        "\x8d\xb4\x00\x00",
+        // nop; lea 0w(%si),%si
+        "\x90\x8d\xb4\x00\x00",
----------------
jyknight wrote:
> If you want to go the complex route, and support multi-byte nops in 16bit mode:
> 
> The 5 and larger-byte strings here should be removed, as they're combinations of smaller instructions. Leave only the initial 4 elements in the Nops16Bit array. Therefore, getMaximumNopSize() should return 4 in 16bit mode.
> 
That is needlessly complex. These patterns are from binutils, which claims that they're "efficient." The code in this patch is far easier to understand, in my opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97268