[PowerPC] Update Vector Costs for P9

[RolandF]

Summary

For the power9 CPU, vector operations consume a pair of execution units rather than one execution unit like a scalar operation. Update the target transform cost functions to reflect the higher cost of vector operations when targeting power9.

[jsji]

I am confused by this patch.

In LoopVectorize.cpp, getInstructionCost returns the execution time cost of an instruction for a given vector.

So I think the cost model here should be related to latency.

Why we need to take into consideration of execution units for execution time cost ?
Considering execution units looks more like throughput cost model?

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

337

A isVectorTy here will not always be legal vector type, and maybe expanded or scalarized. In BaseT:: getArithmeticInstrCost we may calculate the cost depends on the legalization.

So I think we might have problem here if we simply check isVectorTy to increase the cost.

[RolandF]

In D55461#1326192, @jsji wrote:

I am confused by this patch.

In LoopVectorize.cpp, getInstructionCost returns the execution time cost of an instruction for a given vector.

So I think the cost model here should be related to latency.

Why we need to take into consideration of execution units for execution time cost ?
Considering execution units looks more like throughput cost model?

The calculation the loop vectorizer is making is comparing vector cost / vector factor to scalar cost. The calculation the SLP vectorizer is making is comparing vector cost to scalar cost * vector factor. In either case the vector factor is involved, and once the vector factor is involved it is really a throughput calculation and not a latency calculation. I don't think vectorization can make sense as a latency calculation - the scalar ops are always going to be at least as cheap. The win is in doing more at the same time, not in delivering a particular result faster.

[jsji]

In D55461#1334814, @RolandF wrote:

The calculation the loop vectorizer is making is comparing vector cost / vector factor to scalar cost. The calculation the SLP vectorizer is making is comparing vector cost to scalar cost * vector factor. In either case the vector factor is involved, and once the vector factor is involved it is really a throughput calculation and not a latency calculation. I don't think vectorization can make sense as a latency calculation - the scalar ops are always going to be at least as cheap. The win is in doing more at the same time, not in delivering a particular result faster.

Thanks for explaining. I am not familiar with SLP enough, maybe @hfinkel can help to see whether it is reasonable to consider execution unit num in vectorizer's cost model. Thanks!

[hfinkel]

In D55461#1335427, @jsji wrote:

In D55461#1334814, @RolandF wrote:

The calculation the loop vectorizer is making is comparing vector cost / vector factor to scalar cost. The calculation the SLP vectorizer is making is comparing vector cost to scalar cost * vector factor. In either case the vector factor is involved, and once the vector factor is involved it is really a throughput calculation and not a latency calculation. I don't think vectorization can make sense as a latency calculation - the scalar ops are always going to be at least as cheap. The win is in doing more at the same time, not in delivering a particular result faster.

Thanks for explaining. I am not familiar with SLP enough, maybe @hfinkel can help to see whether it is reasonable to consider execution unit num in vectorizer's cost model. Thanks!

The vectorization cost models do measure throughput. If a single operation consumes multiple execution units, that presumably is a throughput effect. We do have a latency model, but not much uses it yet (in theory, we'd use it during vectorization to determine interleaving factors, but we don't currently do that).

[hfinkel]

Also, cost model changes can be tested directly. See tests in test/Analysis/CostModel/PowerPC/

[nemanjai]

I think this code could be greatly simplified if we implement int PPCTTIImpl::vectorCostAdjustmentFactor(Type *Ty) which would return 1 by default and 2 if we are on a subtarget with vector operations that take up two execution units and Ty is a vector type.

Also, as Hal pointed out please add cost model tests for the affected costs.

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

331

I don't think we want to change this. For builds with asserts compiled out, this will emit "variable defined but not used" warnings.

343

It seems odd to me that we want to change the cost here since a shuffle won't have a scalar equivalent. So the fact that a vector operation uses two functional units shouldn't really matter. The vectorizer won't insert a shuffle on its own. It seems to me that the added cost of being in the vector domain should already be reflected by the cost of the other operations.

410

I think it would be clearer if this comment appeared before the condition.

[nemanjai]

Also, I've thought a bit about what Jinsong mentioned and I am wondering if maybe we don't want to take a different approach. Namely, the cost model is meant to return the cost of individual instructions with a given type and this cost is the instructions latency relative to the latency of a simple scalar ALU instruction (I think a comment somewhere uses add as the basis for what 1 means). And the vectorizers use these values to essentially add up the costs of all the instructions in a loop/block and then compute what provides the lowest total cost assuming that the cost of an N-wide vectorization can be computed by simply dividing the total cost by N.

So I would argue that it is that last assumption that isn't valid on the P9 CPU (and perhaps others out there as well). I'm wondering if it wouldn't be a better approach to define something like float TargetTransformInfo::vectorUnitThroughputReductionFactor() and using that in the cost computations. On P9, that would presumably be set to 2.0 since we can dispatch half as many vector instructions in each cycle as we can scalar ones.

Let me know what you think @jsji @hfinkel @mkuper @RolandF.

[RolandF]

I don't think adding a new TTI function is necessary, as I think the way I am modeling the costs here is what is expected. The following comment (from TargetTransformInfo.h: getArithmeticInstrCost()) may shed some light:

/// This is an approximation of reciprocal throughput of a math/logic op.
/// A higher cost indicates less expected throughput.
/// From Agner Fog's guides, reciprocal throughput is "the average number of
/// clock cycles per instruction when the instructions are not part of a
/// limiting dependency chain."
/// Therefore, costs should be scaled to account for multiple execution units
/// on the target that can process this type of instruction. For example, if
/// there are 5 scalar integer units and 2 vector integer units that can
/// calculate an 'add' in a single cycle, this model should indicate that the
/// cost of the vector add instruction is 2.5 times the cost of the scalar
/// add instruction.

[jsji]

Thanks Roland! This is great comment from Sanjay! Also aligned with Hal's comments.
So now, we should all agree on that the cost model here is regarding throughput.

However, according to Sanjay's example, we should account for multiple execution units that can process this type of instruction.

So to my understanding, the cost * 2 is because we have 4 slices for scalar arithmetic, while 2 superslices for vector arithmetic, so 4/2 = 2.

But for other operations like shuffle, memory op, looks like we may need to calculate them one by one, not just always use 2?

[RolandF]

For memory ops it should be the same as arithmetic. The LSUs are a separate resource from the slices, but a vector load or store still consumes multiple LSUs (2x if aligned, 3x if not). I don't follow why there should be a problem with shuffle - I assume a shuffle will require one or more vector ALU ops.

[RolandF]

Added a new TTI method vectorCostAdjustment to consolidate and make uniform for all instruction types the checks and cost modification. Also added direct cost model test.

[RolandF]

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

343

SLP does create shuffles, and it asks about the cost of shuffles. It's really a total cost vs total cost comparison, not instruction by instruction, so I think it's appropriate to factor in the execution unit cost of additions.

[nemanjai]

Thank you for the updates. I just have a couple of minor nits to add comments which can be done on the commit. LGTM.

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

333

// If type legalization involves splitting the vector, we don't want to
// double the cost at every step - only the last step.

437

Please remove the code that's commented out.

[nemanjai]

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

326

// Adjust the cost of vector instructions on targets on which there is overlap
// between the vector and scalar units, thereby reducing the overall throughput
// of vector code wrt. scalar code.

[RolandF]

Deleted commented out code and added suggested comments.

[nemanjai]

@RolandF Do you need me to commit this for you have commit access now?

[RolandF]

@nemanjai Yes, please, I do need it committed for me. Last time I promise!

llvm/lib/Target/PowerPC/PPC.td

def FeatureISA3_0 : SubtargetFeature<"isa-v30-instructions", "IsISA3_0",
                                      "Enable instructions added in ISA 3.0.">;
 def FeatureP9Altivec : SubtargetFeature<"power9-altivec", "HasP9Altivec", "true",
                                         "Enable POWER9 Altivec instructions",
                                         [FeatureISA3_0, FeatureP8Altivec]>;
 def FeatureP9Vector  : SubtargetFeature<"power9-vector", "HasP9Vector", "true",
                                         "Enable POWER9 vector instructions",
                                         [FeatureISA3_0, FeatureP8Vector,
                                          FeatureP9Altivec]>;
+// A separate feature for this even though it is equivalent to P9Vector
+// because this is a feature of the implementation rather than the architecture
+// and may go away with future CPU's.
+def FeatureVectorsUseTwoUnits : SubtargetFeature<"vectors-use-two-units",
+                                                 "VectorsUseTwoUnits",
+                                                 "true",
+                                                 "Vectors use two units">;
 
 // Since new processors generally contain a superset of features of those that
 // came before them, the idea is to make implementations of new processors
 // less error prone and easier to read.
 // Namely:
 //     list<SubtargetFeature> Power8FeatureList = ...
 //     list<SubtargetFeature> FutureProcessorSpecificFeatureList =
 //         [ features that Power8 does not support ]
list<SubtargetFeature> Power7FeatureList =
        FeatureMFTB, DeprecatedDST];
   list<SubtargetFeature> Power8SpecificFeatures =
       [DirectivePwr8, FeatureP8Altivec, FeatureP8Vector, FeatureP8Crypto,
        FeatureHTM, FeatureDirectMove, FeatureICBT, FeaturePartwordAtomic,
        FeatureFusion];
   list<SubtargetFeature> Power8FeatureList =
       !listconcat(Power7FeatureList, Power8SpecificFeatures);
   list<SubtargetFeature> Power9SpecificFeatures =
-      [DirectivePwr9, FeatureP9Altivec, FeatureP9Vector, FeatureISA3_0];
+      [DirectivePwr9, FeatureP9Altivec, FeatureP9Vector, FeatureISA3_0,
+       FeatureVectorsUseTwoUnits];
   list<SubtargetFeature> Power9FeatureList =
       !listconcat(Power8FeatureList, Power9SpecificFeatures);
 }
 
 // Note: Future features to add when support is extended to more
 // recent ISA levels:
 //
 // DFP          p6, p6x, p7        decimal floating-point instructions

llvm/lib/Target/PowerPC/PPCSubtarget.h

protected:
   bool HasPartwordAtomics;
   bool HasDirectMove;
   bool HasHTM;
   bool HasFusion;
   bool HasFloat128;
   bool IsISA3_0;
   bool UseLongCalls;
   bool SecurePlt;
+  bool VectorsUseTwoUnits;
 
   POPCNTDKind HasPOPCNTD;
 
   /// When targeting QPX running a stock PPC64 Linux kernel where the stack
   /// alignment has not been changed, we need to keep the 16-byte alignment
   /// of the stack.
   bool IsQPXStackUnaligned;
 
public:
   bool hasExtDiv() const { return HasExtDiv; }
   bool hasCMPB() const { return HasCMPB; }
   bool hasLDBRX() const { return HasLDBRX; }
   bool isBookE() const { return IsBookE; }
   bool hasOnlyMSYNC() const { return HasOnlyMSYNC; }
   bool isPPC4xx() const { return IsPPC4xx; }
   bool isPPC6xx() const { return IsPPC6xx; }
   bool isSecurePlt() const {return SecurePlt; }
+  bool vectorsUseTwoUnits() const {return VectorsUseTwoUnits; }
   bool isE500() const { return IsE500; }
   bool isFeatureMFTB() const { return FeatureMFTB; }
   bool isDeprecatedDST() const { return DeprecatedDST; }
   bool hasICBT() const { return HasICBT; }
   bool hasInvariantFunctionDescriptors() const {
     return HasInvariantFunctionDescriptors;
   }
   bool hasPartwordAtomics() const { return HasPartwordAtomics; }

llvm/lib/Target/PowerPC/PPCSubtarget.cpp

void PPCSubtarget::initializeEnvironment() {
   HasDirectMove = false;
   IsQPXStackUnaligned = false;
   HasHTM = false;
   HasFusion = false;
   HasFloat128 = false;
   IsISA3_0 = false;
   UseLongCalls = false;
   SecurePlt = false;
+  VectorsUseTwoUnits = false;
 
   HasPOPCNTD = POPCNTD_Unavailable;
 }
 
 void PPCSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
   // Determine default and user specified characteristics
   std::string CPUName = CPU;
   if (CPUName.empty() || CPU == "generic") {

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

unsigned PPCTTIImpl::getMaxInterleaveFactor(unsigned VF) {
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
       Directive == PPC::DIR_PWR9)
     return 12;
 
   // For most things, modern systems have two execution units (and
   // out-of-order execution).
   return 2;
 }
 
 int PPCTTIImpl::getArithmeticInstrCost(
     unsigned Opcode, Type *Ty, TTI::OperandValueKind Op1Info,
     TTI::OperandValueKind Op2Info, TTI::OperandValueProperties Opd1PropInfo,
     TTI::OperandValueProperties Opd2PropInfo, ArrayRef<const Value *> Args) {
-  assert(TLI->InstructionOpcodeToISD(Opcode) && "Invalid opcode");
+  int ISD = TLI->InstructionOpcodeToISD(Opcode);
+  assert(ISD && "Invalid opcode");
 
-  // Fallback to the default implementation.
-  return BaseT::getArithmeticInstrCost(Opcode, Ty, Op1Info, Op2Info,
+  int Cost = BaseT::getArithmeticInstrCost(Opcode, Ty, Op1Info, Op2Info,
                                            Opd1PropInfo, Opd2PropInfo);
+
+  if (Ty->isVectorTy() && ST->vectorsUseTwoUnits())
+    Cost *= 2;
+
+  return Cost;
 }
 
 int PPCTTIImpl::getShuffleCost(TTI::ShuffleKind Kind, Type *Tp, int Index,
                                Type *SubTp) {
   // Legalize the type.
   std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, Tp);
 
   // PPC, for both Altivec/VSX and QPX, support cheap arbitrary permutations
   // (at least in the sense that there need only be one non-loop-invariant
   // instruction). We need one such shuffle instruction for each actual
   // register (this is not true for arbitrary shuffles, but is true for the
   // structured types of shuffles covered by TTI::ShuffleKind).
-  return LT.first;
+  int Cost = LT.first;
+
+  if (ST->vectorsUseTwoUnits() && Tp->isVectorTy())
+    Cost *= 2;
+
+  return Cost;
 }
 
 int PPCTTIImpl::getCastInstrCost(unsigned Opcode, Type *Dst, Type *Src,
                                  const Instruction *I) {
-  assert(TLI->InstructionOpcodeToISD(Opcode) && "Invalid opcode");
+  int ISD = TLI->InstructionOpcodeToISD(Opcode);
+  assert(ISD && "Invalid opcode");
 
-  return BaseT::getCastInstrCost(Opcode, Dst, Src);
+  int Cost =  BaseT::getCastInstrCost(Opcode, Dst, Src);
+
+  if (Dst->isVectorTy() && ST->vectorsUseTwoUnits()) {
+    std::pair<int, MVT> SrcLT = TLI->getTypeLegalizationCost(DL, Src);
+    std::pair<int, MVT> DstLT = TLI->getTypeLegalizationCost(DL, Dst);
+
+    // The base class will call back if the vector is split.  We only want
+    // to double the cost once, so avoid split cases.
+    if (SrcLT.first == 1 && SrcLT.second.isVector() &&
+        DstLT.first == 1 && DstLT.second.isVector() &&
+        !TLI->isOperationExpand(ISD, DstLT.second))
+      Cost *= 2;
+  }
+
+  return Cost;
 }
 
 int PPCTTIImpl::getCmpSelInstrCost(unsigned Opcode, Type *ValTy, Type *CondTy,
                                    const Instruction *I) {
-  return BaseT::getCmpSelInstrCost(Opcode, ValTy, CondTy, I);
+
+  int ISD = TLI->InstructionOpcodeToISD(Opcode);
+  assert(ISD && "Invalid opcode");
+
+  int Cost = BaseT::getCmpSelInstrCost(Opcode, ValTy, CondTy, I);
+
+  if (ValTy->isVectorTy() && ST->vectorsUseTwoUnits())
+      Cost *= 2;
+
+  return Cost;
 }
 
 int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
   assert(Val->isVectorTy() && "This must be a vector type");
 
   int ISD = TLI->InstructionOpcodeToISD(Opcode);
   assert(ISD && "Invalid opcode");
 
+  int Cost = BaseT::getVectorInstrCost(Opcode, Val, Index);
+
+  if (Val->isVectorTy() && ST->vectorsUseTwoUnits())
+    Cost *= 2;
+
   if (ST->hasVSX() && Val->getScalarType()->isDoubleTy()) {
-    // Double-precision scalars are already located in index #0.
-    if (Index == 0)
+    if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
+      // Double-precision scalars are already located in index #0 (or #1 if LE).
       return 0;
 
-    return BaseT::getVectorInstrCost(Opcode, Val, Index);
+    return Cost;
+
   } else if (ST->hasQPX() && Val->getScalarType()->isFloatingPointTy()) {
     // Floating point scalars are already located in index #0.
     if (Index == 0)
       return 0;
 
-    return BaseT::getVectorInstrCost(Opcode, Val, Index);
+    return Cost;
   }
 
   // Estimated cost of a load-hit-store delay.  This was obtained
   // experimentally as a minimum needed to prevent unprofitable
   // vectorization for the paq8p benchmark.  It may need to be
   // raised further if other unprofitable cases remain.
   unsigned LHSPenalty = 2;
   if (ISD == ISD::INSERT_VECTOR_ELT)
     LHSPenalty += 7;
 
   // Vector element insert/extract with Altivec is very expensive,
   // because they require store and reload with the attendant
   // processor stall for load-hit-store.  Until VSX is available,
   // these need to be estimated as very costly.
   if (ISD == ISD::EXTRACT_VECTOR_ELT ||
       ISD == ISD::INSERT_VECTOR_ELT)
-    return LHSPenalty + BaseT::getVectorInstrCost(Opcode, Val, Index);
+    return LHSPenalty + Cost;
 
-  return BaseT::getVectorInstrCost(Opcode, Val, Index);
+  return Cost;
 }
 
 int PPCTTIImpl::getMemoryOpCost(unsigned Opcode, Type *Src, unsigned Alignment,
                                 unsigned AddressSpace, const Instruction *I) {
   // Legalize the type.
   std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, Src);
   assert((Opcode == Instruction::Load || Opcode == Instruction::Store) &&
          "Invalid Opcode");
 
   int Cost = BaseT::getMemoryOpCost(Opcode, Src, Alignment, AddressSpace);
 
+  if (Src->isVectorTy() && ST->vectorsUseTwoUnits())
+    Cost *= 2;
+
   bool IsAltivecType = ST->hasAltivec() &&
                        (LT.second == MVT::v16i8 || LT.second == MVT::v8i16 ||
                         LT.second == MVT::v4i32 || LT.second == MVT::v4f32);
   bool IsVSXType = ST->hasVSX() &&
                    (LT.second == MVT::v2f64 || LT.second == MVT::v2i64);
   bool IsQPXType = ST->hasQPX() &&
                    (LT.second == MVT::v4f64 || LT.second == MVT::v4f32);
 

llvm/test/Transforms/SLPVectorizer/PowerPC/short-to-double.ll

+; RUN: opt -S -mtriple=powerpc64-linux-gnu -mcpu=pwr9 -mattr=+vsx -slp-vectorizer < %s | FileCheck %s --check-prefix=CHECK-P9
+; RUN: opt -S -mtriple=powerpc64-linux-gnu -mcpu=pwr8 -mattr=+vsx -slp-vectorizer < %s | FileCheck %s --check-prefix=CHECK-P8
+
+%struct._pp = type { i16, i16, i16, i16 }
+
+; Function Attrs: norecurse nounwind readonly
+define [5 x double] @foo(double %k, i64 %n, %struct._pp* nocapture readonly %p) local_unnamed_addr #0 {
+entry:
+  %cmp17 = icmp sgt i64 %n, 0
+  br i1 %cmp17, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup:                                 ; preds = %for.body, %entry
+  %retval.sroa.0.0.lcssa = phi double [ 0.000000e+00, %entry ], [ %add, %for.body ]
+  %retval.sroa.4.0.lcssa = phi double [ 0.000000e+00, %entry ], [ %add10, %for.body ]
+  %.fca.0.insert = insertvalue [5 x double] undef, double %retval.sroa.0.0.lcssa, 0
+  %.fca.1.insert = insertvalue [5 x double] %.fca.0.insert, double %retval.sroa.4.0.lcssa, 1
+  ret [5 x double] %.fca.1.insert
+
+for.body:                                         ; preds = %entry, %for.body
+  %i.020 = phi i64 [ %inc, %for.body ], [ 0, %entry ]
+  %retval.sroa.4.019 = phi double [ %add10, %for.body ], [ 0.000000e+00, %entry ]
+  %retval.sroa.0.018 = phi double [ %add, %for.body ], [ 0.000000e+00, %entry ]
+  %r1 = getelementptr inbounds %struct._pp, %struct._pp* %p, i64 %i.020, i32 2
+  %0 = load i16, i16* %r1, align 2
+  %conv2 = uitofp i16 %0 to double
+  %mul = fmul double %conv2, %k
+  %add = fadd double %retval.sroa.0.018, %mul
+  %g5 = getelementptr inbounds %struct._pp, %struct._pp* %p, i64 %i.020, i32 1
+  %1 = load i16, i16* %g5, align 2
+  %conv7 = uitofp i16 %1 to double
+  %mul8 = fmul double %conv7, %k
+  %add10 = fadd double %retval.sroa.4.019, %mul8
+  %inc = add nuw nsw i64 %i.020, 1
+  %exitcond = icmp eq i64 %inc, %n
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body
+}
+
+; CHECK-P8: load <2 x i16>
+; CHECK-P9-NOT: load <2 x i16>