cjdb added a comment.

In D129951#4178154 <https://reviews.llvm.org/D129951#4178154>, @philnik wrote:

> I don't think libc++ can adopt this without having to essentially duplicate 
> our code, since GCC doesn't support `__disable_adl` (and AFAICT there is no 
> coordination between GCC and Clang to add it to both).

I haven't had a lot of time to drive this in Clang, let alone GCC. Even if 
libc++ can't ultimately use it (which would be sad), there are other libraries 
that can. For example, Abseil has a similar attitude towards functions as 
Niebloids, and could wrap it behind a macro.

> Have you tested what impact making the members `static` has? Both clang and 
> GCC already support this as an extension back to C++11: 
> https://godbolt.org/z/drE5v8nYo.

A quick change to the original benchmark <https://godbolt.org/z/13z65EY88> 
shows the AST for `static operator()` being substantially larger than a 
function template with ADL being disabled. I haven't properly benchmarked build 
time impact, but here's a quick one 
<https://gist.github.com/cjdb/6ade504f010dc550890a82f3a5c0ea6a>. The averages 
are below:

**`__disable_adl`**

  real  0.1164
  user  0.0706
  sys   0.0488

**`static operator()`**

  real  0.1272
  user  0.081
  sys   0.0488

It is worth acknowledging that the assembly output is now much closer with 
optimised flags (1.63x larger as opposed to 7.56x larger), but 1.26x larger 
with `-g` (this is down from 1.66x as non-static).

> Maybe it would make more sense to add an attribute `[[clang::cpo]]` instead 
> to tell clang that the class should just be treated as an overload set? Make 
> it requirements that the class is empty, there are no non-static member 
> functions and the class is declared `final` and you should get any benefits 
> without the major drawback of basically no portability. It's of course 
> possible that I'm missing something major, but I think that general way would 
> be a lot more acceptable. Any thoughts?

CPOs and Niebloids are different things (and `__disable_adl` is for Niebloids, 
not CPOs), so any such attribute would need a different name. Having said that, 
a struct that hasn't has no base and is final only slightly improves the AST 
size <https://godbolt.org/z/ncq1qx5Ys> with respect to the improvement by using 
an actual overload set. Finally, there would still be a portability issue 
because even if `[[clang::niebloid]]` works on Clang, there would still need to 
be coordination for it to work on GCC; otherwise GCC w/ libc++ mode would have 
copyable Niebloids; something that the original libc++ design worked hard to 
ensure wasn't possible so that a feature like this could exist.

It is again worth acknowledging that the assembly output in an optimised build 
would have parity, but a build using `-O0 -g` will still be ~1.26x larger.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129951/new/

https://reviews.llvm.org/D129951

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to