[PATCH] D155556: [GlobalISel] GIntrinsic subclass to represent intrinsics in Generic Machine IR

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.
Herald added a subscriber: pmatos.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h:362
+/// Represents a call to an intrinsic.
+class GIntrinsic : public GenericMachineInstr {
+public:
----------------
Pierre-vh wrote:
> Very nitpicky, but if we're going down the route of using this class more, could we have some `is` method?
> e.g. instead of 
> ```
> cast<GIntrinsic>(MI).getIntrinsicID() == Intrinsic::amdgcn_foo
> ```
> have
> ```
> cast<GIntrinsic>(MI).is(Intrinsic::amdgcn_foo)
> ```
> Also maybe some method like `hasSideEffect` to check the opcode?
> 
> My point is that it's nice to have this type of class, but if we need to fallback to manual checking of the opcode/intrinsic id for any trivial check, this class is just adding verbosity to the code and doesn't add much value, IMO.
final?


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h:364
+public:
+  unsigned getIntrinsicID() const {
+    return getOperand(getNumExplicitDefs()).getIntrinsicID();
----------------
can this be Intrinsic::ID


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155556