Skip to content

[Local] Only intersect alias.scope,noalias & parallel_loop if inst moves #117716

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 26, 2024

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Nov 26, 2024

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata on the replacement instruction, if it does not move. In that case, the program would be UB, if the aliasing property encoded in the metadata does not hold. This makes use of the clarification re aliasing metadata implying UB if the property does not hold: #116220

Same as #115868, but for !alias.scope, !noalias and !mem.parallel_loop_access.

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata
on the replacement instruction, if it does not move. In that case, the
program would be UB, if the aliasing property encoded in the metadata
does not hold. This makes use of the clarification re aliasing metadata
implying UB if the property does not hold: llvm#116220

Same as llvm#115868, but for !alias.scope, !noalias and
!mem.parallel_loop_access.
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Nov 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata on the replacement instruction, if it does not move. In that case, the program would be UB, if the aliasing property encoded in the metadata does not hold. This makes use of the clarification re aliasing metadata implying UB if the property does not hold: #116220

Same as #115868, but for !alias.scope, !noalias and !mem.parallel_loop_access.


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

5 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/Local.cpp (+4-2)
  • (modified) llvm/test/Transforms/GVN/noalias.ll (+3-4)
  • (modified) llvm/test/Transforms/InstCombine/loadstore-metadata.ll (+6-3)
  • (modified) llvm/test/Transforms/JumpThreading/thread-loads.ll (+7-2)
  • (modified) llvm/test/Transforms/NewGVN/noalias.ll (+3-3)
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index b74d2e8cff0aae..bead85cf46b0a6 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3330,11 +3330,13 @@ void llvm::combineMetadata(Instruction *K, const Instruction *J,
           K->setMetadata(Kind, MDNode::getMostGenericTBAA(JMD, KMD));
         break;
       case LLVMContext::MD_alias_scope:
-        K->setMetadata(Kind, MDNode::getMostGenericAliasScope(JMD, KMD));
+        if (DoesKMove)
+          K->setMetadata(Kind, MDNode::getMostGenericAliasScope(JMD, KMD));
         break;
       case LLVMContext::MD_noalias:
       case LLVMContext::MD_mem_parallel_loop_access:
-        K->setMetadata(Kind, MDNode::intersect(JMD, KMD));
+        if (DoesKMove)
+          K->setMetadata(Kind, MDNode::intersect(JMD, KMD));
         break;
       case LLVMContext::MD_access_group:
         if (DoesKMove)
diff --git a/llvm/test/Transforms/GVN/noalias.ll b/llvm/test/Transforms/GVN/noalias.ll
index 931564db47b398..98cc930fa2df90 100644
--- a/llvm/test/Transforms/GVN/noalias.ll
+++ b/llvm/test/Transforms/GVN/noalias.ll
@@ -2,8 +2,7 @@
 
 define i32 @test1(ptr %p, ptr %q) {
 ; CHECK-LABEL: @test1(ptr %p, ptr %q)
-; CHECK: load i32, ptr %p
-; CHECK-NOT: noalias
+; CHECK: load i32, ptr %p, align 4, !noalias ![[SCOPE1:[0-9]+]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, ptr %p, !noalias !3
   %b = load i32, ptr %p
@@ -13,7 +12,7 @@ define i32 @test1(ptr %p, ptr %q) {
 
 define i32 @test2(ptr %p, ptr %q) {
 ; CHECK-LABEL: @test2(ptr %p, ptr %q)
-; CHECK: load i32, ptr %p, align 4, !alias.scope ![[SCOPE1:[0-9]+]]
+; CHECK: load i32, ptr %p, align 4, !alias.scope ![[SCOPE1]]
 ; CHECK: %c = add i32 %a, %a
   %a = load i32, ptr %p, !alias.scope !3
   %b = load i32, ptr %p, !alias.scope !3
@@ -32,7 +31,7 @@ define i32 @test3(ptr %p, ptr %q) {
 }
 
 ; CHECK:   ![[SCOPE1]] = !{!{{[0-9]+}}}
-; CHECK:   ![[SCOPE2]] = !{!{{[0-9]+}}, !{{[0-9]+}}}
+; CHECK:   ![[SCOPE2]] = !{!{{[0-9]+}}}
 declare i32 @foo(ptr) readonly
 
 !0 = distinct !{!0, !2, !"callee0: %a"}
diff --git a/llvm/test/Transforms/InstCombine/loadstore-metadata.ll b/llvm/test/Transforms/InstCombine/loadstore-metadata.ll
index 4976eace72a9f9..54649251e4cb15 100644
--- a/llvm/test/Transforms/InstCombine/loadstore-metadata.ll
+++ b/llvm/test/Transforms/InstCombine/loadstore-metadata.ll
@@ -278,8 +278,8 @@ entry:
 define double @preserve_load_metadata_after_select_transform_metadata_missing_4(ptr %a, ptr %b) {
 ; CHECK-LABEL: @preserve_load_metadata_after_select_transform_metadata_missing_4(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[L_A:%.*]] = load double, ptr [[A:%.*]], align 8, !tbaa [[TBAA0]], !llvm.access.group [[META6]]
-; CHECK-NEXT:    [[L_B:%.*]] = load double, ptr [[B:%.*]], align 8, !tbaa [[TBAA0]], !llvm.access.group [[ACC_GRP10:![0-9]+]]
+; CHECK-NEXT:    [[L_A:%.*]] = load double, ptr [[A:%.*]], align 8, !tbaa [[TBAA0]], !alias.scope [[META3]], !noalias [[META3]], !llvm.access.group [[META6]]
+; CHECK-NEXT:    [[L_B:%.*]] = load double, ptr [[B:%.*]], align 8, !tbaa [[TBAA0]], !alias.scope [[META10:![0-9]+]], !noalias [[META10]], !llvm.access.group [[ACC_GRP13:![0-9]+]]
 ; CHECK-NEXT:    [[CMP_I:%.*]] = fcmp fast olt double [[L_A]], [[L_B]]
 ; CHECK-NEXT:    [[L_SEL:%.*]] = select i1 [[CMP_I]], double [[L_B]], double [[L_A]]
 ; CHECK-NEXT:    ret double [[L_SEL]]
@@ -322,5 +322,8 @@ entry:
 ; CHECK: [[META7]] = !{i32 1}
 ; CHECK: [[META8]] = !{i64 8}
 ; CHECK: [[ACC_GRP9]] = distinct !{}
-; CHECK: [[ACC_GRP10]] = distinct !{}
+; CHECK: [[META10]] = !{[[META11:![0-9]+]]}
+; CHECK: [[META11]] = distinct !{[[META11]], [[META12:![0-9]+]]}
+; CHECK: [[META12]] = distinct !{[[META12]]}
+; CHECK: [[ACC_GRP13]] = distinct !{}
 ;.
diff --git a/llvm/test/Transforms/JumpThreading/thread-loads.ll b/llvm/test/Transforms/JumpThreading/thread-loads.ll
index 6f19b3d17ff1d4..6db4deffa85820 100644
--- a/llvm/test/Transforms/JumpThreading/thread-loads.ll
+++ b/llvm/test/Transforms/JumpThreading/thread-loads.ll
@@ -321,7 +321,7 @@ bb3:
 define void @test8(ptr, ptr, ptr) {
 ; CHECK-LABEL: @test8(
 ; CHECK-NEXT:  ret2:
-; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[TMP0:%.*]], align 4, !tbaa [[TBAA0]], !range [[RNG4:![0-9]+]], !noundef [[META5:![0-9]+]]
+; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[TMP0:%.*]], align 4, !tbaa [[TBAA0]], !range [[RNG4:![0-9]+]], !alias.scope [[META5:![0-9]+]], !noalias [[META8:![0-9]+]], !noundef [[META10:![0-9]+]]
 ; CHECK-NEXT:    store i32 [[A]], ptr [[TMP1:%.*]], align 4
 ; CHECK-NEXT:    [[XXX:%.*]] = tail call i32 (...) @f1() #[[ATTR0]]
 ; CHECK-NEXT:    ret void
@@ -698,5 +698,10 @@ right_x:
 ; CHECK: [[META2]] = !{!"omnipotent char", [[META3:![0-9]+]]}
 ; CHECK: [[META3]] = !{!"Simple C/C++ TBAA"}
 ; CHECK: [[RNG4]] = !{i32 0, i32 1}
-; CHECK: [[META5]] = !{}
+; CHECK: [[META5]] = !{[[META6:![0-9]+]]}
+; CHECK: [[META6]] = distinct !{[[META6]], [[META7:![0-9]+]]}
+; CHECK: [[META7]] = distinct !{[[META7]]}
+; CHECK: [[META8]] = !{[[META9:![0-9]+]]}
+; CHECK: [[META9]] = distinct !{[[META9]], [[META7]]}
+; CHECK: [[META10]] = !{}
 ;.
diff --git a/llvm/test/Transforms/NewGVN/noalias.ll b/llvm/test/Transforms/NewGVN/noalias.ll
index 2cb5c196a7abf7..a4fc3d1eb16e1d 100644
--- a/llvm/test/Transforms/NewGVN/noalias.ll
+++ b/llvm/test/Transforms/NewGVN/noalias.ll
@@ -4,7 +4,7 @@
 define i32 @test1(ptr %p, ptr %q) {
 ; CHECK-LABEL: define i32 @test1(
 ; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]]) {
-; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[P]], align 4
+; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[P]], align 4, !noalias [[META0:![0-9]+]]
 ; CHECK-NEXT:    [[C:%.*]] = add i32 [[A]], [[A]]
 ; CHECK-NEXT:    ret i32 [[C]]
 ;
@@ -17,7 +17,7 @@ define i32 @test1(ptr %p, ptr %q) {
 define i32 @test2(ptr %p, ptr %q) {
 ; CHECK-LABEL: define i32 @test2(
 ; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]]) {
-; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[P]], align 4, !alias.scope [[META0:![0-9]+]]
+; CHECK-NEXT:    [[A:%.*]] = load i32, ptr [[P]], align 4, !alias.scope [[META0]]
 ; CHECK-NEXT:    [[C:%.*]] = add i32 [[A]], [[A]]
 ; CHECK-NEXT:    ret i32 [[C]]
 ;
@@ -53,6 +53,6 @@ declare i32 @foo(ptr) readonly
 ; CHECK: [[META0]] = !{[[META1:![0-9]+]]}
 ; CHECK: [[META1]] = distinct !{[[META1]], [[META2:![0-9]+]], !"callee0: %a"}
 ; CHECK: [[META2]] = distinct !{[[META2]], !"callee0"}
-; CHECK: [[META3]] = !{[[META4:![0-9]+]], [[META1]]}
+; CHECK: [[META3]] = !{[[META4:![0-9]+]]}
 ; CHECK: [[META4]] = distinct !{[[META4]], [[META2]], !"callee0: %b"}
 ;.

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

@fhahn fhahn merged commit 46a0857 into llvm:main Nov 26, 2024
11 checks passed
@fhahn fhahn deleted the perserve-noalias-aliasscope branch November 26, 2024 20:45
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 26, 2024

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building llvm at step 16 "test-check-lldb-api".

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

Here is the relevant piece of the build log for the reference
Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure)
...
PASS: lldb-api :: types/TestIntegerType.py (1204 of 1213)
PASS: lldb-api :: types/TestRecursiveTypes.py (1205 of 1213)
PASS: lldb-api :: types/TestIntegerTypeExpr.py (1206 of 1213)
PASS: lldb-api :: types/TestLongTypes.py (1207 of 1213)
PASS: lldb-api :: types/TestShortType.py (1208 of 1213)
PASS: lldb-api :: types/TestShortTypeExpr.py (1209 of 1213)
PASS: lldb-api :: types/TestLongTypesExpr.py (1210 of 1213)
PASS: lldb-api :: tools/lldb-server/TestNonStop.py (1211 of 1213)
PASS: lldb-api :: tools/lldb-server/TestLldbGdbServer.py (1212 of 1213)
UNRESOLVED: lldb-api :: tools/lldb-server/TestGdbRemote_vCont.py (1213 of 1213)
******************** TEST 'lldb-api :: tools/lldb-server/TestGdbRemote_vCont.py' FAILED ********************
Script:
--
/usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/make --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/tools/lldb-server -p TestGdbRemote_vCont.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 46a08579f2b86e39b367b83ff4ca0e92302d2168)
  clang revision 46a08579f2b86e39b367b83ff4ca0e92302d2168
  llvm revision 46a08579f2b86e39b367b83ff4ca0e92302d2168
Setting up remote platform 'remote-linux'
Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-2198.lab.llvm.org:1234'...
Connected.
Setting remote platform working directory to '/home/ubuntu/lldb-tests'...
Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap']
connect to debug monitor on port 12805 failed, attempt #1 of 20
connect to debug monitor on port 15555 failed, attempt #2 of 20
connect to debug monitor on port 14659 failed, attempt #3 of 20
connect to debug monitor on port 13969 failed, attempt #4 of 20
connect to debug monitor on port 15702 failed, attempt #5 of 20
connect to debug monitor on port 15045 failed, attempt #6 of 20
connect to debug monitor on port 12711 failed, attempt #7 of 20
connect to debug monitor on port 15720 failed, attempt #8 of 20
connect to debug monitor on port 13011 failed, attempt #9 of 20
connect to debug monitor on port 15213 failed, attempt #10 of 20
connect to debug monitor on port 14900 failed, attempt #11 of 20
connect to debug monitor on port 12687 failed, attempt #12 of 20
connect to debug monitor on port 13237 failed, attempt #13 of 20
connect to debug monitor on port 14528 failed, attempt #14 of 20
connect to debug monitor on port 12983 failed, attempt #15 of 20
connect to debug monitor on port 14171 failed, attempt #16 of 20
connect to debug monitor on port 14903 failed, attempt #17 of 20
connect to debug monitor on port 13039 failed, attempt #18 of 20
connect to debug monitor on port 13243 failed, attempt #19 of 20
connect to debug monitor on port 15650 failed, attempt #20 of 20

--

fhahn added a commit to fhahn/llvm-project that referenced this pull request Jan 9, 2025
…ves (llvm#117716)

Preserve !alias.scope, !noalias and !mem.parallel_loop_access metadata
on the replacement instruction, if it does not move. In that case, the
program would be UB, if the aliasing property encoded in the metadata
does not hold. This makes use of the clarification re aliasing metadata
implying UB if the property does not hold: llvm#116220

Same as llvm#115868, but for !alias.scope, !noalias and
!mem.parallel_loop_access.

PR: llvm#117716
(cherry picked from commit 46a0857)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants