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

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Overall this seems reasonable. The only alternative I can think of would need more complicated isel patterns that do the reassociation gated by predicates that check the divergence, something like:

  let Predicate = doesNotHaveXNOR in
  def : GCNPat<
    (i32 (xor (xor_oneuse i32:$src0, i32:$src1), i32:$src2)),
    (i32 (V_XOR_B32 $src0, (V_XOR_B32 $src1, $src2))),
    [{ return src0->isDivergent() && !src1->isDivergent() && !src2->isDivergent(); }]
  >;

... and lots of commuted versions of the same thing, and the same for any other isel pattern that matches something that could be reassoicated. So that doesn't sound very scalable.

> Currently not (xor_one_use) pattern is always selected to S_XNOR irrelative od the node divergence.

"irrelative od" -> "irrespective of" or "regardless of".



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3293
 
+  virtual bool isReassocProfitable(SelectionDAG &DAG, SDValue N0,
+                                   SDValue N1) const {
----------------
Needs a proper descriptive comment.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12483
+                                                 SDValue N1) const {
+  if (N0.hasOneUse()) {
+    // Take care of the oportunity to keep N0 uniform
----------------
Return early if `!N0.hasOneUse()`.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12485
+    // Take care of the oportunity to keep N0 uniform
+    if (!(!N0->isDivergent() && N1->isDivergent()))
+      return true;
----------------
De-Morgan this to `N0->isDivergent() || !N1->isDivergent()`.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12491-12493
+    if (isCommutativeBinOp(N1->getOpcode()) &&
+        DAG.isConstantIntBuildVectorOrConstantInt(N1->getOperand(1)))
+      return true;
----------------
I don't understand this heuristic. Can you give an example of when it would help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116270