This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: Move FIELD_DECL into it's own structure


Daniel Berlin wrote:
This moves FIELD_DECL into it's own structure, as well as unsharing
previously unshared fields shared between FIELD_DECL and other *_DECL's.

FIELD_DECL's now have their own struct, struct tree_field_decl, derived
from struct tree_decl_common.

tree_decl_common is now the common base for _DECL nodes, and will
decrease in size as more things get split out and unshared (IE i don't
expect it to stay the size listed below forever, as some things don't
need even those fields).  This means that if we decide one of the DECL's
(CONST_DECL maybe) can't have attributes, we'll make a substructure
shared by everything but CONST_DECL that contains the attributes tree).

I gave up trying to split out DECL_CONTEXT for the moment, the C++ FE
and Java FE use it all over the place on FIELD_DECL's, and i stopped
after changing about 20 users :)

All things not in common with FIELD_DECL and all other DECL's was moved
to tree_decl_extra. This is just a temporary storage place as
structures get moved around and split.



I've moved the FIELD_DECL macros to right before the structure, in line with the placement of the rest of the file macros.

I like the idea of this patch, but I'm concerned about a few issues.


One is that if we go a little way down this path, but not all the way, we could end up truly tangled. I don't know exactly what to do about that, except ask you to promise to keep going for a while. In other words, I'm a little nervous about this being a weekend project that then gets dropped after a weekend or two with no clear plan. In fact, it would be nice if there were a Wiki page with objectives and methodology explaining what you're trying to do so that other people could help. I'd include things like how you plan to emulate inheritance in C, and how the macros and such will have to work for that. In the end, this is documentation that we'll want anyhow for future development.

I'm not trying to throw up roadblocks, nor asking you to write a book. But, this is a major change to the way GCC's IR works, and nobody is happy with the current lack of design and documentation. I think you need to lay the appropriate foundation, and get buy-in, before we start hacking.

All things not in common with FIELD_DECL and all other DECL's was moved
to tree_decl_extra. This is just a temporary storage place as
structures get moved around and split.

In the way of temporary changes, this will likely live forever. And, as you say, you want to split things out of CONST_DECL, etc., later, leading to additional shared structure types. So, tree_decl_extra isn't a good name. For now, tree_non_field_decl_common is probably the best we can do.


A tree_class_except macro was added to be able to check that something
is any member of a tree class *except* some member.  This is used to
make sure nothing is still trying to access the moved fields in a
FIELD_DECL.

I'm not sure this is a very good idea. You've made the logic rather more complex by adding negation. And, a front-end that inherits from FIELD_DECL would now not work. I'd rather we stay with positive-only checks.


It doesn't make sense to have DECL_FIELD_CONTEXT. All DECLs have a scope and always will. DECL_CONTEXT should work on all DECLs.

And, even if you were going to invent DECL_FIELD_CONTEXT this:

+  if (TREE_CODE (olddecl) != FIELD_DECL)
+    {
+      unsigned olddecl_uid = DECL_UID (olddecl);
+      tree olddecl_context = DECL_CONTEXT (olddecl);
+
+      memcpy ((char *) olddecl + sizeof (struct tree_common),
+	      (char *) newdecl + sizeof (struct tree_common),
+	      sizeof (struct tree_decl_common) - sizeof (struct tree_common));
+      memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
+	      (char *) newdecl + sizeof (struct tree_decl_common),
+	      sizeof (struct tree_decl_extra) - sizeof (struct tree_decl_common));
+      DECL_UID (olddecl) = olddecl_uid;
+      DECL_CONTEXT (olddecl) = olddecl_context;
+    }
+  else
+    {
+      unsigned olddecl_uid = DECL_UID (olddecl);
+      tree olddecl_context = DECL_FIELD_CONTEXT (olddecl);
+
+      memcpy ((char *) olddecl + sizeof (struct tree_common),
+	      (char *) newdecl + sizeof (struct tree_common),
+	      sizeof (struct tree_decl_common) - sizeof (struct tree_common));
+      memcpy ((char *) olddecl + sizeof (struct tree_decl_common),
+	      (char *) newdecl + sizeof (struct tree_decl_common),
+	      sizeof (struct tree_field_decl) - sizeof (struct tree_decl_common));
+      DECL_UID (olddecl) = olddecl_uid;
+      DECL_FIELD_CONTEXT (olddecl) = olddecl_context;

would be mostly unnecessary duplication.

This:

+ gcc_assert ((TREE_CODE (decl) == FIELD_DECL) || !DECL_ASSEMBLER_NAME_SET_P (decl));

seems ugly. We're going to be forever updating such conditions as additional things (CONST_DECL?) lose their DECL_ASSEMBLER_NAME slot. Maybe it would be better to have DECL_ASSEMBLER_NAME_SET_P return false for thse cases. Or to introduce CAN_HAVE_DECL_ASSEMBLER_NAME_P?

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]