Am 15.05.26 um 14:17 schrieb Thomas Schwinge:
[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 Thomas,

as far as I can see, there is not a single use of
TARGET_ADDR_SPACE_FOR_ARTIFICIAL_RODATA resp.
targetm.addr_space.for_artificial_rodata in the patch.
There should be a new purpose enum like ARTIFICIAL_RODATA_VTABLE
when vtables are built.  The new enum would be defined in coretypes.h.

The comment in target.def states it is "for optimization purposes only",
which is no more true for vtables (for vtables it's an ABI thing).

It is great so see that finally, named address spaces will be in g++.

Johann

The 'gcc/targhooks.cc:default_addr_space_subset_p' change that appeared
in this v4:

      bool
      default_addr_space_subset_p (addr_space_t subset, addr_space_t superset)
      {
     -  return (subset == superset);
     +  return (subset == 2 && superset == 0) || (subset == superset);
      }

... is entirely unmotivated, as far as I can tell: probably a local hack
for testing, that sneaked into the patch posted?  This change regresses
'gcc.target/i386/addr-space-typeck-1.c' (notice: C test case), which
again PASSes with that change undone.

The new test case 'g++.dg/template/spec-addr-space.C' needs minor
adjusting per changed GCC diagnostics (not related to the new named
addess space support):

     @@ -1,8 +1,9 @@
      // { dg-do compile { target { i?86-*-* x86_64-*-* } } }
template <class T>
     -int f (T __seg_gs *p) { return *p; } // { dg-note "candidate: 'template<class 
T> int f.__seg_gs T\*." }
     +int f (T __seg_gs *p) { return *p; } // { dg-note "candidate 1: 'template<class 
T> int f.__seg_gs T\*." }
                                          // { dg-note "template argument 
deduction/substitution failed:" "" { target *-*-* } .-1 }
      __seg_fs int *a;
      int main() { f(a); } // { dg-error "no matching" }
     -// { dg-note "types .__seg_gs T. and .__seg_fs int. have incompatible 
cv-qualifiers" "" { target *-*-* } .-1 }
     +// { dg-note "there is 1 candidate" "" { target *-*-* } .-1 }
     +// { dg-note "types .__seg_gs T. and .__seg_fs int. have incompatible 
cv-qualifiers" "" { target *-*-* } .-2 }

With that, all new test cases PASS, and no regressions.

If we get a general "go", I'll then offer to fix up a few places for GCC
coding style conformance (so please don't review for that, yet), and I'll
work on test cases some more.  I'll also examine if, since then, any new
code has appeared where 'ADDR_SPACE_CONVERT_EXPR' needs to be handled.

PR69549 "Named Address Spaces does not compile in C++" is going to be
resolved by this patch, so should get referenced in the Git log.

Full-quote of Paul's email from back then:

On 2022-11-10T16:42:22+0100, Paul Iannetta <[email protected]> wrote:
Hi,

It took a bit of time to rework the rough corners.  I tried to be
mirror as much as possible the C front-end, especially when it comes
to implicit conversions, and warnings; and expose the same hooks as
the C front-end does.  The C++ front-end produces as much warnings
that the C front-end, however they are sometimes a bit less
informative (especially so, when passing arguments to functions).

I also address the following points:

   1.  The handling of the register storage class is grouped with the
   other variables with automatic storage.  This incurs a slight
   dis-alignment since you cannot have global register variables do not
   trigger an error, only a warning.

   2. In template unification, I maintain that we don't want any
   changes to address spaces whatsoever during the unification process,
   hence ignoring the strict flag.  Nevertheless, we allow implicit
   conversion, and I have verified that, indeed,


template <class T> void f(T **);
struct A {
    template <class T> operator T*__seg_fs*();
};
int main()
{
    f((void* __seg_fs *)0);           // (1): void*__seg_fs* -> void** should 
be OK
    void (*p)(void * __seg_fs *) = f; // (2): error
}

works as intended. That is, (1) works if we set __seg_fs as a subspace
of the generic address space, and (2) is always an error.

   3. In template unification, when unifying __as1 T = __as2 U we want
   to unify to the __as1 at most, never to __as2 at most, because the
   function requiring __as1 T may want to mix address space from the
   bigger address space, therefore I think that we want to have the
   smaller address-space to be unified into the bigger of the two.

   4. I left untouched same_type_ignoring_top_level_qualifiers_p, even
   though that was very convenient and often lead to better error
   messages since error were caught earlier.

   5. The handling of conversions is done very late in the calling
   chain, because I absolutely want to fold the conversion and force
   the conversion to appear as an ADDR_SPACE_CONV_EXPR after
   gimplification.

   6.  Currently, I do not handle classes. I see what I can do in a
   further revision and maybe add a target hook to hand down to targets
   the choice of the address space of the vtable.

   7.  This can't be added as a test case, but I checked that:

  // In this test case, __seg_gs is a subset of the generic address
  // space.

  int f (int *);
  int main ()
  {
    int __seg_fs *pa;
    int __seg_gs *pb;
    int *pc;
    pa = (__seg_fs int *) pb; return *pa; // warning: cast to ‘__seg_fs’ 
address space pointer from disjoint ‘__seg_gs’ address space pointer
    pa = (int __seg_fs *) pc; return *pa; // warning: cast to ‘__seg_fs’ 
address space pointer from disjoint generic address space pointer
    pa = pb; return *pa; //  error: invalid conversion from ‘__seg_gs int*’ to 
‘__seg_fs int*’
    pc = pb; return *pc; // __seg_gs int * -> int * converted through 
ADDR_SPACE_CONV_EXPR
    pb = pc; return *pb; // error: invalid conversion from ‘int*’ to ‘__seg_gs 
int*’
    pc = pa; return *pb; //  error: invalid conversion from ‘__seg_fs int*’ to 
‘int*’
    return f (pb); // __seg_gs int * -> int * converted through 
ADDR_SPACE_CONV_EXPR
    // return f (pa); // error: invalid conversion from ‘__seg_fs int*’ to 
‘int*’
                      // note:   initializing argument 1 of ‘int f(int*)’
  }

Thanks,
Paul

Rebased on current trunk, bootstrapped and regtested.



Reply via email to