RFR (XL): 6934604: enable parts of EliminateAutoBox by default

[Christian Thalinger]

On Apr 30, 2013, at 7:46 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> http://cr.openjdk.java.net/~kvn/6934604.v2/webrev/

src/share/vm/ci/ciInstanceKlass.cpp:

+ bool ciInstanceKlass::is_boxed_value_offset(int offset) const {

I wanted to ask why don't you check is_loaded() but my question now is:  why don't you call is_box_klass() from is_boxed_value_offset()?

src/share/vm/opto/bytecodeInfo.cpp:

+ static bool is_unboxing(ciMethod* callee_method, Compile* C) {

Can we also name it is_unboxing_method?

src/share/vm/opto/callGenerator.cpp:

!   for (uint i1 = 0; i1 < nargs; i1++) {
+     map->set_req(jvms->argoff() + i1, call->in(TypeFunc::Parms + i1));
    }

Use map->set_argument instead of map->set_req.  We should change this wherever we touch code.

src/share/vm/opto/callnode.cpp:

+ //---------------------------clone_deep----------------------------------------
+ void JVMState::set_map_deep(SafePointNode* map) {

Name wrong in comment.  Could you use Doxygen format here?

src/share/vm/opto/callnode.hpp:

    int            arg_size() const { return monoff() - argoff(); }

Why did you remove this one?

+   bool is_autoboxing() const {
+     return is_macro() && (method() != NULL) && method()->is_boxing_method();
+   }

We should either use "autoboxing" or "boxing".  Mixing them is confusing.

src/share/vm/opto/compile.cpp:

+                   _eliminate_autobox(eliminate_boxing),

Same here.

src/share/vm/opto/compile.hpp:

+   bool has_late_inline() const { return ((_late_inlines.length() + _string_late_inlines.length()) > 0); }

This method is not used.  If you want to keep it, don't you also have to add _boxing_late_inlines.length()?

src/share/vm/opto/escape.cpp:

!   for( int i=0; i < cnt; i++ ) {
!   for (int i=0; i < cnt; i++) {

When you're already changing the parens could also change spacing around "="?

src/share/vm/opto/memnode.cpp:

+         switch (constant.basic_type()) {
+           case T_BOOLEAN:  return TypeInt::make(constant.as_boolean());
+           case T_INT:      return TypeInt::make(constant.as_int());
+           case T_CHAR:     return TypeInt::make(constant.as_char());
+           case T_BYTE:     return TypeInt::make(constant.as_byte());
+           case T_SHORT:    return TypeInt::make(constant.as_short());
+           case T_FLOAT:    return TypeF::make(constant.as_float());
+           case T_DOUBLE:   return TypeD::make(constant.as_double());
+           case T_LONG:     return TypeLong::make(constant.as_long());
+           default:
+             ShouldNotReachHere();
+         }

This might be useful in a method for later reuse.

src/share/vm/opto/node.cpp:

+ //----------------------------replace_edge_in_range-----------------------------
+ int Node::replace_edges_in_range(Node* old, Node* neww, int start, int end) {

Name wrong in comment.  Doxygen? ;-)

Why are there no tests for Character and Short?

-- Chris

> 
> Resurrected autobox elimination code and enabled it by default.
> Added new experimental C2 flag in preparation for 8012974.
> 
> After static fields were moved to JavaMirror the existing autobox elimination code (LoadNode::eliminate_autobox()) stopped working because graph was changed. Fixed it for loading from Boxing cache array, moved spit through Phi code from eliminate_autobox() to LoadNode::split_through_phi(). Also use incremental inlining to delay inlining of valueOf() methods to get value from argument. Mark such method as Macro nodes and treat them the same way as Allocations. Added value load from constant boxing objects. Eliminated MemBarRelease for non-escaping allocations (such membar generated when final fields are initialized in constructor).
> 
> Did not get any significant performance improvements because very few places have opportunity for current optimization (when all objects do not escape).
> 
> Additional changes:
> Added ability to recompile method without EliminateAutoBox if compilation with it failed.
> 
> Added assert live_nodes < MaxNodeLimit into Node costructor.
> 
> Added timer for incremental inlining.
> 
> Fixed TypeAryPtr::narrow_size_type() to return correct type for arr[MAXINT] (currently it returns incorrect arr[0]).
> 
> Moved the check in assert to condition in loopPredicate.cpp because I hit the problem during CTW testing.
> 
> Fixed tiered compilation process dependence on the order of -Xcomp -XX:+TieredCompilation flags.
> 
> Added regression tests.
> 
> Tested with jprt, ctw, nashorn.
> 
> Thanks,
> Vladimir
>