[SelectionDAG] Peek through freeze to see XOR

[AZero13]

It does not matter if xor is undef or not, because x ^ x is 0, even undef.

Yes this is more of a change that aligns with x ^ x being 0 even if undef.

[llvmbot]

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: AZero13 (AZero13)

Changes

It does not matter if xor is undef or not, because x ^ x is 0, even undef.

---

Full diff: https://github.com/llvm/llvm-project/pull/147821.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+8-1)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 9ffdda28f7899..b10a6cc9a6678 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -10016,8 +10016,15 @@ SDValue DAGCombiner::visitXOR(SDNode *N) {
     }
   }
 
+  auto PeekThroughFreeze = [](SDValue N) {
+    if (N->getOpcode() == ISD::FREEZE && N.hasOneUse())
+      return N->getOperand(0);
+    return N;
+  };
+
   // fold (xor x, x) -> 0
-  if (N0 == N1)
+  // FIXME: Refactor this and sub and other similar operations together.
+  if (PeekThroughFreeze(N0) == PeekThroughFreeze(N1))
     return tryFoldToZero(DL, TLI, VT, DAG, LegalOperations);
 
   // fold (xor (shl 1, x), -1) -> (rotl ~1, x)

[RKSimon]

DAG now aggressively pushes freeze up through logic ops, why is this patch necessary?

[topperc]

DAG now aggressively pushes freeze up through logic ops, why is this patch necessary?

In this case the input the XOR is a freeze. We don't know what the input the freeze is. Could the input to the freeze be an operation we don't push up across?

[AZero13]

#148022

Well tests say otherwise.

[topperc]

#148022

Well tests say otherwise.

Ok then I'll write the test https://godbolt.org/z/v65a16W86 or https://godbolt.org/z/aWohvzx3T

[AZero13]

@topperc Okay, it works.

[RKSimon]

please can you put these in freeze-binary.ll

[RKSimon]

@nikic @topperc isn't the real problem that we've ended up with frozen and unfrozen versions of the same value? It looks like DAG is missing an equivalent of InstCombinerImpl::freezeOtherUses - WDYT?

[RKSimon]

#150017 should make this redundant