Harmony
  1. Harmony
  2. HARMONY-1969

[drlvm][jit] nested synchronized blocks make Jitrino.OPT crash meeting multiple loop entrances to a single loop

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: DRLVM
    • Labels:
      None
    • Environment:
      any
    • Patch Info:
      Patch Available

      Description

      this is a sub-bug for HARMONY-1908. The reason is $subj. I suggest to restore a normal loop structure in this case (i. e. generalize the eliminateUnnestedLoopsOnDispatch() a little)

      of course, loop nesting may be broken in specially crafted bytecode samples (and we will not repair them with the proposed solution), but a reasonable behavoiur in this case would be to move bytecodes like this to JET. Here I am not for moving to JET because of the common nature of the code pattern (nested synches, javac) that we need to run heavily optimized.

      To reproduce, disable profiling-related asserts in ControlFlowGraph.cpp, rebuild Jitrino in debug mode.

      run DoubleSync (attached) with -Xem:opt

      java: /export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/shared/ControlFlowGraph.cpp:384: bool Jitrino::ControlFlowGraph::isEdgeProfileConsistent(bool, bool, bool): Assertion `doAssert?false:true' failed.
      SIGABRT in VM code.
      Stack trace:
      1: ?? (??:-1)
      2: abort (??:-1)
      3: __assert_fail (??:-1)
      4: Jitrino::ControlFlowGraph::isEdgeProfileConsistent(bool, bool, bool) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/shared/ControlFlowGraph.cpp:385)
      5: Jitrino::ControlFlowGraph::smoothEdgeProfile() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/shared/ControlFlowGraph.cpp:947)
      6: Jitrino::StaticProfiler::estimateGraph(Jitrino::IRManager&, double, bool) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/dynopt/StaticProfiler.cpp:253)
      7: Jitrino::StaticProfilerPass::_run(Jitrino::IRManager&) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/dynopt/StaticProfiler.cpp:293)
      8: Jitrino::OptPass::run() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/optimizer/optpass.cpp:61)
      9: Jitrino::runPipeline(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:226)
      10: Jitrino::compileMethod(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:261)
      <snip>

      dumping dot files helps significantly

        Issue Links

          Activity

          Hide
          Alexey Varlamov added a comment -

          Yep, I meant 1908, thanks. Closing as requested.

          Show
          Alexey Varlamov added a comment - Yep, I meant 1908, thanks. Closing as requested.
          Hide
          Alexei Fedotov added a comment -

          I believe Alexey meant HARMONY-1908.

          Show
          Alexei Fedotov added a comment - I believe Alexey meant HARMONY-1908 .
          Hide
          Egor Pasko added a comment -

          Alexey,
          nested synchronized blocks are fixed. So, this bug is to be closed. I planned to add a loop nesting check assertion to HARMONY-1908. Now since HARMONY-1908 is closed, I am going to create a special JIRA for the loop nesting check. This is going to be a low priority bug.

          Show
          Egor Pasko added a comment - Alexey, nested synchronized blocks are fixed. So, this bug is to be closed. I planned to add a loop nesting check assertion to HARMONY-1908 . Now since HARMONY-1908 is closed, I am going to create a special JIRA for the loop nesting check. This is going to be a low priority bug.
          Hide
          Alexey Varlamov added a comment -

          Egor,
          I applied the patch to fix HARMONY-1808. How should we resolve this JIRA, taking into account you statement about more general problem [1] - would you create new issue?
          [1] http://issues.apache.org/jira/browse/HARMONY-1969#action_12445527

          Show
          Alexey Varlamov added a comment - Egor, I applied the patch to fix HARMONY-1808 . How should we resolve this JIRA, taking into account you statement about more general problem [1] - would you create new issue? [1] http://issues.apache.org/jira/browse/HARMONY-1969#action_12445527
          Hide
          Alexei Fedotov added a comment -

          With patch applied the test passes smoothly.

          Show
          Alexei Fedotov added a comment - With patch applied the test passes smoothly.
          Hide
          Alexei Fedotov added a comment -

          In addition to the change of build.sh file I disabled fatal warnings on Linux. Then I was able to reproduce an asserion.

          Index: drlvm/trunk/build/make/components/vm/jitrino.xml
          ===================================================================
          — drlvm/trunk/build/make/components/vm/jitrino.xml (revision 470383)
          +++ drlvm/trunk/build/make/components/vm/jitrino.xml (working copy)
          @@ -234,7 +234,7 @@
          <select cxx="gcc">
          <compilerarg value="-fmessage-length=0" />
          <compilerarg value="-Wall" />

          • <compilerarg value="-Werror" />
            +<!-- <compilerarg value="Werror" />->
            </select>

          <compilerarg value="-x" />

          Show
          Alexei Fedotov added a comment - In addition to the change of build.sh file I disabled fatal warnings on Linux. Then I was able to reproduce an asserion. Index: drlvm/trunk/build/make/components/vm/jitrino.xml =================================================================== — drlvm/trunk/build/make/components/vm/jitrino.xml (revision 470383) +++ drlvm/trunk/build/make/components/vm/jitrino.xml (working copy) @@ -234,7 +234,7 @@ <select cxx="gcc"> <compilerarg value="-fmessage-length=0" /> <compilerarg value="-Wall" /> <compilerarg value="-Werror" /> +<!-- <compilerarg value=" Werror" /> -> </select> <compilerarg value="-x" />
          Hide
          Alexei Fedotov added a comment -

          Mikhail, thanks for a good comment. I was trying to unedrstnand how to get a debug version of jitrino. Shoulf I just change release to debug in the following line of build.bat or build.sh file?

          drlvm/trunk/build/build.bat:
          REM Note: vm.jitrino is always complied in release mode, otherwise it makes VM debug too slow
          CALL "%ANT_COMMAND%" -f make/build.xml -Dvm.jitrino.cfg=release %*

          Show
          Alexei Fedotov added a comment - Mikhail, thanks for a good comment. I was trying to unedrstnand how to get a debug version of jitrino. Shoulf I just change release to debug in the following line of build.bat or build.sh file? drlvm/trunk/build/build.bat: REM Note: vm.jitrino is always complied in release mode, otherwise it makes VM debug too slow CALL "%ANT_COMMAND%" -f make/build.xml -Dvm.jitrino.cfg=release %*
          Hide
          Mikhail Fursov added a comment -

          Geir, are you sure that libjitrino.so you tested was built in debug mode? When you build DRLVM in debug mode, jitrino builds in release.
          ?

          Show
          Mikhail Fursov added a comment - Geir, are you sure that libjitrino.so you tested was built in debug mode? When you build DRLVM in debug mode, jitrino builds in release. ?
          Hide
          Geir Magnusson Jr added a comment -

          I've tried to get this bug to exhibit itself to no avail.

          On Ubuntu 6 , gcc 3.4.6, I cannot get a debug build to break w/ -Xem:opt. I didn't comment out the asserts, but had a clean run w/ them uncommented

          Show
          Geir Magnusson Jr added a comment - I've tried to get this bug to exhibit itself to no avail. On Ubuntu 6 , gcc 3.4.6, I cannot get a debug build to break w/ -Xem:opt. I didn't comment out the asserts, but had a clean run w/ them uncommented
          Hide
          Alexei Fedotov added a comment -

          This bug prevents Jitrino.OPT from running class library tests.
          See http://wiki.apache.org/harmony/Unit_Tests_Pass_on_DRLVM

          Show
          Alexei Fedotov added a comment - This bug prevents Jitrino.OPT from running class library tests. See http://wiki.apache.org/harmony/Unit_Tests_Pass_on_DRLVM
          Hide
          Egor Pasko added a comment -

          0001-more-monexit-patterns-transformed-to-correct-loop-structure.txt fixes the problem. But the general problem with unnested loops still persists (broken loop structure makes IA-32 CG (and statprof?) crash in several places). Need to write a detector of such situations, first of all, to be able to move them to JET.

          c-unit, smoke, kernel, SUSE9

          Show
          Egor Pasko added a comment - 0001-more-monexit-patterns-transformed-to-correct-loop-structure.txt fixes the problem. But the general problem with unnested loops still persists (broken loop structure makes IA-32 CG (and statprof?) crash in several places). Need to write a detector of such situations, first of all, to be able to move them to JET. c-unit, smoke, kernel, SUSE9
          Hide
          Egor Pasko added a comment -

          The above stack trace was taken with profiling asserts uncommented (by mistake). Here is the correct one:
          1: ?? (??:-1)
          2: abort (??:-1)
          3: __assert_fail (??:-1)
          4: Jitrino::Ia32::IRManager::calculateLivenessInfo() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:1540)
          5: Jitrino::Ia32::IRManager::updateLivenessInfo() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.h:300)
          6: Jitrino::Ia32::SessionAction::debugOutput(char const*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:2195)
          7: Jitrino::Ia32::SessionAction::run() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:2149)
          8: Jitrino::runPipeline(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:226)
          9: Jitrino::compileMethod(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:261)
          <snip>

          Show
          Egor Pasko added a comment - The above stack trace was taken with profiling asserts uncommented (by mistake). Here is the correct one: 1: ?? (??:-1) 2: abort (??:-1) 3: __assert_fail (??:-1) 4: Jitrino::Ia32::IRManager::calculateLivenessInfo() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:1540) 5: Jitrino::Ia32::IRManager::updateLivenessInfo() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.h:300) 6: Jitrino::Ia32::SessionAction::debugOutput(char const*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:2195) 7: Jitrino::Ia32::SessionAction::run() (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/codegenerator/ia32/Ia32IRManager.cpp:2149) 8: Jitrino::runPipeline(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:226) 9: Jitrino::compileMethod(Jitrino::CompilationContext*) (/export/users/evpasko/svn/3/trunk/working_vm/vm/jitrino/src/main/Jitrino.cpp:261) <snip>
          Hide
          Egor Pasko added a comment -

          DoubleSync.java attached

          Show
          Egor Pasko added a comment - DoubleSync.java attached

            People

            • Assignee:
              Alexey Varlamov
              Reporter:
              Egor Pasko
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development