[ARM] Do not test for CPUs, use SubtargetFeatures. Also remove 2 flags.

[rovka]

Summary

This is a follow-up for r273544.

The end goal is to get rid of the isSwift / isCortexXY / isWhatever methods.

This commit also removes two command-line flags that weren't used in any of the
tests: widen-vmovs and swift-partial-update-clearance. The former may be easily
replaced with the mattr mechanism, but the latter may not (as it is a subtarget
property, and not a proper feature). If anyone needs to control that from the
command line, please speak now (and help me add a test).

[rovka]

Ping?

[rengolin]

I personally prefer "widenVMOVS" than "dontWidenVMOVS", but otherwise, LGTM. Thanks!

[rovka]

In D21797#474927, @rengolin wrote:

I personally prefer "widenVMOVS" than "dontWidenVMOVS", but otherwise, LGTM. Thanks!

Indeed, but widening seems to be the default across targets, so it would be unpleasant to add it everywhere (including for every new target). If we want to keep it on a positive tone, we could name it "KeepVMOVS" or "PreferVMOVS" (we already have a "PreferVMOVSR", I'm not sure if that makes things better or worse).

[rengolin]

In D21797#474938, @rovka wrote:

Indeed, but widening seems to be the default across targets, so it would be unpleasant to add it everywhere (including for every new target).

Good point. LGTM as it is.

lib/Target/ARM/ARM.td

 def FeaturePreferVMOVSR : SubtargetFeature<"prefer-vmovsr", "PreferVMOVSR",
                                            "true", "Prefer VMOVSR">;
 
 // Swift has ISHST barriers compatible with Atomic Release semantics but weaker
 // than ISH
 def FeaturePrefISHSTBarrier : SubtargetFeature<"prefer-ishst", "PreferISHST",
                                            "true", "Prefer ISHST barriers">;
 
+// Some targets (e.g. Cortex-A15) never want VMOVS to be widened to VMOVD.
+def FeatureDontWidenVMOVS : SubtargetFeature<"dont-widen-vmovs",
+                                             "DontWidenVMOVS", "true",
+                                             "Don't widen VMOVS to VMOVD">;
+
 // Some targets (e.g. Cortex-A9) want to convert VMOVRS, VMOVSR and VMOVS from
 // VFP to NEON, as an execution domain optimization.
 def FeatureNEONForFPMovs : SubtargetFeature<"neon-fpmovs", "UseNEONForFPMovs",
                               "true", "Convert VMOVSR, VMOVRS, VMOVS to NEON">;
 
 // Some processors benefit from using NEON instructions for scalar
 // single-precision FP operations. This affects instruction selection and should
 // only be enabled if the handling of denormals is not important.
def : ProcessorModel<"cortex-a12",  CortexA9Model,      [ARMv7a, ProcA12,
                                                          FeatureHWDiv,
                                                          FeatureHWDivARM,
                                                          FeatureAvoidPartialCPSR,
                                                          FeatureVirtualization,
                                                          FeatureMP]>;
 
 // FIXME: A15 has currently the same Schedule model as A9.
 def : ProcessorModel<"cortex-a15",  CortexA9Model,      [ARMv7a, ProcA15,
+                                                         FeatureDontWidenVMOVS,
                                                          FeatureHasRetAddrStack,
                                                          FeatureTrustZone,
                                                          FeatureT2XtPk,
                                                          FeatureVFP4,
                                                          FeatureMP,
                                                          FeatureCheckVLDnAlign,
                                                          FeatureHWDiv,
                                                          FeatureHWDivARM,

lib/Target/ARM/ARMBaseInstrInfo.cpp

 
 #define GET_INSTRINFO_CTOR_DTOR
 #include "ARMGenInstrInfo.inc"
 
 static cl::opt<bool>
 EnableARM3Addr("enable-arm-3-addr-conv", cl::Hidden,
                cl::desc("Enable ARM 2-addr to 3-addr conv"));
 
-static cl::opt<bool>
-WidenVMOVS("widen-vmovs", cl::Hidden, cl::init(true),
-           cl::desc("Widen ARM vmovs to vmovd when possible"));
-
-static cl::opt<unsigned>
-SwiftPartialUpdateClearance("swift-partial-update-clearance",
-     cl::Hidden, cl::init(12),
-     cl::desc("Clearance before partial register updates"));
-
 /// ARM_MLxEntry - Record information about MLA / MLS instructions.
 struct ARM_MLxEntry {
   uint16_t MLxOpc;     // MLA / MLS opcode
   uint16_t MulOpc;     // Expanded multiplication opcode
   uint16_t AddSubOpc;  // Expanded add / sub opcode
   bool NegAcc;         // True if the acc is negated before the add / sub.
   bool HasLane;        // True if instruction has an extra "lane" operand.
 };

     expandMEMCPY(MI);
     return true;
   }
 
   // This hook gets to expand COPY instructions before they become
   // copyPhysReg() calls.  Look for VMOVS instructions that can legally be
   // widened to VMOVD.  We prefer the VMOVD when possible because it may be
   // changed into a VORR that can go down the NEON pipeline.
-  if (!WidenVMOVS || !MI->isCopy() || Subtarget.isCortexA15() ||
-      Subtarget.isFPOnlySP())
+  if (!MI->isCopy() || Subtarget.dontWidenVMOVS() || Subtarget.isFPOnlySP())
     return false;
 
   // Look for a copy between even S-registers.  That is where we keep floats
   // when using NEON v2f32 instructions for f32 arithmetic.
   unsigned DstRegS = MI->getOperand(0).getReg();
   unsigned SrcRegS = MI->getOperand(1).getReg();
   if (!ARM::SPRRegClass.contains(DstRegS, SrcRegS))
     return false;

 // VLD1LNd32 - Writes all D-regs, explicit partial D update, 2 uops.
 // VLD1DUPd32 - Writes all D-regs, no partial reg update, 2 uops.
 //
 // FCONSTD can be used as a dependency-breaking instruction.
 unsigned ARMBaseInstrInfo::
 getPartialRegUpdateClearance(const MachineInstr *MI,
                              unsigned OpNum,
                              const TargetRegisterInfo *TRI) const {
-  if (!SwiftPartialUpdateClearance ||
-      !(Subtarget.isSwift() || Subtarget.isCortexA15()))
+  auto PartialUpdateClearance = Subtarget.getPartialUpdateClearance();
+  if (!PartialUpdateClearance)
     return 0;
 
   assert(TRI && "Need TRI instance");
 
   const MachineOperand &MO = MI->getOperand(OpNum);
   if (MO.readsReg())
     return 0;
   unsigned Reg = MO.getReg();

     unsigned DReg = TRI->getMatchingSuperReg(Reg, ARM::ssub_0,
                                              &ARM::DPRRegClass);
     if (!DReg || !MI->definesRegister(DReg, TRI))
       return 0;
   }
 
   // MI has an unwanted D-register dependency.
   // Avoid defs in the previous N instructrions.
-  return SwiftPartialUpdateClearance;
+  return PartialUpdateClearance;
 }
 
 // Break a partial register dependency after getPartialRegUpdateClearance
 // returned non-zero.
 void ARMBaseInstrInfo::
 breakPartialRegDependency(MachineBasicBlock::iterator MI,
                           unsigned OpNum,
                           const TargetRegisterInfo *TRI) const {

lib/Target/ARM/ARMSubtarget.h

protected:
   bool HasSlowVDUP32 = false;
 
   /// If true, VMOVSR will be favored over VMOVDRR.
   bool PreferVMOVSR = false;
 
   /// If true, ISHST barriers will be used for Release semantics.
   bool PreferISHST = false;
 
+  /// If true, VMOVS will never be widened to VMOVD (regardless of the
+  /// widen-vmovs flag).
+  bool DontWidenVMOVS = false;
+
   /// If true, VMOVRS, VMOVSR and VMOVS will be converted from VFP to NEON.
   bool UseNEONForFPMovs = false;
 
   /// If true, VLDn instructions take an extra cycle for unaligned accesses.
   bool CheckVLDnAlign = false;
 
   /// If true, VFP instructions are not pipelined.
   bool NonpipelinedVFP = false;
protected:
   /// entry to the function and which must be maintained by every function.
   unsigned stackAlignment = 4;
 
   /// CPUString - String name of used CPU.
   std::string CPUString;
 
   unsigned MaxInterleaveFactor = 1;
 
+  /// Clearance before partial register updates (in number of instructions)
+  unsigned PartialUpdateClearance = 0;
+
   /// What kind of timing do load multiple/store multiple have (double issue,
   /// single issue etc).
   ARMLdStMultipleTiming LdStMultipleTiming = SingleIssue;
 
   /// The adjustment that we need to apply to get the operand latency from the
   /// operand cycle returned by the itinerary data for pre-ISel operands.
   int PreISelOperandLatencyAdjustment = 2;
 
public:
   bool hasTrustZone() const { return HasTrustZone; }
   bool has8MSecExt() const { return Has8MSecExt; }
   bool hasZeroCycleZeroing() const { return HasZeroCycleZeroing; }
   bool isProfitableToUnpredicate() const { return IsProfitableToUnpredicate; }
   bool hasSlowVGETLNi32() const { return HasSlowVGETLNi32; }
   bool hasSlowVDUP32() const { return HasSlowVDUP32; }
   bool preferVMOVSR() const { return PreferVMOVSR; }
   bool preferISHSTBarriers() const { return PreferISHST; }
+  bool dontWidenVMOVS() const { return DontWidenVMOVS; }
   bool useNEONForFPMovs() const { return UseNEONForFPMovs; }
   bool checkVLDnAccessAlignment() const { return CheckVLDnAlign; }
   bool nonpipelinedVFP() const { return NonpipelinedVFP; }
   bool prefers32BitThumb() const { return Pref32BitThumb; }
   bool avoidCPSRPartialUpdate() const { return AvoidCPSRPartialUpdate; }
   bool avoidMOVsShifterOperand() const { return AvoidMOVsShifterOperand; }
   bool hasRetAddrStack() const { return HasRetAddrStack; }
   bool hasMPExtension() const { return HasMPExtension; }
public:
 
   /// getStackAlignment - Returns the minimum alignment known to hold of the
   /// stack frame on entry to the function and which must be maintained by every
   /// function for this subtarget.
   unsigned getStackAlignment() const { return stackAlignment; }
 
   unsigned getMaxInterleaveFactor() const { return MaxInterleaveFactor; }
 
+  unsigned getPartialUpdateClearance() const { return PartialUpdateClearance; }
+
   ARMLdStMultipleTiming getLdStMultipleTiming() const {
     return LdStMultipleTiming;
   }
 
   int getPreISelOperandLatencyAdjustment() const {
     return PreISelOperandLatencyAdjustment;
   }
 

lib/Target/ARM/ARMSubtarget.cpp

case CortexA9:
     LdStMultipleTiming = DoubleIssueCheckUnalignedAccess;
     PreISelOperandLatencyAdjustment = 1;
     break;
   case CortexA12:
     break;
   case CortexA15:
     MaxInterleaveFactor = 2;
     PreISelOperandLatencyAdjustment = 1;
+    PartialUpdateClearance = 12;
     break;
   case CortexA17:
   case CortexA32:
   case CortexA35:
   case CortexA53:
   case CortexA57:
   case CortexA72:
   case CortexA73:
   case CortexR4:
   case CortexR4F:
   case CortexR5:
   case CortexR7:
   case CortexM3:
   case ExynosM1:
     break;
   case Krait:
     PreISelOperandLatencyAdjustment = 1;
     break;
   case Swift:
     MaxInterleaveFactor = 2;
     LdStMultipleTiming = SingleIssuePlusExtras;
     PreISelOperandLatencyAdjustment = 1;
+    PartialUpdateClearance = 12;
     break;
   }
 }
 
 bool ARMSubtarget::isAPCS_ABI() const {
   assert(TM.TargetABI != ARMBaseTargetMachine::ARM_ABI_UNKNOWN);
   return TM.TargetABI == ARMBaseTargetMachine::ARM_ABI_APCS;
 }

test/CodeGen/ARM/widen-vmovs.ll

-; RUN: llc < %s -widen-vmovs -mcpu=cortex-a8 -verify-machineinstrs -disable-block-placement | FileCheck %s
+; RUN: llc < %s -mcpu=cortex-a8 -verify-machineinstrs -disable-block-placement | FileCheck %s
 target triple = "thumbv7-apple-ios"
 
 ; The 1.0e+10 constant is loaded from the constant pool and kept in a register.
 ; CHECK: %entry
 ; CHECK: vldr s
 ; The float loop variable is initialized with a vmovs from the constant register.
 ; The vmovs is first widened to a vmovd, and then converted to a vorr because of the v2f32 vadd.f32.
 ; CHECK: vorr [[DL:d[0-9]+]], [[DN:d[0-9]+]]