[PATCH] D22721: [CFLAA] Add an initial implementation of field-sensitivity to CFLAndersAliasAnalyiss

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 10:51:56 PDT 2016


george.burgess.iv added a comment.

> So I'll just check in the intraprocedural analysis works to ease the burden of code review.


Thank you :)


================
Comment at: lib/Analysis/AliasAnalysisSummary.h:148
@@ -147,1 +147,3 @@
 }
+inline int64_t subOffset(int64_t LHS, int64_t RHS) {
+  if (LHS == UnknownOffset || RHS == UnknownOffset)
----------------
If we get many more of these, it may be nice to just have a `cflaa::PointerOffset` type (or whatever) with operators that know how to handle checking for unknown. If only because it would make it more difficult to cause subtle bugs.

================
Comment at: lib/Analysis/CFLAndersAliasAnalysis.cpp:145
@@ +144,3 @@
+// Specialize DenseMapInfo for OffsetValue.
+template <> struct DenseMapInfo<OffsetValue> {
+  static OffsetValue getEmptyKey() {
----------------
Please rebase; Having `DenseMapInfo` outside of `namespace llvm {` upset GCC and made it issue warnings. :)

================
Comment at: lib/Analysis/CFLAndersAliasAnalysis.cpp:458
@@ -445,4 +457,3 @@
 
-        ExtRelations.push_back(ExternalRelation{
-            InterfaceValue{SrcIndex, SrcLevel},
-            InterfaceValue{DstIndex, DstLevel}, UnknownOffset});
+        // FIXME: The following ExternalRelation is INCORRECT when SrcLevel > 0
+        // or DstLevel > 0.
----------------
Until we make it correct, would it make sense to put an `assert(not_using_broken_params && "This won't produce correct output for ${case}.")`? Or will that loudly break lots of things?


https://reviews.llvm.org/D22721





More information about the llvm-commits mailing list