<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>BTW, JFYI, I found the following comment in the source:</p>
    <p><span style="color: rgb(0, 0, 0); font-family: "Segoe
        UI", "Segoe UI Emoji", "Segoe UI
        Symbol", Lato, "Helvetica Neue", Helvetica,
        Arial, sans-serif; font-size: 13px; font-style: normal;
        font-variant-ligatures: normal; font-variant-caps: normal;
        font-weight: bold; letter-spacing: normal; orphans: 2;
        text-align: left; text-indent: 0px; text-transform: none;
        white-space: nowrap; widows: 2; word-spacing: 0px;
        -webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
        255); text-decoration-style: initial; text-decoration-color:
        initial; display: inline !important; float: none;">  // 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>
      </span><br>
    </p>
    <p>The approach with Flag will allow to implement it.</p>
    <p>Andrew<br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 27.09.2017 12:18, Andrew Tischenko
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:adf6bb19-0550-f4a6-5ab6-74bc610e95c9@xenzu.com">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <p>OK, I'll try to change the assembler properly but there are
        some questions:</p>
      <ol>
        <li>Should I do it in the same patch?</li>
        <li>Currently if we have:</li>
      </ol>
      <p>repz repnz repe cmpsb</p>
      <p>    then we produce with 'llvm-mc -triple
        x86_64-unknown-unknown -x86-asm-syntax=intel -show-encoding
        intel-syntax.s':</p>
      <p>        rep                             # encoding: [0xf3]<br>
                repne                           # encoding: [0xf2]<br>
                rep                             # encoding: [0xf3]<br>
                cmpsb   %es:(%rdi), (%rsi)      # encoding: [0xa6]</p>
      <p>    but after the change we'll get only the following:<br>
      </p>
      <p>        rep                             # encoding: [0xf3]<br>
                cmpsb   %es:(%rdi), (%rsi)      # encoding: [0xa6] </p>
      <p>    Is it OK? (IMHO, Yes.)</p>
      <p>    If YES, do we need any warnings here? (IMHO, No.)<br>
      </p>
      Andrew<br>
      <br>
      <div class="moz-cite-prefix">On 26.09.2017 22:31, Rafael Avila de
        Espindola wrote:<br>
      </div>
      <blockquote type="cite"
cite="mid:87lgl1dzol.fsf@dell.i-did-not-set--mail-host-address--so-tickle-me">
        <pre wrap="">The assembler and disassembler should use the same path.

I would be OK with always producing 1 or N instructions, as long as both
the assembler and disassembler do the same. That is, it is OK to have
Flags, as long as the assembler uses that instead of creating a separate
instruction for prefixes.

It seems that allowing the disassembler to create multiple instructions
would have the advantage of not needing Flags, but that is secondary
IMHO.

Cheers,
Rafael

Craig Topper <a class="moz-txt-link-rfc2396E" href="mailto:craig.topper@gmail.com" moz-do-not-send="true"><craig.topper@gmail.com></a> writes:

</pre>
        <blockquote type="cite">
          <pre wrap="">Here's my understanding of what I think happens today.

-For a very select few instructions if the AsmParser sees a repne/repe
prefix it creates a special version of the instruction that has the REP
bits set in TSFlags. For any other instruction it emits the repne/rep/repe
as a separate MCInst.
-For the disassembler if it sees a repne/repe byte at the start that it
doesn't think goes with an instruction it will emit a MCInst containing
just the REP.
-If the disassembler encounters a repne/repe byte not at the start of the
instruction that doesn't go with the instruction we drop it and don't print
anything. The disassembler interface only allows us to return one
instruction. So we can't return a separate repne/repe instruction and a
real instruction from the same byte sequence. I don't believe the assembler
can ever produce a byte sequence that hits this case, but that doesn't mean
some binary couldn't contain that string of bytes created by hand. So this
patch is trying to preserve the extra prefix information in the one MCInst
we're allowed to emit. Maybe another option would be to allow creating
multiple MCInsts from the disassembler?

~Craig

On Tue, Sep 26, 2017 at 10:37 AM, Rafael Avila de Espindola <
<a class="moz-txt-link-abbreviated" href="mailto:rafael.espindola@gmail.com" moz-do-not-send="true">rafael.espindola@gmail.com</a>> wrote:

</pre>
          <blockquote type="cite">
            <pre wrap="">The question is why it is different for disassembler than for the
assembler?

How does the assembler handle trepne?

Cheers,
Rafael

Andrew Tischenko <a class="moz-txt-link-rfc2396E" href="mailto:tishenandr@xenzu.com" moz-do-not-send="true"><tishenandr@xenzu.com></a> writes:

</pre>
            <blockquote type="cite">
              <pre wrap="">It is not a simple flag, it's some data. And this data could be useful
for any other component because it's some opaque info which could be
send via MCInst from one low level target component to another one.
Without this (additional) data MCInst loosing (potentially very useful)
info about the given instruction.

Andrew

On 25.09.2017 22:05, Rafael Avila de Espindola wrote:
</pre>
              <blockquote type="cite">
                <pre wrap="">Having a flag field that is used only on disassembly seems wrong.

Don't we support parsing our own output? I don't see trepne in any .s
test for example.

Cheers,
Rafael

Craig Topper via Phabricator <a class="moz-txt-link-rfc2396E" href="mailto:reviews@reviews.llvm.org" moz-do-not-send="true"><reviews@reviews.llvm.org></a> writes:

</pre>
                <blockquote type="cite">
                  <pre wrap="">craig.topper added a comment.

I'm not sure I can approve growing the size of MCInst. Though I can't
</pre>
                </blockquote>
              </blockquote>
            </blockquote>
            <pre wrap="">see anyway around it. @rafael what do you think?
</pre>
            <blockquote type="cite">
              <blockquote type="cite">
                <blockquote type="cite">
                  <pre wrap="">
<a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D37262" moz-do-not-send="true">https://reviews.llvm.org/D37262</a>
</pre>
                </blockquote>
              </blockquote>
            </blockquote>
          </blockquote>
        </blockquote>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>