[PATCH] D86364: [ValueTracking] Interpret GEPs as a series of adds multiplied by the related scaling factor

Tue Oct 20 12:55:15 PDT 2020

nikic added a comment.

In D86364#2342451 <https://reviews.llvm.org/D86364#2342451>, @spatel wrote:

> I have not stepped through this in detail, so not sure if it helps:
>
> 1. It's been a couple of months, so we should confirm that the time impact is still the same.

Here's some new numbers: https://llvm-compile-time-tracker.com/compare.php?from=0c0fcea557e4a7cfd51216ad20aa67c82733ab52&to=2bf154f40abacf90d0adffaaec20b01f6bd06481&stat=instructions So this is now at 0.35% geomean, with 1.3% on tramp3d-v4. This is definitely much better than where this started.

> 2. It was suggested earlier to trigger the analysis from a different/dedicated pass rather than instcombine. Is that not feasible?

Right. Most of the cost here comes from the fact that we compute alignment for all loads and stores something like 10 times (due to many instcombine runs, which are likely to also visit instructions multiple times), and those alignment calculations will commonly compute GEP known bits. I understand that the actual use case for this patch is not actually the alignment calculation, that's just where this ends up being expensive. If we remove that from the equation, I expect the compile-time impact of this patch would be pretty minimal. I started some work on this, but I'm not sure when I'll be able to finish it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86364