Ensure symmetric results in PossibleConstantValues

[kripken]

Previously we could return different results depending on the order we
noted things:

note(anyref.null);
note(funcref.null);
get() => anyref.null

note(funcref.null);
note(anyref.null);
get() => funcref.null

This is correct, as nulls are equal anyhow, and any could be used in
the location we are optimizing. However, it can lead to nondeterminism
if the caller's order of notes is nondeterministic. That is the case in
DeadArgumentElimination, where we scan functions in parallel, then
merge them without special ordering.

To fix this, make the note operation symmetric. That seems simplest and
least likely to be confusing. We can use the LUB to do that.

To avoid duplicating the null logic, refactor note() to use combine().

[tlively]

LGTM, although I guess we will have to update this code somehow if we remove func <: any and end up with multiple unrelated nulls.

[aheejin]

LGTM, although I guess we will have to update this code somehow if we remove func <: any and end up with multiple unrelated nulls.

Is func not a subtype anymore? And I noticed externref was deleted from Binaryen recently. Have we decided to use anyref for that purpose and get rid of externref eventually?

[aheejin]

Can we directly use Type::getLeastUpperBound?

[kripken]

We can, but I think the clearer thing is to LUB on the heap type, because nullability is not relevant here... but maybe the real issue here is that makeNull gets a type and not a heap type - if it got a heap type then we wouldn't have two options here, which seems better. Let me look into that as a refactor PR.

[aheejin]

Both of the types are null at this point. Doesn't that mean both types are nullable anyway here?

[kripken]

Yes, exactly, the types must be nullable anyhow. So we really just need the least upper bound of the heap type, which is the only thing that can be different between them.

After #4664 the code here will be something like this, which I think is good:

auto lub = HeapType::getLeastUpperBound(type, otherType)
if (lub != type) {
  value = Literal::makeNull(lub);
  return true;
}

[kripken]

Updated after that PR landed.

[tlively]

Is func not a subtype anymore?

func is still a subtype of any for now, although we're discussing changing this: WebAssembly/gc#293

And I noticed externref was deleted from Binaryen recently. Have we decided to use anyref for that purpose and get rid of externref eventually?

Yep, we decided to make extern and any the same type, so we didn't need both of them in Binaryen anymore.