RFR(M): 8182471: [MVT] Eliminate redundant value type allocations

Tobias Hartmann tobias.hartmann at oracle.com
Wed Jun 21 15:01:09 UTC 2017


Hi Roland,

thanks for reviewing this!

On 21.06.2017 15:45, Roland Westrelin wrote:
>> I also added code to recognize cases when we copy a value type by
>> "manually" loading its field values from memory. In this case, we can
>> re-use the oop (see test87-90) and avoid allocations.
> 
> Is this what this code does:
> 
>  462   if (!is_allocated(phase)) {
>  463     // Check if this value type is loaded from memory
>  464     Node* base = is_loaded(phase, type()->is_valuetype());
>  465     if (base != NULL) {
>  466       // Save the oop
>  467       set_oop(base);
>  468       assert(is_allocated(phase), "should now be allocated");
>  469     }
>  470   }

Yes, it checks if the input field values are field loads from the same base oop (with correct type and offset). If so, this oop is re-used and the value type does not need to be allocated.

> I suppose val77 in the test case should be renamed.

Right, I renamed it to 'staticVal3' and updated the webrev in-place.

> When we discussed this privately, you said that before allocating we
> have a runtime test to verify the value was not yet allocated and that
> that runtime test got in the way of your optimization. Did you find a
> way around that problem?

Unfortunately, I don't remember the exact issue but I think the problem was not the runtime oop check but the dominator check not being able to determine domination of another allocation (that's what I meant by "the GVN version of is_dominator() and also MemNode::all_controls_dominate() are not strong/good enough"). The current version works well in all the cases I've tested.

Thanks,
Tobias


More information about the valhalla-dev mailing list