[PATCH] D22008: GlobalISel: implement low-level type suitable for MachineInstr selection

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:56
@@ +55,3 @@
+  /// width.
+  static LLT vector(int NumElements, int ScalarSizeInBits) {
+    assert(NumElements > 1 && "invalid number of vector elements");
----------------
Add a comment saying that NumElements must be > 1.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:62
@@ +61,3 @@
+  /// \brif get an unsized but valid low-level type (e.g. for a label).
+  static LLT unsized() {
+    return LLT{Unsized, 1, 0};
----------------
brif => brief

================
Comment at: include/llvm/CodeGen/LowLevelType.h:66
@@ +65,3 @@
+
+  explicit LLT(TypeKind Kind, int NumElements, int ScalarSizeInBits)
+    : ScalarSize(ScalarSizeInBits), NumElements(NumElements), Kind(Kind) {
----------------
unsigned instead of int for NumElements and ScalarSizeInBits.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:75
@@ +74,3 @@
+  /// \brief construct a low-level type based on an LLVM type.
+  explicit LLT(Type *Ty);
+
----------------
const &.
I assume we do not support nullptr.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:83
@@ +82,3 @@
+
+  int getNumElements() const {
+    assert(isVector() && "cannot get number of elements on scalar/aggregate");
----------------
Add a comment saying this should be call only for vector type.
/// \pre isVector() == true.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:83
@@ +82,3 @@
+
+  int getNumElements() const {
+    assert(isVector() && "cannot get number of elements on scalar/aggregate");
----------------
qcolombet wrote:
> Add a comment saying this should be call only for vector type.
> /// \pre isVector() == true.
unsigned instead of int.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:88
@@ +87,3 @@
+
+  int getSizeInBits() const {
+    assert(isSized() && "attempt to get size of unsized type");
----------------
/// \pre isSized() == true.

================
Comment at: include/llvm/CodeGen/LowLevelType.h:98
@@ +97,3 @@
+
+  LLT getElementType() const {
+    assert(isVector() && "cannot get element type of scalar/aggregate");
----------------
/// \pre isVector() == true.

================
Comment at: include/llvm/CodeGen/MachineInstr.h:28
@@ -26,1 +27,3 @@
+#include "llvm/CodeGen/LowLevelType.h"
+#endif
 #include "llvm/IR/DebugLoc.h"
----------------
The ifdef is probably useless.


Repository:
  rL LLVM

http://reviews.llvm.org/D22008