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

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 15:28:31 PDT 2023


Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:764-765
                       return true;
                     auto *VPI = cast<VPInstruction>(U);
                     return VPI->getOpcode() ==
+                           VPInstruction::CanonicalIVIncrement;
----------------
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!


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


================
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);
     }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157194



More information about the llvm-commits mailing list