[PGO][PGSO][LV] Fix loop not vectorized issue under profile guided size opts.

[hjyamauchi]

Summary

D81345 appears to accidentally disables vectorization when explicitly
enabled. As PGSO isn't currently accessible from LoopAccessInfo, revert back to
the vectorization with versioning-for-unit-stride for PGSO.

[davidxl]

If shouldOptimizeForSize is not available for loop analysis, should the check of be removed unconditionally instead of gated upon whether there is a hint?

[hjyamauchi]

In D85784#2211641, @davidxl wrote:

If shouldOptimizeForSize is not available for loop analysis, should the check of be removed unconditionally instead of gated upon whether there is a hint?

Which check are you referring to to be removed? Can you express in terms of the code?

[davidxl]

I mean the check llvm::shouldOptimizeForSize(L->getHeader(), PSI, BFI,

PGSOQueryType::IRPass) on loop header.

[hjyamauchi]

In D85784#2211641, @davidxl wrote:

If shouldOptimizeForSize is not available for loop analysis, should the check of be removed unconditionally instead of gated upon whether there is a hint?

I think no, as it'd be an unintended removal of this piece of PGSO, which can be avoided by this patch.

Here's a longer description of my understanding.

D81345 changed the vectorization scheme for -Os from "vectorization with versioning" to "vectorization without versioning" (saving some code size). It didn't change the fact that the explicit vectorization (via the pragma) takes precedence over the -Os flag (ie. still vectorize with pragma even with -Os.) But as it moved the size opt check (hasOptSize) from LoopVectorize.cpp to LoopAccessAnalysis.cpp (by not collecting stride accesses for hasOptSize cases earlier in LoopAccessAnalysis), but does not do similar for the PGSO cases (which is not easy), it inadvertently changed it so that the explicit vectorization does no longer take precedence over PGSO (as it removes the "Hints.getForce() != LoopVectorizeHints::FK_Enabled" check.) This is the unintended behavior change that's manifested as (internal) b/163165125.

Since shouldOptimizeForSize is not available for loop analysis (due to the stated reasons), this patch simply recovers the old behavior just under PGSO, which is "vectorize with versioning" if explicitly enabled, rather than "not vectorize" (which is unexpected), while keeping the new behavior for -Os.

[davidxl]

lgtm

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

void InnerLoopVectorizer::emitSCEVChecks(Loop *L, BasicBlock *Bypass) {
   Value *SCEVCheck = Exp.expandCodeForPredicate(
       &PSE.getUnionPredicate(), SCEVCheckBlock->getTerminator());
 
   if (auto *C = dyn_cast<ConstantInt>(SCEVCheck))
     if (C->isZero())
       return;
 
   assert(!(SCEVCheckBlock->getParent()->hasOptSize() ||
-           OptForSizeBasedOnProfile) &&
+           (OptForSizeBasedOnProfile &&
+            Cost->Hints->getForce() != LoopVectorizeHints::FK_Enabled)) &&
          "Cannot SCEV check stride or overflow when optimizing for size");
 
   SCEVCheckBlock->setName("vector.scevcheck");
   // Create new preheader for vector loop.
   LoopVectorPreHeader =
       SplitBlock(SCEVCheckBlock, SCEVCheckBlock->getTerminator(), DT, LI,
                  nullptr, "vector.ph");
 

 // for minimum code-size, 2) predicate compiler options, 3) loop hints forcing
 // predication, and 4) a TTI hook that analyses whether the loop is suitable
 // for predication.
 static ScalarEpilogueLowering getScalarEpilogueLowering(
     Function *F, Loop *L, LoopVectorizeHints &Hints, ProfileSummaryInfo *PSI,
     BlockFrequencyInfo *BFI, TargetTransformInfo *TTI, TargetLibraryInfo *TLI,
     AssumptionCache *AC, LoopInfo *LI, ScalarEvolution *SE, DominatorTree *DT,
     LoopVectorizationLegality &LVL) {
-  bool OptSize =
-      F->hasOptSize() || llvm::shouldOptimizeForSize(L->getHeader(), PSI, BFI,
-                                                     PGSOQueryType::IRPass);
   // 1) OptSize takes precedence over all other options, i.e. if this is set,
   // don't look at hints or options, and don't request a scalar epilogue.
-  if (OptSize)
+  // (For PGSO, as shouldOptimizeForSize isn't currently accessible from
+  // LoopAccessInfo (due to code dependency and not being able to reliably get
+  // PSI/BFI from a loop analysis under NPM), we cannot suppress the collection
+  // of strides in LoopAccessInfo::analyzeLoop() and vectorize without
+  // versioning when the vectorization is forced, unlike hasOptSize. So revert
+  // back to the old way and vectorize with versioning when forced. See D81345.)
+  if (F->hasOptSize() || (llvm::shouldOptimizeForSize(L->getHeader(), PSI, BFI,
+                                                      PGSOQueryType::IRPass) &&
+                          Hints.getForce() != LoopVectorizeHints::FK_Enabled))
     return CM_ScalarEpilogueNotAllowedOptSize;
 
   bool PredicateOptDisabled = PreferPredicateOverEpilog.getNumOccurrences() &&
                               !PreferPredicateOverEpilog;
 
   // 2) Next, if disabling predication is requested on the command line, honour
   // this and request a scalar epilogue.
   if (PredicateOptDisabled)

llvm/test/Transforms/LoopVectorize/optsize.ll

for.body:
   %iv.next = add nuw nsw i32 %iv, 1
   %exitcond = icmp eq i32 %iv.next, 1025
   br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !15
 
 for.end:
   ret void
 }
 
+; Vectorize with versioning for unit stride for PGSO and enabled vectorization.
+;
+define void @stride1_pgso(i16* noalias %B, i32 %BStride) !prof !14 {
+; CHECK-LABEL: @stride1_pgso(
+; CHECK: vector.body
+;
+; PGSO-LABEL: @stride1_pgso(
+; PGSO: vector.body
+;
+; NPGSO-LABEL: @stride1_pgso(
+; NPGSO: vector.body
+
+entry:
+  br label %for.body
+
+for.body:
+  %iv = phi i32 [ %iv.next, %for.body ], [ 0, %entry ]
+  %mulB = mul nsw i32 %iv, %BStride
+  %gepOfB = getelementptr inbounds i16, i16* %B, i32 %mulB
+  store i16 42, i16* %gepOfB, align 4
+  %iv.next = add nuw nsw i32 %iv, 1
+  %exitcond = icmp eq i32 %iv.next, 1025
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !15
+
+for.end:
+  ret void
+}
+
 ; PR46652: Check that the need for stride==1 check prevents vectorizing a loop
 ; having tiny trip count, when compiling w/o -Os/-Oz.
 ; CHECK-LABEL: @pr46652
 ; CHECK-NOT: vector.scevcheck
 ; CHECK-NOT: vector.body
 ; CHECK-LABEL: for.body
 
 @g = external global [1 x i16], align 1