Skip to content

Commit 9f8c057

Browse files
tlivelytlively
authored
Fix up RMW operand types in TypeRefining (#8525)
When TypeRefining manages to refine a field type using GUFA as its analysis, it's possible that the written operand is not actually a subtype of the refined field type. This can happen when GUFA determines that the writing operation is never actually executed. To maintain valid code in this case, TypeRefining inserts a cast on the operand to fix up its type. We were missing this fixup logic for struct RMW operations. Fix the bug and add tests, including for array operations. TypeRefining does not optimize arrays, but if it does in the future, these tests will prevent a similar bug from occurring.
1 parent 9b9f3f2 commit 9f8c057

File tree

2 files changed

+193
-0
lines changed

2 files changed

+193
-0
lines changed

‎src/passes/TypeRefining.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,32 @@ struct TypeRefining : public Pass {
535535
curr->value = fixType(curr->value, fieldType);
536536
}
537537

538+
void visitStructRMW(StructRMW* curr) {
539+
if (curr->type == Type::unreachable) {
540+
return;
541+
}
542+
auto type = curr->ref->type.getHeapType();
543+
if (type.isBottom()) {
544+
return;
545+
}
546+
547+
auto fieldType = type.getStruct().fields[curr->index].type;
548+
curr->value = fixType(curr->value, fieldType);
549+
}
550+
551+
void visitStructCmpxchg(StructCmpxchg* curr) {
552+
if (curr->type == Type::unreachable) {
553+
return;
554+
}
555+
auto type = curr->ref->type.getHeapType();
556+
if (type.isBottom()) {
557+
return;
558+
}
559+
560+
auto fieldType = type.getStruct().fields[curr->index].type;
561+
curr->replacement = fixType(curr->replacement, fieldType);
562+
}
563+
538564
bool refinalize = false;
539565

540566
// Fix up a given value so it fits into the type the location it is

‎test/lit/passes/type-refining-gufa-rmw.wast‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,3 +248,170 @@
248248
)
249249
)
250250
)
251+
252+
(module
253+
;; Test that we fix up types correctly after optimizing.
254+
(rec
255+
;; NRML: (rec
256+
;; NRML-NEXT: (type $struct (struct (field (mut (ref null (exact $inner))))))
257+
;; GUFA: (rec
258+
;; GUFA-NEXT: (type $struct (struct (field (mut (ref (exact $inner))))))
259+
(type $struct (struct (field (mut (ref null $inner)))))
260+
;; TODO: Optimize array types.
261+
;; NRML: (type $array (array (mut (ref null $inner))))
262+
;; GUFA: (type $array (array (mut (ref null $inner))))
263+
(type $array (array (field (mut (ref null $inner)))))
264+
;; NRML: (type $inner (struct))
265+
;; GUFA: (type $inner (struct))
266+
(type $inner (struct))
267+
)
268+
;; NRML: (type $3 (func (param (ref $struct))))
269+
270+
;; NRML: (type $4 (func (param (ref $array))))
271+
272+
;; NRML: (func $fix-struct-type (type $3) (param $uninhabited (ref $struct))
273+
;; NRML-NEXT: (drop
274+
;; NRML-NEXT: (struct.new $struct
275+
;; NRML-NEXT: (struct.new_default $inner)
276+
;; NRML-NEXT: )
277+
;; NRML-NEXT: )
278+
;; NRML-NEXT: (drop
279+
;; NRML-NEXT: (struct.atomic.rmw.xchg $struct 0
280+
;; NRML-NEXT: (local.get $uninhabited)
281+
;; NRML-NEXT: (ref.null none)
282+
;; NRML-NEXT: )
283+
;; NRML-NEXT: )
284+
;; NRML-NEXT: (drop
285+
;; NRML-NEXT: (struct.atomic.rmw.cmpxchg $struct 0
286+
;; NRML-NEXT: (local.get $uninhabited)
287+
;; NRML-NEXT: (ref.null none)
288+
;; NRML-NEXT: (ref.null none)
289+
;; NRML-NEXT: )
290+
;; NRML-NEXT: )
291+
;; NRML-NEXT: )
292+
;; GUFA: (type $3 (func (param (ref $struct))))
293+
294+
;; GUFA: (type $4 (func (param (ref $array))))
295+
296+
;; GUFA: (func $fix-struct-type (type $3) (param $uninhabited (ref $struct))
297+
;; GUFA-NEXT: (drop
298+
;; GUFA-NEXT: (struct.new $struct
299+
;; GUFA-NEXT: (struct.new_default $inner)
300+
;; GUFA-NEXT: )
301+
;; GUFA-NEXT: )
302+
;; GUFA-NEXT: (drop
303+
;; GUFA-NEXT: (struct.atomic.rmw.xchg $struct 0
304+
;; GUFA-NEXT: (local.get $uninhabited)
305+
;; GUFA-NEXT: (ref.cast (ref none)
306+
;; GUFA-NEXT: (ref.null none)
307+
;; GUFA-NEXT: )
308+
;; GUFA-NEXT: )
309+
;; GUFA-NEXT: )
310+
;; GUFA-NEXT: (drop
311+
;; GUFA-NEXT: (struct.atomic.rmw.cmpxchg $struct 0
312+
;; GUFA-NEXT: (local.get $uninhabited)
313+
;; GUFA-NEXT: (ref.null none)
314+
;; GUFA-NEXT: (ref.cast (ref none)
315+
;; GUFA-NEXT: (ref.null none)
316+
;; GUFA-NEXT: )
317+
;; GUFA-NEXT: )
318+
;; GUFA-NEXT: )
319+
;; GUFA-NEXT: )
320+
(func $fix-struct-type (param $uninhabited (ref $struct))
321+
(drop
322+
;; This will make us refine the field type to (ref (exact $inner)).
323+
(struct.new $struct
324+
(struct.new $inner)
325+
)
326+
)
327+
(drop
328+
;; GUFA knows that this cannot possibly be executed because the $uninhabited
329+
;; parameter is never given a value by a caller. As a result, this write of
330+
;; (ref null $inner) does not prevent refinement of the field and is instead
331+
;; fixed up by a cast. We must be sure to update the xchg's type afterward to
332+
;; maintain validity.
333+
(struct.atomic.rmw.xchg $struct 0
334+
(local.get $uninhabited)
335+
(ref.null none)
336+
)
337+
)
338+
(drop
339+
;; Same, but with a cmpxchg.
340+
(struct.atomic.rmw.cmpxchg $struct 0
341+
(local.get $uninhabited)
342+
(ref.null none)
343+
(ref.null none)
344+
)
345+
)
346+
)
347+
;; NRML: (func $fix-array-type (type $4) (param $uninhabited (ref $array))
348+
;; NRML-NEXT: (drop
349+
;; NRML-NEXT: (array.new_fixed $array 1
350+
;; NRML-NEXT: (struct.new_default $inner)
351+
;; NRML-NEXT: )
352+
;; NRML-NEXT: )
353+
;; NRML-NEXT: (drop
354+
;; NRML-NEXT: (array.atomic.rmw.xchg $array
355+
;; NRML-NEXT: (local.get $uninhabited)
356+
;; NRML-NEXT: (i32.const 0)
357+
;; NRML-NEXT: (ref.null none)
358+
;; NRML-NEXT: )
359+
;; NRML-NEXT: )
360+
;; NRML-NEXT: (drop
361+
;; NRML-NEXT: (array.atomic.rmw.cmpxchg $array
362+
;; NRML-NEXT: (local.get $uninhabited)
363+
;; NRML-NEXT: (i32.const 0)
364+
;; NRML-NEXT: (ref.null none)
365+
;; NRML-NEXT: (ref.null none)
366+
;; NRML-NEXT: )
367+
;; NRML-NEXT: )
368+
;; NRML-NEXT: )
369+
;; GUFA: (func $fix-array-type (type $4) (param $uninhabited (ref $array))
370+
;; GUFA-NEXT: (drop
371+
;; GUFA-NEXT: (array.new_fixed $array 1
372+
;; GUFA-NEXT: (struct.new_default $inner)
373+
;; GUFA-NEXT: )
374+
;; GUFA-NEXT: )
375+
;; GUFA-NEXT: (drop
376+
;; GUFA-NEXT: (array.atomic.rmw.xchg $array
377+
;; GUFA-NEXT: (local.get $uninhabited)
378+
;; GUFA-NEXT: (i32.const 0)
379+
;; GUFA-NEXT: (ref.null none)
380+
;; GUFA-NEXT: )
381+
;; GUFA-NEXT: )
382+
;; GUFA-NEXT: (drop
383+
;; GUFA-NEXT: (array.atomic.rmw.cmpxchg $array
384+
;; GUFA-NEXT: (local.get $uninhabited)
385+
;; GUFA-NEXT: (i32.const 0)
386+
;; GUFA-NEXT: (ref.null none)
387+
;; GUFA-NEXT: (ref.null none)
388+
;; GUFA-NEXT: )
389+
;; GUFA-NEXT: )
390+
;; GUFA-NEXT: )
391+
(func $fix-array-type (param $uninhabited (ref $array))
392+
(drop
393+
;; If we optimized array types, this will make us refine the element type to
394+
;; (ref (exact $inner)). TODO.
395+
(array.new_fixed $array 1
396+
(struct.new $inner)
397+
)
398+
)
399+
(drop
400+
;; Same as before, but with an array xchg.
401+
(array.atomic.rmw.xchg $array
402+
(local.get $uninhabited)
403+
(i32.const 0)
404+
(ref.null none)
405+
)
406+
)
407+
(drop
408+
;; Same, but with a cmpxchg.
409+
(array.atomic.rmw.cmpxchg $array
410+
(local.get $uninhabited)
411+
(i32.const 0)
412+
(ref.null none)
413+
(ref.null none)
414+
)
415+
)
416+
)
417+
)

0 commit comments

Comments
 (0)