[PATCH] D11352: Emit argmemonly attribute for intrinsics
Igor Laevsky
igor at azulsystems.com
Wed Jul 22 07:45:48 PDT 2015
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
More information about the llvm-commits
mailing list