RFR(M): 8206141: [lworld] Improve accessing a flattened value type array passed as Object[]

Ioi Lam ioi.lam at oracle.com
Thu Sep 27 19:47:21 UTC 2018

Hi Roland,

I started studying your patch so that I can implement the corresponding 
profiling support inside the interpreter. I have to admit that I don't 
quite understand what's going on :-(, but I have a comment about this 
new block of code in parser2.cpp:

   83     Node* kls = load_object_klass(ary);
   84     Node* lhp = basic_plus_adr(kls, 
   85     Node* layout_val = make_load(NULL, lhp, TypeInt::INT, T_INT, 
   86     Node* tag = _gvn.transform(new RShiftINode(layout_val, 
   87     ideal.if_then(tag, BoolTest::ne, 
intcon(Klass::_lh_array_tag_vt_value)); {

which looks the same as this existing block in the same file:

  206         Node* kls = load_object_klass(ary);
  207         Node* lhp = basic_plus_adr(kls, 
  208         Node* layout_val = make_load(NULL, lhp, TypeInt::INT, 
T_INT, MemNode::unordered);
  209         layout_val = _gvn.transform(new RShiftINode(layout_val, 
  210         ideal.if_then(layout_val, BoolTest::ne, 
intcon(Klass::_lh_array_tag_vt_value)); {

Could the above blocks be somehow refactored, along with the following 
method, so we can avoid duplicated code?

3392 Node* GraphKit::gen_lh_array_test(Node* kls, unsigned int lh_value) {
3393   Node* lhp = basic_plus_adr(kls, 
3394   Node* layout_val = make_load(NULL, lhp, TypeInt::INT, T_INT, 
3395   layout_val = _gvn.transform(new RShiftINode(layout_val, 
3396   Node* cmp = _gvn.transform(new CmpINode(layout_val, 

- Ioi
On 9/27/18 5:35 AM, Roland Westrelin wrote:
> Here is a new webrev for this:
> http://cr.openjdk.java.net/~roland/8206141/webrev.01/
> that should address the comments from your review and some comments you
> made offline.
> The failure you reported happens because an array store is emitted with
> a test for a flattened array and an ArrayCopyNode but after some
> optimizations the type of the array becomes known and the code in
> ArrayCopyNode::Ideal() sees a clone from an instance to an array which
> is unexpected. I added a test case for that and fixed it by bailing out
> of ArrayCopyNode::Ideal() when a mix of array and instance is seen.
> I moved the check for a null value from array_store_check() to
> array_store() as you mentioned offline that this would require less
> runtime checks to be executed in some cases.
> I also noticed that if we know the type of the value being stored and
> that value cannot be a value type then we're guaranteed the store is to
> a non value array. I changed the logic that emits the runtime check
> accordingly.
> Roland.

More information about the valhalla-dev mailing list