ebevhan added inline comments.
================
Comment at: test/SemaCXX/address-space-method-overloading.cpp:28
+ //inas4.bar();
+ noas.bar(); // expected-error{{call to member function 'bar' is ambiguous}}
+}
----------------
rjmccall wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > ebevhan wrote:
> > > > Just as an aside (I think I mentioned this on one of the earlier
> > > > patches as well); treating a non-AS-qualified object as being in a
> > > > 'generic' AS even for normal C++ isn't a great idea IMO. The object
> > > > initialization code should be checking if the ASes of the actual object
> > > > and the desired object type are compatible, and only if so, permit the
> > > > overload.
> > > I think changing this would require changing AS compatibility rules in
> > > general, not just for overloading but for example for conversions too.
> > > This seems like a wider scope change that might affect backwards
> > > compatibility. We might need to start an RFC on this first. John has
> > > suggested adding a target setting for this on the other review. I think
> > > that should work. Another idea could be to add some compiler directives
> > > to specify what generic address space is (if any).
> > >
> > > Using default (no address space) as generic has a lot of advantages since
> > > it doesn't need many parser changes. In OpenCL we weren't lucky that
> > > initial implementation was added that used default for private memory and
> > > therefore when generic was introduced we had to map it to a new lang addr
> > > space value. That required a lot of changes in the parser. But once we
> > > fix those actually, anyone should be able to map generic to anything. I
> > > initially thought it has no other use cases than in OpenCL but now I feel
> > > there might be a value to map default case (no address space) for
> > > something specific and then use generic to specify a placeholder address
> > > space on pointers or references. We could just expose generic address
> > > space generically and also have a mode with no generic address space. The
> > > latter means that conversions between different address spaces is never
> > > valid. Would this make sense?
> > The big problem with the address space support in Clang is that it isn't
> > really very well formalized unless you're on OpenCL. There's no real way
> > for a target to express the relations between its address spaces; most of
> > the relations that exist are hard-coded.
> >
> > The support should probably be generalized for both targets and for
> > LangASes like OpenCL's. Maybe:
> >
> > * the ASes would be defined in a TableGen file which specifies which ASes
> > exist, which ones are compatible/subsets/supersets, etc,
> > * or, just have a target hook that lets a target express that a conversion
> > from AS A to AS B is prohibited/permitted explicitly/permitted implicitly.
> >
> > Just some loose ideas; an RFC for this is possibly the right way forward.
> >
> > The reason I bring all of this up is primarily because in our target, the
> > 'default' AS is disjoint from our other ones, so there's no 'generic' AS to
> > be had. Both implicit and explicit conversion between these ASes is
> > prohibited. Since Clang currently doesn't allow you to express that ASes
> > are truly incompatible, we have a flag that blocks such conversions
> > unconditionally. Ideally it would be target-expressible.
> >
> > -----
> >
> > > I think changing this would require changing AS compatibility rules in
> > > general, not just for overloading but for example for conversions too.
> > > This seems like a wider scope change that might affect backwards
> > > compatibility. We might need to start an RFC on this first. John has
> > > suggested adding a target setting for this on the other review. I think
> > > that should work. Another idea could be to add some compiler directives
> > > to specify what generic address space is (if any).
> >
> > I'm unsure whether any changes to the current semantics really need to be
> > done, at least for the overloading problem.
> >
> > For example, explicit conversion from a non-address space qualified pointer
> > type to an address space qualified pointer type (and vice versa) is
> > permitted, but implicit conversion is an error in both C and C++:
> > https://godbolt.org/z/UhOC0g
> >
> > For an overload candidate to be viable, there has to be an implicit
> > conversion sequence for the implicit object argument to the implicit object
> > parameter type. But no such implicit conversion sequence exists for types
> > in different ASes, even today, or the implicit example in the godbolt would
> > pass. So, an overload candidate with a different AS qualification than the
> > object should not be viable.
> >
> > This is clearly not compatible with OpenCL's notion of the `__generic` AS,
> > but OpenCL already has logic for `__generic` in Qualifiers to handle that
> > case (`isAddressSpaceSupersetOf`). Arguably, that behavior should probably
> > not be hard coded, but that's how it is today.
> >
> > (Also, just because an AS is a superset of another AS does not necessarily
> > mean that the language should permit an implicit conversion between them,
> > but it's a reasonable behavior, I guess.)
> >
> > -----
> >
> > Ultimately, whether or not a conversion of a pointer/reference from address
> > space A to address space B is permitted should both be a function of what
> > the target and the language allows, but that support doesn't exist. I also
> > don't think that exposing a 'generic' AS is the right approach. There are
> > ASes, and some conversions from ASes to other ASes are legal, while others
> > are not. There's also the question of the difference between implicit and
> > explicit conversions; blocking all implicit conversions between ASes would
> > not work for OpenCL, so there must be a way to express that as well.
> >
> > -----
> >
> > These are all just loose thoughts from my head. I'm not terribly familiar
> > with OpenCL except for what I've seen around the Clang code, so there might
> > be details there that I'm not aware of.
> > just have a target hook that lets a target express that a conversion from
> > AS A to AS B is prohibited/permitted explicitly/permitted implicitly.
>
> I think it has to be this, given the possibility of target-specific
> subspacing rules in numbered address spaces. We can't make targets use
> disjoint sets of numbered address spaces.
>
> I agree that, in general, the rule has to be that we check whether the object
> type is convertible to the expected address space of the method; we can't
> assume that there's a generic address space that works for everything.
> That's not even true in OpenCL, since `constant` is not a subspace of the
> generic address space.
>
> > John has suggested adding a target setting for this on the other review.
>
> The suggestion is just that we recognize some way to control what the default
> address space for methods is.
>
> This is difficult with member pointers because a function type can be turned
> into a member pointer type arbitrarily later; that is, we need to be able to
> canonically distinguish a function type that was written without any address
> space qualifier (and hence is `generic` if turned into a member pointer) from
> one that was written with our representationally-default address space
> qualifier (`private`). We can deal with this later, I think, but it might
> require rethinking some things.
> I think it has to be this, given the possibility of target-specific
> subspacing rules in numbered address spaces. We can't make targets use
> disjoint sets of numbered address spaces.
I'm in favor of this option as well.
> This is difficult with member pointers because a function type can be turned
> into a member pointer type arbitrarily later; that is, we need to be able to
> canonically distinguish a function type that was written without any address
> space qualifier (and hence is generic if turned into a member pointer) from
> one that was written with our representationally-default address space
> qualifier (private). We can deal with this later, I think, but it might
> require rethinking some things.
Oh, now I get the problem with those. That is tricky.
So when a function type is used as the type of a pointer-to-member and it is
method-qualified with Default, it should be changed to be method-qualified with
__generic instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57464/new/
https://reviews.llvm.org/D57464
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
