[llvm] [IR] Don't mark experimental.guard as willreturn (PR #69433)

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/69433

Control flow does not necessary continue past guard intrinsics, so don't mark them as willreturn.

This fixes the miscompile in the sdiv-guard.ll test.

>From 892cd89da74edecf6c0a7a44ebcacef4128f3e65 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 18 Oct 2023 10:03:31 +0200
Subject: [PATCH] [IR] Don't mark experimental.guard as willreturn

Control flow does not necessary continue past guard intrinsics,
so don't mark them as willreturn.
---
 llvm/include/llvm/IR/Intrinsics.td            |  2 +-
 llvm/lib/Transforms/Utils/Local.cpp           | 19 +++++++++++--------
 .../Attributor/lvi-after-jumpthreading.ll     |  1 -
 .../test/Transforms/InstCombine/sdiv-guard.ll |  5 +++--
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index ab15b1f1e0ee888..b22da112f578fd6 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1697,7 +1697,7 @@ def int_experimental_deoptimize : Intrinsic<[llvm_any_ty], [llvm_vararg_ty],
                                             [Throws]>;
 
 // Support for speculative runtime guards
-def int_experimental_guard : DefaultAttrsIntrinsic<[], [llvm_i1_ty, llvm_vararg_ty],
+def int_experimental_guard : Intrinsic<[], [llvm_i1_ty, llvm_vararg_ty],
                                        [Throws]>;
 
 // Supports widenable conditions for guards represented as explicit branches.
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index ddb47e693a643d8..e0467a319caa302 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -443,9 +443,16 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
     if (!II)
       return false;
 
+    switch (II->getIntrinsicID()) {
+    case Intrinsic::experimental_guard: {
+      // Guards on true are operationally no-ops.  In the future we can
+      // consider more sophisticated tradeoffs for guards considering potential
+      // for check widening, but for now we keep things simple.
+      auto *Cond = dyn_cast<ConstantInt>(II->getArgOperand(0));
+      return Cond && Cond->isOne();
+    }
     // TODO: These intrinsics are not safe to remove, because this may remove
     // a well-defined trap.
-    switch (II->getIntrinsicID()) {
     case Intrinsic::wasm_trunc_signed:
     case Intrinsic::wasm_trunc_unsigned:
     case Intrinsic::ptrauth_auth:
@@ -484,13 +491,9 @@ bool llvm::wouldInstructionBeTriviallyDead(const Instruction *I,
       return false;
     }
 
-    // Assumptions are dead if their condition is trivially true.  Guards on
-    // true are operationally no-ops.  In the future we can consider more
-    // sophisticated tradeoffs for guards considering potential for check
-    // widening, but for now we keep things simple.
-    if ((II->getIntrinsicID() == Intrinsic::assume &&
-         isAssumeWithEmptyBundle(cast<AssumeInst>(*II))) ||
-        II->getIntrinsicID() == Intrinsic::experimental_guard) {
+    // Assumptions are dead if their condition is trivially true.
+    if (II->getIntrinsicID() == Intrinsic::assume &&
+        isAssumeWithEmptyBundle(cast<AssumeInst>(*II))) {
       if (ConstantInt *Cond = dyn_cast<ConstantInt>(II->getArgOperand(0)))
         return !Cond->isZero();
 
diff --git a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
index 562b1a4a7182537..2aa95216a665694 100644
--- a/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
+++ b/llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
@@ -185,7 +185,6 @@ declare void @llvm.experimental.guard(i1, ...)
 ; CHECK: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
 ; CHECK: attributes #[[ATTR1:[0-9]+]] = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) }
 ; CHECK: attributes #[[ATTR2]] = { nounwind }
-; CHECK: attributes #[[ATTR3:[0-9]+]] = { nocallback nofree nosync willreturn }
 ;.
 ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
 ; CGSCC: {{.*}}
diff --git a/llvm/test/Transforms/InstCombine/sdiv-guard.ll b/llvm/test/Transforms/InstCombine/sdiv-guard.ll
index ba9670924108b19..cff2f6aefda06a3 100644
--- a/llvm/test/Transforms/InstCombine/sdiv-guard.ll
+++ b/llvm/test/Transforms/InstCombine/sdiv-guard.ll
@@ -6,8 +6,9 @@ declare void @llvm.experimental.guard(i1, ...)
 ; Regression test. If %flag is false then %s == 0 and guard should be triggered.
 define i32 @a(i1 %flag, i32 %X) nounwind readnone {
 ; CHECK-LABEL: @a(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[X:%.*]], 0
-; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 [[CMP]]) #[[ATTR2:[0-9]+]] [ "deopt"() ]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[X:%.*]], 0
+; CHECK-NEXT:    [[CMP:%.*]] = select i1 [[FLAG:%.*]], i1 [[CMP1]], i1 false
+; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 [[CMP]]) #[[ATTR1:[0-9]+]] [ "deopt"() ]
 ; CHECK-NEXT:    [[R:%.*]] = sdiv i32 100, [[X]]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;