Make ConstantDataArray::get constructor templated. Will support signed integers.

[asbirlea]

Summary

Make ConstantDataArray::get() constructors a single templated one.

[timshen]

include/llvm/IR/Constants.h

706 ↗

(On Diff #137871)

Would it be good to create Type::getTyByScalar:

class Type {
  template <typename ScalarType>
  Type* getTyByScalar(LLVMContext*) {
    switch (sizeof(ScalarType)) { ... }
  }

Also, it'd be better to expand getTyByScalar to pointer and all sorts of other crazy types, but Scalar would be a good start.

Then ConstantDataArray::get will be like

return ArrayType::get(Type::getTyByScalar<ElementTy>(Context), Elts.size());

[sanjoy]

Minor drive by comments.

include/llvm/IR/Constants.h

701 ↗

(On Diff #137871)

I'm not sure of the rules here, but perhaps this needs to be sizeof(ElementTy) * CHAR_BIT / 8 to be technically correct?

708 ↗

(On Diff #137871)

There is a Type::getIntNTy that you can use here.

732 ↗

(On Diff #137871)

Use llvm_unreachable.

[asbirlea]

Address comments.

include/llvm/IR/Constants.h

701 ↗

(On Diff #137871)

It made sense to add * CHAR_BIT when passing no of bits to getIntNTy.
But having no_bytes = sizeof * CHAR_BIT / 8, and no bits = no_bytes *CHAR_BIT together doesn't make sense. I dropped the first, but I'm not clear if that's correct.

[asbirlea]

Switch to noOfBits in Type.h.

[timshen]

LGTM with some nits.

include/llvm/IR/Constants.h

704 ↗

(On Diff #138052)

Maybe just Elts.size() * sizeof(ElementTy)?

include/llvm/IR/Type.h

410 ↗

(On Diff #138052)

getScalarTy seems a more consistent name.

[asbirlea]

Address comments.
Thank you for the review!

llvm/trunk/include/llvm/IR/Constants.h

class ConstantDataArray final : public ConstantDataSequential {
   friend class ConstantDataSequential;
 
   explicit ConstantDataArray(Type *ty, const char *Data)
       : ConstantDataSequential(ty, ConstantDataArrayVal, Data) {}
 
 public:
   ConstantDataArray(const ConstantDataArray &) = delete;
 
-  /// get() constructors - Return a constant with array type with an element
+  /// get() constructor - Return a constant with array type with an element
   /// count and element type matching the ArrayRef passed in.  Note that this
   /// can return a ConstantAggregateZero object.
-  static Constant *get(LLVMContext &Context, ArrayRef<uint8_t> Elts);
-  static Constant *get(LLVMContext &Context, ArrayRef<uint16_t> Elts);
-  static Constant *get(LLVMContext &Context, ArrayRef<uint32_t> Elts);
-  static Constant *get(LLVMContext &Context, ArrayRef<uint64_t> Elts);
-  static Constant *get(LLVMContext &Context, ArrayRef<float> Elts);
-  static Constant *get(LLVMContext &Context, ArrayRef<double> Elts);
+  template <typename ElementTy>
+  static Constant *get(LLVMContext &Context, ArrayRef<ElementTy> Elts) {
+    const char *Data = reinterpret_cast<const char *>(Elts.data());
+    Type *Ty =
+        ArrayType::get(Type::getScalarTy<ElementTy>(Context), Elts.size());
+    return getImpl(StringRef(Data, Elts.size() * sizeof(ElementTy)), Ty);
+  }
+
+  /// get() constructor - ArrayTy needs to be compatible with
+  /// ArrayRef<ElementTy>. Calls get(LLVMContext, ArrayRef<ElementTy>).
+  template <typename ArrayTy>
+  static Constant *get(LLVMContext &Context, ArrayTy &Elts) {
+    return ConstantDataArray::get(Context, makeArrayRef(Elts));
+  }
 
   /// getFP() constructors - Return a constant with array type with an element
   /// count and element type of float with precision matching the number of
   /// bits in the ArrayRef passed in. (i.e. half for 16bits, float for 32bits,
   /// double for 64bits) Note that this can return a ConstantAggregateZero
   /// object.
   static Constant *getFP(LLVMContext &Context, ArrayRef<uint16_t> Elts);
   static Constant *getFP(LLVMContext &Context, ArrayRef<uint32_t> Elts);

llvm/trunk/include/llvm/IR/Type.h

public:
   static Type *getTokenTy(LLVMContext &C);
   static IntegerType *getIntNTy(LLVMContext &C, unsigned N);
   static IntegerType *getInt1Ty(LLVMContext &C);
   static IntegerType *getInt8Ty(LLVMContext &C);
   static IntegerType *getInt16Ty(LLVMContext &C);
   static IntegerType *getInt32Ty(LLVMContext &C);
   static IntegerType *getInt64Ty(LLVMContext &C);
   static IntegerType *getInt128Ty(LLVMContext &C);
+  template <typename ScalarTy> static Type *getScalarTy(LLVMContext &C) {
+    int noOfBits = sizeof(ScalarTy) * CHAR_BIT;
+    if (std::is_integral<ScalarTy>::value) {
+      return Type::getIntNTy(C, noOfBits);
+    } else if (std::is_floating_point<ScalarTy>::value) {
+      switch (noOfBits) {
+      case 32:
+        return Type::getFloatTy(C);
+      case 64:
+        return Type::getDoubleTy(C);
+      }
+    }
+    llvm_unreachable("Unsupported type in Type::getScalarTy");
+  }
 
   //===--------------------------------------------------------------------===//
   // Convenience methods for getting pointer types with one of the above builtin
   // types as pointee.
   //
   static PointerType *getHalfPtrTy(LLVMContext &C, unsigned AS = 0);
   static PointerType *getFloatPtrTy(LLVMContext &C, unsigned AS = 0);
   static PointerType *getDoublePtrTy(LLVMContext &C, unsigned AS = 0);

llvm/trunk/lib/IR/Constants.cpp

if (!(*Entry)->Next) {
     }
   }
 
   // If we were part of a list, make sure that we don't delete the list that is
   // still owned by the uniquing map.
   Next = nullptr;
 }
 
-/// get() constructors - Return a constant with array type with an element
-/// count and element type matching the ArrayRef passed in.  Note that this
-/// can return a ConstantAggregateZero object.
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<uint8_t> Elts) {
-  Type *Ty = ArrayType::get(Type::getInt8Ty(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 1), Ty);
-}
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<uint16_t> Elts){
-  Type *Ty = ArrayType::get(Type::getInt16Ty(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 2), Ty);
-}
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<uint32_t> Elts){
-  Type *Ty = ArrayType::get(Type::getInt32Ty(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 4), Ty);
-}
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<uint64_t> Elts){
-  Type *Ty = ArrayType::get(Type::getInt64Ty(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 8), Ty);
-}
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<float> Elts) {
-  Type *Ty = ArrayType::get(Type::getFloatTy(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 4), Ty);
-}
-Constant *ConstantDataArray::get(LLVMContext &Context, ArrayRef<double> Elts) {
-  Type *Ty = ArrayType::get(Type::getDoubleTy(Context), Elts.size());
-  const char *Data = reinterpret_cast<const char *>(Elts.data());
-  return getImpl(StringRef(Data, Elts.size() * 8), Ty);
-}
-
 /// getFP() constructors - Return a constant with array type with an element
 /// count and element type of float with precision matching the number of
 /// bits in the ArrayRef passed in. (i.e. half for 16bits, float for 32bits,
 /// double for 64bits) Note that this can return a ConstantAggregateZero
 /// object.
 Constant *ConstantDataArray::getFP(LLVMContext &Context,
                                    ArrayRef<uint16_t> Elts) {
   Type *Ty = ArrayType::get(Type::getHalfTy(Context), Elts.size());