RFR: 8314999: IR framework fails to detect allocation [v6]

[Marc Chevalier]

On Wed, 19 Mar 2025 13:26:51 GMT, Marc Chevalier <duke at openjdk.org> wrote:

>> Bring the `ALLOC(_ARRAY)?(_OF)?` IR framework regex in the modern era!
>> 
>> Rather than matching on the OptoAssembly, we now match before macro expansion. Ideed, matching on OptoAssembly is fragile: between the register being assigned the class to allocate and the actual call to `_new_instance_Java`, there might
>> be register spill, making the match hard, and fragile. Now, these regex are applied before macro expansion.
>> 
>> To make that work, I needed to adapt the dump_spec of `AllocateNode` to print the allocated class, in case it is a precise constant. This is also nice to have as a human reading the output.
>> 
>> The new feature is also slightly more precise in case we want to match the allocation of a given class (that is `ALLOC(_ARRAY)?_OF`). It used to be along the lines of:
>> 
>> "precise .*" + IS_REPLACED + ":"
>> 
>> which is actually too lenient: it only assert the suffix is what is expected. On the plus side, if we wanted to have `MyClass`, then `some/package/MyClass` would match, but on the other hand, `ItLooksLikeButIsNotMyClass` would also match. The new regex use a word boundary:
>> 
>> "allocationKlass:.*\\b" + IS_REPLACED + "\\s"
>> 
>> which make it a bit more specific: the given name cannot be extended with only letters: there must be a non-letter char. For instance, `ItLooksLikeButIsNotMyClass` wouldn't work anymore, since there is no word boundary between `ItLooksLikeButIsNot` and `MyClass`. It can also not be extended on the right into `MyClassButNotReally` because of the space character that must exist after the searched string.
>> 
>> The case of array allocations is slightly more tricky, but essentially similar.
>> 
>> It is not quite fool-proof since a package path can still be extended, e.g.
>> 
>> @IR(counts = {IRNode.ALLOC_OF, "some/package/MyClass", "1"})
>> 
>> will also match allocations of `a/prefix/some/package/MyClass`.
>> 
>> I think it's an acceptable limitation.
>> 
>> Let's have a little preview of the new `dump_spec`. For instance, it could have been something like:
>> 
>>  315  Allocate  === 290 287 273 8 1 (94 313 23 1 1 10 43 43 10 43 ) [[ 316 317 318 325 326 327 ]]  rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, top, bool ) Test$MyClass::<init> @ bci:23 (line 43) Test::test @ bci:5 (line 48) !jvms: Test$MyClass::<init> @ bci:23 (line 43) Test::test @ bci:5 (line 48)
>> 
>> and now it is
>> 
>>  315  Allocate  === 290 287 273 8 1 (94 313 23 1 1 10 43 43 10 43 ) [[ 316 317 318 325 326 327 ]]  rawptr:NotNull ( int:>=0, java/l...
>
> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update copyright year

Thanks for reviews!

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

PR Comment: https://git.openjdk.org/jdk/pull/24093#issuecomment-2742693410