Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
Hi,
On Wed, 20 Apr 2011, Richard Guenther wrote:
I had occasion to try this today; this inheritance structure doesn't
work. The truncated inheritance tree looks like:
* decl_common
* field_decl
* const_decl
* decl_with_rtl
* label_decl
* result_decl
* parm_decl
* decl_with_vis...
In particular, FIELD_DECLs have a size, but they have no RTL associated
with them. And LABEL_DECLs have RTL, but no size.
Blaeh. So far about nice clean ideas :) One hacky idea: change my
proposal to this:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl_or_size : decl_common {
# add align, pt_uid
union {
rtx rtl;
tree size;
} u;
}
decl_with_size : decl_with_rtl_or_size {
# add size, size_unit
}
Use the rtl_or_size struct primarily for the current _with_rtl things that
don't naturally have a size, but use it also for FIELD_DECLs via the
union.
Alternatively I could also envision making a new tree_ struct for just
field_decls, that would contain the size and other fields that currently
are in decl_common just for fields (in particular the off_align) member.
The various accessors like DECL_SIZE would then need to dispatch based on
tree code.
Also doesn't sound terribly sexy.
FWIW I'm usually against on the side mappings A-B if most As will most of
the time be associated with a B. A size _is_ associated with the entity
always (for entities where it makes sense to talk about sizes), so that's
exactly where I would find on the side tables strange. For DECL_RTL it's
less clear.
Ciao,
Michael.
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
On Thu, Apr 21, 2011 at 05:54:28PM +0200, Michael Matz wrote:
In particular, FIELD_DECLs have a size, but they have no RTL associated
with them. And LABEL_DECLs have RTL, but no size.
Blaeh. So far about nice clean ideas :) One hacky idea: change my
proposal to this:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl_or_size : decl_common {
# add align, pt_uid
union {
rtx rtl;
tree size;
} u;
}
decl_with_size : decl_with_rtl_or_size {
# add size, size_unit
}
Use the rtl_or_size struct primarily for the current _with_rtl things that
don't naturally have a size, but use it also for FIELD_DECLs via the
union.
I'm not sure I follow this wrt FIELD_DECLs. Before you have:
...lots of decl fields..
tree size;
tree size_unit;
After you have:
...lots of decl fields...
union { rtx rtl; tree size; } u;
tree size;
tree size_unit;
Or did you mean something like:
/* As above. */
decl_with_rtl_or_size
/* Add a size_unit field. */
decl_with_size_unit : decl_with_rtl_or_size /* FIELD_DECL */
/* Add a size field for DECLs that do have RTL. */
decl_with_rtl_and_size : decl_with_size_unit /* VAR_DECL, PARM_DECL, etc. */
which looks just awful. :)
Alternatively I could also envision making a new tree_ struct for just
field_decls, that would contain the size and other fields that currently
are in decl_common just for fields (in particular the off_align) member.
The various accessors like DECL_SIZE would then need to dispatch based on
tree code.
You could also do something like:
struct tree_decl_size {
tree size;
tree size_unit;
...
};
struct tree_field_decl {
...
struct tree_decl_size size;
};
struct tree_var_decl {
...
struct tree_decl_size size;
};
static inline tree *
decl_size_1 (tree node)
{
switch (TREE_CODE (node))
{
case FIELD_DECL: return node-field_decl.size.size;
case VAR_DECL: return node-var_decl.size.size;
...
default: gcc_unreachable ();
}
}
/* Might also need a HAS_DECL_SIZE_P predicate or similar. */
#define DECL_SIZE(NODE) (*decl_size_1 (NODE))
...
which would make it somewhat more obvious when things have sizes, as
well as letting you remove DECL_SIZE{,_UNIT} from FUNCTION_DECL.
Slimming CONST_DECL and LABEL_DECL like this is useful mostly for code
cleanliness, but eliminating such fields from FUNCTION_DECL would have a
much more noticeable memory size impact.
-Nathan
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
On Wed, Apr 20, 2011 at 12:00 AM, Nathan Froyd froy...@codesourcery.com wrote:
On Tue, Apr 05, 2011 at 05:55:33PM +0200, Michael Matz wrote:
I have a preference in having just one DECL_RTL field for conceptual
reasons:
Most DECLs are actually objects (there are some prominent exceptions, but
those always would be better described with something like NAMED_ENTITY,
if we had something like that, namespaces and translation_unit would
qualify). All these have a RTL representation, so one field for them
seems appropriate. That some of those don't have a size (either because
size makes no sense or is always available via type size) hints towards a
problem in the inheritance. I would think it should look like so:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl : decl_common {
# add rtl, align, pt_uid
}
decl_with_size : decl_with_rtl {
# add size, size_unit
}
Then decl_common can still be used for
imported_decl/namespace/translation_unit; objects
are at least decl_with_rtl, and some objects will be decl_with_size.
I had occasion to try this today; this inheritance structure doesn't
work. The truncated inheritance tree looks like:
* decl_common
* field_decl
* const_decl
* decl_with_rtl
* label_decl
* result_decl
* parm_decl
* decl_with_vis...
In particular, FIELD_DECLs have a size, but they have no RTL associated
with them. And LABEL_DECLs have RTL, but no size. So if you went with
the above, FIELD_DECLs would grow by one (useless) word. And the
reverse is the situation we have today, where CONST_DECLs and
LABEL_DECLs (at least) have a pointless DECL_SIZE. Ideally, we could
fix things like FUNCTION_DECLs having a size, too...
And I didn't check the C++ FE to see if there are problematic cases
there, either.
What do you think is the next step? To address this issue, we could
just give LABEL_DECL its own rtx field as in the original patch, and
that would resolve that. But maybe we should go further, say by making
DECL_SIZE{,_UNIT} and/or DECL_RTL into actual (out-of-line function)
accessors; these accessors can then access structure-specific bits of
data. Then we don't have to worry about the inheritance structure, and
maybe could adopt alternate storage schemes for different DECLs, such as
the off-to-the-side table that Steven suggested.
Another option is to change nothing ;)
Conceptually I'd say not storing DECL_RTL in the decls themselves but
on the side would make sense, at least from a stylish view. I'm not sure
it'll work out very well though in terms of cost benefit.
What we could do is, if we ever can dispose of DECL/TYPE_LANG_SPECIFIC
after lowering to gimple, overload that field with a DECL/TYPE_RTL_SPECIFIC
field ...
Richard.
-Nathan
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
On Tue, Apr 05, 2011 at 05:55:33PM +0200, Michael Matz wrote:
I have a preference in having just one DECL_RTL field for conceptual
reasons:
Most DECLs are actually objects (there are some prominent exceptions, but
those always would be better described with something like NAMED_ENTITY,
if we had something like that, namespaces and translation_unit would
qualify). All these have a RTL representation, so one field for them
seems appropriate. That some of those don't have a size (either because
size makes no sense or is always available via type size) hints towards a
problem in the inheritance. I would think it should look like so:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl : decl_common {
# add rtl, align, pt_uid
}
decl_with_size : decl_with_rtl {
# add size, size_unit
}
Then decl_common can still be used for
imported_decl/namespace/translation_unit; objects
are at least decl_with_rtl, and some objects will be decl_with_size.
I had occasion to try this today; this inheritance structure doesn't
work. The truncated inheritance tree looks like:
* decl_common
* field_decl
* const_decl
* decl_with_rtl
* label_decl
* result_decl
* parm_decl
* decl_with_vis...
In particular, FIELD_DECLs have a size, but they have no RTL associated
with them. And LABEL_DECLs have RTL, but no size. So if you went with
the above, FIELD_DECLs would grow by one (useless) word. And the
reverse is the situation we have today, where CONST_DECLs and
LABEL_DECLs (at least) have a pointless DECL_SIZE. Ideally, we could
fix things like FUNCTION_DECLs having a size, too...
And I didn't check the C++ FE to see if there are problematic cases
there, either.
What do you think is the next step? To address this issue, we could
just give LABEL_DECL its own rtx field as in the original patch, and
that would resolve that. But maybe we should go further, say by making
DECL_SIZE{,_UNIT} and/or DECL_RTL into actual (out-of-line function)
accessors; these accessors can then access structure-specific bits of
data. Then we don't have to worry about the inheritance structure, and
maybe could adopt alternate storage schemes for different DECLs, such as
the off-to-the-side table that Steven suggested.
-Nathan
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
Hi,
On Mon, 4 Apr 2011, Nathan Froyd wrote:
On Mon, Apr 04, 2011 at 05:52:00PM +0200, Steven Bosscher wrote:
Have you looked into maybe putting the CODE_LABEL for a LABEL_DECL in
an on-the-side structure (hash table, whatever)? It looks like it is
only used during expansion of SWITCH statements.
I haven't, though it'd be easy enough once all accesses go through
label_rtx. I'd be equally happy doing that instead of pushing DECL_RTX
down.
I have a preference in having just one DECL_RTL field for conceptual
reasons:
Most DECLs are actually objects (there are some prominent exceptions, but
those always would be better described with something like NAMED_ENTITY,
if we had something like that, namespaces and translation_unit would
qualify). All these have a RTL representation, so one field for them
seems appropriate. That some of those don't have a size (either because
size makes no sense or is always available via type size) hints towards a
problem in the inheritance. I would think it should look like so:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl : decl_common {
# add rtl, align, pt_uid
}
decl_with_size : decl_with_rtl {
# add size, size_unit
}
Then decl_common can still be used for
imported_decl/namespace/translation_unit; objects
are at least decl_with_rtl, and some objects will be decl_with_size.
Ciao,
Michael.
5B
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
On Tue, Apr 05, 2011 at 05:55:33PM +0200, Michael Matz wrote:
I have a preference in having just one DECL_RTL field for conceptual
reasons:
Most DECLs are actually objects (there are some prominent exceptions, but
those always would be better described with something like NAMED_ENTITY,
if we had something like that, namespaces and translation_unit would
qualify). All these have a RTL representation, so one field for them
seems appropriate. That some of those don't have a size (either because
size makes no sense or is always available via type size) hints towards a
problem in the inheritance. I would think it should look like so:
decl_common {} # no size, no rtl, no align, no pt_uid
decl_with_rtl : decl_common {
# add rtl, align, pt_uid
}
decl_with_size : decl_with_rtl {
# add size, size_unit
}
Then decl_common can still be used for
imported_decl/namespace/translation_unit; objects
are at least decl_with_rtl, and some objects will be decl_with_size.
This is what I was heading for, but I hadn't fully worked out how to
switch around the inheritance structure. Thanks!
Consider this patch dropped, then; I'll work towards something like the
above instead.
-Nathan
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
Hi Nathan, Have you looked into maybe putting the CODE_LABEL for a LABEL_DECL in an on-the-side structure (hash table, whatever)? It looks like it is only used during expansion of SWITCH statements. Ciao! Steven
Re: [PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
On Mon, Apr 04, 2011 at 05:52:00PM +0200, Steven Bosscher wrote: Have you looked into maybe putting the CODE_LABEL for a LABEL_DECL in an on-the-side structure (hash table, whatever)? It looks like it is only used during expansion of SWITCH statements. I haven't, though it'd be easy enough once all accesses go through label_rtx. I'd be equally happy doing that instead of pushing DECL_RTX down. -Nathan
[PATCH] make LABEL_DECL has its own rtx field for its associated CODE_LABEL
This patch does just what $SUBJECT suggests: pushes down the DECL_RTL
field into LABEL_DECL. In this way, LABEL_DECL can inherit from
tree_decl_common instead of tree_decl_with_rtl.
I realize this looks like pure code shuffling; the reason for doing this
is that I want to split tree_decl_common into two structures: one that
includes DECL_SIZE and DECL_SIZE_UNIT and one that doesn't. The latter
can then be used for CONST_DECL, LABEL_DECL, and--possibly--RESULT_DECL
and--*maybe*--PARM_DECL. (Once the latter two have DECL_RTL pushed
down as well, of course.) And I think that less multipurposing of
DECL_RTL is not a bad thing.
Tested on x86_64-unknown-linux-gnu. OK to commit?
-Nathan
* tree.h (struct tree_label_decl): Inherit from tree_decl_common.
Add `label' field.
(LABEL_DECL_CODE_LABEL): New macro.
* stmt.c (label_rtx): Use it.
(expand_label): Use `label_r', rather than fetching DECL_RTL.
* tree.c (initialize_tree_contains_struct): Adjust LABEL_DECL
inheritance and tree_contains_struct asserts.
diff --git a/gcc/stmt.c b/gcc/stmt.c
index 1a9f9e5..13a906f 100644
--- a/gcc/stmt.c
+++ b/gcc/stmt.c
@@ -135,17 +135,15 @@ static struct case_node *add_case_node (struct case_node
*, tree,
rtx
label_rtx (tree label)
{
- gcc_assert (TREE_CODE (label) == LABEL_DECL);
-
- if (!DECL_RTL_SET_P (label))
+ if (!LABEL_DECL_CODE_LABEL (label))
{
rtx r = gen_label_rtx ();
- SET_DECL_RTL (label, r);
+ LABEL_DECL_CODE_LABEL (label) = r;
if (FORCED_LABEL (label) || DECL_NONLOCAL (label))
LABEL_PRESERVE_P (r) = 1;
}
- return DECL_RTL (label);
+ return LABEL_DECL_CODE_LABEL (label);
}
/* As above, but also put it on the forced-reference list of the
@@ -207,7 +205,7 @@ expand_label (tree label)
do_pending_stack_adjust ();
emit_label (label_r);
if (DECL_NAME (label))
-LABEL_NAME (DECL_RTL (label)) = IDENTIFIER_POINTER (DECL_NAME (label));
+LABEL_NAME (label_r) = IDENTIFIER_POINTER (DECL_NAME (label));
if (DECL_NONLOCAL (label))
{
diff --git a/gcc/tree.c b/gcc/tree.c
index ee47982..3c2154f 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -440,6 +440,7 @@ initialize_tree_contains_struct (void)
case TS_DECL_WRTL:
case TS_CONST_DECL:
+ case TS_LABEL_DECL:
MARK_TS_DECL_COMMON (code);
break;
@@ -449,7 +450,6 @@ initialize_tree_contains_struct (void)
case TS_DECL_WITH_VIS:
case TS_PARM_DECL:
- case TS_LABEL_DECL:
case TS_RESULT_DECL:
MARK_TS_DECL_WRTL (code);
break;
@@ -492,7 +492,6 @@ initialize_tree_contains_struct (void)
gcc_assert (tree_contains_struct[PARM_DECL][TS_DECL_WRTL]);
gcc_assert (tree_contains_struct[RESULT_DECL][TS_DECL_WRTL]);
gcc_assert (tree_contains_struct[FUNCTION_DECL][TS_DECL_WRTL]);
- gcc_assert (tree_contains_struct[LABEL_DECL][TS_DECL_WRTL]);
gcc_assert (tree_contains_struct[CONST_DECL][TS_DECL_MINIMAL]);
gcc_assert (tree_contains_struct[VAR_DECL][TS_DECL_MINIMAL]);
gcc_assert (tree_contains_struct[PARM_DECL][TS_DECL_MINIMAL]);
diff --git a/gcc/tree.h b/gcc/tree.h
index fcdebd9..952e13d 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2940,6 +2940,11 @@ struct GTY(()) tree_field_decl {
#define LABEL_DECL_UID(NODE) \
(LABEL_DECL_CHECK (NODE)-label_decl.label_decl_uid)
+/* The CODE_LABEL associated with a LABEL_DECL. This macro should not
+ be used directly; use label_rtx instead. */
+#define LABEL_DECL_CODE_LABEL(NODE) \
+ (LABEL_DECL_CHECK (NODE)-label_decl.label)
+
/* In a LABEL_DECL, the EH region number for which the label is the
post_landing_pad. */
#define EH_LANDING_PAD_NR(NODE) \
@@ -2951,7 +2956,8 @@ struct GTY(()) tree_field_decl {
(LABEL_DECL_CHECK (NODE)-decl_common.decl_flag_0)
struct GTY(()) tree_label_decl {
- struct tree_decl_with_rtl common;
+ struct tree_decl_common common;
+ rtx label;
int label_decl_uid;
int eh_landing_pad_nr;
};
