[llvm] Fix Potential bug when handling PHI in AAPointerInfo (PR #97321)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 10:16:29 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Vidush Singhal (vidsinghal)

<details>
<summary>Changes</summary>

If a cycle Info object does not exist, we should return false that an instruction is not cyclic.

for the test:
`llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll`

Debug info Before the fix: 

```
Accesses by bin after update:
[8-12] : 1
     - 9 -   store i32 100, ptr %field2, align 4
       - c: i32 100
[32-36] : 1
     - 9 -   store i32 100, ptr %field8, align 4
       - c: i32 100
[2147483647-4294967294] : 1
     - 6 -   %ret = load i32, ptr %x, align 4
       - c: <unknown>
```

Debug info After the fix: 

```
Accesses by bin after update:
[8-12] : 2
     - 9 -   store i32 %0, ptr %field2, align 4
       - c:   %0 = load i32, ptr %val, align 4
     - 6 -   %ret = load i32, ptr %x, align 4
       - c: <unknown>
[32-36] : 2
     - 9 -   store i32 %1, ptr %field8, align 4
       - c:   %1 = load i32, ptr %val2, align 4
     - 6 -   %ret = load i32, ptr %x, align 4
       - c: <unknown>
```




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


3 Files Affected:

- (modified) llvm/lib/Transforms/IPO/AttributorAttributes.cpp (+1-1) 
- (added) llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll (+41) 
- (modified) llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll (+14-50) 


``````````diff
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index c4b9375a53a27..210327a6e8234 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -214,7 +214,7 @@ ChangeStatus clampStateAndIndicateChange<DerefState>(DerefState &S,
 static bool mayBeInCycle(const CycleInfo *CI, const Instruction *I,
                          bool HeaderOnly, Cycle **CPtr = nullptr) {
   if (!CI)
-    return true;
+    return false;
   auto *BB = I->getParent();
   auto *C = CI->getCycle(BB);
   if (!C)
diff --git a/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll b/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll
new file mode 100644
index 0000000000000..bb423e10f2c72
--- /dev/null
+++ b/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes --check-globals --version 2
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -debug-only=attributor -attributor-manifest-internal -attributor-annotate-decl-cs  -S < %s 2>&1 | FileCheck %s
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -debug-only=attributor -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s 2>&1 | FileCheck %s
+; REQUIRES: asserts
+
+
+ at globalBytes = internal global [1024 x i8] zeroinitializer
+
+; CHECK: Accesses by bin after update:
+; CHECK: [8-12] : 2
+; CHECK:    - 9 -   store i32 %0, ptr %field2, align 4
+; CHECK:      - c:   %0 = load i32, ptr %val, align 4
+; CHECK:    - 6 -   %ret = load i32, ptr %x, align 4
+; CHECK:      - c: <unknown>
+; CHECK: [32-36] : 2
+; CHECK:    - 9 -   store i32 %1, ptr %field8, align 4
+; CHECK:      - c:   %1 = load i32, ptr %val2, align 4
+; CHECK:    - 6 -   %ret = load i32, ptr %x, align 4
+; CHECK:      - c: <unknown>
+define dso_local i32 @phi_different_offsets(ptr nocapture %val, ptr nocapture %val2, i1 %cmp) {
+entry:
+  br i1 %cmp, label %then, label %else
+
+then:
+  %field2 = getelementptr i32, ptr @globalBytes, i32 2
+  %1 = load i32, ptr %val
+  store i32 %1, ptr %field2
+  br label %end
+
+else:
+  %field8 = getelementptr i32, ptr @globalBytes, i32 8
+  %2 = load i32, ptr %val2
+  store i32 %2, ptr %field8
+  br label %end
+
+end:
+  %x = phi ptr [ %field2, %then ], [ %field8, %else ]
+  %ret = load i32, ptr %x
+  ret i32 %ret
+
+}
diff --git a/llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll b/llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll
index 7a35b5c856097..1dce6e105bf29 100644
--- a/llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll
@@ -2092,47 +2092,35 @@ end:
 ; FIXME: This function returns 1.
 define i8 @phi_no_store_1() {
 ;
-; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn
+; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(write)
 ; TUNIT-LABEL: define {{[^@]+}}@phi_no_store_1
-; TUNIT-SAME: () #[[ATTR3]] {
+; TUNIT-SAME: () #[[ATTR5]] {
 ; TUNIT-NEXT:  entry:
 ; TUNIT-NEXT:    br label [[LOOP:%.*]]
 ; TUNIT:       loop:
 ; TUNIT-NEXT:    [[P:%.*]] = phi ptr [ @a1, [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
 ; TUNIT-NEXT:    [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
-; TUNIT-NEXT:    store i8 1, ptr [[P]], align 1
 ; TUNIT-NEXT:    [[G]] = getelementptr i8, ptr [[P]], i64 1
 ; TUNIT-NEXT:    [[O]] = add nsw i8 [[I]], 1
 ; TUNIT-NEXT:    [[C:%.*]] = icmp eq i8 [[O]], 3
 ; TUNIT-NEXT:    br i1 [[C]], label [[END:%.*]], label [[LOOP]]
 ; TUNIT:       end:
-; TUNIT-NEXT:    [[S11:%.*]] = getelementptr i8, ptr @a1, i64 2
-; TUNIT-NEXT:    [[L11:%.*]] = load i8, ptr [[S11]], align 2
-; TUNIT-NEXT:    [[S12:%.*]] = getelementptr i8, ptr @a1, i64 3
-; TUNIT-NEXT:    [[L12:%.*]] = load i8, ptr [[S12]], align 1
-; TUNIT-NEXT:    [[ADD:%.*]] = add i8 [[L11]], [[L12]]
-; TUNIT-NEXT:    ret i8 [[ADD]]
+; TUNIT-NEXT:    ret i8 0
 ;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn
+; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(write)
 ; CGSCC-LABEL: define {{[^@]+}}@phi_no_store_1
-; CGSCC-SAME: () #[[ATTR5]] {
+; CGSCC-SAME: () #[[ATTR6]] {
 ; CGSCC-NEXT:  entry:
 ; CGSCC-NEXT:    br label [[LOOP:%.*]]
 ; CGSCC:       loop:
 ; CGSCC-NEXT:    [[P:%.*]] = phi ptr [ @a1, [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
 ; CGSCC-NEXT:    [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
-; CGSCC-NEXT:    store i8 1, ptr [[P]], align 1
 ; CGSCC-NEXT:    [[G]] = getelementptr i8, ptr [[P]], i64 1
 ; CGSCC-NEXT:    [[O]] = add nsw i8 [[I]], 1
 ; CGSCC-NEXT:    [[C:%.*]] = icmp eq i8 [[O]], 3
 ; CGSCC-NEXT:    br i1 [[C]], label [[END:%.*]], label [[LOOP]]
 ; CGSCC:       end:
-; CGSCC-NEXT:    [[S11:%.*]] = getelementptr i8, ptr @a1, i64 2
-; CGSCC-NEXT:    [[L11:%.*]] = load i8, ptr [[S11]], align 2
-; CGSCC-NEXT:    [[S12:%.*]] = getelementptr i8, ptr @a1, i64 3
-; CGSCC-NEXT:    [[L12:%.*]] = load i8, ptr [[S12]], align 1
-; CGSCC-NEXT:    [[ADD:%.*]] = add i8 [[L11]], [[L12]]
-; CGSCC-NEXT:    ret i8 [[ADD]]
+; CGSCC-NEXT:    ret i8 0
 ;
 entry:
   br label %loop
@@ -2172,9 +2160,7 @@ define i8 @phi_no_store_2() {
 ; TUNIT:       end:
 ; TUNIT-NEXT:    [[S21:%.*]] = getelementptr i8, ptr @a2, i64 2
 ; TUNIT-NEXT:    [[L21:%.*]] = load i8, ptr [[S21]], align 2
-; TUNIT-NEXT:    [[S22:%.*]] = getelementptr i8, ptr @a2, i64 3
-; TUNIT-NEXT:    [[L22:%.*]] = load i8, ptr [[S22]], align 1
-; TUNIT-NEXT:    [[ADD:%.*]] = add i8 [[L21]], [[L22]]
+; TUNIT-NEXT:    [[ADD:%.*]] = add i8 [[L21]], 0
 ; TUNIT-NEXT:    ret i8 [[ADD]]
 ;
 ; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn
@@ -2193,9 +2179,7 @@ define i8 @phi_no_store_2() {
 ; CGSCC:       end:
 ; CGSCC-NEXT:    [[S21:%.*]] = getelementptr i8, ptr @a2, i64 2
 ; CGSCC-NEXT:    [[L21:%.*]] = load i8, ptr [[S21]], align 2
-; CGSCC-NEXT:    [[S22:%.*]] = getelementptr i8, ptr @a2, i64 3
-; CGSCC-NEXT:    [[L22:%.*]] = load i8, ptr [[S22]], align 1
-; CGSCC-NEXT:    [[ADD:%.*]] = add i8 [[L21]], [[L22]]
+; CGSCC-NEXT:    [[ADD:%.*]] = add i8 [[L21]], 0
 ; CGSCC-NEXT:    ret i8 [[ADD]]
 ;
 entry:
@@ -2218,57 +2202,37 @@ end:
 }
 
 define i8 @phi_no_store_3() {
-; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn
+; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(write)
 ; TUNIT-LABEL: define {{[^@]+}}@phi_no_store_3
-; TUNIT-SAME: () #[[ATTR3]] {
+; TUNIT-SAME: () #[[ATTR5]] {
 ; TUNIT-NEXT:  entry:
 ; TUNIT-NEXT:    [[S30:%.*]] = getelementptr i8, ptr @a3, i64 3
-; TUNIT-NEXT:    store i8 0, ptr [[S30]], align 1
 ; TUNIT-NEXT:    br label [[LOOP:%.*]]
 ; TUNIT:       loop:
 ; TUNIT-NEXT:    [[P:%.*]] = phi ptr [ @a3, [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
 ; TUNIT-NEXT:    [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
-; TUNIT-NEXT:    store i8 1, ptr [[P]], align 1
 ; TUNIT-NEXT:    [[G]] = getelementptr i8, ptr @a3, i64 2
 ; TUNIT-NEXT:    [[O]] = add nsw i8 [[I]], 1
 ; TUNIT-NEXT:    [[C:%.*]] = icmp eq i8 [[O]], 7
 ; TUNIT-NEXT:    br i1 [[C]], label [[END:%.*]], label [[LOOP]]
 ; TUNIT:       end:
-; TUNIT-NEXT:    [[S31:%.*]] = getelementptr i8, ptr @a3, i64 2
-; TUNIT-NEXT:    [[L31:%.*]] = load i8, ptr [[S31]], align 2
-; TUNIT-NEXT:    [[S32:%.*]] = getelementptr i8, ptr @a3, i64 3
-; TUNIT-NEXT:    [[L32:%.*]] = load i8, ptr [[S32]], align 1
-; TUNIT-NEXT:    [[ADD:%.*]] = add i8 [[L31]], [[L32]]
-; TUNIT-NEXT:    [[S34:%.*]] = getelementptr i8, ptr @a3, i64 4
-; TUNIT-NEXT:    [[L34:%.*]] = load i8, ptr [[S34]], align 4
-; TUNIT-NEXT:    [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
-; TUNIT-NEXT:    ret i8 [[ADD2]]
+; TUNIT-NEXT:    ret i8 poison
 ;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn
+; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(write)
 ; CGSCC-LABEL: define {{[^@]+}}@phi_no_store_3
-; CGSCC-SAME: () #[[ATTR5]] {
+; CGSCC-SAME: () #[[ATTR6]] {
 ; CGSCC-NEXT:  entry:
 ; CGSCC-NEXT:    [[S30:%.*]] = getelementptr i8, ptr @a3, i64 3
-; CGSCC-NEXT:    store i8 0, ptr [[S30]], align 1
 ; CGSCC-NEXT:    br label [[LOOP:%.*]]
 ; CGSCC:       loop:
 ; CGSCC-NEXT:    [[P:%.*]] = phi ptr [ @a3, [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
 ; CGSCC-NEXT:    [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
-; CGSCC-NEXT:    store i8 1, ptr [[P]], align 1
 ; CGSCC-NEXT:    [[G]] = getelementptr i8, ptr @a3, i64 2
 ; CGSCC-NEXT:    [[O]] = add nsw i8 [[I]], 1
 ; CGSCC-NEXT:    [[C:%.*]] = icmp eq i8 [[O]], 7
 ; CGSCC-NEXT:    br i1 [[C]], label [[END:%.*]], label [[LOOP]]
 ; CGSCC:       end:
-; CGSCC-NEXT:    [[S31:%.*]] = getelementptr i8, ptr @a3, i64 2
-; CGSCC-NEXT:    [[L31:%.*]] = load i8, ptr [[S31]], align 2
-; CGSCC-NEXT:    [[S32:%.*]] = getelementptr i8, ptr @a3, i64 3
-; CGSCC-NEXT:    [[L32:%.*]] = load i8, ptr [[S32]], align 1
-; CGSCC-NEXT:    [[ADD:%.*]] = add i8 [[L31]], [[L32]]
-; CGSCC-NEXT:    [[S34:%.*]] = getelementptr i8, ptr @a3, i64 4
-; CGSCC-NEXT:    [[L34:%.*]] = load i8, ptr [[S34]], align 4
-; CGSCC-NEXT:    [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
-; CGSCC-NEXT:    ret i8 [[ADD2]]
+; CGSCC-NEXT:    ret i8 poison
 ;
 entry:
   %s30 = getelementptr i8, ptr @a3, i64 3

``````````

</details>


https://github.com/llvm/llvm-project/pull/97321


More information about the llvm-commits mailing list