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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


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


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:972
+    assert(OpType == OperationType::OverflowingBinOp &&
+           "recipe doesn't have a NUW flag");
+    return WrapFlags.HasNSW;
----------------
Ayal wrote:
> 
Adjusted in b6d994de0f1f, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:323
     Value *Step = createStepForVF(Builder, IV->getType(), State.VF, Part);
-    return Builder.CreateAdd(IV, Step, Name, IsNUW, false);
+    return Builder.CreateAdd(IV, Step, Name, hasNoUnsignedWrap(), false);
   }
----------------
Ayal wrote:
> 
Adjusted in b6d994de0f1f, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:247
+    : VPRecipeWithIRFlags(VPDef::VPInstructionSC, Operands, WrapFlags),
+      VPValue(this), Opcode(Opcode), DL(DL), Name(Name.str()) {}
+
----------------
Ayal wrote:
> nit: can be provided inline.
Moved, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:325
           createStepForVF(Builder, Phi->getType(), State.VF, State.UF);
-      return Builder.CreateAdd(Phi, Step, Name, IsNUW, false);
+      return Builder.CreateAdd(Phi, Step, Name, hasNoUnsignedWrap(), false);
     }
----------------
Ayal wrote:
> nit: can replace `false` with hasNoSignedWrap(), here and below.
> 
> Whether this VPInstruction un/sign wraps better be decided upon its construction rather than execution.
> Plus it's clearer.
Updated, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157194