Re: [PATCH v3] c++: parser - Support for target address spaces in C++

[Paul Iannetta <piannetta@kalrayinc.com>]

On Thu, Nov 03, 2022 at 02:38:39PM +0100, Georg-Johann Lay wrote:
> > [PATCH v3] c++: parser - Support for target address spaces in C++
> 
> First of all, it is great news that GCC is going to implement named address
> spaces for C++.
> 
> I have some questions:
> 
> 1. How is name-mangling going to work?
> ======================================
> 
> Clang supports address spaces in C++, and for address-space 1 it does
> generate code like the following:
> 
> #define __flash __attribute__((__address_space__(1)))
> 
> char get_p (const __flash char *p)
> {
>     return *p;
> }
> 
> 
> _Z5get_pPU3AS1Kc:
>    ...
> 
> I.e. address-space 1 is mangled as "AS1".
> 
> (Notice that Clang's attribute actually works like a qualifier here, one
> could not get this to work with GCC attributes.)
> 

Currently, the __address_space__ attribute does not exist in GCC, and
the definition of address-spaces as well as how they are laid-out can
only modified in the back-end.

I agree that this is a convenient attribute to define address-spaces
on the fly, but this lacks the flexibility of specifying which
address-spaces are subsets of others and how they should be promoted.

The mangling will be done with the extended qualifiers extension, for
example, your example would be mangled into "_Z5get_pPU7__flashKc".

> 
> 2. Will it work with compound literals?
> =======================================
> 
> Currently, the following C code works for target avr:
> 
> const __flash char *pHallo = (const __flash char[]) { "Hallo" };
> 
> This is a pointer in RAM (AS0) that holds the address of a string in flash
> (AS1) and is initialized with that address. Unfortunately, this does not
> work locally:
> 
> const __flash char* get_hallo (void)
> {
>     [static] const __flash char *p2 = (const __flash char[]) { "Hallo2" };
>     return p2;
> }
> 
> foo.c: In function 'get_hallo':
> foo.c: error: compound literal qualified by address-space qualifier
> 
> Is there any way to make this work now? Would be great!
> 

Currently, I implement the same restrictions as the C front-end, but I
think that this restriction could be lifted.

> 
> 3. Will TARGET_ADDR_SPACE_DIAGNOSE_USAGE still work?
> ====================================================
> 
> Currently there is target hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE.
> I did not see it in your patches, so maybe I just missed it? See
> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gccint/Named-Address-Spaces.html#index-TARGET_005fADDR_005fSPACE_005fDIAGNOSE_005fUSAGE
> 

That was a point I overlooked in my previous patch.  This will be in
my new revision where I also add implicit conversion between
address spaces and also expose TARGET_ADDR_SPACE_CONVERT.

> 
> 4. Will it be possible to put C++ virtual tables in ASs, and how?
> =================================================================
> 

Currently, I do not allow the declaration of instances of classes in
an address space, mainly to not have to cope with the handling of the
this pointer.  That is,

  __flash Myclass *t;

does not work.  Nevertheless, I admit that this is would be nice to
have.

> One big complaint about avr-g++ is that there is no way to put vtables in
> flash (address-space 1) and to access them accordingly.  How can this be
> achieved with C++ address spaces?

Do you want only the vtables in the flash address space or do you want
to be able to have the whole class content.

1. If you only want the vtables, I think that a target hook called
at vtable creation would do the trick.

2. If you want to be able to have pointer to classes in __flash, I
will need to further the support I have currently implemented to
support the pointer this qualified with an address space.
Retrospectively, I think this have to be implemented.

Paul

> 
> Background: The AVR architecture has non-linear address space, and you
> cannot tell from the numeric value of an address whether it's in RAM or
> flash. You will have to use different instructions depending on the
> location.
> 
> This means that .rodata must be located in RAM, because otherwise one would
> not know whether const char* pointed to RAM or flash, but to de-reference
> you's need different instructions.
> 
> One way out is named address spaces, so we could finally fix
> 
> https://gcc.gnu.org/PR43745
> 
> 
> Regards,
> 
> Johann
>