Repository navigation
[BasicAA] Preserve GEP element sizes at the pointer index width - #242013
cjsrxzdyzds wants to merge 1 commit into
Conversation
|
Hello @cjsrxzdyzds 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-llvm-analysis Author: Jiaye Fang (cjsrxzdyzds) Changes
Construct the Validated on Fixes #242012. Assisted-by: Claude Code and OpenAI Codex. Full diff: https://github.com/llvm/llvm-project/pull/242013.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 9bb8396de4554..7fb42ee7022ca 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -753,8 +753,9 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
CastedValue(Index, 0, SExtBits, TruncBits, NonNeg), DL, 0, AC, DT);
// Scale by the type size.
- unsigned TypeSize = AllocTypeSize.getFixedValue();
- LE = LE.mul(APInt(IndexSize, TypeSize), NUW, NUSW);
+ APInt TypeSize(IndexSize, AllocTypeSize.getFixedValue(),
+ /*isSigned=*/false, /*implicitTrunc=*/true);
+ LE = LE.mul(TypeSize, NUW, NUSW);
Decomposed.Offset += LE.Offset;
APInt Scale = LE.Scale;
if (!LE.IsNUW)
diff --git a/llvm/test/Analysis/BasicAA/gep-large-element.ll b/llvm/test/Analysis/BasicAA/gep-large-element.ll
new file mode 100644
index 0000000000000..891da12644245
--- /dev/null
+++ b/llvm/test/Analysis/BasicAA/gep-large-element.ll
@@ -0,0 +1,105 @@
+; RUN: opt -aa-pipeline=basic-aa -passes=aa-eval -print-all-alias-modref-info -disable-output %s 2>&1 | FileCheck %s --check-prefix=AA
+; RUN: opt -aa-pipeline=basic-aa -passes=gvn -S %s | FileCheck %s --check-prefix=GVN
+
+target datalayout = "e-p:64:64-p1:8:8-p2:16:16-p3:64:64:64:32"
+
+; The element size must not be truncated to 32 bits for a 64-bit index.
+; The pointers coincide for x == 0, but differ for x == 1.
+define i8 @large_element(ptr %p, i64 %x) {
+; AA-LABEL: Function: large_element:
+; AA: MayAlias: i8* %a, i8* %b
+; GVN-LABEL: define i8 @large_element(
+; GVN: %v = load i8, ptr %a
+; GVN: ret i8 %v
+ %a = getelementptr [4294967300 x i8], ptr %p, i64 %x
+ %b = getelementptr i32, ptr %p, i64 %x
+ store i8 1, ptr %a
+ store i8 2, ptr %b
+ %v = load i8, ptr %a
+ ret i8 %v
+}
+
+; In particular, a nonzero element size must not become zero.
+define i8 @large_element_zero_low_bits(ptr %p, i64 %x) {
+; AA-LABEL: Function: large_element_zero_low_bits:
+; AA: MayAlias: i8* %a, i8* %b
+; GVN-LABEL: define i8 @large_element_zero_low_bits(
+; GVN: %v = load i8, ptr %a
+; GVN: ret i8 %v
+ %a = getelementptr [4294967296 x i8], ptr %p, i64 %x
+ %b = getelementptr i8, ptr %p, i64 0
+ store i8 1, ptr %a
+ store i8 2, ptr %b
+ %v = load i8, ptr %a
+ ret i8 %v
+}
+
+; Constant indices already use the full element size.
+define i8 @constant_index(ptr %p) {
+; AA-LABEL: Function: constant_index:
+; AA: NoAlias: i8* %a, i8* %b
+; GVN-LABEL: define i8 @constant_index(
+; GVN: ret i8 1
+ %a = getelementptr [4294967300 x i8], ptr %p, i64 1
+ %b = getelementptr i8, ptr %p, i64 4
+ store i8 1, ptr %a
+ store i8 2, ptr %b
+ %v = load i8, ptr %a
+ ret i8 %v
+}
+
+; Element sizes wider than the index type must be truncated without asserting.
+define i8 @index8(ptr addrspace(1) %p, i8 %x) {
+; AA-LABEL: Function: index8:
+; AA: NoAlias: i8 addrspace(1)* %a, i8 addrspace(1)* %b
+; GVN-LABEL: define i8 @index8(
+; GVN: ret i8 1
+ %a = getelementptr [300 x i8], ptr addrspace(1) %p, i8 %x
+ %b = getelementptr i8, ptr addrspace(1) %p, i8 1
+ store i8 1, ptr addrspace(1) %a
+ store i8 2, ptr addrspace(1) %b
+ %v = load i8, ptr addrspace(1) %a
+ ret i8 %v
+}
+
+; 300 * x and 44 * x are equal modulo 2^8.
+define i8 @index8_equivalent(ptr addrspace(1) %p, i8 %x) {
+; AA-LABEL: Function: index8_equivalent:
+; AA: MustAlias: i8 addrspace(1)* %a, i8 addrspace(1)* %b
+; GVN-LABEL: define i8 @index8_equivalent(
+; GVN: ret i8 2
+ %a = getelementptr [300 x i8], ptr addrspace(1) %p, i8 %x
+ %offset = mul i8 %x, 44
+ %b = getelementptr i8, ptr addrspace(1) %p, i8 %offset
+ store i8 1, ptr addrspace(1) %a
+ store i8 2, ptr addrspace(1) %b
+ %v = load i8, ptr addrspace(1) %a
+ ret i8 %v
+}
+
+define i8 @index16(ptr addrspace(2) %p, i16 %x) {
+; AA-LABEL: Function: index16:
+; AA: NoAlias: i8 addrspace(2)* %a, i8 addrspace(2)* %b
+; GVN-LABEL: define i8 @index16(
+; GVN: ret i8 1
+ %a = getelementptr [70000 x i8], ptr addrspace(2) %p, i16 %x
+ %b = getelementptr i8, ptr addrspace(2) %p, i16 1
+ store i8 1, ptr addrspace(2) %a
+ store i8 2, ptr addrspace(2) %b
+ %v = load i8, ptr addrspace(2) %a
+ ret i8 %v
+}
+
+; Use the index width, even when the pointer representation is wider.
+define i8 @index32_pointer64(ptr addrspace(3) %p, i32 %x) {
+; AA-LABEL: Function: index32_pointer64:
+; AA: MustAlias: i8 addrspace(3)* %a, i8 addrspace(3)* %b
+; GVN-LABEL: define i8 @index32_pointer64(
+; GVN: ret i8 2
+ %a = getelementptr [4294967300 x i8], ptr addrspace(3) %p, i32 %x
+ %b = getelementptr i32, ptr addrspace(3) %p, i32 %x
+ store i8 1, ptr addrspace(3) %a
+ store i8 2, ptr addrspace(3) %b
+ %v = load i8, ptr addrspace(3) %a
+ ret i8 %v
+}
|
| @@ -0,0 +1,105 @@ | |||
| ; RUN: opt -aa-pipeline=basic-aa -passes=aa-eval -print-all-alias-modref-info -disable-output %s 2>&1 | FileCheck %s --check-prefix=AA | |||
| ; RUN: opt -aa-pipeline=basic-aa -passes=gvn -S %s | FileCheck %s --check-prefix=GVN | |||
There was a problem hiding this comment.
Please drop the GVN checks, there is no need to test both. (In that case, also make the tests only access each pointer once.)
There was a problem hiding this comment.
Done. Dropped the GVN checks and reduced each test to one load per pointer.
|
Please make sure your PR descriptions do not contain slop like information about testing. |
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- llvm/lib/Analysis/BasicAliasAnalysis.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 7fb42ee70..1a788538d 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -754,7 +754,7 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL,
// Scale by the type size.
APInt TypeSize(IndexSize, AllocTypeSize.getFixedValue(),
- /*isSigned=*/false, /*implicitTrunc=*/true);
+ /*isSigned=*/false, /*implicitTrunc=*/true);
LE = LE.mul(TypeSize, NUW, NUSW);
Decomposed.Offset += LE.Offset;
APInt Scale = LE.Scale;
|
Converting a GEP element size to unsigned truncates sizes of 4 GiB or more, so BasicAA can return MustAlias for different pointers. Constructing the scale also asserts if the size does not fit a narrower pointer index. Construct the APInt directly from the full allocation size, allowing truncation only to the pointer index width. Fixes llvm#242012. Assisted-by: Claude Code Assisted-by: OpenAI Codex
8904b53 to
0b4be03
Compare
|
Removed the testing notes from this PR, my bad |
Converting a GEP element size to
unsignedtruncates sizes of 4 GiB ormore, so BasicAA can return
MustAliasfor different pointers. Constructingthe scale also asserts if the size does not fit a narrower pointer index.
Construct the
APIntdirectly from the full allocation size, allowingtruncation only to the pointer index width.
Fixes #242012.
Assisted-by: Claude Code
Assisted-by: OpenAI Codex