Refactor Host expression to MemorySize and MemoryGrow

[dcodeIO]

The complexity of the Host expression in the C and JS APIs has bugged me for quite a while, and in #3130 (comment) I was reminded of it, so I thought I may as well just refactor it to get it out of the way. Now aligns well with other memory instructions, passes can optimize memory.size away if unused, and the APIs are much simpler.

[dcodeIO]

Invocations so far:
   FuzzExec: 26917
   CompareVMs: 7320
   CheckDeterminism: 2327
   Wasm2JS: 6618
   Asyncify: 6903

ITERATION: 31718

[tlively]

Thanks for doing this :)

[tlively]

Can you add readsMemory to memory.grow as well? It technically does a read-modify-write operation on the memory size in the successful case and just a read operation on the memory size in the failure case.

[dcodeIO]

Invocations so far:
   FuzzExec: 12727
   CompareVMs: 3544
   CheckDeterminism: 1045
   Wasm2JS: 3104
   Asyncify: 3340

ITERATION: 15019

[aardappel]

This caused a ton of conflicts in #3130.. given that this was just a nice to have refactor, that could have waited.

[dcodeIO]

Oh, sorry, I actually thought this would help to tackle the Host related issues. If you want, I can take a look at merging it into your PR?

[aardappel]

I've already made all the changes.
It does not change the issue I mentioned in that PR, we still need to find a way to pass the current memory index type to those finalize calls.

[dcodeIO]

Would it be possible to pass it upon construction of the respective expression and keep it as a field, like in the builder?

[aardappel]

That would be fine with me.. @kripken does that sound acceptable? I guess the total number of memory.size / memory.grow ops will never affect total memory usage :)

[tlively]

I would add it as an argument to the finalize calls and force everyone that explicitly calls finalize on those expressions to supply it. That way it just has to be a field passed in the Builder or ReFinalize or ReFinalizeNode constructors.

[kripken]

Given these are rare nodes, memory isn't an issue, yeah. So it could be either way. I think I agree with @tlively that passing it to finalize makes sense (avoids duplicating info that could get stale).

[aardappel]

@tlively that's what we had so far, but it was problematic, since finalize is called from sites that don't have the current memory/module already available.

[tlively]

@aardappel can you share a link to such a site in your PR?

[aardappel]

@tlively it's in one of the comments on the PR #3130:

Ok, changed the way Host::finalize works and is called to satisfy the tests, but it is very broken. What really needs to happen is for it to use the current pointer size, which depends on the current memory/module, which is not accessible from most of these functions calling finalize. We could stick an is64 bool in the visitors calling this function, e.g. BinaryenExpressionFinalize would need to take a module argument, changing the JS API etc.

But I've already changed it to work the way @dcodeIO suggested.