<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols;" dir="ltr">
<p style="margin-top:0;margin-bottom:0">Hi Andrew,</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Thanks for your early feedback. Yes, your concerns are valid and we would have to look into both alternatives in more details, especially in terms of performance measurements with and without OpenCL mode.<br>
</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">We will be working on the prototype now and try to put something upstream ASAP for evaluation. If we then prefer this alternative we can continue improving it and go for the complete solution. If not, we can go ahead
with your solution then. But the main message is that we should implement a solution to speed up parsing/loading of OpenCL builtin function declarations.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">Cheers,</p>
<p style="margin-top:0;margin-bottom:0">Anastasia<br>
</p>
<br>
<div style="color: rgb(0, 0, 0);">
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" color="#000000" face="Calibri, sans-serif"><b>From:</b> Andrew Savonichev <andrew.savonichev@intel.com><br>
<b>Sent:</b> 28 September 2018 15:14<br>
<b>To:</b> Anastasia Stulova<br>
<b>Cc:</b> nd; Sven Van Haastregt; Joey Gouly; clang-dev developer list<br>
<b>Subject:</b> Re: [cfe-dev] [RFC][OpenCL] Implement OpenCL builtin functions in Clang</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi Anastasia,<br>
<br>
Thank you for taking time to review.<br>
<br>
> Potential problems with this approach are:<br>
><br>
> - The maintenance costs as it's harder to modify Clang than just<br>
> change the header in case we want to extend the list of builtins or<br>
> modify them. Meaning that you have to add logic to clang to handle<br>
> what is essentially just a list of declarations. Clang builtins<br>
> weren't really intended to solve this problem.<br>
<br>
It is definitely harder to modify Clang vs a change in a header file,<br>
although it is expected that all difficult code is shared between all<br>
builtins, making it easy to add a new builtin.<br>
<br>
> - It is more risky in terms of bugs because it might not be easy to<br>
> cover all use cases.<br>
<br>
This might be true, considering a custom typecheck code that we have to<br>
write to handle implicit conversions (which comes for free if we use<br>
headers).<br>
<br>
On the other hand, you'll need to write this typecheck code once, and<br>
all other OpenCL builtins can re-use it. Surely some bugs may happen,<br>
but the problem is localized and it is easy to test.<br>
<br>
> - Affects parsing time to check all the extra switch cases.<br>
<br>
Switch is generally faster than any other lookup, so I assume that there<br>
will be no performance implications after adding more switch-cases.<br>
<br>
It also makes sense to put these switch-cases under `if (Lang.OpenCL)',<br>
so it will have a minimal impact on other languages.<br>
<br>
> - We will need to replace the builtins from the header with macros<br>
> aliasing Clang builtins. I find macros really bad in terms of error<br>
> reporting.<br>
<br>
Agree. Any diagnostic related to builtins will be accompanied by "note:<br>
expanded from macro 'convert_char'". This makes error messages a bit<br>
harder to read.<br>
<br>
I wonder if there is a way to have a user-facing alias for a builtin<br>
function without using a preprocessor.<br>
<br>
> But also they [macros] will still need to be parsed.<br>
<br>
Right, but the number of macros should not be high: we need only one<br>
macro to handle all overloadings. I expect ~600 macros for different<br>
`convert_xxx' builtins, ~200 macros for math builtins, ~50 macros for<br>
vload/vstore, etc.<br>
<br>
In essence, there should be ~1000 macro declarations vs existing ~15000<br>
function declarations. This has to be measured of course, but I expect a<br>
significant improvement in parsing in comparison with the existing<br>
(header) design.<br>
<br>
> Also something important to consider is the impact on parsing for<br>
> other languages, i.e. C/C++. I think we should evaluate this<br>
> carefully. Do you have any numbers for this already?<br>
<br>
>From my understanding, there shouldn't be any impact on parsing other<br>
languages: __builtin_opencl_* are LANGBUILTIN, and they should only be<br>
parsed for OpenCL.<br>
<br>
> My team has a slightly different solution to this problem. It is based<br>
> on generation of tries. Something similar to attribute parsing, see<br>
> generated AttrParsedAttrKinds.inc. Unfortunately it's not in a shape<br>
> that we can currently share. But it should not take longer than 2-3<br>
> weeks to upload a prototype if there is interest.<br>
<br>
Having an alternative to consider is always better. This can help us to<br>
get a solution that fits for everyone.<br>
<br>
> To give a rough idea, it uses a TableGen mechanism to describe the<br>
> OpenCL builtin functions (similar to intrinsics in LLVM). During the<br>
> Clang build phase, a trie file is generated. There is a hook in Clang<br>
> that uses the generated trie to lookup the builtin function name<br>
> during call resolution in Sema. After a successful lookup it generates<br>
> the AST declaration of a function with overloads and the rest just<br>
> follows the unmodified Clang flow (i.e. checking overloading,<br>
> mangling, etc). This will cover all builtins, not just conversions and<br>
> maths. It is easy to extend. It doesn't need to change much of Clang<br>
> code, as it allows to follow normal compilation flow.<br>
<br>
So when Sema reaches a call to, say, `acos', and cannot resolve it to<br>
any existing declaration, the hook creates all `acos' function<br>
declarations for all combination of types:<br>
<br>
float acos(float);<br>
float2 acos(float2);<br>
...<br>
// same for other vector types, double and half<br>
<br>
Then Clang's standard machinery chooses which overloading to use and<br>
performs all necessary conversions.<br>
<br>
Do I understand this correctly?<br>
<br>
> Do you think it would make sense to evaluate this solution as well?<br>
<br>
Absolutely. Although, there are several drawbacks/things to consider:<br>
<br>
1) This approach introduces another type of 'builtin function' to<br>
Clang: it is built-in into Clang, but it looks and behaves exactly<br>
as a user-defined function (except that it doesn't have a<br>
SourceLocation).<br>
<br>
This is something new for Clang (unless I miss something), so we<br>
probably need more people from Clang community to review this.<br>
<br>
2) If OpenCL builtin looks and behaves as a user-defined function, it<br>
will always be mangled the same way as it is mangled now (with<br>
headers).<br>
<br>
We cannot emit LLVM intrinsics instead of mangled functions, and,<br>
as I mentioned, LLVM intrinsics for builtins can make SPIR-V<br>
translation easier. Again, see [1].<br>
<br>
3) If my assumption is correct, you'll be generating dozens of<br>
function declarations (overloadings for all types) for every<br>
builtin that is used in a program. This might not be optimal in<br>
terms of performance, but it'll probably be good enough for general<br>
use-case.<br>
<br>
Anyway, this approach looks pretty interesting.<br>
<br>
<br>
[1]: <a href="https://lists.llvm.org/pipermail/llvm-dev/2018-September/125977.html">
https://lists.llvm.org/pipermail/llvm-dev/2018-September/125977.html</a><br>
<br>
-- <br>
Andrew<br>
<br>
--------------------------------------------------------------------<br>
Joint Stock Company Intel A/O<br>
Registered legal address: Krylatsky Hills Business Park,<br>
17 Krylatskaya Str., Bldg 4, Moscow 121614,<br>
Russian Federation<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<br>
<br>
</div>
</span></font></div>
</div>
</div>
</body>
</html>