On Mon, 27 Jan 2020 at 17:36, Richard Biener <[email protected]> wrote:
>
> On Fri, Jan 24, 2020 at 11:53 PM Joseph Myers <[email protected]> wrote:
> >
> > On Fri, 24 Jan 2020, Prathamesh Kulkarni wrote:
> >
> > > The middle-end representation issue of ERF_RETURNS_ARG still remains,
> > > which restricts the attribute till first four args. The patch simply
> > > emits sorry(), for arguments beyond first four..
> >
> > I think this should be fixed (e.g. make the middle-end check for the
> > attribute, or something like that).
>
> Since it's pure optimization you can also simply chose to ignore this without
> notice.
>
> Note ERF_RETURN_ARG_MASK is limited to the number of available
> bits in an int and practically the only current setter was via "fn spec"
> which uses a single digit [1-9] to denote the argument (so limiting to
> three is indeed an odd choice but matches builtins using this at the moment).
>
> Feel free to up ERF_RETURN_ARG_MASK (but then you need to adjust
> the ERF_ flag defines).
Hi,
Thanks for the suggestions. In the attached patch I bumped up value of
ERF_RETURNS_ARG_MASK
to UINT_MAX >> 2, and use highest two bits for ERF_NOALIAS and ERF_RETURNS_ARG.
And use fn spec "Z<argnum>" to store the argument number in fn-spec format.
Does that look OK ?
In gimple_call_return_flags, I didn't remove the existing fn spec
"0-3" in this patch, since RET1 (and possibly others?) depend on it.
I will remove that and adjust other cases to use fn-spec "Z<argnum>"
if that's OK, in follow-up patch.
Thanks,
Prathamesh
>
> > The language semantics of the
> > attribute should not be driven by such internal implementation details;
> > rather, implementation details should be determined by the language
> > semantics to be implemented.
> >
> > The sorry () has coding style issues. Diagnostics should not end with '.'
> > or '\n', should use full words (attribute not attr, arguments not args)
> > and programming language text in them should be surrounded by %<%> (so
> > %<returns_arg%>).
> >
> > --
> > Joseph S. Myers
> > [email protected]
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index dc9579c5c60..2ed41ed136d 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -150,6 +150,7 @@ static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
int, bool *);
static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
+static tree handle_returns_arg_attribute (tree *, tree, tree, int, bool *);
/* Helper to define attribute exclusions. */
#define ATTR_EXCL(name, function, type, variable) \
@@ -484,6 +485,8 @@ const struct attribute_spec c_common_attribute_table[] =
handle_noinit_attribute, attr_noinit_exclusions },
{ "access", 1, 3, false, true, true, false,
handle_access_attribute, NULL },
+ { "returns_arg", 1, 1, true, false, false, false,
+ handle_returns_arg_attribute, NULL },
{ NULL, 0, 0, false, false, false, false, NULL, NULL }
};
@@ -4603,6 +4606,53 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
return NULL_TREE;
}
+/* Handle a "returns_arg" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_returns_arg_attribute (tree *node, tree name, tree args,
+ int flags, bool *no_add_attrs)
+{
+ tree decl = *node;
+ tree rettype = TREE_TYPE (decl);
+
+ if (TREE_CODE (rettype) == METHOD_TYPE
+ || TREE_CODE (rettype) == FUNCTION_TYPE)
+ rettype = TREE_TYPE (rettype);
+
+ if (VOID_TYPE_P (rettype))
+ {
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "%qE attribute ignored on a function returning %qT.",
+ name, rettype);
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ if (!positional_argument (TREE_TYPE (decl), name, TREE_VALUE (args),
+ TREE_CODE (rettype)))
+ {
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ *no_add_attrs = false;
+ gcc_assert (args);
+ tree val = TREE_VALUE (args);
+ unsigned HOST_WIDE_INT argnum = tree_to_uhwi (val);
+ char *s = (char *) xmalloc (sizeof (char) * BITS_PER_WORD);
+ s[0] = 'Z';
+ sprintf (s + 1, "%lu", argnum);
+
+ tree attr = tree_cons (get_identifier ("fn spec"),
+ build_tree_list (NULL_TREE,
+ build_string (strlen (s), s)),
+ NULL_TREE);
+ decl_attributes (node, attr, flags);
+ free (s);
+ return NULL_TREE;
+}
+
/* Attempt to partially validate a single attribute ATTR as if
it were to be applied to an entity OPER. */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index ec99c38a607..3531e0c8292 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3566,6 +3566,19 @@ diagnosed. Because a pure function cannot have any observable side
effects it does not make sense for such a function to return @code{void}.
Declaring such a function is diagnosed.
+@item returns_arg (@var{position})
+@cindex @code{returns_arg} function attribute
+The @code{returns_arg}, when applied to an argument states that
+the function returns that argument umodified.
+For instance, the declaration:
+
+@smallexample
+int foo(int x) __attribute__((returns_arg(1)));
+@end smallexample
+
+@noindent
+lets the compiler know that foo returns x.
+
@item returns_nonnull
@cindex @code{returns_nonnull} function attribute
The @code{returns_nonnull} attribute specifies that the function
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 324e706d508..ca26f671e32 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -1563,6 +1563,13 @@ gimple_call_return_flags (const gcall *stmt)
if (!attr || TREE_STRING_LENGTH (attr) < 1)
return 0;
+ const char *attr_str = TREE_STRING_POINTER (attr);
+ if (*attr_str == 'Z')
+ {
+ unsigned HOST_WIDE_INT argnum = strtoul (attr_str + 1, NULL, 10);
+ return ERF_RETURNS_ARG | argnum;
+ }
+
switch (TREE_STRING_POINTER (attr)[0])
{
case '1':
diff --git a/gcc/testsuite/g++.dg/Wattributes-6.C b/gcc/testsuite/g++.dg/Wattributes-6.C
new file mode 100644
index 00000000000..fcf660a4684
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wattributes-6.C
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* Check that 'this' is counted as first arg to the attribute. */
+
+struct X
+{
+ X *f() __attribute__((returns_arg(1)));
+};
+
+int main()
+{
+ X x;
+ x.f();
+}
diff --git a/gcc/testsuite/gcc.dg/Wattributes-11.c b/gcc/testsuite/gcc.dg/Wattributes-11.c
new file mode 100644
index 00000000000..81245b44cfa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wattributes-11.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wattributes" } */
+
+int f1 (int) __attribute__((returns_arg)); /* { dg-error "wrong number of arguments specified for 'returns_arg' attribute" } */
+
+void f2 (int) __attribute__((returns_arg (1))); /* { dg-warning "'returns_arg' attribute ignored on a function returning 'void'" } */
+
+int f3 (int) __attribute__((returns_arg ("foo"))); /* { dg-warning "'returns_arg' attribute argument has type 'char\\\[4\\\]'" } */
+
+int f4 (char *) __attribute__((returns_arg (1))); /* { dg-warning "'returns_arg' attribute argument value '1' refers to parameter type 'char \\*'" } */
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 765ea2a9542..4d76f4bb97e 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -111,17 +111,17 @@ struct die_struct;
#define EAF_UNUSED (1 << 3)
/* Call return flags. */
-/* Mask for the argument number that is returned. Lower two bits of
- the return flags, encodes argument slots zero to three. */
-#define ERF_RETURN_ARG_MASK (3)
+/* Mask for the argument number that is returned. The higher two bits
+ are reserved for return flags, and the rest for argument slots. */
+#define ERF_RETURN_ARG_MASK (UINT_MAX >> 2)
/* Nonzero if the return value is equal to the argument number
flags & ERF_RETURN_ARG_MASK. */
-#define ERF_RETURNS_ARG (1 << 2)
+#define ERF_RETURNS_ARG (1 << (BITS_PER_WORD - 2))
/* Nonzero if the return value does not alias with anything. Functions
with the malloc attribute have this set on their return value. */
-#define ERF_NOALIAS (1 << 3)
+#define ERF_NOALIAS (1 << (BITS_PER_WORD - 1))
/*---------------------------------------------------------------------------