Skip to content

[NewGVN] Precommit test #141362

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ManuelJBrito
Copy link
Contributor

Following 14dee0a and 3416d4f the first function gets miscompiled.

@llvmbot
Copy link
Member

llvmbot commented May 24, 2025

@llvm/pr-subscribers-llvm-transforms

Author: None (ManuelJBrito)

Changes

Following 14dee0a and 3416d4f the first function gets miscompiled.


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

1 Files Affected:

  • (added) llvm/test/Transforms/NewGVN/coercion-different-ptr.ll (+53)
diff --git a/llvm/test/Transforms/NewGVN/coercion-different-ptr.ll b/llvm/test/Transforms/NewGVN/coercion-different-ptr.ll
new file mode 100644
index 0000000000000..c6eb4a2cd179b
--- /dev/null
+++ b/llvm/test/Transforms/NewGVN/coercion-different-ptr.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=newgvn -S | FileCheck %s
+
+
+; FIXME: MemorySSA says that load1 depends on the lifetime start.
+; That's OK since MemorySSA is may-alias; however, NewGVN should
+; check whether the lifetime start *actually* defines the loaded pointer
+; before simplifying to uninitialized memory.
+define void @foo(ptr %arg) {
+; CHECK-LABEL: define void @main(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT:  [[BB:.*:]]
+; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca i8, align 16
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 1, ptr [[ALLOCA]])
+; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[ARG]], align 8
+; CHECK-NEXT:    [[CALL:%.*]] = call ptr undef(ptr [[ALLOCA]])
+; CHECK-NEXT:    ret void
+;
+bb:
+  %alloca = alloca i8, align 16
+  call void @llvm.lifetime.start.p0(i64 1, ptr %alloca)
+  %load = load ptr, ptr %arg, align 8
+  %load1 = load ptr, ptr %load, align 8
+  %call = call ptr %load1(ptr %alloca)
+  ret void
+}
+
+declare void @llvm.lifetime.start.p0(i64 immarg, ptr captures(none)) #0
+
+declare ptr @malloc(i64)
+
+; This case is handled correctly. 
+; Since malloc returns a pointer, NewGVN checks whether
+; it is the same pointer, an equivalent one, or a must-alias.
+define void @wombat(ptr %arg) {
+; CHECK-LABEL: define void @wombat(
+; CHECK-SAME: ptr [[ARG:%.*]]) {
+; CHECK-NEXT:  [[BB:.*:]]
+; CHECK-NEXT:    [[CALL:%.*]] = call ptr @malloc(i64 1)
+; CHECK-NEXT:    [[LOAD:%.*]] = load ptr, ptr [[ARG]], align 8
+; CHECK-NEXT:    [[LOAD1:%.*]] = load ptr, ptr [[LOAD]], align 8
+; CHECK-NEXT:    [[CALL2:%.*]] = call ptr [[LOAD1]](ptr [[CALL]])
+; CHECK-NEXT:    ret void
+;
+bb:
+  %call = call ptr @malloc(i64 1)
+  %load = load ptr, ptr %arg, align 8
+  %load1 = load ptr, ptr %load, align 8
+  %call2 = call ptr %load1(ptr %call)
+  ret void
+}
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn memory(argmem: readwrite) }

Copy link

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/test/Transforms/NewGVN/coercion-different-ptr.ll

The following files introduce new uses of undef:

  • llvm/test/Transforms/NewGVN/coercion-different-ptr.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

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.

3 participants