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

[Jia Chen via llvm-commits]

grievejia added inline comments.

================
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)
----------------
george.burgess.iv wrote:
> 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.
Agreed. The addOffset() and subOffset() functions are just quick helpers I wrote to get things going. At some point we may want to switch to a dedicated newtype.

================
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.
----------------
george.burgess.iv wrote:
> 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?
Unfortunately I think compared to those cases where this work, there are far more cases where this doesn't work :(

I'm going to abandon this patch for now, since committing into the codebase will just break CFLAnders and I don't think it can be fixed quickly in the near future. I'll bring it back once I get a better understanding of how to properly construct a field-sensitive function summary. 


https://reviews.llvm.org/D22721