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

[Lindenmaier, Goetz]

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.
> >