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

Code before:
}

static constexpr uint32_t PackTypes(var_types toType, var_types fromType)
{
    if (toType == TYP_BYREF)
    {
        toType = TYP_I_IMPL;
    }
    if (fromType == TYP_BYREF)
    {
        fromType = TYP_I_IMPL;
    }
    const int shift1 = ConstLog2<TYP_COUNT>::value + 1;
    return ((uint32_t)toType) | ((uint32_t)fromType << shift1);
}

void CodeGen::genIntToIntCast(GenTreeCast* cast)
{
    GenIntCastDesc desc(cast);
    var_types      toType     = genActualType(cast->CastToType());
    var_types      fromType   = genActualType(cast->CastOp()->TypeGet());
    int            extendSize = desc.ExtendSrcSize();
    instruction    ins        = INS_none;
    assert(fromType == TYP_INT || fromType == TYP_LONG);

    genConsumeRegs(cast->CastOp());

    switch (desc.ExtendKind())
    {
        case GenIntCastDesc::ExtendKind::COPY:
        {
            if (toType == TYP_INT && fromType == TYP_LONG)
            {
                ins = INS_i32_wrap_i64;
            }
            else
            {
                assert(toType == fromType);
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            if (toType == TYP_LONG)
            {
                ins = INS_i64_extend_u_i32;
            }
            else
            {

Review comment (suggestion):
Consistency nit: we've generally been using `genConsumeOperands(parent)` (here `genConsumeOperands(cast)`) instead of `genConsumeRegs(operand)`. But they are indeed equivalent.

Code after:

        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_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
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_COND));

    genConsumeOperands(jtrue);

    BasicBlock* const trueTarget  = block->GetTrueTarget();
    BasicBlock* const falseTarget = block->GetFalseTarget();

    // We don't expect degenerate BBJ_COND
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
        toType = TYP_I_IMPL;
    }
    const int shift1 = ConstLog2<TYP_COUNT>::value + 1;
    const int shift2 = shift1 + ConstLog2<GT_COUNT>::value + 1;
    return ((uint32_t)oper << shift1) | ((uint32_t)fromType) | ((uint32_t)toType << shift2);
}

static constexpr uint32_t PackTypes(var_types toType, var_types fromType)
{
    if (toType == TYP_BYREF)
    {
        toType = TYP_I_IMPL;
    }
    if (fromType == TYP_BYREF)
    {
        fromType = TYP_I_IMPL;
    }
    const int shift1 = ConstLog2<TYP_COUNT>::value + 1;
    return ((uint32_t)toType) | ((uint32_t)fromType << shift1);
}

void CodeGen::genIntToIntCast(GenTreeCast* cast)
{
    GenIntCastDesc desc(cast);
    var_types      toType     = genActualType(cast->CastToType());
    var_types      fromType   = genActualType(cast->CastOp()->TypeGet());
    int            extendSize = desc.ExtendSrcSize();
    instruction    ins        = INS_none;
    assert(fromType == TYP_INT || fromType == TYP_LONG);

    genConsumeRegs(cast->CastOp());

    switch (desc.ExtendKind())
    {
        case GenIntCastDesc::ExtendKind::COPY:
        {
            if (toType == TYP_INT && fromType == TYP_LONG)
            {
                ins = INS_i32_wrap_i64;
            }
            else
            {
                assert(toType == fromType);
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            if (toType == TYP_LONG)

Review comment (suggestion):
```suggestion
    var_types      fromType   = genActualType(cast->CastOp());
```

Code after:
            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_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
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_COND));

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

Code before:
    if (toType == TYP_BYREF)
    {
        toType = TYP_I_IMPL;
    }
    if (fromType == TYP_BYREF)
    {
        fromType = TYP_I_IMPL;
    }
    const int shift1 = ConstLog2<TYP_COUNT>::value + 1;
    return ((uint32_t)toType) | ((uint32_t)fromType << shift1);
}

void CodeGen::genIntToIntCast(GenTreeCast* cast)
{
    GenIntCastDesc desc(cast);
    var_types      toType     = genActualType(cast->CastToType());
    var_types      fromType   = genActualType(cast->CastOp()->TypeGet());
    int            extendSize = desc.ExtendSrcSize();
    instruction    ins        = INS_none;
    assert(fromType == TYP_INT || fromType == TYP_LONG);

    genConsumeRegs(cast->CastOp());

    switch (desc.ExtendKind())
    {
        case GenIntCastDesc::ExtendKind::COPY:
        {
            if (toType == TYP_INT && fromType == TYP_LONG)
            {
                ins = INS_i32_wrap_i64;
            }
            else
            {
                assert(toType == fromType);
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            if (toType == TYP_LONG)
            {
                ins = INS_i64_extend_u_i32;
            }
            else
            {
                // We expect the underlying types to both be int here,
                // so we don't need to do anything more
                assert(toType == TYP_INT && fromType == TYP_INT);
                ins = INS_none;

Review comment (suggestion):
```suggestion
        case GenIntCastDesc::COPY:
```
(Applies to all other cases as well)

Code after:
            genCodeForConstant(treeNode);
            break;

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

        case GT_NEG:
        case GT_NOT:
            genCodeForNegNot(treeNode->AsOp());
            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
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_COND));

    genConsumeOperands(jtrue);

    BasicBlock* const trueTarget  = block->GetTrueTarget();
    BasicBlock* const falseTarget = block->GetFalseTarget();

    // We don't expect degenerate BBJ_COND
    //
    assert(trueTarget != falseTarget);

    // We don't expect the true target to be the next block.
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            if (toType == TYP_LONG)
            {
                ins = INS_i64_extend_u_i32;
            }
            else
            {
                // We expect the underlying types to both be int here,
                // so we don't need to do anything more
                assert(toType == TYP_INT && fromType == TYP_INT);
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::SIGN_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            switch (extendSize)
            {
                case 1:
                    ins = INS_i32_extend8_s;
                    break;
                case 2:
                    ins = INS_i32_extend16_s;
                    break;
                default:
                    unreached();
            }

            // A sign-extended cast from small int->long requires two instructions; first sign extended
            // small int -> i32, then sign extended i32 -> i64
            if (toType == TYP_LONG)
            {
                GetEmitter()->emitIns(ins);
                ins = INS_i64_extend_s_i32;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_INT:
        {
            assert(toType == TYP_LONG);
            ins = INS_i64_extend_u_i32;
            break;
        }
        case GenIntCastDesc::ExtendKind::SIGN_EXTEND_INT:

Review comment (suggestion):
Missing possibly-needed `INS_i32_wrap_i64`.

Nit: could be smaller with a `?`.

Code after:
//    treeNode - predicate value
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_COND));

    genConsumeOperands(jtrue);

    BasicBlock* const trueTarget  = block->GetTrueTarget();
    BasicBlock* const falseTarget = block->GetFalseTarget();

    // We don't expect degenerate BBJ_COND
    //
    assert(trueTarget != falseTarget);

    // We don't expect the true target to be the next block.
    //
    assert(trueTarget != block->Next());

    // br_if for true target
    //
    inst_JMP(EJ_jmpif, trueTarget);

    // br for false target, if not fallthrough
    //
    if (falseTarget != block->Next())
    {
        inst_JMP(EJ_jmp, falseTarget);
    }
}

//------------------------------------------------------------------------
// genTableBasedSwitch: emit Wasm br_table
//
// Arguments:
//    treeNode - value to switch on
//
void CodeGen::genTableBasedSwitch(GenTree* treeNode)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_SWITCH));

    genConsumeOperands(treeNode->AsOp());

    BBswtDesc* const desc      = block->GetSwitchTargets();
    unsigned const   caseCount = desc->GetCaseCount();

    // We don't expect degenerate or default-less switches
    //
    assert(caseCount > 0);
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::SIGN_EXTEND_SMALL_INT:
        {
            assert(extendSize <= 2);
            switch (extendSize)
            {
                case 1:
                    ins = INS_i32_extend8_s;
                    break;
                case 2:
                    ins = INS_i32_extend16_s;
                    break;
                default:
                    unreached();
            }

            // A sign-extended cast from small int->long requires two instructions; first sign extended
            // small int -> i32, then sign extended i32 -> i64
            if (toType == TYP_LONG)
            {
                GetEmitter()->emitIns(ins);
                ins = INS_i64_extend_s_i32;
            }
            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_INT:
        {
            assert(toType == TYP_LONG);
            ins = INS_i64_extend_u_i32;
            break;
        }
        case GenIntCastDesc::ExtendKind::SIGN_EXTEND_INT:
        {
            assert(toType == TYP_LONG);
            ins = INS_i64_extend_s_i32;
            break;
        }
        default:
            unreached();
    }

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

void CodeGen::genFloatToIntCast(GenTree* tree)

Review comment (suggestion):
```suggestion
```
Dead code; this cast always produces `TYP_INT`.

Code after:
    //
    assert(trueTarget != block->Next());

    // br_if for true target
    //
    inst_JMP(EJ_jmpif, trueTarget);

    // br for false target, if not fallthrough
    //
    if (falseTarget != block->Next())
    {
        inst_JMP(EJ_jmp, falseTarget);
    }
}

//------------------------------------------------------------------------
// genTableBasedSwitch: emit Wasm br_table
//
// Arguments:
//    treeNode - value to switch on
//
void CodeGen::genTableBasedSwitch(GenTree* treeNode)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_SWITCH));

    genConsumeOperands(treeNode->AsOp());

    BBswtDesc* const desc      = block->GetSwitchTargets();
    unsigned const   caseCount = desc->GetCaseCount();

    // We don't expect degenerate or default-less switches
    //
    assert(caseCount > 0);
    assert(desc->HasDefaultCase());

    GetEmitter()->emitIns_I(INS_br_table, EA_4BYTE, caseCount);

    for (unsigned caseNum = 0; caseNum < caseCount; caseNum++)
    {
        BasicBlock* const caseTarget = desc->GetCase(caseNum)->getDestinationBlock();
        unsigned          depth      = findTargetDepth(caseTarget);

        GetEmitter()->emitIns_J(INS_label, EA_4BYTE, depth, caseTarget);
    }
}

//------------------------------------------------------------------------
// PackOperAndType: Pack a genTreeOps and var_types into a uint32_t
//
// Arguments:
Repository: dotnet/runtime
File: src/coreclr/jit/codegenwasm.cpp (C++)

Code before:

    switch (desc.ExtendKind())
    {
        case GenIntCastDesc::COPY:
        {
            if (toType == TYP_INT && fromType == TYP_LONG)
            {
                ins = INS_i32_wrap_i64;
            }
            else
            {
                assert(toType == fromType);
                ins = INS_none;
            }
            break;
        }
        case GenIntCastDesc::ZERO_EXTEND_SMALL_INT:
        {
            int andAmount = extendSize == 1 ? 255 : 65535;
            if (fromType == TYP_LONG)
            {
                GetEmitter()->emitIns(INS_i32_wrap_i64);
            }
            GetEmitter()->emitIns_I(INS_i32_const, EA_4BYTE, andAmount);
            GetEmitter()->emitIns(INS_i32_and);
            ins = (toType == TYP_LONG) ? INS_i64_extend_u_i32 : INS_none;
            break;
        }
        case GenIntCastDesc::SIGN_EXTEND_SMALL_INT:
        {
            if (fromType == TYP_LONG)
            {
                GetEmitter()->emitIns(INS_i32_wrap_i64);
            }
            ins = (extendSize == 1) ? INS_i32_extend8_s : INS_i32_extend16_s;

            break;
        }
        case GenIntCastDesc::ExtendKind::ZERO_EXTEND_INT:
        {
            ins = INS_i64_extend_u_i32;
            break;
        }
        case GenIntCastDesc::ExtendKind::SIGN_EXTEND_INT:
        {
            ins = INS_i64_extend_s_i32;
            break;
        }
        default:
            unreached();
    }

Review comment (suggestion):
```suggestion
            ins = INS_i32_and;
```
This type of cast always produces a `TYP_INT` value.

Code after:
    BasicBlock* const falseTarget = block->GetFalseTarget();

    // We don't expect degenerate BBJ_COND
    //
    assert(trueTarget != falseTarget);

    // We don't expect the true target to be the next block.
    //
    assert(trueTarget != block->Next());

    // br_if for true target
    //
    inst_JMP(EJ_jmpif, trueTarget);

    // br for false target, if not fallthrough
    //
    if (falseTarget != block->Next())
    {
        inst_JMP(EJ_jmp, falseTarget);
    }
}

//------------------------------------------------------------------------
// genTableBasedSwitch: emit Wasm br_table
//
// Arguments:
//    treeNode - value to switch on
//
void CodeGen::genTableBasedSwitch(GenTree* treeNode)
{
    BasicBlock* const block = compiler->compCurBB;
    assert(block->KindIs(BBJ_SWITCH));

    genConsumeOperands(treeNode->AsOp());

    BBswtDesc* const desc      = block->GetSwitchTargets();
    unsigned const   caseCount = desc->GetCaseCount();

    // We don't expect degenerate or default-less switches
    //
    assert(caseCount > 0);
    assert(desc->HasDefaultCase());

    GetEmitter()->emitIns_I(INS_br_table, EA_4BYTE, caseCount);

    for (unsigned caseNum = 0; caseNum < caseCount; caseNum++)
    {
        BasicBlock* const caseTarget = desc->GetCase(caseNum)->getDestinationBlock();
        unsigned          depth      = findTargetDepth(caseTarget);

        GetEmitter()->emitIns_J(INS_label, EA_4BYTE, depth, caseTarget);
Repository: dotnet/runtime
File: src/libraries/System.Private.CoreLib/src/System/Text/EncodingTable.cs (C#)

Code before:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;

namespace System.Text
{
    //
    // Data table for encoding classes.  Used by System.Text.Encoding.
    // This class contains two hashtables to allow System.Text.Encoding
    // to retrieve the data item either by codepage value or by webName.
    //
    internal static partial class EncodingTable
    {
        private static readonly ConcurrentDictionary<string, int> s_nameToCodePage = new(StringComparer.OrdinalIgnoreCase);
        private static CodePageDataItem?[]? s_codePageToCodePageData;

        /*=================================GetCodePageFromName==========================
        **Action: Given a encoding name, return the correct code page number for this encoding.
        **Returns: The code page for the encoding.
        **Arguments:
        **  name    the name of the encoding
        **Exceptions:
        **  ArgumentNullException if name is null.
        **  internalGetCodePageFromName will throw ArgumentException if name is not a valid encoding name.

Review comment (suggestion):
The System.Collections namespace import is no longer needed after replacing Hashtable with ConcurrentDictionary. Consider removing this unused using directive.
```suggestion

```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;

namespace System.Text
{
    //
    // Data table for encoding classes.  Used by System.Text.Encoding.
    // This class contains two hashtables to allow System.Text.Encoding
    // to retrieve the data item either by codepage value or by webName.
    //
    internal static partial class EncodingTable
    {
        private static readonly ConcurrentDictionary<string, int> s_nameToCodePage = new(StringComparer.OrdinalIgnoreCase);
        private static CodePageDataItem?[]? s_codePageToCodePageData;

        /*=================================GetCodePageFromName==========================
        **Action: Given a encoding name, return the correct code page number for this encoding.
        **Returns: The code page for the encoding.
        **Arguments:
        **  name    the name of the encoding
        **Exceptions:
        **  ArgumentNullException if name is null.
        **  internalGetCodePageFromName will throw ArgumentException if name is not a valid encoding name.
        ============================================================================*/
Repository: dotnet/runtime
File: src/libraries/System.Private.CoreLib/src/System/CurrentSystemTimeZone.cs (C#)

Code before:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.Concurrent;
using System.Globalization;

namespace System
{
    /// <summary>
    /// Represents the current system timezone.
    /// </summary>
    [Obsolete("System.CurrentSystemTimeZone has been deprecated. Investigate the use of System.TimeZoneInfo.Local instead.")]
    internal sealed class CurrentSystemTimeZone : TimeZone
    {
        // Standard offset in ticks to the Universal time if
        // no daylight saving is in used.
        // E.g. the offset for PST (Pacific Standard time) should be -8 * 60 * 60 * 1000 * 10000.
        // (1 millisecond = 10000 ticks)
        private readonly long m_ticksOffset;
        private readonly string m_standardName;
        private readonly string m_daylightName;

        internal CurrentSystemTimeZone()
        {
            TimeZoneInfo local = TimeZoneInfo.Local;

            m_ticksOffset = local.BaseUtcOffset.Ticks;
            m_standardName = local.StandardName;

Review comment (suggestion):
The System.Collections namespace import is no longer needed after replacing Hashtable with ConcurrentDictionary. Consider removing this unused using directive.
```suggestion

```

Code after:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Concurrent;
using System.Globalization;

namespace System
{
    /// <summary>
    /// Represents the current system timezone.
    /// </summary>
    [Obsolete("System.CurrentSystemTimeZone has been deprecated. Investigate the use of System.TimeZoneInfo.Local instead.")]
    internal sealed class CurrentSystemTimeZone : TimeZone
    {
        // Standard offset in ticks to the Universal time if
        // no daylight saving is in used.
        // E.g. the offset for PST (Pacific Standard time) should be -8 * 60 * 60 * 1000 * 10000.
        // (1 millisecond = 10000 ticks)
        private readonly long m_ticksOffset;
        private readonly string m_standardName;
        private readonly string m_daylightName;

        internal CurrentSystemTimeZone()
        {
            TimeZoneInfo local = TimeZoneInfo.Local;

            m_ticksOffset = local.BaseUtcOffset.Ticks;
            m_standardName = local.StandardName;
            m_daylightName = local.DaylightName;