RFR: 8284433: Cleanup Disassembler::find_prev_instr() on all platforms

[Lutz Schmidt]

On Wed, 6 Apr 2022 08:06:17 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

> Hi team,
> 
> This is a trivial cleanup for `Disassembler::find_prev_instr()` on all platforms. This function is used nowhere and seems to be loaded in JDK-13 (JDK-8213084) [1]. I looked through the code and found it very solid that it checks many corner cases (unreadable pages on s390x, etc.). But it is not used so modifying and testing it might be hard, especially the corner cases that could increase burdens to maintenance. On RISC-V we also need to make the current semantics of this function[2] right (Thanks to Felix @RealFYang pointing out that) because there is an extension 'C' that can compress the size of some instructions from 4-byte to 2-byte. Though I have written one version on my local branch, I began to think if removing this might be a better choice anyway.
> 
> Tested by building hotspot on x86, x86-zero, AArch64, RISC-V 64, and s390x. (I don't have access to s390x and ppc real machines, but have an s390x sysroot and qemu)
> 
> (I feel also pleased to retract this patch if there are objections.)
> 
> [1] https://github.com/openjdk/jdk13u-dev/commit/b730805159695107fbf950a0ef48e6bed5cf5bba
> [2] https://github.com/openjdk/jdk/blob/0a67d686709000581e29440ef13324d1f2eba9ff/src/hotspot/cpu/riscv/disassembler_riscv.hpp#L38-L44
> 
> Thanks,
> Xiaolin

Hi Xiaolin,

no worries, there was a bit of a fun undertone in my comments. It's hard to _hear_ such nuances when you are _writing_. I can live with the code being removed. Fresh and simple opinions are valuable, particularly as contrast to old sentimentality. As said above, I can add the code again should I find time to do it right (and complete) somewhen in the future. As of today, I can't say when somewhen will be.

So please, go ahead with your PR.

Best, Lutz

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

PR: https://git.openjdk.java.net/jdk/pull/8120