<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Vladimir,<br>
    <br>
    Webrev was changed according to your notices<br>
    Please see
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.02/">http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.02/</a><br>
    <br>
    Thanks,<br>
    Jane<br>
    <div class="moz-cite-prefix">On 17.09.2014 13:50, Vladimir Ivanov
      wrote:<br>
    </div>
    <blockquote cite="mid:54195963.9090704@oracle.com" type="cite">Evgeniya,
      <br>
      <br>
      Please, keep the following configurations in test/compiler/stable
      tests:
      <br>
        * -XX:-TieredCompilation
      <br>
        * -XX:+TieredCompilation -XX:TieredStopAtLevel=1
      <br>
        * -XX:+-FoldStableValues
      <br>
      <br>
      Since there's a difference in behavior between C1 & C2 w.r.t
      @Stable, the tests assume there's only C1 or C2 working during
      test execution (see StableConfiguration). Otherwise, they fail.
      <br>
      <br>
      Best regards,
      <br>
      Vladimir Ivanov
      <br>
      <br>
      On 9/17/14, 12:20 PM, Vladimir Kozlov wrote:
      <br>
      <blockquote type="cite">Wow! You really did not hold back :)
        <br>
        <br>
        I think you should keep -XX:TieredStopAtLevel=1 runs in stable
        tests. On
        <br>
        some platforms (Solaris, MacOSX) we don't have Client VM and
        pure C1
        <br>
        compilation anymore. In tiered C1 does mostly tier 3
        compilation. The
        <br>
        only way to test pure C1 on such platforms is with
        TieredStopAtLevel=1.
        <br>
        You don't need other options, it has effect only with Tiered
        Compilation:
        <br>
        <br>
            *                   -XX:+UnlockDiagnosticVMOptions
        -XX:+WhiteBoxAPI
        <br>
        -Xcomp
        <br>
            *                   -XX:TieredStopAtLevel=1
        -XX:+FoldStableValues
        <br>
        <br>
        Please, restore 2 @run commands with -XX:+FoldStableValues and
        <br>
        -XX:-FoldStableValues with TieredStopAtLevel=1.
        <br>
        <br>
        Yes, in non-tiered and Client VM runs we will waste these 2 runs
        but I
        <br>
        don't want to lost the coverage.
        <br>
        <br>
        Thanks,
        <br>
        Vladimir
        <br>
        <br>
        <br>
        On 9/17/14 12:16 AM, Evgeniya Stepanova wrote:
        <br>
        <blockquote type="cite">Hi all!
          <br>
          <br>
          I've changed tests mentioned: VM flavor and TieredCompilation
          options
          <br>
          removed since these are rotated values in the
          <br>
          testcycle.
          <br>
          Please see
          <br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.01/">http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.01/</a>
          <br>
          <br>
          Thanks,
          <br>
          Jane
          <br>
          <br>
          On 16.09.2014 19:57, Vladimir Kozlov wrote:
          <br>
          <blockquote type="cite">On 9/16/14 4:51 AM, Filipp Zhinkin
            wrote:
            <br>
            <blockquote type="cite">Igor, Vladimir,
              <br>
              <br>
              RTM tests will be skipped if the class of JVM used for a
              test
              <br>
              execution is not 'server'.
              <br>
              Such check is done before the '-server' is passed in
              RTMTestBase, so
              <br>
              '-server'
              <br>
              removal won't increase coverage.
              <br>
            </blockquote>
            <br>
            Okay, it make sense to have it here then.
            <br>
            I agree that it does not make sense to run RTM tests  with
            client VM.
            <br>
            <br>
            Vladimir
            <br>
            <br>
            <blockquote type="cite">
              <br>
              If we'll remove it, then I don't think that it will affect
              test
              <br>
              execution:
              <br>
              if JVM class was not explicitly specified on the command
              line, then
              <br>
              a java launcher will choose it depending on available
              physical memory
              <br>
              and available processors count. So if JVM was 'server'
              when we're
              <br>
              checking
              <br>
              its class, then it's unlikely that JVM started in
              RTMTestBase will
              <br>
              become 'client'.
              <br>
              <br>
              But if we'll leave '-server' then tests continue to work
              as expected
              <br>
              even
              <br>
              if a java launcher will start choosing JVM class using
              some other
              <br>
              policy.
              <br>
              <br>
              Thanks,
              <br>
              Filipp.
              <br>
              <br>
              <br>
              On 09/16/2014 02:35 PM, Igor Ignatyev wrote:
              <br>
              <blockquote type="cite">Vladimir,
                <br>
                I doubt that we can remove -server from
                <br>
                compiler/testlibrary/rtm/RTMTestBase.java. AFAIR, it
                requires
                <br>
                -server, since
                <br>
                it uses server only flag 'UseRTMLocking'
                <br>
                <br>
                Filipp,
                <br>
                can you please verify it?
                <br>
                <br>
                Igor
                <br>
                <br>
                On 09/15/2014 09:42 PM, Vladimir Kozlov wrote:
                <br>
                <blockquote type="cite">Hi Evgeniya
                  <br>
                  <br>
                  What about LoadWithMask.java test?
                  <br>
                  <br>
                  Please, simple remove '-server' option. We always do
                  testing with
                  <br>
                  server
                  <br>
                  JVM so we can't miss the bug if it returns. We did go
                  through such
                  <br>
                  cleanup in tests before and did not have any problems.
                  <br>
                  <br>
                  Flag -XX:-TieredCompilation may affect bug
                  reproduction. That is
                  <br>
                  why we
                  <br>
                  specify it sometimes.
                  <br>
                  In general we can remove it too since we run Nightly
                  in both
                  <br>
                  configurations with and without Tiered. But since it
                  does not affect
                  <br>
                  what JVM is used we keep it.
                  <br>
                  <br>
                  In compiler tests we should not specify neither
                  -server or -client to
                  <br>
                  make sure the correct JVM is tested. This is one of
                  our rules when we
                  <br>
                  add tests. Recently we added tests which have -server
                  specified:
                  <br>
                  <br>
                  compiler/IntegerArithmetic/TestIntegerComparison.java
                  <br>
                  compiler/testlibrary/rtm/RTMTestBase.java
                  <br>
                  compiler/uncommontrap/UncommonTrapStackBang.java
                  <br>
                  <br>
                  compiler/stable/* test have  @run command with both
                  -server and
                  <br>
                  -client
                  <br>
                  configurations.
                  <br>
                  <br>
                  All these tests should be cleaned up.
                  <br>
                  <br>
                  Regards,
                  <br>
                  Vladimir
                  <br>
                  <br>
                  On 9/15/14 7:11 AM, Evgeniya Stepanova wrote:
                  <br>
                  <blockquote type="cite">Hi Filipp,
                    <br>
                    <br>
                    I want to keep first string "as is" because it is a
                    regression test
                    <br>
                    and if we just remove -server option, it is possible
                    <br>
                    that bug have returned, but we won't find it before
                    release because
                    <br>
                    options will not compose in the right way.
                    <br>
                  </blockquote>
                  <br>
                  <br>
                  <br>
                  <blockquote type="cite">The test is very fast, so the
                    second run will not take significant
                    <br>
                    time, but we will be sure that there is no
                    regression.
                    <br>
                    <br>
                    As for C2-specific options, I agree. I've removed
                    -client and
                    <br>
                    TieredCompilation options to use jtreg submitted.
                    <br>
                    <br>
                    Please see updated diff:
                    <br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.00/">http://cr.openjdk.java.net/~iignatyev/eistepan/8038098/webrev.00/</a>
                    <br>
                    <br>
                    Thanks,
                    <br>
                    Jane
                    <br>
                    On 15.09.2014 16:01, Filipp Zhinkin wrote:
                    <br>
                    <blockquote type="cite">Hi Jane,
                      <br>
                      <br>
                      mail's summary does not match the bug's name.
                      <br>
                      <br>
                      As about your fix: I thought that such configs as
                      client/server
                      <br>
                      and tired/non-tired should be rotated in nightly,
                      aren't it?
                      <br>
                      <br>
                      Maybe it does more sense to just remove '-server'
                      option?
                      <br>
                      With '-client' the test may fail during test runs
                      that were started
                      <br>
                      with some C2-specific options, like UseRTMLocking.
                      <br>
                      <br>
                      Thanks,
                      <br>
                      Filipp.
                      <br>
                      <br>
                      On 09/15/2014 03:42 PM, Evgeniya Stepanova wrote:
                      <br>
                      <blockquote type="cite">Hi all!
                        <br>
                        <br>
                        Please review fix for 8038098
                        <br>
                        <br>
                        bug:
                        <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8038098">https://bugs.openjdk.java.net/browse/JDK-8038098</a>
                        <br>
                        <br>
                        Problem: Since we have inlining in C1 now, we
                        should check that
                        <br>
                        issue 8031743 will not appear in C1.
                        <br>
                        Solution: Added one more run in test for C1
                        inlining checking
                        <br>
                        <br>
                        A am also need someone to push changes.
                        <br>
                        <br>
                        Diff is in the attachments.
                        <br>
                        I've also included diff to this letter since it
                        is very small.
                        <br>
                        <br>
                        <br>
                        Thanks,
                        <br>
                        Jane
                        <br>
                        <br>
                        diff -r 087443edb74a
                        test/compiler/codegen/LoadWithMask2.java
                        <br>
                        ---
                        a/test/compiler/codegen/LoadWithMask2.java   
                        Mon Sep 15
                        <br>
                        08:08:22 2014 +0200
                        <br>
                        +++
                        b/test/compiler/codegen/LoadWithMask2.java   
                        Mon Sep 15
                        <br>
                        14:35:50 2014 +0400
                        <br>
                        @@ -26,6 +26,7 @@
                        <br>
                          * @bug 8031743
                        <br>
                          * @summary loadI2L_immI broken for negative
                        memory values
                        <br>
                          * @run main/othervm -server -Xbatch
                        -XX:-TieredCompilation
                        <br>
                        -XX:CompileCommand=compileonly,*.foo*
                        LoadWithMask2
                        <br>
                        + * @run main/othervm -client -Xbatch
                        -XX:+TieredCompilation
                        <br>
                        -XX:CompileCommand=compileonly,*.foo*
                        LoadWithMask2
                        <br>
                          *
                        <br>
                          */
                        <br>
                         public class LoadWithMask2 {
                        <br>
                      </blockquote>
                      <br>
                    </blockquote>
                    <br>
                    --
                    <br>
                    /Evgeniya Stepanova/
                    <br>
                  </blockquote>
                </blockquote>
              </blockquote>
              <br>
            </blockquote>
          </blockquote>
          <br>
          --
          <br>
          /Evgeniya Stepanova/
          <br>
        </blockquote>
      </blockquote>
    </blockquote>
    <br>
    <div class="moz-signature">-- <br>
      <i>Evgeniya Stepanova</i></div>
  </body>
</html>