Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/29303 )

Change subject: scons: Fix how partial linking is disabled.
......................................................................

scons: Fix how partial linking is disabled.

Setting disable_partial part way through the checks for various build
targets is incorrect and will affect targets based on the order they're
checked.

This change moves the check earlier, makes it consistent across all
builds whether fast is included or not, and stops passing it in as an
option to makeEnv since it now applies universally.

By disabling partial linking consistently, we avoid missing bugs where
only the "fast" version of gem5 doesn't build correctly because of the
multitude of g++ bugs having to do with combining LTO and partial
linking.

This also simplifies the logic in the SConscript by having fewer
independently moving parts.

Change-Id: Iff69f39868e948d3b9a5b11ea80bbfed19419b59
---
M src/SConscript
1 file changed, 17 insertions(+), 23 deletions(-)



diff --git a/src/SConscript b/src/SConscript
index 134e2a5..ce732fd 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1211,11 +1211,22 @@

 gem5_binary = Gem5('gem5')

+# Disable partial linking if mixing it with LTO is broken and LTO is enabled.
+disable_partial = (env.get('BROKEN_INCREMENTAL_LTO', False) and
+                   GetOption('force_lto'))
+if env['PLATFORM'] == 'darwin':
+    # Up until Apple LLVM version 10.0.0 (clang-1000.11.45.5), partial
+    # linked objects do not expose symbols that are marked with the
+    # hidden visibility and consequently building gem5 on Mac OS
+    # fails. As a workaround, we disable partial linking, however, we
+    # may want to revisit in the future.
+    disable_partial = True
+
 # Function to create a new build environment as clone of current
 # environment 'env' with modified object suffix and optional stripped
 # binary.  Additional keyword arguments are appended to corresponding
 # build environment vars.
-def makeEnv(env, label, objsfx, strip=False, disable_partial=False, **kwargs):
+def makeEnv(env, label, objsfx, strip=False, **kwargs):
# SCons doesn't know to append a library suffix when there is a '.' in the
     # name.  Use '_' instead.
     libname = 'gem5_' + label
@@ -1370,54 +1381,37 @@
 if 'all' in needed_envs:
     needed_envs += target_types

-disable_partial = False
-if env['PLATFORM'] == 'darwin':
-    # Up until Apple LLVM version 10.0.0 (clang-1000.11.45.5), partial
-    # linked objects do not expose symbols that are marked with the
-    # hidden visibility and consequently building gem5 on Mac OS
-    # fails. As a workaround, we disable partial linking, however, we
-    # may want to revisit in the future.
-    disable_partial = True
-
 # Debug binary
 if 'debug' in needed_envs:
     makeEnv(env, 'debug', '.do',
             CCFLAGS = Split(ccflags['debug']),
             CPPDEFINES = ['DEBUG', 'TRACING_ON=1'],
-            LINKFLAGS = Split(ldflags['debug']),
-            disable_partial=disable_partial)
+            LINKFLAGS = Split(ldflags['debug']))

 # Optimized binary
 if 'opt' in needed_envs:
     makeEnv(env, 'opt', '.o',
             CCFLAGS = Split(ccflags['opt']),
             CPPDEFINES = ['TRACING_ON=1'],
-            LINKFLAGS = Split(ldflags['opt']),
-            disable_partial=disable_partial)
+            LINKFLAGS = Split(ldflags['opt']))

 # "Fast" binary
 if 'fast' in needed_envs:
-    disable_partial = disable_partial or \
-            (env.get('BROKEN_INCREMENTAL_LTO', False) and \
-            GetOption('force_lto'))
     makeEnv(env, 'fast', '.fo', strip = True,
             CCFLAGS = Split(ccflags['fast']),
             CPPDEFINES = ['NDEBUG', 'TRACING_ON=0'],
-            LINKFLAGS = Split(ldflags['fast']),
-            disable_partial=disable_partial)
+            LINKFLAGS = Split(ldflags['fast']))

 # Profiled binary using gprof
 if 'prof' in needed_envs:
     makeEnv(env, 'prof', '.po',
             CCFLAGS = Split(ccflags['prof']),
             CPPDEFINES = ['NDEBUG', 'TRACING_ON=0'],
-            LINKFLAGS = Split(ldflags['prof']),
-            disable_partial=disable_partial)
+            LINKFLAGS = Split(ldflags['prof']))

 # Profiled binary using google-pprof
 if 'perf' in needed_envs:
     makeEnv(env, 'perf', '.gpo',
             CCFLAGS = Split(ccflags['perf']),
             CPPDEFINES = ['NDEBUG', 'TRACING_ON=0'],
-            LINKFLAGS = Split(ldflags['perf']),
-            disable_partial=disable_partial)
+            LINKFLAGS = Split(ldflags['perf']))

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29303
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Iff69f39868e948d3b9a5b11ea80bbfed19419b59
Gerrit-Change-Number: 29303
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to