[llvm] r321309 - [ModRefInfo] Add must alias info to ModRefInfo.

[Nuno Lopes via llvm-commits]

Ah, this was my misunderstanding; the picture of the lattice I sent before 
was wrong..

Ok, now assuming I understand the meaning of each value, I'll claim the 
lattice now has 3 bottom values: NoModRef, MustRef, MustMod. These 3 are the 
most precise values and intersection of these with any other value cannot 
yield a more precision value (well, assuming there's no UB that could lead 
to some ⊥ value, which we don't have).

This function doesn't seem right to me:
inline bool isNoModRef(const ModRefInfo MRI) {
  return (MRI & MustModRef) == Must;
}

There's only one value representing NoModRef, so shouldn't this function 
just do a simple comparison "MRI == NoModRef"?


This function is then used here (and in other similar places):

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;
  }


NoModRef is only one of the bottom lattice values. I think that's not the 
correct function to call here. I think you need that check if the value is a 
power of 2 or 0 (just in case of UB):
inline bool isBottomModRef(const ModRefInfo MRI) {
  return MRI & (MRI-1) == 0;
}


Have a nice Christmas! :)

Nuno


-----Original Message----- 
From: Alina Sbirlea
Sent: Friday, December 22, 2017 9:19 PM
To: Nuno Lopes
Cc: llvm-commits
Subject: Re: [llvm] r321309 - [ModRefInfo] Add must alias info to 
ModRefInfo.


Hi Nuno,

The definition for setting Must is whether must alias with the location was 
found and there is no alias with other locations.
Yes, it's possible a write does not occur for cmpxchg, and that's fine. 
ModRef means it may read and it may write, but it doesn't necessarily mean 
it will. MustModRef means it may read and it may write, and if so, there is 
a must alias with that location and with no other locations. It doesn't mean 
it must read and write to that location.


The opposite scenario: if, say, you knew for sure statement S writes to A, 
and may write to B, then Must will not be set, because of B. So you get Mod.
Whereas if S *may* write to A and accesses nothing else, then you get 
MustMod.


Does this make sense?

Please let me know if I'm missing something obvious here or whether I should 
update the documentation to make this clearer.

Thanks,
Alina



On Fri, Dec 22, 2017 at 8:10 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
Hi Alina,

Some comments below; I think some operations cannot be marked as MustModRef:


@@ -440,9 +479,17 @@ ModRefInfo AAResults::getModRefInfo(cons
   if (isStrongerThanMonotonic(CX->getSuccessOrdering()))
     return ModRefInfo::ModRef;

-  // If the cmpxchg address does not alias the location, it does not access 
it.
-  if (Loc.Ptr && !alias(MemoryLocation::get(CX), Loc))
-    return ModRefInfo::NoModRef;
+  if (Loc.Ptr) {
+    AliasResult AR = alias(MemoryLocation::get(CX), Loc);
+    // If the cmpxchg address does not alias the location, it does not 
access
+    // it.
+    if (AR == NoAlias)
+      return ModRefInfo::NoModRef;
+
+    // If the cmpxchg address aliases the pointer as must alias, set Must.
+    if (AR == MustAlias)
+      return ModRefInfo::MustModRef;
+  }

According to the manual (http://llvm.org/docs/LangRef.html#id205), cmpxchg 
may or may not write to the location, depending on whether the comparison 
succeeds. For weak operations, the write may not occur even if the 
comparison succeeds.


@@ -453,9 +500,17 @@ ModRefInfo AAResults::getModRefInfo(cons
   if (isStrongerThanMonotonic(RMW->getOrdering()))
     return ModRefInfo::ModRef;

-  // If the atomicrmw address does not alias the location, it does not 
access it.
-  if (Loc.Ptr && !alias(MemoryLocation::get(RMW), Loc))
-    return ModRefInfo::NoModRef;
+  if (Loc.Ptr) {
+    AliasResult AR = alias(MemoryLocation::get(RMW), Loc);
+    // If the atomicrmw address does not alias the location, it does not 
access
+    // it.
+    if (AR == NoAlias)
+      return ModRefInfo::NoModRef;
+
+    // If the atomicrmw address aliases the pointer as must alias, set 
Must.
+    if (AR == MustAlias)
+      return ModRefInfo::MustModRef;
+  }


According to the manual (http://llvm.org/docs/LangRef.html#id210) not all 
'atomicrmw' operations read the pointer. The instruction always writes, but 
may not read (depending on the underlying operation).

Nuno