[ModRefInfo] Return NoModRef for Must and NoModRef.

[asbirlea]

Summary

In ModRefInfo "Must" was introduced to track presence of MustAlias, but we still want to return NoModRef when there is neither Mod or Ref, even when MustAlias is found. Patch has small fixes to ensure this happens.
Minor cleanup to compact 2 if statements into one when calling getModRefInfo for 2 ImmutableCallSites.

[sanjoy]

lib/Analysis/AliasAnalysis.cpp

256–258

I'm a bit confused by this -- before if onlyAccessesArgPointees was true and doesAccessArgPointees was false then we'd directly return ModRefInfo::NoModRef. However, now if onlyAccessesArgPointees is true and doesAccessArgPointees is false we're going to return Result after falling through to the end. Is that equivalent? If so, can you please add an assert(Result == NoModRef) or something like that?

[asbirlea]

Add shortcut path to NoModRef when second if test is false, unintended change of meaning in initial patch.

[sanjoy]

lib/Analysis/BasicAliasAnalysis.cpp

858

[minor] Instead of

if (X)
  return A;
else
  return B;

you can do

if (X)
  return A;
return B;

[asbirlea]

Address comment.
Thank you the review!

lib/Analysis/AliasAnalysis.cpp

 ModRefInfo AAResults::getArgModRefInfo(ImmutableCallSite CS, unsigned ArgIdx) {
   ModRefInfo Result = ModRefInfo::ModRef;
 
   for (const auto &AA : AAs) {
     Result = intersectModRef(Result, AA->getArgModRefInfo(CS, ArgIdx));
 
     // Early-exit the moment we reach the bottom of the lattice.
     if (isNoModRef(Result))
-      return Result;
+      return ModRefInfo::NoModRef;
   }
 
   return Result;
 }
 
 ModRefInfo AAResults::getModRefInfo(Instruction *I, ImmutableCallSite Call) {
   // We may have two calls.
   if (auto CS = ImmutableCallSite(I)) {
ModRefInfo AAResults::getModRefInfo(ImmutableCallSite CS,
                                     const MemoryLocation &Loc) {
   ModRefInfo Result = ModRefInfo::ModRef;
 
   for (const auto &AA : AAs) {
     Result = intersectModRef(Result, AA->getModRefInfo(CS, Loc));
 
     // Early-exit the moment we reach the bottom of the lattice.
     if (isNoModRef(Result))
-      return Result;
+      return ModRefInfo::NoModRef;
   }
 
   // Try to refine the mod-ref info further using other API entry points to the
   // aggregate set of AA results.
   auto MRB = getModRefBehavior(CS);
   if (MRB == FMRB_DoesNotAccessMemory ||
       MRB == FMRB_OnlyAccessesInaccessibleMem)
     return ModRefInfo::NoModRef;
ModRefInfo AAResults::getModRefInfo(ImmutableCallSite CS1,
                                     ImmutableCallSite CS2) {
   ModRefInfo Result = ModRefInfo::ModRef;
 
   for (const auto &AA : AAs) {
     Result = intersectModRef(Result, AA->getModRefInfo(CS1, CS2));
 
     // Early-exit the moment we reach the bottom of the lattice.
     if (isNoModRef(Result))
-      return Result;
+      return ModRefInfo::NoModRef;
   }
 
   // Try to refine the mod-ref info further using other API entry points to the
   // aggregate set of AA results.
 
   // If CS1 or CS2 are readnone, they don't interact.
   auto CS1B = getModRefBehavior(CS1);
   if (CS1B == FMRB_DoesNotAccessMemory)
ModRefInfo AAResults::getModRefInfo(ImmutableCallSite CS1,
   if (onlyReadsMemory(CS1B))
     Result = clearMod(Result);
   else if (doesNotReadMemory(CS1B))
     Result = clearRef(Result);
 
   // If CS2 only access memory through arguments, accumulate the mod/ref
   // information from CS1's references to the memory referenced by
   // CS2's arguments.
   if (onlyAccessesArgPointees(CS2B)) {
+    if (!doesAccessArgPointees(CS2B))
+      return ModRefInfo::NoModRef;
     ModRefInfo R = ModRefInfo::NoModRef;
-    if (doesAccessArgPointees(CS2B)) {
-      bool IsMustAlias = true;
-      for (auto I = CS2.arg_begin(), E = CS2.arg_end(); I != E; ++I) {
-        const Value *Arg = *I;
-        if (!Arg->getType()->isPointerTy())
-          continue;
-        unsigned CS2ArgIdx = std::distance(CS2.arg_begin(), I);
-        auto CS2ArgLoc = MemoryLocation::getForArgument(CS2, CS2ArgIdx, TLI);
 
-        // ArgModRefCS2 indicates what CS2 might do to CS2ArgLoc, and the
-        // dependence of CS1 on that location is the inverse:
-        // - If CS2 modifies location, dependence exists if CS1 reads or writes.
-        // - If CS2 only reads location, dependence exists if CS1 writes.
-        ModRefInfo ArgModRefCS2 = getArgModRefInfo(CS2, CS2ArgIdx);
-        ModRefInfo ArgMask = ModRefInfo::NoModRef;
-        if (isModSet(ArgModRefCS2))
-          ArgMask = ModRefInfo::ModRef;
-        else if (isRefSet(ArgModRefCS2))
-          ArgMask = ModRefInfo::Mod;
 
-        // ModRefCS1 indicates what CS1 might do to CS2ArgLoc, and we use
-        // above ArgMask to update dependence info.
-        ModRefInfo ModRefCS1 = getModRefInfo(CS1, CS2ArgLoc);
-        ArgMask = intersectModRef(ArgMask, ModRefCS1);
 
-        // Conservatively clear IsMustAlias unless only MustAlias is found.
-        IsMustAlias &= isMustSet(ModRefCS1);
 
-        R = intersectModRef(unionModRef(R, ArgMask), Result);
-        if (R == Result) {
-          // On early exit, not all args were checked, cannot set Must.
-          if (I + 1 != E)
-            IsMustAlias = false;
-          break;
-        }
-      }
-      // If Alias found and only MustAlias found above, set Must bit.
-      R = IsMustAlias ? setMust(R) : clearMust(R);
-    }
-    return R;
+
+    if (isNoModRef(R))
+      return ModRefInfo::NoModRef;
+
+    // If MustAlias found above, set Must bit.
+    return IsMustAlias ? setMust(R) : clearMust(R);
   }
 
   // If CS1 only accesses memory through arguments, check if CS2 references
   // any of the memory referenced by CS1's arguments. If not, return NoModRef.
   if (onlyAccessesArgPointees(CS1B)) {
+    if (!doesAccessArgPointees(CS1B))
+      return ModRefInfo::NoModRef;
     ModRefInfo R = ModRefInfo::NoModRef;
-    if (doesAccessArgPointees(CS1B)) {
-      bool IsMustAlias = true;
-      for (auto I = CS1.arg_begin(), E = CS1.arg_end(); I != E; ++I) {
-        const Value *Arg = *I;
-        if (!Arg->getType()->isPointerTy())
-          continue;
-        unsigned CS1ArgIdx = std::distance(CS1.arg_begin(), I);
-        auto CS1ArgLoc = MemoryLocation::getForArgument(CS1, CS1ArgIdx, TLI);
 
-        // ArgModRefCS1 indicates what CS1 might do to CS1ArgLoc; if CS1 might
-        // Mod CS1ArgLoc, then we care about either a Mod or a Ref by CS2. If
-        // CS1 might Ref, then we care only about a Mod by CS2.
-        ModRefInfo ArgModRefCS1 = getArgModRefInfo(CS1, CS1ArgIdx);
-        ModRefInfo ModRefCS2 = getModRefInfo(CS2, CS1ArgLoc);
-        if ((isModSet(ArgModRefCS1) && isModOrRefSet(ModRefCS2)) ||
-            (isRefSet(ArgModRefCS1) && isModSet(ModRefCS2)))
-          R = intersectModRef(unionModRef(R, ArgModRefCS1), Result);
 
-        // Conservatively clear IsMustAlias unless only MustAlias is found.
-        IsMustAlias &= isMustSet(ModRefCS2);
 
-        if (R == Result) {
-          // On early exit, not all args were checked, cannot set Must.
-          if (I + 1 != E)
-            IsMustAlias = false;
-          break;
-        }
-      }
-      // If Alias found and only MustAlias found above, set Must bit.
-      R = IsMustAlias ? setMust(R) : clearMust(R);
-    }
-    return R;
+
+    if (isNoModRef(R))
+      return ModRefInfo::NoModRef;
+
+    // If MustAlias found above, set Must bit.
+    return IsMustAlias ? setMust(R) : clearMust(R);
   }
 
   return Result;
 }
 
 FunctionModRefBehavior AAResults::getModRefBehavior(ImmutableCallSite CS) {
   FunctionModRefBehavior Result = FMRB_UnknownModRefBehavior;
 

lib/Analysis/BasicAliasAnalysis.cpp

if (!isa<Constant>(Object) && CS.getInstruction() != Object &&
     }
 
     // No operand aliases, reset Must bit. Add below if at least one aliases
     // and all aliases found are MustAlias.
     if (isNoModRef(Result))
       IsMustAlias = false;
 
     // Early return if we improved mod ref information
-    if (!isModAndRefSet(Result))
+    if (!isModAndRefSet(Result)) {
+      if (isNoModRef(Result))
+        return ModRefInfo::NoModRef;
+      else
         return IsMustAlias ? setMust(Result) : clearMust(Result);
     }
+  }
 
   // If the CallSite is to malloc or calloc, we can assume that it doesn't
   // modify any IR visible value.  This is only valid because we assume these
   // routines do not read values visible in the IR.  TODO: Consider special
   // casing realloc and strdup routines which access only their arguments as
   // well.  Or alternatively, replace all of this with inaccessiblememonly once
   // that's implemented fully.
   auto *Inst = CS.getInstruction();