AArch64: Do not test for CPUs, use SubtargetFeatures

[MatzeB]

Summary

Testing for specific CPUs has a number of problems, better use subtarget
features:

• When some tweak is added for a specific CPU it is often desirable for the next version of that CPU as well, yet we often forget to add it.
• It is hard to keep track of checks scattered around the target code; Declaring all target specifics together with the CPU in the tablegen file is a clear representation.
• Subtarget features can be tweaked from the command line.

To discourage people from using CPU checks in the future I removed the
isCortexXX(), isCyclone(), ... functions. I added an getProcFamily()
function for exceptional circumstances but made it clear in the comment
that usage is discouraged.

Reformat feature list in AArch64.td to have 1 feature per line in
alphabetical order to simplify merging and sorting for out of tree
tweaks.

No functional change intended.

[rovka]

LGTM, but you should wait for someone more versed to approve.

lib/Target/AArch64/AArch64.td

72 ↗

(On Diff #58874)

typo: Predictable

lib/Target/AArch64/AArch64ISelLowering.cpp

8813 ↗

(On Diff #58874)

You could make this comment sound more generic (e.g. use "Some subtargets" instead of "Cyclone").

lib/Target/AArch64/AArch64InstrInfo.cpp

1524 ↗

(On Diff #58874)

I think you can remove or rephrase this comment.

1804 ↗

(On Diff #58874)

I think you can remove/rephrase this comment as well.

[rengolin]

Hi Matthias,

I think this is a really good change to make, but I have some comments in addition to Diana...

cheers,
--renato

lib/Target/AArch64/AArch64.td

76 ↗

(On Diff #58874)

Can't this be a property?

102 ↗

(On Diff #58874)

This sounds like an odd one.

lib/Target/AArch64/AArch64Subtarget.cpp

153 ↗

(On Diff #58874)

What does load balancing FP have to do with PBQP?

Also, this moves from just A57 to both A57 and A53.

lib/Target/AArch64/AArch64Subtarget.h

36 ↗

(On Diff #58874)

I'm not convinced that this will discourage people to avoid CPU checking more than having the isCPU methods... But it also won't encourage.

49 ↗

(On Diff #58874)

Since we only have one, maybe try to not use the CPU name in it? Otherwise, we'd encourage things like "CycloneLike", which aren't much of an advantage of isCyclone.

[rengolin]

Arnaud, please check the PBQP constraint, if it makes sense...

[MatzeB]

Thanks for the reviews and irc discussions!
Some of the subtarget features I felt indeed strange because they were adjusting some cost settings rather than just enabling/disabling a tweak. We lack a good mechanism to specify those in tablegen today so instead I opted to move all of those into a newly introduced initializeProperties() function.

We should support setting non-boolean properties based on the CPU with a tablegen feature. However that is material for a future patch.

[rengolin]

Thanks Matthias, LGTM!

lib/Target/AArch64/AArch64Subtarget.cpp

52 ↗

(On Diff #59118)

Maybe a FIXME line saying this should also be table-gen'd. (your future patch)

[mssimpso]

lib/Target/AArch64/AArch64Subtarget.cpp

69 ↗

(On Diff #59118)

Hi Matthias,

I haven't yet looked at this patch in much detail. But would you mind double checking the Exynos properties before committing this change? I didn't think Exynos and Kryo shared these value. Shouldn't MaxInterleaveFactor = 2 and VectorInsertExtractBaseCost = 3 for Exynos? The Kryo numbers look correct. Perhaps there's a missing "break" here?

[MatzeB]

lib/Target/AArch64/AArch64Subtarget.cpp

69 ↗

(On Diff #59118)

Indeed there are too many fallthroughs happening here, I'll double check this switch and update the diff here.

[MatzeB]

Remove unintented switch fallthroughs in initializeProperties().

I'll wait at least for @mssimpso and @aadg approval before committing this.

[rengolin]

Ok

[mssimpso]

lib/Target/AArch64/AArch64Subtarget.cpp

69 ↗

(On Diff #59283)

Hi Matthias,

The switch looks good to me now. Thanks.

[aadg]

This looks OK to me from a PBQP perspective (see comment).

lib/Target/AArch64/AArch64Subtarget.cpp

167 ↗

(On Diff #59283)

FP load balancing has to be done with a specific pass when using the GreedyRegAlloc. PBQPRegAlloc can do it right away (no need for a separate pass), but the allocator need to learn the FP load balancing constraint it has to solve (on top of the "standard" constraints).

Enabling it for both A57 and A53 is I think the right thing to do.

[evandro]

Somewhat an RFC, but perhaps ARMProcFamilyEnum values should be defined as masks so that, when checking for multiple families, one can write:
Subtarget.getProcFamily() & (AArch64Subtarget::CortexA57 | AArch64Subtarget::ExynosM1)

Instead of:
Subtarget.getProcFamily() == AArch64Subtarget::CortexA57 || Subtarget.getProcFamily() == AArch64Subtarget::ExynosM1

Thoughts?

[rengolin]

In D20762#453405, @evandro wrote:

Somewhat an RFC, but perhaps ARMProcFamilyEnum values should be defined as masks so that, when checking for multiple families,

Hi Evandro,

That'd be a good idea if we were trying to encourage people to use them, but we're not.

They're just there because not all could be easily moved into features, but they should, eventually. And all new such decisions should be made into features, so we should also avoid usage of that function like the plague. :)

cheers,
--renato

[evandro]

Oi, Renato.

I'm for discouraging the use of Subtarget.getProcFamily(), but I don't think that it's realistic to suppose that every instance should be replaced with a feature. The only use for now can hardly be folded into a feature. How would you envision such minutiae as features?

Obrigado.

[rengolin]

In D20762#453433, @evandro wrote:

I'm for discouraging the use of Subtarget.getProcFamily(), but I don't think that it's realistic to suppose that every instance should be replaced with a feature. The only use for now can hardly be folded into a feature. How would you envision such minutiae as features?

I don't think we'll be able to (or if it's even desirable that we do), but continue discouraging its usage (by making it awkward) will move the balance towards more features, less CPU names. I think that's all we're trying to do.

Makes sense?

Flw,
--renato

[MatzeB]

In D20762#453433, @evandro wrote:

Oi, Renato.

I'm for discouraging the use of Subtarget.getProcFamily(), but I don't think that it's realistic to suppose that every instance should be replaced with a feature. The only use for now can hardly be folded into a feature. How would you envision such minutiae as features?

Obrigado.

I just wanted to stress again that we really really should try to create subtarget features as much as possible (or alternatively initialize properties in initializeProperties()). The last remaining usage of getProcFamily() in isAsCheapAsAMove() smells bad to me. I just don't have a good alternative yet. It would be nice to express things there in terms of the scheduling model, but that needs some more investigation in how to that exactly.

[evandro]

Not everything is visible at initializeProperties(). For example, were a target to set its own function alignment, setPrefFunctionAlignment() cannot be invoked outside of the TargetLowering hierarchy.

[MatzeB]

In D20762#453857, @evandro wrote:

Not everything is visible at initializeProperties(). For example, were a target to set its own function alignment, setPrefFunctionAlignment() cannot be invoked outside of the TargetLowering hierarchy.

You could still add an unsigned PrefFunctionAlignment; field in SubtargetInfo and then do setPrefFunctionAlignment(Subtarget.getPrefFunctionAlignment());` in the TargetLowering constructor. We already use this tactic for some properties of TargetTransformInfo (CacheLineSize, PrefetchDistance, ...).

It does require adding an extra indirection but I believe it is worth it: Otherwise when you add a new CPU how will you find all the places relevant to your new CPU? What if you want to experiment with code generation pre silicon in simulators (say the next version of your CPU pretty much behaves like the previous one but has no problem with misaligned functions anymore). I also liked the fact how adding a subtarget feature forced you to write a little description...

[evandro]

OK, let me hack a patch and take the discussion to a concrete case and let's see what can be sensibly done there.

Thanks for your patience.

llvm/trunk/lib/Target/AArch64/AArch64.td

def FeatureStrictAlign : SubtargetFeature<"strict-align",
                                           "StrictAlign", "true",
                                           "Disallow all unaligned memory "
                                           "access">;
 
 def FeatureReserveX18 : SubtargetFeature<"reserve-x18", "ReserveX18", "true",
                                          "Reserve X18, making it unavailable "
                                          "as a GPR">;
 
+def FeatureMergeNarrowLd : SubtargetFeature<"merge-narrow-ld",
+                                            "MergeNarrowLoads", "true",
+                                            "Merge narrow load instructions">;
+
+def FeatureUseAA : SubtargetFeature<"use-aa", "UseAA", "true",
+                                    "Use alias analysis during codegen">;
+
+def FeatureBalanceFPOps : SubtargetFeature<"balance-fp-ops", "BalanceFPOps",
+    "true",
+    "balance mix of odd and even D-registers for fp multiply(-accumulate) ops">;
+
+def FeaturePredictableSelectIsExpensive : SubtargetFeature<
+    "predictable-select-expensive", "PredictableSelectIsExpensive", "true",
+    "Prefer likely predicted branches over selects">;
+
+def FeatureCustomCheapAsMoveHandling : SubtargetFeature<"custom-cheap-as-move",
+    "CustomAsCheapAsMove", "true",
+    "Use custom code for TargetInstrInfo::isAsCheapAsAMove()">;
+
+def FeaturePostRAScheduler : SubtargetFeature<"use-postra-scheduler",
+    "UsePostRAScheduler", "true", "Schedule again after register allocation">;
+
+def FeatureSlowMisaligned128Store : SubtargetFeature<"slow-misaligned-128store",
+    "Misaligned128StoreIsSlow", "true", "Misaligned 128 bit stores are slow">;
+
+def FeatureAvoidQuadLdStPairs : SubtargetFeature<"no-quad-ldst-pairs",
+    "AvoidQuadLdStPairs", "true",
+    "Do not form quad load/store pair operations">;
+
+def FeatureAlternateSExtLoadCVTF32Pattern : SubtargetFeature<
+    "alternate-sextload-cvt-f32-pattern", "UseAlternateSExtLoadCVTF32Pattern",
+    "true", "Use alternative pattern for sextload convert to f32">;
+
+def FeatureMacroOpFusion : SubtargetFeature<
+    "macroop-fusion", "HasMacroOpFusion", "true",
+    "CPU supports macro op fusion">;
+
+def FeatureDisableLatencySchedHeuristic : SubtargetFeature<
+    "disable-latency-sched-heuristic", "DisableLatencySchedHeuristic", "true",
+    "Disable latency scheduling heuristic">;
+
+def FeatureUseRSqrt : SubtargetFeature<
+    "use-reverse-square-root", "UseRSqrt", "true", "Use reverse square root">;
+
 //===----------------------------------------------------------------------===//
 // Architectures.
 //
 
 def HasV8_1aOps : SubtargetFeature<"v8.1a", "HasV8_1aOps", "true",
   "Support ARM v8.1a instructions", [FeatureCRC]>;
 
 def HasV8_2aOps : SubtargetFeature<"v8.2a", "HasV8_2aOps", "true",

 //
 include "AArch64SchedA53.td"
 include "AArch64SchedA57.td"
 include "AArch64SchedCyclone.td"
 include "AArch64SchedM1.td"
 include "AArch64SchedKryo.td"
 
 def ProcA35     : SubtargetFeature<"a35", "ARMProcFamily", "CortexA35",
-                                   "Cortex-A35 ARM processors",
-                                   [FeatureFPARMv8,
-                                   FeatureNEON,
-                                   FeatureCrypto,
+                                   "Cortex-A35 ARM processors", [
                                    FeatureCRC,
-                                   FeaturePerfMon]>;
+                                   FeatureCrypto,
+                                   FeatureFPARMv8,
+                                   FeatureNEON,
+                                   FeaturePerfMon
+                                   ]>;
 
 def ProcA53     : SubtargetFeature<"a53", "ARMProcFamily", "CortexA53",
-                                   "Cortex-A53 ARM processors",
-                                   [FeatureFPARMv8,
-                                   FeatureNEON,
-                                   FeatureCrypto,
+                                   "Cortex-A53 ARM processors", [
+                                   FeatureBalanceFPOps,
                                    FeatureCRC,
-                                   FeaturePerfMon]>;
+                                   FeatureCrypto,
+                                   FeatureCustomCheapAsMoveHandling,
+                                   FeatureFPARMv8,
+                                   FeatureNEON,
+                                   FeaturePerfMon,
+                                   FeaturePostRAScheduler,
+                                   FeatureUseAA
+                                   ]>;
 
 def ProcA57     : SubtargetFeature<"a57", "ARMProcFamily", "CortexA57",
-                                   "Cortex-A57 ARM processors",
-                                   [FeatureFPARMv8,
-                                   FeatureNEON,
-                                   FeatureCrypto,
+                                   "Cortex-A57 ARM processors", [
+                                   FeatureBalanceFPOps,
                                    FeatureCRC,
-                                   FeaturePerfMon]>;
+                                   FeatureCrypto,
+                                   FeatureCustomCheapAsMoveHandling,
+                                   FeatureFPARMv8,
+                                   FeatureMergeNarrowLd,
+                                   FeatureNEON,
+                                   FeaturePerfMon,
+                                   FeaturePostRAScheduler,
+                                   FeaturePredictableSelectIsExpensive
+                                   ]>;
 
 def ProcCyclone : SubtargetFeature<"cyclone", "ARMProcFamily", "Cyclone",
-                                   "Cyclone",
-                                   [FeatureFPARMv8,
-                                   FeatureNEON,
+                                   "Cyclone", [
+                                   FeatureAlternateSExtLoadCVTF32Pattern,
                                    FeatureCrypto,
+                                   FeatureDisableLatencySchedHeuristic,
+                                   FeatureFPARMv8,
+                                   FeatureMacroOpFusion,
+                                   FeatureNEON,
                                    FeaturePerfMon,
-                                   FeatureZCRegMove, FeatureZCZeroing]>;
+                                   FeatureSlowMisaligned128Store,
+                                   FeatureZCRegMove,
+                                   FeatureZCZeroing
+                                   ]>;
 
 def ProcExynosM1 : SubtargetFeature<"exynosm1", "ARMProcFamily", "ExynosM1",
-                                    "Samsung Exynos-M1 processors",
-                                    [FeatureFPARMv8,
-                                    FeatureNEON,
-                                    FeatureCrypto,
+                                    "Samsung Exynos-M1 processors", [
+                                    FeatureAvoidQuadLdStPairs,
                                     FeatureCRC,
-                                    FeaturePerfMon]>;
+                                    FeatureCrypto,
+                                    FeatureCustomCheapAsMoveHandling,
+                                    FeatureFPARMv8,
+                                    FeatureNEON,
+                                    FeaturePerfMon,
+                                    FeatureUseRSqrt
+                                    ]>;
 
 def ProcKryo    : SubtargetFeature<"kryo", "ARMProcFamily", "Kryo",
-                                   "Qualcomm Kryo processors",
-                                   [FeatureFPARMv8,
-                                   FeatureNEON,
-                                   FeatureCrypto,
+                                   "Qualcomm Kryo processors", [
                                    FeatureCRC,
-                                   FeaturePerfMon]>;
-
-def : ProcessorModel<"generic", NoSchedModel, [FeatureFPARMv8,
-                                              FeatureNEON,
-                                              FeatureCRC,
-                                              FeaturePerfMon]>;
+                                   FeatureCrypto,
+                                   FeatureCustomCheapAsMoveHandling,
+                                   FeatureFPARMv8,
+                                   FeatureMergeNarrowLd,
+                                   FeaturePerfMon,
+                                   FeaturePostRAScheduler,
+                                   FeaturePredictableSelectIsExpensive
+                                   ]>;
+
+def : ProcessorModel<"generic", NoSchedModel, [
+                     FeatureFPARMv8,
+                     FeatureNEON,
+                     FeaturePerfMon,
+                     FeaturePostRAScheduler
+                     ]>;
 
 // FIXME: Cortex-A35 is currently modelled as a Cortex-A53
 def : ProcessorModel<"cortex-a35", CortexA53Model, [ProcA35]>;
 def : ProcessorModel<"cortex-a53", CortexA53Model, [ProcA53]>;
 def : ProcessorModel<"cortex-a57", CortexA57Model, [ProcA57]>;
 // FIXME: Cortex-A72 and Cortex-A73 are currently modelled as an Cortex-A57.
 def : ProcessorModel<"cortex-a72", CortexA57Model, [ProcA57]>;
 def : ProcessorModel<"cortex-a73", CortexA57Model, [ProcA57]>;

llvm/trunk/lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp

 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
 
 bool AArch64A57FPLoadBalancing::runOnMachineFunction(MachineFunction &F) {
   if (skipFunction(*F.getFunction()))
     return false;
 
-  // Don't do anything if this isn't an A53 or A57.
-  if (!(F.getSubtarget<AArch64Subtarget>().isCortexA53() ||
-        F.getSubtarget<AArch64Subtarget>().isCortexA57()))
+  if (!F.getSubtarget<AArch64Subtarget>().balanceFPOps())
     return false;
 
   bool Changed = false;
   DEBUG(dbgs() << "***** AArch64A57FPLoadBalancing *****\n");
 
   MRI = &F.getRegInfo();
   TRI = F.getRegInfo().getTargetRegisterInfo();
   RCI.runOnMachineFunction(F);

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

for (MVT Ty : {MVT::v2f32, MVT::v4f32, MVT::v2f64}) {
       setOperationAction(ISD::FNEARBYINT, Ty, Legal);
       setOperationAction(ISD::FCEIL, Ty, Legal);
       setOperationAction(ISD::FRINT, Ty, Legal);
       setOperationAction(ISD::FTRUNC, Ty, Legal);
       setOperationAction(ISD::FROUND, Ty, Legal);
     }
   }
 
-  // Prefer likely predicted branches to selects on out-of-order cores.
-  if (Subtarget->isCortexA57() || Subtarget->isKryo())
-    PredictableSelectIsExpensive = true;
+  PredictableSelectIsExpensive = Subtarget->predictableSelectIsExpensive();
 }
 
 void AArch64TargetLowering::addTypeForNEON(MVT VT, MVT PromotedBitwiseVT) {
   if (VT == MVT::v2f32 || VT == MVT::v4f16) {
     setOperationAction(ISD::LOAD, VT, Promote);
     AddPromotedToType(ISD::LOAD, VT, MVT::v2i32);
 
     setOperationAction(ISD::STORE, VT, Promote);

 
 bool AArch64TargetLowering::allowsMisalignedMemoryAccesses(EVT VT,
                                                            unsigned AddrSpace,
                                                            unsigned Align,
                                                            bool *Fast) const {
   if (Subtarget->requiresStrictAlign())
     return false;
 
-  // FIXME: This is mostly true for Cyclone, but not necessarily others.
   if (Fast) {
-    // FIXME: Define an attribute for slow unaligned accesses instead of
-    // relying on the CPU type as a proxy.
-    // On Cyclone, unaligned 128-bit stores are slow.
-    *Fast = !Subtarget->isCyclone() || VT.getStoreSize() != 16 ||
+    // Some CPUs are fine with unaligned stores except for 128-bit ones.
+    *Fast = !Subtarget->isMisaligned128StoreSlow() || VT.getStoreSize() != 16 ||
             // See comments in performSTORECombine() for more details about
             // these conditions.
 
             // Code that uses clang vector extensions can mark that it
             // wants unaligned accesses to be treated as fast by
             // underspecifying alignment to be 1 or 2.
             Align <= 2 ||
 
static SDValue split16BStores(SDNode *N, TargetLowering::DAGCombinerInfo &DCI,
   StoreSDNode *S = cast<StoreSDNode>(N);
   if (S->isVolatile())
     return SDValue();
 
   // FIXME: The logic for deciding if an unaligned store should be split should
   // be included in TLI.allowsMisalignedMemoryAccesses(), and there should be
   // a call to that function here.
 
-  // Cyclone has bad performance on unaligned 16B stores when crossing line and
-  // page boundaries. We want to split such stores.
-  if (!Subtarget->isCyclone())
+  if (!Subtarget->isMisaligned128StoreSlow())
     return SDValue();
 
   // Don't split at -Oz.
   if (DAG.getMachineFunction().getFunction()->optForMinSize())
     return SDValue();
 
   SDValue StVal = S->getValue();
   EVT VT = StVal.getValueType();

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp

static bool canBeExpandedToORR(const MachineInstr *MI, unsigned BitSize) {
   uint64_t UImm = Imm << (64 - BitSize) >> (64 - BitSize);
   uint64_t Encoding;
   return AArch64_AM::processLogicalImmediate(UImm, BitSize, Encoding);
 }
 
 // FIXME: this implementation should be micro-architecture dependent, so a
 // micro-architecture target hook should be introduced here in future.
 bool AArch64InstrInfo::isAsCheapAsAMove(const MachineInstr *MI) const {
-  if (!Subtarget.isCortexA57() && !Subtarget.isCortexA53() &&
-      !Subtarget.isExynosM1() && !Subtarget.isKryo())
+  if (!Subtarget.hasCustomCheapAsMoveHandling())
     return MI->isAsCheapAsAMove();
 
   unsigned Imm;
 
   switch (MI->getOpcode()) {
   default:
     return false;
 
   // add/sub on register without shift
   case AArch64::ADDWri:
   case AArch64::ADDXri:
   case AArch64::SUBWri:
   case AArch64::SUBXri:
-    return (Subtarget.isExynosM1() ||
+    return (Subtarget.getProcFamily() == AArch64Subtarget::ExynosM1 ||
             MI->getOperand(3).getImm() == 0);
 
   // add/sub on register with shift
   case AArch64::ADDWrs:
   case AArch64::ADDXrs:
   case AArch64::SUBWrs:
   case AArch64::SUBXrs:
     Imm = MI->getOperand(3).getImm();
-    return (Subtarget.isExynosM1() &&
+    return (Subtarget.getProcFamily() == AArch64Subtarget::ExynosM1 &&
             AArch64_AM::getArithShiftValue(Imm) < 4);
 
   // logical ops on immediate
   case AArch64::ANDWri:
   case AArch64::ANDXri:
   case AArch64::EORWri:
   case AArch64::EORXri:
   case AArch64::ORRWri:
bool AArch64InstrInfo::isAsCheapAsAMove(const MachineInstr *MI) const {
   case AArch64::EONXrs:
   case AArch64::EORWrs:
   case AArch64::EORXrs:
   case AArch64::ORNWrs:
   case AArch64::ORNXrs:
   case AArch64::ORRWrs:
   case AArch64::ORRXrs:
     Imm = MI->getOperand(3).getImm();
-    return (Subtarget.isExynosM1() &&
+    return (Subtarget.getProcFamily() == AArch64Subtarget::ExynosM1 &&
             AArch64_AM::getShiftValue(Imm) < 4 &&
             AArch64_AM::getShiftType(Imm) == AArch64_AM::LSL);
 
   // If MOVi32imm or MOVi64imm can be expanded into ORRWri or
   // ORRXri, it is as cheap as MOV
   case AArch64::MOVi32imm:
     return canBeExpandedToORR(MI, 32);
   case AArch64::MOVi64imm:
bool AArch64InstrInfo::isCandidateToMergeOrPair(MachineInstr *MI) const {
   if (MI->modifiesRegister(BaseReg, TRI))
     return false;
 
   // Check if this load/store has a hint to avoid pair formation.
   // MachineMemOperands hints are set by the AArch64StorePairSuppress pass.
   if (isLdStPairSuppressed(MI))
     return false;
 
-  // Do not pair quad ld/st for Exynos.
-  if (Subtarget.isExynosM1()) {
+  // On some CPUs quad load/store pairs are slower than two single load/stores.
+  if (Subtarget.avoidQuadLdStPairs()) {
     switch (MI->getOpcode()) {
     default:
       break;
 
     case AArch64::LDURQi:
     case AArch64::STURQi:
     case AArch64::LDRQui:
     case AArch64::STRQui:
bool AArch64InstrInfo::shouldClusterMemOps(MachineInstr *FirstLdSt,
 
   // The caller should already have ordered First/SecondLdSt by offset.
   assert(Offset1 <= Offset2 && "Caller should have ordered offsets.");
   return Offset1 + 1 == Offset2;
 }
 
 bool AArch64InstrInfo::shouldScheduleAdjacent(MachineInstr *First,
                                               MachineInstr *Second) const {
-  if (Subtarget.isCyclone()) {
-    // Cyclone can fuse CMN, CMP, TST followed by Bcc.
+  if (Subtarget.hasMacroOpFusion()) {
+    // Fuse CMN, CMP, TST followed by Bcc.
     unsigned SecondOpcode = Second->getOpcode();
     if (SecondOpcode == AArch64::Bcc) {
       switch (First->getOpcode()) {
       default:
         return false;
       case AArch64::SUBSWri:
       case AArch64::ADDSWri:
       case AArch64::ANDSWri:
       case AArch64::SUBSXri:
       case AArch64::ADDSXri:
       case AArch64::ANDSXri:
         return true;
       }
     }
-    // Cyclone B0 also supports ALU operations followed by CBZ/CBNZ.
+    // Fuse ALU operations followed by CBZ/CBNZ.
     if (SecondOpcode == AArch64::CBNZW || SecondOpcode == AArch64::CBNZX ||
         SecondOpcode == AArch64::CBZW || SecondOpcode == AArch64::CBZX) {
       switch (First->getOpcode()) {
       default:
         return false;
       case AArch64::ADDWri:
       case AArch64::ADDXri:
       case AArch64::ANDWri:

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td

 def HasPerfMon       : Predicate<"Subtarget->hasPerfMon()">;
 def HasFullFP16      : Predicate<"Subtarget->hasFullFP16()">,
                                  AssemblerPredicate<"FeatureFullFP16", "fullfp16">;
 def HasSPE           : Predicate<"Subtarget->hasSPE()">,
                                  AssemblerPredicate<"FeatureSPE", "spe">;
 
 def IsLE             : Predicate<"Subtarget->isLittleEndian()">;
 def IsBE             : Predicate<"!Subtarget->isLittleEndian()">;
-def IsCyclone        : Predicate<"Subtarget->isCyclone()">;
+def UseAlternateSExtLoadCVTF32
+    : Predicate<"Subtarget->useAlternateSExtLoadCVTF32Pattern()">;
 
 //===----------------------------------------------------------------------===//
 // AArch64-specific DAG Nodes.
 //
 
 // SDTBinaryArithWithFlagsOut - RES1, FLAGS = op LHS, RHS
 def SDTBinaryArithWithFlagsOut : SDTypeProfile<2, 2,
                                               [SDTCisSameAs<0, 2>,
: Pat<(f32 (sint_to_fp (i32 (sextloadi8 addrmode)))),
                                 (EXTRACT_SUBREG
                                   (SSHLLv8i8_shift
                                     (INSERT_SUBREG (f64 (IMPLICIT_DEF)),
                                         INST,
                                         bsub),
                                     0),
                                   dsub)),
                                0),
-                             ssub)))>, Requires<[NotForCodeSize, IsCyclone]>;
+                             ssub)))>,
+    Requires<[NotForCodeSize, UseAlternateSExtLoadCVTF32]>;
 
 def : SExtLoadi8CVTf32Pat<(ro8.Wpat GPR64sp:$Rn, GPR32:$Rm, ro8.Wext:$ext),
                           (LDRBroW  GPR64sp:$Rn, GPR32:$Rm, ro8.Wext:$ext)>;
 def : SExtLoadi8CVTf32Pat<(ro8.Xpat GPR64sp:$Rn, GPR64:$Rm, ro8.Xext:$ext),
                           (LDRBroX  GPR64sp:$Rn, GPR64:$Rm, ro8.Xext:$ext)>;
 def : SExtLoadi8CVTf32Pat<(am_indexed8 GPR64sp:$Rn, uimm12s1:$offset),
                           (LDRBui GPR64sp:$Rn, uimm12s1:$offset)>;
 def : SExtLoadi8CVTf32Pat<(am_unscaled8 GPR64sp:$Rn, simm9:$offset),
: Pat <(f64 (sint_to_fp (i32 (sextloadi16 addrmode)))),
                                   (EXTRACT_SUBREG
                                     (SSHLLv4i16_shift
                                       (INSERT_SUBREG (f64 (IMPLICIT_DEF)),
                                         INST,
                                         hsub),
                                      0),
                                    dsub)),
                                0),
-                             dsub)))>, Requires<[NotForCodeSize, IsCyclone]>;
+                             dsub)))>,
+    Requires<[NotForCodeSize, UseAlternateSExtLoadCVTF32]>;
  
 def : SExtLoadi16CVTf64Pat<(ro16.Wpat GPR64sp:$Rn, GPR32:$Rm, ro16.Wext:$ext),
                            (LDRHroW GPR64sp:$Rn, GPR32:$Rm, ro16.Wext:$ext)>;
 def : SExtLoadi16CVTf64Pat<(ro16.Xpat GPR64sp:$Rn, GPR64:$Rm, ro16.Xext:$ext),
                            (LDRHroX GPR64sp:$Rn, GPR64:$Rm, ro16.Xext:$ext)>;
 def : SExtLoadi16CVTf64Pat<(am_indexed16 GPR64sp:$Rn, uimm12s2:$offset),
                            (LDRHui GPR64sp:$Rn, uimm12s2:$offset)>;
 def : SExtLoadi16CVTf64Pat<(am_unscaled16 GPR64sp:$Rn, simm9:$offset),

llvm/trunk/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

struct AArch64LoadStoreOpt : public MachineFunctionPass {
   bool tryToMergeLdStInst(MachineBasicBlock::iterator &MBBI);
 
   // Find and pair ldr/str instructions.
   bool tryToPairLdStInst(MachineBasicBlock::iterator &MBBI);
 
   // Find and promote load instructions which read directly from store.
   bool tryToPromoteLoadFromStore(MachineBasicBlock::iterator &MBBI);
 
-  // Check if converting two narrow loads into a single wider load with
-  // bitfield extracts could be enabled.
-  bool enableNarrowLdMerge(MachineFunction &Fn);
-
   bool optimizeBlock(MachineBasicBlock &MBB, bool enableNarrowLdOpt);
 
   bool runOnMachineFunction(MachineFunction &Fn) override;
 
   MachineFunctionProperties getRequiredProperties() const override {
     return MachineFunctionProperties().set(
         MachineFunctionProperties::Property::AllVRegsAllocated);
   }
case AArch64::STPXi: {
       break;
     }
     }
   }
 
   return Modified;
 }
 
-bool AArch64LoadStoreOpt::enableNarrowLdMerge(MachineFunction &Fn) {
-  bool ProfitableArch = Subtarget->isCortexA57() || Subtarget->isKryo();
-  // FIXME: The benefit from converting narrow loads into a wider load could be
-  // microarchitectural as it assumes that a single load with two bitfield
-  // extracts is cheaper than two narrow loads. Currently, this conversion is
-  // enabled only in cortex-a57 on which performance benefits were verified.
-  return ProfitableArch && !Subtarget->requiresStrictAlign();
-}
-
 bool AArch64LoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
   if (skipFunction(*Fn.getFunction()))
     return false;
 
   Subtarget = &static_cast<const AArch64Subtarget &>(Fn.getSubtarget());
   TII = static_cast<const AArch64InstrInfo *>(Subtarget->getInstrInfo());
   TRI = Subtarget->getRegisterInfo();
 
   // Resize the modified and used register bitfield trackers.  We do this once
   // per function and then clear the bitfield each time we optimize a load or
   // store.
   ModifiedRegs.resize(TRI->getNumRegs());
   UsedRegs.resize(TRI->getNumRegs());
 
   bool Modified = false;
-  bool enableNarrowLdOpt = enableNarrowLdMerge(Fn);
+  bool enableNarrowLdOpt =
+    Subtarget->mergeNarrowLoads() && !Subtarget->requiresStrictAlign();
   for (auto &MBB : Fn)
     Modified |= optimizeBlock(MBB, enableNarrowLdOpt);
 
   return Modified;
 }
 
 // FIXME: Do we need/want a pre-alloc pass like ARM has to try to keep
 // loads and stores near one another?

llvm/trunk/lib/Target/AArch64/AArch64Subtarget.h

 #include "AArch64GenSubtargetInfo.inc"
 
 namespace llvm {
 class GlobalValue;
 class StringRef;
 class Triple;
 
 class AArch64Subtarget : public AArch64GenSubtargetInfo {
-protected:
-  enum ARMProcFamilyEnum {
+public:
+  enum ARMProcFamilyEnum : uint8_t {
     Others,
     CortexA35,
     CortexA53,
     CortexA57,
     Cyclone,
     ExynosM1,
     Kryo
   };
 
+protected:
   /// ARMProcFamily - ARM processor family: Cortex-A53, Cortex-A57, and others.
   ARMProcFamilyEnum ARMProcFamily = Others;
 
   bool HasV8_1aOps = false;
   bool HasV8_2aOps = false;
 
   bool HasFPARMv8 = false;
   bool HasNEON = false;
   bool HasCrypto = false;
   bool HasCRC = false;
   bool HasPerfMon = false;
   bool HasFullFP16 = false;
   bool HasSPE = false;
 
   // HasZeroCycleRegMove - Has zero-cycle register mov instructions.
   bool HasZeroCycleRegMove = false;
 
   // HasZeroCycleZeroing - Has zero-cycle zeroing instructions.
   bool HasZeroCycleZeroing = false;
 
   // StrictAlign - Disallow unaligned memory accesses.
   bool StrictAlign = false;
+  bool MergeNarrowLoads = false;
+  bool UseAA = false;
+  bool PredictableSelectIsExpensive = false;
+  bool BalanceFPOps = false;
+  bool CustomAsCheapAsMove = false;
+  bool UsePostRAScheduler = false;
+  bool Misaligned128StoreIsSlow = false;
+  bool AvoidQuadLdStPairs = false;
+  bool UseAlternateSExtLoadCVTF32Pattern = false;
+  bool HasMacroOpFusion = false;
+  bool DisableLatencySchedHeuristic = false;
+  bool UseRSqrt = false;
+  uint8_t MaxInterleaveFactor = 2;
+  uint8_t VectorInsertExtractBaseCost = 3;
+  uint16_t CacheLineSize = 0;
+  uint16_t PrefetchDistance = 0;
+  uint16_t MinPrefetchStride = 1;
+  unsigned MaxPrefetchIterationsAhead = UINT_MAX;
 
   // ReserveX18 - X18 is not available as a general purpose register.
   bool ReserveX18;
 
   bool IsLittle;
 
   /// CPUString - String name of used CPU.
   std::string CPUString;
protected:
   std::unique_ptr<GISelAccessor> GISel;
 
 private:
   /// initializeSubtargetDependencies - Initializes using CPUString and the
   /// passed in feature string so that we can use initializer lists for
   /// subtarget initialization.
   AArch64Subtarget &initializeSubtargetDependencies(StringRef FS);
 
+  /// Initialize properties based on the selected processor family.
+  void initializeProperties();
+
 public:
   /// This constructor initializes the data members to match that
   /// of the specified triple.
   AArch64Subtarget(const Triple &TT, const std::string &CPU,
                    const std::string &FS, const TargetMachine &TM,
                    bool LittleEndian);
 
   /// This object will take onwership of \p GISelAccessor.
public:
   const AArch64RegisterInfo *getRegisterInfo() const override {
     return &getInstrInfo()->getRegisterInfo();
   }
   const CallLowering *getCallLowering() const override;
   const RegisterBankInfo *getRegBankInfo() const override;
   const Triple &getTargetTriple() const { return TargetTriple; }
   bool enableMachineScheduler() const override { return true; }
   bool enablePostRAScheduler() const override {
-    return isGeneric() || isCortexA53() || isCortexA57() || isKryo();
+    return UsePostRAScheduler;
+  }
+
+  /// Returns ARM processor family.
+  /// Avoid this function! CPU specifics should be kept local to this class
+  /// and preferably modeled with SubtargetFeatures or properties in
+  /// initializeProperties().
+  ARMProcFamilyEnum getProcFamily() const {
+    return ARMProcFamily;
   }
 
   bool hasV8_1aOps() const { return HasV8_1aOps; }
   bool hasV8_2aOps() const { return HasV8_2aOps; }
 
   bool hasZeroCycleRegMove() const { return HasZeroCycleRegMove; }
 
   bool hasZeroCycleZeroing() const { return HasZeroCycleZeroing; }
 
   bool requiresStrictAlign() const { return StrictAlign; }
 
   bool isX18Reserved() const { return ReserveX18; }
   bool hasFPARMv8() const { return HasFPARMv8; }
   bool hasNEON() const { return HasNEON; }
   bool hasCrypto() const { return HasCrypto; }
   bool hasCRC() const { return HasCRC; }
+  bool mergeNarrowLoads() const { return MergeNarrowLoads; }
+  bool balanceFPOps() const { return BalanceFPOps; }
+  bool predictableSelectIsExpensive() const {
+    return PredictableSelectIsExpensive;
+  }
+  bool hasCustomCheapAsMoveHandling() const { return CustomAsCheapAsMove; }
+  bool isMisaligned128StoreSlow() const { return Misaligned128StoreIsSlow; }
+  bool avoidQuadLdStPairs() const { return AvoidQuadLdStPairs; }
+  bool useAlternateSExtLoadCVTF32Pattern() const {
+    return UseAlternateSExtLoadCVTF32Pattern;
+  }
+  bool hasMacroOpFusion() const { return HasMacroOpFusion; }
+  bool useRSqrt() const { return UseRSqrt; }
+  unsigned getMaxInterleaveFactor() const { return MaxInterleaveFactor; }
+  unsigned getVectorInsertExtractBaseCost() const {
+    return VectorInsertExtractBaseCost;
+  }
+  unsigned getCacheLineSize() const { return CacheLineSize; }
+  unsigned getPrefetchDistance() const { return PrefetchDistance; }
+  unsigned getMinPrefetchStride() const { return MinPrefetchStride; }
+  unsigned getMaxPrefetchIterationsAhead() const {
+    return MaxPrefetchIterationsAhead;
+  }
+
   /// CPU has TBI (top byte of addresses is ignored during HW address
   /// translation) and OS enables it.
   bool supportsAddressTopByteIgnored() const;
 
   bool hasPerfMon() const { return HasPerfMon; }
   bool hasFullFP16() const { return HasFullFP16; }
   bool hasSPE() const { return HasSPE; }
 
   bool isLittleEndian() const { return IsLittle; }
 
   bool isTargetDarwin() const { return TargetTriple.isOSDarwin(); }
   bool isTargetIOS() const { return TargetTriple.isiOS(); }
   bool isTargetLinux() const { return TargetTriple.isOSLinux(); }
   bool isTargetWindows() const { return TargetTriple.isOSWindows(); }
   bool isTargetAndroid() const { return TargetTriple.isAndroid(); }
 
   bool isTargetCOFF() const { return TargetTriple.isOSBinFormatCOFF(); }
   bool isTargetELF() const { return TargetTriple.isOSBinFormatELF(); }
   bool isTargetMachO() const { return TargetTriple.isOSBinFormatMachO(); }
 
-  bool isGeneric() const { return CPUString == "generic"; }
-  bool isCyclone() const { return CPUString == "cyclone"; }
-  bool isCortexA57() const { return CPUString == "cortex-a57"; }
-  bool isCortexA53() const { return CPUString == "cortex-a53"; }
-  bool isExynosM1() const { return CPUString == "exynos-m1"; }
-  bool isKryo() const { return CPUString == "kryo"; }
-
-  bool useAA() const override { return isCortexA53(); }
+  bool useAA() const override { return UseAA; }
 
   /// getMaxInlineSizeThreshold - Returns the maximum memset / memcpy size
   /// that still makes it profitable to inline the call.
   unsigned getMaxInlineSizeThreshold() const { return 64; }
 
   /// ParseSubtargetFeatures - Parses features string setting specified
   /// subtarget options.  Definition of function is auto generated by tblgen.
   void ParseSubtargetFeatures(StringRef CPU, StringRef FS);

llvm/trunk/lib/Target/AArch64/AArch64Subtarget.cpp

 AArch64Subtarget &
 AArch64Subtarget::initializeSubtargetDependencies(StringRef FS) {
   // Determine default and user-specified characteristics
 
   if (CPUString.empty())
     CPUString = "generic";
 
   ParseSubtargetFeatures(CPUString, FS);
+  initializeProperties();
+
   return *this;
 }
 
+void AArch64Subtarget::initializeProperties() {
+  // Initialize CPU specific properties. We should add a tablegen feature for
+  // this in the future so we can specify it together with the subtarget
+  // features.
+  switch (ARMProcFamily) {
+  case Cyclone:
+    CacheLineSize = 64;
+    PrefetchDistance = 280;
+    MinPrefetchStride = 2048;
+    MaxPrefetchIterationsAhead = 3;
+    break;
+  case CortexA57:
+    MaxInterleaveFactor = 4;
+    break;
+  case Kryo:
+    MaxInterleaveFactor = 4;
+    VectorInsertExtractBaseCost = 2;
+    break;
+  case Others: break;
+  case CortexA35: break;
+  case CortexA53: break;
+  case ExynosM1: break;
+  }
+}
+
 AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
                                    const std::string &FS,
                                    const TargetMachine &TM, bool LittleEndian)
     : AArch64GenSubtargetInfo(TT, CPU, FS), ReserveX18(TT.isOSDarwin()),
       IsLittle(LittleEndian), CPUString(CPU), TargetTriple(TT), FrameLowering(),
       InstrInfo(initializeSubtargetDependencies(FS)), TSInfo(),
       TLInfo(TM, *this), GISel() {}
 
void AArch64Subtarget::overrideSchedPolicy(MachineSchedPolicy &Policy,
                                          unsigned NumRegionInstrs) const {
   // LNT run (at least on Cyclone) showed reasonably significant gains for
   // bi-directional scheduling. 253.perlbmk.
   Policy.OnlyTopDown = false;
   Policy.OnlyBottomUp = false;
   // Enabling or Disabling the latency heuristic is a close call: It seems to
   // help nearly no benchmark on out-of-order architectures, on the other hand
   // it regresses register pressure on a few benchmarking.
-  if (isCyclone())
-    Policy.DisableLatencyHeuristic = true;
+  Policy.DisableLatencyHeuristic = DisableLatencySchedHeuristic;
 }
 
 bool AArch64Subtarget::enableEarlyIfConversion() const {
   return EnableEarlyIfConvert;
 }
 
 bool AArch64Subtarget::supportsAddressTopByteIgnored() const {
   if (!UseAddressTopByteIgnored)
     return false;
 
   if (TargetTriple.isiOS()) {
     unsigned Major, Minor, Micro;
     TargetTriple.getiOSVersion(Major, Minor, Micro);
     return Major >= 8;
   }
 
   return false;
 }
 
 std::unique_ptr<PBQPRAConstraint>
 AArch64Subtarget::getCustomPBQPConstraints() const {
-  if (!isCortexA57())
-    return nullptr;
-
-  return llvm::make_unique<A57ChainingConstraint>();
+  return balanceFPOps() ? llvm::make_unique<A57ChainingConstraint>() : nullptr;
 }

llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp

 {
   // For the estimates, convergence is quadratic, so essentially the number of
   // digits is doubled after each iteration. ARMv8, the minimum architected
   // accuracy of the initial estimate is 2^-8.  Therefore, the number of extra
   // steps to refine the result for float (23 mantissa bits) and for double
   // (52 mantissa bits) are 2 and 3, respectively.
   unsigned ExtraStepsF = 2,
            ExtraStepsD = ExtraStepsF + 1;
-  // FIXME: Enable x^-1/2 only for Exynos M1 at the moment.
-  bool UseRsqrt = ST.isExynosM1();
+  bool UseRsqrt = ST.useRSqrt();
 
   TM.Options.Reciprocals.setDefaults("sqrtf", UseRsqrt, ExtraStepsF);
   TM.Options.Reciprocals.setDefaults("sqrtd", UseRsqrt, ExtraStepsD);
   TM.Options.Reciprocals.setDefaults("vec-sqrtf", UseRsqrt, ExtraStepsF);
   TM.Options.Reciprocals.setDefaults("vec-sqrtd", UseRsqrt, ExtraStepsD);
 
   TM.Options.Reciprocals.setDefaults("divf", false, ExtraStepsF);
   TM.Options.Reciprocals.setDefaults("divd", false, ExtraStepsD);

llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

if (Index != -1U) {
     Index = Index % Width;
 
     // The element at index zero is already inside the vector.
     if (Index == 0)
       return 0;
   }
 
   // All other insert/extracts cost this much.
-  if (ST->isKryo())
-    return 2;
-  return 3;
+  return ST->getVectorInsertExtractBaseCost();
 }
 
 int AArch64TTIImpl::getArithmeticInstrCost(
     unsigned Opcode, Type *Ty, TTI::OperandValueKind Opd1Info,
     TTI::OperandValueKind Opd2Info, TTI::OperandValueProperties Opd1PropInfo,
     TTI::OperandValueProperties Opd2PropInfo) {
   // Legalize the type.
   std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, Ty);
for (auto *I : Tys) {
     if (I->getScalarSizeInBits() * I->getVectorNumElements() == 128)
       Cost += getMemoryOpCost(Instruction::Store, I, 128, 0) +
         getMemoryOpCost(Instruction::Load, I, 128, 0);
   }
   return Cost;
 }
 
 unsigned AArch64TTIImpl::getMaxInterleaveFactor(unsigned VF) {
-  if (ST->isCortexA57() || ST->isKryo())
-    return 4;
-  return 2;
+  return ST->getMaxInterleaveFactor();
 }
 
 void AArch64TTIImpl::getUnrollingPreferences(Loop *L,
                                              TTI::UnrollingPreferences &UP) {
   // Enable partial unrolling and runtime unrolling.
   BaseT::getUnrollingPreferences(L, UP);
 
   // For inner loop, it is more likely to be a hot one, and the runtime check
bool AArch64TTIImpl::getTgtMemIntrinsic(IntrinsicInst *Inst,
   case Intrinsic::aarch64_neon_st4:
     Info.MatchingId = VECTOR_LDST_FOUR_ELEMENTS;
     break;
   }
   return true;
 }
 
 unsigned AArch64TTIImpl::getCacheLineSize() {
-  if (ST->isCyclone())
-    return 64;
-  return BaseT::getCacheLineSize();
+  return ST->getCacheLineSize();
 }
 
 unsigned AArch64TTIImpl::getPrefetchDistance() {
-  if (ST->isCyclone())
-    return 280;
-  return BaseT::getPrefetchDistance();
+  return ST->getPrefetchDistance();
 }
 
 unsigned AArch64TTIImpl::getMinPrefetchStride() {
-  if (ST->isCyclone())
-    // The HW prefetcher handles accesses with strides up to 2KB.
-    return 2048;
-  return BaseT::getMinPrefetchStride();
+  return ST->getMinPrefetchStride();
 }
 
 unsigned AArch64TTIImpl::getMaxPrefetchIterationsAhead() {
-  if (ST->isCyclone())
-    // Be conservative for now and don't prefetch ahead too much since the loop
-    // may terminate early.
-    return 3;
-  return BaseT::getMaxPrefetchIterationsAhead();
+  return ST->getMaxPrefetchIterationsAhead();
 }