[PATCH] D116270: [AMDGPU] Enable divergence-driven XNOR selection

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In general, I don't like the idea of making the DAGCombiner::reassociateOpsCommutative take into account the divergence.

We have 2 different heuristics to apply in the hope they make a profit.

Considering the arithmetic sub-tree user as root:

1. keep commutative binary operation with one constant operand as close to the root in the tree as possible. This lets memory access patterns take advantage of the constant offset.
2. given that the constant operand is uniform and the divergent node may use a uniform node but not vice-versa, keep the commutative binary operation with one constant operand as outer in the tree as possible in hope that it will be selected to the scalar operation.

          t104: i32 = shl [ORD=15] [ID=17] # D:1 t27, Constant:i32<2> [ID=9]
        t105: i64 = zero_extend [ORD=15] [ID=19] # D:1 t104
      t31: i64 = add [ORD=15] [ID=29] # D:1 t97, t105
    t33: i64 = add [ORD=16] [ID=31] # D:1 t31, Constant:i64<4> [ID=6]
  t50: i32,ch = load<(load (s32) from %ir.b_ptr, addrspace 1)> [ORD=18] [ID=33] # D:1 t0, t33, undef:i64 [ID=2] # D:0

In the example above constant goes to the offset field in the load and the rest of the arithmetic sub-tree form the base. Since it is exposed to further combining, we end up with the simple move instead of 2 additions. Hence, the reassociation moving the constant to the operation that is uniform is not profitable here.

In our XOR example, we'd like to move constant operand up to another xor that already has one operand uniform.
To not introduce yet another regression fixing the current one, we would have to expose the heuristics details to the common code. For example: moving constant operand up in the commutative pattern is good if it is XOR but is unwanted if it is ADD that is the base of the load or store.
In general, it is a good idea to let the target decide if the concrete operation reassociation is profitable. So we may want to add TargetLoweringInfo::reassociateOpsCommutative hook to let the target attempt and then apply general reassociation if the target has not changed the pattern.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116270/new/

https://reviews.llvm.org/D116270