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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:865
   VPRecipeWithIRFlags(const unsigned char SC, IterT Operands,
                       Instruction &I)
       : VPRecipeWithIRFlags(SC, Operands) {
----------------
(Nice to see `I` is not set/needed as underlying?)


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:892
            R->getVPDefID() == VPRecipeBase::VPWidenGEPSC ||
            R->getVPDefID() == VPRecipeBase::VPReplicateSC;
   }
----------------
Add VPInstruction?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:387
 
+bool VPInstruction::isFPMathOp() const {
+  return Opcode == Instruction::FAdd || Opcode == Instruction::FMul ||
----------------
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()?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:397
   IRBuilderBase::FastMathFlagGuard FMFGuard(State.Builder);
-  State.Builder.setFastMathFlags(FMF);
+  if (isFPMathOp())
+    State.Builder.setFastMathFlags(getFastMathFlags());
----------------
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()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157144