[PATCH] D78203: [VP,Integer,#2] ExpandVectorPredication pass

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/IR/PredicatedInst.h:34
+
+class PredicatedInstruction : public User {
+public:
----------------
simoll wrote:
> craig.topper wrote:
> > simoll wrote:
> > > craig.topper wrote:
> > > > Can this inherit from Instruction? Its a little surprising that it has Instruction in its name but has to be casted to Instruction.
> > > Conceptually, `PredicatedInstruction` is a super class of `Instruction`: An `Instruction` is a `PredicatedInstruction` with the mask/evl parameters hard-wired to "all lanes true". That's why we can't make Instruction an ancestor of this one.
> > > I'll clarify this with a comment.
> > But a VPIntrinsic is also an Instruction isn't it? At least the CallInst for the vp intrinsic is an Instruction.
> The whole reason for having the `PredicatedInstruction` class is that we want to add a new perspective on the IR where all instructions may be predicated (through a mask and evl). There are no VPIntrinsics in that perspective.
> 
> In the standard IR perspective, using the `Instruction` type, VPIntrinsics are just regular `CallInst`s calling intrinsics.
I was just wanting to get rid of the need to cast to Instruction in copyIRFlags and getParent and replaceOperation etc. Everything this class represents must be an Instruction whether its predicated or not. No type safety comes from this class inheriting from User instead of Instruction since a CallInst to a VP intrinsic is also an Instruction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78203