[PATCH] D23432: [AliasSetTracker] Degrade AliasSetTracker results when may-alias sets get too large.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: include/llvm/Analysis/AliasSetTracker.h:130
@@ +129,3 @@
+
+  // Singifies that this set should be considered to alias any pointer.
+  // Use when the tracker holding this set is saturated.
----------------
Singifies --> Signifies

================
Comment at: include/llvm/Analysis/AliasSetTracker.h:132-133
@@ -127,2 +131,4 @@
+  // Use when the tracker holding this set is saturated.
+  unsigned AlwaysAlias : 1;
 
   /// The kinds of access this alias set models.
----------------
How about it be called  'AliasAny' ?

================
Comment at: include/llvm/Analysis/AliasSetTracker.h:418
@@ +417,3 @@
+  // all pointers into a single "May" set.
+  AliasSet *SaturatedAS;
+
----------------
nit: AliasAnyAS may sound better

================
Comment at: include/llvm/Analysis/AliasSetTracker.h:436
@@ -419,1 +435,3 @@
 
+  AliasSet &mergeAllAliasSets();
+
----------------
Add brief documentation.

================
Comment at: lib/Analysis/AliasSetTracker.cpp:29
@@ -28,1 +28,3 @@
 
+static cl::opt<unsigned> SaturationThreshold("alias-set-saturation-threshold",
+                                             cl::Hidden, cl::init(250));
----------------
add option description.

================
Comment at: lib/Analysis/AliasSetTracker.cpp:82
@@ -70,1 +81,3 @@
   if (AS.PtrList) {
+    SetSize += AS.SetSize;
+    AS.SetSize = 0;
----------------
AS.SetSize --> AS.size() to be consistent

================
Comment at: lib/Analysis/AliasSetTracker.cpp:294
@@ +293,3 @@
+  if (SaturatedAS) {
+    // Update the data structure, for the sake of consistency.
+    // Note that we will never need a merge here, since there's only one
----------------
can this logic of checking SaturatedAS be folded into 'mergeAliasSetsForPointer such that there is no need to make any change in this method.

================
Comment at: lib/Analysis/AliasSetTracker.cpp:554
@@ +553,3 @@
+    // Otherwise, we need to do the actual work of copying.
+    if (!Cur->UnknownInsts.empty()) {
+      if (SaturatedAS->UnknownInsts.empty())
----------------
Use SaturatedAS->MergeSetIn(Cur ...) ?

================
Comment at: lib/Analysis/AliasSetTracker.cpp:590
@@ +589,3 @@
+  if (!SaturatedAS && (TotalMayAliasSetSize > SaturationThreshold)) {
+    // The AS is now saturated. From here on, we conservatively consider all
+    // pointers to alias each-other.
----------------
The AST is now ..


https://reviews.llvm.org/D23432