RFD: C2: Poor code quality for Unsafe

[Paul Sandoz]

> On 27 Jul 2016, at 15:54, Andrew Haley <aph at redhat.com> wrote:
> 
> On 27/07/16 14:45, Paul Sandoz wrote:
>> 
>>> On 27 Jul 2016, at 15:36, Andrew Haley <aph at redhat.com> wrote:
>>> 
>>> On 27/07/16 13:57, Paul Sandoz wrote:
>>>> What JDK build are you using? dev, hs, or hs-comp?
>>> 
>>> hs-comp.
>> 
>> Only hs has the buggy fix for ByteBufferAs-X-Buffer to use Unsafe to
>> read/write as wider primitives, so hs-comp will resort to
>> reading/writing as bytes and composing/decomposing as view types via
>> Bits.java.
> 
> Yabbut: remember, I reviewed that patch, so my copy does.

Ah, ok, i did not realize you had that patch applied as well, i was thrown off base because you mentioned the following method:

   protected int ix(int i) {
       return (i << $LG_BYTES_PER_VALUE$) + offset;
   }

which is not used for access [*] in the patch(s).

Paul.

[*]
Bad patch:

 111     protected int ix(int i) {
 112         return (i << $LG_BYTES_PER_VALUE$) + offset;
 113     }
 114
 115     private long byteOffset(long i) {
 116         return (i << $LG_BYTES_PER_VALUE$) + bb.address + offset;
 117     }
 118
 119     public $type$ get() {
 120         $memtype$ x = unsafe.get$Memtype$Unaligned(bb.hb, byteOffset(nextGetIndex()),
 121             {#if[boB]?true:false});
 122         return $fromBits$(x);
 123     }

Good (hopefully!) patch:

 110     private int ix(int i) {
 111         int off = (int) (address - bb.address);
 112         return (i << $LG_BYTES_PER_VALUE$) + off;
 113     }
 114
 115     protected long byteOffset(long i) {
 116         return (i << $LG_BYTES_PER_VALUE$) + address;
 117     }
 118
 119     public $type$ get() {
 120         $memtype$ x = unsafe.get$Memtype$Unaligned(bb.hb, byteOffset(nextGetIndex()),
 121             {#if[boB]?true:false});
 122         return $fromBits$(x);
 123     }


> This code
> quality issue came up during that review.  I determined that it was a
> HotSpot issue.
> 
>>> Sure.  It doesn't seem to make any difference to the exact issue I'm
>>> addressing.
>> 
>> Thanks. Perhaps it will if Unsafe is used to read wider primitives?
> 
> No.  I now have everything that you have, I think.  :-)
> 
> Andrew.