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-H8300] Handling tiny_data attribute


On Mon, 2004-05-03 at 07:56, Grant Edwards wrote:
> Somebody will have to send me a copy of the patch.  The mailing
> list web archive doesn't seem to have a way to retrieve
> unmolested messages.

If patches are put in mime attachments, then they should be unmolested. 
patches added as text get fixed to be valid html.  The conversion isn't
complicated.  Unsafe characters get converted to hex and preceded by an
=.  So =20 is a space.  Etc.  "man ascii" will give you a hex table on
most any unix system.

> OK. I can fix that.  I a little confused, though. None of the
> other functions in my gcc sources have ISO prototypes.

You must be looking at older gcc sources.  Current sources are written
in ISO C, and use of prototypes is required.  We made this change over
the last year, so 3.3 is old-style, but 3.4 is new-style.

> >Changing .tiny sections support won't work without appropriate
> >binutils support.
> I'm using it without any binutils changes and it works just fine.

Your patch creates new sections that did not exist before.  If these
sections are not in the linker script (and they are not in the standard
linker scripts in binutils), then the linker will use default rules to
place them.  Default rules use the section flags, so tinybss gets put
next to bss, and tinyrodata gets put next to rodata.  This means neither
tinybss nor tinyrodata is placed in the tiny data segment, and hence
code that requires this will fail.  You might not notice a problem if
your program is small, but I would expect this to fail for a large
enough program.  I haven't done a toolchain build to verify this, but I
don't see how this can possibly work.

We should consider whether the pain of asking everyone to fix their
linker scripts is worthwhile.  Most people aren't going to know that
they need to fix their linker scripts to continue using the tiny_data
attribute.  Their programs will fail in strange ways, they will file bug
reports to us, and then we will have to explain.  I would prefer a
solution that doesn't require us to answer bug reports about this.

It might be simpler to fix gcc so that these errors don't happen.  I
suggested a solution before, which is to force the section flags to be
what we want, "aw".  Another alternative is to reject variables that
require wrong section flags, e.g. if the tiny_data attribute is applied
to a static const, then give an error instead of creating a section with
the wrong section flags.  Or give a warning and ignore the const
attribute.  Or maybe create a new tiny_rodata attribute, which maps to
the new section.  That way only people using the new attribute need to
worry about fixing their linker scripts.

It might be useful to look at other ports and see if any others have
already run into this problem, and look at how they solved it.

I tried compiling some simple examples.  I don't see the need for the
tinybss section.  An uninitialized variables seems to be handled
correctly.  A static const variable does cause a problem though, as the
section flags get set to "a", which conflict with the expected "aw".

> IMO, you're better off getting an error if you use the default
> linker script than you are getting an output file with all your
> sections in the wrong places.

You won't get an error.  The linker will automatically place unknown
sections based on the section flags, and for tinybss and tinyrodata it
will put them in the wrong place.

> BTW, what target platform is the default linker script for?

Every target platform has its own default linker script.

> >It might be easier to provide default section flags for the
> >tiny section rather than letting gcc compute them.
> Sorry, I don't know what that means.

Gcc is automatically computing section flags (the "aw" or "a") based on
the data that is put into the section.  This happens in
default_section_type_flags, which is called from the
TARGET_SECTION_TYPE_FLAGS hook.  So a possibly solution is to define
this hook for the h8300 port, and force the tiny section to have "aw"
section flags.

> The whole point of the exercise was to create three different
> sections, so I don't really understand what you're suggesting.
> Will "default section flags" provide a way for "tiny" variables
> to be placed into three separate sections?

No.  It avoids the need to create 3 separate sections.  3 separate
sections is bad because it will break the compiler for any user who
doesn't modify their linker script.  It is generally not a good idea to
change an existing feature in this way.  If you really need 3 sections,
then it is probably better to create a new feature than to change an old
one.  You could maybe create two new attributes that do what you want
instead of changing the meaning of the existing attribute.  It isn't
clear to me why you need 3 different sections though.  The only reason I
was given was to avoid section flag conflicts, and there are easier ways
to solve that problem.

Your patch breaks backwards ABI compatibility, which is usually a bad
thing.  If you can get major h8300 users to agree that this is
necessary, then you can go forward.  However, since I am not a h8300
maintainer, and I don't know who the major users are, I am not
comfortable approving changes that break the ABI.

Another issue, the tiny_data attribute is documented in the gcc manual. 
If we are changing the behaviour of it, then we also need to change the
documentation.  Search in gcc/doc/extend.texi for tiny_data.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


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