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


> Your suggestion of splitting tiny into 3 sections is a new
> feature that provides additional optimization possibilities.
> It is not a requirement for this old tiny_data feature.

Fair enough.

> Another issue we should be concerned about is compatibility
> with other targets.  There are other targets that have these
> kinds of attributes, the v850 for instance, and it will be
> confusing for users if similar attribute behave differently on
> different targets.

I'll take a look at the v850 target to see what it does.

>> Quite frankly, I see no point in the existing tiny_data
>> attribute.  The compiler doesn't emit 16-bit addressing mode
>> instructions so it's exactly the same as 
>> __attribute__((section(".tiny_data")).
> 
> That isn't what I see.

You're right.  Either I was hallucinating or confusing this
with some other feature. But, I would have sworn it was being
left to the linker to fixup the instructions.  I was wrong, the
compiler is generating 16-bit address instructions directly.

>> I swear I tried it, and it goes into the "tinydata"
>> initialized data section even though it's not an initialized
>> varable.
> 
> This is OK, and correct.  This is what the tiny_data attribute
> is supposed to do.  There is nothing wrong with placing
> uninitialized data in a data section.  It will work.

It will certainly work, but it uses up ROM for unneeded
zero-valued initializer data.  IOW, it will generate a correct
program, but needlessly large one. If I declare a 4KB
uninitialized buffer, I don't want to burn 4KB of ROM space for
initializer data.

> If you didn't want it there, then you shouldn't have added the
> tiny_data attribute.

Point taken.

>> I wasn't aware of any "target platforms" except the generic
>> "elf" one.  That one doesn't correspond to any real hardware,
>> does it? Is the default linker script actually supposed to
>> work for somebody? 
> 
> It works for the simulators.

Ah!  That's the clue I was missing.  I wasn't aware of a
simulator.

>> The other thing I was trying to do (which hasn't been
>> discussed yet) was fix it so that garbage collection works for
>> tiny variables.  The -fdata-sections option (and hence garbage
>> collection at link time) doesn't work for tiny variables in
>> the existing implimentation.
> 
> My concern here is that if we have to add special
> -fdata-sections support to the h8300 backend, then we also
> need to add it to every other target port, in which it seems
> obvious that something in the gcc design is broken, as we
> should just be able to add this code once in the machine
> independent part of the compiler instead of adding it N times,
> once in every target port.

Handling it one place is obviously better, but I didn't think
I'd be allowed to force changes in non-target-specific stuff
just to make the h8300 stuff work the way I proposed.  If the
new feature I'm suggesting is useful to other targets, then
perhaps a general solution should be discussed.

>> I think it a better idea to fix the tiny_data attribute so
>> that it works properly
> 
> It isn't working properly, according to its specification.

You mean it _is_ working properly, I presume, and you're right. :)

> It just isn't working the way you want it to.

Right.

-- 
Grant Edwards
grante@visi.com


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