ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/runtime
File: src/coreclr/jit/emitwasm.cpp (C++)

Code before:
        case IF_OPCODE:
            break;
        case IF_BLOCK:
            size += 1;
            break;
        case IF_RAW_ULEB128:
            assert(!idIsCnsReloc());
            size = SizeOfULEB128(emitGetInsSC(this));
            break;
        case IF_LOCAL_DECL:
        {
            assert(idIsLclVarDecl());
            uint8_t typeCode = GetWasmValueTypeCode(emitGetLclVarDeclType(this));
            size             = SizeOfULEB128(emitGetLclVarDeclCount(this)) + sizeof(typeCode);
            break;
        }
        case IF_ULEB128:
            size += idIsCnsReloc() ? PADDED_RELOC_SIZE : SizeOfULEB128(emitGetInsSC(this));
            break;
        case IF_SLEB128:
            size += idIsCnsReloc() ? PADDED_RELOC_SIZE : SizeOfSLEB128(emitGetInsSC(this));
            break;
        case IF_CALL_INDIRECT:
        {
            size += SizeOfULEB128(0);
            size += SizeOfULEB128(emitGetInsSC(this));
            break;
        }
        case IF_F32:
            size += 4;
            break;
        case IF_F64:
            size += 8;
            break;
        case IF_MEMARG:
        {
            uint64_t align = emitGetAlignHintLog2(this);
            assert(align < 64); // spec says align > 2^6 produces a memidx for multiple memories.
            size += SizeOfULEB128(align);
            size += idIsCnsReloc() ? PADDED_RELOC_SIZE : SizeOfULEB128(emitGetInsSC(this));
            break;
        }
        default:
            unreached();
    }
    return size;
}

void emitter::emitSetShortJump(instrDescJmp* id)
{
    NYI_WASM("emitSetShortJump");

Review comment (suggestion):
```suggestion
            size += SizeOfULEB128(emitGetInsSC(this));
            size += SizeOfULEB128(0);
```
Just to match the actual operand order.

Code after:
            size += 4;
            break;
        case IF_F64:
            size += 8;
            break;
        case IF_MEMARG:
        {
            uint64_t align = emitGetAlignHintLog2(this);
            assert(align < 64); // spec says align > 2^6 produces a memidx for multiple memories.
            size += SizeOfULEB128(align);
            size += idIsCnsReloc() ? PADDED_RELOC_SIZE : SizeOfULEB128(emitGetInsSC(this));
            break;
        }
        default:
            unreached();
    }
    return size;
}

void emitter::emitSetShortJump(instrDescJmp* id)
{
    NYI_WASM("emitSetShortJump");
}

size_t emitter::emitOutputULEB128(uint8_t* destination, uint64_t value)
{
    uint8_t* buffer = destination + writeableOffset;
    if (value >= 0x80)
    {
        int pos = 0;
        do
        {
            buffer[pos++] = (uint8_t)((value & 0x7F) | ((value >= 0x80) ? 0x80u : 0));
            value >>= 7;
        } while (value > 0);

        return pos;
    }
    else
    {
        buffer[0] = (uint8_t)value;
        return 1;
    }
}

size_t emitter::emitOutputSLEB128(uint8_t* destination, int64_t value)
{
    uint8_t* buffer = destination + writeableOffset;
    bool     cont   = true;
    int      pos    = 0;
    while (cont)
Repository: dotnet/runtime
File: src/coreclr/jit/lower.cpp (C++)

Code before:
//------------------------------------------------------------------------
// Lowering::CheckCallArg: check that a call argument is in an expected
//                         form after lowering.
//
// Arguments:
//   arg - the argument to check.
//
void Lowering::CheckCallArg(GenTree* arg)
{
    if (!arg->IsValue() && !arg->OperIsPutArgStk())
    {
        assert(arg->OperIsStore());
        return;
    }

    switch (arg->OperGet())
    {
        case GT_FIELD_LIST:
        {
            GenTreeFieldList* list = arg->AsFieldList();
            assert(list->isContained());

            for (GenTreeFieldList::Use& use : list->Uses())
            {
                // WASM-FIXME
                assert(!HAS_FIXED_REGISTER_SET || use.GetNode()->OperIsPutArg());
            }
        }
        break;

        default:
            assert(!HAS_FIXED_REGISTER_SET || arg->OperIsPutArg());
            break;
    }
}

//------------------------------------------------------------------------
// Lowering::CheckCall: check that a call is in an expected form after
//                      lowering. Currently this amounts to checking its
//                      arguments, but could be expanded to verify more
//                      properties in the future.
//
// Arguments:
//   call - the call to check.
//
void Lowering::CheckCall(GenTreeCall* call)
{
    for (CallArg& arg : call->gtArgs.EarlyArgs())
    {
        CheckCallArg(arg.GetEarlyNode());
    }

Review comment (suggestion):
```suggestion
                assert(!HAS_FIXED_REGISTER_SET || use.GetNode()->OperIsPutArg());
```
This is indeed how it should be.

Code after:
// Lowering::CheckCallArg: check that a call argument is in an expected
//                         form after lowering.
//
// Arguments:
//   arg - the argument to check.
//
void Lowering::CheckCallArg(GenTree* arg)
{
    if (!arg->IsValue() && !arg->OperIsPutArgStk())
    {
        assert(arg->OperIsStore());
        return;
    }

    switch (arg->OperGet())
    {
        case GT_FIELD_LIST:
        {
            GenTreeFieldList* list = arg->AsFieldList();
            assert(list->isContained());

            for (GenTreeFieldList::Use& use : list->Uses())
            {
                assert(!HAS_FIXED_REGISTER_SET || use.GetNode()->OperIsPutArg());
            }
        }
        break;

        default:
            assert(!HAS_FIXED_REGISTER_SET || arg->OperIsPutArg());
            break;
    }
}

//------------------------------------------------------------------------
// Lowering::CheckCall: check that a call is in an expected form after
//                      lowering. Currently this amounts to checking its
//                      arguments, but could be expanded to verify more
//                      properties in the future.
//
// Arguments:
//   call - the call to check.
//
void Lowering::CheckCall(GenTreeCall* call)
{
    for (CallArg& arg : call->gtArgs.EarlyArgs())
    {
        CheckCallArg(arg.GetEarlyNode());
    }

    for (CallArg& arg : call->gtArgs.LateArgs())
Repository: dotnet/runtime
File: src/coreclr/jit/lower.cpp (C++)

Code before:
//    has no arguments.
//
GenTree* Lowering::FindEarliestPutArg(GenTreeCall* call)
{
    size_t numMarkedNodes = MarkCallPutArgAndFieldListNodes(call);

    if (numMarkedNodes <= 0)
    {
        return nullptr;
    }

    GenTree* node = call;
    do
    {
        node = node->gtPrev;

        assert((node != nullptr) && "Reached beginning of basic block while looking for marked nodes");

        if ((node->gtLIRFlags & LIR::Flags::Mark) != 0)
        {
            node->gtLIRFlags &= ~LIR::Flags::Mark;
            numMarkedNodes--;
        }
    } while (numMarkedNodes > 0);

    // WASM-FIXME: !HAS_FIXED_REGISTER_SET ||
    assert(node->OperIsPutArg());
    return node;
}

//------------------------------------------------------------------------
// MarkCallPutArgNodes: Mark all operand FIELD_LIST and PUTARG nodes
// corresponding to a call.
//
// Arguments:
//   call - the call
//
// Returns:
//   The number of nodes marked.
//
// Remarks:
//   FIELD_LIST operands are marked too, and their PUTARG operands are in turn
//   marked as well.
//
size_t Lowering::MarkCallPutArgAndFieldListNodes(GenTreeCall* call)
{
    size_t numMarkedNodes = 0;
    for (CallArg& arg : call->gtArgs.Args())
    {
        if (arg.GetEarlyNode() != nullptr)
        {

Review comment (suggestion):
```suggestion
```
We're still only marking putargs (`PUTARG_STK`, which we don't use currently), so this assert is correct as-is.

Code after:
//    has no arguments.
//
GenTree* Lowering::FindEarliestPutArg(GenTreeCall* call)
{
    size_t numMarkedNodes = MarkCallPutArgAndFieldListNodes(call);

    if (numMarkedNodes <= 0)
    {
        return nullptr;
    }

    GenTree* node = call;
    do
    {
        node = node->gtPrev;

        assert((node != nullptr) && "Reached beginning of basic block while looking for marked nodes");

        if ((node->gtLIRFlags & LIR::Flags::Mark) != 0)
        {
            node->gtLIRFlags &= ~LIR::Flags::Mark;
            numMarkedNodes--;
        }
    } while (numMarkedNodes > 0);

    assert(node->OperIsPutArg());
    return node;
}

//------------------------------------------------------------------------
// MarkCallPutArgNodes: Mark all operand FIELD_LIST and PUTARG nodes
// corresponding to a call.
//
// Arguments:
//   call - the call
//
// Returns:
//   The number of nodes marked.
//
// Remarks:
//   FIELD_LIST operands are marked too, and their PUTARG operands are in turn
//   marked as well.
//
size_t Lowering::MarkCallPutArgAndFieldListNodes(GenTreeCall* call)
{
    size_t numMarkedNodes = 0;
    for (CallArg& arg : call->gtArgs.Args())
    {
        if (arg.GetEarlyNode() != nullptr)
        {
            numMarkedNodes += MarkPutArgAndFieldListNodes(arg.GetEarlyNode());
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:

    GetEmitter()->emitIns(ins);
    genProduceReg(treeNode);
}

//------------------------------------------------------------------------
// genCodeForConstant: Generate code for an integer or floating point constant
//
// Arguments:
//    treeNode - The constant.
//
void CodeGen::genCodeForConstant(GenTree* treeNode)
{
    instruction    ins  = INS_none;
    cnsval_ssize_t bits = 0;
    var_types      type = treeNode->TypeIs(TYP_REF, TYP_BYREF) ? TYP_I_IMPL : treeNode->TypeGet();
    static_assert(sizeof(cnsval_ssize_t) >= sizeof(double));

    GenTreeIntConCommon* icon = nullptr;
    if ((type == TYP_INT) || (type == TYP_LONG))
    {
        icon = treeNode->AsIntConCommon();
        if (icon->ImmedValNeedsReloc(compiler))
        {
            // WASM-TODO: Generate reloc for this handle
            ins  = INS_I_const;
            bits = 0;
        }
        else
        {
            bits = icon->IntegralValue();
        }
    }

    if (ins == INS_none)
    {
        switch (type)
        {
            case TYP_INT:
            {
                ins = INS_i32_const;
                assert(((INT64)(INT32)bits) == bits);
                break;
            }
            case TYP_LONG:
            {
                ins = INS_i64_const;
                break;
            }
            case TYP_FLOAT:
            {

Review comment (suggestion):
INS_I_const is not a defined instruction. This should use either INS_i32_const or INS_i64_const depending on the pointer size. For WASM, TYP_I_IMPL is either TYP_INT or TYP_LONG depending on the target. The instruction should be selected based on the actual type being used.

Code after:

    GetEmitter()->emitIns(ins);
    genProduceReg(treeNode);
}

//------------------------------------------------------------------------
// genCodeForConstant: Generate code for an integer or floating point constant
//
// Arguments:
//    treeNode - The constant.
//
void CodeGen::genCodeForConstant(GenTree* treeNode)
{
    instruction    ins  = INS_none;
    cnsval_ssize_t bits = 0;
    var_types      type = treeNode->TypeIs(TYP_REF, TYP_BYREF) ? TYP_I_IMPL : treeNode->TypeGet();
    static_assert(sizeof(cnsval_ssize_t) >= sizeof(double));

    GenTreeIntConCommon* icon = nullptr;
    if ((type == TYP_INT) || (type == TYP_LONG))
    {
        icon = treeNode->AsIntConCommon();
        if (icon->ImmedValNeedsReloc(compiler))
        {
            // WASM-TODO: Generate reloc for this handle
            ins  = INS_I_const;
            bits = 0;
        }
        else
        {
            bits = icon->IntegralValue();
        }
    }

    if (ins == INS_none)
    {
        switch (type)
        {
            case TYP_INT:
            {
                ins = INS_i32_const;
                assert(FitsIn<INT32>(bits));
                break;
            }
            case TYP_LONG:
            {
                ins = INS_i64_const;
                break;
            }
            case TYP_FLOAT:
            {
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
//
// Arguments:
//    tree - the node
//
void CodeGen::genCodeForCompare(GenTreeOp* tree)
{
    assert(tree->OperIsCmpCompare());

    GenTree* const  op1     = tree->gtGetOp1();
    var_types const op1Type = op1->TypeGet();

    if (varTypeIsFloating(op1Type))
    {
        genCompareFloat(tree);
    }
    else
    {
        genCompareInt(tree);
    }
}

//------------------------------------------------------------------------
// genCompareInt: Generate code for comparing ints or longs
//
// Arguments:
//    treeNode - the compare tree
//
void CodeGen::genCompareInt(GenTreeOp* treeNode)
{
    assert(treeNode->OperIsCmpCompare());
    genConsumeOperands(treeNode);

    instruction ins;
    switch (PackOperAndType(treeNode->OperGet(), genActualType(treeNode->gtGetOp1()->TypeGet())))
    {
        case PackOperAndType(GT_EQ, TYP_INT):
            ins = INS_i32_eq;
            break;
        case PackOperAndType(GT_EQ, TYP_LONG):
            ins = INS_i64_eq;
            break;
        case PackOperAndType(GT_NE, TYP_INT):
            ins = INS_i32_ne;
            break;
        case PackOperAndType(GT_NE, TYP_LONG):
            ins = INS_i64_ne;
            break;
        case PackOperAndType(GT_LT, TYP_INT):
            ins = treeNode->IsUnsigned() ? INS_i32_lt_u : INS_i32_lt_s;
            break;
        case PackOperAndType(GT_LT, TYP_LONG):

Review comment (suggestion):
The error message should be more specific about what functionality is not yet implemented. Consider rephrasing to: 'Null checks for calls not yet implemented; should be inserted during lowering phase'.
```suggestion
        NYI_WASM("Null checks for calls not yet implemented; should be inserted during lowering phase");
```

Code after:
    else // A normal store, not a WriteBarrier store
    {
        // We must consume the operands in the proper execution order,
        // so that liveness is updated appropriately.
        genConsumeAddress(addr);
        genConsumeRegs(data);

        var_types   type = tree->TypeGet();
        instruction ins  = ins_Store(type);

        // TODO-WASM: Memory barriers

        GetEmitter()->emitIns_I(ins, emitActualTypeSize(type), 0);
    }

    genUpdateLife(tree);
}

//------------------------------------------------------------------------
// genCall: Produce code for a GT_CALL node
//
void CodeGen::genCall(GenTreeCall* call)
{
    if (call->NeedsNullCheck())
    {
        NYI_WASM("Insert nullchecks for calls that need it in lowering");
    }

    assert(!call->IsTailCall());

    for (CallArg& arg : call->gtArgs.EarlyArgs())
    {
        genConsumeReg(arg.GetEarlyNode());
    }

    for (CallArg& arg : call->gtArgs.LateArgs())
    {
        genConsumeReg(arg.GetLateNode());
    }

    genCallInstruction(call);
    genProduceReg(call);
}

//------------------------------------------------------------------------
// genCallInstruction - Generate instructions necessary to transfer control to the call.
//
// Arguments:
//    call - the GT_CALL node
//
void CodeGen::genCallInstruction(GenTreeCall* call)
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
            ins = INS_f64_neg;
            break;

        default:
            unreached();
            break;
    }

    GetEmitter()->emitIns(ins);
    genProduceReg(tree);
}

//---------------------------------------------------------------------
// genCodeForNullCheck - generate code for a GT_NULLCHECK node
//
// Arguments
//    tree - the GT_NULLCHECK node
//
// Return value:
//    None
//
void CodeGen::genCodeForNullCheck(GenTreeIndir* tree)
{
    assert(compiler->fgUseThrowHelperBlocks());
    genConsumeOperand(tree->Addr());
    GetEmitter()->Ins(INS_i32_eqz);
    genJumpToThrowHlpBlk(EJ_jmpif, SCK_NULL_REF);
}

//------------------------------------------------------------------------
// genCodeForLclAddr: Generates the code for GT_LCL_ADDR.
//
// Arguments:
//    lclAddrNode - the node.
//
void CodeGen::genCodeForLclAddr(GenTreeLclFld* lclAddrNode)
{
    assert(lclAddrNode->OperIs(GT_LCL_ADDR));

    GetEmitter()->emitIns_I(INS_local_get, EA_PTRSIZE, WasmRegToIndex(GetFramePointerReg()));
    GetEmitter()->emitIns_S(INS_I_const, EA_PTRSIZE, lclAddrNode->GetLclNum(), lclAddrNode->GetLclOffs());
    GetEmitter()->emitIns(INS_I_add);
    genProduceReg(lclAddrNode);
}

//------------------------------------------------------------------------
// genCodeForLclFld: Produce code for a GT_LCL_FLD node.
//
// Arguments:
//    tree - the GT_LCL_FLD node
//

Review comment (suggestion):
Ref https://github.com/dotnet/runtime/pull/123053#issuecomment-3730968365 for what value this should check against.

Code after:
// Arguments:
//    tree - The GT_CAST node for the int-to-float cast operation
//
// Notes:
//    Handles casts from TYP_INT/TYP_LONG to TYP_FLOAT/TYP_DOUBLE.
//    Currently not implemented (NYI_WASM).
//
void CodeGen::genIntToFloatCast(GenTree* tree)
{
    NYI_WASM("genIntToFloatCast");
}

//------------------------------------------------------------------------
// genFloatToFloatCast: Generate code for a float to float cast
//
// Arguments:
//    tree - The GT_CAST node for the float-to-float cast operation
//
void CodeGen::genFloatToFloatCast(GenTree* tree)
{
    var_types   toType   = tree->TypeGet();
    var_types   fromType = tree->AsCast()->CastOp()->TypeGet();
    instruction ins      = INS_none;

    genConsumeOperands(tree->AsCast());

    switch (PackTypes(toType, fromType))
    {
        case PackTypes(TYP_FLOAT, TYP_DOUBLE):
            ins = INS_f32_demote_f64;
            break;
        case PackTypes(TYP_DOUBLE, TYP_FLOAT):
            ins = INS_f64_promote_f32;
            break;
        case PackTypes(TYP_FLOAT, TYP_FLOAT):
        case PackTypes(TYP_DOUBLE, TYP_DOUBLE):
            ins = INS_none;
            break;
        default:
            unreached();
    }

    if (ins != INS_none)
    {
        GetEmitter()->emitIns(ins);
    }
    genProduceReg(tree);
}

//------------------------------------------------------------------------
// genCodeForBinary: Generate code for a binary arithmetic operator
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
        case PackOperAndType(GT_NEG, TYP_DOUBLE):
            ins = INS_f64_neg;
            break;

        default:
            unreached();
            break;
    }

    GetEmitter()->emitIns(ins);
    genProduceReg(tree);
}

//---------------------------------------------------------------------
// genCodeForNullCheck - generate code for a GT_NULLCHECK node
//
// Arguments
//    tree - the GT_NULLCHECK node
//
// Return value:
//    None
//
void CodeGen::genCodeForNullCheck(GenTreeIndir* tree)
{
    assert(compiler->fgUseThrowHelperBlocks());
    genConsumeOperand(tree->Addr());
    GetEmitter()->Ins(INS_i32_eqz);
    genJumpToThrowHlpBlk(EJ_jmpif, SCK_NULL_REF);
}

//------------------------------------------------------------------------
// genCodeForLclAddr: Generates the code for GT_LCL_ADDR.
//
// Arguments:
//    lclAddrNode - the node.
//
void CodeGen::genCodeForLclAddr(GenTreeLclFld* lclAddrNode)
{
    assert(lclAddrNode->OperIs(GT_LCL_ADDR));

    GetEmitter()->emitIns_I(INS_local_get, EA_PTRSIZE, WasmRegToIndex(GetFramePointerReg()));
    GetEmitter()->emitIns_S(INS_I_const, EA_PTRSIZE, lclAddrNode->GetLclNum(), lclAddrNode->GetLclOffs());
    GetEmitter()->emitIns(INS_I_add);
    genProduceReg(lclAddrNode);
}

//------------------------------------------------------------------------
// genCodeForLclFld: Produce code for a GT_LCL_FLD node.
//
// Arguments:
//    tree - the GT_LCL_FLD node

Review comment (suggestion):
```suggestion
    genConsumeAddress(tree->Addr());
```

Code after:
//
// Arguments:
//    tree - The GT_CAST node for the int-to-float cast operation
//
// Notes:
//    Handles casts from TYP_INT/TYP_LONG to TYP_FLOAT/TYP_DOUBLE.
//    Currently not implemented (NYI_WASM).
//
void CodeGen::genIntToFloatCast(GenTree* tree)
{
    NYI_WASM("genIntToFloatCast");
}

//------------------------------------------------------------------------
// genFloatToFloatCast: Generate code for a float to float cast
//
// Arguments:
//    tree - The GT_CAST node for the float-to-float cast operation
//
void CodeGen::genFloatToFloatCast(GenTree* tree)
{
    var_types   toType   = tree->TypeGet();
    var_types   fromType = tree->AsCast()->CastOp()->TypeGet();
    instruction ins      = INS_none;

    genConsumeOperands(tree->AsCast());

    switch (PackTypes(toType, fromType))
    {
        case PackTypes(TYP_FLOAT, TYP_DOUBLE):
            ins = INS_f32_demote_f64;
            break;
        case PackTypes(TYP_DOUBLE, TYP_FLOAT):
            ins = INS_f64_promote_f32;
            break;
        case PackTypes(TYP_FLOAT, TYP_FLOAT):
        case PackTypes(TYP_DOUBLE, TYP_DOUBLE):
            ins = INS_none;
            break;
        default:
            unreached();
    }

    if (ins != INS_none)
    {
        GetEmitter()->emitIns(ins);
    }
    genProduceReg(tree);
}

//------------------------------------------------------------------------
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
            break;

        case GT_NOP:
            break;

        case GT_NO_OP:
            instGen(INS_nop);
            break;

        case GT_CNS_INT:
        case GT_CNS_LNG:
        case GT_CNS_DBL:
            genCodeForConstant(treeNode);
            break;

        case GT_CAST:
            genCodeForCast(treeNode->AsOp());
            break;

        case GT_NEG:
        case GT_NOT:
            genCodeForNegNot(treeNode->AsOp());
            break;

        case GT_NULLCHECK:
            genCodeForNullCheck(treeNode->AsIndir());

        case GT_IND:
            genCodeForIndir(treeNode->AsIndir());
            break;

        case GT_STOREIND:
            genCodeForStoreInd(treeNode->AsStoreInd());
            break;

        default:
#ifdef DEBUG
            NYIRAW(GenTree::OpName(treeNode->OperGet()));
#else
            NYI_WASM("Opcode not implemented");
#endif
            break;
    }
}

//------------------------------------------------------------------------
// genCodeForJTrue: emit Wasm br_if
//
// Arguments:
//    treeNode - predicate value
//

Review comment (suggestion):
```suggestion
            genCodeForNullCheck(treeNode->AsIndir());
            break;
```

Code after:
            break;

        case GT_NOP:
            break;

        case GT_NO_OP:
            instGen(INS_nop);
            break;

        case GT_CNS_INT:
        case GT_CNS_LNG:
        case GT_CNS_DBL:
            genCodeForConstant(treeNode);
            break;

        case GT_CAST:
            genCodeForCast(treeNode->AsOp());
            break;

        case GT_NEG:
        case GT_NOT:
            genCodeForNegNot(treeNode->AsOp());
            break;

        case GT_NULLCHECK:
            genCodeForNullCheck(treeNode->AsIndir());
            break;

        case GT_IND:
            genCodeForIndir(treeNode->AsIndir());
            break;

        case GT_STOREIND:
            genCodeForStoreInd(treeNode->AsStoreInd());
            break;

        default:
#ifdef DEBUG
            NYIRAW(GenTree::OpName(treeNode->OperGet()));
#else
            NYI_WASM("Opcode not implemented");
#endif
            break;
    }
}

//------------------------------------------------------------------------
// genCodeForJTrue: emit Wasm br_if
//
// Arguments:
//    treeNode - predicate value