[PATCH] D123051: [RISCV][VP] Add basic RVV codegen for vp.fcmp

Tue Apr 5 08:32:38 PDT 2022

frasercrmck marked an inline comment as done.
frasercrmck added a comment.

In D123051#3427240 <https://reviews.llvm.org/D123051#3427240>, @craig.topper wrote:

> What canonicalizations are missing. For integer, I know we're missing canonicalizing constants to RHS, but we don't do that for FP ISD::SETCC.

I was thinking of some things in `TargetLowering::SimplifySetCC` which does a couple of FP-specific things on constants and `SelectionDAG::FoldSetCC` which does too. But you're right these are far fewer than for integers, and maybe they're technically not canonicalizations?

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3656
     int TrueValue;
     switch (TLI.getBooleanContents(Tmp1.getValueType())) {
     case TargetLowering::ZeroOrOneBooleanContent:
----------------
craig.topper wrote:
> Not related to this patch, but is this equivalent to SelectionDAG::getBoolConstant with true for the `V` parameter? If we agree it is, I'll write a patch to replace it.
Makes sense to me, yeah.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:9198
   NeedInvert = false;
+  bool IsVP = Mask != SDValue() || EVL != SDValue();
   switch (TLI.getCondCodeAction(CCCode, OpVT)) {
----------------
craig.topper wrote:
> It would be an error if only one was SDValue() right? Should we assert that?
Ah yeah so it would. Not sure what I was thinking.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123051