Skip to content

Commit

Permalink
Use Names::getValidNameGivenExisting in binary reading (#6793)
Browse files Browse the repository at this point in the history
We had a TODO to use it once Names was optimized, which it has been.

The Names version is also far faster. When building
https://github.com/JetBrains/kotlinconf-app it saves 70 seconds(!).
  • Loading branch information
kripken authored Jul 31, 2024
1 parent e6bbff7 commit c689781
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 36 deletions.
11 changes: 3 additions & 8 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "ir/eh-utils.h"
#include "ir/module-utils.h"
#include "ir/names.h"
#include "ir/table-utils.h"
#include "ir/type-updating.h"
#include "support/bits.h"
Expand Down Expand Up @@ -3551,14 +3552,8 @@ class NameProcessor {
std::unordered_set<Name> usedNames;

Name deduplicate(Name base) {
// TODO: Consider using Names::getValidNameGivenExisting but that does give
// longer names, and it is very noticeable in this location, so
// perhaps optimize that first.
Name name = base;
// De-duplicate names by appending .1, .2, etc.
for (int i = 1; !usedNames.insert(name).second; ++i) {
name = std::string(base.str) + std::string(".") + std::to_string(i);
}
auto name = Names::getValidNameGivenExisting(base, usedNames);
usedNames.insert(name);
return name;
}
};
Expand Down
Binary file removed test/duplicated_names.wasm
Binary file not shown.
13 changes: 0 additions & 13 deletions test/duplicated_names.wasm.fromBinary

This file was deleted.

13 changes: 0 additions & 13 deletions test/duplicated_names_collision.wasm.fromBinary

This file was deleted.

21 changes: 21 additions & 0 deletions test/lit/binary/duplicated_names_collision.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
;; RUN: wasm-opt %s.wasm -S -o - | filecheck %s

;; Test handling of duplicated names where the name in the binary format
;; overlaps with the suffix we add to deduplicate. The binary here uses a '.'
;; in one of the names, which will overlap if we use '.2' etc. to differentiate.


;; CHECK: (type $0 (func (result i32)))

;; CHECK: (func $foo (result i32)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )

;; CHECK: (func $foo_1 (result i32)
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )

;; CHECK: (func $foo.1 (result i32)
;; CHECK-NEXT: (i32.const 2)
;; CHECK-NEXT: )
File renamed without changes.
21 changes: 21 additions & 0 deletions test/lit/binary/duplicated_names_collision_underscore.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
;; RUN: wasm-opt %s.wasm -S -o - | filecheck %s

;; Test handling of duplicated names where the name in the binary format
;; overlaps with the suffix we add to deduplicate. This is similar to
;; the non-underscore version of this test, but has an '_' in the name.


;; CHECK: (type $0 (func (result i32)))

;; CHECK: (func $foo (result i32)
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )

;; CHECK: (func $foo_1 (result i32)
;; CHECK-NEXT: (i32.const 1)
;; CHECK-NEXT: )

;; CHECK: (func $foo_1_2 (result i32)
;; CHECK-NEXT: (i32.const 2)
;; CHECK-NEXT: )
Binary file not shown.
5 changes: 3 additions & 2 deletions test/lit/binary/name-overlap.test
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
;; that we leave the name from the names section as it is, and only adjust the
;; temp name.)

;; CHECK: (global $global$1 i32 (i32.const 1))
;; CHECK-NEXT: (global $global$1.1 i32 (i32.const 0))

;; CHECK: (global $global$1 i32 (i32.const 1))

;; CHECK: (global $global$1_1 i32 (i32.const 0))

0 comments on commit c689781

Please sign in to comment.