RFR(M): 8173470: [C2] Mask shift operands in ideal graph.

[Vladimir Kozlov]

Thank you, Goetz

Very nice cleanup!

On 2/16/17 9:03 AM, Lindenmaier, Goetz wrote:
> Hi Lutz,
>
> thanks for the review.
>
> What about naming them con and maskedCon?

shift and maskedShift

And since you are doing cleanup ;) I want to ask you to factor out in separate methods (for long and int) checks which 
are duplicated in all this methods. All of them check if in(2) is integer constant and mask it.

Thanks,
Vladimir

>
> Best regards,
>   Goetz
>
>> -----Original Message-----
>> From: Schmidt, Lutz
>> Sent: Donnerstag, 16. Februar 2017 17:55
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-compiler-
>> dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
>> Subject: Re: RFR(M): 8173470: [C2] Mask shift operands in ideal graph.
>>
>> Goetz,
>>
>> Your changes look good. Thanks for cleaning up!
>>
>> I do have one request, though:
>> In the Ideal methods, you are using either local variable names con/in2_con
>> or shift/in2_shift. Could you please decide for one variant? Thank you.
>>
>> Best regards,
>> Lutz
>>
>> From: hotspot-compiler-dev <hotspot-compiler-dev-
>> bounces at openjdk.java.net <mailto:hotspot-compiler-dev-
>> bounces at openjdk.java.net> > on behalf of Götz Lindenmaier
>> <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> >
>> Date: Donnerstag, 16. Februar 2017 um 09:26
>> To: "'hotspot-compiler-dev at openjdk.java.net <mailto:'hotspot-compiler-
>> dev at openjdk.java.net> '" <hotspot-compiler-dev at openjdk.java.net
>> <mailto:hotspot-compiler-dev at openjdk.java.net> >
>> Subject: RFR(M): 8173470: [C2] Mask shift operands in ideal graph.
>>
>>
>>
>> Hi,
>>
>>
>>
>> Constant shift operand > 31 (int) or > 63 (long) are pointless.
>>
>> Mask them in the ideal graph, i.e.
>>
>> if a constant is too large replace it by a smaller one.
>>
>> The code also should use BitsPerJavaInteger instead of BitsPerInt.
>>
>> Please review this change. I please need a sponsor.
>> http://cr.openjdk.java.net/~goetz/wr17/8173470-maskShift/webrev.01
>>
>>
>> From the standard:
>> https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.19
>> <https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.19>
>> If the promoted type of the left-hand operand is int, only the five lowest-
>> order bits of the right-hand operand are used as the shift distance. It is as if
>> the right-hand operand were subjected to a bitwise logical AND operator &
>> (§15.22.1) with the mask value 0x1f (0b11111). The shift distance actually
>> used is therefore always in the range 0 to 31, inclusive.
>>
>> If the promoted type of the left-hand operand is long, then only the six
>> lowest-order bits of the right-hand operand are used as the shift distance. It
>> is as if the right-hand operand were subjected to a bitwise logical AND
>> operator & (§15.22.1) with the mask value 0x3f (0b111111). The shift
>> distance actually used is therefore always in the range 0 to 63, inclusive.
>>
>>
>>
>> Best regards,
>>
>>   Goetz.
>