From 4f7ca1ec8a99c43de0a9315cca61408ea9d46842 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Feb 2024 11:16:38 -0800 Subject: [PATCH 1/3] [NFC] Assert we do not use --low-memory-unused when STACK_FIRST --- test/test_other.py | 23 +++++++++++++++++++++-- tools/link.py | 3 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index acc284d42c1b..379e659a1322 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -63,12 +63,12 @@ def uses_canonical_tmp(func): test to satisfy the leak detector. """ @wraps(func) - def decorated(self): + def decorated(self, *args, **kwargs): # Before running the test completely remove the canonical_tmp if os.path.exists(self.canonical_temp_dir): shutil.rmtree(self.canonical_temp_dir) try: - func(self) + func(self, *args, **kwargs) finally: # Make sure the test isn't lying about the fact that it uses # canonical_tmp @@ -8194,6 +8194,25 @@ def test_binaryen_ignore_implicit_traps(self): # sizes must be different, as the flag has an impact self.assertEqual(len(set(sizes)), 2) + @uses_canonical_tmp + @parameterized({ + # In a simple -O0 build we do not set --low-memory-unused (as the stack is + # first, which is nice for debugging but bad for code size (larger globals) + # and bad for the low-memory-unused trick. + '': ([], False), + # When we optimize, we do. + 'O2': (['-O2'], True), + # But a low global base prevents it. + 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=512'], False), + # A large-enough global base allows it. + 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=1024'], True), + }) + def test_binaryen_low_memory_unused(self, args, low_memory_unused): + with env_modify({'EMCC_DEBUG': '1'}): + cmd = [EMCC, test_file('hello_world.c')] + args + err = self.run_process(cmd, stdout=PIPE, stderr=PIPE).stderr + self.assertContainedIf('--low-memory-unused ', err, low_memory_unused) + def test_binaryen_passes_extra(self): def build(args): return self.run_process([EMCC, test_file('hello_world.c'), '-O3'] + args, stdout=PIPE).stdout diff --git a/tools/link.py b/tools/link.py index 48b517ecfecb..ecc2b9518a3d 100644 --- a/tools/link.py +++ b/tools/link.py @@ -360,6 +360,9 @@ def get_binaryen_passes(memfile): # hardcoded value in the binaryen pass) if optimizing and settings.GLOBAL_BASE >= 1024: passes += ['--low-memory-unused'] + # we cannot do this if the stack is first, but we only put it first if not + # optimizing + assert not settings.STACK_FIRST if settings.AUTODEBUG: # adding '--flatten' here may make these even more effective passes += ['--instrument-locals'] From f879661018722dfa0f4f84e830559020bb72e762 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Feb 2024 12:29:27 -0800 Subject: [PATCH 2/3] splify --- test/test_other.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 379e659a1322..7effdf32a6b9 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -8194,7 +8194,6 @@ def test_binaryen_ignore_implicit_traps(self): # sizes must be different, as the flag has an impact self.assertEqual(len(set(sizes)), 2) - @uses_canonical_tmp @parameterized({ # In a simple -O0 build we do not set --low-memory-unused (as the stack is # first, which is nice for debugging but bad for code size (larger globals) @@ -8205,13 +8204,12 @@ def test_binaryen_ignore_implicit_traps(self): # But a low global base prevents it. 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=512'], False), # A large-enough global base allows it. - 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=1024'], True), + 'O2_GB_1024': (['-O2', '-sGLOBAL_BASE=1024'], True), }) def test_binaryen_low_memory_unused(self, args, low_memory_unused): - with env_modify({'EMCC_DEBUG': '1'}): - cmd = [EMCC, test_file('hello_world.c')] + args - err = self.run_process(cmd, stdout=PIPE, stderr=PIPE).stderr - self.assertContainedIf('--low-memory-unused ', err, low_memory_unused) + cmd = [EMCC, test_file('hello_world.c'), '-v'] + args + err = self.run_process(cmd, stdout=PIPE, stderr=PIPE).stderr + self.assertContainedIf('--low-memory-unused ', err, low_memory_unused) def test_binaryen_passes_extra(self): def build(args): From bd6fa4d15e8e4bcb714b068e704d0ac1a9a23d62 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Feb 2024 15:39:12 -0800 Subject: [PATCH 3/3] read STACK_FIRST --- test/test_other.py | 36 +++++++++++++++++++----------------- tools/link.py | 8 +++----- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 7effdf32a6b9..81ec355aedf1 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -1206,6 +1206,25 @@ def test_wl_stackfirst(self): err = self.expect_fail(cmd + ['-sGLOBAL_BASE=1024']) self.assertContained('error: --stack-first is not compatible with -sGLOBAL_BASE', err) + @parameterized({ + # In a simple -O0 build we do not set --low-memory-unused (as the stack is + # first, which is nice for debugging but bad for code size (larger globals) + # and bad for the low-memory-unused trick. + '': ([], False), + # When we optimize, we do. + 'O2': (['-O2'], True), + # But a low global base prevents it. + 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=512'], False), + # A large-enough global base allows it. + 'O2_GB_1024': (['-O2', '-sGLOBAL_BASE=1024'], True), + # Forcing the stack to be first in the linker prevents it. + 'linker_flag': (['-O2', '-Wl,--stack-first'], False), + }) + def test_binaryen_low_memory_unused(self, args, low_memory_unused): + cmd = [EMCC, test_file('hello_world.c'), '-v'] + args + err = self.run_process(cmd, stdout=PIPE, stderr=PIPE).stderr + self.assertContainedIf('--low-memory-unused ', err, low_memory_unused) + def test_l_link(self): # Linking with -lLIBNAME and -L/DIRNAME should work, also should work with spaces create_file('main.c', ''' @@ -8194,23 +8213,6 @@ def test_binaryen_ignore_implicit_traps(self): # sizes must be different, as the flag has an impact self.assertEqual(len(set(sizes)), 2) - @parameterized({ - # In a simple -O0 build we do not set --low-memory-unused (as the stack is - # first, which is nice for debugging but bad for code size (larger globals) - # and bad for the low-memory-unused trick. - '': ([], False), - # When we optimize, we do. - 'O2': (['-O2'], True), - # But a low global base prevents it. - 'O2_GB_512': (['-O2', '-sGLOBAL_BASE=512'], False), - # A large-enough global base allows it. - 'O2_GB_1024': (['-O2', '-sGLOBAL_BASE=1024'], True), - }) - def test_binaryen_low_memory_unused(self, args, low_memory_unused): - cmd = [EMCC, test_file('hello_world.c'), '-v'] + args - err = self.run_process(cmd, stdout=PIPE, stderr=PIPE).stderr - self.assertContainedIf('--low-memory-unused ', err, low_memory_unused) - def test_binaryen_passes_extra(self): def build(args): return self.run_process([EMCC, test_file('hello_world.c'), '-O3'] + args, stdout=PIPE).stdout diff --git a/tools/link.py b/tools/link.py index ecc2b9518a3d..24bad45f443c 100644 --- a/tools/link.py +++ b/tools/link.py @@ -357,12 +357,10 @@ def get_binaryen_passes(memfile): if optimizing: passes += [building.opt_level_to_str(settings.OPT_LEVEL, settings.SHRINK_LEVEL)] # when optimizing, use the fact that low memory is never used (1024 is a - # hardcoded value in the binaryen pass) - if optimizing and settings.GLOBAL_BASE >= 1024: + # hardcoded value in the binaryen pass). we also cannot do it when the stack + # is first, as then the stack is in the low memory that should be unused. + if optimizing and settings.GLOBAL_BASE >= 1024 and not settings.STACK_FIRST: passes += ['--low-memory-unused'] - # we cannot do this if the stack is first, but we only put it first if not - # optimizing - assert not settings.STACK_FIRST if settings.AUTODEBUG: # adding '--flatten' here may make these even more effective passes += ['--instrument-locals']