review (S) for 6982370: SIGBUS in jbyte_fill

[Vladimir Kozlov]

Tom,

Did you run your new test on x86 (32- and 64-bit)?

I don't see that O4 and O5 are used after you save 'to', 'count'.

Could you use shifts?:

       __ sllx(O3, 60, O3);
       __ srlx(O3, 60, O3);

instead of:

1641       __ set(0xffff, O3); << 2 instructions
1642       __ and3(value, O3, value);

Tom, sorry I don't like your changes which increase the code.
How about this:

     __ BIND(L_fill_4_bytes);
     __ brx(Assembler::zero, false, Assembler::pt, L_fill_2_bytes);
     if (t == T_BYTE || t == T_SHORT) {
       __ delayed()->andcc(count, 1<<(shift-1), G0);
+     if (t == T_SHORT) {
+       __ sth(value, to, 0);
+       __ sth(value, to, 2);
+     } else {
+       __ stb(value, to, 0);
+       __ stb(value, to, 1);
+       __ stb(value, to, 2);
+       __ stb(value, to, 3);
+     }
     } else {
       __ delayed()->nop();
+     __ stw(value, to, 0);
     }
-   __ stw(value, to, 0);
     if (t == T_BYTE || t == T_SHORT) {
       __ inc(to, 4);
       // fill trailing 2 bytes
       __ andcc(count, 1<<(shift-1), G0); // in delay slot of branches
       __ BIND(L_fill_2_bytes);
       __ brx(Assembler::zero, false, Assembler::pt, L_fill_byte);
       __ delayed()->andcc(count, 1, count);
-     __ sth(value, to, 0);
       if (t == T_BYTE) {
+       __ stb(value, to, 0);
+       __ stb(value, to, 1);
         __ inc(to, 2);
         // fill trailing byte
         __ andcc(count, 1, count);  // in delay slot of branches
         __ BIND(L_fill_byte);
         __ brx(Assembler::zero, false, Assembler::pt, L_exit);
         __ delayed()->nop();
         __ stb(value, to, 0);
       } else {
+       __ sth(value, to, 0);
         __ BIND(L_fill_byte);
       }
     } else {
       __ BIND(L_fill_2_bytes);
     }
     __ BIND(L_exit);

Vladimir

Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/6982370
> 
> 6982370: SIGBUS in jbyte_fill
> Summary:
> Reviewed-by:
> 
> In the new fill routines copy less than 8 bytes long skip over the
> main code and go the final cleanup stage.  On sparc this isn't right
> since it still hasn't accounted for alignment.  The fix is to have a
> special path for handling that.  The code doesn't crash on 32 bit
> sparc because handling of misaligned stores seems to be enabled.
> Additionally I noticed a problem with the logic for clearing the high
> bits of a short value.  The logic I wrote works correctly for clean
> ints but doesn't work for 64 bit signed extended ints resulting in
> invalid fill values in some cases.  Tested with failing nightly and
> new test case to exercise the various alignments.