RFR: 8342393: Promote commutative vector IR node sharing [v2]

Emanuel Peter epeter at openjdk.org
Thu Jan 16 13:21:41 UTC 2025


On Thu, 16 Jan 2025 11:56:08 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Nice improvement, Jatin.
>
> Hi @iwanowww , @eme64 , @XiaohongGong , your comments have been addressed.

@jatin-bhateja 

I discussed it quickly with @TobiHartmann . We think it is nicer if we can do a general refactoring:
- Annotate all commutative nodes with `Flag_is_commutative_oper`, both for vector and scalar node.
- Apply your idea with checking commutative cases different in `find_hash`
![image](https://github.com/user-attachments/assets/524b442a-efeb-4978-8de7-cf741534b180)
![image](https://github.com/user-attachments/assets/b2373c57-8312-43f8-a3be-eeb5fd02876f)

For us it is ok if you just apply it basically as is - and file a follow-up RFE so we can refactor away all these:

static bool commute(PhaseGVN* phase, Node* add) {
...
  // Otherwise, sort inputs (commutativity) to help value numbering.
  if( in1->_idx > in2->_idx ) {
    add->swap_edges(1, 2);
    return true;
  }
  return false;
}

We have such `swap_edges` in a few places, we would have to carefully remove all and test things well.

Alternatively, we first do this refactoring just for the scalar nodes, and then your patch only has to apply it for vector cases.

Since this is touching core `Node::hash` functionality, I think we need to get @iwanowww to agree on this idea as well.

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

PR Comment: https://git.openjdk.org/jdk/pull/22863#issuecomment-2595645620


More information about the hotspot-compiler-dev mailing list