* C PATCH to use is_global_var
@ 2015-06-24 10:35 Marek Polacek
2015-06-24 16:52 ` Jeff Law
2015-06-24 17:34 ` Joseph Myers
0 siblings, 2 replies; 5+ messages in thread
From: Marek Polacek @ 2015-06-24 10:35 UTC (permalink / raw)
To: GCC Patches
This patch makes the C FE use the predicate is_global_var in place of direct
TREE_STATIC (t) || DECL_EXTERNAL (t)
It should improve readability a bit and make predicates easier to follow.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-06-24 Marek Polacek <polacek@redhat.com>
* c-common.c (handle_no_reorder_attribute): Use is_global_var.
* cilk.c (extract_free_variables): Likewise.
* c-decl.c: Use is_global_var throughout.
* c-parser.c: Likewise.
* c-typeck.c: Likewise.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index dee6550..d315854 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -7446,8 +7446,7 @@ handle_no_reorder_attribute (tree *pnode,
{
tree node = *pnode;
- if (!VAR_OR_FUNCTION_DECL_P (node)
- && !(TREE_STATIC (node) || DECL_EXTERNAL (node)))
+ if (!VAR_OR_FUNCTION_DECL_P (node) && !is_global_var (node))
{
warning (OPT_Wattributes,
"%qE attribute only affects top level objects",
diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
index c38e05f..347e4b9 100644
--- gcc/c-family/cilk.c
+++ gcc/c-family/cilk.c
@@ -1063,7 +1063,7 @@ extract_free_variables (tree t, struct wrapper_data *wd,
TREE_ADDRESSABLE (t) = 1;
case VAR_DECL:
case PARM_DECL:
- if (!TREE_STATIC (t) && !DECL_EXTERNAL (t))
+ if (!is_global_var (t))
add_variable (wd, t, how);
return;
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index fc1fdf9..ab54db9 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2650,9 +2650,8 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
tree_code_size (TREE_CODE (olddecl)) - sizeof (struct tree_decl_common));
olddecl->decl_with_vis.symtab_node = snode;
- if ((DECL_EXTERNAL (olddecl)
- || TREE_PUBLIC (olddecl)
- || TREE_STATIC (olddecl))
+ if ((is_global_var (olddecl)
+ || TREE_PUBLIC (olddecl))
&& DECL_SECTION_NAME (newdecl) != NULL)
set_decl_section_name (olddecl, DECL_SECTION_NAME (newdecl));
@@ -4395,7 +4394,7 @@ c_decl_attributes (tree *node, tree attributes, int flags)
/* Add implicit "omp declare target" attribute if requested. */
if (current_omp_declare_target_attribute
&& ((TREE_CODE (*node) == VAR_DECL
- && (TREE_STATIC (*node) || DECL_EXTERNAL (*node)))
+ && is_global_var (*node))
|| TREE_CODE (*node) == FUNCTION_DECL))
{
if (TREE_CODE (*node) == VAR_DECL
@@ -4794,8 +4793,7 @@ finish_decl (tree decl, location_t init_loc, tree init,
TREE_TYPE (decl) = error_mark_node;
}
- if ((DECL_EXTERNAL (decl) || TREE_STATIC (decl))
- && DECL_SIZE (decl) != 0)
+ if (is_global_var (decl) && DECL_SIZE (decl) != 0)
{
if (TREE_CODE (DECL_SIZE (decl)) == INTEGER_CST)
constant_expression_warning (DECL_SIZE (decl));
@@ -4911,8 +4909,7 @@ finish_decl (tree decl, location_t init_loc, tree init,
{
/* Recompute the RTL of a local array now
if it used to be an incomplete type. */
- if (was_incomplete
- && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
+ if (was_incomplete && !is_global_var (decl))
{
/* If we used it already as memory, it must stay in memory. */
TREE_ADDRESSABLE (decl) = TREE_USED (decl);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index e0ab0a1..f4d18bd 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -14769,7 +14769,7 @@ c_parser_omp_threadprivate (c_parser *parser)
error_at (loc, "%qD is not a variable", v);
else if (TREE_USED (v) && !C_DECL_THREADPRIVATE_P (v))
error_at (loc, "%qE declared %<threadprivate%> after first use", v);
- else if (! TREE_STATIC (v) && ! DECL_EXTERNAL (v))
+ else if (! is_global_var (v))
error_at (loc, "automatic variable %qE cannot be %<threadprivate%>", v);
else if (TREE_TYPE (v) == error_mark_node)
;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index aeb1043..3dc1f07 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4380,7 +4380,7 @@ c_mark_addressable (tree exp)
if (C_DECL_REGISTER (x)
&& DECL_NONLOCAL (x))
{
- if (TREE_PUBLIC (x) || TREE_STATIC (x) || DECL_EXTERNAL (x))
+ if (TREE_PUBLIC (x) || is_global_var (x))
{
error
("global register variable %qD used in nested function", x);
@@ -4390,7 +4390,7 @@ c_mark_addressable (tree exp)
}
else if (C_DECL_REGISTER (x))
{
- if (TREE_PUBLIC (x) || TREE_STATIC (x) || DECL_EXTERNAL (x))
+ if (TREE_PUBLIC (x) || is_global_var (x))
error ("address of global register variable %qD requested", x);
else
error ("address of register variable %qD requested", x);
@@ -9470,8 +9470,7 @@ c_finish_return (location_t loc, tree retval, tree origtype)
inner = TREE_OPERAND (inner, 0);
if (DECL_P (inner)
- && !DECL_EXTERNAL (inner)
- && !TREE_STATIC (inner)
+ && !is_global_var (inner)
&& DECL_CONTEXT (inner) == current_function_decl)
{
if (TREE_CODE (inner) == LABEL_DECL)
Marek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C PATCH to use is_global_var
2015-06-24 10:35 C PATCH to use is_global_var Marek Polacek
@ 2015-06-24 16:52 ` Jeff Law
2015-06-24 17:34 ` Joseph Myers
1 sibling, 0 replies; 5+ messages in thread
From: Jeff Law @ 2015-06-24 16:52 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 06/24/2015 04:22 AM, Marek Polacek wrote:
> This patch makes the C FE use the predicate is_global_var in place of direct
>
> TREE_STATIC (t) || DECL_EXTERNAL (t)
>
> It should improve readability a bit and make predicates easier to follow.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2015-06-24 Marek Polacek <polacek@redhat.com>
>
> * c-common.c (handle_no_reorder_attribute): Use is_global_var.
> * cilk.c (extract_free_variables): Likewise.
>
> * c-decl.c: Use is_global_var throughout.
> * c-parser.c: Likewise.
> * c-typeck.c: Likewise.
OK. If you find other places where you can use is_global_var to replace
the TREE_STATIC || DECL_EXTERNAL check, consider them pre-approved.
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C PATCH to use is_global_var
2015-06-24 10:35 C PATCH to use is_global_var Marek Polacek
2015-06-24 16:52 ` Jeff Law
@ 2015-06-24 17:34 ` Joseph Myers
2015-06-25 12:46 ` Marek Polacek
1 sibling, 1 reply; 5+ messages in thread
From: Joseph Myers @ 2015-06-24 17:34 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Wed, 24 Jun 2015, Marek Polacek wrote:
> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> index fc1fdf9..ab54db9 100644
> --- gcc/c/c-decl.c
> +++ gcc/c/c-decl.c
> @@ -2650,9 +2650,8 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
> tree_code_size (TREE_CODE (olddecl)) - sizeof (struct tree_decl_common));
> olddecl->decl_with_vis.symtab_node = snode;
>
> - if ((DECL_EXTERNAL (olddecl)
> - || TREE_PUBLIC (olddecl)
> - || TREE_STATIC (olddecl))
> + if ((is_global_var (olddecl)
> + || TREE_PUBLIC (olddecl))
> && DECL_SECTION_NAME (newdecl) != NULL)
> set_decl_section_name (olddecl, DECL_SECTION_NAME (newdecl));
>
At least this case covers both FUNCTION_DECL and VAR_DECL. If
is_global_var is appropriate for functions as well as variables, I think
it should be renamed (and have its comment updated to explain what it
means for functions).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C PATCH to use is_global_var
2015-06-24 17:34 ` Joseph Myers
@ 2015-06-25 12:46 ` Marek Polacek
2015-06-25 16:24 ` Jeff Law
0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2015-06-25 12:46 UTC (permalink / raw)
To: Joseph Myers; +Cc: GCC Patches
On Wed, Jun 24, 2015 at 05:16:33PM +0000, Joseph Myers wrote:
> On Wed, 24 Jun 2015, Marek Polacek wrote:
>
> > diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> > index fc1fdf9..ab54db9 100644
> > --- gcc/c/c-decl.c
> > +++ gcc/c/c-decl.c
> > @@ -2650,9 +2650,8 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
> > tree_code_size (TREE_CODE (olddecl)) - sizeof (struct tree_decl_common));
> > olddecl->decl_with_vis.symtab_node = snode;
> >
> > - if ((DECL_EXTERNAL (olddecl)
> > - || TREE_PUBLIC (olddecl)
> > - || TREE_STATIC (olddecl))
> > + if ((is_global_var (olddecl)
> > + || TREE_PUBLIC (olddecl))
> > && DECL_SECTION_NAME (newdecl) != NULL)
> > set_decl_section_name (olddecl, DECL_SECTION_NAME (newdecl));
> >
>
> At least this case covers both FUNCTION_DECL and VAR_DECL. If
> is_global_var is appropriate for functions as well as variables, I think
> it should be renamed (and have its comment updated to explain what it
> means for functions).
You raise a good point. After fair amount of investigating, I don't think
is_global_var is appropriate for functions. (DECL_EXTERNAL || TREE_STATIC)
for a function is only false for weird cases such as an inline function
definition followed by redeclaring this function with noinline attribute, i.e.:
inline void a (void) {}
void a (void) __attribute__ ((noinline));
void
b ()
{
a ();
}
is_global_var with a FUNCTION_DECL is only called in tree-ssa-structalias.c
and it seems like a mistake.
So I propose to commit the following patch, which uses is_global_var only
at places where we're dealing with a variable.
Ok for trunk?
2015-06-25 Marek Polacek <polacek@redhat.com>
* cilk.c (extract_free_variables): Use is_global_var.
* c-decl.c: Use is_global_var throughout.
* c-parser.c: Likewise.
* c-typeck.c: Likewise.
diff --git gcc/c-family/cilk.c gcc/c-family/cilk.c
index c38e05f..347e4b9 100644
--- gcc/c-family/cilk.c
+++ gcc/c-family/cilk.c
@@ -1063,7 +1063,7 @@ extract_free_variables (tree t, struct wrapper_data *wd,
TREE_ADDRESSABLE (t) = 1;
case VAR_DECL:
case PARM_DECL:
- if (!TREE_STATIC (t) && !DECL_EXTERNAL (t))
+ if (!is_global_var (t))
add_variable (wd, t, how);
return;
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index fc1fdf9..ab54db9 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -4395,7 +4394,7 @@ c_decl_attributes (tree *node, tree attributes, int flags)
/* Add implicit "omp declare target" attribute if requested. */
if (current_omp_declare_target_attribute
&& ((TREE_CODE (*node) == VAR_DECL
- && (TREE_STATIC (*node) || DECL_EXTERNAL (*node)))
+ && is_global_var (*node))
|| TREE_CODE (*node) == FUNCTION_DECL))
{
if (TREE_CODE (*node) == VAR_DECL
@@ -4794,8 +4793,7 @@ finish_decl (tree decl, location_t init_loc, tree init,
TREE_TYPE (decl) = error_mark_node;
}
- if ((DECL_EXTERNAL (decl) || TREE_STATIC (decl))
- && DECL_SIZE (decl) != 0)
+ if (is_global_var (decl) && DECL_SIZE (decl) != 0)
{
if (TREE_CODE (DECL_SIZE (decl)) == INTEGER_CST)
constant_expression_warning (DECL_SIZE (decl));
@@ -4911,8 +4909,7 @@ finish_decl (tree decl, location_t init_loc, tree init,
{
/* Recompute the RTL of a local array now
if it used to be an incomplete type. */
- if (was_incomplete
- && !TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
+ if (was_incomplete && !is_global_var (decl))
{
/* If we used it already as memory, it must stay in memory. */
TREE_ADDRESSABLE (decl) = TREE_USED (decl);
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index e0ab0a1..f4d18bd 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -14769,7 +14769,7 @@ c_parser_omp_threadprivate (c_parser *parser)
error_at (loc, "%qD is not a variable", v);
else if (TREE_USED (v) && !C_DECL_THREADPRIVATE_P (v))
error_at (loc, "%qE declared %<threadprivate%> after first use", v);
- else if (! TREE_STATIC (v) && ! DECL_EXTERNAL (v))
+ else if (! is_global_var (v))
error_at (loc, "automatic variable %qE cannot be %<threadprivate%>", v);
else if (TREE_TYPE (v) == error_mark_node)
;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index aeb1043..3dc1f07 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -4380,7 +4380,7 @@ c_mark_addressable (tree exp)
if (C_DECL_REGISTER (x)
&& DECL_NONLOCAL (x))
{
- if (TREE_PUBLIC (x) || TREE_STATIC (x) || DECL_EXTERNAL (x))
+ if (TREE_PUBLIC (x) || is_global_var (x))
{
error
("global register variable %qD used in nested function", x);
@@ -4390,7 +4390,7 @@ c_mark_addressable (tree exp)
}
else if (C_DECL_REGISTER (x))
{
- if (TREE_PUBLIC (x) || TREE_STATIC (x) || DECL_EXTERNAL (x))
+ if (TREE_PUBLIC (x) || is_global_var (x))
error ("address of global register variable %qD requested", x);
else
error ("address of register variable %qD requested", x);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C PATCH to use is_global_var
2015-06-25 12:46 ` Marek Polacek
@ 2015-06-25 16:24 ` Jeff Law
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Law @ 2015-06-25 16:24 UTC (permalink / raw)
To: Marek Polacek, Joseph Myers; +Cc: GCC Patches
On 06/25/2015 06:44 AM, Marek Polacek wrote:
> On Wed, Jun 24, 2015 at 05:16:33PM +0000, Joseph Myers wrote:
>> On Wed, 24 Jun 2015, Marek Polacek wrote:
>>
>>> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
>>> index fc1fdf9..ab54db9 100644
>>> --- gcc/c/c-decl.c
>>> +++ gcc/c/c-decl.c
>>> @@ -2650,9 +2650,8 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
>>> tree_code_size (TREE_CODE (olddecl)) - sizeof (struct tree_decl_common));
>>> olddecl->decl_with_vis.symtab_node = snode;
>>>
>>> - if ((DECL_EXTERNAL (olddecl)
>>> - || TREE_PUBLIC (olddecl)
>>> - || TREE_STATIC (olddecl))
>>> + if ((is_global_var (olddecl)
>>> + || TREE_PUBLIC (olddecl))
>>> && DECL_SECTION_NAME (newdecl) != NULL)
>>> set_decl_section_name (olddecl, DECL_SECTION_NAME (newdecl));
>>>
>>
>> At least this case covers both FUNCTION_DECL and VAR_DECL. If
>> is_global_var is appropriate for functions as well as variables, I think
>> it should be renamed (and have its comment updated to explain what it
>> means for functions).
>
> You raise a good point. After fair amount of investigating, I don't think
> is_global_var is appropriate for functions. (DECL_EXTERNAL || TREE_STATIC)
> for a function is only false for weird cases such as an inline function
> definition followed by redeclaring this function with noinline attribute, i.e.:
>
> inline void a (void) {}
> void a (void) __attribute__ ((noinline));
>
> void
> b ()
> {
> a ();
> }
>
> is_global_var with a FUNCTION_DECL is only called in tree-ssa-structalias.c
> and it seems like a mistake.
>
> So I propose to commit the following patch, which uses is_global_var only
> at places where we're dealing with a variable.
>
> Ok for trunk?
>
> 2015-06-25 Marek Polacek <polacek@redhat.com>
>
> * cilk.c (extract_free_variables): Use is_global_var.
>
> * c-decl.c: Use is_global_var throughout.
> * c-parser.c: Likewise.
> * c-typeck.c: Likewise.
OK.
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-25 16:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-24 10:35 C PATCH to use is_global_var Marek Polacek
2015-06-24 16:52 ` Jeff Law
2015-06-24 17:34 ` Joseph Myers
2015-06-25 12:46 ` Marek Polacek
2015-06-25 16:24 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).