Skip to content

Commit 686c76b

Browse files
kripkenkripken
authored
[Strings] StringGathering: Handle uses of strings before their definitions (#7008)
When we gather strings, we create new globals for each one, that is then the canonical defining global for it, which will then be used everywhere else. We create such a global if we lack one, but if we happen to have such a global - a global that simply defines a string - then we reuse it. But we didn't handle the case where there was a use before the definition, and failed to sort the definition before the use.
1 parent 93883fd commit 686c76b

File tree

2 files changed

+84
-21
lines changed

2 files changed

+84
-21
lines changed

‎src/passes/StringLowering.cpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,18 +111,15 @@ struct StringGathering : public Pass {
111111
// then we can just use that as the global for that string. This avoids
112112
// repeated executions of the pass adding more and more globals.
113113
//
114-
// Note that we don't note these in newNames: They are already in the right
115-
// sorted position, before any uses, as we use the first of them for each
116-
// string. Only actually new names need sorting.
117-
//
118114
// Any time we reuse a global, we must not modify its body (or else we'd
119115
// replace the global that all others read from); we note them here and
120116
// avoid them in replaceStrings later to avoid such trampling.
121117
std::unordered_set<Expression**> stringPtrsToPreserve;
122118

123119
void addGlobals(Module* module) {
124-
// Note all the new names we create for the sorting later.
125-
std::unordered_set<Name> newNames;
120+
// The names of the globals that define a string. Such globals may be
121+
// referred to by others, and so we will need to sort them, later.
122+
std::unordered_set<Name> definingNames;
126123

127124
// Find globals to reuse (see comment on stringPtrsToPreserve for context).
128125
for (auto& global : module->globals) {
@@ -143,7 +140,8 @@ struct StringGathering : public Pass {
143140
for (Index i = 0; i < strings.size(); i++) {
144141
auto& globalName = stringToGlobalName[strings[i]];
145142
if (globalName.is()) {
146-
// We are reusing a global for this one.
143+
// We are reusing a global for this one, with its existing name.
144+
definingNames.insert(globalName);
147145
continue;
148146
}
149147

@@ -160,22 +158,22 @@ struct StringGathering : public Pass {
160158
auto name = Names::getValidGlobalName(
161159
*module, std::string("string.const_") + std::string(escaped.str()));
162160
globalName = name;
163-
newNames.insert(name);
161+
definingNames.insert(name);
164162
auto* stringConst = builder.makeStringConst(string);
165163
auto global =
166164
builder.makeGlobal(name, nnstringref, stringConst, Builder::Immutable);
167165
module->addGlobal(std::move(global));
168166
}
169167

170-
// Sort our new globals to the start, as other global initializers may use
168+
// Sort defining globals to the start, as other global initializers may use
171169
// them (and it would be invalid for us to appear after a use). This sort is
172170
// a simple way to ensure that we validate, but it may be unoptimal (we
173171
// leave that for reorder-globals).
174172
std::stable_sort(
175173
module->globals.begin(),
176174
module->globals.end(),
177175
[&](const std::unique_ptr<Global>& a, const std::unique_ptr<Global>& b) {
178-
return newNames.count(a->name) && !newNames.count(b->name);
176+
return definingNames.count(a->name) && !definingNames.count(b->name);
179177
});
180178
}
181179

‎test/lit/passes/string-gathering.wast‎

Lines changed: 76 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@
1717

1818
;; CHECK: (type $0 (func))
1919

20+
;; CHECK: (global $global (ref string) (string.const "foo"))
21+
(global $global (ref string) (string.const "foo"))
22+
2023
;; CHECK: (global $"string.const_\"bar\"" (ref string) (string.const "bar"))
2124

2225
;; CHECK: (global $"string.const_\"other\"" (ref string) (string.const "other"))
2326

24-
;; CHECK: (global $global (ref string) (string.const "foo"))
25-
(global $global (ref string) (string.const "foo"))
26-
2727
;; CHECK: (global $global2 stringref (global.get $"string.const_\"bar\""))
2828
;; LOWER: (type $0 (array (mut i16)))
2929

@@ -45,11 +45,11 @@
4545

4646
;; LOWER: (type $9 (func (param externref i32 i32) (result (ref extern))))
4747

48-
;; LOWER: (import "string.const" "0" (global $"string.const_\"bar\"" (ref extern)))
48+
;; LOWER: (import "string.const" "0" (global $global (ref extern)))
4949

50-
;; LOWER: (import "string.const" "1" (global $"string.const_\"other\"" (ref extern)))
50+
;; LOWER: (import "string.const" "1" (global $"string.const_\"bar\"" (ref extern)))
5151

52-
;; LOWER: (import "string.const" "2" (global $global (ref extern)))
52+
;; LOWER: (import "string.const" "2" (global $"string.const_\"other\"" (ref extern)))
5353

5454
;; LOWER: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $3) (param (ref null $0) i32 i32) (result (ref extern))))
5555

@@ -165,17 +165,19 @@
165165

166166
;; LOWER: (type $8 (func (param externref i32 i32) (result (ref extern))))
167167

168+
;; LOWER: (import "string.const" "0" (global $global1 (ref extern)))
169+
170+
;; LOWER: (import "string.const" "1" (global $global4 (ref extern)))
171+
168172
;; LOWER: (import "a" "b" (global $import (ref extern)))
169173
(import "a" "b" (global $import (ref string)))
170174

171175
;; CHECK: (global $global1 (ref string) (string.const "foo"))
172176
(global $global1 (ref string) (string.const "foo"))
173177

174-
;; CHECK: (global $global2 (ref string) (global.get $global1))
175-
;; LOWER: (import "string.const" "0" (global $global1 (ref extern)))
176-
177-
;; LOWER: (import "string.const" "1" (global $global4 (ref extern)))
178+
;; CHECK: (global $global4 (ref string) (string.const "bar"))
178179

180+
;; CHECK: (global $global2 (ref string) (global.get $global1))
179181
;; LOWER: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $2) (param (ref null $0) i32 i32) (result (ref extern))))
180182

181183
;; LOWER: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $3) (param i32) (result (ref extern))))
@@ -201,7 +203,6 @@
201203
;; LOWER: (global $global3 (ref extern) (global.get $global1))
202204
(global $global3 (ref string) (string.const "foo"))
203205

204-
;; CHECK: (global $global4 (ref string) (string.const "bar"))
205206
(global $global4 (ref string) (string.const "bar"))
206207

207208
;; CHECK: (global $global5 (ref string) (global.get $global4))
@@ -279,3 +280,67 @@
279280
)
280281
)
281282
)
283+
284+
;; A module where a string (in this case the empty string) appears twice, so we
285+
;; will use a single global for both. The first use of the string appears in a
286+
;; nested position, inside a struct constructor, so we cannot use that one as
287+
;; our defining global, but there is an appropriate global after it. We must be
288+
;; careful to then sort the globals, as $string must then appear before $struct.
289+
(module
290+
;; CHECK: (type $struct (struct (field stringref)))
291+
;; LOWER: (type $0 (array (mut i16)))
292+
293+
;; LOWER: (type $struct (struct (field externref)))
294+
(type $struct (struct (field stringref)))
295+
296+
;; CHECK: (global $string (ref string) (string.const ""))
297+
298+
;; CHECK: (global $struct (ref $struct) (struct.new $struct
299+
;; CHECK-NEXT: (global.get $string)
300+
;; CHECK-NEXT: ))
301+
;; LOWER: (type $2 (func (param externref externref) (result i32)))
302+
303+
;; LOWER: (type $3 (func (param (ref null $0) i32 i32) (result (ref extern))))
304+
305+
;; LOWER: (type $4 (func (param i32) (result (ref extern))))
306+
307+
;; LOWER: (type $5 (func (param externref externref) (result (ref extern))))
308+
309+
;; LOWER: (type $6 (func (param externref (ref null $0) i32) (result i32)))
310+
311+
;; LOWER: (type $7 (func (param externref) (result i32)))
312+
313+
;; LOWER: (type $8 (func (param externref i32) (result i32)))
314+
315+
;; LOWER: (type $9 (func (param externref i32 i32) (result (ref extern))))
316+
317+
;; LOWER: (import "string.const" "0" (global $string (ref extern)))
318+
319+
;; LOWER: (import "wasm:js-string" "fromCharCodeArray" (func $fromCharCodeArray (type $3) (param (ref null $0) i32 i32) (result (ref extern))))
320+
321+
;; LOWER: (import "wasm:js-string" "fromCodePoint" (func $fromCodePoint (type $4) (param i32) (result (ref extern))))
322+
323+
;; LOWER: (import "wasm:js-string" "concat" (func $concat (type $5) (param externref externref) (result (ref extern))))
324+
325+
;; LOWER: (import "wasm:js-string" "intoCharCodeArray" (func $intoCharCodeArray (type $6) (param externref (ref null $0) i32) (result i32)))
326+
327+
;; LOWER: (import "wasm:js-string" "equals" (func $equals (type $2) (param externref externref) (result i32)))
328+
329+
;; LOWER: (import "wasm:js-string" "compare" (func $compare (type $2) (param externref externref) (result i32)))
330+
331+
;; LOWER: (import "wasm:js-string" "length" (func $length (type $7) (param externref) (result i32)))
332+
333+
;; LOWER: (import "wasm:js-string" "charCodeAt" (func $charCodeAt (type $8) (param externref i32) (result i32)))
334+
335+
;; LOWER: (import "wasm:js-string" "substring" (func $substring (type $9) (param externref i32 i32) (result (ref extern))))
336+
337+
;; LOWER: (global $struct (ref $struct) (struct.new $struct
338+
;; LOWER-NEXT: (global.get $string)
339+
;; LOWER-NEXT: ))
340+
(global $struct (ref $struct) (struct.new $struct
341+
(string.const "")
342+
))
343+
344+
(global $string (ref string) (string.const ""))
345+
)
346+

0 commit comments

Comments
 (0)