[llvm] [DA] Dependence analysis does not handle array accesses of different sizes (PR #116630)

[Sebastian Pop via llvm-commits]

https://github.com/sebpop updated https://github.com/llvm/llvm-project/pull/116630

>From 61f200318d9952bc469b20f548fea78b857d6bbd Mon Sep 17 00:00:00 2001
From: Sebastian Pop <spop at nvidia.com>
Date: Thu, 14 Nov 2024 18:17:04 +0000
Subject: [PATCH] [DA] Dependence analysis does not handle array accesses of
 different sizes

This fixes bug https://github.com/llvm/llvm-project/issues/16183
where the elements of a same array are accesses as i32 and i64.
This is not handled by the array dependence analysis.
---
 llvm/lib/Analysis/DependenceAnalysis.cpp      | 38 +++++++++++--------
 .../DependenceAnalysis/DifferentAccessSize.ll | 22 +++++++++++
 2 files changed, 44 insertions(+), 16 deletions(-)
 create mode 100644 llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll

diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index 6ce2875beeccac..3c86b40cbdf6e2 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -3604,14 +3604,10 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
     return std::make_unique<Dependence>(Src, Dst);
   }
 
-  assert(isLoadOrStore(Src) && "instruction is not load or store");
-  assert(isLoadOrStore(Dst) && "instruction is not load or store");
-  Value *SrcPtr = getLoadStorePointerOperand(Src);
-  Value *DstPtr = getLoadStorePointerOperand(Dst);
+  const MemoryLocation &DstLoc = MemoryLocation::get(Dst);
+  const MemoryLocation &SrcLoc = MemoryLocation::get(Src);
 
-  switch (underlyingObjectsAlias(AA, F->getDataLayout(),
-                                 MemoryLocation::get(Dst),
-                                 MemoryLocation::get(Src))) {
+  switch (underlyingObjectsAlias(AA, F->getDataLayout(), DstLoc, SrcLoc)) {
   case AliasResult::MayAlias:
   case AliasResult::PartialAlias:
     // cannot analyse objects if we don't understand their aliasing.
@@ -3625,16 +3621,15 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
     break; // The underlying objects alias; test accesses for dependence.
   }
 
-  // establish loop nesting levels
-  establishNestingLevels(Src, Dst);
-  LLVM_DEBUG(dbgs() << "    common nesting levels = " << CommonLevels << "\n");
-  LLVM_DEBUG(dbgs() << "    maximum nesting levels = " << MaxLevels << "\n");
-
-  FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels);
-  ++TotalArrayPairs;
+  if (DstLoc.Size != SrcLoc.Size) {
+    // The dependence test gets confused if the size of the memory accesses
+    // differ.
+    LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n");
+    return std::make_unique<Dependence>(Src, Dst);
+  }
 
-  unsigned Pairs = 1;
-  SmallVector<Subscript, 2> Pair(Pairs);
+  Value *SrcPtr = getLoadStorePointerOperand(Src);
+  Value *DstPtr = getLoadStorePointerOperand(Dst);
   const SCEV *SrcSCEV = SE->getSCEV(SrcPtr);
   const SCEV *DstSCEV = SE->getSCEV(DstPtr);
   LLVM_DEBUG(dbgs() << "    SrcSCEV = " << *SrcSCEV << "\n");
@@ -3649,6 +3644,17 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
     LLVM_DEBUG(dbgs() << "can't analyze SCEV with different pointer base\n");
     return std::make_unique<Dependence>(Src, Dst);
   }
+
+  // establish loop nesting levels
+  establishNestingLevels(Src, Dst);
+  LLVM_DEBUG(dbgs() << "    common nesting levels = " << CommonLevels << "\n");
+  LLVM_DEBUG(dbgs() << "    maximum nesting levels = " << MaxLevels << "\n");
+
+  FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels);
+  ++TotalArrayPairs;
+
+  unsigned Pairs = 1;
+  SmallVector<Subscript, 2> Pair(Pairs);
   Pair[0].Src = SrcSCEV;
   Pair[0].Dst = DstSCEV;
 
diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll
new file mode 100644
index 00000000000000..2dded8f3b13a19
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll
@@ -0,0 +1,22 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -disable-output "-passes=print<da>" -aa-pipeline=basic-aa 2>&1 \
+; RUN: | FileCheck %s
+
+; The dependence test does not handle array accesses of different sizes: i32 and i64.
+; Bug 16183 - https://github.com/llvm/llvm-project/issues/16183
+
+define i64 @bug16183_alias(ptr nocapture %A) {
+; CHECK-LABEL: 'bug16183_alias'
+; CHECK-NEXT:  Src: store i32 2, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4
+; CHECK-NEXT:    da analyze - none!
+; CHECK-NEXT:  Src: store i32 2, ptr %arrayidx, align 4 --> Dst: %0 = load i64, ptr %A, align 8
+; CHECK-NEXT:    da analyze - confused!
+; CHECK-NEXT:  Src: %0 = load i64, ptr %A, align 8 --> Dst: %0 = load i64, ptr %A, align 8
+; CHECK-NEXT:    da analyze - none!
+;
+entry:
+  %arrayidx = getelementptr inbounds i32, ptr %A, i64 1
+  store i32 2, ptr %arrayidx, align 4
+  %0 = load i64, ptr %A, align 8
+  ret i64 %0
+}