<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
Hi,
<div class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 29 Jan 2020, at 15:42, Timothy Keith <<a href="mailto:tkeith@nvidia.com" class="">tkeith@nvidia.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Thanks for the reply.<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Just picking out a few points:<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Module in the context of F18 means parser, semantics, evaluate etc.<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Those are abstract things. What are the concrete things you want to capitalize? Namespaces? Directory names?</div>
</div>
</div>
</blockquote>
<div>If we were to match LLVM and Clang, the directory names would be capitalised and the namespaces would remain lower case.</div>
<div><span style="font-family: Calibri, sans-serif; font-size: 11pt;" class=""> </span></div>
<blockquote type="cite" class="">
<div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> But what we meant here was to make sure that std::string is not being used as a parameter to functions etc, these should be using StringRef (or possibly Twine where appropriate).<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
What's the benefit of using StringRef over std::string for parameters?</div>
</div>
</blockquote>
If the function being passed the string doesn’t need ownership of it, you avoid an unnecessary copy. It also allows passing of a string literal or C-style character array without requiring an allocation and a copy of the array.</div>
<div>Twine is designed to take arguments that are usually concatenations of strings, and it makes that concatenation very efficient.<br class="">
<blockquote type="cite" class="">
<div class="">
<div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Regarding "always use early exit":<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> The LLVM Coding Standards have a strong preference for this, with justification given here:<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
><span class="Apple-converted-space"> </span><a href="https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return" style="color: blue; text-decoration: underline;" class="">https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return</a><o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
That's a pretty thin justification. It gives an example where else should not be used and generalizes it to all cases. When you encounter exceptional conditions early exit it certainly appropriate. But in non-exceptional cases it does not always correctly express
intent.<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Here's an example from resolve-names.cpp:<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> if (type.IsAssumedType()) {<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeTypeStarType();<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> } else if (type.IsUnlimitedPolymorphic()) {<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeClassStarType();<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> } else {<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeDerivedType(...);<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> }<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
You could write this as:<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> if (type.IsAssumedType()) {<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeTypeStarType();<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> }<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> if (type.IsUnlimitedPolymorphic()) {<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeClassStarType();<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> }<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-family: "Courier New";" class=""> return currScope().MakeDerivedType(...);<o:p class=""></o:p></span></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
That makes it look like MakeDerivedType is the "normal" case and the other two are exceptions.<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
But that's wrong -- they are all normal and it's just a return based on a three-way condition.<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Tim<o:p class=""></o:p></div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(181, 196, 223); padding: 3pt 0in 0in;" class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<b class=""><span style="font-size: 12pt;" class="">From:<span class="Apple-converted-space"> </span></span></b><span style="font-size: 12pt;" class="">David Truby <<a href="mailto:David.Truby@arm.com" style="color: blue; text-decoration: underline;" class="">David.Truby@arm.com</a>><br class="">
<b class="">Date:<span class="Apple-converted-space"> </span></b>Wednesday, January 29, 2020 at 6:51 AM<br class="">
<b class="">To:<span class="Apple-converted-space"> </span></b>Timothy Keith <<a href="mailto:tkeith@nvidia.com" style="color: blue; text-decoration: underline;" class="">tkeith@nvidia.com</a>><br class="">
<b class="">Cc:<span class="Apple-converted-space"> </span></b>Richard Barton <<a href="mailto:Richard.Barton@arm.com" style="color: blue; text-decoration: underline;" class="">Richard.Barton@arm.com</a>>, "<a href="mailto:flang-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class="">flang-dev@lists.llvm.org</a>"
<<a href="mailto:flang-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class="">flang-dev@lists.llvm.org</a>><br class="">
<b class="">Subject:<span class="Apple-converted-space"> </span></b>Re: [flang-dev] (Strawman) Costed plan for LLVM-ification of F18<o:p class=""></o:p></span></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
</div>
<table class="MsoNormalTable" border="1" cellpadding="0" style="background-color: rgb(255, 235, 156); background-position: initial initial; background-repeat: initial initial;">
<tbody class="">
<tr class="">
<td style="padding: 0.75pt;" class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<b class=""><span style="font-size: 7.5pt; font-family: Verdana, sans-serif;" class="">External email: Use caution opening links or attachments</span></b><span style="font-size: 7.5pt; font-family: Verdana, sans-serif;" class=""></span><o:p class=""></o:p></div>
</td>
</tr>
</tbody>
</table>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Hi Tim,<span class="Apple-converted-space"> </span><o:p class=""></o:p></div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
I had some input into putting this list together so I’ve added some clarifications to some things inline<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Thanks<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
David Truby<o:p class=""></o:p></div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
On 29 Jan 2020, at 14:12, Timothy Keith via flang-dev <<a href="mailto:flang-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class="">flang-dev@lists.llvm.org</a>> wrote:<o:p class=""></o:p></div>
</div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""> </o:p></div>
<div class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
My comments and questions are below in line.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Tim<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Hi list<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> As discussed on previous threads, we need to propose a plan for making F18 more LLVM-like in style and use of LLVM APIs and facilities. David is working on getting the permissions to put this into a github project in LLVM so we can collaborate . In order
to make progress ahead of that happening, I'd like to have a go on email.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> For each area I would like to capture a list of work items we'll commit to doing before merge to monorepo. I want us to be happy we can achieve these in a short time period so we can propose a new merging date. I'd also like to create a list of items we'll
commit to fixing after merging to the monorepo and a timeline for getting that done.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Note that I'll only cover API and coding style work items in this thread. The other initiatives we can discuss in other threads. I'll just say that we'll propose cmake changes and porting test suite to lit in the pre-merge stuff and setting up build bots
in the post-merge stuff.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> When he has access, David can create two projects one for pre-merge and one for post-merge and we'll put everything in there.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Please consider the below a strawman. All dates can change, all list items can change position, new ones can be added, items can be deleted, etc.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> What do people think about these lists? Do we think those two dates are do-able?<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Ta<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Rich<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Proposal:<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> We will make the following style changes before merging to the monorepo<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> F18 changes to make it more LLVM-like in code style<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 1. Rationalise headers to put public headers in /include and not /lib<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 2. Unify F18's clang-format file to match LLVMs<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
We should discuss what should be in .clang-format with an eye to make it more like LLVM's.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 3. Rename all .cc files to .cpp<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 4. Switch module names to starting with capital letters<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
What does "module" mean in this context?<o:p class=""></o:p></div>
</div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Module in the context of F18 means parser, semantics, evaluate etc.<br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Increase use of LLVM APIs and utilities in F18<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> a. Switch F18 custom File handling to LLVM's File handling (helps with non-POSIX platform support)<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> b. Change uses of <iostream> to LLVM's streams<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
F18 doesn't use <iostream>.<o:p class=""></o:p></div>
</div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
I think we are (perhaps ambiguously) using <iostream> here to refer to the entire standard stream IO library. E.g. I do see a lot of uses of <ostream> in F18. LLVM has its own replacement for these for various reasons and highly discourages the use of the standard
ones.<br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> c. Migrate use of std::list to a suitable alternative in LLVM's API<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
I would say: replace std::list with a better data structure where appropriate.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> d. Use llvm_unreachable with an error message for unreachable cases<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
What does llvm_unreachable do for us?<o:p class=""></o:p></div>
</div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
llvm_unreachable effectively behaves like an abort in debug builds, however in release builds it adds optimiser hints abstracted across the various supported compilers that the code in question isn’t reachable (so no need to generate a branch there or whatever).<br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> e. Use llvm::Error instead of error codes if and when error codes are used.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
I don't know of any use of error codes.<o:p class=""></o:p></div>
</div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
What error handling facility does F18 currently use if error codes and exceptions aren’t used? I am familiar with a few places that optionals are returned, we could consider looking at changing these to using llvm::Expected to provide more context about what
has gone wrong.<br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> We would like to aim for a merge date of Monday 24th Feb to merge to the monorepo with all of the above changes committed to F18 master.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> We then propose to make the following changes after merging to the monorepo.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> F18 changes to make it more LLVM-like in code style<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> We will commit to a perform a one-off exercise where we automatically audit the code to find these instances and bring them in line.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 1. Eliminate braces from all single-line if statements<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
This seems like a really bad idea and would like to see a reason for it.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 2. Eliminate all uses of else-after-return<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Doing this blindly is also a bad idea.<o:p class=""></o:p></div>
</div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
The LLVM Coding Standards have a strong preference for this, with justification given here:<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<a href="https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return" style="color: blue; text-decoration: underline;" class="">https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return</a><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> 3. Add doxygen infrastructure so we can generate docs<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Increase use of LLVM APIs and utilities in F18<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> a. std::string → StringRef where appropriate<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
These aren't similar data structures. std::string owns the string data and StringRef does not. std::string_view is now a standard class for the latter case.<o:p class=""></o:p></div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
LLVM still currently prefers the use of StringRef over std::string_view. I am not sure if there are plans to change this when LLVM switches the majority of its code to C++17. But what we meant here was to make sure that std::string is not being used as a parameter
to functions etc, these should be using StringRef (or possibly Twine where appropriate).<o:p class=""></o:p></div>
</div>
<div class="">
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> b. std::vector → llvm::SmallVector where appropriate<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> c. std::set → llvm::SmallSet/llvm::StringSet/llvm::DenseSet where appropriate<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> d. std::map → llvm::StringMap/llvm::DenseMap where appropriate<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
As long as "where appropriate" means there is tangible benefit to changing from a standard type to a non-standard one. Not just change for the sake of change.<o:p class=""></o:p></div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
This is indeed what we meant by where appropriate. The LLVM guidelines are perfectly happy for the use of standard data structures in general, however the LLVM specific ones are much better optimised for certain use cases. For example, the DenseMap class in
particular is much more efficient than std::map when the key and value are both small objects.<o:p class=""></o:p></div>
</div>
<div class="">
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> e. Audit functions in include/flang/common and port to LLVM equivalents (e.g. builtin_popcount)<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> Assuming we hit the above merge date, we think we can commit to making these changes before the LLVM11 branch is taken in June.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> After that date, we will continue to make progress towards LLVM style and API usage by fixing things as we find them during development and enforce the new style through code review.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> <span class="apple-converted-space"> </span><o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> A few specific areas that have been mentioned before that we will tackle in this way are:<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> - Add doxygen style comments and file comments<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> - Find more expressive names for classes, files, etc.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Names are already supposed to be expressive. Improvements are always welcome.<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
> - Refactor code to use early exits<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Where it improves the code, not blindly. As with improving names, there is no reason to wait for any specific date to make improvements to the code.<o:p class=""></o:p></div>
</div>
</blockquote>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
Again, early exists are something the LLVM Coding Standards have a strong preference for, with justification given here:<o:p class=""></o:p></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<a href="https://llvm.org/docs/CodingStandards.html#early-exits" style="color: blue; text-decoration: underline;" class="">https://llvm.org/docs/CodingStandards.html#early-exits</a><br class="">
<br class="">
<o:p class=""></o:p></div>
<blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="">
<div class="">
<div class="MsoNormal" align="center" style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; text-align: center;">
<span style="font-size: 9pt; font-family: Helvetica;" class="">
<hr size="0" width="100%" align="center" class="">
</span></div>
</div>
<div class="">
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-size: 9pt; font-family: Helvetica;" class="">This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are
not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.<o:p class=""></o:p></span></div>
</div>
<div class="">
<div class="MsoNormal" align="center" style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif; text-align: center;">
<span style="font-size: 9pt; font-family: Helvetica;" class="">
<hr size="0" width="100%" align="center" class="">
</span></div>
</div>
<div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">
<span style="font-size: 9pt; font-family: Helvetica;" class="">_______________________________________________<br class="">
flang-dev mailing list<br class="">
</span><a href="mailto:flang-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica;" class="">flang-dev@lists.llvm.org</span></a><span style="font-size: 9pt; font-family: Helvetica;" class=""><br class="">
</span><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev" style="color: blue; text-decoration: underline;" class=""><span style="font-size: 9pt; font-family: Helvetica;" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev</span></a></div>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</body>
</html>