<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>The slightly unexpected bit for me in these responses is the
      willingness to accept layout changes if documented.  Let me lay
      out some options, which of these seem reasonable?</p>
    <p>1) Default to automatic padding, provide option to disable,
      document where padding is inserted.</p>
    <p>2) Default to not automatically padding, provide option to
      enable, document where padding would be inserted if enabled.</p>
    <p>And orthogonality to the above, two interpretations of each:<br>
    </p>
    <p>a) auto padding is allowed to break common idioms (because you
      enabled it explicitly or because you can disable).  If it does,
      you are expected to either simply not use it, or possibly use fine
      grained directives in source.<br>
    </p>
    <p>b) auto padding conservative even when explicitly enabled.  One
      implication of this scheme is that different versions of the
      assembler will almost by definition have to tweak insertion as we
      find new problematic idioms.  This creates both documentation
      difficulties, and user confusion. <br>
    </p>
    <p>So, four choices total.  Which seem reasonable?<br>
    </p>
    <p>Personally, I'd have no problem w/ 2a.  Any of the other variants
      concern me.</p>
    <p>Philip<br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 3/25/20 1:08 PM, Eric Christopher
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CALehDX764+43GhVKwjkJCH_0qOSerNfgFsa3cBjAXFn_tfYyaQ@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">FWIW I'm with Eli here if you need any more data
        points.
        <div><br>
        </div>
        <div>-eric</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Tue, Mar 24, 2020 at 8:21
          PM Eli Friedman via llvm-dev <<a
            href="mailto:llvm-dev@lists.llvm.org" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px
          0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div lang="EN-US">
            <div class="gmail-m_-358140349290300678WordSection1">
              <p class="MsoNormal">Changing the length of a sequence of
                assembly instructions will break someone’s code at some
                point.  The length of a sequence of instructions is
                known, in general, and people will write code to take
                advantage of that. For example, I’ve seen assembly code
                using something like Duff’s device, except that instead
                of using a jump table, it just computed the destination
                as “base+n*caselength”.  Given that, I don’t think it’s
                reasonable to hide this mechanism from user control.
              </p>
              <p class="MsoNormal"> </p>
              <p class="MsoNormal">We definitely should not have any
                undocumented or unpredictable behavior in the
                assembler.  The actual instruction bytes matter.  That
                said, I’m not sure there’s a strong line between
                “automagic” and “explicit”, as long as the rules are
                documented.</p>
              <p class="MsoNormal"> </p>
              <p class="MsoNormal">-Eli</p>
              <p class="MsoNormal"> </p>
              <div
style="border-top:none;border-right:none;border-bottom:none;border-left:1.5pt
                solid blue;padding:0in 0in 0in 4pt">
                <div>
                  <div
style="border-right:none;border-bottom:none;border-left:none;border-top:1pt
                    solid 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" moz-do-not-send="true">llvm-dev-bounces@lists.llvm.org</a>>
                      <b>On Behalf Of
                      </b>Philip Reames via llvm-dev<br>
                      <b>Sent:</b> Tuesday, March 24, 2020 3:55 PM<br>
                      <b>To:</b> llvm-dev <<a
                        href="mailto:llvm-dev@lists.llvm.org"
                        target="_blank" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>><br>
                      <b>Cc:</b> Luo, Yuanke <<a
                        href="mailto:yuanke.luo@intel.com"
                        target="_blank" moz-do-not-send="true">yuanke.luo@intel.com</a>>;
                      Zhang, Annita <<a
                        href="mailto:annita.zhang@intel.com"
                        target="_blank" moz-do-not-send="true">annita.zhang@intel.com</a>>;
                      Craig Topper <<a
                        href="mailto:craig.topper@intel.com"
                        target="_blank" moz-do-not-send="true">craig.topper@intel.com</a>><br>
                      <b>Subject:</b> [EXT] [llvm-dev] Status of Intel
                      JCC Mitigations and Next Steps</p>
                  </div>
                </div>
                <p class="MsoNormal"> </p>
                <p>TLDR - We have a choice to make about assembler
                  support, and a disagreement about how to move
                  forward.  Community input needed.</p>
                <p> </p>
                <p>Background</p>
                <p>Intel has a hardware bug in Skylake and later whose
                  mitigation requires padding of branches to avoid
                  performance degradation.  Background here:
                  <a
href="https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf"
                    target="_blank" moz-do-not-send="true">
https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf</a></p>
                <p>We now have in tree support for alignment of such
                  branches via nop padding, and limited support for
                  padding existing instructions with either prefixes or
                  larger immediate values.  This has survived several
                  days of dedicated testing and appears to be reasonably
                  robust.  The padding support applies both to branch
                  alignment for the mitigation, but also normal align
                  directives. 
                </p>
                <p>The original patches proposed a somewhat different
                  approach than we've ended up taking - primarily
                  because of memory overhead concerns.  However, there
                  was also a deeper disagreement on the original review
                  threads (D70157 and others) which was never settled,
                  and we seem to be at a point where this needs
                  attention.  In short, the question is how assembler
                  support should be handled.</p>
                <p> </p>
                <p>The Choice</p>
                <p>The problematic use case comes when assembling user
                  provided .s files.  (Instead of the more restricted
                  output of the compiler.)  Our basic choice is do we
                  want to force a new directive syntax (and thus a
                  source code change to use the new feature), or attempt
                  to automatically infer where it's safe to insert
                  padding?</p>
                <p>The options as I see them:</p>
                <ul type="disc">
                  <li class="MsoNormal">
                    Assembler directives w/explicit opt in - In this
                    model, assembler input is assumed to only enable
                    padding in regions where it is safe to do so.</li>
                  <li class="MsoNormal">
                    Automagic assembler - In this model, the assembler
                    is responsible for inferring where it is legal to
                    pad without breaking user expectations. 
                  </li>
                </ul>
                <p>(I'll stop and disclaim that I'm strongly in favor of
                  the former.  I've tried to describe the pros/cons of
                  each, but my perspective is definitely biased.)</p>
                <p>The difference between the two is a huge amount of
                  complexity, and a very fundamental correctness risk. 
                  The basic problem is that assemblers have to handle
                  unconstrained inputs, and IMO, the semantics of
                  assembler as used in practice is so under specified
                  that it's really hard to infer semantics in any useful
                  way.  As a couple of examples, is the fault behavior
                  of an instruction well defined?  Is the label near an
                  instruction used by the signal handler?  Is that data
                  byte just before an instruction actually decoded as
                  part of the instruction?</p>
                <p>The benefit of the later option is that existing
                  assembly files can be used without modification.  This
                  is a huge advantage in terms of ease of mitigation for
                  existing code bases.  It's also the approach the
                  original patch sets for GCC took. 
                </p>
                <p>In the original review thread(s), I had taken the
                  position that we should reject the automagic assembler
                  based on the correctness concerns mentioned.  I had
                  thought the consensus in the review was clearly in
                  that direction as well, but this has recently come up
                  again.  Given that, I wanted to open it to a wider
                  audience.</p>
                <p> </p>
                <p>Why am I pushing for a decision now?</p>
                <p>There are two major reasons.  First, there have
                  recently been a couple of patches posted and landed
                  (D76176, and D76052) building towards the automagic
                  assembler variant.  And second, I've started getting
                  review comments (<a
                    href="https://reviews.llvm.org/D76398#1930383"
                    target="_blank" moz-do-not-send="true">https://reviews.llvm.org/D76398#1930383</a>)
                  which block forward progress on generalized padding
                  support assuming the automagic interpretation. 
                  Implementing the automatic assembler variant for
                  prefix and immediate padding adds substantial
                  complexity and I would very much like not to bother
                  with if I don't have to.</p>
                <p> </p>
                <p>Current implementation details</p>
                <p>We have support in the integrated assembler only for
                  autopadding suppression.  This allows a LLVM based
                  compiler to effectively apply padding selectively.  In
                  particular, we've instrumented lowering from MI to MC
                  (X86MCInstLowering.cpp) to selectively disable padding
                  around constructs which are thought to be
                  problematic.  We do not have an agreed upon syntax for
                  this in assembler; the code that got checked in is
                  modeled closely around the last seriously discussed
                  variant (see below).  This support is able to use all
                  of the padding variants: nop, prefix, and immediate.</p>
                <p>We also have limited support in the assembler for not
                  inserting nops between fragments where doing so would
                  break known idioms.  The list of such idioms is, IMO,
                  ad hoc.  This assembler support does not include
                  prefix or immediate padding. 
                </p>
                <p> </p>
                <p>Philip</p>
                <p>p.s. For those interested, here's roughly what the
                  last round of assembler syntax I remember being
                  discussed looked like.</p>
                <p>.autopadding<br>
                  .noautopadding</p>
                <p>These two directives would respectively enable and
                  disable automatic padding of instructions within the
                  region defined.  It's presumed to be legal to insert
                  nops between instructions, modify encodings, or
                  otherwise adjust offsets of instruction boundaries
                  within the region to achieve target specific desired
                  alignments.  Similarly, it's presumed not to be legal
                  to change relative offsets outside an explicitly
                  enabled region.  (Except for existing cases - e.g.
                  relaxation of branches, etc...)</p>
                <p>The assembler would provide a command line flag which
                  conceptually wrapped the whole file in a pair of
                  enable/disable directives. 
                </p>
                <p>We'd previously discussed a variant with push/pop
                  semantics and more fine grained control over alignment
                  requests, but I believe we decided that was overkill
                  in the end.  (I walked away with that impression based
                  on the integrated assembler work at least.)</p>
                <p> </p>
              </div>
            </div>
          </div>
          _______________________________________________<br>
          LLVM Developers mailing list<br>
          <a href="mailto:llvm-dev@lists.llvm.org" target="_blank"
            moz-do-not-send="true">llvm-dev@lists.llvm.org</a><br>
          <a
            href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev"
            rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
        </blockquote>
      </div>
    </blockquote>
  </body>
</html>