[PATCH] D147965: [LV] Use VPValue to get expanded value for SCEV step expressions.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 8 09:37:48 PDT 2023


fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:418
+  /// Map SCEVs to their expanded values. Populated when executing
+  /// VPExpandSCEVRecipes.
+  DenseMap<const SCEV *, Value *> ExpandedSCEVs;
----------------
Ayal wrote:
> fhahn wrote:
> > Ayal wrote:
> > > Worth defining and passing around, say,
> > > `  using SCEV2ValueTy = DenseMap<const SCEV *, Value *>;`
> > > instead of the full VPTransformState?
> > > 
> > > vputils could take care of
> > > `  getExpandedSCEV(const SCEV2ValueTy &ExpandedSCEVs, const SCEV *ToExpand);`
> > Updated to pass a function_ref to get the expanded SCEV. Just passing the map isn't enough (without more changes to ILV), as we need to also handle expanding constants/SCEVUnknown. 
> Passing the map complemented with moving `VPTransformState::getExpandedSCEV(const SCEV*)` to `vputils::getExpandedSCEV(const SCEV2ValueTy&, const SCEV*)` to handle expanding constants/SCEVUnkown?
Hm, I am not sure about moving the functionality to `vputils` as there's no recipes/other VPlan components involved, only mapping from SCEV -> Value and it is only temporarily needed. Happy to move it if you still prefer to move it, 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147965



More information about the llvm-commits mailing list