Skip to content

Clang can also use C call cache #2628

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 29, 2019
Merged

Clang can also use C call cache #2628

merged 1 commit into from
Oct 29, 2019

Conversation

jhawthorn
Copy link
Member

@jhawthorn jhawthorn commented Oct 29, 2019 •
edited
Loading

Previously this was restricted to only gcc because of the GCC_VERSION_SINCE check (which explicitly excludes clang).

GCC 3.3.0 is quite old so I feel relatively safe assuming that all reasonable versions of clang support this.

EDIT: it seems to be compatible at least as far back as clang 3.0.0 (the oldest version on godbolt)

I get similar numbers to #2627 (comment), with the same benchmark

require "benchmark/ips"

list = ('a'..'z').to_a

Benchmark.ips do |x|
  x.report("grep") { list.grep('z') { 'z' } }
end

Warming up --------------------------------------
                grep    51.002k i/100ms
Calculating -------------------------------------
                grep    591.171k (± 4.9%) i/s -      2.958M in   5.017473s

Warming up --------------------------------------
                grep    70.983k i/100ms
Calculating -------------------------------------
                grep    856.779k (± 1.5%) i/s -      4.330M in   5.054946s

cc @tenderlove I don't know if you want to fold this into #2627 ❤️

@jhawthorn
Clang can also use C call cache
165f921 
Previously this was restricted to only gcc because of the
GCC_VERSION_SINCE check (which explicitly excludes clang).

GCC 3.3.0 is quite old so I feel relatively safe assuming that all
reasonable versions of clang support this.
@shyouhei
Copy link
Member

The GCC >= 3.3.0 restriction is for __attribute__((nonnull)). According to GCC manual (3.2 versus 3.3), that attribute seems implemented around then.

shyouhei
shyouhei approved these changes Oct 29, 2019
View reviewed changes
Copy link
Member

@shyouhei shyouhei left a comment

Choose a reason for hiding this comment

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

The patch itself looks good to me. 👍

@jhawthorn
Copy link
Member Author

jhawthorn commented Oct 29, 2019 •
edited
Loading

Digging a little further it looks like clang has supported this since late 2008 (clang 1.0 was first released in 2009) so I think we can safely assume all versions of clang support this 🎉

@nobu nobu merged commit b86e5c9 into ruby:master Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyouhei shyouhei shyouhei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jhawthorn @shyouhei @nobu