[PATCH] D11352: Emit argmemonly attribute for intrinsics

[Philip Reames]

reames added a comment.

I can't point at a particular problem in the code, but the changes in the test files posted looks highly suspicious.  I strongly suspect this patch is buggy.  If you're convinced it's not, please explain the test changes I highlighted.


================
Comment at: test/Analysis/BasicAA/cs-cs.ll:239
@@ +238,3 @@
+attributes #2 = { noinline nounwind readonly }
+attributes #3 = { nounwind ssp }
+attributes #4 = { nounwind }
----------------
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.

================
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 }
----------------
Again, your change should be splitting existing attribute sets, not adding new combination of existing attributes.  

================
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
----------------
You can add the name without repeating the type if desired:
enum ModRefKind { .. } ModRef;

================
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"
----------------
I would expect to see this table disappear.  All of the information in it should now be driven by attributes right?


Repository:
  rL LLVM

http://reviews.llvm.org/D11352