[PATCH] D11352: Emit argmemonly attribute for intrinsics

Philip Reames listmail at philipreames.com
Wed Jul 22 13:44:20 PDT 2015


reames added a comment.

I'd like to have someone more familiar with this code take a look before submission, but with the explanation of the test changes, the code LGTM.


================
Comment at: test/Analysis/BasicAA/cs-cs.ll:239
@@ +238,3 @@
+attributes #2 = { noinline nounwind readonly }
+attributes #3 = { nounwind ssp }
+attributes #4 = { nounwind }
----------------
igor-laevsky wrote:
> 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 }
> ```
> 
> 
Ah!  Please submit a separate change to make the existing tests more complete.  That test cleanup change doesn't need review.  That will make the diff far more obvious.  

Thanks for explaining this.


Repository:
  rL LLVM

http://reviews.llvm.org/D11352







More information about the llvm-commits mailing list