[PATCH] D135202: [IR] Add an opaque type to LLVM.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3112
+  SmallVector<Type *, 4> TypeParams;
+  SmallVector<unsigned, 8> IntParams;
+  bool SeenInt = false;
----------------
nit: Can drop explicit SmallVector sizes here.


================
Comment at: llvm/lib/IR/Type.cpp:855
+    return TargetTypeInfo(Type::getInt8PtrTy(C, 0),
+        TargetExtType::HasZeroInit,
+        TargetExtType::CanBeGlobal);
----------------
It looks like the HasZeroInit property exists, but is never actually checked anywhere? Should either drop it or check it.


================
Comment at: llvm/lib/IR/Type.cpp:858
+  }
+  if (Name == "aarch64.svcount") {
+    return TargetTypeInfo(ScalableVectorType::get(Type::getInt1Ty(C), 16));
----------------
Probably want to drop this one before landing, as it's part of a different RFC.


================
Comment at: llvm/lib/IR/Type.cpp:861
+  }
+  return TargetTypeInfo(Type::getInt1Ty(C));
+}
----------------
I'd probably go for defaulting to an unsized (and thus maximally limited) type. `i1` seems like an odd choice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135202/new/

https://reviews.llvm.org/D135202