Request for review (S): C2 should recognize "obj.getClass() == A.class" code pattern

[John Rose]

Looking at it…  It looks good so far.  Nice work.  I haven't wrapped my head around the match logic yet, but it seems reasonable.

I have a suggestion.

It appears that the following new transformation will occur:

CmpP(LoadP(LoadKlass(obj._klass).java_mirror), ConP(Foo.class))
 =Ideal=>
CmpP(LoadKlass(obj._klass)), ConP(Foo.klass))

Then, while parsing Remi's or Mark's code, a checkcast on Foo will be emitted, causing this call in the parser:

GraphKit::gen_checkcast(obj, ConP(Foo.klass))

In maybe_cast_profiled_receiver, a type profile on the checkcast instruction reports a monomorphic object, so the generated check is again:

CmpP(LoadKlass(obj._klass)), ConP(Foo.klass))

(BTW, this should work for instanceof instructions also, since they also have type profiles.  But I expect a blind cast to follow the obj.getClass()==Foo comparison.)

Failure goes to an uncommon trap, probably.  The main point here is that IfNode::Ideal on the second test notices the first (identical) test in a dominating position, and so elides the second test.  (At least, I think that's what's happening.  You can't be sure without single-stepping.)

This is a win, since the redundant test goes away.  The win is enabled by normalizing CmpP on a pair of java-mirrors to CmpP on a pair of low-level classes, and then recognizing a redundant dominating check.

This win is brittle, however, because it requires a close match inside IfNode::Ideal.  A way to strengthen the win would be to actually change the type of obj in the parser when the first CmpP is created.

Therefore, please look at the code in parse2.cpp, around this comment:

  // Check for a comparison to a constant, and "know" that the compared
  // value is constrained on this path.

The idea there is that a CmpP might fold up (during parsing) in a way that reveals type information about a related value.  This only works if the related value P is in a (JVMS) register; it does not track P._klass, etc.  But it would be reasonable (given your change) to note that the comparison is CmpP(LoadKlass(obj._klass), ConP(Foo.klass)), and constrain the type of obj downstream.  This is likely to make your optimization more productive, since it will not rely so much on the type profile at the checkcast instruction.  (Type profiles are great when they work but they can become polluted inside of shared subroutines.)

Would you mind trying it out?

— John