[PATCH] D81811: [ValueLattice] Allocate value lattice elements on a pool (WIP)

Sun Jun 14 12:48:48 PDT 2020

fhahn added a comment.

Interesting, thanks for working on this!

> This introduces a uniqued, bump-pointer allocated pool of value lattice elements. ValueLatticeElement is still used directly for some temporaries, but the persisted values are fetched from the pool and represented as const ValueLatticeElement *.

It would be interesting how many duplicated elements are actually eliminated. Using a single element for undef/unknown/overdefiened is definitely very profitable, as most lattice values probably end up overdefined. Removing the overdefined set in LVI is definitely a nice thing and SCCP currently does not have anything similar, so there should be a clear benefit there.

For constant/constantrange I am not sure if the extra book-keeping is actually worth it. Without deduplication, we should be able to get rid of the extra sets/maps. Also, SCCP currently updates the lattice elements directly, so caching them might be problematic (or we might have to allocate new objects for each state change?).

Unless there are large savings from the de-duplication, starting out with only de-duplicating overdefined for both SCCP and LVI, together with the bump allocator without de-duplication. That would be quite straight-forward and  should be a clear benefit, at least for SCCP. Not sure if uniquing undef/unknown would be beneficial for SCCP, given the amount of code changes that would be needed.

> https://llvm-compile-time-tracker.com/compare.php?from=862db369f8a8c543735c475ed05cf512846c3868&to=4388725f333c69cf29724528405038f8e143e579&stat=instructions

It looks like there are a few max-rss regressions? Is that noise?

Another thing to consider might be using a bump-allocator for ConstantRange and then using ConstantRange* to reduce the total size of ValueLatticeElement.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81811