Do not allocate vtable slots to consteval functions

[zygoloid]

Also clarify that deleted functions do get assigned vtable slots (even though they don't need them), and consolidate three different places and ways in which we enumerate the same set of rules for which functions get vtable slots and when a return type adjustment is necessary, so that we only need to say "non-consteval" in one place.

In passing, fix missing description of where and how to allocate vtable slots to implicit assignment operators (which might override a virtual assignment operator declared in a base class).

Fixes #82

[zygoloid]

Preview: https://htmlpreview.github.io/?https://github.com/itanium-cxx-abi/cxx-abi/blob/8a9a0923bd86e6c840b09beadbc70480e77482d3/abi.html

[rjmccall]

No further comment after a month; accepting.

[jicama]

I note that clang trunk crashes on this testcase because it still allocates a slot to f().

Avoiding the slot allocation seems like a lot of complication for a feature of minimal utility; what if we allocate a slot normally and just leave it null or pointing to something like __cxa_pure_virtual if we actually write out the vtable?

`
struct A
{
virtual consteval int f() const { return 1; };
};

struct B: A
{
virtual consteval int f() const { return 2; };
virtual void g() { }
};

consteval int f()
{
const A& ar = B();
return ar.f();
}

static_assert (f() == 2);

B b;
`

[rjmccall]

I think it's fine to revisit this decision since it looks like nobody has adopted it until recently. @zygoloid, thoughts?

I personally agree with the committed design here: consteval virtual declarations shouldn't add any runtime overhead beyond the fact that the type is required to become polymorphic. I wouldn't expect this to be difficult for implementations. Most of the complexity of the ABI patch here is actually clarifying existing rules.

[zygoloid]

It seems wasteful to me to allocate a vtable slot that we're never going to use. The other case where we do that (for =delete virtual functions) is a feature that I understand to be explicitly intended to support reserving vtable slots for ABI-compatible extension / retraction of virtual functions, but that's not the case here. The binary size cost of vtables is measurable and important for some applications (particularly the cost of relocations and symbol table entries). Also, the purpose of consteval is to state "code must not be generated for this", and extending that to vtables doesn't seem entirely unreasonable.

That said, I agree with @jicama that this feature is unlikely to ever be widely used and the costs listed above should be considered largely theoretical (and also not fully applicable to this case), so it's probably not worth exchanging a large amount of implementation complexity for it. I went ahead and implemented this in Clang (for both Itanium and, after discussion with some MS folks, the MS ABI), and it was largely straightforward for us, so I suppose the question is just how much implementation complexity we're talking about here. I could imagine this being a bit annoying to support for an implementation whose constant evaluation model works on a lowered representation that operates in terms of vtable accesses; is that the concern?

One other thought: do we consider changing a function from being virtual consteval to being virtual constexpr to be a plausible change, that we would want to permit as an ABI-compatible change if made simultaneously for the declaration and all overriders -- in much the same way that changing from virtual =delete to virtual-but-not-deleted can be done as an ABI-compatible change? If so, I think that would argue strongly in favor of reserving a vtable slot, and adding a __cxa_consteval_virtual entry point for such a slot to optionally invoke. I don't think this seems worth supporting, but perhaps there are use cases for virtual consteval that would motivate this kind of flexibility?

[jicama]

On Sun, Jul 5, 2020 at 3:19 PM Richard Smith ***@***.***> wrote: It seems wasteful to me to allocate a vtable slot that we're never going to use. The other case where we do that (for =delete virtual functions) is a feature that I understand to be explicitly intended to support reserving vtable slots for ABI-compatible extension / retraction of virtual functions, but that's not the case here. The binary size cost of vtables is measurable and important for some applications (particularly the cost of relocations and symbol table entries). Also, the purpose of consteval is to state "code must not be generated for this", and extending that to vtables doesn't seem entirely unreasonable. That said, I agree with @jicama <https://github.com/jicama> that this feature is unlikely to ever be widely used and the costs listed above should be considered largely theoretical (and also not fully applicable to this case), so it's probably not worth exchanging a large amount of implementation complexity for it. I went ahead and implemented this in Clang (for both Itanium and, after discussion with some MS folks, the MS ABI), and it was largely straightforward for us, so I suppose the question is just how much implementation complexity we're talking about here. I could imagine this being a bit annoying to support for an implementation whose constant evaluation model works on a lowered representation that operates in terms of vtable accesses; is that the concern?

Yes, that's how our virtual constexpr implementation works currently; to avoid the vtable slot we'll need to reimplement virtual function lookup in the constexpr evaluator. Which is certainly doable, it just isn't clear to me that it's worthwhile.

One other thought: do we consider changing a function from being virtual consteval to being virtual constexpr to be a plausible change, that we would want to permit as an ABI-compatible change if made simultaneously for the declaration and all overriders -- in much the same way that changing from virtual =delete to virtual-but-not-deleted can be done as an ABI-compatible change? If so, I think that would argue strongly in favor of reserving a vtable slot, and adding a __cxa_consteval_virtual entry point for such a slot to optionally invoke. I don't think this seems worth supporting, but perhaps there are use cases for virtual consteval that would motivate this kind of flexibility?

I'm just not sure what the use cases for virtual consteval are at all. For cases where the objects are created and destroyed as part of constant evaluation, no vtable is needed, so it doesn't matter whether there's a slot, and constexpr would do just as well. Jason

[daveedvdv]

On Jul 5, 2020, at 3:19 PM, Richard Smith ***@***.***> wrote: It seems wasteful to me to allocate a vtable slot that we're never going to use. The other case where we do that (for =delete virtual functions) is a feature that I understand to be explicitly intended to support reserving vtable slots for ABI-compatible extension / retraction of virtual functions, but that's not the case here. The binary size cost of vtables is measurable and important for some applications (particularly the cost of relocations and symbol table entries). Also, the purpose of consteval is to state "code must not be generated for this", and extending that to vtables doesn't seem entirely unreasonable.

Agreed. Especially since it’s not always easy to avoid emitting virtual functions even when they’re not used.

That said, I agree with @jicama <https://github.com/jicama> that this feature is unlikely to ever be widely used and the costs listed above should be considered largely theoretical (and also not fully applicable to this case), so it's probably not worth exchanging a large amount of implementation complexity for it. I went ahead and implemented this in Clang (for both Itanium and, after discussion with some MS folks, the MS ABI), and it was largely straightforward for us, so I suppose the question is just how much implementation complexity we're talking about here.

FWIW, our (EDG’s) constexpr virtual dispatch doesn’t use the data from code-gen (or even a lowered representation) at all. So no significant extra work was needed for consteval. Daveed

[jicama]

OK, sounds like I'm in the minority here, I'll fix my compiler. Thanks.