[PATCH] New alias analysis for static global variables

[hfinkel at anl.gov]

The compile-time numbers look good (in the sense that there is no significant difference between the even- and odd-numbered runs).

Please comment on exactly what makes this lighter weight than GlobalsModRef AA, and why this should be a separate pass.


================
Comment at: include/llvm/Analysis/Passes.h:27
@@ -26,1 +26,3 @@
 
+  Pass *createStaticGlobalAAPass();
+
----------------
This needs a comment like the others.

================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:28
@@ +27,3 @@
+
+    SmallPtrSet<const GlobalValue*, 32> NonAddressTakenGlobals;
+
----------------
You should collect some statistics on this. It is not clear to be that this is necessarily small, nor that 32 is the right number.

================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:57
@@ +56,3 @@
+    void deleteValue(Value *V) override;
+    //void copyValue(Value *From, Value *To) override;
+    void addEscapingUse(Use &U) override;
----------------
Remove commented-out code.

================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:82
@@ +81,3 @@
+
+static bool hasAddressTaken(Value *V) {
+
----------------
This function seems to be doing exactly what PointerMayBeCaptured does (see lib/Analysis/CaptureTracking.cpp), can you use that instead?


================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:103
@@ +102,3 @@
+    else if (isa<GetElementPtrInst>(VU) || isa<BitCastInst>(VU)) {
+      if (!hasAddressTaken(VU))
+        continue;
----------------
Unguarded recursion should be avoided. Add a depth limit or use a worklist.

================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:105
@@ +104,3 @@
+        continue;
+    }
+    DEBUG(dbgs() << "StaticGlobalAA: " << V->getName()
----------------
You probably want to filter out comparisons with null, which also don't really capture the address.

================
Comment at: lib/Analysis/IPA/StaticGlobalsAA.cpp:138
@@ +137,3 @@
+  const Value *UV1 = GetUnderlyingObject(LocA.Ptr, *DL);
+  const Value *UV2 = GetUnderlyingObject(LocB.Ptr, *DL);
+
----------------
This seems unnecessarily limiting. Why not use GetUnderlyingObjects, and just make sure the condition is satisfied for all underlying pointers?

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:202
@@ -201,2 +201,3 @@
   addInitialAliasAnalysisPasses(MPM);
+  MPM.add(createStaticGlobalAAPass());
 
----------------
Should this be part of the preceeding function?

================
Comment at: lib/Transforms/IPO/PassManagerBuilder.cpp:275
@@ -273,1 +274,3 @@
 
+  //MPM.add(createStaticGlobalAAPass());
+
----------------
Remove commented-out code.

http://reviews.llvm.org/D10059

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/