Skip to content

Commit

Permalink
[NFC] Assert we do not use --low-memory-unused when STACK_FIRST (#21291)
Browse files Browse the repository at this point in the history
Assuming low memory was unused when the stack was first - which is where low
memory is - would be wrong. We never did that wrong thing as we only use that
optimization when not optimizing. Add an assertion + tests to prevent regressions.
  • Loading branch information
kripken committed Feb 8, 2024
1 parent 7bba399 commit 7080012
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
23 changes: 21 additions & 2 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', '''
Expand Down
5 changes: 3 additions & 2 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,9 @@ 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']
if settings.AUTODEBUG:
# adding '--flatten' here may make these even more effective
Expand Down

0 comments on commit 7080012

Please sign in to comment.