Address review

Author: SamTebbs33

llvm/include/llvm/CodeGen/BasicTTIImpl.h

@@ -2190,53 +2190,6 @@ class BasicTTIImplBase : public TargetTransformInfoImplCRTPBase<T> {
       // Otherwise, fallback to default scalarization cost.
       break;
     }
-    case Intrinsic::loop_dependence_raw_mask:
-    case Intrinsic::loop_dependence_war_mask: {
-      InstructionCost Cost = 0;
-      Type *PtrTy = ICA.getArgTypes()[0];
-      bool IsReadAfterWrite = IID == Intrinsic::loop_dependence_raw_mask;
-
-      Cost +=
-          thisT()->getArithmeticInstrCost(Instruction::Sub, PtrTy, CostKind);
-      if (IsReadAfterWrite) {
-        IntrinsicCostAttributes AbsAttrs(Intrinsic::abs, PtrTy, {PtrTy}, {});
-        Cost += thisT()->getIntrinsicInstrCost(AbsAttrs, CostKind);
-      }
-
-      Cost +=
-          thisT()->getArithmeticInstrCost(Instruction::SDiv, PtrTy, CostKind);
-      Type *CmpTy =
-          getTLI()
-              ->getSetCCResultType(
-                  thisT()->getDataLayout(), RetTy->getContext(),
-                  getTLI()->getValueType(thisT()->getDataLayout(), PtrTy))
-              .getTypeForEVT(RetTy->getContext());
-      Cost += thisT()->getCmpSelInstrCost(
-          BinaryOperator::ICmp, CmpTy, PtrTy,
-          IsReadAfterWrite ? CmpInst::ICMP_EQ : CmpInst::ICMP_SLE, CostKind);
-
-      // The deconstructed active lane mask
-      VectorType *RetTyVec = cast<VectorType>(RetTy);
-      VectorType *SplatTy = cast<VectorType>(RetTyVec->getWithNewType(PtrTy));
-      Cost += thisT()->getShuffleCost(TTI::SK_Broadcast, SplatTy, SplatTy, {},
-                                      CostKind, 0, nullptr);
-      IntrinsicCostAttributes StepVecAttrs(Intrinsic::stepvector, SplatTy, {},
-                                           FMF);
-      Cost += thisT()->getIntrinsicInstrCost(StepVecAttrs, CostKind);
-      Cost += thisT()->getCmpSelInstrCost(BinaryOperator::ICmp, SplatTy,
-                                          SplatTy, CmpInst::ICMP_ULT, CostKind);
-
-      Cost +=
-          thisT()->getCastInstrCost(Instruction::CastOps::ZExt, RetTy, SplatTy,
-                                    TTI::CastContextHint::None, CostKind);
-      Cost += thisT()->getCastInstrCost(Instruction::CastOps::ZExt,
-                                        RetTyVec->getElementType(), CmpTy,
-                                        TTI::CastContextHint::None, CostKind);
-      Cost += thisT()->getShuffleCost(TTI::SK_Broadcast, RetTyVec, RetTyVec, {},
-                                      CostKind, 0, nullptr);
-      Cost += thisT()->getArithmeticInstrCost(Instruction::Or, RetTy, CostKind);
-      return Cost;
-    }
     }
 
     // Assume that we need to scalarize this intrinsic.)

llvm/include/llvm/Transforms/Utils/LoopUtils.h

@@ -614,11 +614,6 @@ addRuntimeChecks(Instruction *Loc, Loop *TheLoop,
                  const SmallVectorImpl<RuntimePointerCheck> &PointerChecks,
                  SCEVExpander &Expander, bool HoistRuntimeChecks = false);
 
-LLVM_ABI Value *addSafeEltsRuntimeChecks(Instruction *Loc,
-                                         ArrayRef<PointerDiffInfo> Checks,
-                                         SCEVExpander &Expander,
-                                         ElementCount VF);
-
 LLVM_ABI Value *addDiffRuntimeChecks(
     Instruction *Loc, ArrayRef<PointerDiffInfo> Checks, SCEVExpander &Expander,
     function_ref<Value *(IRBuilderBase &, unsigned)> GetVF, unsigned IC);

llvm/lib/Transforms/Utils/LoopUtils.cpp

@@ -2135,58 +2135,6 @@ Value *llvm::addRuntimeChecks(
   return MemoryRuntimeCheck;
 }
 
-Value *llvm::addSafeEltsRuntimeChecks(Instruction *Loc,
-                                      ArrayRef<PointerDiffInfo> Checks,
-                                      SCEVExpander &Expander, ElementCount VF) {
-  IRBuilder ChkBuilder(Loc->getContext(),
-                       InstSimplifyFolder(Loc->getDataLayout()));
-  ChkBuilder.SetInsertPoint(Loc);
-  Value *MemoryRuntimeCheck = nullptr;
-
-  // Map to keep track of created compares, The key is the pair of operands for
-  // the compare, to allow detecting and re-using redundant compares.
-  DenseMap<std::pair<Value *, Value *>, Value *> SeenCompares;
-  Value *AliasLaneMask = nullptr;
-  for (const auto &[SrcStart, SinkStart, AccessSize, NeedsFreeze,
-                    WriteAfterRead] : Checks) {
-    Type *Ty = SinkStart->getType();
-    Value *Sink = Expander.expandCodeFor(SinkStart, Ty, Loc);
-    Value *Src = Expander.expandCodeFor(SrcStart, Ty, Loc);
-    if (SeenCompares.lookup({Sink, Src}))
-      continue;
-
-    unsigned IntOpc = WriteAfterRead ? Intrinsic::loop_dependence_war_mask
-                                     : Intrinsic::loop_dependence_raw_mask;
-    Value *SourceAsPtr = ChkBuilder.CreateCast(Instruction::IntToPtr, Src,
-                                               ChkBuilder.getPtrTy());
-    Value *SinkAsPtr = ChkBuilder.CreateCast(Instruction::IntToPtr, Sink,
-                                             ChkBuilder.getPtrTy());
-    Value *M = ChkBuilder.CreateIntrinsic(
-        IntOpc, {VectorType::get(ChkBuilder.getInt1Ty(), VF)},
-        {SourceAsPtr, SinkAsPtr, ChkBuilder.getInt64(AccessSize)}, nullptr,
-        "alias.lane.mask");
-    SeenCompares.insert({{Sink, Src}, M});
-    if (AliasLaneMask)
-      M = ChkBuilder.CreateAnd(AliasLaneMask, M);
-    else
-      AliasLaneMask = M;
-  }
-  assert(AliasLaneMask && "Expected an alias lane mask to have been created.");
-  auto *VecVT = VectorType::get(ChkBuilder.getInt1Ty(), VF);
-  // Extend to an i8 since i1 is too small to add with
-  Value *PopCount = ChkBuilder.CreateCast(
-      Instruction::ZExt, AliasLaneMask,
-      VectorType::get(ChkBuilder.getInt8Ty(), VecVT->getElementCount()));
-
-  PopCount =
-      ChkBuilder.CreateUnaryIntrinsic(Intrinsic::vector_reduce_add, PopCount);
-  PopCount = ChkBuilder.CreateCast(Instruction::ZExt, PopCount,
-                                   ChkBuilder.getInt64Ty());
-  MemoryRuntimeCheck = ChkBuilder.CreateICmpUGT(
-      PopCount, ConstantInt::get(ChkBuilder.getInt64Ty(), 0));
-  return MemoryRuntimeCheck;
-}
-
 Value *llvm::addDiffRuntimeChecks(
     Instruction *Loc, ArrayRef<PointerDiffInfo> Checks, SCEVExpander &Expander,
     function_ref<Value *(IRBuilderBase &, unsigned)> GetVF, unsigned IC) {
@@ -2201,8 +2149,7 @@ Value *llvm::addDiffRuntimeChecks(
   // Map to keep track of created compares, The key is the pair of operands for
   // the compare, to allow detecting and re-using redundant compares.
   DenseMap<std::pair<Value *, Value *>, Value *> SeenCompares;
-  for (const auto &[SrcStart, SinkStart, AccessSize, NeedsFreeze,
-                    WriteAfterRead] : Checks) {
+  for (const auto &[SrcStart, SinkStart, AccessSize, NeedsFreeze, _] : Checks) {
     Type *Ty = SinkStart->getType();
     // Compute VF * IC * AccessSize.
     auto *VFTimesICTimesSize =
@@ -2211,8 +2158,8 @@ Value *llvm::addDiffRuntimeChecks(
     Value *Diff =
         Expander.expandCodeFor(SE.getMinusSCEV(SinkStart, SrcStart), Ty, Loc);
 
-    // Check if the same compare has already been created earlier. In that
-    // case, there is no need to check it again.
+    // Check if the same compare has already been created earlier. In that case,
+    // there is no need to check it again.
     Value *IsConflict = SeenCompares.lookup({Diff, VFTimesICTimesSize});
     if (IsConflict)
       continue;

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -1334,23 +1334,10 @@ class LoopVectorizationCostModel {
                                : ChosenTailFoldingStyle->second;
   }
 
-  RTCheckStyle getRTCheckStyle(TailFoldingStyle TFStyle,
-                               const TargetTransformInfo &TTI) const {
-    if (!TTI.useSafeEltsMask())
-      return RTCheckStyle::ScalarDifference;
-
-    switch (TFStyle) {
-    case TailFoldingStyle::Data:
-    case TailFoldingStyle::DataAndControlFlow:
-    case TailFoldingStyle::DataAndControlFlowWithoutRuntimeCheck:
-      return RTCheckStyle::UseSafeEltsMask;
-    default:
-      return RTCheckStyle::ScalarDifference;
-    }
-  }
-
   RTCheckStyle getRTCheckStyle(const TargetTransformInfo &TTI) const {
-    return getRTCheckStyle(getTailFoldingStyle(), TTI);
+    if (TTI.useSafeEltsMask())
+      return RTCheckStyle::UseSafeEltsMask;
+    return RTCheckStyle::ScalarDifference;
   }
 
   /// Selects and saves TailFoldingStyle for 2 options - if IV update may
@@ -1883,10 +1870,7 @@ class GeneratedRTChecks {
                                  "vector.memcheck");
 
       auto DiffChecks = RtPtrChecking.getDiffChecks();
-      if (UseSafeEltsMask) {
-        MemRuntimeCheckCond = addSafeEltsRuntimeChecks(
-            MemCheckBlock->getTerminator(), *DiffChecks, MemCheckExp, VF);
-      } else if (DiffChecks) {
+      if (DiffChecks) {
         Value *RuntimeVF = nullptr;
         MemRuntimeCheckCond = addDiffRuntimeChecks(
             MemCheckBlock->getTerminator(), *DiffChecks, MemCheckExp,
@@ -8585,7 +8569,7 @@ VPlanPtr LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(
     std::optional<ArrayRef<PointerDiffInfo>> RTChecks =
         CM.Legal->getRuntimePointerChecking()->getDiffChecks();
     if (RTChecks.has_value() &&
-        CM.getRTCheckStyle(Style, TTI) == RTCheckStyle::UseSafeEltsMask)
+        CM.getRTCheckStyle(TTI) == RTCheckStyle::UseSafeEltsMask)
       DiffChecks = *RTChecks;
 
     VPlanTransforms::addActiveLaneMask(*Plan, ForControlFlow,
@@ -9008,11 +8992,91 @@ void LoopVectorizationPlanner::attachRuntimeChecks(
     assert((!CM.OptForSize ||
             CM.Hints->getForce() == LoopVectorizeHints::FK_Enabled) &&
            "Cannot SCEV check stride or overflow when optimizing for size");
-    VPlanTransforms::attachCheckBlock(Plan, SCEVCheckCond, SCEVCheckBlock,
+    VPlanTransforms::attachCheckBlock(Plan, Plan.getOrAddLiveIn(SCEVCheckCond),
+                                      Plan.createVPIRBasicBlock(SCEVCheckBlock),
                                       HasBranchWeights);
   }
   const auto &[MemCheckCond, MemCheckBlock] = RTChecks.getMemRuntimeChecks();
   if (MemCheckBlock && MemCheckBlock->hasNPredecessors(0)) {
+    VPValue *MemCheckCondVPV = Plan.getOrAddLiveIn(MemCheckCond);
+    VPBasicBlock *MemCheckBlockVP = Plan.createVPIRBasicBlock(MemCheckBlock);
+    std::optional<ArrayRef<PointerDiffInfo>> ChecksOpt =
+        CM.Legal->getRuntimePointerChecking()->getDiffChecks();
+
+    // Create a mask enabling safe elements for each iteration.
+    if (CM.getRTCheckStyle(TTI) == RTCheckStyle::UseSafeEltsMask &&
+        ChecksOpt.has_value() && ChecksOpt->size() > 0) {
+      ArrayRef<PointerDiffInfo> Checks = *ChecksOpt;
+      VPRegionBlock *LoopRegion = Plan.getVectorLoopRegion();
+      VPBasicBlock *LoopBody = LoopRegion->getEntryBasicBlock();
+      VPBuilder Builder(MemCheckBlockVP);
+
+      /// Create a mask for each possibly-aliasing pointer pair, ANDing them if
+      /// there's more than one pair.
+      VPValue *AliasMask = nullptr;
+      for (PointerDiffInfo Check : Checks) {
+        VPValue *Sink =
+            vputils::getOrCreateVPValueForSCEVExpr(Plan, Check.SinkStart);
+        VPValue *Src =
+            vputils::getOrCreateVPValueForSCEVExpr(Plan, Check.SrcStart);
+        VPAliasLaneMaskRecipe *M = new VPAliasLaneMaskRecipe(
+            Src, Sink, Check.AccessSize, Check.WriteAfterRead);
+        MemCheckBlockVP->appendRecipe(M);
+        if (AliasMask)
+          AliasMask = Builder.createAnd(AliasMask, M);
+        else
+          AliasMask = M;
+      }
+      assert(AliasMask && "Expected an alias mask to have been created");
+
+      // Replace uses of the loop body's active lane mask phi with an AND of the
+      // phi and the alias mask.
+      for (VPRecipeBase &R : *LoopBody) {
+        auto *MaskPhi = dyn_cast<VPActiveLaneMaskPHIRecipe>(&R);
+        if (!MaskPhi)
+          continue;
+        VPInstruction *And = new VPInstruction(Instruction::BinaryOps::And,
+                                               {MaskPhi, AliasMask});
+        MaskPhi->replaceUsesWithIf(And, [And](VPUser &U, unsigned) {
+          auto *UR = dyn_cast<VPRecipeBase>(&U);
+          // If this is the first user, instert the AND.
+          if (UR && !And->getParent())
+            And->insertBefore(UR);
+          bool Replace = UR != And;
+          return Replace;
+        });
+      }
+
+      // An empty mask would cause an infinite loop since the induction variable
+      // is updated with the number of set elements in the mask. Make sure we
+      // don't execute the vector loop when the mask is empty.
+      VPInstruction *PopCount =
+          new VPInstruction(VPInstruction::PopCount, {AliasMask});
+      PopCount->insertAfter(AliasMask->getDefiningRecipe());
+      VPValue *Cmp =
+          Builder.createICmp(CmpInst::Predicate::ICMP_EQ, PopCount,
+                             Plan.getOrAddLiveIn(ConstantInt::get(
+                                 IntegerType::get(Plan.getContext(), 64), 0)));
+      MemCheckCondVPV = Cmp;
+
+      // Update the IV by the number of active lanes in the mask.
+      auto *CanonicalIVPHI = LoopRegion->getCanonicalIV();
+      auto *CanonicalIVIncrement =
+          cast<VPInstruction>(CanonicalIVPHI->getBackedgeValue());
+
+      // Increment phi by correct amount.
+      VPValue *IncrementBy = PopCount;
+      Type *IVType = CanonicalIVPHI->getScalarType();
+
+      if (IVType->getScalarSizeInBits() < 64) {
+        Builder.setInsertPoint(CanonicalIVIncrement);
+        IncrementBy =
+            Builder.createScalarCast(Instruction::Trunc, IncrementBy, IVType,
+                                     CanonicalIVIncrement->getDebugLoc());
+      }
+      CanonicalIVIncrement->setOperand(1, IncrementBy);
+    }
+
     // VPlan-native path does not do any analysis for runtime checks
     // currently.
     assert((!EnableVPlanNativePath || OrigLoop->isInnermost()) &&
@@ -9033,7 +9097,7 @@ void LoopVectorizationPlanner::attachRuntimeChecks(
                   "(e.g., adding 'restrict').";
       });
     }
-    VPlanTransforms::attachCheckBlock(Plan, MemCheckCond, MemCheckBlock,
+    VPlanTransforms::attachCheckBlock(Plan, MemCheckCondVPV, MemCheckBlockVP,
                                       HasBranchWeights);
   }
 }
@@ -10001,9 +10065,8 @@ bool LoopVectorizePass::processLoop(Loop *L) {
     // Optimistically generate runtime checks if they are needed. Drop them if
     //  they turn out to not be profitable.
     if (VF.Width.isVector() || SelectedIC > 1) {
-      TailFoldingStyle TFStyle = CM.getTailFoldingStyle();
       bool UseSafeEltsMask =
-          CM.getRTCheckStyle(TFStyle, *TTI) == RTCheckStyle::UseSafeEltsMask;
+          CM.getRTCheckStyle(*TTI) == RTCheckStyle::UseSafeEltsMask;
       if (UseSafeEltsMask)
         LoopsAliasMasked++;
       Checks.create(L, *LVL.getLAI(), PSE.getPredicate(), VF.Width, SelectedIC,

llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp

@@ -640,11 +640,9 @@ void VPlanTransforms::createLoopRegions(VPlan &Plan) {
 // including memory overlap checks block and wrapping/unit-stride checks block.
 static constexpr uint32_t CheckBypassWeights[] = {1, 127};
 
-void VPlanTransforms::attachCheckBlock(VPlan &Plan, Value *Cond,
-                                       BasicBlock *CheckBlock,
+void VPlanTransforms::attachCheckBlock(VPlan &Plan, VPValue *CondVPV,
+                                       VPBasicBlock *CheckBlockVPBB,
                                        bool AddBranchWeights) {
-  VPValue *CondVPV = Plan.getOrAddLiveIn(Cond);
-  VPBasicBlock *CheckBlockVPBB = Plan.createVPIRBasicBlock(CheckBlock);
   VPBlockBase *VectorPH = Plan.getVectorPreheader();
   VPBlockBase *ScalarPH = Plan.getScalarPreheader();
   VPBlockBase *PreVectorPH = VectorPH->getSinglePredecessor();

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

@@ -2409,22 +2409,6 @@ static VPValue *addVPLaneMaskPhiAndUpdateExitBranch(
   auto *VecPreheader = Plan.getVectorPreheader();
   VPBuilder Builder(VecPreheader);
 
-  // Create an alias mask for each possibly-aliasing pointer pair. If there
-  // are multiple they are combined together with ANDs.
-  VPValue *AliasMask = nullptr;
-
-  for (auto C : RTChecks) {
-    VPValue *Sink = vputils::getOrCreateVPValueForSCEVExpr(Plan, C.SinkStart);
-    VPValue *Src = vputils::getOrCreateVPValueForSCEVExpr(Plan, C.SrcStart);
-    VPAliasLaneMaskRecipe *M =
-        new VPAliasLaneMaskRecipe(Src, Sink, C.AccessSize, C.WriteAfterRead);
-    VecPreheader->appendRecipe(M);
-    if (AliasMask)
-      AliasMask = Builder.createAnd(AliasMask, M);
-    else
-      AliasMask = M;
-  }
-
   // Create the ActiveLaneMask instruction using the correct start values.
   VPValue *TC = Plan.getTripCount();
 
@@ -2451,38 +2435,15 @@ static VPValue *addVPLaneMaskPhiAndUpdateExitBranch(
   VPValue *ALMMultiplier =
       Plan.getConstantInt(TopRegion->getCanonicalIVType(), 1);
   auto *Mask = Builder.createNaryOp(VPInstruction::ActiveLaneMask,
-                                        {EntryIncrement, TC, ALMMultiplier}, DL,
-                                        "active.lane.mask.entry");
+                                    {EntryIncrement, TC, ALMMultiplier}, DL,
+                                    "active.lane.mask.entry");
 
   // Now create the ActiveLaneMaskPhi recipe in the main loop using the
   // preheader ActiveLaneMask instruction.
   auto *LaneMaskPhi =
       new VPActiveLaneMaskPHIRecipe(Mask, DebugLoc::getUnknown());
   LaneMaskPhi->insertAfter(CanonicalIVPHI);
   VPValue *LaneMask = LaneMaskPhi;
-  if (AliasMask) {
-    VPBuilder CountBuilder =
-        VPBuilder::getToInsertAfter(AliasMask->getDefiningRecipe());
-    VPValue *PopCount = CountBuilder.createNaryOp(VPInstruction::PopCount,
-                                                  {AliasMask}, DL, "popcount");
-    // Increment phi by correct amount.
-    Builder.setInsertPoint(CanonicalIVIncrement);
-
-    VPValue *IncrementBy = PopCount;
-    Type *IVType = CanonicalIVPHI->getScalarType();
-
-    if (IVType->getScalarSizeInBits() < 64)
-      IncrementBy =
-          Builder.createScalarCast(Instruction::Trunc, IncrementBy, IVType,
-                                   CanonicalIVIncrement->getDebugLoc());
-    CanonicalIVIncrement->setOperand(1, IncrementBy);
-
-    // And the alias mask so the iteration only processes non-aliasing lanes
-    Builder.setInsertPoint(CanonicalIVPHI->getParent(),
-                           CanonicalIVPHI->getParent()->getFirstNonPhi());
-    LaneMask = Builder.createNaryOp(Instruction::BinaryOps::And,
-                                    {LaneMaskPhi, AliasMask}, DL);
-  }
 
   // Create the active lane mask for the next iteration of the loop before the
   // original terminator.

llvm/lib/Transforms/Vectorize/VPlanTransforms.h

@@ -129,10 +129,10 @@ struct VPlanTransforms {
   /// flat CFG into a hierarchical CFG.
   LLVM_ABI_FOR_TEST static void createLoopRegions(VPlan &Plan);
 
-  /// Wrap runtime check block \p CheckBlock in a VPIRBB and \p Cond in a
-  /// VPValue and connect the block to \p Plan, using the VPValue as branch
+  /// Connect \p CheckBlockVPBB to \p Plan, using the \p CondVPV as branch
   /// condition.
-  static void attachCheckBlock(VPlan &Plan, Value *Cond, BasicBlock *CheckBlock,
+  static void attachCheckBlock(VPlan &Plan, VPValue *CondVPV,
+                               VPBasicBlock *CheckBlockVPBB,
                                bool AddBranchWeights);
 
   /// Replaces the VPInstructions in \p Plan with corresponding