named address spaces: addr_space_convert never called

[David Brown]

On 09/11/10 18:45, Georg Lay wrote:
> David Brown schrieb:
>> On 08/11/10 16:59, Georg Lay wrote:
>>> Richard Guenther schrieb:
>>>> On Mon, Nov 8, 2010 at 3:39 PM, Georg Lay<avr@gjlay.de>   wrote:
>>>>> Hi, I just started playing around with named address spaces for avr.
>>>>> Besides general space (ram), I introduced a second one, __pgm, which
>>>>> shall address program memory where also constants may live. avr is
>>>>> havard architecture, and both program memory and ram start at
>>>>> address 0.
>>>>>
>>>>>   From this and the internals on TARGET_ADDR_SPACE_CONVERT I understand
>>>>> that pointer casting will not work as expected, because that hook will
>>>>> only get called if the respective address spaces are subsets. However,
>>>>> neither is space-1 a subset of space-0 nor vice versa (or am I midlead
>>>>> by internals?)
>>>>>
>>>>> Is there a way to make it work in the case where the address spaces
>>>>> are disjoint? Started this morning and everything went smooth until I
>>>>> started messing around with pointer casts:
>
> [...]
>
>>>>> So as of internals doc, named address spaces are not intended to
>>>>> implement this kind of memory?
>>>>
>>>> If they are not subsets of each other how'd you convert a pointer
>>>> pointing into one to point into the other address-space?  I think
>>>> the frontend should diagnose this as invalid.
>>>
>>> The front end emits warning. However, explicit casts should yield only
>>> a warning if explicit requested.
>>>
>>> With subsets relation returning true the code is like that (I changed
>>> the test case to add 5, the there is an additional *addphi).
>
> [...]
>
>>> This is fine.
>>>
>>> However, I am still confused:
>>>
>>> "A is a subset of B iff every member of A is alse member of B".
>>>
>>> But in this case, an element of ram is not an element of flash nor is
>>> an element of flash an element of ram. Written down as numbers, these
>>> numers are the same, yes, so that information gets encoded in the
>>> machine mode to know what addresses are legal and what instruction
>>> must be used.
>>>
>>> Memory is not linearized because that would imply to take the decision
>>> at runtime.
>>>
>> Would be be possible to define a third memory space as "global" memory,
>> of which both the ram and the flash are subsets?  It is important to
>> keep ram as the default memory space, but perhaps an artificial global
>> memory space would let you do  conversions like this safely and without
>> warnings.
>>
>> Even better would be if the global memory space could have 24-bit (or
>> 32-bit if necessary) pointers, so that it would actually encompass all
>> memory, with ram pointers at 0x800000 to match the addresses used by the
>> linker.  It would also make it a lot easier to use the full flash space
>> on AVR's with more than 64K flash.
>
> The linker has more information becase it sees relocs/symbols/sections
> whatever. The compiler doesn't have this info when it has to access
> the memory. In the example given above, there are two cases becase the
> application like to destinguith between ram and flash, but I think
> that case ir rare. In most cases the application known how to handle a
> pointer, and users don't want to see LPM/SBRC/LD sequences or even
> more in case of multiple byte access.
>

I agree that users normally know whether data (or a pointer) is in ram 
or flash.  Sometimes, however, it would be nice to be able to use a 
global pointer even if it is less efficient.

But if you can make "const char _pgm msg[]" work in avr-gcc like "const 
char flash msg[]" in IAR's avr compiler, then it will be a huge step 
forward for avr-gcc.  I think the current inconvenience of needing the 
pgm_read_xxx macros is very much an unfortunate necessity of the current 
compiler.



>> May I say I think it's great that you are looking into this?  Program
>> space access on the AVR was the first thing I thought of when I heard of
>> the concept of named address spaces in C.
>
> It's great work that this is part of gcc now! Just remember all the
> hacks to get near/far support into a C16x target.
>
> Besides access and pointer size, maybe even thinks like
>
> int __far __atomic x;
>

"__far" as a memory space makes sense.  It may even make sense to have 
three extra spaces so that (along with __pgm) you can use all the flash 
space in an Mega256, if that is easier or more efficient than using 
24-bit pointers to flash.

I don't think "__atomic" is appropriate for a memory space, however.  I 
don't know if you've read Linus Torvald's rant against "volatile", but 
his point applies here too.  Variables cannot be volatile or atomic in 
themselves - it is /accesses/ that are volatile or atomic (declaring a 
variable to be "volatile" is simply a shorthand for saying that all 
accesses to it should be volatile).

While volatile accesses are fairly cheap, and not /too/ hard to 
understand (though many people misunderstand them), atomic accesses are 
expensive (since they involve disabling interrupts) and hard to 
understand.  Supposing "ax" and "ay" are declared as "int __atomic ax, 
ay".  Should the statement "ax += 2" be atomic?  Should "ax = ay"?  I 
think it is better to stick to the macros in <util/atomic.h> - they are 
clear and explicit.  An alternative would be to implement the gcc atomic 
builtins, since these are also explicit.


> are sensible for basic types if users aren't confused that in x ^= 1
> just the accesses would be atomic, not the operation itself (and don't
> mind the overhead if gcc cannot prove that IRQs are disallowed in ISR,
> as in presence of call, asm, or memory-access).
>
> However, at present avr backend badly needs avr developers. It's not a
> good idea to build such enhancements atop of all the PRs there...
>

I agree with that - prioritising is important.  But it is also good to 
move forward with new features.

Of course, if you are feeling enthusiastic the next step would be an 
__eeprom memory space that worked efficiently on all AVRs...

mvh.,

David