MemoryDependenceAnalysis: Consider a pointer clobbered if it is saved

[wzssyqa]

In MemoryDependenceResults::getSimplePointerDependencyFrom, when we find the instruction that a LoadInst depends, if we find an store instruction that save the pointer used by LoadInst, we consider the pointer may be clobbered.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: YunQiang Su (wzssyqa)

Changes

In MemoryDependenceResults::getSimplePointerDependencyFrom, when we find the instruction that a LoadInst depends, if we find an store instruction that save the pointer used by LoadInst, we consider the pointer may be clobbered.

---

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

1 Files Affected:

• (modified) llvm/lib/Analysis/MemoryDependenceAnalysis.cpp (+5)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index f062189bac6a0..00f36b9e088ad 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -564,6 +564,11 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
         if (!QueryInst || QueryInst->isVolatile())
           return MemDepResult::getClobber(SI);
 
+      // If we store the pointer of QueryInst, it is danger due to that the address
+      // may be modified with other reference.
+      if (QueryInst && QueryInst->getOperand(0) == SI->getOperand(0))
+        return MemDepResult::getClobber(SI);
+
       // If alias analysis can tell that this store is guaranteed to not modify
       // the query pointer, ignore it.  Use getModRefInfo to handle cases where
       // the query pointer points to constant memory etc.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Analysis/MemoryDependenceAnalysis.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 00f36b9e0..a907e22ba 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -564,8 +564,8 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
         if (!QueryInst || QueryInst->isVolatile())
           return MemDepResult::getClobber(SI);
 
-      // If we store the pointer of QueryInst, it is danger due to that the address
-      // may be modified with other reference.
+      // If we store the pointer of QueryInst, it is danger due to that the
+      // address may be modified with other reference.
       if (QueryInst && QueryInst->getOperand(0) == SI->getOperand(0))
         return MemDepResult::getClobber(SI);
 

[wzssyqa]

I find this problem when working on x264.
The attached is the IR file.

./bin/opt --passes="default<O3>" x264.ll -S -o xx.ll

can reproduce this problem.

The difference is in function x264_8_macroblock_cache_allocate:
a load instruction

%82 = load ptr, ptr %slice_table, align 16, !tbaa !545

in baiscblock cond.end380 is remove mistakenly.

In fact the contents of ptr %slice_table is clobbered in while.body.

For C code, please see:
https://github.com/ShiftMediaProject/x264/blob/2597c3645a721f4ab1870f13a3751edf78328ff3/common/macroblock.c#L248

x264.tgz

I failed to reduce this cases, thus I cannot figure out a test case.

[nikic]

This should be handled by AA/CaptureTracking. Needs a reduced reproducer to understand why it fails.