[NOT YET FOR REVIEW][AArch64][LV] Implement AArch64TTIImpl::getRegisterClassForType

[peterwaller-arm]

Summary

NOTE: I'm sending this for early review. Here's a possible solution to fixing the register class issue, if it needs fixing. Another solution would be to have getRegUsageForType return a tuple of (Count, Class), but this might not be straightforward to implement for all targets, I haven't investigated.

The loop vectorizer uses getRegisterClassForType to determine the
register class occupied by a given an llvm::Type* for the purposes of
register pressure estimation, which in turn feeds into the choice of the
a maximum VF to use.

As part of this, the loop vectorizer has a heuristic
isScalarAfterVectorization, which `getRegisterClassForType(bool
Vector, Type *Ty)` effectively receives as its 'Vector' parameter. The
default implementation of getRegisterClassForType ignores the Type
parameter, just returning a 'VectorRC' if Vector is True.

However, some large vector types (e.g. v128i1) are represented with a
number of scalar registers. This is known by TLI->getRegisterType, so
add a TTI interface getRegMVTForType and propagate this information
up to the newly implemented getRegisterClassForType.

[dmgreen]

It might be worth teaching AArch64TTIImpl::getRegisterClassForType that fp regs and vector regs are the same thing. That would require some testing to make sure it actually produces better results though.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

229

Can this just use the existing TLI->getTypeLegalizationCost call to get the MVT?

[peterwaller-arm]

A fair point on the vector/fp overlap, will consider.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

229

As far as I can see, no, because the return value of getTypeLegalizationCost is wrong, deriving its value from getValueType. This just does EVT::getVectorVT(Ty->getContext(), EVT::getEVT(EltTy, false), under the hood, which is different than TLI->getRegisterType, which uses information derived from computeRegisterProperties.

[dmgreen]

In D125956#3524706, @peterwaller-arm wrote:

A fair point on the vector/fp overlap, will consider.

Yeah, it at least deserves to be a separate patch.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

229

I would expect getTypeLegalizationCost(..).second to be a scalar type if the original vector was scalarized. It goes via the legalization mechanism in getTypeConversion.

[peterwaller-arm]

On the effect of this patch: there are 10 translation units out of ~2,000 in the LNT benchmarks which have differing codegen as a consequence. I'll check for any performance effect next week before proposing this.

MultiSource/Applications/JM/ldecod/CMakeFiles/ldecod.dir/erc_do_p.o
MultiSource/Applications/JM/lencod/CMakeFiles/lencod.dir/me_distortion.o
MultiSource/Benchmarks/mediabench/mpeg2/mpeg2dec/CMakeFiles/mpeg2decode.dir/recon.o
MultiSource/Benchmarks/mediabench/jpeg/jpeg-6a/CMakeFiles/cjpeg.dir/jquant2.o
MultiSource/Benchmarks/Bullet/CMakeFiles/bullet.dir/btQuantizedBvh.cpp.opt.yaml
MultiSource/Benchmarks/MiBench/automotive-susan/CMakeFiles/automotive-susan.dir/susan.o
MultiSource/Benchmarks/MiBench/consumer-jpeg/CMakeFiles/consumer-jpeg.dir/jquant2.o
MultiSource/Benchmarks/7zip/CMakeFiles/7zip-benchmark.dir/CPP/7zip/Crypto/WzAes.cpp.opt.yaml
MultiSource/Benchmarks/7zip/CMakeFiles/7zip-benchmark.dir/C/Delta.o
SingleSource/Benchmarks/Misc/CMakeFiles/mandel.dir/mandel.o

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

229

getTypeConversion appears to use TransformToType. Here is the information I've dumped from computeRegisterProperties with vscale_range(4,4) (512 bit reg):

CRP:     v1i1 PreferredAction=TypeScalarizeVector NumRegisters=1 NumIntermediates=1 VT=    v1i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeScalarizeVector TransformToType=    v1i1
CRP:     v2i1 PreferredAction=TypePromoteInteger Promote    v2i32
CRP:     v4i1 PreferredAction=TypePromoteInteger Promote    v4i16
CRP:     v8i1 PreferredAction=TypePromoteInteger Promote     v8i8
CRP:    v16i1 PreferredAction=TypePromoteInteger Promote    v16i8
CRP:    v32i1 PreferredAction=TypePromoteInteger Promote    v32i8
CRP:    v64i1 PreferredAction=TypePromoteInteger Promote    v64i8
CRP:   v128i1 PreferredAction=TypePromoteInteger NumRegisters=128 NumIntermediates=128 VT=  v128i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v128i1
CRP:   v256i1 PreferredAction=TypePromoteInteger NumRegisters=256 NumIntermediates=256 VT=  v256i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v256i1
CRP:   v512i1 PreferredAction=TypePromoteInteger NumRegisters=512 NumIntermediates=512 VT=  v512i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType=  v512i1
CRP:  v1024i1 PreferredAction=TypePromoteInteger NumRegisters=1024 NumIntermediates=1024 VT= v1024i1 IntermediateVT=      i1 RegisterVT=     i32 Final ValueTypeActions[VT]= TypeSplitVector TransformToType= v1024i1

For example, getTypeLegalizationCost uses the TransformToType of v128i1, not understanding that it use the RegisterVT of i32.

This could be considered buggy too, I started playing around with this but found myself mired in failing tests, so I chose the easier option as a first pass.

[peterwaller-arm]

I'm going to abandon this for now, though I may take another swipe at this later. I've identified while testing other types that this patch is wrong because it's not returning VectorRC when it should.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

229

Can this just use the existing TLI->getTypeLegalizationCost call to get the MVT?

It can however directly use getTLI()->getRegisterType, so I'm thinking I shouldn't introduce another TTI. I was however thinking along the lines of getRegUsage returning this information alongside the register count, hence putting it nearby.

[peterwaller-arm]

Forgot to press the abandon button, and changed my mind, I'll leave it in changes planned for a bit since there are no reviewers tagged yet.

llvm/include/llvm/Analysis/TargetTransformInfo.h

 
 #include "llvm/IR/FMF.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/InstructionCost.h"
+#include "llvm/Support/MachineValueType.h"
 #include <functional>
 #include <utility>
 
 namespace llvm {
 
 namespace Intrinsic {
 typedef unsigned ID;
 }
public:
   bool useAA() const;
 
   /// Return true if this type is legal.
   bool isTypeLegal(Type *Ty) const;
 
   /// Returns the estimated number of registers required to represent \p Ty.
   unsigned getRegUsageForType(Type *Ty) const;
 
+  /// Returns the MVT used to represent \p Ty.
+  MVT getRegMVTForType(Type *Ty) const;
+
   /// Return true if switches should be turned into lookup tables for the
   /// target.
   bool shouldBuildLookupTables() const;
 
   /// Return true if switches should be turned into lookup tables
   /// containing this constant value for the target.
   bool shouldBuildLookupTablesForConstant(Constant *C) const;
 
virtual InstructionCost getScalingFactorCost(Type *Ty, GlobalValue *BaseGV,
                                                bool HasBaseReg, int64_t Scale,
                                                unsigned AddrSpace) = 0;
   virtual bool LSRWithInstrQueries() = 0;
   virtual bool isTruncateFree(Type *Ty1, Type *Ty2) = 0;
   virtual bool isProfitableToHoist(Instruction *I) = 0;
   virtual bool useAA() = 0;
   virtual bool isTypeLegal(Type *Ty) = 0;
   virtual unsigned getRegUsageForType(Type *Ty) = 0;
+  virtual MVT getRegMVTForType(Type *Ty) const = 0;
   virtual bool shouldBuildLookupTables() = 0;
   virtual bool shouldBuildLookupTablesForConstant(Constant *C) = 0;
   virtual bool shouldBuildRelLookupTables() = 0;
   virtual bool useColdCCForColdCall(Function &F) = 0;
   virtual InstructionCost getScalarizationOverhead(VectorType *Ty,
                                                    const APInt &DemandedElts,
                                                    bool Insert,
                                                    bool Extract) = 0;
public:
   bool isProfitableToHoist(Instruction *I) override {
     return Impl.isProfitableToHoist(I);
   }
   bool useAA() override { return Impl.useAA(); }
   bool isTypeLegal(Type *Ty) override { return Impl.isTypeLegal(Ty); }
   unsigned getRegUsageForType(Type *Ty) override {
     return Impl.getRegUsageForType(Ty);
   }
+  MVT getRegMVTForType(Type *Ty) const override {
+    return Impl.getRegMVTForType(Ty);
+  }
   bool shouldBuildLookupTables() override {
     return Impl.shouldBuildLookupTables();
   }
   bool shouldBuildLookupTablesForConstant(Constant *C) override {
     return Impl.shouldBuildLookupTablesForConstant(C);
   }
   bool shouldBuildRelLookupTables() override {
     return Impl.shouldBuildRelLookupTables();

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

public:
 
   bool isProfitableToHoist(Instruction *I) const { return true; }
 
   bool useAA() const { return false; }
 
   bool isTypeLegal(Type *Ty) const { return false; }
 
   unsigned getRegUsageForType(Type *Ty) const { return 1; }
+  MVT getRegMVTForType(Type *Ty) const { assert(false && "unimplemented"); }
 
   bool shouldBuildLookupTables() const { return true; }
 
   bool shouldBuildLookupTablesForConstant(Constant *C) const { return true; }
 
   bool shouldBuildRelLookupTables() const { return false; }
 
   bool useColdCCForColdCall(Function &F) const { return false; }

llvm/include/llvm/CodeGen/BasicTTIImpl.h

bool isTypeLegal(Type *Ty) {
     return getTLI()->isTypeLegal(VT);
   }
 
   unsigned getRegUsageForType(Type *Ty) {
     EVT ETy = getTLI()->getValueType(DL, Ty);
     return getTLI()->getNumRegisters(Ty->getContext(), ETy);
   }
 
+  MVT getRegMVTForType(Type *Ty) const {
+    EVT ETy = getTLI()->getValueType(DL, Ty);
+    return getTLI()->getRegisterType(Ty->getContext(), ETy);
+  }
+
   InstructionCost getGEPCost(Type *PointeeType, const Value *Ptr,
                              ArrayRef<const Value *> Operands,
                              TTI::TargetCostKind CostKind) {
     return BaseT::getGEPCost(PointeeType, Ptr, Operands, CostKind);
   }
 
   unsigned getEstimatedNumberOfCaseClusters(const SwitchInst &SI,
                                             unsigned &JumpTableSize,

llvm/lib/Analysis/TargetTransformInfo.cpp

 bool TargetTransformInfo::isTypeLegal(Type *Ty) const {
   return TTIImpl->isTypeLegal(Ty);
 }
 
 unsigned TargetTransformInfo::getRegUsageForType(Type *Ty) const {
   return TTIImpl->getRegUsageForType(Ty);
 }
 
+MVT TargetTransformInfo::getRegMVTForType(Type *Ty) const {
+  return TTIImpl->getRegMVTForType(Ty);
+}
+
 bool TargetTransformInfo::shouldBuildLookupTables() const {
   return TTIImpl->shouldBuildLookupTables();
 }
 
 bool TargetTransformInfo::shouldBuildLookupTablesForConstant(
     Constant *C) const {
   return TTIImpl->shouldBuildLookupTablesForConstant(C);
 }

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h

unsigned getNumberOfRegisters(unsigned ClassID) const {
     if (Vector) {
       if (ST->hasNEON())
         return 32;
       return 0;
     }
     return 31;
   }
 
+  unsigned getRegisterClassForType(bool Vector, Type *Ty) const;
+
   InstructionCost getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
                                         TTI::TargetCostKind CostKind);
 
   Optional<Instruction *> instCombineIntrinsic(InstCombiner &IC,
                                                IntrinsicInst &II) const;
 
   Optional<Value *> simplifyDemandedVectorEltsIntrinsic(
       InstCombiner &IC, IntrinsicInst &II, APInt DemandedElts, APInt &UndefElts,

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

 AArch64TTIImpl::getPopcntSupport(unsigned TyWidth) {
   assert(isPowerOf2_32(TyWidth) && "Ty width must be power of 2");
   if (TyWidth == 32 || TyWidth == 64)
     return TTI::PSK_FastHardware;
   // TODO: AArch64TargetLowering::LowerCTPOP() supports 128bit popcount.
   return TTI::PSK_Software;
 }
 
+unsigned AArch64TTIImpl::getRegisterClassForType(bool Vector, Type *Ty) const {
+  if (Ty == nullptr)
+    return Vector ? 1 : 0;
+  return getRegMVTForType(Ty).isVector() ? 1 : 0;
+}
+
 InstructionCost
 AArch64TTIImpl::getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
                                       TTI::TargetCostKind CostKind) {
   auto *RetTy = ICA.getReturnType();
   switch (ICA.getID()) {
   case Intrinsic::umin:
   case Intrinsic::umax:
   case Intrinsic::smin:

llvm/test/Transforms/LoopVectorize/AArch64/i1-reg-usage.ll

 ; RUN: opt -loop-vectorize -debug-only=loop-vectorize -disable-output 2>&1 < %s | FileCheck %s
 ; REQUIRES: asserts
 
 target triple = "aarch64"
 
 ; Test that shows how many registers the loop vectorizer thinks an illegal <VF x i1> will consume.
 
 ; CHECK-LABEL: LV: Checking a loop in 'or_reduction_neon' from <stdin>
 ; CHECK: LV(REG): VF = 32
-; CHECK-NEXT: LV(REG): Found max usage: 2 item
-; CHECK-NEXT: LV(REG): RegisterClass: Generic::VectorRC, 72 registers
-; CHECK-NEXT: LV(REG): RegisterClass: Generic::ScalarRC, 1 registers
+; CHECK-NEXT: LV(REG): Found max usage: 1 item
+; CHECK-NEXT: LV(REG): RegisterClass: Generic::ScalarRC, 72 registers
 
 define i1 @or_reduction_neon(i32 %arg, ptr %ptr) "target-features"="+neon" {
 entry:
   br label %loop
 exit:
   ret i1 %reduction_next
 loop:
   %induction = phi i32 [ 0, %entry ], [ %induction_next, %loop ]
   %reduction = phi i1 [ 0, %entry ], [ %reduction_next, %loop ]
   %gep = getelementptr inbounds i32, ptr %ptr, i32 %induction
   %loaded = load i32, ptr %gep
   %i1 = icmp eq i32 %loaded, %induction
   %reduction_next = or i1 %i1, %reduction
   %induction_next = add nuw i32 %induction, 1
   %cond = icmp eq i32 %induction_next, %arg
   br i1 %cond, label %exit, label %loop, !llvm.loop !32
 }
 
 ; CHECK-LABEL: LV: Checking a loop in 'or_reduction_sve'
 ; CHECK: LV(REG): VF = 64
-; CHECK-NEXT: LV(REG): Found max usage: 2 item
-; CHECK-NEXT: LV(REG): RegisterClass: Generic::VectorRC, 136 registers
-; CHECK-NEXT: LV(REG): RegisterClass: Generic::ScalarRC, 1 registers
+; CHECK-NEXT: LV(REG): Found max usage: 1 item
+; CHECK-NEXT: LV(REG): RegisterClass: Generic::ScalarRC, 136 registers
 
 define i1 @or_reduction_sve(i32 %arg, ptr %ptr) vscale_range(2,2) "target-features"="+sve" {
 entry:
   br label %loop
 exit:
   ret i1 %reduction_next
 loop:
   %induction = phi i32 [ 0, %entry ], [ %induction_next, %loop ]