[PATCH] D23077: [CFLAA] Make CFLAnders more conservative when it sees newly created values

[Jia Chen via llvm-commits]

grievejia created this revision.
grievejia added reviewers: george.burgess.iv, hfinkel.
grievejia added a subscriber: llvm-commits.

There was a patch https://reviews.llvm.org/D22981 that deals with how CFLAnders interacts with values it hasn't seen before. In that patch the workaround is to let getAttrs() return AttrUnknown when newly created values are queried on. 

However, the aforementioned workaround fails to work for queries of the form mayAlias(L, N), where L is marked AttrNone and N is created after CFLAnders has executed. The sound was to handle the situation is to let the may-alias query return true (since N may be of the form "bitcast L to ...", "gep L, ...", etc.), but our current implementation will always return false because AttrNone values never may-alias AttrUnknown values.

This patch separate the new-value checks and attribute checks, so that soundness issues mentioned in the previous paragraph will not happen. 

https://reviews.llvm.org/D23077

Files:
  lib/Analysis/CFLAndersAliasAnalysis.cpp

Index: lib/Analysis/CFLAndersAliasAnalysis.cpp
===================================================================
--- lib/Analysis/CFLAndersAliasAnalysis.cpp
+++ lib/Analysis/CFLAndersAliasAnalysis.cpp
@@ -305,7 +305,7 @@
   /// Summary of externally visible effects.
   AliasSummary Summary;
 
-  AliasAttrs getAttrs(const Value *) const;
+  Optional<AliasAttrs> getAttrs(const Value *) const;
 
 public:
   FunctionInfo(const Function &, const SmallVectorImpl<Value *> &,
@@ -479,28 +479,33 @@
   populateExternalRelations(Summary.RetParamRelations, Fn, RetVals, ReachSet);
 }
 
-AliasAttrs CFLAndersAAResult::FunctionInfo::getAttrs(const Value *V) const {
+Optional<AliasAttrs>
+CFLAndersAAResult::FunctionInfo::getAttrs(const Value *V) const {
   assert(V != nullptr);
 
-  // Return AttrUnknown if V is not found in AttrMap. Sometimes V can be created
-  // after the analysis gets executed, and we want to be conservative in
-  // those cases.
-  AliasAttrs Attr = getAttrUnknown();
   auto Itr = AttrMap.find(V);
   if (Itr != AttrMap.end())
-    Attr = Itr->second;
-  return Attr;
+    return Itr->second;
+  return None;
 }
 
 bool CFLAndersAAResult::FunctionInfo::mayAlias(const Value *LHS,
                                                uint64_t LHSSize,
                                                const Value *RHS,
                                                uint64_t RHSSize) const {
   assert(LHS && RHS);
 
-  // Check AliasAttrs first since it's cheaper
-  auto AttrsA = getAttrs(LHS);
-  auto AttrsB = getAttrs(RHS);
+  // Check if we've seen LHS and RHS before. Sometimes LHS or RHS can be created
+  // after the analysis gets executed, and we want to be conservative in those
+  // cases.
+  auto MaybeAttrsA = getAttrs(LHS);
+  auto MaybeAttrsB = getAttrs(RHS);
+  if (!MaybeAttrsA || !MaybeAttrsB)
+    return true;
+
+  // Check AliasAttrs before AliasMap lookup since it's cheaper
+  auto AttrsA = *MaybeAttrsA;
+  auto AttrsB = *MaybeAttrsB;
   if (hasUnknownOrCallerAttr(AttrsA))
     return AttrsB.any();
   if (hasUnknownOrCallerAttr(AttrsB))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23077.66517.patch
Type: text/x-patch
Size: 2083 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160802/3964eda8/attachment.bin>