From aaeaea13997b16911226a053f984ce2ae8443e23 Mon Sep 17 00:00:00 2001
From: Anna Thomas <anna@azul.com>
Date: Fri, 20 Jan 2017 00:21:33 +0000
Subject: [PATCH] [AliasAnalysis] Fences do not modify constant memory location

Summary:
Fence instructions are currently marked as `ModRef` for all memory locations.

We can improve this for constant memory locations (such as constant globals),
since fence instructions cannot modify these locations.

This helps us to forward constant loads across fences (added test case in GVN).
There were no changes in behaviour for similar test cases in early-cse and licm.

Reviewers: dberlin, sanjoy, reames

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D28914

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@292546 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/llvm/Analysis/AliasAnalysis.h |  6 +-----
 lib/Analysis/AliasAnalysis.cpp        |  8 ++++++++
 test/Transforms/GVN/fence.ll          | 20 ++++++++++++++++++++
 test/Transforms/NewGVN/fence.ll       | 20 ++++++++++++++++++++
 4 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/include/llvm/Analysis/AliasAnalysis.h b/include/llvm/Analysis/AliasAnalysis.h
index d8e50438e72..484df2f55d9 100644
--- a/include/llvm/Analysis/AliasAnalysis.h
+++ b/include/llvm/Analysis/AliasAnalysis.h
@@ -443,11 +443,7 @@ public:
 
   /// getModRefInfo (for fences) - Return information about whether
   /// a particular store modifies or reads the specified memory location.
-  ModRefInfo getModRefInfo(const FenceInst *S, const MemoryLocation &Loc) {
-    // Conservatively correct.  (We could possibly be a bit smarter if
-    // Loc is a alloca that doesn't escape.)
-    return MRI_ModRef;
-  }
+  ModRefInfo getModRefInfo(const FenceInst *S, const MemoryLocation &Loc);
 
   /// getModRefInfo (for fences) - A convenience wrapper.
   ModRefInfo getModRefInfo(const FenceInst *S, const Value *P, uint64_t Size) {
diff --git a/lib/Analysis/AliasAnalysis.cpp b/lib/Analysis/AliasAnalysis.cpp
index 84da76be98b..fbc030b26aa 100644
--- a/lib/Analysis/AliasAnalysis.cpp
+++ b/lib/Analysis/AliasAnalysis.cpp
@@ -367,6 +367,14 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
   return MRI_Mod;
 }
 
+ModRefInfo AAResults::getModRefInfo(const FenceInst *S, const MemoryLocation &Loc) {
+  // If we know that the location is a constant memory location, the fence
+  // cannot modify this location.
+  if (Loc.Ptr && pointsToConstantMemory(Loc))
+    return MRI_Ref;
+  return MRI_ModRef;
+}
+
 ModRefInfo AAResults::getModRefInfo(const VAArgInst *V,
                                     const MemoryLocation &Loc) {
 
diff --git a/test/Transforms/GVN/fence.ll b/test/Transforms/GVN/fence.ll
index f68861dad1a..a2d98e13b38 100644
--- a/test/Transforms/GVN/fence.ll
+++ b/test/Transforms/GVN/fence.ll
@@ -1,5 +1,6 @@
 ; RUN: opt -S -basicaa -gvn < %s | FileCheck %s
 
+@a = external constant i32
 ; We can value forward across the fence since we can (semantically) 
 ; reorder the following load before the fence.
 define i32 @test(i32* %addr.i) {
@@ -52,6 +53,25 @@ define i32 @test3(i32* noalias %addr.i, i32* noalias %otheraddr) {
   ret i32 %res
 }
 
+; We can forward the value forward the load
+; across both the fences, because the load is from
+; a constant memory location.
+define i32 @test4(i32* %addr) {
+; CHECK-LABEL: @test4
+; CHECK-NOT: load
+; CHECK: fence release
+; CHECK: store
+; CHECK: fence seq_cst
+; CHECK: ret i32 0
+  %var = load i32, i32* @a
+  fence release
+  store i32 42, i32* %addr, align 8
+  fence seq_cst
+  %var2 = load i32, i32* @a
+  %var3 = sub i32 %var, %var2
+  ret i32 %var3
+}
+
 ; Another example of why forwarding across an acquire fence is problematic
 ; can be seen in a normal locking operation.  Say we had:
 ; *p = 5; unlock(l); lock(l); use(p);
diff --git a/test/Transforms/NewGVN/fence.ll b/test/Transforms/NewGVN/fence.ll
index ac4270d9ace..190fd734492 100644
--- a/test/Transforms/NewGVN/fence.ll
+++ b/test/Transforms/NewGVN/fence.ll
@@ -1,6 +1,7 @@
 ; XFAIL: *
 ; RUN: opt -S -basicaa -newgvn < %s | FileCheck %s
 
+@a = external constant i32
 ; We can value forward across the fence since we can (semantically) 
 ; reorder the following load before the fence.
 define i32 @test(i32* %addr.i) {
@@ -53,6 +54,25 @@ define i32 @test3(i32* noalias %addr.i, i32* noalias %otheraddr) {
   ret i32 %res
 }
 
+; We can forward the value forward the load
+; across both the fences, because the load is from
+; a constant memory location.
+define i32 @test4(i32* %addr) {
+; CHECK-LABEL: @test4
+; CHECK-NOT: load
+; CHECK: fence release
+; CHECK: store
+; CHECK: fence seq_cst
+; CHECK: ret i32 0
+  %var = load i32, i32* @a
+  fence release
+  store i32 42, i32* %addr, align 8
+  fence seq_cst
+  %var2 = load i32, i32* @a
+  %var3 = sub i32 %var, %var2
+  ret i32 %var3
+}
+
 ; Another example of why forwarding across an acquire fence is problematic
 ; can be seen in a normal locking operation.  Say we had:
 ; *p = 5; unlock(l); lock(l); use(p);
-- 
2.40.0