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

[via llvm-commits]

Author: Nikita Popov
Date: 2023-10-19T09:01:48+02:00
New Revision: a3bbab18527b57c19ef0d55c68727af01ed55b1e

URL: https://github.com/llvm/llvm-project/commit/a3bbab18527b57c19ef0d55c68727af01ed55b1e
DIFF: https://github.com/llvm/llvm-project/commit/a3bbab18527b57c19ef0d55c68727af01ed55b1e.diff

LOG: [IR] Don't mark experimental.guard as willreturn (#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.

Added: 
    

Modified: 
    llvm/include/llvm/IR/Intrinsics.td
    llvm/lib/Transforms/Utils/Local.cpp
    llvm/test/Transforms/Attributor/lvi-after-jumpthreading.ll
    llvm/test/Transforms/InstCombine/sdiv-guard.ll

Removed: 
    


################################################################################
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]]
 ;