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

[Tim Northover via llvm-commits]

t.p.northover marked 9 inline comments as done.

================
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:
> qcolombet wrote:
> > Add a comment saying this should be call only for vector type.
> > /// \pre isVector() == true.
> unsigned instead of int.
I'll use a normal comment because "\pre" seems to be ignored by our documentation generator (and I prefer it).

================
Comment at: include/llvm/CodeGen/LowLevelType.h:136
@@ +135,3 @@
+  LLT doubleElements() const {
+    assert(isVector());
+    return LLT{Kind, NumElements * 2, ScalarSize};
----------------
eli.friedman wrote:
> Should this work on a scalar type?  (`s32` -> `<2 x s32>`).
For symmetry with halfElements, probably. It would make `MoreElements` also act as "Vectorize" in the same way `FewerElements` doubles as Scalarize which doesn't seem like a bad thing.

================
Comment at: include/llvm/CodeGen/MachineInstr.h:28
@@ -26,1 +27,3 @@
+#include "llvm/CodeGen/LowLevelType.h"
+#endif
 #include "llvm/IR/DebugLoc.h"
----------------
qcolombet wrote:
> The ifdef is probably useless.
Why? It seems rather weird to #ifdef the use of LLT but not its definition.


Repository:
  rL LLVM

http://reviews.llvm.org/D22008