[PATCH] D155556: [AMDGPU] Isolate target intrinsics that are not GMIR intrinsics.

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1944
+  /// Returns Intrinsic::ID if this is an intrinsic, else returns not_intrinsic.
   unsigned getIntrinsicID() const {
+    if (!isIntrinsic())
----------------
@arsenm, do you see any value in introducing `GIntrinsic` as a subclass of `GenericMachineInstr` and moving `getIntrinsicID()` to it? I tried half of that in D154766, but I don't see any real advantage. I am thinking I'll roll it back in D154766.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1945
   unsigned getIntrinsicID() const {
+    if (!isIntrinsic())
+      return Intrinsic::not_intrinsic;
----------------
arsenm wrote:
> I think making this an assert would be fine?
This allows to reduce verbosity at the client. For example, rather than doing `isIntrinsic() { switch (ID) }` you can just do `switch (ID)`. Also instead of doing `isIntrinsic() && getIntrinsicID() == XX` you can simply do `getIntrinsicID() == XX`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155556