[wasm] f32.min/max interpreter and inline support

[commonlisp]

Working on supporting missing f32 opcodes in wasm

---

This change is

[msftclas]

Hi @commonlisp, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[msftclas]

Hi @commonlisp, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[MikeHolman]

I took a look and lowering codepath also needs to be changed, i.e. LowerMDShared.cpp around line 9200 we need a case for float32.

[MikeHolman]

if you want to wait and implement in another checkin that is ok, but I would like if we have explicit failure here. Otherwise if someone tries to JIT this it might lead to confusing failure (crash will not happen until lowering or maybe even further)

[abchatra]

Any update on this PR?

[commonlisp]

This PR is unfortunately blocked on #831 for now.

[MikeHolman]

@commonlisp Is this still blocked?

[commonlisp]

@MikeHolman @Cellule Please take a look when you have a chance.

[Cellule]

Could you remove the .amplxeproj files from the commit ?

---

Reviewed 13 of 16 files at r1, 1 of 4 files at r2, 1 of 3 files at r3, 13 of 13 files at r4.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

lib/Backend/LowerMDShared.cpp, line 9292 [r4] (raw file):

                //      JMP $doneLabel
                //
                // $labelNegZeroAndNaNCheckHelper

Could you update this comment to reflect the changes you made

---

lib/Backend/LowerMDShared.cpp, line 9333 [r4] (raw file):

                    if (src1->IsFloat32())
                    {
                        src1f64 = IR::RegOpnd::New(TyFloat64, m_func);

You should call EmitFloat32ToFloat64 which contains additional checks

---

lib/Backend/LowerMDShared.cpp, line 9418 [r4] (raw file):

IR::Opnd* LowererMD::IsOpndNaN(IR::Opnd* opnd, IR::Instr* instr)
{
    IR::Opnd * isNaN = IR::RegOpnd::New(TyInt32, this->m_func);

I think it would be worth to rewrite the NaN implementation here instead of a helper call.
Maybe not right now, but a //todo:: if not

---

lib/Runtime/ByteCode/OpCodes.h, line 759 [r4] (raw file):

MACRO_BACKEND_ONLY(     Trunc_A,            Empty,          OpTempNumberSources | OpCanCSE | OpProducesNumber)
MACRO_BACKEND_ONLY(     Nearest_A,          Empty,          OpTempNumberSources | OpCanCSE | OpProducesNumber)
MACRO_BACKEND_ONLY(MinNaN, Empty, OpTempNumberSources | OpCanCSE | OpProducesNumber)

nit: follow alignment of the file

---

lib/Runtime/Library/JavascriptNumber.h, line 51 [r4] (raw file):

        static bool TryToVarFastWithCheck(double value, Var* result);

        inline static BOOL IsNan(double value) { return NumberUtilities::IsNan(value); }

No need to change this. NumberUtilities::IsNan returns bool

---

lib/Runtime/Math/AsmJsMath.inl, line 9 [r4] (raw file):

    template<typename T>
    __inline T minCheckNan(T aLeft, T aRight)

We are using inline now instead of __inline

---

test/wasm/f32.js, line 6 [r4] (raw file):

//-------------------------------------------------------------------------------------------------------

const blob = WScript.LoadBinaryFile('f32.wasm');

nit: trailing whitespaces

---

test/wasm/rlexe.xml, line 45 [r4] (raw file):

       <files>f32.js</files>
       <baseline>f32.baseline</baseline>
       <compile-flags>-On:Wasm</compile-flags>

Why have a version without -on:wasm ? I think we'll need to have all our tests with -on:wasm if we are to merge with master sometime soon.

---

Comments from Reviewable

[commonlisp]

Review status: 8 of 18 files reviewed at latest revision, 8 unresolved discussions.

---

lib/Runtime/Library/JavascriptNumber.h, line 51 [r4] (raw file):

Previously, Cellule (Michael Ferris) wrote…

No need to change this. NumberUtilities::IsNan returns bool

The subsequent instructions (especially the comparison) are expecting a uint32 which BOOL is defined to be. It breaks with bool.

---

Comments from Reviewable

[Cellule]

---

Reviewed 10 of 20 files at r5.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

Comments from Reviewable

[MikeHolman]

I don't really like calling this MinNaN. IMO would rather call this something like WasmMin, or not create new opcode and in lowering check to see if the function is a wasm function

[commonlisp]

Ok.

[MikeHolman]