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