diff --git a/set.c b/set.c index 203a0abfd1bb82..75b7708043b425 100644 --- a/set.c +++ b/set.c @@ -858,11 +858,19 @@ set_i_delete_if(VALUE set) /* * call-seq: - * reject! { |o| ... } -> self + * reject! {|element| ... } -> self or nil * reject! -> enumerator * - * Equivalent to Set#delete_if, but returns nil if no changes were made. - * Returns an enumerator if no block is given. + * With a block given, like #delete_if, but returns +nil+ if no changes were made: + * + * set = Set[*0..9] # => Set[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + * set.reject! {|element| element.even? } # => Set[1, 3, 5, 7, 9] + * set.reject! {|element| element.even? } # => nil + * set.reject! {|element| element.odd? } # => Set[] + * + * With no block given, returns an Enumerator. + * + * Related: see {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_reject(VALUE set) @@ -999,26 +1007,48 @@ static void set_merge_enum_into(VALUE set, VALUE arg); /* * call-seq: - * divide { |o1, o2| ... } -> set - * divide { |o| ... } -> set + * divide {|ele| ... } -> new_set + * divide {|ele0, ele1| ... } -> new_set * divide -> enumerator * - * Divides the set into a set of subsets according to the commonality - * defined by the given block. + * With a block given, returns a set of sets. * - * If the arity of the block is 2, elements o1 and o2 are in common - * if both block.call(o1, o2) and block.call(o2, o1) are true. - * Otherwise, elements o1 and o2 are in common if - * block.call(o1) == block.call(o2). + * For a block that accepts one argument, + * calls the block with each element; + * creates a set for each distinct block return value: * - * numbers = Set[1, 3, 4, 6, 9, 10, 11] - * set = numbers.divide { |i,j| (i - j).abs == 1 } - * set #=> Set[Set[1], - * # Set[3, 4], - * # Set[6], - * # Set[9, 10, 11]] + * set = Set[*0..9] + * # => Set[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + * # Divide into mod 3 sets. + * set.divide {|ele| ele % 3 } + * # => Set[Set[0, 3, 6, 9], Set[1, 4, 7], Set[2, 5, 8]] + * # Divide into mod 5 sets. + * set.divide {|ele| ele % 5 } + * # => Set[Set[0, 5], Set[1, 6], Set[2, 7], Set[3, 8], Set[4, 9]] * - * Returns an enumerator if no block is given. + * Set[0].divide {|ele| anything } # => Set[Set[0]] + * Set[].divide {|ele| not called } # => Set[] + * + * For a block that accepts two arguments, + * divides +self+ into connected components based on the binary + * relation defined by the block, calling the block with each 2-element + * permutation of the elements of +self+: + * + * set = Set[*0..9] + * # => Set[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + * # Divide into mod 2 sets. + * set.divide {|i, j| (i - j) % 2 == 0 } + * # => Set[Set[0, 2, 4, 6, 8], Set[1, 3, 5, 7, 9]] + * # Divide into mod 3 sets. + * set.divide {|i, j| (i - j) % 3 == 0 } + * # => Set[Set[0, 3, 6, 9], Set[1, 4, 7], Set[2, 5, 8]] + * + * Set[0].divide {|i, j| not called } # => Set[Set[0]] + * Set[].divide {|i, j| not called } # => Set[] + * + * With no block given, returns an Enumerator. + * + * Related: see {Methods for Converting}[rdoc-ref:Set@Methods+for+Converting]. */ static VALUE set_i_divide(VALUE set) @@ -1216,10 +1246,17 @@ set_merge_enum_into(VALUE set, VALUE arg) /* * call-seq: - * merge(*enums, **nil) -> self + * merge(*enumerables, **nil) -> self + * + * Adds each element of each of the given +enumerables+ to +self+; + * returns +self+: + * + * set = Set[*0..2] # => Set[0, 1, 2] + * set.merge('a'..'c', %w[foo bar]) # => Set[0, 1, 2, "a", "b", "c", "foo", "bar"] + * set.merge('a'..'c', %w[foo bar]) # => Set[0, 1, 2, "a", "b", "c", "foo", "bar"] + * + * Related: see {Methods for Assigning}[rdoc-ref:Set@Methods+for+Assigning]. * - * Merges the elements of the given enumerable objects to the set and - * returns self. */ static VALUE set_i_merge(int argc, VALUE *argv, VALUE set) @@ -1545,11 +1582,21 @@ set_keep_if_i(st_data_t key, st_data_t into) /* * call-seq: - * keep_if { |o| ... } -> self + * keep_if {|element| ... } -> self * keep_if -> enumerator * - * Deletes every element of the set for which block evaluates to false, and - * returns self. Returns an enumerator if no block is given. + * With a block given, + * calls the block with each element in +self+, + * deleting the element if the block returns +false+ or +nil+; + * returns +self+: + * + * set = Set[*0..9] # => Set[0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + * set.keep_if {|i| i.even? } # => Set[0, 2, 4, 6, 8] + * set.keep_if {|i| i.odd? } # => Set[] + * + * With no block given, returns an Enumerator. + * + * Related: see {Methods for Deleting}[rdoc-ref:Set@Methods+for+Deleting]. */ static VALUE set_i_keep_if(VALUE set) @@ -1585,14 +1632,15 @@ set_i_select(VALUE set) /* * call-seq: - * replace(enum) -> self + * replace(enumerable) -> self + * + * Replaces the contents +self+ with the contents of the given +enumerable+; + * returns +self+: * - * Replaces the contents of the set with the contents of the given - * enumerable object and returns self. + * set = Set[1, 'c', :s] # => Set[1, "c", :s] + * set.replace([1, 2]) # => Set[1, 2] * - * set = Set[1, 'c', :s] #=> Set[1, "c", :s] - * set.replace([1, 2]) #=> Set[1, 2] - * set #=> Set[1, 2] + * Related: see {Methods for Assigning}[rdoc-ref:Set@Methods+for+Assigning]. */ static VALUE set_i_replace(VALUE set, VALUE other) @@ -1621,8 +1669,22 @@ set_i_replace(VALUE set, VALUE other) * call-seq: * reset -> self * - * Resets the internal state after modification to existing elements - * and returns self. Elements will be reindexed and deduplicated. + * Resets the internal state of +self+; return +self+. + * + * A set relies on the #hash results of each element being consistent. + * Modifying an element in a way that changes the results of #hash + * may allow duplicate elements in the set: + * + * array = [1] + * set = Set[array] # => Set[[1]] + * array << 2 + * set.add(array) # => Set[[1, 2], [1, 2]] + * + * Calling #reset will recalculate all of the hash values and remove + * duplicate elements: + * + * set.reset # => Set[[1, 2]] + * */ static VALUE set_i_reset(VALUE set) @@ -1763,9 +1825,17 @@ set_le(VALUE set, VALUE other) /* * call-seq: - * proper_subset?(set) -> true or false + * proper_subset?(other_set) -> true or false + * + * Returns whether +self+ is + * a {proper subset}[https://en.wikipedia.org/wiki/Subset] + * of the given +other_set+: + * + * set = Set[*'b'..'e'] + * set.proper_subset?(set) # => false + * set.proper_subset?(Set[*'a'..'f']) # => true * - * Returns true if the set is a proper subset of the given set. + * Related: {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_proper_subset(VALUE set, VALUE other) @@ -1791,9 +1861,17 @@ set_i_subset(VALUE set, VALUE other) /* * call-seq: - * proper_superset?(set) -> true or false + * proper_superset?(other_set) -> true or false * - * Returns true if the set is a proper superset of the given set. + * Returns whether +self+ is + * a {proper superset}[https://en.wikipedia.org/wiki/Subset] + * of the given +other_set+: + * + * set = Set[*'a'..'f'] + * set.proper_superset?(set) # => false + * set.proper_superset?(Set[*'b'..'e']) # => true + * + * Related: {Methods for Querying}[rdoc-ref:Set@Methods+for+Querying]. */ static VALUE set_i_proper_superset(VALUE set, VALUE other) @@ -2015,7 +2093,12 @@ set_hash_i(st_data_t item, st_data_t(arg)) * call-seq: * hash -> integer * - * Returns hash code for set. + * Returns the integer hash value for +self+. + * + * Two sets with the same content have the same hash value. + * + * Set[0, 1].hash == Set[1, 0].hash # => true + * Set[0, 1].hash == Set[0].hash # => false */ static VALUE set_i_hash(VALUE set) @@ -2334,15 +2417,13 @@ rb_set_size(VALUE set) * === Methods for Converting * * - #classify: - * Returns a hash that classifies the elements, + * Returns a hash that partitions the elements, * as determined by the given block. * - #collect! (aliased as #map!): * Replaces each element with a block return-value. * - #divide: - * Returns a hash that classifies the elements, - * as determined by the given block; - * differs from #classify in that the block may accept - * either one or two arguments. + * Returns a set of sets that partition the elements, + * as determined by the given block. * - #flatten: * Returns a new set that is a recursive flattening of +self+. * - #flatten!: @@ -2367,7 +2448,7 @@ rb_set_size(VALUE set) * === Other Methods * * - #reset: - * Resets the internal state; useful if an object + * Resets the internal state; useful if an element * has been modified while an element in the set. * */ diff --git a/spec/bundler/commands/add_spec.rb b/spec/bundler/commands/add_spec.rb index 162650f2e5c578..130a50a9d1d214 100644 --- a/spec/bundler/commands/add_spec.rb +++ b/spec/bundler/commands/add_spec.rb @@ -153,9 +153,10 @@ describe "with --git and --ref" do it "adds dependency with specified git source and branch" do - bundle "add foo --git=#{lib_path("foo-2.0")} --ref=#{revision_for(lib_path("foo-2.0"))}" + ref = revision_for(lib_path("foo-2.0")) + bundle "add foo --git=#{lib_path("foo-2.0")} --ref=#{ref}" - expect(bundled_app_gemfile.read).to match(/gem "foo", ">= 2\.0", git: "#{lib_path("foo-2.0")}", ref: "#{revision_for(lib_path("foo-2.0"))}"/) + expect(bundled_app_gemfile.read).to match(/gem "foo", ">= 2\.0", git: "#{lib_path("foo-2.0")}", ref: "#{ref}"/) expect(the_bundle).to include_gems "foo 2.0" end end @@ -229,9 +230,10 @@ describe "with --git and --ref and --glob" do it "adds dependency with specified git source and branch" do - bundle "add foo --git=#{lib_path("foo-2.0")} --ref=#{revision_for(lib_path("foo-2.0"))} --glob='./*.gemspec'" + ref = revision_for(lib_path("foo-2.0")) + bundle "add foo --git=#{lib_path("foo-2.0")} --ref=#{ref} --glob='./*.gemspec'" - expect(bundled_app_gemfile.read).to match(%r{gem "foo", ">= 2\.0", git: "#{lib_path("foo-2.0")}", ref: "#{revision_for(lib_path("foo-2.0"))}", glob: "\./\*\.gemspec"}) + expect(bundled_app_gemfile.read).to match(%r{gem "foo", ">= 2\.0", git: "#{lib_path("foo-2.0")}", ref: "#{ref}", glob: "\./\*\.gemspec"}) expect(the_bundle).to include_gems "foo 2.0" end end @@ -397,7 +399,7 @@ def run describe "when a gem is added which is already specified in Gemfile with version" do it "shows an error when added with different version requirement" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "myrack", "1.0" G @@ -409,7 +411,7 @@ def run end it "shows error when added without version requirements" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "myrack", "1.0" G @@ -424,7 +426,7 @@ def run describe "when a gem is added which is already specified in Gemfile without version" do it "shows an error when added with different version requirement" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "myrack" G diff --git a/spec/bundler/commands/lock_spec.rb b/spec/bundler/commands/lock_spec.rb index 0a1f1f8902ba9e..4914c11cc3f8b5 100644 --- a/spec/bundler/commands/lock_spec.rb +++ b/spec/bundler/commands/lock_spec.rb @@ -704,8 +704,9 @@ build_gem "qux", %w[1.0.0 1.0.1 1.1.0 2.0.0] end - # establish a lockfile set to 1.4.3 - install_gemfile <<-G + # establish a lockfile set to 1.4.3 (these examples only assert on the + # generated lockfile, so resolve-and-lock without the install) + lock_gemfile <<-G source "https://gem.repo4" gem 'foo', '1.4.3' gem 'bar', '2.0.3' @@ -988,7 +989,7 @@ end simulate_platform "x86_64-darwin-22" do - install_gemfile <<~G + lock_gemfile <<~G source "https://gem.repo4" gem "nokogiri" diff --git a/spec/bundler/commands/newgem_spec.rb b/spec/bundler/commands/newgem_spec.rb index 65fbad05aa2590..9ef4d873573cce 100644 --- a/spec/bundler/commands/newgem_spec.rb +++ b/spec/bundler/commands/newgem_spec.rb @@ -43,9 +43,16 @@ def installed_go? let(:gem_name) { "mygem" } before do - git("config --global user.name 'Bundler User'") - git("config --global user.email user@example.com") - git("config --global github.user bundleuser") + # Write the global git config directly instead of shelling out to `git + # config --global` three times per example: this `before` runs for every + # example in the file, and each `git` call is a separate subprocess. + File.write(home(".gitconfig"), <<~GITCONFIG) + [user] + name = Bundler User + email = user@example.com + [github] + user = bundleuser + GITCONFIG bundle_config_global "gem.mit false" bundle_config_global "gem.test false" diff --git a/spec/bundler/commands/outdated_spec.rb b/spec/bundler/commands/outdated_spec.rb index 28ed51d61e5b6a..dea78ba83f0c5d 100644 --- a/spec/bundler/commands/outdated_spec.rb +++ b/spec/bundler/commands/outdated_spec.rb @@ -86,8 +86,10 @@ end G - update_repo2 { build_gem "activesupport", "3.0" } - update_repo2 { build_gem "terranova", "9" } + update_repo2 do + build_gem "activesupport", "3.0" + build_gem "terranova", "9" + end bundle "outdated", raise_on_error: false @@ -122,8 +124,10 @@ it "shows the location of the latest version's gemspec if installed" do bundle_config "clean false" - update_repo2 { build_gem "activesupport", "3.0" } - update_repo2 { build_gem "terranova", "9" } + update_repo2 do + build_gem "activesupport", "3.0" + build_gem "terranova", "9" + end install_gemfile <<-G source "https://gem.repo2" diff --git a/spec/bundler/install/gemfile/path_spec.rb b/spec/bundler/install/gemfile/path_spec.rb index b0694885314887..51579c6cb16cc7 100644 --- a/spec/bundler/install/gemfile/path_spec.rb +++ b/spec/bundler/install/gemfile/path_spec.rb @@ -355,7 +355,7 @@ s.add_dependency "graphql", "~> 2.0" end - bundle "install", dir: lib_path("foo") + bundle "lock", dir: lib_path("foo") expect(lockfile_path).to read_as(original_lockfile.gsub("foo (0.1.0)", "foo (0.1.1)")) end @@ -512,8 +512,6 @@ G expect(the_bundle).to include_gems "foo 1.0" - - expect(the_bundle).to include_gems "foo 1.0" end it "works when the path does not have a gemspec but there is a lockfile" do diff --git a/spec/bundler/install/gemfile/specific_platform_spec.rb b/spec/bundler/install/gemfile/specific_platform_spec.rb index ab194dbcc52dbc..65af91a35cfc79 100644 --- a/spec/bundler/install/gemfile/specific_platform_spec.rb +++ b/spec/bundler/install/gemfile/specific_platform_spec.rb @@ -558,7 +558,7 @@ it "adds the foreign platform" do simulate_platform "x86_64-darwin-15" do setup_multiplatform_gem - install_gemfile(google_protobuf) + lock_gemfile(google_protobuf) bundle "lock --add-platform=x64-mingw-ucrt" expect(the_bundle.locked_platforms).to include("x64-mingw-ucrt", "universal-darwin") @@ -572,7 +572,7 @@ it "falls back on plain ruby when that version doesn't have a platform-specific gem" do simulate_platform "x86_64-darwin-15" do setup_multiplatform_gem - install_gemfile(google_protobuf) + lock_gemfile(google_protobuf) bundle "lock --add-platform=java" expect(the_bundle.locked_platforms).to include("java", "universal-darwin") @@ -938,7 +938,7 @@ end simulate_platform "x86_64-linux" do - install_gemfile <<~G + lock_gemfile <<~G source "https://gem.repo4" gem "native_tool" diff --git a/spec/bundler/install/gems/dependency_api_spec.rb b/spec/bundler/install/gems/dependency_api_spec.rb index 32a1b98b6d04f5..9f28bfcebe79b3 100644 --- a/spec/bundler/install/gems/dependency_api_spec.rb +++ b/spec/bundler/install/gems/dependency_api_spec.rb @@ -157,7 +157,6 @@ "actionpack 2.3.2", "actionmailer 2.3.2", "activeresource 2.3.2", - "activesupport 2.3.2", "thin 1.0.0", "myrack 1.0.0", "rails 2.3.2" diff --git a/spec/bundler/lock/lockfile_spec.rb b/spec/bundler/lock/lockfile_spec.rb index 0a2aa8aca85235..84b74823d47759 100644 --- a/spec/bundler/lock/lockfile_spec.rb +++ b/spec/bundler/lock/lockfile_spec.rb @@ -10,7 +10,7 @@ c.checksum(gem_repo2, "myrack", "1.0.0") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "myrack" @@ -149,7 +149,7 @@ myrack L - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "myrack", "> 0" @@ -220,7 +220,7 @@ end it "generates a simple lockfile for a single source, gem with dependencies" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack-obama" @@ -251,7 +251,7 @@ end it "generates a simple lockfile for a single source, gem with a version requirement" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack-obama", ">= 1.0" @@ -477,7 +477,7 @@ it "generates a simple lockfile for a single pinned source, gem with a version requirement" do git = build_git "foo" - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" gem "foo", :git => "#{lib_path("foo-1.0")}" G @@ -554,7 +554,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" git "#{lib_path("foo-1.0")}" do gem "foo" @@ -591,7 +591,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" gem "foo", :git => "#{lib_path("foo-1.0")}", :branch => "omg" G @@ -627,7 +627,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" gem "foo", :git => "#{lib_path("foo-1.0")}", :tag => "omg" G @@ -748,7 +748,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" gem "foo", :path => "#{lib_path("foo-1.0")}" G @@ -820,7 +820,7 @@ c.checksum gem_repo2, "myrack", "1.0.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack" @@ -859,7 +859,7 @@ end it "removes redundant sources" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack", :source => "https://gem.repo2/" @@ -887,7 +887,7 @@ end it "lists gems alphabetically" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "thin" @@ -930,7 +930,7 @@ end it "orders dependencies' dependencies in alphabetical order" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "rails" @@ -989,7 +989,7 @@ end end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem 'double_deps' G @@ -1020,7 +1020,7 @@ end it "does not add the :require option to the lockfile" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack-obama", ">= 1.0", :require => "myrack/obama" @@ -1051,7 +1051,7 @@ end it "does not add the :group option to the lockfile" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack-obama", ">= 1.0", :group => :test @@ -1088,7 +1088,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" path "foo" do gem "foo" @@ -1123,7 +1123,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" path "../foo" do gem "foo" @@ -1154,7 +1154,7 @@ it "stores relative paths when the path is provided in an absolute fashion but is relative" do build_lib "foo", path: bundled_app("foo") - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" path File.expand_path("foo", __dir__) do gem "foo" @@ -1193,7 +1193,7 @@ c.no_checksum "foo", "1.0" end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo1" gemspec :path => "../foo" G @@ -1240,7 +1240,7 @@ #{Bundler::VERSION} G - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack" @@ -1318,7 +1318,7 @@ end simulate_platform "universal-java-16" do - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2" gem "platform_specific" G @@ -1353,12 +1353,12 @@ c.checksum(gem_repo2, "myrack", "1.0.0") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack" G - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack" gem "activesupport" @@ -1388,7 +1388,7 @@ c.checksum(gem_repo2, "myrack", "1.0.0") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack" gem "myrack" @@ -1416,7 +1416,7 @@ c.checksum(gem_repo2, "myrack", "1.0.0") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack", "1.0" gem "myrack", "1.0" @@ -1444,7 +1444,7 @@ c.checksum(gem_repo2, "myrack", "1.0.0") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack", "1.0", :group => :one gem "myrack", "1.0", :group => :two @@ -1494,7 +1494,7 @@ c.checksum(gem_repo2, "myrack", "0.9.1") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" gem "myrack", "> 0.9", "< 1.0" G @@ -1521,7 +1521,7 @@ c.checksum(gem_repo2, "myrack", "0.9.1") end - install_gemfile <<-G + lock_gemfile <<-G source "https://gem.repo2/" ruby '#{Gem.ruby_version}' gem "myrack", "> 0.9", "< 1.0" diff --git a/spec/bundler/other/major_deprecation_spec.rb b/spec/bundler/other/major_deprecation_spec.rb index 55cb1996a066dc..8e30b531d4e036 100644 --- a/spec/bundler/other/major_deprecation_spec.rb +++ b/spec/bundler/other/major_deprecation_spec.rb @@ -97,7 +97,7 @@ context "bundle check --path" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -117,7 +117,7 @@ context "bundle check --path=" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -137,7 +137,7 @@ context "bundle binstubs --path=" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -157,7 +157,7 @@ context "bundle cache --all" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -177,7 +177,7 @@ context "bundle cache --no-all" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -197,7 +197,7 @@ context "bundle cache --path" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -217,7 +217,7 @@ context "bundle cache --path=" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -237,7 +237,7 @@ context "bundle cache --frozen" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -257,7 +257,7 @@ context "bundle cache --no-prune" do before do - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G @@ -437,7 +437,7 @@ before do bundle_config "path vendor/bundle" - install_gemfile <<-G + gemfile <<-G source "https://gem.repo1" gem "myrack" G diff --git a/spec/bundler/spec_helper.rb b/spec/bundler/spec_helper.rb index 27ddc6a77125a2..24b0e71020ccdd 100644 --- a/spec/bundler/spec_helper.rb +++ b/spec/bundler/spec_helper.rb @@ -175,6 +175,26 @@ def self.ruby=(ruby) reset! end + # Opt-in per-example runtime log (set BUNDLER_SPEC_RUNTIME_LOG to a file path). + # Each parallel worker appends one "\t" line per example to its + # own "." file (a single shared file would hit Windows + # cross-process sharing violations), so the heaviest specs can be found after a + # full run. turbo_tests only writes its own --runtime-log when invoked with the + # bare "spec" path, which the build never does, so it produces no log otherwise. + if (runtime_log = ENV["BUNDLER_SPEC_RUNTIME_LOG"]) + worker = ENV["TEST_ENV_NUMBER"].to_s + worker = "1" if worker.empty? + runtime_log = "#{runtime_log}.#{worker}" + config.before(:each) { @__runtime_start = Process.clock_gettime(Process::CLOCK_MONOTONIC) } + config.after(:each) do |example| + next unless @__runtime_start + dt = Process.clock_gettime(Process::CLOCK_MONOTONIC) - @__runtime_start + File.write(runtime_log, "#{format("%.4f", dt)}\t#{example.metadata[:file_path]}\n", mode: "a") + rescue StandardError + # never let runtime logging break a test run + end + end + Spec::Shards::EXAMPLE_MAPPINGS.each do |tag, file_paths| file_pattern = Regexp.union(file_paths.map {|path| Regexp.new(Regexp.escape(path) + "$") }) diff --git a/spec/bundler/support/shards.rb b/spec/bundler/support/shards.rb index ce33896539bfc2..5554c87b0c1ad3 100644 --- a/spec/bundler/support/shards.rb +++ b/spec/bundler/support/shards.rb @@ -52,6 +52,8 @@ module Shards "spec/bundler/ui_spec.rb", "spec/bundler/plugin/source_list_spec.rb", "spec/bundler/source/path_spec.rb", + "spec/lock/lockfile_spec.rb", + "spec/runtime/inline_spec.rb", ], shard_b: [ "spec/install/gemfile/git_spec.rb", @@ -64,7 +66,6 @@ module Shards "spec/plugins/install_spec.rb", "spec/commands/binstubs_spec.rb", "spec/install/gems/flex_spec.rb", - "spec/runtime/inline_spec.rb", "spec/commands/post_bundle_message_spec.rb", "spec/runtime/executable_spec.rb", "spec/lock/git_spec.rb", @@ -107,7 +108,6 @@ module Shards "spec/commands/cache_spec.rb", "spec/commands/check_spec.rb", "spec/commands/list_spec.rb", - "spec/install/path_spec.rb", "spec/bundler/cli_spec.rb", "spec/install/bundler_spec.rb", "spec/install/git_spec.rb", @@ -148,7 +148,6 @@ module Shards "spec/install/cooldown_spec.rb", "spec/commands/outdated_spec.rb", "spec/commands/update_spec.rb", - "spec/lock/lockfile_spec.rb", "spec/install/deploy_spec.rb", "spec/install/gemfile/sources_spec.rb", "spec/runtime/self_management_spec.rb", @@ -194,6 +193,7 @@ module Shards "spec/install/gems/no_install_plugin_spec.rb", "spec/bundler/override_spec.rb", "spec/install/gemfile/override_spec.rb", + "spec/install/path_spec.rb", ], }.freeze end diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 4294cb377305de..38796e18a2026b 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -476,7 +476,7 @@ impl Assembler { same_opnd_if_test && if let Some( Insn::Jz(target) | Insn::Je(target) | Insn::Jnz(target) | Insn::Jne(target) ) = iterator.peek() { - matches!(target, Target::SideExit { .. }) + matches!(target, Target::SideExit(..)) } else { false } @@ -815,11 +815,11 @@ impl Assembler { asm.push_insn(insn); } // For compile_exits, support splitting simple C arguments here - Insn::CCall { opnds, .. } if !opnds.is_empty() => { - for (i, opnd) in opnds.iter().enumerate() { + Insn::CCall { data } if !data.opnds.is_empty() => { + for (i, opnd) in data.opnds.iter().enumerate() { asm.load_into(C_ARG_OPNDS[i], *opnd); } - *opnds = vec![]; + data.opnds = vec![]; asm.push_insn(insn); } // For compile_exits, support splitting simple return values here @@ -890,8 +890,8 @@ impl Assembler { _ => asm.push_insn(insn), } } - &mut Insn::PatchPoint { ref target, invariant, version } => { - split_patch_point(asm, target, invariant, version); + &mut Insn::PatchPoint(ref data) => { + split_patch_point(asm, &data.target, data.invariant, data.version); } _ => { asm.push_insn(insn); @@ -987,7 +987,7 @@ impl Assembler { }, Target::Label(l) => l, Target::Block(ref edge) => asm.block_label(edge.target), - Target::SideExit { .. } => { + Target::SideExit(..) => { unreachable!("Target::SideExit should have been compiled by compile_exits") }, }; @@ -1499,7 +1499,7 @@ impl Assembler { } }); }, - Target::SideExit { .. } => { + Target::SideExit(..) => { unreachable!("Target::SideExit should have been compiled by compile_exits") }, }; @@ -1534,7 +1534,7 @@ impl Assembler { Insn::Jonz(opnd, target) => { emit_cmp_zero_jump(cb, opnd.into(), false, target.clone()); }, - Insn::PatchPoint { .. } => unreachable!("PatchPoint should have been lowered to PadPatchPoint in arm64_scratch_split"), + Insn::PatchPoint(..) => unreachable!("PatchPoint should have been lowered to PadPatchPoint in arm64_scratch_split"), Insn::PadPatchPoint => { // If patch points are too close to each other or the end of the block, fill nop instructions if let Some(last_patch_pos) = last_patch_pos { @@ -1774,7 +1774,7 @@ mod tests { let val64 = asm.add(CFP, Opnd::UImm(64)); asm.store(Opnd::mem(64, SP, 0x10), val64); - let side_exit = Target::SideExit { reason: SideExitReason::Interrupt, exit: Box::new(SideExit { pc: 0.into(), iseq: std::ptr::null(), stack: vec![], locals: vec![], recompile: None }) }; + let side_exit = Target::SideExit(Box::new(SideExitTarget { reason: SideExitReason::Interrupt, exit: SideExit { pc: 0.into(), iseq: std::ptr::null(), stack: vec![], locals: vec![], recompile: None } })); asm.push_insn(Insn::Joz(val64, side_exit)); asm.mov(C_ARG_OPNDS[0], C_RET_OPND.with_num_bits(32)); asm.mov(C_ARG_OPNDS[1], Opnd::mem(64, SP, -8)); diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index da4fe79b7c6c81..7321fc1877a06c 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -14,14 +14,32 @@ use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, side_exit_coun use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; use crate::state::{ZJITState, rb_zjit_record_exit_stack}; +use crate::cast::IntoUsize; /// LIR Block ID. Unique ID for each block, and also defined in LIR so /// we can differentiate it from HIR block ids. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] pub struct BlockId(pub usize); +/// Underlying integer width of a virtual-register id. Narrow to keep `Opnd`/`Mem` small. +pub type VRegIdBase = u32; +/// Width of a stack-slot index inside `MemBase`. Separate id space from `VRegId`. +pub type StackIdx = u32; + #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] -pub struct VRegId(pub usize); +pub struct VRegId(pub VRegIdBase); + +impl IntoUsize for VRegId { + fn to_usize(self) -> usize { + self.0.to_usize() + } +} + +impl From for VRegId { + fn from(val: usize) -> Self { + VRegId(val.try_into().unwrap()) + } +} impl From for usize { fn from(val: BlockId) -> Self { @@ -29,10 +47,23 @@ impl From for usize { } } -impl From for usize { - fn from(val: VRegId) -> Self { - val.0 - } +impl std::ops::Index for [T] { + type Output = T; + #[inline] + fn index(&self, i: VRegId) -> &T { &self[i.to_usize()] } +} +impl std::ops::IndexMut for [T] { + #[inline] + fn index_mut(&mut self, i: VRegId) -> &mut T { &mut self[i.to_usize()] } +} +impl std::ops::Index for Vec { + type Output = T; + #[inline] + fn index(&self, i: VRegId) -> &T { &self[i.to_usize()] } +} +impl std::ops::IndexMut for Vec { + #[inline] + fn index_mut(&mut self, i: VRegId) -> &mut T { &mut self[i.to_usize()] } } impl std::fmt::Display for BlockId { @@ -140,7 +171,7 @@ impl BasicBlock { assert!(self.insns.last().unwrap().is_terminator()); let extract_edge = |insn: &Insn| -> Option { if let Some(Target::Block(edge)) = insn.target() { - Some(edge.clone()) + Some((**edge).clone()) } else { None } @@ -220,13 +251,13 @@ pub enum MemBase /// Stack slot: a direct stack access. `stack_membase_to_mem()` turns this /// into `[NATIVE_BASE_PTR + disp]`, so scratch splitting can use it as a /// normal memory operand without first loading a pointer from the stack. - Stack { stack_idx: usize, num_bits: u8 }, + Stack { stack_idx: StackIdx, num_bits: u8 }, /// A pointer stored in a stack slot, used as a memory base. /// Unlike Stack, this first loads the pointer value from the stack slot /// into a scratch register, then uses that register as the base for the /// memory access with the Mem's displacement. /// Created when a VReg used as MemBase is spilled to the stack. - StackIndirect { stack_idx: usize }, + StackIndirect { stack_idx: StackIdx }, } // Memory location @@ -422,6 +453,11 @@ impl Opnd } } + /// Unwrap the index of a VReg as a `usize`, for raw-`usize` APIs (bitsets, etc.). + pub fn vreg_idx_usize(&self) -> usize { + self.vreg_idx().to_usize() + } + /// Extract VReg indices from this operand, including memory base VRegs. /// Returns an iterator over all VRegIds referenced by this operand. pub fn vreg_ids(&self) -> impl Iterator { @@ -466,10 +502,10 @@ impl Opnd pub fn map_index(self, indices: &[usize]) -> Opnd { match self { Opnd::VReg { idx, num_bits } => { - Opnd::VReg { idx: VRegId(indices[idx.0]), num_bits } + Opnd::VReg { idx: indices[idx].into(), num_bits } } Opnd::Mem(Mem { base: MemBase::VReg(idx), disp, num_bits }) => { - Opnd::Mem(Mem { base: MemBase::VReg(VRegId(indices[idx.0])), disp, num_bits }) + Opnd::Mem(Mem { base: MemBase::VReg(indices[idx].into()), disp, num_bits }) }, _ => self } @@ -564,6 +600,16 @@ pub struct SideExitRecompile { pub insn_idx: u32, } +/// Payload of `Target::SideExit`, boxed to keep `Target` (and every `Insn` +/// variant that embeds it) small. +#[derive(Clone)] +pub struct SideExitTarget { + /// Context used for compiling the side exit. + pub exit: SideExit, + /// We use this to increment exit counters. + pub reason: SideExitReason, +} + /// Branch target (something that we can jump to) /// for branch instructions #[derive(Clone)] @@ -573,16 +619,10 @@ pub enum Target CodePtr(CodePtr), /// A label within the generated code Label(Label), - /// An LIR branch edge - Block(BranchEdge), - /// Side exit to the interpreter - SideExit { - /// Context used for compiling the side exit. Boxed to keep `Target` - /// (and every `Insn` variant that embeds it) small. - exit: Box, - /// We use this to increment exit counters - reason: SideExitReason, - }, + /// An LIR branch edge. Boxed to keep `Target` small. + Block(Box), + /// Side exit to the interpreter. Boxed (see `SideExitTarget`) to keep `Target` small. + SideExit(Box), } impl fmt::Debug for Target { @@ -604,8 +644,8 @@ impl fmt::Debug for Target { write!(f, "))") } } - Target::SideExit { exit, reason } => { - write!(f, "SideExit {{ exit: {:?}, reason: {:?} }}", exit, reason) + Target::SideExit(data) => { + write!(f, "SideExit {{ exit: {:?}, reason: {:?} }}", data.exit, data.reason) } } } @@ -636,11 +676,16 @@ impl From for Target { type PosMarkerFn = Rc; -/// Cold fields of `Insn::CCall`, boxed to keep `Insn` small. The operand-bearing -/// fields (`opnds`, `stack_map`) stay inline on the variant so the operand -/// iteration macros can reach them by reference. +/// All fields of `Insn::CCall`, boxed to keep `Insn` small. The operand-bearing +/// fields (`opnds`, `stack_map`) are reached through the box by the operand +/// iteration macros -- `opnds` directly (via `.iter()`/`.iter_mut()`) and +/// `stack_map` via a per-iterator reborrow, the same idea as `PatchPointData`. #[derive(Clone)] pub struct CCallData { + /// The arguments to the C call. + pub opnds: Vec, + /// VM stack contents to materialize a JIT frame if the call side-exits. + pub stack_map: Option, /// The function pointer to be called. This should be Opnd::const_ptr /// (Opnd::UImm) in most cases. gen_entry_trampoline() uses Opnd::Reg. pub fptr: Opnd, @@ -655,6 +700,21 @@ pub struct CCallData { pub out: Opnd, } +/// Cold fields of `Insn::PatchPoint`, boxed to keep `Insn` small. `target` is +/// operand-bearing (it's a `Target::SideExit` until `compile_exits` lowers it to +/// a `Target::Label`), so the operand-iteration macros reach it through the box +/// via a per-iterator reborrow -- the same idea as `CCallData` and the HIR +/// `CCallWithFrame` pattern. +#[derive(Clone)] +pub struct PatchPointData { + /// Patch point target. Rewritten to a jump to a side exit on invalidation. + pub target: Target, + /// The invariant whose violation triggers invalidation of this patch point. + pub invariant: Invariant, + /// ISEQ version invalidated to force a recompile when the invariant breaks. + pub version: IseqVersionRef, +} + /// ZJIT Low-level IR instruction #[derive(Clone)] pub enum Insn { @@ -700,10 +760,8 @@ pub enum Insn { CPushPair(Opnd, Opnd), // C function call with N arguments (variadic) + // All fields are boxed (see `CCallData`) to keep `Insn` small. CCall { - opnds: Vec, - stack_map: Option, - /// Cold fields (fptr, markers, out), boxed to keep `Insn` small. data: Box, }, @@ -825,7 +883,8 @@ pub enum Insn { Or { left: Opnd, right: Opnd, out: Opnd }, /// Patch point that will be rewritten to a jump to a side exit on invalidation. - PatchPoint { target: Target, invariant: Invariant, version: IseqVersionRef }, + /// Cold fields are boxed (see `PatchPointData`) to keep `Insn` small. + PatchPoint(Box), /// Make sure the last PatchPoint has enough space to insert a jump. /// We insert this instruction at the end of each block so that the jump @@ -861,9 +920,9 @@ pub enum Insn { macro_rules! target_for_each_operand_impl { ($self:expr, $visit_many:ident) => { match $self { - Target::SideExit { exit, .. } => { - visit_many!(exit.stack); - visit_many!(exit.locals); + Target::SideExit(data) => { + visit_many!(data.exit.stack); + visit_many!(data.exit.locals); } Target::Block(edge) => { visit_many!(edge.args); @@ -877,7 +936,7 @@ macro_rules! target_for_each_operand_impl { /// macro for a single `Opnd` field and `$visit_many` macro for a slice/`Vec` of `Opnd`s. Used by /// both `for_each_operand` and `for_each_operand_mut`. macro_rules! for_each_operand_impl { - ($self:expr, $visit_one:ident, $visit_many:ident $(, $const:expr)?) => { + ($self:expr, $visit_one:ident, $visit_many:ident, $reborrow:ident $(, $const:expr)?) => { match $self { Insn::Jbe(target) | Insn::Jb(target) | @@ -892,10 +951,14 @@ macro_rules! for_each_operand_impl { Insn::JoMul(target) | Insn::Jz(target) | Insn::Label(target) | - Insn::LeaJumpTarget { target, .. } | - Insn::PatchPoint { target, .. } => { + Insn::LeaJumpTarget { target, .. } => { target_for_each_operand_impl!(target, $visit_many); } + // `target` is behind a Box. `$reborrow` turns the box field into a `&`/`&mut Target` + // matching the iterator, so the same operand-walk works for both. + Insn::PatchPoint(data) => { + target_for_each_operand_impl!($reborrow!(data.target), $visit_many); + } Insn::Joz(opnd, target) | Insn::Jonz(opnd, target) => { visit_one!(opnd); @@ -947,9 +1010,12 @@ macro_rules! for_each_operand_impl { visit_one!(opnd0); visit_one!(opnd1); } - Insn::CCall { opnds, stack_map, .. } => { - visit_many!(opnds); - if let Some(StackMap { stack, .. }) = stack_map { + Insn::CCall { data } => { + visit_many!(data.opnds); + // `data` is behind a Box. `$reborrow` turns the box field into a `&`/`&mut + // Option` matching the iterator, so the same operand-walk works for + // both. + if let Some(StackMap { stack, .. }) = $reborrow!(data.stack_map) { visit_many!(stack); } } @@ -977,23 +1043,26 @@ impl Insn { pub fn for_each_operand(&self, mut f: impl FnMut(Opnd)) { macro_rules! visit_one { ($id:expr) => { f(*$id) }; } macro_rules! visit_many { ($s:expr) => { for id in ($s).iter() { f(*id) } }; } + macro_rules! reborrow { ($e:expr) => { & $e }; } // Extra () is a throw-away parameter to avoid iterating over FrameSetup/FrameTeardown // preserved in the mutable iterator. - for_each_operand_impl!(self, visit_one, visit_many, ()); + for_each_operand_impl!(self, visit_one, visit_many, reborrow, ()); } /// Call `f` on a mutable reference to each operand (Opnd) of this instruction. pub fn for_each_operand_mut(&mut self, mut f: impl FnMut(&mut Opnd)) { macro_rules! visit_one { ($id:expr) => { f($id) }; } macro_rules! visit_many { ($s:expr) => { for id in ($s).iter_mut() { f(id) } }; } - for_each_operand_impl!(self, visit_one, visit_many); + macro_rules! reborrow { ($e:expr) => { &mut $e }; } + for_each_operand_impl!(self, visit_one, visit_many, reborrow); } /// Call `f` on each operand, short-circuiting on the first error. pub fn try_for_each_operand(&self, mut f: impl FnMut(Opnd) -> Result<(), E>) -> Result<(), E> { macro_rules! visit_one { ($id:expr) => { f(*$id)? }; } macro_rules! visit_many { ($s:expr) => { for id in ($s).iter() { f(*id)? } }; } - for_each_operand_impl!(self, visit_one, visit_many, ()); + macro_rules! reborrow { ($e:expr) => { & $e }; } + for_each_operand_impl!(self, visit_one, visit_many, reborrow, ()); Ok(()) } @@ -1015,10 +1084,10 @@ impl Insn { Insn::Joz(_, target) | Insn::Jonz(_, target) | Insn::Label(target) | - Insn::LeaJumpTarget { target, .. } | - Insn::PatchPoint { target, .. } => { + Insn::LeaJumpTarget { target, .. } => { Some(target) } + Insn::PatchPoint(data) => Some(&mut data.target), _ => None, } } @@ -1077,7 +1146,7 @@ impl Insn { Insn::Mov { .. } => "Mov", Insn::Not { .. } => "Not", Insn::Or { .. } => "Or", - Insn::PatchPoint { .. } => "PatchPoint", + Insn::PatchPoint(..) => "PatchPoint", Insn::PadPatchPoint => "PadPatchPoint", Insn::PosMarker(_) => "PosMarker", Insn::RShift { .. } => "RShift", @@ -1172,8 +1241,8 @@ impl Insn { Insn::Joz(_, target) | Insn::Jonz(_, target) | Insn::Label(target) | - Insn::LeaJumpTarget { target, .. } | - Insn::PatchPoint { target, .. } => Some(target), + Insn::LeaJumpTarget { target, .. } => Some(target), + Insn::PatchPoint(data) => Some(&data.target), _ => None } } @@ -1272,12 +1341,12 @@ impl LiveRange { #[derive(Clone)] pub struct Interval { pub range: LiveRange, - pub id: usize, + pub id: VRegId, } impl Interval { /// Create a new Interval with no range - pub fn new(i: usize) -> Self { + pub fn new(i: VRegId) -> Self { Self { range: LiveRange { start: None, @@ -1480,8 +1549,8 @@ impl StackState { } /// Convert a stack index to the `disp` of the stack slot - fn stack_idx_to_disp(&self, stack_idx: usize) -> i32 { - (self.stack_base_idx + stack_idx + 1) as i32 * -SIZEOF_VALUE_I32 + fn stack_idx_to_disp(&self, stack_idx: StackIdx) -> i32 { + (self.stack_base_idx + stack_idx.to_usize() + 1) as i32 * -SIZEOF_VALUE_I32 } /// Convert MemBase::Stack to Mem @@ -1850,7 +1919,7 @@ impl Assembler /// Build an Opnd::VReg pub fn new_vreg(&mut self, num_bits: u8) -> Opnd { - let vreg = Opnd::VReg { idx: VRegId(self.num_vregs), num_bits }; + let vreg = Opnd::VReg { idx: self.num_vregs.into(), num_bits }; self.num_vregs += 1; vreg } @@ -1953,10 +2022,10 @@ impl Assembler { if let Some(Opnd::VReg { idx: out_idx, .. }) = prev.out_opnd() { if out_idx == idx - && intervals[idx.0].born_at(prev_id.0) - && intervals[idx.0].dies_at(insn_id.0) + && intervals[*idx].born_at(prev_id.0) + && intervals[*idx].dies_at(insn_id.0) { - preferred[idx.0].get_or_insert(*dest_reg); + preferred[*idx].get_or_insert(*dest_reg); } } } @@ -2127,7 +2196,7 @@ impl Assembler let reg_copies: Vec> = edge.args .iter() .zip(params.iter()) - .filter(|(_arg, param)| assignments[param.vreg_idx().0].is_some() ) + .filter(|(_arg, param)| assignments[param.vreg_idx()].is_some() ) .map(|(arg, param)| parcopy::RegisterCopy:: { destination: Self::rewritten_opnd(*param, assignments), source: Self::rewritten_opnd(*arg, assignments), @@ -2162,10 +2231,10 @@ impl Assembler for mov in moves { self.basic_blocks[new_block_id.0].push_insn(mov); } - self.basic_blocks[new_block_id.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { + self.basic_blocks[new_block_id.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: successor, args: vec![], - }))); + })))); // Redirect predecessor's branch to the new block let pred_insns = &mut self.basic_blocks[pred_id.0].insns; @@ -2293,21 +2362,21 @@ impl Assembler let mut new_ids = Vec::with_capacity(old_ids.len()); for (insn, insn_id) in old_insns.into_iter().zip(old_ids.into_iter()) { - if let Insn::CCall { opnds, stack_map, data } = insn { - let CCallData { out, start_marker, end_marker, fptr } = *data; + if let Insn::CCall { data } = insn { + let CCallData { opnds, stack_map, out, start_marker, end_marker, fptr } = *data; let insn_number = insn_id.map(|id| id.0).unwrap_or(0); // Do we have a case where a ccall is emitted, but nobody // uses the result? let call_result_live = out.is_vreg() - && intervals[out.vreg_idx().0] + && intervals[out.vreg_idx()] .range .end .is_some_and(|end| end > insn_number); // Build a set of VRegIds that can be referenced by JITFrame for materializing the VM stack - let stack_vreg_ids: HashSet = if let Some(StackMap { stack, .. }) = &stack_map { + let stack_vreg_ids: HashSet = if let Some(StackMap { stack, .. }) = &stack_map { stack.iter().filter_map(|opnd| match opnd { - Opnd::VReg { idx: VRegId(vreg_id), .. } => Some(*vreg_id), + Opnd::VReg { idx, .. } => Some(*idx), _ => None, }).collect() } else { @@ -2318,7 +2387,7 @@ impl Assembler // We need to preserve the "surviving" registers past the ccall, // so we're going to push them all on the stack, then pop // after we make the ccall - let survivors: Vec = intervals.iter() + let survivors: Vec = intervals.iter() .filter(|interval| { // We need to spill register intervals on this CCall in two cases: // 1) The VReg is referenced in an instruction after the CCall @@ -2359,10 +2428,10 @@ impl Assembler assert!(value.special_const_p(), "StackMap should only materialize immediate VALUEs, but got: {value:?}"); value } - Opnd::VReg { idx: VRegId(vreg_id), .. } => { - let vreg_stack_index = match assignments[*vreg_id].expect("StackMap VReg should have an allocation") { + Opnd::VReg { idx: vreg, .. } => { + let vreg_stack_index = match assignments[*vreg].expect("StackMap VReg should have an allocation") { Allocation::Reg(_) | Allocation::Fixed(_) => { - let caller_saved_reg_idx = survivors.iter().position(|&survivor_id| survivor_id == *vreg_id).unwrap(); + let caller_saved_reg_idx = survivors.iter().position(|&survivor_id| survivor_id == *vreg).unwrap(); let stack_idx = self.stack_state.stack_idx_for_caller_saved_reg(caller_saved_reg_idx); self.stack_state.stack_map_index_for_spill(stack_idx, frame_depth) } @@ -2419,10 +2488,10 @@ impl Assembler // The CCall itself new_insns.push(Insn::CCall { - opnds: vec![], // We've moved everything in to ccall regs, so this should - // be empty now - stack_map: None, data: Box::new(CCallData { + opnds: vec![], // We've moved everything in to ccall regs, so this should + // be empty now + stack_map: None, out: C_RET_OPND, start_marker: None, end_marker: None, @@ -2507,7 +2576,7 @@ impl Assembler match opnd { Opnd::VReg { idx, num_bits } => { - if let Some(assignment) = assignments[idx.0] { + if let Some(assignment) = assignments[*idx] { match assignment { Allocation::Reg(n) => { let mut reg = regs[n]; @@ -2521,7 +2590,7 @@ impl Assembler Allocation::Stack(n) => { let num_bits = *num_bits; *opnd = Opnd::Mem(Mem { - base: MemBase::Stack { stack_idx: n, num_bits }, + base: MemBase::Stack { stack_idx: n.try_into().unwrap(), num_bits }, disp: 0, num_bits, }); @@ -2532,7 +2601,7 @@ impl Assembler } } Opnd::Mem(Mem { base: MemBase::VReg(idx), .. }) => { - match assignments[idx.0].unwrap() { + match assignments[*idx].unwrap() { Allocation::Reg(n) => { if let Opnd::Mem(mem) = opnd { mem.base = MemBase::Reg(regs[n].reg_no); @@ -2548,7 +2617,7 @@ impl Assembler // Mark it as StackIndirect so arm64_scratch_split can load // the pointer from the stack into a scratch register. if let Opnd::Mem(mem) = opnd { - mem.base = MemBase::StackIndirect { stack_idx: n }; + mem.base = MemBase::StackIndirect { stack_idx: n.try_into().unwrap() }; } } } @@ -2686,7 +2755,7 @@ impl Assembler for block_id in self.block_order() { let block = &self.basic_blocks[block_id.0]; for (idx, insn) in block.insns.iter().enumerate() { - if let Some(target @ Target::SideExit { .. }) = insn.target() { + if let Some(target @ Target::SideExit(..)) = insn.target() { targets.insert((block_id.0, idx), target.clone()); } } @@ -2724,7 +2793,8 @@ impl Assembler for ((block_id, idx), target) in targets { // Compile a side exit. Note that this is past register assignment, // so you can't use an instruction that returns a VReg. - if let Target::SideExit { exit, reason } = target { + if let Target::SideExit(data) = target { + let SideExitTarget { exit, reason } = *data; // Only record the exit if `trace_side_exits` is defined and the counter is either the one specified let should_record_exit = get_option!(trace_side_exits).map(|trace| match trace { TraceExits::All => true, @@ -2770,7 +2840,7 @@ impl Assembler self.write_label(new_exit.clone()); asm_comment!(self, "Exit: {}", exit.pc); compile_exit(self, &exit, None); - compiled_exits.insert(*exit, new_exit.unwrap_label()); + compiled_exits.insert(exit, new_exit.unwrap_label()); new_exit }; @@ -2896,15 +2966,15 @@ impl Assembler // If the instruction has an output that is a VReg, add to kill set if let Some(out) = insn.out_opnd() { if let Opnd::VReg { idx, .. } = out { - kill_set.insert(idx.0); + kill_set.insert(idx.to_usize()); } } // For all input operands that are VRegs (including memory base VRegs), add to gen set insn.for_each_operand(|opnd| { for idx in opnd.vreg_ids() { - assert!(!kill_set.get(idx.0)); - gen_set.insert(idx.0); + assert!(!kill_set.get(idx.to_usize())); + gen_set.insert(idx.to_usize()); } }); } @@ -2912,7 +2982,7 @@ impl Assembler // Add block parameters to kill set for param in &block.parameters { if let Opnd::VReg { idx, .. } = param { - kill_set.insert(idx.0); + kill_set.insert(idx.to_usize()); } } @@ -2929,7 +2999,7 @@ impl Assembler pub fn build_intervals(&self, live_in: Vec>) -> Vec { let num_vregs = self.num_vregs; let mut intervals: Vec = (0..num_vregs) - .map(|i| Interval::new(i)) + .map(|i| Interval::new(i.into())) .collect(); let blocks = self.block_order(); @@ -2945,7 +3015,7 @@ impl Assembler // Add out_vregs to live set for idx in block.out_vregs() { - live.insert(idx.0); + live.insert(idx.to_usize()); } // For each live vreg, add entire block range @@ -2961,14 +3031,14 @@ impl Assembler // If instruction has VReg output, set_from if let Some(out) = insn.out_opnd() { if let Opnd::VReg { idx, .. } = out { - intervals[idx.0].set_from(insn_id.0); + intervals[*idx].set_from(insn_id.0); } } // For each VReg input (including memory base VRegs), add_range from block start to insn insn.for_each_operand(|opnd| { for idx in opnd.vreg_ids() { - intervals[idx.0].add_range(block.from.0, insn_id.0); + intervals[idx].add_range(block.from.0, insn_id.0); } }); } @@ -3152,7 +3222,7 @@ fn format_insn_compact(asm: &Assembler, insn: &Insn) -> String { match target { Target::CodePtr(code_ptr) => output.push_str(&format!(" {code_ptr:?}")), Target::Label(Label(label_idx)) => output.push_str(&format!(" {}", asm.label_names[*label_idx])), - Target::SideExit { reason, .. } => output.push_str(&format!(" Exit({reason})")), + Target::SideExit(data) => output.push_str(&format!(" Exit({})", data.reason)), Target::Block(edge) => { let label = asm.block_label(edge.target); let name = &asm.label_names[label.0]; @@ -3173,7 +3243,7 @@ fn format_insn_compact(asm: &Assembler, insn: &Insn) -> String { } // Print operands (but skip branch args since they're already printed with target) - if let Some(Target::SideExit { .. }) = insn.target() { + if let Some(Target::SideExit(..)) = insn.target() { match insn { Insn::Joz(opnd, _) | Insn::Jonz(opnd, _) | @@ -3277,7 +3347,7 @@ impl fmt::Display for Assembler { match target { Target::CodePtr(code_ptr) => write!(f, " {code_ptr:?}")?, Target::Label(Label(label_idx)) => write!(f, " {}", label_name(self, *label_idx, &label_counts))?, - Target::SideExit { reason, .. } => write!(f, " Exit({reason})")?, + Target::SideExit(data) => write!(f, " Exit({})", data.reason)?, Target::Block(edge) => { let label = self.block_label(edge.target); let name = label_name(self, label.0, &label_counts); @@ -3298,7 +3368,7 @@ impl fmt::Display for Assembler { } // Print list of operands - if let Some(Target::SideExit { .. }) = insn.target() { + if let Some(Target::SideExit(..)) = insn.target() { // If the instruction has a SideExit, avoid using opnd_iter(), which has stack/locals. // Here, only handle instructions that have both Opnd and Target. match insn { @@ -3452,7 +3522,7 @@ impl Assembler { let out = self.new_vreg(Opnd::match_num_bits(&opnds)); let fptr = Opnd::const_ptr(fptr); let stack_map = self.stack_map.take(); - self.push_insn(Insn::CCall { opnds, stack_map, data: Box::new(CCallData { fptr, start_marker: None, end_marker: None, out }) }); + self.push_insn(Insn::CCall { data: Box::new(CCallData { opnds, stack_map, fptr, start_marker: None, end_marker: None, out }) }); self.clear_stack_canary(canary_opnd); out } @@ -3462,7 +3532,7 @@ impl Assembler { pub fn ccall_into(&mut self, out: Opnd, fptr: *const u8, opnds: Vec) { let fptr = Opnd::const_ptr(fptr); let stack_map = self.stack_map.take(); - self.push_insn(Insn::CCall { opnds, stack_map, data: Box::new(CCallData { fptr, start_marker: None, end_marker: None, out }) }); + self.push_insn(Insn::CCall { data: Box::new(CCallData { opnds, stack_map, fptr, start_marker: None, end_marker: None, out }) }); } /// Call a C function stored in a register @@ -3470,7 +3540,7 @@ impl Assembler { assert!(matches!(fptr, Opnd::Reg(_)), "ccall_reg must be called with Opnd::Reg: {fptr:?}"); let out = self.new_vreg(num_bits); let stack_map = self.stack_map.take(); - self.push_insn(Insn::CCall { opnds: vec![], stack_map, data: Box::new(CCallData { fptr, start_marker: None, end_marker: None, out }) }); + self.push_insn(Insn::CCall { data: Box::new(CCallData { opnds: vec![], stack_map, fptr, start_marker: None, end_marker: None, out }) }); out } @@ -3486,9 +3556,9 @@ impl Assembler { let out = self.new_vreg(Opnd::match_num_bits(&opnds)); let stack_map = self.stack_map.take(); self.push_insn(Insn::CCall { - opnds, - stack_map, data: Box::new(CCallData { + opnds, + stack_map, fptr: Opnd::const_ptr(fptr), start_marker: Some(Rc::new(start_marker)), end_marker: Some(Rc::new(end_marker)), @@ -3704,7 +3774,7 @@ impl Assembler { } pub fn patch_point(&mut self, target: Target, invariant: Invariant, version: IseqVersionRef) { - self.push_insn(Insn::PatchPoint { target, invariant, version }); + self.push_insn(Insn::PatchPoint(Box::new(PatchPointData { target, invariant, version }))); } pub fn pad_patch_point(&mut self) { @@ -3853,7 +3923,20 @@ mod tests { #[test] fn test_size_of_insn() { - assert_eq!(std::mem::size_of::(), 80); + // PatchPoint (PatchPointData), CCall (CCallData), and Target (Block/SideExit + // payloads) are all boxed, so none dominates the enum. The floor is now the + // 3-operand arithmetic/CSel variants: 3 * Opnd (48) + 8 discriminant (Opnd has + // no niche to absorb it) = 56. Going lower means boxing the hottest insns. + assert_eq!(std::mem::size_of::(), 56); + } + + #[test] + fn test_size_of_opnd() { + assert_eq!(std::mem::size_of::(), 4); + assert_eq!(std::mem::size_of::(), 8); + assert_eq!(std::mem::size_of::(), 16); + assert_eq!(std::mem::size_of::(), 16); + assert_eq!(std::mem::size_of::(), 16); } #[test] @@ -4025,10 +4108,10 @@ mod tests { asm.write_label(label_b1); asm.basic_blocks[b1.0].add_parameter(r10); asm.basic_blocks[b1.0].add_parameter(r11); - asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { + asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b2, args: vec![Opnd::UImm(1), r11], - }))); + })))); // Build b2: define(r12, r13) { cmp(r13, imm(1)); blt(...) } asm.set_current_block(b2); @@ -4037,8 +4120,8 @@ mod tests { asm.basic_blocks[b2.0].add_parameter(r12); asm.basic_blocks[b2.0].add_parameter(r13); asm.basic_blocks[b2.0].push_insn(Insn::Cmp { left: r13, right: Opnd::UImm(1) }); - asm.basic_blocks[b2.0].push_insn(Insn::Jl(Target::Block(BranchEdge { target: b4, args: vec![] }))); - asm.basic_blocks[b2.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { target: b3, args: vec![] }))); + asm.basic_blocks[b2.0].push_insn(Insn::Jl(Target::Block(Box::new(BranchEdge { target: b4, args: vec![] })))); + asm.basic_blocks[b2.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b3, args: vec![] })))); // Build b3: r14 = mul(r12, r13); r15 = sub(r13, imm(1)); jump(edge(b2, [r14, r15])) asm.set_current_block(b3); @@ -4048,10 +4131,10 @@ mod tests { let r15 = asm.new_vreg(64); asm.basic_blocks[b3.0].push_insn(Insn::Mul { left: r12, right: r13, out: r14 }); asm.basic_blocks[b3.0].push_insn(Insn::Sub { left: r13, right: Opnd::UImm(1), out: r15 }); - asm.basic_blocks[b3.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { + asm.basic_blocks[b3.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b2, args: vec![r14, r15], - }))); + })))); // Build b4: out = add(r10, r12); ret out asm.set_current_block(b4); @@ -4075,18 +4158,18 @@ mod tests { assert_eq!(bitset_to_vreg_indices(&live_in[b1.0], num_vregs), vec![]); // b2: [r10] - r10 is live-in (used in b4 which is reachable) - assert_eq!(bitset_to_vreg_indices(&live_in[b2.0], num_vregs), vec![r10.vreg_idx().0]); + assert_eq!(bitset_to_vreg_indices(&live_in[b2.0], num_vregs), vec![r10.vreg_idx_usize()]); // b3: [r10, r12, r13] - all are live-in assert_eq!( bitset_to_vreg_indices(&live_in[b3.0], num_vregs), - vec![r10.vreg_idx().0, r12.vreg_idx().0, r13.vreg_idx().0] + vec![r10.vreg_idx_usize(), r12.vreg_idx_usize(), r13.vreg_idx_usize()] ); // b4: [r10, r12] - both are live-in assert_eq!( bitset_to_vreg_indices(&live_in[b4.0], num_vregs), - vec![r10.vreg_idx().0, r12.vreg_idx().0] + vec![r10.vreg_idx_usize(), r12.vreg_idx_usize()] ); } @@ -4150,10 +4233,10 @@ mod tests { asm.set_current_block(b1); let label_b1 = asm.new_label("bb0"); asm.write_label(label_b1); - asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { + asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b2, args: vec![Opnd::mem(64, base, 8)], - }))); + })))); let out_vregs = asm.basic_blocks[b1.0].out_vregs(); assert_eq!(out_vregs, vec![base.vreg_idx()]); @@ -4161,7 +4244,7 @@ mod tests { #[test] fn test_interval_add_range() { - let mut interval = Interval::new(1); + let mut interval = Interval::new(VRegId(1)); // Add range to empty interval interval.add_range(5, 10); @@ -4181,7 +4264,7 @@ mod tests { #[test] fn test_interval_survives() { - let mut interval = Interval::new(1); + let mut interval = Interval::new(VRegId(1)); interval.add_range(3, 10); assert!(!interval.survives(2)); // Before range @@ -4193,7 +4276,7 @@ mod tests { #[test] fn test_interval_set_from() { - let mut interval = Interval::new(1); + let mut interval = Interval::new(VRegId(1)); // With no range, sets both start and end interval.set_from(10); @@ -4210,14 +4293,14 @@ mod tests { #[test] #[should_panic(expected = "Invalid range")] fn test_interval_add_range_invalid() { - let mut interval = Interval::new(1); + let mut interval = Interval::new(VRegId(1)); interval.add_range(10, 5); } #[test] #[should_panic(expected = "survives called on interval with no range")] fn test_interval_survives_panics_without_range() { - let interval = Interval::new(1); + let interval = Interval::new(VRegId(1)); interval.survives(5); } @@ -4244,23 +4327,23 @@ mod tests { // Assert expected ranges // Note: Rust CFG differs from Ruby due to conditional branches requiring two instructions (Jl + Jmp) - assert_eq!(intervals[r10_idx.0].range.start, Some(16)); - assert_eq!(intervals[r10_idx.0].range.end, Some(38)); + assert_eq!(intervals[r10_idx].range.start, Some(16)); + assert_eq!(intervals[r10_idx].range.end, Some(38)); - assert_eq!(intervals[r11_idx.0].range.start, Some(16)); - assert_eq!(intervals[r11_idx.0].range.end, Some(20)); + assert_eq!(intervals[r11_idx].range.start, Some(16)); + assert_eq!(intervals[r11_idx].range.end, Some(20)); - assert_eq!(intervals[r12_idx.0].range.start, Some(20)); - assert_eq!(intervals[r12_idx.0].range.end, Some(38)); + assert_eq!(intervals[r12_idx].range.start, Some(20)); + assert_eq!(intervals[r12_idx].range.end, Some(38)); - assert_eq!(intervals[r13_idx.0].range.start, Some(20)); - assert_eq!(intervals[r13_idx.0].range.end, Some(32)); + assert_eq!(intervals[r13_idx].range.start, Some(20)); + assert_eq!(intervals[r13_idx].range.end, Some(32)); - assert_eq!(intervals[r14_idx.0].range.start, Some(30)); - assert_eq!(intervals[r14_idx.0].range.end, Some(36)); + assert_eq!(intervals[r14_idx].range.start, Some(30)); + assert_eq!(intervals[r14_idx].range.end, Some(36)); - assert_eq!(intervals[r15_idx.0].range.start, Some(32)); - assert_eq!(intervals[r15_idx.0].range.end, Some(36)); + assert_eq!(intervals[r15_idx].range.start, Some(32)); + assert_eq!(intervals[r15_idx].range.end, Some(36)); } #[test] @@ -4299,12 +4382,12 @@ mod tests { // r13: [20,38) gets Reg(2) // r14: [36,42) gets Reg(1) (r12 expired, reuses its register) // r15: [38,42) gets Reg(2) (r13 expired, reuses its register) - assert_eq!(assignments[r10_idx.0], Some(Allocation::Reg(0))); - assert_eq!(assignments[r11_idx.0], Some(Allocation::Reg(1))); - assert_eq!(assignments[r12_idx.0], Some(Allocation::Reg(1))); - assert_eq!(assignments[r13_idx.0], Some(Allocation::Reg(2))); - assert_eq!(assignments[r14_idx.0], Some(Allocation::Reg(3))); - assert_eq!(assignments[r15_idx.0], Some(Allocation::Reg(2))); + assert_eq!(assignments[r10_idx], Some(Allocation::Reg(0))); + assert_eq!(assignments[r11_idx], Some(Allocation::Reg(1))); + assert_eq!(assignments[r12_idx], Some(Allocation::Reg(1))); + assert_eq!(assignments[r13_idx], Some(Allocation::Reg(2))); + assert_eq!(assignments[r14_idx], Some(Allocation::Reg(3))); + assert_eq!(assignments[r15_idx], Some(Allocation::Reg(2))); } #[test] @@ -4327,12 +4410,12 @@ mod tests { let r15_idx = if let Opnd::VReg { idx, .. } = r15 { idx } else { panic!() }; assert_eq!(num_stack_slots, 1); - assert_eq!(assignments[r10_idx.0], Some(Allocation::Stack(0))); - assert_eq!(assignments[r11_idx.0], Some(Allocation::Reg(1))); - assert_eq!(assignments[r12_idx.0], Some(Allocation::Reg(1))); - assert_eq!(assignments[r13_idx.0], Some(Allocation::Reg(2))); - assert_eq!(assignments[r14_idx.0], Some(Allocation::Reg(0))); - assert_eq!(assignments[r15_idx.0], Some(Allocation::Reg(2))); + assert_eq!(assignments[r10_idx], Some(Allocation::Stack(0))); + assert_eq!(assignments[r11_idx], Some(Allocation::Reg(1))); + assert_eq!(assignments[r12_idx], Some(Allocation::Reg(1))); + assert_eq!(assignments[r13_idx], Some(Allocation::Reg(2))); + assert_eq!(assignments[r14_idx], Some(Allocation::Reg(0))); + assert_eq!(assignments[r15_idx], Some(Allocation::Reg(2))); } #[test] @@ -4355,12 +4438,12 @@ mod tests { let r15_idx = if let Opnd::VReg { idx, .. } = r15 { idx } else { panic!() }; assert_eq!(num_stack_slots, 3); - assert_eq!(assignments[r10_idx.0], Some(Allocation::Stack(0))); - assert_eq!(assignments[r11_idx.0], Some(Allocation::Reg(0))); - assert_eq!(assignments[r12_idx.0], Some(Allocation::Stack(1))); - assert_eq!(assignments[r13_idx.0], Some(Allocation::Reg(0))); - assert_eq!(assignments[r14_idx.0], Some(Allocation::Stack(2))); - assert_eq!(assignments[r15_idx.0], Some(Allocation::Reg(0))); + assert_eq!(assignments[r10_idx], Some(Allocation::Stack(0))); + assert_eq!(assignments[r11_idx], Some(Allocation::Reg(0))); + assert_eq!(assignments[r12_idx], Some(Allocation::Stack(1))); + assert_eq!(assignments[r13_idx], Some(Allocation::Reg(0))); + assert_eq!(assignments[r14_idx], Some(Allocation::Stack(2))); + assert_eq!(assignments[r15_idx], Some(Allocation::Reg(0))); } #[test] @@ -4382,11 +4465,11 @@ mod tests { let preferred_registers = asm.preferred_register_assignments(&intervals); let vreg_idx = new_sp.vreg_idx(); - assert_eq!(preferred_registers[vreg_idx.0], Some(sp.unwrap_reg())); + assert_eq!(preferred_registers[vreg_idx], Some(sp.unwrap_reg())); let (assignments, num_stack_slots) = asm.linear_scan(intervals, 0, &preferred_registers); assert_eq!(num_stack_slots, 0); - assert_eq!(assignments[vreg_idx.0], Some(Allocation::Fixed(sp.unwrap_reg()))); + assert_eq!(assignments[vreg_idx], Some(Allocation::Fixed(sp.unwrap_reg()))); } #[test] @@ -4535,8 +4618,8 @@ mod tests { asm.basic_blocks[b1.0].push_insn(Insn::Add { left: Opnd::UImm(123), right: Opnd::UImm(0), out: v0 }); asm.basic_blocks[b1.0].push_insn(Insn::Add { left: v0, right: Opnd::UImm(456), out: v1 }); asm.basic_blocks[b1.0].push_insn(Insn::Cmp { left: v1, right: Opnd::UImm(0) }); - asm.basic_blocks[b1.0].push_insn(Insn::Jl(Target::Block(BranchEdge { target: b2, args: vec![v0] }))); - asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { target: b3, args: vec![v1] }))); + asm.basic_blocks[b1.0].push_insn(Insn::Jl(Target::Block(Box::new(BranchEdge { target: b2, args: vec![v0] })))); + asm.basic_blocks[b1.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b3, args: vec![v1] })))); // b2(v2): v3 = Add(v2, 789), Jmp(b3, [v3]) asm.set_current_block(b2); @@ -4546,7 +4629,7 @@ mod tests { asm.basic_blocks[b2.0].add_parameter(v2); let v3 = asm.new_vreg(64); asm.basic_blocks[b2.0].push_insn(Insn::Add { left: v2, right: Opnd::UImm(789), out: v3 }); - asm.basic_blocks[b2.0].push_insn(Insn::Jmp(Target::Block(BranchEdge { target: b3, args: vec![v3] }))); + asm.basic_blocks[b2.0].push_insn(Insn::Jmp(Target::Block(Box::new(BranchEdge { target: b3, args: vec![v3] })))); // b3(v4): CRet(v4) asm.set_current_block(b3); @@ -4573,8 +4656,8 @@ mod tests { assert_eq!(asm.basic_blocks.len(), 3); // Verify v1 and v4 have different allocations (so moves are needed) - let v1_alloc = assignments[v1.vreg_idx().0].unwrap(); - let v4_alloc = assignments[v4.vreg_idx().0].unwrap(); + let v1_alloc = assignments[v1.vreg_idx()].unwrap(); + let v4_alloc = assignments[v4.vreg_idx()].unwrap(); assert_ne!(v1_alloc, v4_alloc, "Test setup: v1 and v4 should have different allocations"); asm.resolve_ssa(&intervals, &assignments); @@ -4609,7 +4692,7 @@ mod tests { assert!(has_mov, "Expected Mov in split block for v1->v4"); // b2->b3 is not a critical edge (b2 has single succ), so moves go before Jmp in b2 - let v3_alloc = assignments[v3.vreg_idx().0].unwrap(); + let v3_alloc = assignments[v3.vreg_idx()].unwrap(); let b2_insns = &asm.basic_blocks[b2.0].insns; if v3_alloc != v4_alloc { // Check that a Mov was inserted before the Jmp in b2 @@ -4645,9 +4728,9 @@ mod tests { // v3 = CCall { fptr: UImm(0xF00), opnds: [v2] } let v3 = asm.new_vreg(64); asm.basic_blocks[b1.0].push_insn(Insn::CCall { - opnds: vec![v2], - stack_map: None, data: Box::new(CCallData { + opnds: vec![v2], + stack_map: None, fptr: Opnd::UImm(0xF00), start_marker: None, end_marker: None, @@ -4678,10 +4761,10 @@ mod tests { let regs = &ALLOC_REGS[..num_regs]; // v0 should be spilled (long-lived, only 2 regs) - assert!(matches!(assignments[v0.vreg_idx().0], Some(Allocation::Stack(_))), + assert!(matches!(assignments[v0.vreg_idx()], Some(Allocation::Stack(_))), "v0 should be spilled to stack"); // v1 should be in a register - assert!(matches!(assignments[v1.vreg_idx().0], Some(Allocation::Reg(_))), + assert!(matches!(assignments[v1.vreg_idx()], Some(Allocation::Reg(_))), "v1 should be in a register"); // Run the pipeline: handle_caller_saved_regs then resolve_ssa @@ -4697,7 +4780,7 @@ mod tests { assert!(!pushes.is_empty(), "Expected at least one saved register across CCall"); // The survivor register should match v1's allocation - let v1_reg = match assignments[v1.vreg_idx().0].unwrap() { + let v1_reg = match assignments[v1.vreg_idx()].unwrap() { Allocation::Reg(n) => Opnd::Reg(regs[n]), Allocation::Fixed(reg) => Opnd::Reg(reg), _ => unreachable!(), @@ -4709,8 +4792,8 @@ mod tests { // The CCall should have empty opnds and out = C_RET_OPND (rewritten to regs[0]) let ccall = insns.iter().find(|i| matches!(i, Insn::CCall { .. })).unwrap(); - if let Insn::CCall { opnds, .. } = ccall { - assert!(opnds.is_empty(), "CCall opnds should be empty after handle_caller_saved_regs"); + if let Insn::CCall { data } = ccall { + assert!(data.opnds.is_empty(), "CCall opnds should be empty after handle_caller_saved_regs"); } // v0 should be rewritten to a Stack operand diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 60e735131a1770..7748c2418c06f4 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -298,8 +298,8 @@ impl Assembler { }; asm.push_insn(insn); }, - Insn::CCall { opnds, .. } => { - assert!(opnds.len() <= C_ARG_OPNDS.len()); + Insn::CCall { data } => { + assert!(data.opnds.len() <= C_ARG_OPNDS.len()); // CCall argument setup is handled by handle_caller_saved_regs. asm.push_insn(insn); }, @@ -553,11 +553,11 @@ impl Assembler { asm.push_insn(insn); } // For compile_exits, support splitting simple C arguments here - Insn::CCall { opnds, .. } if !opnds.is_empty() => { - for (i, opnd) in opnds.iter().enumerate() { + Insn::CCall { data } if !data.opnds.is_empty() => { + for (i, opnd) in data.opnds.iter().enumerate() { asm.load_into(C_ARG_OPNDS[i], *opnd); } - *opnds = vec![]; + data.opnds = vec![]; asm.push_insn(insn); } Insn::CSelZ { truthy: left, falsy: right, out } | @@ -658,8 +658,8 @@ impl Assembler { }; asm.store(dest, src); } - &mut Insn::PatchPoint { ref target, invariant, version } => { - split_patch_point(asm, target, invariant, version); + &mut Insn::PatchPoint(ref data) => { + split_patch_point(asm, &data.target, data.invariant, data.version); } _ => { asm.push_insn(insn); @@ -959,7 +959,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jmp_ptr(cb, code_ptr), Target::Label(label) => jmp_label(cb, label), Target::Block(ref edge) => jmp_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } @@ -968,7 +968,7 @@ impl Assembler { Target::CodePtr(code_ptr) => je_ptr(cb, code_ptr), Target::Label(label) => je_label(cb, label), Target::Block(ref edge) => je_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } @@ -977,7 +977,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jne_ptr(cb, code_ptr), Target::Label(label) => jne_label(cb, label), Target::Block(ref edge) => jne_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } @@ -986,7 +986,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jl_ptr(cb, code_ptr), Target::Label(label) => jl_label(cb, label), Target::Block(ref edge) => jl_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } }, @@ -995,7 +995,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jg_ptr(cb, code_ptr), Target::Label(label) => jg_label(cb, label), Target::Block(ref edge) => jg_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } }, @@ -1004,7 +1004,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jge_ptr(cb, code_ptr), Target::Label(label) => jge_label(cb, label), Target::Block(ref edge) => jge_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } }, @@ -1013,7 +1013,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jbe_ptr(cb, code_ptr), Target::Label(label) => jbe_label(cb, label), Target::Block(ref edge) => jbe_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } }, @@ -1022,7 +1022,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jb_ptr(cb, code_ptr), Target::Label(label) => jb_label(cb, label), Target::Block(ref edge) => jb_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } }, @@ -1031,7 +1031,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jz_ptr(cb, code_ptr), Target::Label(label) => jz_label(cb, label), Target::Block(ref edge) => jz_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } @@ -1040,7 +1040,7 @@ impl Assembler { Target::CodePtr(code_ptr) => jnz_ptr(cb, code_ptr), Target::Label(label) => jnz_label(cb, label), Target::Block(ref edge) => jnz_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } @@ -1050,13 +1050,13 @@ impl Assembler { Target::CodePtr(code_ptr) => jo_ptr(cb, code_ptr), Target::Label(label) => jo_label(cb, label), Target::Block(ref edge) => jo_label(cb, self.block_label(edge.target)), - Target::SideExit { .. } => unreachable!("Target::SideExit should have been compiled by compile_exits"), + Target::SideExit(..) => unreachable!("Target::SideExit should have been compiled by compile_exits"), } } Insn::Joz(..) | Insn::Jonz(..) => unreachable!("Joz/Jonz should be unused for now"), - Insn::PatchPoint { .. } => unreachable!("PatchPoint should have been lowered to PadPatchPoint in x86_scratch_split"), + Insn::PatchPoint(..) => unreachable!("PatchPoint should have been lowered to PadPatchPoint in x86_scratch_split"), Insn::PadPatchPoint => { // If patch points are too close to each other or the end of the block, fill nop instructions if let Some(last_patch_pos) = last_patch_pos { @@ -1260,7 +1260,7 @@ mod tests { (asm, CodeBlock::new_dummy()) } - fn stack_mem(stack_idx: usize) -> Opnd { + fn stack_mem(stack_idx: StackIdx) -> Opnd { Opnd::Mem(Mem { base: MemBase::Stack { stack_idx, num_bits: 64 }, disp: 0, @@ -1268,7 +1268,7 @@ mod tests { }) } - fn stack_indirect_mem(stack_idx: usize) -> Opnd { + fn stack_indirect_mem(stack_idx: StackIdx) -> Opnd { Opnd::Mem(Mem { base: MemBase::StackIndirect { stack_idx }, disp: 0, @@ -1388,7 +1388,7 @@ mod tests { let val64 = asm.add(CFP, Opnd::UImm(64)); asm.store(Opnd::mem(64, SP, 0x10), val64); - let side_exit = Target::SideExit { reason: SideExitReason::Interrupt, exit: Box::new(SideExit { pc: 0.into(), iseq: std::ptr::null(), stack: vec![], locals: vec![], recompile: None }) }; + let side_exit = Target::SideExit(Box::new(SideExitTarget { reason: SideExitReason::Interrupt, exit: SideExit { pc: 0.into(), iseq: std::ptr::null(), stack: vec![], locals: vec![], recompile: None } })); asm.push_insn(Insn::Joz(val64, side_exit)); asm.mov(C_ARG_OPNDS[0], C_RET_OPND.with_num_bits(32)); asm.mov(C_ARG_OPNDS[1], Opnd::mem(64, SP, -8)); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index bca08013edba79..a3cbade3609a3e 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -19,7 +19,7 @@ use crate::state::ZJITState; use crate::stats::{CompileError, exit_counter_for_compile_error, exit_counter_for_unhandled_hir_insn, incr_counter, incr_counter_by, send_fallback_counter, send_fallback_counter_for_method_type, send_fallback_counter_for_super_method_type, send_fallback_counter_ptr_for_opcode, send_without_block_fallback_counter_for_method_type, send_without_block_fallback_counter_for_optimized_method_type}; use crate::stats::{counter_ptr, with_time_stat, trace_compile_phase, Counter, Counter::{compile_time_ns, exit_compile_error}}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; -use crate::backend::lir::{self, Assembler, C_ARG_OPNDS, C_RET_OPND, CFP, EC, NATIVE_BASE_PTR, Opnd, SP, SideExit, SideExitRecompile, Target, asm_ccall, asm_comment}; +use crate::backend::lir::{self, Assembler, C_ARG_OPNDS, C_RET_OPND, CFP, EC, NATIVE_BASE_PTR, Opnd, SP, SideExit, SideExitRecompile, SideExitTarget, Target, asm_ccall, asm_comment}; use crate::hir::{iseq_to_hir, BlockId, Invariant, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; use crate::hir::{BlockHandler, CCallVariadicData, CCallWithFrameData, Const, FieldName, FrameState, Function, Insn, InsnId, Recompile, SendFallbackReason}; use crate::hir_type::{types, Type}; @@ -122,7 +122,7 @@ impl Assembler { args: vec![], }; emit(self, target); - self.jmp(Target::Block(fall_through_edge)); + self.jmp(Target::Block(Box::new(fall_through_edge))); self.set_current_block(fall_through_target); @@ -486,8 +486,8 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func }; asm.test(val_opnd, val_opnd); - asm.push_insn(lir::Insn::Jnz(Target::Block(true_branch))); - asm.jmp(Target::Block(false_branch)); + asm.push_insn(lir::Insn::Jnz(Target::Block(Box::new(true_branch)))); + asm.jmp(Target::Block(Box::new(false_branch))); assert!(asm.current_block().insns.last().unwrap().is_terminator()); } @@ -497,7 +497,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func target: lir_target, args: target.args.iter().map(|insn_id| jit.get_opnd(*insn_id)).collect() }; - asm.jmp(Target::Block(branch_edge)); + asm.jmp(Target::Block(Box::new(branch_edge))); assert!(asm.current_block().insns.last().unwrap().is_terminator()); // Jump should always be the last instruction in an HIR block @@ -968,7 +968,7 @@ fn gen_patch_point(jit: &mut JITState, asm: &mut Assembler, invariant: &Invarian let exit = build_side_exit(jit, state); // Let compile_exits compile a side exit. Let scratch_split lower it with split_patch_point. - asm.patch_point(Target::SideExit { exit: Box::new(exit), reason: PatchPoint(invariant) }, invariant, jit.version); + asm.patch_point(Target::SideExit(Box::new(SideExitTarget { exit, reason: PatchPoint(invariant) })), invariant, jit.version); } /// This is used by scratch_split to lower PatchPoint into PadPatchPoint and PosMarker. @@ -1391,7 +1391,7 @@ fn gen_write_barrier(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, val: O let hir_block_id = asm.current_block().hir_block_id; let rpo_idx = asm.current_block().rpo_index; let result_block = asm.new_block(hir_block_id, false, rpo_idx); - let result_edge = Target::Block(lir::BranchEdge { target: result_block, args: vec![] }); + let result_edge = Target::Block(Box::new(lir::BranchEdge { target: result_block, args: vec![] })); // If non-false immediate, don't fire write barrier asm.test(val, Opnd::UImm(RUBY_IMMEDIATE_MASK as u64)); @@ -2218,10 +2218,10 @@ fn gen_is_a(jit: &mut JITState, asm: &mut Assembler, obj: Opnd, class: Opnd) -> // Create a result block that all paths converge to let result_block = asm.new_block(hir_block_id, false, rpo_idx); - let result_edge = |v| Target::Block(lir::BranchEdge { + let result_edge = |v| Target::Block(Box::new(lir::BranchEdge { target: result_block, args: vec![v], - }); + })); let val = asm.load_mem(obj); @@ -2614,10 +2614,10 @@ fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, val_typ // Create a result block that all paths converge to let result_block = asm.new_block(hir_block_id, false, rpo_idx); - let result_edge = |v| Target::Block(lir::BranchEdge { + let result_edge = |v| Target::Block(Box::new(lir::BranchEdge { target: result_block, args: vec![v], - }); + })); // If val isn't in a register, load it to use it as the base of Opnd::mem later. // TODO: Max thinks codegen should not care about the shapes of the operands except to create them. (Shopify/ruby#685) @@ -2653,10 +2653,10 @@ fn gen_has_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, val_typ // Create a result block that all paths converge to let result_block = asm.new_block(hir_block_id, false, rpo_idx); - let result_edge = |v| Target::Block(lir::BranchEdge { + let result_edge = |v| Target::Block(Box::new(lir::BranchEdge { target: result_block, args: vec![v], - }); + })); // If val isn't in a register, load it to use it as the base of Opnd::mem later. let val = asm.load_mem(val); @@ -3179,7 +3179,7 @@ fn compile_iseq(iseq: IseqPtr) -> Result { /// Build a Target::SideExit fn side_exit(jit: &JITState, state: &FrameState, reason: SideExitReason) -> Target { let exit = build_side_exit(jit, state); - Target::SideExit { exit: Box::new(exit), reason } + Target::SideExit(Box::new(SideExitTarget { exit, reason })) } /// Build a Target::SideExit that optionally triggers exit_recompile on the exit path. @@ -3189,7 +3189,7 @@ fn side_exit_with_recompile(jit: &JITState, state: &FrameState, reason: SideExit compiled_iseq: Opnd::Value(VALUE::from(jit.iseq())), insn_idx: state.insn_idx() as u32, }); - Target::SideExit { exit: Box::new(exit), reason } + Target::SideExit(Box::new(SideExitTarget { exit, reason })) } /// Build a side-exit context