[PATCH] D16204: ValueTracking: Use fixed array for assumption exclude set in Query; NFC

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 27 16:47:50 PST 2016


reames accepted this revision.
reames added a reviewer: reames.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/minor comments addressed.


================
Comment at: lib/Analysis/ValueTracking.cpp:96
@@ +95,3 @@
+  /// Set of assumptions that should be excluded from further queries.
+  /// Many of these functions have internal versions that take an assumption
+  /// exclusion set. This is because of the potential for mutual recursion to
----------------
Stale comment.  

================
Comment at: lib/Analysis/ValueTracking.cpp:115
@@ +114,3 @@
+    Excluded = Q.Excluded;
+    assert(NumExcluded < Excluded.size());
+    Excluded[NumExcluded++] = NewExcl;
----------------
Place this assert after the increment as an <=.  They're equivalent, but the alternative code is easier to read.

================
Comment at: lib/Analysis/ValueTracking.cpp:121
@@ -111,1 +120,3 @@
+    auto End = Excluded.begin() + NumExcluded;
+    return std::find(Excluded.begin(), End, Value) != End;
   }
----------------
If you zero initialize the array, you can simply this by just scanning the extra elements. (optional)

Adding an early exit when NumExcluded == 0 is likely to be worthwhile.  


Repository:
  rL LLVM

http://reviews.llvm.org/D16204





More information about the llvm-commits mailing list