[PATCH] D135202: [IR] Add a target extension type to LLVM.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a comment.

Some pedantic nits.
"opaque" remains in some places.



================
Comment at: llvm/docs/ReleaseNotes.rst:108
+* Target extension types have been added, which allow targets to have
+  target-specific types whose types need to be preserved through the optimizer.
+
----------------
"target-specific types whose types" is confusing.
Do I read it wrong or did you mean something different? In any case this should be clarified.



================
Comment at: llvm/include/llvm-c/Core.h:289
+  LLVMPoisonValueValueKind,
+  LLVMConstantTargetNoneValueKind
 } LLVMValueKind;
----------------
Nit: add comma


================
Comment at: llvm/include/llvm/IR/Constants.h:867
+
+  /// Methods for support type inquiry through isa, cast, and dyn_cast:
+  static bool classof(const Value *V) {
----------------
Above comments are missing '.' in the end and this one ends with a colon.


================
Comment at: llvm/lib/IR/Type.cpp:839
+
+struct TargetTypeInfo {
+  Type *LayoutType;
----------------
The struct should be in anonymous namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135202