[PATCH] Add read_write_arg_mem attribute

[Philip Reames]

================
Comment at: include/llvm/Analysis/AliasAnalysis.h:208
@@ -207,3 +207,3 @@
     ///
     /// This property corresponds to the IntrReadArgMem LLVM intrinsic flag.
     OnlyReadsArgumentPointees = ArgumentPointees | Ref,
----------------
This now matches read_write_arg_mem + readonly right?  If so, please update comment.  

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:684
@@ -683,1 +683,3 @@
 
+  if (CS.readsWritesOnlyArgMem())
+    Min = OnlyAccessesArgumentPointees;
----------------
I think you can get the only reads arguments from this attribute and readonly.

================
Comment at: lib/IR/Verifier.cpp:1449
@@ +1448,3 @@
+  Assert(
+      !(Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadOnly) &&
+        Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadWriteArgMem)),
----------------
igor-laevsky wrote:
> sanjoy wrote:
> > I'm not sure these should fail the verifier -- we should just drop `ReadWriteArgMem` in the `readonly` case, in in the readnone case replace with `ReadArgMem`.
> I think it should fail here. It is safe to consider readonly function as ReadWriteArgMem, but not safe to do the opposite. If function is marked with both attributes it would mean that we can always consider it as readonly. If in reality it was ReadWriteArgMem, something would fail. Anyway even from intuitive point of view it is weird two have this two attributes on one function. It is like saying this function can write memory, but it also can not write memory.
> 
It is specifically not legal to consider a readonly function as ReadWriteArgMem.  A readonly function could read from a global which is not passed as an argument.  

I think the combination of read_write_arg_mem and readonly is meaningful.  This is specifically read_arg_mem (i.e. only read memory pointer to by arguments).

One way of framing this is that read_write_arg_mem is a special form of a read/write set for the function.  Where the set is interpreted as RW or just R depends on other knowledge of the function (i.e. the readonly attribute.)

For this reason, I'd be tempted to rename this as "access_arg_mem_only" or something.  Before actually doing that mechanical change, please raise this in an email to llvmdev.  I think this needs broader attention.  (In particular, I wouldn't feel comfortable signing off on this.  The scope is too large.)

http://reviews.llvm.org/D10398

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/