RFR (M): Enable HotSpot build with Clang

Thu May 23 05:05:43 PDT 2013

On Thu, May 23, 2013 at 1:13 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:

>  On 5/23/2013 12:01 AM, Christian Thalinger wrote:
>
>
>  On May 22, 2013, at 1:41 PM, Volker Simonis <volker.simonis at gmail.com>
> wrote:
>
>  Hi,
>
> I'd like to propose the following change:
>
> http://cr.openjdk.java.net/~simonis/webrevs/hotspot_clang.v1/
>
>   -#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)+#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) && (__GNUC_MINOR__ > 2))) || defined(__clang__) || __has_attribute(visibility)
>
>
>  clang generally supports __has_attribute.  So it doesn't support the
> visibility attribute?
>
>
> I hope you can revert these changes to the jni_x.hpp files because we've
> finally just gotten the jdk and jvm versions to match.     Otherwise,
> please contribute the same change to the openjdk version of these files
> also.
>
>
Christian is absolutely right - these changes aren’t needed. Actually,
"__has_attribute()" is a clang feature and currently supported by clang
only.

I've prepared a new webrev with the correct bug ID and the changes in
jni_x.hpp removed:

http://cr.openjdk.java.net/~simonis/webrevs/8015252/

> Does this have a high probability of breaking?   Should it be added to the
> JPRT build?
>
>

I don't think these changes will have any impact at all on the normal GCC
build. Actually the only code changes contained in the patch are in
src/os_cpu/linux_x86/vm/linux_x86_32.s:

-        .=.+8+        .space 8

because clang doesn't support the ".=.+8" syntax, but according to the GNU
Assembler Manual (e.g.
http://www.sourceware.org/binutils/docs-2.12/as.info/Dot.html) "..the
expression .=.+4 is the same as saying .space 4.." (and I've verified that
it builds with gcc).

Or do you mean if there's a high probability that the Clang build will
break over time?
I'm not sure, but if it will be possible to add Clang builds to JPRT I
think it would be definitely worth doing so. The compiler may catch
errors/warnngs not detected by GCC and as I wrote, we may gradually
re-enable some of the warnings which are currently switched off for Clang
to clean up the code.

>
> http://cr.openjdk.java.net/~simonis/webrevs/hotspot_clang.v1/src/share/vm/memory/allocation.hpp.udiff.html
>
> I don't know why we have NOINLINE here.   It's not for NMT.
>
>
Don't know either, but it was introduced by " 6995781: Native Memory
Tracking (Phase 1"

These changes look fine to me.
>
>
Great, thanks!

Can you or Christian then please push them.

Regards,
Volker

> Coleen
>
>
>
>
>  Otherwise this change looks good.  It's a pity that we can't reuse the
> gcc files.
>
>  -- Chris
>
>  It would be great if somebody could open a BugID for it. Following some
> details:
>
> With this patch it becomes possible to build the HotSpot VM with the clang<http://clang.llvm.org/>,
> the "C language family frontend for LLVM". The changes have been tested
> with clang 3.0 and 3.2 on 64-bit Linux/Intel (Ubuntu 12.04) building both
> 32-bit and 64-bit HotSpot VMs. The clang-build can be simply triggered by
> setting USE_CLANG=true on the HotSpot build command line (in hotspot/make):
>
>
> make jvmg USE_CLANG=true ALT_BOOTDIR=/share/software/Java/jdk1.7.0 ARCH_DATA_MODEL=32 HOTSPOT_BUILD_JOBS=8
> make product USE_CLANG=true ALT_BOOTDIR=/share/software/Java/jdk1.7.0 ARCH_DATA_MODEL=64 HOTSPOT_BUILD_JOBS=8
> make product USE_CLANG=true ALT_BOOTDIR=/share/software/Java/jdk1.7.0 ARCH_DATA_MODEL=64 HOTSPOT_BUILD_JOBS=8 USE_PRECOMPILED_HEADER=0
>
> What is it good for?
>
> The clang compiler provides very good diagnostic messages and warnings.
> Currently all the following warnings are switched off in order to
> successfully build the HotSpot with -Werror:
>
> -Wno-unused-value -Wno-logical-op-parentheses -Wno-parentheses-equality -Wno-parentheses
> -Wno-switch -Wno-tautological-constant-out-of-range-compare -Wno-tautological-compare
> -Wno-delete-non-virtual-dtor -Wno-deprecated -Wno-format -Wno-dynamic-class-memaccess
> -Wno-return-type -Wno-empty-body
>
>  Gradually turning these warnings (or at least some of them) on and fixing
> the corresponding responsible source code locations may considerably
> improve the code quality and even reveal some real bugs.
>
> Another advantage of a clang build is the fact that clang is the default
> compiler on MacOS. So this can be also seen as a first step towards 7123056
> : Update compiler used in Mac 10.7 to clang<http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123056>.
>
> Implementation Details
>
> Currently the build will pick clang/clang++ from the current PATH much in
> the same way as this is done for the gcc build. It is therefore possible to
> test the build with different versions of clang by simply changing the
> PATH settings.
>
> The changes are minimal in the sense that they don't change anything in
> the current default build tool chain on Linux/Intel (i.e. without
> USE_CLANG=true nothing will change). The very few source code changes
> basically insert new code variants for clang in places where we already
> dispatched based on the compiler anyway.
>
> The clang build only supports precompiled headers in 64-bit builds because
> for 32-bit builds some files are compiled with -fPIC while others are
> compiled without (see NONPIC_OBJ_FILES in make/linux/makefiles/rules.make).
> But clang fails with an error if the PCH file was compiled with other
> options than the actual compilation unit. Therefore the default is to use
> PCHs for 64-bit builds (but this can be turned off with
> USE_PRECOMPILED_HEADER=0) and switch them off for 32-bit builds.
>
> Regards,
>
> Volker
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130523/1cb58f24/attachment-0001.html 