[crac] RFR: 8368929: [CRaC] Move CPUFeatures check to C/R engine [v4]

[Radim Vansa]

On Tue, 7 Oct 2025 10:46:52 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:

>>> for std::move I would have to pass rvalue reference (therefore the method wouldn't be usable with const value).
>> 
>> To clarify, my suggestion was to leave the parameter as `T value` so that we can safely use `std::move` to treat it as an rvalue reference and move from it. An lvalue argument will be copied into `value` and then the copy will be moved into `*value_ptr`. An rvalue argument will be moved into `value` and then moved into `*value_ptr` --- probably the compiler optimizes these moves but I haven't researched this enough.
>
>> This should be a idiomatic use of perfect forwarding, since `T` is deduced the `T &&` is a universal reference and is treated as lvalue, depending on its use. The current code copies the argument; for `std::move` I would have to pass rvalue reference (therefore the method wouldn't be usable with const value). Sources:
> 
> One can try it does not compile:
> 
> #include "hashtable.hpp"
> int main() {
>   Hashtable<int> h(nullptr, 10);
>   int i = 42;
>   h.put("foo", i);
> }
> h.cpp:5:16: error: rvalue reference to type 'int' cannot bind to lvalue of type 'int'
>     5 |   h.put("foo", i);
>       |                ^
> src/java.base/share/native/libcrexec/hashtable.hpp:47:33: note: passing argument to parameter 'value' here
>    47 |   bool put(const char *key, T&& value);
>       |                                 ^
> 1 error generated.
> 
> Couldn't we use a new template parameter for the method?
> [tu.patch](https://github.com/user-attachments/files/22741639/tu.patch)

You are right, it didn't work when I've tried to pass lvalue references in a test (and thanks to testing separately I found a bug in the list impl itself...). While this is not critical for CRaC itself I took the opportunity to improve my C++ knowledge and do the structures right :) Here is a standalone check to ensure that the containers work as expected: https://gist.github.com/rvansa/5f4cb76ddb4f694d2feb9a1ffb09d630
Note that I use two methods in `linkedlist.hpp` rather than universal reference in the end, as the universal reference template would consider `{ ... }` a braced initializer list rather than a struct initializer.

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

PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2410380629