<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 6, 2018 at 3:09 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_8712384487290472447gmail-m_2565003435785155913gmail-">Ryan Prichard <<a href="mailto:rprichard@google.com" target="_blank">rprichard@google.com</a>> writes:<br>
<br>> I don't think the optimization prevents the creation of new fragments. For<br>
> an absolute (Hi - Lo) expression, the general code path calls<br>
> MCObjectStreamer::EmitValueImp<wbr>l,<br>
> then MCObjectStreamer::EmitBytes. EmitValueImpl and EmitBytes do a lot of<br>
> the same work. With the optimization, EmitValueImpl is skipped, and EmitBytes<br>
> is called more directly. I think the important part is that it avoids<br>
> allocating and evaluating an MCExpr tree.<br>
<br>
</span>Oh, that is less than what I would expect. It is tempting to remove the<br>
optimization in a followup patch.<br></blockquote><div><br></div><div>The optimization was added in 2015 to reduce memory usage on Mach-O. There was some discussion of it on llvm-commits:</div><div><br></div><div> - <a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150601/279975.html" target="_blank">http://lists.llvm.org/piper<wbr>mail/llvm-commits/Week-of-Mon-<wbr>20150601/279975.html</a></div><div> - <a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20150608/281392.html" target="_blank">http://lists.llvm.org/piper<wbr>mail/llvm-commits/Week-of-Mon-<wbr>20150608/281392.html</a></div><div><br></div><div>Removing it for ULEB output would create 6 or 9 MCExpr nodes per entry in the call site table. That seems acceptable to me, though I'm not sure why there was a memory issue with Mach-O in the first place.<br></div><div><br></div><div>I noticed that the comment on emitAbsoluteSymbolDiff in MCObjectStreamer.h was out-of-date regarding the return type:</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">  /// Emit the absolute difference between two symbols if possible.<br>  ///<br>  /// Emit the absolute difference between \c Hi and \c Lo, as long as we can<br>  /// compute it.  Currently, that requires that both symbols are in the same<br>  /// data fragment.  Otherwise, do nothing and return \c false.<br>  ///<br>  /// \pre Offset of \c Hi is greater than the offset \c Lo.<br>  void emitAbsoluteSymbolDiff(const MCSymbol *Hi, const MCSymbol *Lo,<br>                              unsigned Size) override;</blockquote></div><div><br></div><div>I've split the patch into two parts now:<br></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">- <a href="https://reviews.llvm.org/D42720" target="_blank">https://reviews.llvm.org/<wbr>D42720</a>: uses assembler directives</span><br style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">- <a href="https://reviews.llvm.org/D43001" target="_blank">https://reviews.llvm.org/<wbr>D43001</a>: switches to uleb128</span><br></div><div><br></div><div>-Ryan</div><div><br></div></div></div></div>