[VPlan] Replace FMF in VPInstruction with VPRecipeWithIRFlags (NFC).

[fhahn]

Summary

Update VPInstruction to use VPRecipeWithIRFlags to manage FMFs for
VPInstruction.

A follow-up patch will also use VPRecipeWithIRFlags to remove the
CanonicalIVIncrementForPartNUW and CanonicalIVIncrementNUW opcodes.

[tschuett]

Thanks!. Have you ever considered to add something like getCalledFunction() when Opode == Instruction::Call to identify vscale?

[fhahn]

In D157144#4562654, @tschuett wrote:

Thanks!. Have you ever considered to add something like getCalledFunction() when Opode == Instruction::Call to identify vscale?

AFAIK vscale isn't currently modeled as VPInstruction yet, but once it does that certainly makes sense, thanks!

[tschuett]

In D157144#4563196, @fhahn wrote:

In D157144#4562654, @tschuett wrote:

Thanks!. Have you ever considered to add something like getCalledFunction() when Opode == Instruction::Call to identify vscale?

AFAIK vscale isn't currently modeled as VPInstruction yet, but once it does that certainly makes sense, thanks!

Given its importance maybe model it as a dedicated derived class of VPInstruction, e.g., VPVscaleInstruction.

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.h

833

Add a constructor and/or operator= from llvm::FastMathFlags?

857

Is this change independent of this patch?

857

Could FMFs be set at construction time, only, similar to other flags?

864

Can this also be simplified into IterT alone?

879

OpType = OperationType::FPMathOp;
FMFs = Op->getFastMathFlags();

?

951–952

Worth documenting. Can be called if OpType is other than FPMathOp, but then returns arbitrary results?

Possibly worth taking out of line to VPlan.cpp

1024

Can this be another constructor rather than static member, one that takes an FMF as another parameter?

1025

Should clone also copy the flags? (Independent of this patch)

[fhahn]

Address latest comments, thanks!

llvm/lib/Transforms/Vectorize/VPlan.h

833

Added, thanks!

857

Yes updated the patch! This requires adding new constructors for both VPRecipeWithIRFlags and VPInstruciton (and the same for each variant (i.e. wrap flags and others), but should be more in line with the existing mechanisms.

857

Split off, will submit separately, thanks!

864

Yes, Split off, will submit separately, thanks!

879

Simplified, thanks!

951–952

Done in 0b17e9d2859a with an assertion added.

1024

Updated to use separate constructor, thanks!

1025

Yes this needs updating, will do separately.

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.h

865

(Nice to see I is not set/needed as underlying?)

892

Add VPInstruction?

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

387

Suffice to assert that Opcode is one having FMF, only when the Opcode and FMF are provided to VPInstruction's constructor, and its OpType is set?

Check inspired by / should match / worth comment FPMathOperator:: classof()?

397

Better have isFPMathOp() check if OpType == OperationType::FPMathOp, as done in getFastMathFlags()'s assert (which it tries to guard)?

(If so) Better have isFPMathOp() in VPRecipeWithIRFlags rather than VPInstruction, to assist potential future callers of VPRecipeWithIRFlags::getFastMathFlags()?

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.h

865

Yes, at the moment VPInstructions are completely independent of any underlying IR.

892

Added in af635a5547ec.

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

387

Added a note about the relation to FPMathOperator::classof() and the current differences. I've kept the function for now, as per my response to the comment below.

397

At the moment, the subclasses of VPRecipeWithIRFlags don't have direct access to its internal fields, including OpTypes and it's the subclasses responsibility to make sure they don't access flags that are not available for the recipe.

I think it would probably be worth keeping things locked down that way for now and check directly in VPInstruction if it is an op that has fast-math flags (we do something similar for the CanonicalIVIncrement. WDYT?

[fhahn]

Address comments, thanks!

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.h

865

(Then perhaps it's setUnderlyingInstr() below can be dropped if unused.)

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

397

Keeping OpTypes and fields private and locked down inside VPRecipeWithIRFlags is fine, suggest to provide a public VPRecipeWithIRFlags::isFPMathOp() { return OpType == OperationType::FPMathOp; } in order to support callers of public VPRecipeWithIRFlags::getFastMathFlags(), efficiently.

Can have a private VPInstruction::isFPMathOp() (or isFPMathOpcode()? Under #ifndef NDEBUG?) to support the assert upon construction only, less efficiently.

Sounds right?

[fhahn]

Add hasFastMathFlags helper

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.h

865

Ah right sorry it is needed for some recipes that use this constructor, just not for VPInstructions which use the different one.

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

397

Added VPRecipeWithIRFlags::hasFastMathFlags() and use it during execute. Also added an assert using isFPMathOp to make sure they don't diverge (the original reason for using isFPMathOp).

[fhahn]

Fix build error, guard isFPMathOp with ifdef as it's only used in assertions.

[Ayal]

Looks good to me! Adding couple of final minor nits.

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

235

nit: redundant empty line

396

nit: this is if and only if, can assert(hasFastMathFlags() == isFPMathOp() && "...").

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

for (VPRecipeBase *CurrentLink : Worklist.getArrayRef().drop_front()) {
                 isa<VPWidenCallRecipe>(CurrentLink)) &&
                CurrentLink->getOperand(2) == PreviousLinkV &&
                "expected a call where the previous link is the added operand");
 
         // If the instruction is a call to the llvm.fmuladd intrinsic then we
         // need to create an fmul recipe (multiplying the first two operands of
         // the fmuladd together) to use as the vector operand for the fadd
         // reduction.
-        VPInstruction *FMulRecipe =
-            new VPInstruction(Instruction::FMul, {CurrentLink->getOperand(0),
-                                                  CurrentLink->getOperand(1)});
-        FMulRecipe->setFastMathFlags(CurrentLinkI->getFastMathFlags());
+        VPInstruction *FMulRecipe = new VPInstruction(
+            Instruction::FMul,
+            {CurrentLink->getOperand(0), CurrentLink->getOperand(1)},
+            CurrentLinkI->getFastMathFlags());
         LinkVPBB->insert(FMulRecipe, CurrentLink->getIterator());
         VecOp = FMulRecipe;
       } else {
         if (RecurrenceDescriptor::isMinMaxRecurrenceKind(Kind)) {
           if (auto *Cmp = dyn_cast<VPWidenRecipe>(CurrentLink)) {
             assert(isa<CmpInst>(CurrentLinkI) &&
                    "need to have the compare of the select");
             continue;

llvm/lib/Transforms/Vectorize/VPlan.h

struct WrapFlagsTy {
     char HasNSW : 1;
   };
   struct ExactFlagsTy {
     char IsExact : 1;
   };
   struct GEPFlagsTy {
     char IsInBounds : 1;
   };
   struct FastMathFlagsTy {
     char AllowReassoc : 1;
     char NoNaNs : 1;
     char NoInfs : 1;
     char NoSignedZeros : 1;
     char AllowReciprocal : 1;
     char AllowContract : 1;
     char ApproxFunc : 1;
+
+    FastMathFlagsTy(const FastMathFlags &FMF);
   };
 
   OperationType OpType;
 
   union {
     WrapFlagsTy WrapFlags;
     ExactFlagsTy ExactFlags;
     GEPFlagsTy GEPFlags;
     FastMathFlagsTy FMFs;
     unsigned char AllFlags;
   };
 
 public:
   template <typename IterT>
   VPRecipeWithIRFlags(const unsigned char SC, IterT Operands)
       : VPRecipeBase(SC, Operands) {
     OpType = OperationType::Other;
     AllFlags = 0;
   }
 
   template <typename IterT>
   VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
                       Instruction &I)
       : VPRecipeWithIRFlags(SC, Operands) {
     if (auto *Op = dyn_cast<OverflowingBinaryOperator>(&I)) {
       OpType = OperationType::OverflowingBinOp;
       WrapFlags.HasNUW = Op->hasNoUnsignedWrap();
       WrapFlags.HasNSW = Op->hasNoSignedWrap();
     } else if (auto *Op = dyn_cast<PossiblyExactOperator>(&I)) {
       OpType = OperationType::PossiblyExactOp;
       ExactFlags.IsExact = Op->isExact();
     } else if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
       OpType = OperationType::GEPOp;
       GEPFlags.IsInBounds = GEP->isInBounds();
     } else if (auto *Op = dyn_cast<FPMathOperator>(&I)) {
       OpType = OperationType::FPMathOp;
-      FastMathFlags FMF = Op->getFastMathFlags();
-      FMFs.AllowReassoc = FMF.allowReassoc();
-      FMFs.NoNaNs = FMF.noNaNs();
-      FMFs.NoInfs = FMF.noInfs();
-      FMFs.NoSignedZeros = FMF.noSignedZeros();
-      FMFs.AllowReciprocal = FMF.allowReciprocal();
-      FMFs.AllowContract = FMF.allowContract();
-      FMFs.ApproxFunc = FMF.approxFunc();
+      FMFs = Op->getFastMathFlags();
     }
   }
 
+  template <typename IterT>
+  VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
+                      FastMathFlags FMFs)
+      : VPRecipeBase(SC, Operands), OpType(OperationType::FPMathOp),
+        FMFs(FMFs) {}
+
   static inline bool classof(const VPRecipeBase *R) {
     return R->getVPDefID() == VPRecipeBase::VPWidenSC ||
            R->getVPDefID() == VPRecipeBase::VPWidenGEPSC ||
            R->getVPDefID() == VPRecipeBase::VPReplicateSC;
   }
 
   /// Drop all poison-generating flags.
   void dropPoisonGeneratingFlags() {
     // NOTE: This needs to be kept in-sync with
     // Instruction::dropPoisonGeneratingFlags.
     switch (OpType) {
     case OperationType::OverflowingBinOp:
void setFlags(Instruction *I) const {
     }
   }
 
   bool isInBounds() const {
     assert(OpType == OperationType::GEPOp &&
            "recipe doesn't have inbounds flag");
     return GEPFlags.IsInBounds;
   }
 
   FastMathFlags getFastMathFlags() const;
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   void printFlags(raw_ostream &O) const;
 #endif
 };
 
 /// This is a concrete Recipe that models a single VPlan-level instruction.
 /// While as any Recipe it may generate a sequence of IR instructions when
 /// executed, these instructions would always form a single-def expression as
 /// the VPInstruction is also a single def-use vertex.
-class VPInstruction : public VPRecipeBase, public VPValue {
+class VPInstruction : public VPRecipeWithIRFlags, public VPValue {
   friend class VPlanSlp;
 
 public:
   /// VPlan opcodes, extending LLVM IR with idiomatics instructions.
   enum {
     FirstOrderRecurrenceSplice =
         Instruction::OtherOpsEnd + 1, // Combines the incoming and previous
                                       // values of a first-order recurrence.
enum {
     CanonicalIVIncrementForPartNUW,
     BranchOnCount,
     BranchOnCond
   };
 
 private:
   typedef unsigned char OpcodeTy;
   OpcodeTy Opcode;
-  FastMathFlags FMF;
   DebugLoc DL;
 
   /// An optional name that can be used for the generated IR instruction.
   const std::string Name;
 
   /// Utility method serving execute(): generates a single instance of the
   /// modeled instruction. \returns the generated value for \p Part.
   /// In some cases an existing value is returned rather than a generated
   /// one.
   Value *generateInstruction(VPTransformState &State, unsigned Part);
 
+  /// Return true if the VPInstruction is a floating point math operation, i.e.
+  /// has fast-math flags.
+  bool isFPMathOp() const;
+
 protected:
   void setUnderlyingInstr(Instruction *I) { setUnderlyingValue(I); }
 
 public:
   VPInstruction(unsigned Opcode, ArrayRef<VPValue *> Operands, DebugLoc DL,
                 const Twine &Name = "")
-      : VPRecipeBase(VPDef::VPInstructionSC, Operands), VPValue(this),
+      : VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands), VPValue(this),
         Opcode(Opcode), DL(DL), Name(Name.str()) {}
 
   VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands,
                 DebugLoc DL = {}, const Twine &Name = "")
       : VPInstruction(Opcode, ArrayRef<VPValue *>(Operands), DL, Name) {}
 
+  VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands,
+                FastMathFlags FMFs, DebugLoc DL = {}, const Twine &Name = "");
+
   VP_CLASSOF_IMPL(VPDef::VPInstructionSC)
 
   VPInstruction *clone() const {
     SmallVector<VPValue *, 2> Operands(operands());
     return new VPInstruction(Opcode, Operands, DL, Name);
   }
 
   unsigned getOpcode() const { return Opcode; }
 
   /// Generate the instruction.
   /// TODO: We currently execute only per-part unless a specific instance is
   /// provided.
bool hasResult() const {
     case VPInstruction::BranchOnCond:
     case VPInstruction::BranchOnCount:
       return false;
     default:
       return true;
     }
   }
 
-  /// Set the fast-math flags.
-  void setFastMathFlags(FastMathFlags FMFNew);
-
   /// Returns true if the recipe only uses the first lane of operand \p Op.
   bool onlyFirstLaneUsed(const VPValue *Op) const override {
     assert(is_contained(operands(), Op) &&
            "Op must be an operand of the recipe");
     if (getOperand(0) != Op)
       return false;
     switch (getOpcode()) {
     default:

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp

FastMathFlags VPRecipeWithIRFlags::getFastMathFlags() const {
   Res.setNoInfs(FMFs.NoInfs);
   Res.setNoSignedZeros(FMFs.NoSignedZeros);
   Res.setAllowReciprocal(FMFs.AllowReciprocal);
   Res.setAllowContract(FMFs.AllowContract);
   Res.setApproxFunc(FMFs.ApproxFunc);
   return Res;
 }
 
+VPInstruction::VPInstruction(unsigned Opcode,
+                             std::initializer_list<VPValue *> Operands,
+                             FastMathFlags FMFs, DebugLoc DL, const Twine &Name)
+    : VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, FMFs),
+      VPValue(this), Opcode(Opcode), DL(DL), Name(Name.str()) {
+  // Make sure the VPInstruction is a floating-point operation.
+  assert(isFPMathOp() && "this op can't take fast-math flags");
+}
+
 Value *VPInstruction::generateInstruction(VPTransformState &State,
                                           unsigned Part) {
   IRBuilderBase &Builder = State.Builder;
   Builder.SetCurrentDebugLocation(DL);
 
   if (Instruction::isBinaryOp(getOpcode())) {
     Value *A = State.get(getOperand(0), Part);
     Value *B = State.get(getOperand(1), Part);
case VPInstruction::BranchOnCount: {
     Builder.GetInsertBlock()->getTerminator()->eraseFromParent();
     return CondBr;
   }
   default:
     llvm_unreachable("Unsupported opcode for instruction");
   }
 }
 
+bool VPInstruction::isFPMathOp() const {
+  return Opcode == Instruction::FAdd || Opcode == Instruction::FMul ||
+         Opcode == Instruction::FNeg || Opcode == Instruction::FSub ||
+         Opcode == Instruction::FDiv || Opcode == Instruction::FRem ||
+         Opcode == Instruction::FCmp;
+}
+
 void VPInstruction::execute(VPTransformState &State) {
   assert(!State.Instance && "VPInstruction executing an Instance");
   IRBuilderBase::FastMathFlagGuard FMFGuard(State.Builder);
-  State.Builder.setFastMathFlags(FMF);
+  if (isFPMathOp())
+    State.Builder.setFastMathFlags(getFastMathFlags());
   for (unsigned Part = 0; Part < State.UF; ++Part) {
     Value *GeneratedValue = generateInstruction(State, Part);
     if (!hasResult())
       continue;
     assert(GeneratedValue && "generateInstruction must produce a value");
     State.set(this, GeneratedValue, Part);
   }
 }
case VPInstruction::CanonicalIVIncrementForPartNUW:
     break;
   case VPInstruction::BranchOnCount:
     O << "branch-on-count ";
     break;
   default:
     O << Instruction::getOpcodeName(getOpcode());
   }
 
-  O << FMF;
+  printFlags(O);
 
   for (const VPValue *Operand : operands()) {
     O << " ";
     Operand->printAsOperand(O, SlotTracker);
   }
 
   if (DL) {
     O << ", !dbg ";
     DL.print(O);
   }
 }
 #endif
 
-void VPInstruction::setFastMathFlags(FastMathFlags FMFNew) {
-  // Make sure the VPInstruction is a floating-point operation.
-  assert((Opcode == Instruction::FAdd || Opcode == Instruction::FMul ||
-          Opcode == Instruction::FNeg || Opcode == Instruction::FSub ||
-          Opcode == Instruction::FDiv || Opcode == Instruction::FRem ||
-          Opcode == Instruction::FCmp) &&
-         "this op can't take fast-math flags");
-  FMF = FMFNew;
-}
-
 void VPWidenCallRecipe::execute(VPTransformState &State) {
   assert(State.VF.isVector() && "not widening");
   auto &CI = *cast<CallInst>(getUnderlyingInstr());
   assert(!isa<DbgInfoIntrinsic>(CI) &&
          "DbgInfoIntrinsic should have been dropped during VPlan construction");
   State.setDebugLocFromInst(&CI);
 
   for (unsigned Part = 0; Part < State.UF; ++Part) {
for (unsigned Part = 0; Part < State.UF; ++Part) {
     Value *Op0 = State.get(getOperand(1), Part);
     Value *Op1 = State.get(getOperand(2), Part);
     Value *Sel = State.Builder.CreateSelect(Cond, Op0, Op1);
     State.set(this, Sel, Part);
     State.addMetadata(Sel, &I);
   }
 }
 
+VPRecipeWithIRFlags::FastMathFlagsTy::FastMathFlagsTy(
+    const FastMathFlags &FMF) {
+  AllowReassoc = FMF.allowReassoc();
+  NoNaNs = FMF.noNaNs();
+  NoInfs = FMF.noInfs();
+  NoSignedZeros = FMF.noSignedZeros();
+  AllowReciprocal = FMF.allowReciprocal();
+  AllowContract = FMF.allowContract();
+  ApproxFunc = FMF.approxFunc();
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 void VPRecipeWithIRFlags::printFlags(raw_ostream &O) const {
   switch (OpType) {
   case OperationType::PossiblyExactOp:
     if (ExactFlags.IsExact)
       O << " exact";
     break;
   case OperationType::OverflowingBinOp: