[PATCH] D129757: [RISCV] Optimize SELECT_CC when the true value of select is Constant

[Liao Chunyu via Phabricator via llvm-commits]

liaolucy added a comment.

> Thanks, that fixes that regression. I also see increased instruction counts for other tests including: pr20100-1, pr25125, pr37102, pr68506, pr83383, pr89634 (O3 <https://reviews.llvm.org/owners/package/3/>, rv64imafdc, lp64d). The overall impact across the suite seems positive, but it's likely worth taking a look.

I used a stupid way to count valid static instructions. Dump .s file and remove comments, label and other useless information. If there is a better way I am willing to try it.

pr20100-1: The number of instructions is the same, both are 70.

pr37102, pr68506, pr89634: This patch has 1 more j instructions.
pr83383: This patch has 2 more j instructions.

  Eg, from Pr83383.c: 
  original：
         li      a0, 0
         beqz    a4, .LBB0_2
  
  This patch:
         bnez    a3, .LBB0_2
  newbb: li      a0, 0 
         j       .LBB0_3      -------------------------------- +1

Pr25125.c: +3 instructions.

  original：
  f:                                      # @f
  # %bb.0:                                # %entry
          li      a1, 0
          bgtz    a0, .LBB0_2
  # %bb.1:                                # %entry
          lui     a1, 8
          xor     a1, a1, a0
  .LBB0_2:                                # %entry
          slli    a0, a1, 48
          srli    a0, a0, 48
          ret
  
  
  This patch：
  f:                                      # @f
  # %bb.0:                                # %entry
          blez    a0, .LBB0_2
  # %bb.1:                                # %entry
          li      a0, 0
          slli    a0, a0, 48  --------------------copy--------- +1
          srli    a0, a0, 48  --------------------copy--------- +2
          ret                 --------------------copy--------- +3
  .LBB0_2:
          lui     a1, 8
          xor     a0, a0, a1
          slli    a0, a0, 48
          srli    a0, a0, 48
          ret





================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3900
+          DAG.getCondCode(ISD::getSetCCInverse(CCVal, LHS.getValueType()));
+      SDValue Ops[] = {LHS, RHS, TargetCC, FalseV, TrueV};
+      return DAG.getNode(RISCVISD::SELECT_CC, DL, Op.getValueType(), Ops);
----------------
jrtc27 wrote:
> frasercrmck wrote:
> > Could we instead just `std::swap(TrueV, FalseV)` and let it fall through? I think that's clearer to read.
> This still isn't falling through. Ops and the return are identical to outside the if, you can just reuse them.
I'll update it back later.
SDValue Ops[] = {LHS, RHS, TargetCC, FalseV, TrueV,};


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129757