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

release/18.x: [ValueTracking] Treat phi as underlying obj when not decomposing further (#84339) #84950

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Mar 12, 2024

Backport 4cfd4a7 b274b23

Requested by: @nikic

@llvmbot llvmbot requested a review from nikic as a code owner March 12, 2024 17:04
@llvmbot llvmbot added this to the LLVM 18.X Release milestone Mar 12, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 12, 2024

@nikic What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-llvm-analysis

Author: None (llvmbot)

Changes

Backport 4cfd4a7 b274b23

Requested by: @nikic


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+2)
  • (added) llvm/test/Analysis/LoopAccessAnalysis/underlying-object-loop-varying-phi.ll (+180)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 412115eb649c2f..9f9451e4e814ac 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -5986,6 +5986,8 @@ void llvm::getUnderlyingObjects(const Value *V,
       if (!LI || !LI->isLoopHeader(PN->getParent()) ||
           isSameUnderlyingObjectInLoop(PN, LI))
         append_range(Worklist, PN->incoming_values());
+      else
+        Objects.push_back(P);
       continue;
     }
 
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-loop-varying-phi.ll b/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-loop-varying-phi.ll
new file mode 100644
index 00000000000000..106dc8c13a49fa
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/underlying-object-loop-varying-phi.ll
@@ -0,0 +1,180 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes='print<access-info>' -disable-output %s 2>&1 | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+
+; Test case for https://github.com/llvm/llvm-project/issues/82665.
+define void @indirect_ptr_recurrences_read_write(ptr %A, ptr %B) {
+; CHECK-LABEL: 'indirect_ptr_recurrences_read_write'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+; CHECK-NEXT:  Unsafe indirect dependence.
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:        IndidrectUnsafe:
+; CHECK-NEXT:            %l = load i32, ptr %ptr.recur, align 4, !tbaa !4 ->
+; CHECK-NEXT:            store i32 %xor, ptr %ptr.recur, align 4, !tbaa !4
+; CHECK-EMPTY:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 1, %entry ], [ %iv.next, %loop ]
+  %ptr.recur = phi ptr [ %A, %entry ], [ %ptr.next, %loop ]
+  %gep.B = getelementptr inbounds ptr, ptr %B, i64 %iv
+  %ptr.next = load ptr, ptr %gep.B, align 8, !tbaa !6
+  %l = load i32, ptr %ptr.recur, align 4, !tbaa !10
+  %xor = xor i32 %l, 1
+  store i32 %xor, ptr %ptr.recur, align 4, !tbaa !10
+  %iv.next = add nuw nsw i64 %iv, 1
+  %ec = icmp eq i64 %iv.next, 5
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+define i32 @indirect_ptr_recurrences_read_only_loop(ptr %A, ptr %B) {
+; CHECK-LABEL: 'indirect_ptr_recurrences_read_only_loop'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Memory dependences are safe
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 1, %entry ], [ %iv.next, %loop ]
+  %ptr.recur = phi ptr [ %A, %entry ], [ %ptr.next, %loop ]
+  %red = phi i32 [ 0, %entry ], [ %xor, %loop ]
+  %gep.B = getelementptr inbounds ptr, ptr %B, i64 %iv
+  %ptr.next = load ptr, ptr %gep.B, align 8, !tbaa !6
+  %l = load i32, ptr %ptr.recur, align 4, !tbaa !10
+  %xor = xor i32 %l, 1
+  %iv.next = add nuw nsw i64 %iv, 1
+  %ec = icmp eq i64 %iv.next, 5
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret i32 %xor
+}
+
+define void @indirect_ptr_recurrences_read_write_may_alias_no_tbaa(ptr %A, ptr %B) {
+; CHECK-LABEL: 'indirect_ptr_recurrences_read_write_may_alias_no_tbaa'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Report: cannot identify array bounds
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 1, %entry ], [ %iv.next, %loop ]
+  %ptr.recur = phi ptr [ %A, %entry ], [ %ptr.next, %loop ]
+  %gep.B = getelementptr inbounds ptr, ptr %B, i64 %iv
+  %ptr.next = load ptr, ptr %gep.B, align 8, !tbaa !6
+  %l = load i32, ptr %ptr.recur, align 4
+  %xor = xor i32 %l, 1
+  store i32 %xor, ptr %ptr.recur, align 4
+  %iv.next = add nuw nsw i64 %iv, 1
+  %ec = icmp eq i64 %iv.next, 5
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+define void @indirect_ptr_recurrences_read_write_may_alias_different_obj(ptr %A, ptr %B, ptr %C) {
+; CHECK-LABEL: 'indirect_ptr_recurrences_read_write_may_alias_different_obj'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Report: cannot identify array bounds
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 1, %entry ], [ %iv.next, %loop ]
+  %ptr.recur = phi ptr [ %A, %entry ], [ %ptr.next, %loop ]
+  %gep.B = getelementptr inbounds ptr, ptr %B, i64 %iv
+  %ptr.next = load ptr, ptr %gep.B, align 8, !tbaa !6
+  %l = load i32, ptr %ptr.recur, align 4
+  %xor = xor i32 %l, 1
+  %gep.C = getelementptr inbounds ptr, ptr %C, i64 %iv
+  store i32 %xor, ptr %gep.C, align 4
+  %iv.next = add nuw nsw i64 %iv, 1
+  %ec = icmp eq i64 %iv.next, 5
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+define void @indirect_ptr_recurrences_read_write_may_noalias_different_obj(ptr %A, ptr %B, ptr noalias %C) {
+; CHECK-LABEL: 'indirect_ptr_recurrences_read_write_may_noalias_different_obj'
+; CHECK-NEXT:    loop:
+; CHECK-NEXT:      Memory dependences are safe
+; CHECK-NEXT:      Dependences:
+; CHECK-NEXT:      Run-time memory checks:
+; CHECK-NEXT:      Grouped accesses:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Non vectorizable stores to invariant address were not found in loop.
+; CHECK-NEXT:      SCEV assumptions:
+; CHECK-EMPTY:
+; CHECK-NEXT:      Expressions re-written:
+;
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 1, %entry ], [ %iv.next, %loop ]
+  %ptr.recur = phi ptr [ %A, %entry ], [ %ptr.next, %loop ]
+  %gep.B = getelementptr inbounds ptr, ptr %B, i64 %iv
+  %ptr.next = load ptr, ptr %gep.B, align 8, !tbaa !6
+  %l = load i32, ptr %ptr.recur, align 4
+  %xor = xor i32 %l, 1
+  %gep.C = getelementptr inbounds ptr, ptr %C, i64 %iv
+  store i32 %xor, ptr %gep.C, align 4
+  %iv.next = add nuw nsw i64 %iv, 1
+  %ec = icmp eq i64 %iv.next, 5
+  br i1 %ec, label %exit, label %loop
+
+exit:
+  ret void
+}
+
+
+!6 = !{!7, !7, i64 0}
+!7 = !{!"any pointer", !8, i64 0}
+!8 = !{!"omnipotent char", !9, i64 0}
+!9 = !{!"Simple C/C++ TBAA"}
+!10 = !{!11, !11, i64 0}
+!11 = !{!"int", !8, i64 0}

Test case for llvm#82665.

(cherry picked from commit 4cfd4a7)
…her (llvm#84339)

At the moment, getUnderlyingObjects simply continues for phis that do
not refer to the same underlying object in loops, without adding them to
the list of underlying objects, effectively ignoring those phis.

Instead of ignoring those phis, add them to the list of underlying
objects. This fixes a miscompile where LoopAccessAnalysis fails to
identify a memory dependence, because no underlying objects can be found
for a set of memory accesses.

Fixes llvm#82665.

PR: llvm#84339
(cherry picked from commit b274b23)
@tstellar tstellar merged commit c7eb919 into llvm:release/18.x Mar 13, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants