> On Sep 29, 2020, at 3:59 AM, Matthias Baesken <mbaes...@openjdk.java.net> > wrote: > > On Fri, 25 Sep 2020 02:23:07 GMT, David Holmes <dhol...@openjdk.org> wrote: > >>> Please review this small patch to enable the OSX build using Xcode 12.0. >>> >>> Thanks, >>> Paul >> >> src/hotspot/share/runtime/sharedRuntime.cpp line 2851: >> >>> 2849: if (buf != NULL) { >>> 2850: CodeBuffer buffer(buf); >>> 2851: short locs_buf[80]; >> >> This code is just weird. Why is the locs_buf array not declared to be the >> desired type? If the compiler rejects double >> because it isn't relocInfo* then why does it accept short? And if it accepts >> short will it accept int or long long or >> int64_t? Using int64_t would be a clearer change. Though semantically this >> code is awful. :( Should it be intptr_t ? > > Currently we have in the existing code various kind of buffers passed into > initialize_shared_locs that compile nicely > on all supported compilers and on Xcode 12 as well ,see > > c1_Compilation.cpp > > 326 char* locs_buffer = NEW_RESOURCE_ARRAY(char, locs_buffer_size); > 327 code->insts()->initialize_shared_locs((relocInfo*)locs_buffer, > > sharedRuntime.cpp > > 2681 CodeBuffer buffer(buf); > 2682 short buffer_locs[20]; > 2683 buffer.insts()->initialize_shared_locs((relocInfo*)buffer_locs, > 2684 > sizeof(buffer_locs)/sizeof(relocInfo)); > > So probably using short would be consistent to what we do already in the > coding at another place (looking into > relocInfo it would be probably better to use unsigned short or to typedef > unsigned short in the relocInfo class > and use the typedef).
That’s *not* consistent, because of buffer alignment. The call to NEW_RESOURCE_ARRAY is going to return a pointer to memory that is 2*word aligned. (Interesting, possibly a 32/64 bit “bug” here.) The existing code in sharedRuntime.cpp is allocating double-aligned. iniitalize_shared_locs wants a HeapWordSize-aligned buffer, and adjusts the pointer it uses accordingly. (I think with existing code it could just make the alignment of the buffer a precondition, but I haven’t looked at all callers.) Changing the declaration in sharedRuntime.cpp to short[] reduces the alignment requirement for the array, possibly requiring alignment adjustment (and hence size reduction) by initialize_shared_locs. Since initialize_shared_locs specifically adjusts the alignment, some downstream use of the buffer probably actually cares. > > ------------- > > PR: https://git.openjdk.java.net/jdk/pull/348