-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Releases/m71 #3
base: master
Are you sure you want to change the base?
Releases/m71 #3
Changes from 50 commits
8c55557
de13037
8b4c5bf
29c9904
acffe89
9b0c7e4
b8cc22a
9851a7e
64a61fb
179bc02
6bbd16d
8564530
a1534f6
bb7e263
da87ccc
8057d7a
9603dd0
698ec30
dedb4ef
feea555
751c5ad
e8c23ed
1fc19f3
45c72c4
dfa8786
5b0d5bd
be194b2
fae04de
2e8dba4
c52de61
d5a2139
542b170
497094e
914501a
c835c66
813c5e3
d357f52
3579185
b5211f9
794296b
4557dc7
cba7ef1
f0ce0a7
9941608
170c225
0946a25
f80b444
8ba9407
b32cf47
9407778
ffa7641
a6ce039
4e120bb
4ea2218
0139ca4
916dd19
8d91370
f0e0893
ce61deb
5c4a1bc
495bba6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,11 +54,10 @@ if (current_os == target_os && current_cpu == target_cpu && | |
} else if (_script_arch == "x64") { | ||
_script_arch = "amd64" | ||
} | ||
assert( | ||
exec_script("//build/dir_exists.py", | ||
[ rebase_path(sysroot) ], | ||
"string") == "True", | ||
"Missing sysroot ($sysroot). To fix, run: build/linux/sysroot_scripts/install-sysroot.py --arch=$_script_arch") | ||
if (exec_script("//build/dir_exists.py", [ rebase_path(sysroot) ], "string") != "True") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this something that we'd be able to push back to google? If not, we will need to change this... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is added because of broken preparation for Linux x86. Not sure if this fix is still required. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you see if "official" has resolved this issue? |
||
arch_arg = "--arch=$_script_arch" | ||
exec_script("//build/linux/sysroot_scripts/install-sysroot-alt.py", [ arch_arg ]) | ||
} | ||
} | ||
} else if (is_mac) { | ||
import("//build/config/mac/mac_sdk.gni") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,7 +87,7 @@ config("compiler") { | |
if (is_clang) { | ||
cflags += [ "-fmsc-version=1911" ] | ||
|
||
if (current_cpu == "x86") { | ||
if (current_cpu == "x86" || current_cpu == "arm") { | ||
cflags += [ "-m32" ] | ||
} else { | ||
cflags += [ "-m64" ] | ||
|
@@ -243,7 +243,7 @@ config("runtime_library") { | |
"_SECURE_ATL", | ||
] | ||
|
||
if (!use_vs_code_analysis) { | ||
if ((!use_vs_code_analysis) && (current_os != "winuwp")) { | ||
# This is required for ATL to use XP-safe versions of its functions. | ||
# However it is prohibited when using /analyze | ||
defines += [ "_USING_V110_SDK71_" ] | ||
|
@@ -282,9 +282,20 @@ config("runtime_library") { | |
defines += [ "WINAPI_FAMILY=WINAPI_FAMILY_DESKTOP_APP" ] | ||
} | ||
cflags_cc += [ | ||
"/ZW", | ||
"/EHsc", | ||
"/std:c++17", | ||
"/Zc:__cplusplus", | ||
] | ||
|
||
#TEMPORARY!!! Clang-cl.exe missing unwind support for Windows arm and arm64. This will generate errors in places where try/catch are used | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please rephrase in a way that we can push back to google. It's okay to say this can be removed once clang-cl.exe has a feature but better to phrase properly for something we can push as I intend to push it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something that definitely shouldn't be pushed to Google. Added just to see what other issues exist for arm and arm64. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need it in such a way that I can submit to google. We don't want to have differences we can't submit back. |
||
if (is_clang && (current_cpu == "arm" || current_cpu == "arm64")) { | ||
cflags_cc += [ | ||
"/EHs-c-", | ||
] | ||
} else { | ||
cflags_cc += [ | ||
"/EHsc" | ||
] | ||
} | ||
|
||
# This warning is given because the linker cannot tell the difference | ||
# between consuming WinRT APIs versus authoring WinRT within static | ||
|
@@ -376,6 +387,20 @@ config("common_linker_setup") { | |
"/DYNAMICBASE", | ||
] | ||
|
||
# ASLR makes debugging with windbg difficult because Chrome.exe and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did you find this information about chrome? just wondering where it came from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is Google comment and it was present in older version of that file. I believe that is from m62. |
||
# Chrome.dll share the same base name. As result, windbg will name the | ||
# Chrome.dll module like chrome_<base address>, where <base address> | ||
# typically changes with each launch. This in turn means that breakpoints in | ||
# Chrome.dll don't stick from one launch to the next. For this reason, we | ||
# turn ASLR off in debug builds. | ||
if (is_debug) { | ||
if (current_cpu != "arm" && current_cpu != "arm64") { | ||
ldflags += [ "/DYNAMICBASE:NO" ] | ||
} | ||
} else { | ||
ldflags += [ "/DYNAMICBASE" ] | ||
} | ||
|
||
if (win_linker_timing) { | ||
ldflags += [ | ||
"/time", | ||
|
@@ -421,7 +446,7 @@ config("default_crt") { | |
configs = [ ":dynamic_crt" ] | ||
} else { | ||
# Desktop Windows: static CRT. | ||
configs = [ ":static_crt" ] | ||
configs = [ ":dynamic_crt" ] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment does not match implementation. Are you sure this must be dynamic? Seems google wants that value for a reason. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed back to static_crt and it is building correctly for win and winuwp There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No idea why it changed for winuwp; probably a reason but I don't know what it would be; it's been a LONG time like this... |
||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -171,24 +171,41 @@ template("msvc_toolchain") { | |
outputs = [ | ||
"$object_subdir/{{source_name_part}}.obj", | ||
] | ||
|
||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_c}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\"" | ||
if (target_name == "uwp_clang_arm" || target_name == "win_clang_arm") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make it an rspFile for all paths. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, not sure why one path has rspFile and others do not. Original does not have rspPath. |
||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_c}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\" --target=arm-windows-msvc /D_ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE" | ||
} else if (target_name == "uwp_clang_arm64" || target_name == "win_clang_arm64") { | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_c}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\" --target=aarch64-windows-msvc /DWindowsSDKDesktopARM64Support=1" | ||
} else { | ||
rspfile = "{{output}}.rsp" | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} @$rspfile /c {{source}} /Fo{{output}} /Fd\"$pdbname\"" | ||
rspfile_content = "$sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_c}}" | ||
} | ||
} | ||
|
||
tool("cxx") { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bogus space added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
precompiled_header_type = "msvc" | ||
|
||
# The PDB name needs to be different between C and C++ compiled files. | ||
pdbname = "{{target_out_dir}}/{{label_name}}_cc.pdb" | ||
|
||
# See comment in CC tool about quoting. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bogus space added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
depsformat = "msvc" | ||
description = "CXX {{output}}" | ||
outputs = [ | ||
"$object_subdir/{{source_name_part}}.obj", | ||
] | ||
|
||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\"" | ||
|
||
if (target_name == "uwp_clang_arm" || target_name == "win_clang_arm") { | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\" --target=arm-windows-msvc /D_ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE" | ||
} else if (target_name == "uwp_clang_arm64" || target_name == "win_clang_arm64") { | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} $sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}} /c {{source}} /Fo{{output}} /Fd\"$pdbname\" --target=aarch64-windows-msvc /DWindowsSDKDesktopARM64Support=1" | ||
} else { | ||
rspfile = "{{output}}.rsp" | ||
command = "$env_wrapper$cl /nologo /showIncludes ${clflags} @$rspfile /c {{source}} /Fo{{output}} /Fd\"$pdbname\"" | ||
rspfile_content = "$sys_include_flags{{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rsp consistency ? |
||
} | ||
} | ||
|
||
tool("rc") { | ||
|
@@ -203,10 +220,19 @@ template("msvc_toolchain") { | |
tool("asm") { | ||
if (toolchain_args.current_cpu == "x64") { | ||
ml = "ml64.exe" | ||
} else { | ||
} else if (toolchain_args.current_cpu == "x86") { | ||
ml = "ml.exe" | ||
} | ||
command = "$python_path $tool_wrapper_path asm-wrapper $env $ml {{defines}} {{include_dirs}} {{asmflags}} /c /Fo{{output}} {{source}}" | ||
else if (toolchain_args.current_cpu == "arm"){ | ||
ml = "armasm.exe" | ||
} else { | ||
ml = "armasm64.exe" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it needs to be an explicit check for arm64 in case another CPU is ever added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe assert "Unsupported CPU tartget" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
} | ||
if (toolchain_args.current_cpu == "x64" || toolchain_args.current_cpu == "x86") { | ||
command = "$python_path $tool_wrapper_path asm-wrapper $env $ml {{defines}} {{include_dirs}} {{asmflags}} /c /Fo{{output}} {{source}}" | ||
} else { | ||
command = "$python_path $tool_wrapper_path asm-wrapper $env $ml -oldit -16 {{include_dirs}} {{source}} -o {{output}}" | ||
} | ||
description = "ASM {{output}}" | ||
outputs = [ | ||
"$object_subdir/{{source_name_part}}.obj", | ||
|
@@ -338,44 +364,38 @@ if (host_os == "win") { | |
clang_cl = "clang-cl" | ||
} | ||
|
||
if (target_cpu == "x86" || target_cpu == "x64") { | ||
win_build_host_cpu = target_cpu | ||
} else { | ||
win_build_host_cpu = host_cpu | ||
} | ||
|
||
# x86, arm and arm64 build cpu toolchains for Windows (not WinUWP). Only | ||
# define when the build cpu is one of these architectures since we don't | ||
# do any cross compiles when targeting x64-bit (the build does generate | ||
# some 64-bit stuff from x86/arm/arm64 target builds). | ||
if (win_build_host_cpu != "x64") { | ||
if (target_cpu != "x64") { | ||
build_cpu_toolchain_data = exec_script("setup_toolchain.py", | ||
[ | ||
visual_studio_path, | ||
windows_sdk_path, | ||
visual_studio_runtime_dirs, | ||
host_os, | ||
win_build_host_cpu, | ||
"environment." + win_build_host_cpu, | ||
target_cpu, | ||
"environment." + target_cpu, | ||
], | ||
"scope") | ||
|
||
msvc_toolchain(win_build_host_cpu) { | ||
environment = "environment." + win_build_host_cpu | ||
msvc_toolchain(target_cpu) { | ||
environment = "environment." + target_cpu | ||
cl = "${goma_prefix}\"${build_cpu_toolchain_data.vc_bin_dir}/cl.exe\"" | ||
if (host_os != "win") { | ||
# For win cross build. | ||
sys_lib_flags = "${build_cpu_toolchain_data.libpath_flags}" | ||
} | ||
toolchain_args = { | ||
current_os = "win" | ||
current_cpu = win_build_host_cpu | ||
current_cpu = target_cpu | ||
is_clang = false | ||
} | ||
} | ||
|
||
msvc_toolchain("win_clang_" + win_build_host_cpu) { | ||
environment = "environment." + win_build_host_cpu | ||
msvc_toolchain("win_clang_" + target_cpu) { | ||
environment = "environment." + target_cpu | ||
prefix = rebase_path("$clang_base_path/bin", root_build_dir) | ||
cl = "${goma_prefix}$prefix/${clang_cl}" | ||
sys_include_flags = "${build_cpu_toolchain_data.include_flags_imsvc}" | ||
|
@@ -386,7 +406,7 @@ if (win_build_host_cpu != "x64") { | |
|
||
toolchain_args = { | ||
current_os = "win" | ||
current_cpu = win_build_host_cpu | ||
current_cpu = target_cpu | ||
is_clang = true | ||
} | ||
} | ||
|
@@ -486,4 +506,16 @@ if (target_os == "winuwp") { | |
is_clang = false | ||
} | ||
} | ||
msvc_toolchain("uwp_clang_" + target_cpu) { | ||
environment = "environment.store_" + target_cpu | ||
prefix = rebase_path("$clang_base_path/bin", root_build_dir) | ||
cl = "${goma_prefix}$prefix/${clang_cl}" | ||
sys_include_flags = "${store_cpu_toolchain_data.include_flags_imsvc}" | ||
|
||
toolchain_args = { | ||
current_os = "winuwp" | ||
current_cpu = target_cpu | ||
is_clang = true | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -276,7 +276,8 @@ def q(s): # Quote s if it contains spaces or other weird characters. | |
print 'vc_lib_path = ' + gn_helpers.ToGNString(vc_lib_path) | ||
if (target_store != True): | ||
# Path is assumed not to exist for desktop applications | ||
assert vc_lib_atlmfc_path | ||
if (cpu != 'arm' and cpu != 'arm64'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure this is ok for other non uwp builds. |
||
assert vc_lib_atlmfc_path | ||
# Possible atlmfc library path gets introduced in the future for store thus | ||
# output result if a result exists. | ||
if (vc_lib_atlmfc_path != ''): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,8 +190,12 @@ def _CopyUCRTRuntime(target_dir, source_dir, target_cpu, dll_pattern, suffix): | |
os.environ.get('WINDOWSSDKDIR', | ||
os.path.expandvars('%ProgramFiles(x86)%' | ||
'\\Windows Kits\\10'))) | ||
ucrt_dll_dirs = os.path.join(win_sdk_dir, 'Redist', 'ucrt', 'DLLs', | ||
target_cpu) | ||
if target_cpu != 'arm64': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd make this an explicit == 'arm64' then default to standard behaviour. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
ucrt_dll_dirs = os.path.join(win_sdk_dir, 'Redist', 'ucrt', 'DLLs', | ||
target_cpu) | ||
else: | ||
ucrt_dll_dirs = os.path.join(win_sdk_dir, 'Redist', 'ucrt', 'DLLs', | ||
'arm') | ||
ucrt_files = glob.glob(os.path.join(ucrt_dll_dirs, 'api-ms-win-*.dll')) | ||
assert len(ucrt_files) > 0 | ||
for ucrt_src_file in ucrt_files: | ||
|
@@ -234,10 +238,12 @@ def _CopyPGORuntime(target_dir, target_cpu): | |
if env_version == '2017': | ||
pgo_runtime_root = FindVCToolsRoot() | ||
assert pgo_runtime_root | ||
print 'pgo_runtime_root %s ' % (pgo_runtime_root) | ||
# There's no version of pgosweep.exe in HostX64/x86, so we use the copy | ||
# from HostX86/x86. | ||
pgo_x86_runtime_dir = os.path.join(pgo_runtime_root, 'HostX86', 'x86') | ||
pgo_x64_runtime_dir = os.path.join(pgo_runtime_root, 'HostX64', 'x64') | ||
pgo_arm_runtime_dir = os.path.join(pgo_runtime_root, 'arm') | ||
else: | ||
raise Exception('Unexpected toolchain version: %s.' % env_version) | ||
|
||
|
@@ -250,6 +256,8 @@ def _CopyPGORuntime(target_dir, target_cpu): | |
source = os.path.join(pgo_x86_runtime_dir, runtime) | ||
elif target_cpu == 'x64': | ||
source = os.path.join(pgo_x64_runtime_dir, runtime) | ||
elif target_cpu == 'arm': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No arm64 needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
source = os.path.join(pgo_arm_runtime_dir, runtime) | ||
else: | ||
raise NotImplementedError("Unexpected target_cpu value: " + target_cpu) | ||
if not os.path.exists(source): | ||
|
@@ -285,7 +293,8 @@ def CopyDlls(target_dir, configuration, target_cpu): | |
if configuration == 'Debug': | ||
_CopyRuntime(target_dir, runtime_dir, target_cpu, debug=True) | ||
else: | ||
_CopyPGORuntime(target_dir, target_cpu) | ||
if not (target_cpu in ('arm', 'arm64')): | ||
_CopyPGORuntime(target_dir, target_cpu) | ||
|
||
_CopyDebugger(target_dir, target_cpu) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about the "arm" -- seems to me this should only be arm64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added because of compiler warning 4267 (conversion from 'size_t' to 'type', possible loss of data). Warning is generated for arm and arm64 builds, so it should be added. It could be put in another config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but does it apply to arm? seems to me this is only arm64, no?