[PATCH] D94730: [RISCV] Add DAG combine to turn (setcc X, 1, setne) -> (setcc X, 0, seteq) if we can prove X is 0/1.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1853
   }
+  case ISD::SETCC: {
+    // (setcc X, 1, setne) -> (setcc X, 0, seteq) if we can prove X is 0/1.
----------------
lenary wrote:
> Is this not a general purpose combine if you have the same boolean contents? Or does it hinder optimisations on other platforms with a wider variety of condition codes?
I tried to make it generic and failed a bunch of AMDGPU test. The majority where an assertion in SITargetLowering::LowerBRCOND that it expects control flow intrinsics results to be used by a setne with 1. Which this patch broke. I restricted the code to ignore setcc's with MVT::i1 inputs which avoids this assert. This left 12 failing tests.

Some of those tests got worse because SIOptimizeExecMaskingPreRA::optimizeVcndVcmpPair was optimizing the setne sequence but there's nothing to optimize the code with seteq. The comments specifically mention DAGCombiner::visitBRCOND and rebuildSetCC. This accounts for at least 4 of the 12. I stopped looking after that.

The was one test changed on AArch64 but it didn't reduce instruction count. A few tests changed on X86. One decreased instruction count. One went from a couple scalar instruction to some vector sequence(the IR contained vectors that were being simplified). Not sure what happened.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94730