JIT: Loop unrolling does not validate predecessor BBJ_COND condition properly

[jakobbotsch]

The following program prints 0 1 2 3. It should not print anything.

using System;
using System.Runtime.CompilerServices;

public class Program
{
    public static void Main()
    {
        Foo(0);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Foo(int y)
    {
        int x = 0;
        if (y != 0)
        {
            do
            {
                Console.WriteLine(x);
                x++;
            }
            while (x < 4);
        }
    }
}

The problem is this code in loop unrolling, which unconditionally removes the init block condition, for reasons unclear to me:

runtime/src/coreclr/jit/optimizer.cpp

Lines 4481 to 4508 in c33557d

	// Control will fall through from the initBlock to its successor, which is either
	// the preheader HEAD (if it exists), or the now empty TOP (if totalIter == 0),
	// or the first cloned top.
	//
	// If the initBlock is a BBJ_COND drop the condition (and make initBlock a BBJ_ALWAYS block).
	//
	// TODO: Isn't this missing validity checks? This seems dangerous.
	//
	if (initBlock->KindIs(BBJ_COND))
	{
	assert(dupCond);
	Statement* initBlockBranchStmt = initBlock->lastStmt();
	noway_assert(initBlockBranchStmt->GetRootNode()->OperIs(GT_JTRUE));
	fgRemoveStmt(initBlock, initBlockBranchStmt);
	fgRemoveRefPred(initBlock->GetTrueTarget(), initBlock);
	initBlock->SetKindAndTarget(BBJ_ALWAYS, initBlock->GetFalseTarget());
	// TODO-NoFallThrough: If bbFalseTarget can diverge from bbNext, it may not make sense to set
	// BBF_NONE_QUIRK
	initBlock->SetFlags(BBF_NONE_QUIRK);
	}
	else
	{
	// the loop must execute
	assert(!dupCond);
	assert(totalIter > 0);
	noway_assert(initBlock->KindIs(BBJ_ALWAYS));
	}

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The following program prints 1 2 3 4. It should not print anything.

using System;
using System.Runtime.CompilerServices;

public class Program
{
    public static void Main()
    {
        Foo(0);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Foo(int y)
    {
        int x = 0;
        if (y != 0)
        {
            do
            {
                Console.WriteLine(x);
                x++;
            }
            while (x < 4);
        }
    }
}

The problem is this code in loop unrolling, which unconditionally removes the init block condition, for reasons unclear to me:

runtime/src/coreclr/jit/optimizer.cpp

Lines 4481 to 4508 in c33557d

	// Control will fall through from the initBlock to its successor, which is either
	// the preheader HEAD (if it exists), or the now empty TOP (if totalIter == 0),
	// or the first cloned top.
	//
	// If the initBlock is a BBJ_COND drop the condition (and make initBlock a BBJ_ALWAYS block).
	//
	// TODO: Isn't this missing validity checks? This seems dangerous.
	//
	if (initBlock->KindIs(BBJ_COND))
	{
	assert(dupCond);
	Statement* initBlockBranchStmt = initBlock->lastStmt();
	noway_assert(initBlockBranchStmt->GetRootNode()->OperIs(GT_JTRUE));
	fgRemoveStmt(initBlock, initBlockBranchStmt);
	fgRemoveRefPred(initBlock->GetTrueTarget(), initBlock);
	initBlock->SetKindAndTarget(BBJ_ALWAYS, initBlock->GetFalseTarget());
	// TODO-NoFallThrough: If bbFalseTarget can diverge from bbNext, it may not make sense to set
	// BBF_NONE_QUIRK
	initBlock->SetFlags(BBF_NONE_QUIRK);
	}
	else
	{
	// the loop must execute
	assert(!dupCond);
	assert(totalIter > 0);
	noway_assert(initBlock->KindIs(BBJ_ALWAYS));
	}

Author:	jakobbotsch
Assignees:	-
Labels:	area-CodeGen-coreclr, untriaged
Milestone:	-

[jakobbotsch]

There is also an implicit assumption by loop unrolling that this BBJ_COND before the preheader is a zero-trip test. E.g. the following program should print 0, but it does not print anything:

using System;
using System.Runtime.CompilerServices;

public class Program
{
    public static void Main()
    {
        Foo(1);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Foo(int y)
    {
        int x = 0;
        if (y != 0)
        {
            do
            {
                Console.WriteLine(x);
                x++;
            }
            while (x < 0);
        }
    }
}