[PATCH] D11352: Emit argmemonly attribute for intrinsics

[Igor Laevsky]

igor-laevsky added inline comments.

================
Comment at: test/Analysis/BasicAA/cs-cs.ll:239
@@ +238,3 @@
+attributes #2 = { noinline nounwind readonly }
+attributes #3 = { nounwind ssp }
+attributes #4 = { nounwind }
----------------
reames wrote:
> The fact you're having to add argument sets containing anything other than the attributes which were there before and your argmemonly attribute is highly suspicious.  I suspect this is the symptom of a bug.
In original test not all attributes were specified explicitly in the IR file. What actually happens is that before my change there was this set of attributes:

```
attributes #0 = { nounwind readonly }
attributes #1 = { nounwind }
attributes #2 = { noinline nounwind readonly }
attributes #3 = { nounwind ssp }
```

And after my change:

```
attributes #0 = { nounwind readonly argmemonly }
attributes #1 = { nounwind argmemonly }
attributes #2 = { noinline nounwind readonly }
attributes #3 = { nounwind ssp }
attributes #4 = { nounwind }
```



================
Comment at: test/Transforms/Inline/inline_invoke.ll:347
@@ -346,2 +346,3 @@
 ; CHECK: attributes #2 = { ssp uwtable }
-; CHECK: attributes #3 = { noreturn nounwind }
+; CHECK: attributes #3 = { nounwind argmemonly }
+; CHECK: attributes #4 = { noreturn nounwind }
----------------
reames wrote:
> Again, your change should be splitting existing attribute sets, not adding new combination of existing attributes.  
Don't see a problem here. {nounwind} is now split into {nounwind} and {nounwind,argmemonly}

================
Comment at: utils/TableGen/CodeGenIntrinsics.h:63
@@ -62,3 +62,3 @@
     // Memory mod/ref behavior of this intrinsic.
-    enum {
+    enum ModRefKind {
       NoMem, ReadArgMem, ReadMem, ReadWriteArgMem, ReadWriteMem
----------------
reames wrote:
> You can add the name without repeating the type if desired:
> enum ModRefKind { .. } ModRef;
I like style with two definitions more. But I really don't have a strong preference on this.

================
Comment at: utils/TableGen/IntrinsicEmitter.cpp:708
@@ -718,3 +707,3 @@
 EmitModRefBehavior(const std::vector<CodeGenIntrinsic> &Ints, raw_ostream &OS){
   OS << "// Determine intrinsic alias analysis mod/ref behavior.\n"
      << "#ifdef GET_INTRINSIC_MODREF_BEHAVIOR\n"
----------------
reames wrote:
> I would expect to see this table disappear.  All of the information in it should now be driven by attributes right?
Yes, that the plan. I will remove usages of this table in a next few changes. After them it will become fully redundant.


Repository:
  rL LLVM

http://reviews.llvm.org/D11352