Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8065618

C2 RA incorrectly removes kill projections

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P2
    • Resolution: Fixed
    • Affects Version/s: 7u80, 8u40, 9
    • Fix Version/s: 9
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
    • Resolved In Build:
      b42
    • CPU:
      generic
    • OS:
      generic

      Backports

        Description

        I found this problem while working on one of my projects.
        Some nodes have SCMemProj projection node to keep them alive even when their result is not used because they have memory side effect.
        For example, EncodeISOArrayNode and loadstore nodes like CompareAndSwapPNode.
        Such node's code may kill some registers and flags when it is executed:

        instruct encode_iso_array(rsi_RegP src, rdi_RegP dst, rdx_RegI len,
                                  regD tmp1, regD tmp2, regD tmp3, regD tmp4,
                                  rcx_RegI tmp5, rax_RegI result, rFlagsReg cr) %{
          match(Set result (EncodeISOArray src (Binary dst len)));
          effect(TEMP tmp1, TEMP tmp2, TEMP tmp3, TEMP tmp4, USE_KILL src, USE_KILL dst, USE_KILL len, KILL tmp5, KILL cr);

          format %{ "Encode array $src,$dst,$len -> $result // KILL RCX, RDX, $tmp1, $tmp2, $tmp3, $tmp4, RSI, RDI " %}
          ins_encode %{
            __ encode_iso_array($src$$Register, $dst$$Register, $len$$Register,
                                $tmp1$$XMMRegister, $tmp2$$XMMRegister, $tmp3$$XMMRegister,
                                $tmp4$$XMMRegister, $tmp5$$Register, $result$$Register);
          %}

        To keep such nodes alive when their result is not used there is RA code in PhaseChaitin::add_input_to_liveout() which does that by putting node's LiveRange on liveout list when it has SCMemProj projection.

        Unfortunately SCMemProj node could be placed in a block before kill MachProj nodes and will be process in add_input_to_liveout() too late. When kill MachProj are processed their def node looks like dead and these nodes are removed from graph. As result killed register is not marked as killed anymore and it could be used after instructions which kills/modifies it. For example in EncodeISOArray case its dst value could be used after it.

        These code existed forever. I think we did not hit it before because it was used only for loadstore nodes before which may have only kill flags MachProj projection. And EncodeISOArray was added very recently.

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  kvn Vladimir Kozlov
                  Reporter:
                  kvn Vladimir Kozlov
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  2 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: