[9] RFR (M): 8063137: Never-taken branches should be pruned when GWT LambdaForms are shared

[Vladimir Ivanov]

> Looking very good, thanks.  Ship it!
Thanks, John!

> Actually, can you insert a comment why the injected counts are not scaled?  (Or perhaps they should be??)
Sure! I intentionally don't scale the counts because I don't see any 
reason to do so. Profiling is done on per-MethodHandle basis, so the 
counts should be very close (considering racy updates) to the actual 
behavior.

> Also, we may need a followup bug for the code with this comment:
>    // Look for the following shape: AndI (ProfileBoolean) (ConI 1))
>
> Since profileBoolean returns a TypeInt::BOOL, the AndI with (ConI 1) should fold up.
> So there's some work to do in MulNode, which may allow that special pattern match to go away.
> But I don't want to divert the present bug by a possibly complex dive into fixing AndI::Ideal.
Good catch! It's an overlook on my side.
The following change for ProfileBooleanNode solves the problem:
-  virtual const Type *bottom_type() const { return TypeInt::INT; }
+  virtual const Type *bottom_type() const { return TypeInt::BOOL; }

I polished the change a little according to your comments (diff against 
v03):
http://cr.openjdk.java.net/~vlivanov/8063137/webrev.03-04/hotspot

Changes:
   - added short explanation why injected counts aren't scaled

   - adjusted ProfileBooleanNode type to TypeInt::BOOL and removed 
excessive pattern matching in has_injected_profile()

   - added an assert when ProfileBooleanNode is removed to catch the 
cases when injected profile isn't used: if we decide to generalize the 
API, I'd be happy to remove it, but current usages assumes that injected 
counts are always consumed during parsing and missing cases can cause 
hard-to-diagnose performance problems.

Best regards,
Vladimir Ivanov

>
> (Generally speaking, pattern matching should assume strong normalization of its inputs.  Otherwise you end up duplicating pattern match code in many places, inconsistently.  Funny one-off idiom checks like this are evidence of incomplete IR normalization.  See http://en.wikipedia.org/wiki/Rewriting for some background on terms like "normalization" and "confluence" which are relevant to C2.)
>
> — John
>
> On Jan 27, 2015, at 8:05 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>>
>> Thanks for the feedback, John!
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~vlivanov/8063137/webrev.03/jdk
>> http://cr.openjdk.java.net/~vlivanov/8063137/webrev.03/hotspot
>>
>> Changes:
>>   - renamed MHI::profileBranch to MHI::profileBoolean, and ProfileBranchNode to ProfileBooleanNode;
>>   - restructured profile layout ([0] => false_cnt, [1] => true_cnt)
>>   - factored out profile injection in a separate function (has_injected_profile() in parse2.cpp)
>>   - ProfileBooleanNode stores true/false counts instead of taken/not_taken counts
>>   - matching from value counts to taken/not_taken happens in has_injected_profile();
>>   - added BoolTest::ne support
>>   - sharpened test for AndI case: now it checks AndI (ProfileBoolean) (ConI 1) shape
>>
>> Best regards,
>> Vladimir Ivanov
>