RFR(L): 8185265 [MVT] improve performance of return of value types with new calling convention

Roland Westrelin rwestrel at redhat.com
Wed Aug 9 15:42:25 UTC 2017


Thanks for looking at this, Tobias.

> sharedRuntime_x86_64.cpp
> - What is MacroAssembler* masm used for?

__ is masm-> so we need a macro assembler to generate the code.

> macro.cpp
> - There is a UseTLAB assert but I don't see where this is checked when adding the macro node. Also, we could fall back 
> to the runtime call if UseTLAB is disabled (like we do in the interpreter), right?

We could have a fall back for UseTLAB but is it worth the trouble?
There's no use case other than maybe testing that uses -UseTLAB, right?
I could disable the return convention if UseTLAB is false so at least we
don't crash. What do you think?

> ValueTypeTestBench.java:
> - Why did you change COMP_LEVEL_ANY to -2?

// Enumeration to distinguish tiers of compilation
enum CompLevel {
  CompLevel_any               = -2,

Shouldn't the VM definition and COMP_LEVEL_ANY be in sync? FWIW, I
noticed some methods for which compilation was disabled were actually
compiled.

> - Should we enable StressValueTypeReturnedAsFields in our tests?

I wondered about that but then we always take the path that reallocates
a new value type and the other one is not tested. 

> I've run this through JPRT and found the following crash during class unloading with some internal test on Solaris and 
> Windows:

Is it because just allocated metadata is not zeroed (patch below)? Not
sure if it is or not but I don't see any code that clears it.

Roland.

diff --git a/src/share/vm/oops/valueKlass.cpp b/src/share/vm/oops/valueKlass.cpp
--- a/src/share/vm/oops/valueKlass.cpp
+++ b/src/share/vm/oops/valueKlass.cpp
@@ -344,6 +344,11 @@
 }
 
 void ValueKlass::initialize_calling_convention() {
+  *((Array<SigEntry>**)adr_extended_sig()) = NULL;
+  *((Array<VMRegPair>**)adr_return_regs()) = NULL;
+  *((address*)adr_pack_handler()) = NULL;
+  *((address*)adr_unpack_handler()) = NULL;
+
   if (ValueTypeReturnedAsFields || ValueTypePassFieldsAsArgs) {
     Thread* THREAD = Thread::current();
     assert(!HAS_PENDING_EXCEPTION, "should have no exception");


More information about the valhalla-dev mailing list