On Fri, 3 Jan 2025 16:23:31 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> There are a lot of format modifiers that are noisy and unnecessary in the
>> code. This change removes the INTX variants. It's not that disruptive even
>> for backporting because %z modifier has been available for a long time so
>> should backport fine. This was mostly done with a sed script plus some hand
>> fixups.
>>
>> Testing mach5 and other platform cross compilations in progress. Opening
>> this for GHA testing.
>
> Coleen Phillimore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix %Ix to %zx.
Uses of `[U]INTX_FORMAT_X` have been replaced with `0x%zx`. I mentioned the
possibility of instead using `%#zx`. I don't know if we really want to use
some of
the (to me) more obscure flag options though.
src/hotspot/cpu/x86/vm_version_x86.cpp line 1725:
> 1723: ArrayOperationPartialInlineSize = MaxVectorSize >= 16 ?
> MaxVectorSize : 0;
> 1724: if (ArrayOperationPartialInlineSize) {
> 1725: warning("Setting ArrayOperationPartialInlineSize as
> MaxVectorSize%zd)", MaxVectorSize);
pre-existing: seems like there should be a separator of some kind between
"MaxVectorSize" and the
value, either a space or an "=" would be okay.
src/hotspot/os/linux/os_linux.cpp line 1370:
> 1368:
> 1369: #define _UFM "%zu"
> 1370: #define _DFM "%zd"
Why not get rid of these?
src/hotspot/share/ci/ciMethodData.cpp line 788:
> 786: // which makes comparing it with the SA version of this output
> 787: // harder. data()'s element type is intptr_t.
> 788: out->print(" 0x%zx", data()[i]);
Could instead use " %#zx".
src/hotspot/share/compiler/disassembler.cpp line 600:
> 598: st->print("Stub::%s", desc->name());
> 599: if (desc->begin() != adr) {
> 600: st->print("%+zd " PTR_FORMAT, adr - desc->begin(), p2i(adr));
Oh, that's an interesting "abuse" of the `_W` variant.
src/hotspot/share/gc/shared/ageTable.cpp line 38:
> 36: #include "logging/logStream.hpp"
> 37:
> 38: /* Copyright (c) 1992, 2025, Oracle and/or its affiliates, and Stanford
> University.
Well this is weird. An atypical copyright down inside the file?
src/hotspot/share/oops/instanceKlass.cpp line 3695:
> 3693:
> 3694: st->print(BULLET"hash_slot: %d", hash_slot()); st->cr();
> 3695: st->print(BULLET"secondary bitmap: " LP64_ONLY("0x%016zu")
> NOT_LP64("0x%08zu"), _secondary_supers_bitmap); st->cr();
Should be using "zx" rather than "zu". I think this could be written as
`"%#0*zx", (2 * BytesPerWord + 2), _secondary_supers_bitmap`
That's looking a lot like line noise though. I think this and ones like it
probably ought not be
changed at all.
src/hotspot/share/oops/klass.cpp line 1308:
> 1306: if (secondary_supers() != nullptr) {
> 1307: st->print(" - "); st->print("%d elements;",
> _secondary_supers->length());
> 1308: st->print_cr(" bitmap: " LP64_ONLY("0x%016zu") NOT_LP64("0x%08zu"),
> _secondary_supers_bitmap);
Same as in instanceKlass - maybe this shouldn't be changed at all.
src/hotspot/share/utilities/globalDefinitions.hpp line 156:
> 154: #define UINTX_FORMAT_X_0 "0x%016" PRIxPTR
> 155: #else
> 156: #define UINTX_FORMAT_X_0 "0x%08" PRIxPTR
As noted in places where it's used, I'm not sure we should remove and replace
UINTX_FORMAT_X_0.
test/hotspot/gtest/utilities/test_globalDefinitions.cpp line 281:
> 279:
> 280: check_format("%zd", (intx)123, "123");
> 281: check_format("0x%zx", (intx)0x123, "0x123");
Could be "%#zx".
test/hotspot/gtest/utilities/test_globalDefinitions.cpp line 286:
> 284:
> 285: check_format("%zu", (uintx)123u, "123");
> 286: check_format("0x%zx", (uintx)0x123u, "0x123");
Could be "%#zx".
-------------
Changes requested by kbarrett (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22916#pullrequestreview-2530503795
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902879593
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902886743
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902972028
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902912020
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902916165
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902944144
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902945394
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902960940
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902965078
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1902966477
