[PATCH] Add read_write_arg_mem attribute

Igor Laevsky igor at azulsystems.com
Fri Jun 12 08:57:30 PDT 2015


I believe that all pointer arguments are ReadWrite by default. So there is no point in adding explicit ReadWrite attribute for each argument separately, if I understood you correctly.

If user wants to encode function which can read and write only from specific set of arguments he can use readonly or readnone attributes. I will add a test for this case.


================
Comment at: lib/AsmParser/LLParser.cpp:937
@@ -936,29 +936,3 @@
     }
-    case lltok::kw_alwaysinline:      B.addAttribute(Attribute::AlwaysInline); break;
-    case lltok::kw_builtin:           B.addAttribute(Attribute::Builtin); break;
-    case lltok::kw_cold:              B.addAttribute(Attribute::Cold); break;
-    case lltok::kw_convergent:        B.addAttribute(Attribute::Convergent); break;
-    case lltok::kw_inlinehint:        B.addAttribute(Attribute::InlineHint); break;
-    case lltok::kw_jumptable:         B.addAttribute(Attribute::JumpTable); break;
-    case lltok::kw_minsize:           B.addAttribute(Attribute::MinSize); break;
-    case lltok::kw_naked:             B.addAttribute(Attribute::Naked); break;
-    case lltok::kw_nobuiltin:         B.addAttribute(Attribute::NoBuiltin); break;
-    case lltok::kw_noduplicate:       B.addAttribute(Attribute::NoDuplicate); break;
-    case lltok::kw_noimplicitfloat:   B.addAttribute(Attribute::NoImplicitFloat); break;
-    case lltok::kw_noinline:          B.addAttribute(Attribute::NoInline); break;
-    case lltok::kw_nonlazybind:       B.addAttribute(Attribute::NonLazyBind); break;
-    case lltok::kw_noredzone:         B.addAttribute(Attribute::NoRedZone); break;
-    case lltok::kw_noreturn:          B.addAttribute(Attribute::NoReturn); break;
-    case lltok::kw_nounwind:          B.addAttribute(Attribute::NoUnwind); break;
-    case lltok::kw_optnone:           B.addAttribute(Attribute::OptimizeNone); break;
-    case lltok::kw_optsize:           B.addAttribute(Attribute::OptimizeForSize); break;
-    case lltok::kw_readnone:          B.addAttribute(Attribute::ReadNone); break;
-    case lltok::kw_readonly:          B.addAttribute(Attribute::ReadOnly); break;
-    case lltok::kw_returns_twice:     B.addAttribute(Attribute::ReturnsTwice); break;
-    case lltok::kw_ssp:               B.addAttribute(Attribute::StackProtect); break;
-    case lltok::kw_sspreq:            B.addAttribute(Attribute::StackProtectReq); break;
-    case lltok::kw_sspstrong:         B.addAttribute(Attribute::StackProtectStrong); break;
-    case lltok::kw_sanitize_address:  B.addAttribute(Attribute::SanitizeAddress); break;
-    case lltok::kw_sanitize_thread:   B.addAttribute(Attribute::SanitizeThread); break;
-    case lltok::kw_sanitize_memory:   B.addAttribute(Attribute::SanitizeMemory); break;
-    case lltok::kw_uwtable:           B.addAttribute(Attribute::UWTable); break;
+    case lltok::kw_alwaysinline:       B.addAttribute(Attribute::AlwaysInline); break;
+    case lltok::kw_builtin:            B.addAttribute(Attribute::Builtin); break;
----------------
sanjoy wrote:
> There's a whitespace issue here.
Since length of "kw_read_write_arg_mem" is one symbol more than maximum length of other kw_* identifiers in this case, I was forced to align all cases accordingly. Not sure what llvm coding standart says about such cases, but it seems reasonable to keep the alignment.

================
Comment at: lib/IR/Verifier.cpp:1449
@@ +1448,3 @@
+  Assert(
+      !(Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadOnly) &&
+        Attrs.hasAttribute(AttributeSet::FunctionIndex, Attribute::ReadWriteArgMem)),
----------------
sanjoy wrote:
> I'm not sure these should fail the verifier -- we should just drop `ReadWriteArgMem` in the `readonly` case, in in the readnone case replace with `ReadArgMem`.
I think it should fail here. It is safe to consider readonly function as ReadWriteArgMem, but not safe to do the opposite. If function is marked with both attributes it would mean that we can always consider it as readonly. If in reality it was ReadWriteArgMem, something would fail. Anyway even from intuitive point of view it is weird two have this two attributes on one function. It is like saying this function can write memory, but it also can not write memory.

http://reviews.llvm.org/D10398

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list