On Mon, 10 Feb 2025 11:04:38 GMT, Andrew Dinn <ad...@openjdk.org> wrote:
>> Vladimir Kozlov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix Zero and Minimal VM builds
>
> src/hotspot/share/code/codeBlob.cpp line 58:
>
>> 56: #include <type_traits>
>> 57:
>> 58: // Virtual methods are not allowed in code blobs to simplify caching
>> compiled code.
>
> Is it worth considering generating this code plus also some of the existing
> code in the header using an iterator template macro? e.g.
>
> #define CODEBLOBS_DO(do_codeblob_abstract, do_codeblob_nonleaf, \
> do_codeblob_leaf) \
> do_codeblob_abstract(CodeBlob) \
> do_codeblob_leaf(nmethod, Nmethod, nmethod) \
> do_codeblob_abstract(RuntimeBlob) \
> do_codeblob_nonleaf(BufferBlob, Buffer, buffer) \
> do_codeblob_leaf(AdapterBlob, Adapter, adapter) \
> . . . \
> do_codeblob_leaf(RuntimeStub, Runtime_Stub, runtime_stub) \
> . . .
>
> The macro arguments to the templates would themselves be macros:
>
> do_codeblob_abstract(classname) // abstract, non-instantiable class
> do_codeblob_nonleaf(classname, kindname, accessorname) // instantiable,
> subclassable
> do_codeblob_leaf(classname, kindname, accessorname) // instantiable,
> non-subclassable
>
> Using a template macro like this to generate the code below -- *plus also*
> some of the code currently declared piecemeal in the header -- would
> guarantee all cases are covered now and will remain so later so when the
> macro is updated. I think it would probably also allow case handling code in
> AOT cache code to be generated.
>
> So, we would generate the code here as follows
>
> #define EMPTY1(classname)
> #define EMPTY3(classname, kindname, accessorname)
>
> #define assert_nonvirtual_leaf(classname, kindname, accessorname) \
> static_assert(!std::is_polymorphic<classname>::value, \
> "no virtual methods are allowed in " # classname );
>
> CODEBLOBS_DO(empty1, empty3, assert_nonvirtual_leaf)
>
> #undef assert_nonvirtual_leaf
>
> Likewise in codeBlob.hpp we could generate `enum CodeBlobKind` to cover all
> the non-abstract classes and likewise generate the accessor methods
> `is_nmethod()`, `is_buffer_blob()` in class `CodeBlob` which allow the kind
> to be tested.
>
> #define codekind_enum_tag(classname, kindname, accessorname) \
> kindname,
>
> enum CodeBlobKind : u1 {
> None,
> CODEBLOBS_DO(empty1, codekind_enum_tag, codekind_enum_tag)
> Number_Of_Kinds
> };
>
> ...
Thank you @adinn for suggestion but no, I don't like macros - hard to debug and
they add more complexity in this case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23533#discussion_r1949483501
