RFR: 8308869: C2: use profile data in subtype checks when profile has more than one class [v6]

[Roland Westrelin]

On Thu, 22 Jun 2023 17:59:00 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> > It felt easier in terms of memory management. If we have some extra data embedded in the SubTypeCheck node, is it a pointer or the full data structure?
> 
> `ciCallProfile` has fixed size and is passed by value. Embedding the whole structure inside `SubTypeCheck` doesn't look problematic. It refers to CI entities which should be kept alive for the duration of the compilation.

Some `SubTypeCheck` nodes have no profile data associated with them. It doesn't seem right that all of them have to carry around `ciCallProfile`. That's maybe a small overhead at this point but we could profile more than 2 receivers in the future so that overhead could change.

> I'd prefer to see `SubTypeCheck` to have control input which is explicitly relaxed to accommodate commoning.

I don't how that would simplify things. `SubTypeCheck` is a `Cmp` node. As a consequence some optimizations that apply to `Cmp` nodes apply to it. `Cmp` nodes have no control. Setting the control to one of them will break things and require extra logic to accomodate the extra control (split if will likely break for instance). 

Beyond that, we'll need logic to find  `SubTypeCheck` nodes that be commoned which I expect would be in the same locations I already extra code. I don't understand what the control edge would make simpler.

> Overall, I'm fine with late expansion of profile-guided type checks for now, but embedding profile data info SubTypeCheck should significantly simplify the patch without compromising the benefits.

At this point, I fail to see how. I'm not saying the solution I propose is great but it seems to me the problem it solves need to be solved in any case.

> Also, enhancing profiling support separately may be a viable tradeoff as well. Inaccuracies in code shape classification don't look like a critical issue when the guards are introduced during macro expansion.

Is that out of concern that getting the code  done on all platforms will be too complicated?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14375#issuecomment-1604428685