Hi, On 6/12/19 10:35 AM, Nick Gasson wrote:
> Sorry for the delay, I've put an updated webrev here: > > http://cr.openjdk.java.net/~ngasson/8224851/webrev.1/ > > Changes since the last patch: > > * Replaced ~((1<<12) - 1) with -1u<<12 > > * Use __atomic_add_fetch in Atomic::PlatformAdd #ifdef __clang__ > > * Use __builtin_frame_address(0) in os::current_frame() > > * Use placement new / copy assignment in pf() > > I also replaced the call to _get_previous_fp() in os::current_frame() with > > *(intptr_t **)__builtin_frame_address(0); > > As it generates the same code and avoids the `register intptr_t **fp > __asm__ (SPELL_REG_FP);' declaration which clang doesn't support. Also > the following comment in _get_previous_fp seems to be wrong: > > // fp is for this frame (_get_previous_fp). We want the fp for the > // caller of os::current_frame*(), so go up two frames. However, for > // optimized builds, _get_previous_fp() will be inlined, so only go > // up 1 frame in that case. > #ifdef _NMT_NOINLINE_ > return **(intptr_t***)fp; > #else > return *fp; > #endif > > Even on -O0 gcc won't generate a stack frame for this function so if > _NMT_NOINLINE_ is defined you get the caller's caller's FP rather than > the caller's FP. I just deleted the function as it's not used anywhere else. > > BTW we can't use __builtin_frame_address(1) as GCC gives a warning when > this is called with a non-zero argument (-Wframe-address). > > Tested jtreg hotspot_all_no_apps and jdk_core (gcc 7.3). OK, that's all good. Thanks. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> https://keybase.io/andrewhaley EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
