[PATCH] D17573: [X86] PR26554: Enable using of true long nops for x86-64 for every CPU

[Simon Pilgrim via llvm-commits]

RKSimon added inline comments.

================
Comment at: lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:353
@@ -345,3 +352,3 @@
   // Select the right NOP table.
   // FIXME: Can we get if CPU supports long nops from the subtarget somehow?
   const uint8_t (*Nops)[10] = HasNopl ? TrueNops : AltNops;
----------------
aturetsk wrote:
> RKSimon wrote:
> > bruno wrote:
> > > aturetsk wrote:
> > > > bruno wrote:
> > > > > Any reason why this shouldn't be done as a target feature as suggested in the FIXME above?
> > > > Target feature can be used as indicator of true long nop support, but that won't fix the issue.
> > > > 
> > > > The alternative nop instructions I introduced in http://reviews.llvm.org/D14178 are not actually nops for x86-64 arch. But all CPUs which don't support true long nops are 32-bit-only. So I propose to use true long nops if we compile for x86-64 arch for any CPU even if it's marked as 'doesn't support true long nops'.
> > > > Well, we do generate 64-bit instructions for 32-bit-only CPUs when x86-64 arch is used, why not generate true long nops as well?
> > > > And I think 'generic' should use true long nops in 64-bit mode (since every real 64 bit CPU seems to support them) and use alternative long nops in 32 bit mode.
> > > I see. However, we could have the opposite logic, something like "FeatureShortNop", and mark generic, i386, etc with them. I looked up these CPUs and none of them have Feature64Bit, so guess HasNopl will simply become !FeatureShortNop.
> > Basing this off target features would also make it much easier to chose the optimal nop length for intel/amd (as discussed on PR22965).
> Oh, I think my understanding of 'generic' CPU wasn't quite right. It's not supposed to be used in 64-bit mode, right? ('x86-64' CPU seems to be 64-bit 'generic')
> If that's so then I see no reason to avoid using target features and I'll rework the patch (using FeatureLongNop).
> 
> Simon, how do you suggest to choose the optimal nop length in this case?
> Use different target features for different max lenghts like this:
> FeatureLongNop - allow nops of any length
> FeatureLongNop7 - only allow nops smaller than 7 bytes
> ... etc?
Probably provide a X86Subtarget::getMaxNopLength() helper function that uses feature bits internally.


http://reviews.llvm.org/D17573