RFR: 8349139: C2: Div looses dependency on condition that guarantees divisor not null in counted loop [v2]

[Roland Westrelin]

On Fri, 14 Feb 2025 18:24:25 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

> For this bug, I think a more general fix is to try to compare the type of the `Phi` with that of the input it is going to be replaced with. If the former is not wider than the latter then we add a `CastNode`, since the cast is only about value range, not strict dependency, we can use `CarryDependency` instead of `UnconditionalDependency`. Am I right?

I don't think so. Consider this test case:


ublic class TestMainLoopNoBackedgeFloatingDiv {
    public static void main(String[] args) {
        for (int i = 0; i < 20_000; i++) {
            test1(1000, 0, false);
            test1Helper(1000, 0, false, false);
        }
        test1(1, 0, false);
    }

    private static int test1(int stop, int res, boolean alwaysTrueInMain) {
        stop = Integer.max(stop, 1);
        res = test1Helper(stop, res, alwaysTrueInMain, true);
        return res;
    }

    private static int test1Helper(int stop, int res, boolean alwaysTrueInMain, boolean flag) {
        for (int i = stop; i >= 1; i--) {
            res = res / i;
            if (alwaysTrueInMain) {
                break;
            }
            alwaysTrueInMain = flag;
        }
        return res;
    }
}


It reproduces this issue and is actually a better test case because it doesn't even need `StressGCM`:

All we know about the loop `Phi` is that it's expected to be in `[1, max]`. That allows the removal of the control dependency on the `Div` node. pre/main/post loops are then created and then because `alwaysTrueInMain` is true in the main and post loops, the main and post loops loose there backedge. All that's left from their loop body is:

res = res / (i-1);

(one copy for each one). The 2 `Div` nodes are commoned and placed above the checks that were added to guard the main and post loops and it's always executed out of the pre loop. Depending on the value of `stop`, the test may execute the pre loop and what's left of the main or post loops or only the pre loop. With `stop = 1`, only the pre loop needs to be executed, but the `Div` is executed anyway and the crash occurs.

The type of the input to the main loop `Phi` is the same as the type of the loop `Phi` in this case.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23617#issuecomment-2665574210