<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 10, 2022 at 11:08 PM Maksim Panchenko <<a href="mailto:maks@fb.com" target="_blank">maks@fb.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">





<div lang="EN-US" style="word-wrap:break-word">
<div>
<p class="MsoNormal">Thank you, everyone, for the review!<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">If there are no further objections, we’d like to proceed with the merge.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">As Rafael posted earlier, the preview is available at <a href="https://github.com/facebookincubator/BOLT" target="_blank">https://github.com/facebookincubator/BOLT</a>.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">For the merge, I’ve created a special branch “bolt-project-only” at
<a href="https://github.com/facebookincubator/BOLT/tree/bolt-project-only" target="_blank">https://github.com/facebookincubator/BOLT/tree/bolt-project-only</a>. This branch contains only “/bolt” project. All commits in this branch touch just “/bolt”.<u></u><u></u></p>
<p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal">To merge BOLT, we have to execute the following commands from the LLVM monorepo:<u></u><u></u></p>
<p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal">  $ cd llvm-project<u></u><u></u></p>
<p class="MsoNormal">  $ git remote add bolt-project git@github.com:facebookincubator/BOLT.git<u></u><u></u></p>
<p class="MsoNormal">  $ git fetch bolt-project bolt-project-only<u></u><u></u></p>
<p class="MsoNormal">  $ git pull<u></u><u></u></p>
<p class="MsoNormal">  $ git merge --allow-unrelated-histories bolt-project/bolt-project-only -m "Merge BOLT into LLVM monorepo"<u></u><u></u></p>
<p class="MsoNormal">  $ git remote remove bolt-project<u></u><u></u></p>
<p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal">Then push to GH. I think we have to disable triggers (email, buildbots) before the push, but I’m not sure how merge commits are treated.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Does this make sense and look similar to how flang and MLIR were merged?</p></div></div></blockquote><div><br></div><div>This looks right (I haven't tried locally though).</div><div><br></div><div>Something that is worth doing before pushing is also to repack the repo locally: it can impact the future clone size in a non-negligible way (~15% smaller when we repacked before pushing the MLIR merge, but not much for Flang, so your mileage may vary...).</div><div><br></div><div>Cheers,</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div lang="EN-US" style="word-wrap:break-word"><div><p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
<p class="MsoNormal">Maksim<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On 1/10/22, 6:22 PM, "llvm-dev" <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Yes, I am supportive for the merge and those are only suggestions and are not the block issues for the merge.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">See more comments below.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">Best<u></u><u></u></p>
<p class="MsoNormal">Shengchen<u></u><u></u></p>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> Amir Aupov <<a href="mailto:amir.aupov@gmail.com" target="_blank">amir.aupov@gmail.com</a>> <br>
<b>Sent:</b> Tuesday, January 11, 2022 6:24 AM<br>
<b>To:</b> Craig Topper <<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>><br>
<b>Cc:</b> Kan, Shengchen <<a href="mailto:shengchen.kan@intel.com" target="_blank">shengchen.kan@intel.com</a>>; <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm-dev] Preparing BOLT for LLVM monorepo<u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">Thanks for clarifying, Craig! <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">+Shengchen:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">It looks like for the rest of the suggestions we'd need to make changes to tablegen.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">We will address them after the merge as we will have a use case in monorepo.<u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal">Just to clarify: our understanding is that you support the merge but just have suggestions.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">We don't want to be blocked right now as we've started the merge process this week.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Does that sound right to you?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">On Mon, Jan 10, 2022 at 2:02 PM Craig Topper <<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal">On Mon, Jan 10, 2022 at 1:12 PM Amir Aupov via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal">Hi Shengchen,<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Thank you for the code review and your suggestions!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">We've addressed some of them (see inline) but still in the process of addressing them all.<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Jan 4, 2022 at 7:56 PM Kan, Shengchen <<a href="mailto:shengchen.kan@intel.com" target="_blank">shengchen.kan@intel.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal">Hi Amir<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">I read the MC-related code and it looks fine to me in general. I’m also supportive for the merge. The following are some suggestions.<u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<ol start="1" type="1">
<li class="MsoNormal">
<a href="https://llvm.org/docs/CodingStandards.html#braced-initializer-lists" target="_blank">https://llvm.org/docs/CodingStandards.html#braced-initializer-lists</a>
  If you use a braced initializer list when initializing a variable, use an equals before the open curly brace. e.g BinaryBasicBlock.h<u></u><u></u></li></ol>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Addressed, soon to be published.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<ol start="1" type="1">
<li class="MsoNormal">
<a href="https://llvm.org/docs/ProgrammersManual.html#fine-grained-debug-info-with-debug-type-and-the-debug-only-option" target="_blank">https://llvm.org/docs/ProgrammersManual.html#fine-grained-debug-info-with-debug-type-and-the-debug-only-option</a>
 DEBUG_TYPE should not be target specific,<u></u><u></u></li></ol>
<p>e.g, the definition of DEBUG_TYPE should be same in X86MCPlusBuilder.cpp and AArch64MCPlusBuilder.cpp<u></u><u></u></p>
</div>
</blockquote>
<div>
<p class="MsoNormal">Addressed in <a href="https://github.com/facebookincubator/BOLT/commit/7ca0c5a26714754ec1dd72576ddecaa8b459842c" target="_blank">https://github.com/facebookincubator/BOLT/commit/7ca0c5a26714754ec1dd72576ddecaa8b459842c</a> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<ol start="3" type="1">
<li class="MsoNormal">
Functions getShortArithOpcode, getShortBranchOpcode in X86MCPlusBuilder.cpp use reversed mappings as functions getRelaxedOpcodeBranch, getRelaxedOpcodeArith in X86AsmBackend.cpp. Writing twice in two places makes the code difficult to maintain, could we generate
 the table with tablgen? The memory folding and unfolding tables in X86InstrFoldTables.cpp give an example.<u></u><u></u></li></ol>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">To make sure I get it right: the tables in X86InstrFoldTables.cpp were generated with tablegen at some point, but are checked in as source code? Is there a way to rebuild these tables, so I can use that as an example?<u></u><u></u></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">There is a tablegen backend that can generate it, but it has some bugs. Especially around some oddities with the SSE movd/movq instructions if I remember right. I used to run it after adding new instructions and picked the table entries
 I thought were ok. The code is in llvm/utils/TableGen/<span style="font-size:8.5pt;font-family:Menlo;color:black">X86FoldTablesEmitter.cpp but I'm not sure how helpful it is. It's mostly trying to find pairs of instructions that different in the register vs
 memory form of the modrm byte with rest of the encoding being the same. For your case, the instructions differ in the opcode byte. I guess you'd have to write a tablegen backend that knew rules like if opcode==0x81 or 0x83, the other instruction is the one
 with the other opcode, but rest of the encoding is the same.</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(112,48,160)">I think we can just check the instruction name rather than opcode in the tablgen, e,g. ADD32ri and ADD32ri8 have the same prefix “ADD32ri”, but one of them has suffix “8”.  If you do not want to hand-write the
 table rather than generate it w/ tablgen, at least getShortArithOpcode can share the same table with getRelaxedOpcodeArith. The function pair lookupUnfoldTable and lookupFoldTable is an example.</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<ol start="3" type="1">
<li class="MsoNormal">
It would better if functions isADD, isAND, isCMP, isDEC, isINC, isSUB and isTEST can be generated with tablgen.<u></u><u></u></li></ol>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Removed our definitions of isINC/isDEC: <a href="https://github.com/facebookincubator/BOLT/commit/9fec2d58964d643a7436ba2804f8fb967f23881e" target="_blank">https://github.com/facebookincubator/BOLT/commit/9fec2d58964d643a7436ba2804f8fb967f23881e</a><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">How do we approach generating isADD/... with tablegen? There are appropriate definitions (`defm ADD`, etc) in llvm/lib/Target/X86/X86InstrArithmetic.td but I'm not sure how to get tablegen to generate the tables.<u></u><u></u></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">I think you'd have to add isAdd, isCmp, etc. fields to the X86Inst class and set them on the appropriate instructions. Then have a tablegen backend collect all the instructions with each property. Or you could figure out the different encoding
 rules like if the opcode is 0x80-0x83 than the format being MRM4r/MRM4m/MRM4X mean the opcode is an AND.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(112,48,160)">I think we can just check the instruction name in tablgen, e.g, ADD8ri, ADD16ri, ADD32ri, ADD32ri8 and ADD64ri have same prefix “ADD” followed by a number .</span><u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">-Amir<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal">Best<u></u><u></u></p>
<p class="MsoNormal">Shengchen<u></u><u></u></p>
<div>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>Wang, Phoebe via llvm-dev<br>
<b>Sent:</b> Thursday, December 30, 2021 5:33 PM<br>
<b>To:</b> Amir Aupov <<a href="mailto:amir.aupov@gmail.com" target="_blank">amir.aupov@gmail.com</a>><br>
<b>Cc:</b> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm-dev] Preparing BOLT for LLVM monorepo<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">Hi Amir,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">Amazing job! Thanks for the quick and perfect changes. I don’t have any other problems now. Hoping seeing the code in main trunk soon.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">Thanks</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">Phoebe</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)"> </span><u></u><u></u></p>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> Amir Aupov <<a href="mailto:amir.aupov@gmail.com" target="_blank">amir.aupov@gmail.com</a>>
<br>
<b>Sent:</b> Thursday, December 30, 2021 4:32 PM<br>
<b>To:</b> Wang, Phoebe <<a href="mailto:phoebe.wang@intel.com" target="_blank">phoebe.wang@intel.com</a>><br>
<b>Cc:</b> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm-dev] Preparing BOLT for LLVM monorepo<u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">Hi Phoebe,<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Thank you again for reviewing the codebase and for your feedback. We've addressed it in recent commits:<u></u><u></u></p>
</div>
<div>
<pre style="white-space:pre-wrap"><span style="color:black">>  1.  Use the monorepo of LLVM in the example for convenience in <a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/docs/OptimizingClang.md#getting-clang-7-sources" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/docs/OptimizingClang.md#getting-clang-7-sources</a></span><u></u><u></u></pre>
<pre><span style="color:black">E.g, git clone --branch=release/7.x <a href="https://github.com/llvm/llvm-project.git" target="_blank">https://github.com/llvm/llvm-project.git</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/c4ed6a11eb264660f5cfe7f5165df35b63d52608" target="_blank">https://github.com/facebookincubator/BOLT/commit/c4ed6a11eb264660f5cfe7f5165df35b63d52608</a></span><u></u><u></u></pre>
<pre><span style="color:black"> </span><u></u><u></u></pre>
<pre><span style="color:black">>  1.  I found there are 142 code with “not implemented”, most of which are in Core/MCPlusBuilder.h.</span><u></u><u></u></pre>
<pre><span style="color:black">     *   Do they affect the functionality of BOLT?</span><u></u><u></u></pre>
<pre><span style="color:black">     *   Do you have plan to implement them recently or can they be removed instead?</span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black">Explained above.</span><u></u><u></u></pre>
<pre><span style="color:black">>  2.  I noticed some inconsistent use of braces in the code. Maybe better to follow with LLVM coding standard<<a href="https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements" target="_blank">https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements</a>>.</span><u></u><u></u></pre>
<pre><span style="color:black">     *   <a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryBasicBlock.cpp#L241," target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryBasicBlock.cpp#L241,</a> L289<<a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryBasicBlock.cpp#L289" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryBasicBlock.cpp#L289</a>>, etc.</span><u></u><u></u></pre>
<pre><span style="color:black">     *   <a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/DebugData.cpp#L193" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/DebugData.cpp#L193</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/fe3fcf094e320af4da4a98a5387820cd0ca826b9" target="_blank">https://github.com/facebookincubator/BOLT/commit/fe3fcf094e320af4da4a98a5387820cd0ca826b9</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/b40699ed90882465e49ec46e4d3ad28623e54c69" target="_blank">https://github.com/facebookincubator/BOLT/commit/b40699ed90882465e49ec46e4d3ad28623e54c69</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/c21ee425083276a7af700e232df15e2a3d31dacb" target="_blank">https://github.com/facebookincubator/BOLT/commit/c21ee425083276a7af700e232df15e2a3d31dacb</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/ebc06afcfc35fbb4360b3788adeb6228c27ad830" target="_blank">https://github.com/facebookincubator/BOLT/commit/ebc06afcfc35fbb4360b3788adeb6228c27ad830</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/dca3003fce711daee387bbbf66bf4ecbe1b9ab9b" target="_blank">https://github.com/facebookincubator/BOLT/commit/dca3003fce711daee387bbbf66bf4ecbe1b9ab9b</a></span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black"><a href="https://github.com/facebookincubator/BOLT/commit/cd526a4550455b0f203771088cef6e512b0dd1a2" target="_blank">https://github.com/facebookincubator/BOLT/commit/cd526a4550455b0f203771088cef6e512b0dd1a2</a></span><u></u><u></u></pre>
<pre><span style="color:black">>  3.  Some files don’t have a descriptions in the first line, e.g.  DynoStats.cpp<<a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/DynoStats.cpp#L1" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/DynoStats.cpp#L1</a>>, ParallelUtilities.cpp<<a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/ParallelUtilities.cpp#L1" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/ParallelUtilities.cpp#L1</a>>, etc.</span><u></u><u></u></pre>
<pre style="white-space:pre-wrap"><span style="color:black">>  4.  Leaving without descriptions might be fine, but the format should be consistent. Leaving with spaces like in BinaryFunctionProfile.cpp<<a href="https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryFunctionProfile.cpp#L1" target="_blank">https://github.com/facebookincubator/BOLT/blob/main/bolt/lib/Core/BinaryFunctionProfile.cpp#L1</a>> doesn’t make sense.</span><u></u><u></u></pre>
<p class="MsoNormal"><a href="https://github.com/facebookincubator/BOLT/commit/349c8abc696919e989418895f4ade24a4c910617" target="_blank">https://github.com/facebookincubator/BOLT/commit/349c8abc696919e989418895f4ade24a4c910617</a><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Dec 21, 2021 at 4:22 AM Wang, Phoebe <<a href="mailto:phoebe.wang@intel.com" target="_blank">phoebe.wang@intel.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt">
<div>
<div>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">Hi Amir,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)">It makes sense to me. Thanks!</span><u></u><u></u></p>
<p class="MsoNormal"><span style="color:rgb(31,73,125)"> </span><u></u><u></u></p>
<div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>>
<b>On Behalf Of </b>Amir Aupov via llvm-dev<br>
<b>Sent:</b> Tuesday, December 21, 2021 8:57 AM<br>
<b>To:</b> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm-dev] Preparing BOLT for LLVM monorepo<u></u><u></u></p>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">Hi Phoebe,<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks again for taking the time to look at the codebase. We've added your feedback to our upstreaming tracking issue on github: <a href="https://github.com/facebookincubator/BOLT/issues/248" target="_blank">https://github.com/facebookincubator/BOLT/issues/248</a>.
 We've started addressing the issues.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Regarding "not implemented” in Core/MCPlusBuilder.h: it's a design decision to prevent the use of non-implemented target-specific methods (these are virtual methods). Target-specific
 implementation goes to X86MCPlusBuilder and AArch64MCPlusBuilder. Some methods only make sense for specific targets, so we intend to keep llvm_unreachable statements in MCPlusBuilder.h.<u></u><u></u></p>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>

</blockquote></div></div>