[clang] bafdd11 - [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy

David Sherwood via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 28 00:27:50 PDT 2020


Author: David Sherwood
Date: 2020-09-28T08:03:00+01:00
New Revision: bafdd11326a46421b68f68c794fd189c77a32e15

URL: https://github.com/llvm/llvm-project/commit/bafdd11326a46421b68f68c794fd189c77a32e15
DIFF: https://github.com/llvm/llvm-project/commit/bafdd11326a46421b68f68c794fd189c77a32e15.diff

LOG: [SVE] Replace / operator in TypeSize/ElementCount with divideCoefficientBy

After some recent upstream discussion we decided that it was best
to avoid having the / operator for both ElementCount and TypeSize,
since this could give the impression that these classes can be used
in the same way as basic integer integer types. However, division
for scalable types is a bit odd because we are only dividing the
minimum quantity by a value, as opposed to something like:

  (MinSize * Vscale) / SomeValue

This is why when performing division it's important the caller
first establishes whether the operation makes sense, perhaps by
calling isKnownMultipleOf() prior to division. The caller must now
explictly call divideCoefficientBy() on the class to perform the
operation.

Differential Revision: https://reviews.llvm.org/D87700

Added: 
    

Modified: 
    clang/lib/CodeGen/CGBuiltin.cpp
    llvm/include/llvm/CodeGen/ValueTypes.h
    llvm/include/llvm/IR/DerivedTypes.h
    llvm/include/llvm/Support/MachineValueType.h
    llvm/include/llvm/Support/TypeSize.h
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
    llvm/lib/CodeGen/TargetLoweringBase.cpp
    llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
    llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
    llvm/unittests/IR/VectorTypesTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 498d8640f329..102b5aefe8ff 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -5650,7 +5650,7 @@ Value *CodeGenFunction::EmitCommonNeonBuiltinExpr(
     if (BuiltinID == NEON::BI__builtin_neon_splatq_lane_v)
       NumElements = NumElements * 2;
     if (BuiltinID == NEON::BI__builtin_neon_splat_laneq_v)
-      NumElements = NumElements / 2;
+      NumElements = NumElements.divideCoefficientBy(2);
 
     Ops[0] = Builder.CreateBitCast(Ops[0], VTy);
     return EmitNeonSplat(Ops[0], cast<ConstantInt>(Ops[1]), NumElements);
@@ -8483,8 +8483,7 @@ Value *CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   case SVE::BI__builtin_sve_svtbl2_f64: {
     SVETypeFlags TF(Builtin->TypeModifier);
     auto VTy = cast<llvm::VectorType>(getSVEType(TF));
-    auto TupleTy = llvm::VectorType::get(VTy->getElementType(),
-                                         VTy->getElementCount() * 2);
+    auto TupleTy = llvm::VectorType::getDoubleElementsVectorType(VTy);
     Function *FExtr =
         CGM.getIntrinsic(Intrinsic::aarch64_sve_tuple_get, {VTy, TupleTy});
     Value *V0 = Builder.CreateCall(FExtr, {Ops[0], Builder.getInt32(0)});

diff  --git a/llvm/include/llvm/CodeGen/ValueTypes.h b/llvm/include/llvm/CodeGen/ValueTypes.h
index 164518faef22..b6f3fabd7f6a 100644
--- a/llvm/include/llvm/CodeGen/ValueTypes.h
+++ b/llvm/include/llvm/CodeGen/ValueTypes.h
@@ -414,7 +414,16 @@ namespace llvm {
       EVT EltVT = getVectorElementType();
       auto EltCnt = getVectorElementCount();
       assert(EltCnt.isKnownEven() && "Splitting vector, but not in half!");
-      return EVT::getVectorVT(Context, EltVT, EltCnt / 2);
+      return EVT::getVectorVT(Context, EltVT, EltCnt.divideCoefficientBy(2));
+    }
+
+    // Return a VT for a vector type with the same element type but
+    // double the number of elements. The type returned may be an
+    // extended type.
+    EVT getDoubleNumVectorElementsVT(LLVMContext &Context) const {
+      EVT EltVT = getVectorElementType();
+      auto EltCnt = getVectorElementCount();
+      return EVT::getVectorVT(Context, EltVT, EltCnt * 2);
     }
 
     /// Returns true if the given vector is a power of 2.

diff  --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index 619c699c2b97..7e9ea0e34c6b 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -504,7 +504,8 @@ class VectorType : public Type {
     auto EltCnt = VTy->getElementCount();
     assert(EltCnt.isKnownEven() &&
            "Cannot halve vector with odd number of elements.");
-    return VectorType::get(VTy->getElementType(), EltCnt/2);
+    return VectorType::get(VTy->getElementType(),
+                           EltCnt.divideCoefficientBy(2));
   }
 
   /// This static method returns a VectorType with twice as many elements as the

diff  --git a/llvm/include/llvm/Support/MachineValueType.h b/llvm/include/llvm/Support/MachineValueType.h
index b0d16ba3ef82..713f847535e8 100644
--- a/llvm/include/llvm/Support/MachineValueType.h
+++ b/llvm/include/llvm/Support/MachineValueType.h
@@ -425,7 +425,7 @@ namespace llvm {
       MVT EltVT = getVectorElementType();
       auto EltCnt = getVectorElementCount();
       assert(EltCnt.isKnownEven() && "Splitting vector, but not in half!");
-      return getVectorVT(EltVT, EltCnt / 2);
+      return getVectorVT(EltVT, EltCnt.divideCoefficientBy(2));
     }
 
     /// Returns true if the given vector is a power of 2.

diff  --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 801151048302..47aa41b851bf 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -44,10 +44,6 @@ class ElementCount {
   ElementCount operator*(unsigned RHS) {
     return { Min * RHS, Scalable };
   }
-  ElementCount operator/(unsigned RHS) {
-    assert(Min % RHS == 0 && "Min is not a multiple of RHS.");
-    return { Min / RHS, Scalable };
-  }
 
   friend ElementCount operator-(const ElementCount &LHS,
                                 const ElementCount &RHS) {
@@ -70,15 +66,24 @@ class ElementCount {
     return *this;
   }
 
-  ElementCount &operator/=(unsigned RHS) {
-    Min /= RHS;
-    return *this;
+  /// We do not provide the '/' operator here because division for polynomial
+  /// types does not work in the same way as for normal integer types. We can
+  /// only divide the minimum value (or coefficient) by RHS, which is not the
+  /// same as
+  ///   (Min * Vscale) / RHS
+  /// The caller is recommended to use this function in combination with
+  /// isKnownMultipleOf(RHS), which lets the caller know if it's possible to
+  /// perform a lossless divide by RHS.
+  ElementCount divideCoefficientBy(unsigned RHS) const {
+    return ElementCount(Min / RHS, Scalable);
   }
 
   ElementCount NextPowerOf2() const {
     return {(unsigned)llvm::NextPowerOf2(Min), Scalable};
   }
 
+  /// This function tells the caller whether the element count is known at
+  /// compile time to be a multiple of the scalar value RHS.
   bool isKnownMultipleOf(unsigned RHS) const {
     return Min % RHS == 0;
   }
@@ -234,8 +239,16 @@ class TypeSize {
     return { LHS * RHS.MinSize, RHS.IsScalable };
   }
 
-  TypeSize operator/(unsigned RHS) const {
-    return { MinSize / RHS, IsScalable };
+  /// We do not provide the '/' operator here because division for polynomial
+  /// types does not work in the same way as for normal integer types. We can
+  /// only divide the minimum value (or coefficient) by RHS, which is not the
+  /// same as
+  ///   (MinSize * Vscale) / RHS
+  /// The caller is recommended to use this function in combination with
+  /// isKnownMultipleOf(RHS), which lets the caller know if it's possible to
+  /// perform a lossless divide by RHS.
+  TypeSize divideCoefficientBy(uint64_t RHS) const {
+    return {MinSize / RHS, IsScalable};
   }
 
   TypeSize &operator-=(TypeSize RHS) {
@@ -258,18 +271,6 @@ class TypeSize {
     return {LHS.MinSize - RHS.MinSize, LHS.IsScalable};
   }
 
-  friend TypeSize operator/(const TypeSize &LHS, const TypeSize &RHS) {
-    assert(LHS.IsScalable == RHS.IsScalable &&
-           "Arithmetic using mixed scalable and fixed types");
-    return {LHS.MinSize / RHS.MinSize, LHS.IsScalable};
-  }
-
-  friend TypeSize operator%(const TypeSize &LHS, const TypeSize &RHS) {
-    assert(LHS.IsScalable == RHS.IsScalable &&
-           "Arithmetic using mixed scalable and fixed types");
-    return {LHS.MinSize % RHS.MinSize, LHS.IsScalable};
-  }
-
   // Return the minimum size with the assumption that the size is exact.
   // Use in places where a scalable size doesn't make sense (e.g. non-vector
   // types, or vectors in backends which don't support scalable vectors).
@@ -301,6 +302,10 @@ class TypeSize {
   // Returns true if the type size is zero.
   bool isZero() const { return MinSize == 0; }
 
+  /// This function tells the caller whether the type size is known at
+  /// compile time to be a multiple of the scalar value RHS.
+  bool isKnownMultipleOf(uint64_t RHS) const { return MinSize % RHS == 0; }
+
   // Casts to a uint64_t if this is a fixed-width size.
   //
   // This interface is deprecated and will be removed in a future version
@@ -357,18 +362,6 @@ class TypeSize {
     return { LHS * RHS.MinSize, RHS.IsScalable };
   }
 
-  TypeSize operator/(uint64_t RHS) const {
-    return { MinSize / RHS, IsScalable };
-  }
-
-  TypeSize operator/(int RHS) const {
-    return { MinSize / RHS, IsScalable };
-  }
-
-  TypeSize operator/(int64_t RHS) const {
-    return { MinSize / RHS, IsScalable };
-  }
-
   TypeSize NextPowerOf2() const {
     return TypeSize(llvm::NextPowerOf2(MinSize), IsScalable);
   }

diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 2b6b430efa29..0b6aca4ca34c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -19522,8 +19522,9 @@ SDValue DAGCombiner::visitEXTRACT_SUBVECTOR(SDNode *N) {
     }
     if ((DestNumElts % SrcNumElts) == 0) {
       unsigned DestSrcRatio = DestNumElts / SrcNumElts;
-      if ((NVT.getVectorMinNumElements() % DestSrcRatio) == 0) {
-        ElementCount NewExtEC = NVT.getVectorElementCount() / DestSrcRatio;
+      if (NVT.getVectorElementCount().isKnownMultipleOf(DestSrcRatio)) {
+        ElementCount NewExtEC =
+            NVT.getVectorElementCount().divideCoefficientBy(DestSrcRatio);
         EVT ScalarVT = SrcVT.getScalarType();
         if ((ExtIdx % DestSrcRatio) == 0) {
           SDLoc DL(N);
@@ -20690,7 +20691,8 @@ SDValue DAGCombiner::visitINSERT_SUBVECTOR(SDNode *N) {
       } else if ((N1SrcSVT.getSizeInBits() % EltSizeInBits) == 0) {
         unsigned Scale = N1SrcSVT.getSizeInBits() / EltSizeInBits;
         if (NumElts.isKnownMultipleOf(Scale) && (InsIdx % Scale) == 0) {
-          NewVT = EVT::getVectorVT(Ctx, N1SrcSVT, NumElts / Scale);
+          NewVT = EVT::getVectorVT(Ctx, N1SrcSVT,
+                                   NumElts.divideCoefficientBy(Scale));
           NewIdx = DAG.getVectorIdxConstant(InsIdx / Scale, DL);
         }
       }

diff  --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index b1952225ca10..356eb1ce0964 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -2636,7 +2636,7 @@ SDValue DAGTypeLegalizer::SplitVecOp_TruncateHelper(SDNode *N) {
     EVT::getFloatingPointVT(InElementSize/2) :
     EVT::getIntegerVT(*DAG.getContext(), InElementSize/2);
   EVT HalfVT = EVT::getVectorVT(*DAG.getContext(), HalfElementVT,
-                                NumElements/2);
+                                NumElements.divideCoefficientBy(2));
 
   SDValue HalfLo;
   SDValue HalfHi;
@@ -5060,11 +5060,12 @@ SDValue DAGTypeLegalizer::GenWidenVectorLoads(SmallVectorImpl<SDValue> &LdChain,
     EVT NewLdTy = LdOps[i].getValueType();
     if (NewLdTy != LdTy) {
       // Create a larger vector.
+      TypeSize LdTySize = LdTy.getSizeInBits();
+      TypeSize NewLdTySize = NewLdTy.getSizeInBits();
+      assert(NewLdTySize.isScalable() == LdTySize.isScalable() &&
+             NewLdTySize.isKnownMultipleOf(LdTySize.getKnownMinSize()));
       unsigned NumOps =
-          (NewLdTy.getSizeInBits() / LdTy.getSizeInBits()).getKnownMinSize();
-      assert(
-          (NewLdTy.getSizeInBits() % LdTy.getSizeInBits()).getKnownMinSize() ==
-          0);
+          NewLdTySize.getKnownMinSize() / LdTySize.getKnownMinSize();
       SmallVector<SDValue, 16> WidenOps(NumOps);
       unsigned j = 0;
       for (; j != End-Idx; ++j)
@@ -5085,7 +5086,8 @@ SDValue DAGTypeLegalizer::GenWidenVectorLoads(SmallVectorImpl<SDValue> &LdChain,
                        makeArrayRef(&ConcatOps[Idx], End - Idx));
 
   // We need to fill the rest with undefs to build the vector.
-  unsigned NumOps = (WidenWidth / LdTy.getSizeInBits()).getKnownMinSize();
+  unsigned NumOps =
+      WidenWidth.getKnownMinSize() / LdTy.getSizeInBits().getKnownMinSize();
   SmallVector<SDValue, 16> WidenOps(NumOps);
   SDValue UndefVal = DAG.getUNDEF(LdTy);
   {

diff  --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 26ec3f18589e..84b596b49823 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -831,9 +831,7 @@ TargetLoweringBase::getTypeConversion(LLVMContext &Context, EVT VT) const {
            "Promote may not follow Expand or Promote");
 
     if (LA == TypeSplitVector)
-      return LegalizeKind(LA,
-                          EVT::getVectorVT(Context, SVT.getVectorElementType(),
-                                           SVT.getVectorElementCount() / 2));
+      return LegalizeKind(LA, EVT(SVT).getHalfNumVectorElementsVT(Context));
     if (LA == TypeScalarizeVector)
       return LegalizeKind(LA, SVT.getVectorElementType());
     return LegalizeKind(LA, NVT);
@@ -889,7 +887,7 @@ TargetLoweringBase::getTypeConversion(LLVMContext &Context, EVT VT) const {
     //  <4 x i140> -> <2 x i140>
     if (LK.first == TypeExpandInteger)
       return LegalizeKind(TypeSplitVector,
-                          EVT::getVectorVT(Context, EltVT, NumElts / 2));
+                          VT.getHalfNumVectorElementsVT(Context));
 
     // Promote the integer element types until a legal vector type is found
     // or until the element integer type is too big. If a legal type was not
@@ -949,7 +947,8 @@ TargetLoweringBase::getTypeConversion(LLVMContext &Context, EVT VT) const {
   }
 
   // Vectors with illegal element types are expanded.
-  EVT NVT = EVT::getVectorVT(Context, EltVT, VT.getVectorElementCount() / 2);
+  EVT NVT = EVT::getVectorVT(Context, EltVT,
+                             VT.getVectorElementCount().divideCoefficientBy(2));
   return LegalizeKind(TypeSplitVector, NVT);
 }
 
@@ -982,7 +981,7 @@ static unsigned getVectorTypeBreakdownMVT(MVT VT, MVT &IntermediateVT,
   // scalar.
   while (EC.getKnownMinValue() > 1 &&
          !TLI->isTypeLegal(MVT::getVectorVT(EltTy, EC))) {
-    EC /= 2;
+    EC = EC.divideCoefficientBy(2);
     NumVectorRegs <<= 1;
   }
 
@@ -1482,7 +1481,7 @@ unsigned TargetLoweringBase::getVectorTypeBreakdown(LLVMContext &Context, EVT VT
   // end with a scalar if the target doesn't support vectors.
   while (EltCnt.getKnownMinValue() > 1 &&
          !isTypeLegal(EVT::getVectorVT(Context, EltTy, EltCnt))) {
-    EltCnt /= 2;
+    EltCnt = EltCnt.divideCoefficientBy(2);
     NumVectorRegs <<= 1;
   }
 

diff  --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 4522dc054ed2..b57a8edd4578 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -10598,8 +10598,9 @@ SDValue AArch64TargetLowering::LowerSVEStructLoad(unsigned Intrinsic,
   assert(VT.getVectorElementCount().getKnownMinValue() % N == 0 &&
          "invalid tuple vector type!");
 
-  EVT SplitVT = EVT::getVectorVT(*DAG.getContext(), VT.getVectorElementType(),
-                                 VT.getVectorElementCount() / N);
+  EVT SplitVT =
+      EVT::getVectorVT(*DAG.getContext(), VT.getVectorElementType(),
+                       VT.getVectorElementCount().divideCoefficientBy(N));
   assert(isTypeLegal(SplitVT));
 
   SmallVector<EVT, 5> VTs(N, SplitVT);
@@ -14393,9 +14394,7 @@ performSignExtendInRegCombine(SDNode *N, TargetLowering::DAGCombinerInfo &DCI,
     assert((EltTy == MVT::i8 || EltTy == MVT::i16 || EltTy == MVT::i32) &&
            "Sign extending from an invalid type");
 
-    EVT ExtVT = EVT::getVectorVT(*DAG.getContext(),
-                                 VT.getVectorElementType(),
-                                 VT.getVectorElementCount() * 2);
+    EVT ExtVT = VT.getDoubleNumVectorElementsVT(*DAG.getContext());
 
     SDValue Ext = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, ExtOp.getValueType(),
                               ExtOp, DAG.getValueType(ExtVT));

diff  --git a/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp b/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
index f76b8db602ec..fb00a12f4851 100644
--- a/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ b/llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -61,9 +61,10 @@ TEST(ScalableVectorMVTsTest, HelperFuncs) {
   EXPECT_EQ(Vnx2i32.widenIntegerVectorElementType(Ctx), Vnx2i64);
   EXPECT_EQ(Vnx4i32.getHalfNumVectorElementsVT(Ctx), Vnx2i32);
 
-  // Check that overloaded '*' and '/' operators work
+  // Check that operators work
   EXPECT_EQ(EVT::getVectorVT(Ctx, MVT::i64, EltCnt * 2), MVT::nxv4i64);
-  EXPECT_EQ(EVT::getVectorVT(Ctx, MVT::i64, EltCnt / 2), MVT::nxv1i64);
+  EXPECT_EQ(EVT::getVectorVT(Ctx, MVT::i64, EltCnt.divideCoefficientBy(2)),
+            MVT::nxv1i64);
 
   // Check that float->int conversion works
   EVT Vnx2f64 = EVT::getVectorVT(Ctx, MVT::f64, ElementCount::getScalable(2));

diff  --git a/llvm/unittests/IR/VectorTypesTest.cpp b/llvm/unittests/IR/VectorTypesTest.cpp
index 7525ee9872b7..37137053bf15 100644
--- a/llvm/unittests/IR/VectorTypesTest.cpp
+++ b/llvm/unittests/IR/VectorTypesTest.cpp
@@ -71,8 +71,8 @@ TEST(VectorTypesTest, FixedLength) {
   EXPECT_EQ(V4Int64Ty->getNumElements(), 4U);
   EXPECT_EQ(V4Int64Ty->getElementType()->getScalarSizeInBits(), 64U);
 
-  auto *V2Int64Ty =
-      dyn_cast<FixedVectorType>(VectorType::get(Int64Ty, EltCnt / 2));
+  auto *V2Int64Ty = dyn_cast<FixedVectorType>(
+      VectorType::get(Int64Ty, EltCnt.divideCoefficientBy(2)));
   ASSERT_NE(nullptr, V2Int64Ty);
   EXPECT_EQ(V2Int64Ty->getNumElements(), 2U);
   EXPECT_EQ(V2Int64Ty->getElementType()->getScalarSizeInBits(), 64U);
@@ -166,8 +166,8 @@ TEST(VectorTypesTest, Scalable) {
   EXPECT_EQ(ScV4Int64Ty->getMinNumElements(), 4U);
   EXPECT_EQ(ScV4Int64Ty->getElementType()->getScalarSizeInBits(), 64U);
 
-  auto *ScV2Int64Ty =
-      dyn_cast<ScalableVectorType>(VectorType::get(Int64Ty, EltCnt / 2));
+  auto *ScV2Int64Ty = dyn_cast<ScalableVectorType>(
+      VectorType::get(Int64Ty, EltCnt.divideCoefficientBy(2)));
   ASSERT_NE(nullptr, ScV2Int64Ty);
   EXPECT_EQ(ScV2Int64Ty->getMinNumElements(), 2U);
   EXPECT_EQ(ScV2Int64Ty->getElementType()->getScalarSizeInBits(), 64U);


        


More information about the cfe-commits mailing list