[PATCH] D44337: Make ConstantDataArray::get constructor templated. Will support signed integers.

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

Minor drive by comments.



================
Comment at: include/llvm/IR/Constants.h:701
+    const char *Data = reinterpret_cast<const char *>(Elts.data());
+    int noOfBytes = sizeof(ElementTy);
+    Type *Ty;
----------------
I'm not sure of the rules here, but perhaps this needs to be `sizeof(ElementTy) * CHAR_BIT / 8` to be technically correct?


================
Comment at: include/llvm/IR/Constants.h:708
+        break;
+      case 2:
+        Ty = ArrayType::get(Type::getInt16Ty(Context), Elts.size());
----------------
There is a `Type::getIntNTy` that you can use here.


================
Comment at: include/llvm/IR/Constants.h:732
+    } else {
+      assert(0 && "Unsupported type in ConstantDataArray::get()");
+    }
----------------
Use `llvm_unreachable`.


Repository:
  rL LLVM

https://reviews.llvm.org/D44337