Skip to content

Restore -O3 default when we build with clang #351

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

Merged
merged 1 commit into from
Apr 26, 2013

Conversation

jeremy
Copy link
Member

@jeremy jeremy commented Apr 26, 2013

When we set the -Wno-error=shorten-64-to-32 cflags, Ruby says "oh, someone set cflags, I won't set any" and we lose its default -O3. That kills 1.9.3 and 2.0.0 perf.

Since clang can only build 1.9/2.0, we can safely assume that if we're adding the -Wno-error flag, we should also provide -O3 (-O2 is default for 1.8).

We set -O3 first since later -O options take precedence, so you can override with RUBY_CFLAGS="-Os" ...

sstephenson added a commit that referenced this pull request Apr 26, 2013
Restore -O3 default when we build with clang
@sstephenson sstephenson merged commit 2f8dcaf into rbenv:master Apr 26, 2013
@sstephenson
Copy link
Contributor

😁

@jeremy jeremy deleted the restore-o3-cflags branch April 26, 2013 23:20
@mroth
Copy link

mroth commented Apr 30, 2013

This is great! Could we please get a more recent v release version tag of the repository that contains it?

@rubemz
Copy link

rubemz commented Apr 30, 2013

+1

@sferik
Copy link
Contributor

sferik commented May 1, 2013

@mroth
Copy link

mroth commented May 1, 2013

@sferik excellent, thanks!

@SamSaffron
Copy link

I just grabbed source and did a 2.0.0-p0 install, ran my test suite and it half a minute slower than it is on a manual install of 2.0.0-p0

@jeremy
Copy link
Member Author

jeremy commented May 2, 2013

@SamSaffron did you diagnose why?

@SamSaffron
Copy link

not yet ... need to read through the logs, which means I need to reinstall

@jeremy
Copy link
Member Author

jeremy commented May 5, 2013

@SamSaffron you can check RbConfig::CONFIG['CC'] for your compiler and RbConfig::CONFIG['CFLAGS'] for -O3

@SamSaffron
Copy link

I am seeing:

config.status: creating Makefile
config.status: creating ruby-2.0.pc
CC = gcc
LD = ld
LDSHARED = gcc -shared
CFLAGS =
XCFLAGS = -include ruby/config.h -include ruby/missing.h -D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow -fvisibility=hidden -DRUBY_EXPORT -fPIE
CPPFLAGS = -I'/home/sam/.rbenv/versions/2.0.0-p0/include' -I. -I.ext/include/x86_64-linux -I./include -I.
DLDFLAGS = -fstack-protector -pie
SOLIBS =

something that is probably not that good.

where do I look up RbConfig::CONFIG['CFLAGS'] ?

@jeremy
Copy link
Member Author

jeremy commented May 6, 2013

That's Ruby code 😁

On Sunday, May 5, 2013, Sam wrote:

I am seeing:

config.status: creating Makefile
config.status: creating ruby-2.0.pc
CC = gcc
LD = ld
LDSHARED = gcc -shared
CFLAGS =
XCFLAGS = -include ruby/config.h -include ruby/missing.h
-D_FORTIFY_SOURCE=2 -fstack-protector -fno-strict-overflow
-fvisibility=hidden -DRUBY_EXPORT -fPIE
CPPFLAGS = -I'/home/sam/.rbenv/versions/2.0.0-p0/include' -I.
-I.ext/include/x86_64-linux -I./include -I.
DLDFLAGS = -fstack-protector -pie

SOLIBS =

something that is probably not that good.

where do I look up RbConfig::CONFIG['CFLAGS'] ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/351#issuecomment-17463131
.

@SamSaffron
Copy link

Yeah my manual install has:

irb(main):002:0> RbConfig::CONFIG['CFLAGS']
=> " -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wno-long-long -Wno-missing-field-initializers -Wunused-variable -Werror=pointer-arith -Werror=write-strings -Werror=declaration-after-statement -Werror=implicit-function-declaration"
irb(main):003:0>

Brand new ruby-build install

irb(main):004:0> RbConfig::CONFIG['CFLAGS']
=> " "


maybe related to me running autoconf prior to configuring local when I did it manually ?

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.

6 participants