RFR(M): 8031754: Type speculation should favor profile data from outermost inlined method (original) (raw)

Christian Thalinger christian.thalinger at oracle.com
Tue Feb 18 17:25:49 PST 2014

Sorry it took me so long. This looks good.

On Feb 13, 2014, at 2:01 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:

Can I have another review for this? last webrev: http://cr.openjdk.java.net/~roland/8031754/webrev.01/ Roland.

I think it is good.

Can you add "Use" to flag name: UseInlineDepthForSpeculativeTypes? It is more clear this way. Sure. Thanks for the review, Vladimir. Roland.

Thanks, Vladimir On 2/10/14 3:40 PM, Roland Westrelin wrote: Here is a new webrev: http://cr.openjdk.java.net/~roland/8031754/webrev.01/ As suggested by Vladimir, I use a positive value for InlineDepthBottom and MAX for the meet of two depths. I also added asserts in removespeculative. Whether this is used or not is under a command line option. There was also a missing argument to a call to TypeAryPtr::make() that went unnoticed because of default arguments. Roland.

On Feb 6, 2014, at 7:29 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote: On 2/6/14 10:13 AM, Roland Westrelin wrote: Hi Vladimir,

In general I am comfortable to have inlinedepth as additional type's attribute. But only for speculative types when it make sense.

So why you keep inlinedepth in removespeculative()? Because it’s never used in practice with non speculative types (it should always be InlineDepthBottom for a non speculative type). The only place where it changes the behavior of compilation is in GraphKit::recordprofileforspeculation() where it’s used only for speculative types. Anyway, I can reset it to InlineDepthBottom in removespeculative() if you like. Yes, please, set to InlineDepthBottom. It will be consistent with types without speculative types. Actually, that’s not possible. Because then, that code: 2589 if (resoopptr->removespeculative() == resoopptr->speculative()) { 2590 return resoopptr->removespeculative(); in TypeOopPtr::xmeet() doesn’t trigger if resoopptr has an inline depth of InlineDepthTop. removespeculative() transforms it to InlineDepthBottom. And the type system is no longer symmetric. Do you know why it is top? The default value is InlineDepthBottom, how it is converted to top? It’s top because it’s a join and for a join the dual of the inlinedepth is used. Sorry, I misunderstood the code in removespeculative(). I thought you replacing type's inlinedepth with one from speculative type but you simple preserving original inlinedepth of this type. The original code you had is good. Thanks, Vladimir Roland.

Instead shouldn’t I assert in removespeculative() that if speculative is non NULL then inlinedepth is InlineDepthTop or InlineDepthBottom? To have assert is good but we need to understand why in such case inlinedepth could be top. Thanks, Vladimir Roland.

More information about the hotspot-compiler-dev mailing list