@AndyAyersMS @mikedn

Please check there is any source-level issue on it.

Sorry for pushing 2 formatting commits.
I didn't know how to format on this, I used clang format and didn't worked well.

I didn't know how to format on this, I used clang format and didn't worked well.

See the jit-format utility in the jitutils repository - https://github.com/dotnet/jitutils/blob/master/doc/formatting.md

@mikedn Thanks. I will try it out right away! :>

This will generate following code for optimized assemblies.

static unsafe int Test(int[] arr)
{
    int total = 0;

    fixed (int* pArr = &arr[0])
    {
        for (int i = 0; i < 4; ++i)
        {
            total += pArr[i];
        }
    }

    return total;
}

G_M46070_IG01:
       4883EC28             sub      rsp, 40
       33C0                 xor      rax, rax
       4889442420           mov      qword ptr [rsp+20H], rax

G_M46070_IG02:
       83790800             cmp      dword ptr [rcx+8], 0
       7627                 jbe      SHORT G_M46070_IG04
       4883C110             add      rcx, 16
       48894C2420           mov      bword ptr [rsp+20H], rcx
       488B442420           mov      rax, bword ptr [rsp+20H]
       8B10                 mov      edx, dword ptr [rax]
       035004               add      edx, dword ptr [rax+4]
       035008               add      edx, dword ptr [rax+8]
       03500C               add      edx, dword ptr [rax+12]
       33C0                 xor      rax, rax
       4889442420           mov      bword ptr [rsp+20H], rax
       8BC2                 mov      eax, edx

G_M46070_IG03:
       4883C428             add      rsp, 40
       C3                   ret      

G_M46070_IG04:
       E88391BA5E           call     CORINFO_HELP_RNGCHKFAIL
       CC                   int3     

@mikedn
I can't sure its better that copying stmt tree on original block or cloning block.
there is no implements that replacing LCL_VAR node into LCL_VAR, not CNS_INT.

current implements walk all trees and extract parent of all it has same iterVar (which is LoopDsc::lpIterVar()) and changing it. maybe better way to solve it?

because its seems unsafe if its failed to modify into partial unrolled loop. cannot rollback.

or is there are way to indirect block to block? without condition. just to execute block to next block.

I ill try it after go home. its school here. :> anyways thanks.

I think thats bug of fgWalkTree. I don't understand why assert triggered on follow code.

All I used on fgWalkTree is this. is something wrong with it?

fgWalkTreePre(&gtIncr->gtStmt.gtStmtExpr, gtVisitor, &data, true, true);

fgWalkTreePre(&gtClonedStmt, gtVisitor, &data, true, true);

    auto gtVisitor = [](GenTree** pTree, fgWalkData* data) -> fgWalkResult {
        GenTree* tree = *pTree;

        if (GenTreeLclVar* var = tree->AsLclVar())
        {
            GTExtractData* gtData = (GTExtractData*)data->pCallbackData;

            if (var->GetLclNum() == gtData->varId && data->parent->OperGet() != genTreeOps::GT_ASG)
            {
                // Get parent tree from matched local varaible.
                gtData->varExtracted->push_back(data->parent);
            }
        }
        return fgWalkResult::WALK_CONTINUE;
    };

Same reason here.

JIT_Performance._CodeQuality_Benchstones_BenchI_TreeInsert_TreeInsert_TreeInsert_._CodeQuality_Benchstones_BenchI_TreeInsert_TreeInsert_TreeInsert_cmd [FAIL] [D:\j\workspace\x64_checked_w---eac6a79c\tests\runtest.proj]
20:43:54         
20:43:54         Assert failure(PID 5348 [0x000014e4], Thread: 12072 [0x2f28]): Assertion failed 'node->OperIsBinary()' in 'Benchstone.BenchI.TreeInsert:Bench():bool:this' (IL size 174)
20:43:54         
20:43:54             File: d:\j\workspace\x64_checked_w---eac6a79c\src\jit\compiler.h Line: 9784
20:43:54             Image: D:\j\workspace\x64_checked_w---eac6a79c\bin\tests\Windows_NT.x64.Checked\Tests\Core_Root\CoreRun.exe
20:43:54         
20:43:54         
20:43:54   
20:43:54   Return code:      1
20:43:54   Raw output file:      D:\j\workspace\x64_checked_w---eac6a79c\bin\tests\Windows_NT.x64.Checked\Reports\JIT.Performance\CodeQuality\Benchstones\BenchI\TreeInsert\TreeInsert\TreeInsert.output.txt

Something is wrong with fgWalkTree, I think something isn't handled.

@mikedn can I ask that how can I run test with debugging? Thanks :>

Something is wrong with fgWalkTree, I think something isn't handled.

It looks to me that you're calling it with a GT_STMT node instead of calling it with the statement's expression.

can I ask that how can I run test with debugging?

Test executables can be run using corerun.exe. You'll probably need to set the environment variable CORE_LIBRARIES to the directory containing the test runtime (something like D:\Projects\coreclr\bin\tests\Windows_NT.x64.Checked\Tests\Core_Root). Once you get it running you can debug it like any other native program.

Note that corerun has a /d option that make it wait for you to attach a debugger.

If you're using Visual Studio you can configure it to start corerun with the appropriate parameters and environment variables in the Debugging property page of a project.

I don't understand whats happening on formatting.... that new format is ACTUALLY same.
just whu.....

[ADDED] That empty space after comment made that check failed.... LOL.

Ready for review.

@mikedn @RussKeldorph @AndyAyersMS

This code needs review. Is no one reviewing this?

Ping :>

Its fine that notice why can't I get reviewed.
also I can refactor this code if its needed (please notice if its needed.)
I just wants feedback. Thanks.

@ArtBlnd sorry for not responding sooner.

I skimmed over the code and it looks like you've done some promising work here. I have a few things I'd like to see happen before we do a deeper review.

First, reorganize your changes so that the first commit takes care of all of the restructuring of the original code -- things like renaming existing variables and factoring out the full unroll into its own method.

For new methods make sure to introduce the header comments that we now recommend for jit code. See the jit coding conventions guidefor more information.

Second, create a second commit that then adds in your new logic. Please add comments here as well to match the coding conventions. I think it is fine to squash down most of what you have that is new (and not refactoring) into just this one commit for now.

Third, become familiar with how to run jit diffs (there are useful tools over in the jitutils repo and report back on how many methods in framework and test code are impacted by your changes, and the overall size impact. Let me know if you need more guidance on how to do this part.

Fourth (in reaction to diffs):

• if you see very few methods with diffs
  • consider creating test cases to add to help evaluate the behavior of your code
  • add instrumentation and look for loops in existing code that you perhaps should be unrolling
• if you see many methods with diffs
  • look closely at enough examples to understand what leads to the diffs
  • make sure you're not altering code in cases where there is no partial unrolling

First, reorganize your changes so that the first commit takes care of all of the restructuring of the original code -- things like renaming existing variables and factoring out the full unroll into its own method.

That was needed because it was using optUnrollLoops()'s variables.
I needed to make it separately (for method) and needs to make it new. (I didn't copy and past it actually I just see and rewrote it. that's why it has refactored variable name)

For new methods make sure to introduce the header comments that we now recommend for jit code. See the jit coding conventions guidefor more information.

Second, create a second commit that then adds in your new logic. Please add comments here as well to match the coding conventions. I think it is fine to squash down most of what you have that is new (and not refactoring) into just this one commit for now.

Thanks. I will take care of it.
should I close and reopen issue to make it as one commit? (for now)

Fourth (in reaction to diffs):
if you see very few methods with diffs
consider creating test cases to add to help evaluate the behavior of your code
add instrumentation and look for loops in existing code that you perhaps should be unrolling
if you see many methods with diffs
look closely at enough examples to understand what leads to the diffs
make sure you're not altering code in cases where there is no partial unrolling

I will add it ASAP.

And. yes thanks everyone to help me out.

Is it possible to check IR dump with tests?
in this case. I can't make a test case if there is no IR dump check(or asm-gen test).

I think I have to close this PR and reopen when its fixed without any bugs.
Sorry for everyone bothering with this. :< Thanks

Reworking on last branch. thanks :>