[PATCH] D13243: [GlobalsAA] Don't assume anything about functions that may be overridden

[James Molloy via llvm-commits]

jmolloy created this revision.
jmolloy added reviewers: chandlerc, hfinkel.
jmolloy added a subscriber: llvm-commits.
jmolloy set the repository for this revision to rL LLVM.

Weak linkage and friends allow a symbol to be overriden outside the
code generator's model, so GlobalsAA shouldn't assume that anything it
can compute about such a symbol is valid.

This should make GlobalsAA more conservative and fix the last problem seen in the wild.

Chandler, you had concerns about GlobalsAA's conservativeness. Do you think this is sufficient or do you still have worries?

Repository:
  rL LLVM

http://reviews.llvm.org/D13243

Files:
  lib/Analysis/GlobalsModRef.cpp
  test/Analysis/GlobalsModRef/weak-interposition.ll

Index: test/Analysis/GlobalsModRef/weak-interposition.ll
===================================================================
--- /dev/null
+++ test/Analysis/GlobalsModRef/weak-interposition.ll
@@ -0,0 +1,24 @@
+; RUN: opt -S -O1 -enable-non-lto-gmr=true < %s | FileCheck %s
+
+ at a = common global i32 0, align 4
+
+; @hook_me is weak, so GMR must not eliminate the reload of @a in @f,
+; even though @hook_me doesn't mod or ref @a.
+
+; Function Attrs: nounwind ssp uwtable
+define weak i32 @hook_me() {
+  ret i32 0
+}
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @f() {
+  %1 = alloca i32, align 4
+  store i32 4, i32* @a, align 4
+  %2 = call i32 @hook_me()
+  ; CHECK: load i32, i32* @a, align 4
+  %3 = load i32, i32* @a, align 4
+  %4 = add nsw i32 %3, %2
+  store i32 %4, i32* @a, align 4
+  %5 = load i32, i32* %1
+  ret i32 %5
+}
Index: lib/Analysis/GlobalsModRef.cpp
===================================================================
--- lib/Analysis/GlobalsModRef.cpp
+++ lib/Analysis/GlobalsModRef.cpp
@@ -480,8 +480,8 @@
     const std::vector<CallGraphNode *> &SCC = *I;
     assert(!SCC.empty() && "SCC with no functions?");
 
-    if (!SCC[0]->getFunction()) {
-      // Calls externally - can't say anything useful.  Remove any existing
+    if (!SCC[0]->getFunction() || SCC[0]->getFunction()->mayBeOverridden()) {
+      // Calls externally or is weak - can't say anything useful. Remove any existing
       // function records (may have been created when scanning globals).
       for (auto *Node : SCC)
         FunctionInfos.erase(Node->getFunction());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13243.35966.patch
Type: text/x-patch
Size: 1584 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150929/b53c1eb6/attachment.bin>