This is an archive of the discontinued LLVM Phabricator instance.

[GlobalIsel][X86] Legalization of G_IS_FPCLASS
Needs RevisionPublic

Authored by sepavloff on Mar 9 2022, 8:01 AM.

Details

Summary

Legalization of G_IS_FPCLASS is added. It is implemented similar to
the DAG version with necessary changes.

Diff Detail

Event Timeline

sepavloff created this revision.Mar 9 2022, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2022, 8:01 AM
sepavloff requested review of this revision.Mar 9 2022, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2022, 8:01 AM
Herald added a subscriber: wdng. · View Herald Transcript

Do you think you could split this into two patches?

  1. The part where you add G_IS_FPCLASS and add verifier checks
  2. The part where you add legalization support
sepavloff updated this revision to Diff 414627.Mar 11 2022, 3:05 AM

G_IS_FPCLASS is added in a separate file

sepavloff retitled this revision from [GlobalIsel][X86] Support llvm.is_fpclass to [GlobalIsel][X86] Legalization of G_IS_FPCLASS.Mar 11 2022, 3:11 AM
sepavloff edited the summary of this revision. (Show Details)

Could also use FewerElementsVector handling

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
7914

Isn't this implied by this not being a strict opcode?

7953

This will function the same without Optional

foad added inline comments.Mar 24 2022, 3:49 AM
llvm/test/CodeGen/X86/GlobalISel/is_fpclass.ll
2

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.

sepavloff updated this revision to Diff 421950.Apr 11 2022, 9:50 AM
sepavloff edited the summary of this revision. (Show Details)

Addressed reviewers' notes

  • Added test to check legalization in target-independent way,
  • Do not use Optional with MachineInstrBuilder,
  • Add some support for using fewerElements vector handling.

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.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
7914

This intrinsic does not interact with floating-point environment, it is a pure function. So it does not need strict variant.

7953

Indeed, thank you.

llvm/test/CodeGen/X86/GlobalISel/is_fpclass.ll
2

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.

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.

Are there any plans for X86 in GlobalIsel? It kind of stopped.

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.

Are there any plans for X86 in GlobalIsel? It kind of stopped.

I have plans to make some work for X86 in GlobalISel, related to FP support.

arsenm requested changes to this revision.Dec 7 2022, 5:47 PM
arsenm added a subscriber: JanekvO.

Oh, oops, @JanekvO just did most of this in D139128. Can you compare and merge what's different here (like the x86 parts?)

This revision now requires changes to proceed.Dec 7 2022, 5:47 PM
Matt added a subscriber: Matt.Jan 17 2023, 10:00 AM