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

[GlobalOpt] Precommit tests for PR84694 #87443

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Conversation

gandhi56
Copy link
Contributor

@gandhi56 gandhi56 commented Apr 3, 2024

PR link: #84694

@gandhi56
Copy link
Contributor Author

gandhi56 commented Apr 6, 2024

ping

@gandhi56 gandhi56 requested review from nikic and dtcxzyw April 8, 2024 07:52
Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting off the tests. Would it make sense to have them all in a single file?

Also would be good to add a prefix like [GlobalOpt] to the title.

}

;; This test includes a memcpy with @c as it's source and destination
;; operands. CleanupPointerRootUsers is not called in this case.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to avoid mentioning internal functions here and in the name of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, where is this?

@gandhi56 gandhi56 changed the title Precommit tests for PR84694 [GlobalOpt] Precommit tests for PR84694 Apr 10, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 11, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Anshil Gandhi (gandhi56)

Changes

PR link: #84694


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

2 Files Affected:

  • (added) llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-negative.ll (+26)
  • (added) llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-ops.ll (+71)
diff --git a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-negative.ll b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-negative.ll
new file mode 100644
index 00000000000000..97504ffb3fd59d
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-negative.ll
@@ -0,0 +1,26 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=globalopt < %s -S | FileCheck %s
+
+@gv = internal unnamed_addr global [3 x ptr] zeroinitializer, align 16
+@gv2 = internal unnamed_addr global i32 0, align 4
+
+;; This test includes a load from @gv. No stores
+;; or memintrinsics with destination @gv should be removed.
+define i32 @main_with_load_from_b() local_unnamed_addr {
+; CHECK-LABEL: define i32 @main_with_load_from_b() local_unnamed_addr {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[E:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    store ptr [[E]], ptr getelementptr inbounds ([3 x ptr], ptr @gv, i64 0, i64 2), align 16
+; CHECK-NEXT:    [[LOAD_B:%.*]] = load ptr, ptr getelementptr inbounds ([3 x ptr], ptr @gv, i64 0, i64 2), align 16
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr @gv, ptr @gv2, i64 8, i1 false)
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  %e = alloca i32, align 4
+  store ptr %e, ptr getelementptr inbounds ([3 x ptr], ptr @gv, i64 0, i64 2), align 16
+  %load.b = load ptr, ptr getelementptr inbounds ([3 x ptr], ptr @gv, i64 0, i64 2), align 16
+  call void @llvm.memcpy.p0i8.p0i8.i64(ptr getelementptr inbounds ([3 x ptr], ptr @gv, i64 0, i64 0), ptr @gv2, i64 8, i1 false)
+  ret i32 0
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(ptr nocapture, ptr nocapture readonly, i64, i1) local_unnamed_addr
diff --git a/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-ops.ll b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-ops.ll
new file mode 100644
index 00000000000000..6b3d0885809dfc
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/cleanup-pointer-root-users-ops.ll
@@ -0,0 +1,71 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=globalopt < %s -S | FileCheck %s
+
+@a = internal unnamed_addr global i32 0, align 4
+@b = internal unnamed_addr global [3 x ptr] zeroinitializer, align 16
+
+;; This test is extracted from the issue reported in #64680, with an
+;; additional memcpy and a memset. Ensure all stores and memintrinsics with
+;; destination @b are removed as @b is dead.
+define i32 @main() local_unnamed_addr {
+; CHECK-LABEL: define i32 @main() local_unnamed_addr {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[E:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[DOTPR:%.*]] = load i32, ptr @a, align 4
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp slt i32 [[DOTPR]], 3
+; CHECK-NEXT:    br i1 [[CMP1]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    store i32 8, ptr [[E]], align 4
+; CHECK-NEXT:    call void @bar()
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[E]], align 4
+; CHECK-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL_NOT]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    store ptr [[E]], ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 2), align 16
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr @a, align 4
+; CHECK-NEXT:    [[INC:%.*]] = add nsw i32 [[TMP1]], 1
+; CHECK-NEXT:    store i32 [[INC]], ptr @a, align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[TMP1]], 2
+; CHECK-NEXT:    br i1 [[CMP]], label [[FOR_BODY]], label [[FOR_END]]
+; CHECK:       for.end:
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr @b, ptr null, i64 8, i1 false)
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  %e = alloca i32, align 4
+  %.pr = load i32, ptr @a, align 4
+  %cmp1 = icmp slt i32 %.pr, 3
+  br i1 %cmp1, label %for.body, label %for.end
+
+for.body:                                         ; preds = %entry, %if.end
+  store i32 8, ptr %e, align 4
+  call void @bar()
+  %0 = load i32, ptr %e, align 4
+  %tobool.not = icmp eq i32 %0, 0
+  br i1 %tobool.not, label %if.then, label %if.end
+
+if.then:                                          ; preds = %for.body
+  call void @foo()
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %for.body
+  store ptr %e, ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 2), align 16
+  %1 = load i32, ptr @a, align 4
+  %inc = add nsw i32 %1, 1
+  store i32 %inc, ptr @a, align 4
+  %cmp = icmp slt i32 %1, 2
+  call void @llvm.memset.p0i8.i64(ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 0), i8 0, i64 8, i1 false)
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %if.end, %entry
+  call void @llvm.memcpy.p0i8.p0i8.i64(ptr getelementptr inbounds ([3 x ptr], ptr @b, i64 0, i64 0), ptr null, i64 8, i1 false)
+  ret i32 0
+}
+
+declare void @bar() local_unnamed_addr
+declare void @foo() local_unnamed_addr
+declare void @llvm.memcpy.p0i8.p0i8.i64(ptr nocapture, ptr nocapture readonly, i64, i1) local_unnamed_addr
+declare void @llvm.memset.p0i8.i64(ptr nocapture, i8, i64, i1) local_unnamed_addr

@gandhi56
Copy link
Contributor Author

Thanks for splitting off the tests. Would it make sense to have them all in a single file?

Also would be good to add a prefix like [GlobalOpt] to the title.

Done, I figured it would be best to add these tests in the existing test file specific to gep and constexpr operators.

@gandhi56
Copy link
Contributor Author

ping

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks (looks like GH won't send emails when a conversation has been resolved)

@gandhi56 gandhi56 merged commit 39bfdb7 into llvm:main Apr 18, 2024
4 checks passed
@gandhi56
Copy link
Contributor Author

That's a bummer, thanks for the review!

@gandhi56 gandhi56 deleted the precommit-pr84694 branch April 18, 2024 22:08
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

3 participants