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

[goldstein.w.n]

Summary

This just replaces the exact constant requirements with known-bits
which can prove better results.

This change also adds a new flag PreservesOpCharacteristics.
If PreservesOpCharacteristics is true, isSafeToSpeculativelyExecute{WithOpcode}
will use knownbits analysis to help determine if the Inst is safe
to speculatively execute. This can improve accuracy, but makes it easier to
misuse. For example if isSafeToSpeculativelyExecute{WithOpcode} returns
true for an Inst, then a Transform modifies the operands or hoists it from a
BB that had a dominating condition relevant to one of the operands, the analysis
done by isSafeToSpeculativelyExecute{WithOpcode} may no longer hold true.
If the user is certain their use-case doesn't change any of the characteristics of
the operands, then this is the better option. So for udiv, it would be a misuse
to set PreservesOpCharacteristics to true, then use the result to assume its
safe to mask/truncate the denominator.

This patch goes through the uses of isSafeToSpeculativelyExecute{WithOpcode}
and sets that flag. I tried to be conservative about where I set it to true.

[nikic]

LGTM

llvm/lib/Analysis/ValueTracking.cpp

6216–6217

We don't know it is -1 anymore, just that it might be.

[goldstein.w.n]

Update comment + rebase

[goldstein.w.n]

fixed comment.

[goldstein.w.n]

Rebase with tests in LICM

[nikic]

Ooops, I missed an important pre-condition here: You also need to check that the value isGuaranteedNotToBePoison. All the "known" APIs have an implicit "OrPoison" at the end, and this is one of the case where it makes a difference, as division by poison is UB as well. See for example https://alive2.llvm.org/ce/z/WAFYny.

[nikic]

In D149423#4309375, @nikic wrote:

Ooops, I missed an important pre-condition here: You also need to check that the value isGuaranteedNotToBePoison. All the "known" APIs have an implicit "OrPoison" at the end, and this is one of the case where it makes a difference, as division by poison is UB as well. See for example https://alive2.llvm.org/ce/z/WAFYny.

Unfortunately alive2 doesn't detect this in the LICM tests. It looks like we should be able to test this in a way that alive2 understands using SimplifyCFG block speculation: https://llvm.godbolt.org/z/ehbMKqqxz Apparently this transform is willing to always speculate one instruction even if it's expensive, so it should work well for this purpose and be understood by alive.

[cmtice]

Just a heads up: This commit appears to be breaking one of our tests. I'll see if I can get a reproducible test case. Would you consider reverting this in the meantime?

[goldstein.w.n]

In D149423#4310229, @cmtice wrote:

Just a heads up: This commit appears to be breaking one of our tests. I'll see if I can get a reproducible test case. Would you consider reverting this in the meantime?

Done.

[nikic]

I'd expect the test failure is due to the poison handling issue mentioned above.

[goldstein.w.n]

In D149423#4310385, @nikic wrote:

I'd expect the test failure is due to the poison handling issue mentioned above.

Oh I didn't see that. Will have fix up for that shortly.

[goldstein.w.n]

Also check for poison/undef denom

[goldstein.w.n]

Make poison only: https://alive2.llvm.org/ce/z/vEMLxe seems to be okay if undef included

[goldstein.w.n]

@cmtice think fix is up. Any chance you can verify?

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

6168

Omit Depth=0, which is the default.

6207

either case -> cases or need -> needs.

6219

This also needs to check that op0 is not poison.

[goldstein.w.n]

Also check poison on -1 denum case

[nikic]

LGTM

[cmtice]

Yes, you're updated patch seems to have fixed my problem :-)

[steelannelida]

@goldstein.w.n The newest commit seams to break speculate-div.ll test. I'm working on a reproduciton in upstream, but in our setup the
output is:

llvm/test/Transforms/LICM/speculate-div.ll:54:15: error: CHECK-NEXT: is not on the line after the previous match
; CHECK-NEXT: br label [[LOOP:%.*]]
              ^
<stdin>:37:2: note: 'next' match was here
 br label %loop
 ^
<stdin>:35:23: note: previous match ended here
 %x = and i16 %xo, 123
                      ^
<stdin>:36:1: note: non-matching line after previous match is here
 %div = sdiv i16 %n, %x
^
llvm/test/Transforms/LICM/speculate-div.ll:77:15: error: CHECK-NEXT: is not on the line after the previous match
; CHECK-NEXT: br label [[LOOP:%.*]]
              ^
<stdin>:50:2: note: 'next' match was here
 br label %loop
 ^
<stdin>:48:20: note: previous match ended here
 %x = or i16 %xx, 1
                   ^
<stdin>:49:1: note: non-matching line after previous match is here
 %div = srem i16 %n, %x
^
llvm/test/Transforms/LICM/speculate-div.ll:166:15: error: CHECK-NEXT: is not on the line after the previous match
; CHECK-NEXT: br label [[LOOP:%.*]]
              ^
<stdin>:100:2: note: 'next' match was here
 br label %loop
 ^
<stdin>:98:20: note: previous match ended here
 %x = or i16 %xx, 1
                   ^
<stdin>:99:1: note: non-matching line after previous match is here
 %div = udiv i16 %n, %x
^

Input file: <stdin>
Check file: llvm/test/Transforms/LICM/speculate-div.ll

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          .
          .
          .
         32: define void @sdiv_ok(i16 %n, i16 noundef %xx) { 
         33: entry: 
         34:  %xo = or i16 %xx, 1 
         35:  %x = and i16 %xo, 123 
         36:  %div = sdiv i16 %n, %x 
         37:  br label %loop 
next:54       !~~~~~~~~~~~~~  error: match on wrong line
         38:  
         39: loop: ; preds = %loop, %entry 
         40:  call void @maythrow() 
         41:  call void @use(i16 %div) 
         42:  br label %loop 
         43: } 
         44:  
         45: define void @srem_ok2(i16 noundef %nn, i16 noundef %xx) { 
         46: entry: 
         47:  %n = and i16 %nn, 123 
         48:  %x = or i16 %xx, 1 
         49:  %div = srem i16 %n, %x 
         50:  br label %loop 
next:77       !~~~~~~~~~~~~~  error: match on wrong line
         51:  
         52: loop: ; preds = %loop, %entry 
         53:  call void @maythrow() 
         54:  call void @use(i16 %div) 
         55:  br label %loop 
          .
          .
          .
         95:  
         96: define void @udiv_ok(i16 %n, i16 noundef %xx) { 
         97: entry: 
         98:  %x = or i16 %xx, 1 
         99:  %div = udiv i16 %n, %x 
        100:  br label %loop 
next:166      !~~~~~~~~~~~~~  error: match on wrong line
        101:  
        102: loop: ; preds = %loop, %entry 
        103:  call void @maythrow() 
        104:  call void @use(i16 %div) 
        105:  br label %loop 
          .
          .
          .
>>>>>>

[steelannelida]

It passes upstream though. Probably the issue is with lit version.

[goldstein.w.n]

In D149423#4311944, @steelannelida wrote:

It passes upstream though. Probably the issue is with lit version.

@steelannelida where you able to resolve the issue or do you think the patch is still broken?

[steelannelida]

@goldstein.w.n the problem is on our side, thanks!

[alexfh]

This patch is causing a miscompile:

$ cat q.cc 
extern unsigned q();
int main() {
  unsigned qq = q();
  if (qq == 0) qq = 1;
  int rows = 8192 / qq;
  if (rows == 0) rows = 1;
  return rows;
}
$ cat w.cc
unsigned q() { return 524288; }
$ clang -fsanitize=memory -O2 q.cc w.cc && ./a.out
MemorySanitizer:DEADLYSIGNAL
==885020==ERROR: MemorySanitizer: FPE on unknown address 0x0000002cc0c4 (pc 0x0000002cc0c4 bp 0x000000000001 sp 0x7ffd25df6e50 T885020)
...

Please revert or fix soon. Thanks!

[alexfh]

$ 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)"}

[alexfh]

For clarity: I'm talking about the second attempt on this (https://reviews.llvm.org/rG6c667abf3294d61e4fbe1238e1755c79f7547f1b). For some reason, it's missing a link to this review thread.

[goldstein.w.n]

In 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)"}

I can't see how this change causes this issue. Somehow making the udiv speculative (which is it)
is affecting the ConstantRange pass which in turn allows for narrowUDivOrURem.

I think there must be a misuse of isSafeToSpeculativelyExecute somewhere but can't quite
find it.

[goldstein.w.n]

In D149423#4330861, @alexfh wrote:

This patch is causing a miscompile:

$ cat q.cc 
extern unsigned q();
int main() {
  unsigned qq = q();
  if (qq == 0) qq = 1;
  int rows = 8192 / qq;
  if (rows == 0) rows = 1;
  return rows;
}
$ cat w.cc
unsigned q() { return 524288; }
$ clang -fsanitize=memory -O2 q.cc w.cc && ./a.out
MemorySanitizer:DEADLYSIGNAL
==885020==ERROR: MemorySanitizer: FPE on unknown address 0x0000002cc0c4 (pc 0x0000002cc0c4 bp 0x000000000001 sp 0x7ffd25df6e50 T885020)
...

Please revert or fix soon. Thanks!

Reverting, wasn't able to track down the issue. I think this showing another bug
elsewhere but will revert while I investigate.

Edit: Checking build on revert, thats the delay

But the reason this change causes the bug is because:

if (!CurrI->hasOneUse() || !isSafeToSpeculativelyExecute(CurrI))

Which short stop getConstantRangeAtUse. I think there is almost
definetly a bug in getConstantRangeAtUse or the trunc logic. It was
just hidden because until now isSafeToSpeculativelyExecute only
worked if the div denominator was constant.

[goldstein.w.n]

In 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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

[alexfh]

Thanks for taking care of this!

[StephenFan]

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

[StephenFan]

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I have implemented that at D150353 :)

[goldstein.w.n]

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on narrowUDivOrURem but are these implicit assumptions elsewhere in the project? If the common usage of isSpeculativelyExecutable has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to isSpeculativelyExecutable, not with narrowUDivOrURem

[DaniilSuchkov]

In D149423#4335035, @goldstein.w.n wrote:

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on narrowUDivOrURem but are these implicit assumptions elsewhere in the project? If the common usage of isSpeculativelyExecutable has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to isSpeculativelyExecutable, not with narrowUDivOrURem

In GuardWidening there's a similar problem: when it first analyzes code, it calls isSafeToSpeculativelyExecute for a pair {instruction, insertion point} (here) and then it starts hoisting the instruction along with its operands, starting from the operands. While hoisting, it asserts that isSafeToSpeculativelyExecute should be true (here) since it should have been checked prior to hoisting and should still remain true. With this patch that assertion in some cases fails after the operands get hoisted. The return value of isSafeToSpeculativelyExecute changes because isGuaranteedNotToBePoison for some reason fails to prove that the denominator isn't poison, even though the hoisting had no impact on what values the operands can have. I understand that isGuaranteedNotToBePoison by its nature is always allowed to return false, so I don't think there's any problem with it.
To illustrate my point, here's what the function looks like when GuardWidening calls isSafeToSpeculativelyExecute('%sdiv', '%call') for the first time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %zext = zext i32 %phi to i64
  %sdiv = sdiv i64 1, %zext
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

And here's what the IR looks like when it calls isSafeToSpeculativelyExecute('%sdiv', '%call') the second time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %zext = zext i32 %phi to i64
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %sdiv = sdiv i64 1, %zext
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

The only difference is that %zext, %add and %add4 were hoisted to bb1.

That problem isn't critical, and it probably can be resolved by removing the corresponding part of the assertion, but it demonstrates that some users indeed expect isSafeToSpeculativelyExecute to be resilient in face of some changes to the operands.

[goldstein.w.n]

In D149423#4336520, @DaniilSuchkov wrote:

In D149423#4335035, @goldstein.w.n wrote:

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on narrowUDivOrURem but are these implicit assumptions elsewhere in the project? If the common usage of isSpeculativelyExecutable has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to isSpeculativelyExecutable, not with narrowUDivOrURem

In GuardWidening there's a similar problem: when it first analyzes code, it calls isSafeToSpeculativelyExecute for a pair {instruction, insertion point} (here) and then it starts hoisting the instruction along with its operands, starting from the operands. While hoisting, it asserts that isSafeToSpeculativelyExecute should be true (here) since it should have been checked prior to hoisting and should still remain true. With this patch that assertion in some cases fails after the operands get hoisted. The return value of isSafeToSpeculativelyExecute changes because isGuaranteedNotToBePoison for some reason fails to prove that the denominator isn't poison, even though the hoisting had no impact on what values the operands can have. I understand that isGuaranteedNotToBePoison by its nature is always allowed to return false, so I don't think there's any problem with it.
To illustrate my point, here's what the function looks like when GuardWidening calls isSafeToSpeculativelyExecute('%sdiv', '%call') for the first time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %zext = zext i32 %phi to i64
  %sdiv = sdiv i64 1, %zext
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

And here's what the IR looks like when it calls isSafeToSpeculativelyExecute('%sdiv', '%call') the second time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %zext = zext i32 %phi to i64
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %sdiv = sdiv i64 1, %zext
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

The only difference is that %zext, %add and %add4 were hoisted to bb1.

That problem isn't critical, and it probably can be resolved by removing the corresponding part of the assertion, but it demonstrates that some users indeed expect isSafeToSpeculativelyExecute to be resilient in face of some changes to the operands.

Yeah thats what I suspected. Maybe a new API is inorder? isSafeToSpeculativelyExecuteConst so that things like hoisting (which don't change the operands) can use the improve analysis but not things like GuardWidening or CVP. We could then replace where it makes sense rather than forcing the change to a 100 places which may rely on the _may change_ behavior.

[DaniilSuchkov]

Maybe a new API is inorder? isSafeToSpeculativelyExecuteConst so that things like hoisting (which don't change the operands) can use the improve analysis but not things like GuardWidening or CVP.

Yeah, that may be a good idea.

[StephenFan]

In D149423#4336520, @DaniilSuchkov wrote:

In D149423#4335035, @goldstein.w.n wrote:

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on narrowUDivOrURem but are these implicit assumptions elsewhere in the project? If the common usage of isSpeculativelyExecutable has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to isSpeculativelyExecutable, not with narrowUDivOrURem

In GuardWidening there's a similar problem: when it first analyzes code, it calls isSafeToSpeculativelyExecute for a pair {instruction, insertion point} (here) and then it starts hoisting the instruction along with its operands, starting from the operands. While hoisting, it asserts that isSafeToSpeculativelyExecute should be true (here) since it should have been checked prior to hoisting and should still remain true. With this patch that assertion in some cases fails after the operands get hoisted. The return value of isSafeToSpeculativelyExecute changes because isGuaranteedNotToBePoison for some reason fails to prove that the denominator isn't poison, even though the hoisting had no impact on what values the operands can have. I understand that isGuaranteedNotToBePoison by its nature is always allowed to return false, so I don't think there's any problem with it.
To illustrate my point, here's what the function looks like when GuardWidening calls isSafeToSpeculativelyExecute('%sdiv', '%call') for the first time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %zext = zext i32 %phi to i64
  %sdiv = sdiv i64 1, %zext
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

For this IR, I think isSafeToSpeculativelyExecute('%sdiv', '%call') should return false. I implemented a patch D150542 to give my opinion (may be totally wrong :)).

[goldstein.w.n]

In D149423#4341319, @StephenFan wrote:

In D149423#4336520, @DaniilSuchkov wrote:

In D149423#4335035, @goldstein.w.n wrote:

In D149423#4334277, @StephenFan wrote:

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.
@nikic, any thoughts on how best to proceed in trying to safely
strengthen the isSpeculativelyExecutable case?

I think strengthening the 'isSpeculativlyExecutable' is a bit conservative. I found that this bug is only exposed in CVP's narrowUDivOrURem, we can add some checks to avoid it.

I tend to agree that this issue leans more on narrowUDivOrURem but are these implicit assumptions elsewhere in the project? If the common usage of isSpeculativelyExecutable has come to mean that its speculatively executable even if the non-const operands change then it is a bug in the change to isSpeculativelyExecutable, not with narrowUDivOrURem

In GuardWidening there's a similar problem: when it first analyzes code, it calls isSafeToSpeculativelyExecute for a pair {instruction, insertion point} (here) and then it starts hoisting the instruction along with its operands, starting from the operands. While hoisting, it asserts that isSafeToSpeculativelyExecute should be true (here) since it should have been checked prior to hoisting and should still remain true. With this patch that assertion in some cases fails after the operands get hoisted. The return value of isSafeToSpeculativelyExecute changes because isGuaranteedNotToBePoison for some reason fails to prove that the denominator isn't poison, even though the hoisting had no impact on what values the operands can have. I understand that isGuaranteedNotToBePoison by its nature is always allowed to return false, so I don't think there's any problem with it.
To illustrate my point, here's what the function looks like when GuardWidening calls isSafeToSpeculativelyExecute('%sdiv', '%call') for the first time:

define void @foo() {
bb:
  br label %bb1

bb1:                                              ; preds = %bb8, %bb
  %phi = phi i32 [ 1, %bb ], [ %add9, %bb8 ]
  %call = call i1 @llvm.experimental.widenable.condition()
  br i1 %call, label %bb3, label %bb2

bb2:                                              ; preds = %bb1
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb3:                                              ; preds = %bb1
  %zext = zext i32 %phi to i64
  %sdiv = sdiv i64 1, %zext
  %add = add nuw nsw i64 %zext, 1
  %add4 = add nsw i64 %add, 1
  %add5 = add nsw i64 %add4, %sdiv
  %icmp = icmp sgt i64 %add5, 1
  %call6 = call i1 @llvm.experimental.widenable.condition()
  %and = and i1 %icmp, %call6
  br i1 %and, label %bb8, label %bb7

bb7:                                              ; preds = %bb3
  call void (...) @llvm.experimental.deoptimize.isVoid() [ "deopt"() ]
  ret void

bb8:                                              ; preds = %bb3
  %add9 = add nuw nsw i32 %phi, 1
  br label %bb1
}

For this IR, I think isSafeToSpeculativelyExecute('%sdiv', '%call') should return false. I implemented a patch D150542 to give my opinion (may be totally wrong :)).

Took a look, think you are right.
Plan for this patch is to update the API by adding overload bool meaning isSafeToSpeculativelyExecute(...., bool PreservesOperandCharacteristics = false).
If PreservesOperandCharacteristics is set, it means we the analysis promises the use will not modify the operands in a way that will invalidate the analysis (so only trunc if we known its still safe to speculatively execute). This should be usable to things like hoisting.

[goldstein.w.n]

Add a new flag PreservesOpCharacteristics. If its true it is
assumed that the user will not do anything to the Inst argument that
would change the result of
isSafeToSpeculativelyExecute{WithOpcode}. If it true,
isSafeToSpeculativelyExecute{WithOpcode} will use KnownBits / proper
ValueTracking analysis. If its false,
isSafeToSpeculativelyExecute{WithOpcode} not use KnownBits / proper
ValueTracking analysis and the user may modify the operands in a way
that may change their values with respect to the analysis.

[goldstein.w.n]

Rebase

[goldstein.w.n]

ping regarding the new codes

[xbolva00]

llvm/include/llvm/Analysis/ValueTracking.h

712

Document PreservesOpCharacteristics?

[goldstein.w.n]

Document new flag

[goldstein.w.n]

na

[goldstein.w.n]

ping.

[goldstein.w.n]

ping. Is this still worth pursing?

[goldstein.w.n]

ping.

[nikic]

Okay, so as I understand it, this patch ran into two separate problems:

The first is that isSafeToSpeculativeExecute() is used for two different purposes: One is to speculate the instruction (i.e. execute it where it may not have previously been executed, keeping the same arguments), and the other is to execute the instruction at the same position but with different operands (with the implicit assumption that constant operands stay the same). Previously, the same function worked for both of those, but this is no longer the case if we analyze variables.

Actually, I think this is not quite true and we have a pre-existing issue for loads. Consider this example: https://alive2.llvm.org/ce/z/weHUyL If there were a control flow exit between the load and the select, this might speculate a trapping load.

With this in mind, this is clearly something that needs to be solved. My preference would to provide two separate functions (with a shared implementation) for the two different use cases, rather than specifying a flag everywhere. For example, one stays isSafeToSpeculativelyExecute() and the other becomes isSafeToExecuteWithDifferentOperands(). I think we can do this independently of the div change, with the load case as the motivating example.

The second problem is related to the poison check and the context instruction. As far as I understand, the issue are callers of isSafeToSpeculativelyExecute() which don't speculate instructions one-by-one (as LICM does), but rather check a whole sequence for speculability first and then perform the actual transform. This ends up being problematic, because we perform poison queries on instructions in their previous positions, where the fact that they are passed to the division itself implies that they cannot be poison. So what we really want to determine here is "is this instruction non-poison at this context under the assumption that it has already been hoisted there", which is not what the existing APIs do.

This problem is not as simple as not doing the programUndefinedIfUndefOrPoison() check, it can also occur in other contexts. Let's say we have a sequence like this:

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

Even if we ignore that %y is non-poison due to being used in the udiv, we also know that it is non-poison due to the !noundef metadata. However, that !noundef metadata would be dropped if the load were actually speculated. So in this case, the udiv is actually not safe to speculate, because %y may be poison after speculation.

The two possible ways of solving this I can see are:

1. Add an extra argument to isGuaranteedNotToBeUndefOrPoison() that pretends instructions have been speculated.
2. Only support cases where the div arguments dominate the context instruction (if no context, that would effectively limit to constants/arguments). In that case the plain isGuaranteedNotToBeUndefOrPoison() should work fine.

In D149423#4402529, @goldstein.w.n wrote:

Is this still worth pursing?

Not sure, to be honest. I liked this conceptually, but the problem turned out to be much more complicated than anticipated.

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2781

I don't think we should worry about that here and specify whatever is correct semantically.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

1249

This should be false.

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

300

Why false here?

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

450

Why false here? This should be simple speculation.

[goldstein.w.n]

In D149423#4416984, @nikic wrote:

Okay, so as I understand it, this patch ran into two separate problems:

The first is that isSafeToSpeculativeExecute() is used for two different purposes: One is to speculate the instruction (i.e. execute it where it may not have previously been executed, keeping the same arguments), and the other is to execute the instruction at the same position but with different operands (with the implicit assumption that constant operands stay the same). Previously, the same function worked for both of those, but this is no longer the case if we analyze variables.

Actually, I think this is not quite true and we have a pre-existing issue for loads. Consider this example: https://alive2.llvm.org/ce/z/weHUyL If there were a control flow exit between the load and the select, this might speculate a trapping load.

With this in mind, this is clearly something that needs to be solved. My preference would to provide two separate functions (with a shared implementation) for the two different use cases, rather than specifying a flag everywhere. For example, one stays isSafeToSpeculativelyExecute() and the other becomes isSafeToExecuteWithDifferentOperands(). I think we can do this independently of the div change, with the load case as the motivating example.

The second problem is related to the poison check and the context instruction. As far as I understand, the issue are callers of isSafeToSpeculativelyExecute() which don't speculate instructions one-by-one (as LICM does), but rather check a whole sequence for speculability first and then perform the actual transform. This ends up being problematic, because we perform poison queries on instructions in their previous positions, where the fact that they are passed to the division itself implies that they cannot be poison. So what we really want to determine here is "is this instruction non-poison at this context under the assumption that it has already been hoisted there", which is not what the existing APIs do.

This problem is not as simple as not doing the programUndefinedIfUndefOrPoison() check, it can also occur in other contexts. Let's say we have a sequence like this:

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

Even if we ignore that %y is non-poison due to being used in the udiv, we also know that it is non-poison due to the !noundef metadata. However, that !noundef metadata would be dropped if the load were actually speculated. So in this case, the udiv is actually not safe to speculate, because %y may be poison after speculation.

The two possible ways of solving this I can see are:

1. Add an extra argument to isGuaranteedNotToBeUndefOrPoison() that pretends instructions have been speculated.
2. Only support cases where the div arguments dominate the context instruction (if no context, that would effectively limit to constants/arguments). In that case the plain isGuaranteedNotToBeUndefOrPoison() should work fine.

Is a third alternative to refactor the usage of isSafeToExecuteWithDifferentOperands() s.t instead of batching the analysis, it does the analysis/transform piece-meal? Or is that dramatically more work/have compile time/codegen impact? At the very least we could add a "IsBatchedAnalysis" flag (or API) and only apply the first two constraints if its set.

Is there not a final concern that isKnownNonZero uses dominating conditions to determine zero/non-zero, so w.o a CxtI instruction we can't actually use IsKnownNonZero as the transform may change them i.e:

%c = icmp ule i32 %x, 1
br i1 %c, label %T, label %F
T:
ret i32 %y
F:
%r = udiv %y, %x
ret i32 %r

we can't transform this into a select or something.

Think you could do the same with assume i.e:

%c = icmp ule i32 %z, 1
br i1 %c, label %T, label %F
T:
ret i32 %y
F:
%nz = icmp ne i32 %x, 0
call void @llvm.assume(i1 %nz)
%r = udiv %y, %x
ret i32 %r

For this, think we would need a flag is IsKnownNonZero to indicate we can't use assumes/dominating conditions to do isKnowNonZero (or just rely on computeKnownBits.isNonZero())

In D149423#4402529, @goldstein.w.n wrote:

Is this still worth pursing?

Not sure, to be honest. I liked this conceptually, but the problem turned out to be much more complicated than anticipated.

I think given that we have an issue with loads a fix is needed either way. no?

[nikic]

In D149423#4418573, @goldstein.w.n wrote:

Is a third alternative to refactor the usage of isSafeToExecuteWithDifferentOperands() s.t instead of batching the analysis, it does the analysis/transform piece-meal? Or is that dramatically more work/have compile time/codegen impact? At the very least we could add a "IsBatchedAnalysis" flag (or API) and only apply the first two constraints if its set.

For some transforms, we need to ensure that e.g. all instructions in a block are speculatable, otherwise the transform is not legal. We can't really do that incrementally.

Is there not a final concern that isKnownNonZero uses dominating conditions to determine zero/non-zero, so w.o a CxtI instruction we can't actually use IsKnownNonZero as the transform may change them i.e:

%c = icmp ule i32 %x, 1
br i1 %c, label %T, label %F
T:
ret i32 %y
F:
%r = udiv %y, %x
ret i32 %r

we can't transform this into a select or something.

In this case we would be performing the query on %x, with the definition point of %x as the context, at which point there should be no dominating conditions for it. So I think there would be no problem.

Think you could do the same with assume i.e:

%c = icmp ule i32 %z, 1
br i1 %c, label %T, label %F
T:
ret i32 %y
F:
%nz = icmp ne i32 %x, 0
call void @llvm.assume(i1 %nz)
%r = udiv %y, %x
ret i32 %r

The assume case may be problematic because we have limited backwards reasoning for assumes. Your example wouldn't hit this, but if you defined %x in the F block that could be a problem.

For this, think we would need a flag is IsKnownNonZero to indicate we can't use assumes/dominating conditions to do isKnowNonZero (or just rely on computeKnownBits.isNonZero())

In D149423#4402529, @goldstein.w.n wrote:

Is this still worth pursing?

Not sure, to be honest. I liked this conceptually, but the problem turned out to be much more complicated than anticipated.

I think given that we have an issue with loads a fix is needed either way. no?

It's two separate problems: The load issue is only about the isSafeToSpeculativelyExecute/isSafeToExecuteWithDifferentOperands split, it is not affected (as far as I can tell) by the trickier context instruction problem.

[StephenFan]

In D149423#4416984, @nikic wrote:

The second problem is related to the poison check and the context instruction. As far as I understand, the issue are callers of isSafeToSpeculativelyExecute() which don't speculate instructions one-by-one (as LICM does), but rather check a whole sequence for speculability first and then perform the actual transform. This ends up being problematic, because we perform poison queries on instructions in their previous positions, where the fact that they are passed to the division itself implies that they cannot be poison. So what we really want to determine here is "is this instruction non-poison at this context under the assumption that it has already been hoisted there", which is not what the existing APIs do.

This problem is not as simple as not doing the programUndefinedIfUndefOrPoison() check, it can also occur in other contexts. Let's say we have a sequence like this:

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

Even if we ignore that %y is non-poison due to being used in the udiv, we also know that it is non-poison due to the !noundef metadata. However, that !noundef metadata would be dropped if the load were actually speculated. So in this case, the udiv is actually not safe to speculate, because %y may be poison after speculation.

The two possible ways of solving this I can see are:

1. Add an extra argument to isGuaranteedNotToBeUndefOrPoison() that pretends instructions have been speculated.
2. Only support cases where the div arguments dominate the context instruction (if no context, that would effectively limit to constants/arguments). In that case the plain isGuaranteedNotToBeUndefOrPoison() should work fine.

Can we have the third solution:
Continue or improving what we do in D150542.
And as for

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

When we check if %y is guaranteed not to be undef or poison, %CtxI argument should be respected. In other words, in IsGuaranteedNotToBeUndefOrPoison, if the load instruction's parent is not equal to %CtxI's, then we should not reason about the result from the instruction's metadata. Since metadata may be dropped after moving to a different block. But I am also confused that if metadata like !noundef can be dropped randomly, why can we still reason about the result from metadata?

[nikic]

In D149423#4420186, @StephenFan wrote:

Can we have the third solution:
Continue or improving what we do in D150542.
And as for

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

When we check if %y is guaranteed not to be undef or poison, %CtxI argument should be respected. In other words, in IsGuaranteedNotToBeUndefOrPoison, if the load instruction's parent is not equal to %CtxI's, then we should not reason about the result from the instruction's metadata. Since metadata may be dropped after moving to a different block. But I am also confused that if metadata like !noundef can be dropped randomly, why can we still reason about the result from metadata?

I don't think this is appropriate, because it overloads the meaning of %CtxI, between "not poison at this point" and "not poison if hoisted to this point". Generally speaking, specifying a context instruction should only improve ValueTracking results, not make the more conservative.

We can do what you suggest in general, but it should not use the existing CtxI argument.

However, together with the additional concern around context-sensitive facts (like assumes) that @goldstein.w.n raised, I think that going for "require div operands to dominate the context instruction" is going to be the most robust solution.

An unrelated thought I had is that speculating these divs might run into a cost issue: We currently only speculate divs with constant divisor, which also ensures that the div will be expanded to arithmetic as a side-effect, so we never actually end up speculating real division instructions.

[goldstein.w.n]

In D149423#4420208, @nikic wrote:

In D149423#4420186, @StephenFan wrote:

Can we have the third solution:
Continue or improving what we do in D150542.
And as for

%y = load i32, ptr %p, !noundef !{}, !range !{i32 1, i32 10}
%d = udiv i32 %x, %y

When we check if %y is guaranteed not to be undef or poison, %CtxI argument should be respected. In other words, in IsGuaranteedNotToBeUndefOrPoison, if the load instruction's parent is not equal to %CtxI's, then we should not reason about the result from the instruction's metadata. Since metadata may be dropped after moving to a different block. But I am also confused that if metadata like !noundef can be dropped randomly, why can we still reason about the result from metadata?

I don't think this is appropriate, because it overloads the meaning of %CtxI, between "not poison at this point" and "not poison if hoisted to this point". Generally speaking, specifying a context instruction should only improve ValueTracking results, not make the more conservative.

We can do what you suggest in general, but it should not use the existing CtxI argument.

However, together with the additional concern around context-sensitive facts (like assumes) that @goldstein.w.n raised, I think that going for "require div operands to dominate the context instruction" is going to be the most robust solution.

An unrelated thought I had is that speculating these divs might run into a cost issue: We currently only speculate divs with constant divisor, which also ensures that the div will be expanded to arithmetic as a side-effect, so we never actually end up speculating real division instructions.

Going to post refactor patch that fixes load soonish (probably tomorrow), will run div patches through comile-time-tracker and maybe we will drop (or add flag for heavy analysis).

llvm/include/llvm/Analysis/ValueTracking.h

 /// If the CtxI is NOT specified this method only looks at the instruction
 /// itself and its operands, so if this method returns true, it is safe to
 /// move the instruction as long as the correct dominance relationships for
 /// the operands and users hold.
 ///
 /// This method can return true for instructions that read memory;
 /// for such instructions, moving them may change the resulting value.
 bool isSafeToSpeculativelyExecute(const Instruction *I,
+                                  bool PreservesOpCharacteristics,
                                   const Instruction *CtxI = nullptr,
                                   AssumptionCache *AC = nullptr,
                                   const DominatorTree *DT = nullptr,
                                   const TargetLibraryInfo *TLI = nullptr);
 
 /// This returns the same result as isSafeToSpeculativelyExecute if Opcode is
 /// the actual opcode of Inst. If the provided and actual opcode differ, the
 /// function (virtually) overrides the opcode of Inst with the provided
 /// Opcode. There are come constraints in this case:
 /// * If Opcode has a fixed number of operands (eg, as binary operators do),
 ///   then Inst has to have at least as many leading operands. The function
 ///   will ignore all trailing operands beyond that number.
 /// * If Opcode allows for an arbitrary number of operands (eg, as CallInsts
 ///   do), then all operands are considered.
 /// * The virtual instruction has to satisfy all typing rules of the provided
 ///   Opcode.
 /// * This function is pessimistic in the following sense: If one actually
 ///   materialized the virtual instruction, then isSafeToSpeculativelyExecute
 ///   may say that the materialized instruction is speculatable whereas this
 ///   function may have said that the instruction wouldn't be speculatable.
 ///   This behavior is a shortcoming in the current implementation and not
 ///   intentional.
 bool isSafeToSpeculativelyExecuteWithOpcode(
-    unsigned Opcode, const Instruction *Inst, const Instruction *CtxI = nullptr,
-    AssumptionCache *AC = nullptr, const DominatorTree *DT = nullptr,
-    const TargetLibraryInfo *TLI = nullptr);
+    unsigned Opcode, const Instruction *Inst, bool PreservesOpCharacteristics,
+    const Instruction *CtxI = nullptr, AssumptionCache *AC = nullptr,
+    const DominatorTree *DT = nullptr, const TargetLibraryInfo *TLI = nullptr);
 
 /// Returns true if the result or effects of the given instructions \p I
 /// depend values not reachable through the def use graph.
 /// * Memory dependence arises for example if the instruction reads from
 ///   memory or may produce effects or undefined behaviour. Memory dependent
 ///   instructions generally cannot be reorderd with respect to other memory
 ///   dependent instructions.
 /// * Control dependence arises for example if the instruction may fault

llvm/lib/Analysis/IVUsers.cpp

if (!Processed.insert(I).second)
     return true;    // Instruction already handled.
 
   if (!SE->isSCEVable(I->getType()))
     return false;   // Void and FP expressions cannot be reduced.
 
   // IVUsers is used by LSR which assumes that all SCEV expressions are safe to
   // pass to SCEVExpander. Expressions are not safe to expand if they represent
   // operations that are not safe to speculate, namely integer division.
-  if (!isa<PHINode>(I) && !isSafeToSpeculativelyExecute(I))
+  if (!isa<PHINode>(I) &&
+      !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true))
     return false;
 
   // LSR is not APInt clean, do not touch integers bigger than 64-bits.
   // Also avoid creating IVs of non-native types. For example, we don't want a
   // 64-bit IV in 32-bit code just because the loop has one 64-bit cast.
   uint64_t Width = SE->getTypeSizeInBits(I->getType());
   if (Width > 64 || !DL.isLegalInteger(Width))
     return false;

llvm/lib/Analysis/LazyValueInfo.cpp

for (unsigned I = 0; I < MaxUsesToInspect; ++I) {
     // If there are multiple uses, we would have to intersect with the union of
     // all conditions at different uses.
     // Stop walking if we hit a non-speculatable instruction. Even if the
     // result is only used under a specific condition, executing the
     // instruction itself may cause side effects or UB already.
     // This also disallows looking through phi nodes: If the phi node is part
     // of a cycle, we might end up reasoning about values from different cycle
     // iterations (PR60629).
-    if (!CurrI->hasOneUse() || !isSafeToSpeculativelyExecute(CurrI))
+    if (!CurrI->hasOneUse() ||
+        !isSafeToSpeculativelyExecute(CurrI,
+                                      /* PreservesOpCharacteristics */ false))
       break;
     CurrU = &*CurrI->use_begin();
   }
   return CR;
 }
 
 /// Determine whether the specified value is known to be a
 /// constant on the specified edge. Return null if not.

llvm/lib/Analysis/LoopInfo.cpp

 }
 
 bool Loop::makeLoopInvariant(Instruction *I, bool &Changed,
                              Instruction *InsertPt, MemorySSAUpdater *MSSAU,
                              ScalarEvolution *SE) const {
   // Test if the value is already loop-invariant.
   if (isLoopInvariant(I))
     return true;
-  if (!isSafeToSpeculativelyExecute(I))
+  if (!isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true))
     return false;
   if (I->mayReadFromMemory())
     return false;
   // EH block instructions are immobile.
   if (I->isEHPad())
     return false;
   // Determine the insertion point, unless one was given.
   if (!InsertPt) {

llvm/lib/Analysis/LoopNestAnalysis.cpp

 }
 
 static bool checkSafeInstruction(const Instruction &I,
                                  const CmpInst *InnerLoopGuardCmp,
                                  const CmpInst *OuterLoopLatchCmp,
                                  std::optional<Loop::LoopBounds> OuterLoopLB) {
 
   bool IsAllowed =
-      isSafeToSpeculativelyExecute(&I) || isa<PHINode>(I) || isa<BranchInst>(I);
+      isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true) ||
+      isa<PHINode>(I) || isa<BranchInst>(I);
   if (!IsAllowed)
     return false;
   // The only binary instruction allowed is the outer loop step instruction,
   // the only comparison instructions allowed are the inner loop guard
   // compare instruction and the outer loop latch compare instruction.
   if ((isa<BinaryOperator>(I) && &I != &OuterLoopLB->getStepInst()) ||
       (isa<CmpInst>(I) && &I != OuterLoopLatchCmp && &I != InnerLoopGuardCmp)) {
     return false;

llvm/lib/Analysis/ValueTracking.cpp

bool llvm::mustSuppressSpeculation(const LoadInst &LI) {
   // Speculative load may create a race that did not exist in the source.
   return F.hasFnAttribute(Attribute::SanitizeThread) ||
     // Speculative load may load data from dirty regions.
     F.hasFnAttribute(Attribute::SanitizeAddress) ||
     F.hasFnAttribute(Attribute::SanitizeHWAddress);
 }
 
 bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst,
+                                        bool PreservesOpCharacteristics,
                                         const Instruction *CtxI,
                                         AssumptionCache *AC,
                                         const DominatorTree *DT,
                                         const TargetLibraryInfo *TLI) {
-  return isSafeToSpeculativelyExecuteWithOpcode(Inst->getOpcode(), Inst, CtxI,
-                                                AC, DT, TLI);
+  return isSafeToSpeculativelyExecuteWithOpcode(
+      Inst->getOpcode(), Inst, PreservesOpCharacteristics, CtxI, AC, DT, TLI);
 }
 
 bool llvm::isSafeToSpeculativelyExecuteWithOpcode(
-    unsigned Opcode, const Instruction *Inst, const Instruction *CtxI,
-    AssumptionCache *AC, const DominatorTree *DT,
+    unsigned Opcode, const Instruction *Inst, bool PreservesOpCharacteristics,
+    const Instruction *CtxI, AssumptionCache *AC, const DominatorTree *DT,
     const TargetLibraryInfo *TLI) {
 #ifndef NDEBUG
   if (Inst->getOpcode() != Opcode) {
     // Check that the operands are actually compatible with the Opcode override.
     auto hasEqualReturnAndLeadingOperandTypes =
         [](const Instruction *Inst, unsigned NumLeadingOperands) {
           if (Inst->getNumOperands() < NumLeadingOperands)
             return false;
assert(!Instruction::isUnaryOp(Opcode) ||
            hasEqualReturnAndLeadingOperandTypes(Inst, 1));
   }
 #endif
 
   switch (Opcode) {
   default:
     return true;
   case Instruction::UDiv:
-  case Instruction::URem: {
-    // x / y is undefined if y == 0.
-    const APInt *V;
-    if (match(Inst->getOperand(1), m_APInt(V)))
-      return *V != 0;
-    return false;
-  }
+  case Instruction::URem:
   case Instruction::SDiv:
   case Instruction::SRem: {
-    // x / y is undefined if y == 0 or x == INT_MIN and y == -1
-    const APInt *Numerator, *Denominator;
-    if (!match(Inst->getOperand(1), m_APInt(Denominator)))
-      return false;
-    // We cannot hoist this division if the denominator is 0.
-    if (*Denominator == 0)
+    const DataLayout &DL = Inst->getModule()->getDataLayout();
+
+    // We wrap isGuaranteedNotToBePoison, isKnownNonZero, and computeKnownBits.
+    // If PreservesOpCharacteristics is true we use proper ValueTracking
+    // analysis. Otherwise we just check based on if the value is constant. The
+    // reason for this is `isSafeToSpeculativelyExecute` is used in a variety of
+    // places where the user expects to be able to modify the operands and
+    // fundementally change their characteristics with regards to the
+    // ValueTracking queries we use here (for example truncate the Num/Denom).
+    // So, unless the use gurantees they will preserve the characteristics,
+    // conservatively only do analysis on constant operands.
+    auto OpIsNonPoison = [&](Value *Op) {
+      return PreservesOpCharacteristics
+                 ? isGuaranteedNotToBePoison(Op, AC, CtxI, DT)
+                 : match(Op, m_ImmConstant());
+    };
+
+    auto OpIsNonZero = [&](Value *Op) {
+      const APInt *C;
+      return PreservesOpCharacteristics
+                 ? isKnownNonZero(Op, DL, /*Depth*/ 0, AC, CtxI, DT)
+                 : (match(Op, m_APInt(C)) && !C->isZero());
+    };
+
+    auto OpKnownBits = [&](Value *Op) {
+      if (PreservesOpCharacteristics)
+        return computeKnownBits(Op, DL, /*Depth*/ 0, AC, CtxI, DT);
+
+      const APInt *C;
+      if (match(Op, m_APInt(C)))
+        return KnownBits::makeConstant(*C);
+
+      KnownBits Known(getBitWidth(Op->getType(), DL));
+      Known.resetAll();
+      return Known;
+    };
+
+    // x / y is undefined if y == 0 or y is poison.
+    if (!OpIsNonPoison(Inst->getOperand(1)) ||
+        !OpIsNonZero(Inst->getOperand(1)))
       return false;
+
+    // Unsigned case only needs to avoid denominator == 0 or poison.
+    if (Opcode == Instruction::UDiv || Opcode == Instruction::URem)
+      return true;
+
+    // x s/ y is also undefined if x == INT_MIN and y == -1
+    KnownBits KnownDenominator = OpKnownBits(Inst->getOperand(1));
+
     // It's safe to hoist if the denominator is not 0 or -1.
-    if (!Denominator->isAllOnes())
+    if (!KnownDenominator.Zero.isZero())
       return true;
-    // At this point we know that the denominator is -1.  It is safe to hoist as
-    // long we know that the numerator is not INT_MIN.
-    if (match(Inst->getOperand(0), m_APInt(Numerator)))
-      return !Numerator->isMinSignedValue();
-    // The numerator *might* be MinSignedValue.
+
+    // At this point denominator may be -1.  It is safe to hoist as
+    // long we know that the numerator is neither poison nor INT_MIN.
+    if (!OpIsNonPoison(Inst->getOperand(0)))
       return false;
+    KnownBits KnownNumerator = OpKnownBits(Inst->getOperand(0));
+    return !KnownNumerator.getSignedMinValue().isMinSignedValue();
   }
   case Instruction::Load: {
     const LoadInst *LI = dyn_cast<LoadInst>(Inst);
     if (!LI)
       return false;
     if (mustSuppressSpeculation(*LI))
       return false;
     const DataLayout &DL = LI->getModule()->getDataLayout();
case Instruction::CleanupRet:
     return false; // Misc instructions which have effects
   }
 }
 
 bool llvm::mayHaveNonDefUseDependency(const Instruction &I) {
   if (I.mayReadOrWriteMemory())
     // Memory dependency possible
     return true;
-  if (!isSafeToSpeculativelyExecute(&I))
+  if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true))
     // Can't move above a maythrow call or infinite loop.  Or if an
     // inalloca alloca, above a stacksave call.
     return true;
   if (!isGuaranteedToTransferExecutionToSuccessor(&I))
     // 1) Can't reorder two inf-loop calls, even if readonly
     // 2) Also can't reorder an inf-loop call below a instruction which isn't
     //    safe to speculative execute.  (Inverse of above)
     return true;

llvm/lib/CodeGen/Analysis.cpp

for (BasicBlock::const_iterator BBI = std::prev(ExitBB->end(), 2);; --BBI) {
     // A lifetime end, assume or noalias.decl intrinsic should not stop tail
     // call optimization.
     if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(BBI))
       if (II->getIntrinsicID() == Intrinsic::lifetime_end ||
           II->getIntrinsicID() == Intrinsic::assume ||
           II->getIntrinsicID() == Intrinsic::experimental_noalias_scope_decl)
         continue;
     if (BBI->mayHaveSideEffects() || BBI->mayReadFromMemory() ||
-        !isSafeToSpeculativelyExecute(&*BBI))
+        !isSafeToSpeculativelyExecute(&*BBI,
+                                      /* PreservesOpCharacteristics */ true))
       return false;
   }
 
   const Function *F = ExitBB->getParent();
   return returnTypeIsEligibleForTailCall(
       F, &Call, Ret, *TM.getSubtargetImpl(*F)->getTargetLowering());
 }
 

llvm/lib/CodeGen/CodeGenPrepare.cpp

 }
 
 /// Check if V (an operand of a select instruction) is an expensive instruction
 /// that is only used once.
 static bool sinkSelectOperand(const TargetTransformInfo *TTI, Value *V) {
   auto *I = dyn_cast<Instruction>(V);
   // If it's safe to speculatively execute, then it should not have side
   // effects; therefore, it's safe to sink and possibly *not* execute.
-  return I && I->hasOneUse() && isSafeToSpeculativelyExecute(I) &&
+  return I && I->hasOneUse() &&
+         isSafeToSpeculativelyExecute(I,
+                                      /* PreservesOpCharacteristics */ true) &&
          TTI->isExpensiveToSpeculativelyExecute(I);
 }
 
 /// Returns true if a SelectInst should be turned into an explicit branch.
 static bool isFormingBranchFromSelectProfitable(const TargetTransformInfo *TTI,
                                                 const TargetLowering *TLI,
                                                 SelectInst *SI) {
   // If even a predictable select is cheap, then a branch can't be cheaper.

llvm/lib/CodeGen/ExpandVectorPredication.cpp

 
 static bool maySpeculateLanes(VPIntrinsic &VPI) {
   // The result of VP reductions depends on the mask and evl.
   if (isa<VPReductionIntrinsic>(VPI))
     return false;
   // Fallback to whether the intrinsic is speculatable.
   std::optional<unsigned> OpcOpt = VPI.getFunctionalOpcode();
   unsigned FunctionalOpc = OpcOpt.value_or((unsigned)Instruction::Call);
-  return isSafeToSpeculativelyExecuteWithOpcode(FunctionalOpc, &VPI);
+  return isSafeToSpeculativelyExecuteWithOpcode(
+      FunctionalOpc, &VPI, /* PreservesOpCharacteristics */ true);
 }
 
 //// } Helpers
 
 namespace {
 
 // Expansion pass state at function scope.
 struct CachingVPExpander {

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

case Intrinsic::assume: {
     break;
   }
   case Intrinsic::experimental_guard: {
     // Is this guard followed by another guard?  We scan forward over a small
     // fixed window of instructions to handle common cases with conditions
     // computed between guards.
     Instruction *NextInst = II->getNextNonDebugInstruction();
     for (unsigned i = 0; i < GuardWideningWindow; i++) {
-      // Note: Using context-free form to avoid compile time blow up
-      if (!isSafeToSpeculativelyExecute(NextInst))
+      // Note: Using context-free form to avoid compile time blow up. Likewise
+      // don't set 'PreservesOpCharacteristics' to keep less expensive analysis.
+      if (!isSafeToSpeculativelyExecute(NextInst,
+                                        /* PreservesOpCharacteristics */ false))
         break;
       NextInst = NextInst->getNextNonDebugInstruction();
     }
     Value *NextCond = nullptr;
     if (match(NextInst,
               m_Intrinsic<Intrinsic::experimental_guard>(m_Value(NextCond)))) {
       Value *CurrCond = II->getArgOperand(0);
 

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

 
 static bool replaceInInstruction(Value *V, Value *Old, Value *New,
                                  InstCombiner &IC, unsigned Depth = 0) {
   // Conservatively limit replacement to two instructions upwards.
   if (Depth == 2)
     return false;
 
   auto *I = dyn_cast<Instruction>(V);
-  if (!I || !I->hasOneUse() || !isSafeToSpeculativelyExecute(I))
+  if (!I || !I->hasOneUse() ||
+      !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true))
     return false;
 
   bool Changed = false;
   for (Use &U : I->operands()) {
     if (U == Old) {
       IC.replaceUse(U, New);
       Changed = true;
     } else {

llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

if (!match(Op0, m_BinOp(m_Shuffle(m_Value(X), m_Undef(), m_ZeroMask()),
                           m_Value(Y))) &&
       !match(Op0, m_BinOp(m_Value(X),
                           m_Shuffle(m_Value(Y), m_Undef(), m_ZeroMask()))))
     return nullptr;
   if (X->getType() != Y->getType())
     return nullptr;
 
   auto *BinOp = cast<BinaryOperator>(Op0);
-  if (!isSafeToSpeculativelyExecute(BinOp))
+  if (!isSafeToSpeculativelyExecute(BinOp,
+                                    /* PreservesOpCharacteristics */ false))
     return nullptr;
 
   Value *NewBO = Builder.CreateBinOp(BinOp->getOpcode(), X, Y);
   if (auto NewBOI = dyn_cast<Instruction>(NewBO))
     NewBOI->copyIRFlags(BinOp);
 
   return new ShuffleVectorInst(NewBO, SVI.getShuffleMask());
 }

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Instruction *InstCombinerImpl::foldVectorBinop(BinaryOperator &Inst) {
   }
   // Op(LHSSplat, rev(V2)) -> rev(Op(LHSSplat, V2))
   else if (isSplatValue(LHS) && match(RHS, m_OneUse(m_VecReverse(m_Value(V2)))))
     return createBinOpReverse(LHS, V2);
 
   // It may not be safe to reorder shuffles and things like div, urem, etc.
   // because we may trap when executing those ops on unknown vector elements.
   // See PR20059.
-  if (!isSafeToSpeculativelyExecute(&Inst))
+  if (!isSafeToSpeculativelyExecute(&Inst,
+                                    /* PreservesOpCharacteristics */ false))
     return nullptr;
 
   auto createBinOpShuffle = [&](Value *X, Value *Y, ArrayRef<int> M) {
     Value *XY = Builder.CreateBinOp(Opcode, X, Y);
     if (auto *BO = dyn_cast<BinaryOperator>(XY))
       BO->copyIRFlags(&Inst);
     return new ShuffleVectorInst(XY, M);
   };

llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp

return isa<BinaryOperator>(I) || isa<CastInst>(I) || isa<SelectInst>(I) ||
       isa<ShuffleVectorInst>(I) || isa<ExtractValueInst>(I) ||
       isa<InsertValueInst>(I);
 }
 
 // Return true if the given instruction can be hoisted by CHR.
 static bool isHoistable(Instruction *I, DominatorTree &DT) {
   if (!isHoistableInstructionType(I))
     return false;
-  return isSafeToSpeculativelyExecute(I, nullptr, nullptr, &DT);
+  return isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true,
+                                      nullptr, nullptr, &DT);
 }
 
 // Recursively traverse the use-def chains of the given value and return a set
 // of the unhoistable base values defined within the scope (excluding the
 // first-region entry block) or the (hoistable or unhoistable) base values that
 // are defined outside (including the first-region entry block) of the
 // scope. The returned set doesn't include constants.
 static const std::set<Value *> &

llvm/lib/Transforms/Scalar/GVN.cpp

bool GVNPass::PerformLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
   // that one block.
   if (NumUnavailablePreds != 1)
       return false;
 
   // Now we know where we will insert load. We must ensure that it is safe
   // to speculatively execute the load at that points.
   if (MustEnsureSafetyOfSpeculativeExecution) {
     if (CriticalEdgePred.size())
-      if (!isSafeToSpeculativelyExecute(Load, LoadBB->getFirstNonPHI(), AC, DT))
+      if (!isSafeToSpeculativelyExecute(Load,
+                                        /* PreservesOpCharacteristics */ true,
+                                        LoadBB->getFirstNonPHI(), AC, DT))
         return false;
     for (auto &PL : PredLoads)
-      if (!isSafeToSpeculativelyExecute(Load, PL.first->getTerminator(), AC,
-                                        DT))
+      if (!isSafeToSpeculativelyExecute(Load,
+                                        /* PreservesOpCharacteristics */ true,
+                                        PL.first->getTerminator(), AC, DT))
         return false;
   }
 
   // Split critical edges, and update the unavailable predecessors accordingly.
   for (BasicBlock *OrigPred : CriticalEdgePred) {
     BasicBlock *NewPred = splitCriticalEdges(OrigPred, LoadBB);
     assert(!PredLoads.count(OrigPred) && "Split edges shouldn't be in map!");
     PredLoads[NewPred] = nullptr;
if (NumWithout > 1 || NumWith == 0)
     return false;
 
   // We may have a case where all predecessors have the instruction,
   // and we just need to insert a phi node. Otherwise, perform
   // insertion.
   Instruction *PREInstr = nullptr;
 
   if (NumWithout != 0) {
-    if (!isSafeToSpeculativelyExecute(CurInst)) {
+    if (!isSafeToSpeculativelyExecute(CurInst,
+                                      /* PreservesOpCharacteristics */ true)) {
       // It is only valid to insert a new instruction if the current instruction
       // is always executed. An instruction with implicit control flow could
       // prevent us from doing it. If we cannot speculate the execution, then
       // PRE should be prohibited.
       if (ICF->isDominatedByICFIFromSameBlock(CurInst))
         return false;
     }
 

llvm/lib/Transforms/Scalar/GuardWidening.cpp

 
 bool GuardWideningImpl::canBeHoistedTo(
     const Value *V, const Instruction *Loc,
     SmallPtrSetImpl<const Instruction *> &Visited) const {
   auto *Inst = dyn_cast<Instruction>(V);
   if (!Inst || DT.dominates(Inst, Loc) || Visited.count(Inst))
     return true;
 
-  if (!isSafeToSpeculativelyExecute(Inst, Loc, &AC, &DT) ||
+  // TODO: We may be able to set PreservesOpCharacteristics to true. AFAICT the
+  // only reason its not possible is because of the assert in makeAvailableAt.
+  if (!isSafeToSpeculativelyExecute(
+          Inst, /* PreservesOpCharacteristics */ false, Loc, &AC, &DT) ||
       Inst->mayReadFromMemory())
     return false;
 
   Visited.insert(Inst);
 
   // We only want to go _up_ the dominance chain when recursing.
   assert(!isa<PHINode>(Loc) &&
          "PHIs should return false for isSafeToSpeculativelyExecute");
   assert(DT.isReachableFromEntry(Inst->getParent()) &&
          "We did a DFS from the block entry!");
   return all_of(Inst->operands(),
                 [&](Value *Op) { return canBeHoistedTo(Op, Loc, Visited); });
 }
 
 void GuardWideningImpl::makeAvailableAt(Value *V, Instruction *Loc) const {
   auto *Inst = dyn_cast<Instruction>(V);
   if (!Inst || DT.dominates(Inst, Loc))
     return;
 
-  assert(isSafeToSpeculativelyExecute(Inst, Loc, &AC, &DT) &&
+  assert(isSafeToSpeculativelyExecute(
+             Inst, /* PreservesOpCharacteristics */ false, Loc, &AC, &DT) &&
          !Inst->mayReadFromMemory() &&
          "Should've checked with canBeHoistedTo!");
 
   for (Value *Op : Inst->operands())
     makeAvailableAt(Op, Loc);
 
   Inst->moveBefore(Loc);
 }

llvm/lib/Transforms/Scalar/JumpThreading.cpp

bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
   // inserting a new instruction into them. It is only valid if all the
   // instructions before LoadI are guaranteed to pass execution to its
   // successor, or if LoadI is safe to speculate.
   // TODO: If this logic becomes more complex, and we will perform PRE insertion
   // farther than to a predecessor, we need to reuse the code from GVN's PRE.
   // It requires domination tree analysis, so for this simple case it is an
   // overkill.
   if (PredsScanned.size() != AvailablePreds.size() &&
-      !isSafeToSpeculativelyExecute(LoadI))
+      !isSafeToSpeculativelyExecute(LoadI,
+                                    /* PreservesOpCharacteristics */ true))
     for (auto I = LoadBB->begin(); &*I != LoadI; ++I)
       if (!isGuaranteedToTransferExecutionToSuccessor(&*I))
         return false;
 
   // If there is exactly one predecessor where the value is unavailable, the
   // already computed 'OneUnavailablePred' block is it.  If it ends in an
   // unconditional branch, we know that it isn't a critical edge.
   if (PredsScanned.size() == AvailablePreds.size()+1 &&

llvm/lib/Transforms/Scalar/LICM.cpp

 /// or if the instruction is known not to trap when moved to the preheader.
 /// or if it is a trapping instruction and is guaranteed to execute.
 static bool isSafeToExecuteUnconditionally(
     Instruction &Inst, const DominatorTree *DT, const TargetLibraryInfo *TLI,
     const Loop *CurLoop, const LoopSafetyInfo *SafetyInfo,
     OptimizationRemarkEmitter *ORE, const Instruction *CtxI,
     AssumptionCache *AC, bool AllowSpeculation) {
   if (AllowSpeculation &&
-      isSafeToSpeculativelyExecute(&Inst, CtxI, AC, DT, TLI))
+      isSafeToSpeculativelyExecute(&Inst, /* PreservesOpCharacteristics */ true,
+                                   CtxI, AC, DT, TLI))
     return true;
 
   bool GuaranteedToExecute =
       SafetyInfo->isGuaranteedToExecute(Inst, DT, CurLoop);
 
   if (!GuaranteedToExecute) {
     auto *LI = dyn_cast<LoadInst>(&Inst);
     if (LI && CurLoop->isLoopInvariant(LI->getPointerOperand()))

llvm/lib/Transforms/Scalar/LoopFlatten.cpp

checkOuterLoopInsts(FlattenInfo &FI,
   // iteration of the inner loop).
   InstructionCost RepeatedInstrCost = 0;
   for (auto *B : FI.OuterLoop->getBlocks()) {
     if (FI.InnerLoop->contains(B))
       continue;
 
     for (auto &I : *B) {
       if (!isa<PHINode>(&I) && !I.isTerminator() &&
-          !isSafeToSpeculativelyExecute(&I)) {
+          !isSafeToSpeculativelyExecute(
+              &I, /* PreservesOpCharacteristics */ true)) {
         LLVM_DEBUG(dbgs() << "Cannot flatten because instruction may have "
                              "side effects: ";
                    I.dump());
         return false;
       }
       // The execution count of the outer loop's iteration instructions
       // (increment, compare and branch) will be increased, but the
       // equivalent instructions will be removed from the inner loop, so

llvm/lib/Transforms/Scalar/LoopRerollPass.cpp

while (BaseIt != Uses.end() && RootIt != Uses.end()) {
           continue;
         if (I->mayWriteToMemory())
           AST.add(I);
         // Note: This is specifically guarded by a check on isa<PHINode>,
         // which while a valid (somewhat arbitrary) micro-optimization, is
         // needed because otherwise isSafeToSpeculativelyExecute returns
         // false on PHI nodes.
         if (!isa<PHINode>(I) && !isUnorderedLoadStore(I) &&
-            !isSafeToSpeculativelyExecute(I))
+            !isSafeToSpeculativelyExecute(
+                I, /* PreservesOpCharacteristics */ true))
           // Intervening instructions cause side effects.
           FutureSideEffects = true;
       }
 
       // Make sure that this instruction, which is in the use set of this
       // root instruction, does not also belong to the base set or the set of
       // some other root instruction.
       if (RootIt->second.count() > 1) {
while (BaseIt != Uses.end() && RootIt != Uses.end()) {
           }
         }
       }
 
       // If we've past an instruction from a future iteration that may have
       // side effects, and this instruction might also, then we can't reorder
       // them, and this matching fails. As an exception, we allow the alias
       // set tracker to handle regular (unordered) load/store dependencies.
-      if (FutureSideEffects && ((!isUnorderedLoadStore(BaseInst) &&
-                                 !isSafeToSpeculativelyExecute(BaseInst)) ||
-                                (!isUnorderedLoadStore(RootInst) &&
-                                 !isSafeToSpeculativelyExecute(RootInst)))) {
+      if (FutureSideEffects &&
+          ((!isUnorderedLoadStore(BaseInst) &&
+            !isSafeToSpeculativelyExecute(
+                BaseInst, /* PreservesOpCharacteristics */ true)) ||
+            !isSafeToSpeculativelyExecute(
+                RootInst, /* PreservesOpCharacteristics */ true)))) {
         LLVM_DEBUG(dbgs() << "LRR: iteration root match failed at " << *BaseInst
                           << " vs. " << *RootInst
                           << " (side effects prevent reordering)\n");
         return false;
       }
 
       // For instructions that are part of a reduction, if the operation is
       // associative, then don't bother matching the operands (because we

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) {
     }
     return true;
   };
 
   InstructionCost TotalSpeculationCost = 0;
   unsigned NotHoistedInstCount = 0;
   for (const auto &I : FromBlock) {
     const InstructionCost Cost = ComputeSpeculationCost(&I, *TTI);
-    if (Cost.isValid() && isSafeToSpeculativelyExecute(&I) &&
+    if (Cost.isValid() &&
+        isSafeToSpeculativelyExecute(&I,
+                                     /* PreservesOpCharacteristics */ false) &&
         AllPrecedingUsesFromBlockHoisted(&I)) {
       TotalSpeculationCost += Cost;
       if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
         return false;  // too much to hoist
     } else {
       // Debug info intrinsics should not be counted for threshold.
       if (!isa<DbgInfoIntrinsic>(I))
         NotHoistedInstCount++;

llvm/lib/Transforms/Utils/CodeMoverUtils.cpp

bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
   Instruction &EndInst = (MoveForward ? InsertPoint : I);
   SmallPtrSet<Instruction *, 10> InstsToCheck;
   collectInstructionsInBetween(StartInst, EndInst, InstsToCheck);
   if (!MoveForward)
     InstsToCheck.insert(&InsertPoint);
 
   // Check if there exists instructions which may throw, may synchonize, or may
   // never return, from I to InsertPoint.
-  if (!isSafeToSpeculativelyExecute(&I))
+  if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true))
     if (llvm::any_of(InstsToCheck, [](Instruction *I) {
           if (I->mayThrow())
             return true;
 
           const CallBase *CB = dyn_cast<CallBase>(I);
           if (!CB)
             return false;
           if (!CB->hasFnAttr(Attribute::WillReturn))

llvm/lib/Transforms/Utils/FlattenCFG.cpp

if (PP && Preds.count(PP)) {
       if (Pred->hasAddressTaken())
         return false;
 
       // Instructions in the internal condition blocks should be safe
       // to hoist up.
       for (BasicBlock::iterator BI = Pred->begin(), BE = PBI->getIterator();
            BI != BE;) {
         Instruction *CI = &*BI++;
-        if (isa<PHINode>(CI) || !isSafeToSpeculativelyExecute(CI))
+        // TODO: Can we set PreservesOpCharacteristics to true? Concern is
+        // `PreservesOpCharacteristics` might be based on dominating conditions
+        // which this transform might destroy.
+        if (isa<PHINode>(CI) || !isSafeToSpeculativelyExecute(
+                                    CI, /* PreservesOpCharacteristics */ false))
           return false;
       }
     } else {
       // This is the condition block to be merged into, e.g. BB1 in
       // both cases.
       if (FirstCondBlock)
         return false;
       FirstCondBlock = Pred;
bool FlattenCFGOpt::MergeIfRegion(BasicBlock *BB, IRBuilder<> &Builder) {
   Instruction *PTI2 = SecondEntryBlock->getTerminator();
   Instruction *PBI2 = &SecondEntryBlock->front();
 
   // Check whether \param SecondEntryBlock has side-effect and is safe to
   // speculate.
   for (BasicBlock::iterator BI(PBI2), BE(PTI2); BI != BE; ++BI) {
     Instruction *CI = &*BI;
     if (isa<PHINode>(CI) || CI->mayHaveSideEffects() ||
-        !isSafeToSpeculativelyExecute(CI))
+        !isSafeToSpeculativelyExecute(CI,
+                                      /* PreservesOpCharacteristics */ true))
       return false;
   }
 
   // Merge \param SecondEntryBlock into \param FirstEntryBlock.
   FirstEntryBlock->back().eraseFromParent();
   FirstEntryBlock->splice(FirstEntryBlock->end(), SecondEntryBlock);
   BranchInst *PBI = cast<BranchInst>(FirstEntryBlock->getTerminator());
   assert(PBI->getCondition() == CInst2);

llvm/lib/Transforms/Utils/LoopRotationUtils.cpp

static bool shouldSpeculateInstrs(BasicBlock::iterator Begin,
   bool seenIncrement = false;
   bool MultiExitLoop = false;
 
   if (!L->getExitingBlock())
     MultiExitLoop = true;
 
   for (BasicBlock::iterator I = Begin; I != End; ++I) {
 
-    if (!isSafeToSpeculativelyExecute(&*I))
+    if (!isSafeToSpeculativelyExecute(&*I,
+                                      /* PreservesOpCharacteristics */ true))
       return false;
 
     if (isa<DbgInfoIntrinsic>(I))
       continue;
 
     switch (I->getOpcode()) {
     default:
       return false;

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

 
 /// Compute an abstract "cost" of speculating the given instruction,
 /// which is assumed to be safe to speculate. TCC_Free means cheap,
 /// TCC_Basic means less cheap, and TCC_Expensive means prohibitively
 /// expensive.
 static InstructionCost computeSpeculationCost(const User *I,
                                               const TargetTransformInfo &TTI) {
   assert((!isa<Instruction>(I) ||
-          isSafeToSpeculativelyExecute(cast<Instruction>(I))) &&
+          isSafeToSpeculativelyExecute(
+              cast<Instruction>(I), /* PreservesOpCharacteristics */ false)) &&
          "Instruction is not safe to speculatively execute!");
   return TTI.getInstructionCost(I, TargetTransformInfo::TCK_SizeAndLatency);
 }
 
 /// If we have a merge point of an "if condition" as accepted above,
 /// return true if the specified value dominates the block.  We
 /// don't handle the true generality of domination here, just a special case
 /// which works well enough for us.
static bool dominatesMergePoint(Value *V, BasicBlock *BB,
 
   // If we have seen this instruction before, don't count it again.
   if (AggressiveInsts.count(I))
     return true;
 
   // Okay, it looks like the instruction IS in the "condition".  Check to
   // see if it's a cheap instruction to unconditionally compute, and if it
   // only uses stuff defined outside of the condition.  If so, hoist it out.
-  if (!isSafeToSpeculativelyExecute(I))
+  if (!isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ false))
     return false;
 
   Cost += computeSpeculationCost(I, TTI);
 
   // Allow exactly one instruction to be speculated regardless of its cost
   // (as long as it is safe to do so).
   // This is intended to flatten the CFG even if the instruction is a division
   // or other expensive operation. The speculation of an expensive instruction
static bool isSafeToHoistInstr(Instruction *I, unsigned Flags) {
   // If we have seen an instruction with side effects, it's unsafe to reorder an
   // instruction which reads memory or itself has side effects.
   if ((Flags & SkipSideEffect) &&
       (I->mayReadFromMemory() || I->mayHaveSideEffects() || isa<AllocaInst>(I)))
     return false;
 
   // Reordering across an instruction which does not necessarily transfer
   // control to the next instruction is speculation.
-  if ((Flags & SkipImplicitControlFlow) && !isSafeToSpeculativelyExecute(I))
+  if ((Flags & SkipImplicitControlFlow) &&
+      !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true))
     return false;
 
   // Hoisting of llvm.deoptimize is only legal together with the next return
   // instruction, which this pass is not always able to do.
   if (auto *CB = dyn_cast<CallBase>(I))
     if (CB->getIntrinsicID() == Intrinsic::experimental_deoptimize)
       return false;
 
if (!followedByDeoptOrUnreachable) {
       // predecessors. However, if not all predecessors are unconditional,
       // this transformation might be pessimizing. So as a rule of thumb,
       // don't do it unless we'd sink at least one non-speculatable instruction.
       // See https://bugs.llvm.org/show_bug.cgi?id=30244
       LRI.reset();
       int Idx = 0;
       bool Profitable = false;
       while (Idx < ScanIdx) {
-        if (!isSafeToSpeculativelyExecute((*LRI)[0])) {
+        if (!isSafeToSpeculativelyExecute(
+                (*LRI)[0], /* PreservesOpCharacteristics */ true)) {
           Profitable = true;
           break;
         }
         --LRI;
         ++Idx;
       }
       if (!Profitable)
         return false;
for (Instruction &I : reverse(drop_end(*ThenBB))) {
 
     // Only speculatively execute a single instruction (not counting the
     // terminator) for now.
     ++SpeculatedInstructions;
     if (SpeculatedInstructions > 1)
       return false;
 
     // Don't hoist the instruction if it's unsafe or expensive.
-    if (!isSafeToSpeculativelyExecute(&I) &&
+    if (!isSafeToSpeculativelyExecute(&I,
+                                      /* PreservesOpCharacteristics */ false) &&
         !(HoistCondStores && (SpeculatedStoreValue = isSafeToSpeculateStore(
                                   &I, BB, ThenBB, EndBB))))
       return false;
     if (!SpeculatedStoreValue &&
         computeSpeculationCost(&I, TTI) >
             PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic)
       return false;
 
bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
   for (Instruction &I : *BB) {
     // Don't check the branch condition comparison itself.
     if (&I == Cond)
       continue;
     // Ignore dbg intrinsics, and the terminator.
     if (isa<DbgInfoIntrinsic>(I) || isa<BranchInst>(I))
       continue;
     // I must be safe to execute unconditionally.
-    if (!isSafeToSpeculativelyExecute(&I))
+    if (!isSafeToSpeculativelyExecute(&I,
+                                      /* PreservesOpCharacteristics */ false))
       return false;
     SawVectorOp |= isVectorOp(I);
 
     // Account for the cost of duplicating this instruction into each
     // predecessor. Ignore free instructions.
     if (!TTI || TTI->getInstructionCost(&I, CostKind) !=
                     TargetTransformInfo::TCC_Free) {
       NumBonusInsts += PredCount;

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

case Instruction::Store: {
     return true;
   }
   case Instruction::UDiv:
   case Instruction::SDiv:
   case Instruction::SRem:
   case Instruction::URem:
     // TODO: We can use the loop-preheader as context point here and get
     // context sensitive reasoning
-    return !isSafeToSpeculativelyExecute(I);
+    return !isSafeToSpeculativelyExecute(I,
+                                         /* PreservesOpCharacteristics */ true);
   case Instruction::Call:
     return Legal->isMaskRequired(I);
   }
 }
 
 std::pair<InstructionCost, InstructionCost>
 LoopVectorizationCostModel::getDivRemSpeculationCost(Instruction *I,
                                                     ElementCount VF) const {
   assert(I->getOpcode() == Instruction::UDiv ||
          I->getOpcode() == Instruction::SDiv ||
          I->getOpcode() == Instruction::SRem ||
          I->getOpcode() == Instruction::URem);
-  assert(!isSafeToSpeculativelyExecute(I));
+  assert(
+      !isSafeToSpeculativelyExecute(I, /* PreservesOpCharacteristics */ true));
 
   const TTI::TargetCostKind CostKind = TTI::TCK_RecipThroughput;
 
   // Scalarization isn't legal for scalable vector types
   InstructionCost ScalarizationCost = InstructionCost::getInvalid();
   if (!VF.isScalable()) {
     // Get the scalarization cost and scale this amount by the probability of
     // executing the predicated block. If the instruction is not predicated,

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

for (ScheduleData *BundleMember = SD; BundleMember;
       };
 
       // Any instruction which isn't safe to speculate at the beginning of the
       // block is control dependend on any early exit or non-willreturn call
       // which proceeds it.
       if (!isGuaranteedToTransferExecutionToSuccessor(BundleMember->Inst)) {
         for (Instruction *I = BundleMember->Inst->getNextNode();
              I != ScheduleEnd; I = I->getNextNode()) {
-          if (isSafeToSpeculativelyExecute(I, &*BB->begin(), SLP->AC))
+          if (isSafeToSpeculativelyExecute(
+                  I, /* PreservesOpCharacteristics */ true, &*BB->begin(),
+                  SLP->AC))
             continue;
 
           // Add the dependency
           makeControlDependent(I);
 
           if (!isGuaranteedToTransferExecutionToSuccessor(I))
             // Everything past here must be control dependent on I.
             break;

llvm/lib/Transforms/Vectorize/VectorCombine.cpp

void VectorCombine::foldExtExtBinop(ExtractElementInst *Ext0,
   Value *NewExt = Builder.CreateExtractElement(VecBO, Ext0->getIndexOperand());
   replaceValue(I, *NewExt);
 }
 
 /// Match an instruction with extracted vector operands.
 bool VectorCombine::foldExtractExtract(Instruction &I) {
   // It is not safe to transform things like div, urem, etc. because we may
   // create undefined behavior when executing those on unknown vector elements.
-  if (!isSafeToSpeculativelyExecute(&I))
+  if (!isSafeToSpeculativelyExecute(&I, /* PreservesOpCharacteristics */ true))
     return false;
 
   Instruction *I0, *I1;
   CmpInst::Predicate Pred = CmpInst::BAD_ICMP_PREDICATE;
   if (!match(&I, m_Cmp(Pred, m_Instruction(I0), m_Instruction(I1))) &&
       !match(&I, m_BinOp(m_Instruction(I0), m_Instruction(I1))))
     return false;
 

llvm/test/Transforms/LICM/speculate-div.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=licm -S | FileCheck %s
 
 declare void @maythrow()
 declare void @use(i16)
 
-define void @sdiv_not_ok(i16 %n, i16 %xx) {
+define void @sdiv_not_ok(i16 %n, i16 noundef %xx) {
 ; CHECK-LABEL: @sdiv_not_ok(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
 ; CHECK-NEXT:    [[DIV:%.*]] = sdiv i16 [[N:%.*]], [[X]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %x = or i16 %xx, 1
   br label %loop
 loop:
   call void @maythrow()
   %div = sdiv i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }
 
-define void @srem_not_ok2(i16 %nn, i16 %x) {
+define void @srem_not_ok2(i16 %nn, i16 noundef %x) {
 ; CHECK-LABEL: @srem_not_ok2(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[N:%.*]] = and i16 [[NN:%.*]], 1323
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
 ; CHECK-NEXT:    [[DIV:%.*]] = srem i16 [[N]], [[X:%.*]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %n = and i16 %nn, 1323
   br label %loop
 loop:
   call void @maythrow()
   %div = srem i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }
 
-define void @sdiv_ok(i16 %n, i16 %xx) {
+define void @sdiv_ok(i16 %n, i16 noundef %xx) {
 ; CHECK-LABEL: @sdiv_ok(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[XO:%.*]] = or i16 [[XX:%.*]], 1
 ; CHECK-NEXT:    [[X:%.*]] = and i16 [[XO]], 123
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv i16 [[N:%.*]], [[X]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
-; CHECK-NEXT:    [[DIV:%.*]] = sdiv i16 [[N:%.*]], [[X]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %xo = or i16 %xx, 1
   %x = and i16 %xo, 123
   br label %loop
 loop:
   call void @maythrow()
   %div = sdiv i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }
 
-define void @srem_ok2(i16 %nn, i16 %xx) {
+define void @srem_ok2(i16 noundef %nn, i16 noundef %xx) {
 ; CHECK-LABEL: @srem_ok2(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[N:%.*]] = and i16 [[NN:%.*]], 123
 ; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
+; CHECK-NEXT:    [[DIV:%.*]] = srem i16 [[N]], [[X]]
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
-; CHECK-NEXT:    [[DIV:%.*]] = srem i16 [[N]], [[X]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %n = and i16 %nn, 123
   %x = or i16 %xx, 1
   br label %loop
 loop:
   call void @maythrow()
   %div = srem i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }
 
-define void @udiv_not_ok(i16 %n, i16 %xx) {
+define void @sdiv_not_ok3_maybe_poison_denum(i16 noundef %nn, i16 %xx) {
+; CHECK-LABEL: @sdiv_not_ok3_maybe_poison_denum(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N:%.*]] = and i16 [[NN:%.*]], 123
+; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    call void @maythrow()
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv i16 [[N]], [[X]]
+; CHECK-NEXT:    call void @use(i16 [[DIV]])
+; CHECK-NEXT:    br label [[LOOP]]
+;
+entry:
+  %n = and i16 %nn, 123
+  %x = or i16 %xx, 1
+  br label %loop
+loop:
+  call void @maythrow()
+  %div = sdiv i16 %n, %x
+  call void @use(i16 %div)
+  br label %loop
+}
+
+define void @sdiv_not_ok3_maybe_poison_num(i16 %nn, i16 noundef  %xx) {
+; CHECK-LABEL: @sdiv_not_ok3_maybe_poison_num(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[N:%.*]] = and i16 [[NN:%.*]], 123
+; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    call void @maythrow()
+; CHECK-NEXT:    [[DIV:%.*]] = sdiv i16 [[N]], [[X]]
+; CHECK-NEXT:    call void @use(i16 [[DIV]])
+; CHECK-NEXT:    br label [[LOOP]]
+;
+entry:
+  %n = and i16 %nn, 123
+  %x = or i16 %xx, 1
+  br label %loop
+loop:
+  call void @maythrow()
+  %div = sdiv i16 %n, %x
+  call void @use(i16 %div)
+  br label %loop
+}
+
+define void @udiv_not_ok(i16 %n, i16 noundef %xx) {
 ; CHECK-LABEL: @udiv_not_ok(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[X:%.*]] = xor i16 [[XX:%.*]], 1
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
 ; CHECK-NEXT:    [[DIV:%.*]] = udiv i16 [[N:%.*]], [[X]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %x = xor i16 %xx, 1
   br label %loop
 loop:
   call void @maythrow()
   %div = udiv i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }
 
-define void @udiv_ok(i16 %n, i16 %xx) {
+define void @udiv_ok(i16 %n, i16 noundef %xx) {
 ; CHECK-LABEL: @udiv_ok(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
+; CHECK-NEXT:    [[DIV:%.*]] = udiv i16 [[N:%.*]], [[X]]
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    call void @maythrow()
+; CHECK-NEXT:    call void @use(i16 [[DIV]])
+; CHECK-NEXT:    br label [[LOOP]]
+;
+entry:
+  %x = or i16 %xx, 1
+  br label %loop
+loop:
+  call void @maythrow()
+  %div = udiv i16 %n, %x
+  call void @use(i16 %div)
+  br label %loop
+}
+
+define void @urem_not_ok_maybe_poison(i16 %n, i16 %xx) {
+; CHECK-LABEL: @urem_not_ok_maybe_poison(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[X:%.*]] = or i16 [[XX:%.*]], 1
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; CHECK-NEXT:    call void @maythrow()
 ; CHECK-NEXT:    [[DIV:%.*]] = udiv i16 [[N:%.*]], [[X]]
 ; CHECK-NEXT:    call void @use(i16 [[DIV]])
 ; CHECK-NEXT:    br label [[LOOP]]
 ;
 entry:
   %x = or i16 %xx, 1
   br label %loop
 loop:
   call void @maythrow()
   %div = udiv i16 %n, %x
   call void @use(i16 %div)
   br label %loop
 }