[PATCH] D78298: [LV] Invalidate cost model decisions along with interleave groups.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Clarifying the summary a bit:

> Cost-modeling decisions are tied to the compute interleave groups
>  (widening decisions, scalar and uniform values). We [When] invalidating the
>  interleave groups, those decisions also need to be invalidated.



> Otherwise there is a mis-match during VPlan construction where no
>  VPInterleave recipes are created and VPWidenMemoryRecipes are left
>  around for instructions marked as interleaved. This happens if
>  a vectorization factor is chosen for which computeMaxVF already computed
>  the CM decisions.

VPWidenMemoryRecipes created initially are indeed left around w/o converting them into VPInterleave recipes, but such conversion indeed should not take place, and these gather/scatter recipes may in fact be right. The crux is leaving around obsolete CM_Interleave (and dependent) markings of instructions along with their costs, instead of recalculating decisions, costs, and recipes.



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:749
   /// cannot be filtered by masking the load/store.
   void invalidateGroupsRequiringScalarEpilogue();
 
----------------
Apply a similar fix to invalidateGroupsRequiringScalarEpilogue() as well? I.e., have it return an indication if any groups were invalidated, and if so clear WideningDecisions, Uniforms and Scalars.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1323
+    }
+    InterleaveInfo.reset();
+  }
----------------
If there are no groups there's no need to call reset() either. Seems logical to first do InterleaveInfo.reset(), as it triggers clearing of WideningDecisions (at-least the CM_Interleave ones), which in turn triggers clearing Uniforms and Scalars.

Best have InterleaveInfo.reset() return an indication if any groups were destroyed, and iff so clear the rest? (Can indeed be more selective by also checking if there were any CM_Interleave decisions, etc., but agree that the extra complexity is probably not worth it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78298