RFR (M): 8182367: [MVT] Byte-code verifier support for minimal value types

Karen Kinnear karen.kinnear at oracle.com
Wed Sep 6 16:42:15 UTC 2017


Looks good. Many thanks.

Very happy with the creative classFileParser containment of the temporary derived value class naming
and with the clean verificationType extension.

Couple of questions/comments:
1. verificationType.hpp
line 180
Did I read this incorrectly? There is a Symbol alignment assumption that leaves 0x3, i.e. two bits as 0
Did you want to change that alignment check to be 0x7 for alignment so there is room for the ValueType bit?
And change the hardcoded 0x3 to TypeMask?

2. verificationType.hpp
line 203
Could you possibly explain to me the need for the && !is_valuetype_check()? Or is that potentially an assertion?

3. verificationType.hpp
  Is it worth adding an assertion that neither is_null() nor from.is_null() ? Or did you already throw an exception earlier for that

4. We discussed temporarily using 53.1 for value type versioning until JDK10 moves to 54

5. verifier.cpp
Thank you for the vbytecodes_allowed check. I see the special checking for EnableValhalla and invoke virtual overloading.
Did you want to do specific checking of EnableMVT for vbox/vunbox?

6. verifier.cpp
line 1863:   there is a verify_cp_class_type — I was expecting verify_cp_class_or_value_type?

7. VunboxErrorIndex and VboxUnbox.java
Cleanup. Perhaps remove comments in former lines 98/99 - since they are no longer true, and same lines in original test.
Ok to file as a follow-on rfe

I don’t need to see an update, just answers to the questions to make sure I understand,

> On Sep 5, 2017, at 8:37 PM, Lois Foltan <lois.foltan at oracle.com> wrote:
> Hello,
> Review request for verifier changes to support MVT.
> Webrevs:
>     http://cr.openjdk.java.net/~lfoltan/mvt_verifier_jdk.1/webrev/
> http://cr.openjdk.java.net/~lfoltan/mvt_verifier_hotspot.1/webrev/
> Webrevs include changes to address preliminary review comments by John Rose and Harold Seigel.
> Testing - hotspot jtreg tests, java/lang, java/util, java/io, JDK & Hotspot valhalla tests, jprt hotspot tests, JCK vm (in progress).
> Thanks,
> Lois

More information about the valhalla-dev mailing list