RFR: 8356328: Some C2 IR nodes miss size_of() function

[Christian Hagedorn]

On Wed, 7 May 2025 07:24:30 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> I wrote a test to check if every C2 IR node has correct size_of() function. And I found some of them are missed. They added new fields and not add size_of() to reflect new size. In linux, it does not cause issue so far, because gcc allocate more space for alignment and can keep these additional `bool` flags. But it will report failure on windows. And if anyone modified base class, it will cause problem.
>> 
>> PS, My test is in https://github.com/openjdk/jdk/compare/master...kuaiwei:jdk:test/check_node_size , but it has many hack on IR nodes to make test to run.
>
> Good catch! It would currently only be a problem when we clone nodes which is probably hard to check statically (could, for example, be part of a loop body and then be cloned).
> 
> Some questions:
> - Have you also checked the Mach nodes?
> - Have you also checked that `cmp()` is overridden in case `hash()` is not `NO_HASH` for those nodes that specify at least one field?
> 
> Just a side node, you can also just use `sizeof(*this)` which is often done in the code.

> @chhagedorn I checked `machnode.hpp` manually and found some of them still miss `size_of()` . I added them in new patch. Thanks.

Nice!

> I checked node list in share/opto/classes.hpp, so MachNode/MachNullCheckNode/MachProjNode are checked. For mach nodes created by adlc, I found adlc will always add size_of function.

Thanks for checking that.

> I haven't checked cmp() and hash() , I will check if my test can cover these.

Sounds good, thanks!

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

PR Comment: https://git.openjdk.org/jdk/pull/25081#issuecomment-2862231039