<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Nov 26, 2016 at 11:49 PM, Noel Grandin <span dir="ltr"><<a href="mailto:noelgrandin@gmail.com" target="_blank">noelgrandin@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">Once you're using power of two values I would expect the usual bit manipulation peephole optimizer to do the right thing, that's a pretty standard and simple optimization. <br></blockquote><div><br></div><div>A peephole optimization can't get this, because such an optimization doesn't have the information to figure out that only the values being tested represent a closed universe identified by that bit.</div><div><br></div><div>You can see that the compiler doesn't fold this into a single bitwise test: <a href="https://godbolt.org/g/ZIZrrE">https://godbolt.org/g/ZIZrrE</a></div><div><br></div><div>The thing being done in this patch that the compiler fundamentally can't be expected to do is to ignore the lower bits and only care about an individual predicate bit; in order to do that you have to have some a priori knowledge of how the bits are related. This patch can only do so because the RelExpr values are chosen specifically so that this holds.</div><div><br></div><div>-- Sean Silva</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 class="gmail-HOEnZb"><div class="gmail-h5"><div class="gmail_quote"><div dir="ltr">On Sun, 27 Nov 2016 at 09:47, Sean Silva 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">silvas added a comment.<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
In <a href="https://reviews.llvm.org/D27145#606147" rel="noreferrer" class="gmail-m_-4303913023396509242gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D27145#606147</a>, @grandinj wrote:<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
> Do the use-sites need to be updated? Won't the compiler already simplify such expressions (just concerned that the use-sites may be come less legible)<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
The use sites are updated by this patch (search for the expressions that look like `& RPRED_FOO`).<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
The expressions that are being replaced are of the form `x == 7 || x == 9 || x == 35 || x == 12 || ...`. The best the compiler could do is infer range information (based on the C language rules for enums) and make a lookup table, but I'm not aware of any compiler that will currently do that for this kind of expression.<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
This patch is a type of data representation optimization that a compiler can't be expected to do, even with whole-program analysis.<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
The use sites might become less legible, but ideally we can hide each of these operations behind a helper with a useful name, such as refersToGotEntry, needsPlt, etc. so I don't expect it to matter much. I'm not as much of an expert on relocations to come up with good names for each of them, so some of them I have just left as a raw bit test. Hopefully Rafael and others can help identify good names for these (and for the `RPRED_*` enum values).<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
> Also, for future source code readers, you may want a comment at the definition site of the enum that it is a regular (exclusive) enum and not some kind of flags/bit-field type thing.<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
Once we settle down on the design, I'll need to do another pass through the code and add some strategic comments.<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
<a href="https://reviews.llvm.org/D27145" rel="noreferrer" class="gmail-m_-4303913023396509242gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D27145</a><br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
<br class="gmail-m_-4303913023396509242gmail_msg">
</blockquote></div>
</div></div></blockquote></div><br></div></div>