[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