-
Notifications
You must be signed in to change notification settings - Fork 0
Unique stable name reimpl #1
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
8c2e149
e6b6534
f3dffdd
5a2780b
8f637d8
355ecc4
acd6f9d
ba8491a
7358177
2975b68
17339c1
0affc56
7bc6b92
c55e53e
e74bc41
bcd0493
5c5f66e
7e7552d
55bb855
4ad8f83
43e51b7
ef6ba73
3bb78fe
f934e00
099df82
1df6b08
0fbaef5
4718a96
48e3a92
c9295c2
0779772
89c7b70
8d8db2a
0ff40f9
4c53337
3e06737
81b49f8
9993946
6c01690
e2ae208
be023e8
9ebbc6c
9b0b1be
71c0cc7
a13ec99
c73731b
6840213
a8867d4
375e87d
764a4b0
0ec2d07
8ddd3a9
821dbb6
c636401
52862e8
874d44f
68ec935
e5ac8fc
96bb440
a08a5ac
195218a
bdffe7e
b2584b2
53a414b
41ffe76
4fa5e27
5b0018e
ad7eadb
bf909b0
10643c8
07c5fbd
8e56614
5bc1061
3e19ec4
e2edc31
5003b2b
dbada48
8fd77bd
4be1a00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1766,7 +1766,7 @@ correctly in any circumstances. It can be used if: | |
| metaprogramming algorithms to be able to specify/detect types generically. | ||
|
|
||
| - the generated kernel binary does not contain indirect calls because they | ||
| are eliminated using compiler optimizations e.g. devirtualization. | ||
| are eliminated using compiler optimizations e.g. devirtualization. | ||
|
|
||
| - the selected target supports the function pointer like functionality e.g. | ||
| most CPU targets. | ||
|
|
@@ -2311,6 +2311,30 @@ argument. | |
| int *pb =__builtin_preserve_access_index(&v->c[3].b); | ||
| __builtin_preserve_access_index(v->j); | ||
|
|
||
| ``__builtin_unique_stable_name`` | ||
| ------------------------ | ||
|
|
||
| ``__builtin_unique_stable_name()`` is a builtin that takes a type or expression and | ||
erichkeane marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| produces a string literal containing a unique name for the type (or type of the | ||
| expression) that is stable across split compilations. | ||
|
|
||
| In cases where the split compilation needs to share a unique token for a type | ||
| across the boundary (such as in an offloading situation), this name can be used | ||
| for lookup purposes. | ||
|
|
||
| This builtin is superior to RTTI for this purpose for two reasons. First, this | ||
| value is computed entirely at compile time, so it can be used in constant | ||
| expressions. Second, this value encodes lambda functions based on line-number | ||
| rather than the order in which it appears in a function. This is valuable | ||
| because it is stable in cases where an unrelated lambda is introduced | ||
| conditionally in the same function. | ||
|
|
||
| The current implementation of this builtin uses a slightly modified Itanium | ||
| Mangler to produce the unique name. The lambda ordinal is replaced with one or | ||
| more line/column pairs in the format ``LINE->COL``, separated with a ``~`` | ||
|
||
| character. Typically, only one pair will be included, however in the case of | ||
| macro expansions the entire macro expansion stack is expressed. | ||
|
|
||
| Multiprecision Arithmetic Builtins | ||
| ---------------------------------- | ||
|
|
||
|
|
@@ -2505,7 +2529,7 @@ Guaranteed inlined copy | |
| ``__builtin_memcpy_inline`` has been designed as a building block for efficient | ||
| ``memcpy`` implementations. It is identical to ``__builtin_memcpy`` but also | ||
| guarantees not to call any external functions. See LLVM IR `llvm.memcpy.inline | ||
| <https://llvm.org/docs/LangRef.html#llvm-memcpy-inline-intrinsic>`_ intrinsic | ||
| <https://llvm.org/docs/LangRef.html#llvm-memcpy-inline-intrinsic>`_ intrinsic | ||
| for more information. | ||
|
|
||
| This is useful to implement a custom version of ``memcpy``, implement a | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15154,6 +15154,7 @@ static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) { | |
| case Expr::CoawaitExprClass: | ||
| case Expr::DependentCoawaitExprClass: | ||
| case Expr::CoyieldExprClass: | ||
| case Expr::UniqueStableNameExprClass: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is correct, but we should have some
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that static assert actually wouldn't work for 2 reasons: 1- its a std::string, which isn't constexpr yet :) But more importantly, #2, the value of __builtin_unique_stable_name isn't calculable until we are 'after' constexpr evaluation. Basically, we have to see every template instantiation (and thus the entire TU evaluated) before we can actually 'know' the name. Otherwise lambda numbers are going to be 'off'. The previous version didn't need the knowledge of the entire TU (that is, of every lambda required to name a kernel) to calculate it, so it worked as a constant expression. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://en.cppreference.com/w/cpp/string/basic_string/basic_string sure make it seem like it is constexpr since C++20 (see signature llvm#5) However, the point about having to do this after constant evaluation time is interesting. I'll have to think on that a bit, but IIRC, there were some comments in the review that made me think constexpr was expected to work (so we may need a pass to make sure those comments get corrected).
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oooh, neat! Didn't realize they'd gotten to string for 20, much industrious LEWG/LWG! Not sure what comments you mean, but we'll have to discuss it at that point I think. Thinking further, I hope the lack of constexpr doesn't break some of our use cases.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @romanovvlad : Can you comment on this? Do we need constexpr evaluation for this builtin? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @erichkeane There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I also can't think of a point where at DPC++ level
I'm not exactly sure here - I assume that it would be the same string literal as it is now, which will be located in a global variable and references through some load/stores/geps/addrspacecasts.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constexprness means you couldn't use the builtin in a constexpr function, and you couldn't use the result of it as a template. We were just making sure that is OK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g., the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @schittir -- the |
||
| return ICEDiag(IK_NotICE, E->getBeginLoc()); | ||
|
|
||
| case Expr::InitListExprClass: { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.