Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-18 Thread Coleen Phillimore
On Fri, 18 Aug 2023 02:13:57 GMT, David Holmes wrote: >> template bool primitive_equals(const K& k0, const K& k1) { >> return k0 == k1; >> } >> >> template int primitive_compare(const K& k0, const K& k1) { >> return ((k0 < k1) ? -1 : (k0 == k1) ? 0 : 1); >> } >> >> >> This is the primitive

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread David Holmes
On Fri, 18 Aug 2023 01:20:59 GMT, Coleen Phillimore wrote: >> It is the name `primitive_compare` - I only previously saw it used for >> integer types. Using it with pointers seems "wrong". Don't we have to >> convert to `intptr_t` to compare pointers numerically anyway? > > template bool primit

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread David Holmes
On Fri, 18 Aug 2023 01:24:20 GMT, Coleen Phillimore wrote: >> I don't follow. The fields are int so cast them to jlong before returning >> them. All the callers of these methods expect jlong so there can't be any >> issue there. > > You don't need to cast from int to jlong. The callers of the

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Fri, 18 Aug 2023 00:24:07 GMT, David Holmes wrote: >> If they stay jlong returns (note that these fields are in fact int), then we >> need to add casting to all the callers. Casting is worse than returning the >> correct types. If someone wants to make these fields jlong someday then >> t

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Fri, 18 Aug 2023 00:22:10 GMT, David Holmes wrote: >> Was it odd before or odd now? What we want to do is compare pointers for a >> sort function. This primitive_compare has been used in other places as an >> improvement. > > It is the name `primitive_compare` - I only previously saw it us

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread David Holmes
On Thu, 17 Aug 2023 12:10:51 GMT, Coleen Phillimore wrote: >> src/hotspot/share/logging/logOutput.cpp line 69: >> >>> 67: >>> 68: static int tag_cmp(const LogTagType *a, const LogTagType *b) { >>> 69: return primitive_compare(a, b); >> >> This looks very odd given we are dealing with pointer

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Thu, 17 Aug 2023 00:20:44 GMT, David Holmes wrote: >> Coleen Phillimore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change size of op_index back. > > src/hotspot/share/logging/logOutput.cpp line 69: > >> 67: >> 68: static int ta

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Thu, 17 Aug 2023 07:44:54 GMT, Christian Hagedorn wrote: >> I see Dean indicated this is platform specific, but I don't know how. It >> sounds like this is a bug if it should be 64-bit. > > I think `uint32_t` was wrong here since it could be a larger 64-bit value on > 64-bit platforms. `Elf

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Thu, 17 Aug 2023 12:24:48 GMT, Coleen Phillimore wrote: >> Wasting what? > > This calls move_position with the value next: > > > bool DwarfFile::MarkedDwarfFileReader::move_position(const long offset) { > if (offset == 0) { > return true; > } > return set_position(_current_pos + of

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Coleen Phillimore
On Thu, 17 Aug 2023 12:15:24 GMT, Coleen Phillimore wrote: >> src/hotspot/share/utilities/elfFile.cpp line 792: >> >>> 790: // We must align to twice the address size. >>> 791: uint8_t alignment = DwarfFile::ADDRESS_SIZE * 2; >>> 792: uint64_t padding = alignment - (_reader.get_position()

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-17 Thread Christian Hagedorn
On Thu, 17 Aug 2023 02:06:22 GMT, David Holmes wrote: >> src/hotspot/share/utilities/elfFile.hpp line 486: >> >>> 484: DwarfFile* _dwarf_file; >>> 485: MarkedDwarfFileReader _reader; >>> 486: uintptr_t _section_start_address; >> >> This seems suspicious - is this a 32-bit value or 6

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-16 Thread David Holmes
On Thu, 17 Aug 2023 01:57:46 GMT, David Holmes wrote: >> Coleen Phillimore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change size of op_index back. > > src/hotspot/share/utilities/elfFile.hpp line 486: > >> 484: DwarfFile* _dwa

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-16 Thread David Holmes
On Wed, 16 Aug 2023 23:56:58 GMT, Coleen Phillimore wrote: >> Fix MaxElementPrintSize and casts. Also fixed miscellaneous -Wconversion >> warnings in runtime code. This is the last one I'm going to do for runtime >> for a while. >> Tested with tier1-4. > > Coleen Phillimore has updated the pul

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-16 Thread Coleen Phillimore
On Thu, 17 Aug 2023 00:37:55 GMT, Dean Long wrote: >> Coleen Phillimore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change size of op_index back. > > src/hotspot/share/utilities/elfFile.cpp line 1454: > >> 1452: return false

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-16 Thread Dean Long
On Wed, 16 Aug 2023 23:56:58 GMT, Coleen Phillimore wrote: >> Fix MaxElementPrintSize and casts. Also fixed miscellaneous -Wconversion >> warnings in runtime code. This is the last one I'm going to do for runtime >> for a while. >> Tested with tier1-4. > > Coleen Phillimore has updated the pul

Re: RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]

2023-08-16 Thread Coleen Phillimore
> Fix MaxElementPrintSize and casts. Also fixed miscellaneous -Wconversion > warnings in runtime code. This is the last one I'm going to do for runtime > for a while. > Tested with tier1-4. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last r