Am 18.06.26 um 23:47 schrieb Jason Merrill:
On 6/18/26 12:45 PM, Georg-Johann Lay wrote:
Am 18.06.26 um 17:02 schrieb Jason Merrill:
On 6/18/26 5:35 AM, Georg-Johann Lay wrote:
Am 15.05.26 um 15:11 schrieb Paul IANNETTA:
On Friday, May 15, 2026 at 09:17:14 PM GMT+9, Thomas Schwinge <[email protected]> wrote:

[Note that emails to <[email protected]> bounce; please use Paul's
other email address: <[email protected]>.]

Hi!

I'd like to resume this patch submission here, which is adding support
for named address spaces to GNU C++, as is implemented for GNU C. As far as I can tell, there wasn't any specific technical reason that this patch
review stalled, back then, in 2022-11? (Jason?)

I've now rebased this onto recent GCC trunk, see attached
'0001-c-parser-Support-for-target-address-spaces-in-C.patch'. There were
just a few merge conflicts that I had to fix up (nothing serious), and
I've bootstrap-tested on x86_64-pc-linux-gnu (only, so far).

Hi Paul,

I have one test case where the generated code for avr is not correct:

int func1 (int x)
{
     static const __flash int arr[] = { 123, 456 };
     return arr[x];
}

The code should read from AS1 but reads from generic space.

Sounds like decay_conversion isn't propagating the address space to the pointer type.

Is AS1 the same as __flash here?

Yes.

$ avr-g++ -S -Os as.cpp -v

Target: avr
Configured with: ../../source/gcc-master/configure --target=avr -- enable-languages=c,c++
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 17.0.0 20260615 (experimental) (GCC)

Apart from that, better dumping would be nice to have:

Take for example this function for which correct code is
generated:

int func2 (const __flash int *p, int x)
{
     return p[x];
}

With -fdump-tree-optimized the respective dump file reads:

;; Function func2 (_Z5func2PU7__flashKii, funcdef_no=1, decl_uid=2281, cgraph_uid=3, symbol_order=2)

int func2 (const <address-space-1> int * p, int x)
{
   unsigned int x.0_1;
   unsigned int _2;
   const <address-space-1> int * _3;
   int _7;

   <bb 2> [local count: 1073741824]:
   x.0_1 = (unsigned int) x_4(D);
   _2 = x.0_1 * 2;
   _3 = p_5(D) + _2;  <-- uses AS1
   _7 = *_3;          <-- uses AS1
   return _7;
}

So the dump could show more of the involved non-generic ASes.

Then I have some trouble with constructors:

struct SS { int val; };

const __flash SS ss[] = { 123, 456 };

SS read_ss (int x)
{
     return ss[x];
}

as.cpp: In function 'SS read_ss(int)':
as.cpp:10:16: error: no matching function for call to 'SS(const __flash SS&)'
    10 |     return ss[x];
       |            ~~~~^
   • there are 3 candidates
     • candidate 1: 'constexpr SS::SS(const SS&)' (near match)
       as.cpp:1:8:
           1 | struct SS
             |        ^~
       • conversion of argument 1 would be ill-formed:
       • error: binding reference of type 'const SS&' to 'const __flash SS' discards qualifiers
         as.cpp:10:16:
            10 |     return ss[x];
               |            ~~~~^
     • candidate 2: 'constexpr SS::SS(SS&&)' (near match)
       as.cpp:1:8:
           1 | struct SS
             |        ^~
       • conversion of argument 1 would be ill-formed:
       • error: cannot bind rvalue reference of type 'SS&&' to lvalue of type 'const __flash SS'
         as.cpp:10:16:
            10 |     return ss[x];
               |            ~~~~^
     • candidate 3: 'constexpr SS::SS()'
       as.cpp:1:8:
           1 | struct SS
             |        ^~
       • candidate expects 0 arguments, 1 provided

Shouldn't there be a default constructor that reads from AS1?
At least when the class is trivially copyable?

We might want to add a hack to overload resolution to allow arbitrary name space conversion for a trivial copy constructor argument, or skip overload resolution entirely for copying a trivial class.

Is __flash not a subset of the generic address space?

The avr back implementation is such that each AS is a subset of
each other AS.  The reason is to help code migration from the
old attribute((progmem)) to ASes.  progmem is in the generic AS of
course, but users may want to call __flash functions etc.

So if they're all subsets of each other, then

const __flash SS s;
const SS *p = &s;

should work?  That would also make the above error go away.

I see that the patch has no tests for conversion to/from generic address space pointers.

The old way of reading from flash is qua inline asm (macros provided bv
libc's avr/pgmspace.h for convenience.  So an old code may read:

const int vals[] = { 1, 2 } __attribute((progmem));

... p = &vals[idx];

int flash_read_int (const int *p /*points to flash*/)
{
    return pgm_read_int (p); // #include <avr/pgmspace.h>
}

Using a plain *p would use the wrong instructions and read from RAM.
Without changing the interface of flash_read_int(), a code using AS is:

int flash_read_int (const int *p /*points to flash*/)
{
    return * (const __flash int*) p;
}

That's why casting between address-spaces is allowed.  Though in your
example, wrong code would be generated (except SS *p uses pgm_read_xxx
for access), and -Waddr-space-convert should complain as the cast
is implicit.

Zo find questionable uses, users can -Waddr-space-convert, so that avr's
TARGET_CONVERT_TO_TYPE diagnoses them.  The idea is that explicit casts
are okay (wanted by the user) while implicit casts should be diagnosed.

Unfortunately, the c++ front doesn't call TARGET_CONVERT_TO_TYPE whereas
the c front does. (TARGET_ADDR_SPACE_CONVERT is for emitting actual RTL
for such conversions, be they explicit or implicit. It runs late after LTO streaming, and therefore it is not well suited for diagnostics).

Johann

If the user wants diagnostics for questionable AS casts, they can
-Waddr-space-convert (which doesn't work properly for C++ since the
C++ front doesn't invoke hooks that the C front is invoking).

 From a pure hardware perspective, flash is not a subset of generic AS:
You need different instructions for accessing flash vs. RAM, which is
why all that AS stuff is needed to begin with: You cannot tell from
a 16-bit address whether it must be loaded per LPM (flash) or LDx (RAM).

And adding a qualified constructor doesn't work either:

struct SS
{
     SS (const __flash SS &ss) : val(ss.val) {}

Can you = default this constructor?  I suspect we will need to add support in defaulted_late_check.

No, doesn't work either:

struct SS
{
     int val;
     SS (int i) : val(i) {}
     constexpr SS (const __flash SS&) noexcept = default;
};

const __flash SS ss[2] = { 123, 456 };

SS read_ss (int x)
{
     return ss[x];
}

as-class.cpp:5:15: warning: explicitly defaulted copy constructor is implicitly deleted because its declared type does not match the type of an implicit copy constructor [-Wdefaulted-function-deleted]

Yes, that's what I was thinking of when I said we'd need to adjust defaulted_late_check.

Jason

Reply via email to