[vectorIntrinsics] RFR: 8260668: vectorIntrinsics windows build problem [v2]
Sandhya Viswanathan
sviswanathan at openjdk.java.net
Mon Feb 1 21:23:09 UTC 2021
On Mon, 1 Feb 2021 13:07:44 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
>> Sandhya Viswanathan has updated the pull request incrementally with one additional commit since the last revision:
>>
>> review comments
>
> make/autoconf/toolchain.m4 line 699:
>
>> 697: AS="$CC -c"
>> 698: else
>> 699: if test "x$OPENJDK_TARGET_CPU" = "xx86_64"; then
>
> Is the proper test for x86_64 or for 64-bit CPUs? Or phrased differently, what is the name of the executable on aarch64? The comment below states that the test is for word size, but the test itself is for CPU architecture.
Changed to:
if test "x$OPENJDK_TARGET_CPU_BITS" = "x64"; then
> src/hotspot/os_cpu/windows_x86/globals_vectorApiSupport_windows.hpp line 1:
>
>> 1: ; Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
>
> Maybe you have already established this as a new convention, but I think it is highly confusing to have a file named `.hpp` which is not a C++ header file! I expect IDEs etc will get confused too, and complain about syntax errors. Recommend renaming it to something different. If no conventional suffix exists for assembly language includes, perhaps just name it `.s.inc`?
Renamed to .s.inc
> make/common/NativeCompilation.gmk line 442:
>
>> 440: $$($1_COMPILER) $$($1_FLAGS) \
>> 441: $(CC_OUT_OPTION)$$($1_OBJ) /nologo /c /Ta $$($1_SRC_FILE) 2>&1 \
>> 442: | $(TR) -d '\r' | $(GREP) -v -e "Assembling:" || test "$$$$?" = "1" ;))
>
> Please use the `-nologo -c -Ta` format for the options.
done
-------------
PR: https://git.openjdk.java.net/panama-vector/pull/34
More information about the panama-dev
mailing list