On Thu, 21 Jan 2021 16:54:36 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>>> Unfortunately it is still problematic because you have replaced the >>> intrinsic check (that performed by `Object.checksIndex`, or more >>> specifically >>> [here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/util/Preconditions.java#L261)). >>> >>> Further, you have replaced the bounds check options, in place for >>> experimentation. We are not ready yet to collapse our options, further >>> analysis is required as bounds checks can be very sensitive in C2. >>> >>> I would be open to you adding a third case, so that you can analyze the >>> performance without perturbing the default behavior. I suspect the correct >>> fix is to make intrinsic `Objects.checkFromIndexSize` in a similar manner >>> to `Object.checksIndex`. >> >> Hi @PaulSandoz , >> >> Thanks for your kind review and comments. >> >> To be honest, I didn't get your first point. >> As for the example above, the intrinsified Objects.checkIndex seems to >> generate more code than inlined if-statements. >> So I'm confused why it's a problem to replace the intrinsic check. >> Did you mean an intrinsic is always (or for most cases) better than >> non-intrinc or inlined if-statements? >> And why? >> >> Could you please make it more clearer to us? >> Thanks. > > The intrinsic enables C2 to more reliably elide bounds checks. I don't know > the exact details but at a high-level it transforms signed values into > unsigned values (and therefore the signed comparisons become unsigned > comparisons), which helps C2 remove checks when there is a dominating check > of, for example, an upper loop bound. > > You say "the intrinsified Objects.checkIndex seems to generate more code than > inlined if-statements". Can you present some examples of Java code and the > corresponding C2 generated assembler where this happens? Hi @PaulSandoz , I will show you the assembly code next week since it is already Friday night now. Thanks. ------------- PR: https://git.openjdk.java.net/jdk/pull/2127
