RFR: 8285394: Compiler blackholes can be eliminated due to stale ciMethod::intrinsic_id() [v2]

[Dean Long]

On Thu, 21 Apr 2022 17:14:03 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> This is seen in some tests: if blackhole method is deemed hot for inlining, then at least C2 would inline it without looking back at its intrinsic status. Which silently breaks blackholes.
>> 
>> The cause is that there are *two* places where intrinsic ID is recorded. Current blackhole code only writes down blackhole intrinsic ID in `Method::intrinsic_id()`, but we should also set it in `ciMethod::intrinsic_id()`, which is used from C2 inlining code. `ciMethod` is normally populated from `Method::intrinsic_id()`, but it happens too early, before setting up blackhole intrinsic. 
>> 
>> Additional testing:
>>  - [x] Linux x86_64 {fastdebug,release}, wew test fails before the patch, passes with it
>>  - [x] Linux x86_64 {fastdebug,release} `compiler/blackhole`
>>  - [x] Linux x86_64 fastdebug, sanity microbenchmark corpus run with the patch
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Negative test

Wouldn't it be better to call tag_blackhole_if_possible() in Method::init_intrinsic_id()?  Or is it considered an expensive operation and only the compilers (and never, say, the interpreter) would ever care about this?  If it is an expensive operation, and we call it for every ciMethod, then maybe tag_blackhole_if_possible() should check for "already set" first thing.

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

Changes requested by dlong (Reviewer).

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