<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Sep 12, 2014, at 7:37 AM, Paul Sandoz <<a href="mailto:paul.sandoz@oracle.com">paul.sandoz@oracle.com</a>> wrote:<br><div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 10, 2014, at 10:20 PM, John Rose <<a href="mailto:john.r.rose@oracle.com">john.r.rose@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>On Sep 10, 2014, at 3:47 AM, Paul Sandoz <<a href="mailto:paul.sandoz@oracle.com">paul.sandoz@oracle.com</a>> wrote:</div><div><br class="Apple-interchange-newline"><blockquote type="cite">The patch for JDK-8003585 makes no difference.</blockquote><div><br></div>My first thought, is, why not?  Isn't that just a bug?<br><div><br></div></div></div></blockquote><div><br></div>I would presume so, it seems a general pattern, but I dunno enough about the C2 graph representation. Should i start by rendering graphs and eye-balling them?</div></div></blockquote><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>Yes, that's about the best you can do for this kind of thing.</div><div>Folding cmp1 && cmp2 into cmp3 is no picnic.  It involves editing several control edges at once.</div><div>Happily, there is already code that attempts to do this, so you (or some other intrepid) could start there.</div><div>The location is IfNode::fold_compares.</div><div>It's (maybe, sort of) a bug that that logic doesn't already create CmpU nodes from (i >=0 && i<a.length).</div><div><br></div></div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>For virtualizing arrays, we need either an explicit range-check intrinsic, or robust canonicalization of the standard idioms into CmpU (which is what we use internally).</div><div><br></div></div></blockquote><div><br></div>Ok.</div></div></blockquote><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div><div>That's turning cmp1 into cmp2, which is much more "local".  Should be doable in CmpINode::Ideal, by idiom recognition.</div><div><br></div></div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Actually, we now have Integer.compareUnsigned!  </div></blockquote><div><br></div>Ah! now it becomes obvious :-)</div><div><br></div><div><div>        if (Integer.compareUnsigned(index, a.length) >= 0)</div><div>            throw new ArrayIndexOutOfBoundsException();</div><div><br></div><div>That it should in principle, vis some intrinsification, reduce to something like a cmp/jae or test/jbe depending on what comes before it.</div></div></div></blockquote><div><br></div><div>Yes.  I don't even think it needs intrinsifying, since its expression is simple enough to recognize in CmpINode::Ideal.</div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br></div></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div>If we agree that is an intrinsic for range checks, the JIT will have a better optimization target to aim at.   Ultimately, we need an intrinsic (perhaps more intentional than Integer.compareUnsigned) that will encourage the JIT to treat it like a range check, doing iteration range splitting, predication, and all the rest.</div><br><blockquote type="cite">(Note: in general we cannot assume that "int index = i & (a.length - 1)" always occurs before the bounds checks, otherwise i would have explicitly written "if (a.length == 0) throw ...")</blockquote><div><br></div><div>Right.  You want to factor the range check into a bit of code that doesn't know how it's going to be used, but then gets the same perks as normal array code, including the a.length==0 stuff, and the loop opts I mentioned.</div><br><blockquote type="cite">Ideally similar code as shown for an aaload should be generated. Any suggestions/ideas on how to make that happen?<br></blockquote><div><br></div></div><div>First, agree on a range check intrinsic.  Then, treat optimization equity failures as JIT bugs.</div></div></blockquote><div><br></div>Many thanks, seems like an excellent starting point is to intrinsify Integer.compareUnsigned and see what that yields.</div></div></blockquote><br></div><div>I think that will get you a step forward.</div><div><br></div><div>The next step towards bringing unsafe ops to parity with array ops is finding out why the logic</div><div>for -XX:+RangeCheckElimination doesn't kick in (if that indeed is the case).</div><div>That would start with an examination of IdealLoopTree::policy_range_check.</div><div><br></div><div> John</div><br></body></html>