Skip to content
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

Accept -O4 and larger values, clamping to -O3 and warning. #5520

Merged
merged 2 commits into from Aug 28, 2017

Conversation

etienne02
Copy link
Contributor

Do pull request #3711.

(Sorry for spaces modifications)

@@ -2331,7 +2331,7 @@ def test_llvm_nativizer(self):
output = Popen([os.path.join(self.get_dir(), 'files.o.run')], stdin=open(os.path.join(self.get_dir(), 'stdin')), stdout=PIPE, stderr=PIPE).communicate()
self.assertContained('''size: 37
data: 119,97,107,97,32,119,97,107,97,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35,35
loop: 119 97 107 97 32 119 97 107 97 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35 35
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test may depend on that whitespace

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe worth refactoring the test code so editors don't complain about this (add an explicit + ' ')

out, err = Popen([PYTHON, EMCC, '-O' + str(level), path_from_root('tests', 'hello_world.c')], stdout=PIPE, stderr=PIPE).communicate()
assert os.path.exists('a.out.js'), '-O' + str(level) + ' should produce output'
if level > 3:
self.assertContained("optimization level '-O" + str(level) + "' is not supported; using '-O3' instead", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be just 2 spaces

@kripken
Copy link
Member

kripken commented Aug 25, 2017

Thanks, looks good aside from minor comments.

@kripken
Copy link
Member

kripken commented Aug 28, 2017

Thanks!

@kripken kripken merged commit cd0fefe into emscripten-core:incoming Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants