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

[Tobias Hartmann]

Hi Roland,

On 09.08.2017 21:12, Roland Westrelin wrote:
 > __ is masm-> so we need a macro assembler to generate the code.

Right.

 >> 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?

Yes, it's probably not worth the trouble. We shouldn't crash though.

 > I could disable the return convention if UseTLAB is false so at least we
 > don't crash. What do you think?

I'm fine with that but then you should replace the UseTLAB check in templateInterpreterGenerator_x86.cpp by an assert.

 >> 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?

Right, this was changed from -1 to -2 by the AOT integration:
http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/777aaa19c4b1#l128.7

That's why I've initially set it to -1.

 > FWIW, I
 > noticed some methods for which compilation was disabled were actually
 > compiled.

Yes, I've noticed that too but never found the time to investigate. So you're saying this is now fixed by setting
CompLevel_any to -2?

 >> - 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.

Yes and adding yet another @run statement increases test execution time even further.

Let's leave it for now and run it manually from time to time.

 >> 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.

Unfortunately, the problem persists.

Best regards,
Tobias