RFR(L): 8185556: [MVT] C2 compiler support for non-flattened value type fields
Roland Westrelin
rwestrel at redhat.com
Tue Sep 26 09:20:39 UTC 2017
> http://cr.openjdk.java.net/~thartmann/8185556/webrev.00/
sharedRuntime_x86_64.cpp:
486 case T_VALUETYPE:
Why is that needed? Shouldn't we only see T_VALUETYPEPTR here?
I'm also confused by that comment:
556 // TODO change this when we use T_VALUETYPEPTR
in signature.cpp
callGenerator.cpp:
442 Node* ctl = map->control();
443 arg = ValueTypeNode::make(gvn, ctl, map->memory(), arg);
444 map->set_control(ctl);
Why the set_control()?
That comment confuses me, too:
2117 // TODO enable when using T_VALUETYPEPTR
in escape.cpp
In graphKit.cpp:
3395 Node* GraphKit::new_instance(Node* klass_node,
3396 Node* extra_slow_test,
3397 Node* *return_size_val,
3398 bool deoptimize_on_exception,
3399 ValueTypeBaseNode* value_node) {
when can we pass a ValueTypePtrNode here?
Using GraphKit with an igvn should only be safe if _delay_transform is
true, right? Should there be an assert for _delay_transform in the
GraphKit constructor?
Also, usually after the graph is constructed with GraphKit, there is a
pass of PhaseRemoveUseless to disconnect useless nodes. Your new use of
GraphKit during igvn is not followed by PhaseRemoveUseless. I wonder if
it's a problem and if some untransformed node could stay connected to
graph.
In phaseX.cpp, PhaseIterGVN::transform(), why the change?
I find the overloaded ValueTypeNode::make() calls quite confusing after
all. I think renaming them to something that's more descriptive of what
they do would be better.
Roland.
More information about the valhalla-dev
mailing list