[PATCH] D149423: [ValueTracking] Use knownbits interface for determining if `div`/`rem` are safe to speculate

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D149423#4330891 <https://reviews.llvm.org/D149423#4330891>, @alexfh wrote:

>   $ clang-6cdc229a64be8dacba40d30a9032c14f51ee30c0 -fsanitize=memory -O2 q.cc -S -emit-llvm -o good.ll
>   $ clang-6c667abf3294d61e4fbe1238e1755c79f7547f1b -fsanitize=memory -O2 q.cc -S -emit-llvm -o bad.ll
>   $ diff -u good.ll bad.ll
>   --- good.ll
>   +++ bad.ll
>   @@ -11,8 +11,10 @@
>      %call = tail call noundef i32 @_Z1qv()
>      %spec.store.select = tail call i32 @llvm.umax.i32(i32 %call, i32 1)
>      %1 = icmp ugt i32 %spec.store.select, 8192
>   -  %div = udiv i32 8192, %spec.store.select
>   -  %spec.store.select4 = select i1 %1, i32 1, i32 %div
>   +  %div.rhs.trunc = trunc i32 %spec.store.select to i16
>   +  %div7 = udiv i16 8192, %div.rhs.trunc
>   +  %div.zext = zext i16 %div7 to i32
>   +  %spec.store.select4 = select i1 %1, i32 1, i32 %div.zext
>      ret i32 %spec.store.select4
>    }
>    
>   @@ -40,4 +42,4 @@
>    
>    !0 = !{i32 1, !"wchar_size", i32 4}
>    !1 = !{i32 7, !"uwtable", i32 2}
>   -!2 = !{!"clang version trunk (6cdc229a64be8dacba40d30a9032c14f51ee30c0)"}
>   +!2 = !{!"clang version trunk (6c667abf3294d61e4fbe1238e1755c79f7547f1b)"}

Ah, I think I found the bug.

In

  declare i32 @llvm.umax.i32(i32, i32)
  define dso_local noundef i32 @main(i32 %call, i32 %v) local_unnamed_addr {
  entry:
    %spec.store.select = call i32 @llvm.umax.i32(i32 %call, i32 1)
    %div = udiv i32 8192, %spec.store.select
    %cmp1 = icmp ugt i32 %spec.store.select, 8192
    %spec.store.select4 = select i1 %cmp1, i32 1, i32 %div
    ret i32 %spec.store.select4
  }

`getConstantRangeAtUse` is using the compare after the `udiv`:
`%cmp1 = icmp ugt i32 %spec.store.select, 8192`
To determine that the denominator of `udiv` <= `8192`. This is because
if the denominator `> 8192`, the `udiv` won't be chosen by the select:
`%spec.store.select4 = select i1 %cmp1, i32 1, i32 %div`.
It's only allowed to search past the `udiv` because its speculatively executable.

The issue is `CorrelatedValuePropegation` is using the analysis that
relies on the `udiv` being speculatively executable to modify the `udiv`
operands which may (in this case does) invalidate the lemma of the
analysis.

Short term fix is obviously revert (still building), but longer term
I guess we need an API for "is always speculatively executable"?
or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149423