Adding all X86 Processor families which can help initializing several uArch properties

[magabari]

Summary

Adding all x86 Processor families to initialize several uArch properties (based on the family)
this shows how gather cost can be initialized based on the proc. family

[RKSimon]

I really don't like this approach - we've been getting closer and closer to killing off X86ProcFamilyEnum for years now and move to a mixture of feature bits and scheduler cost driven decisions that works in a more general manner.

Assuming you intend to use getGatherOverhead() inside X86TTIImpl::getGatherScatterOpCost, might not it be better to just provide a Fast/Slow Gather feature bit, or (even better) finally start trying to use the scheduler model data within X86TTIImpl ?

[delena]

I agree with you. We also considered using scheduling model (or any other target micro-architecture data base) from TTI. It requires detailed design and enters to our long term plans.
We already started feeding micro-architecture details to LLVM, step-by-step, and it will take a time to complete.
The cost, that is kept in X86TargetTransformInfo.cpp should also be moved to .td file and it will require a special interface for communication between IR and MachineInstructions.
Using processorFamily as a property allows tuning compiler for a processor meanwhile, otherwise we just tie all patches like "gather-enabling" in one chain, waiting until all .td files are entered.

[filcab]

At the very least: This is adding a ton of unused and unneeded details. If we end up needing to dispatch on processor family, then maybe we can add (some of) these.
Otherwise, I don't think it's reasonable to add all these enum values.

lib/Target/X86/X86Subtarget.cpp

35

GatherOverhead is unsigned. Why set it to INT_MAX?

36

This could just be encoded in a bool ShouldUseGather/ShouldAvoidGather or similar.
What does having a non-boolean get us now*? Why 2?

I'm ok with getting a bool now and changing it to int/unsigned/whatever in the future, but only if we need to.

lib/Target/X86/X86Subtarget.h

19

Stray change.

21

What's a "suitable property", btw?

[delena]

lib/Target/X86/X86Subtarget.cpp

36

Gather is available since Haswell (AVX2 set). So technically, we can generate Gathers on all AVX2 processors.
But the overhead on HSW is high, Mohamed put int_max, it will be replaced later with another number. Skylake Client processor has faster Gathers than HSW and performance is similar to Skylake Server (AVX-512).
The specified overhead is relative to the Load operation. "2" is the number provided by Intel architects, we are already using it to calculate GS cost.

I want to say that "GSOverhead = 2" we have today inside X86TTI. The problem, that TTI can't distinguish between HSW and SKL. They both have Gather, but the cost is different.

I assume that TTI should provide a cost for Gather, for *all* AVX2 and AVX-512 processors, but again, it is not one value for all. Vectorizer compares this cost with scalar and interleave variants and chooses the best solution.

[craig.topper]

If we were to go the route of adding a feature bit for every CPU. I wonder if we should just convert the CPU name into an enum in the target independent subtarget classes. Going through a feature bit to create an enum is just making the number of feature bits larger. Feature bits are stored in a std::bitset that last I checked was 160 bits and is stored in some of the tablegen generated data structures. Due to limitations in various place this max size of 160 is determined by the target with the most feature bits. I believe it got as high as it is because ARM or AArch64 has added a feature bit per CPU similar to this patch.

Another option may be to allow an enum value to be passed as a separate input to ProcModel that would get stored in a separate field in the target independent subtarget. This would also avoid the feature bit pressure.

[RKSimon]

I still don't get why this can't be achieved with a single 'FeatureFastGather' feature bit, just used by Skylake CPU target - then X86TTIImpl::getGatherScatterOpCost can just call ST->hasFastGather() to decide whether to use gathers or not.

Or if you want to keep the overhead as a cost multiplier you can do something like:

unsigned getGatherOverhead() const { return HasFastGather ? 2 : UINT_MAX; }

These would be much easier to remove when you've had time to sort out the scheduler models.

[delena]

In D35348#808219, @RKSimon wrote:

I still don't get why this can't be achieved with a single 'FeatureFastGather' feature bit, just used by Skylake CPU target - then X86TTIImpl::getGatherScatterOpCost can just call ST->hasFastGather() to decide whether to use gathers or not.

Or if you want to keep the overhead as a cost multiplier you can do something like:

unsigned getGatherOverhead() const { return HasFastGather ? 2 : UINT_MAX; }

These would be much easier to remove when you've had time to sort out the scheduler models.

In order to set different numbers to different mach. Each processor that supports Gather should be able to provide a number, not only /NoGather/SlowGather/FastGather.

[RKSimon]

In D35348#808263, @delena wrote:

In order to set different numbers to different mach. Each processor that supports Gather should be able to provide a number, not only /NoGather/SlowGather/FastGather.

So how many numbers are we talking about? Is it just Skylake, SKX and Cannonlake? (KNL?) I guess we don't want to handle weaker AVX2 CPUs so we don't need a feature bit for those (default getGatherOverhead() to UINT_MAX)

Whatever happens, almost all those new processor enums are not required.

[magabari]

removing unnecessary processor
fixing uint issue

lib/Target/X86/X86Subtarget.h

21

it depends in usage of the isAtom\isSLM. for example in X86TargetTransformInfo.cpp::getMaxInterleaveFactor the property will be getMaxInterleaveFactor

[RKSimon]

Before you continue with this, is there any chance that you can create a phab for the follow on patches that are dependent on this please? To get a better idea of what you are needing this for.

lib/Target/X86/X86Subtarget.cpp

31

You got to all that trouble of adding the other intel cpus and then just need IntelSkylake and IntelSKX?

[delena]

lib/Target/X86/X86Subtarget.cpp

32

Mohamed, you have to add ScatterOverhead for SKX.

[magabari]

lib/Target/X86/X86Subtarget.cpp

31

This is only one property. even HSW has Gather and later will add it's overhead (currently we dont recommend generating gathers for HSW).

There is other properties that will need to return different values based on the family. so my patch just getting the infrastructure ready for this.

I will upload my patch soon, it uses the "getGatherOverhead" property from the subtarget.

[magabari]

adding ScatterOverhead and updating the max overhead.
max overhead can't be MAX_INT or MAX_UINT because it will overflow on the CM calculation and causing a wrong decision.

[delena]

lib/Target/X86/X86Subtarget.h

19

You can use Others instead of Generic.

[magabari]

replacing Generic with Others

[craig.topper]

lib/Target/X86/X86Subtarget.cpp

29

Why can't this just be done by comparing the CPU string? What did we get by creating an enum that is named the same as the CPU string?

[delena]

lib/Target/X86/X86.td

22–1

You don't need Others here, I think.

23

Why do you need this change?

[magabari]

lib/Target/X86/X86Subtarget.cpp

29

X86ProcFamily enum was already exist and we have just added more values. In general I think it's better to parse the CPU strings in one place and after that to start using the enum values instead of comparing the strings all the time.

[magabari]

ping

[magabari]

ping ping

[RKSimon]

lib/Target/X86/X86Subtarget.h

18–1

Do this in X86Subtarget::initializeEnvironment() ?

21

Do this in X86Subtarget::initializeEnvironment() ?

[magabari]

Simon, could you please take a look on the changes and see if its okay now?

[delena]

lib/Target/X86/X86Subtarget.cpp

287

Gather is available since Haswell (AVX2 set). So technically, we can generate Gathers on all AVX2 processors.
But the overhead on HSW is high. Skylake Client processor has faster Gathers than HSW and performance is similar to Skylake Server (AVX-512).
The specified overhead is relative to the Load operation. "2" is the number provided by Intel architects, we are already using it to calculate GS cost.
if (X86ProcFamily == IntelSkylake || hasAVX512)

GatherOverhead = 2;

if (hasAVX512) // SKX and KNL fail here

ScatterOverhead = 2;

290

Please remove.

306

Please remove the "else", the values are already initialized.

[magabari]

fixing Elena comments

[magabari]

This comment was removed by magabari.

[delena]

LGTM + a minor comment fix

lib/Target/X86/X86Subtarget.cpp

291

*we are already using it to calculate GS cost ->
This parameter is used for cost estimation of Gather Op and comparison with other alternatives.

[magabari]

fixed comment

[RKSimon]

In D35348#853420, @magabari wrote:

Simon, could you please take a look on the changes and see if its okay now?

LGTM - I still think this patch only needs to add IntelSkylake but it's a lot better than it was.

lib/Target/X86/X86.td

 // Get the target-independent interfaces which we are implementing...
 //
 include "llvm/Target/Target.td"
 
 //===----------------------------------------------------------------------===//
 // X86 Subtarget state
 //
 
 def Mode64Bit : SubtargetFeature<"64bit-mode", "In64BitMode", "true",
                                   "64-bit mode (x86_64)">;
 def Mode32Bit : SubtargetFeature<"32bit-mode", "In32BitMode", "true",
                                   "32-bit mode (80386)">;
 def Mode16Bit : SubtargetFeature<"16bit-mode", "In16BitMode", "true",
                                   "16-bit mode (i8086)">;
 
 //===----------------------------------------------------------------------===//
 // X86 Subtarget features

 include "X86Schedule.td"
 
 def ProcIntelAtom : SubtargetFeature<"atom", "X86ProcFamily", "IntelAtom",
                     "Intel Atom processors">;
 def ProcIntelSLM  : SubtargetFeature<"slm", "X86ProcFamily", "IntelSLM",
                     "Intel Silvermont processors">;
 def ProcIntelGLM  : SubtargetFeature<"glm", "X86ProcFamily", "IntelGLM",
                     "Intel Goldmont processors">;
+def ProcIntelHSW  : SubtargetFeature<"haswell", "X86ProcFamily",
+                    "IntelHaswell", "Intel Haswell processors">;
+def ProcIntelBDW  : SubtargetFeature<"broadwell", "X86ProcFamily",
+                    "IntelBroadwell", "Intel Broadwell processors">;
+def ProcIntelSKL  : SubtargetFeature<"skylake", "X86ProcFamily",
+                    "IntelSkylake", "Intel Skylake processors">;
+def ProcIntelKNL  : SubtargetFeature<"knl", "X86ProcFamily",
+                    "IntelKNL", "Intel Knights Landing processors">;
+def ProcIntelSKX  : SubtargetFeature<"skx", "X86ProcFamily",
+                    "IntelSKX", "Intel Skylake Server processors">;
+def ProcIntelCNL  : SubtargetFeature<"cannonlake", "X86ProcFamily",
+                    "IntelCannonlake", "Intel Cannonlake processors">;
 
 class Proc<string Name, list<SubtargetFeature> Features>
  : ProcessorModel<Name, GenericModel, Features>;
 
 def : Proc<"generic",         [FeatureX87, FeatureSlowUAMem16]>;
 def : Proc<"i386",            [FeatureX87, FeatureSlowUAMem16]>;
 def : Proc<"i486",            [FeatureX87, FeatureSlowUAMem16]>;
 def : Proc<"i586",            [FeatureX87, FeatureSlowUAMem16]>;
def HSWFeatures : ProcessorFeatures<IVBFeatures.Value, [
   FeatureERMSB,
   FeatureFMA,
   FeatureLZCNT,
   FeatureMOVBE,
   FeatureSlowIncDec
 ]>;
 
 class HaswellProc<string Name> : ProcModel<Name, HaswellModel,
-                                           HSWFeatures.Value, []>;
+                                           HSWFeatures.Value, [
+    ProcIntelHSW
+  ]>;
 def : HaswellProc<"haswell">;
 def : HaswellProc<"core-avx2">; // Legacy alias.
 
 def BDWFeatures : ProcessorFeatures<HSWFeatures.Value, [
+  ProcIntelBDW,
   FeatureADX,
   FeatureRDSEED
 ]>;
 class BroadwellProc<string Name> : ProcModel<Name, HaswellModel,
                                              BDWFeatures.Value, []>;
 def : BroadwellProc<"broadwell">;
 
 def SKLFeatures : ProcessorFeatures<BDWFeatures.Value, [
   FeatureMPX,
   FeatureRTM,
   FeatureXSAVEC,
   FeatureXSAVES,
   FeatureSGX,
   FeatureCLFLUSHOPT,
   FeatureFastVectorFSQRT
 ]>;
 
 // FIXME: define SKL model
 class SkylakeClientProc<string Name> : ProcModel<Name, HaswellModel,
-                                                 SKLFeatures.Value, []>;
+                                                 SKLFeatures.Value, [
+    ProcIntelSKL
+  ]>;
 def : SkylakeClientProc<"skylake">;
 
 // FIXME: define KNL model
 class KnightsLandingProc<string Name> : ProcModel<Name, HaswellModel,
                                                   IVBFeatures.Value, [
+  ProcIntelKNL,                                                  
   FeatureAVX512,
   FeatureERI,
   FeatureCDI,
   FeaturePFI,
   FeaturePREFETCHWT1,
   FeatureADX,
   FeatureRDSEED,
   FeatureMOVBE,
def SKXFeatures : ProcessorFeatures<SKLFeatures.Value, [
   FeatureBWI,
   FeatureVLX,
   FeaturePKU,
   FeatureCLWB
 ]>;
 
 // FIXME: define SKX model
 class SkylakeServerProc<string Name> : ProcModel<Name, HaswellModel,
-                                                 SKXFeatures.Value, []>;
+                                                 SKXFeatures.Value, [
+    ProcIntelSKX
+  ]>;
 def : SkylakeServerProc<"skylake-avx512">;
 def : SkylakeServerProc<"skx">; // Legacy alias.
 
 def CNLFeatures : ProcessorFeatures<SKXFeatures.Value, [
   FeatureVBMI,
   FeatureIFMA,
   FeatureSHA
 ]>;
 
 class CannonlakeProc<string Name> : ProcModel<Name, HaswellModel,
-                                              CNLFeatures.Value, []>;
+                                              CNLFeatures.Value, [
+    ProcIntelCNL
+  ]>;
 def : CannonlakeProc<"cannonlake">;
 
 // AMD CPUs.
 
 def : Proc<"k6",              [FeatureX87, FeatureSlowUAMem16, FeatureMMX]>;
 def : Proc<"k6-2",            [FeatureX87, FeatureSlowUAMem16, Feature3DNow]>;
 def : Proc<"k6-3",            [FeatureX87, FeatureSlowUAMem16, Feature3DNow]>;
 def : Proc<"athlon",          [FeatureX87, FeatureSlowUAMem16, Feature3DNowA,

lib/Target/X86/X86Subtarget.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_X86_X86SUBTARGET_H
 #define LLVM_LIB_TARGET_X86_X86SUBTARGET_H
 
 #include "X86FrameLowering.h"
 #include "X86ISelLowering.h"
 #include "X86InstrInfo.h"
 #include "X86SelectionDAGInfo.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelector.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
 #include "llvm/CodeGen/GlobalISel/RegisterBankInfo.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/MC/MCInstrItineraries.h"
 #include "llvm/Target/TargetMachine.h"
enum X86SSEEnum {
     NoSSE, SSE1, SSE2, SSE3, SSSE3, SSE41, SSE42, AVX, AVX2, AVX512F
   };
 
   enum X863DNowEnum {
     NoThreeDNow, MMX, ThreeDNow, ThreeDNowA
   };
 
   enum X86ProcFamilyEnum {
-    Others, IntelAtom, IntelSLM, IntelGLM
+    Others, 
+    IntelAtom,
+    IntelSLM,
+    IntelGLM,
+    IntelHaswell,
+    IntelBroadwell,
+    IntelSkylake,
+    IntelKNL,
+    IntelSKX,
+    IntelCannonlake
   };
 
   /// X86 processor family: Intel Atom, and others
   X86ProcFamilyEnum X86ProcFamily;
 
   /// Which PIC style to use
   PICStyles::Style PICStyle;
 
private:
   bool In64BitMode;
 
   /// True if compiling for 32-bit, false for 16-bit or 64-bit.
   bool In32BitMode;
 
   /// True if compiling for 16-bit, false for 32-bit or 64-bit.
   bool In16BitMode;
 
+  /// Contains the Overhead of gather\scatter instructions
+  int GatherOverhead;
+  int ScatterOverhead;
+
   X86SelectionDAGInfo TSInfo;
   // Ordering here is important. X86InstrInfo initializes X86RegisterInfo which
   // X86TargetLowering needs.
   X86InstrInfo InstrInfo;
   X86TargetLowering TLInfo;
   X86FrameLowering FrameLowering;
 
 public:
public:
   bool hasLAHFSAHF() const { return HasLAHFSAHF; }
   bool hasMWAITX() const { return HasMWAITX; }
   bool hasCLZERO() const { return HasCLZERO; }
   bool isBTMemSlow() const { return IsBTMemSlow; }
   bool isSHLDSlow() const { return IsSHLDSlow; }
   bool isPMULLDSlow() const { return IsPMULLDSlow; }
   bool isUnalignedMem16Slow() const { return IsUAMem16Slow; }
   bool isUnalignedMem32Slow() const { return IsUAMem32Slow; }
+  int getGatherOverhead() const { return GatherOverhead; }
+  int getScatterOverhead() const { return ScatterOverhead; }
   bool hasSSEUnalignedMem() const { return HasSSEUnalignedMem; }
   bool hasCmpxchg16b() const { return HasCmpxchg16b; }
   bool useLeaForSP() const { return UseLeaForSP; }
   bool hasFastPartialYMMorZMMWrite() const {
     return HasFastPartialYMMorZMMWrite;
   }
   bool hasFastScalarFSQRT() const { return HasFastScalarFSQRT; }
   bool hasFastVectorFSQRT() const { return HasFastVectorFSQRT; }
public:
   bool hasBWI() const { return HasBWI; }
   bool hasVLX() const { return HasVLX; }
   bool hasPKU() const { return HasPKU; }
   bool hasMPX() const { return HasMPX; }
   bool hasCLFLUSHOPT() const { return HasCLFLUSHOPT; }
 
   bool isXRaySupported() const override { return is64Bit(); }
 
+  X86ProcFamilyEnum getProcFamily() const { return X86ProcFamily; }
+
+  /// TODO: to be removed later and replaced with suitable properties
   bool isAtom() const { return X86ProcFamily == IntelAtom; }
   bool isSLM() const { return X86ProcFamily == IntelSLM; }
   bool useSoftFloat() const { return UseSoftFloat; }
 
   /// Use mfence if we have SSE2 or we're on x86-64 (even if we asked for
   /// no-sse2). There isn't any reason to disable it if the target processor
   /// supports it.
   bool hasMFence() const { return hasSSE2() || is64Bit(); }

lib/Target/X86/X86Subtarget.cpp

 #include "X86TargetMachine.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/InstructionSelect.h"
 #include "llvm/CodeGen/GlobalISel/Legalizer.h"
 #include "llvm/CodeGen/GlobalISel/RegBankSelect.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include <cassert>
 #include <string>
 
 #if defined(_MSC_VER)
 #include <intrin.h>
 #endif
 
void X86Subtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
 
   // Stack alignment is 16 bytes on Darwin, Linux, kFreeBSD and Solaris (both
   // 32 and 64 bit) and for all 64-bit targets.
   if (StackAlignOverride)
     stackAlignment = StackAlignOverride;
   else if (isTargetDarwin() || isTargetLinux() || isTargetSolaris() ||
            isTargetKFreeBSD() || In64BitMode)
     stackAlignment = 16;
+  
+  switch(X86ProcFamily) {
+    case IntelSkylake:
+      GatherOverhead = 2;
+      ScatterOverhead = 1024; // not relevant for AVX2
+      break;
+    case IntelSKX:
+      GatherOverhead = 2;
+      ScatterOverhead = 2; 
+      break;
+    default:
+      // Currently picking high overheads for other targets in order not to be selected
+      // TODO: need to get uArch overheads for hsw\bdw
+      // FIXME: giving 1024 as a max int because it may overflow in the CM calucation causing a 
+      //   wrong desicion or negative values, maybe need to move to FP?
+      if (hasAVX512()) {
+        GatherOverhead = 2;
+        ScatterOverhead = 2;
+      }
+      else {
+        GatherOverhead = 1024;
+        ScatterOverhead = 1024;
+      }  
+  }
 }
 
 void X86Subtarget::initializeEnvironment() {
   X86SSELevel = NoSSE;
   X863DNowLevel = NoThreeDNow;
   HasX87 = false;
   HasCMov = false;
   HasX86_64 = false;
void X86Subtarget::initializeEnvironment() {
   LEAUsesAG = false;
   SlowLEA = false;
   Slow3OpsLEA = false;
   SlowIncDec = false;
   stackAlignment = 4;
   // FIXME: this is a known good value for Yonah. How about others?
   MaxInlineSizeThreshold = 128;
   UseSoftFloat = false;
+  X86ProcFamily = Others;
+  GatherOverhead = 1024;
+  ScatterOverhead = 1024;
 }
 
 X86Subtarget &X86Subtarget::initializeSubtargetDependencies(StringRef CPU,
                                                             StringRef FS) {
   initializeEnvironment();
   initSubtargetFeatures(CPU, FS);
   return *this;
 }