Skip to content

[GVN] Handle empty attrs in Expression == #115761

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

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

macurtis-amd
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (macurtis-amd)

Changes

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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+1-1)
  • (added) llvm/test/Transforms/GVN/intersect-empty-attr.ll (+19)
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index e27f5a08c85a97..a3df7a0a39e6b2 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -156,7 +156,7 @@ struct llvm::GVNPass::Expression {
       return false;
     if (varargs != other.varargs)
       return false;
-    if (!attrs.isEmpty() && !other.attrs.isEmpty() &&
+    if ((!attrs.isEmpty() || !other.attrs.isEmpty()) &&
         !attrs.intersectWith(type->getContext(), other.attrs).has_value())
       return false;
     return true;
diff --git a/llvm/test/Transforms/GVN/intersect-empty-attr.ll b/llvm/test/Transforms/GVN/intersect-empty-attr.ll
new file mode 100644
index 00000000000000..013b01c700ce4e
--- /dev/null
+++ b/llvm/test/Transforms/GVN/intersect-empty-attr.ll
@@ -0,0 +1,19 @@
+; RUN: opt -S -passes=gvn < %s | FileCheck %s
+
+declare i32 @bar() #0
+
+define i32 @foo() {
+entry:
+  %0 = tail call i32 @bar() #1
+  %1 = tail call i32 @bar()
+  ret i32 1
+}
+
+; CHECK-LABEL:    define i32 @foo(
+; CHECK:            %0 = tail call i32 @bar() #1
+; CHECK-NEXT:       %1 = tail call i32 @bar()
+; CHECK-NEXT:       ret i32 1
+; CHECK-NEXT:     }
+
+attributes #0 = { memory(none) }
+attributes #1 = { "llvm.assume"="ompx_no_call_asm" }

@dtcxzyw dtcxzyw requested a review from nikic November 11, 2024 23:55
@dtcxzyw
Copy link
Member

dtcxzyw commented Nov 11, 2024

Does it address the crash reported in #114545 (comment)?
cc @ronlieb

@ronlieb
Copy link
Contributor

ronlieb commented Nov 12, 2024

Does it address the crash reported in #114545 (comment)? cc @ronlieb

yes it does, thanks!

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LG

Change-Id: I75e876452af701191d36331703654581c6eb46f2
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@macurtis-amd macurtis-amd merged commit eea8b44 into llvm:main Nov 12, 2024
6 of 8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 12, 2024

LLVM Buildbot has detected a new failure on builder libc-riscv64-debian-fullbuild-dbg running on libc-riscv64-debian while building llvm at step 4 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/183/builds/6156

Here is the relevant piece of the build log for the reference
Step 4 (annotate) failure: 'python ../llvm-zorg/zorg/buildbot/builders/annotated/libc-linux.py ...' (failure)
...
[       OK ] LlvmLibcFStatTest.NonExistentFile (4 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[895/1016] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsBasic
[       OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (20 us)
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath
[       OK ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath (433 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[896/1016] Running unit test libc.test.src.sys.statvfs.linux.fstatvfs_test
FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test 
cd /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux && /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.fstatvfs_test.__build__
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsBasic
[       OK ] LlvmLibcSysFStatvfsTest.FStatvfsBasic (32 us)
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
/home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/llvm-project/libc/test/src/sys/statvfs/linux/fstatvfs_test.cpp:40: FAILURE
Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0).
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "File exists".
[  FAILED  ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
Ran 2 tests.  PASS: 1  FAIL: 1
[897/1016] Running unit test libc.test.src.sys.wait.waitpid_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcWaitPidTest.NoHangTest
[       OK ] LlvmLibcWaitPidTest.NoHangTest (8 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[898/1016] Running unit test libc.test.src.sys.utsname.uname_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcUnameTest.GetMachineName
[       OK ] LlvmLibcUnameTest.GetMachineName (7 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[899/1016] Running unit test libc.test.src.sys.wait.wait4_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcwait4Test.NoHangTest
[       OK ] LlvmLibcwait4Test.NoHangTest (11 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[900/1016] Running unit test libc.test.src.inttypes.strtoimax_test.__unit__
[==========] Running 7 tests from 1 test suite.
[ RUN      ] LlvmLibcStrtoimaxTest.InvalidBase
[       OK ] LlvmLibcStrtoimaxTest.InvalidBase (6 us)
[ RUN      ] LlvmLibcStrtoimaxTest.CleanBaseTenDecode
[       OK ] LlvmLibcStrtoimaxTest.CleanBaseTenDecode (28 us)
[ RUN      ] LlvmLibcStrtoimaxTest.MessyBaseTenDecode
[       OK ] LlvmLibcStrtoimaxTest.MessyBaseTenDecode (19 us)
[ RUN      ] LlvmLibcStrtoimaxTest.DecodeInOtherBases
[       OK ] LlvmLibcStrtoimaxTest.DecodeInOtherBases (1622 ms)
[ RUN      ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode
[       OK ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode (31 us)
Step 8 (libc-unit-tests) failure: libc-unit-tests (failure)
...
[       OK ] LlvmLibcFStatTest.NonExistentFile (4 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[895/1016] Running unit test libc.test.src.sys.statvfs.linux.statvfs_test
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsBasic
[       OK ] LlvmLibcSysStatvfsTest.StatvfsBasic (20 us)
[ RUN      ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath
[       OK ] LlvmLibcSysStatvfsTest.StatvfsInvalidPath (433 us)
Ran 2 tests.  PASS: 2  FAIL: 0
[896/1016] Running unit test libc.test.src.sys.statvfs.linux.fstatvfs_test
FAILED: projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/CMakeFiles/libc.test.src.sys.statvfs.linux.fstatvfs_test 
cd /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux && /home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/build/projects/libc/test/src/sys/statvfs/linux/libc.test.src.sys.statvfs.linux.fstatvfs_test.__build__
[==========] Running 2 tests from 1 test suite.
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsBasic
[       OK ] LlvmLibcSysFStatvfsTest.FStatvfsBasic (32 us)
[ RUN      ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
/home/libc_worker/libc-riscv64-debian/libc-riscv64-debian-fullbuild-dbg/llvm-project/libc/test/src/sys/statvfs/linux/fstatvfs_test.cpp:40: FAILURE
Failed to match LIBC_NAMESPACE::mkdirat(AT_FDCWD, TEST_DIR, S_IRWXU) against Succeeds(0).
Expected return value to be equal to 0 but got -1.
Expected errno to be equal to "Success" but got "File exists".
[  FAILED  ] LlvmLibcSysFStatvfsTest.FStatvfsInvalidPath
Ran 2 tests.  PASS: 1  FAIL: 1
[897/1016] Running unit test libc.test.src.sys.wait.waitpid_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcWaitPidTest.NoHangTest
[       OK ] LlvmLibcWaitPidTest.NoHangTest (8 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[898/1016] Running unit test libc.test.src.sys.utsname.uname_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcUnameTest.GetMachineName
[       OK ] LlvmLibcUnameTest.GetMachineName (7 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[899/1016] Running unit test libc.test.src.sys.wait.wait4_test
[==========] Running 1 test from 1 test suite.
[ RUN      ] LlvmLibcwait4Test.NoHangTest
[       OK ] LlvmLibcwait4Test.NoHangTest (11 us)
Ran 1 tests.  PASS: 1  FAIL: 0
[900/1016] Running unit test libc.test.src.inttypes.strtoimax_test.__unit__
[==========] Running 7 tests from 1 test suite.
[ RUN      ] LlvmLibcStrtoimaxTest.InvalidBase
[       OK ] LlvmLibcStrtoimaxTest.InvalidBase (6 us)
[ RUN      ] LlvmLibcStrtoimaxTest.CleanBaseTenDecode
[       OK ] LlvmLibcStrtoimaxTest.CleanBaseTenDecode (28 us)
[ RUN      ] LlvmLibcStrtoimaxTest.MessyBaseTenDecode
[       OK ] LlvmLibcStrtoimaxTest.MessyBaseTenDecode (19 us)
[ RUN      ] LlvmLibcStrtoimaxTest.DecodeInOtherBases
[       OK ] LlvmLibcStrtoimaxTest.DecodeInOtherBases (1622 ms)
[ RUN      ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode
[       OK ] LlvmLibcStrtoimaxTest.CleanBaseSixteenDecode (31 us)

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Nov 12, 2024
relands:
eea8b44 [GVN] Handle empty attrs in Expression == (llvm#115761)
984bca9 [GVN][NewGVN] Take call attributes into account in expressions (llvm#114545)

Reverts: breaks comgr build
41e3919 [clang] Introduce diagnostics suppression mappings (llvm#112517)

Change-Id: I3189bf1b5d66b980d2d711c3fe1d5fe217699bb1
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.

6 participants