Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Add cacheline size macros sufficient to support C++17 std::hardware_{con,de}structive_interference_size #60174

Closed
pkasting opened this issue Jan 20, 2023 · 4 comments · Fixed by #89446
Labels
c++17 clang Clang issues not falling into any other category

Comments

@pkasting
Copy link
Member

libc++ implements std::hardware_{con,de}structive_interference_size on gcc only due to clang lacking sufficient macro support. This bug requests that clang add sufficient support to implement this C++17ism in libc++.

@EugeneZelenko EugeneZelenko added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. c++17 and removed new issue labels Jan 20, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 20, 2023

@llvm/issue-subscribers-c-17

@philnik777 philnik777 added clang Clang issues not falling into any other category and removed libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Jan 21, 2023
@ldionne
Copy link
Member

ldionne commented Mar 1, 2024

Note that #83603 implements the library feature even on Clang, by hardcoding values per platform.

@h-vetinari
Copy link
Contributor

Even if libcxx moves forward independently, copying an outline how to proceed for clang from the discourse thread on this:

@jyknight: While I still think this feature is not particularly useful, it is in the standard, and isn’t going to go away. So, I agree it would be appropriate to implement it – similarly to how GCC has implemented it.

This means:

  • The compiler should define __GCC_DESTRUCTIVE_SIZE and __GCC_CONTRUCTIVE_SIZE values. These are explicitly non-stable, and should vary based on CPU tuning options passed to the compiler.

    When using a generic CPU, we probably want to set destructive to the maximum L1-cache-size of any “reasonably-popular” model in the family, and constructive to the minimum. If a particular CPU model tuning is set, we set both values to the L1-cache-size for that particular CPU model.

    The values we choose in Clang do not need to match to any other compiler’s implementation, nor any previous/future version of Clang. We may choose whichever values we believe will result in the best performance for users, and adjust as seems appropriate at any point in the future.

  • There should be an on-by-default warning when using these values in a header or from within an exported interface of a module.

    That is, code like struct alignas(std::hardware_destructive_interference_size) X{} in a header should emit an on-by-default warning. GCC calls this diagnostic -Winterference-size. The purpose is to notify any potential user that it is (usually) inappropriate to use the values in way that is expected to be stable across multiple TUs – since it will vary depending on both compiler version and CPU tuning. This reduces the risk of implementing the feature substantially.

  • We should allow the user to override the default values via explicit command-line option. GCC permits the user to specify -param=destructive-interference-size=NNN and -param=constructive-interference-size=NNN. I don’t know whether we should implement that same command-line interface, or if we should simply allow users to e.g. use -D__GCC_DESTRUCTIVE_SIZE=64 to override the default values, if they wish.

AaronBallman added a commit to AaronBallman/llvm-project that referenced this issue Apr 19, 2024
These macros are used by STL implementations to support implementation
of std::hardware_destructive_interference_size and
std::hardware_constructive_interference_size

Fixes llvm#60174
AaronBallman added a commit that referenced this issue Apr 26, 2024
These macros are used by STL implementations to support implementation
of std::hardware_destructive_interference_size and
std::hardware_constructive_interference_size

Fixes #60174

---------

Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
@pinskia
Copy link

pinskia commented Sep 10, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++17 clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants