[PATCH] D36085: [globalisel][tablegen] Generate TypeObject table. NFC

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 2 07:05:17 PDT 2017


rovka added a comment.

You should add checks for the GILLT enum in GlobalISelEmitter.td, since now it won't be the same enum for all targets.



================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:116
   bool operator<(const LLTCodeGen &Other) const {
-    if (!Ty.isValid())
-      return Other.Ty.isValid();
-    if (Ty.isScalar()) {
-      if (!Other.Ty.isValid())
-        return false;
-      if (Other.Ty.isScalar())
-        return Ty.getSizeInBits() < Other.Ty.getSizeInBits();
-      return false;
-    }
+    if (Ty.isValid() != Other.Ty.isValid())
+      return Ty.isValid() < Other.Ty.isValid();
----------------
This is going to fall all the way to the unreachable if both types are invalid - you should check that case explicitly.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:123
+
+    if (Ty.isScalar())
+      return Ty.getSizeInBits() < Other.Ty.getSizeInBits();
----------------
I think you can get rid of this check. If Ty.isVector, you check NumElements, and if that doesn't settle it then you can unconditionally look at the SizeInBits and it will be relevant for both scalars and vectors.


https://reviews.llvm.org/D36085





More information about the llvm-commits mailing list