<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 11, 2020 at 4:09 PM Serge Pavlov <<a href="mailto:sepavloff@gmail.com">sepavloff@gmail.com</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 dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">   In this environment, 5 bits are quite a lot for FP flexibility, and I think the complexity of reducing this is small and isolated (rounding + exceptions together fit in 4 bits)  </blockquote><div><br></div><div>Rounding (5 standard variants) and exception (3 variants) already do not fit 4 bits.</div></div></blockquote><div>Sure they do: there are 5 x 3 = 15 combinations, which can be encoded in 4 bits. This would be a short-term fix only as it sounds like the number of bits will grow further.</div><div><br></div><div>Fortunately Melanie is working on a better/more scalable solution of putting the FPOptions in trailing objects of the nodes where they're actually used (e.g. most BinaryOperators aren't floating point at all). Then FPOptions can grow as many bits as needed without affecting many nodes.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div> And there is also precision, denormals treatment, exception mask, they are usually represented in hardware register and make up a floating point environment. Instead of putting all these bits to different places it would be better to collect them in one place, it could make operations simpler and faster. </div><div dir="ltr"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">  both rounding and exceptions seem essentially unused at present. </blockquote><div dir="ltr"> </div><div>Implementation of advanced floating point support is in progress, they will be used more and more.</div></div></blockquote><div>That sounds great! Sorry, I didn't mean to imply at all they were useless, just as an uninformed person I can't reason about them by looking at uses as they don't exist yet.</div><div><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 dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">  (Note that the cost here is not 7 bits per node but 63 - once the bits available in Stmt are full the stmt subclass needs to gain a member, and its alignment is 8 *bytes*)  </blockquote><div><br></div><div>As objects are allocated aligned, if size in not multiple of the alignment, some part of memory is spent uselessly anyway.</div></div></blockquote><div>Right. My point is that today, BinaryOperator uses exactly all its bits. Tomorrow, we need to add a HasError bit to Expr. So tomorrow, the marginal cost of the last bit (or savings from squeezing the last bit) is 64 bits, at least for BinOp. This is certainly a short-term view, the long-term option is probably moving FPOptions out of the bitfields in some way.</div><div><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 dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Use cases vary, and many tools deal with large ASTs but don't use LLVM codegen at all.</blockquote><div><br></div><div>That is true, but developer tools are usually less restricted in computing resources. Besides hard economy in memory often means some loss of performance.</div></div></blockquote><div>I'm not sure this is productive - if you don't think bit-packing is a sensible tradeoff, then I think to some extent it's on you to change it, rather than eat all the bits and walk away! I do happen to work on developer tools with memory constraints, but that's not the hat I'm wearing today - we're just stuck between the established design (which aims for compactness) and existing uneconomical use of it.</div><div><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 dir="ltr"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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>As you say, I don't think this saves anything, unless we can stop storing the pointer in BinaryOperator.</blockquote></div></blockquote><div><br></div><div>We could keep pointer to shared properties, thus saving memory. Moreover such facility could enable some techniques using this shared property as a marker. For example, all floating point operations obtained from the same region where `pragma STDC FENV_ACCESS` is in act could point to the same FPOption object, thus we could distinguish between different regions. It would save memory consumption without sacrifice of maintainability and speed.</div></div></div></blockquote><div>Yeah, just concretely needing an 8-byte pointer to a 1-byte shared structure doesn't save memory, so some larger refactoring/design would be needed. TrailingObjects is simpler.</div><div><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 dir="ltr"><div dir="ltr"><div><br></div><div><div dir="ltr">Thanks,<br>--Serge<br></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 11, 2020 at 8:01 PM Sam McCall <<a href="mailto:sammccall@google.com" target="_blank">sammccall@google.com</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 dir="ltr"><div dir="ltr">On Wed, Mar 11, 2020 at 12:42 PM Serge Pavlov <<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><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></blockquote><div>I agree, and would be happy if we had a design without these scarce bits, but AFAICS we don't. In this environment, 5 bits are quite a lot for FP flexibility, and I think the complexity of reducing this is small and isolated (rounding + exceptions together fit in 4 bits). But I ask about the domain because maybe there's a simple but smaller model I can't really infer this myself, both rounding and exceptions seem essentially unused at present.</div><div>(Note that the cost here is not 7 bits per node but 63 - once the bits available in Stmt are full the stmt subclass needs to gain a member, and its alignment is 8 *bytes*)</div><div><br></div><div>Use cases vary, and many tools deal with large ASTs but don't use LLVM codegen at all.</div><div><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 dir="ltr"><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></div></blockquote><div>As you say, I don't think this saves anything, unless we can stop storing the pointer in BinaryOperator.</div><div><br></div><div>Cheers, Sam</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><br><div><br clear="all"><div><div dir="ltr">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" target="_blank">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>
</blockquote></div></div>
</blockquote></div></div>
</blockquote></div></div>