[PATCH] D131729: [RISCV] Move xori creation for scalar setccs to lowering.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3633
+          SDValue SetCC = DAG.getSetCC(
+              DL, VT, LHS, DAG.getConstant(Imm + 1, DL, OpVT), CCVal);
+          return DAG.getLogicalNOT(DL, SetCC, VT);
----------------
craig.topper wrote:
> craig.topper wrote:
> > reames wrote:
> > > I'm not following something here.  You're increment the rhs of a GT or UGT in the manner I'd expect to apply to a GE or UGE, but also not changing the condition code?
> > > 
> > > This looks like a dag combine btw, why does this need to be done during lowering at all?
> > The condition code was swapped on line 3623.
> Prior to LegalizeDAG it would probably be reversed by DAGCombiner. I think there's an setcc+xor combine that only checks CondCodeLegal after LegalOperations. I could write an opposite DAGCombine that only runs after lowering?
I'm still not following.

Original:
GT LHS, RHS

New:
LTE LHS, RHS+1

That doesn't look correct?  Unless maybe there's some implicit restriction on LHS and RHS I'm missing?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:3638
+
+      // Swap the operands.
+      return DAG.getSetCC(DL, VT, RHS, LHS, CCVal);
----------------
craig.topper wrote:
> reames wrote:
> > Why is swapping the operands correct here?  I don't follow?
> Line 3623 swapped the condition code so it is now setlt or setltu
Agreed.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131729