[llvm] [AST] Fix size merging for MustAlias sets (PR #73820)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

AST checks aliasing with MustAlias sets by only checking the representative pointer (getSomePointer). This is only correct if the Size and AATags information of that pointer also includes the Size/AATags of all other pointers in the set.

When we add a new pointer to the AliasSet, we do perform this update (see the code in AliasSet::addPointer). However, if a pointer already in the MustAlias set is used with a new size, we currently do not update the representative pointer, resulting in miscompilations. Fix this by adding the missing update.

This is targeted fix using the current representation. There are a couple of alternatives:
 * For MustAlias sets, don't store per-pointer Size/AATags at all. This would make it clear that there is only one set of common Size/AATags for all pointers.
 * Check against all pointers in the set even for MustAlias. This is what https://github.com/llvm/llvm-project/pull/65731 proposes to do as part of a larger change to AST representation.

Fixes https://github.com/llvm/llvm-project/issues/64897.

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


2 Files Affected:

- (modified) llvm/lib/Analysis/AliasSetTracker.cpp (+9-1) 
- (modified) llvm/test/Transforms/LICM/pr64897.ll (+3-5) 


``````````diff
diff --git a/llvm/lib/Analysis/AliasSetTracker.cpp b/llvm/lib/Analysis/AliasSetTracker.cpp
index 91b889116dfa2d3..debdd328ce53fdc 100644
--- a/llvm/lib/Analysis/AliasSetTracker.cpp
+++ b/llvm/lib/Analysis/AliasSetTracker.cpp
@@ -348,8 +348,16 @@ AliasSet &AliasSetTracker::getAliasSetFor(const MemoryLocation &MemLoc) {
     // due to a quirk of alias analysis behavior. Since alias(undef, undef)
     // is NoAlias, mergeAliasSetsForPointer(undef, ...) will not find the
     // the right set for undef, even if it exists.
-    if (Entry.updateSizeAndAAInfo(Size, AAInfo))
+    if (Entry.updateSizeAndAAInfo(Size, AAInfo)) {
       mergeAliasSetsForPointer(Pointer, Size, AAInfo, MustAliasAll);
+
+      // For MustAlias sets, also update Size/AAInfo of the representative
+      // pointer.
+      AliasSet &AS = *Entry.getAliasSet(*this);
+      if (AS.isMustAlias())
+        if (AliasSet::PointerRec *P = AS.getSomePointer())
+          P->updateSizeAndAAInfo(Size, AAInfo);
+    }
     // Return the set!
     return *Entry.getAliasSet(*this)->getForwardedTarget(*this);
   }
diff --git a/llvm/test/Transforms/LICM/pr64897.ll b/llvm/test/Transforms/LICM/pr64897.ll
index 1ce78f15d797359..12b11eb6912b0cb 100644
--- a/llvm/test/Transforms/LICM/pr64897.ll
+++ b/llvm/test/Transforms/LICM/pr64897.ll
@@ -1,7 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
 ; RUN: opt -S -passes=licm < %s | FileCheck %s
 
-; FIXME: This is a miscompile.
 define void @test(i1 %c, i8 %x) {
 ; CHECK-LABEL: define void @test(
 ; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]]) {
@@ -10,17 +9,16 @@ define void @test(i1 %c, i8 %x) {
 ; CHECK-NEXT:    [[P:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 8
 ; CHECK-NEXT:    [[P_COPY:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 8
 ; CHECK-NEXT:    [[P2:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 12
-; CHECK-NEXT:    [[P2_PROMOTED:%.*]] = load i8, ptr [[P2]], align 1
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[TMP0:%.*]] = phi i8 [ 0, [[LOOP]] ], [ [[P2_PROMOTED]], [[START:%.*]] ]
 ; CHECK-NEXT:    store i32 286331153, ptr [[P]], align 4
 ; CHECK-NEXT:    store i32 34, ptr [[P_COPY]], align 4
 ; CHECK-NEXT:    store i64 3689348814741910323, ptr [[P_COPY]], align 4
-; CHECK-NEXT:    call void @use(i8 [[TMP0]])
+; CHECK-NEXT:    [[VAL:%.*]] = load i8, ptr [[P2]], align 1
+; CHECK-NEXT:    call void @use(i8 [[VAL]])
+; CHECK-NEXT:    store i8 0, ptr [[P2]], align 1
 ; CHECK-NEXT:    br i1 [[C]], label [[LOOP]], label [[EXIT:%.*]]
 ; CHECK:       exit:
-; CHECK-NEXT:    store i8 0, ptr [[P2]], align 1
 ; CHECK-NEXT:    ret void
 ;
 start:

``````````

</details>


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