[llvm] 1a1448e - [X86] Merge X86MCInstLowering's maxLongNopLength into emitNop and remove check for FeatureNOPL.

[Philip Reames via llvm-commits]

Craig,

In moving the code, you dropped the comment that was there explicitly to 
describe one of the subtleties in the code.  Can I ask that you move 
over the comment?  Personally, I preferred the outline function style 
even with your change, but that's minor. The loss of the explanation is 
more important.

Philip

On 7/25/20 10:52 PM, Craig Topper via llvm-commits wrote:
> Author: Craig Topper
> Date: 2020-07-25T22:11:47-07:00
> New Revision: 1a1448e6568d9b11f198e510fa9c4cb6b1f4216a
>
> URL: https://github.com/llvm/llvm-project/commit/1a1448e6568d9b11f198e510fa9c4cb6b1f4216a
> DIFF: https://github.com/llvm/llvm-project/commit/1a1448e6568d9b11f198e510fa9c4cb6b1f4216a.diff
>
> LOG: [X86] Merge X86MCInstLowering's maxLongNopLength into emitNop and remove check for FeatureNOPL.
>
> The switch in emitNop uses 64-bit registers for nops exceeding
> 2 bytes. This isn't valid outside 64-bit mode. We could fix this
> easily enough, but there are no users that ask for more than 2
> bytes outside 64-bit mode.
>
> Inlining the method to make the coupling between the two methods
> more explicit.
>
> Added:
>      
>
> Modified:
>      llvm/lib/Target/X86/X86MCInstLower.cpp
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
> index b4db72e15060..8f3e32727371 100644
> --- a/llvm/lib/Target/X86/X86MCInstLower.cpp
> +++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
> @@ -1079,29 +1079,27 @@ void X86AsmPrinter::LowerTlsAddr(X86MCInstLower &MCInstLowering,
>     }
>   }
>   
> -/// Return the longest nop which can be efficiently decoded for the given
> -/// target cpu.  15-bytes is the longest single NOP instruction, but some
> -/// platforms can't decode the longest forms efficiently.
> -static unsigned maxLongNopLength(const X86Subtarget *Subtarget) {
> -  if (Subtarget->getFeatureBits()[X86::FeatureFast7ByteNOP])
> -    return 7;
> -  if (Subtarget->getFeatureBits()[X86::FeatureFast15ByteNOP])
> -    return 15;
> -  if (Subtarget->getFeatureBits()[X86::FeatureFast11ByteNOP])
> -    return 11;
> -  if (Subtarget->getFeatureBits()[X86::FeatureNOPL] || Subtarget->is64Bit())
> -    return 10;
> -  if (Subtarget->is32Bit())
> -    return 2;
> -  return 1;
> -}
> -
>   /// Emit the largest nop instruction smaller than or equal to \p NumBytes
>   /// bytes.  Return the size of nop emitted.
>   static unsigned emitNop(MCStreamer &OS, unsigned NumBytes,
>                           const X86Subtarget *Subtarget) {
> +  unsigned MaxNopLength = 1;
> +  if (Subtarget->is64Bit()) {
> +    // FIXME: We can use NOOPL on 32-bit targets with FeatureNOPL, but the
> +    // IndexReg/BaseReg below need to be updated.
> +    if (Subtarget->hasFeature(X86::FeatureFast7ByteNOP))
> +      MaxNopLength = 7;
> +    else if (Subtarget->hasFeature(X86::FeatureFast15ByteNOP))
> +      MaxNopLength = 15;
> +    else if (Subtarget->hasFeature(X86::FeatureFast11ByteNOP))
> +      MaxNopLength = 11;
> +    else
> +      MaxNopLength = 10;
> +  } if (Subtarget->is32Bit())
> +    MaxNopLength = 2;
> +
>     // Cap a single nop emission at the profitable value for the target
> -  NumBytes = std::min(NumBytes, maxLongNopLength(Subtarget));
> +  NumBytes = std::min(NumBytes, MaxNopLength);
>   
>     unsigned NopSize;
>     unsigned Opc, BaseReg, ScaleVal, IndexReg, Displacement, SegmentReg;
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits