[PATCH] D142330: [AssumptionCache] caches @llvm.experimental.guard's

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

In D142330#4150854 <https://reviews.llvm.org/D142330#4150854>, @nikic wrote:

>> Note that with guards we intersect LVI by reverse traversing from the context instruction upwards till start of basic block. The assumption intersection above it does not do that - it just takes all assumes in the block. isValidAssumeForContext allows assumes which are after the context instruction as long as we will reach that assume (this would be incorrect in the case of guards).
>
> Hm, would you mind explaining in more detail why this is not legal for guards (maybe with an IR sample)? It's not completely clear to me what the distinction is.

Sure, consider this IR:

  %local_48_ = phi i32 [ %172, %bci_664 ], [ 4, %bci_381 ]
  ...
  
  bb:
   %145 = shl nsw i32 %local_48_, 16, !dbg !96
    %val = add i32 %145, 393216, !dbg !96
    %146 = ashr exact i32 %val, 16, !dbg !96
    %147 = icmp ult i32 %val, 32768000, !dbg !96
    call void (i1, ...) @llvm.experimental.guard(i1 %147, i32 13)

If we had an assume instead of a guard there, we can use the argument of the assume to make the constant range more precise. This works only when we are guaranteed to transfer execution from %val until the assume (which is part of what `isValidAssumeForContext` checks).

However, with the guard, the `icmp ult i32 %val, 32768000` condition is not guaranteed to be true because the guard can fail. By considering the guard's range for the `val`, we unfortunately used circular logic and folded away the guard based on it (the CR for %val was incorrectly calculated as `constantrange<0, 32768000>`).
This is why when we try to make the constant range more precise with guards, we do backward traversal from the `val` until start of BB to find relevant guards. This is correct because by relying on guards *before* val, we know that we can use the guard's condition because we can reach val only if the guard passes.

> But in any case, we could easily accommodate this by adjusting isValidAssumeForContext() -- it has access to the call, so it could skip that particular logic for guards.

Yes, that would work and handle all cases where the API of Assumption Cache is used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142330