RFR: 8351958: Some compile commands should be made diagnostic [v3]

Marc Chevalier mchevalier at openjdk.org
Fri May 16 16:03:54 UTC 2025


On Fri, 16 May 2025 12:07:04 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> src/hotspot/share/compiler/compilerOracle.cpp line 343:
>> 
>>> 341:     JVMFlag* flag = JVMFlag::find_declared_flag(name);
>>> 342:     if (flag != nullptr && flag->is_diagnostic()) {
>>> 343:       jio_snprintf(errorbuf, buf_size, "VM option '%s' is diagnostic and must be enabled via -XX:+UnlockDiagnosticVMOptions.", name);
>> 
>> Just noticed that code above in line 333 uses `warning(...)`. I think that code should also use `errorbuf` now that we have it.
>
> Done (despite being out of the window so it won't show as outdated).

Actually... That changes the behavior quite a bit. Currently, if the unlock is not supplied, we get a warning and the blackhole option is ignored. See this test:
https://github.com/openjdk/jdk/blob/46a12e781edcbe9da7bd39eb9e101fc680053cef/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java#L68-L73
The exit value of the failing case is still 0, using `error_buf` changes it into a failure (exit 1 + nothing is run). I think it's more explicit and probably better, but quite a change in the behavior.
It also changes this part of the test:
https://github.com/openjdk/jdk/blob/46a12e781edcbe9da7bd39eb9e101fc680053cef/test/hotspot/jtreg/compiler/blackhole/BlackholeExperimentalUnlockTest.java#L105-L115
It would not be possible to silent the warning and continue as if nothing happened. Again, I think the current behavior is not great: currently, an option is silently ignored (or semi-silently, with a warning). As much as I don't approve this behavior, it seems to be like that on purpose since tests exist for it. Wdyt?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25150#discussion_r2093315658


More information about the hotspot-dev mailing list