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

[Magnus Ihse Bursie]

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.andrew at 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.lindenmaier at sap.com), and fixes Serviceability Agent and
>>   disassembler (hsdis).
>>
>> I need a sponsor.
>>
>> Sasha
>>