Page MenuHomePhabricator

[RISCV]Combine (select(seteq (and X, 1 << C), 0) to select_cc
Needs ReviewPublic

Authored by liaolucy on Mar 14 2023, 8:01 AM.



Sometimes (select(seteq (and X, 1 << C), 0) is converted to select + (srl X, C),
this patch will prevent generating srl instructions and
better use of the optimization in translateSetCCForBranch(D130203).

If we enable shouldAvoidTransformToShift, it will have the same effect, but many tests
will have regressions, so if we enable shouldAvoidTransformToShift in the future,
this patch can be removed.

Diff Detail

Event Timeline

liaolucy created this revision.Mar 14 2023, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2023, 8:01 AM
liaolucy requested review of this revision.Mar 14 2023, 8:01 AM

The test cases here only test the most significant bit, but your patch checks any power of 2. Can you add more test?

craig.topper added inline comments.Mar 14 2023, 8:39 AM

This looks like we're using ISD::SELECT_CC to hide from other transforms. We don't normally use ISD::SELECT_CC on RISC-V. Can we detect the srl form as an additional case in translateSetCCForBranch instead?

liaolucy added inline comments.Mar 14 2023, 7:24 PM
Optimized type-legalized selection DAG: %bb.0 'bit_31_nz_select_i32:'
SelectionDAG has 13 nodes:
  t0: ch,glue = EntryToken
        t2: i32,ch = CopyFromReg t0, Register:i32 %0
      t21: i32 = srl t2, Constant:i32<31>
      t4: i32,ch = CopyFromReg t0, Register:i32 %1
      t6: i32,ch = CopyFromReg t0, Register:i32 %2
    t12: i32 = select t21, t4, t6
  t14: ch,glue = CopyToReg t0, Register:i32 $x10, t12
  t15: ch = RISCVISD::RET_FLAG t14, Register:i32 $x10, t14:1

The test is not executed by translateSetCCForBranch. It is returned earlier in lowerSELECT.

// If the condition is not an integer SETCC which operates on XLenVT, we need
// to emit a RISCVISD::SELECT_CC comparing the condition to zero. i.e.:
// (select condv, truev, falsev)
// -> (riscvisd::select_cc condv, zero, setne, truev, falsev)
if (CondV.getOpcode() != ISD::SETCC ||
    CondV.getOperand(0).getSimpleValueType() != XLenVT) {
  SDValue Zero = DAG.getConstant(0, DL, XLenVT);
  SDValue SetNE = DAG.getCondCode(ISD::SETNE);

  SDValue Ops[] = {CondV, Zero, SetNE, TrueV, FalseV};

  return DAG.getNode(RISCVISD::SELECT_CC, DL, VT, Ops);

Maybe the code for LowerSelect needs to be refactored, I'll try it.