[PATCH] D146914: Migrate `IIT_Info` into `Intrinsics.td`

[NAKAMURA Takumi via Phabricator via llvm-commits]

chapuni added inline comments.


================
Comment at: llvm/utils/TableGen/IntrinsicEmitter.cpp:189
+  OS << "#ifdef GET_INTRINSIC_IITINFO\n";
+  std::vector<StringRef> RecsByNumber(256);
+  auto IIT_Base = Records.getAllDerivedDefinitionsIfDefined("IIT_Base");
----------------
arsenm wrote:
> std::array
Could we use other expression instead of `256` here?
Kinda `max<uint8_t> + 1` is not smart, I think. `256` is the obvious value for us.


================
Comment at: llvm/utils/TableGen/IntrinsicEmitter.cpp:193
+    auto Number = Rec->getValueAsInt("Number");
+    assert(0 <= Number && Number < 256 && "IIT_Info.Number should be uint8_t");
+    assert(RecsByNumber[Number].empty() && "Duplicate IIT_Info.Number");
----------------
arsenm wrote:
> arsenm wrote:
> > assert(isUInt<8>(Number))
> This isn't done? isUInt<8> would be clearer (or at least swap the compare to be Number >= 0)
I think I can rely on size of array.
Or may I remove mention `uint8_t` in the failure message?

`m <= x && x < n` is my preference as in_range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146914