RFR: 8309685: Fix -Wconversion warnings in assembler and register code

Coleen Phillimore coleenp at openjdk.org
Tue Jun 13 18:38:48 UTC 2023


On Tue, 13 Jun 2023 15:52:26 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/cpu/x86/assembler_x86.cpp|4421 col 12| error: no matching function for call to 'Assembler::emit_int8(Assembler::ComparisonPredicate&)'                                                                                                                                          
>> ||  4421 |   emit_int8(vcc);
>> ||       |   ~~~~~~~~~^~~~~
>> || In file included from /scratch/cphillim/hg/21more-conversion/src/hotspot/cpu/x86/assembler_x86.cpp:26:
>> 
>> But I made ComparisonPredicate enum a uint8_t.  Why didn't it match it?
>> 
>>   // Comparison predicates for integral types & FP types when using SSE
>>   enum ComparisonPredicate : uint8_t {
>> 
>> There are a bunch of these that don't like the templates.
>
> Taking out the ENABLE_IF<is_integral> at least allows it to compile but there are lots of crashes because of sign extension.
> 
> Maybe we can make checked_cast<> tolerant of that somehow?
> 
> ```template <typename T2, typename T1>
> constexpr T2 checked_cast(T1 thing) {
>   T2 result = static_cast<T2>(thing);
>   assert(static_cast<T1>(result) == thing, "must be");
>   return result;
> }```

How about something like this?

  template<typename T, ENABLE_IF(std::is_same<T, uint8_t>::value)>
  void emit_int8(T v) { code_section()->emit_int8(v); }

  template<typename T, ENABLE_IF(!std::is_same<T, uint8_t>::value)>
  void emit_int8(T v) {
    if (v < 0) {
      int8_t tmp = static_cast<int8_t>(v);
      assert(static_cast<T>(v) == tmp, "signed cast preserves value");
      emit_int8(static_cast<uint8_t>(v));
    } else {
      emit_int8(checked_cast<uint8_t>(v)); 
    }
  }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14396#discussion_r1228547603


More information about the hotspot-dev mailing list