[PATCH] D157194: [VPlan] Model wrap flags directly, remove *NUW opcodes (NFC)

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8788
 
     // Create the active lane mask for the next iteration of the loop.
+    CanonicalIVIncrementParts = VPInstruction::createOverflowingBinOp(
----------------
Ayal wrote:
> nit (independent of this patch): comment above needs fixing.
Will do separately.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:764-765
                       return true;
                     auto *VPI = cast<VPInstruction>(U);
                     return VPI->getOpcode() ==
+                           VPInstruction::CanonicalIVIncrement;
----------------
Ayal wrote:
> nit: can fold into a single
> 
> ```
>   return isa<VPScalarIVStepsRecipe>(U) || isa<VPDerivedIVRecipe>(U)) 
>              || cast<VPInstruction>(U)->getCode() == VPInstruction::CanonicalIVIncrement;
> ```
Will do separately.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:768
                   }) &&
            "the canonical IV should only be used by its increments or "
            "ScalarIVSteps when resetting the start value");
----------------
Ayal wrote:
> 
Adjusted, thanks


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1027
 
+  static VPInstruction *createOverflowingBinOp(unsigned Opcode,
+                                               ArrayRef<VPValue *> Operands,
----------------
Ayal wrote:
> Have a designated constructor that takes wrap flags as parameters and sets them?
Updated! this needed exposing WrapFlagsTy to avoid ambiguous constructor calls (because the default DebugLoc and Twine args can be implicitly converted to bools)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157194