RFR (M): Enable HotSpot build with Clang

[Christian Thalinger]

On May 23, 2013, at 5:05 AM, Volker Simonis <volker.simonis at gmail.com> wrote:

> 
> 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.

Let me first add BSD support as well…  -- Chris

> 
> 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, 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.
>>> 
>>> 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/8171d7ec/attachment.html