[PATCH] D131729: [RISCV] Move xori creation for scalar setccs to lowering.
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 15 12:35:27 PDT 2022
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
More information about the llvm-commits
mailing list