[PATCH] D121296: [GlobalIsel][X86] Legalization of G_IS_FPCLASS

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D121296#3403848 <https://reviews.llvm.org/D121296#3403848>, @arsenm wrote:

> Could also use FewerElementsVector handling

Some support for fewerElements was added, but the support of vector types in X86 Global ISel is weak, and I could not make tests for this case. Some patches for this support are in work, but it is a long trail.



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7901
+  Function &F = MI.getParent()->getParent()->getFunction();
+  if (!F.getAttributes().hasFnAttr(llvm::Attribute::StrictFP)) {
+    if (Test == fcZero) {
----------------
arsenm wrote:
> Isn't this implied by this not being a strict opcode?
This intrinsic does not interact with floating-point environment, it is a pure function. So it does not need strict variant.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:7940
+
+  Optional<MachineInstrBuilder> Res;
+  const auto appendResult = [&](MachineInstrBuilder PartialRes) {
----------------
arsenm wrote:
> This will function the same without Optional
Indeed, thank you.


================
Comment at: llvm/test/CodeGen/X86/GlobalISel/is_fpclass.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64 -global-isel | FileCheck %s -check-prefix=CHECK-64
----------------
foad wrote:
> Could you do this as a MIR test for the benefit of people who don't speak X86? See lots of legalize-*.mir examples in this directory.
New test is added (legalize-is_fpclass.mir), which tests the lowering similar to other legalize-*.mir tests.

The idea of using this test was to simplify comparison of the code generation with DAG-variant. This test is obtained from the test added in dependency review item, with tests for vector types stripped out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121296