RFR(M) 7199175: JSR 292: C1 needs patching when invokedynamic/invokehandle call site is not linked

[Roland Westrelin]

Thanks for the review.

> src/cpu/x86/vm/c1_CodeStubs_x86.cpp:
> 
> Don't we also need something like this on x86 as on SPARC?
> 
> +  } else if (_id == load_mirror_id || _id == load_appendix_id) {

As I understand it, it's required on sparc because when the instructions in the stub are patched they need to update the oop in the oop table of the nmethod. On x86, the oop is inlined in the oop, it's not in oop table.

> src/share/vm/c1/c1_GraphBuilder.cpp:
> 
> +  Values* args = state()->pop_arguments(target->arg_size_no_receiver() + extra_arg);
> 
> Could you rename extra_arg to maybe unlinked_appendix_arg or patching_appendix_arg?

Ok.

> 
> src/share/vm/c1/c1_LIRAssembler.cpp:
> 
> +PatchingStub::PatchID LIR_Assembler::jobject2reg_patching_id(CodeEmitInfo* info) {
> 
> I don't think that's a good method name.  It does not produce any to-register moves; it only returns an id.

renamed to LIR_Assembler::patching_id()

> src/share/vm/c1/c1_Runtime1.cpp:
> 
> +      default: Unimplemented();
> 
> A fatal would be better; there is nothing to be implemented here.  In general, we should not add new ShouldNotReachHere or Unimplemented calls.  Adding fatals with a useful message is the preferred way.

Ok.

> src/share/vm/c1/c1_globals.hpp:
> 
> +  develop(bool, C1PatchInvokeDynamic, true,                                 \
> 
> Do we really want to turn if off?  During development I'm sure it was helpful but after that?

That's something John asked for.

> 
> src/share/vm/ci/ciEnv.cpp:
> 
> +ciInstance* ciEnv::unloaded_ciinstance() {
> +  GUARDED_VM_ENTRY(return _factory->get_unloaded_object_constant();)
> +}
> 
> Can we rename that method to unloaded_object_constant?

I named it that way to be consistent with unloaded_ciinstance_klass, unloaded_ciobjarrayklass, unloaded_cisymbol.

> src/share/vm/c1/c1_LIR.hpp:
> 
> -      is_invokedynamic()  // An invokedynamic is always a MethodHandle call site.
> 
> Why did you remove this?  Did it cause problems?

It did:
Internal Error at nmethod.cpp:1909, pid=6899, tid=7
assert(has_method_handle_invokes() == (_deoptimize_mh_offset != -1)) failed: must have deopt mh handler

The is_method_handle_invoke() in c1_LIR.hpp is not consistent with what c2 does.

Roland.