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