[X86][GlobalISel] Support G_FCMP for scalar cases #123598

mahesh-attarde · 2025-01-20T12:04:07Z

With this patch, we are enabling comparisons for Long double (80) types on X87 stack. It lowers G_FCMP.

llvmbot · 2025-01-20T12:04:42Z

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-x86

Author: Mahesh-Attarde (mahesh-attarde)

Changes

With this patch, we are enabling comparisons for Long double (80) types on X87 stack. It lowers G_FCMP.

Patch is 37.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123598.diff

4 Files Affected:

(modified) llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp (+14-2)
(modified) llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp (+2-1)
(modified) llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp (+2-2)
(added) llvm/test/CodeGen/X86/isel-fcmp-x87.ll (+1054)

diff --git a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
index ee456a11d58441..9c35b1c06b187a 100644
--- a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
+++ b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
@@ -1048,6 +1048,13 @@ bool X86InstructionSelector::selectFCmp(MachineInstr &I,
     break;
   }
 
+  assert((LhsReg.isVirtual() && RhsReg.isVirtual()) &&
+         "Both arguments of FCMP need to be virtual!");
+  auto *LhsBank = RBI.getRegBank(LhsReg, MRI, TRI);
+  auto *RhsBank = RBI.getRegBank(RhsReg, MRI, TRI);
+  assert((LhsBank == RhsBank) &&
+         "Both banks assigned to FCMP arguments need to be same!");
+
   // Compute the opcode for the CMP instruction.
   unsigned OpCmp;
   LLT Ty = MRI.getType(LhsReg);
@@ -1055,10 +1062,15 @@ bool X86InstructionSelector::selectFCmp(MachineInstr &I,
   default:
     return false;
   case 32:
-    OpCmp = X86::UCOMISSrr;
+    OpCmp = LhsBank->getID() == X86::PSRRegBankID ? X86::UCOM_FpIr32
+                                                  : X86::UCOMISSrr;
     break;
   case 64:
-    OpCmp = X86::UCOMISDrr;
+    OpCmp = LhsBank->getID() == X86::PSRRegBankID ? X86::UCOM_FpIr64
+                                                  : X86::UCOMISDrr;
+    break;
+  case 80:
+    OpCmp = X86::UCOM_FpIr80;
     break;
   }
 
diff --git a/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp b/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
index bab7fe9d25e441..1136438ee742d1 100644
--- a/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
+++ b/llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
@@ -450,7 +450,8 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
   getActionDefinitionsBuilder(G_FCMP)
       .legalIf([=](const LegalityQuery &Query) {
         return (HasSSE1 && typePairInSet(0, 1, {{s8, s32}})(Query)) ||
-               (HasSSE2 && typePairInSet(0, 1, {{s8, s64}})(Query));
+               (HasSSE2 && typePairInSet(0, 1, {{s8, s64}})(Query)) ||
+               (UseX87 && typePairInSet(0, 1, {{s8, s80}})(Query));
       })
       .clampScalar(0, s8, s8)
       .clampScalar(1, s32, HasSSE2 ? s64 : s32)
diff --git a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
index 43c0145ec8e2ad..42faf4299c6d5d 100644
--- a/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
+++ b/llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
@@ -321,8 +321,8 @@ X86RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
 
     unsigned Size = Ty1.getSizeInBits();
     (void)Size;
-    assert((Size == 32 || Size == 64) && "Unsupported size for G_FCMP");
-
+    assert((Size == 32 || Size == 64 || Size == 80) &&
+           "Unsupported size for G_FCMP");
     auto FpRegBank = getPartialMappingIdx(MI, Ty1, /* isFP= */ true);
     OpRegBankIdx = {PMI_GPR8,
                     /* Predicate */ PMI_None, FpRegBank, FpRegBank};
diff --git a/llvm/test/CodeGen/X86/isel-fcmp-x87.ll b/llvm/test/CodeGen/X86/isel-fcmp-x87.ll
new file mode 100644
index 00000000000000..c46b354c9c410e
--- /dev/null
+++ b/llvm/test/CodeGen/X86/isel-fcmp-x87.ll
@@ -0,0 +1,1054 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s                               -mtriple=x86_64-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefix=SDAG-X64
+; RUN: llc < %s -fast-isel -fast-isel-abort=1 -mtriple=x86_64-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefix=FAST-X64
+; RUN: llc < %s -global-isel -global-isel-abort=1 -mtriple=x86_64-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefixes=GISEL-X64
+; RUN: llc < %s                               -mtriple=i686-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefixes=SDAG-X86
+; Allow fast-isel to fallback to selection dag on x86
+; RUN: llc < %s -fast-isel -mtriple=i686-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefixes=FAST-X86
+; RUN: llc < %s -global-isel -global-isel-abort=1 -mtriple=i686-apple-darwin10 -mattr=+x87,-sse,-sse2 -verify-machineinstrs | FileCheck %s --check-prefixes=GISEL-X86
+
+  define i1 @fcmp_x86_fp80_oeq(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_oeq:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    setnp %cl
+; SDAG-X64-NEXT:    sete %al
+; SDAG-X64-NEXT:    andb %cl, %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_oeq:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    setnp %cl
+; FAST-X64-NEXT:    sete %al
+; FAST-X64-NEXT:    andb %cl, %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_oeq:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fxch %st(1)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    sete %cl
+; GISEL-X64-NEXT:    setnp %al
+; GISEL-X64-NEXT:    andb %cl, %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_oeq:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    setnp %cl
+; SDAG-X86-NEXT:    sete %al
+; SDAG-X86-NEXT:    andb %cl, %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_oeq:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    setnp %cl
+; FAST-X86-NEXT:    sete %al
+; FAST-X86-NEXT:    andb %cl, %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_oeq:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fxch %st(1)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    sete %cl
+; GISEL-X86-NEXT:    setnp %al
+; GISEL-X86-NEXT:    andb %cl, %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp oeq x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_ogt(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_ogt:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    seta %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_ogt:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    seta %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_ogt:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fxch %st(1)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    seta %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_ogt:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    seta %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_ogt:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    seta %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_ogt:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fxch %st(1)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    seta %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp ogt x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_oge(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_oge:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    setae %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_oge:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    setae %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_oge:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fxch %st(1)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    setae %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_oge:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    setae %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_oge:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    setae %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_oge:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fxch %st(1)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    setae %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp oge x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_olt(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_olt:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    seta %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_olt:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fxch %st(1)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    seta %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_olt:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    seta %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_olt:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    seta %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_olt:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fxch %st(1)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    seta %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_olt:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    seta %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp olt x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_ole(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_ole:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    setae %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_ole:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fxch %st(1)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    setae %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_ole:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    setae %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_ole:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    setae %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_ole:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fxch %st(1)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    setae %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_ole:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    setae %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp ole x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_one(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_one:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    setne %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_one:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    setne %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_one:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fxch %st(1)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    setne %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_one:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:    sahf
+; SDAG-X86-NEXT:    setne %al
+; SDAG-X86-NEXT:    addl $12, %esp
+; SDAG-X86-NEXT:    retl
+;
+; FAST-X86-LABEL: fcmp_x86_fp80_one:
+; FAST-X86:       ## %bb.0:
+; FAST-X86-NEXT:    subl $12, %esp
+; FAST-X86-NEXT:    .cfi_def_cfa_offset 16
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; FAST-X86-NEXT:    fucompp
+; FAST-X86-NEXT:    fnstsw %ax
+; FAST-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; FAST-X86-NEXT:    sahf
+; FAST-X86-NEXT:    setne %al
+; FAST-X86-NEXT:    addl $12, %esp
+; FAST-X86-NEXT:    retl
+;
+; GISEL-X86-LABEL: fcmp_x86_fp80_one:
+; GISEL-X86:       ## %bb.0:
+; GISEL-X86-NEXT:    subl $12, %esp
+; GISEL-X86-NEXT:    .cfi_def_cfa_offset 16
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; GISEL-X86-NEXT:    fxch %st(1)
+; GISEL-X86-NEXT:    fucompi %st(1), %st
+; GISEL-X86-NEXT:    fstp %st(0)
+; GISEL-X86-NEXT:    setne %al
+; GISEL-X86-NEXT:    addl $12, %esp
+; GISEL-X86-NEXT:    retl
+    %1 = fcmp one x86_fp80 %x, %y
+    ret i1 %1
+  }
+
+  define i1 @fcmp_x86_fp80_ord(x86_fp80 %x, x86_fp80 %y) {
+; SDAG-X64-LABEL: fcmp_x86_fp80_ord:
+; SDAG-X64:       ## %bb.0:
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; SDAG-X64-NEXT:    fucompi %st(1), %st
+; SDAG-X64-NEXT:    fstp %st(0)
+; SDAG-X64-NEXT:    setnp %al
+; SDAG-X64-NEXT:    retq
+;
+; FAST-X64-LABEL: fcmp_x86_fp80_ord:
+; FAST-X64:       ## %bb.0:
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; FAST-X64-NEXT:    fucompi %st(1), %st
+; FAST-X64-NEXT:    fstp %st(0)
+; FAST-X64-NEXT:    setnp %al
+; FAST-X64-NEXT:    retq
+;
+; GISEL-X64-LABEL: fcmp_x86_fp80_ord:
+; GISEL-X64:       ## %bb.0:
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fldt {{[0-9]+}}(%rsp)
+; GISEL-X64-NEXT:    fxch %st(1)
+; GISEL-X64-NEXT:    fucompi %st(1), %st
+; GISEL-X64-NEXT:    fstp %st(0)
+; GISEL-X64-NEXT:    setnp %al
+; GISEL-X64-NEXT:    retq
+;
+; SDAG-X86-LABEL: fcmp_x86_fp80_ord:
+; SDAG-X86:       ## %bb.0:
+; SDAG-X86-NEXT:    subl $12, %esp
+; SDAG-X86-NEXT:    .cfi_def_cfa_offset 16
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fldt {{[0-9]+}}(%esp)
+; SDAG-X86-NEXT:    fucompp
+; SDAG-X86-NEXT:    fnstsw %ax
+; SDAG-X86-NEXT:    ## kill: def $ah killed $ah killed $ax
+; SDAG-X86-NEXT:...
[truncated]

llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp

llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp

arsenm · 2025-01-24T15:16:59Z

llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp

@@ -449,8 +449,11 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
  // fp comparison
  getActionDefinitionsBuilder(G_FCMP)
      .legalIf([=](const LegalityQuery &Query) {


Use the new form of legalIf with the subtarget predicate?

Are you suggesting LegalizeRuleSet &legalIf(LegalityPredicate Predicate) use instead of LegalityQuery?
HasSSE1 , HasSSE2 and UseX87 are coming from subtarget in constructor.

ping @arsenm

I mean like

llvm-project/llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h

Line 629 in 178f471

LegalizeRuleSet &legalFor(bool Pred,

llvm/test/CodeGen/X86/isel-fcmp.ll

llvm/test/CodeGen/X86/GlobalISel/isel-fcmp-i686.mir

RKSimon · 2025-01-30T11:30:12Z

llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp

-               (HasSSE2 && typePairInSet(0, 1, {{s8, s64}})(Query));
-      })
+      .legalFor((HasSSE1 || UseX87), {s8, s32})
+      .legalFor((HasSSE2 || UseX87), {s8, s64})


(style) unnecessary brackets .legalFor(HasSSE2 || UseX87,....

mahesh-attarde · 2025-01-31T07:03:15Z

ping. @e-kud @arsenm

e-kud

LGTM. However GISel doesn't respect that fucompi may not set EFLAGS and sequence of FSTSW,SAHF could be required. I think this is not crucial nowadays.

mahesh-attarde · 2025-02-03T05:42:56Z

@arsenm @RKSimon
If no issue, can we merge?

arsenm · 2025-02-03T06:06:27Z

llvm/test/CodeGen/X86/GlobalISel/isel-fcmp-i686.mir

+# NOTE: This MIR test is required because the support for 64 bit memory ops is missing in i686 mode, Due to distinction between float/int types, support is expected in near future and there is this RFC in place https://discourse.llvm.org/t/rfc-globalisel-adding-fp-type-information-to-llt/83349. Once this support is introduced this test must be dropped and integrated into the LLVM IR tests.
+# RUN: llc -O2 -mtriple=i686-linux-gnu -mattr=+x87,-sse,-sse2 -run-pass=regbankselect,instruction-select -disable-gisel-legality-check -global-isel -verify-machineinstrs %s -o - | FileCheck %s --check-prefixes GISEL-X86
+
+--- |


Don't need the IR section

I saw similar testcase having IR in same dir BTW. done.

arsenm · 2025-02-03T06:06:36Z

llvm/test/CodeGen/X86/GlobalISel/isel-fcmp-i686.mir

@@ -0,0 +1,473 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# NOTE: This MIR test is required because the support for 64 bit memory ops is missing in i686 mode, Due to distinction between float/int types, support is expected in near future and there is this RFC in place https://discourse.llvm.org/t/rfc-globalisel-adding-fp-type-information-to-llt/83349. Once this support is introduced this test must be dropped and integrated into the LLVM IR tests.
+# RUN: llc -O2 -mtriple=i686-linux-gnu -mattr=+x87,-sse,-sse2 -run-pass=regbankselect,instruction-select -disable-gisel-legality-check -global-isel -verify-machineinstrs %s -o - | FileCheck %s --check-prefixes GISEL-X86


Don't need -O2 or -verify-machineinstrs

RKSimon

LGTM - if you're after jobs, we should probably try to use legalFor in more places in X86LegalizerInfo.cpp

mahesh-attarde · 2025-02-03T15:46:37Z

@arsenm can you merge ?

mahesh-attarde · 2025-02-07T06:17:04Z

ping @arsenm @e-kud

With this patch, we are enabling comparisons for Long double (80) types on X87 stack. It lowers G_FCMP. --------- Co-authored-by: mattarde <[email protected]>

llvmbot added the backend:X86 label Jan 20, 2025

RKSimon requested review from RKSimon and e-kud January 20, 2025 12:45

e-kud reviewed Jan 20, 2025

View reviewed changes

llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp Show resolved Hide resolved

llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp Outdated Show resolved Hide resolved

mahesh-attarde force-pushed the gisel_fcmp branch from 94c3fca to 39b2776 Compare January 24, 2025 11:23

llvmbot added the llvm:globalisel label Jan 24, 2025

arsenm reviewed Jan 24, 2025

View reviewed changes

e-kud reviewed Jan 27, 2025

View reviewed changes

llvm/test/CodeGen/X86/isel-fcmp.ll Outdated Show resolved Hide resolved

llvm/test/CodeGen/X86/GlobalISel/isel-fcmp-i686.mir Outdated Show resolved Hide resolved

mattarde added 4 commits January 30, 2025 02:24

Support G_FCMP for scalar cases

c974d71

add 32 and 64 fcmp test

87ef7b6

update i686 with x87 having doubles

09557a0

update review comments

9b967b9

mahesh-attarde force-pushed the gisel_fcmp branch from 39b2776 to 9b967b9 Compare January 30, 2025 10:25

RKSimon reviewed Jan 30, 2025

View reviewed changes

update style

e3759e0

e-kud approved these changes Jan 31, 2025

View reviewed changes

arsenm reviewed Feb 3, 2025

View reviewed changes

remove ir from test

7777c64

RKSimon approved these changes Feb 3, 2025

View reviewed changes

abhishek-kaushik22 merged commit 35afd02 into llvm:main Feb 7, 2025
8 checks passed

mahesh-attarde deleted the gisel_fcmp branch February 8, 2025 10:07

[X86][GlobalISel] Support G_FCMP for scalar cases #123598

[X86][GlobalISel] Support G_FCMP for scalar cases #123598

Uh oh!

Conversation

mahesh-attarde commented Jan 20, 2025

Uh oh!

llvmbot commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahesh-attarde commented Jan 31, 2025

Uh oh!

e-kud left a comment

Choose a reason for hiding this comment

Uh oh!

mahesh-attarde commented Feb 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

mahesh-attarde commented Feb 3, 2025

Uh oh!

mahesh-attarde commented Feb 7, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jan 20, 2025 •

edited

Loading