RFR: 8349686: [s390x] C1: Improve Class.isInstance intrinsic [v8]

Andrew Haley aph at openjdk.org
Thu Feb 20 14:42:53 UTC 2025


On Thu, 20 Feb 2025 11:48:23 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> I have moved the result in volatile-float registers, instead of using stack for the shuffling. With that we can get rid of `Z_R0_scratch` from `assert_different_registers` as per your suggestion.
>
> For verification I made it to crash manually, and we are seeing correct values only: 
> 
> 
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/home/amit/isInstanceIntrinsic/jdk/src/hotspot/share/oops/klass.cpp:1330), pid=3145509, tid=3145513
> #  fatal error: mismatch: java.nio.HeapByteBuffer implements sun.nio.ch.DirectBuffer: linear_search: 1; table_lookup: 1
> #
> # JRE version: OpenJDK Runtime Environment (25.0) (fastdebug build 25-internal-adhoc.amit.jdk)
> # Java VM: OpenJDK 64-Bit Server VM (fastdebug 25-internal-adhoc.amit.jdk, mixed mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-s390x)
> 
> 
> All of the registers have retrieved the correct values back.

> I have moved the result in volatile-float registers, instead of using stack for the shuffling. With that we can get rid of `Z_R0_scratch` from `assert_different_registers` as per your suggestion.

Fair enough, but this is a general recommendation to do with good practice in hand-coded assembler. Using aliases for registers can result in a maintenance programmer not noticing that two different names refer to the same thing. , This has led to bugs that aren't revealed in tests but crash in production. Sure, aliases (e.g `obj`, `klass`, `index` etc. can help readbility, but be careful.

This isn't intended to be a total ban, but a note of caution.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23535#discussion_r1963691768


More information about the hotspot-dev mailing list