<div dir="ltr">It is a matter of taste, but for me it looks strange to develop complex and error-prone system to save 7 bits at expense of readability and maintainability. My observations show that clang AST consumes much less memory than llvm objects.<div><br></div><div>FPOption could be shared between user using something like shared_ptr, but it means expenses of 64 bit for a pointer. Don't know if it makes sense...<br><div><br><div><br clear="all"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">Thanks,<br>--Serge<br></div></div><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 11, 2020 at 6:30 PM Sam McCall via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.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">sammccall added a comment.<br>
<br>
This patch increased the used size of BinaryOperator by 5 bits.<br>
Those bits were all padding, but now BinaryOperatorBitfields is completely full at 32 bits and we can't add any new bits to Stmt without increasing BinaryOperator by 8 bytes. (See D75443 <<a href="https://reviews.llvm.org/D75443" rel="noreferrer" target="_blank">https://reviews.llvm.org/D75443</a>> and D54526 <<a href="https://reviews.llvm.org/D54526" rel="noreferrer" target="_blank">https://reviews.llvm.org/D54526</a>> for the optimization this would revert).<br>
<br>
To squeeze in the new bit I'm planning to suggest squeezing getInt() to 7 bits (it encodes 3x2x5x3 = 90 states, so this is possible) but I'm not really familiar with this domain - if many of the 90 states are not possible it'd probably be useful to have some more bits back :-)<br>
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D65994/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D65994/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D65994" rel="noreferrer" target="_blank">https://reviews.llvm.org/D65994</a><br>
<br>
<br>
<br>
</blockquote></div>