[PATCH] D105751: GlobalISel: Introduce GenericMachineInstr classes and derivatives for idiomatic LLVM RTTI.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GenericMachineInstrs.h:24
+/// A base class for all GenericMachineInstrs.
+class GenericMachineInstr : public MachineInstr {
+public:
----------------
arsenm wrote:
> aemerson wrote:
> > arsenm wrote:
> > > Can we get away with allocating these as regular MachineInstrs since there are no real members?
> > Not sure what you mean. Have the others inherit directly from MachineInstr? If so we wouldn't be able to declare GISel helpers like `getReg()`, unless we add those to MachineInstr. Even then, it would be useful to have this to enforce more type safety in future for places which only expect to deal with generic instructions.
> I mean is it legal C++ to do:
> 
> X = new MachineInstr()
> Y = cast<GenericMachineInstr>(X)
> 
> with guaranteed sizeof(*X) == sizeof(*Y), or would we need to allocate these initially as GenericMachineInstr?
Yes that's legal as long as we don't store any data in GenericMachineInstr or its derivatives. I think the same mechanism is already used in the IR, e.g. `IntrinsicInst` is a convenience class around `CallInst` that just provides `classof` and other helpers, like we're doing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105751