RFR 8167065 - Adding support for double precision shifting for x86

[Paul Sandoz]

Hi,

Doh! cross eyed…

This looks good (i was tempted to suggest some cleanup of the use of numIter, but the logic is consistent between the left/right shift methods.)

Paul.

> On 17 Oct 2016, at 19:31, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
> 
> Paul,
> 
> Thanks for the review again! :)
> 
> So actually there is no change in the in value for "numIter" between version 2 and version 3. The difference that you are looking at is because shiftLeft and shiftRight implementations have a different value for "numIter". Namely, for shift left, this is numIter: "int numIter = magLen - 1;". For shift right, this is numIter: "int numIter = magLen - nInts - 1;"
> These values are consistent with the implementation before my change.
> 
> You might also notice that the bounds I pass in to Objects.checkFromToIndex differ from my shiftImplRangeCheck which was in version 2. This happens because the "max" in my range check implementation was inclusive while in your recommended approach it is exclusive. Thus I add in plus one to each compared to version 2 of my patch.
> 
> Let me know if you have any other questions and I appreciate you looking in detail to make sure my patch is correct.
> 
> Thanks,
> Razvan
> 
> -----Original Message-----
> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> Sent: Monday, October 17, 2016 12:48 PM
> To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com>; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot-compiler-dev at openjdk.java.net; Berg, Michael C <michael.c.berg at intel.com>
> Subject: Re: RFR 8167065 - Adding support for double precision shifting for x86
> 
> Hi,
> 
>> On 17 Oct 2016, at 11:09, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
>> 
>> Paul,
>> 
>> Thanks again! I have implemented your suggestion and it definitely cleaned up the code a bit. The updated patch is available at http://cr.openjdk.java.net/~mcberg/8167065/jdk/webrev.03/
>> 
>> Can I please get another consideration of this patch? Thanks again!
>> 
> 
> Previously you had in v02
> 
> 3350             int numIter = magLen - nInts - 1;
> 
> and now it is in v03:
> 
> 3267             int numIter = magLen - 1;
> 
> Which one is correct?
> 
> Paul.
> 
> 
>> --Razvan
>> 
>> -----Original Message-----
>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Lupusoru, Razvan A
>> Sent: Friday, October 14, 2016 4:15 PM
>> To: Paul Sandoz <paul.sandoz at oracle.com>
>> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com>; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; hotspot-compiler-dev at openjdk.java.net
>> Subject: RE: RFR 8167065 - Adding support for double precision shifting for x86
>> 
>> Paul,
>> 
>> Great suggestion! I will look into making the update and let you know if it works. Thanks for informing me about this option.
>> 
>> —

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20161018/0b778952/signature.asc>