[PATCH] D141850: [SCEV] Preserve divisibility and min/max information in applyLoopGuards

[Alon Kom via Phabricator via llvm-commits]

alonkom added a comment.

In D141850#4064477 <https://reviews.llvm.org/D141850#4064477>, @caojoshua wrote:

> The improved trip multiples from the test results look good. Ordering in applyLoopGuards is an issue. However, I think we can simplify this down a bit. What if we always applied min/max first, before we apply divisibility guards? For example, given:
>
>   __builtin_assume(TC % 8 == 0);
>   __builtin_assume(TC > 0);
>   __builtin_assume(TC < 100);
>
>
>
> 1. apply max: `umax(1, TC)`
> 2. apply min: `umin(100, umax(1, TC))`
> 3. apply divisibility info: `8 * (umin(100, umax(1, TC))) / 8`
>
> This makes divisibility info obvious. And traversing the SCEV, we can still see `TC > 0` and `TC < 100`. I believe that if we always apply max/min first, we will never lose this info. This approach seems much simpler and easier to understand.
>
> FYI. I haven't been around that long, and this change is non-trivial enough where I prefer a longstanding developer to give the final approval. I will still be around to give my thoughts.

Thanks for the reply.
This works in this example, but what would happen if we had only these assumes:

  __builtin_assume(TC % 8 == 0);
  __builtin_assume(TC > 0);

In that case, we would have created the following SCEV:

  8 * (umax(1, TC) / 8)

since umax(1, TC) may still be between [1,7], when we divide it by 8, and multiply by 8, we get 0. So this doesn't preserve the fact that TC > 0.
In order to do that we must align up 1 to 8, and then umax(8, TC) / 8 is always > 0.


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

https://reviews.llvm.org/D141850