RFR: 8289943: Simplify some object allocation merges [v6]

Cesar Soares duke at openjdk.org
Fri Sep 2 20:22:49 UTC 2022


On Fri, 2 Sep 2022 00:34:49 GMT, Cesar Soares <duke at openjdk.org> wrote:

>> Allocations in `testPollutedPolymorphic()` are removed because both classes have the same `Shape` class which have all fields. Would be interesting if `l` field is declared only in both subclasses.
>
> @vnkozlov - Thank you for clarifying that. I've been playing with lifting the restriction and I actually found a corner case:
> 
> 
> public static Class test(boolean c1, boolean c2, boolean c3, int x, int y, int w, int z) {
>     Animal s = new Dog(x, y, z);
> 
>     if (c1) {
>         s = new Cat("Fisker");
>     }
> 
>     Unloaded u = new Unloaded(); // assumes this is converted to a uncommon_trap(unloaded, reinterpret)
> 
>     return s.getClass();
> }
> 
> 
> It seems that when merging allocations of different subtypes I'll need to add a special `Phi` node merging the `Klass` of the input allocations and assign the output of that `Phi` to `SafepointScalarReplacedNode`. If I don't do that, the method above will return Animal.class instead of `Dog.class` or `Cat.class`. I'm wondering if I'll actually have to do the same for the Header/Mark word of the input allocations.

> @JohnTortugo Yes, you would need to construct Phi when you replace RAM with SafePointScalarObjectNode. Hmm, may be you would need to construct Phi in other cases too (getClass intrinsic).

Yes. I'll take a look into `getClass` intrinsic. I thought that just adding input to `SafePointScalarObjectNode`+Safepoint with a Phi of the input allocations Klass fields would be enough for the code to correctly access fields/methods of the input objects. However, it looks like the Klass of the object rematerialized from SafePointScalarObjectNode is [the type of `SafePointScalarObjectNode`](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/output.cpp#L859), not the Klass field input edge set in the Safepoint.

> Add cases when class is loaded from argument for allocation: Unsafe.allocateInstance() and Object.clone() to test class Load instead of simple constant on some paths of such Phi.

I'll do that. Thanks for the tip.

> Why you need Phi for mark word? For identity check (hashCode/identityHashCode intrinsics)?

Yes, I was wondering about a case where I'd need any of the information present in the Mark word. The code in the PR is able to solve merges where only some of the merged allocations are removed. I believe I'll need, at least, to preserve the Mark word of the allocations not removed.

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

PR: https://git.openjdk.org/jdk/pull/9073


More information about the hotspot-compiler-dev mailing list