Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dse] Skip llvm.allow.{runtime,ubsan}.check() #86067

Draft
wants to merge 1 commit into
base: users/vitalybuka/spr/main.dse-skip-llvmallowruntimeubsancheck
Choose a base branch
from

Conversation

vitalybuka
Copy link
Collaborator

Doing this because it's similar to assume, but I
can't see a difference neither for assume nor for
allow.

RFC: https://discourse.llvm.org/t/rfc-add-llvm-experimental-hot-intrinsic-or-llvm-hot/77641

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Vitaly Buka (vitalybuka)

Changes

Doing this because it's similar to assume, but I
can't see a difference neither for assume nor for
allow.

RFC: https://discourse.llvm.org/t/rfc-add-llvm-experimental-hot-intrinsic-or-llvm-hot/77641


Full diff: https://github.com/llvm/llvm-project/pull/86067.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp (+2)
  • (modified) llvm/test/Transforms/DeadStoreElimination/libcalls.ll (+17)
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index bfc8bd5970bf27..5a9ea420392f75 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -760,6 +760,8 @@ namespace {
 bool isNoopIntrinsic(Instruction *I) {
   if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
     switch (II->getIntrinsicID()) {
+    case Intrinsic::allow_runtime_check:
+    case Intrinsic::allow_ubsan_check:
     case Intrinsic::lifetime_start:
     case Intrinsic::lifetime_end:
     case Intrinsic::invariant_end:
diff --git a/llvm/test/Transforms/DeadStoreElimination/libcalls.ll b/llvm/test/Transforms/DeadStoreElimination/libcalls.ll
index 4d9a767e08d490..7a4908d550a96a 100644
--- a/llvm/test/Transforms/DeadStoreElimination/libcalls.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/libcalls.ll
@@ -487,3 +487,20 @@ define void @dse_strncpy_test6(ptr noalias %out1, ptr noalias %out2, ptr noalias
   %call = tail call ptr @strncpy(ptr %out2, ptr %in, i64 100)
   ret void
 }
+
+define i32 @test_strcat_with_allow_check(ptr %src) {
+; CHECK-LABEL: @test_strcat_with_allow_check(
+; CHECK-NEXT:    [[ALLOW1:%.*]] = call i1 @llvm.allow.runtime.check(metadata !"test_check")
+; CHECK-NEXT:    [[ALLOW2:%.*]] = call i1 @llvm.allow.ubsan.check(i8 7)
+; CHECK-NEXT:    tail call void @llvm.memset.p0.i64(ptr [[B:%.*]], i8 44, i64 16, i1 false)
+; CHECK-NEXT:    [[RET:%.*]] = load i32, ptr [[B]], align 4
+; CHECK-NEXT:    ret i32 [[RET]]
+;
+  tail call void @llvm.memset.p0.i64(ptr %src, i8 42, i64 16, i1 false)
+  %allow1 = call i1 @llvm.allow.runtime.check(metadata !"test_check")
+  tail call void @llvm.memset.p0.i64(ptr %src, i8 43, i64 16, i1 false)
+  %allow2 = call i1 @llvm.allow.ubsan.check(i8 7)
+  tail call void @llvm.memset.p0.i64(ptr %src, i8 44, i64 16, i1 false)
+  %ret = load i32, ptr %src, align 4
+  ret i32 %ret
+}

@nikic
Copy link
Contributor

nikic commented Mar 21, 2024

I don't think that this change is necessary. The InaccessibleMemOnly modelling will take care of it.

@vitalybuka
Copy link
Collaborator Author

vitalybuka commented Mar 21, 2024

I don't think that this change is necessary. The InaccessibleMemOnly modelling will take care of it.

Thanks. That makes sense. As I wrote in description, I tried to trigger code difference in tests, but failed for these and for assume.
Converting to draft.

@vitalybuka vitalybuka marked this pull request as draft March 21, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants