[PATCH] D147697: [IR] Add TargetExtTypeClass

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle marked 3 inline comments as done.
nhaehnle added inline comments.


================
Comment at: llvm/docs/LangRef.rst:3802-3803
+``zeroinitializer`` constant be valid. A complete list of type properties may be
+found in the documentation for ``llvm::TargetExtType::Property`` (`doxygen
+<https://llvm.org/doxygen/classllvm_1_1TargetExtType.html>`_).
+
----------------
arsenm wrote:
> I don't think the LangRef should refer to doxygen for things like this and should explicitly list the properties 
That text was pre-existing, but I'll replace it.


================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4009
+    } else {
+      llvm_unreachable("sdata value type not implemented");
+    }
----------------
arsenm wrote:
> Seems like it should be report_fatal_error
This is the bitcode *writer*, so getting here is always a bug in the code, not a bug in any input (extending sdata::Value without updating this code here).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147697