Heap2Local bug on cmpxchng

[kripken]

(module
 (rec
  (type $0 (sub (shared (array v128))))
  (type $1 (sub (shared (struct (field (mut (ref null $1)))))))
  (type $2 (shared (func (param (ref $5)) (result i32))))
  (type $3 (sub (func (param f64) (result contref))))
  (type $4 (sub (shared (struct (field i32) (field i8) (field (ref null $8)) (field (mut (ref (shared any)))) (field i32)))))
  (type $5 (func (param v128 f64) (result (ref null $1))))
  (type $6 (sub $0 (shared (array v128))))
  (type $7 (sub (shared (array v128))))
  (type $8 (sub $4 (shared (struct (field i32) (field i8) (field (ref null $8)) (field (mut (ref (shared any)))) (field i32)))))
  (type $9 (sub $4 (shared (struct (field i32) (field i8) (field (ref $8)) (field (mut (ref (shared any)))) (field i32) (field (mut (ref null (shared extern))))))))
  (type $10 (shared (func (param (ref null $5) f64 (ref null $3) i64 (ref $2) nullcontref (ref null $1)) (result (ref null $4) i32 (ref null $4) (ref $9) (ref null $11)))))
  (type $11 (sub $9 (shared (struct (field i32) (field i8) (field (ref $8)) (field (mut (ref (shared any)))) (field i32) (field (mut (ref null (shared extern))))))))
 )
 (export "func_482" (func $0))
 (@binaryen.js.called)
 (func $0 (type $5) (param $0 v128) (param $1 f64) (result (ref null $1))
  (local $2 (ref $1))
  (struct.atomic.rmw.xchg acqrel acqrel $1 0
   (local.tee $2
    (struct.new_default $1)
   )
   (struct.new $1
    (struct.atomic.rmw.cmpxchg acqrel acqrel $1 0
     (local.get $2)
     (struct.new_default $1)
     (struct.new_default $1)
    )
   )
  )
 )
)

$ bin/wasm-opt -all --fuzz-exec w.wat --heap2local
[fuzz-exec] export func_482
[fuzz-exec] note result: func_482 => null
[fuzz-exec] export func_482
[fuzz-exec] note result: func_482 => object(null)
[fuzz-exec] comparing func_482
values not identical! shared [ref (type $struct.0 (sub (shared (struct (field (mut (ref null $struct.0))))))) shared nullref] != shared nullref
[fuzz-exec] optimization passes changed results

This looks related to a worry we had back here:

#8491 (comment)

@tlively perhaps worth revisiting that, what do you think?

[tlively]

Simplified:

(module
 (type $struct (shared (struct (field (mut (ref null (shared eq)))))))
 (export "func" (func $func))
 (func $func (result i32)
  (local $local (ref null $struct))
  (drop
   (struct.atomic.rmw.cmpxchg acqrel acqrel $struct 0
    (local.tee $local
     (struct.new_default $struct)
    )
    (struct.new_default $struct)
    (struct.new_default $struct)
   )
  )
  (ref.is_null
   (struct.get $struct 0 (local.get $local))
  )
 )
)

Will investigate further, but the urge is strong to just rewrite Heap2Local to stop interleaving analysis and optimization :)

[kripken]

I hope that instead of a full rewrite, we just need to keep those maps fully updated all the time, as I suggested back in that other issue? Though there could be other issues I'm missing, or other benefits to a rewrite, ofc

[tlively]

The problem is that we don't optimize the ref.eq that's created when we optimize the cmpxchg. If we did, we would see that an allocation flows into one side but not the other, and we would correctly optimize it to a constant 0 instead of letting incorrectly compare two separately optimized values. From most surgical (but hacky) to most general, I can think of the following fixes:

1. Update analyzer.parents and analyzer.scratchInfo enough to let the new ref.eq get optimized correctly when we optimize the "expected" allocation. (Heap2Local: fix stale data cmpxchg bug #8856)
2. Simply clear and recompute analyzer.parents and analyzer.localGraph after each optimized allocation to ensure they are not stale. (Heap2Local: reset data structures after optimizing #8857)
3. Rewrite the pass to analyze all allocations before optimizing, allowing the optimizer to see all allocations flowing into the cmpxchg and letting it optimize the whole thing down to a single local.get. (WIP)