Skip to content

Commit 1b9b280

Browse files
kripkenkripken
authored
SimplifyGlobals: Do not assume the effect of reading/writing globals implies global.get/set (#8433)
This pass counts global.get/sets carefully, to figure out when all gets and sets are accounted for in the "reads only to write" pattern. However, it could miscount due to function effects: we could find the effect of a read/ write without an actual global.get/set, if we had a call to a function with such contents, and we computed global effects for it.
1 parent 0d9ceef commit 1b9b280

File tree

2 files changed

+355
-0
lines changed

2 files changed

+355
-0
lines changed

‎src/passes/SimplifyGlobals.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,42 @@ struct GlobalUseScanner : public WalkerPass<PostWalker<GlobalUseScanner>> {
170170
if (codeEffects.hasAnything()) {
171171
return Name();
172172
}
173+
// Verify that we actually have a global.set here. We could also have a
174+
// call to a function for whom we have computed function effects, but that
175+
// is not what we want: such a function can be called from other places too.
176+
// This read-only-to-write pattern must contain an actual global.set, so we
177+
// can count the sets in the entire program to confirm that no dangerous
178+
// ones exist. (In other words, we cannot take the shortcut of assuming that
179+
// the effect "writes global $foo" means we actually have a global.set $foo
180+
// here.)
181+
auto found = false;
182+
for (auto* set : FindAll<GlobalSet>(code).list) {
183+
if (set->name == writtenGlobal) {
184+
found = true;
185+
break;
186+
}
187+
}
188+
if (!found) {
189+
return Name();
190+
}
173191

174192
// See if we read that global in the condition expression.
175193
EffectAnalyzer conditionEffects(getPassOptions(), *getModule(), condition);
176194
if (!conditionEffects.mutableGlobalsRead.count(writtenGlobal)) {
177195
return Name();
178196
}
197+
// As above, confirm we see an actual global.get, and not a call to one with
198+
// computed effects.
199+
found = false;
200+
for (auto* get : FindAll<GlobalGet>(condition).list) {
201+
if (get->name == writtenGlobal) {
202+
found = true;
203+
break;
204+
}
205+
}
206+
if (!found) {
207+
return Name();
208+
}
179209

180210
// If the condition has no other (non-removable) effects other than reading
181211
// that global then we have found what we looked for.

‎test/lit/passes/simplify-globals_func-effects.wast‎

Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,328 @@
8989
)
9090
)
9191
)
92+
93+
;; We do not optimize here, as while we have a read-only-to-write global and
94+
;; function, there is another get and set. This setup will be used below to
95+
;; test for effects in calls to those other functions. Note that it is clear we
96+
;; do not optimize by the fact that the global remains mutable and the gets and
97+
;; sets are not modified.
98+
(module
99+
;; CHECK: (type $0 (func))
100+
101+
;; CHECK: (type $1 (func (result i32)))
102+
103+
;; CHECK: (global $global (mut i32) (i32.const 0))
104+
(global $global (mut i32) (i32.const 0))
105+
106+
;; CHECK: (export "read-only-to-write" (func $read-only-to-write))
107+
108+
;; CHECK: (export "set" (func $set))
109+
110+
;; CHECK: (export "get" (func $get))
111+
112+
;; CHECK: (func $read-only-to-write
113+
;; CHECK-NEXT: (if
114+
;; CHECK-NEXT: (i32.eqz
115+
;; CHECK-NEXT: (global.get $global)
116+
;; CHECK-NEXT: )
117+
;; CHECK-NEXT: (then
118+
;; CHECK-NEXT: (global.set $global
119+
;; CHECK-NEXT: (i32.const 1)
120+
;; CHECK-NEXT: )
121+
;; CHECK-NEXT: )
122+
;; CHECK-NEXT: )
123+
;; CHECK-NEXT: )
124+
(func $read-only-to-write (export "read-only-to-write")
125+
(if
126+
(i32.eqz
127+
(global.get $global)
128+
)
129+
(then
130+
(global.set $global
131+
(i32.const 1)
132+
)
133+
)
134+
)
135+
)
136+
137+
;; CHECK: (func $set
138+
;; CHECK-NEXT: (global.set $global
139+
;; CHECK-NEXT: (i32.const 1)
140+
;; CHECK-NEXT: )
141+
;; CHECK-NEXT: )
142+
(func $set (export "set")
143+
(global.set $global
144+
(i32.const 1)
145+
)
146+
)
147+
148+
;; CHECK: (func $get (result i32)
149+
;; CHECK-NEXT: (global.get $global)
150+
;; CHECK-NEXT: )
151+
(func $get (export "get") (result i32)
152+
(global.get $global)
153+
)
154+
)
155+
156+
(module
157+
;; CHECK: (type $0 (func))
158+
159+
;; CHECK: (type $1 (func (result i32)))
160+
161+
;; CHECK: (global $global (mut i32) (i32.const 0))
162+
(global $global (mut i32) (i32.const 0))
163+
164+
;; CHECK: (export "read-only-to-write" (func $read-only-to-write))
165+
166+
;; CHECK: (export "set" (func $set))
167+
168+
;; CHECK: (export "get" (func $get))
169+
170+
;; CHECK: (func $read-only-to-write
171+
;; CHECK-NEXT: (if
172+
;; CHECK-NEXT: (block (result i32)
173+
;; CHECK-NEXT: (drop
174+
;; CHECK-NEXT: (call $get)
175+
;; CHECK-NEXT: )
176+
;; CHECK-NEXT: (i32.const 1)
177+
;; CHECK-NEXT: )
178+
;; CHECK-NEXT: (then
179+
;; CHECK-NEXT: (global.set $global
180+
;; CHECK-NEXT: (i32.const 0)
181+
;; CHECK-NEXT: )
182+
;; CHECK-NEXT: )
183+
;; CHECK-NEXT: )
184+
;; CHECK-NEXT: )
185+
(func $read-only-to-write (export "read-only-to-write")
186+
;; This looks like it reads the global only to write it: we read the global
187+
;; through a call, then write it. However, that call is not an actual
188+
;; global.get, so we must not miscount here. There is only one global.get in
189+
;; the program, but it is *not* here, rather it is in a place it could be
190+
;; read from, separately (indeed, it is an export), so we *cannot* optimize
191+
;; the global as a read-only-to-write global: it has a read we do not
192+
;; control, which will in fact contain 1 after the "set" global is called, so
193+
;; we should not turn it into a constant 0 (and the global should remain
194+
;; mutable).
195+
(if
196+
(block (result i32)
197+
(drop
198+
(call $get)
199+
)
200+
(i32.const 1)
201+
)
202+
(then
203+
(global.set $global
204+
(i32.const 0)
205+
)
206+
)
207+
)
208+
)
209+
210+
;; CHECK: (func $set
211+
;; CHECK-NEXT: (global.set $global
212+
;; CHECK-NEXT: (i32.const 1)
213+
;; CHECK-NEXT: )
214+
;; CHECK-NEXT: )
215+
(func $set (export "set")
216+
(global.set $global
217+
(i32.const 1)
218+
)
219+
)
220+
221+
;; CHECK: (func $get (result i32)
222+
;; CHECK-NEXT: (global.get $global)
223+
;; CHECK-NEXT: )
224+
(func $get (export "get") (result i32)
225+
(global.get $global)
226+
)
227+
)
228+
229+
;; As above, but now we call out to set the global, rather than get. Again, we
230+
;; do not optimize.
231+
(module
232+
;; CHECK: (type $0 (func))
233+
234+
;; CHECK: (type $1 (func (result i32)))
235+
236+
;; CHECK: (global $global (mut i32) (i32.const 0))
237+
(global $global (mut i32) (i32.const 0))
238+
239+
;; CHECK: (export "read-only-to-write" (func $read-only-to-write))
240+
241+
;; CHECK: (export "set" (func $set))
242+
243+
;; CHECK: (export "get" (func $get))
244+
245+
;; CHECK: (func $read-only-to-write
246+
;; CHECK-NEXT: (if
247+
;; CHECK-NEXT: (i32.eqz
248+
;; CHECK-NEXT: (global.get $global)
249+
;; CHECK-NEXT: )
250+
;; CHECK-NEXT: (then
251+
;; CHECK-NEXT: (call $set)
252+
;; CHECK-NEXT: )
253+
;; CHECK-NEXT: )
254+
;; CHECK-NEXT: )
255+
(func $read-only-to-write (export "read-only-to-write")
256+
(if
257+
(i32.eqz
258+
(global.get $global)
259+
)
260+
(then
261+
(call $set)
262+
)
263+
)
264+
)
265+
266+
;; CHECK: (func $set
267+
;; CHECK-NEXT: (global.set $global
268+
;; CHECK-NEXT: (i32.const 1)
269+
;; CHECK-NEXT: )
270+
;; CHECK-NEXT: )
271+
(func $set (export "set")
272+
(global.set $global
273+
(i32.const 1)
274+
)
275+
)
276+
277+
;; CHECK: (func $get (result i32)
278+
;; CHECK-NEXT: (global.get $global)
279+
;; CHECK-NEXT: )
280+
(func $get (export "get") (result i32)
281+
(global.get $global)
282+
)
283+
)
284+
285+
;; As above, but now we call out to set and get. Again, we do not optimize.
286+
(module
287+
;; CHECK: (type $0 (func))
288+
289+
;; CHECK: (type $1 (func (result i32)))
290+
291+
;; CHECK: (global $global (mut i32) (i32.const 0))
292+
(global $global (mut i32) (i32.const 0))
293+
294+
;; CHECK: (export "read-only-to-write" (func $read-only-to-write))
295+
296+
;; CHECK: (export "set" (func $set))
297+
298+
;; CHECK: (export "get" (func $get))
299+
300+
;; CHECK: (func $read-only-to-write
301+
;; CHECK-NEXT: (if
302+
;; CHECK-NEXT: (block (result i32)
303+
;; CHECK-NEXT: (drop
304+
;; CHECK-NEXT: (call $get)
305+
;; CHECK-NEXT: )
306+
;; CHECK-NEXT: (i32.const 1)
307+
;; CHECK-NEXT: )
308+
;; CHECK-NEXT: (then
309+
;; CHECK-NEXT: (call $set)
310+
;; CHECK-NEXT: )
311+
;; CHECK-NEXT: )
312+
;; CHECK-NEXT: )
313+
(func $read-only-to-write (export "read-only-to-write")
314+
(if
315+
(block (result i32)
316+
(drop
317+
(call $get)
318+
)
319+
(i32.const 1)
320+
)
321+
(then
322+
(call $set)
323+
)
324+
)
325+
)
326+
327+
;; CHECK: (func $set
328+
;; CHECK-NEXT: (global.set $global
329+
;; CHECK-NEXT: (i32.const 1)
330+
;; CHECK-NEXT: )
331+
;; CHECK-NEXT: )
332+
(func $set (export "set")
333+
(global.set $global
334+
(i32.const 1)
335+
)
336+
)
337+
338+
;; CHECK: (func $get (result i32)
339+
;; CHECK-NEXT: (global.get $global)
340+
;; CHECK-NEXT: )
341+
(func $get (export "get") (result i32)
342+
(global.get $global)
343+
)
344+
)
345+
346+
;; As above, but with a second global.
347+
(module
348+
;; CHECK: (type $0 (func))
349+
350+
;; CHECK: (type $1 (func (result i32)))
351+
352+
;; CHECK: (global $global (mut i32) (i32.const 0))
353+
(global $global (mut i32) (i32.const 0))
354+
355+
;; CHECK: (global $other i32 (i32.const 0))
356+
(global $other (mut i32) (i32.const 0))
357+
358+
;; CHECK: (export "read-only-to-write" (func $read-only-to-write))
359+
360+
;; CHECK: (export "set" (func $set))
361+
362+
;; CHECK: (export "get" (func $get))
363+
364+
;; CHECK: (func $read-only-to-write
365+
;; CHECK-NEXT: (if
366+
;; CHECK-NEXT: (block (result i32)
367+
;; CHECK-NEXT: (drop
368+
;; CHECK-NEXT: (call $get)
369+
;; CHECK-NEXT: )
370+
;; CHECK-NEXT: (i32.const 0)
371+
;; CHECK-NEXT: )
372+
;; CHECK-NEXT: (then
373+
;; CHECK-NEXT: (global.set $global
374+
;; CHECK-NEXT: (i32.const 0)
375+
;; CHECK-NEXT: )
376+
;; CHECK-NEXT: )
377+
;; CHECK-NEXT: )
378+
;; CHECK-NEXT: )
379+
(func $read-only-to-write (export "read-only-to-write")
380+
;; We *do* have a global.get here, but it is of the wrong global, $other, so
381+
;; we should not optimize $global (we can optimize $other to be immutable,
382+
;; though, and apply its value of 0 here).
383+
(if
384+
(block (result i32)
385+
(drop
386+
(call $get)
387+
)
388+
(global.get $other)
389+
)
390+
(then
391+
(global.set $global
392+
(i32.const 0)
393+
)
394+
)
395+
)
396+
)
397+
398+
;; CHECK: (func $set
399+
;; CHECK-NEXT: (global.set $global
400+
;; CHECK-NEXT: (i32.const 1)
401+
;; CHECK-NEXT: )
402+
;; CHECK-NEXT: )
403+
(func $set (export "set")
404+
(global.set $global
405+
(i32.const 1)
406+
)
407+
)
408+
409+
;; CHECK: (func $get (result i32)
410+
;; CHECK-NEXT: (global.get $global)
411+
;; CHECK-NEXT: )
412+
(func $get (export "get") (result i32)
413+
(global.get $global)
414+
)
415+
)
416+

0 commit comments

Comments
 (0)