[llvm] 753c51b - [AST] Fix size merging for MustAlias sets (#73820)

[via llvm-commits]

Author: Nikita Popov
Date: 2023-12-07T10:45:48+01:00
New Revision: 753c51bf889e605a2daf92e1710d7ad5ebc76ec3

URL: https://github.com/llvm/llvm-project/commit/753c51bf889e605a2daf92e1710d7ad5ebc76ec3
DIFF: https://github.com/llvm/llvm-project/commit/753c51bf889e605a2daf92e1710d7ad5ebc76ec3.diff

LOG: [AST] Fix size merging for MustAlias sets (#73820)

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 a 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.

Added: 
    

Modified: 
    llvm/lib/Analysis/AliasSetTracker.cpp
    llvm/test/Transforms/LICM/pr64897.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/AliasSetTracker.cpp b/llvm/lib/Analysis/AliasSetTracker.cpp
index 91b889116dfa2..debdd328ce53f 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 1ce78f15d7973..12b11eb6912b0 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: