RFR: 8073139 PPC64: User-visible arch directory and os.arch value on ppc64le cause issues with Java tooling

[David Holmes]

On 1/12/2015 1:31 PM, Alexander Smundak wrote:
>> On 2015-11-30 05:23, David Holmes wrote:
>>> ..
>>> At the top level if we see ppc64le then we set VAR_CPU to ppc64le instead
>>> of ppc64. However, once we get into hotspot build we want ARCH to be ppc64
>>> again (in hotspot-spec.gmk.in) - why is that?
>>>
>>> Inside hotpot we want:
>>>
>>> SRCARCH := ppc
>>> BUILDARCH := ppc64
>>> LIBARCH := ppc64le
>>>
>>> right? So can ARCH not be ppc64le from the top-level and then we adjust
>>> SRCARCH and BUILDARCH? And avoid the top-level changes to ARCH.
> The new revision does that:
> http://cr.openjdk.java.net/~asmundak/8073139/root/webrev.03/

Looks good.

> http://cr.openjdk.java.net/~asmundak/8073139/jdk/webrev.03

Looks good.

> http://cr.openjdk.java.net/~asmundak/8073139/hotspot/webrev.03

agent code:

I'm still unclear why you can't, or shouldn't, pass through -Dppc64 and 
-Dppc64le, such that you don't need to check for either being defined ??

---

src/share/vm/runtime/vm_version.cpp

Just a follow up note that I was surprised to see CPU not being set. 
This code needs cleaning up in the future as we definitely do not need 
this additional place that defines architecture names.

Thanks,
David
-----

>
> common/autoconf/hotspot-spec.gmk.in has been left alone, and
> hotspot/make/defs.make sets BUILDARCH and LIBARCH
> (BUILDARCH should be the same for ppc64 and ppc64le because the file
> linux/makefiels/$)BUILDARCH).make is pulled in by vm.make)
> I have also reduced the changeset for hotspot by making
> sun.jvm.hotspot.utilities.PlatformInfo.getCPU() return "ppc64" both
> when
> "os.arch" propertie's value is "ppc64" and "ppc64le".
>
>>>
>>> agent/src/os/linux/LinuxDebuggerLocal.c
>>> agent/src/os/linux/libproc.h
>>>
>>> Is it not the case that ppc64le -> ppc64, so that we can avoid "if
>>> defined(ppc64) | defined(ppc64le) ? I would hope that the only places in the
>>> sources where you need to check for ppc64le is where that code differs from
>>> the ppc64 code.
> This is defined as -D$(OPENJDK_TARGET_CPU_LEGACY), but the same variable
> is used in the generated hotspot-spec.gmk file, so I decided it's
> easier to fix the
> SA code.
>
>>> src/share/vm/runtime/vm_version.cpp
>>>
>>> I think this messy code block relates to builds where CPU is not set -
>>> which should never be the case these days. Maybe just put in a check "ifndef
>>> CPU -> error" ?
>>
>> I agree. There might be a case for handling zero, though. If I remember
>> correctly the hotspot build might not set CPU, or set it incorrectly, when
>> building zero. (Then again, zero is a strange beast, a mix between a variant
>> and a platform...)
>
> I have listed platform-specific options for building Hotspot above,
> but here's the full command line to build vm_version.o for ppc64le:
>
> /usr/bin/g++ -DLINUX -D_GNU_SOURCE -DPPC64 -DPRODUCT -I.
> -I<JDK9>/hotspot/src/share/vm/prims -I<JDK9>/hotspot/src/share/vm
> -I<JDK9>/hotspot/src/share/vm/precompiled
> -I<JDK9>/hotspot/src/cpu/ppc/vm
> -I<JDK9>/hotspot/src/os_cpu/linux_ppc/vm
> -I<JDK9>/hotspot/src/os/linux/vm -I<JDK9>/hotspot/src/os/posix/vm
> -I../generated -DHOTSPOT_BUILD_USER="\"asmundak\""
> -DHOTSPOT_LIB_ARCH=\"ppc64le\" -DHOTSPOT_VM_DISTRO="\"OpenJDK\""
> -DHOTSPOT_RELEASE_VERSION="\"1.9.0-internal-asmundak_2015_11_26_17_11-b00\""
> -DJRE_RELEASE_VERSION="\"1.9.0-internal-asmundak_2015_11_26_17_11-b00\""
> -DJDK_MAJOR_VERSION="\"1\"" -DJDK_MINOR_VERSION="\"9\""
> -DJDK_MICRO_VERSION="\"0\"" -DJDK_BUILD_NUMBER="\"b00\""
> -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_ppc -DTARGET_ARCH_MODEL_ppc_64
> -DTARGET_OS_ARCH_linux_ppc -DTARGET_OS_ARCH_MODEL_linux_ppc_64
> -DTARGET_COMPILER_gcc -DCOMPILER2 -DINCLUDE_JVMCI=0 -fPIC -fno-rtti
> -fno-exceptions -D_REENTRANT -fcheck-new -fvisibility=hidden -m64
> -pipe -fno-strict-aliasing -fno-omit-frame-pointer -O3   -g -D_LP64=1
> -DVM_LITTLE_ENDIAN -DABI_ELFv2 -mcpu=power7 -mtune=power8
> -minsert-sched-nops=regroup_exact -mno-multiple -mno-string -Werror
> -Wpointer-arith -Wsign-compare -Wundef -Wunused-function
> -Wunused-value -Wformat=2 -Wreturn-type -Woverloaded-virtual
> -Wtype-limits -Wno-format-zero-length -Wuninitialized   -c -MMD -MP
> -MF ../generated/dependencies/vm_version.o.d -fpch-deps -o
> vm_version.o <JDK9>/hotspot/src/share/vm/runtime/vm_version.cpp
> (I have replaced the actual path to the source tree with <JDK9>)
>
> Preprocessor variable 'CPU' is not defined on the command line, and if I add
> #error CPU is not defined
> right after
> #fndef CPU
> on line 193 in vm_version.cpp,
> it will fail to compile at least on Linux ppc64le and Linux x86_64.
>
>> I also think something's very weird here. OPENJDK_TARGET_CPU_ARCH is defined
>> to ppc for both ppc64 and the new ppc64le platform, that it, it explicitely
>> excludes address size. I think it's reasonable that it excludes endianness
>> as well, but we have not really had to make that discussion before. In any
>> case, it should not contain "64", otherwise the value will be "ppc" for
>> ppc64 and "ppc64le" for ppc64le, and that's just plain wrong.
> With the proposed patches the generated spec.gmk will contain
>     OPENJDK_TARGET_CPU_OSARCH:=ppc64le
> when building JDK for the little endian and ppc64 for the big endian.
> "os.arch" property is set from ARCHPROPNAME preprocessor variable,
> whose value is $(OPENJDK_TARGET_CPU_ARCH). Did I misunderstand
> something?
>