[llvm] 253d6be - [Attributor][FIX] Properly check for accesses to globals

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-04-16T00:55:34-05:00
New Revision: 253d6be0f6fa7d96f78127a6da527d38f02e81d8

URL: https://github.com/llvm/llvm-project/commit/253d6be0f6fa7d96f78127a6da527d38f02e81d8
DIFF: https://github.com/llvm/llvm-project/commit/253d6be0f6fa7d96f78127a6da527d38f02e81d8.diff

LOG: [Attributor][FIX] Properly check for accesses to globals

The check if globals were accessed was not always working because two
bits are set for NO_GLOBAL_MEM. The new check works also if only on kind
of globals (internal/external) is accessed.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/AttributorAttributes.cpp
    llvm/test/Transforms/Attributor/memory_locations.ll
    llvm/test/Transforms/Attributor/nocapture-1.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 8cf45fd42222..be54fb3eb536 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -6251,8 +6251,9 @@ AAMemoryLocationImpl::categorizeAccessedLocations(Attributor &A, Instruction &I,
                                 nullptr, Changed);
     }
 
-    // Now handle global memory if it might be accessed.
-    bool HasGlobalAccesses = !(ICSAssumedNotAccessedLocs & NO_GLOBAL_MEM);
+    // Now handle global memory if it might be accessed. This is slightly tricky
+    // as NO_GLOBAL_MEM has multiple bits set.
+    bool HasGlobalAccesses = ((~ICSAssumedNotAccessedLocs) & NO_GLOBAL_MEM);
     if (HasGlobalAccesses) {
       auto AccessPred = [&](const Instruction *, const Value *Ptr,
                             AccessKind Kind, MemoryLocationsKind MLK) {
@@ -6270,7 +6271,7 @@ AAMemoryLocationImpl::categorizeAccessedLocations(Attributor &A, Instruction &I,
                << getMemoryLocationsAsStr(AccessedLocs.getAssumed()) << "\n");
 
     // Now handle argument memory if it might be accessed.
-    bool HasArgAccesses = !(ICSAssumedNotAccessedLocs & NO_ARGUMENT_MEM);
+    bool HasArgAccesses = ((~ICSAssumedNotAccessedLocs) & NO_ARGUMENT_MEM);
     if (HasArgAccesses) {
       for (unsigned ArgNo = 0, e = ICS.getNumArgOperands(); ArgNo < e;
            ++ArgNo) {

diff  --git a/llvm/test/Transforms/Attributor/memory_locations.ll b/llvm/test/Transforms/Attributor/memory_locations.ll
index c3222367360a..237617d6a9cc 100644
--- a/llvm/test/Transforms/Attributor/memory_locations.ll
+++ b/llvm/test/Transforms/Attributor/memory_locations.ll
@@ -397,3 +397,47 @@ define void @callerE(i8* %arg) {
   ret void
 }
 
+ at G = external dso_local global i32, align 4
+
+; CHECK: Function Attrs:
+; CHECK-SAME: writeonly
+define void @write_global() {
+; CHECK-LABEL: define {{[^@]+}}@write_global()
+; CHECK-NEXT:    store i32 0, i32* @G, align 4
+; CHECK-NEXT:    ret void
+;
+  store i32 0, i32* @G, align 4
+  ret void
+}
+; CHECK: Function Attrs: argmemonly
+; CHECK-SAME: writeonly
+define void @write_global_via_arg(i32* %GPtr) {
+; CHECK-LABEL: define {{[^@]+}}@write_global_via_arg
+; CHECK-SAME: (i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[GPTR:%.*]])
+; CHECK-NEXT:    store i32 0, i32* [[GPTR]], align 4
+; CHECK-NEXT:    ret void
+;
+  store i32 0, i32* %GPtr, align 4
+  ret void
+}
+
+; CHECK: Function Attrs:
+; CHECK-SAME: writeonly
+define void @writeonly_global() {
+; CHECK-LABEL: define {{[^@]+}}@writeonly_global()
+; CHECK-NEXT:    call void @write_global()
+; CHECK-NEXT:    ret void
+;
+  call void @write_global()
+  ret void
+}
+; CHECK: Function Attrs:
+; CHECK-SAME: writeonly
+define void @writeonly_global_via_arg() {
+; CHECK-LABEL: define {{[^@]+}}@writeonly_global_via_arg()
+; CHECK-NEXT:    call void @write_global_via_arg(i32* nofree nonnull writeonly align 4 dereferenceable(4) @G)
+; CHECK-NEXT:    ret void
+;
+  call void @write_global_via_arg(i32* @G)
+  ret void
+}

diff  --git a/llvm/test/Transforms/Attributor/nocapture-1.ll b/llvm/test/Transforms/Attributor/nocapture-1.ll
index 9a16f5e8ad3d..24018b289579 100644
--- a/llvm/test/Transforms/Attributor/nocapture-1.ll
+++ b/llvm/test/Transforms/Attributor/nocapture-1.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
-; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
+; RUN: opt -attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
 ; RUN: opt -attributor-cgscc -attributor-manifest-internal -attributor-disable=false -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM
 ; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-disable=false -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM