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

[Vladimir Kozlov]

I was think about using find_int_con() and pass in(2) to maskedShiftAmount()

static int maskShiftAmount(PhaseGVN *phase, Node *shiftNode, int nBits) {
   jint       shift = phase->find_int_con(shiftNode, 0); // Use 0 as default for the check to work in both cases
   jint maskedShift = shift & (nBits - 1);

   if (maskedShift == 0) return 0;     // Let Identity() handle 0 shift count.

   if (shift != maskedShift) {
     shiftNode->set_req(2, phase->intcon(maskedShift)); // Replace shift count with masked value.
   }

   return maskedShift;
}

And all Identity (with corresponding mask):

Node* LShiftINode::Identity(PhaseGVN* phase) {
   return (phase->find_int_con(in(2), -1) & (BitsPerJavaInteger - 1)) == 0) ? in(1) : this;


Thanks,
Vladimir

On 2/16/17 11:25 PM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> Thanks for looking at this.
>
> I moved the code to a new method "maskShiftAmount"
> http://cr.openjdk.java.net/~goetz/wr17/8173470-maskShift/webrev.02/
>
> Best regards,
>   Goetz.
>
>> -----Original Message-----
>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
>> bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
>> Sent: Donnerstag, 16. Februar 2017 21:50
>> To: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8173470: [C2] Mask shift operands in ideal graph.
>>
>> 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.
>>>