Revert "GlobalsAA: Take advantage of ArgMemOnly, InaccessibleMemOnly and InaccessibleMemOrArgMemOnly attributes"

[deadalnix]

Summary

This reverts commit 5a9e526f29cf8510ab5c3d566fbdcf47ac24e1e9.

As per discussion in D15665

This also add a test case so that regression introduced by that diff are not reintroduced.

[vaivaswatha]

Thanks for doing this.

lib/Analysis/GlobalsModRef.cpp

388

In my opinion, this part can actually stay. I'll let the others decide though.

[deadalnix]

lib/Analysis/GlobalsModRef.cpp

388

Even if we want to go that road, I think it makes sense to do a clean revert and then reintroduce the changes that makes sense in their own diifs, with their own tests.

[jmolloy]

lib/Analysis/GlobalsModRef.cpp

388

Agreed. Let's just do the revert and then reapply any applicable hunks with tests.

lib/Analysis/GlobalsModRef.cpp

if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
             return true;
           if (Readers)
             Readers->insert(ParentF);
           if (Writers)
             Writers->insert(ParentF);
         } else {
           return true; // Argument of an unknown call.
         }
-        // If the Callee is not ReadNone, it may read the global,
-        // and if it is not ReadOnly, it may also write to it.
-        Function *CalleeF = CS.getCalledFunction();
-        if (!CalleeF->doesNotAccessMemory()) {
-          if (Readers)
-            Readers->insert(CalleeF);
-          if (Writers && !CalleeF->onlyReadsMemory())
-            Writers->insert(CalleeF);
-        }
       }
     } else if (ICmpInst *ICI = dyn_cast<ICmpInst>(I)) {
       if (!isa<ConstantPointerNull>(ICI->getOperand(1)))
         return true; // Allow comparison against null.
     } else {
       return true;
     }
   }
 
for (unsigned i = 0, e = SCC.size(); i != e && !KnowNothing; ++i) {
       Function *F = SCC[i]->getFunction();
       if (!F) {
         KnowNothing = true;
         break;
       }
 
       if (F->isDeclaration()) {
         // Try to get mod/ref behaviour from function attributes.
-        if (F->doesNotAccessMemory() || F->onlyAccessesInaccessibleMemory()) {
+        if (F->doesNotAccessMemory()) {
           // Can't do better than that!
         } else if (F->onlyReadsMemory()) {
           FI.addModRefInfo(MRI_Ref);
           if (!F->isIntrinsic())
             // This function might call back into the module and read a global -
             // consider every global as possibly being read by this function.
             FI.setMayReadAnyGlobal();
-        } else if (F->onlyAccessesArgMemory() || 
-                   F->onlyAccessesInaccessibleMemOrArgMem()) {
-          // This function may only access (read/write) memory pointed to by its
-          // arguments. If this pointer is to a global, this escaping use of the
-          // pointer is captured in AnalyzeUsesOfPointer().
-          FI.addModRefInfo(MRI_ModRef);
         } else {
           FI.addModRefInfo(MRI_ModRef);
           // Can't say anything useful unless it's an intrinsic - they don't
           // read or write global variables of the kind considered here.
           KnowNothing = !F->isIntrinsic();
         }
         continue;
       }

test/Analysis/GlobalsModRef/argmemonly-escape.ll

-; RUN: opt < %s -O1 -S -enable-non-lto-gmr=true | FileCheck %s
-
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.10.0"
-
-@a = internal global [3 x i32] zeroinitializer, align 4
-
-; The important thing we're checking for here is the reload of (some element of)
-; @a after the memset.
-
-; CHECK-LABEL: @main
-; CHECK: load i32, i32* getelementptr {{.*}} @a
-; CHECK-NEXT: call void @memsetp0i8i64{{.*}} @a
-; CHECK-NEXT: load i32, i32* getelementptr {{.*}} @a
-; CHECK-NEXT: call void @memsetp0i8i64A{{.*}} @a
-; CHECK-NEXT: load i32, i32* getelementptr {{.*}} @a
-; CHECK: icmp eq
-; CHECK: br i1
-
-define i32 @main() {
-entry:
-  %0 = bitcast [3 x i32]* @a to i8*
-  %1 = load i32, i32* getelementptr inbounds ([3 x i32], [3 x i32]* @a, i64 0, i64 2), align 4
-  call void @memsetp0i8i64(i8* %0, i8 0, i64 4, i32 4, i1 false)
-  %2 = load i32, i32* getelementptr inbounds ([3 x i32], [3 x i32]* @a, i64 0, i64 2), align 4
-  call void @memsetp0i8i64A(i8* %0, i8 0, i64 4, i32 4, i1 false)
-  %3 = load i32, i32* getelementptr inbounds ([3 x i32], [3 x i32]* @a, i64 0, i64 2), align 4
-  %4 = add i32 %2, %3
-  %cmp1 = icmp eq i32 %1, %4
-  br i1 %cmp1, label %if.then, label %if.end
-
-if.then:                                          ; preds = %entr
-  call void @abort() #3
-  unreachable
-
-if.end:                                           ; preds = %entry
-  ret i32 0
-}
-
-; Function Attrs: nounwind argmemonly
-declare void @memsetp0i8i64(i8* nocapture, i8, i64, i32, i1) nounwind argmemonly
-
-; Function Attrs: nounwind inaccessiblemem_or_argmemonly
-declare void @memsetp0i8i64A(i8* nocapture, i8, i64, i32, i1) nounwind inaccessiblemem_or_argmemonly
-
-; Function Attrs: noreturn nounwind
-declare void @abort() noreturn nounwind

test/Analysis/GlobalsModRef/inaccessiblememonly.ll

+; RUN: opt -O3 -S < %s | FileCheck %s
+
+target datalayout = "e-i64:64-f80:128-n8:16:32:64"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @donteliminate() {
+; CHECK-LABEL: donteliminate
+; CHECK-NEXT: tail call noalias i8* @allocmemory()
+; CHECK-NEXT: tail call noalias i8* @allocmemory()
+; CHECK-NEXT: tail call noalias i8* @allocmemory()
+; CHECK-NEXT: ret void
+  %1 = tail call noalias i8* @allocmemory()
+  %2 = tail call noalias i8* @allocmemory()
+  %3 = tail call noalias i8* @allocmemory()
+  ret void
+}
+
+; Function Attrs: inaccessiblememonly
+declare noalias i8* @allocmemory() #0
+
+attributes #0 = { inaccessiblememonly }

test/Analysis/GlobalsModRef/modreftest.ll

; CHECK-NEXT: ret i32 12
   call void @doesnotmodX( )
   %V = load i32, i32* @X    ; <i32> [#uses=1]
   ret i32 %V
 }
 
 define void @doesnotmodX() {
   ret void
 }
-
-declare void @InaccessibleMemOnlyFunc( ) #0
-declare void @InaccessibleMemOrArgMemOnlyFunc( ) #1
-
-define i32 @test2(i32* %P) {
-; CHECK:      @test2
-; CHECK-NEXT: store i32 12, i32* @X
-; CHECK-NEXT: call void @InaccessibleMemOnlyFunc()
-; CHECK-NEXT: call void @InaccessibleMemOrArgMemOnlyFunc()
-; CHECK-NOT:  load i32
-; CHECK-NEXT: ret i32 12
-  store i32 12, i32* @X
-  call void @InaccessibleMemOnlyFunc( )
-        call void @InaccessibleMemOrArgMemOnlyFunc( )
-  %V = load i32, i32* @X    ; <i32> [#uses=1]
-  ret i32 %V
-}
-
-attributes #0 = { inaccessiblememonly }
-attributes #1 = { inaccessiblemem_or_argmemonly }