[PATCH] D70376: [LVI] Restructure caching

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Hi @nikic, I just tracked down a big compile time increase to this patch. The issue cropped up for a very large function, where a cycle profile showed hotspots in:

llvm::DenseMapBase<llvm::SmallDenseMap<llvm::AssertingVH<llvm::Value>, llvm::ValueLatticeElement, 4u, llvm::DenseMapInfo<llvm::AssertingVH<llvm::Value> >, llvm::detail::DenseMapPair<llvm::AssertingVH<llvm::Value>, llvm::ValueLatticeElement> >, llvm::AssertingVH<llvm::Value>, llvm::ValueLatticeElement, llvm::DenseMapInfo<llvm::AssertingVH<llvm::Value> >, llvm::detail::DenseMapPair<llvm::AssertingVH<llvm::Value>, llvm::ValueLatticeElement> >::erase(llvm::AssertingVH<llvm::Value> const&)

and

(anonymous namespace)::LVIValueHandle::deleted()

The problem is related to this patch's restructuring of the LazyValueInfoCache so that instead of a 2-level map from Value* -> BB -> ValueLatticeElement, it now has a 2-level map from BB -> Value -> ValueLatticeElement. The problem is that LVIValueHandle::deleted invokes LazyValueInfoCache::eraseValue on a Value*, which now needs to walk through every entry in the outer map to remove the Value* from every block containing it in its lattice. Before, it could simply do a single lookup on the outer map to remove the Value.

When I revert this patch the compile time goes down ~35%.

I noticed in your description this comment:
"A possible alternative would be to always cache by value first and have per-BB maps/sets in the each cache entry. In that case we could use a ValueMap and would avoid the separate value handle set. I went with the BB indexing at the top level to make it easier to integrate D69914 <https://reviews.llvm.org/D69914>, but possibly that's not the right choice."

It sounds like your proposed alternative would address this issue. Would you be able to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70376