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

Jan Kratochvil jkratochvil at openjdk.org
Tue Oct 7 10:49:44 UTC 2025


On Mon, 6 Oct 2025 19:18:19 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> I understand what was the intention here, and thanks for the links. But I believe in this case `value` is not a universal reference because `T` gets deduced from the `Hashtable<T>` object, not from the `value` parameter.
>> 
>> Looks like this is discussed in your second link:
>>> Sometimes you can see `T&&` in a function template declaration where `T` is a template parameter, yet there’s still no type deduction.  Consider this `push_back` function in `std::vector`:
>>> 
>>> ```
>>> template <class T, class Allocator = allocator<T> >
>>> class vector {
>>> public:
>>>     ...
>>>     void push_back(T&& x);       // fully specified parameter type ⇒ no type deduction;
>>>     ...                          // && ≡ rvalue reference
>>> };
>>> ```
>>> Here, `T` is a template parameter, and `push_back` takes a `T&&`, yet the parameter is not a universal reference!  How can that be?
>>> 
>>> The answer becomes apparent if we look at how `push_back` would be declared outside the class. I’m going to pretend that `std::vector`’s `Allocator` parameter doesn’t exist, because it’s irrelevant to the discussion, and it just clutters up the code.  With that in mind, here’s the declaration for this version of `std::vector::push_back`:
>>> 
>>> ```
>>> template <class T>
>>> void vector<T>::push_back(T&& x);
>>> ```
>>> `push_back` can’t exist without the class `std::vector<T>` that contains it.  But if we have a class `std::vector<T>`, we already know what `T` is, so there’s no need to deduce it.
>>> 
>>> An example will help.  If I write
>>> 
>>> ```
>>> Widget makeWidget();             // factory function for Widget
>>> std::vector<Widget> vw;
>>> ...
>>> Widget w;
>>> vw.push_back(makeWidget());      // create Widget from factory, add it to vw
>>> ```
>>> my use of `push_back` will cause the compiler to instantiate that function for the class `std::vector<Widget>`. The declaration for that `push_back` looks like this:
>>> ```
>>> void std::vector<Widget>::push_back(Widget&& x);
>>> ```
>>> See?  Once we know that the class is `std::vector<Widget>`, the type of `push_back`’s parameter is fully determined:  it’s `Widget&&`.  There’s no role here for type deduction.
>> 
>> I think something like [this](https://stackoverflow.com/a/70435367/16360266) is needed to make it a universal reference.
>
>> 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)

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

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


More information about the crac-dev mailing list