[llvm] [GISel][RISCV][AArch64] Support legalizing G_SCMP/G_UCMP to sub(isgt,islt). (PR #119265)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 15 00:34:12 PST 2024


================
@@ -7929,16 +7930,30 @@ LegalizerHelper::lowerThreewayCompare(MachineInstr &MI) {
                                        ? CmpInst::Predicate::ICMP_SGT
                                        : CmpInst::Predicate::ICMP_UGT;
 
-  auto One = MIRBuilder.buildConstant(DstTy, 1);
   auto Zero = MIRBuilder.buildConstant(DstTy, 0);
   auto IsGT = MIRBuilder.buildICmp(GTPredicate, CmpTy, Cmp->getLHSReg(),
                                    Cmp->getRHSReg());
-  auto SelectZeroOrOne = MIRBuilder.buildSelect(DstTy, IsGT, One, Zero);
-
-  auto MinusOne = MIRBuilder.buildConstant(DstTy, -1);
   auto IsLT = MIRBuilder.buildICmp(LTPredicate, CmpTy, Cmp->getLHSReg(),
                                    Cmp->getRHSReg());
-  MIRBuilder.buildSelect(Dst, IsLT, MinusOne, SelectZeroOrOne);
+
+  auto &Ctx = MIRBuilder.getMF().getFunction().getContext();
+  const DataLayout &DL = MIRBuilder.getDataLayout();
+  if (TLI.shouldExpandCmpUsingSelects(
----------------
aemerson wrote:

> > I'm actually worried the lack of setcc equivalent here. For SVE for example we return i32 for scalars (fine) but `<vscale x N x i1>` for scalable predicates, which would be incorrect if used with these intrinsics.
> > Maybe we should a wrapper around `getSetCCResultType()` that supports LLTs and just calls the MVT version? @arsenm thoughts?
> 
> I'm not sure I follow what would be incorrect. The G_ICMP we create here will use i1 results and go through legalization. Nothing should be wrong just not optimal.

Looking at the source SDAG comment:
> // We can't perform arithmetic on i1 values.

I guess the issue is that if your setcc result type is -1, you end up needing to be able to generate `0b11` but your `SUB` inputs will still be 1-bit booleans?


https://github.com/llvm/llvm-project/pull/119265


More information about the llvm-commits mailing list