On 2015-11-30 05:23, David Holmes wrote:
Hi Sasha,
Trying to trace through this is somewhat complex :)
So ...
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.
More comments below ...
On 30/11/2015 12:39 PM, Alexander Smundak wrote:
Please review the patch set that fixes
https://bugs.openjdk.java.net/browse/JDK-8073139: PPC64: User-visible
arch directory and os.arch value on ppc64le cause issues with Java
tooling:
http://cr.openjdk.java.net/~asmundak/8073139/hotspot/webrev.02
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.
---
make/defs.make
See above discussion re ARCH etc.
---
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...)
http://cr.openjdk.java.net/~asmundak/8073139/jdk/webrev.02
No comments.
http://cr.openjdk.java.net/~asmundak/8073139/root/webrev.02
Again referring back to earlier ARCH discussion, I don't like seeing
the platform specific code in hotspot-spec.gmk.in.
I agree. No logic in the spec files, if it can at all be avoided.
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.
I'm not sure what would be the proper way to solve this, but as a start,
I'm not sure you need to really change any of the defines used to build
hotspot, as long as the build is correct. Instead, if you need to update
os.arch, maybe you should just add a check for endianness if you're
running on ppc64 where os.arch is set?
/Magnus
Thanks,
David
The patch is based on the patch by Andrew Hughes
(gnu.and...@redhat.com),
please see the thread
http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-February/017148.html
The current patch set adds the changes to show the correct
architecture in the
'java -Xinternalversion' output and test_env.sh (both pointed by
goetz.lindenma...@sap.com), and fixes Serviceability Agent and
disassembler (hsdis).
I need a sponsor.
Sasha
