RFR (M): 8143925: Enhancing CounterMode.crypt() for AES
vitalyd at gmail.com
Fri Jan 8 12:38:18 UTC 2016
Roland, thanks for elaborating; a few comments inline ...
On Friday, January 8, 2016, Roland Westrelin <roland.westrelin at oracle.com>
> > Does checkIndex match on it? If so, is there a reason to proceed with
> intrinsifying checkIndex?
> I expect it would in some cases but not all.
> The pattern matching needs profiling to tell the branches that would
> trigger an exception are never taken, then only can the tests be folded and
> made to look like a range check for the next optimization passes. Profiling
> can be polluted or not mature enough. The intrinsic assumes the exception
> path are never taken and doesn't rely on profiling (then if the check does
> fail we recompile and don't use the intrinsic). We take the use of the
> checkIndex API as a hint that the checks are not expected to fail.
As a general comment, would it make sense to assume exceptional paths are
not taken in most Java code? That is, for code optimization purposes it's
probably a reasonable assumption. It seems like having an exceptional path
is already a hint that it's not expected to fail; most Java devs know not
to use exceptions for expected control flow.
> Also, for the pattern matching to work, in i <0 || i >= length the
> compiler needs to know enough on the range of values taken by length to be
> able to fold. Again we see checkIndex as an indication that length is
> positive and if we can't prove it we compile a predicate to verify that it
> is so we can safely use an unsigned compare. Again we take the use of
> checkIndex as a hint that the length argument is positive.
Could bytecode shape just like checkIndex be treated as same hint? Are
there cases where something looks like checkIndex but really isn't?
> > On Wednesday, January 6, 2016, Vladimir Kozlov <
> > Note, we already have range check pattern matching code in C2 (thanks to
> > https://bugs.openjdk.java.net/browse/JDK-8137168
> > Vladimir
> > On 1/6/16 12:39 PM, Vitaly Davidovich wrote:
> > I don't think there's a need to write out 20 different ways to do a
> > range check -- I think nobody would expect all 20 to be covered by the
> > optimizer. Some of those variations may not map cleanly to
> > Object::checkIndex either, nor is there any guarantee that people will
> > update all their existing range checks (or even know about) to use
> > Object::checkIndex -- some code will be left unoptimized no matter what.
> > But my point is the same as Andrew's, I think; instead of making
> > checkIndex an intrinsic, simply add a pattern match against that exact
> > bytecode shape (perhaps with basic canonicalization) and then still
> > encourage people to use Object::checkIndex. This is better than
> > intrinsic (modulo profile pollution) since any other code that happens
> > to use same pattern will match as well, and not require an update to use
> > checkIndex. Then, if someone comes to this list with an unoptimized
> > example with a different bytecode shape and has a convincing argument
> > that the code shape is "common", you guys can consider pattern matching
> > that as well.
> > On Wed, Jan 6, 2016 at 2:50 PM, John Rose <john.r.rose at oracle.com
> > > On Jan 6, 2016, at 9:56 AM, Vitaly Davidovich <vitalyd at gmail.com
> > >
> > > better canonicalization
> > That's our first and most important tactic. (Actually inlining is.)
> > But the various idioms for checkIndex do not canonicalize easily. In
> > this case the correct trade-off is not to invest more time and
> > research and code into stronger canonicalization.
> > We do have canonicalization of if-expressions. It's just that in
> > this case strengthening it to cover range checks reliably is harder
> > than the reasonable alternative.
> > – John
> > PS. I am tempted to write out a list of 20 different ways to code a
> > range check but will leave that as a exercise.
> > --
> > Sent from my phone
Sent from my phone
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-compiler-dev