<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"Segoe UI";
panose-1:2 11 5 2 4 2 4 2 2 3;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:411463480;
mso-list-template-ids:250245046;}
@list l1
{mso-list-id:2096004512;
mso-list-template-ids:1322930882;}
@list l1:level1
{mso-level-tab-stop:36.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level2
{mso-level-tab-stop:72.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level3
{mso-level-tab-stop:108.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level4
{mso-level-tab-stop:144.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level5
{mso-level-tab-stop:180.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level6
{mso-level-tab-stop:216.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level7
{mso-level-tab-stop:252.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level8
{mso-level-tab-stop:288.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l1:level9
{mso-level-tab-stop:324.0pt;
mso-level-number-position:left;
text-indent:-18.0pt;}
ol
{margin-bottom:0cm;}
ul
{margin-bottom:0cm;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">It was added when I’ve start poking around with prefixes, to implement the proper recognition of xaquire/xrelease (</span><a href="https://reviews.llvm.org/rL311309"><span style="font-size:10.5pt;font-family:"Segoe UI",sans-serif;color:#6E5CB6;text-decoration:none">rL311309</span></a>)<span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">I can suggest some additional views to the matter at hand:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Enhancing prefix digestion by the parser is highly recommended – aforementioned FIXME note describes those issues I’ve found on a brief exploring, surly there’s
more.<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Currently it is emitted as a standalone instruction, which I don’t see much sense in.<o:p></o:p></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">IMHO, we should aggregate prefixes till we have consumed an ‘actual’ instruction, and then make queries about whether they form a legal combination, and I deem
it to be the right course for the disassembler as well, unless we don’t care about tolerating nonsense in disassembly (how does others disassemblers handle such phenomena?)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Personally, I see prefixes as a part of a particular instruction, so at least on the concept level I’m in favor of ‘Flags’.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">More generally, whether producing multiple MCInsts, using flag or whatever other approach – it’s technicalities.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Agreement should be first reached on what would be considered as a proper handling for both ends (parser, disassembler).<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">p.s. can you guyz please use Phab for further discussion? Hard (for me) to keep track on mail correspondence.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext"> Andrew Tischenko [mailto:tishenandr@xenzu.com]
<br>
<b>Sent:</b> Wednesday, September 27, 2017 12:56<br>
<b>To:</b> Rafael Avila de Espindola <rafael.espindola@gmail.com>; Craig Topper <craig.topper@gmail.com><br>
<b>Cc:</b> reviews+D37262+public+6b8fd5b17a169c12@reviews.llvm.org; Eric Christopher <echristo@gmail.com>; Simon Pilgrim <llvm-dev@redking.me.uk>; Davide Italiano <dccitaliano@gmail.com>; Dinar Temirbulatov <dtemirbulatov@gmail.com>; Tayree, Coby <coby.tayree@intel.com>;
llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> Re: [PATCH] D37262: The issues with X86 prefixes: step 2<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p>BTW, JFYI, I found the following comment in the source:<o:p></o:p></p>
<p><b><span style="font-size:10.0pt;font-family:"Arial",sans-serif"> // FIXME:<br>
// Enhace prefixes integrity robustness. for example, following forms<br>
// are currently tolerated:<br>
// repz repnz <insn> ; GAS errors for the use of two similar prefixes<br>
// lock addq %rax, %rbx ; Destination operand must be of memory type<br>
// xacquire <insn> ; xacquire must be accompanied by 'lock'<br>
<br>
</span></b><o:p></o:p></p>
<p>The approach with Flag will allow to implement it.<o:p></o:p></p>
<p>Andrew<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On 27.09.2017 12:18, Andrew Tischenko wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p>OK, I'll try to change the assembler properly but there are some questions:<o:p></o:p></p>
<ol start="1" type="1">
<li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3">
Should I do it in the same patch?<o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3">
Currently if we have:<o:p></o:p></li></ol>
<p>repz repnz repe cmpsb<o:p></o:p></p>
<p> then we produce with 'llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel -show-encoding intel-syntax.s':<o:p></o:p></p>
<p> rep # encoding: [0xf3]<br>
repne # encoding: [0xf2]<br>
rep # encoding: [0xf3]<br>
cmpsb %es:(%rdi), (%rsi) # encoding: [0xa6]<o:p></o:p></p>
<p> but after the change we'll get only the following:<o:p></o:p></p>
<p> rep # encoding: [0xf3]<br>
cmpsb %es:(%rdi), (%rsi) # encoding: [0xa6] <o:p></o:p></p>
<p> Is it OK? (IMHO, Yes.)<o:p></o:p></p>
<p> If YES, do we need any warnings here? (IMHO, No.)<o:p></o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt">Andrew<o:p></o:p></p>
<div>
<p class="MsoNormal">On 26.09.2017 22:31, Rafael Avila de Espindola wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>The assembler and disassembler should use the same path.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>I would be OK with always producing 1 or N instructions, as long as both<o:p></o:p></pre>
<pre>the assembler and disassembler do the same. That is, it is OK to have<o:p></o:p></pre>
<pre>Flags, as long as the assembler uses that instead of creating a separate<o:p></o:p></pre>
<pre>instruction for prefixes.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>It seems that allowing the disassembler to create multiple instructions<o:p></o:p></pre>
<pre>would have the advantage of not needing Flags, but that is secondary<o:p></o:p></pre>
<pre>IMHO.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Cheers,<o:p></o:p></pre>
<pre>Rafael<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Craig Topper <a href="mailto:craig.topper@gmail.com"><craig.topper@gmail.com></a> writes:<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Here's my understanding of what I think happens today.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>-For a very select few instructions if the AsmParser sees a repne/repe<o:p></o:p></pre>
<pre>prefix it creates a special version of the instruction that has the REP<o:p></o:p></pre>
<pre>bits set in TSFlags. For any other instruction it emits the repne/rep/repe<o:p></o:p></pre>
<pre>as a separate MCInst.<o:p></o:p></pre>
<pre>-For the disassembler if it sees a repne/repe byte at the start that it<o:p></o:p></pre>
<pre>doesn't think goes with an instruction it will emit a MCInst containing<o:p></o:p></pre>
<pre>just the REP.<o:p></o:p></pre>
<pre>-If the disassembler encounters a repne/repe byte not at the start of the<o:p></o:p></pre>
<pre>instruction that doesn't go with the instruction we drop it and don't print<o:p></o:p></pre>
<pre>anything. The disassembler interface only allows us to return one<o:p></o:p></pre>
<pre>instruction. So we can't return a separate repne/repe instruction and a<o:p></o:p></pre>
<pre>real instruction from the same byte sequence. I don't believe the assembler<o:p></o:p></pre>
<pre>can ever produce a byte sequence that hits this case, but that doesn't mean<o:p></o:p></pre>
<pre>some binary couldn't contain that string of bytes created by hand. So this<o:p></o:p></pre>
<pre>patch is trying to preserve the extra prefix information in the one MCInst<o:p></o:p></pre>
<pre>we're allowed to emit. Maybe another option would be to allow creating<o:p></o:p></pre>
<pre>multiple MCInsts from the disassembler?<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>~Craig<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>On Tue, Sep 26, 2017 at 10:37 AM, Rafael Avila de Espindola <<o:p></o:p></pre>
<pre><a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>The question is why it is different for disassembler than for the<o:p></o:p></pre>
<pre>assembler?<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>How does the assembler handle trepne?<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Cheers,<o:p></o:p></pre>
<pre>Rafael<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Andrew Tischenko <a href="mailto:tishenandr@xenzu.com"><tishenandr@xenzu.com></a> writes:<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>It is not a simple flag, it's some data. And this data could be useful<o:p></o:p></pre>
<pre>for any other component because it's some opaque info which could be<o:p></o:p></pre>
<pre>send via MCInst from one low level target component to another one.<o:p></o:p></pre>
<pre>Without this (additional) data MCInst loosing (potentially very useful)<o:p></o:p></pre>
<pre>info about the given instruction.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Andrew<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>On 25.09.2017 22:05, Rafael Avila de Espindola wrote:<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Having a flag field that is used only on disassembly seems wrong.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Don't we support parsing our own output? I don't see trepne in any .s<o:p></o:p></pre>
<pre>test for example.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Cheers,<o:p></o:p></pre>
<pre>Rafael<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Craig Topper via Phabricator <a href="mailto:reviews@reviews.llvm.org"><reviews@reviews.llvm.org></a> writes:<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>craig.topper added a comment.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>I'm not sure I can approve growing the size of MCInst. Though I can't<o:p></o:p></pre>
</blockquote>
</blockquote>
</blockquote>
<pre>see anyway around it. @rafael what do you think?<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre><a href="https://reviews.llvm.org/D37262">https://reviews.llvm.org/D37262</a><o:p></o:p></pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>
<p>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.</p></body>
</html>