Min and Max Ideal missing AddNode::Ideal optimisations

XMLWordPrintable

    • Type: Enhancement
    • Resolution: Unresolved
    • Priority: P4
    • tbd
    • Affects Version/s: 26
    • Component/s: hotspot

      While investigating JDK-8373134, Roland gave me a hand creating the following test case that failed with VerifyIterativeGVN:

      ```
      public class TestIntMax
      {
          private static int test(int a, int b)
          {
              int i;
              for (i = -10; i < 1; i++)
              {
              }
              int c = a * i;
              return Integer.max(a, Integer.max(b, c));
          }

          public static void main(String[] args)
          {
              for (int i = 0; i < 20_000; i++)
              {
                  test(42, 42);
              }
          }
      }
      ```

      To understand why it didn't work, I compared to what was happening with the same case if applied to long and that one worked. So, it seemed like the case above was not really caused by JDK-8373134 but by something else.

      After digging I realised that long worked because MaxLNode::Ideal calls AddNode::Ideal which in turn calls commute and that causes an additional optimization that makes MaxLNode work. When compared with MaxINode, there's no calls to AddNode::Ideal, so there is a bunch of optimisations missing for MaxI nodes.

      The following IR test, inspired by a simple Roland test case created, highlights the commutation issue:

      ```
      package compiler.c2.templated;

      /mport compiler.lib.generators.*;
      import compiler.lib.ir_framework.*;
      import compiler.lib.verify.*;

      public class MinMaxIdeal {
        public static void main(String[] vmFlags) {
          TestFramework framework = new TestFramework(MinMaxIdeal.class);
          framework.addFlags(
              "-classpath",
              "/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg/compiler/c2/irTests:/Users/g/src/jdk-commute-et-al/JTwork/classes/compiler/c2/irTests/TestMinMaxIdeal.d/test/lib:/Users/g/src/jdk-commute-et-al/test/lib:/Users/g/src/jdk-commute-et-al/test/hotspot/jtreg:/Users/g/opt/jtreg/build/images/jtreg/lib/javatest.jar:/Users/g/opt/jtreg/build/images/jtreg/lib/jtreg.jar:/Users/g/src/jdk-commute-et-al/JTwork/scratch/./compile-framework-classes-15731717948002588134");
          framework.addFlags(vmFlags);
          framework.start();
        }

        @Test
        @IR(counts = {IRNode.MAX_I, "= 1"})
        @Arguments(values = {Argument.NUMBER_42, Argument.NUMBER_42})
        public int test_MAX_I(int a, int b) {
          return Integer.max(a, b) + Integer.max(b, a);
        }
      }
      ```

      Fails with:

      ```
      Failed IR Rules (1) of Methods (1)
      ----------------------------------
      1) Method "public int compiler.c2.templated.MinMaxIdeal.test_MAX_I(int,int)" - [Failed IR rules: 1]:
         * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#MAX_I#_", "= 1"}, failOn={}, applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
           > Phase "PrintIdeal":
             - counts: Graph contains wrong number of nodes:
               * Constraint 1: "(\d+(\s){2}(MaxI.*)+(\s){2}===.*)"
                 - Failed comparison: [found] 2 = 1 [given]
                   - Matched nodes (2):
                     * 34 MaxI === _ 11 12 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:2 (line 23)
                     * 46 MaxI === _ 12 11 [[ 47 ]] !jvms: Integer::max @ bci:2 (line 1934) MinMaxIdeal::test_MAX_I @ bci:7 (line 23)

      >>> Check stdout for compilation output of the failed methods
      ```

      The IR test has been generated with the template framework which is attached.

      The aim of this this bug is to add AddNode::Ideal to MaxINode::Ideal and create tests that verify that commutation and other optimisations inside AddNode::Ideal apply correctly. The test will also explore what happens with other min/max node types (float, half float, double, long) but if any issues appear there, a separate bug will be filled.

            Assignee:
            Galder Zamarreño
            Reporter:
            Galder Zamarreño
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: